feat: doc mgrs can edit/remind action holders (ietf-tools#7971)

* feat: doc mgrs can edit/remind action holders

* test: test docman_roles, not "chair"

Author: jennifer-richards

ietf/doc/tests.py

@@ -59,7 +59,7 @@
 from ietf.meeting.factories import ( MeetingFactory, SessionFactory, SessionPresentationFactory,
      ProceedingsMaterialFactory )
 
-from ietf.name.models import SessionStatusName, BallotPositionName, DocTypeName
+from ietf.name.models import SessionStatusName, BallotPositionName, DocTypeName, RoleName
 from ietf.person.models import Person
 from ietf.person.factories import PersonFactory, EmailFactory
 from ietf.utils.mail import outbox, empty_outbox
@@ -1450,6 +1450,14 @@ def test_document_draft_action_holders_buttons(self, mock_method):
         """Buttons for action holders should be shown when AD or secretary"""
         draft = WgDraftFactory()
         draft.action_holders.set([PersonFactory()])
+        other_group = GroupFactory(type_id=draft.group.type_id)
+
+        # create a test RoleName and put it in the docman_roles for the document group
+        RoleName.objects.create(slug="wrangler", name="Wrangler", used=True)
+        draft.group.features.docman_roles.append("wrangler")
+        draft.group.features.save()
+        wrangler = RoleFactory(group=draft.group, name_id="wrangler").person
+        wrangler_of_other_group = RoleFactory(group=other_group, name_id="wrangler").person
 
         url = urlreverse('ietf.doc.views_doc.document_main', kwargs=dict(name=draft.name))
         edit_ah_url = urlreverse('ietf.doc.views_doc.edit_action_holders', kwargs=dict(name=draft.name))
@@ -1482,6 +1490,8 @@ def _run_test(username=None, expect_buttons=False):
 
         _run_test(None, False)
         _run_test('plain', False)
+        _run_test(wrangler_of_other_group.user.username, False)
+        _run_test(wrangler.user.username, True)
         _run_test('ad', True)
         _run_test('secretary', True)
 

ietf/doc/tests_draft.py

@@ -26,7 +26,7 @@
     WriteupDocEvent, DocRelationshipName, IanaExpertDocEvent )
 from ietf.doc.utils import get_tags_for_stream_id, create_ballot_if_not_open
 from ietf.doc.views_draft import AdoptDraftForm
-from ietf.name.models import StreamName, DocTagName
+from ietf.name.models import StreamName, DocTagName, RoleName
 from ietf.group.factories import GroupFactory, RoleFactory
 from ietf.group.models import Group, Role
 from ietf.person.factories import PersonFactory, EmailFactory
@@ -935,6 +935,7 @@ def setUp(self):
         super().setUp()
         doc = WgDraftFactory(group__acronym='mars',shepherd=PersonFactory(user__username='plain',name='Plain Man').email_set.first())
         self.docname = doc.name
+        self.doc_group = doc.group
 
     def test_doc_change_stream(self):
         url = urlreverse('ietf.doc.views_draft.change_stream', kwargs=dict(name=self.docname))
@@ -1319,8 +1320,10 @@ def do_doc_change_action_holders_test(self, username):
         RoleFactory(name_id='techadv', person=PersonFactory(), group=doc.group)
         RoleFactory(name_id='editor', person=PersonFactory(), group=doc.group)
         RoleFactory(name_id='secr', person=PersonFactory(), group=doc.group)
-        
+        some_other_chair = RoleFactory(name_id="chair").person
+
         url = urlreverse('ietf.doc.views_doc.edit_action_holders', kwargs=dict(name=doc.name))
+        login_testing_unauthorized(self, some_other_chair.user.username, url)  # other chair can't edit action holders
         login_testing_unauthorized(self, username, url)
 
         r = self.client.get(url)
@@ -1363,6 +1366,14 @@ def _test_changing_ah(action_holders, reason):
         _test_changing_ah(doc.authors(), 'authors can do it, too')
         _test_changing_ah([], 'clear it back out')
 
+    def test_doc_change_action_holders_as_doc_manager(self):
+        # create a test RoleName and put it in the docman_roles for the document group
+        RoleName.objects.create(slug="wrangler", name="Wrangler", used=True)
+        self.doc_group.features.docman_roles.append("wrangler")
+        self.doc_group.features.save()
+        wrangler = RoleFactory(group=self.doc_group, name_id="wrangler").person
+        self.do_doc_change_action_holders_test(wrangler.user.username)
+
     def test_doc_change_action_holders_as_secretary(self):
         self.do_doc_change_action_holders_test('secretary')
 
@@ -1372,9 +1383,11 @@ def test_doc_change_action_holders_as_ad(self):
     def do_doc_remind_action_holders_test(self, username):
         doc = Document.objects.get(name=self.docname)
         doc.action_holders.set(PersonFactory.create_batch(3))
-        
+        some_other_chair = RoleFactory(name_id="chair").person
+    
         url = urlreverse('ietf.doc.views_doc.remind_action_holders', kwargs=dict(name=doc.name))
 
