Skip to content

Commit c606461

Browse files
levkowetzlevkowetz
committed
Merged in [16924] from sasha@dashcare.nl:
 Fix ietf-tools#2217 - Allow submission of unsolicited reviews by secretaries. - For team secretaries, a button 'Submit unsolicited review' will now appear next to 'Request review' on the document's main page. - If the secretary is a secretary for multiple teams, they are taken through an intermediate page to select for which team they are submitting their review. - The form is similar (and using the same code) as the usual review completion, with a few extra fields for the review type and reviewer, which would usually already be known. - When submitting the review, a ReviewRequest and ReviewAssignment are automatically created. The assignment is then immediately closed in the usual way. - Other workflows are unchanged. The issues with the review form in ietf-tools#2061 are slightly worse for the unsolicited review scenario, but that will be improved when ietf-tools#2061 is fixed. - Legacy-Id: 16932 Note: SVN reference [16924] has been migrated to Git commit 871a4b6
2 parents 00fb8d4 + 871a4b6 commit c606461

8 files changed

Lines changed: 285 additions & 67 deletions

File tree

‎ietf/doc/tests_review.py‎

Lines changed: 82 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
from ietf.group.factories import RoleFactory, ReviewTeamFactory
2828
from ietf.group.models import Group
2929
from ietf.message.models import Message
30-
from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName
30+
from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName, \
31+
ReviewTypeName
3132
from ietf.person.models import Email, Person
3233
from ietf.review.factories import ReviewRequestFactory, ReviewAssignmentFactory
3334
from ietf.review.models import (ReviewRequest, ReviewerSettings,
@@ -564,7 +565,7 @@ def test_search_mail_archive(self):
564565
assignment = ReviewAssignmentFactory(review_request=review_req, reviewer=rev_role.person.email_set.first(), state_id='accepted')
565566

566567
# test URL construction
567-
query_urls = ietf.review.mailarch.construct_query_urls(review_req)
568+
query_urls = ietf.review.mailarch.construct_query_urls(doc, review_team)
568569
self.assertTrue(review_req.doc.name in query_urls["query_data_url"])
569570

570571
# test parsing
@@ -575,16 +576,21 @@ def test_search_mail_archive(self):
575576
# to work, the module (and not the function) must be
576577
# imported in the view
577578
real_fn = ietf.review.mailarch.construct_query_urls
578-
ietf.review.mailarch.construct_query_urls = lambda review_req, query=None: { "query_data_url": "file://" + os.path.abspath(mbox_path) }
579-
579+
ietf.review.mailarch.construct_query_urls = lambda doc, team, query=None: { "query_data_url": "file://" + os.path.abspath(mbox_path) }
580580
url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "assignment_id": assignment.pk })
581+
url2 = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "acronym": review_team.acronym })
581582
login_testing_unauthorized(self, "reviewsecretary", url)
582583

583584
r = self.client.get(url)
584585
self.assertEqual(r.status_code, 200)
585586
messages = r.json()["messages"]
586587
self.assertEqual(len(messages), 2)
587588

589+
r = self.client.get(url2)
590+
self.assertEqual(r.status_code, 200)
591+
messages = r.json()["messages"]
592+
self.assertEqual(len(messages), 2)
593+
588594
today = datetime.date.today()
589595

590596
self.assertEqual(messages[0]["url"], "https://www.example.com/testmessage")
@@ -605,7 +611,7 @@ def test_search_mail_archive(self):
605611
no_result_path = os.path.join(self.review_dir, "mailarch_no_result.html")
606612
with io.open(no_result_path, "w") as f:
607613
f.write('Content-Type: text/html\n\n<html><body><div class="xtr"><div class="xtd no-results">No results found</div></div>')
608-
ietf.review.mailarch.construct_query_urls = lambda review_req, query=None: { "query_data_url": "file://" + os.path.abspath(no_result_path) }
614+
ietf.review.mailarch.construct_query_urls = lambda doc, team, query=None: { "query_data_url": "file://" + os.path.abspath(no_result_path) }
609615

