feat: separate MeetingRegistration rows for each registration type. updates the registration API (ietf-tools#3641)

* Registration API Update

- change MeetingRegistration.reg_type field to hold only one type
- allow multiple MeetingRegistration records per person/meeting
  (one for each reg_type)

* Fix scope claims

* Add meeting 114 to MeetingRegistration migration

* fix: update stats views for MeetingRegistration model use changes

* refactor: remove unused imports

Author: rpcross

ietf/api/tests.py

@@ -279,23 +279,23 @@ def test_api_v2_person_export_view(self):
     def test_api_new_meeting_registration(self):
         meeting = MeetingFactory(type_id='ietf')
         reg = {
-                'apikey': 'invalid',
-                'affiliation': "Alguma Corporação",
-                'country_code': 'PT',
-                'email': 'foo@example.pt',
-                'first_name': 'Foo',
-                'last_name': 'Bar',
-                'meeting': meeting.number,
-                'reg_type': 'hackathon',
-                'ticket_type': '',
-            }
+            'apikey': 'invalid',
+            'affiliation': "Alguma Corporação",
+            'country_code': 'PT',
+            'email': 'foo@example.pt',
+            'first_name': 'Foo',
+            'last_name': 'Bar',
+            'meeting': meeting.number,
+            'reg_type': 'hackathon',
+            'ticket_type': '',
+        }
         url = urlreverse('ietf.api.views.api_new_meeting_registration')
         r = self.client.post(url, reg)
         self.assertContains(r, 'Invalid apikey', status_code=403)
         oidcp = PersonFactory(user__is_staff=True)
         # Make sure 'oidcp' has an acceptable role
         RoleFactory(name_id='robot', person=oidcp, email=oidcp.email(), group__acronym='secretariat')
-        key  = PersonalApiKey.objects.create(person=oidcp, endpoint=url)
+        key = PersonalApiKey.objects.create(person=oidcp, endpoint=url)
         reg['apikey'] = key.hash()
         #
         # Test valid POST
@@ -313,7 +313,7 @@ def test_api_new_meeting_registration(self):
         #
         # Check record
         obj = MeetingRegistration.objects.get(email=reg['email'], meeting__number=reg['meeting'])
-        for key in [ 'affiliation', 'country_code', 'first_name', 'last_name', 'person', 'reg_type', 'ticket_type', ]:
+        for key in ['affiliation', 'country_code', 'first_name', 'last_name', 'person', 'reg_type', 'ticket_type']:
             self.assertEqual(getattr(obj, key), reg.get(key), "Bad data for field '%s'" % key)
         #
         # Test with existing user
@@ -328,15 +328,15 @@ def test_api_new_meeting_registration(self):
         # There should be no new outgoing mail
         self.assertEqual(len(outbox), old_len + 1)
         #
-        # Test combination of reg types
+        # Test multiple reg types
         reg['reg_type'] = 'remote'
         reg['ticket_type'] = 'full_week_pass'
         r = self.client.post(url, reg)
-        self.assertContains(r, "Accepted, Updated registration", status_code=202)
-        obj = MeetingRegistration.objects.get(email=reg['email'], meeting__number=reg['meeting'])
-        self.assertIn('hackathon', set(obj.reg_type.split()))
-        self.assertIn('remote', set(obj.reg_type.split()))
-        self.assertIn('full_week_pass', set(obj.ticket_type.split()))
+        self.assertContains(r, "Accepted, New registration", status_code=202)
+        objs = MeetingRegistration.objects.filter(email=reg['email'], meeting__number=reg['meeting'])
+        self.assertEqual(len(objs), 2)
+        self.assertEqual(objs.filter(reg_type='hackathon').count(), 1)
+        self.assertEqual(objs.filter(reg_type='remote', ticket_type='full_week_pass').count(), 1)
         self.assertEqual(len(outbox), old_len + 1)
         #
         # Test incomplete POST
@@ -346,7 +346,7 @@ def test_api_new_meeting_registration(self):
         r = self.client.post(url, reg)        
         self.assertContains(r, 'Missing parameters:', status_code=400)
         err, fields = r.content.decode().split(':', 1)
-        missing_fields = [ f.strip() for f in fields.split(',') ]
+        missing_fields = [f.strip() for f in fields.split(',')]
         self.assertEqual(set(missing_fields), set(drop_fields))
 
     def test_api_version(self):
