fix: correct meeting attendance calculations (ietf-tools#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>

Author: rjsparks

ietf/meeting/models.py

@@ -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'))
+        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

ietf/meeting/tests_models.py

@@ -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 @@ def test_get_attendance_pre110(self):
         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):

ietf/nomcom/tests.py

@@ -2289,7 +2289,7 @@ def setUp(self):
             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 @@ def setUp(self):
         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 @@ def setUp(self):
             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 @@ def test_elig_by_meetings(self):
                 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 @@ def test_volunteer(self):
         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 @@ def test_decorate_volunteers_with_qualifications(self):
             ('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)
 

ietf/stats/factories.py

@@ -15,4 +15,5 @@ class Meta:
     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

ietf/stats/tests.py

@@ -21,12 +21,13 @@
 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 @@ def test_document_stats(self):
     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)
 

ietf/stats/utils.py

@@ -7,6 +7,7 @@
 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()

ietf/stats/views.py

@@ -817,8 +817,10 @@ def reg_name(r):
         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 reg_name(r):
             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"

ietf/templates/meeting/proceedings/title.html

@@ -13,9 +13,9 @@ <h1>
     {% 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>