fix: Add Reviews history page to review teams group page. (fixes #9190) (#9191)

kivinen · web-flow · commit ea8377db0d4b · 2025-07-28T10:18:13.000-03:00
* Add Reviews history page to review teams group page. * fix: Added test cases. Fixed html to validate, moved sort text to data-text from comments. Added test cases. fixes #9190
diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py
@@ -815,3 +815,131 @@ def test_reset_next_reviewer(self):
             self.assertEqual(NextReviewerInTeam.objects.get(team=group).next_reviewer, reviewers[target_index].person)
             self.client.logout()
             target_index += 2
+
+class RequestsHistoryTests(TestCase):
+    def test_requests_history_overview_page(self):
+        # Make assigned assignment
+        review_req = ReviewRequestFactory(state_id='assigned')
+        assignment = ReviewAssignmentFactory(review_request=review_req,
+                                             state_id='assigned',
+                                             reviewer=EmailFactory(),
+                                             assigned_on = review_req.time)
+        group = review_req.team
+
+        for url in [urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym }),
+                    urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym ,
+                                        'group_type': group.type_id}),
+                    urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym }) +
+                    '?since=3m',
+                    urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym ,
+                                        'group_type': group.type_id }) +
+                    '?since=3m']:
+            r = self.client.get(url)
+            self.assertEqual(r.status_code, 200)
+            self.assertContains(r, review_req.doc.name)
+            self.assertContains(r, 'Assigned')
+            self.assertContains(r, escape(assignment.reviewer.person.name))
+
+        url = urlreverse(ietf.group.views.review_requests_history,
+                         kwargs={ 'acronym': group.acronym })
+
+        assignment.state = ReviewAssignmentStateName.objects.get(slug="completed")
+        assignment.result = ReviewResultName.objects.get(slug="ready")
+        assignment.save()
+
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, review_req.doc.name)
+        self.assertContains(r, 'Assigned')
+        self.assertContains(r, 'Completed')
+
+    def test_requests_history_filter_page(self):
+        # First assignment as assigned
+        review_req = ReviewRequestFactory(state_id = 'assigned',
+                                          doc = DocumentFactory())
+        assignment = ReviewAssignmentFactory(review_request = review_req,
+                                             state_id = 'assigned',
+                                             reviewer = EmailFactory(),
+                                             assigned_on = review_req.time)
+        group = review_req.team
+
+        # Second assignment in same group as accepted
+        review_req2 = ReviewRequestFactory(state_id = 'assigned',
+                                           team = review_req.team,
+                                           doc = DocumentFactory())
+        assignment2 = ReviewAssignmentFactory(review_request = review_req2,
+                                              state_id='accepted',
+                                              reviewer = EmailFactory(),
+                                              assigned_on = review_req2.time)
+
+        # Modify the assignment to be completed, and mark it ready
+        assignment2.state = ReviewAssignmentStateName.objects.get(slug="completed")
+        assignment2.result = ReviewResultName.objects.get(slug="ready")
+        assignment2.save()
+
+        # Check that we have all information when we do not filter
+        url = urlreverse(ietf.group.views.review_requests_history,
+                         kwargs={ 'acronym': group.acronym })
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, review_req.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        self.assertContains(r, 'Assigned')
+        self.assertContains(r, 'Accepted')
+        self.assertContains(r, 'Completed')
+        self.assertContains(r, 'Ready')
+        self.assertContains(r, escape(assignment.reviewer.person.name))
+        self.assertContains(r, escape(assignment2.reviewer.person.name))
+
+        # Check first reviewer history
+        for url in [urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym }) +
+                    '?reviewer_email=' + str(assignment.reviewer),
+                    urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym ,
+                                        'group_type': group.type_id}) +
+                    '?reviewer_email=' + str(assignment.reviewer)]:
+            r = self.client.get(url)
+            self.assertEqual(r.status_code, 200)
+            self.assertContains(r, review_req.doc.name)
+            self.assertNotContains(r, review_req2.doc.name)
+            self.assertContains(r, 'Assigned')
+            self.assertNotContains(r, 'Accepted')
+            self.assertNotContains(r, 'Completed')
+            self.assertNotContains(r, 'Ready')
+            self.assertContains(r, escape(assignment.reviewer.person.name))
+            self.assertNotContains(r, escape(assignment2.reviewer.person.name))
+
+        # Check second reviewer history
+        for url in [urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym }) +
+                    '?reviewer_email=' + str(assignment2.reviewer),
+                    urlreverse(ietf.group.views.review_requests_history,
+                               kwargs={ 'acronym': group.acronym ,
+                                        'group_type': group.type_id}) +
+                    '?reviewer_email=' + str(assignment2.reviewer)]:
+            r = self.client.get(url)
+            self.assertEqual(r.status_code, 200)
+            self.assertNotContains(r, review_req.doc.name)
+            self.assertContains(r, review_req2.doc.name)
+            self.assertNotContains(r, 'Assigned')
+            self.assertContains(r, 'Accepted')
+            self.assertContains(r, 'Completed')
+            self.assertContains(r, 'Ready')
+            self.assertNotContains(r, escape(assignment.reviewer.person.name))
+            self.assertContains(r, escape(assignment2.reviewer.person.name))
+
+        # Check for reviewer that does not have anything
+        url = urlreverse(ietf.group.views.review_requests_history,
+                         kwargs={ 'acronym': group.acronym }) + '?reviewer_email=nobody@nowhere.example.org'
+
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertNotContains(r, review_req.doc.name)
+        self.assertNotContains(r, 'Assigned')
+        self.assertNotContains(r, 'Accepted')
+        self.assertNotContains(r, 'Completed')
diff --git a/ietf/group/urls.py b/ietf/group/urls.py
@@ -24,6 +24,7 @@
     url(r'^about/status/edit/$', views.group_about_status_edit),
     url(r'^about/status/meeting/(?P<num>\d+)/$', views.group_about_status_meeting),
     url(r'^history/$',views.history),