@@ -422,4 +422,4 @@ def test_all_model_resources_exist(self):
                 if not model._meta.model_name in list(app_resources.keys()):
                     #print("There doesn't seem to be any resource for model %s.models.%s"%(app.__name__,model.__name__,))
                     self.assertIn(model._meta.model_name, list(app_resources.keys()),
-                        "There doesn't seem to be any API resource for model %s.models.%s"%(app.__name__,model.__name__,))
+                        "There doesn't seem to be any API resource for model %s.models.%s"%(app.__name__,model.__name__,))

ietf/api/views.py

@@ -162,30 +162,28 @@ def err(code, text):
             meeting = Meeting.objects.get(number=number)
         except Meeting.DoesNotExist:
             return err(400, "Invalid meeting value: '%s'" % (number, ))
+        reg_type = data['reg_type']
         email = data['email']
         try:
             validate_email(email)
         except ValidationError:
             return err(400, "Invalid email value: '%s'" % (email, ))
         if request.POST.get('cancelled', 'false') == 'true':
-            MeetingRegistration.objects.filter(meeting_id=meeting.pk, email=email).delete()
+            MeetingRegistration.objects.filter(
+                meeting_id=meeting.pk,
+                email=email,
+                reg_type=reg_type).delete()
             return HttpResponse('OK', status=200, content_type='text/plain')
         else:
-            object, created = MeetingRegistration.objects.get_or_create(meeting_id=meeting.pk, email=email)
+            object, created = MeetingRegistration.objects.get_or_create(
+                meeting_id=meeting.pk,
+                email=email,
+                reg_type=reg_type)
             try:
-                # Set attributes not already in the object
-                for key in set(data.keys())-set(['attended', 'apikey', 'meeting', 'email',]):
+                # Update attributes
+                for key in set(data.keys())-set(['attended', 'apikey', 'meeting', 'email']):
                     new = data.get(key)
-                    cur = getattr(object, key, None)
-                    if key in ['reg_type', 'ticket_type', ] and new:
-                        # Special handling for multiple reg types
-                        if cur:
-                            if not new in cur:
-                                setattr(object, key, cur+' '+new)
-                        else:
-                            setattr(object, key, new)
-                    else:
-                        setattr(object, key, new)
+                    setattr(object, key, new)
                 person = Person.objects.filter(email__address=email)
                 if person.exists():
                     object.person = person.first()

ietf/ietfauth/utils.py

@@ -291,15 +291,13 @@ def scope_registration(self):
             ticket_types = set([])
             reg_types = set([])
             for reg in regs:
-                for t in reg.ticket_type.split():
-                    ticket_types.add(t)
-                for r in reg.reg_type.split():
-                    reg_types.add(r)
+                ticket_types.add(reg.ticket_type)
+                reg_types.add(reg.reg_type)
             info = {
                 'meeting':      meeting.number,
                 # full_week, one_day, student:
                 'ticket_type':  ' '.join(ticket_types),
-                # in_person, onliine, hackathon:
+                # onsite, remote, hackathon_onsite, hackathon_remote:
                 'reg_type':     ' '.join(reg_types),
                 'affiliation':  ([ reg.affiliation for reg in regs if reg.affiliation ] or [''])[0],
             }

ietf/stats/migrations/0004_split_records.py

@@ -0,0 +1,33 @@
+# Generated by Django 2.2.26 on 2022-01-19 16:36
+
+from django.db import migrations
+
+
+def forward(apps, schema_editor):
+    '''Split records that have 2 reg_types into two separate records'''
+    MeetingRegistration = apps.get_model('stats', 'MeetingRegistration')
+    meetings = [108, 109, 110, 111, 112, 113, 114]
+    for reg in MeetingRegistration.objects.filter(meeting__number__in=meetings):
+        reg_types = reg.reg_type.split()
+        if len(reg_types) == 2:
+            reg.reg_type = reg_types[0]
+            reg.save()
+            # create copy
+            reg.pk = None
+            reg.reg_type = reg_types[1]
+            reg.save()
+
+
+def reverse(apps, schema_editor):
+    pass
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('stats', '0003_meetingregistration_attended'),
+    ]
+
+    operations = [
+        migrations.RunPython(forward, reverse)
+    ]

ietf/stats/tests.py

@@ -13,7 +13,6 @@
 import debug    # pyflakes:ignore
 
 from django.urls import reverse as urlreverse
