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 +