altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 3 Packages Projects Releases Wiki Activity

Merged in [16933] from sasha@dashcare.nl:

Browse source 
Fix #2119 - Allow specifying review type for suggested reviews in LC and telechat
If a review is suggested on the 'manage unassigned reviews' page, and
the document is in both last call and telechat, the assign form now asks
for the type of review that should be assigned.
This commit also fixes two bugs in this process:
- Comparisons in some cases between strings and integers
  (group/views.py:1485/1487)
- Rejections when assigning suggested reviews, as they could be
  considered a newly opened request due to not having a pk
  (group/views.py:1508)
 - Legacy-Id: 16951
Note: SVN reference [16933] has been migrated to Git commit ee4bc0cb07
This commit is contained in:
Henrik Levkowetz 2019-10-31 17:31:21 +00:00
parent 0f12d477ad ee4bc0cb07
commit 77a56473c3
5 changed files with 125 additions and 18 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
ietf/group/forms.py
View file

@ -16,6 +16,7 @@ from django.utils.html import mark_safe # type:ignore
# 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
@ -229,6 +230,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):
@ -257,6 +259,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

99
ietf/group/tests_review.py
View file

@ -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,
send_unavaibility_period_ending_reminder, send_reminder_all_open_reviews,
send_review_reminder_overdue_assignment, send_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)

8
ietf/group/views.py
View file

@ -1481,9 +1481,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]
@ -1504,7 +1504,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())
@ -1529,6 +1529,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" and review_req.form.cleaned_data["reviewer"]:
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()

25
ietf/review/utils.py
View file

@ -608,7 +608,6 @@ def suggested_review_requests_for_team(team):
requested_by=system_person,
state=requested_state,
)
seen_deadlines[doc.pk] = deadline
@ -642,16 +641,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

6
ietf/templates/group/manage_review_requests.html
View file

@ -37,7 +37,7 @@
<h3 class="panel-title">
<span class="pull-right">
{{ 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 %}<span class="label label-warning">{{ r.due }} day{{ r.due|pluralize }}</span>{% endif %}
</span>
@ -134,6 +134,10 @@
{{ r.form.action }}
<span class="reviewer-controls form-inline">
{% if r.form.review_type %}
<label for="{{ r.form.review_type.id_for_label }}">Review Type:</label>
{{ r.form.review_type }}
{% endif %}
<label for="{{ r.form.reviewer.id_for_label }}">Assign:</label>
{{ r.form.reviewer }}
{{ r.form.add_skip }} <label for="{{ r.form.add_skip.id_for_label }}">Skip next time</label>