From 698f031b7f4486abdb2f8062f6a6ebd8e497111b Mon Sep 17 00:00:00 2001 From: rpcross Date: Thu, 16 Jun 2022 13:39:34 -0700 Subject: [PATCH] feat: separate MeetingRegistration rows for each registration type. updates the registration API (#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 --- ietf/api/tests.py | 40 ++++----- ietf/api/views.py | 26 +++--- ietf/ietfauth/utils.py | 8 +- ietf/stats/migrations/0004_split_records.py | 33 +++++++ ietf/stats/tests.py | 62 +++++++------- ietf/stats/utils.py | 95 ++++++--------------- ietf/stats/views.py | 10 ++- 7 files changed, 137 insertions(+), 137 deletions(-) create mode 100644 ietf/stats/migrations/0004_split_records.py diff --git a/ietf/api/tests.py b/ietf/api/tests.py index 66ce09133..b2cbc2c5d 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -279,23 +279,23 @@ class CustomApiTests(TestCase): 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 @@ class CustomApiTests(TestCase): # # 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 @@ class CustomApiTests(TestCase): # 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 @@ class CustomApiTests(TestCase): 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 @@ class TastypieApiTestCase(ResourceTestCaseMixin, TestCase): 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__,)) \ No newline at end of file + "There doesn't seem to be any API resource for model %s.models.%s"%(app.__name__,model.__name__,)) diff --git a/ietf/api/views.py b/ietf/api/views.py index 17ec1bdfe..2867addee 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -162,30 +162,28 @@ def api_new_meeting_registration(request): 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() diff --git a/ietf/ietfauth/utils.py b/ietf/ietfauth/utils.py index c626b98b9..280a92574 100644 --- a/ietf/ietfauth/utils.py +++ b/ietf/ietfauth/utils.py @@ -291,15 +291,13 @@ class OidcExtraScopeClaims(oidc_provider.lib.claims.ScopeClaims): 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], } diff --git a/ietf/stats/migrations/0004_split_records.py b/ietf/stats/migrations/0004_split_records.py new file mode 100644 index 000000000..83b2be24a --- /dev/null +++ b/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) + ] diff --git a/ietf/stats/tests.py b/ietf/stats/tests.py index 798cc8ba6..0204091b6 100644 --- a/ietf/stats/tests.py +++ b/ietf/stats/tests.py @@ -13,7 +13,6 @@ from requests import Response 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 @@ class StatisticsTests(TestCase): @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) diff --git a/ietf/stats/utils.py b/ietf/stats/utils.py index 75c7e56cc..4597d3f78 100644 --- a/ietf/stats/utils.py +++ b/ietf/stats/utils.py @@ -7,16 +7,17 @@ import requests 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,65 +291,7 @@ 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() @@ -352,9 +299,23 @@ def get_meeting_registration_data(meeting): 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() diff --git a/ietf/stats/views.py b/ietf/stats/views.py index 05adc2a35..ad7404dea 100644 --- a/ietf/stats/views.py +++ b/ietf/stats/views.py @@ -811,7 +811,10 @@ def meeting_stats(request, num=None, stats_type=None): return email.utils.formataddr(((r.first_name + " " + r.last_name).strip(), r.email)) if meeting and any(stats_type == t[0] for t in possible_stats_types): - attendees = MeetingRegistration.objects.filter(meeting=meeting, attended=True) + attendees = MeetingRegistration.objects.filter( + meeting=meeting, + attended=True, + reg_type__in=['onsite', 'remote']) if stats_type == "country": stats_title = "Number of attendees for {} {} per country".format(meeting.type.name, meeting.number) @@ -883,7 +886,10 @@ def meeting_stats(request, num=None, stats_type=None): elif not meeting and any(stats_type == t[0] for t in possible_stats_types): template_name = "overview" - attendees = MeetingRegistration.objects.filter(meeting__type="ietf", attended=True).select_related('meeting') + attendees = MeetingRegistration.objects.filter( + meeting__type="ietf", + attended=True, + reg_type__in=['onsite', 'remote']).select_related('meeting') if stats_type == "overview": stats_title = "Number of attendees per meeting"