diff --git a/ietf/bin/send-review-reminders b/ietf/bin/send-review-reminders index 36eb5a235..c662da38d 100755 --- a/ietf/bin/send-review-reminders +++ b/ietf/bin/send-review-reminders @@ -27,7 +27,8 @@ today = datetime.date.today() for review_req in review_requests_needing_reviewer_reminder(today): email_reviewer_reminder(review_req) - print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_req.reviewer.address, review_req.doc_id, review_req.team.acronym, review_req.pk)) + for review_assignment in review_req.reviewassignment_set.all(): + print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_assignment.reviewer.address, review_req.doc_id, review_req.team.acronym, review_req.pk)) for review_req, secretary_role in review_requests_needing_secretary_reminder(today): email_secretary_reminder(review_req, secretary_role) diff --git a/ietf/doc/models.py b/ietf/doc/models.py index 6541010d1..ab5d00da0 100644 --- a/ietf/doc/models.py +++ b/ietf/doc/models.py @@ -20,7 +20,7 @@ import debug # pyflakes:ignore from ietf.group.models import Group from ietf.name.models import ( DocTypeName, DocTagName, StreamName, IntendedStdLevelName, StdLevelName, - DocRelationshipName, DocReminderTypeName, BallotPositionName, ReviewRequestStateName, FormalLanguageName, + DocRelationshipName, DocReminderTypeName, BallotPositionName, ReviewRequestStateName, ReviewAssignmentStateName, FormalLanguageName, DocUrlTagName) from ietf.person.models import Email, Person from ietf.person.utils import get_active_ads @@ -1144,6 +1144,10 @@ class ReviewRequestDocEvent(DocEvent): review_request = ForeignKey('review.ReviewRequest') state = ForeignKey(ReviewRequestStateName, blank=True, null=True) +class ReviewAssignmentDocEvent(DocEvent): + review_assignment = ForeignKey('review.ReviewAssignment') + state = ForeignKey(ReviewAssignmentStateName, blank=True, null=True) + # charter events class InitialReviewDocEvent(DocEvent): expires = models.DateTimeField(blank=True, null=True) diff --git a/ietf/doc/resources.py b/ietf/doc/resources.py index 62f928ced..64fd71a15 100644 --- a/ietf/doc/resources.py +++ b/ietf/doc/resources.py @@ -12,7 +12,7 @@ from ietf.doc.models import (BallotType, DeletedEvent, StateType, State, Documen TelechatDocEvent, DocReminder, LastCallDocEvent, NewRevisionDocEvent, WriteupDocEvent, InitialReviewDocEvent, DocHistoryAuthor, BallotDocEvent, RelatedDocument, RelatedDocHistory, BallotPositionDocEvent, AddedMessageEvent, SubmissionDocEvent, - ReviewRequestDocEvent, EditedAuthorsDocEvent, DocumentURL) + ReviewRequestDocEvent, ReviewAssignmentDocEvent, EditedAuthorsDocEvent, DocumentURL) from ietf.name.resources import BallotPositionNameResource, DocTypeNameResource class BallotTypeResource(ModelResource): @@ -652,3 +652,32 @@ class DocumentURLResource(ModelResource): api.doc.register(DocumentURLResource()) + + +from ietf.person.resources import PersonResource +from ietf.review.resources import ReviewAssignmentResource +from ietf.name.resources import ReviewAssignmentStateNameResource +class ReviewAssignmentDocEventResource(ModelResource): + by = ToOneField(PersonResource, 'by') + doc = ToOneField(DocumentResource, 'doc') + docevent_ptr = ToOneField(DocEventResource, 'docevent_ptr') + review_assignment = ToOneField(ReviewAssignmentResource, 'review_assignment') + state = ToOneField(ReviewAssignmentStateNameResource, 'state', null=True) + class Meta: + queryset = ReviewAssignmentDocEvent.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewassignmentdocevent' + filtering = { + "id": ALL, + "time": ALL, + "type": ALL, + "rev": ALL, + "desc": ALL, + "by": ALL_WITH_RELATIONS, + "doc": ALL_WITH_RELATIONS, + "docevent_ptr": ALL_WITH_RELATIONS, + "review_assignment": ALL_WITH_RELATIONS, + "state": ALL_WITH_RELATIONS, + } +api.doc.register(ReviewAssignmentDocEventResource()) diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 9103a65c1..d0104ee8d 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -21,8 +21,8 @@ from django.urls import reverse as urlreverse from ietf.doc.models import (Document, NewRevisionDocEvent, State, DocAlias, LastCallDocEvent, ReviewRequestDocEvent, DocumentAuthor) -from ietf.name.models import ReviewRequestStateName, ReviewResultName, DocTypeName -from ietf.review.models import ReviewRequest +from ietf.name.models import ReviewRequestStateName, ReviewAssignmentStateName, ReviewResultName, DocTypeName +from ietf.review.models import ReviewRequest, ReviewAssignment from ietf.group.models import Group from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has_role from ietf.message.models import Message @@ -30,9 +30,9 @@ from ietf.message.utils import infer_message from ietf.person.fields import PersonEmailChoiceField, SearchablePersonField 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_review_request_change, make_new_review_request_from_existing, - close_review_request_states, close_review_request, - setup_reviewer_field) + email_review_assignment_change, email_review_request_change, + make_new_review_request_from_existing, close_review_request_states, + close_review_request, setup_reviewer_field) from ietf.review import mailarch from ietf.utils.fields import DatepickerDateField from ietf.utils.text import strip_prefix, xslugify @@ -281,7 +281,7 @@ class AssignReviewerForm(forms.Form): @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"]) + review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "assigned"]) if not can_manage_review_requests_for_team(request.user, review_req.team): return HttpResponseForbidden("You do not have permission to perform this action") @@ -307,15 +307,15 @@ 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 if filled in", strip=False) @login_required -def reject_reviewer_assignment(request, name, request_id): +def reject_reviewer_assignment(request, name, assignment_id): doc = get_object_or_404(Document, name=name) - review_req = get_object_or_404(ReviewRequest, pk=request_id, state__in=["requested", "accepted"]) + review_assignment = get_object_or_404(ReviewAssignment, pk=assignment_id, state__in=["assigned", "accepted"]) - if not review_req.reviewer: - return redirect(review_request, name=review_req.doc.name, request_id=review_req.pk) + if not review_assignment.reviewer: + return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.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) + is_reviewer = user_is_person(request.user, review_assignment.reviewer.person) + can_manage_request = can_manage_review_requests_for_team(request.user, review_assignment.review_request.team) if not (is_reviewer or can_manage_request): return HttpResponseForbidden("You do not have permission to perform this action") @@ -323,42 +323,39 @@ def reject_reviewer_assignment(request, name, request_id): if request.method == "POST" and request.POST.get("action") == "reject": form = RejectReviewerAssignmentForm(request.POST) if form.is_valid(): - # reject the request - review_req.state = ReviewRequestStateName.objects.get(slug="rejected") - review_req.save() + # reject the assignment + review_assignment.state = ReviewAssignmentStateName.objects.get(slug="rejected") + review_assignment.save() - ReviewRequestDocEvent.objects.create( - type="closed_review_request", - doc=review_req.doc, - rev=review_req.doc.rev, - 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, - ), - review_request=review_req, - state=review_req.state, - ) - - # make a new unassigned review request - new_review_req = make_new_review_request_from_existing(review_req) - new_review_req.save() + # TODO: this needs to be reworked as a ReviewAssignmentDocEvent + #ReviewRequestDocEvent.objects.create( + # type="closed_review_request", + # doc=review_req.doc, + # rev=review_req.doc.rev, + # 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, + # ), + # review_request=review_req, + # state=review_req.state, + #) msg = render_to_string("review/reviewer_assignment_rejected.txt", { "by": request.user.person, "message_to_secretary": form.cleaned_data.get("message_to_secretary") }) - email_review_request_change(request, review_req, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False) + email_review_assignment_change(request, review_assignment, "Reviewer assignment rejected", msg, by=request.user.person, notify_secretary=True, notify_reviewer=True, notify_requested_by=False) - return redirect(review_request, name=new_review_req.doc.name, request_id=new_review_req.pk) + return redirect(review_request, name=review_assignment.review_request.doc.name, request_id=review_assignment.review_request.pk) else: form = RejectReviewerAssignmentForm() return render(request, 'doc/review/reject_reviewer_assignment.html', { 'doc': doc, - 'review_req': review_req, + 'review_req': review_assignment.review_request, 'form': form, }) diff --git a/ietf/group/views.py b/ietf/group/views.py index 326eb385e..5c96263cc 100644 --- a/ietf/group/views.py +++ b/ietf/group/views.py @@ -85,7 +85,7 @@ from ietf.meeting.helpers import get_meeting from ietf.meeting.utils import group_sessions from ietf.name.models import GroupTypeName, StreamName from ietf.person.models import Email -from ietf.review.models import ReviewRequest, ReviewerSettings, ReviewSecretarySettings +from ietf.review.models import ReviewRequest, ReviewAssignment, ReviewerSettings, ReviewSecretarySettings from ietf.review.utils import (can_manage_review_requests_for_team, can_access_review_stats_for_team, @@ -1262,27 +1262,26 @@ def group_menu_data(request): # --- Review views ----------------------------------------------------- def get_open_review_requests_for_team(team, assignment_status=None): - open_review_requests = ReviewRequest.objects.filter( - team=team, - state__in=("requested", "accepted") + open_review_requests = ReviewRequest.objects.filter(team=team).filter( + Q(state_id='requested') | Q(state_id='assigned',reviewassignment__state__in=('assigned','accepted')) ).prefetch_related( - "reviewer__person", "type", "state", "doc", "doc__states", + "type", "state", "doc", "doc__states", ).order_by("-time", "-id") if assignment_status == "unassigned": - open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(reviewer=None)) + open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(state_id='requested')) elif assignment_status == "assigned": - open_review_requests = list(open_review_requests.exclude(reviewer=None)) + open_review_requests = list(open_review_requests.filter(state_id='assigned')) else: open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests) - today = datetime.date.today() - unavailable_periods = current_unavailable_periods_for_reviewers(team) - for r in open_review_requests: - if r.reviewer: - r.reviewer_unavailable = any(p.availability == "unavailable" - for p in unavailable_periods.get(r.reviewer.person_id, [])) - r.due = max(0, (today - r.deadline).days) + #today = datetime.date.today() + #unavailable_periods = current_unavailable_periods_for_reviewers(team) + #for r in open_review_requests: + #if r.reviewer: + # r.reviewer_unavailable = any(p.availability == "unavailable" + # for p in unavailable_periods.get(r.reviewer.person_id, [])) + #r.due = max(0, (today - r.deadline).days) return open_review_requests @@ -1291,25 +1290,19 @@ def review_requests(request, acronym, group_type=None): if not group.features.has_reviews: raise Http404 - assigned_review_requests = [] - unassigned_review_requests = [] + unassigned_review_requests = [r for r in get_open_review_requests_for_team(group) if not r.state_id=='assigned'] - for r in get_open_review_requests_for_team(group): - if r.reviewer: - assigned_review_requests.append(r) - else: - unassigned_review_requests.append(r) + open_review_assignments = list(ReviewAssignment.objects.filter(review_request__team=group, state_id__in=('assigned','accepted')).order_by('-assigned_on')) + today = datetime.date.today() + unavailable_periods = current_unavailable_periods_for_reviewers(group) + for a in open_review_assignments: + a.reviewer_unavailable = any(p.availability == "unavailable" + for p in unavailable_periods.get(a.reviewer.person_id, [])) + a.due = max(0, (today - a.review_request.deadline).days) - open_review_requests = [ - ("Unassigned", unassigned_review_requests), - ("Assigned", assigned_review_requests), - ] + closed_review_assignments = ReviewAssignment.objects.filter(review_request__team=group).exclude(state_id__in=('assigned','accepted')).prefetch_related("state","result").order_by('-assigned_on') - closed_review_requests = ReviewRequest.objects.filter( - team=group, - ).exclude( - state__in=("requested", "accepted") - ).prefetch_related("reviewer__person", "type", "state", "doc", "result").order_by("-time", "-id") + closed_review_requests = ReviewRequest.objects.filter(team=group).exclude(state__in=("requested", "assigned")).prefetch_related("type", "state", "doc").order_by("-time", "-id") since_choices = [ (None, "1 month"), @@ -1337,10 +1330,14 @@ def review_requests(request, acronym, group_type=None): | Q(reviewrequestdocevent__isnull=True, time__gte=datetime.date.today() - date_limit) ).distinct() + closed_review_assignments = closed_review_assignments.filter(completed_on__gte = datetime.date.today() - date_limit) + return render(request, 'group/review_requests.html', construct_group_menu_context(request, group, "review requests", group_type, { - "open_review_requests": open_review_requests, + "unassigned_review_requests": unassigned_review_requests, + "open_review_assignments": open_review_assignments, "closed_review_requests": closed_review_requests, + "closed_review_assignments": closed_review_assignments, "since_choices": since_choices, "since": since, "can_manage_review_requests": can_manage_review_requests_for_team(request.user, group), diff --git a/ietf/name/models.py b/ietf/name/models.py index ac96ab9bc..5a257c22f 100644 --- a/ietf/name/models.py +++ b/ietf/name/models.py @@ -95,8 +95,9 @@ class LiaisonStatementEventTypeName(NameModel): class LiaisonStatementTagName(NameModel): "Action Required, Action Taken" class ReviewRequestStateName(NameModel): - """Requested, Accepted, Rejected, Withdrawn, Overtaken By Events, - No Response, No Review of Version, No Review of Document, Partially Completed, Completed""" + """Requested, Assigned, Withdrawn, Overtaken By Events, No Review of Version, No Review of Document""" +class ReviewAssignmentStateName(NameModel): + """Accepted, Rejected, Withdrawn, Overtaken By Events, No Response, Partially Completed, Completed""" class ReviewTypeName(NameModel): """Early Review, Last Call, Telechat""" class ReviewResultName(NameModel): diff --git a/ietf/name/resources.py b/ietf/name/resources.py index 76dca2a15..dca6b8d94 100644 --- a/ietf/name/resources.py +++ b/ietf/name/resources.py @@ -14,8 +14,9 @@ from ietf.name.models import ( AgendaTypeName, BallotPositionName, ConstraintNam ImportantDateName, IntendedStdLevelName, IprDisclosureStateName, IprEventTypeName, IprLicenseTypeName, LiaisonStatementEventTypeName, LiaisonStatementPurposeName, LiaisonStatementState, LiaisonStatementTagName, MeetingTypeName, NomineePositionStateName, - ReviewRequestStateName, ReviewResultName, ReviewTypeName, RoleName, RoomResourceName, - SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, TopicAudienceName, ) + ReviewAssignmentStateName, ReviewRequestStateName, ReviewResultName, ReviewTypeName, + RoleName, RoomResourceName, SessionStatusName, StdLevelName, StreamName, TimeSlotTypeName, + TopicAudienceName, ) class TimeSlotTypeNameResource(ModelResource): class Meta: @@ -567,3 +568,19 @@ class AgendaTypeNameResource(ModelResource): "order": ALL, } api.name.register(AgendaTypeNameResource()) + + +class ReviewAssignmentStateNameResource(ModelResource): + class Meta: + queryset = ReviewAssignmentStateName.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewassignmentstatename' + filtering = { + "slug": ALL, + "name": ALL, + "desc": ALL, + "used": ALL, + "order": ALL, + } +api.name.register(ReviewAssignmentStateNameResource()) diff --git a/ietf/review/admin.py b/ietf/review/admin.py index 691cee738..7b2a01205 100644 --- a/ietf/review/admin.py +++ b/ietf/review/admin.py @@ -53,11 +53,11 @@ admin.site.register(NextReviewerInTeam, NextReviewerInTeamAdmin) class ReviewRequestAdmin(admin.ModelAdmin): list_display = ["doc", "time", "type", "team", "deadline"] list_display_links = ["doc"] - list_filter = ["team", "type", "state", "result"] + list_filter = ["team", "type", "state"] ordering = ["-id"] - raw_id_fields = ["doc", "team", "requested_by", "reviewer", "review"] + raw_id_fields = ["doc", "team", "requested_by"] date_hierarchy = "time" - search_fields = ["doc__name", "reviewer__person__name"] + search_fields = ["doc__name"] admin.site.register(ReviewRequest, ReviewRequestAdmin) diff --git a/ietf/review/models.py b/ietf/review/models.py index d4a89619d..cf6f997d8 100644 --- a/ietf/review/models.py +++ b/ietf/review/models.py @@ -7,7 +7,7 @@ from django.db import models from ietf.doc.models import Document from ietf.group.models import Group from ietf.person.models import Person, Email -from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName +from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName, ReviewAssignmentStateName from ietf.utils.validators import validate_regular_expression_string from ietf.utils.models import ForeignKey, OneToOneField @@ -105,9 +105,7 @@ class NextReviewerInTeam(models.Model): verbose_name_plural = "next reviewer in team settings" 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.""" + """Represents a request for a review and the process it goes through.""" state = ForeignKey(ReviewRequestStateName) # Fields filled in on the initial record creation - these @@ -121,16 +119,19 @@ class ReviewRequest(models.Model): 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") comment = models.TextField(verbose_name="Requester's comments and instructions", max_length=2048, blank=True, help_text="Provide any additional information to show to the review team secretary and reviewer", default='') + # Moved to class Review: # Fields filled in as reviewer is assigned and as the review is # uploaded. Once these are filled in and we progress beyond being # requested/assigned, any changes to the assignment happens by # closing down the current request and making a new one, copying # the request-part fields above. - reviewer = ForeignKey(Email, blank=True, null=True) + # TODO: Change that - changing an assingment should only be creating a new Assignment and marking the old one as withdrawn + # These exist only to facilitate data migrations. They will be removed in the next release. + unused_reviewer = ForeignKey(Email, blank=True, null=True) - review = OneToOneField(Document, blank=True, null=True) - reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) - result = ForeignKey(ReviewResultName, blank=True, null=True) + unused_review = OneToOneField(Document, blank=True, null=True) + unused_reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) + unused_result = 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) @@ -141,14 +142,33 @@ class ReviewRequest(models.Model): def other_completed_requests(self): return self.other_requests().filter(state_id__in=['completed','part-completed']) - def review_done_time(self): - # First check if this is completed review having review and if so take time from there. - if self.review and self.review.time: - return self.review.time - # If not, then it is closed review, so it either has event in doc or if not then take - # time from the request. - time = self.doc.request_closed_time(self) - return time if time else self.time + #def review_done_time(self): + # # First check if this is completed review having review and if so take time from there. + # if self.review and self.review.time: + # return self.review.time + # # If not, then it is closed review, so it either has event in doc or if not then take + # # time from the request. + # time = self.doc.request_closed_time(self) + # return time if time else self.time + + def request_closed_time(self): + return self.doc.request_closed_time(self) or self.time + +class ReviewAssignment(models.Model): + """ One of possibly many reviews assigned in response to a ReviewRequest """ + review_request = ForeignKey(ReviewRequest) + state = ForeignKey(ReviewAssignmentStateName) + reviewer = ForeignKey(Email) + assigned_on = models.DateTimeField(blank=True, null=True) + completed_on = models.DateTimeField(blank=True, null=True) + review = OneToOneField(Document, blank=True, null=True) + reviewed_rev = models.CharField(verbose_name="reviewed revision", max_length=16, blank=True) + result = ForeignKey(ReviewResultName, blank=True, null=True) + mailarch_url = models.URLField(blank=True, null = True) + + def __unicode__(self): + return u"Assignment for %s (%s) : %s %s of %s" % (self.reviewer.person, self.state, self.review_request.team.acronym, self.review_request.type, self.review_request.doc) + def get_default_review_types(): return ReviewTypeName.objects.filter(slug__in=['early','lc','telechat']) diff --git a/ietf/review/resources.py b/ietf/review/resources.py index 059b801c5..abbf10202 100644 --- a/ietf/review/resources.py +++ b/ietf/review/resources.py @@ -7,7 +7,7 @@ from tastypie.cache import SimpleCache from ietf import api from ietf.api import ToOneField # pyflakes:ignore -from ietf.review.models import (ReviewerSettings, ReviewRequest, +from ietf.review.models import (ReviewerSettings, ReviewRequest, ReviewAssignment, UnavailablePeriod, ReviewWish, NextReviewerInTeam, ReviewSecretarySettings, ReviewTeamSettings, HistoricalReviewerSettings ) @@ -45,9 +45,6 @@ class ReviewRequestResource(ModelResource): doc = ToOneField(DocumentResource, 'doc') team = ToOneField(GroupResource, 'team') requested_by = ToOneField(PersonResource, 'requested_by') - reviewer = ToOneField(EmailResource, 'reviewer', null=True) - review = ToOneField(DocumentResource, 'review', null=True) - result = ToOneField(ReviewResultNameResource, 'result', null=True) class Meta: queryset = ReviewRequest.objects.all() serializer = api.Serializer() @@ -59,17 +56,38 @@ class ReviewRequestResource(ModelResource): "deadline": ALL, "requested_rev": ALL, "comment": ALL, - "reviewed_rev": ALL, "state": ALL_WITH_RELATIONS, "type": ALL_WITH_RELATIONS, "doc": ALL_WITH_RELATIONS, "team": ALL_WITH_RELATIONS, "requested_by": ALL_WITH_RELATIONS, + } +api.review.register(ReviewRequestResource()) + +from ietf.name.resources import ReviewAssignmentStateNameResource +class ReviewAssignmentResource(ModelResource): + review_request = ToOneField(ReviewRequest, 'review_request') + state = ToOneField(ReviewAssignmentStateNameResource, 'state') + reviewer = ToOneField(EmailResource, 'reviewer', null=True) + review = ToOneField(DocumentResource, 'review', null=True) + result = ToOneField(ReviewResultNameResource, 'result', null=True) + class Meta: + queryset = ReviewAssignment.objects.all() + serializer = api.Serializer() + cache = SimpleCache() + #resource_name = 'reviewassignment' + filtering = { + "id": ALL, + "reviewed_rev": ALL, + "mailarch_url": ALL, + "assigned_on": ALL, + "completed_on": ALL, + "state": ALL_WITH_RELATIONS, "reviewer": ALL_WITH_RELATIONS, "review": ALL_WITH_RELATIONS, "result": ALL_WITH_RELATIONS, } -api.review.register(ReviewRequestResource()) +api.review.register(ReviewAssignmentResource()) from ietf.person.resources import PersonResource from ietf.group.resources import GroupResource diff --git a/ietf/review/utils.py b/ietf/review/utils.py index 85497ba49..c9285cf4e 100644 --- a/ietf/review/utils.py +++ b/ietf/review/utils.py @@ -14,7 +14,7 @@ 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, +from ietf.review.models import (ReviewRequest, ReviewAssignment, ReviewRequestStateName, ReviewTypeName, ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam, ReviewTeamSettings, ReviewSecretarySettings) from ietf.utils.mail import send_mail, get_email_addresses_from_text @@ -333,6 +333,46 @@ def make_new_review_request_from_existing(review_req): obj.state = ReviewRequestStateName.objects.get(slug="requested") return obj +def email_review_assignment_change(request, review_assignment, subject, msg, by, notify_secretary, notify_reviewer, notify_requested_by): + + system_email = Person.objects.get(name="(System)").formatted_email() + + to = set() + + def extract_email_addresses(objs): + for o in objs: + if o and o.person!=by: + e = o.formatted_email() + if e != system_email: + to.add(e) + + if notify_secretary: + rts = ReviewTeamSettings.objects.filter(group=review_assignment.review_request.team).first() + if rts and rts.secr_mail_alias and rts.secr_mail_alias.strip() != '': + for addr in get_email_addresses_from_text(rts.secr_mail_alias): + to.add(addr) + else: + extract_email_addresses(Role.objects.filter(name="secr", group=review_assignment.review_request.team).distinct()) + if notify_reviewer: + extract_email_addresses([review_assignment.reviewer]) + if notify_requested_by: + extract_email_addresses([review_assignment.review_request.requested_by.email()]) + + if not to: + return + + to = list(to) + + url = urlreverse("ietf.doc.views_review.review_request_forced_login", kwargs={ "name": review_assignment.review_request.doc.name, "request_id": review_assignment.review_request.pk }) + url = request.build_absolute_uri(url) + # TODO : Why is this a bare send_mail? + send_mail(request, to, request.user.person.formatted_email(), subject, "review/review_request_changed.txt", { + "review_req_url": url, + "review_req": review_assignment.review_request, + "msg": msg, + }) + + def email_review_request_change(request, review_req, subject, msg, by, notify_secretary, notify_reviewer, notify_requested_by): """Notify stakeholders about change, skipping a party if the change @@ -417,24 +457,23 @@ def email_reviewer_availability_change(request, team, reviewer_role, msg, by): }) def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=False): - assert review_req.state_id in ("requested", "accepted") + assert review_req.state_id in ("requested", "assigned") - if reviewer == review_req.reviewer: + if review_req.reviewassignment_set.filter(reviewer=reviewer).exists(): return - if review_req.reviewer: - email_review_request_change( - 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, notify_requested_by=False) + # Unassignment now has to be explicit + #if review_req.reviewer: + # email_review_request_change( + # 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, notify_requested_by=False) - review_req.state = ReviewRequestStateName.objects.get(slug="requested") - review_req.reviewer = reviewer - review_req.save() + review_req.reviewassignment_set.create(state_id='requested', reviewer = reviewer, assigned_on = datetime.datetime.now()) - if review_req.reviewer: - possibly_advance_next_reviewer_for_team(review_req.team, review_req.reviewer.person_id, add_skip) + if reviewer: + possibly_advance_next_reviewer_for_team(review_req.team, reviewer.person_id, add_skip) ReviewRequestDocEvent.objects.create( type="assigned_review_request", @@ -451,19 +490,19 @@ def assign_review_request_to_reviewer(request, review_req, reviewer, add_skip=Fa ) msg = "%s has assigned you as a reviewer for this document." % request.user.person.ascii - prev_team_reviews = ReviewRequest.objects.filter( + prev_team_reviews = ReviewAssignment.objects.filter( doc=review_req.doc, state="completed", team=review_req.team, ) if prev_team_reviews.exists(): msg = msg + '\n\nThis team has completed other reviews of this document:\n' - for req in prev_team_reviews: + for assignment in prev_team_reviews: msg += u'%s %s -%s %s\n'% ( - req.review_done_time().strftime('%d %b %Y'), - req.reviewer.person.ascii, - req.reviewed_rev or req.requested_rev, - req.result.name, + assignment.completed_on.strftime('%d %b %Y'), + assignment.reviewer.person.ascii, + assignment.reviewed_rev or assignment.review_request.requested_rev, + assignment.result.name, ) email_review_request_change( @@ -630,7 +669,7 @@ def suggested_review_requests_for_team(team): seen_deadlines[doc_pk] = deadline - # filter those with existing requests + # filter those with existing explicit requests existing_requests = defaultdict(list) for r in ReviewRequest.objects.filter(doc__in=requests.iterkeys(), team=team): existing_requests[r.doc_id].append(r) @@ -640,12 +679,14 @@ def suggested_review_requests_for_team(team): return False no_review_document = existing.state_id == "no-review-document" - pending = (existing.state_id in ("requested", "accepted") + no_review_rev = ( existing.state_id == "no-review-version") and (not existing.requested_rev or existing.requested_rev == request.doc.rev) + pending = (existing.state_id == "assigned" + and existing.reviewassignment_set.filter(state_id__in=("assigned", "accepted")).exists() and (not existing.requested_rev or existing.requested_rev == request.doc.rev)) - completed_or_closed = (existing.state_id not in ("part-completed", "rejected", "overtaken", "no-response") - and existing.reviewed_rev == request.doc.rev) + request_closed = existing.state_id not in ('requested','assigned') + all_assignments_completed = not existing.reviewassignment_set.filter(state_id__in=('assigned','accepted')).exists() - return no_review_document or pending or completed_or_closed + return any([no_review_document, no_review_rev, pending, request_closed, all_assignments_completed]) res = [r for r in requests.itervalues() if not any(blocks(e, r) for e in existing_requests[r.doc_id])] diff --git a/ietf/templates/group/review_requests.html b/ietf/templates/group/review_requests.html index 6122f07a9..bcae4ccbf 100644 --- a/ietf/templates/group/review_requests.html +++ b/ietf/templates/group/review_requests.html @@ -17,61 +17,103 @@

  

