Split up open review requests on the review team page in assigned and

OleLaursen · OleLaursen · commit 58dd25ff7936 · 2016-11-04T17:19:33.000Z
unassigned requests to make it easier to just work with the unassigned
ones. Use same split on the manage reviews page which is now two
pages.
 - Legacy-Id: 12266
diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py
@@ -148,11 +148,12 @@ def test_manage_review_requests(self):
 
         group = review_req1.team
 
-        url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym })
+        url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, "assignment_status": "assigned" })
 
         login_testing_unauthorized(self, "secretary", url)
 
-        url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })
+        assigned_url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id, "assignment_status": "assigned" })
+        unassigned_url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id, "assignment_status": "unassigned" })
 
         review_req2 = ReviewRequest.objects.create(
             doc=review_req1.doc,
@@ -201,14 +202,14 @@ def test_manage_review_requests(self):
         )
         
         # get
-        r = self.client.get(url)
+        r = self.client.get(assigned_url)
         self.assertEqual(r.status_code, 200)
         self.assertTrue(review_req1.doc.name in unicontent(r))
 
-        # can't save: conflict
+        # can't save assigned: conflict
         new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman")
         # provoke conflict by posting bogus data
-        r = self.client.post(url, {
+        r = self.client.post(assigned_url, {
             "reviewrequest": [str(review_req1.pk), str(review_req2.pk), str(123456)],
 
             # close
@@ -226,13 +227,21 @@ def test_manage_review_requests(self):
         self.assertEqual(r.status_code, 200)
         content = unicontent(r).lower()
         self.assertTrue("1 request closed" in content)
-        self.assertTrue("1 request opened" in content)
         self.assertTrue("2 requests changed assignment" in content)
 
-        # close and assign
+        # can't save unassigned: conflict
+        r = self.client.post(unassigned_url, {
+            "reviewrequest": [str(123456)],
+            "action": "save-continue",
+        })
+        self.assertEqual(r.status_code, 200)
+        content = unicontent(r).lower()
+        self.assertTrue("1 request opened" in content)
+
+        # close and reassign assigned
         new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman")
-        r = self.client.post(url, {
-            "reviewrequest": [str(review_req1.pk), str(review_req2.pk), str(review_req3.pk)],
+        r = self.client.post(assigned_url, {
+            "reviewrequest": [str(review_req1.pk), str(review_req2.pk)],
 
             # close
             "r{}-existing_reviewer".format(review_req1.pk): review_req1.reviewer_id or "",
@@ -244,12 +253,20 @@ def test_manage_review_requests(self):
             "r{}-action".format(review_req2.pk): "assign",
             "r{}-reviewer".format(review_req2.pk): new_reviewer.pk,
 
+            "action": "save",
+        })
+        self.assertEqual(r.status_code, 302)
+
+        # no change on unassigned
+        r = self.client.post(unassigned_url, {
+            "reviewrequest": [str(review_req3.pk)],
+
             # no change
             "r{}-existing_reviewer".format(review_req3.pk): review_req3.reviewer_id or "",
             "r{}-action".format(review_req3.pk): "",
             "r{}-close".format(review_req3.pk): "no-response",
             "r{}-reviewer".format(review_req3.pk): "",
-
+            
             "action": "save",
         })
         self.assertEqual(r.status_code, 302)
diff --git a/ietf/group/urls_info_details.py b/ietf/group/urls_info_details.py
@@ -31,7 +31,7 @@
     (r'^archives/$', 'ietf.group.views.derived_archives'),
     (r'^photos/$', views.group_photos),
     (r'^reviews/$', views_review.review_requests),
-    (r'^reviews/manage/$', views_review.manage_review_requests),
+    (r'^reviews/manage/(?P<assignment_status>assigned|unassigned)/$', views_review.manage_review_requests),
     (r'^reviews/email-assignments/$', views_review.email_open_review_assignments),
     (r'^reviewers/$', views_review.reviewer_overview),
     (r'^reviewers/(?P<reviewer_email>[\w%+-.@]+)/settings/$', views_review.change_reviewer_settings),
diff --git a/ietf/group/utils.py b/ietf/group/utils.py
@@ -213,7 +213,8 @@ def construct_group_menu_context(request, group, selected, group_type, others):
 
     if group.features.has_reviews and can_manage_review_requests_for_team(request.user, group):
         import ietf.group.views_review
-        actions.append((u"Manage review requests", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=kwargs)))
+        actions.append((u"Manage unassigned reviews", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=dict(assignment_status="unassigned", **kwargs))))
+        actions.append((u"Manage assigned reviews", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=dict(assignment_status="assigned", **kwargs))))
 
     if group.state_id != "conclude" and (is_admin or can_manage):
         actions.append((u"Edit group", urlreverse("group_edit", kwargs=kwargs)))
diff --git a/ietf/group/views_review.py b/ietf/group/views_review.py
@@ -30,33 +30,55 @@
 from ietf.utils.fields import DatepickerDateField
 from ietf.ietfauth.utils import user_is_person
 
+def get_open_review_requests_for_team(team, assignment_status=None):
+    open_review_requests = ReviewRequest.objects.filter(
+        team=team,
+        state__in=("requested", "accepted")
+    ).prefetch_related(
+        "reviewer__person", "type", "state"
+    ).order_by("-time", "-id")
+
+    if assignment_status == "unassigned":
+        open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(reviewer=None))
+    elif assignment_status == "assigned":
+        open_review_requests = list(open_review_requests.exclude(reviewer=None))
+    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)
+
+    return open_review_requests
 
 def review_requests(request, acronym, group_type=None):
     group = get_group_or_404(acronym, group_type)
     if not group.features.has_reviews:
         raise Http404
 
-    open_review_requests = list(ReviewRequest.objects.filter(
-        team=group, state__in=("requested", "accepted")
-    ).prefetch_related("reviewer", "type", "state").order_by("-time", "-id"))
-
-    unavailable_periods = current_unavailable_periods_for_reviewers(group)
-    for review_req in open_review_requests:
-        if review_req.reviewer:
-            review_req.reviewer_unavailable = any(p.availability == "unavailable"
-                                                  for p in unavailable_periods.get(review_req.reviewer.person_id, []))
+    assigned_review_requests = []
+    unassigned_review_requests = []
 
-    open_review_requests = suggested_review_requests_for_team(group) + open_review_requests
+    for r in get_open_review_requests_for_team(group):
+        if r.reviewer:
+            assigned_review_requests.append(r)
+        else:
+            unassigned_review_requests.append(r)
 
-    today = datetime.date.today()
-    for r in open_review_requests:
-        r.due = max(0, (today - r.deadline).days)
+    open_review_requests = [
+        ("Unassigned", unassigned_review_requests),
+        ("Assigned", assigned_review_requests),
+    ]
 
     closed_review_requests = ReviewRequest.objects.filter(
         team=group,
     ).exclude(
         state__in=("requested", "accepted")
-    ).prefetch_related("reviewer", "type", "state", "doc").order_by("-time", "-id")
+    ).prefetch_related("reviewer__person", "type", "state", "doc", "result").order_by("-time", "-id")
 
     since_choices = [
         (None, "1 month"),
@@ -192,26 +214,15 @@ def __init__(self, review_req, *args, **kwargs):
 
 
 @login_required
-def manage_review_requests(request, acronym, group_type=None):
+def manage_review_requests(request, acronym, group_type=None, assignment_status=None):
     group = get_group_or_404(acronym, group_type)
     if not group.features.has_reviews:
         raise Http404
 
     if not can_manage_review_requests_for_team(request.user, group):
         return HttpResponseForbidden("You do not have permission to perform this action")
 
-    unavailable_periods = current_unavailable_periods_for_reviewers(group)
-
-    open_review_requests = list(ReviewRequest.objects.filter(
-        team=group, state__in=("requested", "accepted")
-    ).prefetch_related("reviewer", "type", "state").order_by("-time", "-id"))
-
-    for review_req in open_review_requests:
-        if review_req.reviewer:
-            review_req.reviewer_unavailable = any(p.availability == "unavailable"
-                                                  for p in unavailable_periods.get(review_req.reviewer.person_id, []))
-
-    review_requests = suggested_review_requests_for_team(group) + open_review_requests
+    review_requests = get_open_review_requests_for_team(group, assignment_status=assignment_status)
 
     document_requests = extract_revision_ordered_review_requests_for_documents_and_replaced(
         ReviewRequest.objects.filter(state__in=("part-completed", "completed"), team=group).prefetch_related("result"),
@@ -221,10 +232,12 @@ def manage_review_requests(request, acronym, group_type=None):
     # we need a mutable query dict for resetting upon saving with
     # conflicts
     query_dict = request.POST.copy() if request.method == "POST" else None
+
     for req in review_requests:
+        # add previous requests
         l = []
-        # take all on the latest reviewed rev
         for r in document_requests.get(req.doc_id, []):
+            # take all on the latest reviewed rev
             if l and l[0].reviewed_rev:
                 if r.doc_id == l[0].doc_id and r.reviewed_rev:
                     if int(r.reviewed_rev) > int(l[0].reviewed_rev):
@@ -292,18 +305,28 @@ def manage_review_requests(request, acronym, group_type=None):
                 kwargs["group_type"] = group_type
 
             if form_action == "save-continue":
+                if assignment_status:
+                    kwargs["assignment_status"] = assignment_status
+                
                 return redirect(manage_review_requests, **kwargs)
             else:
                 import ietf.group.views_review
                 return redirect(ietf.group.views_review.review_requests, **kwargs)
 
+    other_assignment_status = {
+        "unassigned": "assigned",
+        "assigned": "unassigned",
+    }.get(assignment_status)
+
     return render(request, 'group/manage_review_requests.html', {
         'group': group,
         'review_requests': review_requests,
         'newly_closed': newly_closed,
         'newly_opened': newly_opened,
         'newly_assigned': newly_assigned,
         'saving': saving,
+        'assignment_status': assignment_status,
+        'other_assignment_status': other_assignment_status,
     })
 
 class EmailOpenAssignmentsForm(forms.Form):
@@ -327,16 +350,21 @@ def email_open_review_assignments(request, acronym, group_type=None):
         reviewer=None,
     ).prefetch_related("reviewer", "type", "state", "doc").distinct().order_by("deadline", "reviewer"))
 
