Checkpointing. This is an incomplete idea. The tests will fail massively.

- Legacy-Id: 15985

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)

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)

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())

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,
     })
 

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),

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):

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())

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)
 

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'])

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

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])]

ietf/templates/group/review_requests.html

@@ -17,61 +17,103 @@
     <h1 class="pull-right"><a href="{% url "ietf.stats.views.review_stats" %}" class="icon-link">&nbsp;<span class="small fa fa-bar-chart">&nbsp;</span></a></h1>
   {% endif %}
 
-  {% for label, review_requests in open_review_requests %}
-    {% if review_requests %}
+  {% if unassigned_review_requests %}
 
-      <h2>{{ label }} open review requests</h2>
+    <h2 id="unassigned-review-requests">Unassigned review requests</h2>
 
-      <table class="table table-condensed table-striped tablesorter">
-        <thead>
+    <table class="table table-condensed table-striped tablesorter">
+      <thead>
+        <tr>
+          <th class="col-md-4">Request</th>
+          <th class="col-md-1">Type</th>
+          <th class="col-md-2">Requested</th>
+          <th class="col-md-1">Deadline</th>
+          {% if review_requests.0.reviewer %}
+            <th class="col-md-2">Reviewer</th>
+            <th class="col-md-1">Document state</th>
+          {% else %}
+            <th clas="col-md-3">Document state</th>
+          {% endif %}
+          <th class="col-md-1">IESG Telechat</th>
+        </tr>
+      </thead>
+      <tbody>
+        {% for r in unassigned_review_requests %}
           <tr>