+    url(r'^requestshistory/$',views.review_requests_history),
     url(r'^history/addcomment/$',views.add_comment),
     url(r'^email/$', views.email),
     url(r'^deps\.json$', views.dependencies),
diff --git a/ietf/group/utils.py b/ietf/group/utils.py
@@ -236,6 +236,7 @@ def construct_group_menu_context(request, group, selected, group_type, others):
         import ietf.group.views
         entries.append(("Review requests", urlreverse(ietf.group.views.review_requests, kwargs=kwargs)))
         entries.append(("Reviewers", urlreverse(ietf.group.views.reviewer_overview, kwargs=kwargs)))
+        entries.append(("Reviews History", urlreverse(ietf.group.views.review_requests_history, kwargs=kwargs)))
 
     if group.features.has_meetings:
         entries.append(("Meetings", urlreverse("ietf.group.views.meetings", kwargs=kwargs)))
diff --git a/ietf/group/views.py b/ietf/group/views.py
@@ -686,6 +686,56 @@ def history(request, acronym, group_type=None):
                       "can_add_comment": can_add_comment,
                   }))
 
+def review_requests_history(request, acronym, group_type=None):
+    group = get_group_or_404(acronym, group_type)
+    if not group.features.has_reviews:
+        raise Http404
+
+    reviewer_email = request.GET.get("reviewer_email", None)
+
+    if reviewer_email:
+        history = ReviewAssignment.history.model.objects.filter(
+            review_request__team__acronym=acronym,
+            reviewer=reviewer_email)
+    else:
+        history = ReviewAssignment.history.model.objects.filter(
+            review_request__team__acronym=acronym)
+        reviewer_email = ''
+        
+    since_choices = [
+        (None, "1 month"),
+        ("3m", "3 months"),
+        ("6m", "6 months"),
+        ("1y", "1 year"),
+        ("2y", "2 years"),
+        ("all", "All"),
+    ]
+    since = request.GET.get("since", None)
+    
+    if since not in [key for key, label in since_choices]:
+        since = None
+
+    if since != "all":
+        date_limit = {
+            None: datetime.timedelta(days=31),
+            "3m": datetime.timedelta(days=31 * 3),
+            "6m": datetime.timedelta(days=180),
+            "1y": datetime.timedelta(days=365),
+            "2y": datetime.timedelta(days=2 * 365),
+        }[since]
+
+        history = history.filter(review_request__time__gte=datetime_today(DEADLINE_TZINFO) - date_limit)
+
+    return render(request, 'group/review_requests_history.html',
+                  construct_group_menu_context(request, group, "reviews history", group_type, {
+                      "group": group,
+                      "acronym": acronym,
+                      "history": history,
+                      "since_choices": since_choices,
+                      "since": since,
+                      "reviewer_email": reviewer_email
+                  }))
+
 def materials(request, acronym, group_type=None):
     group = get_group_or_404(acronym, group_type)
     if not group.features.has_nonsession_materials:
