-
Notifications
You must be signed in to change notification settings - Fork 758
fix: correct meeting attendance calculations #4536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9119992
de526f6
d1373c3
8549cd0
a73fd55
df82bfa
39b3663
190796a
368bdc0
d5bae3d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -244,18 +244,35 @@ def get_attendance(self): | |
| number = self.get_number() | ||
| if number is None or number < 110: | ||
| return None | ||
| Attendance = namedtuple('Attendance', 'onsite online') | ||
| Attendance = namedtuple('Attendance', 'onsite remote') | ||
|
|
||
| # MeetingRegistration.attended started conflating badge-pickup and session attendance before IETF 114. | ||
| # We've separated session attendence off to ietf.meeting.Attended, but need to report attendance at older | ||
| # meetings correctly. | ||
|
|
||
| attended_per_meetingregistration = ( | ||
| Q(meetingregistration__meeting=self) & ( | ||
| Q(meetingregistration__attended=True) | | ||
| Q(meetingregistration__checkedin=True) | ||
| ) | ||
| ) | ||
| attended_per_meeting_attended = ( | ||
| Q(attended__session__meeting=self) | ||
| # Note that we are not filtering to plenary, wg, or rg sessions | ||
| # as we do for nomcom eligibility - if picking up a badge (see above) | ||
| # is good enough, just attending e.g. a training session is also good enough | ||
| ) | ||
| attended = Person.objects.filter( | ||
| attended_per_meetingregistration | attended_per_meeting_attended | ||
| ).distinct() | ||
|
|
||
| onsite=set(attended.filter(meetingregistration__meeting=self, meetingregistration__reg_type='onsite')) | ||
| remote=set(attended.filter(meetingregistration__meeting=self, meetingregistration__reg_type='remote')) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be better to do the exclusion in the query and just pull the counts over to Python? I'm thinking something like: (I'm not sure if that's an improvement.)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I started there and it produced surprisingly wrong results. i have a dim memory that there is a bug in the stack that causes the sql query to fail - don't remember if the issue was in Django2 or in mysql. So I just avoided it for now, and I think the result is actually a little more readable.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does trade off doing things in django's memory rather than the databases, but this isn't a performance crunchpoint. |
||
| remote.difference_update(onsite) | ||
|
|
||
| return Attendance( | ||
| onsite=Person.objects.filter( | ||
| meetingregistration__meeting=self, | ||
| meetingregistration__attended=True, | ||
| meetingregistration__reg_type__contains='in_person', | ||
| ).distinct().count(), | ||
| online=Person.objects.filter( | ||
| meetingregistration__meeting=self, | ||
| meetingregistration__attended=True, | ||
| meetingregistration__reg_type__contains='remote', | ||
| ).distinct().count(), | ||
| onsite=len(onsite), | ||
| remote=len(remote) | ||
| ) | ||
|
|
||
| @property | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.