+        login_testing_unauthorized(self, some_other_chair.user.username, url)  # other chair can't send reminder
         login_testing_unauthorized(self, username, url)
         r = self.client.get(url)
         self.assertEqual(r.status_code, 200)
@@ -1401,6 +1414,14 @@ def do_doc_remind_action_holders_test(self, username):
         self.client.post(url)
         self.assertEqual(len(outbox), 1)  # still 1
 
+    def test_doc_remind_action_holders_as_doc_manager(self):
+        # create a test RoleName and put it in the docman_roles for the document group
+        RoleName.objects.create(slug="wrangler", name="Wrangler", used=True)
+        self.doc_group.features.docman_roles.append("wrangler")
+        self.doc_group.features.save()
+        wrangler = RoleFactory(group=self.doc_group, name_id="wrangler").person
+        self.do_doc_remind_action_holders_test(wrangler.user.username)
+
     def test_doc_remind_action_holders_as_ad(self):
         self.do_doc_remind_action_holders_test('ad')
 

ietf/doc/views_doc.py

@@ -42,6 +42,7 @@
 from pathlib import Path
 
 from django.core.cache import caches
+from django.core.exceptions import PermissionDenied
 from django.db.models import Max
 from django.http import HttpResponse, Http404, HttpResponseBadRequest
 from django.shortcuts import render, get_object_or_404, redirect
@@ -403,6 +404,10 @@ def document_main(request, name, rev=None, document_html=False):
 
         can_edit_replaces = has_role(request.user, ("Area Director", "Secretariat", "IRTF Chair", "WG Chair", "RG Chair", "WG Secretary", "RG Secretary"))
 
+        can_edit_action_holders = can_edit or (
+            request.user.is_authenticated and group.has_role(request.user, group.features.docman_roles)
+        )
+
         is_author = request.user.is_authenticated and doc.documentauthor_set.filter(person__user=request.user).exists()
         can_view_possibly_replaces = can_edit_replaces or is_author
 
@@ -660,6 +665,7 @@ def document_main(request, name, rev=None, document_html=False):
                                        can_edit_iana_state=can_edit_iana_state,
                                        can_edit_consensus=can_edit_consensus,
                                        can_edit_replaces=can_edit_replaces,
+                                       can_edit_action_holders=can_edit_action_holders,
                                        can_view_possibly_replaces=can_view_possibly_replaces,
                                        can_request_review=can_request_review,
                                        can_submit_unsolicited_review_for_teams=can_submit_unsolicited_review_for_teams,
@@ -1871,11 +1877,21 @@ def add_fields(self, form, index):
         })
 
 
-@role_required('Area Director', 'Secretariat')
+@login_required
 def edit_action_holders(request, name):
     """Change the set of action holders for a doc"""
     doc = get_object_or_404(Document, name=name)
-    
+
+    can_edit = has_role(request.user, ("Area Director", "Secretariat")) or (
+        doc.group and doc.group.has_role(request.user, doc.group.features.docman_roles)
+    )
+    if not can_edit:
+        # Keep the list of roles in this message up-to-date with the can_edit logic
+        message = "Restricted to roles: Area Director, Secretariat"
+        if doc.group and doc.group.acronym != "none":
+            message += f", and document managers for the {doc.group.acronym} group"
+        raise PermissionDenied(message)
+
     if request.method == 'POST':
         form = ActionHoldersForm(request.POST)
         if form.is_valid():
@@ -1985,10 +2001,20 @@ class ReminderEmailForm(forms.Form):
         strip=True,
     )
 
-@role_required('Area Director', 'Secretariat')
+@login_required
 def remind_action_holders(request, name):
     doc = get_object_or_404(Document, name=name)
-    
+
+    can_edit = has_role(request.user, ("Area Director", "Secretariat")) or (
+        doc.group and doc.group.has_role(request.user, doc.group.features.docman_roles)
+    )
+    if not can_edit:
+        # Keep the list of roles in this message up-to-date with the can_edit logic
+        message = "Restricted to roles: Area Director, Secretariat"
+        if doc.group and doc.group.acronym != "none":
+            message += f", and document managers for the {doc.group.acronym} group"
+        raise PermissionDenied(message)
+
     if request.method == 'POST':
         form = ReminderEmailForm(request.POST)
         if form.is_valid():

ietf/templates/doc/document_draft.html

@@ -304,7 +304,7 @@
                             Action Holder{{ doc.documentactionholder_set.all|pluralize }}
                         </th>
                         <td class="edit">
-                            {% if can_edit %}
+                            {% if can_edit_action_holders %}
                                 <a class="btn btn-primary btn-sm"
                                    href="{% url 'ietf.doc.views_doc.edit_action_holders' name=doc.name %}">
                                     Edit
@@ -319,7 +319,7 @@
                                             {% person_link action_holder.person title=action_holder.role_for_doc %} {{ action_holder|action_holder_badge }}
                                         </div>
                                     {% endfor %}
-                                    {% if can_edit %}
+                                    {% if can_edit_action_holders %}
                                         <a class="btn btn-primary btn-sm mt-3"
                                            href="{% url "ietf.doc.views_doc.remind_action_holders" name=doc.name %}">
                                             <i class="bi bi-envelope">