From d9cc26be96c78cb32b02939e14f789503e0d2d45 Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Wed, 24 Jan 2024 11:00:19 -0600 Subject: [PATCH] feat: replace references to User with references to Person (#6024) * refactor: change references from User to Person (#5821) * refactor: Change CommunityList reference from User to Person * refactor: Convert more user references to person * refactor: Change augment_docs_and_user_with_user_info to person * refactor: Change Nomination and Feedback references from User to Person * refactor: Change a few test case function signatures to be more pythonic * refactor: Harmonize how profile and photo views look up email_or_name * refactor: Rework community views to operate on Person instead of User (#5859) * test: Update tests to try all of the person's emails and aliases * fix: Recode a test case to avoid an exception if there's Unicode in the URL This only happens using the form-filling and submission feature of WebTest, which is only used in this one test case, so just it rip out. * test: Add duplicate-person tests * fix: If there are multiple matching users, prefer the logged-in one. * chore: We no longer use WebTest, so don't include it. * fix: Address review comments * fix: case-insensitive person name or email matching (#6096) * chore: Renumber migrations * fix: Update merged code so tests pass (#6887) * fix: Use refactored method * fix: Don't assume user has person * fix: Use new view param name * chore: Drop community lists w/o person; cleanup (#6896) * fix: Don't assume user has person * fix: user->person in update_community_list_index.py * feat: Remove CommunityLists without Person * refactor: Speed up nomcom migrations --------- Co-authored-by: Paul Selkirk Co-authored-by: Jennifer Richards --- ietf/api/views.py | 2 +- ietf/community/admin.py | 4 +- ietf/community/forms.py | 5 +- .../0004_delete_useless_community_lists.py | 26 ++ .../migrations/0005_user_to_person.py | 54 +++ ietf/community/models.py | 11 +- ietf/community/tests.py | 375 ++++++++++-------- ietf/community/urls.py | 14 +- ietf/community/utils.py | 22 +- ietf/community/views.py | 92 ++++- ietf/doc/utils.py | 31 +- ietf/doc/utils_search.py | 5 +- ietf/doc/views_doc.py | 8 +- ietf/nomcom/admin.py | 8 +- ietf/nomcom/factories.py | 6 +- ietf/nomcom/forms.py | 48 +-- ietf/nomcom/migrations/0005_user_to_person.py | 100 +++++ ietf/nomcom/models.py | 5 +- ietf/nomcom/resources.py | 12 +- ietf/nomcom/tests.py | 60 ++- ietf/nomcom/utils.py | 27 +- ietf/nomcom/views.py | 74 ++-- ietf/person/tests.py | 43 +- ietf/person/utils.py | 47 ++- ietf/person/views.py | 28 +- ietf/secr/rolodex/views.py | 2 - ietf/templates/community/list_menu.html | 10 +- ietf/templates/community/view_list.html | 4 +- ietf/templates/doc/document_draft.html | 10 +- ietf/templates/doc/document_rfc.html | 4 +- .../doc/search/search_result_row.html | 10 +- .../commands/update_community_list_index.py | 2 +- requirements.txt | 1 - 33 files changed, 691 insertions(+), 459 deletions(-) create mode 100644 ietf/community/migrations/0004_delete_useless_community_lists.py create mode 100644 ietf/community/migrations/0005_user_to_person.py create mode 100644 ietf/nomcom/migrations/0005_user_to_person.py diff --git a/ietf/api/views.py b/ietf/api/views.py index 73b873f5f..78e323684 100644 --- a/ietf/api/views.py +++ b/ietf/api/views.py @@ -89,7 +89,7 @@ class PersonalInformationExportView(DetailView, JsonExportMixin): '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/community/admin.py b/ietf/community/admin.py index 890819d9d..4c947ad3f 100644 --- a/ietf/community/admin.py +++ b/ietf/community/admin.py @@ -7,8 +7,8 @@ from django.contrib import admin 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 ad8570896..d3fa01dd1 100644 --- a/ietf/community/forms.py +++ b/ietf/community/forms.py @@ -114,14 +114,13 @@ class SearchRuleForm(forms.ModelForm): 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 000000000..9f657a3c3 --- /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 000000000..01d8950ed --- /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 9b2383f21..2e4003176 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.person.models import Person, Email 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 @@ class CommunityList(models.Model): 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 ee5827ee2..387877887 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.models import Group 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 @@ class CommunityListTests(WebTest): 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 @@ class CommunityListTests(WebTest): # 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 @@ class CommunityListTests(WebTest): 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) + 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}'") - # 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() + # 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') - self.assertContains(page, draft.name) + # 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.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, + # 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")) + }) + 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) - clist = CommunityList.objects.get(user__username="plain") - self.assertTrue(clist.searchrule_set.filter(rule_type="name_contains")) + # 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) - rule = clist.searchrule_set.filter(rule_type="author_rfc").first() - q = PyQuery(r.content) - self.assertEqual(len(q('#r%s' % rule.pk)), 1) + # 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 @@ class CommunityListTests(WebTest): 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]) - 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()), []) + # 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, 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) + # 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(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) + # 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 @@ class CommunityListTests(WebTest): 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) + # 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(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) + # 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 @@ class CommunityListTests(WebTest): 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 @@ class CommunityListTests(WebTest): 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 @@ class CommunityListTests(WebTest): # 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 f80547ffa..3ab132f2d 100644 --- a/ietf/community/urls.py +++ b/ietf/community/urls.py @@ -4,11 +4,11 @@ from ietf.community import views 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 f411af6a5..f23e8d26a 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 @@ import debug # pyflakes:ignore 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 fdaaffec0..a9d3bd584 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 @@ from django.utils.html import strip_tags 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) - existing_subscriptions = EmailSubscription.objects.filter(community_list=clist, email__person__user=request.user) + person = request.user.person + + 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/utils.py b/ietf/doc/utils.py index d93c9f453..46ecccc31 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 8e89139e3..59b64ad30 100644 --- a/ietf/doc/utils_search.py +++ b/ietf/doc/utils_search.py @@ -11,7 +11,7 @@ from django.conf import settings 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 293d32daf..64f9de4f5 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -62,7 +62,7 @@ from ietf.doc.utils import (augment_events_with_revision, 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/nomcom/admin.py b/ietf/nomcom/admin.py index 4b18cc005..1675d731a 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 @@ class FeedbackAdmin(admin.ModelAdmin): 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 7999228c2..286e0229a 100644 --- a/ietf/nomcom/factories.py +++ b/ietf/nomcom/factories.py @@ -9,7 +9,7 @@ from faker import Faker 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 NominationFactory(factory.django.DjangoModelFactory): 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 919ed6e18..7db500912 100644 --- a/ietf/nomcom/forms.py +++ b/ietf/nomcom/forms.py @@ -15,7 +15,7 @@ from ietf.name.models import FeedbackTypeName, NomineePositionStateName 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 @@ class NominateForm(forms.ModelForm): 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 @@ class NominateForm(forms.ModelForm): 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 @@ class NominateForm(forms.ModelForm): 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 @@ class NominateForm(forms.ModelForm): 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 @@ class NominateNewPersonForm(forms.ModelForm): 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 @@ class NominateNewPersonForm(forms.ModelForm): 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 @@ class NominateNewPersonForm(forms.ModelForm): 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 @@ class NominateNewPersonForm(forms.ModelForm): 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 @@ class FeedbackForm(forms.ModelForm): 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 @@ class FeedbackForm(forms.ModelForm): 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 @@ class FeedbackForm(forms.ModelForm): 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 @@ class QuestionnaireForm(forms.ModelForm): 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 PendingFeedbackForm(forms.ModelForm): 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 @@ class PendingFeedbackForm(forms.ModelForm): 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 MutableFeedbackForm(forms.ModelForm): 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 @@ class MutableFeedbackForm(forms.ModelForm): 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 000000000..66a6e9964 --- /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 b514f4fb1..2ed1124c5 100644 --- a/ietf/nomcom/models.py +++ b/ietf/nomcom/models.py @@ -7,7 +7,6 @@ import os 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 c87e72eae..109a13641 100644 --- a/ietf/nomcom/resources.py +++ b/ietf/nomcom/resources.py @@ -115,11 +115,11 @@ class NomineePositionResource(ModelResource): 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 FeedbackResource(ModelResource): "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 NominationResource(ModelResource): "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 d3da0bddd..cdbb860c4 100644 --- a/ietf/nomcom/tests.py +++ b/ietf/nomcom/tests.py @@ -689,20 +689,16 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class NomcomViewsTest(TestCase): 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 @@ class ReminderTest(TestCase): 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 @@ class AcceptingTests(TestCase): 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 220f2e401..23e89026d 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 7705be569..d34126b1e 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 view_feedback(request, year): 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 view_feedback(request, year): 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 bb75b438d..be3cfc056 100644 --- a/ietf/person/tests.py +++ b/ietf/person/tests.py @@ -25,11 +25,11 @@ from ietf.group.models import Group 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 @@ class PersonTests(TestCase): 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 @@ class PersonUtilsTests(TestCase): 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 @@ class PersonUtilsTests(TestCase): 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 942c2aaab..eb2742ed3 100755 --- a/ietf/person/utils.py +++ b/ietf/person/utils.py @@ -12,10 +12,11 @@ from django.contrib import admin 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 23a8dea3d..6d9daf4a8 100644 --- a/ietf/person/views.py +++ b/ietf/person/views.py @@ -8,16 +8,16 @@ from PIL import Image 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/secr/rolodex/views.py b/ietf/secr/rolodex/views.py index 7dd8201f0..9fd4a8b10 100644 --- a/ietf/secr/rolodex/views.py +++ b/ietf/secr/rolodex/views.py @@ -7,7 +7,6 @@ from django.urls import reverse 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/templates/community/list_menu.html b/ietf/templates/community/list_menu.html index d54695775..0552c76a4 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 17650beb1..7ca9f5274 100644 --- a/ietf/templates/community/view_list.html +++ b/ietf/templates/community/view_list.html @@ -12,7 +12,7 @@ {% 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 @@ {% 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 8d9abf284..ccfb23f06 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 36c9babd7..9e889afaf 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 c27315346..476c81f59 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/update_community_list_index.py b/ietf/utils/management/commands/update_community_list_index.py index 609577763..7f4951fd6 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 @@ class Command(BaseCommand): 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 87d794663..fd750ec70 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