{% endif %} - {% for label, review_requests in open_review_requests %} - {% if review_requests %} + {% if unassigned_review_requests %} -

{{ label }} open review requests

+

Unassigned review requests

- - +
+ + + + + + + {% if review_requests.0.reviewer %} + + + {% else %} + + {% endif %} + + + + + {% for r in unassigned_review_requests %} - - - - - {% if review_requests.0.reviewer %} - - - {% else %} - + + + + + {% if r.reviewer %} + {% endif %} - - - - - {% for r in review_requests %} - - - - - - {% if r.reviewer %} - + + - - - {% endfor %} - -
RequestTypeRequestedDeadlineReviewerDocument stateDocument stateIESG Telechat
RequestTypeRequestedDeadlineReviewerDocument stateDocument state{% if r.pk != None %}{% endif %}{{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% if r.pk != None %}{% endif %}{{ r.type.name }}X{% if r.pk %}{{ r.time|date:"Y-m-d" }} by {{r.requested_by.plain_name}}{% else %}auto-suggested{% endif %}X + {{ r.deadline|date:"Y-m-d" }} + {% if r.due %}{{ r.due }} day{{ r.due|pluralize }}{% endif %} + + {{ r.reviewer.person }} + {% if r.state_id == "accepted" %}Accepted{% endif %} + {% if r.reviewer_unavailable %}Unavailable{% endif %} + IESG Telechat
{% if r.pk != None %}{% endif %}{{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% if r.pk != None %}{% endif %}{{ r.type.name }}X{% if r.pk %}{{ r.time|date:"Y-m-d" }} by {{r.requested_by.plain_name}}{% else %}auto-suggested{% endif %}X - {{ r.deadline|date:"Y-m-d" }} - {% if r.due %}{{ r.due }} day{{ r.due|pluralize }}{% endif %} - - {{ r.reviewer.person }} - {% if r.state_id == "accepted" %}Accepted{% endif %} - {% if r.reviewer_unavailable %}Unavailable{% endif %} - + {{ r.doc.friendly_state }} + + {% if r.doc.telechat_date %} + {{ r.doc.telechat_date }} {% endif %} - - {{ r.doc.friendly_state }} - - {% if r.doc.telechat_date %} - {{ r.doc.telechat_date }} - {% endif %} -
+ + + {% endfor %} + + - {% endif %} - {% endfor %} + {% endif %} -

Closed review requests

+ {% if open_review_assignments %} +

Open review assignments

+ + + + + + + + + + + + + + + {% for a in open_review_assignments %} + + + + + + + + + + {% endfor %} + +
RequestTypeAssignedDeadlineReviewerDocument stateIESG Telechat
{{ a.review_request.doc.name }}-{% if a.review_request.requested_rev %}{{ a.review_requests.requested_rev }}{% else %}{{ a.review_request.doc.rev }}{% endif %}{{ a.review_request.type.name }}X{{ a.assigned_on|date:"Y-m-d" }}X + {{ a.review_request.deadline|date:"Y-m-d" }} + {% if a.due %}{{ a.due }} day{{ a.due|pluralize }}{% endif %} + + {{ a.reviewer.person }} + {% if a.state_id == "accepted" %}Accepted{% endif %} + {% if a.reviewer_unavailable %}Unavailable{% endif %} + + {{ a.review_request.doc.friendly_state }} + + {% if a.review_request.doc.telechat_date %} + {{ a.review_request.doc.telechat_date }} + {% endif %} +
+ {% endif %} + +

Closed review requests and assignments

Past: @@ -83,7 +125,8 @@
{% if closed_review_requests %} - +

Closed review requests

+
@@ -91,9 +134,7 @@ - - @@ -103,27 +144,49 @@ - - + - {% endfor %}
RequestRequested Deadline ClosedReviewer StateResult
{{ r.type }} X{{ r.time|date:"Y-m-d" }} by {{ r.requested_by.plain_name }} {{ r.deadline|date:"Y-m-d" }}{{ r.review_done_time|date:"Y-m-d" }} - {% if r.reviewer %} - {{ r.reviewer.person }} - {% else %} - not yet assigned - {% endif %} - {{ r.request_closed_time|date:"Y-m-d" }} {{ r.state.name }} - {% if r.result %} - {{ r.result.name }} - {% endif %} -
- - {% else %} -

No closed requests found.

+ {% endif %} + + {% if closed_review_assignments %} +

Closed review assignments

+ + + + + + + + + + + + + + + {% for a in closed_review_assignments %} + + + + + + + + + + + {% endfor %} + +
RequestTypeAssignedDeadlineClosedReviewerStateResult
{{ a.review_request.doc.name }}{% if a.review_request.requested_rev %}-{{ a.review_request.requested_rev }}{% endif %}{{ a.review_request.type }}{{ a.assigned_on|date:"Y-m-d" }}{{ a.review_request.deadline|date:"Y-m-d" }}{{ a.completed_on|date:"Y-m-d" }}{{ a.reviewer.person }}{{ a.state }}{% if a.result %}{{ a.result }}{% endif %}
+ + {% endif %} + + {% if not closed_review_requests and not closed_review_assignments %} +
None found
{% endif %} {% endblock %}