diff --git a/ietf/templates/group/review_requests_history.html b/ietf/templates/group/review_requests_history.html
@@ -0,0 +1,90 @@
+{% extends "group/group_base.html" %}
+{# Copyright The IETF Trust 2015, All Rights Reserved #}
+{% load origin %}
+{% load tz %}
+{% load ietf_filters person_filters textfilters %}
+{% load static %}
+{% block pagehead %}
+    <link rel="stylesheet" href="{% static "ietf/css/list.css" %}">
+{% endblock %}
+{% block group_content %}
+    {% origin %}
+    {% if reviewer_email %}
+    <h2 class="my-3">Review requests history of {{ reviewer_email }}</h2>
+    {% else %}
+    <h2 class="my-3">Review requests history</h2>
+    {% endif %}
+    <form class="reviews-history-filter" action="#reviewshistory">
+      <div>
+	<label for="reviewer_email"
+	       class="query-input-label">Reviewer filter:</label>
+	<input type="text" class="query-input" id="reviewer_email"
+	       value="{{ reviewer_email }}" name="reviewer_email">
+	<button class="btn btn-primary" type="submit">Filter</button>
+      </div>
+      <div>
+	Past:
+	<div class="btn-group btn-group-sm" role="group">
+          {% for key, label in since_choices %}
+          <button class="btn btn-outline-primary {% if since == key %}active{% endif %}"
+                  {% if key %}name="since" value="{{ key }}"{% endif %}
+                  type="submit">
+            {{ label }}
+          </button>
+          {% endfor %}
+	</div>
+      </div>
+    </form>
+
+    <table class="table table-sm table-striped tablesorter">
+        <thead>
+            <tr>
+                <th scope="col" data-sort="date">Date (UTC)</th>
+                <th scope="col" data-sort="by">By</th>
+                <th scope="col" data-sort="doc">Document</th>
+                <th scope="col" data-sort="state">State</th>
+                <th scope="col" data-sort="review">Reviewer</th>
+                <th scope="col" data-sort="result">Result</th>
+                <th scope="col" data-sort="description">Description</th>
+            </tr>
+        </thead>
+        {% if history %}
+            <tbody>
+                {% for h in history %}
+                    <tr>
+                        <td>{{ h.history_date|utc|date:"Y-m-d H:i:s" }}</td>
+                        <td>{% person_link h.history_user.person %}</td>
+                        <td>{% if h.reviewed_rev %}
+			  <a href="{% url "ietf.doc.views_doc.document_main" name=h.review_request.doc.name rev=h.reviewed_rev %}">
+                        {{ h.review_request.doc.name }}-{{ h.reviewed_rev }}
+			  </a>
+			  {% else %}
+			  <a href="{% url "ietf.doc.views_doc.document_main" name=h.review_request.doc.name %}">{{ h.review_request.doc.name }}</a>
+			  {% endif %}
+			</td>
+                        <td data-text="{{ h.state }}">
+			  <a href="{% url "ietf.doc.views_review.review_request" name=h.review_request.doc.name request_id=h.review_request.pk %}">{{ h.state }} </a>
+			</td>
+                        <td data-text="{{ h.reviewer }}">
+			  {% person_link h.reviewer.person %}
+			  <a href="?reviewer_email={{h.reviewer.email_address}}#reviewshistory">
+			    (set&nbsp;as&nbsp;filter)
+			  </a>
+			</td>
+                        <td data-text="{{ h.result }}">
+			  {% if h.review %}<a href="{{ h.review.get_absolute_url }}">
+			    {{ h.result }}</a>
+			  {% else %}
+			  {{ h.result }}
+			  {% endif %}
+			</td>
+                        <td>{{ h.history_change_reason }}</td>
+                    </tr>
+                {% endfor %}
+            </tbody>
+        {% endif %}
+    </table>
+{% endblock %}
+{% block js %}
+    <script src="{% static "ietf/js/list.js" %}"></script>
+{% endblock %}
