fix: correct meeting attendance calculations (#4536)
* fix: correct meeting attendance calculations * test: change meetingregistration factory defaults * test: Setup stats tests to verify counts honor meeting.Attended But the tests aren't actually looking to see what numbers get generated yet. * test: add test for attendance cross-talk between meetings * fix: limit attendance count query to single meeting * refactor: rename attendance.online to .remote * fix: only count a given person as onsite or remote, but never both * test: align tests with cleanup Co-authored-by: Jennifer Richards <jennifer@painless-security.com>
This commit is contained in:
Robert Sparks
GitHub
parent
50668c97cd
commit
6dd6165444
|
|
@ -244,18 +244,35 @@ class Meeting(models.Model):
|
|||
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'))
|
||||
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
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
"""Tests of models in the Meeting application"""
|
||||
import datetime
|
||||
|
||||
from ietf.meeting.factories import MeetingFactory, SessionFactory
|
||||
from ietf.meeting.factories import MeetingFactory, SessionFactory, AttendedFactory
|
||||
from ietf.stats.factories import MeetingRegistrationFactory
|
||||
from ietf.utils.test_utils import TestCase
|
||||
|
||||
|
|
@ -17,41 +17,75 @@ class MeetingTests(TestCase):
|
|||
MeetingRegistrationFactory.create_batch(5, meeting=meeting, reg_type='in_person')
|
||||
self.assertIsNone(meeting.get_attendance())
|
||||
|
||||
def test_get_attendance(self):
|
||||
"""Post-110 meetings do calculate attendance"""
|
||||
def test_get_attendance_110(self):
|
||||
"""Look at attendance as captured at 110"""
|
||||
meeting = MeetingFactory(type_id='ietf', number='110')
|
||||
|
||||
# start with attendees that should be ignored
|
||||
MeetingRegistrationFactory.create_batch(3, meeting=meeting, reg_type='')
|
||||
MeetingRegistrationFactory.create_batch(3, meeting=meeting, reg_type='', attended=True)
|
||||
MeetingRegistrationFactory(meeting=meeting, reg_type='', attended=False)
|
||||
attendance = meeting.get_attendance()
|
||||
self.assertIsNotNone(attendance)
|
||||
self.assertEqual(attendance.online, 0)
|
||||
self.assertEqual(attendance.remote, 0)
|
||||
self.assertEqual(attendance.onsite, 0)
|
||||
|
||||
# add online attendees with at least one who registered but did not attend
|
||||
MeetingRegistrationFactory.create_batch(4, meeting=meeting, reg_type='remote')
|
||||
MeetingRegistrationFactory.create_batch(4, meeting=meeting, reg_type='remote', attended=True)
|
||||
MeetingRegistrationFactory(meeting=meeting, reg_type='remote', attended=False)
|
||||
attendance = meeting.get_attendance()
|
||||
self.assertIsNotNone(attendance)
|
||||
self.assertEqual(attendance.online, 4)
|
||||
self.assertEqual(attendance.remote, 4)
|
||||
self.assertEqual(attendance.onsite, 0)
|
||||
|
||||
# and the same for onsite attendees
|
||||
MeetingRegistrationFactory.create_batch(5, meeting=meeting, reg_type='in_person')
|
||||
MeetingRegistrationFactory.create_batch(5, meeting=meeting, reg_type='onsite', attended=True)
|
||||
MeetingRegistrationFactory(meeting=meeting, reg_type='in_person', attended=False)
|
||||
attendance = meeting.get_attendance()
|
||||
self.assertIsNotNone(attendance)
|
||||
self.assertEqual(attendance.online, 4)
|
||||
self.assertEqual(attendance.remote, 4)
|
||||
self.assertEqual(attendance.onsite, 5)
|
||||
|
||||
# and once more after removing all the online attendees
|
||||
meeting.meetingregistration_set.filter(reg_type='remote').delete()
|
||||
attendance = meeting.get_attendance()
|
||||
self.assertIsNotNone(attendance)
|
||||
self.assertEqual(attendance.online, 0)
|
||||
self.assertEqual(attendance.remote, 0)
|
||||
self.assertEqual(attendance.onsite, 5)
|
||||
|
||||
def test_get_attendance_113(self):
|
||||
"""Simulate IETF 113 attendance gathering data"""
|
||||
meeting = MeetingFactory(type_id='ietf', number='113')
|
||||
MeetingRegistrationFactory(meeting=meeting, reg_type='onsite', attended=True, checkedin=False)
|
||||
MeetingRegistrationFactory(meeting=meeting, reg_type='onsite', attended=False, checkedin=True)
|
||||
p1 = MeetingRegistrationFactory(meeting=meeting, reg_type='onsite', attended=False, checkedin=False).person
|
||||
AttendedFactory(session__meeting=meeting, person=p1)
|
||||
p2 = MeetingRegistrationFactory(meeting=meeting, reg_type='remote', attended=False, checkedin=False).person
|
||||
AttendedFactory(session__meeting=meeting, person=p2)
|
||||
attendance = meeting.get_attendance()
|
||||
self.assertEqual(attendance.onsite, 3)
|
||||
self.assertEqual(attendance.remote, 1)
|
||||
|
||||
def test_get_attendance_keeps_meetings_distinct(self):
|
||||
"""No cross-talk between attendance for different meetings"""
|
||||
# numbers are arbitrary here
|
||||
first_mtg = MeetingFactory(type_id='ietf', number='114')
|
||||
second_mtg = MeetingFactory(type_id='ietf', number='115')
|
||||
|
||||
# Create a person who attended a remote session for first_mtg and onsite for second_mtg without
|
||||
# checking in for either.
|
||||
p = MeetingRegistrationFactory(meeting=second_mtg, reg_type='onsite', attended=False, checkedin=False).person
|
||||
AttendedFactory(session__meeting=first_mtg, person=p)
|
||||
MeetingRegistrationFactory(meeting=first_mtg, person=p, reg_type='remote', attended=False, checkedin=False)
|
||||
AttendedFactory(session__meeting=second_mtg, person=p)
|
||||
|
||||
att = first_mtg.get_attendance()
|
||||
self.assertEqual(att.onsite, 0)
|
||||
self.assertEqual(att.remote, 1)
|
||||
|
||||
att = second_mtg.get_attendance()
|
||||
self.assertEqual(att.onsite, 1)
|
||||
self.assertEqual(att.remote, 0)
|
||||
|
||||
|
||||
class SessionTests(TestCase):
|
||||
def test_chat_archive_url_with_jabber(self):
|
||||
|
|
|
|||
|
|
@ -2289,7 +2289,7 @@ class rfc8713EligibilityTests(TestCase):
|
|||
for combo in combinations(meetings,combo_len):
|
||||
p = PersonFactory()
|
||||
for m in combo:
|
||||
MeetingRegistrationFactory(person=p, meeting=m)
|
||||
MeetingRegistrationFactory(person=p, meeting=m, attended=True)
|
||||
if combo_len<3:
|
||||
self.ineligible_people.append(p)
|
||||
else:
|
||||
|
|
@ -2302,7 +2302,7 @@ class rfc8713EligibilityTests(TestCase):
|
|||
self.other_date = datetime.date(2009,5,1)
|
||||
self.other_people = PersonFactory.create_batch(1)
|
||||
for date in (datetime.date(2009,3,1), datetime.date(2008,11,1), datetime.date(2008,7,1)):
|
||||
MeetingRegistrationFactory(person=self.other_people[0],meeting__date=date, meeting__type_id='ietf')
|
||||
MeetingRegistrationFactory(person=self.other_people[0],meeting__date=date, meeting__type_id='ietf', attended=True)
|
||||
|
||||
|
||||
def test_is_person_eligible(self):
|
||||
|
|
@ -2347,7 +2347,7 @@ class rfc8788EligibilityTests(TestCase):
|
|||
for combo in combinations(meetings,combo_len):
|
||||
p = PersonFactory()
|
||||
for m in combo:
|
||||
MeetingRegistrationFactory(person=p, meeting=m)
|
||||
MeetingRegistrationFactory(person=p, meeting=m, attended=True)
|
||||
if combo_len<3:
|
||||
self.ineligible_people.append(p)
|
||||
else:
|
||||
|
|
@ -2395,7 +2395,7 @@ class rfc8989EligibilityTests(TestCase):
|
|||
for combo in combinations(prev_five,combo_len):
|
||||
p = PersonFactory()
|
||||
for m in combo:
|
||||
MeetingRegistrationFactory(person=p, meeting=m)
|
||||
MeetingRegistrationFactory(person=p, meeting=m, attended=True) # not checkedin because this forces looking at older meetings
|
||||
AttendedFactory(session__meeting=m, session__type_id='plenary',person=p)
|
||||
if combo_len<3:
|
||||
ineligible_people.append(p)
|
||||
|
|
@ -2638,7 +2638,7 @@ class VolunteerTests(TestCase):
|
|||
self.assertContains(r, 'NomCom is not accepting volunteers at this time', status_code=200)
|
||||
nomcom.is_accepting_volunteers = True
|
||||
nomcom.save()
|
||||
MeetingRegistrationFactory(person=person, affiliation='mtg_affiliation')
|
||||
MeetingRegistrationFactory(person=person, affiliation='mtg_affiliation', checkedin=True)
|
||||
r = self.client.get(url)
|
||||
self.assertContains(r, 'Volunteer for NomCom', status_code=200)
|
||||
self.assertContains(r, 'mtg_affiliation')
|
||||
|
|
@ -2710,7 +2710,7 @@ class VolunteerDecoratorUnitTests(TestCase):
|
|||
('106', datetime.date(2019, 11, 16)),
|
||||
]]
|
||||
for m in meetings:
|
||||
MeetingRegistrationFactory(meeting=m,person=meeting_person)
|
||||
MeetingRegistrationFactory(meeting=m, person=meeting_person, attended=True)
|
||||
AttendedFactory(session__meeting=m, session__type_id='plenary', person=meeting_person)
|
||||
nomcom.volunteer_set.create(person=meeting_person)
|
||||
|
||||
|
|
|
|||
|
|
@ -15,4 +15,5 @@ class MeetingRegistrationFactory(factory.django.DjangoModelFactory):
|
|||
reg_type = 'onsite'
|
||||
first_name = factory.LazyAttribute(lambda obj: obj.person.first_name())
|
||||
last_name = factory.LazyAttribute(lambda obj: obj.person.last_name())
|
||||
attended = True
|
||||
attended = False
|
||||
checkedin = False
|
||||
|
|
|
|||
|
|
@ -21,12 +21,13 @@ from ietf.submit.models import Submission
|
|||
from ietf.doc.factories import WgDraftFactory, WgRfcFactory
|
||||
from ietf.doc.models import Document, DocAlias, State, RelatedDocument, NewRevisionDocEvent, DocumentAuthor
|
||||
from ietf.group.factories import RoleFactory
|
||||
from ietf.meeting.factories import MeetingFactory
|
||||
from ietf.meeting.factories import MeetingFactory, AttendedFactory
|
||||
from ietf.person.factories import PersonFactory
|
||||
from ietf.person.models import Person, Email
|
||||
from ietf.name.models import FormalLanguageName, DocRelationshipName, CountryName
|
||||
from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory, ReviewAssignmentFactory
|
||||
from ietf.stats.models import MeetingRegistration, CountryAlias
|
||||
from ietf.stats.factories import MeetingRegistrationFactory
|
||||
from ietf.stats.utils import get_meeting_registration_data
|
||||
|
||||
|
||||
|
|
@ -122,11 +123,11 @@ class StatisticsTests(TestCase):
|
|||
def test_meeting_stats(self):
|
||||
# create some data for the statistics
|
||||
meeting = MeetingFactory(type_id='ietf', date=datetime.date.today(), number="96")
|
||||
MeetingRegistration.objects.create(first_name='John', last_name='Smith', country_code='US', email="john.smith@example.us", meeting=meeting, attended=True)
|
||||
MeetingRegistrationFactory(first_name='John', last_name='Smith', country_code='US', email="john.smith@example.us", meeting=meeting, attended=True)
|
||||
CountryAlias.objects.get_or_create(alias="US", country=CountryName.objects.get(slug="US"))
|
||||
MeetingRegistration.objects.create(first_name='Jaume', last_name='Guillaume', country_code='FR', email="jaume.guillaume@example.fr", meeting=meeting, attended=True)
|
||||
p = MeetingRegistrationFactory(first_name='Jaume', last_name='Guillaume', country_code='FR', email="jaume.guillaume@example.fr", meeting=meeting, attended=False).person
|
||||
CountryAlias.objects.get_or_create(alias="FR", country=CountryName.objects.get(slug="FR"))
|
||||
|
||||
AttendedFactory(session__meeting=meeting,person=p)
|
||||
# check redirect
|
||||
url = urlreverse(ietf.stats.views.meeting_stats)
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import requests
|
|||
from collections import defaultdict
|
||||
|
||||
from django.conf import settings
|
||||
from django.db.models import Q
|
||||
|
||||
import debug # pyflakes:ignore
|
||||
|
||||
|
|
@ -320,8 +321,10 @@ def get_meeting_registration_data(meeting):
|
|||
raise RuntimeError("Bad response from registrations API: %s, '%s'" % (response.status_code, response.content))
|
||||
num_total = MeetingRegistration.objects.filter(
|
||||
meeting_id=meeting.pk,
|
||||
attended=True,
|
||||
reg_type__in=['onsite', 'remote']).count()
|
||||
reg_type__in=['onsite', 'remote']
|
||||
).filter(
|
||||
Q(attended=True) | Q(checkedin=True)
|
||||
).count()
|
||||
if meeting.attendees is None or num_total > meeting.attendees:
|
||||
meeting.attendees = num_total
|
||||
meeting.save()
|
||||
|
|
|
|||
|
|
@ -817,8 +817,10 @@ def meeting_stats(request, num=None, stats_type=None):
|
|||
if meeting and any(stats_type == t[0] for t in possible_stats_types):
|
||||
attendees = MeetingRegistration.objects.filter(
|
||||
meeting=meeting,
|
||||
attended=True,
|
||||
reg_type__in=['onsite', 'remote'])
|
||||
reg_type__in=['onsite', 'remote']
|
||||
).filter(
|
||||
Q( attended=True) | Q( checkedin=True )
|
||||
)
|
||||
|
||||
if stats_type == "country":
|
||||
stats_title = "Number of attendees for {} {} per country".format(meeting.type.name, meeting.number)
|
||||
|
|
@ -893,7 +895,10 @@ def meeting_stats(request, num=None, stats_type=None):
|
|||
attendees = MeetingRegistration.objects.filter(
|
||||
meeting__type="ietf",
|
||||
attended=True,
|
||||
reg_type__in=['onsite', 'remote']).select_related('meeting')
|
||||
reg_type__in=['onsite', 'remote']
|
||||
).filter(
|
||||
Q( attended=True) | Q( checkedin=True )
|
||||
).select_related('meeting')
|
||||
|
||||
if stats_type == "overview":
|
||||
stats_title = "Number of attendees per meeting"
|
||||
|
|
|
|||
|
|
@ -13,9 +13,9 @@ This renders the title block for the meeting proceedings page.
|
|||
{% if attendance is not None %}
|
||||
<div class="proceedings-info lead">
|
||||
{% if attendance.onsite > 0 %}
|
||||
{{ attendance.onsite }} onsite participant{{ attendance.onsite|pluralize }}{% if attendance.online > 0 %},{% endif %}
|
||||
{{ attendance.onsite }} onsite participant{{ attendance.onsite|pluralize }}{% if attendance.remote > 0 %},{% endif %}
|
||||
{% endif %}
|
||||
{% if attendance.online > 0 %}{{ attendance.online }} online participant{{ attendance.online|pluralize }}{% endif %}
|
||||
{% if attendance.remote > 0 %}{{ attendance.remote }} online participant{{ attendance.remote|pluralize }}{% endif %}
|
||||
</div>
|
||||
{% endif %}
|
||||
</div>
|
||||
|
|
|
|||
Loading…
Reference in a new issue