From 54c4c5efc561297591db538f3212ac3974862cf6 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Thu, 19 May 2016 15:33:02 +0000 Subject: [PATCH 01/16] Make the test data new revision event a proper NewRevisionDocEvent so it's consistent with what the code expects - Legacy-Id: 11205 --- ietf/utils/test_data.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ietf/utils/test_data.py b/ietf/utils/test_data.py index 7d04ff6f1..638294058 100644 --- a/ietf/utils/test_data.py +++ b/ietf/utils/test_data.py @@ -6,7 +6,7 @@ from django.contrib.auth.models import User import debug # pyflakes:ignore -from ietf.doc.models import Document, DocAlias, State, DocumentAuthor, BallotType, DocEvent, BallotDocEvent, RelatedDocument +from ietf.doc.models import Document, DocAlias, State, DocumentAuthor, BallotType, DocEvent, BallotDocEvent, RelatedDocument, NewRevisionDocEvent from ietf.group.models import Group, GroupHistory, Role, RoleHistory from ietf.iesg.models import TelechatDate from ietf.ipr.models import HolderIprDisclosure, IprDocRel, IprDisclosureStateName, IprLicenseTypeName @@ -247,11 +247,12 @@ def make_test_data(): desc="Started IESG process", ) - DocEvent.objects.create( + NewRevisionDocEvent.objects.create( type="new_revision", by=ad, doc=draft, desc="New revision available", + rev="01", ) BallotDocEvent.objects.create( From 64a65340a21123b2896b9ddf502d6ab489f828b6 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Thu, 19 May 2016 15:35:30 +0000 Subject: [PATCH 02/16] Add review tracking models, add a request review page (with test), show review requests on doc page - Legacy-Id: 11206 --- ietf/doc/models.py | 5 +- ietf/doc/tests_review.py | 82 +++++++ ietf/doc/urls.py | 3 + ietf/doc/utils.py | 11 +- ietf/doc/views_doc.py | 14 +- ietf/doc/views_review.py | 108 +++++++++ ietf/name/admin.py | 6 +- ietf/name/fixtures/names.json | 211 +++++++++++++++++- ietf/name/generate_fixtures.py | 2 + ...atename_reviewresultname_reviewtypename.py | 61 +++++ .../0012_insert_review_name_data.py | 48 ++++ ietf/name/models.py | 11 + ietf/name/resources.py | 46 +++- ietf/review/__init__.py | 0 ietf/review/migrations/0001_initial.py | 50 +++++ ietf/review/migrations/__init__.py | 0 ietf/review/models.py | 40 ++++ ietf/review/utils.py | 6 + ietf/settings.py | 1 + ietf/templates/doc/document_draft.html | 22 ++ ietf/templates/doc/review/request_review.html | 34 +++ 21 files changed, 753 insertions(+), 8 deletions(-) create mode 100644 ietf/doc/tests_review.py create mode 100644 ietf/doc/views_review.py create mode 100644 ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py create mode 100644 ietf/name/migrations/0012_insert_review_name_data.py create mode 100644 ietf/review/__init__.py create mode 100644 ietf/review/migrations/0001_initial.py create mode 100644 ietf/review/migrations/__init__.py create mode 100644 ietf/review/models.py create mode 100644 ietf/review/utils.py create mode 100644 ietf/templates/doc/review/request_review.html diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 4a772db77..157255dfd 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -703,7 +703,10 @@ EVENT_TYPES = [ # RFC Editor ("rfc_editor_received_announcement", "Announcement was received by RFC Editor"), - ("requested_publication", "Publication at RFC Editor requested") + ("requested_publication", "Publication at RFC Editor requested"), + + # review + ("requested_review", "Requested review"), ] class DocEvent(models.Model): diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py new file mode 100644 index 000000000..fa0893587 --- /dev/null +++ b/ietf/doc/tests_review.py @@ -0,0 +1,82 @@ +# -*- coding: utf-8 -*- + +import datetime +from pyquery import PyQuery + +from django.core.urlresolvers import reverse as urlreverse + +import debug # pyflakes:ignore + +from ietf.review.models import ReviewRequest +from ietf.person.models import Person +from ietf.group.models import Group, Role +from ietf.name.models import ReviewResultName +from ietf.utils.test_utils import TestCase +from ietf.utils.test_data import make_test_data +from ietf.utils.test_utils import login_testing_unauthorized + +def make_review_data(): + review_team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team") + review_team.reviewresultname_set.add(ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"])) + + p = Person.objects.get(user__username="plain") + Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=review_team) + + return review_team + +class ReviewTests(TestCase): + def test_request_review(self): + doc = make_test_data() + review_team = make_review_data() + + url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name }) + login_testing_unauthorized(self, "secretary", url) + + # get + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + deadline_date = datetime.date.today() + datetime.timedelta(days=10) + + # post request + r = self.client.post(url, { + "type": "early", + "team": review_team.pk, + "deadline_date": deadline_date.isoformat(), + "requested_rev": "01" + }) + self.assertEqual(r.status_code, 302) + + req = ReviewRequest.objects.get(doc=doc) + self.assertEqual(req.deadline.date(), deadline_date) + self.assertEqual(req.deadline.time(), datetime.time(23, 59, 59)) + self.assertEqual(req.state_id, "requested") + self.assertEqual(req.team, review_team) + self.assertEqual(req.requested_rev, "01") + self.assertEqual(doc.latest_event().type, "requested_review") + + def test_request_review_by_reviewer(self): + doc = make_test_data() + review_team = make_review_data() + + url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name }) + login_testing_unauthorized(self, "plain", url) + + # post request + deadline_date = datetime.date.today() + datetime.timedelta(days=10) + + r = self.client.post(url, { + "type": "early", + "team": review_team.pk, + "deadline_date": deadline_date.isoformat(), + "requested_rev": "01" + }) + self.assertEqual(r.status_code, 302) + + req = ReviewRequest.objects.get(doc=doc) + self.assertEqual(req.state_id, "requested") + self.assertEqual(req.team, review_team) + + def test_doc_page(self): + pass + diff --git a/ietf/doc/urls.py b/ietf/doc/urls.py index 235a95c17..188ccbb6d 100644 --- a/ietf/doc/urls.py +++ b/ietf/doc/urls.py @@ -36,6 +36,7 @@ from django.views.generic import RedirectView from ietf.doc import views_search, views_draft, views_ballot from ietf.doc import views_status_change from ietf.doc import views_doc +from ietf.doc import views_review session_patterns = [ url(r'^add$', views_doc.add_sessionpresentation), @@ -73,6 +74,8 @@ urlpatterns = patterns('', url(r'^(?P[A-Za-z0-9._+-]+)/ballot/$', views_doc.document_ballot, name="doc_ballot"), (r'^(?P[A-Za-z0-9._+-]+)/(?:(?P[0-9-]+)/)?doc.json$', views_doc.document_json), (r'^(?P[A-Za-z0-9._+-]+)/ballotpopup/(?P[0-9]+)/$', views_doc.ballot_popup), + url(r'^(?P[A-Za-z0-9._+-]+)/requestreview/$', views_review.request_review), + url(r'^(?P[A-Za-z0-9._+-]+)/review/(?P[0-9]+)/$', views_review.review), url(r'^(?P[A-Za-z0-9._+-]+)/email-aliases/$', RedirectView.as_view(pattern_name='doc_email', permanent=False),name='doc_specific_email_aliases'), diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index bb483e14d..923715895 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -16,7 +16,7 @@ from ietf.doc.models import DocEvent, ConsensusDocEvent, BallotDocEvent, NewRevi from ietf.doc.models import save_document_in_history from ietf.name.models import DocReminderTypeName, DocRelationshipName from ietf.group.models import Role -from ietf.ietfauth.utils import has_role +from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream from ietf.utils import draft, markup_txt from ietf.utils.mail import send_mail from ietf.mailtrigger.utils import gather_address_lists @@ -89,6 +89,15 @@ def can_adopt_draft(user, doc): group__state="active", person__user=user).exists()) +def can_request_review_of_doc(user, doc): + if not user.is_authenticated(): + return False + + from ietf.review.utils import active_review_teams + if Role.objects.filter(name="reviewer", person__user=user, group__in=active_review_teams()): + return True + + return is_authorized_in_doc_stream(user, doc) def two_thirds_rule( recused=0 ): # For standards-track, need positions from 2/3 of the non-recused current IESG. diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 5be36fd26..d25d88fc3 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -48,7 +48,8 @@ from ietf.doc.models import ( Document, DocAlias, DocHistory, DocEvent, BallotDo from ietf.doc.utils import ( add_links_in_new_revision_events, augment_events_with_revision, can_adopt_draft, get_chartering_type, get_document_content, get_tags_for_stream_id, needed_ballot_positions, nice_consensus, prettify_std_name, update_telechat, has_same_ballot, - get_initial_notify, make_notify_changed_event, crawl_history, default_consensus) + get_initial_notify, make_notify_changed_event, crawl_history, default_consensus, + can_request_review_of_doc ) from ietf.community.utils import augment_docs_with_tracking_info from ietf.group.models import Role from ietf.group.utils import can_manage_group, can_manage_materials @@ -57,10 +58,11 @@ from ietf.name.models import StreamName, BallotPositionName from ietf.person.models import Email from ietf.utils.history import find_history_active_at from ietf.doc.forms import TelechatForm, NotifyForm -from ietf.doc.mails import email_comment +from ietf.doc.mails import email_comment from ietf.mailtrigger.utils import gather_relevant_expansions from ietf.meeting.models import Session from ietf.meeting.utils import group_sessions, get_upcoming_manageable_sessions, sort_sessions +from ietf.review.models import ReviewRequest def render_document_top(request, doc, tab, name): tabs = [] @@ -279,8 +281,8 @@ def document_main(request, name, rev=None): can_edit_stream_info = is_authorized_in_doc_stream(request.user, doc) can_edit_shepherd_writeup = can_edit_stream_info or user_is_person(request.user, doc.shepherd and doc.shepherd.person) or has_role(request.user, ["Area Director"]) can_edit_notify = can_edit_shepherd_writeup - can_edit_consensus = False + can_edit_consensus = False consensus = nice_consensus(default_consensus(doc)) if doc.stream_id == "ietf" and iesg_state: show_in_states = set(IESG_BALLOT_ACTIVE_STATES) @@ -294,6 +296,8 @@ def document_main(request, name, rev=None): e = doc.latest_event(ConsensusDocEvent, type="changed_consensus") consensus = nice_consensus(e and e.consensus) + can_request_review = can_request_review_of_doc(request.user, doc) + # mailing list search archive search_archive = "www.ietf.org/mail-archive/web/" if doc.stream_id == "ietf" and group.type_id == "wg" and group.list_archive: @@ -353,6 +357,8 @@ 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) + return render_to_response("doc/document_draft.html", dict(doc=doc, group=group, @@ -374,6 +380,7 @@ def document_main(request, name, rev=None): can_edit_consensus=can_edit_consensus, can_edit_replaces=can_edit_replaces, can_view_possibly_replaces=can_view_possibly_replaces, + can_request_review=can_request_review, rfc_number=rfc_number, draft_name=draft_name, @@ -412,6 +419,7 @@ def document_main(request, name, rev=None): search_archive=search_archive, actions=actions, presentations=presentations, + review_requests=review_requests, ), context_instance=RequestContext(request)) diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py new file mode 100644 index 000000000..5578dcd97 --- /dev/null +++ b/ietf/doc/views_review.py @@ -0,0 +1,108 @@ +import datetime + +from django.http import HttpResponseForbidden +from django.shortcuts import render, get_object_or_404, redirect +from django.core.urlresolvers import reverse as urlreverse +from django import forms +from django.contrib.auth.decorators import login_required + +from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent +from ietf.doc.utils import can_request_review_of_doc +from ietf.ietfauth.utils import is_authorized_in_doc_stream +from ietf.review.models import ReviewRequest, ReviewRequestStateName +from ietf.review.utils import active_review_teams +from ietf.utils.fields import DatepickerDateField + + +class RequestReviewForm(forms.ModelForm): + deadline_date = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={ "autoclose": "1", "start-date": "+0d" }) + deadline_time = forms.TimeField(widget=forms.TextInput(attrs={ 'placeholder': "HH:MM" }), help_text="If time is not specified, end of day is assumed", required=False) + + class Meta: + model = ReviewRequest + fields = ('type', 'team', 'deadline', 'requested_rev') + + def __init__(self, user, doc, *args, **kwargs): + super(RequestReviewForm, self).__init__(*args, **kwargs) + + self.doc = doc + + self.fields['type'].widget = forms.RadioSelect(choices=[t for t in self.fields['type'].choices if t[0]]) + + f = self.fields["team"] + f.queryset = active_review_teams() + if not is_authorized_in_doc_stream(user, doc): # user is a reviewer + f.queryset = f.queryset.filter(role__name="reviewer", role__person__user=user) + if len(f.queryset) < 6: + f.widget = forms.RadioSelect(choices=[t for t in f.choices if t[0]]) + + self.fields["deadline"].required = False + self.fields["requested_rev"].label = "Document revision" + + def clean_deadline_date(self): + v = self.cleaned_data.get('deadline_date') + if v < datetime.date.today(): + raise forms.ValidationError("Select a future date.") + return v + + def clean_requested_rev(self): + rev = self.cleaned_data.get("requested_rev") + if rev: + rev = rev.rjust(2, "0") + + if not NewRevisionDocEvent.objects.filter(doc=self.doc, rev=rev).exists(): + raise forms.ValidationError("Could not find revision '{}' of the document.".format(rev)) + + return rev + + def clean(self): + deadline_date = self.cleaned_data.get('deadline_date') + deadline_time = self.cleaned_data.get('deadline_time', None) + + if deadline_date: + if deadline_time is None: + deadline_time = datetime.time(23, 59, 59) + + self.cleaned_data["deadline"] = datetime.datetime.combine(deadline_date, deadline_time) + + return self.cleaned_data + +@login_required +def request_review(request, name): + doc = get_object_or_404(Document, name=name) + + if not can_request_review_of_doc(request.user, doc): + return HttpResponseForbidden("You do not have permission to perform this action") + + if request.method == "POST": + form = RequestReviewForm(request.user, doc, request.POST) + + if form.is_valid(): + review_req = form.save(commit=False) + review_req.doc = doc + review_req.state = ReviewRequestStateName.objects.get(slug="requested", used=True) + review_req.save() + + DocEvent.objects.create( + type="requested_review", + doc=doc, + by=request.user.person, + desc="{} review by {} requested".format(review_req.type.name, review_req.team.acronym.upper()), + ) + + # FIXME: if I'm a reviewer, auto-assign to myself? + return redirect('doc_view', name=doc.name) + + else: + form = RequestReviewForm(request.user, doc) + + return render(request, 'doc/review/request_review.html', { + 'doc': doc, + 'form': form, + }) + +def review(request, name, request_id): + doc = get_object_or_404(Document, name=name) + review_request = get_object_or_404(ReviewRequest, pk=request_id) + + print doc, review_request diff --git a/ietf/name/admin.py b/ietf/name/admin.py index 177501f1d..608620ff1 100644 --- a/ietf/name/admin.py +++ b/ietf/name/admin.py @@ -3,7 +3,8 @@ from ietf.name.models import (GroupTypeName, GroupStateName, RoleName, StreamNam DocRelationshipName, DocTypeName, DocTagName, StdLevelName, IntendedStdLevelName, DocReminderTypeName, BallotPositionName, SessionStatusName, TimeSlotTypeName, ConstraintName, NomineePositionStateName, FeedbackTypeName, DBTemplateTypeName, - DraftSubmissionStateName, RoomResourceName) + DraftSubmissionStateName, RoomResourceName, + ReviewRequestStateName, ReviewTypeName, ReviewResultName) class NameAdmin(admin.ModelAdmin): @@ -35,3 +36,6 @@ admin.site.register(FeedbackTypeName, NameAdmin) admin.site.register(DBTemplateTypeName, NameAdmin) admin.site.register(DraftSubmissionStateName, NameAdmin) admin.site.register(RoomResourceName, NameAdmin) +admin.site.register(ReviewRequestStateName, NameAdmin) +admin.site.register(ReviewTypeName, NameAdmin) +admin.site.register(ReviewResultName, NameAdmin) diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 0f355ee2f..5d9e8def4 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -188,7 +188,7 @@ "order": 0, "revname": "Conflict reviewed by", "used": true, - "name": "Conflict reviews", + "name": "conflict reviews", "desc": "" }, "model": "name.docrelationshipname", @@ -1752,6 +1752,205 @@ "model": "name.nomineepositionstatename", "pk": "declined" }, +{ + "fields": { + "order": 1, + "used": true, + "name": "Requested", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "requested" +}, +{ + "fields": { + "order": 2, + "used": true, + "name": "Accepted", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "accepted" +}, +{ + "fields": { + "order": 3, + "used": true, + "name": "Rejected", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "rejected" +}, +{ + "fields": { + "order": 4, + "used": true, + "name": "Withdrawn", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "withdrawn" +}, +{ + "fields": { + "order": 5, + "used": true, + "name": "Overtaken By Events", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "overtaken" +}, +{ + "fields": { + "order": 6, + "used": true, + "name": "No Response", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "noresponse" +}, +{ + "fields": { + "order": 7, + "used": true, + "name": "Completed", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "completed" +}, +{ + "fields": { + "order": 1, + "used": true, + "teams": [], + "name": "Almost Ready", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "almost-ready" +}, +{ + "fields": { + "order": 2, + "used": true, + "teams": [], + "name": "Has Issues", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "issues" +}, +{ + "fields": { + "order": 3, + "used": true, + "teams": [], + "name": "Has Nits", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "nits" +}, +{ + "fields": { + "order": 4, + "used": true, + "teams": [], + "name": "Not Ready", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "not-ready" +}, +{ + "fields": { + "order": 5, + "used": true, + "teams": [], + "name": "On the Right Track", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "right-track" +}, +{ + "fields": { + "order": 6, + "used": true, + "teams": [], + "name": "Ready", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "ready" +}, +{ + "fields": { + "order": 7, + "used": true, + "teams": [], + "name": "Ready with Issues", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "ready-issues" +}, +{ + "fields": { + "order": 8, + "used": true, + "teams": [], + "name": "Ready with Nits", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "ready-nits" +}, +{ + "fields": { + "order": 9, + "used": true, + "teams": [], + "name": "Serious Issues", + "desc": "" + }, + "model": "name.reviewresultname", + "pk": "serious-issues" +}, +{ + "fields": { + "order": 1, + "used": true, + "name": "Early", + "desc": "" + }, + "model": "name.reviewtypename", + "pk": "early" +}, +{ + "fields": { + "order": 2, + "used": true, + "name": "Last Call", + "desc": "" + }, + "model": "name.reviewtypename", + "pk": "lc" +}, +{ + "fields": { + "order": 3, + "used": true, + "name": "Telechat", + "desc": "" + }, + "model": "name.reviewtypename", + "pk": "telechat" +}, { "fields": { "order": 0, @@ -1942,6 +2141,16 @@ "model": "name.rolename", "pk": "matman" }, +{ + "fields": { + "order": 14, + "used": true, + "name": "Reviewer", + "desc": "" + }, + "model": "name.rolename", + "pk": "reviewer" +}, { "fields": { "order": 0, diff --git a/ietf/name/generate_fixtures.py b/ietf/name/generate_fixtures.py index a413cbd13..753f6b8ad 100644 --- a/ietf/name/generate_fixtures.py +++ b/ietf/name/generate_fixtures.py @@ -1,5 +1,7 @@ #!/usr/bin/python +# simple script for exporting name related base data for the tests + # boiler plate import os, sys import django diff --git a/ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py b/ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py new file mode 100644 index 000000000..fb62deea3 --- /dev/null +++ b/ietf/name/migrations/0011_reviewrequeststatename_reviewresultname_reviewtypename.py @@ -0,0 +1,61 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('group', '0008_auto_20160505_0523'), + ('name', '0010_new_liaison_names'), + ] + + operations = [ + migrations.CreateModel( + name='ReviewRequestStateName', + fields=[ + ('slug', models.CharField(max_length=32, serialize=False, primary_key=True)), + ('name', models.CharField(max_length=255)), + ('desc', models.TextField(blank=True)), + ('used', models.BooleanField(default=True)), + ('order', models.IntegerField(default=0)), + ], + options={ + 'ordering': ['order'], + 'abstract': False, + }, + bases=(models.Model,), + ), + migrations.CreateModel( + name='ReviewResultName', + fields=[ + ('slug', models.CharField(max_length=32, serialize=False, primary_key=True)), + ('name', models.CharField(max_length=255)), + ('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'], + 'abstract': False, + }, + bases=(models.Model,), + ), + migrations.CreateModel( + name='ReviewTypeName', + fields=[ + ('slug', models.CharField(max_length=32, serialize=False, primary_key=True)), + ('name', models.CharField(max_length=255)), + ('desc', models.TextField(blank=True)), + ('used', models.BooleanField(default=True)), + ('order', models.IntegerField(default=0)), + ], + options={ + 'ordering': ['order'], + 'abstract': False, + }, + bases=(models.Model,), + ), + ] diff --git a/ietf/name/migrations/0012_insert_review_name_data.py b/ietf/name/migrations/0012_insert_review_name_data.py new file mode 100644 index 000000000..ed1a4bfe6 --- /dev/null +++ b/ietf/name/migrations/0012_insert_review_name_data.py @@ -0,0 +1,48 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations + + +def insert_initial_review_data(apps, schema_editor): + ReviewRequestStateName = apps.get_model("name", "ReviewRequestStateName") + ReviewRequestStateName.objects.get_or_create(slug="requested", name="Requested", order=1) + ReviewRequestStateName.objects.get_or_create(slug="accepted", name="Accepted", order=2) + 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="completed", name="Completed", order=7) + + 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) + + ReviewResultName = apps.get_model("name", "ReviewResultName") + ReviewResultName.objects.get_or_create(slug="almost-ready", name="Almost Ready", order=1) + ReviewResultName.objects.get_or_create(slug="issues", name="Has Issues", order=2) + ReviewResultName.objects.get_or_create(slug="nits", name="Has Nits", order=3) + ReviewResultName.objects.get_or_create(slug="not-ready", name="Not Ready", order=4) + ReviewResultName.objects.get_or_create(slug="right-track", name="On the Right Track", order=5) + ReviewResultName.objects.get_or_create(slug="ready", name="Ready", order=6) + ReviewResultName.objects.get_or_create(slug="ready-issues", name="Ready with Issues", order=7) + ReviewResultName.objects.get_or_create(slug="ready-nits", name="Ready with Nits", order=8) + ReviewResultName.objects.get_or_create(slug="serious-issues", name="Serious Issues", order=9) + + RoleName = apps.get_model("name", "RoleName") + RoleName.objects.get_or_create(slug="reviewer", name="Reviewer", order=max(r.order for r in RoleName.objects.all()) + 1) + +def noop(apps, schema_editor): + pass + +class Migration(migrations.Migration): + + dependencies = [ + ('name', '0011_reviewrequeststatename_reviewresultname_reviewtypename'), + ('group', '0001_initial'), + ] + + operations = [ + migrations.RunPython(insert_initial_review_data, noop), + ] diff --git a/ietf/name/models.py b/ietf/name/models.py index 3dd29d5ba..dad83c97f 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -87,3 +87,14 @@ class LiaisonStatementEventTypeName(NameModel): "Submitted, Modified, Approved, Posted, Killed, Resurrected, MsgIn, MsgOut, Comment" class LiaisonStatementTagName(NameModel): "Action Required, Action Taken" +class ReviewRequestStateName(NameModel): + """Requested, Accepted, Rejected, Withdrawn, Overtaken By Events, + No Response , Completed""" +class ReviewTypeName(NameModel): + """Early Review, Last Call, Telechat""" +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 57e03841a..521f4ee79 100644 --- a/ietf/name/resources.py +++ b/ietf/name/resources.py @@ -13,7 +13,8 @@ from ietf.name.models import (TimeSlotTypeName, GroupStateName, DocTagName, Inte IprEventTypeName, GroupMilestoneStateName, SessionStatusName, DocReminderTypeName, ConstraintName, MeetingTypeName, DocRelationshipName, RoomResourceName, IprLicenseTypeName, LiaisonStatementTagName, FeedbackTypeName, LiaisonStatementState, StreamName, - BallotPositionName, DBTemplateTypeName, NomineePositionStateName) + BallotPositionName, DBTemplateTypeName, NomineePositionStateName, + ReviewRequestStateName, ReviewTypeName, ReviewResultName) class TimeSlotTypeNameResource(ModelResource): @@ -413,3 +414,46 @@ class NomineePositionStateNameResource(ModelResource): } api.name.register(NomineePositionStateNameResource()) +class ReviewRequestStateNameResource(ModelResource): + class Meta: + cache = SimpleCache() + queryset = ReviewRequestStateName.objects.all() + #resource_name = 'reviewrequeststatename' + filtering = { + "slug": ALL, + "name": ALL, + "desc": ALL, + "used": ALL, + "order": ALL, + } +api.name.register(ReviewRequestStateNameResource()) + +class ReviewTypeNameResource(ModelResource): + class Meta: + cache = SimpleCache() + queryset = ReviewTypeName.objects.all() + #resource_name = 'reviewtypename' + filtering = { + "slug": ALL, + "name": ALL, + "desc": ALL, + "used": ALL, + "order": ALL, + } +api.name.register(ReviewTypeNameResource()) + +class ReviewResultNameResource(ModelResource): + class Meta: + cache = SimpleCache() + queryset = ReviewResultName.objects.all() + #resource_name = 'reviewresultname' + filtering = { + "slug": ALL, + "name": ALL, + "desc": ALL, + "used": ALL, + "order": ALL, + "teams": ALL_WITH_RELATIONS, + } +api.name.register(ReviewResultNameResource()) + diff --git a/ietf/review/__init__.py b/ietf/review/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ietf/review/migrations/0001_initial.py b/ietf/review/migrations/0001_initial.py new file mode 100644 index 000000000..523b73c5e --- /dev/null +++ b/ietf/review/migrations/0001_initial.py @@ -0,0 +1,50 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import models, migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('group', '0008_auto_20160505_0523'), + ('name', '0012_insert_review_name_data'), + ('doc', '0012_auto_20160207_0537'), + ] + + operations = [ + migrations.CreateModel( + name='Reviewer', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('frequency', models.IntegerField(help_text=b'Can review every N days')), + ('available', 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')), + ('role', models.ForeignKey(to='group.Role')), + ], + options={ + }, + bases=(models.Model,), + ), + migrations.CreateModel( + name='ReviewRequest', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=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)), + ('reviewed_rev', models.CharField(max_length=16, verbose_name=b'reviewed revision', blank=True)), + ('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='review.Reviewer', null=True)), + ('state', models.ForeignKey(to='name.ReviewRequestStateName')), + ('team', models.ForeignKey(to='group.Group')), + ('type', models.ForeignKey(to='name.ReviewTypeName')), + ], + options={ + }, + bases=(models.Model,), + ), + ] diff --git a/ietf/review/migrations/__init__.py b/ietf/review/migrations/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/ietf/review/models.py b/ietf/review/models.py new file mode 100644 index 000000000..c4a1f5475 --- /dev/null +++ b/ietf/review/models.py @@ -0,0 +1,40 @@ +from django.db import models + +from ietf.doc.models import Document +from ietf.group.models import Group, Role +from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName + +class Reviewer(models.Model): + """ + These records associate reviewers with review teams and keep track + of admin data associated with the reviewer in the particular team. + There will be one record for each combination of reviewer and team. + """ + role = models.ForeignKey(Role) + frequency = models.IntegerField(help_text="Can review every N days") + available = 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") + +class ReviewRequest(models.Model): + """ + There should be one ReviewRequest entered for each combination of + document, rev, and reviewer. + """ + # Fields filled in on the initial record creation: + time = models.DateTimeField(auto_now_add=True) + type = models.ForeignKey(ReviewTypeName) + doc = models.ForeignKey(Document, related_name='review_request_set') + team = models.ForeignKey(Group) + 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") + state = models.ForeignKey(ReviewRequestStateName) + # Fields filled in as reviewer is assigned, and as the review + # is uploaded + reviewer = models.ForeignKey(Reviewer, 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) + result = models.ForeignKey(ReviewResultName, blank=True, null=True) + + def __unicode__(self): + return u"%s review on %s by %s %s" % (self.type, self.doc, self.team, self.state) diff --git a/ietf/review/utils.py b/ietf/review/utils.py new file mode 100644 index 000000000..0a1bd86bb --- /dev/null +++ b/ietf/review/utils.py @@ -0,0 +1,6 @@ +from ietf.group.models import Group + +def active_review_teams(): + # if there's a ReviewResultName defined, it's a review team + return Group.objects.filter(state="active").exclude(reviewresultname=None) + diff --git a/ietf/settings.py b/ietf/settings.py index ae716769e..ec91d6fb0 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -273,6 +273,7 @@ INSTALLED_APPS = ( 'ietf.person', 'ietf.redirects', 'ietf.release', + 'ietf.review', 'ietf.submit', 'ietf.sync', 'ietf.utils', diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 4c66aa8b3..20f4a2761 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -192,6 +192,28 @@ + {% if review_requests or can_request_review %} + + + Reviews + + + {% for r in review_requests %} + + {% endfor %} + + {% if can_request_review %} + + {% endif %} + + + {% endif %} + + {% if conflict_reviews %} diff --git a/ietf/templates/doc/review/request_review.html b/ietf/templates/doc/review/request_review.html new file mode 100644 index 000000000..d05b27733 --- /dev/null +++ b/ietf/templates/doc/review/request_review.html @@ -0,0 +1,34 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block pagehead %} + +{% endblock %} + +{% block title %}Request review of {{ doc.name }} {% endblock %} + +{% block content %} + {% origin %} +

Request review
{{ doc.name }}

+ +

Submit a request to have the document reviewed.

+ +
+ {% csrf_token %} + {% bootstrap_field form.type layout="horizontal" %} + {% bootstrap_field form.team layout="horizontal" %} + {% bootstrap_field form.deadline_date layout="horizontal" %} + {% bootstrap_field form.deadline_time layout="horizontal" %} + {% bootstrap_field form.requested_rev layout="horizontal" %} + + {% buttons %} + + Back + {% endbuttons %} +
+{% endblock %} + +{% block js %} + +{% endblock %} From 44e135345c9ff741d4b49a1b4fa425ef4746f0a3 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Fri, 20 May 2016 14:14:31 +0000 Subject: [PATCH 03/16] Add a page for displaying a review request, add support for withdrawing requests, add tests for these two pages - Legacy-Id: 11217 --- ietf/doc/models.py | 1 + ietf/doc/tests_review.py | 69 +++++++++--- ietf/doc/urls.py | 4 +- ietf/doc/urls_review.py | 9 ++ ietf/doc/views_review.py | 44 +++++++- ietf/name/models.py | 4 +- ietf/review/migrations/0001_initial.py | 2 +- ietf/review/models.py | 2 +- ietf/templates/doc/document_draft.html | 2 +- ietf/templates/doc/review/request_review.html | 2 +- ietf/templates/doc/review/review_request.html | 103 ++++++++++++++++++ .../doc/review/withdraw_request.html | 22 ++++ ietf/utils/test_utils.py | 13 ++- 13 files changed, 247 insertions(+), 30 deletions(-) create mode 100644 ietf/doc/urls_review.py create mode 100644 ietf/templates/doc/review/review_request.html create mode 100644 ietf/templates/doc/review/withdraw_request.html diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 157255dfd..26d610c03 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -707,6 +707,7 @@ EVENT_TYPES = [ # review ("requested_review", "Requested review"), + ("withdrew_review_request", "Withdrew review"), ] class DocEvent(models.Model): diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index fa0893587..01c27711f 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -1,33 +1,44 @@ # -*- coding: utf-8 -*- import datetime -from pyquery import PyQuery from django.core.urlresolvers import reverse as urlreverse import debug # pyflakes:ignore -from ietf.review.models import ReviewRequest +from ietf.review.models import ReviewRequest, Reviewer from ietf.person.models import Person from ietf.group.models import Group, Role -from ietf.name.models import ReviewResultName +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_utils import login_testing_unauthorized +from ietf.utils.test_utils import login_testing_unauthorized, unicontent, reload_db_objects -def make_review_data(): - review_team = Group.objects.create(state_id="active", acronym="reviewteam", name="Review Team", type_id="team") - review_team.reviewresultname_set.add(ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"])) +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.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=review_team) + role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team) + reviewer = Reviewer.objects.create(role=role, frequency=14, skip_next=0) - return review_team + review_req = ReviewRequest.objects.create( + doc=doc, + team=team, + type_id="early", + deadline=datetime.datetime.now() + datetime.timedelta(days=20), + state_id="ready", + reviewer=reviewer, + reviewed_rev="01", + ) + + return review_req class ReviewTests(TestCase): def test_request_review(self): doc = make_test_data() - review_team = make_review_data() + review_req = make_review_data(doc) + review_team = review_req.team url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name }) login_testing_unauthorized(self, "secretary", url) @@ -47,17 +58,17 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - req = ReviewRequest.objects.get(doc=doc) + req = ReviewRequest.objects.get(doc=doc, state="requested") self.assertEqual(req.deadline.date(), deadline_date) self.assertEqual(req.deadline.time(), datetime.time(23, 59, 59)) - self.assertEqual(req.state_id, "requested") self.assertEqual(req.team, review_team) self.assertEqual(req.requested_rev, "01") self.assertEqual(doc.latest_event().type, "requested_review") def test_request_review_by_reviewer(self): doc = make_test_data() - review_team = make_review_data() + review_req = make_review_data(doc) + review_team = review_req.team url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name }) login_testing_unauthorized(self, "plain", url) @@ -73,10 +84,40 @@ class ReviewTests(TestCase): }) self.assertEqual(r.status_code, 302) - req = ReviewRequest.objects.get(doc=doc) + req = ReviewRequest.objects.get(doc=doc, state="requested") self.assertEqual(req.state_id, "requested") self.assertEqual(req.team, review_team) def test_doc_page(self): + # FIXME: fill in pass + def test_review_request(self): + doc = make_test_data() + review_req = make_review_data(doc) + + url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + self.assertTrue(review_req.team.acronym.upper() in unicontent(r)) + + def test_withdraw_request(self): + doc = make_test_data() + review_req = make_review_data(doc) + review_req.state = ReviewRequestStateName.objects.get(slug="accepted") + review_req.save() + + url = urlreverse('ietf.doc.views_review.withdraw_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) + login_testing_unauthorized(self, "secretary", url) + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + # withdraw + r = self.client.post(url, { "action": "withdraw" }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "withdrawn") + self.assertEqual(doc.latest_event().type, "withdrew_review_request") diff --git a/ietf/doc/urls.py b/ietf/doc/urls.py index 188ccbb6d..d75459813 100644 --- a/ietf/doc/urls.py +++ b/ietf/doc/urls.py @@ -36,7 +36,6 @@ from django.views.generic import RedirectView from ietf.doc import views_search, views_draft, views_ballot from ietf.doc import views_status_change from ietf.doc import views_doc -from ietf.doc import views_review session_patterns = [ url(r'^add$', views_doc.add_sessionpresentation), @@ -74,8 +73,7 @@ urlpatterns = patterns('', url(r'^(?P[A-Za-z0-9._+-]+)/ballot/$', views_doc.document_ballot, name="doc_ballot"), (r'^(?P[A-Za-z0-9._+-]+)/(?:(?P[0-9-]+)/)?doc.json$', views_doc.document_json), (r'^(?P[A-Za-z0-9._+-]+)/ballotpopup/(?P[0-9]+)/$', views_doc.ballot_popup), - url(r'^(?P[A-Za-z0-9._+-]+)/requestreview/$', views_review.request_review), - url(r'^(?P[A-Za-z0-9._+-]+)/review/(?P[0-9]+)/$', views_review.review), + url(r'^(?P[A-Za-z0-9._+-]+)/reviewrequest/', include("ietf.doc.urls_review")), url(r'^(?P[A-Za-z0-9._+-]+)/email-aliases/$', RedirectView.as_view(pattern_name='doc_email', permanent=False),name='doc_specific_email_aliases'), diff --git a/ietf/doc/urls_review.py b/ietf/doc/urls_review.py new file mode 100644 index 000000000..afc2f6536 --- /dev/null +++ b/ietf/doc/urls_review.py @@ -0,0 +1,9 @@ +from django.conf.urls import patterns, url +from ietf.doc import views_review + +urlpatterns = patterns('', + url(r'^$', views_review.request_review), + url(r'^(?P[0-9]+)/$', views_review.review_request), + url(r'^(?P[0-9]+)/withdraw/$', views_review.withdraw_request), +) + diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 5578dcd97..d1f7861bb 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -2,7 +2,6 @@ import datetime from django.http import HttpResponseForbidden from django.shortcuts import render, get_object_or_404, redirect -from django.core.urlresolvers import reverse as urlreverse from django import forms from django.contrib.auth.decorators import login_required @@ -13,7 +12,6 @@ from ietf.review.models import ReviewRequest, ReviewRequestStateName from ietf.review.utils import active_review_teams from ietf.utils.fields import DatepickerDateField - class RequestReviewForm(forms.ModelForm): deadline_date = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={ "autoclose": "1", "start-date": "+0d" }) deadline_time = forms.TimeField(widget=forms.TextInput(attrs={ 'placeholder': "HH:MM" }), help_text="If time is not specified, end of day is assumed", required=False) @@ -87,7 +85,8 @@ def request_review(request, name): type="requested_review", doc=doc, by=request.user.person, - desc="{} review by {} requested".format(review_req.type.name, review_req.team.acronym.upper()), + desc="Requested {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()), + time=review_req.time, ) # FIXME: if I'm a reviewer, auto-assign to myself? @@ -101,8 +100,41 @@ def request_review(request, name): 'form': form, }) -def review(request, name, request_id): +def review_request(request, name, request_id): doc = get_object_or_404(Document, name=name) - review_request = get_object_or_404(ReviewRequest, pk=request_id) + review_req = get_object_or_404(ReviewRequest, pk=request_id) - print doc, review_request + return render(request, 'doc/review/review_request.html', { + 'doc': doc, + 'review_req': review_req, + 'can_withdraw_request': review_req.state_id in ["requested", "accepted"] and is_authorized_in_doc_stream(request.user, doc), + }) + +def withdraw_request(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"]) + + if not is_authorized_in_doc_stream(request.user, doc): + return HttpResponseForbidden("You do not have permission to perform this action") + + if request.method == "POST" and request.POST.get("action") == "withdraw": + review_req.state = ReviewRequestStateName.objects.get(slug="withdrawn") + review_req.save() + + DocEvent.objects.create( + type="withdrew_review_request", + doc=doc, + by=request.user.person, + desc="Withdrew request for {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()), + ) + + if review_req.state_id != "requested": + # FIXME: handle this case - by emailing? + pass + + return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + + return render(request, 'doc/review/withdraw_request.html', { + 'doc': doc, + 'review_req': review_req, + }) diff --git a/ietf/name/models.py b/ietf/name/models.py index dad83c97f..733f957b3 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -89,12 +89,12 @@ class LiaisonStatementTagName(NameModel): "Action Required, Action Taken" class ReviewRequestStateName(NameModel): """Requested, Accepted, Rejected, Withdrawn, Overtaken By Events, - No Response , Completed""" + No Response, Completed""" class ReviewTypeName(NameModel): """Early Review, Last Call, Telechat""" 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) + 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/review/migrations/0001_initial.py b/ietf/review/migrations/0001_initial.py index 523b73c5e..2c19afa60 100644 --- a/ietf/review/migrations/0001_initial.py +++ b/ietf/review/migrations/0001_initial.py @@ -21,7 +21,7 @@ class Migration(migrations.Migration): ('available', 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')), - ('role', models.ForeignKey(to='group.Role')), + ('role', models.OneToOneField(to='group.Role')), ], options={ }, diff --git a/ietf/review/models.py b/ietf/review/models.py index c4a1f5475..7e166333c 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -10,7 +10,7 @@ class Reviewer(models.Model): of admin data associated with the reviewer in the particular team. There will be one record for each combination of reviewer and team. """ - role = models.ForeignKey(Role) + role = models.OneToOneField(Role) frequency = models.IntegerField(help_text="Can review every N days") available = models.DateTimeField(blank=True, null=True, help_text="When will this reviewer be available again") filter_re = models.CharField(max_length=255, blank=True) diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 20f4a2761..853d6fcc6 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -200,7 +200,7 @@ {% for r in review_requests %} {% endfor %} diff --git a/ietf/templates/doc/review/request_review.html b/ietf/templates/doc/review/request_review.html index d05b27733..aa1700aab 100644 --- a/ietf/templates/doc/review/request_review.html +++ b/ietf/templates/doc/review/request_review.html @@ -23,7 +23,7 @@ {% bootstrap_field form.requested_rev layout="horizontal" %} {% buttons %} - + Back {% endbuttons %} diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html new file mode 100644 index 000000000..f1087cd84 --- /dev/null +++ b/ietf/templates/doc/review/review_request.html @@ -0,0 +1,103 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Review request for {{ review_req.doc.name }}{% endblock %} + +{% block content %} + {% origin %} +

Review request
{{ review_req.doc.name }}

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + {% if review_req.reviewer %} + + + + + + {% endif %} + + {% if review_req.review %} + + + + + + {% endif %} + + {% if review_req.reviewed_rev %} + + + + + + {% endif %} + + {% if review_req.result %} + + + + + + {% endif %} + +
RequestReview of + {% if review_req.requested_rev %} + {{ review_req.doc.name }}-{{ review_req.requested_rev }} + {% else %} + {{ review_req.doc.name }} + {% endif %} +
Type{{ review_req.type.name }} Review
Team{{ review_req.team.acronym|upper }}
Deadline + {% if review_req.deadline|date:"H:i" != "23:59" %} + {{ review_req.deadline|date:"Y-m-d H:i" }} + {% else %} + {{ review_req.deadline|date:"Y-m-d" }} + {% endif %} +
Requested{{ review_req.time|date:"Y-m-d" }}
ReviewState{{ review_req.state.name }}
Reviewer{{ review_req.reviewer.role.person }}
Review{{ review_req.review.name }}
Reviewed revision{{ review_req.reviewed_rev }}
Result of review{{ review_req.result.name }
+ +
+ {% if can_withdraw_request %} + Withdraw request + {% endif %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/review/withdraw_request.html b/ietf/templates/doc/review/withdraw_request.html new file mode 100644 index 000000000..191f236dc --- /dev/null +++ b/ietf/templates/doc/review/withdraw_request.html @@ -0,0 +1,22 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Withdraw review request for {{ review_req.doc.name }}{% endblock %} + +{% block content %} + {% origin %} +

Withdraw review request
{{ review_req.doc.name }}

+ +

Do you want to withdraw the review request?

+ +
+ {% csrf_token %} + + {% buttons %} + Cancel + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/utils/test_utils.py b/ietf/utils/test_utils.py index bc4ed9b77..b3253aa14 100644 --- a/ietf/utils/test_utils.py +++ b/ietf/utils/test_utils.py @@ -255,7 +255,7 @@ def canonicalize_sitemap(s): def login_testing_unauthorized(test_case, username, url, password=None): r = test_case.client.get(url) - test_case.assertTrue(r.status_code in (302, 403)) + test_case.assertIn(r.status_code, (302, 403)) if r.status_code == 302: test_case.assertTrue("/accounts/login" in r['Location']) if not password: @@ -272,6 +272,17 @@ def unicontent(r): encoding = 'utf-8' return r.content.decode(encoding) +def reload_db_objects(*objects): + """Rerequest the given arguments from the database so they're refreshed, to be used like + + foo, bar = reload_objects(foo, bar)""" + + t = tuple(o.__class__.objects.get(pk=o.pk) for o in objects) + if len(objects) == 1: + return t[0] + else: + return t + class ReverseLazyTest(django.test.TestCase): def test_redirect_with_lazy_reverse(self): response = self.client.get('/ipr/update/') From b5ef179a6e66b34cac7533b67dfd94b8ee834f79 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Mon, 23 May 2016 13:57:01 +0000 Subject: [PATCH 04/16] Add support for rejecting a review assignment - Legacy-Id: 11225 --- ietf/doc/models.py | 2 +- ietf/doc/tests_review.py | 57 +++++++++++++-- ietf/doc/urls_review.py | 1 + ietf/doc/utils.py | 6 ++ ietf/doc/views_doc.py | 2 +- ietf/doc/views_review.py | 69 +++++++++++++++++-- .../0012_insert_review_name_data.py | 4 +- ietf/name/models.py | 2 +- .../doc/review/reject_request_assignment.html | 22 ++++++ ietf/templates/doc/review/review_request.html | 8 ++- 10 files changed, 159 insertions(+), 14 deletions(-) create mode 100644 ietf/templates/doc/review/reject_request_assignment.html diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 26d610c03..ea969cfe2 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -707,7 +707,7 @@ EVENT_TYPES = [ # review ("requested_review", "Requested review"), - ("withdrew_review_request", "Withdrew review"), + ("changed_review_request", "Changed review request"), ] class DocEvent(models.Model): diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index 01c27711f..99e307b90 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -108,16 +108,63 @@ class ReviewTests(TestCase): review_req.state = ReviewRequestStateName.objects.get(slug="accepted") review_req.save() - url = urlreverse('ietf.doc.views_review.withdraw_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) - login_testing_unauthorized(self, "secretary", url) + withdraw_url = urlreverse('ietf.doc.views_review.withdraw_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) - r = self.client.get(url) + + # follow link + req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) + self.client.login(username="secretary", password="secretary+password") + r = self.client.get(req_url) + self.assertEqual(r.status_code, 200) + self.assertTrue(withdraw_url in unicontent(r)) + self.client.logout() + + # get withdraw page + login_testing_unauthorized(self, "secretary", withdraw_url) + r = self.client.get(withdraw_url) self.assertEqual(r.status_code, 200) # withdraw - r = self.client.post(url, { "action": "withdraw" }) + r = self.client.post(withdraw_url, { "action": "withdraw" }) self.assertEqual(r.status_code, 302) review_req = reload_db_objects(review_req) self.assertEqual(review_req.state_id, "withdrawn") - self.assertEqual(doc.latest_event().type, "withdrew_review_request") + e = doc.latest_event() + self.assertEqual(e.type, "changed_review_request") + self.assertTrue("Withdrew" in e.desc) + + def test_reject_request_assignment(self): + doc = make_test_data() + review_req = make_review_data(doc) + review_req.state = ReviewRequestStateName.objects.get(slug="accepted") + review_req.save() + + reject_url = urlreverse('ietf.doc.views_review.reject_request_assignment', kwargs={ "name": doc.name, "request_id": review_req.pk }) + + + # follow link + req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) + self.client.login(username="secretary", password="secretary+password") + r = self.client.get(req_url) + self.assertEqual(r.status_code, 200) + self.assertTrue(reject_url in unicontent(r)) + self.client.logout() + + # get reject page + login_testing_unauthorized(self, "secretary", reject_url) + r = self.client.get(reject_url) + self.assertEqual(r.status_code, 200) + self.assertTrue(unicode(review_req.reviewer.role.person) in unicontent(r)) + + # reject + r = self.client.post(reject_url, { "action": "reject" }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "rejected") + e = doc.latest_event() + self.assertEqual(e.type, "changed_review_request") + self.assertTrue("unassigned" in e.desc) + self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="accepted").count(), 1) + diff --git a/ietf/doc/urls_review.py b/ietf/doc/urls_review.py index afc2f6536..8ed0b5094 100644 --- a/ietf/doc/urls_review.py +++ b/ietf/doc/urls_review.py @@ -5,5 +5,6 @@ urlpatterns = patterns('', url(r'^$', views_review.request_review), url(r'^(?P[0-9]+)/$', views_review.review_request), url(r'^(?P[0-9]+)/withdraw/$', views_review.withdraw_request), + url(r'^(?P[0-9]+)/rejectassignment/$', views_review.reject_request_assignment), ) diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 923715895..3aaa1604b 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -99,6 +99,12 @@ def can_request_review_of_doc(user, doc): return is_authorized_in_doc_stream(user, doc) +def can_manage_review_requests_for_team(user, team): + if not user.is_authenticated(): + return False + + return Role.objects.filter(name="secretary", person__user=user, group=team).exists() or has_role(user, "Secretariat") + def two_thirds_rule( recused=0 ): # For standards-track, need positions from 2/3 of the non-recused current IESG. active = Role.objects.filter(name="ad",group__type="area",group__state="active").count() diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index d25d88fc3..0e7bc7838 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) + review_requests = ReviewRequest.objects.filter(doc=doc).exclude(state__in=["withdrawn", "rejected"]) 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 d1f7861bb..5c8face33 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -6,8 +6,8 @@ from django import forms from django.contrib.auth.decorators import login_required from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent -from ietf.doc.utils import can_request_review_of_doc -from ietf.ietfauth.utils import is_authorized_in_doc_stream +from ietf.doc.utils import can_request_review_of_doc, can_manage_review_requests_for_team +from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person from ietf.review.models import ReviewRequest, ReviewRequestStateName from ietf.review.utils import active_review_teams from ietf.utils.fields import DatepickerDateField @@ -104,10 +104,21 @@ def review_request(request, name, request_id): doc = get_object_or_404(Document, name=name) review_req = get_object_or_404(ReviewRequest, pk=request_id) + is_reviewer = review_req.reviewer and user_is_person(request.user, review_req.reviewer.role.person) + can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team) + + can_withdraw_request = (review_req.state_id in ["requested", "accepted"] + and is_authorized_in_doc_stream(request.user, doc)) + + can_reject_request_assignment = (review_req.state_id in ["requested", "accepted"] + and review_req.reviewer_id is not None + and (is_reviewer or can_manage_req)) + return render(request, 'doc/review/review_request.html', { 'doc': doc, 'review_req': review_req, - 'can_withdraw_request': review_req.state_id in ["requested", "accepted"] and is_authorized_in_doc_stream(request.user, doc), + 'can_withdraw_request': can_withdraw_request, + 'can_reject_request_assignment': can_reject_request_assignment, }) def withdraw_request(request, name, request_id): @@ -122,7 +133,7 @@ def withdraw_request(request, name, request_id): review_req.save() DocEvent.objects.create( - type="withdrew_review_request", + type="changed_review_request", doc=doc, by=request.user.person, desc="Withdrew request for {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()), @@ -138,3 +149,53 @@ def withdraw_request(request, name, request_id): 'doc': doc, 'review_req': review_req, }) + +class RejectRequestAssignmentForm(forms.Form): + message_to_secretary = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional explanation of rejection, will be emailed to team secretary") + +def reject_request_assignment(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"]) + + if not review_req.reviewer: + return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + + is_reviewer = user_is_person(request.user, review_req.reviewer.role.person) + can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team) + + if not (is_reviewer or can_manage_req): + return HttpResponseForbidden("You do not have permission to perform this action") + + if request.method == "POST" and request.POST.get("action") == "reject": + # reject the old request + prev_state = review_req.state + review_req.state = ReviewRequestStateName.objects.get(slug="rejected") + review_req.save() + + # assignment of reviewer is currently considered minutia, so + # not reported in the log + if prev_state.slug == "accepted": + DocEvent.objects.create( + type="changed_review_request", + doc=doc, + by=request.user.person, + desc="Request for {} review by {} is unassigned".format(review_req.type.name, review_req.team.acronym.upper()), + ) + + # make a new, open review request + ReviewRequest.objects.create( + time=review_req.time, + type=review_req.type, + doc=review_req.doc, + team=review_req.team, + deadline=review_req.deadline, + requested_rev=review_req.requested_rev, + state=prev_state, + ) + + return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + + return render(request, 'doc/review/reject_request_assignment.html', { + 'doc': doc, + 'review_req': review_req, + }) diff --git a/ietf/name/migrations/0012_insert_review_name_data.py b/ietf/name/migrations/0012_insert_review_name_data.py index ed1a4bfe6..1ebbba69f 100644 --- a/ietf/name/migrations/0012_insert_review_name_data.py +++ b/ietf/name/migrations/0012_insert_review_name_data.py @@ -12,7 +12,9 @@ def insert_initial_review_data(apps, schema_editor): 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="completed", name="Completed", order=7) + ReviewRequestStateName.objects.get_or_create(slug="part-completed", name="Partially Completed", order=6) + 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) diff --git a/ietf/name/models.py b/ietf/name/models.py index 733f957b3..b16da81f7 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -89,7 +89,7 @@ class LiaisonStatementTagName(NameModel): "Action Required, Action Taken" class ReviewRequestStateName(NameModel): """Requested, Accepted, Rejected, Withdrawn, Overtaken By Events, - No Response, Completed""" + No Response, Partially Completed, Completed""" class ReviewTypeName(NameModel): """Early Review, Last Call, Telechat""" class ReviewResultName(NameModel): diff --git a/ietf/templates/doc/review/reject_request_assignment.html b/ietf/templates/doc/review/reject_request_assignment.html new file mode 100644 index 000000000..9ccd6c56b --- /dev/null +++ b/ietf/templates/doc/review/reject_request_assignment.html @@ -0,0 +1,22 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Reject review assignment for {{ review_req.doc.name }}{% endblock %} + +{% block content %} + {% origin %} +

Reject review assignment
{{ review_req.doc.name }}

+ +

{{ review_req.reviewer.role.person }} is currently assigned to do the review. Do you want to reject this assignment?

+ +
+ {% csrf_token %} + + {% buttons %} + Cancel + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index f1087cd84..954c4adcf 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -64,7 +64,13 @@ Reviewer - {{ review_req.reviewer.role.person }} + + {{ review_req.reviewer.role.person }} + + {% if can_reject_request_assignment %} + Reject request assignment + {% endif %} + {% endif %} From 5dd079e2f8623c67edec65bd957861cefc2f70d0 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Mon, 23 May 2016 16:42:49 +0000 Subject: [PATCH 05/16] Add support for assigning a reviewer to a review request, still some corners missing. Fix a couple of other issues. - Legacy-Id: 11227 --- ietf/doc/tests_review.py | 89 ++++++++++----- ietf/doc/urls_review.py | 3 +- ietf/doc/utils.py | 16 --- ietf/doc/views_doc.py | 4 +- ietf/doc/views_review.py | 107 +++++++++++++----- ietf/review/migrations/0001_initial.py | 10 +- ietf/review/models.py | 38 ++++--- ietf/review/utils.py | 44 ++++++- .../templates/doc/review/assign_reviewer.html | 22 ++++ ...t.html => reject_reviewer_assignment.html} | 2 +- ietf/templates/doc/review/review_request.html | 18 ++- 11 files changed, 246 insertions(+), 107 deletions(-) create mode 100644 ietf/templates/doc/review/assign_reviewer.html rename ietf/templates/doc/review/{reject_request_assignment.html => reject_reviewer_assignment.html} (84%) diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index 99e307b90..b6058ad0d 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -13,6 +13,8 @@ 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_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") @@ -20,7 +22,7 @@ def make_review_data(doc): p = Person.objects.get(user__username="plain") role = Role.objects.create(name_id="reviewer", person=p, email=p.email_set.first(), group=team) - reviewer = Reviewer.objects.create(role=role, frequency=14, skip_next=0) + Reviewer.objects.create(team=team, person=p, frequency=14, skip_next=0) review_req = ReviewRequest.objects.create( doc=doc, @@ -28,10 +30,13 @@ def make_review_data(doc): type_id="early", deadline=datetime.datetime.now() + datetime.timedelta(days=20), state_id="ready", - reviewer=reviewer, + 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) + return review_req class ReviewTests(TestCase): @@ -65,29 +70,6 @@ class ReviewTests(TestCase): self.assertEqual(req.requested_rev, "01") self.assertEqual(doc.latest_event().type, "requested_review") - def test_request_review_by_reviewer(self): - doc = make_test_data() - review_req = make_review_data(doc) - review_team = review_req.team - - url = urlreverse('ietf.doc.views_review.request_review', kwargs={ "name": doc.name }) - login_testing_unauthorized(self, "plain", url) - - # post request - deadline_date = datetime.date.today() + datetime.timedelta(days=10) - - r = self.client.post(url, { - "type": "early", - "team": review_team.pk, - "deadline_date": deadline_date.isoformat(), - "requested_rev": "01" - }) - self.assertEqual(r.status_code, 302) - - req = ReviewRequest.objects.get(doc=doc, state="requested") - self.assertEqual(req.state_id, "requested") - self.assertEqual(req.team, review_team) - def test_doc_page(self): # FIXME: fill in pass @@ -134,13 +116,56 @@ class ReviewTests(TestCase): self.assertEqual(e.type, "changed_review_request") self.assertTrue("Withdrew" in e.desc) - def test_reject_request_assignment(self): + def test_assign_reviewer(self): + doc = make_test_data() + review_req = make_review_data(doc) + review_req.state = ReviewRequestStateName.objects.get(slug="requested") + review_req.reviewer = None + review_req.save() + + assign_url = urlreverse('ietf.doc.views_review.assign_reviewer', kwargs={ "name": doc.name, "request_id": review_req.pk }) + + + # follow link + req_url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) + self.client.login(username="secretary", password="secretary+password") + r = self.client.get(req_url) + self.assertEqual(r.status_code, 200) + self.assertTrue(assign_url in unicontent(r)) + self.client.logout() + + # get assign page + login_testing_unauthorized(self, "secretary", assign_url) + r = self.client.get(assign_url) + self.assertEqual(r.status_code, 200) + + # assign + reviewer = Role.objects.filter(name="reviewer", group=review_req.team).first() + r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "requested") + self.assertEqual(review_req.reviewer, reviewer) + + # re-assign + 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() + r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "requested") + self.assertEqual(review_req.reviewer, reviewer) + + def test_reject_reviewer_assignment(self): doc = make_test_data() review_req = make_review_data(doc) review_req.state = ReviewRequestStateName.objects.get(slug="accepted") review_req.save() - reject_url = urlreverse('ietf.doc.views_review.reject_request_assignment', kwargs={ "name": doc.name, "request_id": review_req.pk }) + reject_url = urlreverse('ietf.doc.views_review.reject_reviewer_assignment', kwargs={ "name": doc.name, "request_id": review_req.pk }) # follow link @@ -155,16 +180,18 @@ class ReviewTests(TestCase): login_testing_unauthorized(self, "secretary", reject_url) r = self.client.get(reject_url) self.assertEqual(r.status_code, 200) - self.assertTrue(unicode(review_req.reviewer.role.person) in unicontent(r)) + self.assertTrue(unicode(review_req.reviewer.person) in unicontent(r)) # reject - r = self.client.post(reject_url, { "action": "reject" }) + empty_outbox() + r = self.client.post(reject_url, { "action": "reject", "message_to_secretary": "Test message" }) self.assertEqual(r.status_code, 302) review_req = reload_db_objects(review_req) self.assertEqual(review_req.state_id, "rejected") e = doc.latest_event() self.assertEqual(e.type, "changed_review_request") - self.assertTrue("unassigned" in e.desc) - self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="accepted").count(), 1) + self.assertTrue("rejected" in e.desc) + self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="requested").count(), 1) + self.assertEqual(len(outbox), 1) diff --git a/ietf/doc/urls_review.py b/ietf/doc/urls_review.py index 8ed0b5094..527833c0a 100644 --- a/ietf/doc/urls_review.py +++ b/ietf/doc/urls_review.py @@ -5,6 +5,7 @@ urlpatterns = patterns('', url(r'^$', views_review.request_review), url(r'^(?P[0-9]+)/$', views_review.review_request), url(r'^(?P[0-9]+)/withdraw/$', views_review.withdraw_request), - url(r'^(?P[0-9]+)/rejectassignment/$', views_review.reject_request_assignment), + url(r'^(?P[0-9]+)/assignreviewer/$', views_review.assign_reviewer), + url(r'^(?P[0-9]+)/rejectreviewerassignment/$', views_review.reject_reviewer_assignment), ) diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 3aaa1604b..500b283bf 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -89,22 +89,6 @@ def can_adopt_draft(user, doc): group__state="active", person__user=user).exists()) -def can_request_review_of_doc(user, doc): - if not user.is_authenticated(): - return False - - from ietf.review.utils import active_review_teams - if Role.objects.filter(name="reviewer", person__user=user, group__in=active_review_teams()): - return True - - return is_authorized_in_doc_stream(user, doc) - -def can_manage_review_requests_for_team(user, team): - if not user.is_authenticated(): - return False - - return Role.objects.filter(name="secretary", person__user=user, group=team).exists() or has_role(user, "Secretariat") - def two_thirds_rule( recused=0 ): # For standards-track, need positions from 2/3 of the non-recused current IESG. active = Role.objects.filter(name="ad",group__type="area",group__state="active").count() diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 0e7bc7838..23d430a72 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -48,8 +48,7 @@ from ietf.doc.models import ( Document, DocAlias, DocHistory, DocEvent, BallotDo from ietf.doc.utils import ( add_links_in_new_revision_events, augment_events_with_revision, can_adopt_draft, get_chartering_type, get_document_content, get_tags_for_stream_id, needed_ballot_positions, nice_consensus, prettify_std_name, update_telechat, has_same_ballot, - get_initial_notify, make_notify_changed_event, crawl_history, default_consensus, - can_request_review_of_doc ) + get_initial_notify, make_notify_changed_event, crawl_history, default_consensus ) from ietf.community.utils import augment_docs_with_tracking_info from ietf.group.models import Role from ietf.group.utils import can_manage_group, can_manage_materials @@ -63,6 +62,7 @@ from ietf.mailtrigger.utils import gather_relevant_expansions from ietf.meeting.models import Session from ietf.meeting.utils import group_sessions, get_upcoming_manageable_sessions, sort_sessions from ietf.review.models import ReviewRequest +from ietf.review.utils import can_request_review_of_doc def render_document_top(request, doc, tab, name): tabs = [] diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 5c8face33..a0722a276 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -6,10 +6,12 @@ from django import forms from django.contrib.auth.decorators import login_required from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent -from ietf.doc.utils import can_request_review_of_doc, can_manage_review_requests_for_team from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person -from ietf.review.models import ReviewRequest, ReviewRequestStateName -from ietf.review.utils import active_review_teams +from ietf.name.models import ReviewRequestStateName +from ietf.group.models import Role +from ietf.review.models import ReviewRequest +from ietf.review.utils import active_review_teams, assign_review_request_to_reviewer +from ietf.review.utils import can_request_review_of_doc, can_manage_review_requests_for_team from ietf.utils.fields import DatepickerDateField class RequestReviewForm(forms.ModelForm): @@ -89,7 +91,6 @@ def request_review(request, name): time=review_req.time, ) - # FIXME: if I'm a reviewer, auto-assign to myself? return redirect('doc_view', name=doc.name) else: @@ -104,21 +105,25 @@ def review_request(request, name, request_id): doc = get_object_or_404(Document, name=name) review_req = get_object_or_404(ReviewRequest, pk=request_id) - is_reviewer = review_req.reviewer and user_is_person(request.user, review_req.reviewer.role.person) + is_reviewer = review_req.reviewer and user_is_person(request.user, review_req.reviewer.person) can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team) can_withdraw_request = (review_req.state_id in ["requested", "accepted"] and is_authorized_in_doc_stream(request.user, doc)) - can_reject_request_assignment = (review_req.state_id in ["requested", "accepted"] - and review_req.reviewer_id is not None - and (is_reviewer or can_manage_req)) + can_assign_reviewer = (review_req.state_id in ["requested", "accepted"] + and is_authorized_in_doc_stream(request.user, doc)) + + can_reject_reviewer_assignment = (review_req.state_id in ["requested", "accepted"] + and review_req.reviewer_id is not None + and (is_reviewer or can_manage_req)) return render(request, 'doc/review/review_request.html', { 'doc': doc, 'review_req': review_req, 'can_withdraw_request': can_withdraw_request, - 'can_reject_request_assignment': can_reject_request_assignment, + 'can_reject_reviewer_assignment': can_reject_reviewer_assignment, + 'can_assign_reviewer': can_assign_reviewer, }) def withdraw_request(request, name, request_id): @@ -150,52 +155,96 @@ def withdraw_request(request, name, request_id): 'review_req': review_req, }) -class RejectRequestAssignmentForm(forms.Form): +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) + + 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) + if review_req.reviewer: + f.initial = review_req.reviewer_id + +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_req = can_manage_review_requests_for_team(request.user, review_req.team) + + if not can_manage_req: + return HttpResponseForbidden("You do not have permission to perform this action") + + if request.method == "POST" and request.POST.get("action") == "assign": + form = AssignReviewerForm(review_req, request.POST) + if form.is_valid(): + reviewer = form.cleaned_data["reviewer"] + assign_review_request_to_reviewer(review_req, reviewer, request.user.person) + + return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + else: + form = AssignReviewerForm(review_req) + + return render(request, 'doc/review/assign_reviewer.html', { + 'doc': doc, + 'review_req': review_req, + 'form': form, + }) + +class RejectReviewerAssignmentForm(forms.Form): message_to_secretary = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional explanation of rejection, will be emailed to team secretary") -def reject_request_assignment(request, name, request_id): +def reject_reviewer_assignment(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"]) if not review_req.reviewer: return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) - is_reviewer = user_is_person(request.user, review_req.reviewer.role.person) + is_reviewer = user_is_person(request.user, review_req.reviewer.person) can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team) if not (is_reviewer or can_manage_req): return HttpResponseForbidden("You do not have permission to perform this action") if request.method == "POST" and request.POST.get("action") == "reject": - # reject the old request - prev_state = review_req.state + # reject the request review_req.state = ReviewRequestStateName.objects.get(slug="rejected") review_req.save() - # assignment of reviewer is currently considered minutia, so - # not reported in the log - if prev_state.slug == "accepted": - DocEvent.objects.create( - type="changed_review_request", - doc=doc, - by=request.user.person, - desc="Request for {} review by {} is unassigned".format(review_req.type.name, review_req.team.acronym.upper()), - ) - - # make a new, open review request - ReviewRequest.objects.create( + DocEvent.objects.create( + type="changed_review_request", + doc=review_req.doc, + by=request.user.person, + desc="Assignment of request for {} review by {} to {} was rejected".format( + review_req.type.name, + review_req.team.acronym.upper(), + review_req.reviewer.person, + ), + ) + + # make a new unassigned review request + new_review_req = ReviewRequest.objects.create( time=review_req.time, type=review_req.type, doc=review_req.doc, team=review_req.team, deadline=review_req.deadline, requested_rev=review_req.requested_rev, - state=prev_state, + state=ReviewRequestStateName.objects.get(slug="requested"), ) - return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk) - return render(request, 'doc/review/reject_request_assignment.html', { + return render(request, 'doc/review/reject_reviewer_assignment.html', { 'doc': doc, 'review_req': review_req, }) diff --git a/ietf/review/migrations/0001_initial.py b/ietf/review/migrations/0001_initial.py index 2c19afa60..d9187804b 100644 --- a/ietf/review/migrations/0001_initial.py +++ b/ietf/review/migrations/0001_initial.py @@ -10,6 +10,7 @@ class Migration(migrations.Migration): ('group', '0008_auto_20160505_0523'), ('name', '0012_insert_review_name_data'), ('doc', '0012_auto_20160207_0537'), + ('person', '0006_auto_20160503_0937'), ] operations = [ @@ -17,11 +18,12 @@ class Migration(migrations.Migration): name='Reviewer', fields=[ ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), - ('frequency', models.IntegerField(help_text=b'Can review every N days')), - ('available', models.DateTimeField(help_text=b'When will this reviewer be available again', null=True, blank=True)), + ('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')), - ('role', models.OneToOneField(to='group.Role')), + ('person', models.ForeignKey(to='person.Person')), + ('team', models.ForeignKey(to='group.Group')), ], options={ }, @@ -38,7 +40,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='review.Reviewer', null=True)), + ('reviewer', models.ForeignKey(blank=True, to='group.Role', null=True)), ('state', models.ForeignKey(to='name.ReviewRequestStateName')), ('team', models.ForeignKey(to='group.Group')), ('type', models.ForeignKey(to='name.ReviewTypeName')), diff --git a/ietf/review/models.py b/ietf/review/models.py index 7e166333c..530e0c939 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -2,36 +2,42 @@ 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.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName class Reviewer(models.Model): - """ - These records associate reviewers with review teams and keep track - of admin data associated with the reviewer in the particular team. - There will be one record for each combination of reviewer and team. - """ - role = models.OneToOneField(Role) - frequency = models.IntegerField(help_text="Can review every N days") - available = models.DateTimeField(blank=True, null=True, help_text="When will this reviewer be available again") + """Keeps track of admin data associated with the reviewer in the + particular team. There will be one record for each combination of + reviewer and team.""" + team = models.ForeignKey(Group) + person = models.ForeignKey(Person) + frequency = models.IntegerField(help_text="Can review every N days", default=30) + 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") class ReviewRequest(models.Model): - """ + """Represents a request for a review and the process it goes through. There should be one ReviewRequest entered for each combination of - document, rev, and reviewer. - """ - # Fields filled in on the initial record creation: + document, rev, and reviewer.""" + state = models.ForeignKey(ReviewRequestStateName) + + # 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) 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") - state = models.ForeignKey(ReviewRequestStateName) - # Fields filled in as reviewer is assigned, and as the review - # is uploaded - reviewer = models.ForeignKey(Reviewer, blank=True, null=True) + + # 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) + review = models.OneToOneField(Document, blank=True, null=True) reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) result = models.ForeignKey(ReviewResultName, blank=True, null=True) diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 0a1bd86bb..43d495e15 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -1,6 +1,48 @@ -from ietf.group.models import Group +from ietf.group.models import Group, Role +from ietf.doc.models import DocEvent +from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream +from ietf.review.models import ReviewRequestStateName def active_review_teams(): # if there's a ReviewResultName defined, it's a review team return Group.objects.filter(state="active").exclude(reviewresultname=None) + +def can_request_review_of_doc(user, doc): + if not user.is_authenticated(): + return False + + return is_authorized_in_doc_stream(user, doc) + +def can_manage_review_requests_for_team(user, team): + if not user.is_authenticated(): + return False + + return Role.objects.filter(name="secretary", person__user=user, group=team).exists() or has_role(user, "Secretariat") + +def assign_review_request_to_reviewer(review_req, reviewer, by): + assert review_req.state_id in ("requested", "accepted") + + if review_req.reviewer == reviewer: + return + + prev_state = review_req.state + prev_reviewer = review_req.reviewer + + review_req.state = ReviewRequestStateName.objects.get(slug="requested") + review_req.reviewer = reviewer + review_req.save() + + DocEvent.objects.create( + type="changed_review_request", + doc=review_req.doc, + by=by, + desc="Request for {} review by {} is assigned to {}".format( + review_req.type.name, + review_req.team.acronym.upper(), + review_req.reviewer.person if review_req.reviewer else "(None)", + ), + ) + if prev_state.slug != "requested" and prev_reviewer: + # FIXME: email old reviewer? + pass diff --git a/ietf/templates/doc/review/assign_reviewer.html b/ietf/templates/doc/review/assign_reviewer.html new file mode 100644 index 000000000..0e7059b03 --- /dev/null +++ b/ietf/templates/doc/review/assign_reviewer.html @@ -0,0 +1,22 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Assign reviewer for {{ review_req.doc.name }}{% endblock %} + +{% block content %} + {% origin %} +

Assign reviewer
{{ review_req.doc.name }}

+ +
+ {% csrf_token %} + + {% bootstrap_form form %} + + {% buttons %} + Cancel + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/review/reject_request_assignment.html b/ietf/templates/doc/review/reject_reviewer_assignment.html similarity index 84% rename from ietf/templates/doc/review/reject_request_assignment.html rename to ietf/templates/doc/review/reject_reviewer_assignment.html index 9ccd6c56b..edc8d9696 100644 --- a/ietf/templates/doc/review/reject_request_assignment.html +++ b/ietf/templates/doc/review/reject_reviewer_assignment.html @@ -8,7 +8,7 @@ {% origin %}

Reject review assignment
{{ review_req.doc.name }}

-

{{ review_req.reviewer.role.person }} is currently assigned to do the review. Do you want to reject this assignment?

+

{{ review_req.reviewer.person }} is currently assigned to do the review. Do you want to reject this assignment?

{% csrf_token %} diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index 954c4adcf..844ff3c94 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -60,19 +60,25 @@ {{ review_req.state.name }} - {% if review_req.reviewer %} Reviewer - {{ review_req.reviewer.role.person }} + {% if review_req.reviewer %} + {{ review_req.reviewer.person }} + {% else %} + None assigned yet + {% endif %} - {% if can_reject_request_assignment %} - Reject request assignment + {% if can_assign_reviewer %} + {% if review_req.reviewer %}Reassign{% else %}Assign{% endif %} reviewer + {% endif %} + + {% if can_reject_reviewer_assignment %} + Reject reviewer assignment {% endif %} - {% endif %} {% if review_req.review %} @@ -94,7 +100,7 @@ Result of review - {{ review_req.result.name } + {{ review_req.result.name }} {% endif %} From b9f4b7005e2d715fbd6c4e82a006da22f71c0005 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 24 May 2016 14:02:59 +0000 Subject: [PATCH 06/16] Add simple email notifications for assigning/rejecting review requests - Legacy-Id: 11236 --- ietf/doc/tests_review.py | 15 +++- ietf/doc/views_review.py | 80 ++++++++++++------- ietf/review/models.py | 2 +- ietf/review/utils.py | 58 +++++++++++--- .../doc/mail/review_request_changed.txt | 7 ++ 5 files changed, 117 insertions(+), 45 deletions(-) create mode 100644 ietf/templates/doc/mail/review_request_changed.txt diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index b6058ad0d..c2fe46549 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -107,6 +107,7 @@ class ReviewTests(TestCase): self.assertEqual(r.status_code, 200) # withdraw + empty_outbox() r = self.client.post(withdraw_url, { "action": "withdraw" }) self.assertEqual(r.status_code, 302) @@ -115,6 +116,8 @@ class ReviewTests(TestCase): e = doc.latest_event() self.assertEqual(e.type, "changed_review_request") self.assertTrue("Withdrew" in e.desc) + self.assertEqual(len(outbox), 1) + self.assertTrue("withdrawn" in unicode(outbox[0])) def test_assign_reviewer(self): doc = make_test_data() @@ -140,6 +143,7 @@ class ReviewTests(TestCase): self.assertEqual(r.status_code, 200) # assign + empty_outbox() reviewer = Role.objects.filter(name="reviewer", group=review_req.team).first() r = self.client.post(assign_url, { "action": "assign", "reviewer": reviewer.pk }) self.assertEqual(r.status_code, 302) @@ -147,8 +151,11 @@ class ReviewTests(TestCase): review_req = reload_db_objects(review_req) self.assertEqual(review_req.state_id, "requested") self.assertEqual(review_req.reviewer, reviewer) + self.assertEqual(len(outbox), 1) + self.assertTrue("assigned" in unicode(outbox[0])) # re-assign + 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() @@ -156,8 +163,11 @@ class ReviewTests(TestCase): self.assertEqual(r.status_code, 302) review_req = reload_db_objects(review_req) - self.assertEqual(review_req.state_id, "requested") + self.assertEqual(review_req.state_id, "requested") # check that state is reset self.assertEqual(review_req.reviewer, reviewer) + self.assertEqual(len(outbox), 2) + self.assertTrue("cancelled your assignment" in unicode(outbox[0])) + self.assertTrue("assigned" in unicode(outbox[1])) def test_reject_reviewer_assignment(self): doc = make_test_data() @@ -194,4 +204,5 @@ class ReviewTests(TestCase): self.assertTrue("rejected" in e.desc) self.assertEqual(ReviewRequest.objects.filter(doc=review_req.doc, team=review_req.team, state="requested").count(), 1) self.assertEqual(len(outbox), 1) - + self.assertTrue("Test message" in unicode(outbox[0])) + diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index a0722a276..eabc02beb 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -10,8 +10,9 @@ from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person from ietf.name.models import ReviewRequestStateName from ietf.group.models import Role from ietf.review.models import ReviewRequest -from ietf.review.utils import active_review_teams, assign_review_request_to_reviewer -from ietf.review.utils import can_request_review_of_doc, can_manage_review_requests_for_team +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) from ietf.utils.fields import DatepickerDateField class RequestReviewForm(forms.ModelForm): @@ -134,6 +135,7 @@ def withdraw_request(request, name, request_id): return HttpResponseForbidden("You do not have permission to perform this action") if request.method == "POST" and request.POST.get("action") == "withdraw": + prev_state = review_req.state review_req.state = ReviewRequestStateName.objects.get(slug="withdrawn") review_req.save() @@ -144,9 +146,12 @@ def withdraw_request(request, name, request_id): desc="Withdrew request for {} review by {}".format(review_req.type.name, review_req.team.acronym.upper()), ) - if review_req.state_id != "requested": - # FIXME: handle this case - by emailing? - pass + if prev_state.slug != "requested": + email_about_review_request( + request, review_req, + "Withdrew review request for %s" % review_req.doc.name, + "Review request has been withdrawn by %s." % request.user.person, + by=request.user.person, notify_secretary=False, notify_reviewer=True) return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) @@ -187,7 +192,7 @@ def assign_reviewer(request, name, request_id): form = AssignReviewerForm(review_req, request.POST) if form.is_valid(): reviewer = form.cleaned_data["reviewer"] - assign_review_request_to_reviewer(review_req, reviewer, request.user.person) + assign_review_request_to_reviewer(request, review_req, reviewer) return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) else: @@ -216,35 +221,48 @@ def reject_reviewer_assignment(request, name, request_id): return HttpResponseForbidden("You do not have permission to perform this action") if request.method == "POST" and request.POST.get("action") == "reject": - # reject the request - review_req.state = ReviewRequestStateName.objects.get(slug="rejected") - review_req.save() + form = RejectReviewerAssignmentForm(request.POST) + if form.is_valid(): + # reject the request + review_req.state = ReviewRequestStateName.objects.get(slug="rejected") + review_req.save() - DocEvent.objects.create( - type="changed_review_request", - doc=review_req.doc, - by=request.user.person, - desc="Assignment of request for {} review by {} to {} was rejected".format( - review_req.type.name, - review_req.team.acronym.upper(), - review_req.reviewer.person, - ), - ) - - # make a new unassigned review request - new_review_req = ReviewRequest.objects.create( - time=review_req.time, - type=review_req.type, - doc=review_req.doc, - team=review_req.team, - deadline=review_req.deadline, - requested_rev=review_req.requested_rev, - state=ReviewRequestStateName.objects.get(slug="requested"), - ) + DocEvent.objects.create( + type="changed_review_request", + doc=review_req.doc, + by=request.user.person, + desc="Assignment of request for {} review by {} to {} was rejected".format( + review_req.type.name, + review_req.team.acronym.upper(), + review_req.reviewer.person, + ), + ) - return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk) + # make a new unassigned review request + new_review_req = ReviewRequest.objects.create( + time=review_req.time, + type=review_req.type, + doc=review_req.doc, + team=review_req.team, + deadline=review_req.deadline, + requested_rev=review_req.requested_rev, + state=ReviewRequestStateName.objects.get(slug="requested"), + ) + + msg = u"Reviewer assignment rejected by %s." % request.user.person + + m = form.cleaned_data.get("message_to_secretary") + if m: + msg += "\n\n" + "Explanation:" + "\n" + m + + email_about_review_request(request, review_req, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True) + + return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk) + else: + form = RejectReviewerAssignmentForm() return render(request, 'doc/review/reject_reviewer_assignment.html', { 'doc': doc, 'review_req': review_req, + 'form': form, }) diff --git a/ietf/review/models.py b/ietf/review/models.py index 530e0c939..79092e8b1 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -27,7 +27,7 @@ class ReviewRequest(models.Model): time = models.DateTimeField(auto_now_add=True) type = models.ForeignKey(ReviewTypeName) doc = models.ForeignKey(Document, related_name='review_request_set') - team = models.ForeignKey(Group) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewresultname=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") diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 43d495e15..8cc692d35 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -1,7 +1,10 @@ +from django.contrib.sites.models import Site + from ietf.group.models import Group, Role from ietf.doc.models import DocEvent from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream from ietf.review.models import ReviewRequestStateName +from ietf.utils.mail import send_mail def active_review_teams(): # if there's a ReviewResultName defined, it's a review team @@ -17,16 +20,47 @@ def can_manage_review_requests_for_team(user, team): if not user.is_authenticated(): return False - return Role.objects.filter(name="secretary", person__user=user, group=team).exists() or has_role(user, "Secretariat") + return Role.objects.filter(name__in=["secretary", "delegate"], person__user=user, group=team).exists() or has_role(user, "Secretariat") -def assign_review_request_to_reviewer(review_req, reviewer, by): - assert review_req.state_id in ("requested", "accepted") +def email_about_review_request(request, review_req, subject, msg, by, notify_secretary, notify_reviewer): + """Notify possibly both secretary and reviewer about change, skipping + a party if the change was done by that party.""" - if review_req.reviewer == reviewer: + def extract_email_addresses(roles): + if any(r.person == by for r in roles if r): + return [] + else: + return [r.formatted_email() for r in roles if r] + + to = [] + + if notify_secretary: + to += extract_email_addresses(Role.objects.filter(name__in=["secretary", "delegate"], group=review_req.team).distinct()) + if notify_reviewer: + to += extract_email_addresses([review_req.reviewer]) + + if not to: return - prev_state = review_req.state - prev_reviewer = review_req.reviewer + send_mail(request, list(set(to)), None, subject, "doc/mail/review_request_changed.txt", { + "domain": Site.objects.get_current().domain, + "review_req": review_req, + "msg": msg, + }) + + +def assign_review_request_to_reviewer(request, review_req, reviewer): + assert review_req.state_id in ("requested", "accepted") + + if reviewer == review_req.reviewer: + return + + if review_req.reviewer: + email_about_review_request( + request, review_req, + "Unassigned from review of %s" % review_req.doc.name, + "%s has cancelled your assignment to the review." % request.user.person, + by=request.user.person, notify_secretary=False, notify_reviewer=True) review_req.state = ReviewRequestStateName.objects.get(slug="requested") review_req.reviewer = reviewer @@ -35,14 +69,16 @@ def assign_review_request_to_reviewer(review_req, reviewer, by): DocEvent.objects.create( type="changed_review_request", doc=review_req.doc, - by=by, + by=request.user.person, desc="Request for {} review by {} is assigned to {}".format( review_req.type.name, review_req.team.acronym.upper(), review_req.reviewer.person if review_req.reviewer else "(None)", ), ) - - if prev_state.slug != "requested" and prev_reviewer: - # FIXME: email old reviewer? - pass + + email_about_review_request( + request, review_req, + "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) diff --git a/ietf/templates/doc/mail/review_request_changed.txt b/ietf/templates/doc/mail/review_request_changed.txt new file mode 100644 index 000000000..5c8f36d51 --- /dev/null +++ b/ietf/templates/doc/mail/review_request_changed.txt @@ -0,0 +1,7 @@ +{% autoescape off %} + {{ review_req.type.name }} review of: {{ review_req.doc.name }}{% if review_req.requested_rev %}-{{ review_req.requested_rev }}{% endif %} + https://{{ domain }}{% url "ietf.doc.views_review.review_request" name=review_req.doc.name request_id=review_req.pk %} + +{{ msg|wordwrap:72 }} + +{% endautoescape %} From 604287e75d370d3eaa4bedd9652acbd7ad9bf677 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 24 May 2016 15:03:51 +0000 Subject: [PATCH 07/16] Support accepting a reviewer assignment - Legacy-Id: 11237 --- ietf/doc/tests_review.py | 23 ++++++++++++++++ ietf/doc/views_review.py | 26 ++++++++++++++----- ietf/templates/doc/review/review_request.html | 10 ++++--- 3 files changed, 49 insertions(+), 10 deletions(-) diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index c2fe46549..b5d6787e7 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -4,6 +4,8 @@ import datetime from django.core.urlresolvers import reverse as urlreverse +from pyquery import PyQuery + import debug # pyflakes:ignore from ietf.review.models import ReviewRequest, Reviewer @@ -169,6 +171,27 @@ class ReviewTests(TestCase): self.assertTrue("cancelled your assignment" in unicode(outbox[0])) self.assertTrue("assigned" in unicode(outbox[1])) + def test_accept_reviewer_assignment(self): + doc = make_test_data() + review_req = make_review_data(doc) + review_req.state = ReviewRequestStateName.objects.get(slug="requested") + review_req.save() + + url = urlreverse('ietf.doc.views_review.review_request', kwargs={ "name": doc.name, "request_id": review_req.pk }) + username = review_req.reviewer.person.user.username + self.client.login(username=username, password=username + "+password") + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertTrue(q("[name=action][value=accept]")) + + # accept + r = self.client.post(url, { "action": "accept" }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "accepted") + def test_reject_reviewer_assignment(self): doc = make_test_data() review_req = make_review_data(doc) diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index eabc02beb..2c618a8bd 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -107,17 +107,28 @@ def review_request(request, name, request_id): review_req = get_object_or_404(ReviewRequest, pk=request_id) is_reviewer = review_req.reviewer and user_is_person(request.user, review_req.reviewer.person) - can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team) + can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) can_withdraw_request = (review_req.state_id in ["requested", "accepted"] - and is_authorized_in_doc_stream(request.user, doc)) + and (is_authorized_in_doc_stream(request.user, doc) + or can_manage_request)) can_assign_reviewer = (review_req.state_id in ["requested", "accepted"] and is_authorized_in_doc_stream(request.user, doc)) + can_accept_reviewer_assignment = (review_req.state_id == "requested" + and review_req.reviewer_id is not None + and (is_reviewer or can_manage_request)) + can_reject_reviewer_assignment = (review_req.state_id in ["requested", "accepted"] and review_req.reviewer_id is not None - and (is_reviewer or can_manage_req)) + and (is_reviewer or can_manage_request)) + + if request.method == "POST" and request.POST.get("action") == "accept" and can_accept_reviewer_assignment: + review_req.state = ReviewRequestStateName.objects.get(slug="accepted") + review_req.save() + + return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) return render(request, 'doc/review/review_request.html', { 'doc': doc, @@ -125,6 +136,7 @@ def review_request(request, name, request_id): 'can_withdraw_request': can_withdraw_request, 'can_reject_reviewer_assignment': can_reject_reviewer_assignment, 'can_assign_reviewer': can_assign_reviewer, + 'can_accept_reviewer_assignment': can_accept_reviewer_assignment, }) def withdraw_request(request, name, request_id): @@ -183,9 +195,9 @@ 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_req = can_manage_review_requests_for_team(request.user, review_req.team) + can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) - if not can_manage_req: + if not can_manage_request: return HttpResponseForbidden("You do not have permission to perform this action") if request.method == "POST" and request.POST.get("action") == "assign": @@ -215,9 +227,9 @@ def reject_reviewer_assignment(request, name, request_id): return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) is_reviewer = user_is_person(request.user, review_req.reviewer.person) - can_manage_req = can_manage_review_requests_for_team(request.user, review_req.team) + can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) - if not (is_reviewer or can_manage_req): + if not (is_reviewer or can_manage_request): return HttpResponseForbidden("You do not have permission to perform this action") if request.method == "POST" and request.POST.get("action") == "reject": diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index 844ff3c94..44fdfa615 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -70,12 +70,16 @@ None assigned yet {% endif %} - {% if can_assign_reviewer %} - {% if review_req.reviewer %}Reassign{% else %}Assign{% endif %} reviewer + {% if can_accept_reviewer_assignment %} + {% endif %} {% if can_reject_reviewer_assignment %} - Reject reviewer assignment + Reject + {% endif %} + + {% if can_assign_reviewer %} + {% if review_req.reviewer %}Reassign{% else %}Assign{% endif %} reviewer {% endif %} From dfd351c220a2b7dfee994ed838234127b6b1465b Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 24 May 2016 15:06:09 +0000 Subject: [PATCH 08/16] Fix missing CSRF token - Legacy-Id: 11238 --- ietf/templates/doc/review/review_request.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index 44fdfa615..aec78a6c1 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -71,7 +71,7 @@ {% endif %} {% if can_accept_reviewer_assignment %} -
+
{% csrf_token %}
{% endif %} {% if can_reject_reviewer_assignment %} From fdfc0bc8f53ed7bcf87a0421c4d97ba11bf950fb Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Thu, 26 May 2016 12:56:00 +0000 Subject: [PATCH 09/16] Insert missing form - Legacy-Id: 11246 --- ietf/templates/doc/review/reject_reviewer_assignment.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ietf/templates/doc/review/reject_reviewer_assignment.html b/ietf/templates/doc/review/reject_reviewer_assignment.html index edc8d9696..26097da19 100644 --- a/ietf/templates/doc/review/reject_reviewer_assignment.html +++ b/ietf/templates/doc/review/reject_reviewer_assignment.html @@ -13,6 +13,8 @@
{% csrf_token %} + {% bootstrap_form form %} + {% buttons %} Cancel From 5757f65598eeee4b5286dbef422b8ba0ac94f723 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Mon, 13 Jun 2016 09:48:37 +0000 Subject: [PATCH 10/16] Resolve name objects ordering order clashes by ordering by .name. Most are ordered, but we have a few without a natural order, and alphabetical helps a bit when debugging those. - Legacy-Id: 11336 --- ietf/name/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ietf/name/models.py b/ietf/name/models.py index b16da81f7..f17a55203 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -14,7 +14,7 @@ class NameModel(models.Model): class Meta: abstract = True - ordering = ['order'] + ordering = ['order', 'name'] class GroupStateName(NameModel): """BOF, Proposed, Active, Dormant, Concluded, Abandoned""" From b6e5aebcd4ec8c0b5c50775ff8351603aaeedac7 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Mon, 13 Jun 2016 10:13:57 +0000 Subject: [PATCH 11/16] Fix a slightly odd unnecessary form.save() - Legacy-Id: 11337 --- ietf/doc/views_charter.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/ietf/doc/views_charter.py b/ietf/doc/views_charter.py index 251324a77..9a94ba863 100644 --- a/ietf/doc/views_charter.py +++ b/ietf/doc/views_charter.py @@ -343,14 +343,6 @@ class UploadForm(forms.Form): def clean_txt(self): return get_cleaned_text_file_content(self.cleaned_data["txt"]) - def save(self, group, rev): - filename = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (group.charter.canonical_name(), rev)) - with open(filename, 'wb') as destination: - if self.cleaned_data['txt']: - destination.write(self.cleaned_data['txt']) - else: - destination.write(self.cleaned_data['content'].encode("utf-8")) - @login_required def submit(request, name=None, option=None): if not name.startswith('charter-'): @@ -390,7 +382,12 @@ def submit(request, name=None, option=None): e.save() # Save file on disk - form.save(group, charter.rev) + filename = os.path.join(settings.CHARTER_PATH, '%s-%s.txt' % (charter.canonical_name(), charter.rev)) + with open(filename, 'wb') as destination: + if form.cleaned_data['txt']: + destination.write(form.cleaned_data['txt']) + else: + destination.write(form.cleaned_data['content'].encode("utf-8")) if option in ['initcharter','recharter'] and charter.ad == None: charter.ad = getattr(group.ad_role(),'person',None) From b790781de992509611309945214baeeea7c98795 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 14 Jun 2016 09:54:37 +0000 Subject: [PATCH 12/16] Add return statements to remaining email sending function so it's possibly to get the message back. We need the message and its Message-ID in the review tracking code, to be able to link to it in the mail archive. - Legacy-Id: 11359 --- ietf/utils/mail.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/ietf/utils/mail.py b/ietf/utils/mail.py index deeaa67e2..1f17c9c14 100644 --- a/ietf/utils/mail.py +++ b/ietf/utils/mail.py @@ -187,7 +187,7 @@ def encode_message(txt): def send_mail_text(request, to, frm, subject, txt, cc=None, extra=None, toUser=False, bcc=None): """Send plain text message.""" msg = encode_message(txt) - send_mail_mime(request, to, frm, subject, msg, cc, extra, toUser, bcc) + return send_mail_mime(request, to, frm, subject, msg, cc, extra, toUser, bcc) def condition_message(to, frm, subject, msg, cc, extra): if isinstance(frm, tuple): @@ -284,6 +284,8 @@ def send_mail_mime(request, to, frm, subject, msg, cc=None, extra=None, toUser=F build_warning_message(request, e) send_error_email(e) + return msg + def parse_preformatted(preformatted, extra={}, override={}): """Parse preformatted string containing mail with From:, To:, ...,""" msg = message_from_string(preformatted.encode("utf-8")) @@ -323,8 +325,8 @@ def send_mail_message(request, message, extra={}): if message.reply_to: e['Reply-to'] = message.reply_to - send_mail_text(request, message.to, message.frm, message.subject, - message.body, cc=message.cc, bcc=message.bcc, extra=e) + return send_mail_text(request, message.to, message.frm, message.subject, + message.body, cc=message.cc, bcc=message.bcc, extra=e) def exception_components(e): # See if it's a non-smtplib exception that we faked From 7cbe36fb6248e7cbafe5aadf292e5f84c924ebfd Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 14 Jun 2016 11:28:53 +0000 Subject: [PATCH 13/16] Implement completing a review with tests. One can currently enter/upload content or retrieve it from an IETF mailarch archive through integrated searching support. Support for partial completion. - Legacy-Id: 11360 --- ietf/doc/tests_review.py | 290 +++- ietf/doc/urls_review.py | 2 + ietf/doc/utils.py | 2 +- ietf/doc/views_review.py | 284 +++- ietf/name/fixtures/names.json | 1266 +++++++++-------- .../0012_insert_review_name_data.py | 22 +- ietf/review/mailarch.py | 95 ++ ietf/review/resources.py | 65 + ietf/review/utils.py | 14 +- ietf/settings.py | 1 + ietf/static/ietf/css/ietf.css | 13 + ietf/static/ietf/js/complete-review.js | 131 ++ ietf/templates/doc/document_draft.html | 2 +- ietf/templates/doc/mail/completed_review.txt | 6 + .../doc/mail/partially_completed_review.txt | 6 + .../doc/mail/reviewer_assignment_rejected.txt | 6 + .../templates/doc/review/complete_review.html | 81 ++ ietf/templates/doc/review/review_request.html | 74 +- ietf/utils/text.py | 11 + ietf/utils/textupload.py | 12 +- 20 files changed, 1695 insertions(+), 688 deletions(-) create mode 100644 ietf/review/mailarch.py create mode 100644 ietf/review/resources.py create mode 100644 ietf/static/ietf/js/complete-review.js create mode 100644 ietf/templates/doc/mail/completed_review.txt create mode 100644 ietf/templates/doc/mail/partially_completed_review.txt create mode 100644 ietf/templates/doc/mail/reviewer_assignment_rejected.txt create mode 100644 ietf/templates/doc/review/complete_review.html create mode 100644 ietf/utils/text.py diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index b5d6787e7..e937bd27e 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -1,14 +1,19 @@ # -*- coding: utf-8 -*- -import datetime +import datetime, os, shutil, json +import tarfile, tempfile, mailbox +import email.mime.multipart, email.mime.text, email.utils +from StringIO import StringIO from django.core.urlresolvers import reverse as urlreverse +from django.conf import settings from pyquery import PyQuery import debug # pyflakes:ignore from ietf.review.models import ReviewRequest, Reviewer +import ietf.review.mailarch from ietf.person.models import Person from ietf.group.models import Group, Role from ietf.name.models import ReviewResultName, ReviewRequestStateName @@ -17,7 +22,6 @@ from ietf.utils.test_data import make_test_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"])) @@ -39,9 +43,28 @@ def make_review_data(doc): 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") + if not os.path.exists(self.review_dir): + os.mkdir(self.review_dir) + + self.old_document_path_pattern = settings.DOCUMENT_PATH_PATTERN + settings.DOCUMENT_PATH_PATTERN = self.review_dir + "/{doc.type_id}/" + + self.review_subdir = os.path.join(self.review_dir, "review") + if not os.path.exists(self.review_subdir): + os.mkdir(self.review_subdir) + + def tearDown(self): + shutil.rmtree(self.review_dir) + settings.DOCUMENT_PATH_PATTERN = self.old_document_path_pattern + def test_request_review(self): doc = make_test_data() review_req = make_review_data(doc) @@ -229,3 +252,266 @@ class ReviewTests(TestCase): self.assertEqual(len(outbox), 1) self.assertTrue("Test message" in unicode(outbox[0])) + def make_test_mbox_tarball(self, review_req): + mbox_path = os.path.join(self.review_dir, "testmbox.tar.gz") + with tarfile.open(mbox_path, "w:gz") as tar: + with tempfile.NamedTemporaryFile(dir=self.review_dir, suffix=".mbox") as tmp: + mbox = mailbox.mbox(tmp.name) + + # plain text + msg = email.mime.text.MIMEText("Hello,\n\nI have reviewed the document and did not find any problems.\n\nJohn Doe") + msg["From"] = "johndoe@example.com" + msg["To"] = review_req.team.list_email + msg["Subject"] = "Review of {}-01".format(review_req.doc.name) + msg["Message-ID"] = email.utils.make_msgid() + msg["Archived-At"] = "" + msg["Date"] = email.utils.formatdate() + + mbox.add(msg) + + # plain text + HTML + msg = email.mime.multipart.MIMEMultipart('alternative') + msg["From"] = "johndoe2@example.com" + msg["To"] = review_req.team.list_email + msg["Subject"] = "Review of {}".format(review_req.doc.name) + msg["Message-ID"] = email.utils.make_msgid() + msg["Archived-At"] = "" + + msg.attach(email.mime.text.MIMEText("Hi!,\r\nLooks OK!\r\n-John", "plain")) + msg.attach(email.mime.text.MIMEText("

Hi!,

Looks OK!

-John

", "html")) + mbox.add(msg) + + tmp.flush() + + tar.add(os.path.relpath(tmp.name)) + + return mbox_path + + def test_search_mail_archive(self): + doc = make_test_data() + review_req = make_review_data(doc) + review_req.state = ReviewRequestStateName.objects.get(slug="accepted") + review_req.save() + review_req.team.list_email = "{}@ietf.org".format(review_req.team.acronym) + review_req.team.save() + + # test URL construction + query_urls = ietf.review.mailarch.construct_query_urls(review_req) + self.assertTrue(review_req.doc.name in query_urls["query_data_url"]) + + # test parsing + mbox_path = self.make_test_mbox_tarball(review_req) + + try: + # mock URL generator and point it to local file - for this + # to work, the module (and not the function) must be + # imported in the view + real_fn = ietf.review.mailarch.construct_query_urls + ietf.review.mailarch.construct_query_urls = lambda review_req, query=None: { "query_data_url": "file://" + os.path.abspath(mbox_path) } + + url = urlreverse('ietf.doc.views_review.search_mail_archive', kwargs={ "name": doc.name, "request_id": review_req.pk }) + login_testing_unauthorized(self, "secretary", url) + + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + messages = json.loads(r.content)["messages"] + self.assertEqual(len(messages), 2) + + self.assertEqual(messages[0]["url"], "https://www.example.com/testmessage") + self.assertTrue("John Doe" in messages[0]["content"]) + self.assertEqual(messages[0]["subject"], "Review of {}-01".format(review_req.doc.name)) + + self.assertEqual(messages[1]["url"], "https://www.example.com/testmessage2") + self.assertTrue("Looks OK" in messages[1]["content"]) + self.assertTrue("" not in messages[1]["content"]) + self.assertEqual(messages[1]["subject"], "Review of {}".format(review_req.doc.name)) + finally: + ietf.review.mailarch.construct_query_urls = real_fn + + def setup_complete_review_test(self): + doc = make_test_data() + review_req = make_review_data(doc) + review_req.state = ReviewRequestStateName.objects.get(slug="accepted") + 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) + review_req.team.save() + + url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "request_id": review_req.pk }) + + return review_req, url + + def test_complete_review_upload_content(self): + review_req, url = self.setup_complete_review_test() + + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + + # get + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + # faulty post + r = self.client.post(url, data={ + "result": "ready", + "state": "completed", + "reviewed_rev": "abc", + "review_submission": "upload", + "review_content": "", + "review_url": "", + "review_file": "", + }) + self.assertEqual(r.status_code, 200) + q = PyQuery(r.content) + self.assertTrue(q("[name=reviewed_rev]").closest(".form-group").filter(".has-error")) + self.assertTrue(q("[name=review_file]").closest(".form-group").filter(".has-error")) + + # complete by uploading file + empty_outbox() + + test_file = StringIO("This is a review\nwith two lines") + test_file.name = "unnamed" + + r = self.client.post(url, data={ + "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "state": ReviewRequestStateName.objects.get(slug="completed").pk, + "reviewed_rev": review_req.doc.rev, + "review_submission": "upload", + "review_content": "", + "review_url": "", + "review_file": test_file, + }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") + self.assertEqual(review_req.result_id, "ready") + self.assertEqual(review_req.reviewed_rev, review_req.doc.rev) + self.assertTrue(review_req.team.acronym.lower() in review_req.review.name) + self.assertTrue(review_req.doc.rev in review_req.review.name) + + with open(os.path.join(self.review_subdir, review_req.review.name + "-" + review_req.review.rev + ".txt")) as f: + self.assertEqual(f.read(), "This is a review\nwith two lines") + + self.assertEqual(len(outbox), 1) + self.assertTrue(review_req.team.list_email in outbox[0]["To"]) + self.assertTrue("This is a review" in unicode(outbox[0])) + + self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url) + + def test_complete_review_enter_content(self): + review_req, url = self.setup_complete_review_test() + + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + + # complete by uploading file + empty_outbox() + + r = self.client.post(url, data={ + "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "state": ReviewRequestStateName.objects.get(slug="completed").pk, + "reviewed_rev": review_req.doc.rev, + "review_submission": "enter", + "review_content": "This is a review\nwith two lines", + "review_url": "", + "review_file": "", + }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") + + with open(os.path.join(self.review_subdir, review_req.review.name + "-" + review_req.review.rev + ".txt")) as f: + self.assertEqual(f.read(), "This is a review\nwith two lines") + + self.assertEqual(len(outbox), 1) + self.assertTrue(review_req.team.list_email in outbox[0]["To"]) + self.assertTrue("This is a review" in unicode(outbox[0])) + + self.assertTrue(settings.MAILING_LIST_ARCHIVE_URL in review_req.review.external_url) + + def test_complete_review_link_to_mailing_list(self): + review_req, url = self.setup_complete_review_test() + + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + + # complete by uploading file + empty_outbox() + + r = self.client.post(url, data={ + "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "state": ReviewRequestStateName.objects.get(slug="completed").pk, + "reviewed_rev": review_req.doc.rev, + "review_submission": "link", + "review_content": "This is a review\nwith two lines", + "review_url": "http://example.com/testreview/", + "review_file": "", + }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") + + with open(os.path.join(self.review_subdir, review_req.review.name + "-" + review_req.review.rev + ".txt")) as f: + self.assertEqual(f.read(), "This is a review\nwith two lines") + + self.assertEqual(len(outbox), 0) + self.assertTrue("http://example.com" in review_req.review.external_url) + + def test_partially_complete_review(self): + review_req, url = self.setup_complete_review_test() + + login_testing_unauthorized(self, review_req.reviewer.person.user.username, url) + + # partially complete + empty_outbox() + + r = self.client.post(url, data={ + "result": ReviewResultName.objects.get(teams=review_req.team, slug="ready").pk, + "state": ReviewRequestStateName.objects.get(slug="part-completed").pk, + "reviewed_rev": review_req.doc.rev, + "review_submission": "enter", + "review_content": "This is a review\nwith two lines", + }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "part-completed") + self.assertTrue(review_req.doc.rev in review_req.review.name) + + self.assertEqual(len(outbox), 2) + self.assertTrue("secretary" in outbox[0]["To"]) + self.assertTrue("partially" in outbox[0]["Subject"].lower()) + self.assertTrue("new review request" in unicode(outbox[0])) + + self.assertTrue(review_req.team.list_email in outbox[1]["To"]) + self.assertTrue("partial review" in outbox[1]["Subject"].lower()) + self.assertTrue("This is a review" in unicode(outbox[1])) + + first_review = review_req.review + first_reviewer = review_req.reviewer + + + # complete + review_req = ReviewRequest.objects.get(state="requested", doc=review_req.doc, team=review_req.team) + self.assertEqual(review_req.reviewer, None) + review_req.reviewer = first_reviewer # same reviewer, so we can test uniquification + review_req.save() + + 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, + "state": ReviewRequestStateName.objects.get(slug="completed").pk, + "reviewed_rev": review_req.doc.rev, + "review_submission": "enter", + "review_content": "This is another review\nwith\nthree lines", + }) + self.assertEqual(r.status_code, 302) + + review_req = reload_db_objects(review_req) + self.assertEqual(review_req.state_id, "completed") + self.assertTrue(review_req.doc.rev in review_req.review.name) + second_review = review_req.review + self.assertTrue(first_review.name != second_review.name) + self.assertTrue(second_review.name.endswith("-2")) # uniquified diff --git a/ietf/doc/urls_review.py b/ietf/doc/urls_review.py index 527833c0a..89be3f732 100644 --- a/ietf/doc/urls_review.py +++ b/ietf/doc/urls_review.py @@ -7,5 +7,7 @@ urlpatterns = patterns('', url(r'^(?P[0-9]+)/withdraw/$', views_review.withdraw_request), url(r'^(?P[0-9]+)/assignreviewer/$', views_review.assign_reviewer), url(r'^(?P[0-9]+)/rejectreviewerassignment/$', views_review.reject_reviewer_assignment), + url(r'^(?P[0-9]+)/complete/$', views_review.complete_review), + url(r'^(?P[0-9]+)/searchmailarchive/$', views_review.search_mail_archive), ) diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 500b283bf..5d7f02f93 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -16,7 +16,7 @@ from ietf.doc.models import DocEvent, ConsensusDocEvent, BallotDocEvent, NewRevi from ietf.doc.models import save_document_in_history from ietf.name.models import DocReminderTypeName, DocRelationshipName from ietf.group.models import Role -from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream +from ietf.ietfauth.utils import has_role from ietf.utils import draft, markup_txt from ietf.utils.mail import send_mail from ietf.mailtrigger.utils import gather_address_lists diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 2c618a8bd..f5c3226b8 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -1,19 +1,36 @@ -import datetime +import datetime, os, email.utils -from django.http import HttpResponseForbidden +from django.contrib.sites.models import Site +from django.http import HttpResponseForbidden, JsonResponse from django.shortcuts import render, get_object_or_404, redirect from django import forms from django.contrib.auth.decorators import login_required +from django.utils.html import mark_safe +from django.core.exceptions import ValidationError +from django.template.loader import render_to_string -from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent +from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent, State from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person -from ietf.name.models import ReviewRequestStateName +from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName from ietf.group.models import Role from ietf.review.models import ReviewRequest 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) + email_about_review_request, make_new_review_request_from_existing) +from ietf.review import mailarch from ietf.utils.fields import DatepickerDateField +from ietf.utils.text import skip_prefix +from ietf.utils.textupload import get_cleaned_text_file_content +from ietf.utils.mail import send_mail + +def clean_doc_revision(doc, rev): + if rev: + rev = rev.rjust(2, "0") + + if not NewRevisionDocEvent.objects.filter(doc=doc, rev=rev).exists(): + raise forms.ValidationError("Could not find revision \"{}\" of the document.".format(rev)) + + return rev class RequestReviewForm(forms.ModelForm): deadline_date = DatepickerDateField(date_format="yyyy-mm-dd", picker_settings={ "autoclose": "1", "start-date": "+0d" }) @@ -47,14 +64,7 @@ class RequestReviewForm(forms.ModelForm): return v def clean_requested_rev(self): - rev = self.cleaned_data.get("requested_rev") - if rev: - rev = rev.rjust(2, "0") - - if not NewRevisionDocEvent.objects.filter(doc=self.doc, rev=rev).exists(): - raise forms.ValidationError("Could not find revision '{}' of the document.".format(rev)) - - return rev + return clean_doc_revision(self.doc, self.cleaned_data.get("requested_rev")) def clean(self): deadline_date = self.cleaned_data.get('deadline_date') @@ -114,16 +124,20 @@ def review_request(request, name, request_id): or can_manage_request)) can_assign_reviewer = (review_req.state_id in ["requested", "accepted"] - and is_authorized_in_doc_stream(request.user, doc)) + and can_manage_request) can_accept_reviewer_assignment = (review_req.state_id == "requested" - and review_req.reviewer_id is not None + and review_req.reviewer and (is_reviewer or can_manage_request)) can_reject_reviewer_assignment = (review_req.state_id in ["requested", "accepted"] - and review_req.reviewer_id is not None + and review_req.reviewer and (is_reviewer or can_manage_request)) + can_complete_review = (review_req.state_id in ["requested", "accepted"] + and review_req.reviewer + and (is_reviewer or can_manage_request)) + if request.method == "POST" and request.POST.get("action") == "accept" and can_accept_reviewer_assignment: review_req.state = ReviewRequestStateName.objects.get(slug="accepted") review_req.save() @@ -137,8 +151,10 @@ def review_request(request, name, request_id): 'can_reject_reviewer_assignment': can_reject_reviewer_assignment, 'can_assign_reviewer': can_assign_reviewer, 'can_accept_reviewer_assignment': can_accept_reviewer_assignment, + 'can_complete_review': can_complete_review, }) +@login_required def withdraw_request(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"]) @@ -191,6 +207,7 @@ class AssignReviewerForm(forms.Form): if review_req.reviewer: f.initial = review_req.reviewer_id +@login_required 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"]) @@ -217,8 +234,9 @@ def assign_reviewer(request, name, request_id): }) class RejectReviewerAssignmentForm(forms.Form): - message_to_secretary = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional explanation of rejection, will be emailed to team secretary") + message_to_secretary = forms.CharField(widget=forms.Textarea, required=False, help_text="Optional explanation of rejection, will be emailed to team secretary if filled in") +@login_required def reject_reviewer_assignment(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"]) @@ -251,21 +269,13 @@ def reject_reviewer_assignment(request, name, request_id): ) # make a new unassigned review request - new_review_req = ReviewRequest.objects.create( - time=review_req.time, - type=review_req.type, - doc=review_req.doc, - team=review_req.team, - deadline=review_req.deadline, - requested_rev=review_req.requested_rev, - state=ReviewRequestStateName.objects.get(slug="requested"), - ) + new_review_req = make_new_review_request_from_existing(review_req) + new_review_req.save() - msg = u"Reviewer assignment rejected by %s." % request.user.person - - m = form.cleaned_data.get("message_to_secretary") - if m: - msg += "\n\n" + "Explanation:" + "\n" + m + msg = render_to_string("doc/mail/reviewer_assignment_rejected.txt", { + "by": request.user.person, + "message_to_secretary": form.cleaned_data.get("message_to_secretary") + }) email_about_review_request(request, review_req, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True) @@ -278,3 +288,215 @@ def reject_reviewer_assignment(request, name, request_id): 'review_req': review_req, 'form': form, }) + +class CompleteReviewForm(forms.Form): + state = forms.ModelChoiceField(queryset=ReviewRequestStateName.objects.filter(slug__in=("completed", "part-completed")).order_by("-order"), widget=forms.RadioSelect, initial="completed") + reviewed_rev = forms.CharField(label="Reviewed revision", max_length=4) + result = forms.ModelChoiceField(queryset=ReviewResultName.objects.filter(used=True), widget=forms.RadioSelect, empty_label=None) + ACTIONS = [ + ("enter", "Enter review content (automatically posts to {mailing_list})"), + ("upload", "Upload review content in text file (automatically posts to {mailing_list})"), + ("link", "Link to review message already sent to {mailing_list}"), + ] + review_submission = forms.ChoiceField(choices=ACTIONS, widget=forms.RadioSelect) + + review_url = forms.URLField(label="Link to message", required=False) + review_file = forms.FileField(label="Text file to upload", required=False) + review_content = forms.CharField(widget=forms.Textarea, required=False) + + def __init__(self, review_req, *args, **kwargs): + self.review_req = review_req + + super(CompleteReviewForm, self).__init__(*args, **kwargs) + + doc = self.review_req.doc + + known_revisions = NewRevisionDocEvent.objects.filter(doc=doc).order_by("-time").values_list("rev", flat=True) + + self.fields["state"].choices = [ + (slug, "{} - extra reviewer is to be assigned".format(label)) if slug == "part-completed" else (slug, label) + for slug, label in self.fields["state"].choices + ] + + self.fields["reviewed_rev"].help_text = mark_safe( + " ".join("{}".format(r) + for r in known_revisions)) + + self.fields["result"].queryset = self.fields["result"].queryset.filter(teams=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 + ] + + def clean_reviewed_rev(self): + return clean_doc_revision(self.review_req.doc, self.cleaned_data.get("reviewed_rev")) + + def clean_review_content(self): + return self.cleaned_data["review_content"].replace("\r", "") + + def clean_review_file(self): + return get_cleaned_text_file_content(self.cleaned_data["review_file"]) + + def clean(self): + def require_field(f): + if not self.cleaned_data.get(f): + self.add_error(f, ValidationError("You must fill in this field.")) + + submission_method = self.cleaned_data.get("review_submission") + if submission_method == "enter": + require_field("review_content") + elif submission_method == "upload": + require_field("review_file") + elif submission_method == "link": + require_field("review_url") + require_field("review_content") + +@login_required +def complete_review(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"]) + + if not review_req.reviewer: + return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + + is_reviewer = user_is_person(request.user, review_req.reviewer.person) + can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) + + if not (is_reviewer or can_manage_request): + return HttpResponseForbidden("You do not have permission to perform this action") + + if request.method == "POST": + form = CompleteReviewForm(review_req, request.POST, request.FILES) + if form.is_valid(): + review_submission = form.cleaned_data['review_submission'] + + # create review doc + for i in range(1, 100): + name_components = [ + "review", + review_req.team.acronym, + review_req.type.slug, + review_req.reviewer.person.ascii_parts()[3], + skip_prefix(review_req.doc.name, "draft-"), + form.cleaned_data["reviewed_rev"], + ] + if i > 1: + name_components.append(str(i)) + + name = "-".join(c for c in name_components if c).lower() + if not Document.objects.filter(name=name).exists(): + review = Document.objects.create(name=name) + break + + review.type = DocTypeName.objects.get(slug="review") + review.rev = "00" + review.title = "Review of {}".format(review_req.doc.name) + review.group = review_req.team + if review_submission == "link": + review.external_url = form.cleaned_data['review_url'] + review.save() + review.set_state(State.objects.get(type="review", slug="active")) + + NewRevisionDocEvent.objects.create( + type="new_revision", + doc=review, + by=request.user.person, + rev=doc.rev, + desc='New revision available', + time=doc.time, + ) + + # save file on disk + if review_submission == "upload": + encoded_content = form.cleaned_data['review_file'] + else: + encoded_content = form.cleaned_data['review_content'].encode("utf-8") + + filename = os.path.join(review.get_file_path(), '{}-{}.txt'.format(review.name, review.rev)) + with open(filename, 'wb') as destination: + destination.write(encoded_content) + + # close review request + review_req.state = form.cleaned_data["state"] + review_req.reviewed_rev = form.cleaned_data["reviewed_rev"] + review_req.result = form.cleaned_data["result"] + review_req.review = review + review_req.save() + + DocEvent.objects.create( + type="changed_review_request", + doc=review_req.doc, + by=request.user.person, + desc="Request for {} review by {} {}".format( + review_req.type.name, + review_req.team.acronym.upper(), + review_req.state.name, + ), + ) + + if review_req.state_id == "part-completed": + new_review_req = make_new_review_request_from_existing(review_req) + new_review_req.save() + + subject = "Review of {}-{} completed partially".format(review_req.doc.name, review_req.reviewed_rev) + + msg = render_to_string("doc/mail/partially_completed_review.txt", { + "domain": Site.objects.get_current().domain, + "by": request.user.person, + "new_review_req": new_review_req, + }) + + email_about_review_request(request, review_req, subject, msg, request.user.person, notify_secretary=True, notify_reviewer=False) + + if review_submission != "link" and review_req.team.list_email: + # email the review + subject = "{} of {}-{}".format("Partial review" if review_req.state_id == "part-completed" else "Review", review_req.doc.name, review_req.reviewed_rev) + msg = send_mail(request, [(review_req.team.name, review_req.team.list_email)], None, + subject, + "doc/mail/completed_review.txt", { + "review_req": review_req, + "content": encoded_content.decode("utf-8"), + }) + + list_name = mailarch.list_name_from_email(review_req.team.list_email) + if list_name: + review.external_url = mailarch.construct_message_url(list_name, email.utils.unquote(msg["Message-ID"])) + review.save() + + return redirect("doc_view", name=review_req.review.name) + else: + form = CompleteReviewForm(review_req) + + mail_archive_query_urls = mailarch.construct_query_urls(review_req) + + return render(request, 'doc/review/complete_review.html', { + 'doc': doc, + 'review_req': review_req, + 'form': form, + 'mail_archive_query_urls': mail_archive_query_urls, + }) + +def search_mail_archive(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"]) + + is_reviewer = user_is_person(request.user, review_req.reviewer.person) + can_manage_request = can_manage_review_requests_for_team(request.user, review_req.team) + + if not (is_reviewer or can_manage_request): + return HttpResponseForbidden("You do not have permission to perform this action") + + res = mailarch.construct_query_urls(review_req, query=request.GET.get("query")) + if not res: + return JsonResponse({ "error": "Couldn't do lookup in mail archive - don't know where to look"}) + + MAX_RESULTS = 30 + + try: + res["messages"] = mailarch.retrieve_messages(res["query_data_url"])[:MAX_RESULTS] + except Exception as e: + res["error"] = "Retrieval from mail archive failed: {}".format(unicode(e)) + # raise # useful when debugging + + return JsonResponse(res) + diff --git a/ietf/name/fixtures/names.json b/ietf/name/fixtures/names.json index 5d9e8def4..72aeb7b2f 100644 --- a/ietf/name/fixtures/names.json +++ b/ietf/name/fixtures/names.json @@ -21,17 +21,6 @@ "model": "name.ballotpositionname", "pk": "noobj" }, -{ - "fields": { - "order": 3, - "used": true, - "name": "Discuss", - "blocking": true, - "desc": "" - }, - "model": "name.ballotpositionname", - "pk": "discuss" -}, { "fields": { "order": 3, @@ -43,6 +32,17 @@ "model": "name.ballotpositionname", "pk": "block" }, +{ + "fields": { + "order": 3, + "used": true, + "name": "Discuss", + "blocking": true, + "desc": "" + }, + "model": "name.ballotpositionname", + "pk": "discuss" +}, { "fields": { "order": 4, @@ -124,11 +124,11 @@ "fields": { "order": 0, "used": true, - "name": "reStructuredText", + "name": "Django", "desc": "" }, "model": "name.dbtemplatetypename", - "pk": "rst" + "pk": "django" }, { "fields": { @@ -144,44 +144,11 @@ "fields": { "order": 0, "used": true, - "name": "Django", + "name": "reStructuredText", "desc": "" }, "model": "name.dbtemplatetypename", - "pk": "django" -}, -{ - "fields": { - "order": 0, - "revname": "Obsoleted by", - "used": true, - "name": "Obsoletes", - "desc": "" - }, - "model": "name.docrelationshipname", - "pk": "obs" -}, -{ - "fields": { - "order": 0, - "revname": "Updated by", - "used": true, - "name": "Updates", - "desc": "" - }, - "model": "name.docrelationshipname", - "pk": "updates" -}, -{ - "fields": { - "order": 0, - "revname": "Replaced by", - "used": true, - "name": "Replaces", - "desc": "" - }, - "model": "name.docrelationshipname", - "pk": "replaces" + "pk": "rst" }, { "fields": { @@ -194,28 +161,6 @@ "model": "name.docrelationshipname", "pk": "conflrev" }, -{ - "fields": { - "order": 0, - "revname": "Normatively Referenced by", - "used": true, - "name": "Normative Reference", - "desc": "Normative Reference" - }, - "model": "name.docrelationshipname", - "pk": "refnorm" -}, -{ - "fields": { - "order": 0, - "revname": "Referenced by", - "used": true, - "name": "Reference", - "desc": "A reference found in a document which does not have split normative/informative reference sections." - }, - "model": "name.docrelationshipname", - "pk": "refold" -}, { "fields": { "order": 0, @@ -227,50 +172,6 @@ "model": "name.docrelationshipname", "pk": "refinfo" }, -{ - "fields": { - "order": 0, - "revname": "Moved to Proposed Standard by", - "used": true, - "name": "Moves to Proposed Standard", - "desc": "" - }, - "model": "name.docrelationshipname", - "pk": "tops" -}, -{ - "fields": { - "order": 0, - "revname": "Moved to Internet Standard by", - "used": true, - "name": "Moves to Internet Standard", - "desc": "" - }, - "model": "name.docrelationshipname", - "pk": "tois" -}, -{ - "fields": { - "order": 0, - "revname": "Moved to Historic by", - "used": true, - "name": "Moves to Historic", - "desc": "" - }, - "model": "name.docrelationshipname", - "pk": "tohist" -}, -{ - "fields": { - "order": 0, - "revname": "Moved to Informational by", - "used": true, - "name": "Moves to Informational", - "desc": "" - }, - "model": "name.docrelationshipname", - "pk": "toinf" -}, { "fields": { "order": 0, @@ -293,6 +194,72 @@ "model": "name.docrelationshipname", "pk": "toexp" }, +{ + "fields": { + "order": 0, + "revname": "Moved to Historic by", + "used": true, + "name": "Moves to Historic", + "desc": "" + }, + "model": "name.docrelationshipname", + "pk": "tohist" +}, +{ + "fields": { + "order": 0, + "revname": "Moved to Informational by", + "used": true, + "name": "Moves to Informational", + "desc": "" + }, + "model": "name.docrelationshipname", + "pk": "toinf" +}, +{ + "fields": { + "order": 0, + "revname": "Moved to Internet Standard by", + "used": true, + "name": "Moves to Internet Standard", + "desc": "" + }, + "model": "name.docrelationshipname", + "pk": "tois" +}, +{ + "fields": { + "order": 0, + "revname": "Moved to Proposed Standard by", + "used": true, + "name": "Moves to Proposed Standard", + "desc": "" + }, + "model": "name.docrelationshipname", + "pk": "tops" +}, +{ + "fields": { + "order": 0, + "revname": "Normatively Referenced by", + "used": true, + "name": "Normative Reference", + "desc": "Normative Reference" + }, + "model": "name.docrelationshipname", + "pk": "refnorm" +}, +{ + "fields": { + "order": 0, + "revname": "Obsoleted by", + "used": true, + "name": "Obsoletes", + "desc": "" + }, + "model": "name.docrelationshipname", + "pk": "obs" +}, { "fields": { "order": 0, @@ -304,6 +271,39 @@ "model": "name.docrelationshipname", "pk": "possibly-replaces" }, +{ + "fields": { + "order": 0, + "revname": "Referenced by", + "used": true, + "name": "Reference", + "desc": "A reference found in a document which does not have split normative/informative reference sections." + }, + "model": "name.docrelationshipname", + "pk": "refold" +}, +{ + "fields": { + "order": 0, + "revname": "Replaced by", + "used": true, + "name": "Replaces", + "desc": "" + }, + "model": "name.docrelationshipname", + "pk": "replaces" +}, +{ + "fields": { + "order": 0, + "revname": "Updated by", + "used": true, + "name": "Updates", + "desc": "" + }, + "model": "name.docrelationshipname", + "pk": "updates" +}, { "fields": { "order": 3, @@ -329,31 +329,11 @@ "fields": { "order": 0, "used": true, - "name": "IANA coordination", - "desc": "RFC-Editor/IANA Registration Coordination" + "name": "Approved in minutes", + "desc": "" }, "model": "name.doctagname", - "pk": "iana-crd" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Holding for references", - "desc": "Holding for normative reference" - }, - "model": "name.doctagname", - "pk": "ref" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Missing references", - "desc": "Awaiting missing normative reference" - }, - "model": "name.doctagname", - "pk": "missref" + "pk": "app-min" }, { "fields": { @@ -369,61 +349,11 @@ "fields": { "order": 0, "used": true, - "name": "Review by RFC Editor", - "desc": "" + "name": "Holding for references", + "desc": "Holding for normative reference" }, "model": "name.doctagname", - "pk": "rfc-rev" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Via RFC Editor", - "desc": "" - }, - "model": "name.doctagname", - "pk": "via-rfc" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Approved in minutes", - "desc": "" - }, - "model": "name.doctagname", - "pk": "app-min" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Shepherd Needed", - "desc": "" - }, - "model": "name.doctagname", - "pk": "need-sh" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Waiting for Dependency on Other Document", - "desc": "" - }, - "model": "name.doctagname", - "pk": "w-dep" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "IESG Review Completed", - "desc": "" - }, - "model": "name.doctagname", - "pk": "iesg-com" + "pk": "ref" }, { "fields": { @@ -439,11 +369,31 @@ "fields": { "order": 0, "used": true, - "name": "Revised I-D Needed - Issue raised by WG", + "name": "IANA coordination", + "desc": "RFC-Editor/IANA Registration Coordination" + }, + "model": "name.doctagname", + "pk": "iana-crd" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "IESG Review Completed", "desc": "" }, "model": "name.doctagname", - "pk": "rev-wg" + "pk": "iesg-com" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Missing references", + "desc": "Awaiting missing normative reference" + }, + "model": "name.doctagname", + "pk": "missref" }, { "fields": { @@ -457,13 +407,53 @@ }, { "fields": { - "order": 1, + "order": 0, "used": true, - "name": "Point Raised - writeup needed", - "desc": "IESG discussions on the document have raised some issues that need to be brought to the attention of the authors/WG, but those issues have not been written down yet. (It is common for discussions during a telechat to result in such situations. An AD may raise a possible issue during a telechat and only decide as a result of that discussion whether the issue is worth formally writing up and bringing to the attention of the authors/WG). A document stays in the \"Point Raised - Writeup Needed\" state until *ALL* IESG comments that have been raised have been documented." + "name": "Review by RFC Editor", + "desc": "" }, "model": "name.doctagname", - "pk": "point" + "pk": "rfc-rev" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Revised I-D Needed - Issue raised by WG", + "desc": "" + }, + "model": "name.doctagname", + "pk": "rev-wg" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Shepherd Needed", + "desc": "" + }, + "model": "name.doctagname", + "pk": "need-sh" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Via RFC Editor", + "desc": "" + }, + "model": "name.doctagname", + "pk": "via-rfc" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Waiting for Dependency on Other Document", + "desc": "" + }, + "model": "name.doctagname", + "pk": "w-dep" }, { "fields": { @@ -485,6 +475,16 @@ "model": "name.doctagname", "pk": "need-ed" }, +{ + "fields": { + "order": 1, + "used": true, + "name": "Point Raised - writeup needed", + "desc": "IESG discussions on the document have raised some issues that need to be brought to the attention of the authors/WG, but those issues have not been written down yet. (It is common for discussions during a telechat to result in such situations. An AD may raise a possible issue during a telechat and only decide as a result of that discussion whether the issue is worth formally writing up and bringing to the attention of the authors/WG). A document stays in the \"Point Raised - Writeup Needed\" state until *ALL* IESG comments that have been raised have been documented." + }, + "model": "name.doctagname", + "pk": "point" +}, { "fields": { "order": 2, @@ -515,16 +515,6 @@ "model": "name.doctagname", "pk": "w-part" }, -{ - "fields": { - "order": 3, - "used": true, - "name": "External Party", - "desc": "The document is awaiting review or input from an external party (i.e, someone other than the shepherding AD, the authors, or the WG). See the \"note\" field for more details on who has the action." - }, - "model": "name.doctagname", - "pk": "extpty" -}, { "fields": { "order": 3, @@ -545,6 +535,16 @@ "model": "name.doctagname", "pk": "w-review" }, +{ + "fields": { + "order": 3, + "used": true, + "name": "External Party", + "desc": "The document is awaiting review or input from an external party (i.e, someone other than the shepherding AD, the authors, or the WG). See the \"note\" field for more details on who has the action." + }, + "model": "name.doctagname", + "pk": "extpty" +}, { "fields": { "order": 4, @@ -645,17 +645,6 @@ "model": "name.doctagname", "pk": "other" }, -{ - "fields": { - "order": 0, - "prefix": "charter", - "used": true, - "name": "Charter", - "desc": "" - }, - "model": "name.doctypename", - "pk": "charter" -}, { "fields": { "order": 0, @@ -670,46 +659,24 @@ { "fields": { "order": 0, - "prefix": "minutes", + "prefix": "bluesheets", "used": true, - "name": "Minutes", + "name": "Bluesheets", "desc": "" }, "model": "name.doctypename", - "pk": "minutes" + "pk": "bluesheets" }, { "fields": { "order": 0, - "prefix": "slides", + "prefix": "charter", "used": true, - "name": "Slides", + "name": "Charter", "desc": "" }, "model": "name.doctypename", - "pk": "slides" -}, -{ - "fields": { - "order": 0, - "prefix": "draft", - "used": true, - "name": "Draft", - "desc": "" - }, - "model": "name.doctypename", - "pk": "draft" -}, -{ - "fields": { - "order": 0, - "prefix": "liai-att", - "used": true, - "name": "Liaison Attachment", - "desc": "" - }, - "model": "name.doctypename", - "pk": "liai-att" + "pk": "charter" }, { "fields": { @@ -725,24 +692,13 @@ { "fields": { "order": 0, - "prefix": "status-change", + "prefix": "draft", "used": true, - "name": "Status Change", + "name": "Draft", "desc": "" }, "model": "name.doctypename", - "pk": "statchg" -}, -{ - "fields": { - "order": 0, - "prefix": "", - "used": false, - "name": "Shepherd's writeup", - "desc": "" - }, - "model": "name.doctypename", - "pk": "shepwrit" + "pk": "draft" }, { "fields": { @@ -755,6 +711,28 @@ "model": "name.doctypename", "pk": "liaison" }, +{ + "fields": { + "order": 0, + "prefix": "liai-att", + "used": true, + "name": "Liaison Attachment", + "desc": "" + }, + "model": "name.doctypename", + "pk": "liai-att" +}, +{ + "fields": { + "order": 0, + "prefix": "minutes", + "used": true, + "name": "Minutes", + "desc": "" + }, + "model": "name.doctypename", + "pk": "minutes" +}, { "fields": { "order": 0, @@ -769,13 +747,46 @@ { "fields": { "order": 0, - "prefix": "bluesheets", + "prefix": "", "used": true, - "name": "Bluesheets", + "name": "Review", "desc": "" }, "model": "name.doctypename", - "pk": "bluesheets" + "pk": "review" +}, +{ + "fields": { + "order": 0, + "prefix": "", + "used": false, + "name": "Shepherd's writeup", + "desc": "" + }, + "model": "name.doctypename", + "pk": "shepwrit" +}, +{ + "fields": { + "order": 0, + "prefix": "slides", + "used": true, + "name": "Slides", + "desc": "" + }, + "model": "name.doctypename", + "pk": "slides" +}, +{ + "fields": { + "order": 0, + "prefix": "status-change", + "used": true, + "name": "Status Change", + "desc": "" + }, + "model": "name.doctypename", + "pk": "statchg" }, { "fields": { @@ -886,11 +897,11 @@ "fields": { "order": 0, "used": true, - "name": "Questionnaire response", + "name": "Junk", "desc": "" }, "model": "name.feedbacktypename", - "pk": "questio" + "pk": "junk" }, { "fields": { @@ -906,11 +917,11 @@ "fields": { "order": 0, "used": true, - "name": "Junk", + "name": "Questionnaire response", "desc": "" }, "model": "name.feedbacktypename", - "pk": "junk" + "pk": "questio" }, { "fields": { @@ -956,21 +967,11 @@ "fields": { "order": 0, "used": true, - "name": "BOF", - "desc": "" + "name": "Abandonded", + "desc": "Formation of the group (most likely a BoF or Proposed WG) was abandoned" }, "model": "name.groupstatename", - "pk": "bof" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Proposed", - "desc": "" - }, - "model": "name.groupstatename", - "pk": "proposed" + "pk": "abandon" }, { "fields": { @@ -986,41 +987,11 @@ "fields": { "order": 0, "used": true, - "name": "Dormant", + "name": "BOF", "desc": "" }, "model": "name.groupstatename", - "pk": "dormant" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Concluded", - "desc": "" - }, - "model": "name.groupstatename", - "pk": "conclude" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Unknown", - "desc": "" - }, - "model": "name.groupstatename", - "pk": "unknown" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Abandonded", - "desc": "Formation of the group (most likely a BoF or Proposed WG) was abandoned" - }, - "model": "name.groupstatename", - "pk": "abandon" + "pk": "bof" }, { "fields": { @@ -1032,6 +1003,36 @@ "model": "name.groupstatename", "pk": "bof-conc" }, +{ + "fields": { + "order": 0, + "used": true, + "name": "Concluded", + "desc": "" + }, + "model": "name.groupstatename", + "pk": "conclude" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Dormant", + "desc": "" + }, + "model": "name.groupstatename", + "pk": "dormant" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Proposed", + "desc": "" + }, + "model": "name.groupstatename", + "pk": "proposed" +}, { "fields": { "order": 0, @@ -1046,21 +1047,11 @@ "fields": { "order": 0, "used": true, - "name": "IETF", + "name": "Unknown", "desc": "" }, - "model": "name.grouptypename", - "pk": "ietf" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Area", - "desc": "" - }, - "model": "name.grouptypename", - "pk": "area" + "model": "name.groupstatename", + "pk": "unknown" }, { "fields": { @@ -1076,81 +1067,21 @@ "fields": { "order": 0, "used": true, - "name": "WG", - "desc": "Working group" - }, - "model": "name.grouptypename", - "pk": "wg" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "RG", - "desc": "Research group" - }, - "model": "name.grouptypename", - "pk": "rg" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Team", + "name": "Area", "desc": "" }, "model": "name.grouptypename", - "pk": "team" + "pk": "area" }, { "fields": { "order": 0, "used": true, - "name": "Individual", - "desc": "" + "name": "Directorate", + "desc": "In many areas, the Area Directors have formed an advisory group or directorate. These comprise experienced members of the IETF and the technical community represented by the area. The specific name and the details of the role for each group differ from area to area, but the primary intent is that these groups assist the Area Director(s), e.g., with the review of specifications produced in the area." }, "model": "name.grouptypename", - "pk": "individ" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "SDO", - "desc": "Standards organization" - }, - "model": "name.grouptypename", - "pk": "sdo" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "IRTF", - "desc": "" - }, - "model": "name.grouptypename", - "pk": "irtf" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "RFC Editor", - "desc": "" - }, - "model": "name.grouptypename", - "pk": "rfcedtyp" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Nomcom", - "desc": "An IETF/IAB Nominating Committee. Use 'SDO' for external nominating committees." - }, - "model": "name.grouptypename", - "pk": "nomcom" + "pk": "dir" }, { "fields": { @@ -1162,6 +1093,36 @@ "model": "name.grouptypename", "pk": "iab" }, +{ + "fields": { + "order": 0, + "used": true, + "name": "IETF", + "desc": "" + }, + "model": "name.grouptypename", + "pk": "ietf" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Individual", + "desc": "" + }, + "model": "name.grouptypename", + "pk": "individ" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "IRTF", + "desc": "" + }, + "model": "name.grouptypename", + "pk": "irtf" +}, { "fields": { "order": 0, @@ -1176,11 +1137,61 @@ "fields": { "order": 0, "used": true, - "name": "Directorate", - "desc": "In many areas, the Area Directors have formed an advisory group or directorate. These comprise experienced members of the IETF and the technical community represented by the area. The specific name and the details of the role for each group differ from area to area, but the primary intent is that these groups assist the Area Director(s), e.g., with the review of specifications produced in the area." + "name": "Nomcom", + "desc": "An IETF/IAB Nominating Committee. Use 'SDO' for external nominating committees." }, "model": "name.grouptypename", - "pk": "dir" + "pk": "nomcom" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "RFC Editor", + "desc": "" + }, + "model": "name.grouptypename", + "pk": "rfcedtyp" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "RG", + "desc": "Research group" + }, + "model": "name.grouptypename", + "pk": "rg" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "SDO", + "desc": "Standards organization" + }, + "model": "name.grouptypename", + "pk": "sdo" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Team", + "desc": "" + }, + "model": "name.grouptypename", + "pk": "team" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "WG", + "desc": "Working group" + }, + "model": "name.grouptypename", + "pk": "wg" }, { "fields": { @@ -1306,61 +1317,31 @@ "fields": { "order": 0, "used": true, - "name": "Submitted", + "name": "Changed disclosure metadata", "desc": "" }, "model": "name.ipreventtypename", - "pk": "submitted" + "pk": "changed_disclosure" }, { "fields": { "order": 0, "used": true, - "name": "Posted", + "name": "Comment", "desc": "" }, "model": "name.ipreventtypename", - "pk": "posted" + "pk": "comment" }, { "fields": { "order": 0, "used": true, - "name": "Removed", + "name": "Legacy", "desc": "" }, "model": "name.ipreventtypename", - "pk": "removed" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Rejected", - "desc": "" - }, - "model": "name.ipreventtypename", - "pk": "rejected" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Pending", - "desc": "" - }, - "model": "name.ipreventtypename", - "pk": "pending" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Parked", - "desc": "" - }, - "model": "name.ipreventtypename", - "pk": "parked" + "pk": "legacy" }, { "fields": { @@ -1386,11 +1367,31 @@ "fields": { "order": 0, "used": true, - "name": "Comment", + "name": "Parked", "desc": "" }, "model": "name.ipreventtypename", - "pk": "comment" + "pk": "parked" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Pending", + "desc": "" + }, + "model": "name.ipreventtypename", + "pk": "pending" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Posted", + "desc": "" + }, + "model": "name.ipreventtypename", + "pk": "posted" }, { "fields": { @@ -1406,11 +1407,31 @@ "fields": { "order": 0, "used": true, - "name": "Legacy", + "name": "Rejected", "desc": "" }, "model": "name.ipreventtypename", - "pk": "legacy" + "pk": "rejected" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Removed", + "desc": "" + }, + "model": "name.ipreventtypename", + "pk": "removed" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Submitted", + "desc": "" + }, + "model": "name.ipreventtypename", + "pk": "submitted" }, { "fields": { @@ -1422,16 +1443,6 @@ "model": "name.ipreventtypename", "pk": "update_notify" }, -{ - "fields": { - "order": 0, - "used": true, - "name": "Changed disclosure metadata", - "desc": "" - }, - "model": "name.ipreventtypename", - "pk": "changed_disclosure" -}, { "fields": { "order": 0, @@ -1722,16 +1733,6 @@ "model": "name.meetingtypename", "pk": "interim" }, -{ - "fields": { - "order": 0, - "used": true, - "name": "Nominated, pending response", - "desc": "" - }, - "model": "name.nomineepositionstatename", - "pk": "pending" -}, { "fields": { "order": 0, @@ -1752,6 +1753,16 @@ "model": "name.nomineepositionstatename", "pk": "declined" }, +{ + "fields": { + "order": 0, + "used": true, + "name": "Nominated, pending response", + "desc": "" + }, + "model": "name.nomineepositionstatename", + "pk": "pending" +}, { "fields": { "order": 1, @@ -1814,7 +1825,17 @@ }, { "fields": { - "order": 7, + "order": 6, + "used": true, + "name": "Partially Completed", + "desc": "" + }, + "model": "name.reviewrequeststatename", + "pk": "part-completed" +}, +{ + "fields": { + "order": 8, "used": true, "name": "Completed", "desc": "" @@ -1827,11 +1848,11 @@ "order": 1, "used": true, "teams": [], - "name": "Almost Ready", + "name": "Serious Issues", "desc": "" }, "model": "name.reviewresultname", - "pk": "almost-ready" + "pk": "serious-issues" }, { "fields": { @@ -1882,11 +1903,11 @@ "order": 6, "used": true, "teams": [], - "name": "Ready", + "name": "Almost Ready", "desc": "" }, "model": "name.reviewresultname", - "pk": "ready" + "pk": "almost-ready" }, { "fields": { @@ -1915,11 +1936,11 @@ "order": 9, "used": true, "teams": [], - "name": "Serious Issues", + "name": "Ready", "desc": "" }, "model": "name.reviewresultname", - "pk": "serious-issues" + "pk": "ready" }, { "fields": { @@ -2001,16 +2022,6 @@ "model": "name.rolename", "pk": "execdir" }, -{ - "fields": { - "order": 3, - "used": true, - "name": "Incoming Area Director", - "desc": "" - }, - "model": "name.rolename", - "pk": "pre-ad" -}, { "fields": { "order": 3, @@ -2023,13 +2034,23 @@ }, { "fields": { - "order": 4, + "order": 3, "used": true, - "name": "Tech Advisor", + "name": "Incoming Area Director", "desc": "" }, "model": "name.rolename", - "pk": "techadv" + "pk": "pre-ad" +}, +{ + "fields": { + "order": 4, + "used": true, + "name": "Advisor", + "desc": "Advisor in a group that has explicit membership, such as the NomCom" + }, + "model": "name.rolename", + "pk": "advisor" }, { "fields": { @@ -2045,21 +2066,11 @@ "fields": { "order": 4, "used": true, - "name": "Advisor", - "desc": "Advisor in a group that has explicit membership, such as the NomCom" - }, - "model": "name.rolename", - "pk": "advisor" -}, -{ - "fields": { - "order": 5, - "used": true, - "name": "Editor", + "name": "Tech Advisor", "desc": "" }, "model": "name.rolename", - "pk": "editor" + "pk": "techadv" }, { "fields": { @@ -2073,13 +2084,13 @@ }, { "fields": { - "order": 6, + "order": 5, "used": true, - "name": "Secretary", + "name": "Editor", "desc": "" }, "model": "name.rolename", - "pk": "secr" + "pk": "editor" }, { "fields": { @@ -2091,6 +2102,16 @@ "model": "name.rolename", "pk": "delegate" }, +{ + "fields": { + "order": 6, + "used": true, + "name": "Secretary", + "desc": "" + }, + "model": "name.rolename", + "pk": "secr" +}, { "fields": { "order": 7, @@ -2155,21 +2176,21 @@ "fields": { "order": 0, "used": true, - "name": "LCD projector", - "desc": "The room will have a computer projector" + "name": "Boardroom Layout", + "desc": "Experimental room setup (boardroom and classroom) subject to availability" }, "model": "name.roomresourcename", - "pk": "project" + "pk": "boardroom" }, { "fields": { "order": 0, "used": true, - "name": "second LCD projector", - "desc": "The room will have a second computer projector" + "name": "LCD projector", + "desc": "The room will have a computer projector" }, "model": "name.roomresourcename", - "pk": "proj2" + "pk": "project" }, { "fields": { @@ -2185,31 +2206,11 @@ "fields": { "order": 0, "used": true, - "name": "Boardroom Layout", - "desc": "Experimental room setup (boardroom and classroom) subject to availability" + "name": "second LCD projector", + "desc": "The room will have a second computer projector" }, "model": "name.roomresourcename", - "pk": "boardroom" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Waiting for Scheduling", - "desc": "" - }, - "model": "name.sessionstatusname", - "pk": "schedw" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Waiting for Approval", - "desc": "" - }, - "model": "name.sessionstatusname", - "pk": "apprw" + "pk": "proj2" }, { "fields": { @@ -2221,16 +2222,6 @@ "model": "name.sessionstatusname", "pk": "appr" }, -{ - "fields": { - "order": 0, - "used": true, - "name": "Scheduled", - "desc": "" - }, - "model": "name.sessionstatusname", - "pk": "sched" -}, { "fields": { "order": 0, @@ -2241,6 +2232,16 @@ "model": "name.sessionstatusname", "pk": "canceled" }, +{ + "fields": { + "order": 0, + "used": true, + "name": "Deleted", + "desc": "" + }, + "model": "name.sessionstatusname", + "pk": "deleted" +}, { "fields": { "order": 0, @@ -2265,21 +2266,41 @@ "fields": { "order": 0, "used": true, - "name": "Deleted", + "name": "Scheduled", "desc": "" }, "model": "name.sessionstatusname", - "pk": "deleted" + "pk": "sched" }, { "fields": { "order": 0, "used": true, - "name": "Internet Standard", + "name": "Waiting for Approval", + "desc": "" + }, + "model": "name.sessionstatusname", + "pk": "apprw" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Waiting for Scheduling", + "desc": "" + }, + "model": "name.sessionstatusname", + "pk": "schedw" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Best Current Practice", "desc": "" }, "model": "name.stdlevelname", - "pk": "std" + "pk": "bcp" }, { "fields": { @@ -2295,11 +2316,21 @@ "fields": { "order": 0, "used": true, - "name": "Proposed Standard", + "name": "Experimental", "desc": "" }, "model": "name.stdlevelname", - "pk": "ps" + "pk": "exp" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Historic", + "desc": "" + }, + "model": "name.stdlevelname", + "pk": "hist" }, { "fields": { @@ -2315,31 +2346,21 @@ "fields": { "order": 0, "used": true, - "name": "Experimental", + "name": "Internet Standard", "desc": "" }, "model": "name.stdlevelname", - "pk": "exp" + "pk": "std" }, { "fields": { "order": 0, "used": true, - "name": "Best Current Practice", + "name": "Proposed Standard", "desc": "" }, "model": "name.stdlevelname", - "pk": "bcp" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Historic", - "desc": "" - }, - "model": "name.stdlevelname", - "pk": "hist" + "pk": "ps" }, { "fields": { @@ -2401,26 +2422,6 @@ "model": "name.streamname", "pk": "legacy" }, -{ - "fields": { - "order": 0, - "used": true, - "name": "Other", - "desc": "" - }, - "model": "name.timeslottypename", - "pk": "other" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Session", - "desc": "" - }, - "model": "name.timeslottypename", - "pk": "session" -}, { "fields": { "order": 0, @@ -2431,46 +2432,6 @@ "model": "name.timeslottypename", "pk": "break" }, -{ - "fields": { - "order": 0, - "used": true, - "name": "Registration", - "desc": "" - }, - "model": "name.timeslottypename", - "pk": "reg" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Plenary", - "desc": "" - }, - "model": "name.timeslottypename", - "pk": "plenary" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Room Unavailable", - "desc": "A room was not booked for the timeslot indicated" - }, - "model": "name.timeslottypename", - "pk": "unavail" -}, -{ - "fields": { - "order": 0, - "used": true, - "name": "Room Reserved", - "desc": "A room has been reserved for use by another body the timeslot indicated" - }, - "model": "name.timeslottypename", - "pk": "reserved" -}, { "fields": { "order": 0, @@ -2491,6 +2452,66 @@ "model": "name.timeslottypename", "pk": "offagenda" }, +{ + "fields": { + "order": 0, + "used": true, + "name": "Other", + "desc": "" + }, + "model": "name.timeslottypename", + "pk": "other" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Plenary", + "desc": "" + }, + "model": "name.timeslottypename", + "pk": "plenary" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Registration", + "desc": "" + }, + "model": "name.timeslottypename", + "pk": "reg" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Room Reserved", + "desc": "A room has been reserved for use by another body the timeslot indicated" + }, + "model": "name.timeslottypename", + "pk": "reserved" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Room Unavailable", + "desc": "A room was not booked for the timeslot indicated" + }, + "model": "name.timeslottypename", + "pk": "unavail" +}, +{ + "fields": { + "order": 0, + "used": true, + "name": "Session", + "desc": "" + }, + "model": "name.timeslottypename", + "pk": "session" +}, { "fields": { "label": "State" @@ -2631,6 +2652,13 @@ "model": "doc.statetype", "pk": "reuse_policy" }, +{ + "fields": { + "label": "Review" + }, + "model": "doc.statetype", + "pk": "review" +}, { "fields": { "used": true, @@ -4378,6 +4406,32 @@ "model": "doc.state", "pk": 142 }, +{ + "fields": { + "used": true, + "name": "Active", + "next_states": [], + "slug": "active", + "type": "review", + "order": 1, + "desc": "" + }, + "model": "doc.state", + "pk": 143 +}, +{ + "fields": { + "used": true, + "name": "Deleted", + "next_states": [], + "slug": "deleted", + "type": "review", + "order": 2, + "desc": "" + }, + "model": "doc.state", + "pk": 144 +}, { "fields": { "used": true, diff --git a/ietf/name/migrations/0012_insert_review_name_data.py b/ietf/name/migrations/0012_insert_review_name_data.py index 1ebbba69f..7cc1a7d52 100644 --- a/ietf/name/migrations/0012_insert_review_name_data.py +++ b/ietf/name/migrations/0012_insert_review_name_data.py @@ -15,25 +15,36 @@ def insert_initial_review_data(apps, schema_editor): ReviewRequestStateName.objects.get_or_create(slug="part-completed", name="Partially Completed", order=6) 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) ReviewResultName = apps.get_model("name", "ReviewResultName") - ReviewResultName.objects.get_or_create(slug="almost-ready", name="Almost Ready", order=1) + ReviewResultName.objects.get_or_create(slug="serious-issues", name="Serious Issues", order=1) ReviewResultName.objects.get_or_create(slug="issues", name="Has Issues", order=2) ReviewResultName.objects.get_or_create(slug="nits", name="Has Nits", order=3) + ReviewResultName.objects.get_or_create(slug="not-ready", name="Not Ready", order=4) ReviewResultName.objects.get_or_create(slug="right-track", name="On the Right Track", order=5) - ReviewResultName.objects.get_or_create(slug="ready", name="Ready", order=6) + ReviewResultName.objects.get_or_create(slug="almost-ready", name="Almost Ready", order=6) + ReviewResultName.objects.get_or_create(slug="ready-issues", name="Ready with Issues", order=7) ReviewResultName.objects.get_or_create(slug="ready-nits", name="Ready with Nits", order=8) - ReviewResultName.objects.get_or_create(slug="serious-issues", name="Serious Issues", order=9) + ReviewResultName.objects.get_or_create(slug="ready", name="Ready", order=9) RoleName = apps.get_model("name", "RoleName") - RoleName.objects.get_or_create(slug="reviewer", name="Reviewer", order=max(r.order for r in RoleName.objects.all()) + 1) + RoleName.objects.get_or_create(slug="reviewer", name="Reviewer", order=max(r.order for r in RoleName.objects.exclude(slug="reviewer")) + 1) + + DocTypeName = apps.get_model("name", "DocTypeName") + DocTypeName.objects.get_or_create(slug="review", name="Review") + + StateType = apps.get_model("doc", "StateType") + review_state_type, _ = StateType.objects.get_or_create(slug="review", label="Review") + + State = apps.get_model("doc", "State") + State.objects.get_or_create(type=review_state_type, slug="active", name="Active", order=1) + State.objects.get_or_create(type=review_state_type, slug="deleted", name="Deleted", order=2) def noop(apps, schema_editor): pass @@ -43,6 +54,7 @@ class Migration(migrations.Migration): dependencies = [ ('name', '0011_reviewrequeststatename_reviewresultname_reviewtypename'), ('group', '0001_initial'), + ('doc', '0001_initial'), ] operations = [ diff --git a/ietf/review/mailarch.py b/ietf/review/mailarch.py new file mode 100644 index 000000000..51f419fda --- /dev/null +++ b/ietf/review/mailarch.py @@ -0,0 +1,95 @@ +# various utilities for working with the mailarch mail archive at +# mailarchive.ietf.org + +import datetime, tarfile, mailbox, tempfile, hashlib, base64, email.utils +import urllib +import urllib2, contextlib + +from django.conf import settings + +def list_name_from_email(list_email): + if not list_email.endswith("@ietf.org"): + return None + + return list_email[:-len("@ietf.org")] + +def hash_list_message_id(list_name, msgid): + # hash in mailarch is computed similar to + # https://www.mail-archive.com/faq.html#listserver except the list + # name (without "@ietf.org") is used instead of the full address, + # and rightmost "=" signs are (optionally) stripped + sha = hashlib.sha1(msgid) + sha.update(list_name) + return base64.urlsafe_b64encode(sha.digest()).rstrip("=") + +def construct_query_urls(review_req, query=None): + list_name = list_name_from_email(review_req.team.list_email) + if not list_name: + return None + + if not query: + query = review_req.doc.name + + encoded_query = "?" + urllib.urlencode({ + "qdr": "c", # custom time frame + "start_date": (datetime.date.today() - datetime.timedelta(days=180)).isoformat(), + "email_list": list_name, + "q": "subject:({})".format(query), + "as": "1", # this is an advanced search + }) + + return { + "query": query, + "query_url": settings.MAILING_LIST_ARCHIVE_URL + "/arch/search/" + encoded_query, + "query_data_url": settings.MAILING_LIST_ARCHIVE_URL + "/arch/export/mbox/" + encoded_query, + } + +def construct_message_url(list_name, msgid): + return "{}/arch/msg/{}/{}".format(settings.MAILING_LIST_ARCHIVE_URL, list_name, hash_list_message_id(list_name, msgid)) + +def retrieve_messages_from_mbox(mbox_fileobj): + """Return selected content in message from mbox from mailarch.""" + res = [] + with tempfile.NamedTemporaryFile(suffix=".mbox") as mbox_file: + # mailbox.mbox needs a path, so we need to put the contents + # into a file + mbox_data = mbox_fileobj.read() + mbox_file.write(mbox_data) + mbox_file.flush() + + mbox = mailbox.mbox(mbox_file.name, create=False) + for msg in mbox: + content = u"" + + for part in msg.walk(): + if part.get_content_type() == "text/plain": + charset = part.get_content_charset() or "utf-8" + content += part.get_payload(decode=True).decode(charset, "ignore") + + res.append({ + "from": msg["From"], + "subject": msg["Subject"], + "content": content.replace("\r\n", "\n").replace("\r", "\n").strip("\n"), + "message_id": email.utils.unquote(msg["Message-ID"]), + "url": email.utils.unquote(msg["Archived-At"]), + "date": msg["Date"], + }) + + return res + +def retrieve_messages(query_data_url): + """Retrieve and return selected content from mailarch.""" + res = [] + + with contextlib.closing(urllib2.urlopen(query_data_url, timeout=15)) as fileobj: + content_type = fileobj.info()["Content-type"] + if not content_type.startswith("application/x-tar"): + raise Exception("Export failed - this usually means no matches were found") + + with tarfile.open(fileobj=fileobj, mode='r|*') as tar: + for entry in tar: + if entry.isfile(): + mbox_fileobj = tar.extractfile(entry) + res.extend(retrieve_messages_from_mbox(mbox_fileobj)) + + return res diff --git a/ietf/review/resources.py b/ietf/review/resources.py new file mode 100644 index 000000000..dff2da8ae --- /dev/null +++ b/ietf/review/resources.py @@ -0,0 +1,65 @@ +# Autogenerated by the makeresources management command 2016-06-14 04:21 PDT +from tastypie.resources import ModelResource +from tastypie.fields import ToManyField # pyflakes:ignore +from tastypie.constants import ALL, ALL_WITH_RELATIONS # pyflakes:ignore +from tastypie.cache import SimpleCache + +from ietf import api +from ietf.api import ToOneField # pyflakes:ignore + +from ietf.review.models import * # pyflakes:ignore + + +from ietf.person.resources import PersonResource +from ietf.group.resources import GroupResource +class ReviewerResource(ModelResource): + team = ToOneField(GroupResource, 'team') + person = ToOneField(PersonResource, 'person') + class Meta: + queryset = Reviewer.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewer' + filtering = { + "id": ALL, + "frequency": ALL, + "unavailable_until": ALL, + "filter_re": ALL, + "skip_next": ALL, + "team": ALL_WITH_RELATIONS, + "person": ALL_WITH_RELATIONS, + } +api.review.register(ReviewerResource()) + +from ietf.doc.resources import DocumentResource +from ietf.group.resources import RoleResource, GroupResource +from ietf.name.resources import ReviewRequestStateNameResource, ReviewResultNameResource, ReviewTypeNameResource +class ReviewRequestResource(ModelResource): + state = ToOneField(ReviewRequestStateNameResource, 'state') + type = ToOneField(ReviewTypeNameResource, 'type') + doc = ToOneField(DocumentResource, 'doc') + team = ToOneField(GroupResource, 'team') + reviewer = ToOneField(RoleResource, 'reviewer', null=True) + review = ToOneField(DocumentResource, 'review', null=True) + result = ToOneField(ReviewResultNameResource, 'result', null=True) + class Meta: + queryset = ReviewRequest.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewrequest' + filtering = { + "id": ALL, + "time": ALL, + "deadline": ALL, + "requested_rev": ALL, + "reviewed_rev": ALL, + "state": ALL_WITH_RELATIONS, + "type": ALL_WITH_RELATIONS, + "doc": ALL_WITH_RELATIONS, + "team": ALL_WITH_RELATIONS, + "reviewer": ALL_WITH_RELATIONS, + "review": ALL_WITH_RELATIONS, + "result": ALL_WITH_RELATIONS, + } +api.review.register(ReviewRequestResource()) + diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 8cc692d35..7da5e1b6b 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -3,7 +3,7 @@ from django.contrib.sites.models import Site from ietf.group.models import Group, Role from ietf.doc.models import DocEvent from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream -from ietf.review.models import ReviewRequestStateName +from ietf.review.models import ReviewRequestStateName, ReviewRequest from ietf.utils.mail import send_mail def active_review_teams(): @@ -22,6 +22,17 @@ def can_manage_review_requests_for_team(user, team): return Role.objects.filter(name__in=["secretary", "delegate"], person__user=user, group=team).exists() or has_role(user, "Secretariat") +def make_new_review_request_from_existing(review_req): + obj = ReviewRequest() + obj.time = review_req.time + obj.type = review_req.type + obj.doc = review_req.doc + obj.team = review_req.team + obj.deadline = review_req.deadline + obj.requested_rev = review_req.requested_rev + obj.state = ReviewRequestStateName.objects.get(slug="requested") + return obj + def email_about_review_request(request, review_req, subject, msg, by, notify_secretary, notify_reviewer): """Notify possibly both secretary and reviewer about change, skipping a party if the change was done by that party.""" @@ -48,7 +59,6 @@ def email_about_review_request(request, review_req, subject, msg, by, notify_sec "msg": msg, }) - def assign_review_request_to_reviewer(request, review_req, reviewer): assert review_req.state_id in ("requested", "accepted") diff --git a/ietf/settings.py b/ietf/settings.py index ec91d6fb0..5e21a2873 100644 --- a/ietf/settings.py +++ b/ietf/settings.py @@ -404,6 +404,7 @@ MEETING_RECORDINGS_DIR = '/a/www/audio' # Mailing list info URL for lists hosted on the IETF servers MAILING_LIST_INFO_URL = "https://www.ietf.org/mailman/listinfo/%(list_addr)s" +MAILING_LIST_ARCHIVE_URL = "https://mailarchive.ietf.org" # Liaison Statement Tool settings (one is used in DOC_HREFS below) LIAISON_UNIVERSAL_FROM = 'Liaison Statement Management Tool ' diff --git a/ietf/static/ietf/css/ietf.css b/ietf/static/ietf/css/ietf.css index 2aacf362b..cc433f0b2 100644 --- a/ietf/static/ietf/css/ietf.css +++ b/ietf/static/ietf/css/ietf.css @@ -459,3 +459,16 @@ label#list-feeds { .email-subscription button[type=submit] { margin-left: 3em; } + +/* Review flow */ + +form.complete-review .mail-archive-search .query-input { + width: 30em; +} + +form.complete-review .mail-archive-search .results .list-group { + margin-left: 1em; + margin-right: 1em; + margin-bottom: 0.5em; +} + diff --git a/ietf/static/ietf/js/complete-review.js b/ietf/static/ietf/js/complete-review.js new file mode 100644 index 000000000..e810f40a2 --- /dev/null +++ b/ietf/static/ietf/js/complete-review.js @@ -0,0 +1,131 @@ +$(document).ready(function () { + var form = $("form.complete-review"); + + var reviewedRev = form.find("[name=reviewed_rev]"); + reviewedRev.closest(".form-group").find("a.rev").on("click", function (e) { + e.preventDefault(); + reviewedRev.val($(this).text()); + }); + + // mail archive search functionality + var mailArchiveSearchTemplate = form.find(".template .mail-archive-search").parent().html(); + var mailArchiveSearchResultTemplate = form.find(".template .mail-archive-search-result").parent().html(); + + form.find("[name=review_url]").closest(".form-group").before(mailArchiveSearchTemplate); + + var mailArchiveSearch = form.find(".mail-archive-search"); + + var retrievingData = null; + + function searchMailArchive() { + if (retrievingData) + return; + + var queryInput = mailArchiveSearch.find(".query-input"); + if (queryInput.length == 0 || !$.trim(queryInput.val())) + return; + + mailArchiveSearch.find(".search").prop("disabled", true); + mailArchiveSearch.find(".error").addClass("hidden"); + mailArchiveSearch.find(".retrieving").removeClass("hidden"); + mailArchiveSearch.find(".results").addClass("hidden"); + + retrievingData = $.ajax({ + url: searchMailArchiveUrl, + method: "GET", + data: { + query: queryInput.val() + }, + dataType: "json", + timeout: 20 * 1000 + }).then(function (data) { + retrievingData = null; + mailArchiveSearch.find(".search").prop("disabled", false); + mailArchiveSearch.find(".retrieving").addClass("hidden"); + + var err = data.error; + if (!err && (!data.messages || !data.messages.length)) + err = "No messages matching document name found in archive"; + + if (err) { + var errorDiv = mailArchiveSearch.find(".error"); + errorDiv.removeClass("hidden"); + errorDiv.find(".content").text(err); + if (data.query && data.query_url && data.query_data_url) { + errorDiv.find(".try-yourself .query").text(data.query); + errorDiv.find(".try-yourself .query-url").prop("href", data.query_url); + errorDiv.find(".try-yourself .query-data-url").prop("href", data.query_data_url); + errorDiv.find(".try-yourself").removeClass("hidden"); + } + } + else { + mailArchiveSearch.find(".results").removeClass("hidden"); + + var results = mailArchiveSearch.find(".results .list-group"); + results.children().remove(); + + for (var i = 0; i < data.messages.length; ++i) { + var msg = data.messages[i]; + var row = $(mailArchiveSearchResultTemplate).attr("title", "Click to fill in link and content from this message"); + row.find(".subject").text(msg.subject); + row.find(".date").text(msg.date); + row.data("url", msg.url); + row.data("content", msg.content); + results.append(row); + } + } + }, function () { + retrievingData = null; + mailArchiveSearch.find(".search").prop("disabled", false); + mailArchiveSearch.find(".retrieving").addClass("hidden"); + + var errorDiv = mailArchiveSearch.find(".error"); + errorDiv.removeClass("hidden"); + errorDiv.find(".content").text("Error trying to retrieve data from mailing list archive."); + }); + } + + mailArchiveSearch.find(".search").on("click", function () { + searchMailArchive(); + }); + + mailArchiveSearch.find(".results").on("click", ".mail-archive-search-result", function (e) { + e.preventDefault(); + + var row = $(this); + if (!row.is(".mail-archive-search-result")) + row = row.closest(".mail-archive-search-result"); + + form.find("[name=review_url]").val(row.data("url")); + form.find("[name=review_content]").val(row.data("content")); + }); + + + // review submission selection + form.find("[name=review_submission]").on("click change", function () { + var val = form.find("[name=review_submission]:checked").val(); + + var shouldBeVisible = { + "enter": ['[name="review_content"]'], + "upload": ['[name="review_file"]'], + "link": [".mail-archive-search", '[name="review_url"]', '[name="review_content"]'] + }; + + for (var v in shouldBeVisible) { + for (var i in shouldBeVisible[v]) { + var selector = shouldBeVisible[v][i]; + var row = form.find(selector); + if (!row.is(".form-group")) + row = row.closest(".form-group"); + + if ($.inArray(selector, shouldBeVisible[val]) != -1) + row.show(); + else + row.hide(); + } + } + + if (val == "link") + searchMailArchive(); + }).trigger("change"); +}); diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 853d6fcc6..d05e60555 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -200,7 +200,7 @@ {% for r in review_requests %} {% endfor %} diff --git a/ietf/templates/doc/mail/completed_review.txt b/ietf/templates/doc/mail/completed_review.txt new file mode 100644 index 000000000..b3ca9d2f4 --- /dev/null +++ b/ietf/templates/doc/mail/completed_review.txt @@ -0,0 +1,6 @@ +{% autoescape off %}{% filter wordwrap:70 %}{% if review_req.state_id == "part-completed" %}Review is partially done. Another review request has been registered for completing it. + +{% endif %}Reviewer: {{ review_req.reviewer.person }} + +{{ content }} +{% endfilter %}{% endautoescape %} diff --git a/ietf/templates/doc/mail/partially_completed_review.txt b/ietf/templates/doc/mail/partially_completed_review.txt new file mode 100644 index 000000000..3e1661e55 --- /dev/null +++ b/ietf/templates/doc/mail/partially_completed_review.txt @@ -0,0 +1,6 @@ +{% autoescape off %}Review was partially completed by {{ by }}. + +A new review request has been registered for completing the review: + +https://{{ domain }}{% url "ietf.doc.views_review.review_request" name=new_review_req.doc.name request_id=new_review_req.pk %} +{% endautoescape %} diff --git a/ietf/templates/doc/mail/reviewer_assignment_rejected.txt b/ietf/templates/doc/mail/reviewer_assignment_rejected.txt new file mode 100644 index 000000000..001de69b3 --- /dev/null +++ b/ietf/templates/doc/mail/reviewer_assignment_rejected.txt @@ -0,0 +1,6 @@ +{% autoescape off %}Reviewer assignment rejected by {{ by }}.{% if message_to_secretary %} + +Explanation: + +{{ message_to_secretary }} +{% endif %}{% endautoescape %} diff --git a/ietf/templates/doc/review/complete_review.html b/ietf/templates/doc/review/complete_review.html new file mode 100644 index 000000000..3dca99e6a --- /dev/null +++ b/ietf/templates/doc/review/complete_review.html @@ -0,0 +1,81 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Complete review of {{ review_req.doc.name }}{% endblock %} + +{% block content %} + {% origin %} +

Complete review
{{ review_req.doc.name }}

+ +

The review findings should be made available here and the review + posted to the mailing list. If you enter the findings below, the + system will post the review for you. If you already have posted + the review, you can try to let the system find a link to the + archive and retrieve the email body.

+ + + {% csrf_token %} + + {% bootstrap_form form layout="horizontal" %} + + {% buttons %} + Cancel + + {% endbuttons %} + + + + + + + +{% endblock %} + +{% block js %} + + +{% endblock %} diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index aec78a6c1..e2c137dd4 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -19,6 +19,10 @@ {% else %} {{ review_req.doc.name }} {% endif %} + + {% if can_withdraw_request %} + Withdraw request + {% endif %} @@ -60,43 +64,51 @@ {{ review_req.state.name }} - - - Reviewer - - {% if review_req.reviewer %} - {{ review_req.reviewer.person }} - {% else %} - None assigned yet - {% endif %} + + + Reviewer + + {% if review_req.reviewer %} + {{ review_req.reviewer.person }} + {% else %} + None assigned yet + {% endif %} - {% if can_accept_reviewer_assignment %} -
{% csrf_token %}
- {% endif %} + {% if can_accept_reviewer_assignment %} +
{% csrf_token %}
+ {% endif %} - {% if can_reject_reviewer_assignment %} - Reject - {% endif %} + {% if can_reject_reviewer_assignment %} + Reject + {% endif %} - {% if can_assign_reviewer %} - {% if review_req.reviewer %}Reassign{% else %}Assign{% endif %} reviewer - {% endif %} - - + {% if can_assign_reviewer %} + {% if review_req.reviewer %}Reassign{% else %}Assign{% endif %} reviewer + {% endif %} + + - {% if review_req.review %} - - - Review - {{ review_req.review.name }} - - {% endif %} + + + Review + + {% if review_req.review %} + {{ review_req.review.name }} + {% else %} + Not completed yet + {% endif %} + + {% if can_complete_review %} + Complete review + {% endif %} + + {% if review_req.reviewed_rev %} Reviewed revision - {{ review_req.reviewed_rev }} + {{ review_req.reviewed_rev }} {% if review_req.reviewed_rev != review_req.doc.rev %}(currently at {{ review_req.doc.rev }}){% endif %} {% endif %} @@ -110,10 +122,4 @@ -
- {% if can_withdraw_request %} - Withdraw request - {% endif %} -
- {% endblock %} diff --git a/ietf/utils/text.py b/ietf/utils/text.py new file mode 100644 index 000000000..39df9a136 --- /dev/null +++ b/ietf/utils/text.py @@ -0,0 +1,11 @@ +def skip_prefix(text, prefix): + if text.startswith(prefix): + return text[len(prefix):] + else: + return text + +def skip_suffix(text, prefix): + if text.endswith(prefix): + return text[:-len(prefix)] + else: + return text diff --git a/ietf/utils/textupload.py b/ietf/utils/textupload.py index 1a4dbe705..7456825a1 100644 --- a/ietf/utils/textupload.py +++ b/ietf/utils/textupload.py @@ -1,18 +1,18 @@ import re -import django.forms +from django.core.exceptions import ValidationError def get_cleaned_text_file_content(uploaded_file): """Read uploaded file, try to fix up encoding to UTF-8 and transform line endings into Unix style, then return the content as a UTF-8 string. Errors are reported as - django.forms.ValidationError exceptions.""" + django.core.exceptions.ValidationError exceptions.""" if not uploaded_file: return u"" if uploaded_file.size and uploaded_file.size > 10 * 1000 * 1000: - raise django.forms.ValidationError("Text file too large (size %s)." % uploaded_file.size) + raise ValidationError("Text file too large (size %s)." % uploaded_file.size) content = "".join(uploaded_file.chunks()) @@ -29,18 +29,18 @@ def get_cleaned_text_file_content(uploaded_file): filetype = m.from_buffer(content) if not filetype.startswith("text"): - raise django.forms.ValidationError("Uploaded file does not appear to be a text file.") + raise ValidationError("Uploaded file does not appear to be a text file.") match = re.search("charset=([\w-]+)", filetype) if not match: - raise django.forms.ValidationError("File has unknown encoding.") + raise ValidationError("File has unknown encoding.") encoding = match.group(1) if "ascii" not in encoding: try: content = content.decode(encoding) except Exception as e: - raise django.forms.ValidationError("Error decoding file (%s). Try submitting with UTF-8 encoding or remove non-ASCII characters." % str(e)) + raise ValidationError("Error decoding file (%s). Try submitting with UTF-8 encoding or remove non-ASCII characters." % str(e)) # turn line-endings into Unix style content = content.replace("\r\n", "\n").replace("\r", "\n") From 6a35431356b1eb22d3f9f75b9d4440e8f13ab3ed Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 14 Jun 2016 12:38:43 +0000 Subject: [PATCH 14/16] Remember to create a DocAlias when creating a review document - Legacy-Id: 11361 --- ietf/doc/views_review.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index f5c3226b8..b4642049f 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -9,7 +9,7 @@ from django.utils.html import mark_safe from django.core.exceptions import ValidationError from django.template.loader import render_to_string -from ietf.doc.models import Document, NewRevisionDocEvent, DocEvent, State +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 @@ -396,6 +396,7 @@ def complete_review(request, name, request_id): review.external_url = form.cleaned_data['review_url'] review.save() review.set_state(State.objects.get(type="review", slug="active")) + DocAlias.objects.create(document=review, name=review.name) NewRevisionDocEvent.objects.create( type="new_revision", From 0ed3d554d60570b3db7b280e50b0f0ff037ad140 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 14 Jun 2016 12:48:08 +0000 Subject: [PATCH 15/16] Add simple /doc/review- page for displaying a review, fix a couple of bugs - Legacy-Id: 11362 --- ietf/doc/views_doc.py | 15 +++++ ietf/doc/views_review.py | 6 +- ietf/templates/doc/document_review.html | 73 +++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 ietf/templates/doc/document_review.html diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 23d430a72..2f83b4529 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -571,6 +571,21 @@ def document_main(request, name, rev=None): ), context_instance=RequestContext(request)) + + if doc.type_id == "review": + basename = "{}-{}.txt".format(doc.name, doc.rev) + pathname = os.path.join(doc.get_file_path(), basename) + content = get_document_content(basename, pathname, split=False) + + return render(request, "doc/document_review.html", + dict(doc=doc, + top=top, + content=content, + revisions=revisions, + latest_rev=latest_rev, + snapshot=snapshot, + )) + raise Http404 diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index b4642049f..6c852e396 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -390,7 +390,7 @@ def complete_review(request, name, request_id): review.type = DocTypeName.objects.get(slug="review") review.rev = "00" - review.title = "Review of {}".format(review_req.doc.name) + review.title = "Review of {}-{}".format(review_req.doc.name, review_req.reviewed_rev) review.group = review_req.team if review_submission == "link": review.external_url = form.cleaned_data['review_url'] @@ -402,9 +402,9 @@ def complete_review(request, name, request_id): type="new_revision", doc=review, by=request.user.person, - rev=doc.rev, + rev=review.rev, desc='New revision available', - time=doc.time, + time=review.time, ) # save file on disk diff --git a/ietf/templates/doc/document_review.html b/ietf/templates/doc/document_review.html new file mode 100644 index 000000000..2c4362771 --- /dev/null +++ b/ietf/templates/doc/document_review.html @@ -0,0 +1,73 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin %} +{% load staticfiles %} +{% load ietf_filters %} + +{% block title %}{{ doc.title }}{% endblock %} + +{% block content %} + {% origin %} + {{ top|safe }} + + {% include "doc/revisions_list.html" %} + + + + + {% if doc.rev != latest_rev %} + + {% else %} + + {% endif %} + + + + + + + + + + + + + + + + + + + + + + + {% if doc.external_url %} + + + + + + {% endif %} + + + + + + + +
The information below is for an old version of the document
Team + {{ doc.group.name }} + ({{ doc.group.acronym }}) + + {% if snapshot %} + Snapshot + {% endif %} +
Title{{ doc.title }}
State{{ doc.get_state.name }}
Posted at{{ doc.external_url }}
Last updated{{ doc.time|date:"Y-m-d" }}
+ +

{{ doc.type.name }}
{{ doc.name }}

+ + {% if doc.rev and content != None %} + {{ content|fill:"80"|safe|linebreaksbr|keep_spacing|sanitize_html|safe }} + {% endif %} +{% endblock %} From 7a406bafc6f9d80f97ceb703fd62d075f94f96a8 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Tue, 14 Jun 2016 13:11:06 +0000 Subject: [PATCH 16/16] Polish the various review pages, adding bits of information here and there - Legacy-Id: 11363 --- ietf/doc/views_doc.py | 3 +++ ietf/templates/doc/document_draft.html | 6 +++++- ietf/templates/doc/document_review.html | 8 ++++++++ ietf/templates/doc/mail/completed_review.txt | 1 + ietf/templates/doc/review/review_request.html | 14 ++++++++++++-- 5 files changed, 29 insertions(+), 3 deletions(-) diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index 2f83b4529..c1f0a6cb2 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -577,6 +577,8 @@ def document_main(request, name, rev=None): pathname = os.path.join(doc.get_file_path(), basename) content = get_document_content(basename, pathname, split=False) + review_req = ReviewRequest.objects.filter(review=doc.name).first() + return render(request, "doc/document_review.html", dict(doc=doc, top=top, @@ -584,6 +586,7 @@ def document_main(request, name, rev=None): revisions=revisions, latest_rev=latest_rev, snapshot=snapshot, + review_req=review_req, )) raise Http404 diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index d05e60555..3a0cd68aa 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -200,7 +200,11 @@ {% for r in review_requests %} {% endfor %} diff --git a/ietf/templates/doc/document_review.html b/ietf/templates/doc/document_review.html index 2c4362771..d0900954e 100644 --- a/ietf/templates/doc/document_review.html +++ b/ietf/templates/doc/document_review.html @@ -49,6 +49,14 @@ {{ doc.get_state.name }} + {% if review_req %} + + Review result + + {{ review_req.result.name }} + + {% endif %} + {% if doc.external_url %} Posted at diff --git a/ietf/templates/doc/mail/completed_review.txt b/ietf/templates/doc/mail/completed_review.txt index b3ca9d2f4..7d81d628f 100644 --- a/ietf/templates/doc/mail/completed_review.txt +++ b/ietf/templates/doc/mail/completed_review.txt @@ -1,6 +1,7 @@ {% autoescape off %}{% filter wordwrap:70 %}{% if review_req.state_id == "part-completed" %}Review is partially done. Another review request has been registered for completing it. {% endif %}Reviewer: {{ review_req.reviewer.person }} +Review result: {{ review_req.result.name }} {{ content }} {% endfilter %}{% endautoescape %} diff --git a/ietf/templates/doc/review/review_request.html b/ietf/templates/doc/review/review_request.html index e2c137dd4..d625a7906 100644 --- a/ietf/templates/doc/review/review_request.html +++ b/ietf/templates/doc/review/review_request.html @@ -104,10 +104,20 @@ + {% if review_req.review and review_req.review.external_url %} + + + Posted at + + {{ review_req.review.external_url }} + + + {% endif %} + {% if review_req.reviewed_rev %} - Reviewed revision + Reviewed rev. {{ review_req.reviewed_rev }} {% if review_req.reviewed_rev != review_req.doc.rev %}(currently at {{ review_req.doc.rev }}){% endif %} {% endif %} @@ -115,7 +125,7 @@ {% if review_req.result %} - Result of review + Review result {{ review_req.result.name }} {% endif %}