-from django.contrib.auth.models import User
 
 from ietf.utils.test_utils import login_testing_unauthorized, TestCase
 import ietf.stats.views
@@ -231,33 +230,38 @@ def test_review_stats(self):
     @patch('requests.get')
     def test_get_meeting_registration_data(self, mock_get):
         '''Test function to get reg data.  Confirm leading/trailing spaces stripped'''
-        response = Response()
-        response.status_code = 200
-        response._content = b'[{"LastName":"Smith ","FirstName":" John","Company":"ABC","Country":"US","Email":"john.doe@example.us"}]'
-        mock_get.return_value = response
-        meeting = MeetingFactory(type_id='ietf', date=datetime.date(2016,7,14), number="96")
+        person = PersonFactory()
+        data = {
+            'LastName': person.last_name() + ' ',
+            'FirstName': person.first_name(),
+            'Company': 'ABC',
+            'Country': 'US',
+            'Email': person.email().address,
+            'RegType': 'onsite'
+        }
+        data2 = data.copy()
+        data2['RegType'] = 'hackathon'
+        response_a = Response()
+        response_a.status_code = 200
+        response_a._content = json.dumps([data, data2]).encode('utf8')
+        # second response one less record, it's been deleted
+        response_b = Response()
+        response_b.status_code = 200
+        response_b._content = json.dumps([data]).encode('utf8')
+        # mock_get.return_value = response
+        mock_get.side_effect = [response_a, response_b]
+        meeting = MeetingFactory(type_id='ietf', date=datetime.date(2016, 7, 14), number="96")
         get_meeting_registration_data(meeting)
-        query = MeetingRegistration.objects.filter(first_name='John',last_name='Smith',country_code='US')
-        self.assertTrue(query.count(), 1)
-        self.assertTrue(isinstance(query[0].person,Person))
-        
-    @patch('requests.get')
-    def test_get_meeting_registration_data_user_exists(self, mock_get):
-        response = Response()
-        response.status_code = 200
-        response._content = b'[{"LastName":"Smith","FirstName":"John","Company":"ABC","Country":"US","Email":"john.doe@example.us"}]'
-        email = "john.doe@example.us"
-        user = User.objects.create(username=email)
-        user.save()
-        
-        mock_get.return_value = response
-        meeting = MeetingFactory(type_id='ietf', date=datetime.date(2016,7,14), number="96")
+        query = MeetingRegistration.objects.filter(
+            first_name=person.first_name(),
+            last_name=person.last_name(),
+            country_code='US')
+        self.assertEqual(query.count(), 2)
+        self.assertEqual(query.filter(reg_type='onsite').count(), 1)
+        self.assertEqual(query.filter(reg_type='hackathon').count(), 1)
+        # call a second time to test delete
         get_meeting_registration_data(meeting)
-        query = MeetingRegistration.objects.filter(first_name='John',last_name='Smith',country_code='US')
-        emails = Email.objects.filter(address=email)
-        self.assertTrue(query.count(), 1)
-        self.assertTrue(isinstance(query[0].person, Person))
-        self.assertTrue(len(emails)>=1)                                
-        self.assertEqual(query[0].person, emails[0].person)                        
-
-        
+        query = MeetingRegistration.objects.filter(meeting=meeting, email=person.email())
+        self.assertEqual(query.count(), 1)
+        self.assertEqual(query.filter(reg_type='onsite').count(), 1)
+        self.assertEqual(query.filter(reg_type='hackathon').count(), 0)

ietf/stats/utils.py

@@ -7,16 +7,17 @@
 from collections import defaultdict
 
 from django.conf import settings
-from django.contrib.auth.models import User
 
 import debug                            # pyflakes:ignore
 
 from ietf.stats.models import AffiliationAlias, AffiliationIgnoredEnding, CountryAlias, MeetingRegistration
 from ietf.name.models import CountryName
-from ietf.person.models import Person, Email, Alias
-from ietf.person.name import unidecode_name
+from ietf.person.models import Person, Email
 from ietf.utils.log import log
 
+import logging
+logger = logging.getLogger('django')
+
 
 def compile_affiliation_ending_stripping_regexp():
     parts = []
@@ -250,7 +251,7 @@ def get_meeting_registration_data(meeting):
                 raise RuntimeError("Could not decode response from registrations API: '%s...'" % (response.content[:64], ))
 
         records = MeetingRegistration.objects.filter(meeting_id=meeting.pk).select_related('person')
