diff --git a/bin/daily b/bin/daily index c65ab56043..8a71b695a1 100755 --- a/bin/daily +++ b/bin/daily @@ -34,7 +34,8 @@ $DTDIR/ietf/manage.py update_external_command_info rsync -avzq --delete /a/www/ietf-ftp/iana/yang-parameters/ /a/www/ietf-ftp/yang/ianamod/ # Get Yang models from Yangcatalog. -rsync -avzq rsync://rsync.yangcatalog.org:10873/yangdeps /a/www/ietf-ftp/yang/catalogmod/ +#rsync -avzq rsync://rsync.yangcatalog.org:10873/yangdeps /a/www/ietf-ftp/yang/catalogmod/ +/a/www/ietf-datatracker/scripts/sync_to_yangcatalog # Populate the yang repositories $DTDIR/ietf/manage.py populate_yang_model_dirs -v0 @@ -56,7 +57,7 @@ $DTDIR/ietf/bin/expire-last-calls # Run an extended version of the rfc editor update, to catch changes # with backdated timestamps # Enable when removed from /a/www/ietf-datatracker/scripts/Cron-runner: -$DTDIR/ietf/bin/rfc-editor-index-updates -d 1969-01-01 + $DTDIR/ietf/bin/rfc-editor-index-updates -d 1969-01-01 # Fetch meeting attendance data from ietf.org/registration/attendees $DTDIR/ietf/manage.py fetch_meeting_attendance --latest 2 diff --git a/ietf/api/views.py b/ietf/api/views.py index 73b873f5f3..78e3236842 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -89,7 +89,7 @@ def get(self, request): 'sendqueue', 'nominee', 'topicfeedbacklastseen', 'alias', 'email', 'apikeys', 'personevent', 'reviewersettings', 'reviewsecretarysettings', 'unavailableperiod', 'reviewwish', 'nextreviewerinteam', 'reviewrequest', 'meetingregistration', 'submissionevent', 'preapproval', - 'user', 'user__communitylist', 'personextresource_set', ] + 'user', 'communitylist', 'personextresource_set', ] return self.json_view(request, filter={'id':person.id}, expand=expand) diff --git a/ietf/checks.py b/ietf/checks.py index c823abf118..f9ad87db70 100644 --- a/ietf/checks.py +++ b/ietf/checks.py @@ -28,26 +28,6 @@ def already_ran(): checks_run.append(name) return False -@checks.register('directories') -def check_cdn_directory_exists(app_configs, **kwargs): - """This checks that the path from which the CDN will serve static files for - this version of the datatracker actually exists. In development and test - mode STATIC_ROOT will normally be just static/, but in production it will be - set to a different part of the file system which is served via CDN, and the - path will contain the datatracker release version. - """ - if already_ran(): - return [] - # - errors = [] - if settings.SERVER_MODE == 'production' and not os.path.exists(settings.STATIC_ROOT): - errors.append(checks.Error( - "The static files directory has not been set up.", - hint="Please run 'ietf/manage.py collectstatic'.", - obj=None, - id='datatracker.E001', - )) - return errors @checks.register('files') def check_group_email_aliases_exists(app_configs, **kwargs): diff --git a/ietf/community/admin.py b/ietf/community/admin.py index 890819d9d9..4c947ad3f7 100644 --- a/ietf/community/admin.py +++ b/ietf/community/admin.py @@ -7,8 +7,8 @@ from ietf.community.models import CommunityList, SearchRule, EmailSubscription class CommunityListAdmin(admin.ModelAdmin): - list_display = ['id', 'user', 'group'] - raw_id_fields = ['user', 'group', 'added_docs'] + list_display = ['id', 'person', 'group'] + raw_id_fields = ['person', 'group', 'added_docs'] admin.site.register(CommunityList, CommunityListAdmin) class SearchRuleAdmin(admin.ModelAdmin): diff --git a/ietf/community/forms.py b/ietf/community/forms.py index ad85708968..d3fa01dd19 100644 --- a/ietf/community/forms.py +++ b/ietf/community/forms.py @@ -114,14 +114,13 @@ def clean_text(self): class SubscriptionForm(forms.ModelForm): - def __init__(self, user, clist, *args, **kwargs): + def __init__(self, person, clist, *args, **kwargs): self.clist = clist - self.user = user super(SubscriptionForm, self).__init__(*args, **kwargs) self.fields["notify_on"].widget = forms.RadioSelect(choices=self.fields["notify_on"].choices) - self.fields["email"].queryset = self.fields["email"].queryset.filter(person__user=user, active=True).order_by("-primary") + self.fields["email"].queryset = self.fields["email"].queryset.filter(person=person, active=True).order_by("-primary") self.fields["email"].widget = forms.RadioSelect(choices=[t for t in self.fields["email"].choices if t[0]]) if self.fields["email"].queryset: diff --git a/ietf/community/migrations/0004_delete_useless_community_lists.py b/ietf/community/migrations/0004_delete_useless_community_lists.py new file mode 100644 index 0000000000..9f657a3c34 --- /dev/null +++ b/ietf/community/migrations/0004_delete_useless_community_lists.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.9 on 2024-01-05 21:28 + +from django.db import migrations + + +def forward(apps, schema_editor): + CommunityList = apps.get_model("community", "CommunityList") + # As of 2024-01-05, there are 570 personal CommunityLists with a user + # who has no associated Person. None of these has an EmailSubscription, + # so the lists are doing nothing and can be safely deleted. + personal_lists_no_person = CommunityList.objects.exclude( + user__isnull=True + ).filter( + user__person__isnull=True + ) + # Confirm the assumption that none of the lists to be deleted has an EmailSubscription + assert not personal_lists_no_person.filter(emailsubscription__isnull=False).exists() + personal_lists_no_person.delete() + + +class Migration(migrations.Migration): + dependencies = [ + ("community", "0003_track_rfcs"), + ] + + operations = [migrations.RunPython(forward)] diff --git a/ietf/community/migrations/0005_user_to_person.py b/ietf/community/migrations/0005_user_to_person.py new file mode 100644 index 0000000000..01d8950edb --- /dev/null +++ b/ietf/community/migrations/0005_user_to_person.py @@ -0,0 +1,54 @@ +# Generated by Django 4.2.2 on 2023-06-12 19:35 + +from django.conf import settings +from django.db import migrations +import django.db.models.deletion +import ietf.utils.models + + +def forward(apps, schema_editor): + CommunityList = apps.get_model('community', 'CommunityList') + for clist in CommunityList.objects.all(): + try: + clist.person = clist.user.person + except: + clist.person = None + clist.save() + +def reverse(apps, schema_editor): + CommunityList = apps.get_model('community', 'CommunityList') + for clist in CommunityList.objects.all(): + try: + clist.user = clist.person.user + except: + clist.user = None + clist.save() + +class Migration(migrations.Migration): + dependencies = [ + ("community", "0004_delete_useless_community_lists"), + ("person", "0001_initial"), + ] + + operations = [ + migrations.AddField( + model_name="communitylist", + name="person", + field=ietf.utils.models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to="person.Person", + ), + ), + migrations.RunPython(forward, reverse), + migrations.RemoveField( + model_name="communitylist", + name="user", + field=ietf.utils.models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/ietf/community/models.py b/ietf/community/models.py index 9b2383f211..2e40031768 100644 --- a/ietf/community/models.py +++ b/ietf/community/models.py @@ -2,7 +2,6 @@ # -*- coding: utf-8 -*- -from django.contrib.auth.models import User from django.db import models from django.db.models import signals from django.urls import reverse as urlreverse @@ -13,13 +12,13 @@ from ietf.utils.models import ForeignKey class CommunityList(models.Model): - user = ForeignKey(User, blank=True, null=True) + person = ForeignKey(Person, blank=True, null=True) group = ForeignKey(Group, blank=True, null=True) added_docs = models.ManyToManyField(Document) def long_name(self): - if self.user: - return 'Personal I-D list of %s' % self.user.username + if self.person: + return 'Personal I-D list of %s' % self.person.plain_name() elif self.group: return 'I-D list for %s' % self.group.name else: @@ -30,8 +29,8 @@ def __str__(self): def get_absolute_url(self): import ietf.community.views - if self.user: - return urlreverse(ietf.community.views.view_list, kwargs={ 'username': self.user.username }) + if self.person: + return urlreverse(ietf.community.views.view_list, kwargs={ 'email_or_name': self.person.email() }) elif self.group: return urlreverse("ietf.group.views.group_documents", kwargs={ 'acronym': self.group.acronym }) return "" diff --git a/ietf/community/tests.py b/ietf/community/tests.py index ee5827ee25..387877887f 100644 --- a/ietf/community/tests.py +++ b/ietf/community/tests.py @@ -1,13 +1,10 @@ -# Copyright The IETF Trust 2016-2020, All Rights Reserved +# Copyright The IETF Trust 2016-2023, All Rights Reserved # -*- coding: utf-8 -*- from pyquery import PyQuery from django.urls import reverse as urlreverse -from django.contrib.auth.models import User - -from django_webtest import WebTest import debug # pyflakes:ignore @@ -19,14 +16,14 @@ from ietf.group.utils import setup_default_community_list_for_group from ietf.doc.models import State from ietf.doc.utils import add_state_change_event -from ietf.person.models import Person, Email -from ietf.utils.test_utils import login_testing_unauthorized +from ietf.person.models import Person, Email, Alias +from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.mail import outbox from ietf.doc.factories import WgDraftFactory from ietf.group.factories import GroupFactory, RoleFactory -from ietf.person.factories import PersonFactory +from ietf.person.factories import PersonFactory, EmailFactory, AliasFactory -class CommunityListTests(WebTest): +class CommunityListTests(TestCase): def test_rule_matching(self): plain = PersonFactory(user__username='plain') ad = Person.objects.get(user__username='ad') @@ -38,7 +35,7 @@ def test_rule_matching(self): states=[('draft-iesg','lc'),('draft','active')], ) - clist = CommunityList.objects.create(user=User.objects.get(username="plain")) + clist = CommunityList.objects.create(person=plain) rule_group = SearchRule.objects.create(rule_type="group", group=draft.group, state=State.objects.get(type="draft", slug="active"), community_list=clist) rule_group_rfc = SearchRule.objects.create(rule_type="group_rfc", group=draft.group, state=State.objects.get(type="rfc", slug="published"), community_list=clist) @@ -89,18 +86,38 @@ def test_rule_matching(self): # rule -> docs self.assertTrue(draft in list(docs_matching_community_list_rule(rule_group_exp))) + def test_view_list_duplicates(self): + person = PersonFactory(name="John Q. Public", user__username="bazquux@example.com") + PersonFactory(name="John Q. Public", user__username="foobar@example.com") + + url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": person.plain_name()}) + r = self.client.get(url) + self.assertEqual(r.status_code, 300) + self.assertIn("bazquux@example.com", r.content.decode()) + self.assertIn("foobar@example.com", r.content.decode()) + + def complex_person(self, *args, **kwargs): + person = PersonFactory(*args, **kwargs) + EmailFactory(person=person) + AliasFactory(person=person) + return person + + def email_or_name_set(self, person): + return [e for e in Email.objects.filter(person=person)] + \ + [a for a in Alias.objects.filter(person=person)] + def test_view_list(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.view_list, kwargs={ "username": "plain" }) - # without list - r = self.client.get(url) - self.assertEqual(r.status_code, 200) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": id }) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") # with list - clist = CommunityList.objects.create(user=User.objects.get(username="plain")) + clist = CommunityList.objects.create(person=person) if not draft in clist.added_docs.all(): clist.added_docs.add(draft) SearchRule.objects.create( @@ -109,80 +126,87 @@ def test_view_list(self): state=State.objects.get(type="draft", slug="active"), text="test", ) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - self.assertContains(r, draft.name) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": id }) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertContains(r, draft.name) def test_manage_personal_list(self): - - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') ad = Person.objects.get(user__username='ad') draft = WgDraftFactory(authors=[ad]) - url = urlreverse(ietf.community.views.manage_list, kwargs={ "username": "plain" }) + url = urlreverse(ietf.community.views.manage_list, kwargs={ "email_or_name": person.email() }) login_testing_unauthorized(self, "plain", url) - page = self.app.get(url, user='plain') - self.assertEqual(page.status_int, 200) - - # add document - self.assertIn('add_document', page.forms) - form = page.forms['add_document'] - form['documents'].options=[(draft.pk, True, draft.name)] - page = form.submit('action',value='add_documents') - self.assertEqual(page.status_int, 302) - clist = CommunityList.objects.get(user__username="plain") - self.assertTrue(clist.added_docs.filter(pk=draft.pk)) - page = page.follow() - - self.assertContains(page, draft.name) - - # remove document - self.assertIn('remove_document_%s' % draft.pk, page.forms) - form = page.forms['remove_document_%s' % draft.pk] - page = form.submit('action',value='remove_document') - self.assertEqual(page.status_int, 302) - clist = CommunityList.objects.get(user__username="plain") - self.assertTrue(not clist.added_docs.filter(pk=draft.pk)) - page = page.follow() - - # add rule - r = self.client.post(url, { - "action": "add_rule", - "rule_type": "author_rfc", - "author_rfc-person": Person.objects.filter(documentauthor__document=draft).first().pk, + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.manage_list, kwargs={ "email_or_name": id }) + r = self.client.get(url, user='plain') + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + + # We can't call post() with follow=True because that 404's if + # the url contains unicode, because the django test client + # apparently re-encodes the already-encoded url. + def follow(r): + redirect_url = r.url or url + return self.client.get(redirect_url, user='plain') + + # add document + self.assertContains(r, 'add_document') + r = self.client.post(url, {'action': 'add_documents', 'documents': draft.pk}) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(clist.added_docs.filter(pk=draft.pk)) + r = follow(r) + self.assertContains(r, draft.name, status_code=200) + + # remove document + self.assertContains(r, 'remove_document_%s' % draft.pk) + r = self.client.post(url, {'action': 'remove_document', 'document': draft.pk}) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(not clist.added_docs.filter(pk=draft.pk)) + r = follow(r) + self.assertNotContains(r, draft.name, status_code=200) + + # add rule + r = self.client.post(url, { + "action": "add_rule", + "rule_type": "author_rfc", + "author_rfc-person": Person.objects.filter(documentauthor__document=draft).first().pk, "author_rfc-state": State.objects.get(type="rfc", slug="published").pk, - }) - self.assertEqual(r.status_code, 302) - clist = CommunityList.objects.get(user__username="plain") - self.assertTrue(clist.searchrule_set.filter(rule_type="author_rfc")) - - # add name_contains rule - r = self.client.post(url, { - "action": "add_rule", - "rule_type": "name_contains", - "name_contains-text": "draft.*mars", - "name_contains-state": State.objects.get(type="draft", slug="active").pk, - }) - self.assertEqual(r.status_code, 302) - clist = CommunityList.objects.get(user__username="plain") - self.assertTrue(clist.searchrule_set.filter(rule_type="name_contains")) - - # rule shows up on GET - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - rule = clist.searchrule_set.filter(rule_type="author_rfc").first() - q = PyQuery(r.content) - self.assertEqual(len(q('#r%s' % rule.pk)), 1) + }) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(clist.searchrule_set.filter(rule_type="author_rfc")) + + # add name_contains rule + r = self.client.post(url, { + "action": "add_rule", + "rule_type": "name_contains", + "name_contains-text": "draft.*mars", + "name_contains-state": State.objects.get(type="draft", slug="active").pk, + }) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(clist.searchrule_set.filter(rule_type="name_contains")) + + # rule shows up on GET + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + rule = clist.searchrule_set.filter(rule_type="author_rfc").first() + q = PyQuery(r.content) + self.assertEqual(len(q('#r%s' % rule.pk)), 1) - # remove rule - r = self.client.post(url, { - "action": "remove_rule", - "rule": rule.pk, - }) + # remove rule + r = self.client.post(url, { + "action": "remove_rule", + "rule": rule.pk, + }) - clist = CommunityList.objects.get(user__username="plain") - self.assertTrue(not clist.searchrule_set.filter(rule_type="author_rfc")) + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(not clist.searchrule_set.filter(rule_type="author_rfc")) def test_manage_group_list(self): draft = WgDraftFactory(group__acronym='mars') @@ -210,77 +234,84 @@ def test_manage_group_list(self): self.assertEqual(r.status_code, 200) def test_track_untrack_document(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.track_document, kwargs={ "username": "plain", "name": draft.name }) + url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": person.email(), "name": draft.name }) login_testing_unauthorized(self, "plain", url) - # track - r = self.client.get(url) - self.assertEqual(r.status_code, 200) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": id, "name": draft.name }) - r = self.client.post(url) - self.assertEqual(r.status_code, 302) - clist = CommunityList.objects.get(user__username="plain") - self.assertEqual(list(clist.added_docs.all()), [draft]) + # track + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") - # untrack - url = urlreverse(ietf.community.views.untrack_document, kwargs={ "username": "plain", "name": draft.name }) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) + r = self.client.post(url) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertEqual(list(clist.added_docs.all()), [draft]) + + # untrack + url = urlreverse(ietf.community.views.untrack_document, kwargs={ "email_or_name": id, "name": draft.name }) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") - r = self.client.post(url) - self.assertEqual(r.status_code, 302) - clist = CommunityList.objects.get(user__username="plain") - self.assertEqual(list(clist.added_docs.all()), []) + r = self.client.post(url) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertEqual(list(clist.added_docs.all()), []) def test_track_untrack_document_through_ajax(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.track_document, kwargs={ "username": "plain", "name": draft.name }) + url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": person.email(), "name": draft.name }) login_testing_unauthorized(self, "plain", url) - # track - r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') - self.assertEqual(r.status_code, 200) - self.assertEqual(r.json()["success"], True) - clist = CommunityList.objects.get(user__username="plain") - self.assertEqual(list(clist.added_docs.all()), [draft]) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": id, "name": draft.name }) - # untrack - url = urlreverse(ietf.community.views.untrack_document, kwargs={ "username": "plain", "name": draft.name }) - r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') - self.assertEqual(r.status_code, 200) - self.assertEqual(r.json()["success"], True) - clist = CommunityList.objects.get(user__username="plain") - self.assertEqual(list(clist.added_docs.all()), []) + # track + r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertEqual(r.json()["success"], True) + clist = CommunityList.objects.get(person__user__username="plain") + self.assertEqual(list(clist.added_docs.all()), [draft]) + + # untrack + url = urlreverse(ietf.community.views.untrack_document, kwargs={ "email_or_name": id, "name": draft.name }) + r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertEqual(r.json()["success"], True) + clist = CommunityList.objects.get(person__user__username="plain") + self.assertEqual(list(clist.added_docs.all()), []) def test_csv(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.export_to_csv, kwargs={ "username": "plain" }) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.export_to_csv, kwargs={ "email_or_name": id }) - # without list - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - - # with list - clist = CommunityList.objects.create(user=User.objects.get(username="plain")) - if not draft in clist.added_docs.all(): - clist.added_docs.add(draft) - SearchRule.objects.create( - community_list=clist, - rule_type="name_contains", - state=State.objects.get(type="draft", slug="active"), - text="test", - ) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - # this is a simple-minded test, we don't actually check the fields - self.assertContains(r, draft.name) + # without list + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + + # with list + clist = CommunityList.objects.create(person=person) + if not draft in clist.added_docs.all(): + clist.added_docs.add(draft) + SearchRule.objects.create( + community_list=clist, + rule_type="name_contains", + state=State.objects.get(type="draft", slug="active"), + text="test", + ) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + # this is a simple-minded test, we don't actually check the fields + self.assertContains(r, draft.name) def test_csv_for_group(self): draft = WgDraftFactory() @@ -294,33 +325,34 @@ def test_csv_for_group(self): self.assertEqual(r.status_code, 200) def test_feed(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.feed, kwargs={ "username": "plain" }) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.feed, kwargs={ "email_or_name": id }) - # without list - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - - # with list - clist = CommunityList.objects.create(user=User.objects.get(username="plain")) - if not draft in clist.added_docs.all(): - clist.added_docs.add(draft) - SearchRule.objects.create( - community_list=clist, - rule_type="name_contains", - state=State.objects.get(type="draft", slug="active"), - text="test", - ) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - self.assertContains(r, draft.name) + # without list + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + + # with list + clist = CommunityList.objects.create(person=person) + if not draft in clist.added_docs.all(): + clist.added_docs.add(draft) + SearchRule.objects.create( + community_list=clist, + rule_type="name_contains", + state=State.objects.get(type="draft", slug="active"), + text="test", + ) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertContains(r, draft.name) - # only significant - r = self.client.get(url + "?significant=1") - self.assertEqual(r.status_code, 200) - self.assertNotContains(r, '') + # only significant + r = self.client.get(url + "?significant=1") + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertNotContains(r, '') def test_feed_for_group(self): draft = WgDraftFactory() @@ -334,19 +366,21 @@ def test_feed_for_group(self): self.assertEqual(r.status_code, 200) def test_subscription(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.subscription, kwargs={ "username": "plain" }) - + url = urlreverse(ietf.community.views.subscription, kwargs={ "email_or_name": person.email() }) login_testing_unauthorized(self, "plain", url) - # subscription without list - r = self.client.get(url) - self.assertEqual(r.status_code, 404) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.subscription, kwargs={ "email_or_name": id }) + + # subscription without list + r = self.client.get(url) + self.assertEqual(r.status_code, 404, msg=f"id='{id}', url='{url}'") # subscription with list - clist = CommunityList.objects.create(user=User.objects.get(username="plain")) + clist = CommunityList.objects.create(person=person) if not draft in clist.added_docs.all(): clist.added_docs.add(draft) SearchRule.objects.create( @@ -355,22 +389,25 @@ def test_subscription(self): state=State.objects.get(type="draft", slug="active"), text="test", ) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - # subscribe - email = Email.objects.filter(person__user__username="plain").first() - r = self.client.post(url, { "email": email.pk, "notify_on": "significant", "action": "subscribe" }) - self.assertEqual(r.status_code, 302) + for email in Email.objects.filter(person=person): + url = urlreverse(ietf.community.views.subscription, kwargs={ "email_or_name": email }) - subscription = EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").first() + r = self.client.get(url) + self.assertEqual(r.status_code, 200) - self.assertTrue(subscription) + # subscribe + r = self.client.post(url, { "email": email.pk, "notify_on": "significant", "action": "subscribe" }) + self.assertEqual(r.status_code, 302) - # delete subscription - r = self.client.post(url, { "subscription_id": subscription.pk, "action": "unsubscribe" }) - self.assertEqual(r.status_code, 302) - self.assertEqual(EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").count(), 0) + subscription = EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").first() + + self.assertTrue(subscription) + + # delete subscription + r = self.client.post(url, { "subscription_id": subscription.pk, "action": "unsubscribe" }) + self.assertEqual(r.status_code, 302) + self.assertEqual(EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").count(), 0) def test_subscription_for_group(self): draft = WgDraftFactory(group__acronym='mars') @@ -385,12 +422,12 @@ def test_subscription_for_group(self): # test GET, rest is tested with personal list r = self.client.get(url) self.assertEqual(r.status_code, 200) - + def test_notification(self): - PersonFactory(user__username='plain') + person = PersonFactory(user__username='plain') draft = WgDraftFactory() - clist = CommunityList.objects.create(user=User.objects.get(username="plain")) + clist = CommunityList.objects.create(person=person) if not draft in clist.added_docs.all(): clist.added_docs.add(draft) diff --git a/ietf/community/urls.py b/ietf/community/urls.py index f80547ffad..3ab132f2dc 100644 --- a/ietf/community/urls.py +++ b/ietf/community/urls.py @@ -4,11 +4,11 @@ from ietf.utils.urls import url urlpatterns = [ - url(r'^personal/(?P[^/]+)/$', views.view_list), - url(r'^personal/(?P[^/]+)/manage/$', views.manage_list), - url(r'^personal/(?P[^/]+)/trackdocument/(?P[^/]+)/$', views.track_document), - url(r'^personal/(?P[^/]+)/untrackdocument/(?P[^/]+)/$', views.untrack_document), - url(r'^personal/(?P[^/]+)/csv/$', views.export_to_csv), - url(r'^personal/(?P[^/]+)/feed/$', views.feed), - url(r'^personal/(?P[^/]+)/subscription/$', views.subscription), + url(r'^personal/(?P[^/]+)/$', views.view_list), + url(r'^personal/(?P[^/]+)/manage/$', views.manage_list), + url(r'^personal/(?P[^/]+)/trackdocument/(?P[^/]+)/$', views.track_document), + url(r'^personal/(?P[^/]+)/untrackdocument/(?P[^/]+)/$', views.untrack_document), + url(r'^personal/(?P[^/]+)/csv/$', views.export_to_csv), + url(r'^personal/(?P[^/]+)/feed/$', views.feed), + url(r'^personal/(?P[^/]+)/subscription/$', views.subscription), ] diff --git a/ietf/community/utils.py b/ietf/community/utils.py index f411af6a5f..f23e8d26ab 100644 --- a/ietf/community/utils.py +++ b/ietf/community/utils.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2016-2020, All Rights Reserved +# Copyright The IETF Trust 2016-2023, All Rights Reserved # -*- coding: utf-8 -*- @@ -11,11 +11,9 @@ from ietf.community.models import CommunityList, EmailSubscription, SearchRule from ietf.doc.models import Document, State -from ietf.group.models import Role, Group +from ietf.group.models import Role from ietf.person.models import Person from ietf.ietfauth.utils import has_role -from django.contrib.auth.models import User -from django.shortcuts import get_object_or_404 from ietf.utils.mail import send_mail @@ -29,24 +27,12 @@ def states_of_significant_change(): Q(type="draft", slug__in=['rfc', 'dead']) ) -def lookup_community_list(username=None, acronym=None): - assert username or acronym - - if acronym: - group = get_object_or_404(Group, acronym=acronym) - clist = CommunityList.objects.filter(group=group).first() or CommunityList(group=group) - else: - user = get_object_or_404(User, username__iexact=username) - clist = CommunityList.objects.filter(user=user).first() or CommunityList(user=user) - - return clist - def can_manage_community_list(user, clist): if not user or not user.is_authenticated: return False - if clist.user: - return user == clist.user + if clist.person: + return user == clist.person.user elif clist.group: if has_role(user, 'Secretariat'): return True diff --git a/ietf/community/views.py b/ietf/community/views.py index fdaaffec0b..a9d3bd5845 100644 --- a/ietf/community/views.py +++ b/ietf/community/views.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2012-2020, All Rights Reserved +# Copyright The IETF Trust 2012-2023, All Rights Reserved # -*- coding: utf-8 -*- @@ -15,18 +15,46 @@ import debug # pyflakes:ignore -from ietf.community.models import SearchRule, EmailSubscription +from ietf.community.models import CommunityList, EmailSubscription, SearchRule from ietf.community.forms import SearchRuleTypeForm, SearchRuleForm, AddDocumentsForm, SubscriptionForm -from ietf.community.utils import lookup_community_list, can_manage_community_list +from ietf.community.utils import can_manage_community_list from ietf.community.utils import docs_tracked_by_community_list, docs_matching_community_list_rule from ietf.community.utils import states_of_significant_change, reset_name_contains_index_for_rule +from ietf.group.models import Group from ietf.doc.models import DocEvent, Document from ietf.doc.utils_search import prepare_document_table +from ietf.person.utils import lookup_persons from ietf.utils.http import is_ajax from ietf.utils.response import permission_denied -def view_list(request, username=None): - clist = lookup_community_list(username) +class MultiplePersonError(Exception): + """More than one Person record matches the given email or name""" + pass + +def lookup_community_list(request, email_or_name=None, acronym=None): + assert email_or_name or acronym + + if acronym: + group = get_object_or_404(Group, acronym=acronym) + clist = CommunityList.objects.filter(group=group).first() or CommunityList(group=group) + else: + persons = lookup_persons(email_or_name) + if len(persons) > 1: + if hasattr(request.user, 'person') and request.user.person in persons: + person = request.user.person + else: + raise MultiplePersonError("\r\n".join([p.user.username for p in persons])) + else: + person = persons[0] + clist = CommunityList.objects.filter(person=person).first() or CommunityList(person=person) + + return clist + +def view_list(request, email_or_name=None): + try: + clist = lookup_community_list(request, email_or_name) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) docs = docs_tracked_by_community_list(clist) docs, meta = prepare_document_table(request, docs, request.GET) @@ -42,10 +70,13 @@ def view_list(request, username=None): }) @login_required -def manage_list(request, username=None, acronym=None, group_type=None): +def manage_list(request, email_or_name=None, acronym=None): # we need to be a bit careful because clist may not exist in the # database so we can't call related stuff on it yet - clist = lookup_community_list(username, acronym) + try: + clist = lookup_community_list(request, email_or_name, acronym) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) if not can_manage_community_list(request.user, clist): permission_denied(request, "You do not have permission to access this view") @@ -128,11 +159,14 @@ def manage_list(request, username=None, acronym=None, group_type=None): @login_required -def track_document(request, name, username=None, acronym=None): +def track_document(request, name, email_or_name=None, acronym=None): doc = get_object_or_404(Document, name=name) if request.method == "POST": - clist = lookup_community_list(username, acronym) + try: + clist = lookup_community_list(request, email_or_name, acronym) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) if not can_manage_community_list(request.user, clist): permission_denied(request, "You do not have permission to access this view") @@ -152,9 +186,12 @@ def track_document(request, name, username=None, acronym=None): }) @login_required -def untrack_document(request, name, username=None, acronym=None): +def untrack_document(request, name, email_or_name=None, acronym=None): doc = get_object_or_404(Document, name=name) - clist = lookup_community_list(username, acronym) + try: + clist = lookup_community_list(request, email_or_name, acronym) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) if not can_manage_community_list(request.user, clist): permission_denied(request, "You do not have permission to access this view") @@ -172,8 +209,11 @@ def untrack_document(request, name, username=None, acronym=None): }) -def export_to_csv(request, username=None, acronym=None, group_type=None): - clist = lookup_community_list(username, acronym) +def export_to_csv(request, email_or_name=None, acronym=None): + try: + clist = lookup_community_list(request, email_or_name, acronym) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) response = HttpResponse(content_type='text/csv') @@ -213,8 +253,11 @@ def export_to_csv(request, username=None, acronym=None, group_type=None): return response -def feed(request, username=None, acronym=None, group_type=None): - clist = lookup_community_list(username, acronym) +def feed(request, email_or_name=None, acronym=None): + try: + clist = lookup_community_list(request, email_or_name, acronym) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) significant = request.GET.get('significant', '') == '1' @@ -249,17 +292,22 @@ def feed(request, username=None, acronym=None, group_type=None): @login_required -def subscription(request, username=None, acronym=None, group_type=None): - clist = lookup_community_list(username, acronym) - if clist.pk is None: - raise Http404 +def subscription(request, email_or_name=None, acronym=None): + try: + clist = lookup_community_list(request, email_or_name, acronym) + if clist.pk is None: + raise Http404 + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) + + person = request.user.person - existing_subscriptions = EmailSubscription.objects.filter(community_list=clist, email__person__user=request.user) + existing_subscriptions = EmailSubscription.objects.filter(community_list=clist, email__person=person) if request.method == 'POST': action = request.POST.get("action") if action == "subscribe": - form = SubscriptionForm(request.user, clist, request.POST) + form = SubscriptionForm(person, clist, request.POST) if form.is_valid(): subscription = form.save(commit=False) subscription.community_list = clist @@ -272,7 +320,7 @@ def subscription(request, username=None, acronym=None, group_type=None): return HttpResponseRedirect("") else: - form = SubscriptionForm(request.user, clist) + form = SubscriptionForm(person, clist) return render(request, 'community/subscription.html', { 'clist': clist, diff --git a/ietf/doc/management/commands/reset_rfc_authors.py b/ietf/doc/management/commands/reset_rfc_authors.py new file mode 100644 index 0000000000..e2ab5f1208 --- /dev/null +++ b/ietf/doc/management/commands/reset_rfc_authors.py @@ -0,0 +1,69 @@ +# Copyright The IETF Trust 2024, All Rights Reserved + +# Reset an RFC's authors to those of the draft it came from +from django.core.management.base import BaseCommand, CommandError + +from ietf.doc.models import Document, DocEvent +from ietf.person.models import Person + + +class Command(BaseCommand): + def add_arguments(self, parser): + parser.add_argument("rfcnum", type=int, help="RFC number to modify") + parser.add_argument( + "--force", + action="store_true", + help="reset even if RFC already has authors", + ) + + def handle(self, *args, **options): + try: + rfc = Document.objects.get(type="rfc", rfc_number=options["rfcnum"]) + except Document.DoesNotExist: + raise CommandError( + f"rfc{options['rfcnum']} does not exist in the Datatracker." + ) + + draft = rfc.came_from_draft() + if draft is None: + raise CommandError(f"{rfc.name} did not come from a draft. Can't reset.") + + orig_authors = rfc.documentauthor_set.all() + if orig_authors.exists(): + # Potentially dangerous, so refuse unless "--force" is specified + if not options["force"]: + raise CommandError( + f"{rfc.name} already has authors. Not resetting. Use '--force' to reset anyway." + ) + removed_auth_names = list(orig_authors.values_list("person__name", flat=True)) + rfc.documentauthor_set.all().delete() + DocEvent.objects.create( + doc=rfc, + by=Person.objects.get(name="(System)"), + type="edited_authors", + desc=f"Removed all authors: {', '.join(removed_auth_names)}", + ) + self.stdout.write( + self.style.SUCCESS( + f"Removed author(s): {', '.join(removed_auth_names)}" + ) + ) + + for author in draft.documentauthor_set.all(): + # Copy the author but point at the new doc. + # See https://docs.djangoproject.com/en/4.2/topics/db/queries/#copying-model-instances + author.pk = None + author.id = None + author._state.adding = True + author.document = rfc + author.save() + self.stdout.write( + self.style.SUCCESS(f"Added author {author.person.name} <{author.email}>") + ) + auth_names = draft.documentauthor_set.values_list("person__name", flat=True) + DocEvent.objects.create( + doc=rfc, + by=Person.objects.get(name="(System)"), + type="edited_authors", + desc=f"Set authors from rev {draft.rev} of {draft.name}: {', '.join(auth_names)}", + ) diff --git a/ietf/doc/management/commands/tests.py b/ietf/doc/management/commands/tests.py new file mode 100644 index 0000000000..8244d87266 --- /dev/null +++ b/ietf/doc/management/commands/tests.py @@ -0,0 +1,72 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# -*- coding: utf-8 -*- + +from io import StringIO + +from django.core.management import call_command, CommandError + +from ietf.doc.factories import DocumentAuthorFactory, WgDraftFactory, WgRfcFactory +from ietf.doc.models import Document, DocumentAuthor +from ietf.utils.test_utils import TestCase + + +class CommandTests(TestCase): + @staticmethod + def _call_command(command_name, *args, **options): + """Call command, capturing (and suppressing) output""" + out = StringIO() + err = StringIO() + options["stdout"] = out + options["stderr"] = err + call_command(command_name, *args, **options) + return out.getvalue(), err.getvalue() + + def test_reset_rfc_authors(self): + command_name = "reset_rfc_authors" + + draft = WgDraftFactory() + DocumentAuthorFactory.create_batch(3, document=draft) + rfc = WgRfcFactory() # rfc does not yet have a draft + DocumentAuthorFactory.create_batch(3, document=rfc) + bad_rfc_num = ( + 1 + + Document.objects.filter(rfc_number__isnull=False) + .order_by("-rfc_number") + .first() + .rfc_number + ) + docauthor_fields = [ + field.name + for field in DocumentAuthor._meta.get_fields() + if field.name not in ["document", "id"] + ] + + with self.assertRaises(CommandError, msg="Cannot reset a bad RFC number"): + self._call_command(command_name, bad_rfc_num) + + with self.assertRaises(CommandError, msg="Cannot reset an RFC with no draft"): + self._call_command(command_name, rfc.rfc_number) + + with self.assertRaises(CommandError, msg="Cannot force-reset an RFC with no draft"): + self._call_command(command_name, rfc.rfc_number, "--force") + + # Link the draft to the rfc + rfc.targets_related.create(relationship_id="became_rfc", source=draft) + + with self.assertRaises(CommandError, msg="Cannot reset an RFC with authors"): + self._call_command(command_name, rfc.rfc_number) + + # Calling with force should work + self._call_command(command_name, rfc.rfc_number, "--force") + self.assertCountEqual( + draft.documentauthor_set.values(*docauthor_fields), + rfc.documentauthor_set.values(*docauthor_fields), + ) + + # Calling on an RFC with no authors should also work + rfc.documentauthor_set.all().delete() + self._call_command(command_name, rfc.rfc_number) + self.assertCountEqual( + draft.documentauthor_set.values(*docauthor_fields), + rfc.documentauthor_set.values(*docauthor_fields), + ) diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index d93c9f4531..46ecccc314 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -1081,29 +1081,26 @@ def build_file_urls(doc: Union[Document, DocHistory]): return file_urls, found_types -def augment_docs_and_user_with_user_info(docs, user): +def augment_docs_and_person_with_person_info(docs, person): """Add attribute to each document with whether the document is tracked - or has a review wish by the user or not, and the review teams the user is on.""" + or has a review wish by the person or not, and the review teams the person is on.""" tracked = set() review_wished = set() - - if user and user.is_authenticated: - user.review_teams = Group.objects.filter( - reviewteamsettings__isnull=False, role__person__user=user, role__name='reviewer') - doc_pks = [d.pk for d in docs] - clist = CommunityList.objects.filter(user=user).first() - if clist: - tracked.update( - docs_tracked_by_community_list(clist).filter(pk__in=doc_pks).values_list("pk", flat=True)) + # used in templates + person.review_teams = Group.objects.filter( + reviewteamsettings__isnull=False, role__person=person, role__name='reviewer') - try: - wishes = ReviewWish.objects.filter(person=Person.objects.get(user=user)) - wishes = wishes.filter(doc__pk__in=doc_pks).values_list("doc__pk", flat=True) - review_wished.update(wishes) - except Person.DoesNotExist: - pass + doc_pks = [d.pk for d in docs] + clist = CommunityList.objects.filter(person=person).first() + if clist: + tracked.update( + docs_tracked_by_community_list(clist).filter(pk__in=doc_pks).values_list("pk", flat=True)) + + wishes = ReviewWish.objects.filter(person=person) + wishes = wishes.filter(doc__pk__in=doc_pks).values_list("doc__pk", flat=True) + review_wished.update(wishes) for d in docs: d.tracked_in_personal_community_list = d.pk in tracked diff --git a/ietf/doc/utils_search.py b/ietf/doc/utils_search.py index 8e89139e31..59b64ad307 100644 --- a/ietf/doc/utils_search.py +++ b/ietf/doc/utils_search.py @@ -11,7 +11,7 @@ from ietf.doc.models import Document, RelatedDocument, DocEvent, TelechatDocEvent, BallotDocEvent, DocTypeName from ietf.doc.expire import expirable_drafts -from ietf.doc.utils import augment_docs_and_user_with_user_info +from ietf.doc.utils import augment_docs_and_person_with_person_info from ietf.meeting.models import SessionPresentation, Meeting, Session from ietf.review.utils import review_assignments_to_list_for_docs from ietf.utils.timezone import date_today @@ -199,7 +199,8 @@ def prepare_document_table(request, docs, query=None, max_results=200, show_ad_a docs = docs[:max_results] fill_in_document_table_attributes(docs) - augment_docs_and_user_with_user_info(docs, request.user) + if request.user.is_authenticated and hasattr(request.user, "person"): + augment_docs_and_person_with_person_info(docs, request.user.person) augment_docs_with_related_docs_info(docs) meta = {} diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 293d32daff..64f9de4f58 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -62,7 +62,7 @@ needed_ballot_positions, nice_consensus, update_telechat, has_same_ballot, get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, add_events_message_info, get_unicode_document_content, - augment_docs_and_user_with_user_info, irsg_needed_ballot_positions, add_action_holder_change_event, + augment_docs_and_person_with_person_info, irsg_needed_ballot_positions, add_action_holder_change_event, build_file_urls, update_documentauthors, fuzzy_find_documents, bibxml_for_draft) from ietf.doc.utils_bofreq import bofreq_editors, bofreq_responsible @@ -287,7 +287,8 @@ def document_main(request, name, rev=None, document_html=False): presentations = doc.future_presentations() - augment_docs_and_user_with_user_info([doc], request.user) + if request.user.is_authenticated and hasattr(request.user, "person"): + augment_docs_and_person_with_person_info([doc], request.user.person) exp_comment = doc.latest_event(IanaExpertDocEvent,type="comment") iana_experts_comment = exp_comment and exp_comment.desc @@ -580,7 +581,8 @@ def document_main(request, name, rev=None, document_html=False): elif can_edit_stream_info and (iesg_state_slug in ('idexists','watching')): actions.append(("Submit to IESG for Publication", urlreverse('ietf.doc.views_draft.to_iesg', kwargs=dict(name=doc.name)))) - augment_docs_and_user_with_user_info([doc], request.user) + if request.user.is_authenticated and hasattr(request.user, "person"): + augment_docs_and_person_with_person_info([doc], request.user.person) published = doc.latest_event(type="published_rfc") # todo rethink this now that published_rfc is on rfc started_iesg_process = doc.latest_event(type="started_iesg_process") diff --git a/ietf/meeting/models.py b/ietf/meeting/models.py index 11a9982283..de9613de10 100644 --- a/ietf/meeting/models.py +++ b/ietf/meeting/models.py @@ -366,10 +366,6 @@ def vtimezone(self): pass return None - def set_official_schedule(self, schedule): - if self.schedule != schedule: - self.schedule = schedule - self.save() def updated(self): # should be Meeting.modified, but we don't have that @@ -457,22 +453,6 @@ class Room(models.Model): def __str__(self): return u"%s size: %s" % (self.name, self.capacity) - def delete_timeslots(self): - for ts in self.timeslot_set.all(): - ts.sessionassignments.all().delete() - ts.delete() - - def create_timeslots(self): - days, time_slices, slots = self.meeting.build_timeslices() - for day in days: - for ts in slots[day]: - TimeSlot.objects.create(type_id=ts.type_id, - meeting=self.meeting, - name=ts.name, - time=ts.time, - location=self, - duration=ts.duration) - #self.meeting.create_all_timeslots() def dom_id(self): return "room%u" % (self.pk) @@ -496,14 +476,6 @@ def right(self): return max(self.x1, self.x2) if (self.x1 and self.x2) else 0 def bottom(self): return max(self.y1, self.y2) if (self.y1 and self.y2) else 0 - def functional_display_name(self): - if not self.functional_name: - return "" - if 'breakout' in self.functional_name.lower(): - return "" - if self.functional_name[0].isdigit(): - return "" - return self.functional_name # audio stream support def audio_stream_url(self): urlresources = [ur for ur in self.urlresource_set.all() if ur.name_id == 'audiostream'] @@ -775,9 +747,6 @@ def official_token(self): else: return "unofficial" - def delete_assignments(self): - self.assignments.all().delete() - @property def qs_assignments_with_sessions(self): return self.assignments.filter(session__isnull=False) @@ -790,10 +759,6 @@ def qs_sessions_scheduled(self): """Get QuerySet containing sessions assigned to timeslots by this schedule""" return Session.objects.filter(timeslotassignments__schedule=self) - def delete_schedule(self): - self.assignments.all().delete() - self.delete() - # to be renamed SchedTimeSessAssignments (stsa) class SchedTimeSessAssignment(models.Model): """ @@ -1143,30 +1108,6 @@ def order_in_meeting(self): self._order_in_meeting = session_list.index(self) + 1 if self in session_list else 0 return self._order_in_meeting - def all_meeting_sessions_cancelled(self): - return set(s.current_status for s in self.all_meeting_sessions_for_group()) == {'canceled'} - - def all_meeting_recordings(self): - recordings = [] # These are not sets because we need to preserve relative ordering or redo the ordering work later - sessions = self.all_meeting_sessions_for_group() - for session in sessions: - recordings.extend([r for r in session.recordings() if r not in recordings]) - return recordings - - def all_meeting_bluesheets(self): - bluesheets = [] - sessions = self.all_meeting_sessions_for_group() - for session in sessions: - bluesheets.extend([b for b in session.bluesheets() if b not in bluesheets]) - return bluesheets - - def all_meeting_drafts(self): - drafts = [] - sessions = self.all_meeting_sessions_for_group() - for session in sessions: - drafts.extend([d for d in session.drafts() if d not in drafts]) - return drafts - def all_meeting_agendas(self): agendas = [] sessions = self.all_meeting_sessions_for_group() @@ -1283,19 +1224,6 @@ def agenda_text(self): else: return "The agenda has not been uploaded yet." - def agenda_file(self): - if not hasattr(self, '_agenda_file'): - self._agenda_file = "" - - agenda = self.agenda() - if not agenda: - return "" - - # FIXME: uploaded_filename should be replaced with a function that computes filenames when they are of a fixed schema and not uploaded names - self._agenda_file = "%s/agenda/%s" % (self.meeting.number, agenda.uploaded_filename) - - return self._agenda_file - def chat_room_name(self): if self.chat_room: return self.chat_room diff --git a/ietf/message/factories.py b/ietf/message/factories.py new file mode 100644 index 0000000000..72781512e4 --- /dev/null +++ b/ietf/message/factories.py @@ -0,0 +1,27 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +import factory + +from ietf.person.models import Person +from .models import Message, SendQueue + + +class MessageFactory(factory.django.DjangoModelFactory): + class Meta: + model = Message + + by = factory.LazyFunction(lambda: Person.objects.get(name="(System)")) + subject = factory.Faker("sentence") + to = factory.Faker("email") + frm = factory.Faker("email") + cc = factory.Faker("email") + bcc = factory.Faker("email") + body = factory.Faker("paragraph") + content_type = "text/plain" + + +class SendQueueFactory(factory.django.DjangoModelFactory): + class Meta: + model = SendQueue + + by = factory.LazyFunction(lambda: Person.objects.get(name="(System)")) + message = factory.SubFactory(MessageFactory) diff --git a/ietf/message/tasks.py b/ietf/message/tasks.py new file mode 100644 index 0000000000..efd776b9d8 --- /dev/null +++ b/ietf/message/tasks.py @@ -0,0 +1,27 @@ +# Copyright The IETF Trust 2024 All Rights Reserved +# +# Celery task definitions +# +from celery import shared_task +from smtplib import SMTPException + +from ietf.message.utils import send_scheduled_message_from_send_queue +from ietf.message.models import SendQueue +from ietf.utils import log +from ietf.utils.mail import log_smtp_exception, send_error_email + + +@shared_task +def send_scheduled_mail_task(): + """Send scheduled email + + This is equivalent to `ietf/bin/send-scheduled-mail all`, which was the only form used in the cron job. + """ + needs_sending = SendQueue.objects.filter(sent_at=None).select_related("message") + for s in needs_sending: + try: + send_scheduled_message_from_send_queue(s) + log.log('Sent scheduled message %s "%s"' % (s.id, s.message.subject)) + except SMTPException as e: + log_smtp_exception(e) + send_error_email(e) diff --git a/ietf/message/tests.py b/ietf/message/tests.py index a027df4473..7fbd29167c 100644 --- a/ietf/message/tests.py +++ b/ietf/message/tests.py @@ -1,8 +1,9 @@ # Copyright The IETF Trust 2013-2020, All Rights Reserved # -*- coding: utf-8 -*- - - import datetime +import mock + +from smtplib import SMTPException from django.urls import reverse as urlreverse from django.utils import timezone @@ -10,7 +11,9 @@ import debug # pyflakes:ignore from ietf.group.factories import GroupFactory +from ietf.message.factories import SendQueueFactory from ietf.message.models import Message, SendQueue +from ietf.message.tasks import send_scheduled_mail_task from ietf.message.utils import send_scheduled_message_from_send_queue from ietf.person.models import Person from ietf.utils.mail import outbox, send_mail_text, send_mail_message, get_payload_text @@ -128,3 +131,22 @@ def test_send_mime_announcement(self): self.assertTrue("This is a test" in outbox[-1]["Subject"]) self.assertTrue("--NextPart" in outbox[-1].as_string()) self.assertTrue(SendQueue.objects.get(id=q.id).sent_at) + + +class TaskTests(TestCase): + @mock.patch("ietf.message.tasks.log_smtp_exception") + @mock.patch("ietf.message.tasks.send_scheduled_message_from_send_queue") + def test_send_scheduled_mail_task(self, mock_send_message, mock_log_smtp_exception): + not_yet_sent = SendQueueFactory() + SendQueueFactory(sent_at=timezone.now()) # already sent + send_scheduled_mail_task() + self.assertEqual(mock_send_message.call_count, 1) + self.assertEqual(mock_send_message.call_args[0], (not_yet_sent,)) + self.assertFalse(mock_log_smtp_exception.called) + + mock_send_message.reset_mock() + mock_send_message.side_effect = SMTPException + send_scheduled_mail_task() + self.assertEqual(mock_send_message.call_count, 1) + self.assertEqual(mock_send_message.call_args[0], (not_yet_sent,)) + self.assertTrue(mock_log_smtp_exception.called) diff --git a/ietf/nomcom/admin.py b/ietf/nomcom/admin.py index 4b18cc005c..1675d731a5 100644 --- a/ietf/nomcom/admin.py +++ b/ietf/nomcom/admin.py @@ -21,9 +21,9 @@ class NomComAdmin(admin.ModelAdmin): admin.site.register(NomCom, NomComAdmin) class NominationAdmin(admin.ModelAdmin): - list_display = ['id', 'position', 'candidate_name', 'candidate_email', 'candidate_phone', 'nominee', 'comments', 'nominator_email', 'user', 'time', 'share_nominator'] + list_display = ['id', 'position', 'candidate_name', 'candidate_email', 'candidate_phone', 'nominee', 'comments', 'nominator_email', 'person', 'time', 'share_nominator'] list_filter = ['time', 'share_nominator'] - raw_id_fields = ['nominee', 'comments', 'user'] + raw_id_fields = ['nominee', 'comments', 'person'] admin.site.register(Nomination, NominationAdmin) class NomineeAdmin(admin.ModelAdmin): @@ -51,9 +51,9 @@ def nominee(self, obj): return ", ".join(n.person.ascii for n in obj.nominees.all()) nominee.admin_order_field = 'nominees__person__ascii' # type: ignore # https://github.com/python/mypy/issues/2087 - list_display = ['id', 'nomcom', 'author', 'nominee', 'subject', 'type', 'user', 'time'] + list_display = ['id', 'nomcom', 'author', 'nominee', 'subject', 'type', 'person', 'time'] list_filter = ['nomcom', 'type', 'time', ] - raw_id_fields = ['positions', 'topics', 'user'] + raw_id_fields = ['positions', 'topics', 'person'] admin.site.register(Feedback, FeedbackAdmin) diff --git a/ietf/nomcom/factories.py b/ietf/nomcom/factories.py index 7999228c24..286e0229ab 100644 --- a/ietf/nomcom/factories.py +++ b/ietf/nomcom/factories.py @@ -9,7 +9,7 @@ from ietf.nomcom.models import NomCom, Position, Feedback, Nominee, NomineePosition, Nomination, Topic from ietf.group.factories import GroupFactory -from ietf.person.factories import PersonFactory, UserFactory +from ietf.person.factories import PersonFactory import debug # pyflakes:ignore @@ -199,7 +199,7 @@ class Meta: candidate_email = factory.LazyAttribute(lambda obj: obj.nominee.person.email()) candidate_phone = factory.Faker('phone_number') comments = factory.SubFactory(FeedbackFactory) - nominator_email = factory.LazyAttribute(lambda obj: obj.user.email) - user = factory.SubFactory(UserFactory) + nominator_email = factory.LazyAttribute(lambda obj: obj.person.user.email) + person = factory.SubFactory(PersonFactory) share_nominator = False diff --git a/ietf/nomcom/forms.py b/ietf/nomcom/forms.py index 919ed6e187..7db5009121 100644 --- a/ietf/nomcom/forms.py +++ b/ietf/nomcom/forms.py @@ -15,7 +15,7 @@ from ietf.nomcom.models import ( NomCom, Nomination, Nominee, NomineePosition, Position, Feedback, ReminderDates, Topic, Volunteer ) from ietf.nomcom.utils import (NOMINATION_RECEIPT_TEMPLATE, FEEDBACK_RECEIPT_TEMPLATE, - get_user_email, validate_private_key, validate_public_key, + get_person_email, validate_private_key, validate_public_key, make_nomineeposition, make_nomineeposition_for_newperson, create_feedback_email) from ietf.person.models import Email @@ -256,7 +256,7 @@ class NominateForm(forms.ModelForm): def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom', None) - self.user = kwargs.pop('user', None) + self.person = kwargs.pop('person', None) self.public = kwargs.pop('public', None) super(NominateForm, self).__init__(*args, **kwargs) @@ -273,7 +273,7 @@ def __init__(self, *args, **kwargs): if not self.public: self.fields.pop('confirmation') - author = get_user_email(self.user) + author = get_person_email(self.person) if author: self.fields['nominator_email'].initial = author.address help_text = """(Nomcom Chair/Member: please fill this in. Use your own email address if the person making the @@ -303,7 +303,7 @@ def save(self, commit=True): author = None if self.public: - author = get_user_email(self.user) + author = get_person_email(self.person) else: if nominator_email: emails = Email.objects.filter(address=nominator_email) @@ -314,7 +314,7 @@ def save(self, commit=True): feedback = Feedback.objects.create(nomcom=self.nomcom, comments=self.nomcom.encrypt(qualifications), type=FeedbackTypeName.objects.get(slug='nomina'), - user=self.user) + person=self.person) feedback.positions.add(position) feedback.nominees.add(nominee) @@ -326,7 +326,7 @@ def save(self, commit=True): nomination.nominee = nominee nomination.comments = feedback nomination.share_nominator = share_nominator - nomination.user = self.user + nomination.person = self.person if commit: nomination.save() @@ -361,7 +361,7 @@ class NominateNewPersonForm(forms.ModelForm): def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom', None) - self.user = kwargs.pop('user', None) + self.person = kwargs.pop('person', None) self.public = kwargs.pop('public', None) super(NominateNewPersonForm, self).__init__(*args, **kwargs) @@ -375,7 +375,7 @@ def __init__(self, *args, **kwargs): if not self.public: self.fields.pop('confirmation') - author = get_user_email(self.user) + author = get_person_email(self.person) if author: self.fields['nominator_email'].initial = author.address help_text = """(Nomcom Chair/Member: please fill this in. Use your own email address if the person making the @@ -416,7 +416,7 @@ def save(self, commit=True): author = None if self.public: - author = get_user_email(self.user) + author = get_person_email(self.person) else: if nominator_email: emails = Email.objects.filter(address=nominator_email) @@ -429,7 +429,7 @@ def save(self, commit=True): feedback = Feedback.objects.create(nomcom=self.nomcom, comments=self.nomcom.encrypt(qualifications), type=FeedbackTypeName.objects.get(slug='nomina'), - user=self.user) + person=self.person) feedback.positions.add(position) feedback.nominees.add(nominee) @@ -441,7 +441,7 @@ def save(self, commit=True): nomination.nominee = nominee nomination.comments = feedback nomination.share_nominator = share_nominator - nomination.user = self.user + nomination.person = self.person if commit: nomination.save() @@ -476,7 +476,7 @@ class FeedbackForm(forms.ModelForm): def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom', None) - self.user = kwargs.pop('user', None) + self.person = kwargs.pop('person', None) self.public = kwargs.pop('public', None) self.position = kwargs.pop('position', None) self.nominee = kwargs.pop('nominee', None) @@ -484,7 +484,7 @@ def __init__(self, *args, **kwargs): super(FeedbackForm, self).__init__(*args, **kwargs) - author = get_user_email(self.user) + author = get_person_email(self.person) if self.public: self.fields.pop('nominator_email') @@ -514,7 +514,7 @@ def save(self, commit=True): author = None if self.public: - author = get_user_email(self.user) + author = get_person_email(self.person) else: nominator_email = self.cleaned_data['nominator_email'] if nominator_email: @@ -525,7 +525,7 @@ def save(self, commit=True): feedback.author = author.address feedback.nomcom = self.nomcom - feedback.user = self.user + feedback.person = self.person feedback.type = FeedbackTypeName.objects.get(slug='comment') feedback.comments = self.nomcom.encrypt(comment_text) feedback.save() @@ -578,7 +578,7 @@ class QuestionnaireForm(forms.ModelForm): def __init__(self, *args, **kwargs): self.nomcom = kwargs.pop('nomcom', None) - self.user = kwargs.pop('user', None) + self.person = kwargs.pop('person', None) super(QuestionnaireForm, self).__init__(*args, **kwargs) self.fields['nominee'] = PositionNomineeField(nomcom=self.nomcom, required=True) @@ -588,13 +588,13 @@ def save(self, commit=True): comment_text = self.cleaned_data['comment_text'] (position, nominee) = self.cleaned_data['nominee'] - author = get_user_email(self.user) + author = get_person_email(self.person) if author: feedback.author = author feedback.nomcom = self.nomcom - feedback.user = self.user + feedback.person = self.person feedback.type = FeedbackTypeName.objects.get(slug='questio') feedback.comments = self.nomcom.encrypt(comment_text) feedback.save() @@ -659,9 +659,9 @@ class Meta: model = Feedback fields = ('type', ) - def set_nomcom(self, nomcom, user): + def set_nomcom(self, nomcom, person): self.nomcom = nomcom - self.user = user + self.person = person #self.fields['nominee'] = MultiplePositionNomineeField(nomcom=self.nomcom, #required=True, #widget=forms.SelectMultiple, @@ -670,7 +670,7 @@ def set_nomcom(self, nomcom, user): def save(self, commit=True): feedback = super(PendingFeedbackForm, self).save(commit=False) feedback.nomcom = self.nomcom - feedback.user = self.user + feedback.person = self.person feedback.save() return feedback @@ -700,9 +700,9 @@ class Meta: model = Feedback fields = ('type', ) - def set_nomcom(self, nomcom, user, instances=None): + def set_nomcom(self, nomcom, person, instances=None): self.nomcom = nomcom - self.user = user + self.person = person instances = instances or [] self.feedback_type = None for i in instances: @@ -782,7 +782,7 @@ def save(self, commit=True): nominee=nominee, comments=feedback, nominator_email=nominator_email, - user=self.user) + person=self.person) return feedback else: feedback.save() diff --git a/ietf/nomcom/migrations/0005_user_to_person.py b/ietf/nomcom/migrations/0005_user_to_person.py new file mode 100644 index 0000000000..66a6e99642 --- /dev/null +++ b/ietf/nomcom/migrations/0005_user_to_person.py @@ -0,0 +1,100 @@ +# Generated by Django 4.2.2 on 2023-06-14 19:47 + +from django.db import migrations +from django.db.models import OuterRef, Subquery +import django.db.models.deletion +import ietf.utils.models + + +def forward(apps, schema_editor): + Nomination = apps.get_model('nomcom', 'Nomination') + Person = apps.get_model("person", "Person") + Nomination.objects.exclude( + user__isnull=True + ).update( + person=Subquery( + Person.objects.filter(user_id=OuterRef("user_id")).values("pk")[:1] + ) + ) + + Feedback = apps.get_model('nomcom', 'Feedback') + Feedback.objects.exclude( + user__isnull=True + ).update( + person=Subquery( + Person.objects.filter(user_id=OuterRef("user_id")).values("pk")[:1] + ) + ) + +def reverse(apps, schema_editor): + Nomination = apps.get_model('nomcom', 'Nomination') + Person = apps.get_model("person", "Person") + Nomination.objects.exclude( + person__isnull=True + ).update( + user_id=Subquery( + Person.objects.filter(pk=OuterRef("person_id")).values("user_id")[:1] + ) + ) + + Feedback = apps.get_model('nomcom', 'Feedback') + Feedback.objects.exclude( + person__isnull=True + ).update( + user_id=Subquery( + Person.objects.filter(pk=OuterRef("person_id")).values("user_id")[:1] + ) + ) + +class Migration(migrations.Migration): + dependencies = [ + ("person", "0001_initial"), + ("nomcom", "0004_volunteer_origin_volunteer_time_volunteer_withdrawn"), + ] + + operations = [ + migrations.AddField( + model_name="feedback", + name="person", + field=ietf.utils.models.ForeignKey( + blank=True, + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="person.person", + ), + ), + migrations.AddField( + model_name="nomination", + name="person", + field=ietf.utils.models.ForeignKey( + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="person.person", + ), + ), + migrations.RunPython(forward, reverse), + migrations.RemoveField( + model_name="feedback", + name="user", + field=ietf.utils.models.ForeignKey( + blank=True, + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="user.user", + ), + ), + migrations.RemoveField( + model_name="nomination", + name="user", + field=ietf.utils.models.ForeignKey( + blank=True, + editable=False, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="user.user", + ), + ), + ] diff --git a/ietf/nomcom/models.py b/ietf/nomcom/models.py index b514f4fb1f..2ed1124c5c 100644 --- a/ietf/nomcom/models.py +++ b/ietf/nomcom/models.py @@ -7,7 +7,6 @@ from django.db import models from django.db.models.signals import post_delete from django.conf import settings -from django.contrib.auth.models import User from django.template.loader import render_to_string from django.template.defaultfilters import linebreaks # type: ignore @@ -128,7 +127,7 @@ class Nomination(models.Model): nominee = ForeignKey('Nominee') comments = ForeignKey('Feedback') nominator_email = models.EmailField(verbose_name='Nominator Email', blank=True) - user = ForeignKey(User, editable=False, null=True, on_delete=models.SET_NULL) + person = ForeignKey(Person, editable=False, null=True, on_delete=models.SET_NULL) time = models.DateTimeField(auto_now_add=True) share_nominator = models.BooleanField(verbose_name='OK to share nominator\'s name with candidate', default=False, help_text='Check this box to allow the NomCom to let the ' @@ -299,7 +298,7 @@ class Feedback(models.Model): subject = models.TextField(verbose_name='Subject', blank=True) comments = models.BinaryField(verbose_name='Comments') type = ForeignKey(FeedbackTypeName, blank=True, null=True) - user = ForeignKey(User, editable=False, blank=True, null=True, on_delete=models.SET_NULL) + person = ForeignKey(Person, editable=False, blank=True, null=True, on_delete=models.SET_NULL) time = models.DateTimeField(auto_now_add=True) objects = FeedbackManager() diff --git a/ietf/nomcom/resources.py b/ietf/nomcom/resources.py index c87e72eae6..109a136419 100644 --- a/ietf/nomcom/resources.py +++ b/ietf/nomcom/resources.py @@ -115,11 +115,11 @@ class Meta: api.nomcom.register(NomineePositionResource()) from ietf.name.resources import FeedbackTypeNameResource -from ietf.utils.resources import UserResource +from ietf.person.resources import PersonResource class FeedbackResource(ModelResource): nomcom = ToOneField(NomComResource, 'nomcom') type = ToOneField(FeedbackTypeNameResource, 'type', null=True) - user = ToOneField(UserResource, 'user', null=True) + person = ToOneField(PersonResource, 'person', null=True) positions = ToManyField(PositionResource, 'positions', null=True) nominees = ToManyField(NomineeResource, 'nominees', null=True) class Meta: @@ -136,18 +136,18 @@ class Meta: "time": ALL, "nomcom": ALL_WITH_RELATIONS, "type": ALL_WITH_RELATIONS, - "user": ALL_WITH_RELATIONS, + "person": ALL_WITH_RELATIONS, "positions": ALL_WITH_RELATIONS, "nominees": ALL_WITH_RELATIONS, } api.nomcom.register(FeedbackResource()) -from ietf.utils.resources import UserResource +from ietf.person.resources import PersonResource class NominationResource(ModelResource): position = ToOneField(PositionResource, 'position') nominee = ToOneField(NomineeResource, 'nominee') comments = ToOneField(FeedbackResource, 'comments') - user = ToOneField(UserResource, 'user', null=True) + person = ToOneField(PersonResource, 'person', null=True) class Meta: cache = SimpleCache() queryset = Nomination.objects.all() @@ -164,7 +164,7 @@ class Meta: "position": ALL_WITH_RELATIONS, "nominee": ALL_WITH_RELATIONS, "comments": ALL_WITH_RELATIONS, - "user": ALL_WITH_RELATIONS, + "person": ALL_WITH_RELATIONS, } api.nomcom.register(NominationResource()) diff --git a/ietf/nomcom/tests.py b/ietf/nomcom/tests.py index d3da0bddd6..cdbb860c43 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -689,20 +689,16 @@ def test_public_nominate_with_automatic_questionnaire(self): self.assertIn('nominee@', outbox[1]['To']) - def nominate_view(self, *args, **kwargs): - public = kwargs.pop('public', True) - searched_email = kwargs.pop('searched_email', None) - nominee_email = kwargs.pop('nominee_email', 'nominee@example.com') + def nominate_view(self, public=True, searched_email=None, + nominee_email='nominee@example.com', + nominator_email=COMMUNITY_USER+EMAIL_DOMAIN, + position='IAOC', confirmation=False): + if not searched_email: - searched_email = Email.objects.filter(address=nominee_email).first() - if not searched_email: - searched_email = EmailFactory(address=nominee_email, primary=True, origin='test') + searched_email = Email.objects.filter(address=nominee_email).first() or EmailFactory(address=nominee_email, primary=True, origin='test') if not searched_email.person: searched_email.person = PersonFactory() searched_email.save() - nominator_email = kwargs.pop('nominator_email', "%s%s" % (COMMUNITY_USER, EMAIL_DOMAIN)) - position_name = kwargs.pop('position', 'IAOC') - confirmation = kwargs.pop('confirmation', False) if public: nominate_url = self.public_nominate_url @@ -726,7 +722,7 @@ def nominate_view(self, *args, **kwargs): q = PyQuery(response.content) self.assertEqual(len(q("#nominate-form")), 1) - position = Position.objects.get(name=position_name) + position = Position.objects.get(name=position) comment_text = 'Test nominate view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' candidate_phone = '123456' @@ -764,12 +760,9 @@ def nominate_view(self, *args, **kwargs): comments=feedback, nominator_email="%s%s" % (COMMUNITY_USER, EMAIL_DOMAIN)) - def nominate_newperson_view(self, *args, **kwargs): - public = kwargs.pop('public', True) - nominee_email = kwargs.pop('nominee_email', 'nominee@example.com') - nominator_email = kwargs.pop('nominator_email', "%s%s" % (COMMUNITY_USER, EMAIL_DOMAIN)) - position_name = kwargs.pop('position', 'IAOC') - confirmation = kwargs.pop('confirmation', False) + def nominate_newperson_view(self, public=True, nominee_email='nominee@example.com', + nominator_email=COMMUNITY_USER+EMAIL_DOMAIN, + position='IAOC', confirmation=False): if public: nominate_url = self.public_nominate_newperson_url @@ -793,7 +786,7 @@ def nominate_newperson_view(self, *args, **kwargs): q = PyQuery(response.content) self.assertEqual(len(q("#nominate-form")), 1) - position = Position.objects.get(name=position_name) + position = Position.objects.get(name=position) candidate_email = nominee_email candidate_name = 'nominee' comment_text = 'Test nominate view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' @@ -847,15 +840,13 @@ def test_add_questionnaire(self): self.access_chair_url(self.add_questionnaire_url) self.add_questionnaire() - def add_questionnaire(self, *args, **kwargs): - public = kwargs.pop('public', False) - nominee_email = kwargs.pop('nominee_email', 'nominee@example.com') - nominator_email = kwargs.pop('nominator_email', "%s%s" % (COMMUNITY_USER, EMAIL_DOMAIN)) - position_name = kwargs.pop('position', 'IAOC') + def add_questionnaire(self, public=False, nominee_email='nominee@example.com', + nominator_email=COMMUNITY_USER+EMAIL_DOMAIN, + position='IAOC'): self.nominate_view(public=public, nominee_email=nominee_email, - position=position_name, + position=position, nominator_email=nominator_email) response = self.client.get(self.add_questionnaire_url) @@ -874,7 +865,7 @@ def add_questionnaire(self, *args, **kwargs): self.assertEqual(response.status_code, 200) self.assertContains(response, "questionnnaireform") - position = Position.objects.get(name=position_name) + position = Position.objects.get(name=position) nominee = Nominee.objects.get(email__address=nominee_email) comment_text = 'Test add questionnaire view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' @@ -924,16 +915,13 @@ def test_private_feedback(self): self.access_member_url(self.private_feedback_url) self.feedback_view(public=False) - def feedback_view(self, *args, **kwargs): - public = kwargs.pop('public', True) - nominee_email = kwargs.pop('nominee_email', 'nominee@example.com') - nominator_email = kwargs.pop('nominator_email', "%s%s" % (COMMUNITY_USER, EMAIL_DOMAIN)) - position_name = kwargs.pop('position', 'IAOC') - confirmation = kwargs.pop('confirmation', False) + def feedback_view(self, public=True, nominee_email='nominee@example.com', + nominator_email=COMMUNITY_USER+EMAIL_DOMAIN, + position='IAOC', confirmation=False): self.nominate_view(public=public, nominee_email=nominee_email, - position=position_name, + position=position, nominator_email=nominator_email) feedback_url = self.public_feedback_url @@ -956,7 +944,7 @@ def feedback_view(self, *args, **kwargs): self.assertEqual(response.status_code, 200) self.assertNotContains(response, "feedbackform") - position = Position.objects.get(name=position_name) + position = Position.objects.get(name=position) nominee = Nominee.objects.get(email__address=nominee_email) feedback_url += "?nominee=%d&position=%d" % (nominee.id, position.id) @@ -972,7 +960,7 @@ def feedback_view(self, *args, **kwargs): comments = 'Test feedback view. Comments with accents äöåÄÖÅ éáíóú âêîôû ü àèìòù.' test_data = {'comment_text': comments, - 'position_name': position.name, + 'position': position.name, 'nominee_name': nominee.email.person.name, 'nominee_email': nominee.email.address, 'confirmation': confirmation} @@ -1168,7 +1156,7 @@ def setUp(self): feedback = Feedback.objects.create(nomcom=self.nomcom, comments=self.nomcom.encrypt('some non-empty comments'), type=FeedbackTypeName.objects.get(slug='questio'), - user=User.objects.get(username=CHAIR_USER)) + person=User.objects.get(username=CHAIR_USER).person) feedback.positions.add(gen) feedback.nominees.add(n) @@ -2192,7 +2180,7 @@ def test_public_accepting_feedback(self): self.assertIn('not currently accepting feedback', unicontent(response)) test_data = {'comment_text': 'junk', - 'position_name': pos.name, + 'position': pos.name, 'nominee_name': pos.nominee_set.first().email.person.name, 'nominee_email': pos.nominee_set.first().email.address, 'confirmation': False, diff --git a/ietf/nomcom/utils.py b/ietf/nomcom/utils.py index 220f2e401d..23e89026d1 100644 --- a/ietf/nomcom/utils.py +++ b/ietf/nomcom/utils.py @@ -88,26 +88,21 @@ def get_year_by_nomcom(nomcom): return m.group(0) -def get_user_email(user): - # a user object already has an email field, but we don't want to - # overwrite anything that might be there, and we don't know that - # what's there is the right thing, so we cache the lookup results in a - # separate attribute - if not hasattr(user, "_email_cache"): - user._email_cache = None - if hasattr(user, "person"): - emails = user.person.email_set.filter(active=True).order_by('-time') - if emails: - user._email_cache = emails[0] - for email in emails: - if email.address.lower() == user.username.lower(): - user._email_cache = email +def get_person_email(person): + if not hasattr(person, "_email_cache"): + person._email_cache = None + emails = person.email_set.filter(active=True).order_by('-time') + if emails: + person._email_cache = emails[0] + for email in emails: + if email.address.lower() == person.user.username.lower(): + person._email_cache = email else: try: - user._email_cache = Email.objects.get(address=user.username) + person._email_cache = Email.objects.get(address=person.user.username) except ObjectDoesNotExist: pass - return user._email_cache + return person._email_cache def get_hash_nominee_position(date, nominee_position_id): return hmac.new(settings.NOMCOM_APP_SECRET, f"{date}{nominee_position_id}".encode('utf-8'), hashlib.sha256).hexdigest() diff --git a/ietf/nomcom/views.py b/ietf/nomcom/views.py index 7705be5697..d34126b1e7 100644 --- a/ietf/nomcom/views.py +++ b/ietf/nomcom/views.py @@ -454,23 +454,24 @@ def nominate(request, year, public, newperson): {'nomcom': nomcom, 'year': year}) + person = request.user.person if request.method == 'POST': if newperson: - form = NominateNewPersonForm(data=request.POST, nomcom=nomcom, user=request.user, public=public) + form = NominateNewPersonForm(data=request.POST, nomcom=nomcom, person=person, public=public) else: - form = NominateForm(data=request.POST, nomcom=nomcom, user=request.user, public=public) + form = NominateForm(data=request.POST, nomcom=nomcom, person=person, public=public) if form.is_valid(): form.save() messages.success(request, 'Your nomination has been registered. Thank you for the nomination.') if newperson: return redirect('ietf.nomcom.views.%s_nominate' % ('public' if public else 'private'), year=year) else: - form = NominateForm(nomcom=nomcom, user=request.user, public=public) + form = NominateForm(nomcom=nomcom, person=person, public=public) else: if newperson: - form = NominateNewPersonForm(nomcom=nomcom, user=request.user, public=public) + form = NominateNewPersonForm(nomcom=nomcom, person=person, public=public) else: - form = NominateForm(nomcom=nomcom, user=request.user, public=public) + form = NominateForm(nomcom=nomcom, person=person, public=public) return render(request, template, {'form': form, @@ -494,6 +495,7 @@ def feedback(request, year, public): nominee = None position = None topic = None + person = request.user.person if nomcom.group.state_id != 'conclude': selected_nominee = request.GET.get('nominee') selected_position = request.GET.get('position') @@ -505,7 +507,7 @@ def feedback(request, year, public): topic = get_object_or_404(Topic,id=selected_topic) if topic.audience_id == 'nomcom' and not nomcom.group.has_role(request.user, ['chair','advisor','liaison','member']): raise Http404() - if topic.audience_id == 'nominees' and not nomcom.nominee_set.filter(person=request.user.person).exists(): + if topic.audience_id == 'nominees' and not nomcom.nominee_set.filter(person=person).exists(): raise Http404() if public: @@ -517,12 +519,12 @@ def feedback(request, year, public): if not nomcom.group.has_role(request.user, ['chair','advisor','liaison','member']): topics = topics.exclude(audience_id='nomcom') - if not nomcom.nominee_set.filter(person=request.user.person).exists(): + if not nomcom.nominee_set.filter(person=person).exists(): topics = topics.exclude(audience_id='nominees') user_comments = Feedback.objects.filter(nomcom=nomcom, type='comment', - author__in=request.user.person.email_set.filter(active='True')) + author__in=person.email_set.filter(active='True')) counter = Counter(user_comments.values_list('positions','nominees')) counts = dict() for pos,nom in counter: @@ -572,11 +574,11 @@ def feedback(request, year, public): if request.method == 'POST': if nominee and position: form = FeedbackForm(data=request.POST, - nomcom=nomcom, user=request.user, + nomcom=nomcom, person=person, public=public, position=position, nominee=nominee) elif topic: form = FeedbackForm(data=request.POST, - nomcom=nomcom, user=request.user, + nomcom=nomcom, person=person, public=public, topic=topic) else: form = None @@ -595,10 +597,10 @@ def feedback(request, year, public): pass else: if nominee and position: - form = FeedbackForm(nomcom=nomcom, user=request.user, public=public, + form = FeedbackForm(nomcom=nomcom, person=person, public=public, position=position, nominee=nominee) elif topic: - form = FeedbackForm(nomcom=nomcom, user=request.user, public=public, + form = FeedbackForm(nomcom=nomcom, person=person, public=public, topic=topic) else: form = None @@ -661,6 +663,7 @@ def private_questionnaire(request, year): has_publickey = nomcom.public_key and True or False questionnaire_response = None template = 'nomcom/private_questionnaire.html' + person = request.user.person if not has_publickey: messages.warning(request, "This Nomcom is not yet accepting questionnaires.") @@ -680,14 +683,14 @@ def private_questionnaire(request, year): if request.method == 'POST': form = QuestionnaireForm(data=request.POST, - nomcom=nomcom, user=request.user) + nomcom=nomcom, person=person) if form.is_valid(): form.save() messages.success(request, 'The questionnaire response has been registered.') questionnaire_response = force_str(form.cleaned_data['comment_text']) - form = QuestionnaireForm(nomcom=nomcom, user=request.user) + form = QuestionnaireForm(nomcom=nomcom, person=person) else: - form = QuestionnaireForm(nomcom=nomcom, user=request.user) + form = QuestionnaireForm(nomcom=nomcom, person=person) return render(request, template, {'form': form, @@ -725,15 +728,13 @@ def process_nomination_status(request, year, nominee_position_id, state, date, h if form.cleaned_data['comments']: # This Feedback object is of type comment instead of nomina in order to not # make answering "who nominated themselves" harder. - who = request.user - if isinstance(who,AnonymousUser): - who = None + who = None if isinstance(request.user, AnonymousUser) else request.user.person f = Feedback.objects.create(nomcom = nomcom, author = nominee_position.nominee.email, subject = '%s nomination %s'%(nominee_position.nominee.name(),state), comments = nomcom.encrypt(form.cleaned_data['comments']), type_id = 'comment', - user = who, + person = who, ) f.positions.add(nominee_position.position) f.nominees.add(nominee_position.nominee) @@ -779,8 +780,9 @@ def nominee_staterank(nominee): sorted_nominees = sorted(nominees,key=lambda x:x.staterank) + reviewer = request.user.person for nominee in sorted_nominees: - last_seen = FeedbackLastSeen.objects.filter(reviewer=request.user.person,nominee=nominee).first() + last_seen = FeedbackLastSeen.objects.filter(reviewer=reviewer,nominee=nominee).first() nominee_feedback = [] for ft in nominee_feedback_types: qs = nominee.feedback_set.by_type(ft.slug) @@ -795,7 +797,7 @@ def nominee_staterank(nominee): nominees_feedback.append( {'nominee':nominee, 'feedback':nominee_feedback} ) independent_feedback = [ft.feedback_set.get_by_nomcom(nomcom).count() for ft in independent_feedback_types] for topic in nomcom.topic_set.all(): - last_seen = TopicFeedbackLastSeen.objects.filter(reviewer=request.user.person,topic=topic).first() + last_seen = TopicFeedbackLastSeen.objects.filter(reviewer=reviewer,topic=topic).first() topic_feedback = [] for ft in topic_feedback_types: qs = topic.feedback_set.by_type(ft.slug) @@ -842,6 +844,7 @@ def view_feedback_pending(request, year): except EmptyPage: feedback_page = paginator.page(paginator.num_pages) extra_step = False + person = request.user.person if request.method == 'POST' and request.POST.get('end'): extra_ids = request.POST.get('extra_ids', None) extra_step = True @@ -850,7 +853,7 @@ def view_feedback_pending(request, year): formset.absolute_max = 2000 formset.validate_max = False for form in formset.forms: - form.set_nomcom(nomcom, request.user) + form.set_nomcom(nomcom, person) if formset.is_valid(): formset.save() if extra_ids: @@ -862,7 +865,7 @@ def view_feedback_pending(request, year): extra.append(feedback) formset = FullFeedbackFormSet(queryset=Feedback.objects.filter(id__in=[i.id for i in extra])) for form in formset.forms: - form.set_nomcom(nomcom, request.user, extra) + form.set_nomcom(nomcom, person, extra) extra_ids = None else: messages.success(request, 'Feedback saved') @@ -870,7 +873,7 @@ def view_feedback_pending(request, year): elif request.method == 'POST': formset = FeedbackFormSet(request.POST) for form in formset.forms: - form.set_nomcom(nomcom, request.user) + form.set_nomcom(nomcom, person) if formset.is_valid(): extra = [] nominations = [] @@ -890,12 +893,12 @@ def view_feedback_pending(request, year): if nominations: formset = FullFeedbackFormSet(queryset=Feedback.objects.filter(id__in=[i.id for i in nominations])) for form in formset.forms: - form.set_nomcom(nomcom, request.user, nominations) + form.set_nomcom(nomcom, person, nominations) extra_ids = ','.join(['%s:%s' % (i.id, i.type.pk) for i in extra]) else: formset = FullFeedbackFormSet(queryset=Feedback.objects.filter(id__in=[i.id for i in extra])) for form in formset.forms: - form.set_nomcom(nomcom, request.user, extra) + form.set_nomcom(nomcom, person, extra) if moved: messages.success(request, '%s messages classified. You must enter more information for the following feedback.' % moved) else: @@ -904,7 +907,7 @@ def view_feedback_pending(request, year): else: formset = FeedbackFormSet(queryset=feedback_page.object_list) for form in formset.forms: - form.set_nomcom(nomcom, request.user) + form.set_nomcom(nomcom, person) return render(request, 'nomcom/view_feedback_pending.html', {'year': year, 'formset': formset, @@ -975,13 +978,14 @@ def view_feedback_topic(request, year, topic_id): topic = get_object_or_404(Topic, id=topic_id) nomcom = get_nomcom_by_year(year) feedback_types = FeedbackTypeName.objects.filter(slug__in=['comment',]) + reviewer = request.user.person - last_seen = TopicFeedbackLastSeen.objects.filter(reviewer=request.user.person,topic=topic).first() + last_seen = TopicFeedbackLastSeen.objects.filter(reviewer=reviewer,topic=topic).first() last_seen_time = (last_seen and last_seen.time) or datetime.datetime(year=1, month=1, day=1, tzinfo=datetime.timezone.utc) if last_seen: last_seen.save() else: - TopicFeedbackLastSeen.objects.create(reviewer=request.user.person,topic=topic) + TopicFeedbackLastSeen.objects.create(reviewer=reviewer,topic=topic) return render(request, 'nomcom/view_feedback_topic.html', {'year': year, @@ -997,7 +1001,7 @@ def view_feedback_nominee(request, year, nominee_id): nomcom = get_nomcom_by_year(year) nominee = get_object_or_404(Nominee, id=nominee_id) feedback_types = FeedbackTypeName.objects.filter(used=True, slug__in=settings.NOMINEE_FEEDBACK_TYPES) - + reviewer = request.user.person if request.method == 'POST': if not nomcom.group.has_role(request.user, ['chair','advisor']): return HttpResponseForbidden('Restricted to roles: Nomcom Chair, Nomcom Advisor') @@ -1036,12 +1040,12 @@ def view_feedback_nominee(request, year, nominee_id): 'is_chair_task': True, }) - last_seen = FeedbackLastSeen.objects.filter(reviewer=request.user.person,nominee=nominee).first() + last_seen = FeedbackLastSeen.objects.filter(reviewer=reviewer,nominee=nominee).first() last_seen_time = (last_seen and last_seen.time) or datetime.datetime(year=1, month=1, day=1, tzinfo=datetime.timezone.utc) if last_seen: last_seen.save() else: - FeedbackLastSeen.objects.create(reviewer=request.user.person,nominee=nominee) + FeedbackLastSeen.objects.create(reviewer=reviewer,nominee=nominee) return render(request, 'nomcom/view_feedback_nominee.html', {'year': year, @@ -1322,15 +1326,15 @@ def configuration_help(request, year): @role_required("Nomcom Chair", "Nomcom Advisor") def edit_members(request, year): nomcom = get_nomcom_by_year(year) - if nomcom.group.state_id=='conclude': permission_denied(request, 'This nomcom is closed.') + person = request.user.person if request.method=='POST': form = EditMembersForm(nomcom, data=request.POST) if form.is_valid(): - update_role_set(nomcom.group, 'member', form.cleaned_data['members'], request.user.person) - update_role_set(nomcom.group, 'liaison', form.cleaned_data['liaisons'], request.user.person) + update_role_set(nomcom.group, 'member', form.cleaned_data['members'], person) + update_role_set(nomcom.group, 'liaison', form.cleaned_data['liaisons'], person) return HttpResponseRedirect(reverse('ietf.nomcom.views.private_index',kwargs={'year':year})) else: form = EditMembersForm(nomcom) diff --git a/ietf/person/tests.py b/ietf/person/tests.py index bb75b438db..be3cfc0562 100644 --- a/ietf/person/tests.py +++ b/ietf/person/tests.py @@ -25,11 +25,11 @@ from ietf.nomcom.models import NomCom from ietf.nomcom.test_data import nomcom_test_data from ietf.nomcom.factories import NomComFactory, NomineeFactory, NominationFactory, FeedbackFactory, PositionFactory -from ietf.person.factories import EmailFactory, PersonFactory, UserFactory +from ietf.person.factories import EmailFactory, PersonFactory from ietf.person.models import Person, Alias from ietf.person.utils import (merge_persons, determine_merge_order, send_merge_notification, handle_users, get_extra_primary, dedupe_aliases, move_related_objects, merge_nominees, - handle_reviewer_settings, merge_users, get_dots) + handle_reviewer_settings, get_dots) from ietf.review.models import ReviewerSettings from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.mail import outbox, empty_outbox @@ -165,6 +165,16 @@ def test_person_photo(self): img = Image.open(BytesIO(r.content)) self.assertEqual(img.width, 200) + def test_person_photo_duplicates(self): + person = PersonFactory(name="bazquux@example.com", user__username="bazquux@example.com", with_bio=True) + PersonFactory(name="bazquux@example.com", user__username="foobar@example.com", with_bio=True) + + url = urlreverse("ietf.person.views.photo", kwargs={ "email_or_name": person.plain_name()}) + r = self.client.get(url) + self.assertEqual(r.status_code, 300) + self.assertIn("bazquux@example.com", r.content.decode()) + self.assertIn("foobar@example.com", r.content.decode()) + def test_name_methods(self): person = PersonFactory(name="Dr. Jens F. Möller", ) @@ -381,13 +391,24 @@ def test_merge_persons(self): request.user = user source = PersonFactory() target = PersonFactory() + mars = RoleFactory(name_id='chair',group__acronym='mars').group source_id = source.pk source_email = source.email_set.first() source_alias = source.alias_set.first() source_user = source.user + communitylist = CommunityList.objects.create(person=source, group=mars) + nomcom = NomComFactory() + position = PositionFactory(nomcom=nomcom) + nominee = NomineeFactory(nomcom=nomcom, person=mars.get_chair().person) + feedback = FeedbackFactory(person=source, author=source.email().address, nomcom=nomcom) + feedback.nominees.add(nominee) + nomination = NominationFactory(nominee=nominee, person=source, position=position, comments=feedback) merge_persons(request, source, target, file=StringIO()) self.assertTrue(source_email in target.email_set.all()) self.assertTrue(source_alias in target.alias_set.all()) + self.assertIn(communitylist, target.communitylist_set.all()) + self.assertIn(feedback, target.feedback_set.all()) + self.assertIn(nomination, target.nomination_set.all()) self.assertFalse(Person.objects.filter(id=source_id)) self.assertFalse(source_user.is_active) @@ -407,24 +428,6 @@ def test_merge_persons_reviewer_settings(self): rs = target.reviewersettings_set.first() self.assertEqual(rs.min_interval, 7) - def test_merge_users(self): - person = PersonFactory() - source = person.user - target = UserFactory() - mars = RoleFactory(name_id='chair',group__acronym='mars').group - communitylist = CommunityList.objects.create(user=source, group=mars) - nomcom = NomComFactory() - position = PositionFactory(nomcom=nomcom) - nominee = NomineeFactory(nomcom=nomcom, person=mars.get_chair().person) - feedback = FeedbackFactory(user=source, author=person.email().address, nomcom=nomcom) - feedback.nominees.add(nominee) - nomination = NominationFactory(nominee=nominee, user=source, position=position, comments=feedback) - - merge_users(source, target) - self.assertIn(communitylist, target.communitylist_set.all()) - self.assertIn(feedback, target.feedback_set.all()) - self.assertIn(nomination, target.nomination_set.all()) - def test_dots(self): noroles = PersonFactory() self.assertEqual(get_dots(noroles),[]) diff --git a/ietf/person/utils.py b/ietf/person/utils.py index 942c2aaab2..eb2742ed30 100755 --- a/ietf/person/utils.py +++ b/ietf/person/utils.py @@ -12,10 +12,11 @@ from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q +from django.http import Http404 import debug # pyflakes:ignore -from ietf.person.models import Person +from ietf.person.models import Person, Alias, Email from ietf.utils.mail import send_mail def merge_persons(request, source, target, file=sys.stdout, verbose=False): @@ -31,6 +32,20 @@ def merge_persons(request, source, target, file=sys.stdout, verbose=False): email.save() changes.append('EMAIL ACTION: {} no longer marked as primary'.format(email.address)) + # handle community list + for communitylist in source.communitylist_set.all(): + source.communitylist_set.remove(communitylist) + target.communitylist_set.add(communitylist) + + # handle feedback + for feedback in source.feedback_set.all(): + feedback.person = target + feedback.save() + # handle nominations + for nomination in source.nomination_set.all(): + nomination.person = target + nomination.save() + changes.append(handle_users(source, target)) reviewer_changes = handle_reviewer_settings(source, target) if reviewer_changes: @@ -103,7 +118,6 @@ def handle_users(source,target,check_only=False): if source.user and target.user: message = "DATATRACKER LOGIN ACTION: retaining login: {}, removing login: {}".format(target.user,source.user) if not check_only: - merge_users(source.user, target.user) syslog.syslog('merge-person-records: deactivating user {}'.format(source.user.username)) user = source.user source.user = None @@ -126,21 +140,6 @@ def move_related_objects(source, target, file, verbose=False): kwargs = { field_name:target } queryset.update(**kwargs) -def merge_users(source, target): - '''Move related objects from source user to target user''' - # handle community list - for communitylist in source.communitylist_set.all(): - source.communitylist_set.remove(communitylist) - target.communitylist_set.add(communitylist) - # handle feedback - for feedback in source.feedback_set.all(): - feedback.user = target - feedback.save() - # handle nominations - for nomination in source.nomination_set.all(): - nomination.user = target - nomination.save() - def dedupe_aliases(person): '''Check person for duplicate aliases and purge''' seen = [] @@ -248,3 +247,17 @@ def get_dots(person): if roles.filter(group__acronym__startswith='nomcom', name_id__in=('chair','member')).exists(): dots.append('nomcom') return dots + +def lookup_persons(email_or_name): + aliases = Alias.objects.filter(name__iexact=email_or_name) + persons = set(a.person for a in aliases) + + if '@' in email_or_name: + emails = Email.objects.filter(address__iexact=email_or_name) + persons.update(e.person for e in emails) + + persons = [p for p in persons if p and p.id] + if not persons: + raise Http404 + persons.sort(key=lambda p: p.id) + return persons diff --git a/ietf/person/views.py b/ietf/person/views.py index 23a8dea3d2..6d9daf4a81 100644 --- a/ietf/person/views.py +++ b/ietf/person/views.py @@ -8,16 +8,16 @@ from django.contrib import messages from django.db.models import Q from django.http import HttpResponse, Http404 -from django.shortcuts import render, get_object_or_404, redirect +from django.shortcuts import render, redirect from django.utils import timezone import debug # pyflakes:ignore from ietf.ietfauth.utils import role_required -from ietf.person.models import Email, Person, Alias +from ietf.person.models import Email, Person from ietf.person.fields import select2_id_name_json from ietf.person.forms import MergeForm -from ietf.person.utils import handle_users, merge_persons +from ietf.person.utils import handle_users, merge_persons, lookup_persons def ajax_select2_search(request, model_name): @@ -69,30 +69,14 @@ def ajax_select2_search(request, model_name): def profile(request, email_or_name): - aliases = Alias.objects.filter(name__iexact=email_or_name) - persons = set(a.person for a in aliases) - - if '@' in email_or_name: - emails = Email.objects.filter(address__iexact=email_or_name) - persons.update(e.person for e in emails) - - persons = [p for p in persons if p and p.id] - if not persons: - raise Http404 - persons.sort(key=lambda p: p.id) + persons = lookup_persons(email_or_name) return render(request, 'person/profile.html', {'persons': persons, 'today': timezone.now()}) def photo(request, email_or_name): - if '@' in email_or_name: - persons = [ get_object_or_404(Email, address=email_or_name).person, ] - else: - aliases = Alias.objects.filter(name=email_or_name) - persons = list(set([ a.person for a in aliases ])) - if not persons: - raise Http404("No such person") + persons = lookup_persons(email_or_name) if len(persons) > 1: - return HttpResponse(r"\r\n".join([p.email() for p in persons]), status=300) + return HttpResponse(r"\r\n".join([p.user.username for p in persons]), status=300) person = persons[0] if not person.photo: raise Http404("No photo found") diff --git a/ietf/review/tasks.py b/ietf/review/tasks.py new file mode 100644 index 0000000000..5d8afa6943 --- /dev/null +++ b/ietf/review/tasks.py @@ -0,0 +1,43 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# +# Celery task definitions +# +from celery import shared_task + +from ietf.review.utils import ( + review_assignments_needing_reviewer_reminder, email_reviewer_reminder, + review_assignments_needing_secretary_reminder, email_secretary_reminder, + send_unavailability_period_ending_reminder, send_reminder_all_open_reviews, + send_review_reminder_overdue_assignment, send_reminder_unconfirmed_assignments) +from ietf.utils.log import log +from ietf.utils.timezone import date_today, DEADLINE_TZINFO + + +@shared_task +def send_review_reminders_task(): + today = date_today(DEADLINE_TZINFO) + + for assignment in review_assignments_needing_reviewer_reminder(today): + email_reviewer_reminder(assignment) + log("Emailed reminder to {} for review of {} in {} (req. id {})".format(assignment.reviewer.address, assignment.review_request.doc_id, assignment.review_request.team.acronym, assignment.review_request.pk)) + + for assignment, secretary_role in review_assignments_needing_secretary_reminder(today): + email_secretary_reminder(assignment, secretary_role) + review_req = assignment.review_request + log("Emailed reminder to {} for review of {} in {} (req. id {})".format(secretary_role.email.address, review_req.doc_id, review_req.team.acronym, review_req.pk)) + + period_end_reminders_sent = send_unavailability_period_ending_reminder(today) + for msg in period_end_reminders_sent: + log(msg) + + overdue_reviews_reminders_sent = send_review_reminder_overdue_assignment(today) + for msg in overdue_reviews_reminders_sent: + log(msg) + + open_reviews_reminders_sent = send_reminder_all_open_reviews(today) + for msg in open_reviews_reminders_sent: + log(msg) + + unconfirmed_assignment_reminders_sent = send_reminder_unconfirmed_assignments(today) + for msg in unconfirmed_assignment_reminders_sent: + log(msg) diff --git a/ietf/review/tests.py b/ietf/review/tests.py index f9d55d9d14..e9ddbd47af 100644 --- a/ietf/review/tests.py +++ b/ietf/review/tests.py @@ -1,9 +1,11 @@ # Copyright The IETF Trust 2019-2020, All Rights Reserved # -*- coding: utf-8 -*- import datetime +import mock import debug # pyflakes:ignore from pyquery import PyQuery + from ietf.group.factories import RoleFactory from ietf.doc.factories import WgDraftFactory from ietf.utils.mail import empty_outbox, get_payload_text, outbox @@ -13,6 +15,7 @@ from .factories import ReviewAssignmentFactory, ReviewRequestFactory, ReviewerSettingsFactory from .mailarch import hash_list_message_id from .models import ReviewerSettings, ReviewSecretarySettings, ReviewTeamSettings, UnavailablePeriod +from .tasks import send_review_reminders_task from .utils import (email_secretary_reminder, review_assignments_needing_secretary_reminder, email_reviewer_reminder, review_assignments_needing_reviewer_reminder, send_reminder_unconfirmed_assignments, send_review_reminder_overdue_assignment, @@ -550,3 +553,66 @@ def test_review_add_comment(self): # But can't have the comment we are goint to add. self.assertContains(r, 'This is a test.') + +class TaskTests(TestCase): + # hyaaa it's mockzilla + @mock.patch("ietf.review.tasks.date_today") + @mock.patch("ietf.review.tasks.review_assignments_needing_reviewer_reminder") + @mock.patch("ietf.review.tasks.email_reviewer_reminder") + @mock.patch("ietf.review.tasks.review_assignments_needing_secretary_reminder") + @mock.patch("ietf.review.tasks.email_secretary_reminder") + @mock.patch("ietf.review.tasks.send_unavailability_period_ending_reminder") + @mock.patch("ietf.review.tasks.send_reminder_all_open_reviews") + @mock.patch("ietf.review.tasks.send_review_reminder_overdue_assignment") + @mock.patch("ietf.review.tasks.send_reminder_unconfirmed_assignments") + def test_send_review_reminders_task( + self, + mock_send_reminder_unconfirmed_assignments, + mock_send_review_reminder_overdue_assignment, + mock_send_reminder_all_open_reviews, + mock_send_unavailability_period_ending_reminder, + mock_email_secretary_reminder, + mock_review_assignments_needing_secretary_reminder, + mock_email_reviewer_reminder, + mock_review_assignments_needing_reviewer_reminder, + mock_date_today, + ): + """Test that send_review_reminders calls functions correctly + + Does not test individual methods, just that they are called as expected. + """ + mock_today = object() + assignment = ReviewAssignmentFactory() + secretary_role = RoleFactory(name_id="secr") + + mock_date_today.return_value = mock_today + mock_review_assignments_needing_reviewer_reminder.return_value = [assignment] + mock_review_assignments_needing_secretary_reminder.return_value = [[assignment, secretary_role]] + mock_send_unavailability_period_ending_reminder.return_value = ["pretending I sent a period end reminder"] + mock_send_review_reminder_overdue_assignment.return_value = ["pretending I sent an overdue reminder"] + mock_send_reminder_all_open_reviews.return_value = ["pretending I sent an open review reminder"] + mock_send_reminder_unconfirmed_assignments.return_value = ["pretending I sent an unconfirmed reminder"] + + send_review_reminders_task() + + self.assertEqual(mock_review_assignments_needing_reviewer_reminder.call_count, 1) + self.assertEqual(mock_review_assignments_needing_reviewer_reminder.call_args[0], (mock_today,)) + self.assertEqual(mock_email_reviewer_reminder.call_count, 1) + self.assertEqual(mock_email_reviewer_reminder.call_args[0], (assignment,)) + + self.assertEqual(mock_review_assignments_needing_secretary_reminder.call_count, 1) + self.assertEqual(mock_review_assignments_needing_secretary_reminder.call_args[0], (mock_today,)) + self.assertEqual(mock_email_secretary_reminder.call_count, 1) + self.assertEqual(mock_email_secretary_reminder.call_args[0], (assignment, secretary_role)) + + self.assertEqual(mock_send_unavailability_period_ending_reminder.call_count, 1) + self.assertEqual(mock_send_unavailability_period_ending_reminder.call_args[0], (mock_today,)) + + self.assertEqual(mock_send_review_reminder_overdue_assignment.call_count, 1) + self.assertEqual(mock_send_review_reminder_overdue_assignment.call_args[0], (mock_today,)) + + self.assertEqual(mock_send_reminder_all_open_reviews.call_count, 1) + self.assertEqual(mock_send_reminder_all_open_reviews.call_args[0], (mock_today,)) + + self.assertEqual(mock_send_reminder_unconfirmed_assignments.call_count, 1) + self.assertEqual(mock_send_reminder_unconfirmed_assignments.call_args[0], (mock_today,)) diff --git a/ietf/secr/rolodex/views.py b/ietf/secr/rolodex/views.py index 7dd8201f0c..9fd4a8b107 100644 --- a/ietf/secr/rolodex/views.py +++ b/ietf/secr/rolodex/views.py @@ -7,7 +7,6 @@ from ietf.ietfauth.utils import role_required from ietf.person.models import Person, Email, Alias -from ietf.person.utils import merge_users from ietf.secr.rolodex.forms import EditPersonForm, EmailForm, NameForm, NewPersonForm, SearchForm @@ -179,7 +178,6 @@ def edit(request, id): if 'user' in person_form.changed_data and person_form.initial['user']: try: source = User.objects.get(username__iexact=person_form.initial['user']) - merge_users(source, person_form.cleaned_data['user']) source.is_active = False source.save() except User.DoesNotExist: diff --git a/ietf/settings.py b/ietf/settings.py index eb5066e974..e3f7e7f5de 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2007-2023, All Rights Reserved +# Copyright The IETF Trust 2007-2024, All Rights Reserved # -*- coding: utf-8 -*- @@ -169,8 +169,8 @@ STATIC_URL = "/static/" STATIC_ROOT = os.path.abspath(BASE_DIR + "/../static/") else: - STATIC_URL = "https://www.ietf.org/lib/dt/%s/"%__version__ - STATIC_ROOT = "/a/www/www6s/lib/dt/%s/"%__version__ + STATIC_URL = "https://static.ietf.org/lib/%s/"%__version__ + # Intentionally not setting STATIC_ROOT - see django/django (the default is None) # List of finder classes that know how to find static files in # various locations. @@ -1153,6 +1153,12 @@ def skip_unreadable_post(record): CELERY_BEAT_SCHEDULER = 'django_celery_beat.schedulers:DatabaseScheduler' CELERY_BEAT_SYNC_EVERY = 1 # update DB after every event CELERY_BROKER_CONNECTION_RETRY_ON_STARTUP = True # the default, but setting it squelches a warning +# Use a result backend so we can chain tasks. This uses the rpc backend, see +# https://docs.celeryq.dev/en/stable/userguide/tasks.html#rpc-result-backend-rabbitmq-qpid +# Results can be retrieved only once and only by the caller of the task. Results will be +# lost if the message broker restarts. +CELERY_RESULT_BACKEND = 'rpc://' # sends a msg via the msg broker +CELERY_TASK_IGNORE_RESULT = True # ignore results unless specifically enabled for a task # Meetecho API setup: Uncomment this and provide real credentials to enable # Meetecho conference creation for interim session requests diff --git a/ietf/stats/management/commands/fetch_meeting_attendance.py b/ietf/stats/management/commands/fetch_meeting_attendance.py index 7f936531d4..82db6570ee 100644 --- a/ietf/stats/management/commands/fetch_meeting_attendance.py +++ b/ietf/stats/management/commands/fetch_meeting_attendance.py @@ -9,7 +9,7 @@ import debug # pyflakes:ignore from ietf.meeting.models import Meeting -from ietf.stats.utils import get_meeting_registration_data +from ietf.stats.utils import fetch_attendance_from_meetings logtag = __name__.split('.')[-1] logname = "user.log" @@ -36,11 +36,11 @@ def handle(self, *args, **options): else: raise CommandError("Please use one of --meeting, --all or --latest") - for meeting in meetings: - added, processed, total = get_meeting_registration_data(meeting) - msg = "Fetched data for meeting %3s: %4d processed, %4d added, %4d in table" % (meeting.number, processed, added, total) + for meeting, stats in zip(meetings, fetch_attendance_from_meetings(meetings)): + msg = "Fetched data for meeting {:>3}: {:4d} processed, {:4d} added, {:4d} in table".format( + meeting.number, stats.processed, stats.added, stats.total + ) if self.stdout.isatty(): self.stdout.write(msg+'\n') # make debugging a bit easier else: syslog.syslog(msg) - diff --git a/ietf/stats/tasks.py b/ietf/stats/tasks.py new file mode 100644 index 0000000000..808e797a40 --- /dev/null +++ b/ietf/stats/tasks.py @@ -0,0 +1,27 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# +# Celery task definitions +# +from celery import shared_task +from django.utils import timezone + +from ietf.meeting.models import Meeting +from ietf.stats.utils import fetch_attendance_from_meetings +from ietf.utils import log + + +@shared_task +def fetch_meeting_attendance_task(): + # fetch most recent two meetings + meetings = Meeting.objects.filter(type="ietf", date__lte=timezone.now()).order_by("-date")[:2] + try: + stats = fetch_attendance_from_meetings(meetings) + except RuntimeError as err: + log.log(f"Error in fetch_meeting_attendance_task: {err}") + else: + for meeting, meeting_stats in zip(meetings, stats): + log.log( + "Fetched data for meeting {:>3}: {:4d} processed, {:4d} added, {:4d} in table".format( + meeting.number, meeting_stats.processed, meeting_stats.added, meeting_stats.total + ) + ) diff --git a/ietf/stats/tests.py b/ietf/stats/tests.py index 5f23b1b0a0..f0e8a19c4a 100644 --- a/ietf/stats/tests.py +++ b/ietf/stats/tests.py @@ -29,7 +29,8 @@ from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory, ReviewAssignmentFactory from ietf.stats.models import MeetingRegistration, CountryAlias from ietf.stats.factories import MeetingRegistrationFactory -from ietf.stats.utils import get_meeting_registration_data +from ietf.stats.tasks import fetch_meeting_attendance_task +from ietf.stats.utils import get_meeting_registration_data, FetchStats, fetch_attendance_from_meetings from ietf.utils.timezone import date_today @@ -300,3 +301,48 @@ def test_get_meeting_registration_data_duplicates(self, mock_get): get_meeting_registration_data(meeting) query = MeetingRegistration.objects.all() self.assertEqual(query.count(), 2) + + @patch("ietf.stats.utils.get_meeting_registration_data") + def test_fetch_attendance_from_meetings(self, mock_get_mtg_reg_data): + mock_meetings = [object(), object(), object()] + mock_get_mtg_reg_data.side_effect = ( + (1, 2, 3), + (4, 5, 6), + (7, 8, 9), + ) + stats = fetch_attendance_from_meetings(mock_meetings) + self.assertEqual( + [mock_get_mtg_reg_data.call_args_list[n][0][0] for n in range(3)], + mock_meetings, + ) + self.assertEqual( + stats, + [ + FetchStats(1, 2, 3), + FetchStats(4, 5, 6), + FetchStats(7, 8, 9), + ] + ) + + +class TaskTests(TestCase): + @patch("ietf.stats.tasks.fetch_attendance_from_meetings") + def test_fetch_meeting_attendance_task(self, mock_fetch_attendance): + today = date_today() + meetings = [ + MeetingFactory(type_id="ietf", date=today - datetime.timedelta(days=1)), + MeetingFactory(type_id="ietf", date=today - datetime.timedelta(days=2)), + MeetingFactory(type_id="ietf", date=today - datetime.timedelta(days=3)), + ] + mock_fetch_attendance.return_value = [FetchStats(1,2,3), FetchStats(1,2,3)] + + fetch_meeting_attendance_task() + self.assertEqual(mock_fetch_attendance.call_count, 1) + self.assertCountEqual(mock_fetch_attendance.call_args[0][0], meetings[0:2]) + + # test handling of RuntimeError + mock_fetch_attendance.reset_mock() + mock_fetch_attendance.side_effect = RuntimeError + fetch_meeting_attendance_task() + self.assertTrue(mock_fetch_attendance.called) + # Good enough that we got here without raising an exception diff --git a/ietf/stats/utils.py b/ietf/stats/utils.py index 1f9c0e3c3a..2e418eb0eb 100644 --- a/ietf/stats/utils.py +++ b/ietf/stats/utils.py @@ -4,7 +4,7 @@ import re import requests -from collections import defaultdict +from collections import defaultdict, namedtuple from django.conf import settings from django.db.models import Q @@ -382,3 +382,13 @@ def find_meetingregistration_person_issues(meetings=None): summary.no_email.add(f'{mr} ({mr.pk}) provides no email address') return summary + + +FetchStats = namedtuple("FetchStats", "added processed total") + + +def fetch_attendance_from_meetings(meetings): + stats = [ + FetchStats(*get_meeting_registration_data(meeting)) for meeting in meetings + ] + return stats diff --git a/ietf/sync/rfceditor.py b/ietf/sync/rfceditor.py index a2f85f478e..8ae1354139 100644 --- a/ietf/sync/rfceditor.py +++ b/ietf/sync/rfceditor.py @@ -336,12 +336,12 @@ def extract_doc_list(parentNode, tagName): def update_docs_from_rfc_index( - index_data, errata_data, skip_older_than_date=None + index_data, errata_data, skip_older_than_date: Optional[datetime.date] = None ) -> Iterator[tuple[int, list[str], Document, bool]]: """Given parsed data from the RFC Editor index, update the documents in the database Returns an iterator that yields (rfc_number, change_list, doc, rfc_published) for the - RFC document and, if applicable, the I-D that it came from. + RFC document and, if applicable, the I-D that it came from. The skip_older_than_date is a bare date, not a datetime. """ @@ -405,7 +405,8 @@ def update_docs_from_rfc_index( abstract, ) in index_data: if skip_older_than_date and rfc_published_date < skip_older_than_date: - # speed up the process by skipping old entries + # speed up the process by skipping old entries (n.b., the comparison above is a + # lexical comparison between "YYYY-MM-DD"-formatted dates) continue # we assume two things can happen: we get a new RFC, or an @@ -462,6 +463,14 @@ def update_docs_from_rfc_index( doc.set_state(rfc_published_state) if draft: doc.formal_languages.set(draft.formal_languages.all()) + for author in draft.documentauthor_set.all(): + # Copy the author but point at the new doc. + # See https://docs.djangoproject.com/en/4.2/topics/db/queries/#copying-model-instances + author.pk = None + author.id = None + author._state.adding = True + author.document = doc + author.save() if draft: draft_events = [] diff --git a/ietf/sync/tasks.py b/ietf/sync/tasks.py new file mode 100644 index 0000000000..1e4cfe0772 --- /dev/null +++ b/ietf/sync/tasks.py @@ -0,0 +1,67 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +# +# Celery task definitions +# +import datetime +import io +import requests +from celery import shared_task + +from django.conf import settings + +from ietf.sync.rfceditor import MIN_ERRATA_RESULTS, MIN_INDEX_RESULTS, parse_index, update_docs_from_rfc_index +from ietf.utils import log +from ietf.utils.timezone import date_today + + +@shared_task +def rfc_editor_index_update_task(full_index=False): + """Update metadata from the RFC index + + Default is to examine only changes in the past 365 days. Call with full_index=True to update + the full RFC index. + + According to comments on the original script, a year's worth took about 20s on production as of + August 2022 + + The original rfc-editor-index-update script had a long-disabled provision for running the + rebuild_reference_relations scripts after the update. That has not been brought over + at all because it should be implemented as its own task if it is needed. + """ + skip_date = None if full_index else date_today() - datetime.timedelta(days=365) + log.log( + "Updating document metadata from RFC index going back to {since}, from {url}".format( + since=skip_date if skip_date is not None else "the beginning", + url=settings.RFC_EDITOR_INDEX_URL, + ) + ) + try: + response = requests.get( + settings.RFC_EDITOR_INDEX_URL, + timeout=30, # seconds + ) + except requests.Timeout as exc: + log.log(f'GET request timed out retrieving RFC editor index: {exc}') + return # failed + rfc_index_xml = response.text + index_data = parse_index(io.StringIO(rfc_index_xml)) + try: + response = requests.get( + settings.RFC_EDITOR_ERRATA_JSON_URL, + timeout=30, # seconds + ) + except requests.Timeout as exc: + log.log(f'GET request timed out retrieving RFC editor errata: {exc}') + return # failed + errata_data = response.json() + if len(index_data) < MIN_INDEX_RESULTS: + log.log("Not enough index entries, only %s" % len(index_data)) + return # failed + if len(errata_data) < MIN_ERRATA_RESULTS: + log.log("Not enough errata entries, only %s" % len(errata_data)) + return # failed + for rfc_number, changes, doc, rfc_published in update_docs_from_rfc_index( + index_data, errata_data, skip_older_than_date=skip_date + ): + for c in changes: + log.log("RFC%s, %s: %s" % (rfc_number, doc.name, c)) diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py index 6ac8f4afb0..6ce1d12521 100644 --- a/ietf/sync/tests.py +++ b/ietf/sync/tests.py @@ -8,19 +8,23 @@ import datetime import mock import quopri +import requests + +from dataclasses import dataclass from django.conf import settings from django.urls import reverse as urlreverse from django.utils import timezone +from django.test.utils import override_settings import debug # pyflakes:ignore -from ietf.doc.factories import WgDraftFactory, RfcFactory +from ietf.doc.factories import WgDraftFactory, RfcFactory, DocumentAuthorFactory from ietf.doc.models import Document, DocEvent, DeletedEvent, DocTagName, RelatedDocument, State, StateDocEvent from ietf.doc.utils import add_state_change_event from ietf.group.factories import GroupFactory from ietf.person.models import Person -from ietf.sync import iana, rfceditor +from ietf.sync import iana, rfceditor, tasks from ietf.utils.mail import outbox, empty_outbox from ietf.utils.test_utils import login_testing_unauthorized from ietf.utils.test_utils import TestCase @@ -235,6 +239,7 @@ def test_rfc_index(self): external_url="http://my-external-url.example.com", note="this is a note", ) + DocumentAuthorFactory.create_batch(2, document=draft_doc) draft_doc.action_holders.add(draft_doc.ad) # not normally set, but add to be sure it's cleared RfcFactory(rfc_number=123) @@ -378,6 +383,7 @@ def test_rfc_index(self): rfc_doc = Document.objects.filter(rfc_number=1234, type_id="rfc").first() self.assertIsNotNone(rfc_doc, "RFC document should have been created") + self.assertEqual(rfc_doc.authors(), draft_doc.authors()) rfc_events = rfc_doc.docevent_set.all() self.assertEqual(len(rfc_events), 8) expected_events = [ @@ -672,3 +678,129 @@ def test_rfceditor_undo(self): e.content_type.model_class().objects.create(**json.loads(e.json)) self.assertTrue(StateDocEvent.objects.filter(desc="First", doc=draft)) + + +class TaskTests(TestCase): + @override_settings( + RFC_EDITOR_INDEX_URL="https://rfc-editor.example.com/index/", + RFC_EDITOR_ERRATA_JSON_URL="https://rfc-editor.example.com/errata/", + ) + @mock.patch("ietf.sync.tasks.update_docs_from_rfc_index") + @mock.patch("ietf.sync.tasks.parse_index") + @mock.patch("ietf.sync.tasks.requests.get") + def test_rfc_editor_index_update_task( + self, requests_get_mock, parse_index_mock, update_docs_mock + ) -> None: # the annotation here prevents mypy from complaining about annotation-unchecked + """rfc_editor_index_update_task calls helpers correctly + + This tests that data flow is as expected. Assumes the individual helpers are + separately tested to function correctly. + """ + @dataclass + class MockIndexData: + """Mock index item that claims to be a specified length""" + length: int + + def __len__(self): + return self.length + + @dataclass + class MockResponse: + """Mock object that contains text and json() that claims to be a specified length""" + text: str + json_length: int = 0 + + def json(self): + return MockIndexData(length=self.json_length) + + # Response objects + index_response = MockResponse(text="this is the index") + errata_response = MockResponse( + text="these are the errata", json_length=rfceditor.MIN_ERRATA_RESULTS + ) + rfc = RfcFactory() + + # Test with full_index = False + requests_get_mock.side_effect = (index_response, errata_response) # will step through these + parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS) + update_docs_mock.return_value = ( + (rfc.rfc_number, ("something changed",), rfc, False), + ) + + tasks.rfc_editor_index_update_task(full_index=False) + + # Check parse_index() call + self.assertTrue(parse_index_mock.called) + (parse_index_args, _) = parse_index_mock.call_args + self.assertEqual( + parse_index_args[0].read(), # arg is a StringIO + "this is the index", + "parse_index is called with the index text in a StringIO", + ) + + # Check update_docs_from_rfc_index call + self.assertTrue(update_docs_mock.called) + (update_docs_args, update_docs_kwargs) = update_docs_mock.call_args + self.assertEqual( + update_docs_args, (parse_index_mock.return_value, errata_response.json()) + ) + self.assertIsNotNone(update_docs_kwargs["skip_older_than_date"]) + + # Test again with full_index = True + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = (index_response, errata_response) # will step through these + tasks.rfc_editor_index_update_task(full_index=True) + + # Check parse_index() call + self.assertTrue(parse_index_mock.called) + (parse_index_args, _) = parse_index_mock.call_args + self.assertEqual( + parse_index_args[0].read(), # arg is a StringIO + "this is the index", + "parse_index is called with the index text in a StringIO", + ) + + # Check update_docs_from_rfc_index call + self.assertTrue(update_docs_mock.called) + (update_docs_args, update_docs_kwargs) = update_docs_mock.call_args + self.assertEqual( + update_docs_args, (parse_index_mock.return_value, errata_response.json()) + ) + self.assertIsNone(update_docs_kwargs["skip_older_than_date"]) + + # Test error handling + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = requests.Timeout # timeout on every get() + tasks.rfc_editor_index_update_task(full_index=False) + self.assertFalse(parse_index_mock.called) + self.assertFalse(update_docs_mock.called) + + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = [index_response, requests.Timeout] # timeout second get() + tasks.rfc_editor_index_update_task(full_index=False) + self.assertFalse(update_docs_mock.called) + + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = [index_response, errata_response] + # feed in an index that is too short + parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS - 1) + tasks.rfc_editor_index_update_task(full_index=False) + self.assertTrue(parse_index_mock.called) + self.assertFalse(update_docs_mock.called) + + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = [index_response, errata_response] + errata_response.json_length = rfceditor.MIN_ERRATA_RESULTS - 1 # too short + parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS) + tasks.rfc_editor_index_update_task(full_index=False) + self.assertFalse(update_docs_mock.called) diff --git a/ietf/templates/community/list_menu.html b/ietf/templates/community/list_menu.html index d54695775f..0552c76a45 100644 --- a/ietf/templates/community/list_menu.html +++ b/ietf/templates/community/list_menu.html @@ -3,18 +3,18 @@
+ href="{% if clist.group %}{% url "ietf.community.views.feed" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.feed" email_or_name=clist.person.email_address %}{% endif %}"> All changes + href="{% if clist.group %}{% url "ietf.community.views.feed" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.feed" email_or_name=clist.person.email_address %}{% endif %}?significant=1"> Significant
{% if clist.pk != None %} + href="{% if clist.group %}{% url "ietf.community.views.subscription" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.subscription" email_or_name=clist.person.email_address %}{% endif %}"> {% if subscribed %} Change subscription @@ -24,7 +24,7 @@ {% endif %} + href="{% if clist.group %}{% url "ietf.community.views.export_to_csv" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.export_to_csv" email_or_name=clist.person.email_address %}{% endif %}"> Export as CSV - \ No newline at end of file + diff --git a/ietf/templates/community/view_list.html b/ietf/templates/community/view_list.html index 17650beb10..7ca9f52748 100644 --- a/ietf/templates/community/view_list.html +++ b/ietf/templates/community/view_list.html @@ -12,7 +12,7 @@

{{ clist.long_name }}

{% bootstrap_messages %} {% if can_manage_list %} + href="{% url "ietf.community.views.manage_list" email_or_name=clist.person.email_address %}"> Manage list @@ -22,4 +22,4 @@

{{ clist.long_name }}

{% endblock %} {% block js %} -{% endblock %} \ No newline at end of file +{% endblock %} diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 8d9abf284f..ccfb23f06a 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -699,21 +699,21 @@ {% if user.is_authenticated %} Untrack Track {% endif %} - {% if user.review_teams %} + {% if user.person.review_teams %} @@ -721,7 +721,7 @@ Remove review wishes - @@ -807,4 +807,4 @@ - {% endblock %} \ No newline at end of file + {% endblock %} diff --git a/ietf/templates/doc/document_rfc.html b/ietf/templates/doc/document_rfc.html index 36c9babd79..9e889afaff 100644 --- a/ietf/templates/doc/document_rfc.html +++ b/ietf/templates/doc/document_rfc.html @@ -129,14 +129,14 @@ {% if user.is_authenticated %} Untrack diff --git a/ietf/templates/doc/search/search_result_row.html b/ietf/templates/doc/search/search_result_row.html index c273153466..476c81f598 100644 --- a/ietf/templates/doc/search/search_result_row.html +++ b/ietf/templates/doc/search/search_result_row.html @@ -9,13 +9,13 @@ {% if user.is_authenticated %} - - @@ -23,14 +23,14 @@
{% endif %} - {% if user.review_teams %} + {% if user.person.review_teams %} - @@ -153,4 +153,4 @@ {% endif %} {% endif %} - \ No newline at end of file + diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py new file mode 100644 index 0000000000..5dc891cfb4 --- /dev/null +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -0,0 +1,139 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +import json +from django_celery_beat.models import CrontabSchedule, PeriodicTask + +from django.core.management.base import BaseCommand + +CRONTAB_DEFS = { + "daily": { + "minute": "5", + "hour": "0", + "day_of_week": "*", + "day_of_month": "*", + "month_of_year": "*", + }, + "hourly": { + "minute": "5", + "hour": "*", + "day_of_week": "*", + "day_of_month": "*", + "month_of_year": "*", + }, + "every_15m": { + "minute": "*/15", + "hour": "*", + "day_of_week": "*", + "day_of_month": "*", + "month_of_year": "*", + }, +} + + +class Command(BaseCommand): + """Manage periodic tasks""" + + def add_arguments(self, parser): + parser.add_argument("--create-default", action="store_true") + parser.add_argument("--enable", type=int, action="append") + parser.add_argument("--disable", type=int, action="append") + + def handle(self, *args, **options): + self.crontabs = self.get_or_create_crontabs() + if options["create_default"]: + self.create_default_tasks() + if options["enable"]: + self.enable_tasks(options["enable"]) + if options["disable"]: + self.disable_tasks(options["disable"]) + self.show_tasks() + + def get_or_create_crontabs(self): + crontabs = {} + for label, definition in CRONTAB_DEFS.items(): + crontabs[label], _ = CrontabSchedule.objects.get_or_create(**definition) + return crontabs + + def create_default_tasks(self): + PeriodicTask.objects.get_or_create( + name="Send scheduled mail", + task="ietf.message.tasks.send_scheduled_mail_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["every_15m"], + description="Send mail scheduled to go out at certain times" + ), + ) + + PeriodicTask.objects.get_or_create( + name="Partial sync with RFC Editor index", + task="ietf.sync.tasks.rfc_editor_index_update_task", + kwargs=json.dumps(dict(full_index=False)), + defaults=dict( + enabled=False, + crontab=self.crontabs["every_15m"], + description=( + "Reparse the last _year_ of RFC index entries until " + "https://github.com/ietf-tools/datatracker/issues/3734 is addressed. " + "This takes about 20s on production as of 2022-08-11." + ) + ), + ) + + PeriodicTask.objects.get_or_create( + name="Full sync with RFC Editor index", + task="ietf.sync.tasks.rfc_editor_index_update_task", + kwargs=json.dumps(dict(full_index=True)), + defaults=dict( + enabled=False, + crontab=self.crontabs["daily"], + description=( + "Run an extended version of the rfc editor update to catch changes with backdated timestamps" + ), + ), + ) + + PeriodicTask.objects.get_or_create( + name="Fetch meeting attendance", + task="ietf.stats.tasks.fetch_meeting_attendance_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["daily"], + description="Fetch meeting attendance data from ietf.org/registration/attendees", + ), + ) + + PeriodicTask.objects.get_or_create( + name="Send review reminders", + task="ietf.review.tasks.send_review_reminders_task", + defaults=dict( + enabled=False, + crontab=self.crontabs["daily"], + description="Send reminders originating from the review app", + ), + ) + + def show_tasks(self): + for label, crontab in self.crontabs.items(): + tasks = PeriodicTask.objects.filter(crontab=crontab).order_by( + "task", "name" + ) + self.stdout.write(f"\n{label} ({crontab.human_readable})\n") + if tasks: + for task in tasks: + desc = f" {task.id:-3d}: {task.task} - {task.name}" + if task.enabled: + self.stdout.write(desc) + else: + self.stdout.write(self.style.NOTICE(f"{desc} - disabled")) + else: + self.stdout.write(" Nothing scheduled") + + def enable_tasks(self, pks): + PeriodicTask.objects.filter( + crontab__in=self.crontabs.values(), pk__in=pks + ).update(enabled=True) + + def disable_tasks(self, pks): + PeriodicTask.objects.filter( + crontab__in=self.crontabs.values(), pk__in=pks + ).update(enabled=False) diff --git a/ietf/utils/management/commands/update_community_list_index.py b/ietf/utils/management/commands/update_community_list_index.py index 609577763e..7f4951fd6e 100644 --- a/ietf/utils/management/commands/update_community_list_index.py +++ b/ietf/utils/management/commands/update_community_list_index.py @@ -32,7 +32,7 @@ def handle(self, *args, **options): person = rule.person if not person and not group: try: - person = rule.community_list.user.person + person = rule.community_list.person except: pass name = ((group and group.acronym) or (person and person.email_address())) or '?' diff --git a/requirements.txt b/requirements.txt index 87d794663c..fd750ec704 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,7 +25,6 @@ django-simple-history>=3.0.0 django-stubs>=4.2.7 # The django-stubs version used determines the the mypy version indicated below django-tastypie>=0.14.5 # Version must be locked in sync with version of Django django-vite>=2.0.2,<3 -django-webtest>=1.9.10 # Only used in tests django-widget-tweaks>=1.4.12 djlint>=1.0.0 # To auto-indent templates via "djlint --profile django --reformat" docutils>=0.18.1 # Used only by dbtemplates for RestructuredText