From b24bdb5bc359579f70cbc3dcb48751897e6ce1eb Mon Sep 17 00:00:00 2001 From: Robert Sparks Date: Wed, 14 Dec 2016 23:26:32 +0000 Subject: [PATCH] Slight refactor of the review models to capture review team settings. Allows configuring review teams to get automatic suggestions for reviews or not. Provides a better admin for creating/managing review teams. Fixes #2048 and #2072. Commit ready for merge. - Legacy-Id: 12520 --- ietf/doc/tests_review.py | 19 +++++------ ietf/doc/views_review.py | 8 ++--- ietf/review/admin.py | 10 +++++- ietf/review/import_from_review_tool.py | 8 ++--- .../migrations/0007_reviewteamsettings.py | 31 +++++++++++++++++ .../0008_populate_reviewteamsettings.py | 29 ++++++++++++++++ ietf/review/models.py | 34 +++++++++++++++---- ietf/review/resources.py | 17 +++++++++- ietf/review/utils.py | 14 ++++---- ietf/utils/test_data.py | 8 ++--- 10 files changed, 141 insertions(+), 37 deletions(-) create mode 100644 ietf/review/migrations/0007_reviewteamsettings.py create mode 100644 ietf/review/migrations/0008_populate_reviewteamsettings.py diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index 9889b8a82..a8b9ebf6d 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -12,7 +12,7 @@ from pyquery import PyQuery import debug # pyflakes:ignore -from ietf.review.models import (ReviewRequest, ResultUsedInReviewTeam, ReviewerSettings, +from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewWish, UnavailablePeriod, NextReviewerInTeam) from ietf.review.utils import reviewer_rotation_list, possibly_advance_next_reviewer_for_team import ietf.review.mailarch @@ -479,8 +479,7 @@ class ReviewTests(TestCase): review_req.state = ReviewRequestStateName.objects.get(slug="accepted") review_req.save() for r in ReviewResultName.objects.filter(slug__in=("issues", "ready")): - ResultUsedInReviewTeam.objects.get_or_create(team=review_req.team, result=r) - review_req.team.save() + review_req.team.reviewteamsettings.review_results.add(r) url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": doc.name, "request_id": review_req.pk }) @@ -517,7 +516,7 @@ class ReviewTests(TestCase): test_file.name = "unnamed" r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "upload", @@ -560,7 +559,7 @@ class ReviewTests(TestCase): empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", @@ -590,7 +589,7 @@ class ReviewTests(TestCase): empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "link", @@ -618,7 +617,7 @@ class ReviewTests(TestCase): empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="part-completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", @@ -652,7 +651,7 @@ class ReviewTests(TestCase): url = urlreverse('ietf.doc.views_review.complete_review', kwargs={ "name": review_req.doc.name, "request_id": review_req.pk }) r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", @@ -677,7 +676,7 @@ class ReviewTests(TestCase): empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", @@ -702,7 +701,7 @@ class ReviewTests(TestCase): # revise again empty_outbox() r = self.client.post(url, data={ - "result": ReviewResultName.objects.get(resultusedinreviewteam__team=review_req.team, slug="ready").pk, + "result": ReviewResultName.objects.get(reviewteamsettings__group=review_req.team, slug="ready").pk, "state": ReviewRequestStateName.objects.get(slug="part-completed").pk, "reviewed_rev": review_req.doc.rev, "review_submission": "enter", diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index f0c9f7939..8be966bf7 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -14,7 +14,7 @@ from django.core.urlresolvers import reverse as urlreverse from ietf.doc.models import (Document, NewRevisionDocEvent, State, DocAlias, LastCallDocEvent, ReviewRequestDocEvent) from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName -from ietf.review.models import ReviewRequest, TypeUsedInReviewTeam +from ietf.review.models import ReviewRequest from ietf.group.models import Group from ietf.person.fields import PersonEmailChoiceField, SearchablePersonField from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has_role @@ -57,7 +57,7 @@ class RequestReviewForm(forms.ModelForm): f.queryset = active_review_teams() f.initial = [group.pk for group in f.queryset if can_manage_review_requests_for_team(user, group, allow_personnel_outside_team=False)] - self.fields['type'].queryset = self.fields['type'].queryset.filter(used=True, typeusedinreviewteam__team__in=self.fields["team"].queryset).distinct() + self.fields['type'].queryset = self.fields['type'].queryset.filter(used=True, reviewteamsettings__group__in=self.fields["team"].queryset).distinct() self.fields['type'].widget = forms.RadioSelect(choices=[t for t in self.fields['type'].choices if t[0]]) self.fields["requested_rev"].label = "Document revision" @@ -83,7 +83,7 @@ class RequestReviewForm(forms.ModelForm): if chosen_type and chosen_teams: for t in chosen_teams: - if not TypeUsedInReviewTeam.objects.filter(type=chosen_type, team=t).exists(): + if chosen_type not in t.reviewteamsettings.review_types.all(): self.add_error("type", "{} does not use the review type {}.".format(t.name, chosen_type.name)) return self.cleaned_data @@ -361,7 +361,7 @@ class CompleteReviewForm(forms.Form): " ".join("{}".format(r) for r in known_revisions)) - self.fields["result"].queryset = self.fields["result"].queryset.filter(resultusedinreviewteam__team=review_req.team) + self.fields["result"].queryset = self.fields["result"].queryset.filter(reviewteamsettings__group=review_req.team) def format_submission_choice(label): if revising_review: diff --git a/ietf/review/admin.py b/ietf/review/admin.py index f6bff5b9c..fd28686b8 100644 --- a/ietf/review/admin.py +++ b/ietf/review/admin.py @@ -2,7 +2,7 @@ from django.contrib import admin from ietf.review.models import (ReviewerSettings, UnavailablePeriod, ReviewWish, ResultUsedInReviewTeam, TypeUsedInReviewTeam, NextReviewerInTeam, - ReviewRequest) + ReviewRequest, ReviewTeamSettings ) class ReviewerSettingsAdmin(admin.ModelAdmin): def acronym(self, obj): @@ -72,3 +72,11 @@ class ReviewRequestAdmin(admin.ModelAdmin): search_fields = ["doc__name", "reviewer__person__name"] admin.site.register(ReviewRequest, ReviewRequestAdmin) + +class ReviewTeamSettingsAdmin(admin.ModelAdmin): + list_display = ["group", ] + search_fields = ["group__acronym", ] + raw_id_fields = ["group", ] + filter_horizontal = ["review_types", "review_results", ] + +admin.site.register(ReviewTeamSettings, ReviewTeamSettingsAdmin) diff --git a/ietf/review/import_from_review_tool.py b/ietf/review/import_from_review_tool.py index 01fc50b3d..fcc89634b 100755 --- a/ietf/review/import_from_review_tool.py +++ b/ietf/review/import_from_review_tool.py @@ -16,8 +16,8 @@ django.setup() import datetime, re, itertools from collections import namedtuple from django.db import connections -from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewResultName, ResultUsedInReviewTeam, - ReviewRequestStateName, ReviewTypeName, TypeUsedInReviewTeam, +from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewResultName, + ReviewRequestStateName, ReviewTypeName, UnavailablePeriod, NextReviewerInTeam) from ietf.group.models import Group, Role, RoleName from ietf.person.models import Person, Email, Alias @@ -190,10 +190,10 @@ with db_con.cursor() as c: summaries = [v.strip().lower() for v in row.value.split(";") if v.strip()] for s in summaries: - ResultUsedInReviewTeam.objects.get_or_create(team=team, result=results[s]) + team.reviewteamsettings.review_results.add(results[s]) for t in ReviewTypeName.objects.filter(slug__in=["early", "lc", "telechat"]): - TypeUsedInReviewTeam.objects.get_or_create(team=team, type=t) + team.reviewteamsettings.review_types.add(t) # review requests diff --git a/ietf/review/migrations/0007_reviewteamsettings.py b/ietf/review/migrations/0007_reviewteamsettings.py new file mode 100644 index 000000000..14e5cd5f9 --- /dev/null +++ b/ietf/review/migrations/0007_reviewteamsettings.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +import ietf.review.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('name', '0016_auto_20161013_1010'), + ('group', '0009_auto_20150930_0758'), + ('review', '0006_auto_20161209_0436'), + ] + + operations = [ + migrations.CreateModel( + name='ReviewTeamSettings', + fields=[ + ('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)), + ('autosuggest', models.BooleanField(default=True, verbose_name=b'Automatically suggest possible review requests')), + ('group', models.OneToOneField(to='group.Group')), + ('review_results', models.ManyToManyField(default=ietf.review.models.get_default_review_results, to='name.ReviewResultName')), + ('review_types', models.ManyToManyField(default=ietf.review.models.get_default_review_types, to='name.ReviewTypeName')), + ], + options={ + 'verbose_name': 'Review team settings', + 'verbose_name_plural': 'Review team settings', + }, + ), + ] diff --git a/ietf/review/migrations/0008_populate_reviewteamsettings.py b/ietf/review/migrations/0008_populate_reviewteamsettings.py new file mode 100644 index 000000000..098fbb00f --- /dev/null +++ b/ietf/review/migrations/0008_populate_reviewteamsettings.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations + +def forward(apps, schema_editor): + ReviewTeamSettings = apps.get_model('review','ReviewTeamSettings') + ResultUsedInReviewTeam = apps.get_model('review','ResultUsedInReviewTeam') + TypeUsedInReviewTeam = apps.get_model('review','TypeUsedInReviewTeam') + + for group_id in ResultUsedInReviewTeam.objects.values_list('team',flat=True).distinct(): + rts = ReviewTeamSettings.objects.create(group_id=group_id) + rts.review_types = TypeUsedInReviewTeam.objects.filter(team_id=group_id).values_list('type',flat=True).distinct() + rts.review_results = ResultUsedInReviewTeam.objects.filter(team_id=group_id).values_list('result',flat=True).distinct() + + +def reverse(apps, schema_editor): + ReviewTeamSettings = apps.get_model('review','ReviewTeamSettings') + ReviewTeamSettings.objects.all().delete() + +class Migration(migrations.Migration): + + dependencies = [ + ('review', '0007_reviewteamsettings'), + ] + + operations = [ + migrations.RunPython(forward, reverse) + ] diff --git a/ietf/review/models.py b/ietf/review/models.py index c17f61500..56073b7ce 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -10,7 +10,7 @@ from ietf.utils.validators import validate_regular_expression_string class ReviewerSettings(models.Model): """Keeps track of admin data associated with a reviewer in a team.""" - team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None)) person = models.ForeignKey(Person) INTERVALS = [ (7, "Once per week"), @@ -34,7 +34,7 @@ class ReviewerSettings(models.Model): class ReviewSecretarySettings(models.Model): """Keeps track of admin data associated with a secretary in a team.""" - team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None)) person = models.ForeignKey(Person) remind_days_before_deadline = models.IntegerField(null=True, blank=True, help_text="To get an email reminder in case a reviewer forgets to do an assigned review, enter the number of days before review deadline you want to receive it. Clear the field if you don't want a reminder.") @@ -45,7 +45,7 @@ class ReviewSecretarySettings(models.Model): verbose_name_plural = "review secretary settings" class UnavailablePeriod(models.Model): - team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None)) person = models.ForeignKey(Person) start_date = models.DateField(default=datetime.date.today, null=True, help_text="Choose the start date so that you can still do a review if it's assigned just before the start date - this usually means you should mark yourself unavailable for assignment some time before you are actually away.") end_date = models.DateField(blank=True, null=True, help_text="Leaving the end date blank means that the period continues indefinitely. You can end it later.") @@ -76,7 +76,7 @@ class UnavailablePeriod(models.Model): class ReviewWish(models.Model): """Reviewer wishes to review a document when it becomes available for review.""" time = models.DateTimeField(default=datetime.datetime.now) - team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None)) person = models.ForeignKey(Person) doc = models.ForeignKey(Document) @@ -104,7 +104,7 @@ class ResultUsedInReviewTeam(models.Model): class TypeUsedInReviewTeam(models.Model): """Captures that a type name is valid for a given team for new reviews. """ - team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None)) type = models.ForeignKey(ReviewTypeName) def __unicode__(self): @@ -115,7 +115,7 @@ class TypeUsedInReviewTeam(models.Model): verbose_name_plural = "review type used in team settings" class NextReviewerInTeam(models.Model): - team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None)) next_reviewer = models.ForeignKey(Person) def __unicode__(self): @@ -138,7 +138,7 @@ class ReviewRequest(models.Model): time = models.DateTimeField(default=datetime.datetime.now) type = models.ForeignKey(ReviewTypeName) doc = models.ForeignKey(Document, related_name='reviewrequest_set') - team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None)) + team = models.ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None)) deadline = models.DateField() requested_by = models.ForeignKey(Person) 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") @@ -156,3 +156,23 @@ class ReviewRequest(models.Model): def __unicode__(self): return u"%s review on %s by %s %s" % (self.type, self.doc, self.team, self.state) + +def get_default_review_types(): + return ReviewTypeName.objects.filter(slug__in=['early','lc','telechat']) + +def get_default_review_results(): + return ReviewResultName.objects.filter(slug__in=['not-ready', 'right-track', 'almost-ready', 'ready-issues', 'ready-nits', 'ready']) + +class ReviewTeamSettings(models.Model): + """Holds configuration specific to groups that are review teams""" + group = models.OneToOneField(Group) + autosuggest = models.BooleanField(default=True, verbose_name="Automatically suggest possible review requests") + review_types = models.ManyToManyField(ReviewTypeName, default=get_default_review_types) + review_results = models.ManyToManyField(ReviewResultName, default=get_default_review_results) + + def __unicode__(self): + return u"%s" % (self.group.acronym,) + + class Meta: + verbose_name = "Review team settings" + verbose_name_plural = "Review team settings" diff --git a/ietf/review/resources.py b/ietf/review/resources.py index 8bd655d45..e7bb1b63d 100644 --- a/ietf/review/resources.py +++ b/ietf/review/resources.py @@ -10,7 +10,7 @@ from ietf.api import ToOneField # pyflakes:ignore from ietf.review.models import (ReviewerSettings, ReviewRequest, ResultUsedInReviewTeam, TypeUsedInReviewTeam, UnavailablePeriod, ReviewWish, NextReviewerInTeam, - ReviewSecretarySettings) + ReviewSecretarySettings, ReviewTeamSettings ) from ietf.person.resources import PersonResource @@ -185,3 +185,18 @@ class ReviewSecretarySettingsResource(ModelResource): } api.review.register(ReviewSecretarySettingsResource()) +class ReviewTeamSettingsResource(ModelResource): + group = ToOneField(GroupResource, 'group') + review_types = ToManyField(ReviewTypeNameResource, 'review_types') + review_results = ToManyField(ReviewResultNameResource, 'review_results') + class Meta: + queryset = ReviewTeamSettings.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + filtering = { + "id": ALL, + "autosuggest": ALL, + "review_types": ALL_WITH_RELATIONS, + "review_results": ALL_WITH_RELATIONS, + } +api.review.register(ReviewTeamSettingsResource()) diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 41b3cab50..7ea50b837 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -14,15 +14,14 @@ from ietf.doc.models import (Document, ReviewRequestDocEvent, State, from ietf.iesg.models import TelechatDate from ietf.person.models import Person from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream -from ietf.review.models import (ReviewRequest, ReviewRequestStateName, ReviewTypeName, TypeUsedInReviewTeam, +from ietf.review.models import (ReviewRequest, ReviewRequestStateName, ReviewTypeName, ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam, ReviewSecretarySettings) from ietf.utils.mail import send_mail from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs def active_review_teams(): - # if there's a ResultUsedInReviewTeam defined, it's a review team - return Group.objects.filter(state="active").exclude(resultusedinreviewteam=None) + return Group.objects.filter(reviewteamsettings__isnull=False,state="active") def close_review_request_states(): return ReviewRequestStateName.objects.filter(used=True).exclude(slug__in=["requested", "accepted", "rejected", "part-completed", "completed"]) @@ -526,6 +525,10 @@ def close_review_request(request, review_req, close_state): by=request.user.person, notify_secretary=False, notify_reviewer=True, notify_requested_by=True) def suggested_review_requests_for_team(team): + + if not team.reviewteamsettings.autosuggest: + return [] + system_person = Person.objects.get(name="(System)") seen_deadlines = {} @@ -539,7 +542,7 @@ def suggested_review_requests_for_team(team): requested_state = ReviewRequestStateName.objects.get(slug="requested", used=True) last_call_type = ReviewTypeName.objects.get(slug="lc") - if TypeUsedInReviewTeam.objects.filter(team=team, type=last_call_type).exists(): + if last_call_type in team.reviewteamsettings.review_types.all(): # in Last Call last_call_docs = reviewable_docs_qs.filter( states=State.objects.get(type="draft-iesg", slug="lc", used=True) @@ -567,7 +570,7 @@ def suggested_review_requests_for_team(team): telechat_type = ReviewTypeName.objects.get(slug="telechat") - if TypeUsedInReviewTeam.objects.filter(team=team, type=telechat_type).exists(): + if telechat_type in team.reviewteamsettings.review_types.all(): # on Telechat Agenda telechat_dates = list(TelechatDate.objects.active().order_by('date').values_list("date", flat=True)[:4]) @@ -910,4 +913,3 @@ def email_secretary_reminder(review_request, secretary_role): "deadline_days": deadline_days, "remind_days": remind_days, }) - diff --git a/ietf/utils/test_data.py b/ietf/utils/test_data.py index 610d03a05..54e57f622 100644 --- a/ietf/utils/test_data.py +++ b/ietf/utils/test_data.py @@ -14,8 +14,7 @@ from ietf.meeting.models import Meeting from ietf.name.models import StreamName, DocRelationshipName from ietf.person.models import Person, Email from ietf.group.utils import setup_default_community_list_for_group -from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewResultName, ResultUsedInReviewTeam, - ReviewTypeName, TypeUsedInReviewTeam) +from ietf.review.models import (ReviewRequest, ReviewerSettings, ReviewResultName, ReviewTypeName, ReviewTeamSettings ) def create_person(group, role_name, name=None, username=None, email_address=None, password=None): """Add person/user/email and role.""" @@ -390,10 +389,11 @@ def make_review_data(doc): team2 = create_group(acronym="reviewteam2", name="Review Team 2", type_id="dir", list_email="reviewteam2@ietf.org", parent=Group.objects.get(acronym="farfut")) team3 = create_group(acronym="reviewteam3", name="Review Team 3", type_id="dir", list_email="reviewteam2@ietf.org", parent=Group.objects.get(acronym="farfut")) for team in (team1, team2, team3): + ReviewTeamSettings.objects.create(group=team) for r in ReviewResultName.objects.filter(slug__in=["issues", "ready-issues", "ready", "not-ready"]): - ResultUsedInReviewTeam.objects.create(team=team, result=r) + team.reviewteamsettings.review_results.add(r) for t in ReviewTypeName.objects.filter(slug__in=["early", "lc", "telechat"]): - TypeUsedInReviewTeam.objects.create(team=team, type=t) + team.reviewteamsettings.review_types.add(t) u = User.objects.create(username="reviewer") u.set_password("reviewer+password")