From e2e66522c7368dd97d3262be18082a020d126816 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Fri, 1 Jul 2016 16:06:16 +0000 Subject: [PATCH] Add review request page for review teams and first draft of manage review requests page. Add importer for importing review data from the existing Perl tool (WIP, gets most but not all of the interesting information out). Fix various bugs. - Legacy-Id: 11508 --- ietf/doc/tests_review.py | 49 +--- ietf/doc/utils.py | 34 +++ ietf/doc/views_doc.py | 2 +- ietf/doc/views_review.py | 25 +- ietf/group/features.py | 7 + ietf/group/tests_info.py | 28 ++- ietf/group/tests_review.py | 72 ++++++ ietf/group/urls_info.py | 4 +- ietf/group/urls_info_details.py | 4 +- ietf/group/views.py | 63 +++++ ietf/group/views_review.py | 142 +++++++++++ ietf/name/fixtures/names.json | 23 +- ...atename_reviewresultname_reviewtypename.py | 1 - .../0012_insert_review_name_data.py | 5 +- ietf/name/models.py | 1 - ietf/name/resources.py | 1 - ietf/person/fields.py | 20 ++ ietf/review/import_from_review_tool.py | 231 ++++++++++++++++++ ietf/review/migrations/0001_initial.py | 20 +- ietf/review/models.py | 33 ++- ietf/review/resources.py | 21 +- ietf/review/utils.py | 126 +++++++++- ietf/static/ietf/css/ietf.css | 15 ++ ietf/static/ietf/js/manage-review-requests.js | 37 +++ ietf/templates/doc/document_draft.html | 4 +- ietf/templates/doc/review/review_request.html | 4 +- .../group/manage_review_requests.html | 109 +++++++++ ietf/templates/group/review_requests.html | 121 +++++++++ ietf/utils/test_data.py | 29 +++ 29 files changed, 1131 insertions(+), 100 deletions(-) create mode 100644 ietf/group/tests_review.py create mode 100644 ietf/group/views_review.py create mode 100755 ietf/review/import_from_review_tool.py create mode 100644 ietf/static/ietf/js/manage-review-requests.js create mode 100644 ietf/templates/group/manage_review_requests.html create mode 100644 ietf/templates/group/review_requests.html diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index e937bd27e..a03b3c060 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -12,42 +12,15 @@ from pyquery import PyQuery import debug # pyflakes:ignore -from ietf.review.models import ReviewRequest, Reviewer +from ietf.review.models import ReviewRequest, ReviewTeamResult import ietf.review.mailarch -from ietf.person.models import Person -from ietf.group.models import Group, Role +from ietf.person.models import Email from ietf.name.models import ReviewResultName, ReviewRequestStateName from ietf.utils.test_utils import TestCase -from ietf.utils.test_data import make_test_data +from ietf.utils.test_data import make_test_data, make_review_data from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects from ietf.utils.mail import outbox, empty_outbox -def make_review_data(doc): - team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team") - team.reviewresultname_set.add(ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"])) - - p = Person.objects.get(user__username="plain") - role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team) - Reviewer.objects.create(team=team, person=p, frequency=14, skip_next=0) - - review_req = ReviewRequest.objects.create( - doc=doc, - team=team, - type_id="early", - deadline=datetime.datetime.now() + datetime.timedelta(days=20), - state_id="ready", - reviewer=role, - reviewed_rev="01", - ) - - p = Person.objects.get(user__username="marschairman") - role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team) - - p = Person.objects.get(user__username="secretary") - role = Role.objects.create(name_id="secretary", person=p, email=p.email_set.first(), group=team) - - return review_req - class ReviewTests(TestCase): def setUp(self): self.review_dir = os.path.abspath("tmp-review-dir") @@ -169,7 +142,7 @@ class ReviewTests(TestCase): # assign empty_outbox() - reviewer = Role.objects.filter(name="reviewer", group=review_req.team).first() + reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team).first() r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk }) self.assertEqual(r.status_code, 302) @@ -183,7 +156,7 @@ class ReviewTests(TestCase): empty_outbox() review_req.state = ReviewRequestStateName.objects.get(slug="accepted") review_req.save() - reviewer = Role.objects.filter(name="reviewer", group=review_req.team).exclude(pk=reviewer.pk).first() + reviewer = Email.objects.filter(role__name="reviewer", role__group=review_req.team).exclude(pk=reviewer.pk).first() r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk }) self.assertEqual(r.status_code, 302) @@ -335,7 +308,7 @@ class ReviewTests(TestCase): review_req.save() review_req.team.list_email = "{}@ietf.org".format(review_req.team.acronym) for r in ReviewResultName.objects.filter(slug__in=("issues", "ready")): - review_req.team.reviewresultname_set.add(r) + ReviewTeamResult.objects.get_or_create(team=review_req.team, result=r) review_req.team.save() url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -373,7 +346,7 @@ class ReviewTests(TestCase): test_file.name = "unnamed" r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "upload", @@ -408,7 +381,7 @@ class ReviewTests(TestCase): empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", @@ -439,7 +412,7 @@ class ReviewTests(TestCase): empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "link", @@ -467,7 +440,7 @@ class ReviewTests(TestCase): empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="part-completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", @@ -501,7 +474,7 @@ class ReviewTests(TestCase): url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": review_req.doc.name, "request_id": review_req.pk }) r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamresult__team=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 5d7f02f93..f37caff4c 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -3,6 +3,7 @@ import re import urllib import math import datetime +from collections import defaultdict from django.conf import settings from django.db.models.query import EmptyQuerySet @@ -556,6 +557,39 @@ def uppercase_std_abbreviated_name(name): else: return name +def extract_complete_replaces_ancestor_mapping_for_docs(names): + """Return dict mapping all replaced by relationships of the + replacement ancestors to docs. So if x is directly replaced by y + and y is in names or replaced by something in names, x in + replaces[y].""" + + replaces = defaultdict(set) + + checked = set() + front = names + while True: + if not front: + break + + relations = RelatedDocument.objects.filter( + source__in=front, relationship="replaces" + ).select_related("target").values_list("source", "target__document") + + if not relations: + break + + checked.update(front) + + front = [] + for source_doc, target_doc in relations: + replaces[source_doc].add(target_doc) + + if target_doc not in checked: + front.append(target_doc) + + return replaces + + def crawl_history(doc): # return document history data for inclusion in doc.json (used by timeline) def get_ancestors(doc): diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index c1f0a6cb2..22af819c6 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -357,7 +357,7 @@ def document_main(request, name, rev=None): published = doc.latest_event(type="published_rfc") started_iesg_process = doc.latest_event(type="started_iesg_process") - review_requests = ReviewRequest.objects.filter(doc=doc).exclude(state__in=["withdrawn", "rejected"]) + review_requests = ReviewRequest.objects.filter(doc=doc).exclude(state__in=["withdrawn", "rejected", "overtaken", "no-response"]).order_by("-time", "-id") return render_to_response("doc/document_draft.html", dict(doc=doc, diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 96eaa816d..6d5fb1d0d 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -12,8 +12,8 @@ from django.template.loader import render_to_string from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent, State, DocAlias from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName -from ietf.group.models import Role from ietf.review.models import ReviewRequest +from ietf.person.fields import PersonEmailChoiceField from ietf.review.utils import (active_review_teams, assign_review_request_to_reviewer, can_request_review_of_doc, can_manage_review_requests_for_team, email_about_review_request, make_new_review_request_from_existing) @@ -188,22 +188,13 @@ def withdraw_request(request, name, request_id): 'review_req': review_req, }) -class PersonEmailLabeledRoleModelChoiceField(forms.ModelChoiceField): - def __init__(self, *args, **kwargs): - if not "queryset" in kwargs: - kwargs["queryset"] = Role.objects.select_related("person", "email") - super(PersonEmailLabeledRoleModelChoiceField, self).__init__(*args, **kwargs) - - def label_from_instance(self, role): - return u"{} <{}>".format(role.person.name, role.email.address) - class AssignReviewerForm(forms.Form): - reviewer = PersonEmailLabeledRoleModelChoiceField(widget=forms.RadioSelect, empty_label="(None)", required=False) + reviewer = PersonEmailChoiceField(widget=forms.RadioSelect, empty_label="(None)", required=False) def __init__(self, review_req, *args, **kwargs): super(AssignReviewerForm, self).__init__(*args, **kwargs) f = self.fields["reviewer"] - f.queryset = f.queryset.filter(name="reviewer", group=review_req.team) + f.queryset = f.queryset.filter(role__name="reviewer", role__group=review_req.team) if review_req.reviewer: f.initial = review_req.reviewer_id @@ -212,9 +203,7 @@ def assign_reviewer(request, name, request_id): doc = get_object_or_404(Document, name=name) review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) - can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) - - if not can_manage_request: + if not can_manage_review_requests_for_team(request.user, review_req.team): return HttpResponseForbidden("You do not have permission to perform this action") if request.method == "POST" and request.POST.get("action") == "assign": @@ -322,7 +311,7 @@ class CompleteReviewForm(forms.Form): " ".join("{}".format(r) for r in known_revisions)) - self.fields["result"].queryset = self.fields["result"].queryset.filter(teams=review_req.team) + self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamresult__team=review_req.team) self.fields["review_submission"].choices = [ (k, label.format(mailing_list=review_req.team.list_email or "[error: team has no mailing list set]")) for k, label in self.fields["review_submission"].choices @@ -428,10 +417,12 @@ def complete_review(request, name, request_id): type="changed_review_request", doc=review_req.doc, by=request.user.person, - desc="Request for {} review by {} {}".format( + desc="Request for {} review by {} {}: {}. Reviewer: {}".format( review_req.type.name, review_req.team.acronym.upper(), review_req.state.name, + review_req.result.name, + review_req.reviewer, ), ) diff --git a/ietf/group/features.py b/ietf/group/features.py index 7e32136b8..190bd83e0 100644 --- a/ietf/group/features.py +++ b/ietf/group/features.py @@ -6,6 +6,7 @@ class GroupFeatures(object): has_chartering_process = False has_documents = False # i.e. drafts/RFCs has_materials = False + has_reviews = False customize_workflow = False about_page = "group_about" default_tab = about_page @@ -24,3 +25,9 @@ class GroupFeatures(object): if self.has_chartering_process: self.about_page = "group_charter" + + from ietf.review.utils import active_review_teams + if group in active_review_teams(): + self.has_reviews = True + import ietf.group.views + self.default_tab = ietf.group.views.review_requests diff --git a/ietf/group/tests_info.py b/ietf/group/tests_info.py index 081553e20..006f04191 100644 --- a/ietf/group/tests_info.py +++ b/ietf/group/tests_info.py @@ -25,10 +25,11 @@ from ietf.name.models import DocTagName, GroupStateName, GroupTypeName from ietf.person.models import Person, Email from ietf.utils.test_utils import TestCase, unicontent from ietf.utils.mail import outbox, empty_outbox -from ietf.utils.test_data import make_test_data, create_person +from ietf.utils.test_data import make_test_data, create_person, make_review_data from ietf.utils.test_utils import login_testing_unauthorized from ietf.group.factories import GroupFactory, RoleFactory, GroupEventFactory from ietf.meeting.factories import SessionFactory +import ietf.group.views class GroupPagesTests(TestCase): def setUp(self): @@ -313,6 +314,31 @@ class GroupPagesTests(TestCase): self.assertEqual(r.status_code, 200) self.assertTrue(doc.title not in unicontent(r)) + def test_review_requests(self): + doc = make_test_data() + review_req = make_review_data(doc) + + group = review_req.team + + for url in [ urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym }), + urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym , 'group_type': group.type_id}), + ]: + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + self.assertTrue(review_req.doc.name in unicontent(r)) + self.assertTrue(unicode(review_req.reviewer.person) in unicontent(r)) + + url = urlreverse(ietf.group.views.review_requests, kwargs={ 'acronym': group.acronym }) + + # close request, listed under closed + review_req.state_id = "completed" + review_req.result_id = "ready" + review_req.save() + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + self.assertTrue(review_req.doc.name in unicontent(r)) + def test_history(self): draft = make_test_data() group = draft.group diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py new file mode 100644 index 000000000..6dc5d5147 --- /dev/null +++ b/ietf/group/tests_review.py @@ -0,0 +1,72 @@ +import datetime + +#from pyquery import PyQuery + +from django.core.urlresolvers import reverse as urlreverse + +from ietf.utils.test_data import make_test_data, make_review_data +from ietf.utils.test_utils import login_testing_unauthorized, TestCase, unicontent, reload_db_objects +from ietf.review.models import ReviewRequest +from ietf.person.models import Email +import ietf.group.views_review + +class ReviewTests(TestCase): + def test_manage_review_requests(self): + doc = make_test_data() + review_req1 = make_review_data(doc) + + group = review_req1.team + + url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym }) + + login_testing_unauthorized(self, "secretary", url) + + review_req2 = ReviewRequest.objects.create( + doc=review_req1.doc, + team=review_req1.team, + type_id="early", + deadline=datetime.datetime.combine(datetime.date.today() + datetime.timedelta(days=30), datetime.time(23, 59, 59)), + state_id="accepted", + reviewer=review_req1.reviewer, + ) + + review_req3 = ReviewRequest.objects.create( + doc=review_req1.doc, + team=review_req1.team, + type_id="early", + deadline=datetime.datetime.combine(datetime.date.today() + datetime.timedelta(days=30), datetime.time(23, 59, 59)), + state_id="requested", + ) + + # get + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + self.assertTrue(review_req1.doc.name in unicontent(r)) + + # close and assign + new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman") + r = self.client.post(url, { + # close + "r{}-action".format(review_req1.pk): "close", + "r{}-close".format(review_req1.pk): "no-response", + + # assign + "r{}-action".format(review_req2.pk): "assign", + "r{}-reviewer".format(review_req2.pk): new_reviewer.pk, + + # no change + "r{}-action".format(review_req3.pk): "", + "r{}-close".format(review_req3.pk): "no-response", + "r{}-reviewer".format(review_req3.pk): "", + }) + self.assertEqual(r.status_code, 302) + + review_req1, review_req2, review_req3 = reload_db_objects(review_req1, review_req2, review_req3) + self.assertEqual(review_req1.state_id, "no-response") + self.assertEqual(review_req2.state_id, "requested") + self.assertEqual(review_req2.reviewer, new_reviewer) + self.assertEqual(review_req3.state_id, "requested") + + # FIXME: test suggested + + diff --git a/ietf/group/urls_info.py b/ietf/group/urls_info.py index c1aadef3c..c96d4d7b1 100644 --- a/ietf/group/urls_info.py +++ b/ietf/group/urls_info.py @@ -3,7 +3,7 @@ from django.conf.urls import patterns, include from django.views.generic import RedirectView -from ietf.group import views, views_edit +from ietf.group import views, views_edit, views_review urlpatterns = patterns('', (r'^$', views.active_groups), @@ -20,5 +20,7 @@ urlpatterns = patterns('', (r'^email-aliases/$', 'ietf.group.views.email_aliases'), (r'^bofs/create/$', views_edit.edit, {'action': "create", }, "bof_create"), (r'^photos/$', views.chair_photos), + (r'^reviews/$', views.review_requests), + (r'^reviews/manage/$', views_review.manage_review_requests), (r'^(?P[a-zA-Z0-9-._]+)/', include('ietf.group.urls_info_details')), ) diff --git a/ietf/group/urls_info_details.py b/ietf/group/urls_info_details.py index 0c60d806a..d5d62c902 100644 --- a/ietf/group/urls_info_details.py +++ b/ietf/group/urls_info_details.py @@ -1,6 +1,6 @@ from django.conf.urls import patterns, url from django.views.generic import RedirectView -import views +from ietf.group import views, views_review urlpatterns = patterns('', (r'^$', 'ietf.group.views.group_home', None, "group_home"), @@ -30,5 +30,7 @@ urlpatterns = patterns('', (r'^materials/new/(?P[\w-]+)/$', 'ietf.doc.views_material.edit_material', { 'action': "new" }, "group_new_material"), (r'^archives/$', 'ietf.group.views.derived_archives'), (r'^photos/$', views.group_photos), + (r'^reviews/$', views.review_requests), + (r'^reviews/manage/$', views_review.manage_review_requests), url(r'^email-aliases/$', RedirectView.as_view(pattern_name='ietf.group.views.email',permanent=False),name='old_group_email_aliases'), ) diff --git a/ietf/group/views.py b/ietf/group/views.py index 5388819c0..292f32ff1 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -38,6 +38,7 @@ import re from tempfile import mkstemp import datetime from collections import OrderedDict +import math import debug # pyflakes:ignore @@ -67,6 +68,8 @@ from ietf.settings import MAILING_LIST_INFO_URL from ietf.mailtrigger.utils import gather_relevant_expansions from ietf.ietfauth.utils import has_role from ietf.meeting.utils import group_sessions +from ietf.review.models import ReviewRequest +from ietf.review.utils import can_manage_review_requests_for_team, suggested_review_requests_for_team def roles(group, role_name): return Role.objects.filter(group=group, name=role_name).select_related("email", "person") @@ -345,6 +348,8 @@ def construct_group_menu_context(request, group, selected, group_type, others): entries.append(("About", urlreverse("group_about", kwargs=kwargs))) if group.features.has_materials and get_group_materials(group).exists(): entries.append(("Materials", urlreverse("ietf.group.views.materials", kwargs=kwargs))) + if group.features.has_reviews: + entries.append(("Review requests", urlreverse(review_requests, kwargs=kwargs))) if group.type_id in ('rg','wg','team'): entries.append(("Meetings", urlreverse("ietf.group.views.meetings", kwargs=kwargs))) entries.append(("History", urlreverse("ietf.group.views.history", kwargs=kwargs))) @@ -375,6 +380,10 @@ def construct_group_menu_context(request, group, selected, group_type, others): if group.features.has_materials and can_manage_materials(request.user, group): actions.append((u"Upload material", urlreverse("ietf.doc.views_material.choose_material_type", kwargs=kwargs))) + if group.features.has_reviews: + import ietf.group.views_review + actions.append((u"Manage review requests", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=kwargs))) + if group.state_id != "conclude" and (is_chair or can_manage): actions.append((u"Edit group", urlreverse("group_edit", kwargs=kwargs))) @@ -633,6 +642,60 @@ def history(request, acronym, group_type=None): "events": events, })) +def review_requests(request, acronym, group_type=None): + group = get_group_or_404(acronym, group_type) + if not group.features.has_reviews: + raise Http404 + + open_review_requests = list(ReviewRequest.objects.filter( + team=group, state__in=("requested", "accepted") + ).prefetch_related("reviewer", "type", "state").order_by("time", "id")) + + open_review_requests += suggested_review_requests_for_team(group) + + now = datetime.datetime.now() + for r in open_review_requests: + delta = now - r.deadline + r.due = max(0, int(math.ceil(delta.total_seconds() / 3600.0))) + + closed_review_requests = ReviewRequest.objects.filter( + team=group, + ).exclude( + state__in=("requested", "accepted") + ).prefetch_related("reviewer", "type", "state").order_by("-time", "-id") + + since_choices = [ + (None, "1 month"), + ("3m", "3 months"), + ("6m", "6 months"), + ("1y", "1 year"), + ("2y", "2 years"), + ("all", "All"), + ] + since = request.GET.get("since", None) + if since not in [key for key, label in since_choices]: + since = None + + if since != "all": + date_limit = { + None: datetime.timedelta(days=31), + "3m": datetime.timedelta(days=31 * 3), + "6m": datetime.timedelta(days=180), + "1y": datetime.timedelta(days=365), + "2y": datetime.timedelta(days=2 * 365), + }[since] + + closed_review_requests = closed_review_requests.filter(time__gte=datetime.date.today() - date_limit) + + return render(request, 'group/review_requests.html', + construct_group_menu_context(request, group, "review requests", group_type, { + "open_review_requests": open_review_requests, + "closed_review_requests": closed_review_requests, + "since_choices": since_choices, + "since": since, + "can_manage_review_requests": can_manage_review_requests_for_team(request.user, group) + })) + def materials(request, acronym, group_type=None): group = get_group_or_404(acronym, group_type) if not group.features.has_materials: diff --git a/ietf/group/views_review.py b/ietf/group/views_review.py new file mode 100644 index 000000000..e46c45ae2 --- /dev/null +++ b/ietf/group/views_review.py @@ -0,0 +1,142 @@ +from django.shortcuts import render, redirect +from django.http import Http404, HttpResponseForbidden +from django.contrib.auth.decorators import login_required +from django import forms + +from ietf.review.models import ReviewRequest, ReviewRequestStateName +from ietf.review.utils import (can_manage_review_requests_for_team, + extract_revision_ordered_review_requests_for_documents, + assign_review_request_to_reviewer, +# email_about_review_request, make_new_review_request_from_existing, + suggested_review_requests_for_team) +from ietf.group.utils import get_group_or_404 +from ietf.person.fields import PersonEmailChoiceField + + +class ManageReviewRequestForm(forms.Form): + ACTIONS = [ + ("assign", "Assign"), + ("close", "Close"), + ] + + action = forms.ChoiceField(choices=ACTIONS, widget=forms.HiddenInput, required=False) + + CLOSE_OPTIONS = [ + ("noreviewversion", "No review of this version"), + ("noreviewdocument", "No review of document"), + ("withdraw", "Withdraw request"), + ("no-response", "No response"), + ("overtaken", "Overtaken by events"), + ] + close = forms.ChoiceField(choices=CLOSE_OPTIONS, required=False) + + reviewer = PersonEmailChoiceField(empty_label="(None)", required=False, label_with="person") + + def __init__(self, review_req, *args, **kwargs): + if not "prefix" in kwargs: + if review_req.pk is None: + kwargs["prefix"] = "r{}-{}".format(review_req.type_id, review_req.doc_id) + else: + kwargs["prefix"] = "r{}".format(review_req.pk) + + super(ManageReviewRequestForm, self).__init__(*args, **kwargs) + + close_initial = None + if review_req.pk is None: + if review_req.latest_reqs: + close_initial = "noreviewversion" + else: + close_initial = "noreviewdocument" + elif review_req.reviewer: + close_initial = "no-response" + else: + close_initial = "overtaken" + + if close_initial: + self.fields["close"].initial = close_initial + + self.fields["close"].widget.attrs["class"] = "form-control input-sm" + + self.fields["reviewer"].queryset = self.fields["reviewer"].queryset.filter( + role__name="reviewer", + role__group=review_req.team, + ) + + self.fields["reviewer"].widget.attrs["class"] = "form-control input-sm" + + if self.is_bound: + action = self.data.get("action") + if action == "close": + self.fields["close"].required = True + elif action == "assign": + self.fields["reviewer"].required = True + + +@login_required +def manage_review_requests(request, acronym, group_type=None): + group = get_group_or_404(acronym, group_type) + if not group.features.has_reviews: + raise Http404 + + if not can_manage_review_requests_for_team(request.user, group): + return HttpResponseForbidden("You do not have permission to perform this action") + + review_requests = list(ReviewRequest.objects.filter( + team=group, state__in=("requested", "accepted") + ).prefetch_related("reviewer", "type", "state").order_by("time", "id")) + + review_requests += suggested_review_requests_for_team(group) + + document_requests = extract_revision_ordered_review_requests_for_documents( + ReviewRequest.objects.filter(state__in=("part-completed", "completed")).prefetch_related("result"), + set(r.doc_id for r in review_requests), + ) + + for req in review_requests: + l = [] + # take all on the latest reviewed rev + for r in document_requests[req.doc_id]: + if l and l[0].reviewed_rev: + if r.doc_id == l[0].doc_id and r.reviewed_rev: + if int(r.reviewed_rev) > int(l[0].reviewed_rev): + l = [r] + elif int(r.reviewed_rev) == int(l[0].reviewed_rev): + l.append(r) + else: + l = [r] + + req.latest_reqs = l + + req.form = ManageReviewRequestForm(req, request.POST if request.method == "POST" else None) + + if request.method == "POST": + form_results = [] + for req in review_requests: + form_results.append(req.form.is_valid()) + + if all(form_results): + for req in review_requests: + action = req.form.cleaned_data.get("action") + if action == "assign": + assign_review_request_to_reviewer(request, req, req.form.cleaned_data["reviewer"]) + elif action == "close": + close_reason = req.form.cleaned_data["close"] + if close_reason in ("withdraw", "no-response", "overtaken"): + req.state = ReviewRequestStateName.objects.get(slug=close_reason, used=True) + req.save() + # FIXME: notify? + else: + FIXME + + kwargs = { "acronym": group.acronym } + if group_type: + kwargs["group_type"] = group_type + import ietf.group.views + return redirect(ietf.group.views.review_requests, **kwargs) + + + return render(request, 'group/manage_review_requests.html', { + 'group': group, + 'review_requests': review_requests, + }) + diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 72aeb7b2f..72837bf88 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -1821,11 +1821,11 @@ "desc": "" }, "model": "name.reviewrequeststatename", - "pk": "noresponse" + "pk": "no-response" }, { "fields": { - "order": 6, + "order": 7, "used": true, "name": "Partially Completed", "desc": "" @@ -1847,7 +1847,6 @@ "fields": { "order": 1, "used": true, - "teams": [], "name": "Serious Issues", "desc": "" }, @@ -1858,7 +1857,6 @@ "fields": { "order": 2, "used": true, - "teams": [], "name": "Has Issues", "desc": "" }, @@ -1869,7 +1867,6 @@ "fields": { "order": 3, "used": true, - "teams": [], "name": "Has Nits", "desc": "" }, @@ -1880,7 +1877,6 @@ "fields": { "order": 4, "used": true, - "teams": [], "name": "Not Ready", "desc": "" }, @@ -1891,7 +1887,6 @@ "fields": { "order": 5, "used": true, - "teams": [], "name": "On the Right Track", "desc": "" }, @@ -1902,7 +1897,6 @@ "fields": { "order": 6, "used": true, - "teams": [], "name": "Almost Ready", "desc": "" }, @@ -1913,7 +1907,6 @@ "fields": { "order": 7, "used": true, - "teams": [], "name": "Ready with Issues", "desc": "" }, @@ -1924,7 +1917,6 @@ "fields": { "order": 8, "used": true, - "teams": [], "name": "Ready with Nits", "desc": "" }, @@ -1935,7 +1927,6 @@ "fields": { "order": 9, "used": true, - "teams": [], "name": "Ready", "desc": "" }, @@ -1972,6 +1963,16 @@ "model": "name.reviewtypename", "pk": "telechat" }, +{ + "fields": { + "order": 4, + "used": false, + "name": "Unknown", + "desc": "" + }, + "model": "name.reviewtypename", + "pk": "unknown" +}, { "fields": { "order": 0, diff --git a/ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py b/ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py index fb62deea3..59e57a76f 100644 --- a/ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py +++ b/ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py @@ -35,7 +35,6 @@ class Migration(migrations.Migration): ('desc', models.TextField(blank=True)), ('used', models.BooleanField(default=True)), ('order', models.IntegerField(default=0)), - ('teams', models.ManyToManyField(help_text=b"Which teams this result can be set for. This also implicitly defines which teams are review teams - if there are no possible review results defined for a given team, it can't be a review team.", to='group.Group', blank=True)), ], options={ 'ordering': ['order'], diff --git a/ietf/name/migrations/0012_insert_review_name_data.py b/ietf/name/migrations/0012_insert_review_name_data.py index 7cc1a7d52..92da06b94 100644 --- a/ietf/name/migrations/0012_insert_review_name_data.py +++ b/ietf/name/migrations/0012_insert_review_name_data.py @@ -11,14 +11,15 @@ def insert_initial_review_data(apps, schema_editor): ReviewRequestStateName.objects.get_or_create(slug="rejected", name="Rejected", order=3) ReviewRequestStateName.objects.get_or_create(slug="withdrawn", name="Withdrawn", order=4) ReviewRequestStateName.objects.get_or_create(slug="overtaken", name="Overtaken By Events", order=5) - ReviewRequestStateName.objects.get_or_create(slug="noresponse", name="No Response", order=6) - ReviewRequestStateName.objects.get_or_create(slug="part-completed", name="Partially Completed", order=6) + ReviewRequestStateName.objects.get_or_create(slug="no-response", name="No Response", order=6) + ReviewRequestStateName.objects.get_or_create(slug="part-completed", name="Partially Completed", order=7) ReviewRequestStateName.objects.get_or_create(slug="completed", name="Completed", order=8) ReviewTypeName = apps.get_model("name", "ReviewTypeName") ReviewTypeName.objects.get_or_create(slug="early", name="Early", order=1) ReviewTypeName.objects.get_or_create(slug="lc", name="Last Call", order=2) ReviewTypeName.objects.get_or_create(slug="telechat", name="Telechat", order=3) + ReviewTypeName.objects.get_or_create(slug="unknown", name="Unknown", order=4, used=False) ReviewResultName = apps.get_model("name", "ReviewResultName") ReviewResultName.objects.get_or_create(slug="serious-issues", name="Serious Issues", order=1) diff --git a/ietf/name/models.py b/ietf/name/models.py index f17a55203..58b861326 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -96,5 +96,4 @@ class ReviewResultName(NameModel): """Almost ready, Has issues, Has nits, Not Ready, On the right track, Ready, Ready with issues, Ready with nits, Serious Issues""" - teams = models.ManyToManyField("group.Group", help_text="Which teams this result can be set for. This also implicitly defines which teams are review teams - if there are no possible review results defined for a given team, it can't be a review team.", blank=True) diff --git a/ietf/name/resources.py b/ietf/name/resources.py index 521f4ee79..64626273c 100644 --- a/ietf/name/resources.py +++ b/ietf/name/resources.py @@ -453,7 +453,6 @@ class ReviewResultNameResource(ModelResource): "desc": ALL, "used": ALL, "order": ALL, - "teams": ALL_WITH_RELATIONS, } api.name.register(ReviewResultNameResource()) diff --git a/ietf/person/fields.py b/ietf/person/fields.py index 177da9cb7..cc86d4fb4 100644 --- a/ietf/person/fields.py +++ b/ietf/person/fields.py @@ -139,3 +139,23 @@ class SearchableEmailField(SearchableEmailsField): return super(SearchableEmailField, self).clean(value).first() +class PersonEmailChoiceField(forms.ModelChoiceField): + """ModelChoiceField targeting Email and displaying choices with the + person name as well as the email address. Needs further + restrictions, e.g. on role, to useful.""" + def __init__(self, *args, **kwargs): + if not "queryset" in kwargs: + kwargs["queryset"] = Email.objects.select_related("person") + + self.label_with = kwargs.pop("label_with", None) + + super(PersonEmailChoiceField, self).__init__(*args, **kwargs) + + def label_from_instance(self, email): + if self.label_with == "person": + return unicode(email.person) + elif self.label_with == "email": + return email.address + else: + return u"{} <{}>".format(email.person, email.address) + diff --git a/ietf/review/import_from_review_tool.py b/ietf/review/import_from_review_tool.py new file mode 100755 index 000000000..325f6e06b --- /dev/null +++ b/ietf/review/import_from_review_tool.py @@ -0,0 +1,231 @@ +#!/usr/bin/env python + +import sys, os + +# boilerplate +basedir = os.path.abspath(os.path.join(os.path.dirname(__file__), "../..")) +sys.path = [ basedir ] + sys.path +os.environ.setdefault("DJANGO_SETTINGS_MODULE", "ietf.settings") + +import django +django.setup() + + +# script + +import datetime +from collections import namedtuple +from django.db import connections +from ietf.review.models import ReviewRequest, Reviewer, ReviewResultName +from ietf.review.models import ReviewRequestStateName, ReviewTypeName, ReviewTeamResult +from ietf.group.models import Group, Role, RoleName +from ietf.person.models import Person, Email, Alias +import argparse +from unidecode import unidecode +from collections import defaultdict + +parser = argparse.ArgumentParser() +parser.add_argument("database", help="database must be included in settings") +parser.add_argument("team", help="team acronym, must exist") +args = parser.parse_args() + +db_con = connections[args.database] +team = Group.objects.get(acronym=args.team) + +def namedtuplefetchall(cursor): + "Return all rows from a cursor as a namedtuple" + desc = cursor.description + nt_result = namedtuple('Result', [col[0] for col in desc]) + return (nt_result(*row) for row in cursor.fetchall()) + +def parse_timestamp(t): + if not t: + return None + return datetime.datetime.fromtimestamp(t) + +# personnel +with db_con.cursor() as c: + c.execute("select distinct reviewer from reviews;") + known_reviewers = { row[0] for row in c.fetchall() } + +with db_con.cursor() as c: + c.execute("select distinct who from doclog;") + docloggers = { row[0] for row in c.fetchall() } + +with db_con.cursor() as c: + c.execute("select distinct login from members where permissions like '%secretary%';") + secretaries = { row[0] for row in c.fetchall() } + +known_personnel = {} +with db_con.cursor() as c: + c.execute("select * from members;") + + needed_personnel = known_reviewers | docloggers | secretaries + + for row in namedtuplefetchall(c): + if row.login not in needed_personnel: + continue + + email = Email.objects.filter(address=row.email).select_related("person").first() + if not email: + person = Person.objects.filter(alias__name=row.name).first() + if not person: + person, created = Person.objects.get_or_create(name=row.name, ascii=unidecode(row.name)) + if created: + print "created person", person + existing_aliases = set(Alias.objects.filter(person=person).values_list("name", flat=True)) + curr_names = set(x for x in [person.name, person.ascii, person.ascii_short, person.plain_name(), ] if x) + new_aliases = curr_names - existing_aliases + for name in new_aliases: + Alias.objects.create(person=person, name=name) + + email, created = Email.objects.get_or_create(address=row.email, person=person) + if created: + print "created email", email + + known_personnel[row.login] = email + + if "secretary" in row.permissions: + role, created = Role.objects.get_or_create(name=RoleName.objects.get(slug="secr"), person=email.person, email=email, group=team) + if created: + print "created role", role + + if row.login in known_reviewers: + if row.comment != "Inactive" and row.available != 2145916800: # corresponds to 2038-01-01 + assert not row.autopolicy or row.autopolicy == "monthly" + + role, created = Role.objects.get_or_create(name=RoleName.objects.get(slug="reviewer"), person=email.person, email=email, group=team) + + if created: + print "created role", role + + reviewer, created = Reviewer.objects.get_or_create( + team=team, + person=email.person, + ) + if reviewer: + print "created reviewer", reviewer + + if row.autopolicy == "monthly": + reviewer.frequency = 30 + reviewer.unavailable_until = parse_timestamp(row.available) + reviewer.filter_re = row.donotassign + reviewer.save() + + +# review requests + +# check that we got the needed names +results = { n.name.lower(): n for n in ReviewResultName.objects.all() } + +with db_con.cursor() as c: + c.execute("select distinct summary from reviews;") + summaries = [r[0].lower() for r in c.fetchall() if r[0]] + missing_result_names = set(summaries) - set(results.keys()) + assert not missing_result_names, "missing result names: {} {}".format(missing_result_names, results.keys()) + + for s in summaries: + ReviewTeamResult.objects.get_or_create(team=team, result=results[s]) + +states = { n.slug: n for n in ReviewRequestStateName.objects.all() } +# map some names +states["assigned"] = states["requested"] +states["done"] = states["completed"] +states["noresponse"] = states["no-response"] + +with db_con.cursor() as c: + c.execute("select distinct docstatus from reviews;") + docstates = [r[0] for r in c.fetchall() if r[0]] + missing_state_names = set(docstates) - set(states.keys()) + assert not missing_state_names, "missing state names: {}".format(missing_state_names) + +type_names = { n.slug: n for n in ReviewTypeName.objects.all() } + +# extract relevant log entries + +request_assigned = defaultdict(list) + +with db_con.cursor() as c: + c.execute("select docname, time, who from doclog where text = 'AUTO UPDATED status TO working' order by time desc;") + for row in namedtuplefetchall(c): + request_assigned[row.docname].append((row.time, row.who)) + +# extract document request metadata + +doc_metadata = {} + +with db_con.cursor() as c: + c.execute("select docname, version, deadline, telechat, lcend, status from documents order by docname, version;") + + for row in namedtuplefetchall(c): + doc_metadata[(row.docname, row.version)] = doc_metadata[row.docname] = (parse_timestamp(row.deadline), parse_timestamp(row.telechat), parse_timestamp(row.lcend), row.status) + + +with db_con.cursor() as c: + c.execute("select * from reviews order by reviewid;") + + for row in namedtuplefetchall(c): + meta = doc_metadata.get((row.docname, row.version)) + if not meta: + meta = doc_metadata.get(row.docname) + + deadline, telechat, lcend, status = meta or (None, None, None, None) + + if not deadline: + deadline = parse_timestamp(row.timeout) + + type_name = type_names["unknown"] + # FIXME: use lcend and telechat to try to deduce type + + reviewed_rev = row.version if row.version and row.version != "99" else "" + if row.summary == "noresponse": + reviewed_rev = "" + + assignment_logs = request_assigned.get(row.docname, []) + if assignment_logs: + time, who = assignment_logs.pop() + + time = parse_timestamp(time) + else: + time = deadline + + if not deadline and row.docstatus == "assigned": + # bogus row + print "SKIPPING WITH NO DEADLINE", time, row + continue + + if status == "done" and row.docstatus in ("assigned", "accepted"): + # filter out some apparently dead requests + print "SKIPPING MARKED DONE even if assigned/accepted", time, row + continue + + req, _ = ReviewRequest.objects.get_or_create( + doc_id=row.docname, + team=team, + old_id=row.reviewid, + defaults={ + "state": states["requested"], + "type": type_name, + "deadline": deadline, + } + ) + + req.reviewer = known_personnel[row.reviewer] if row.reviewer else None + req.result = results.get(row.summary.lower()) if row.summary else None + req.state = states.get(row.docstatus) if row.docstatus else None + req.type = type_name + req.time = time + req.reviewed_rev = reviewed_rev + req.deadline = deadline + req.save() + + # FIXME: add log entries + # FIXME: add review from reviewurl + # adcomments IGNORED + # lccomments IGNORED + # nits IGNORED + # reviewurl review.external_url + + #print meta and meta[0], telechat, lcend, req.type + + print "imported review", row.reviewid, "as", req.pk, req.time, req.deadline, req.type, req.doc_id diff --git a/ietf/review/migrations/0001_initial.py b/ietf/review/migrations/0001_initial.py index d9187804b..cb85178c2 100644 --- a/ietf/review/migrations/0001_initial.py +++ b/ietf/review/migrations/0001_initial.py @@ -7,10 +7,10 @@ from django.db import models, migrations class Migration(migrations.Migration): dependencies = [ - ('group', '0008_auto_20160505_0523'), ('name', '0012_insert_review_name_data'), + ('group', '0008_auto_20160505_0523'), + ('person', '0014_auto_20160613_0751'), ('doc', '0012_auto_20160207_0537'), - ('person', '0006_auto_20160503_0937'), ] operations = [ @@ -21,7 +21,7 @@ class Migration(migrations.Migration): ('frequency', models.IntegerField(default=30, help_text=b'Can review every N days')), ('unavailable_until', models.DateTimeField(help_text=b'When will this reviewer be available again', null=True, blank=True)), ('filter_re', models.CharField(max_length=255, blank=True)), - ('skip_next', models.IntegerField(help_text=b'Skip the next N review assignments')), + ('skip_next', models.IntegerField(default=0, help_text=b'Skip the next N review assignments')), ('person', models.ForeignKey(to='person.Person')), ('team', models.ForeignKey(to='group.Group')), ], @@ -33,6 +33,7 @@ class Migration(migrations.Migration): name='ReviewRequest', fields=[ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('old_id', models.IntegerField(help_text=b'ID in previous review system', null=True, blank=True)), ('time', models.DateTimeField(auto_now_add=True)), ('deadline', models.DateTimeField()), ('requested_rev', models.CharField(help_text=b'Fill in if a specific revision is to be reviewed, e.g. 02', max_length=16, verbose_name=b'requested revision', blank=True)), @@ -40,7 +41,7 @@ class Migration(migrations.Migration): ('doc', models.ForeignKey(related_name='review_request_set', to='doc.Document')), ('result', models.ForeignKey(blank=True, to='name.ReviewResultName', null=True)), ('review', models.OneToOneField(null=True, blank=True, to='doc.Document')), - ('reviewer', models.ForeignKey(blank=True, to='group.Role', null=True)), + ('reviewer', models.ForeignKey(blank=True, to='person.Email', null=True)), ('state', models.ForeignKey(to='name.ReviewRequestStateName')), ('team', models.ForeignKey(to='group.Group')), ('type', models.ForeignKey(to='name.ReviewTypeName')), @@ -49,4 +50,15 @@ class Migration(migrations.Migration): }, bases=(models.Model,), ), + migrations.CreateModel( + name='ReviewTeamResult', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('result', models.ForeignKey(to='name.ReviewResultName')), + ('team', models.ForeignKey(to='group.Group')), + ], + options={ + }, + bases=(models.Model,), + ), ] diff --git a/ietf/review/models.py b/ietf/review/models.py index 79092e8b1..589899b91 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -1,8 +1,8 @@ from django.db import models from ietf.doc.models import Document -from ietf.group.models import Group, Role -from ietf.person.models import Person +from ietf.group.models import Group +from ietf.person.models import Person, Email from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName class Reviewer(models.Model): @@ -11,10 +11,21 @@ class Reviewer(models.Model): reviewer and team.""" team = models.ForeignKey(Group) person = models.ForeignKey(Person) - frequency = models.IntegerField(help_text="Can review every N days", default=30) + frequency = models.IntegerField(default=30, help_text="Can review every N days") unavailable_until = models.DateTimeField(blank=True, null=True, help_text="When will this reviewer be available again") filter_re = models.CharField(max_length=255, blank=True) - skip_next = models.IntegerField(help_text="Skip the next N review assignments") + skip_next = models.IntegerField(default=0, help_text="Skip the next N review assignments") + + def __unicode__(self): + return "{} in {}".format(self.person, self.team) + +class ReviewTeamResult(models.Model): + """Captures that a result name is valid for a given team for new + reviews. This also implicitly defines which teams are review + teams - if there are no possible review results valid for a given + team, it can't be a review team.""" + team = models.ForeignKey(Group) + result = models.ForeignKey(ReviewResultName) class ReviewRequest(models.Model): """Represents a request for a review and the process it goes through. @@ -22,21 +33,23 @@ class ReviewRequest(models.Model): document, rev, and reviewer.""" state = models.ForeignKey(ReviewRequestStateName) + old_id = models.IntegerField(blank=True, null=True, help_text="ID in previous review system") # FIXME: remove this when everything has been migrated + # Fields filled in on the initial record creation - these # constitute the request part. time = models.DateTimeField(auto_now_add=True) type = models.ForeignKey(ReviewTypeName) doc = models.ForeignKey(Document, related_name='review_request_set') - team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewresultname=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamresult=None)) deadline = models.DateTimeField() requested_rev = models.CharField(verbose_name="requested revision", max_length=16, blank=True, help_text="Fill in if a specific revision is to be reviewed, e.g. 02") # Fields filled in as reviewer is assigned and as the review is - # uploaded. Once these are filled in and we progress beyond the - # states requested/assigned, any changes to the assignment happens - # by closing down the current request and making a new one, - # copying the request-part fields above. - reviewer = models.ForeignKey(Role, blank=True, null=True) + # uploaded. Once these are filled in and we progress beyond being + # requested/assigned, any changes to the assignment happens by + # closing down the current request and making a new one, copying + # the request-part fields above. + reviewer = models.ForeignKey(Email, blank=True, null=True) review = models.OneToOneField(Document, blank=True, null=True) reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) diff --git a/ietf/review/resources.py b/ietf/review/resources.py index ee2c9c5d9..02aa29d7b 100644 --- a/ietf/review/resources.py +++ b/ietf/review/resources.py @@ -7,7 +7,7 @@ from tastypie.cache import SimpleCache from ietf import api from ietf.api import ToOneField # pyflakes:ignore -from ietf.review.models import Reviewer, ReviewRequest +from ietf.review.models import Reviewer, ReviewRequest, ReviewTeamResult from ietf.person.resources import PersonResource @@ -63,3 +63,22 @@ class ReviewRequestResource(ModelResource): } api.review.register(ReviewRequestResource()) + + +from ietf.group.resources import GroupResource +from ietf.name.resources import ReviewResultNameResource +class ReviewTeamResultResource(ModelResource): + team = ToOneField(GroupResource, 'team') + result = ToOneField(ReviewResultNameResource, 'result') + class Meta: + queryset = ReviewTeamResult.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewteamresult' + filtering = { + "id": ALL, + "team": ALL_WITH_RELATIONS, + "result": ALL_WITH_RELATIONS, + } +api.review.register(ReviewTeamResultResource()) + diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 7da5e1b6b..911b6390b 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -1,14 +1,19 @@ +import datetime +from collections import defaultdict + from django.contrib.sites.models import Site from ietf.group.models import Group, Role -from ietf.doc.models import DocEvent +from ietf.doc.models import Document, DocEvent, State, LastCallDocEvent +from ietf.iesg.models import TelechatDate from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream -from ietf.review.models import ReviewRequestStateName, ReviewRequest +from ietf.review.models import ReviewRequest, ReviewRequestStateName, ReviewTypeName from ietf.utils.mail import send_mail +from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs def active_review_teams(): - # if there's a ReviewResultName defined, it's a review team - return Group.objects.filter(state="active").exclude(reviewresultname=None) + # if there's a ReviewTeamResult defined, it's a review team + return Group.objects.filter(state="active").exclude(reviewteamresult=None) def can_request_review_of_doc(user, doc): if not user.is_authenticated(): @@ -37,11 +42,11 @@ def email_about_review_request(request, review_req, subject, msg, by, notify_sec """Notify possibly both secretary and reviewer about change, skipping a party if the change was done by that party.""" - def extract_email_addresses(roles): - if any(r.person == by for r in roles if r): + def extract_email_addresses(objs): + if any(o.person == by for o in objs if o): return [] else: - return [r.formatted_email() for r in roles if r] + return [o.formatted_email() for o in objs if o] to = [] @@ -92,3 +97,110 @@ def assign_review_request_to_reviewer(request, review_req, reviewer): "Assigned to review of %s" % review_req.doc.name, "%s has assigned you to review the document." % request.user.person, by=request.user.person, notify_secretary=False, notify_reviewer=True) + +def suggested_review_requests_for_team(team): + def fixup_deadline(d): + if d.time() == datetime.time(0): + d = d - datetime.timedelta(seconds=1) # 23:59:59 is treated specially in the view code + return d + + seen_deadlines = {} + + requests = {} + + if True: + # in Last Call + last_call_type = ReviewTypeName.objects.get(slug="lc") + last_call_docs = Document.objects.filter(states=State.objects.get(type="draft-iesg", slug="lc", used=True)) + last_call_expires = { e.doc_id: e.expires for e in LastCallDocEvent.objects.order_by("time", "id") } + for doc in last_call_docs: + deadline = fixup_deadline(last_call_expires.get(doc.pk)) if doc.pk in last_call_expires else datetime.datetime.now() + + if deadline > seen_deadlines.get(doc.pk, datetime.datetime.max): + continue + + requests[doc.pk] = ReviewRequest( + time=None, + type=last_call_type, + doc=doc, + team=team, + deadline=deadline, + ) + + seen_deadlines[doc.pk] = deadline + + + if True: + # on Telechat Agenda + telechat_dates = list(TelechatDate.objects.active().order_by('date').values_list("date", flat=True)[:4]) + + telechat_type = ReviewTypeName.objects.get(slug="telechat") + telechat_deadline_delta = datetime.timedelta(days=2) + telechat_docs = Document.objects.filter(docevent__telechatdocevent__telechat_date__in=telechat_dates) + for doc in telechat_docs: + d = doc.telechat_date() + if d not in telechat_dates: + continue + + deadline = datetime.datetime.combine(d - telechat_deadline_delta, datetime.time(23, 59, 59)) + + if deadline > seen_deadlines.get(doc.pk, datetime.datetime.max): + continue + + requests[doc.pk] = ReviewRequest( + time=None, + type=telechat_type, + doc=doc, + team=team, + deadline=deadline, + ) + + seen_deadlines[doc.pk] = deadline + + # filter those with existing requests + existing_requests = defaultdict(list) + for r in ReviewRequest.objects.filter(doc__in=requests.iterkeys()): + existing_requests[r.doc_id].append(r) + + def blocks(existing, request): + return (existing.doc_id == request.doc_id + and existing.reviewed_rev == request.doc.rev + and existing.state_id not in ("part-completed", "rejected", "overtaken")) + + res = [r for r in requests.itervalues() if not any(blocks(e, r) for e in existing_requests[r.doc_id])] + res.sort(key=lambda r: (r.deadline, r.doc_id)) + return res + +def extract_revision_ordered_review_requests_for_documents(queryset, names): + names = set(names) + + replaces = extract_complete_replaces_ancestor_mapping_for_docs(names) + + requests_for_each_doc = defaultdict(list) + for r in queryset.filter(doc__in=set(e for l in replaces.itervalues() for e in l) | names).order_by("-reviewed_rev", "-time", "-id").iterator(): + requests_for_each_doc[r.doc_id].append(r) + + # now collect in breadth-first order to keep the revision order intact + res = defaultdict(list) + for name in names: + front = replaces.get(name, []) + res[name].extend(requests_for_each_doc.get(name, [])) + + while front: + replaces_reqs = [] + for replaces_name in front: + reqs = requests_for_each_doc.get(replaces_name, []) + if reqs: + replaces_reqs.append(reqs) + + # in case there are multiple replaces, move the ones with + # the latest reviews up front + replaces_reqs.sort(key=lambda l: l[0].time, reverse=True) + + for reqs in replaces_reqs: + res[name].extend(reqs) + + # move one level down + front = [n for l in requests_for_each_doc.get(replaces_name, []) for n in l] + + return res diff --git a/ietf/static/ietf/css/ietf.css b/ietf/static/ietf/css/ietf.css index 0da7cec77..c8230106c 100644 --- a/ietf/static/ietf/css/ietf.css +++ b/ietf/static/ietf/css/ietf.css @@ -462,6 +462,11 @@ label#list-feeds { /* Review flow */ +.reviewer-assignment-not-accepted { + margin-top: 0.5em; + margin-bottom: 0.5em; +} + form.complete-review .mail-archive-search .query-input { width: 30em; } @@ -472,6 +477,16 @@ form.complete-review .mail-archive-search .results .list-group { margin-bottom: 0.5em; } +.closed-review-filter { + margin-bottom: 1em; +} + +form.review-requests .reviewer-controls, form.review-requests .close-controls { + display: none; +} + +/* Profile */ + .photo-name { height: 3em; } diff --git a/ietf/static/ietf/js/manage-review-requests.js b/ietf/static/ietf/js/manage-review-requests.js new file mode 100644 index 000000000..62a38438d --- /dev/null +++ b/ietf/static/ietf/js/manage-review-requests.js @@ -0,0 +1,37 @@ +$(document).ready(function () { + var form = $("form.review-requests"); + + form.find(".reviewer-action").on("click", function () { + var row = $(this).closest("tr"); + row.find(".close-controls .undo").click(); + row.find("[name$=\"-action\"]").val("assign"); + row.find(".reviewer-action").hide(); + row.find(".reviewer-controls").show(); + }); + + form.find(".reviewer-controls .undo").on("click", function () { + var row = $(this).closest("tr"); + row.find(".reviewer-controls").hide(); + row.find(".reviewer-action").show(); + row.find("[name$=\"-action\"]").val(""); + }); + + form.find(".close-action").on("click", function () { + var row = $(this).closest("tr"); + row.find(".reviewer-controls .undo").click(); + row.find("[name$=\"-action\"]").val("close"); + row.find(".close-action").hide(); + row.find(".close-controls").show(); + }); + + form.find(".close-controls .undo").on("click", function () { + var row = $(this).closest("tr"); + row.find("[name$=\"-action\"]").val(""); + row.find(".close-controls").hide(); + row.find(".close-action").show(); + }); + + form.find("[name$=\"-action\"]").each(function () { + console.log(this); + }); +}); diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 3a0cd68aa..8bb31a851 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -201,9 +201,9 @@ {% for r in review_requests %}
{% if r.state_id == "completed" or r.state_id == "part-completed" %} - {{ r.team.acronym|upper }} {{ r.type.name }} Review{% if r.reviewed_rev and r.reviewed_rev != doc.rev %} (of -{{ r.reviewed_rev }}){% endif %}: {{ r.result.name }} + {{ r.team.acronym|upper }} {{ r.type.name }} Review{% if r.reviewed_rev and r.reviewed_rev != doc.rev %} (of -{{ r.reviewed_rev }}){% endif %}: {{ r.result.name }} {% if r.state_id == "part-completed" %}(partially completed){% endif %} - reviewer: {{ r.reviewer.person }} {% else %} - {{ r.team.acronym|upper }} {{ r.type.name }} Review ({{ r.state.name }}) + {{ r.team.acronym|upper }} {{ r.type.name }} Review{% if r.reviewer %} (reviewer: {{ r.reviewer.person }}){% endif %} {% endif %}
{% endfor %} diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index 5e144904f..3c9826b37 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -102,8 +102,10 @@ {% if review_req.review %} {{ review_req.review.name }} - {% else %} + {% elif review_req.state_id == "requested" or review_req.state_id == "accepted" %} Not completed yet + {% else %} + Not available {% endif %} {% if can_complete_review %} diff --git a/ietf/templates/group/manage_review_requests.html b/ietf/templates/group/manage_review_requests.html new file mode 100644 index 000000000..9e13bb04f --- /dev/null +++ b/ietf/templates/group/manage_review_requests.html @@ -0,0 +1,109 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2015, All Rights Reserved #} +{% load origin %}{% origin %} + +{% load ietf_filters staticfiles bootstrap3 %} + +{% block title %}Manage pending review requests for {{ group.acronym }}{% endblock %} + +{% block pagehead %} + +{% endblock %} + +{% block content %} + {% origin %} + +

