refactor: Rework community views to operate on Person instead of User

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 ""

ietf/community/urls.py

@@ -4,11 +4,11 @@
 from ietf.utils.urls import url
 
 urlpatterns = [
-    url(r'^personal/(?P<username>[^/]+)/$', views.view_list),
-    url(r'^personal/(?P<username>[^/]+)/manage/$', views.manage_list),
-    url(r'^personal/(?P<username>[^/]+)/trackdocument/(?P<name>[^/]+)/$', views.track_document),
-    url(r'^personal/(?P<username>[^/]+)/untrackdocument/(?P<name>[^/]+)/$', views.untrack_document),
-    url(r'^personal/(?P<username>[^/]+)/csv/$', views.export_to_csv),
-    url(r'^personal/(?P<username>[^/]+)/feed/$', views.feed),
-    url(r'^personal/(?P<username>[^/]+)/subscription/$', views.subscription),
+    url(r'^personal/(?P<email_or_name>[^/]+)/$', views.view_list),
+    url(r'^personal/(?P<email_or_name>[^/]+)/manage/$', views.manage_list),
+    url(r'^personal/(?P<email_or_name>[^/]+)/trackdocument/(?P<name>[^/]+)/$', views.track_document),
+    url(r'^personal/(?P<email_or_name>[^/]+)/untrackdocument/(?P<name>[^/]+)/$', views.untrack_document),
+    url(r'^personal/(?P<email_or_name>[^/]+)/csv/$', views.export_to_csv),
+    url(r'^personal/(?P<email_or_name>[^/]+)/feed/$', views.feed),
+    url(r'^personal/(?P<email_or_name>[^/]+)/subscription/$', views.subscription),
 ]

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 -*-
 
 
@@ -13,8 +13,8 @@
 from ietf.doc.models import Document, State
 from ietf.group.models import Role, Group
 from ietf.person.models import Person
+from ietf.person.utils import lookup_persons
 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,15 +29,26 @@ def states_of_significant_change():
         Q(type="draft", slug__in=['rfc', 'dead'])
     )
 
-def lookup_community_list(username=None, acronym=None):
-    assert username or acronym
+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:
-        user = get_object_or_404(User, username__iexact=username)
-        clist = CommunityList.objects.filter(person__user=user).first() or CommunityList(person=user.person)
+        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
 

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 -*-
 
 
@@ -20,13 +20,17 @@
 from ietf.community.utils import lookup_community_list, 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.community.utils import MultiplePersonError
 from ietf.doc.models import DocEvent, Document
 from ietf.doc.utils_search import prepare_document_table
 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)
+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 +46,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 +136,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 +163,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 +186,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 +230,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 +269,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
 

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="[email protected]", user__username="[email protected]", with_bio=True)
+        PersonFactory(name="[email protected]", user__username="[email protected]", 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("[email protected]", r.content.decode())
+        self.assertIn("[email protected]", r.content.decode())
+
     def test_name_methods(self):
         person = PersonFactory(name="Dr. Jens F. Möller", )
 

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

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")

ietf/templates/community/list_menu.html

@@ -3,18 +3,18 @@
     <div class="btn-group" role="group" aria-labelledby="list-feeds">
         <a class="btn btn-primary"
            title="Feed of all changes"
-           href="{% if clist.group %}{% url "ietf.community.views.feed" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.feed" username=clist.person.user.username %}{% endif %}">
+           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 %}">
             <i class="bi bi-rss"></i> All changes
         </a>
         <a class="btn btn-primary"
            title="Feed of only significant state changes"
-           href="{% if clist.group %}{% url "ietf.community.views.feed" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.feed" username=clist.person.user.username %}{% endif %}?significant=1">
+           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">
             <i class="bi bi-rss"></i> Significant
         </a>
     </div>
     {% if clist.pk != None %}
         <a class="btn btn-primary"
-           href="{% if clist.group %}{% url "ietf.community.views.subscription" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.subscription" username=clist.person.user.username %}{% endif %}">
+           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 %}">
             <i class="bi bi-envelope"></i>
             {% if subscribed %}
                 Change subscription
@@ -24,7 +24,7 @@
         </a>
     {% endif %}
     <a class="btn btn-primary"
-       href="{% if clist.group %}{% url "ietf.community.views.export_to_csv" acronym=clist.group.acronym %}{% else %}{% url "ietf.community.views.export_to_csv" username=clist.person.user.username %}{% 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 %}">
         <i class="bi bi-file-ruled"></i> Export as CSV
     </a>
 </div>

ietf/templates/community/view_list.html

@@ -12,7 +12,7 @@ <h1>{{ clist.long_name }}</h1>
     {% bootstrap_messages %}
     {% if can_manage_list %}
         <a class="btn btn-primary my-3"
-           href="{% url "ietf.community.views.manage_list" username=clist.person.user.username %}">
+           href="{% url "ietf.community.views.manage_list" email_or_name=clist.person.email_address %}">
             <i class="bi bi-gear"></i>
             Manage list
         </a>

ietf/templates/doc/document_draft.html

@@ -712,14 +712,14 @@
             </div>
             {% if user.is_authenticated %}
                 <a class="btn btn-primary btn-sm track-untrack-doc {% if not doc.tracked_in_personal_community_list %}hide{% endif %}"
-                   href="{% url "ietf.community.views.untrack_document" username=user.username name=doc.name %}"
+                   href="{% url "ietf.community.views.untrack_document" email_or_name=user.username name=doc.name %}"
                    title="Remove from your personal I-D list">
                     <i class="bi bi-bookmark-check-fill">
                     </i>
                     Untrack
                 </a>
                 <a class="btn btn-primary btn-sm track-untrack-doc {% if doc.tracked_in_personal_community_list %}hide{% endif %}"
-                   href="{% url "ietf.community.views.track_document" username=user.username name=doc.name %}"
+                   href="{% url "ietf.community.views.track_document" email_or_name=user.username name=doc.name %}"
                    title="Add to your personal I-D list">
                     <i class="bi bi-bookmark">
                     </i>

ietf/templates/doc/search/search_result_row.html

@@ -9,13 +9,13 @@
 <tr {% if color_ad_position %}{% with doc|ballotposition:user as pos %}{% if pos %}class="position-{{ pos.slug }}-row"{% endif %}{% endwith %}{% endif %}>
     <td>
         {% if user.is_authenticated %}
-            <a href="{% url "ietf.community.views.untrack_document" username=request.user.username name=doc.name %}"
+            <a href="{% url "ietf.community.views.untrack_document" email_or_name=request.user.username name=doc.name %}"
                class="track-untrack-doc {% if not doc.tracked_in_personal_community_list %}d-none{% endif %}"
                aria-label="Remove from your personal I-D list"
                title="Remove from your personal I-D list">
                 <i class="bi bi-bookmark-check-fill"></i>
             </a>
-            <a href="{% url "ietf.community.views.track_document" username=request.user.username name=doc.name %}"
+            <a href="{% url "ietf.community.views.track_document" email_or_name=request.user.username name=doc.name %}"
                class="track-untrack-doc {% if doc.tracked_in_personal_community_list %}d-none{% endif %}"
                aria-label="Add to your personal I-D list"
                title="Add to your personal I-D list">

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