From 216ec499dffe51a13607a22dfd6ae62a6db71b93 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Sat, 1 May 2021 19:57:04 +0000 Subject: [PATCH 1/4] Checkpointing. Remaining work: convert meetingregistation fixup to a migration and a mgmt comment. Flesh out testing of 8989 rule 2 and fix the known edge case bug. Remove old implementation and connect UI to the new implementation. - Legacy-Id: 18971 --- ietf/doc/factories.py | 13 +- ietf/group/factories.py | 25 +- .../0010_nomcom_first_call_for_volunteers.py | 18 ++ ietf/nomcom/models.py | 1 + ietf/nomcom/tests.py | 305 +++++++++++++++++- ietf/nomcom/utils.py | 110 ++++++- ietf/stats/factories.py | 17 + ietf/stats/utils.py | 46 +++ 8 files changed, 529 insertions(+), 6 deletions(-) create mode 100644 ietf/nomcom/migrations/0010_nomcom_first_call_for_volunteers.py create mode 100644 ietf/stats/factories.py diff --git a/ietf/doc/factories.py b/ietf/doc/factories.py index 2f74fb60b..9824b5ef4 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -13,7 +13,7 @@ from django.conf import settings from ietf.doc.models import ( Document, DocEvent, NewRevisionDocEvent, DocAlias, State, DocumentAuthor, StateDocEvent, BallotPositionDocEvent, BallotDocEvent, BallotType, IRSGBallotDocEvent, TelechatDocEvent, - DocumentActionHolder) + DocumentActionHolder, DocumentAuthor) from ietf.group.models import Group def draft_name_generator(type_id,group,n): @@ -365,3 +365,14 @@ class DocumentActionHolderFactory(factory.DjangoModelFactory): document = factory.SubFactory(WgDraftFactory) person = factory.SubFactory('ietf.person.factories.PersonFactory') + +class DocumentAuthorFactory(factory.DjangoModelFactory): + class Meta: + model = DocumentAuthor + + document = factory.SubFactory(DocumentFactory) + person = factory.SubFactory('ietf.person.factories.PersonFactory') + email = factory.LazyAttribute(lambda obj: obj.person.email()) + +class WgDocumentAuthorFactory(DocumentAuthorFactory): + document = factory.SubFactory(WgDraftFactory) diff --git a/ietf/group/factories.py b/ietf/group/factories.py index 911628ba6..163ebb52e 100644 --- a/ietf/group/factories.py +++ b/ietf/group/factories.py @@ -5,7 +5,8 @@ import factory from typing import List # pyflakes:ignore -from ietf.group.models import Group, Role, GroupEvent, GroupMilestone +from ietf.group.models import Group, Role, GroupEvent, GroupMilestone, \ + GroupHistory, RoleHistory from ietf.review.factories import ReviewTeamSettingsFactory class GroupFactory(factory.DjangoModelFactory): @@ -71,3 +72,25 @@ class DatelessGroupMilestoneFactory(BaseGroupMilestoneFactory): group = factory.SubFactory(GroupFactory, uses_milestone_dates=False) order = factory.Sequence(lambda n: n) +class GroupHistoryFactory(factory.DjangoModelFactory): + class Meta: + model=GroupHistory + + name = factory.LazyAttribute(lambda obj: obj.group.name) + state_id = 'active' + type_id = factory.LazyAttribute(lambda obj: obj.group.type_id) + list_email = factory.LazyAttribute(lambda obj: '%s@ietf.org'% obj.group.acronym) + uses_milestone_dates = True + used_roles = [] # type: List[str] + + group = factory.SubFactory(GroupFactory) + acronym = factory.LazyAttribute(lambda obj: obj.group.acronym) + +class RoleHistoryFactory(factory.DjangoModelFactory): + class Meta: + model=RoleHistory + + group = factory.SubFactory(GroupHistoryFactory) + person = factory.SubFactory('ietf.person.factories.PersonFactory') + email = factory.LazyAttribute(lambda obj: obj.person.email()) + diff --git a/ietf/nomcom/migrations/0010_nomcom_first_call_for_volunteers.py b/ietf/nomcom/migrations/0010_nomcom_first_call_for_volunteers.py new file mode 100644 index 000000000..01f2f3335 --- /dev/null +++ b/ietf/nomcom/migrations/0010_nomcom_first_call_for_volunteers.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.20 on 2021-04-22 14:32 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('nomcom', '0009_auto_20201109_0439'), + ] + + operations = [ + migrations.AddField( + model_name='nomcom', + name='first_call_for_volunteers', + field=models.DateField(blank=True, null=True, verbose_name='Date of the first call for volunteers'), + ), + ] diff --git a/ietf/nomcom/models.py b/ietf/nomcom/models.py index 439759838..ebb8c7abf 100644 --- a/ietf/nomcom/models.py +++ b/ietf/nomcom/models.py @@ -59,6 +59,7 @@ class NomCom(models.Model): help_text='Display pictures of each nominee (if available) on the feedback pages') show_accepted_nominees = models.BooleanField(verbose_name='Show accepted nominees', default=True, help_text='Show accepted nominees on the public nomination page') + first_call_for_volunteers = models.DateField(verbose_name='Date of the first call for volunteers', blank=True, null=True) class Meta: verbose_name_plural = 'NomComs' diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index 80c5b8e9e..6e79f1159 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -9,6 +9,7 @@ import shutil from pyquery import PyQuery from urllib.parse import urlparse +from itertools import combinations from django.db import IntegrityError from django.db.models import Max @@ -22,7 +23,9 @@ import debug # pyflakes:ignore from ietf.dbtemplate.factories import DBTemplateFactory from ietf.dbtemplate.models import DBTemplate -from ietf.group.models import Group +from ietf.doc.factories import DocEventFactory, WgDocumentAuthorFactory +from ietf.group.factories import RoleFactory, RoleHistoryFactory +from ietf.group.models import Group, Role from ietf.meeting.factories import MeetingFactory from ietf.message.models import Message from ietf.nomcom.test_data import nomcom_test_data, generate_cert, check_comments, \ @@ -35,10 +38,13 @@ from ietf.nomcom.management.commands.send_reminders import Command, is_time_to_s from ietf.nomcom.factories import NomComFactory, FeedbackFactory, TopicFactory, \ nomcom_kwargs_for_year, provide_private_key_to_test_client, \ key -from ietf.nomcom.utils import get_nomcom_by_year, make_nomineeposition, get_hash_nominee_position +from ietf.nomcom.utils import get_nomcom_by_year, make_nomineeposition, \ + get_hash_nominee_position, is_eligible, list_eligible, \ + get_eligibility_date from ietf.person.factories import PersonFactory, EmailFactory from ietf.person.models import Email, Person from ietf.stats.models import MeetingRegistration +from ietf.stats.factories import MeetingRegistrationFactory from ietf.utils.mail import outbox, empty_outbox, get_payload_text from ietf.utils.test_utils import login_testing_unauthorized, TestCase, unicontent @@ -2117,3 +2123,298 @@ class TopicTests(TestCase): self.assertEqual(r.status_code,200) self.assertEqual(topic.feedback_set.count(),1) self.client.logout() + +class EligibilityUnitTests(TestCase): + + def test_get_eligibility_date(self): + + # No Nomcoms exist: + self.assertEqual(get_eligibility_date(), datetime.date(datetime.date.today().year,5,1)) + + # a provided date trumps anything in the database + self.assertEqual(get_eligibility_date(date=datetime.date(2001,2,3)), datetime.date(2001,2,3)) + n = NomComFactory(group__acronym='nomcom2015',populate_personnel=False) + self.assertEqual(get_eligibility_date(date=datetime.date(2001,2,3)), datetime.date(2001,2,3)) + self.assertEqual(get_eligibility_date(nomcom=n, date=datetime.date(2001,2,3)), datetime.date(2001,2,3)) + + # Now there's a nomcom in the database + self.assertEqual(get_eligibility_date(nomcom=n), datetime.date(2015,5,1)) + n.first_call_for_volunteers = datetime.date(2015,5,17) + n.save() + self.assertEqual(get_eligibility_date(nomcom=n), datetime.date(2015,5,17)) + # No nomcoms in the database with seated members + self.assertEqual(get_eligibility_date(), datetime.date(datetime.date.today().year,5,1)) + + RoleFactory(group=n.group,name_id='member') + self.assertEqual(get_eligibility_date(),datetime.date(2016,5,1)) + + NomComFactory(group__acronym='nomcom2016', populate_personnel=False, first_call_for_volunteers=datetime.date(2016,5,4)) + self.assertEqual(get_eligibility_date(),datetime.date(2016,5,4)) + + this_year = datetime.date.today().year + NomComFactory(group__acronym=f'nomcom{this_year}', first_call_for_volunteers=datetime.date(this_year,5,6)) + self.assertEqual(get_eligibility_date(),datetime.date(this_year,5,6)) + + +class rfc8713EligibilityTests(TestCase): + + def setUp(self): + self.nomcom = NomComFactory(group__acronym='nomcom2019', populate_personnel=False, first_call_for_volunteers=datetime.date(2019,5,1)) + + meetings = [ MeetingFactory(date=date,type_id='ietf') for date in ( + datetime.date(2019,3,1), + datetime.date(2018,11,1), + datetime.date(2018,7,1), + datetime.date(2018,3,1), + datetime.date(2017,11,1), + )] + + self.eligible_people = list() + self.ineligible_people = list() + + for combo_len in range(0,6): + for combo in combinations(meetings,combo_len): + p = PersonFactory() + for m in combo: + MeetingRegistrationFactory(person=p, meeting=m) + if combo_len<3: + self.ineligible_people.append(p) + else: + self.eligible_people.append(p) + + # No-one is eligible for the other_nomcom + self.other_nomcom = NomComFactory(group__acronym='nomcom2018',first_call_for_volunteers=datetime.date(2018,5,1)) + + # Someone is eligible at this other_date + 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') + + + def test_is_person_eligible(self): + for person in self.eligible_people: + self.assertTrue(is_eligible(person,self.nomcom)) + self.assertTrue(is_eligible(person)) + self.assertFalse(is_eligible(person,nomcom=self.other_nomcom)) + self.assertFalse(is_eligible(person,date=self.other_date)) + + for person in self.ineligible_people: + self.assertFalse(is_eligible(person,self.nomcom)) + + for person in self.other_people: + self.assertTrue(is_eligible(person,date=self.other_date)) + + + def test_list_eligible(self): + self.assertEqual(set(list_eligible()), set(self.eligible_people)) + self.assertEqual(set(list_eligible(self.nomcom)), set(self.eligible_people)) + self.assertEqual(set(list_eligible(self.other_nomcom)),set(self.other_people)) + self.assertEqual(set(list_eligible(date=self.other_date)),set(self.other_people)) + + +class rfc8788EligibilityTests(TestCase): + + def setUp(self): + self.nomcom = NomComFactory(group__acronym='nomcom2020', populate_personnel=False, first_call_for_volunteers=datetime.date(2020,5,1)) + + meetings = [MeetingFactory(number=number, date=date, type_id='ietf') for number,date in [ + ('106', datetime.date(2019, 11, 16)), + ('105', datetime.date(2019, 7, 20)), + ('104', datetime.date(2019, 3, 23)), + ('103', datetime.date(2018, 11, 3)), + ('102', datetime.date(2018, 7, 14)), + ]] + + self.eligible_people = list() + self.ineligible_people = list() + + for combo_len in range(0,6): + for combo in combinations(meetings,combo_len): + p = PersonFactory() + for m in combo: + MeetingRegistrationFactory(person=p, meeting=m) + if combo_len<3: + self.ineligible_people.append(p) + else: + self.eligible_people.append(p) + + def test_is_person_eligible(self): + for person in self.eligible_people: + self.assertTrue(is_eligible(person,self.nomcom)) + + for person in self.ineligible_people: + self.assertFalse(is_eligible(person,self.nomcom)) + + + def test_list_eligible(self): + self.assertEqual(set(list_eligible(self.nomcom)), set(self.eligible_people)) + +class rfc8989EligibilityTests(TestCase): + + def setUp(self): + self.nomcom = NomComFactory(group__acronym='nomcom2021', populate_personnel=False, first_call_for_volunteers=datetime.date(2021,5,15)) + # make_immutable_test_data makes things this test does not want + Role.objects.filter(name_id__in=('chair','secr')).delete() + + def test_elig_by_meetings(self): + + meetings = [MeetingFactory(number=number, date=date, type_id='ietf') for number,date in [ + ('110', datetime.date(2021, 3, 6)), + ('109', datetime.date(2020, 11, 14)), + ('108', datetime.date(2020, 7, 25)), + ('107', datetime.date(2020, 3, 21)), + ('106', datetime.date(2019, 11, 16)), + ]] + + eligible_people = list() + ineligible_people = list() + + for combo_len in range(0,6): + for combo in combinations(meetings,combo_len): + p = PersonFactory() + for m in combo: + MeetingRegistrationFactory(person=p, meeting=m) + if combo_len<3: + ineligible_people.append(p) + else: + eligible_people.append(p) + + self.assertEqual(set(eligible_people),set(list_eligible(self.nomcom))) + + for person in eligible_people: + self.assertTrue(is_eligible(person,self.nomcom)) + + for person in ineligible_people: + self.assertFalse(is_eligible(person,self.nomcom)) + + def test_elig_by_office_active_groups(self): + + chair = RoleFactory(name_id='chair').person + + secr = RoleFactory(name_id='secr').person + + nobody=PersonFactory() + + self.assertTrue(is_eligible(person=chair,nomcom=self.nomcom)) + self.assertTrue(is_eligible(person=secr,nomcom=self.nomcom)) + self.assertFalse(is_eligible(person=nobody,nomcom=self.nomcom)) + + self.assertEqual(set([chair,secr]), set(list_eligible(nomcom=self.nomcom))) + + # Current implementation of 8989 rule 2 has an edge case bug + # If someone was made a wg officer after the elgibility date proscribed by rfc8989, they will still be counted as eligible. + + + def test_elig_by_office_closed_groups(self): + + elig_date=get_eligibility_date(self.nomcom) + day_before = elig_date-datetime.timedelta(days=1) + year_before = datetime.date(elig_date.year-1,elig_date.month,elig_date.day) + three_years_before = datetime.date(elig_date.year-3,elig_date.month,elig_date.day) + just_after_three_years_before = three_years_before + datetime.timedelta(days=1) + just_before_three_years_before = three_years_before - datetime.timedelta(days=1) + + eligible = list() + ineligible = list() + + p1 = RoleHistoryFactory( + name_id='chair', + group__time=day_before, + group__group__state_id='conclude', + ).person + eligible.append(p1) + + p2 = RoleHistoryFactory( + name_id='secr', + group__time=year_before, + group__group__state_id='conclude', + ).person + eligible.append(p2) + + p3 = RoleHistoryFactory( + name_id='secr', + group__time=just_after_three_years_before, + group__group__state_id='conclude', + ).person + eligible.append(p3) + + p4 = RoleHistoryFactory( + name_id='chair', + group__time=three_years_before, + group__group__state_id='conclude', + ).person + eligible.append(p4) + + p5 = RoleHistoryFactory( + name_id='chair', + group__time=just_before_three_years_before, + group__group__state_id='conclude', + ).person + ineligible.append(p5) + + for person in eligible: + self.assertTrue(is_eligible(person,self.nomcom)) + + for person in ineligible: + self.assertFalse(is_eligible(person,self.nomcom)) + + self.assertEqual(set(list_eligible(nomcom=self.nomcom)),set(eligible)) + + + def test_elig_by_author(self): + + elig_date = get_eligibility_date(self.nomcom) + + last_date = elig_date + first_date = datetime.date(last_date.year-5,last_date.month,last_date.day) + day_after_last_date = last_date+datetime.timedelta(days=1) + day_before_first_date = first_date-datetime.timedelta(days=1) + middle_date = datetime.date(last_date.year-3,last_date.month,last_date.day) + + eligible = set() + ineligible = set() + + p = PersonFactory() + ineligible.add(p) + + p = PersonFactory() + da = WgDocumentAuthorFactory(person=p) + DocEventFactory(type='published_rfc',doc=da.document,time=middle_date) + ineligible.add(p) + + p = PersonFactory() + da = WgDocumentAuthorFactory(person=p) + DocEventFactory(type='iesg_approved',doc=da.document,time=last_date) + da = WgDocumentAuthorFactory(person=p) + DocEventFactory(type='published_rfc',doc=da.document,time=first_date) + eligible.add(p) + + p = PersonFactory() + da = WgDocumentAuthorFactory(person=p) + DocEventFactory(type='iesg_approved',doc=da.document,time=middle_date) + da = WgDocumentAuthorFactory(person=p) + DocEventFactory(type='published_rfc',doc=da.document,time=day_before_first_date) + ineligible.add(p) + + p = PersonFactory() + da = WgDocumentAuthorFactory(person=p) + DocEventFactory(type='iesg_approved',doc=da.document,time=day_after_last_date) + da = WgDocumentAuthorFactory(person=p) + DocEventFactory(type='published_rfc',doc=da.document,time=middle_date) + ineligible.add(p) + + for person in eligible: + self.assertTrue(is_eligible(person,self.nomcom)) + + for person in ineligible: + self.assertFalse(is_eligible(person,self.nomcom)) + + self.assertEqual(set(list_eligible(nomcom=self.nomcom)),set(eligible)) + + + def test_combo_elig(self): + + pass + + diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index bf7263720..5f96ce68a 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -13,7 +13,7 @@ from email.header import decode_header from email.iterators import typed_subpart_iterator from email.utils import parseaddr -from django.db.models import Q +from django.db.models import Q, Count from django.conf import settings from django.contrib.sites.models import Site from django.core.exceptions import ObjectDoesNotExist @@ -22,8 +22,11 @@ from django.template.loader import render_to_string from django.shortcuts import get_object_or_404 from ietf.dbtemplate.models import DBTemplate +from ietf.doc.models import DocEvent +from ietf.group.models import Group, Role from ietf.person.models import Email, Person from ietf.mailtrigger.utils import gather_address_lists +from ietf.meeting.models import Meeting from ietf.utils.pipe import pipe from ietf.utils.mail import send_mail_text, send_mail, get_payload_text from ietf.utils.log import log @@ -477,4 +480,107 @@ def create_feedback_email(nomcom, msg): class EncryptedException(Exception): pass - \ No newline at end of file +def is_eligible(person, nomcom=None, date=None): + return list_eligible(nomcom=nomcom, date=date, base_qs=Person.objects.filter(pk=person.pk)).exists() + +def list_eligible(nomcom=None, date=None, base_qs=None): + if not base_qs: + base_qs = Person.objects.all() + eligibility_date = get_eligibility_date(nomcom, date) + if eligibility_date.year in range(2008,2020): + return list_eligible_8713(date=eligibility_date, base_qs=base_qs) + elif eligibility_date.year == 2020: + return list_eligible_8788(date=eligibility_date, base_qs=base_qs) + elif eligibility_date.year == 2021: + return list_eligible_8989(date=eligibility_date, base_qs=base_qs) + else: + return Person.objects.none() + +def list_eligible_8713(date, base_qs=None): + if not base_qs: + base_qs = Person.objects.all() + previous_five = previous_five_meetings(date) + return three_of_five_eligible(previous_five=previous_five, queryset=base_qs) + +def list_eligible_8788(date, base_qs=None): + if not base_qs: + base_qs = Person.objects.all() + previous_five = Meeting.objects.filter(number__in=['102','103','104','105','106']) + return three_of_five_eligible(previous_five=previous_five, queryset=base_qs) + +def list_eligible_8989(date, base_qs=None): + if not base_qs: + base_qs = Person.objects.all() + + previous_five = previous_five_meetings(date) + three_of_five_qs = three_of_five_eligible(previous_five=previous_five, queryset=base_qs) + + three_years_ago = datetime.date(date.year-3,date.month,date.day) + officer_qs = base_qs.filter( + # is currently an officer + Q(role__name_id__in=('chair','secr'), + role__group__state_id='active', + role__group__type_id='wg', + ) + # was an officer since the given date (I think this is wrong - it looks at when roles _start_, not when roles end) + | Q(rolehistory__group__time__gte=three_years_ago, + rolehistory__name_id__in=('chair','secr'), + rolehistory__group__state_id='active', + rolehistory__group__type_id='wg', + ) + ).distinct() + + five_years_ago = datetime.date(date.year-5,date.month,date.day) + rfc_pks = set(DocEvent.objects.filter(type='published_rfc',time__gte=five_years_ago,time__lte=date).values_list('doc__pk',flat=True)) + iesgappr_pks = set(DocEvent.objects.filter(type='iesg_approved',time__gte=five_years_ago,time__lte=date).values_list('doc__pk',flat=True)) + qualifying_pks = rfc_pks.union(iesgappr_pks.difference(rfc_pks)) + author_qs = base_qs.filter( + documentauthor__document__pk__in=qualifying_pks + ).annotate( + document_author_count = Count('documentauthor') + ).filter(document_author_count__gte=2) + +# return three_of_five_qs.union(officer_qs, author_qs) + return Person.objects.filter(pk__in= + set(three_of_five_qs.values_list('pk',flat=True)).union( + set(officer_qs.values_list('pk',flat=True))).union( + set(author_qs.values_list('pk',flat=True))) + ) + +def get_eligibility_date(nomcom=None, date=None): + if date: + return date + elif nomcom: + if nomcom.first_call_for_volunteers: + return nomcom.first_call_for_volunteers + else: + return datetime.date(int(nomcom.group.acronym[6:]),5,1) + else: + last_seated=Role.objects.filter(group__type_id='nomcom',name_id='member').order_by('-group__acronym').first() + if last_seated: + last_nomcom_year = int(last_seated.group.acronym[6:]) + if last_nomcom_year == datetime.date.today().year: + next_nomcom_year = last_nomcom_year + else: + next_nomcom_year = int(last_seated.group.acronym[6:])+1 + next_nomcom_group = Group.objects.filter(acronym=f'nomcom{next_nomcom_year}').first() + if next_nomcom_group and next_nomcom_group.nomcom_set.first().first_call_for_volunteers: + return next_nomcom_group.nomcom_set.first().first_call_for_volunteers + else: + return datetime.date(next_nomcom_year,5,1) + else: + return datetime.date(datetime.date.today().year,5,1) + +def previous_five_meetings(date = datetime.date.today()): + return Meeting.objects.filter(type='ietf',date__lte=date).order_by('-date')[:5] + +def 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. + """ + if not queryset: + 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) + + diff --git a/ietf/stats/factories.py b/ietf/stats/factories.py new file mode 100644 index 000000000..29cda06c5 --- /dev/null +++ b/ietf/stats/factories.py @@ -0,0 +1,17 @@ +# Copyright The IETF Trust 2021, All Rights Reserved + +import factory + +from ietf.stats.models import MeetingRegistration +from ietf.meeting.factories import MeetingFactory +from ietf.person.factories import PersonFactory + +class MeetingRegistrationFactory(factory.DjangoModelFactory): + class Meta: + model = MeetingRegistration + + meeting = factory.SubFactory(MeetingFactory) + person = factory.SubFactory(PersonFactory) + first_name = factory.LazyAttribute(lambda obj: obj.person.first_name()) + last_name = factory.LazyAttribute(lambda obj: obj.person.last_name()) + attended = True \ No newline at end of file diff --git a/ietf/stats/utils.py b/ietf/stats/utils.py index 8fb263d8e..e91a3d737 100644 --- a/ietf/stats/utils.py +++ b/ietf/stats/utils.py @@ -6,6 +6,7 @@ import re import requests from collections import defaultdict +from django.db.models import F, Q from django.conf import settings from django.contrib.auth.models import User @@ -344,3 +345,48 @@ def get_meeting_registration_data(meeting): meeting.attendees = num_total meeting.save() return num_created, num_processed, num_total + +# Might abandon this as too ambitious - if fixed things for 128 people in ietf 100 - 110 though, and that's probably worth doing. +# Yeah - experiments say it _must_ be done. Probably need this as a management command. +def repair_meetingregistration_person(meetings=None): + maybe_address = set() + different_person = set() + no_person = set() + maybe_person = set() + no_email = set() + ok_records = 0 + skipped = 0 + repaired_records = 0 + + +# for mr in MeetingRegistration.objects.exclude(person__email__address=F('email')): + for mr in MeetingRegistration.objects.all(): + if meetings and mr.meeting.number not in meetings: + skipped += 1 + continue + if mr.person and mr.email and mr.email in mr.person.email_set.values_list('address',flat=True): + ok_records += 1 + continue + if mr.email: + email_person = Person.objects.filter(email__address=mr.email).first() + if mr.person: + if not email_person: + maybe_address.add(f'{mr.email} is not present in any Email object. The MeetingRegistration object implies this is an address for {mr.person} ({mr.person.pk})') + elif email_person != mr.person: + different_person.add(f'{mr} ({mr.pk}) has person {mr.person} ({mr.person.pk}) but an email {mr.email} attached to a different person {email_person} ({email_person.pk}).') + elif email_person: + mr.person = email_person + mr.save() + repaired_records += 1 + else: + maybe_person_qs = Person.objects.filter(name__icontains=mr.last_name).filter(name__icontains=mr.first_name) +# if not maybe_person_qs.exists(): +# maybe_person_qs = Person.objects.filter(name__icontains=mr.last_name) + if maybe_person_qs: + maybe_person.add(f'{mr} ({mr.pk}) has email address {mr.email} which cannot be associated with any Person. Consider these possible people {[(p,p.pk) for p in maybe_person_qs]}') + else: + no_person.add(f'{mr} ({mr.pk}) has email address {mr.email} which cannot be associated with any Person') + else: + no_email.add(f'{mr} ({mr.pk}) provides no email address') + + return ok_records, repaired_records, skipped, maybe_address, different_person, maybe_person, no_person, no_email From bcc280fe9ba27a14ffbbddc0af6a98435c2b9845 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Thu, 6 May 2021 13:51:29 +0000 Subject: [PATCH 2/4] tightened edge for group office rule - Legacy-Id: 18972 --- ietf/nomcom/tests.py | 17 ++++++++++++++--- ietf/nomcom/utils.py | 2 ++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index 6e79f1159..c9cce5596 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -24,7 +24,7 @@ import debug # pyflakes:ignore from ietf.dbtemplate.factories import DBTemplateFactory from ietf.dbtemplate.models import DBTemplate from ietf.doc.factories import DocEventFactory, WgDocumentAuthorFactory -from ietf.group.factories import RoleFactory, RoleHistoryFactory +from ietf.group.factories import GroupFactory, GroupHistoryFactory, RoleFactory, RoleHistoryFactory from ietf.group.models import Group, Role from ietf.meeting.factories import MeetingFactory from ietf.message.models import Message @@ -2302,8 +2302,19 @@ class rfc8989EligibilityTests(TestCase): self.assertEqual(set([chair,secr]), set(list_eligible(nomcom=self.nomcom))) - # Current implementation of 8989 rule 2 has an edge case bug - # If someone was made a wg officer after the elgibility date proscribed by rfc8989, they will still be counted as eligible. + + def test_elig_by_office_edge(self): + + elig_date=get_eligibility_date(self.nomcom) + day_after = elig_date + datetime.timedelta(days=1) + two_days_after = elig_date + datetime.timedelta(days=2) + + group = GroupFactory(time=two_days_after) + GroupHistoryFactory(group=group,time=day_after) + + after_chair = RoleFactory(name_id='chair',group=group).person + + self.assertFalse(is_eligible(person=after_chair,nomcom=self.nomcom)) def test_elig_by_office_closed_groups(self): diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 5f96ce68a..6472d1325 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -521,9 +521,11 @@ def list_eligible_8989(date, base_qs=None): Q(role__name_id__in=('chair','secr'), role__group__state_id='active', role__group__type_id='wg', + role__group__time__lte=date, ) # was an officer since the given date (I think this is wrong - it looks at when roles _start_, not when roles end) | Q(rolehistory__group__time__gte=three_years_ago, + rolehistory__group__time__lte=date, rolehistory__name_id__in=('chair','secr'), rolehistory__group__state_id='active', rolehistory__group__type_id='wg', From e3476f5bdbf95c20986eacd981bad0f447c119cb Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Thu, 6 May 2021 16:03:32 +0000 Subject: [PATCH 3/4] wrap the repair and report utilities in management commands - Legacy-Id: 18973 --- .../find_meetingregistration_person_issues.py | 37 ++++++++++ .../repair_meetingregistration_person.py | 18 +++++ ietf/stats/utils.py | 67 +++++++++++-------- 3 files changed, 93 insertions(+), 29 deletions(-) create mode 100644 ietf/stats/management/commands/find_meetingregistration_person_issues.py create mode 100644 ietf/stats/management/commands/repair_meetingregistration_person.py diff --git a/ietf/stats/management/commands/find_meetingregistration_person_issues.py b/ietf/stats/management/commands/find_meetingregistration_person_issues.py new file mode 100644 index 000000000..5dd6919e5 --- /dev/null +++ b/ietf/stats/management/commands/find_meetingregistration_person_issues.py @@ -0,0 +1,37 @@ +# Copyright The IETF Trust 2021, All Rights Reserved + +import debug # pyflakes:ignore + +from django.core.management.base import BaseCommand, CommandError + +from ietf.stats.utils import find_meetingregistration_person_issues + +class Command(BaseCommand): + help = "Find possible Person/Email objects to repair based on MeetingRegistration objects" + + def add_arguments(self, parser): + parser.add_argument('--meeting',action='append') + + def handle(self, *args, **options): + meetings = options['meeting'] or None + summary = find_meetingregistration_person_issues(meetings) + + print(f'{summary.ok_records} records are OK') + + for msg in summary.could_be_fixed: + print(msg) + + for msg in summary.maybe_address: + print(msg) + + for msg in summary.different_person: + print(msg) + + for msg in summary.no_person: + print(msg) + + for msg in summary.maybe_person: + print(msg) + + for msg in summary.no_email: + print(msg) diff --git a/ietf/stats/management/commands/repair_meetingregistration_person.py b/ietf/stats/management/commands/repair_meetingregistration_person.py new file mode 100644 index 000000000..450054c09 --- /dev/null +++ b/ietf/stats/management/commands/repair_meetingregistration_person.py @@ -0,0 +1,18 @@ +# Copyright The IETF Trust 2021, All Rights Reserved + +import debug # pyflakes:ignore + +from django.core.management.base import BaseCommand, CommandError + +from ietf.stats.utils import repair_meetingregistration_person + +class Command(BaseCommand): + help = "Repair MeetingRegistration objects that have no person but an email matching a person" + + def add_arguments(self, parser): + parser.add_argument('--meeting',action='append') + + def handle(self, *args, **options): + meetings = options['meeting'] or None + repaired = repair_meetingregistration_person(meetings) + print(f'Repaired {repaired} MeetingRegistration objects') \ No newline at end of file diff --git a/ietf/stats/utils.py b/ietf/stats/utils.py index e91a3d737..704e8a4ef 100644 --- a/ietf/stats/utils.py +++ b/ietf/stats/utils.py @@ -346,47 +346,56 @@ def get_meeting_registration_data(meeting): meeting.save() return num_created, num_processed, num_total -# Might abandon this as too ambitious - if fixed things for 128 people in ietf 100 - 110 though, and that's probably worth doing. -# Yeah - experiments say it _must_ be done. Probably need this as a management command. def repair_meetingregistration_person(meetings=None): - maybe_address = set() - different_person = set() - no_person = set() - maybe_person = set() - no_email = set() - ok_records = 0 - skipped = 0 repaired_records = 0 + qs = MeetingRegistration.objects.all() + if meetings: + qs = qs.filter(meeting__number__in=meetings) + for mr in qs: + if mr.email and not mr.person: + email_person = Person.objects.filter(email__address=mr.email).first() + if email_person: + mr.person = email_person + mr.save() + repaired_records += 1 + return repaired_records +class MeetingRegistrationIssuesSummary(object): + pass -# for mr in MeetingRegistration.objects.exclude(person__email__address=F('email')): - for mr in MeetingRegistration.objects.all(): - if meetings and mr.meeting.number not in meetings: - skipped += 1 - continue +def find_meetingregistration_person_issues(meetings=None): + summary = MeetingRegistrationIssuesSummary() + + summary.could_be_fixed = set() + summary.maybe_address = set() + summary.different_person = set() + summary.no_person = set() + summary.maybe_person = set() + summary.no_email = set() + summary.ok_records = 0 + + qs = MeetingRegistration.objects.all() + if meetings: + qs = qs.filter(meeting__number__in=meetings) + for mr in qs: if mr.person and mr.email and mr.email in mr.person.email_set.values_list('address',flat=True): - ok_records += 1 - continue - if mr.email: + summary.ok_records += 1 + elif mr.email: email_person = Person.objects.filter(email__address=mr.email).first() if mr.person: if not email_person: - maybe_address.add(f'{mr.email} is not present in any Email object. The MeetingRegistration object implies this is an address for {mr.person} ({mr.person.pk})') + summary.maybe_address.add(f'{mr.email} is not present in any Email object. The MeetingRegistration object implies this is an address for {mr.person} ({mr.person.pk})') elif email_person != mr.person: - different_person.add(f'{mr} ({mr.pk}) has person {mr.person} ({mr.person.pk}) but an email {mr.email} attached to a different person {email_person} ({email_person.pk}).') + summary.different_person.add(f'{mr} ({mr.pk}) has person {mr.person} ({mr.person.pk}) but an email {mr.email} attached to a different person {email_person} ({email_person.pk}).') elif email_person: - mr.person = email_person - mr.save() - repaired_records += 1 + summary.could_be_fixed.add(f'{mr} ({mr.pk}) has no person, but email {mr.email} matches {email_person} ({email_person.pk})') else: maybe_person_qs = Person.objects.filter(name__icontains=mr.last_name).filter(name__icontains=mr.first_name) -# if not maybe_person_qs.exists(): -# maybe_person_qs = Person.objects.filter(name__icontains=mr.last_name) - if maybe_person_qs: - maybe_person.add(f'{mr} ({mr.pk}) has email address {mr.email} which cannot be associated with any Person. Consider these possible people {[(p,p.pk) for p in maybe_person_qs]}') + if maybe_person_qs.exists(): + summary.maybe_person.add(f'{mr} ({mr.pk}) has email address {mr.email} which cannot be associated with any Person. Consider these possible people {[(p,p.pk) for p in maybe_person_qs]}') else: - no_person.add(f'{mr} ({mr.pk}) has email address {mr.email} which cannot be associated with any Person') + summary.no_person.add(f'{mr} ({mr.pk}) has email address {mr.email} which cannot be associated with any Person') else: - no_email.add(f'{mr} ({mr.pk}) provides no email address') + summary.no_email.add(f'{mr} ({mr.pk}) provides no email address') - return ok_records, repaired_records, skipped, maybe_address, different_person, maybe_person, no_person, no_email + return summary From fe82f4d6963faa1c4228a6d978835ed859bb0e3e Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Thu, 6 May 2021 19:00:58 +0000 Subject: [PATCH 4/4] connect the new calculations to the UI. Clean flakes. - Legacy-Id: 18974 --- ietf/doc/factories.py | 2 +- ietf/meeting/utils.py | 26 ++------------ ietf/nomcom/utils.py | 14 +++++--- ietf/nomcom/views.py | 34 +++---------------- ietf/person/templatetags/person_filters.py | 4 +-- .../find_meetingregistration_person_issues.py | 2 +- .../repair_meetingregistration_person.py | 2 +- ietf/stats/utils.py | 1 - ietf/templates/nomcom/eligible.html | 3 -- ietf/templates/registration/edit_profile.html | 13 ++++--- 10 files changed, 27 insertions(+), 74 deletions(-) diff --git a/ietf/doc/factories.py b/ietf/doc/factories.py index 9824b5ef4..f3e525d1d 100644 --- a/ietf/doc/factories.py +++ b/ietf/doc/factories.py @@ -13,7 +13,7 @@ from django.conf import settings from ietf.doc.models import ( Document, DocEvent, NewRevisionDocEvent, DocAlias, State, DocumentAuthor, StateDocEvent, BallotPositionDocEvent, BallotDocEvent, BallotType, IRSGBallotDocEvent, TelechatDocEvent, - DocumentActionHolder, DocumentAuthor) + DocumentActionHolder) from ietf.group.models import Group def draft_name_generator(type_id,group,n): diff --git a/ietf/meeting/utils.py b/ietf/meeting/utils.py index 0ff7115ba..471686cfe 100644 --- a/ietf/meeting/utils.py +++ b/ietf/meeting/utils.py @@ -18,12 +18,11 @@ from django.utils.safestring import mark_safe import debug # pyflakes:ignore from ietf.dbtemplate.models import DBTemplate -from ietf.meeting.models import Session, Meeting, SchedulingEvent, TimeSlot, Constraint, SchedTimeSessAssignment -from ietf.group.models import Group, Role +from ietf.meeting.models import Session, SchedulingEvent, TimeSlot, Constraint, SchedTimeSessAssignment +from ietf.group.models import Group from ietf.group.utils import can_manage_materials from ietf.name.models import SessionStatusName, ConstraintName -from ietf.nomcom.utils import DISQUALIFYING_ROLE_QUERY_EXPRESSION -from ietf.person.models import Person, Email +from ietf.person.models import Person from ietf.secr.proceedings.proc_utils import import_audio_files def session_time_for_sorting(session, use_meeting_date): @@ -171,25 +170,6 @@ def finalize(meeting): meeting.save() return -def attended_ietf_meetings(person): - email_addresses = Email.objects.filter(person=person).values_list('address',flat=True) - return Meeting.objects.filter( - type='ietf', - meetingregistration__email__in=email_addresses, - meetingregistration__attended=True, - ) - -def attended_in_last_five_ietf_meetings(person, date=datetime.datetime.today()): - previous_five = Meeting.objects.filter(type='ietf',date__lte=date).order_by('-date')[:5] - attended = attended_ietf_meetings(person) - return set(previous_five).intersection(attended) - -def is_nomcom_eligible(person, date=datetime.date.today()): - attended = attended_in_last_five_ietf_meetings(person, date) - disqualifying_roles = Role.objects.filter(person=person).filter(DISQUALIFYING_ROLE_QUERY_EXPRESSION) - return len(attended)>=3 and not disqualifying_roles.exists() - - def sort_accept_tuple(accept): tup = [] if accept: diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 6472d1325..6a8787d52 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -480,6 +480,10 @@ def create_feedback_email(nomcom, msg): class EncryptedException(Exception): pass +def remove_disqualified(queryset): + disqualified_roles = Role.objects.filter(DISQUALIFYING_ROLE_QUERY_EXPRESSION) + return queryset.exclude(role__in=disqualified_roles) + def is_eligible(person, nomcom=None, date=None): return list_eligible(nomcom=nomcom, date=date, base_qs=Person.objects.filter(pk=person.pk)).exists() @@ -500,13 +504,13 @@ def list_eligible_8713(date, base_qs=None): if not base_qs: base_qs = Person.objects.all() previous_five = previous_five_meetings(date) - return three_of_five_eligible(previous_five=previous_five, queryset=base_qs) + return remove_disqualified(three_of_five_eligible(previous_five=previous_five, queryset=base_qs)) def list_eligible_8788(date, base_qs=None): if not base_qs: base_qs = Person.objects.all() previous_five = Meeting.objects.filter(number__in=['102','103','104','105','106']) - return three_of_five_eligible(previous_five=previous_five, queryset=base_qs) + return remove_disqualified(three_of_five_eligible(previous_five=previous_five, queryset=base_qs)) def list_eligible_8989(date, base_qs=None): if not base_qs: @@ -542,12 +546,12 @@ def list_eligible_8989(date, base_qs=None): document_author_count = Count('documentauthor') ).filter(document_author_count__gte=2) -# return three_of_five_qs.union(officer_qs, author_qs) - return Person.objects.filter(pk__in= + # Would be nice to use queryset union here, but the annotations make that difficult + return remove_disqualified(Person.objects.filter(pk__in= set(three_of_five_qs.values_list('pk',flat=True)).union( set(officer_qs.values_list('pk',flat=True))).union( set(author_qs.values_list('pk',flat=True))) - ) + )) def get_eligibility_date(nomcom=None, date=None): if date: diff --git a/ietf/nomcom/views.py b/ietf/nomcom/views.py index db77086d7..ca1c8eff3 100644 --- a/ietf/nomcom/views.py +++ b/ietf/nomcom/views.py @@ -24,7 +24,6 @@ from ietf.dbtemplate.views import group_template_edit, group_template_show from ietf.name.models import NomineePositionStateName, FeedbackTypeName from ietf.group.models import Group, GroupEvent, Role from ietf.message.models import Message -from ietf.meeting.models import Meeting from ietf.nomcom.decorators import nomcom_private_key_required from ietf.nomcom.forms import (NominateForm, NominateNewPersonForm, FeedbackForm, QuestionnaireForm, @@ -36,12 +35,11 @@ from ietf.nomcom.forms import (NominateForm, NominateNewPersonForm, FeedbackForm from ietf.nomcom.models import (Position, NomineePosition, Nominee, Feedback, NomCom, ReminderDates, FeedbackLastSeen, Topic, TopicFeedbackLastSeen, ) from ietf.nomcom.utils import (get_nomcom_by_year, store_nomcom_private_key, - get_hash_nominee_position, send_reminder_to_nominees, - HOME_TEMPLATE, NOMINEE_ACCEPT_REMINDER_TEMPLATE,NOMINEE_QUESTIONNAIRE_REMINDER_TEMPLATE, - DISQUALIFYING_ROLE_QUERY_EXPRESSION) + get_hash_nominee_position, send_reminder_to_nominees, list_eligible, + HOME_TEMPLATE, NOMINEE_ACCEPT_REMINDER_TEMPLATE,NOMINEE_QUESTIONNAIRE_REMINDER_TEMPLATE, ) + from ietf.ietfauth.utils import role_required from ietf.person.models import Person -from ietf.utils import log from ietf.utils.response import permission_denied import debug # pyflakes:ignore @@ -1275,31 +1273,7 @@ def extract_email_lists(request, year): def eligible(request, year): nomcom = get_nomcom_by_year(year) - # This should probably be refined. If the nomcom year is this year, then - # today's date makes sense; for previous nomcoms, we should probably get - # the date of the announcement of the Call for Volunteers, instead - date = datetime.date.today() - previous_five = ( Meeting.objects.filter(type='ietf',date__lte=date) - .exclude(city='').exclude(city='Virtual') - .order_by('-date')[:5] ) - log.assertion("len(previous_five) == 5") - attendees = {} - potentials = set() - for m in previous_five: - registration_emails = m.meetingregistration_set.filter(attended=True).values_list('email',flat=True) - attendees[m] = Person.objects.filter(email__address__in=registration_emails).distinct() - # See RFC8713 section 4.15 - disqualified_roles = Role.objects.filter(DISQUALIFYING_ROLE_QUERY_EXPRESSION) - potentials.update(attendees[m].exclude(role__in=disqualified_roles)) - eligible_persons = [] - for p in potentials: - count = 0 - for m in previous_five: - if p in attendees[m]: - count += 1 - if count >= 3: - eligible_persons.append(p) - + eligible_persons = list(list_eligible(nomcom=nomcom)) eligible_persons.sort(key=lambda p: p.last_name() ) return render(request, 'nomcom/eligible.html', diff --git a/ietf/person/templatetags/person_filters.py b/ietf/person/templatetags/person_filters.py index 1756ba9e2..5e1ae40ca 100644 --- a/ietf/person/templatetags/person_filters.py +++ b/ietf/person/templatetags/person_filters.py @@ -6,14 +6,14 @@ from django import template import debug # pyflakes:ignore -from ietf.meeting.utils import is_nomcom_eligible as util_is_nomcom_eligible +from ietf.nomcom.utils import is_eligible from ietf.person.models import Alias register = template.Library() @register.filter def is_nomcom_eligible(person, date=datetime.date.today()): - return util_is_nomcom_eligible(person,date) + return is_eligible(person=person,date=date) @register.filter def person_by_name(name): diff --git a/ietf/stats/management/commands/find_meetingregistration_person_issues.py b/ietf/stats/management/commands/find_meetingregistration_person_issues.py index 5dd6919e5..4eaf6ac23 100644 --- a/ietf/stats/management/commands/find_meetingregistration_person_issues.py +++ b/ietf/stats/management/commands/find_meetingregistration_person_issues.py @@ -2,7 +2,7 @@ import debug # pyflakes:ignore -from django.core.management.base import BaseCommand, CommandError +from django.core.management.base import BaseCommand from ietf.stats.utils import find_meetingregistration_person_issues diff --git a/ietf/stats/management/commands/repair_meetingregistration_person.py b/ietf/stats/management/commands/repair_meetingregistration_person.py index 450054c09..74b1a807a 100644 --- a/ietf/stats/management/commands/repair_meetingregistration_person.py +++ b/ietf/stats/management/commands/repair_meetingregistration_person.py @@ -2,7 +2,7 @@ import debug # pyflakes:ignore -from django.core.management.base import BaseCommand, CommandError +from django.core.management.base import BaseCommand from ietf.stats.utils import repair_meetingregistration_person diff --git a/ietf/stats/utils.py b/ietf/stats/utils.py index 704e8a4ef..f5dbbaf7c 100644 --- a/ietf/stats/utils.py +++ b/ietf/stats/utils.py @@ -6,7 +6,6 @@ import re import requests from collections import defaultdict -from django.db.models import F, Q from django.conf import settings from django.contrib.auth.models import User diff --git a/ietf/templates/nomcom/eligible.html b/ietf/templates/nomcom/eligible.html index 679b2cd08..8c82e1f44 100644 --- a/ietf/templates/nomcom/eligible.html +++ b/ietf/templates/nomcom/eligible.html @@ -14,9 +14,6 @@ {% origin %}

