diff --git a/ietf/community/models.py b/ietf/community/models.py index 91629dcf46..2e40031768 100644 --- a/ietf/community/models.py +++ b/ietf/community/models.py @@ -30,7 +30,7 @@ def __str__(self): def get_absolute_url(self): import ietf.community.views if self.person: - return urlreverse(ietf.community.views.view_list, kwargs={ 'username': self.person.user.username }) + 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 38e243f8d3..8d6662d155 100644 --- a/ietf/community/tests.py +++ b/ietf/community/tests.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 -*- @@ -6,8 +6,6 @@ from django.urls import reverse as urlreverse -from django_webtest import WebTest - import debug # pyflakes:ignore from ietf.community.models import CommunityList, SearchRule, EmailSubscription @@ -18,14 +16,14 @@ from ietf.group.utils import setup_default_community_list_for_group from ietf.doc.models import State from ietf.doc.utils import add_state_change_event -from ietf.person.models import Person, Email -from ietf.utils.test_utils import login_testing_unauthorized +from ietf.person.models import Person, Email, Alias +from ietf.utils.test_utils import TestCase, login_testing_unauthorized from ietf.utils.mail import outbox from ietf.doc.factories import WgDraftFactory from ietf.group.factories import GroupFactory, RoleFactory -from ietf.person.factories import PersonFactory +from ietf.person.factories import PersonFactory, EmailFactory, AliasFactory -class CommunityListTests(WebTest): +class CommunityListTests(TestCase): def test_rule_matching(self): plain = PersonFactory(user__username='plain') ad = Person.objects.get(user__username='ad') @@ -88,15 +86,35 @@ def test_rule_matching(self): # rule -> docs self.assertTrue(draft in list(docs_matching_community_list_rule(rule_group_exp))) + def test_view_list_duplicates(self): + person = PersonFactory(name="John Q. Public", user__username="bazquux@example.com") + PersonFactory(name="John Q. Public", user__username="foobar@example.com") + + url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": person.plain_name()}) + r = self.client.get(url) + self.assertEqual(r.status_code, 300) + self.assertIn("bazquux@example.com", r.content.decode()) + self.assertIn("foobar@example.com", r.content.decode()) + + def complex_person(self, *args, **kwargs): + person = PersonFactory(*args, **kwargs) + EmailFactory(person=person) + AliasFactory(person=person) + return person + + def email_or_name_set(self, person): + return [e for e in Email.objects.filter(person=person)] + \ + [a for a in Alias.objects.filter(person=person)] + def test_view_list(self): - person = 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(person=person) @@ -108,80 +126,87 @@ def test_view_list(self): state=State.objects.get(type="draft", slug="active"), text="test", ) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - self.assertContains(r, draft.name) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.view_list, kwargs={ "email_or_name": id }) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertContains(r, draft.name) def test_manage_personal_list(self): - - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') ad = Person.objects.get(user__username='ad') draft = WgDraftFactory(authors=[ad]) - url = urlreverse(ietf.community.views.manage_list, kwargs={ "username": "plain" }) + url = urlreverse(ietf.community.views.manage_list, kwargs={ "email_or_name": person.email() }) login_testing_unauthorized(self, "plain", url) - page = self.app.get(url, user='plain') - self.assertEqual(page.status_int, 200) - - # add document - self.assertIn('add_document', page.forms) - form = page.forms['add_document'] - form['documents'].options=[(draft.pk, True, draft.name)] - page = form.submit('action',value='add_documents') - self.assertEqual(page.status_int, 302) - clist = CommunityList.objects.get(person__user__username="plain") - self.assertTrue(clist.added_docs.filter(pk=draft.pk)) - page = page.follow() - - self.assertContains(page, draft.name) - - # remove document - self.assertIn('remove_document_%s' % draft.pk, page.forms) - form = page.forms['remove_document_%s' % draft.pk] - page = form.submit('action',value='remove_document') - self.assertEqual(page.status_int, 302) - clist = CommunityList.objects.get(person__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, - "author_rfc-state": State.objects.get(type="draft", slug="rfc").pk, - }) - self.assertEqual(r.status_code, 302) - 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(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) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.manage_list, kwargs={ "email_or_name": id }) + r = self.client.get(url, user='plain') + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + + # We can't call post() with follow=True because that 404's if + # the url contains unicode, because the django test client + # apparently re-encodes the already-encoded url. + def follow(r): + redirect_url = r.url or url + return self.client.get(redirect_url, user='plain') + + # add document + self.assertContains(r, 'add_document') + r = self.client.post(url, {'action': 'add_documents', 'documents': draft.pk}) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(clist.added_docs.filter(pk=draft.pk)) + r = follow(r) + self.assertContains(r, draft.name, status_code=200) + + # remove document + self.assertContains(r, 'remove_document_%s' % draft.pk) + r = self.client.post(url, {'action': 'remove_document', 'document': draft.pk}) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(not clist.added_docs.filter(pk=draft.pk)) + r = follow(r) + self.assertNotContains(r, draft.name, status_code=200) + + # add rule + r = self.client.post(url, { + "action": "add_rule", + "rule_type": "author_rfc", + "author_rfc-person": Person.objects.filter(documentauthor__document=draft).first().pk, + "author_rfc-state": State.objects.get(type="draft", slug="rfc").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="author_rfc")) + + # add name_contains rule + r = self.client.post(url, { + "action": "add_rule", + "rule_type": "name_contains", + "name_contains-text": "draft.*mars", + "name_contains-state": State.objects.get(type="draft", slug="active").pk, + }) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertTrue(clist.searchrule_set.filter(rule_type="name_contains")) + + # rule shows up on GET + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + rule = clist.searchrule_set.filter(rule_type="author_rfc").first() + q = PyQuery(r.content) + self.assertEqual(len(q('#r%s' % rule.pk)), 1) - # remove rule - r = self.client.post(url, { - "action": "remove_rule", - "rule": rule.pk, - }) + # remove rule + r = self.client.post(url, { + "action": "remove_rule", + "rule": rule.pk, + }) - clist = CommunityList.objects.get(person__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') @@ -209,77 +234,84 @@ def test_manage_group_list(self): self.assertEqual(r.status_code, 200) def test_track_untrack_document(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.track_document, kwargs={ "username": "plain", "name": draft.name }) + url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": person.email(), "name": draft.name }) login_testing_unauthorized(self, "plain", url) - # track - r = self.client.get(url) - self.assertEqual(r.status_code, 200) + for id in self.email_or_name_set(person): + url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": id, "name": draft.name }) - r = self.client.post(url) - self.assertEqual(r.status_code, 302) - clist = CommunityList.objects.get(person__user__username="plain") - self.assertEqual(list(clist.added_docs.all()), [draft]) + # track + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") - # untrack - url = urlreverse(ietf.community.views.untrack_document, kwargs={ "username": "plain", "name": draft.name }) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) + r = self.client.post(url) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertEqual(list(clist.added_docs.all()), [draft]) + + # untrack + url = urlreverse(ietf.community.views.untrack_document, kwargs={ "email_or_name": id, "name": draft.name }) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") - r = self.client.post(url) - self.assertEqual(r.status_code, 302) - clist = CommunityList.objects.get(person__user__username="plain") - self.assertEqual(list(clist.added_docs.all()), []) + r = self.client.post(url) + self.assertEqual(r.status_code, 302, msg=f"id='{id}', url='{url}'") + clist = CommunityList.objects.get(person__user__username="plain") + self.assertEqual(list(clist.added_docs.all()), []) def test_track_untrack_document_through_ajax(self): - PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.track_document, kwargs={ "username": "plain", "name": draft.name }) + url = urlreverse(ietf.community.views.track_document, kwargs={ "email_or_name": person.email(), "name": draft.name }) login_testing_unauthorized(self, "plain", url) - # track - r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') - self.assertEqual(r.status_code, 200) - self.assertEqual(r.json()["success"], True) - clist = CommunityList.objects.get(person__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(person__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): - person = PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.export_to_csv, 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.export_to_csv, kwargs={ "email_or_name": id }) - # 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) - # this is a simple-minded test, we don't actually check the fields - self.assertContains(r, draft.name) + # without list + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + + # with list + clist = CommunityList.objects.create(person=person) + if not draft in clist.added_docs.all(): + clist.added_docs.add(draft) + SearchRule.objects.create( + community_list=clist, + rule_type="name_contains", + state=State.objects.get(type="draft", slug="active"), + text="test", + ) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + # this is a simple-minded test, we don't actually check the fields + self.assertContains(r, draft.name) def test_csv_for_group(self): draft = WgDraftFactory() @@ -293,33 +325,34 @@ def test_csv_for_group(self): self.assertEqual(r.status_code, 200) def test_feed(self): - person = PersonFactory(user__username='plain') + person = self.complex_person(user__username='plain') draft = WgDraftFactory() - url = urlreverse(ietf.community.views.feed, 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.feed, kwargs={ "email_or_name": id }) - # 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) - self.assertContains(r, draft.name) + # without list + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + + # with list + clist = CommunityList.objects.create(person=person) + if not draft in clist.added_docs.all(): + clist.added_docs.add(draft) + SearchRule.objects.create( + community_list=clist, + rule_type="name_contains", + state=State.objects.get(type="draft", slug="active"), + text="test", + ) + r = self.client.get(url) + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertContains(r, draft.name) - # only significant - r = self.client.get(url + "?significant=1") - self.assertEqual(r.status_code, 200) - self.assertNotContains(r, '') + # only significant + r = self.client.get(url + "?significant=1") + self.assertEqual(r.status_code, 200, msg=f"id='{id}', url='{url}'") + self.assertNotContains(r, '') def test_feed_for_group(self): draft = WgDraftFactory() @@ -333,16 +366,18 @@ def test_feed_for_group(self): self.assertEqual(r.status_code, 200) def test_subscription(self): - person = 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(person=person) @@ -354,22 +389,25 @@ def test_subscription(self): state=State.objects.get(type="draft", slug="active"), text="test", ) - r = self.client.get(url) - self.assertEqual(r.status_code, 200) - # subscribe - email = Email.objects.filter(person__user__username="plain").first() - r = self.client.post(url, { "email": email.pk, "notify_on": "significant", "action": "subscribe" }) - self.assertEqual(r.status_code, 302) + for email in Email.objects.filter(person=person): + url = urlreverse(ietf.community.views.subscription, kwargs={ "email_or_name": email }) + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + # subscribe + r = self.client.post(url, { "email": email.pk, "notify_on": "significant", "action": "subscribe" }) + self.assertEqual(r.status_code, 302) - subscription = EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").first() + subscription = EmailSubscription.objects.filter(community_list=clist, email=email, notify_on="significant").first() - self.assertTrue(subscription) + 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) + # 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') @@ -384,7 +422,7 @@ def test_subscription_for_group(self): # test GET, rest is tested with personal list r = self.client.get(url) self.assertEqual(r.status_code, 200) - + def test_notification(self): person = PersonFactory(user__username='plain') draft = WgDraftFactory() diff --git a/ietf/community/urls.py b/ietf/community/urls.py index f80547ffad..3ab132f2dc 100644 --- a/ietf/community/urls.py +++ b/ietf/community/urls.py @@ -4,11 +4,11 @@ from ietf.utils.urls import url urlpatterns = [ - url(r'^personal/(?P[^/]+)/$', views.view_list), - url(r'^personal/(?P[^/]+)/manage/$', views.manage_list), - url(r'^personal/(?P[^/]+)/trackdocument/(?P[^/]+)/$', views.track_document), - url(r'^personal/(?P[^/]+)/untrackdocument/(?P[^/]+)/$', views.untrack_document), - url(r'^personal/(?P[^/]+)/csv/$', views.export_to_csv), - url(r'^personal/(?P[^/]+)/feed/$', views.feed), - url(r'^personal/(?P[^/]+)/subscription/$', views.subscription), + url(r'^personal/(?P[^/]+)/$', views.view_list), + url(r'^personal/(?P[^/]+)/manage/$', views.manage_list), + url(r'^personal/(?P[^/]+)/trackdocument/(?P[^/]+)/$', views.track_document), + url(r'^personal/(?P[^/]+)/untrackdocument/(?P[^/]+)/$', views.untrack_document), + url(r'^personal/(?P[^/]+)/csv/$', views.export_to_csv), + url(r'^personal/(?P[^/]+)/feed/$', views.feed), + url(r'^personal/(?P[^/]+)/subscription/$', views.subscription), ] diff --git a/ietf/community/utils.py b/ietf/community/utils.py index 5beb2e2f67..4de95c9e0b 100644 --- a/ietf/community/utils.py +++ b/ietf/community/utils.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2016-2020, All Rights Reserved +# Copyright The IETF Trust 2016-2023, All Rights Reserved # -*- coding: utf-8 -*- @@ -11,11 +11,9 @@ from ietf.community.models import CommunityList, EmailSubscription, SearchRule from ietf.doc.models import Document, State -from ietf.group.models import Role, Group +from ietf.group.models import Role from ietf.person.models import Person from ietf.ietfauth.utils import has_role -from django.contrib.auth.models import User -from django.shortcuts import get_object_or_404 from ietf.utils.mail import send_mail @@ -29,18 +27,6 @@ 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(person__user=user).first() or CommunityList(person=user.person) - - return clist - def can_manage_community_list(user, clist): if not user or not user.is_authenticated: return False diff --git a/ietf/community/views.py b/ietf/community/views.py index 85f78d931e..737094e006 100644 --- a/ietf/community/views.py +++ b/ietf/community/views.py @@ -1,4 +1,4 @@ -# Copyright The IETF Trust 2012-2020, All Rights Reserved +# Copyright The IETF Trust 2012-2023, All Rights Reserved # -*- coding: utf-8 -*- @@ -15,18 +15,46 @@ import debug # pyflakes:ignore -from ietf.community.models import SearchRule, EmailSubscription +from ietf.community.models import CommunityList, EmailSubscription, SearchRule from ietf.community.forms import SearchRuleTypeForm, SearchRuleForm, AddDocumentsForm, SubscriptionForm -from ietf.community.utils import lookup_community_list, can_manage_community_list +from ietf.community.utils import can_manage_community_list from ietf.community.utils import docs_tracked_by_community_list, docs_matching_community_list_rule from ietf.community.utils import states_of_significant_change, reset_name_contains_index_for_rule +from ietf.group.models import Group from ietf.doc.models import DocEvent, Document from ietf.doc.utils_search import prepare_document_table +from ietf.person.utils import lookup_persons from ietf.utils.http import is_ajax from ietf.utils.response import permission_denied -def view_list(request, username=None): - clist = lookup_community_list(username) +class MultiplePersonError(Exception): + """More than one Person record matches the given email or name""" + pass + +def lookup_community_list(request, email_or_name=None, acronym=None): + assert email_or_name or acronym + + if acronym: + group = get_object_or_404(Group, acronym=acronym) + clist = CommunityList.objects.filter(group=group).first() or CommunityList(group=group) + else: + persons = lookup_persons(email_or_name) + if len(persons) > 1: + if hasattr(request.user, 'person') and request.user.person in persons: + person = request.user.person + else: + raise MultiplePersonError("\r\n".join([p.user.username for p in persons])) + else: + person = persons[0] + clist = CommunityList.objects.filter(person=person).first() or CommunityList(person=person) + + return clist + +def view_list(request, email_or_name=None): + try: + clist = lookup_community_list(request, email_or_name) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) docs = docs_tracked_by_community_list(clist) docs, meta = prepare_document_table(request, docs, request.GET) @@ -42,10 +70,13 @@ def view_list(request, username=None): }) @login_required -def manage_list(request, username=None, acronym=None, group_type=None): +def manage_list(request, email_or_name=None, acronym=None): # we need to be a bit careful because clist may not exist in the # database so we can't call related stuff on it yet - clist = lookup_community_list(username, acronym) + try: + clist = lookup_community_list(request, email_or_name, acronym) + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) if not can_manage_community_list(request.user, clist): permission_denied(request, "You do not have permission to access this view") @@ -129,11 +160,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, docalias__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") @@ -153,9 +187,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, docalias__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") @@ -173,8 +210,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') @@ -214,8 +254,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' @@ -250,10 +293,13 @@ def feed(request, username=None, acronym=None, group_type=None): @login_required -def subscription(request, username=None, acronym=None, group_type=None): - clist = lookup_community_list(username, acronym) - if clist.pk is None: - raise Http404 +def subscription(request, email_or_name=None, acronym=None): + try: + clist = lookup_community_list(request, email_or_name, acronym) + if clist.pk is None: + raise Http404 + except MultiplePersonError as err: + return HttpResponse(str(err), status=300) person = request.user.person diff --git a/ietf/person/tests.py b/ietf/person/tests.py index a91527ea66..5d95a05965 100644 --- a/ietf/person/tests.py +++ b/ietf/person/tests.py @@ -157,6 +157,16 @@ def test_person_photo(self): img = Image.open(BytesIO(r.content)) self.assertEqual(img.width, 200) + def test_person_photo_duplicates(self): + person = PersonFactory(name="bazquux@example.com", user__username="bazquux@example.com", with_bio=True) + PersonFactory(name="bazquux@example.com", user__username="foobar@example.com", with_bio=True) + + url = urlreverse("ietf.person.views.photo", kwargs={ "email_or_name": person.plain_name()}) + r = self.client.get(url) + self.assertEqual(r.status_code, 300) + self.assertIn("bazquux@example.com", r.content.decode()) + self.assertIn("foobar@example.com", r.content.decode()) + def test_name_methods(self): person = PersonFactory(name="Dr. Jens F. Möller", ) diff --git a/ietf/person/utils.py b/ietf/person/utils.py index a28aaaefa9..5c97eda42e 100755 --- a/ietf/person/utils.py +++ b/ietf/person/utils.py @@ -12,10 +12,11 @@ from django.core.cache import cache from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q +from django.http import Http404 import debug # pyflakes:ignore -from ietf.person.models import Person +from ietf.person.models import Person, Alias, Email from ietf.utils.mail import send_mail def merge_persons(request, source, target, file=sys.stdout, verbose=False): @@ -246,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=email_or_name) + persons = set(a.person for a in aliases) + + if '@' in email_or_name: + emails = Email.objects.filter(address=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 4e3f7d8c30..6d9daf4a81 100644 --- a/ietf/person/views.py +++ b/ietf/person/views.py @@ -8,16 +8,16 @@ from django.contrib import messages from django.db.models import Q from django.http import HttpResponse, Http404 -from django.shortcuts import render, get_object_or_404, redirect +from django.shortcuts import render, redirect from django.utils import timezone import debug # pyflakes:ignore from ietf.ietfauth.utils import role_required -from ietf.person.models import Email, Person, Alias +from ietf.person.models import Email, Person from ietf.person.fields import select2_id_name_json from ietf.person.forms import MergeForm -from ietf.person.utils import handle_users, merge_persons +from ietf.person.utils import handle_users, merge_persons, lookup_persons def ajax_select2_search(request, model_name): @@ -69,30 +69,14 @@ def ajax_select2_search(request, model_name): def profile(request, email_or_name): - aliases = Alias.objects.filter(name=email_or_name) - persons = set(a.person for a in aliases) - - if '@' in email_or_name: - emails = Email.objects.filter(address=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/templates/community/list_menu.html b/ietf/templates/community/list_menu.html index 58269dccb4..0552c76a45 100644 --- a/ietf/templates/community/list_menu.html +++ b/ietf/templates/community/list_menu.html @@ -3,18 +3,18 @@
+ href="{% if clist.group %}{% url "ietf.community.views.feed" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.feed" email_or_name=clist.person.email_address %}{% endif %}"> All changes + href="{% if clist.group %}{% url "ietf.community.views.feed" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.feed" email_or_name=clist.person.email_address %}{% endif %}?significant=1"> Significant
{% if clist.pk != None %} + href="{% if clist.group %}{% url "ietf.community.views.subscription" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.subscription" email_or_name=clist.person.email_address %}{% endif %}"> {% if subscribed %} Change subscription @@ -24,7 +24,7 @@ {% endif %} + href="{% if clist.group %}{% url "ietf.community.views.export_to_csv" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.export_to_csv" email_or_name=clist.person.email_address %}{% endif %}"> Export as CSV diff --git a/ietf/templates/community/view_list.html b/ietf/templates/community/view_list.html index 81f6d37454..7ca9f52748 100644 --- a/ietf/templates/community/view_list.html +++ b/ietf/templates/community/view_list.html @@ -12,7 +12,7 @@

{{ clist.long_name }}

{% bootstrap_messages %} {% if can_manage_list %} + href="{% url "ietf.community.views.manage_list" email_or_name=clist.person.email_address %}"> Manage list diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 5db93a748c..a5d9ef7790 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -712,14 +712,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 5f956cb602..8b90934270 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 %} - - diff --git a/requirements.txt b/requirements.txt index 86b4e8c6d8..f6d5bdc6f2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -25,7 +25,6 @@ django-simple-history>=3.0.0 django-stubs>=4.2.0 # 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 -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