-        meeting_registrations = {r.email:r for r in records}
+        meeting_registrations = {(r.email, r.reg_type):r for r in records}
         for registration in decoded:
             person = None
             # capture the stripped registration values for later use
@@ -259,11 +260,15 @@ def get_meeting_registration_data(meeting):
             affiliation     = registration['Company'].strip()
             country_code    = registration['Country'].strip()
             address         = registration['Email'].strip()
-            if address in meeting_registrations:
-                object = meeting_registrations[address]
+            reg_type        = registration['RegType'].strip()
+            if (address, reg_type) in meeting_registrations:
+                object = meeting_registrations.pop((address, reg_type))
                 created = False
             else:
-                object = MeetingRegistration.objects.create(meeting_id=meeting.pk, email=address)
+                object = MeetingRegistration.objects.create(
+                    meeting_id=meeting.pk,
+                    email=address,
+                    reg_type=reg_type)
                 created = True
 
             if (object.first_name != first_name[:200] or
@@ -286,75 +291,31 @@ def get_meeting_registration_data(meeting):
                     person = emails.first().person
                 # Create a new Person object
                 else:
-                  try:
-                    # Normalize all-caps or all-lower entries.  Don't touch
-                    # others, there might be names properly spelled with
-                    # internal uppercase letters.
-                    if ( ( first_name == first_name.upper() or first_name == first_name.lower() )
-                        and ( last_name == last_name.upper() or last_name == last_name.lower() ) ):
-                            first_name = first_name.capitalize()
-                            last_name = last_name.capitalize()
-                    regname = "%s %s" % (first_name, last_name)
-                    # if there are any unicode characters decode the string to ascii
-                    ascii_name = unidecode_name(regname)
-
-                    # Create a new user object if it does not exist already
-                    # if the user already exists do not try to create a new one
-                    users = User.objects.filter(username=address)
-                    if users.exists():
-                        user = users.first()
-                    else:
-                        # Create a new user.
-                        user = User.objects.create(
-                            first_name=first_name[:30],
-                            last_name=last_name[:30],
-                            username=address,
-                            email=address,
-                        )
-
-                    try:
-                        person = user.person
-                    except Person.DoesNotExist:
-                        aliases = Alias.objects.filter(name=regname)
-                        if aliases.exists():
-                            person = aliases.first().person
-                        else:
-                            # Create the new Person object.
-                            person = Person.objects.create(
-                                name=regname,
-                                ascii=ascii_name,
-                                user=user,
-                            )
-
-                    # Create an associated Email address for this Person
-                    try:
-                        email = Email.objects.get(person=person, address=address[:64])
-                    except Email.DoesNotExist:
-                        email = Email.objects.create(person=person, address=address[:64], origin='registration: ietf-%s'%meeting.number)
-
-                    # If this is the only email address, set primary to true.
-                    # If the person already existed (found through Alias) and
-                    # had email addresses, we don't do this.
-                    if Email.objects.filter(person=person).count() == 1:
-                        email.primary = True
-                        email.save()
-                  except:
-                      debug.show('first_name')
-                      debug.show('last_name')
-                      debug.show('regname')
-                      debug.show('user')
-                      debug.show('aliases')
-                      raise
+                    logger.error("No Person record for registration. email={}".format(address))
                 # update the person object to an actual value
                 object.person = person
                 object.save()
 
             if created:
                 num_created += 1
             num_processed += 1
+
+        # handle deleted registrations, if count is reasonable
+        # any registrations left in meeting_registrations no longer exist in reg
+        # so must have been deleted
+        if  0 < len(meeting_registrations) < 5:
+            for r in meeting_registrations:
+                try:
+                    MeetingRegistration.objects.get(meeting=meeting,email=r[0],reg_type=r[1]).delete()
+                    logger.info('Removing deleted registration. email={}, reg_type={}'.format(r[0], r[1]))
+                except MeetingRegistration.DoesNotExist:
+                    pass
     else:
         raise RuntimeError("Bad response from registrations API: %s, '%s'" % (response.status_code, response.content))
-    num_total = MeetingRegistration.objects.filter(meeting_id=meeting.pk).count()
+    num_total = MeetingRegistration.objects.filter(
+        meeting_id=meeting.pk,
+        attended=True,
+        reg_type__in=['onsite', 'remote']).count()
     if meeting.attendees is None or num_total > meeting.attendees:
         meeting.attendees = num_total
         meeting.save()