From 5581dc79e1c6f6f07b680b7be47ec2d3811d8f39 Mon Sep 17 00:00:00 2001
From: Jennifer Richards <jennifer@staff.ietf.org>
Date: Tue, 24 Sep 2024 12:46:00 -0300
Subject: [PATCH] feat: doc mgrs can edit/remind action holders (#7971)
* feat: doc mgrs can edit/remind action holders
* test: test docman_roles, not "chair"
---
ietf/doc/tests.py | 12 ++++++++-
ietf/doc/tests_draft.py | 27 +++++++++++++++++---
ietf/doc/views_doc.py | 34 +++++++++++++++++++++++---
ietf/templates/doc/document_draft.html | 4 +--
4 files changed, 67 insertions(+), 10 deletions(-)
diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py
index f5f715162..e3534ba72 100644
--- a/ietf/doc/tests.py
+++ b/ietf/doc/tests.py
@@ -59,7 +59,7 @@ from ietf.meeting.models import Meeting, SessionPresentation, SchedulingEvent
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 @@ Man Expires September 22, 2015 [Page 3]
"""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 @@ Man Expires September 22, 2015 [Page 3]
_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)
diff --git a/ietf/doc/tests_draft.py b/ietf/doc/tests_draft.py
index cc22e2213..dcb71bdf6 100644
--- a/ietf/doc/tests_draft.py
+++ b/ietf/doc/tests_draft.py
@@ -26,7 +26,7 @@ from ietf.doc.models import ( Document, DocReminder, DocEvent,
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 @@ class IndividualInfoFormsTests(TestCase):
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 @@ class IndividualInfoFormsTests(TestCase):
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 @@ class IndividualInfoFormsTests(TestCase):
_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 @@ class IndividualInfoFormsTests(TestCase):
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 @@ class IndividualInfoFormsTests(TestCase):
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')
diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py
index aad45920c..02d59f0df 100644
--- a/ietf/doc/views_doc.py
+++ b/ietf/doc/views_doc.py
@@ -42,6 +42,7 @@ import re
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 edit_authors(request, name):
})
-@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():
diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html
index befbc759f..d5ea07442 100644
--- a/ietf/templates/doc/document_draft.html
+++ b/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">