diff --git a/ietf/group/forms.py b/ietf/group/forms.py index b0c0410af..f95f5f94c 100644 --- a/ietf/group/forms.py +++ b/ietf/group/forms.py @@ -16,6 +16,7 @@ from django.utils.html import mark_safe # IETF imports from ietf.group.models import Group, GroupHistory, GroupStateName +from ietf.name.models import ReviewTypeName from ietf.person.fields import SearchableEmailsField, PersonEmailChoiceField from ietf.person.models import Person from ietf.review.models import ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings @@ -234,6 +235,7 @@ class ManageReviewRequestForm(forms.Form): close = forms.ModelChoiceField(queryset=close_review_request_states(), required=False) close_comment = forms.CharField(max_length=255, required=False) reviewer = PersonEmailChoiceField(empty_label="(None)", required=False, label_with="person") + review_type = forms.ModelChoiceField(queryset=ReviewTypeName.objects.filter(slug__in=['telechat', 'lc']), required=True) add_skip = forms.BooleanField(required=False) def __init__(self, review_req, *args, **kwargs): @@ -262,6 +264,9 @@ class ManageReviewRequestForm(forms.Form): setup_reviewer_field(self.fields["reviewer"], review_req) self.fields["reviewer"].widget.attrs["class"] = "form-control input-sm" + if not getattr(review_req, 'in_lc_and_telechat', False): + del self.fields["review_type"] + if self.is_bound: if self.data.get("action") == "close": self.fields["close"].required = True diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index 7e7c55cae..d729ca2ff 100644 --- a/ietf/group/tests_review.py +++ b/ietf/group/tests_review.py @@ -13,7 +13,7 @@ from pyquery import PyQuery from django.urls import reverse as urlreverse from ietf.utils.test_utils import login_testing_unauthorized, TestCase, reload_db_objects -from ietf.doc.models import TelechatDocEvent +from ietf.doc.models import TelechatDocEvent, LastCallDocEvent, State from ietf.group.models import Role from ietf.iesg.models import TelechatDate from ietf.person.models import Person @@ -26,7 +26,8 @@ from ietf.review.utils import ( reviewer_rotation_list, email_unavaibility_period_ending_reminder, email_reminder_all_open_reviews, email_review_reminder_overdue_assignment, email_reminder_unconfirmed_assignments) -from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName +from ietf.name.models import ReviewResultName, ReviewRequestStateName, ReviewAssignmentStateName, \ + ReviewTypeName import ietf.group.views from ietf.utils.mail import outbox, empty_outbox from ietf.dbtemplate.factories import DBTemplateFactory @@ -91,6 +92,7 @@ class ReviewTests(TestCase): self.assertEqual(len(suggestions), 1) self.assertEqual(suggestions[0].doc, doc) self.assertEqual(suggestions[0].team, team) + self.assertFalse(getattr(suggestions[0], 'in_lc_and_telechat', None)) # blocked by non-versioned refusal review_req.requested_rev = "" @@ -120,6 +122,35 @@ class ReviewTests(TestCase): self.assertEqual(len(suggested_review_requests_for_team(team)), 1) + def test_suggested_review_requests_on_lc_and_telechat(self): + review_req = ReviewRequestFactory(state_id='assigned') + doc = review_req.doc + team = review_req.team + + # put on telechat + TelechatDocEvent.objects.create( + type="scheduled_for_telechat", + by=Person.objects.get(name="(System)"), + doc=doc, + rev=doc.rev, + telechat_date=TelechatDate.objects.all().first().date, + ) + + # Put on last call as well + doc.states.add(State.objects.get(type="draft-iesg", slug="lc", used=True)) + LastCallDocEvent.objects.create( + doc=doc, + expires=datetime.datetime.now() + datetime.timedelta(days=365), + by=Person.objects.get(name="(System)"), + rev=doc.rev + ) + + suggestions = suggested_review_requests_for_team(team) + self.assertEqual(len(suggestions), 1) + self.assertEqual(suggestions[0].doc, doc) + self.assertEqual(suggestions[0].team, team) + self.assertTrue(suggestions[0].in_lc_and_telechat) + def test_reviewer_overview(self): team = ReviewTeamFactory() reviewer = RoleFactory(name_id='reviewer',group=team,person__user__username='reviewer').person @@ -228,7 +259,9 @@ class ReviewTests(TestCase): "r{}-action".format(review_req3.pk): "assign", "r{}-reviewer".format(review_req3.pk): another_reviewer.email_set.first().pk, "r{}-add_skip".format(review_req3.pk): 1, - + # Should be ignored, setting review_type only applies to suggested reviews + "r{}-review_type".format(review_req3.pk): ReviewTypeName.objects.get(slug='telechat').pk, + "action": "save", }) self.assertEqual(r.status_code, 302) @@ -237,7 +270,67 @@ class ReviewTests(TestCase): settings = ReviewerSettings.objects.filter(team=review_req3.team, person=another_reviewer).first() self.assertEqual(settings.skip_next,1) self.assertEqual(review_req3.state_id, "assigned") + self.assertEqual(review_req3.type_id, "lc") + def test_manage_review_requests_assign_suggested_reviews(self): + doc = DocumentFactory() + team = ReviewTeamFactory() + + # put on telechat + TelechatDocEvent.objects.create( + type="scheduled_for_telechat", + by=Person.objects.get(name="(System)"), + doc=doc, + rev=doc.rev, + telechat_date=TelechatDate.objects.all().first().date, + ) + + # Put on last call as well + doc.states.add(State.objects.get(type="draft-iesg", slug="lc", used=True)) + LastCallDocEvent.objects.create( + doc=doc, + expires=datetime.datetime.now() + datetime.timedelta(days=365), + by=Person.objects.get(name="(System)"), + rev=doc.rev + ) + + reviewer = RoleFactory(name_id='reviewer', group=team, person__user__username='reviewer') + unassigned_url = urlreverse(ietf.group.views.manage_review_requests, kwargs={ 'acronym': team.acronym, 'group_type': team.type_id, "assignment_status": "unassigned" }) + + login_testing_unauthorized(self, "secretary", unassigned_url) + + # get + r = self.client.get(unassigned_url) + self.assertEqual(r.status_code, 200) + self.assertContains(r, doc.name) + + # Submit as lc review + r = self.client.post(unassigned_url, { + "rlc-{}-action".format(doc.name): "assign", + "rlc-{}-review_type".format(doc.name): ReviewTypeName.objects.get(slug='lc').pk, + "rlc-{}-reviewer".format(doc.name): reviewer.person.email_set.first().pk, + + "action": "save", + }) + self.assertEqual(r.status_code, 302) + review_request = doc.reviewrequest_set.get() + self.assertEqual(review_request.type_id, 'lc') + + # Clean up and try again as telechat + review_request.reviewassignment_set.all().delete() + review_request.delete() + + r = self.client.post(unassigned_url, { + "rlc-{}-action".format(doc.name): "assign", + "rlc-{}-review_type".format(doc.name): ReviewTypeName.objects.get(slug='telechat').pk, + "rlc-{}-reviewer".format(doc.name): reviewer.person.email_set.first().pk, + + "action": "save", + }) + self.assertEqual(r.status_code, 302) + review_request = doc.reviewrequest_set.get() + self.assertEqual(review_request.type_id, 'telechat') + def test_email_open_review_assignments(self): review_req1 = ReviewRequestFactory() review_assignment_completed = ReviewAssignmentFactory(review_request=review_req1,reviewer=EmailFactory(person__user__username='marschairman'), state_id='completed', reviewed_rev=0) diff --git a/ietf/group/views.py b/ietf/group/views.py index 1d5f1f090..f8f1a5d66 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -1482,9 +1482,9 @@ def manage_review_requests(request, acronym, group_type=None, assignment_status= for a in r.reviewassignment_set.all(): if l and rev: if r.doc_id == l[0].doc_id and a.reviewed_rev: - if int(a.reviewed_rev) > rev: + if int(a.reviewed_rev) > int(rev): l = [r] - elif int(a.reviewed_rev) == rev: + elif int(a.reviewed_rev) == int(rev): l.append(r) else: l = [r] @@ -1505,7 +1505,7 @@ def manage_review_requests(request, acronym, group_type=None, assignment_status= saving = form_action.startswith("save") # check for conflicts - review_requests_dict = { six.text_type(r.pk): r for r in review_requests } + review_requests_dict = { six.text_type(r.pk): r for r in review_requests if r.pk} posted_reqs = set(request.POST.getlist("reviewrequest", [])) current_reqs = set(review_requests_dict.keys()) @@ -1530,6 +1530,8 @@ def manage_review_requests(request, acronym, group_type=None, assignment_status= close_review_request(request, review_req, review_req.form.cleaned_data["close"], review_req.form.cleaned_data["close_comment"]) elif action=="assign": + if review_req.form.cleaned_data.get("review_type"): + review_req.type = review_req.form.cleaned_data.get("review_type") reqs_to_assign.append(review_req) assignments_by_person = dict() diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 705b166f0..818656429 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -605,7 +605,6 @@ def suggested_review_requests_for_team(team): requested_by=system_person, state=requested_state, ) - seen_deadlines[doc.pk] = deadline @@ -639,16 +638,20 @@ def suggested_review_requests_for_team(team): if not deadline or deadline > seen_deadlines.get(doc_pk, datetime.date.max): continue - - requests[doc_pk] = ReviewRequest( - time=event_time, - type=telechat_type, - doc_id=doc_pk, - team=team, - deadline=deadline, - requested_by=system_person, - state=requested_state, - ) + + if doc_pk in requests: + # Document was already added in last call, i.e. it is both in last call and telechat + requests[doc_pk].in_lc_and_telechat = True + else: + requests[doc_pk] = ReviewRequest( + time=event_time, + type=telechat_type, + doc_id=doc_pk, + team=team, + deadline=deadline, + requested_by=system_person, + state=requested_state, + ) seen_deadlines[doc_pk] = deadline diff --git a/ietf/templates/group/manage_review_requests.html b/ietf/templates/group/manage_review_requests.html index 1a2cd83c6..facf1a460 100644 --- a/ietf/templates/group/manage_review_requests.html +++ b/ietf/templates/group/manage_review_requests.html @@ -37,7 +37,7 @@

- {{ r.type.name }} + {% if r.in_lc_and_telechat %}Last Call and telechat{% else %}{{ r.type.name }}{% endif %} - deadline {{ r.deadline|date:"Y-m-d" }} {% if r.due %}{{ r.due }} day{{ r.due|pluralize }}{% endif %} @@ -134,6 +134,10 @@ {{ r.form.action }} + {% if r.form.review_type %} + + {{ r.form.review_type }} + {% endif %} {{ r.form.reviewer }} {{ r.form.add_skip }}