610616
url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "assignment_id": assignment.pk })
611617

@@ -618,6 +624,27 @@ def test_search_mail_archive(self):
618624
finally:
619625
ietf.review.mailarch.construct_query_urls = real_fn
620626

627+
def test_submit_unsolicited_review_choose_team(self):
628+
doc = WgDraftFactory(group__acronym='mars', rev='01')
629+
review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
630+
secretary = RoleFactory(group=review_team,person__user__username='reviewsecretary',person__user__email='reviewsecretary@example.com', name_id='secr')
631+
632+
url = urlreverse('ietf.doc.views_review.submit_unsolicited_review_choose_team',
633+
kwargs={'name': doc.name})
634+
redirect_url = urlreverse("ietf.doc.views_review.complete_review",
635+
kwargs={'name': doc.name, 'acronym': review_team.acronym})
636+
login_testing_unauthorized(self, secretary.person.user.username, url)
637+
638+
r = self.client.get(url)
639+
self.assertEqual(r.status_code, 200)
640+
self.assertContains(r, review_team.name)
641+
642+
r = self.client.post(url, data={'team': review_team.pk})
643+
self.assertRedirects(r, redirect_url)
644+
645+
r = self.client.post(url, data={'team': review_team.pk + 5})
646+
self.assertEqual(r.status_code, 200)
647+
621648
def setup_complete_review_test(self):
622649
doc = WgDraftFactory(group__acronym='mars',rev='01')
623650
NewRevisionDocEventFactory(doc=doc,rev='01')
@@ -873,6 +900,56 @@ def test_complete_review_link_to_mailing_list(self, mock):
873900
self.assertEqual(len(outbox), 0)
874901
self.assertTrue("http://example.com" in assignment.review.external_url)
875902

