diff --git a/ietf/doc/tests.py b/ietf/doc/tests.py index f5f715162d..e3534ba725 100644 --- a/ietf/doc/tests.py +++ b/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) diff --git a/ietf/doc/tests_draft.py b/ietf/doc/tests_draft.py index cc22e22139..dcb71bdf69 100644 --- a/ietf/doc/tests_draft.py +++ b/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') diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index aad45920c6..02d59f0df3 100644 --- a/ietf/doc/views_doc.py +++ b/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(): diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index befbc759f1..d5ea074422 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 }} - {% if can_edit %} + {% if can_edit_action_holders %} Edit @@ -319,7 +319,7 @@ {% person_link action_holder.person title=action_holder.role_for_doc %} {{ action_holder|action_holder_badge }} {% endfor %} - {% if can_edit %} + {% if can_edit_action_holders %}