Manage open review requests for {{ group.acronym }}

+ +

For reference: closed review requests + + {% if review_requests %} +

{% csrf_token %} + + + + + + + + + + + + + {% for r in review_requests %} + + + + + + + + + {% endfor %} + +
DocumentTypeRequestedDeadlineReviewerClose as...
{{ r.doc.name }}{% if r.requested_rev %}-{{ r.requested_rev }}{% endif %} + {% if r.latest_reqs %} +
+ - prev. review: + {% for rlatest in r.latest_reqs %} + {{ rlatest.result.name }} + (diff){% if not forloop.last %},{% endif %} + {% endfor %} + + {% endif %} + 		{{ r.type.name }}{% if r.time %}{{ r.time|date:"Y-m-d" }}{% else %}auto-suggested{% endif %} + {% if r.deadline|date:"H:i" != "23:59" %} + {{ r.deadline|date:"Y-m-d H:i" }} + {% else %} + {{ r.deadline|date:"Y-m-d" }} + {% endif %} + {% if r.due %}{{ r.due }} hour{{ r.due|pluralize }}{% endif %} + + {% if r.reviewer %} + + {% else %} + + {% endif %} + + {{ r.form.action }} + + + {% spaceless %} + {{ r.form.reviewer }} + + {% if r.form.reviewer.errors %} +
+ {{ r.form.reviewer.errors }} + {% endif %} + {% endspaceless %} + + 		+ + + + {% spaceless %} + {{ r.form.close }} + + {% if r.form.close.errors %} +
+ {{ r.form.close.errors }} + {% endif %} + {% endspaceless %} + +
+ + {% buttons %} + Cancel + + {% endbuttons %} +
+ {% else %} +

