Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: doc mgrs can edit/remind action holders #7971

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion ietf/doc/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)

Expand Down
27 changes: 24 additions & 3 deletions ietf/doc/tests_draft.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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')

Expand All @@ -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)
Expand All @@ -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')

Expand Down
34 changes: 30 additions & 4 deletions ietf/doc/views_doc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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():
Expand Down
4 changes: 2 additions & 2 deletions ietf/templates/doc/document_draft.html
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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">
Expand Down
Loading