+    back_url = request.GET.get("next")
+    if not back_url:
+        kwargs = { "acronym": group.acronym }
+        if group_type:
+            kwargs["group_type"] = group_type
+
+        import ietf.group.views_review
+        back_url = urlreverse(ietf.group.views_review.review_requests, kwargs=kwargs)
+
     if request.method == "POST" and request.POST.get("action") == "email":
         form = EmailOpenAssignmentsForm(request.POST)
         if form.is_valid():
             send_mail_text(request, form.cleaned_data["to"], None, form.cleaned_data["subject"], form.cleaned_data["body"])
 
-            kwargs = { "acronym": group.acronym }
-            if group_type:
-                kwargs["group_type"] = group_type
-
-            return redirect(manage_review_requests, **kwargs)
+            return HttpResponseRedirect(back_url)
     else:
         to = group.list_email
         subject = "Open review assignments in {}".format(group.acronym)
@@ -356,6 +384,7 @@ def email_open_review_assignments(request, acronym, group_type=None):
         'group': group,
         'review_requests': review_requests,
         'form': form,
+        'back_url': back_url,
     })
 
 
diff --git a/ietf/templates/group/email_open_review_assignments.html b/ietf/templates/group/email_open_review_assignments.html
@@ -16,7 +16,7 @@ <h1>Email summary of assigned review requests for {{ group.acronym }}</h1>
       {% bootstrap_form form %}
 
       {% buttons %}