There are currently no open requests.

+ {% endif %} +{% endblock %} + +{% block js %} + + +{% endblock %} diff --git a/ietf/templates/group/review_requests.html b/ietf/templates/group/review_requests.html new file mode 100644 index 000000000..38a75e20d --- /dev/null +++ b/ietf/templates/group/review_requests.html @@ -0,0 +1,121 @@ +{% extends "group/group_base.html" %} +{# Copyright The IETF Trust 2015, All Rights Reserved #} +{% load origin %}{% origin %} + +{% load ietf_filters staticfiles bootstrap3 %} + +{% block group_subtitle %}Reviews for {{ group.name }}{% endblock %} + +{% block pagehead %} + +{% endblock %} + +{% block group_content %} + {% origin %} + +

Open review requests

+ + {% if open_review_requests %} + + + + + + + + + + + + {% for r in open_review_requests %} + + + + + + + + {% endfor %} + +
RequestTypeRequestedDeadlineReviewer
{{ r.doc.name }}{% if r.requested_rev %}-{{ r.requested_rev }}{% endif %}{{ r.type.name }}{% if r.time %}{{ r.time|date:"Y-m-d" }}{% else %}auto-suggested{% endif %} + {% if r.deadline|date:"H:i" != "23:59" %} + {{ r.deadline|date:"Y-m-d H:i" }} + {% else %} + {{ r.deadline|date:"Y-m-d" }} + {% endif %} + {% if r.due %}{{ r.due }} hour{{ r.due|pluralize }}{% endif %} + + {% if r.reviewer %} + {{ r.reviewer.person }} {% if r.state_id == "accepted" %}Accepted{% endif %} + {% elif r.pk != None %} + not yet assigned + {% endif %} +
+ + {% else %} +

There are currently no open requests.

+ {% endif %} + +

Closed review requests

+ +
+ Past: +
+ {% for key, label in since_choices %} + + {% endfor %} +
+
+ + {% if closed_review_requests %} + + + + + + + + + + + + + + {% for r in closed_review_requests %} + + + + + + + + + + {% endfor %} + +
RequestTypeRequestedDeadlineReviewerStateResult
{{ r.doc.name }}{% if r.requested_rev %}-{{ r.requested_rev }}{% endif %}{{ r.type }}{{ r.time|date:"Y-m-d" }} + {% if r.deadline|date:"H:i" != "23:59" %} + {{ r.deadline|date:"Y-m-d H:i" }} + {% else %} + {{ r.deadline|date:"Y-m-d" }} + {% endif %} + + {% if r.reviewer %} + {{ r.reviewer.person }} + {% else %} + not yet assigned + {% endif %} + {{ r.state.name }} + {% if r.result %} + {{ r.result.name }} + {% endif %}
+ + {% else %} +

No closed requests found.

+ {% endif %} + +{% endblock %} + +{% block js %} + +{% endblock %} diff --git a/ietf/utils/test_data.py b/ietf/utils/test_data.py index 638294058..d19be0f93 100644 --- a/ietf/utils/test_data.py +++ b/ietf/utils/test_data.py @@ -13,6 +13,7 @@ from ietf.ipr.models import HolderIprDisclosure, IprDocRel, IprDisclosureStateNa from ietf.meeting.models import Meeting from ietf.name.models import StreamName, DocRelationshipName from ietf.person.models import Person, Email +from ietf.review.models import ReviewRequest, Reviewer, ReviewResultName, ReviewTeamResult def create_person(group, role_name, name=None, username=None, email_address=None, password=None): """Add person/user/email and role.""" @@ -357,3 +358,31 @@ def make_test_data(): #other_doc_factory('recording','recording-42-mars-1-00') return draft + +def make_review_data(doc): + team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team") + for r in ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"]): + ReviewTeamResult.objects.create(team=team, result=r) + + p = Person.objects.get(user__username="plain") + email = p.email_set.first() + Role.objects.create(name_id="reviewer", person=p, email=email, group=team) + Reviewer.objects.create(team=team, person=p, frequency=14, skip_next=0) + + review_req = ReviewRequest.objects.create( + doc=doc, + team=team, + type_id="early", + deadline=datetime.datetime.now() + datetime.timedelta(days=20), + state_id="accepted", + reviewer=email, + ) + + p = Person.objects.get(user__username="marschairman") + Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team) + + p = Person.objects.get(user__username="secretary") + Role.objects.create(name_id="secretary", person=p, email=p.email_set.first(), group=team) + + return review_req +