903+
@patch('requests.get')
904+
def test_complete_unsolicited_review_link_to_mailing_list_by_secretary(self, mock):
905+
# Mock up the url response for the request.get() call to retrieve the mailing list url
906+
response = Response()
907+
response.status_code = 200
908+
response._content = b"This is a review\nwith two lines"
909+
mock.return_value = response
910+
911+
# Run the test
912+
doc = WgDraftFactory(group__acronym='mars', rev='01')
913+
NewRevisionDocEventFactory(doc=doc, rev='01')
914+
review_team = ReviewTeamFactory(acronym="reviewteam", name="Review Team", type_id="review", list_email="reviewteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
915+
rev_role = RoleFactory(group=review_team, person__user__username='reviewer', person__user__email='reviewer@example.com', name_id='reviewer')
916+
secretary_role = RoleFactory(group=review_team, person__user__username='reviewsecretary', person__user__email='reviewsecretary@example.com', name_id='secr')
917+
918+
url = urlreverse('ietf.doc.views_review.complete_review',
919+
kwargs={"name": doc.name, "acronym": review_team.acronym})
920+
921+
login_testing_unauthorized(self, secretary_role.person.user.username, url)
922+
923+
empty_outbox()
924+
925+
r = self.client.post(url, data={
926+
"result": ReviewResultName.objects.get(slug="ready").pk,
927+
"state": ReviewAssignmentStateName.objects.get(slug="completed").pk,
928+
"reviewed_rev": '01',
929+
"review_submission": "link",
930+
"review_content": response.content.decode(),
931+
"review_url": "http://example.com/testreview/",
932+
"review_file": "",
933+
"review_type": ReviewTypeName.objects.get(slug="early").pk,
934+
"reviewer": rev_role.person.pk,
935+
"completion_date": "2012-12-24",
936+
"completion_time": "12:13:14",
937+
})
938+
self.assertEqual(r.status_code, 302)
939+
940+
review_req = doc.reviewrequest_set.get()
941+
assignment = review_req.reviewassignment_set.get()
942+
self.assertEqual(review_req.type_id, "early")
943+
self.assertEqual(review_req.requested_by, secretary_role.person)
944+
self.assertEqual(assignment.reviewer, rev_role.person.role_email('reviewer'))
945+
self.assertEqual(assignment.state_id, "completed")
946+
947+
with io.open(os.path.join(self.review_subdir, assignment.review.name + ".txt")) as f:
948+
self.assertEqual(f.read(), "This is a review\nwith two lines")
949+
950+
self.assertEqual(len(outbox), 0)
951+
self.assertTrue("http://example.com" in assignment.review.external_url)
952+
876953
def test_partially_complete_review(self):
877954
assignment, url = self.setup_complete_review_test()
878955

‎ietf/doc/urls_review.py‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
# Copyright The IETF Trust 2016-2019, All Rights Reserved
2+
from django.conf import settings
3+
14
from ietf.doc import views_review
25
from ietf.utils.urls import url
36

@@ -9,9 +12,12 @@
912
url(r'^(?P<request_id>[0-9]+)/assignreviewer/$', views_review.assign_reviewer),
1013
url(r'^(?P<assignment_id>[0-9]+)/rejectreviewerassignment/$', views_review.reject_reviewer_assignment),
1114
url(r'^(?P<assignment_id>[0-9]+)/complete/$', views_review.complete_review),
15+
url(r'^%(acronym)s/submitunsolicitedreview/$' % settings.URL_REGEXPS, views_review.complete_review),
16+
url(r'^submitunsolicitedreview/$', views_review.submit_unsolicited_review_choose_team),
1217
url(r'^(?P<assignment_id>[0-9]+)/withdraw/$', views_review.withdraw_reviewer_assignment),
1318
url(r'^(?P<assignment_id>[0-9]+)/noresponse/$', views_review.mark_reviewer_assignment_no_response),
14-
url(r'^(?P<assignment_id>[0-9]+)/searchmailarchive/$', views_review.search_mail_archive),
19+
url(r'^assignment/(?P<assignment_id>[0-9]+)/searchmailarchive/$', views_review.search_mail_archive),
20+
url(r'^team/%(acronym)s/searchmailarchive/$' % settings.URL_REGEXPS, views_review.search_mail_archive),
1521
url(r'^(?P<request_id>[0-9]+)/editcomment/$', views_review.edit_comment),
1622
url(r'^(?P<request_id>[0-9]+)/editdeadline/$', views_review.edit_deadline),
1723
]

‎ietf/doc/views_doc.py‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus,
6565
add_events_message_info, get_unicode_document_content, build_doc_meta_block)
6666
from ietf.community.utils import augment_docs_with_tracking_info
67-
from ietf.group.models import Role
67+
from ietf.group.models import Role, Group
6868
from ietf.group.utils import can_manage_group_type, can_manage_materials, group_features_role_filter
6969
from ietf.ietfauth.utils import ( has_role, is_authorized_in_doc_stream, user_is_person,
7070
role_required, is_individual_draft_author)
@@ -319,6 +319,10 @@ def document_main(request, name, rev=None):
319319
consensus = nice_consensus(e and e.consensus)
320320

321321
can_request_review = can_request_review_of_doc(request.user, doc)
322+
can_submit_unsolicited_review_for_teams = None
323+
if request.user.is_authenticated:
324+
can_submit_unsolicited_review_for_teams = Group.objects.filter(
325+
reviewteamsettings__isnull=False, role__person__user=request.user, role__name='secr')
322326

323327
# mailing list search archive
324328
search_archive = "www.ietf.org/mail-archive/web/"
@@ -424,6 +428,7 @@ def document_main(request, name, rev=None):
424428
can_edit_replaces=can_edit_replaces,
425429
can_view_possibly_replaces=can_view_possibly_replaces,
426430
can_request_review=can_request_review,
431+
can_submit_unsolicited_review_for_teams=can_submit_unsolicited_review_for_teams,
427432

428433
rfc_number=rfc_number,
429434
draft_name=draft_name,

0 commit comments

Comments
 (0)