-        <a href="{% url "ietf.group.views_review.manage_review_requests" group_type=group.type_id acronym=group.acronym %}" class="btn btn-default pull-right">Cancel</a>
+        <a href="{{ back_url }}" class="btn btn-default pull-right">Cancel</a>
         <button class="btn btn-primary" type="submit" name="action" value="email">Send to team mailing list</button>
       {% endbuttons %}
     </form>
diff --git a/ietf/templates/group/manage_review_requests.html b/ietf/templates/group/manage_review_requests.html
@@ -13,11 +13,14 @@
 {% block content %}
   {% origin %}
 
-  <h1>Manage open review requests for {{ group.acronym }}</h1>
+  <h1>Manage {{ assignment_status }} open review requests for {{ group.acronym }}</h1>
 
   <p>Other options:
     <a href="{% url "ietf.group.views_review.reviewer_overview" group_type=group.type_id acronym=group.acronym %}">Reviewers in team</a>
-    - <a href="{% url "ietf.group.views_review.email_open_review_assignments" group_type=group.type_id acronym=group.acronym  %}">Email open assignments summary</a>
+    - <a href="{% url "ietf.group.views_review.email_open_review_assignments" group_type=group.type_id acronym=group.acronym  %}?next={{ request.get_full_path|urlencode }}">Email open assignments summary</a>
+    {% if other_assignment_status %}
+      - <a href="{% url "ietf.group.views_review.manage_review_requests" group_type=group.type_id acronym=group.acronym assignment_status=other_assignment_status %}">Manage {{ other_assignment_status }} reviews</a>
+    {% endif %}
   </p>
 
   {% if newly_closed > 0 or newly_opened > 0 or newly_assigned > 0 %}