Eligible People for {{ nomcom.group }}

-

- This calculation is experimental and is likely wrong. Check carefully against the secretariat eligibility tools if it matters. This page lists people who would be nomcom eligible if the selection were made today. Thus if today is not between the spring and summer IETF meetings, the list won't reflect eligibility at the time actual selections will be made. -

diff --git a/ietf/templates/registration/edit_profile.html b/ietf/templates/registration/edit_profile.html index 9ca92a3e8..09fe7d1f8 100644 --- a/ietf/templates/registration/edit_profile.html +++ b/ietf/templates/registration/edit_profile.html @@ -69,17 +69,16 @@
{{person|is_nomcom_eligible|yesno:'Yes,No,No'}}

- This calculation is EXPERIMENTAL.
- - If you believe it is incorrect, make sure you've added all the + If you believe this calculation is incorrect, make sure you've added all the email addresses you've registered for IETF meetings with to the list below.
If you've done so and the calculation is still incorrect, please send a note to - {{settings.SECRETARIAT_INFO_EMAIL}}.
- See RFC 7437 - for eligibility requirements. + {{settings.SECRETARIAT_SUPPORT_EMAIL}}.
+ See RFC 8713 + for eligibility requirements. + For the 2021 nomcom, see also RFC 8989.

@@ -181,7 +180,7 @@ dagger symbol () next to it, or listed on your notification subscription page. Most of this information can be edited or removed on these pages. There are some exceptions, such - as photos, which currently require an email to the Secretariat + as photos, which currently require an email to the Secretariat if you wish to update or remove the information.

Last Name