-            <th class="col-md-4">Request</th>
-            <th class="col-md-1">Type</th>
-            <th class="col-md-2">Requested</th>
-            <th class="col-md-1">Deadline</th>
-            {% if review_requests.0.reviewer %}
-              <th class="col-md-2">Reviewer</th>
-              <th class="col-md-1">Document state</th>
-            {% else %}
-              <th clas="col-md-3">Document state</th>
+            <td>{% if r.pk != None %}<a href="{% url "ietf.doc.views_review.review_request" name=r.doc.name request_id=r.pk %}">{% endif %}{{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% if r.pk != None %}</a>{% endif %}</td>
+            <td>{{ r.type.name }}</td>
+            <td><span style="display:none">X</span>{% if r.pk %}{{ r.time|date:"Y-m-d" }} by {{r.requested_by.plain_name}}{% else %}<em>auto-suggested</em>{% endif %}</td>
+            <td><span style="display:none">X</span>
+              {{ r.deadline|date:"Y-m-d" }}
+              {% if r.due %}<span class="label label-warning" title="{{ r.due }} day{{ r.due|pluralize }} past deadline">{{ r.due }} day{{ r.due|pluralize }}</span>{% endif %}
+            </td>
+            {% if r.reviewer %}
+              <td>
+                {{ r.reviewer.person }}
+                {% if r.state_id == "accepted" %}<span class="label label-default">Accepted</span>{% endif %}
+                {% if r.reviewer_unavailable %}<span class="label label-danger">Unavailable</span>{% endif %}
+              </td>
             {% endif %}
-            <th class="col-md-1">IESG Telechat</th>
-          </tr>
-        </thead>
-        <tbody>
-          {% for r in review_requests %}
-            <tr>
-              <td>{% if r.pk != None %}<a href="{% url "ietf.doc.views_review.review_request" name=r.doc.name request_id=r.pk %}">{% endif %}{{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% if r.pk != None %}</a>{% endif %}</td>
-              <td>{{ r.type.name }}</td>
-              <td><span style="display:none">X</span>{% if r.pk %}{{ r.time|date:"Y-m-d" }} by {{r.requested_by.plain_name}}{% else %}<em>auto-suggested</em>{% endif %}</td>
-              <td><span style="display:none">X</span>
-                {{ r.deadline|date:"Y-m-d" }}
-                {% if r.due %}<span class="label label-warning" title="{{ r.due }} day{{ r.due|pluralize }} past deadline">{{ r.due }} day{{ r.due|pluralize }}</span>{% endif %}
-              </td>
-              {% if r.reviewer %}
-                <td>
-                  {{ r.reviewer.person }}
-                  {% if r.state_id == "accepted" %}<span class="label label-default">Accepted</span>{% endif %}
-                  {% if r.reviewer_unavailable %}<span class="label label-danger">Unavailable</span>{% endif %}
-                </td>
+            <td>
+              {{ r.doc.friendly_state }}
+            </td>
+            <td>
+              {% if r.doc.telechat_date %}
+                {{ r.doc.telechat_date }}
               {% endif %}
-              <td>
-                {{ r.doc.friendly_state }}
-              </td>
-              <td>
-                {% if r.doc.telechat_date %}
-                  {{ r.doc.telechat_date }}
-                {% endif %}
-              </td>
-            </tr>
-          {% endfor %}
-        </tbody>
-      </table>
+            </td>
+          </tr>
+        {% endfor %}
+      </tbody>
+    </table>
 
-    {% endif %}
-  {% endfor %}
+  {% endif %}
 
-  <h2 id="closed-review-requests">Closed review requests</h2>
+  {% if open_review_assignments %}
+    <h2 id="open_review_assignments">Open review assignments</h2>
+
+    <table class="table table-condensed table-striped tablesorter">
+      <thead>
+        <tr>
+          <th class="col-md-4">Request</th>
+          <th class="col-md-1">Type</th>
+          <th class="col-md-2">Assigned</th>
+          <th class="col-md-1">Deadline</th>
+          <th class="col-md-2">Reviewer</th>
+          <th class="col-md-1">Document state</th>
+          <th class="col-md-1">IESG Telechat</th>
+        </tr>
+      </thead>
+      <tbody>
+        {% for a in open_review_assignments %}
+          <tr>
+            <td><a href="{% url "ietf.doc.views_review.review_request" name=a.review_request.doc.name request_id=a.review_request.pk %}">{{ a.review_request.doc.name }}-{% if a.review_request.requested_rev %}{{ a.review_requests.requested_rev }}{% else %}{{ a.review_request.doc.rev }}{% endif %}</a></td>
+            <td>{{ a.review_request.type.name }}</td>
+            <td><span style="display:none">X</span>{{ a.assigned_on|date:"Y-m-d" }}</td>
+            <td><span style="display:none">X</span>
+              {{ a.review_request.deadline|date:"Y-m-d" }}
+              {% if a.due %}<span class="label label-warning" title="{{ a.due }} day{{ a.due|pluralize }} past deadline">{{ a.due }} day{{ a.due|pluralize }}</span>{% endif %}
+            </td>
+            <td>
+              {{ a.reviewer.person }}
+              {% if a.state_id == "accepted" %}<span class="label label-default">Accepted</span>{% endif %}
+              {% if a.reviewer_unavailable %}<span class="label label-danger">Unavailable</span>{% endif %}
+            </td>
+            <td>
+              {{ a.review_request.doc.friendly_state }}
+            </td>
+            <td>
+              {% if a.review_request.doc.telechat_date %}
+                {{ a.review_request.doc.telechat_date }}
+              {% endif %}
+            </td>
+          </tr>
+        {% endfor %}
+      </tbody>
+    </table>
+  {% endif %}
+
+  <h2 id="closed-review-requests">Closed review requests and assignments</h2>
 
   <form class="closed-review-filter" action="#closed-review-requests">
     Past:
@@ -83,7 +125,8 @@
   </form>
 
   {% if closed_review_requests %}
-    <table class="table table-condensed table-striped materials tablesorter">
+    <h3>Closed review requests</h3>
+    <table class="table table-condensed table-striped tablesorter">
       <thead>
         <tr>
           <th>Request</th>
@@ -91,9 +134,7 @@
           <th>Requested</th>
           <th>Deadline</th>
           <th>Closed</th>
-          <th>Reviewer</th>
           <th>State</th>
-          <th>Result</th>
         </tr>
       </thead>
       <tbody>
@@ -103,27 +144,49 @@
             <td>{{ r.type }}</td>
             <td><span style="display:none">X</span>{{ r.time|date:"Y-m-d" }} by {{ r.requested_by.plain_name }}</td>
             <td>{{ r.deadline|date:"Y-m-d" }}</td>
-            <td>{{ r.review_done_time|date:"Y-m-d" }}</td>
-            <td>
-              {% if r.reviewer %}
-                {{ r.reviewer.person }}
-              {% else %}
-                <em>not yet assigned</em>
-              {% endif %}
-            </td>
+            <td>{{ r.request_closed_time|date:"Y-m-d" }}</td>
             <td>{{ r.state.name }}</td>
-            <td>
-              {% if r.result %}
-                {{ r.result.name }}
-              {% endif %}
-            </td>
           </tr>
         {% endfor %}
       </tbody>
     </table>
-    
-  {% else %}
-    <p>No closed requests found.</p>
+  {% endif %}
+
+  {% if closed_review_assignments %}
+    <h3>Closed review assignments</h3>
+    <table class="table table-condensed table-striped tablesorter">
+      <thead>
+        <tr>
+          <th>Request</th>
+          <th>Type</th>
+          <th>Assigned</th>
+          <th>Deadline</th>
+          <th>Closed</th>
+          <th>Reviewer</th>
+          <th>State</th>
+          <th>Result</th>
+        </tr>
+      </thead>
+      <tbody>
+        {% for a in closed_review_assignments %}
+          <tr>
+            <td><a href="{% url "ietf.doc.views_review.review_request" name=a.review_request.doc.name request_id=a.review_request.pk %}">{{ a.review_request.doc.name }}{% if a.review_request.requested_rev %}-{{ a.review_request.requested_rev }}{% endif %}</a></td>
+            <td>{{ a.review_request.type }}</td>
+            <td>{{ a.assigned_on|date:"Y-m-d" }}</td>
+            <td>{{ a.review_request.deadline|date:"Y-m-d" }}</td>
+            <td>{{ a.completed_on|date:"Y-m-d" }}</td>
+            <td>{{ a.reviewer.person }}</td>
+            <td>{{ a.state }}</td>
+            <td>{% if a.result %}{{ a.result }}{% endif %}</td>
+          </tr>
+        {% endfor %}
+      </tbody>
+    </table>
+
+  {% endif %}
+
+  {% if not closed_review_requests and not closed_review_assignments %}
+  <div>None found</div>
   {% endif %}
 
 {% endblock %}