@@ -136,7 +139,7 @@ <h1>Manage open review requests for {{ group.acronym }}</h1>
       {% endbuttons %}
     </form>
   {% else %}
-    <p>There are currently no open requests.</p>
+    <p>There are currently no {{ assignment_status }} open requests.</p>
   {% endif %}
 {% endblock %}
 
diff --git a/ietf/templates/group/review_requests.html b/ietf/templates/group/review_requests.html
@@ -17,46 +17,47 @@
     <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 %}
 
-  <h2>Open review requests</h2>
+  {% for label, review_requests in open_review_requests %}
+    {% if review_requests %}
 
-  {% if open_review_requests %}
-    <table class="table table-condensed table-striped tablesorter">
-      <thead>
-        <tr>
-          <th>Request</th>
-          <th>Type</th>
-          <th>Requested</th>
-          <th>Deadline</th>
-          <th>Reviewer</th>
-        </tr>
-      </thead>
-      <tbody>
-        {% for r in open_review_requests %}
+      <h2>{{ label }} open review requests</h2>
+
+      <table class="table table-condensed table-striped tablesorter">
+        <thead>
           <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>{% if r.pk %}{{ r.time|date:"Y-m-d" }}{% else %}<em>auto-suggested</em>{% endif %}</td>
-            <td>
-              {{ 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>
-            <td>
+            <th>Request</th>
+            <th>Type</th>
+            <th>Requested</th>
+            <th>Deadline</th>
+            {% if review_requests.0.reviewer %}
+              <th>Reviewer</th>
+            {% endif %}
+          </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>{% if r.pk %}{{ r.time|date:"Y-m-d" }}{% else %}<em>auto-suggested</em>{% endif %}</td>
+              <td>
+                {{ 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 %}
-                {{ 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 %}
-              {% elif r.pk != None %}
-                <em>not yet assigned</em>
+                <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 %}
-            </td>
-          </tr>
-        {% endfor %}
-      </tbody>
-    </table>
-    
-  {% else %}
-    <p>There are currently no open requests.</p>
-  {% endif %}
+            </tr>
+          {% endfor %}
+        </tbody>
+      </table>
+
+    {% endif %}
+  {% endfor %}
 
   <h2 id="closed-review-requests">Closed review requests</h2>
 
