From 07bfa68a7533b7adfedf8e8c43e9c38e4b566249 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Mon, 20 Jun 2022 10:46:36 -0500 Subject: [PATCH] feat: explicitly model session attendance (#4025) * feat: add model to track session attendance * feat: add model to track session attendance * feat: add api to set session attendees * fix: use user pk instead off person pk in the attended api. * feat: calculate three of five from attended * feat: management utility to populate Attended model history * docs: document why nomcom calculations don't use Attended yet. * fix: add migration to add new personalapikey endpoint to choices * test: verify very old last login prevents api key use, * chore: address review nits * chore: comment on some idiosyncracies of the expected input to populate_attended * fix: add unique_together constraint for the Attended model * fix: correctly handle empty querysets passed to three_of_five_eligible functions. --- ietf/api/tests.py | 70 +++++++++++++++- ietf/api/urls.py | 4 +- ietf/meeting/admin.py | 7 +- .../management/commands/populate_attended.py | 80 +++++++++++++++++++ ietf/meeting/migrations/0053_attended.py | 28 +++++++ ietf/meeting/models.py | 12 ++- ietf/meeting/resources.py | 20 ++++- ietf/meeting/views.py | 34 +++++++- ietf/nomcom/utils.py | 16 +++- .../migrations/0023_auto_20220615_1006.py | 18 +++++ ietf/person/models.py | 3 +- 11 files changed, 284 insertions(+), 8 deletions(-) create mode 100644 ietf/meeting/management/commands/populate_attended.py create mode 100644 ietf/meeting/migrations/0053_attended.py create mode 100644 ietf/person/migrations/0023_auto_20220615_1006.py diff --git a/ietf/api/tests.py b/ietf/api/tests.py index b2cbc2c5d..b887a6d46 100644 --- a/ietf/api/tests.py +++ b/ietf/api/tests.py @@ -1,7 +1,7 @@ # Copyright The IETF Trust 2015-2020, All Rights Reserved # -*- coding: utf-8 -*- - +import datetime import json import html import os @@ -24,7 +24,9 @@ import ietf from ietf.group.factories import RoleFactory from ietf.meeting.factories import MeetingFactory, SessionFactory from ietf.meeting.test_data import make_meeting_test_data +from ietf.meeting.models import Session from ietf.person.factories import PersonFactory, random_faker +from ietf.person.models import User from ietf.person.models import PersonalApiKey from ietf.stats.models import MeetingRegistration from ietf.utils.mail import outbox, get_payload_text @@ -144,6 +146,72 @@ class CustomApiTests(TestCase): event = doc.latest_event() self.assertEqual(event.by, recman) + def test_api_add_session_attendees(self): + url = urlreverse('ietf.meeting.views.api_add_session_attendees') + otherperson = PersonFactory() + recmanrole = RoleFactory(group__type_id='ietf', name_id='recman') + recman = recmanrole.person + meeting = MeetingFactory(type_id='ietf') + session = SessionFactory(group__type_id='wg', meeting=meeting) + apikey = PersonalApiKey.objects.create(endpoint=url, person=recman) + + badrole = RoleFactory(group__type_id='ietf', name_id='ad') + badapikey = PersonalApiKey.objects.create(endpoint=url, person=badrole.person) + badrole.person.user.last_login = timezone.now() + badrole.person.user.save() + + # Improper credentials, or method + r = self.client.post(url, {}) + self.assertContains(r, "Missing apikey parameter", status_code=400) + + r = self.client.post(url, {'apikey': badapikey.hash()} ) + self.assertContains(r, "Restricted to role: Recording Manager", status_code=403) + + r = self.client.post(url, {'apikey': apikey.hash()} ) + self.assertContains(r, "Too long since last regular login", status_code=400) + + recman.user.last_login = timezone.now()-datetime.timedelta(days=365) + recman.user.save() + r = self.client.post(url, {'apikey': apikey.hash()} ) + self.assertContains(r, "Too long since last regular login", status_code=400) + + recman.user.last_login = timezone.now() + recman.user.save() + r = self.client.get(url, {'apikey': apikey.hash()} ) + self.assertContains(r, "Method not allowed", status_code=405) + + recman.user.last_login = timezone.now() + recman.user.save() + + # Malformed requests + r = self.client.post(url, {'apikey': apikey.hash()} ) + self.assertContains(r, "Missing attended parameter", status_code=400) + + for baddict in ( + '{}', + '{"bogons;drop table":"bogons;drop table"}', + '{"session_id":"Not an integer;drop table"}', + f'{{"session_id":{session.pk},"attendees":"not a list;drop table"}}', + f'{{"session_id":{session.pk},"attendees":"not a list;drop table"}}', + f'{{"session_id":{session.pk},"attendees":[1,2,"not an int;drop table",4]}}', + ): + r = self.client.post(url, {'apikey': apikey.hash(), 'attended': baddict}) + self.assertContains(r, "Malformed post", status_code=400) + + bad_session_id = Session.objects.order_by('-pk').first().pk + 1 + r = self.client.post(url, {'apikey': apikey.hash(), 'attended': f'{{"session_id":{bad_session_id},"attendees":[]}}'}) + self.assertContains(r, "Invalid session", status_code=400) + bad_user_id = User.objects.order_by('-pk').first().pk + 1 + r = self.client.post(url, {'apikey': apikey.hash(), 'attended': f'{{"session_id":{session.pk},"attendees":[{bad_user_id}]}}'}) + self.assertContains(r, "Invalid attendee", status_code=400) + + # Reasonable request + r = self.client.post(url, {'apikey':apikey.hash(), 'attended': f'{{"session_id":{session.pk},"attendees":[{recman.user.pk}, {otherperson.user.pk}]}}'}) + + self.assertEqual(session.attended_set.count(),2) + self.assertTrue(session.attended_set.filter(person=recman).exists()) + self.assertTrue(session.attended_set.filter(person=otherperson).exists()) + def test_api_upload_bluesheet(self): url = urlreverse('ietf.meeting.views.api_upload_bluesheet') recmanrole = RoleFactory(group__type_id='ietf', name_id='recman') diff --git a/ietf/api/urls.py b/ietf/api/urls.py index ee1779a09..268d098cc 100644 --- a/ietf/api/urls.py +++ b/ietf/api/urls.py @@ -29,8 +29,10 @@ urlpatterns = [ url(r'^meeting/session/video/url$', meeting_views.api_set_session_video_url), # Let Meetecho trigger recording imports url(r'^notify/meeting/import_recordings/(?P[a-z0-9-]+)/?$', meeting_views.api_import_recordings), - # Let the registration system notify us about registrations + # Let MeetEcho upload bluesheets url(r'^notify/meeting/bluesheet/?$', meeting_views.api_upload_bluesheet), + # Let MeetEcho tell us about session attendees + url(r'^notify/session/attendees/?$', meeting_views.api_add_session_attendees), # Let the registration system notify us about registrations url(r'^notify/meeting/registration/?', api_views.api_new_meeting_registration), # OpenID authentication provider diff --git a/ietf/meeting/admin.py b/ietf/meeting/admin.py index 5b72c0523..131a8a1ab 100644 --- a/ietf/meeting/admin.py +++ b/ietf/meeting/admin.py @@ -4,7 +4,7 @@ from django.contrib import admin -from ietf.meeting.models import (Meeting, Room, Session, TimeSlot, Constraint, Schedule, +from ietf.meeting.models import (Attended, Meeting, Room, Session, TimeSlot, Constraint, Schedule, SchedTimeSessAssignment, ResourceAssociation, FloorPlan, UrlResource, SessionPresentation, ImportantDate, SlideSubmission, SchedulingEvent, BusinessConstraint, ProceedingsMaterial, MeetingHost) @@ -204,3 +204,8 @@ class MeetingHostAdmin(admin.ModelAdmin): list_display = ['name', 'meeting'] raw_id_fields = ['meeting'] admin.site.register(MeetingHost, MeetingHostAdmin) + +class AttendedAdmin(admin.ModelAdmin): + model = Attended + search_fields = ["person__name", "session__group__acronym", "session__meeting__number"] +admin.site.register(Attended, AttendedAdmin) diff --git a/ietf/meeting/management/commands/populate_attended.py b/ietf/meeting/management/commands/populate_attended.py new file mode 100644 index 000000000..4ba0608d4 --- /dev/null +++ b/ietf/meeting/management/commands/populate_attended.py @@ -0,0 +1,80 @@ +# Copyright The IETF Trust 2022, All Rights Reserved + +import debug # pyflakes: ignore + +from tqdm import tqdm + +from django.core.management.base import BaseCommand + +from ietf.meeting.models import Session +from ietf.meeting.utils import sort_sessions +from ietf.person.models import Person, Email + +import json + +class Command(BaseCommand): + + help = 'Populates the meeting Attended table based on bluesheets and registration information' + + def add_arguments(self, parser): + parser.add_argument('filename', nargs='+', type=str) + + def handle(self, *args, **options): + + issues = [] + session_cache = dict() + skipped = 0 + for filename in options['filename']: + records = json.loads(open(filename,'r').read()) + for record in tqdm(records): + user = record['sub'] + session_acronym = record['group'] + meeting_number = record['meeting'] + email = record['email'] + # In the expected dumps from MeetEcho, if there was only one session for group foo, it would just be named 'foo'. + # If there were _three_, we would see 'foo' for the first, 'foo_2' for the second, and 'foo_3' for the third. + # order below is the index into what is returned from sort_sessions -- 0 is the first session for a group at that meeting. + # There is brutal fixup below for older meetings where we had special arrangements where meetecho reported the non-existant + # group of 'plenary', mapping it into the appropriate 'ietf' group session. + # A bug in the export scripts at MeetEcho trimmed the '-t' from 'model-t'. + order = 0 + if session_acronym in ['anrw_test', 'demoanrw', 'hostspeaker']: + skipped = skipped + 1 + continue + if session_acronym=='model': + session_acronym='model-t' + if '_' in session_acronym: + session_acronym, order = session_acronym.split('_') + order = int(order)-1 + if session_acronym == 'plenary': + session_acronym = 'ietf' + if meeting_number == '111': + order = 4 + elif meeting_number == '110': + order = 3 + elif meeting_number == '109': + order = 6 + elif meeting_number == '108': + order = 13 + if not (meeting_number, session_acronym) in session_cache: + session_cache[(meeting_number, session_acronym)] = sort_sessions([s for s in Session.objects.filter(meeting__number=meeting_number,group__acronym=session_acronym) if s.official_timeslotassignment()]) + sessions = session_cache[(meeting_number, session_acronym)] + try: + session = sessions[order] + except IndexError: + issues.append(('session not found',record)) + continue + person = None + email = Email.objects.filter(address=email).first() + if email: + person = email.person + else: + person = Person.objects.filter(user__pk=user).first() + if not person: + issues.append(('person not found',record)) + continue + obj, created = session.attended_set.get_or_create(person=person) + for issue in issues: + print(issue) + print(f'{len(issues)} issues encountered') + print(f'{skipped} records intentionally skipped') diff --git a/ietf/meeting/migrations/0053_attended.py b/ietf/meeting/migrations/0053_attended.py new file mode 100644 index 000000000..110101cf1 --- /dev/null +++ b/ietf/meeting/migrations/0053_attended.py @@ -0,0 +1,28 @@ +# Copyright The IETF Trust 2022, All Rights Reserved +# Generated by Django 2.2.28 on 2022-06-17 08:40 + +from django.db import migrations, models +import django.db.models.deletion +import ietf.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('person', '0023_auto_20220615_1006'), + ('meeting', '0052_auto_20220503_1815'), + ] + + operations = [ + migrations.CreateModel( + name='Attended', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('person', ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='person.Person')), + ('session', ietf.utils.models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='meeting.Session')), + ], + options={ + 'unique_together': {('person', 'session')}, + }, + ), + ] diff --git a/ietf/meeting/models.py b/ietf/meeting/models.py index f73eed4de..83814b2d3 100644 --- a/ietf/meeting/models.py +++ b/ietf/meeting/models.py @@ -1379,4 +1379,14 @@ class MeetingHost(models.Model): class Meta: unique_together = (('meeting', 'name'),) - ordering = ('pk',) \ No newline at end of file + ordering = ('pk',) + +class Attended(models.Model): + person = ForeignKey(Person) + session = ForeignKey(Session) + + class Meta: + unique_together = (('person', 'session'),) + + def __str__(self): + return f'{self.person} at {self.session}' diff --git a/ietf/meeting/resources.py b/ietf/meeting/resources.py index 479b5dc4b..dc273c04c 100644 --- a/ietf/meeting/resources.py +++ b/ietf/meeting/resources.py @@ -14,7 +14,7 @@ from ietf import api from ietf.meeting.models import ( Meeting, ResourceAssociation, Constraint, Room, Schedule, Session, TimeSlot, SchedTimeSessAssignment, SessionPresentation, FloorPlan, UrlResource, ImportantDate, SlideSubmission, SchedulingEvent, - BusinessConstraint, ProceedingsMaterial, MeetingHost) + BusinessConstraint, ProceedingsMaterial, MeetingHost, Attended) from ietf.name.resources import MeetingTypeNameResource class MeetingResource(ModelResource): @@ -414,3 +414,21 @@ class MeetingHostResource(ModelResource): "meeting": ALL_WITH_RELATIONS, } api.meeting.register(MeetingHostResource()) + + +from ietf.person.resources import PersonResource +class AttendedResource(ModelResource): + person = ToOneField(PersonResource, 'person') + session = ToOneField(SessionResource, 'session') + class Meta: + queryset = Attended.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'attended' + ordering = ['id', ] + filtering = { + "id": ALL, + "person": ALL_WITH_RELATIONS, + "session": ALL_WITH_RELATIONS, + } +api.meeting.register(AttendedResource()) diff --git a/ietf/meeting/views.py b/ietf/meeting/views.py index 692197963..bf0870144 100644 --- a/ietf/meeting/views.py +++ b/ietf/meeting/views.py @@ -50,7 +50,7 @@ from ietf.doc.fields import SearchableDocumentsField from ietf.doc.models import Document, State, DocEvent, NewRevisionDocEvent, DocAlias from ietf.group.models import Group from ietf.group.utils import can_manage_session_materials, can_manage_some_groups, can_manage_group -from ietf.person.models import Person +from ietf.person.models import Person, User from ietf.ietfauth.utils import role_required, has_role, user_is_person from ietf.mailtrigger.utils import gather_address_lists from ietf.meeting.models import Meeting, Session, Schedule, FloorPlan, SessionPresentation, TimeSlot, SlideSubmission @@ -3710,6 +3710,38 @@ def api_set_session_video_url(request): return HttpResponse("Done", status=200, content_type='text/plain') +@require_api_key +@role_required('Recording Manager') # TODO : Rework how Meetecho interacts via APIs. There may be better paths to pursue than Personal API keys as they are currently defined. +@csrf_exempt +def api_add_session_attendees(request): + + def err(code, text): + return HttpResponse(text, status=code, content_type='text/plain') + + if request.method != 'POST': + return err(405, "Method not allowed") + attended_post = request.POST.get('attended') + if not attended_post: + return err(400, "Missing attended parameter") + try: + attended = json.loads(attended_post) + except json.decoder.JSONDecodeError: + return err(400, "Malformed post") + if not ( 'session_id' in attended and type(attended['session_id']) is int ): + return err(400, "Malformed post") + session_id = attended['session_id'] + if not ( 'attendees' in attended and type(attended['attendees']) is list and all([type(el) is int for el in attended['attendees']]) ): + return err(400, "Malformed post") + session = Session.objects.filter(pk=session_id).first() + if not session: + return err(400, "Invalid session") + users = User.objects.filter(pk__in=attended['attendees']) + if users.count() != len(attended['attendees']): + return err(400, "Invalid attendee") + for user in users: + session.attended_set.get_or_create(person=user.person) + return HttpResponse("Done", status=200, content_type='text/plain') + @require_api_key @role_required('Recording Manager', 'Secretariat') diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 5f7fc03f5..74285b0d5 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -608,10 +608,24 @@ def three_of_five_eligible(previous_five, queryset=None): 3 of the 5 type_id='ietf' meetings before the given date. Does not disqualify anyone based on held roles. """ - if not queryset: + if queryset is None: queryset = Person.objects.all() return queryset.filter(meetingregistration__meeting__in=list(previous_five),meetingregistration__attended=True).annotate(mtg_count=Count('meetingregistration')).filter(mtg_count__gte=3) +def new_three_of_five_eligible(previous_five, queryset=None): + """ Return a list of Person records who attended at least + 3 of the 5 type_id='ietf' meetings before the given + date. Does not disqualify anyone based on held roles. + This 'new' variant bases the calculation on the Meeting.Session model rather than Stats.MeetingRegistration + Leadership will have to create a new RFC specifying eligibility (RFC8989 is timing out) before it can be used. + """ + if queryset is None: + queryset = Person.objects.all() + return queryset.filter( + Q(attended__session__meeting__in=list(previous_five)), + Q(attended__session__type='plenary')|Q(attended__session__group__type__in=['wg','rg']) + ).annotate(mtg_count=Count('attended__session__meeting',distinct=True)).filter(mtg_count__gte=3) + def suggest_affiliation(person): recent_meeting = person.meetingregistration_set.order_by('-meeting__date').first() affiliation = recent_meeting.affiliation if recent_meeting else '' diff --git a/ietf/person/migrations/0023_auto_20220615_1006.py b/ietf/person/migrations/0023_auto_20220615_1006.py new file mode 100644 index 000000000..22b50a25a --- /dev/null +++ b/ietf/person/migrations/0023_auto_20220615_1006.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.28 on 2022-06-15 10:06 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('person', '0022_auto_20220513_1456'), + ] + + operations = [ + migrations.AlterField( + model_name='personalapikey', + name='endpoint', + field=models.CharField(choices=[('/api/appauth/authortools', '/api/appauth/authortools'), ('/api/appauth/bibxml', '/api/appauth/bibxml'), ('/api/iesg/position', '/api/iesg/position'), ('/api/meeting/session/video/url', '/api/meeting/session/video/url'), ('/api/notify/meeting/bluesheet', '/api/notify/meeting/bluesheet'), ('/api/notify/meeting/registration', '/api/notify/meeting/registration'), ('/api/notify/session/attendees', '/api/notify/session/attendees'), ('/api/v2/person/person', '/api/v2/person/person')], max_length=128), + ), + ] diff --git a/ietf/person/models.py b/ietf/person/models.py index 2b2b337e7..6094ac214 100644 --- a/ietf/person/models.py +++ b/ietf/person/models.py @@ -355,7 +355,8 @@ PERSON_API_KEY_VALUES = [ ("/api/v2/person/person", "/api/v2/person/person", "Robot"), ("/api/meeting/session/video/url", "/api/meeting/session/video/url", "Recording Manager"), ("/api/notify/meeting/registration", "/api/notify/meeting/registration", "Robot"), - ("/api/notify/meeting/bluesheet", "/api/notify/meeting/bluesheet", "Recording Manager"), + ("/api/notify/meeting/bluesheet", "/api/notify/meeting/bluesheet", "Recording Manager"), + ("/api/notify/session/attendees", "/api/notify/session/attendees", "Recording Manager"), ("/api/appauth/authortools", "/api/appauth/authortools", None), ("/api/appauth/bibxml", "/api/appauth/bibxml", None), ]