Merged in [17034] from kivinen@iki.fi:

Added two new configuration settings for the review team secretary,
one to set how many days to include in the reviewers list, and
another one to limit the number of completed items in the list for
each person. This version replaces the one I did earlier, and includes
much more test cases to test different limits on the reviewers page.
 - Legacy-Id: 17057
Note: SVN reference [17034] has been migrated to Git commit b5d864499f

ietf/group/forms.py

@@ -334,6 +334,7 @@ class EndUnavailablePeriodForm(forms.Form):
 class ReviewSecretarySettingsForm(forms.ModelForm):
     class Meta:
         model = ReviewSecretarySettings
-        fields = ['remind_days_before_deadline']
+        fields = ['remind_days_before_deadline', 'max_items_to_show_in_reviewer_list',
+                  'days_to_show_in_reviewer_list']
 
 

ietf/group/tests_review.py

@@ -156,6 +156,7 @@ class ReviewTests(TestCase):
         reviewer = RoleFactory(name_id='reviewer',group=team,person__user__username='reviewer').person
         ReviewerSettingsFactory(person=reviewer,team=team)
         review_req1 = ReviewRequestFactory(state_id='completed',team=team)
+        secretary = Person.objects.get(user__username="secretary")
         ReviewAssignmentFactory(review_request = review_req1, reviewer=reviewer.email())
         PersonFactory(user__username='plain')
 
@@ -212,6 +213,156 @@ class ReviewTests(TestCase):
         # secretariat can see reason for being unavailable
         self.assertContains(r, "Availability")
 
+        # add one closed review with no response and see it is visible
+        review_req2 = ReviewRequestFactory(state_id='completed',team=team)
+        ReviewAssignmentFactory(
+            review_request__doc=review_req2.doc,
+            review_request__team=review_req2.team,
+            review_request__type_id="lc",
+            review_request__deadline=datetime.date.today() - datetime.timedelta(days=30),
+            review_request__state_id="assigned",
+            review_request__requested_by=Person.objects.get(user__username="reviewer"),
+            state_id = "no-response",
+            reviewer=reviewer.email_set.first(),
+        )
+        self.client.login(username="secretary", password="secretary+password")
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        # We should see the new document with status of no response
+        self.assertContains(r, "No Response")
+        self.assertContains(r, review_req1.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        # None of the reviews should be completed this time,
+        # note that "Days Since Completed has soft hypens in it, so it
+        # will not match
+        self.assertNotContains(r, "Completed")
+        # add multiple completed reviews
+        review_req3 = ReviewRequestFactory(state_id='completed', team=team)
+        ReviewAssignmentFactory(
+            review_request__doc=review_req3.doc,
+            review_request__time=datetime.date.today() - datetime.timedelta(days=30),
+            review_request__team=review_req3.team,
+            review_request__type_id="telechat",
+            review_request__deadline=datetime.date.today() - datetime.timedelta(days=25),
+            review_request__state_id="completed",
+            review_request__requested_by=Person.objects.get(user__username="reviewer"),
+            state_id = "completed",
+            reviewer=reviewer.email_set.first(),
+            assigned_on=datetime.date.today() - datetime.timedelta(days=30)
+        )
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, "Completed")
+        self.assertContains(r, review_req1.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        self.assertContains(r, review_req3.doc.name)
+        # Few more to make sure we hit the limit
+        reqs = [ReviewRequestFactory(state_id='completed', team=team) for i in range(10)]
+        for i in range(10):
+            ReviewAssignmentFactory(
+                review_request__doc=reqs[i].doc,
+                review_request__time=datetime.date.today() - datetime.timedelta(days=i*30),
+                review_request__team=reqs[i].team,
+                review_request__type_id="telechat",
+                review_request__deadline=datetime.date.today() - datetime.timedelta(days=i*20),
+                review_request__state_id="completed",
+                review_request__requested_by=Person.objects.get(user__username="reviewer"),
+                state_id = "completed",
+                reviewer=reviewer.email_set.first(),
+                assigned_on=datetime.date.today() - datetime.timedelta(days=i*30)
+            )
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, "Completed")
+        self.assertContains(r, review_req1.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        self.assertContains(r, review_req3.doc.name)
+        # The default is 10 completed entries, we had one before + one No response
+        # so we should have 8 more here
+        for i in range(10):
+            if i < 8:
+                self.assertContains(r, reqs[i].doc.name)
+            else:
+                self.assertNotContains(r, reqs[i].doc.name)
+        # Change the limit to be 15 instead of 10, so we should get all
+        secretary_settings = ReviewSecretarySettings(team=team, person=secretary)
+        secretary_settings.max_items_to_show_in_reviewer_list = 15
+        secretary_settings.save()
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, review_req1.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        self.assertContains(r, review_req3.doc.name)
+        for i in range(10):
+            self.assertContains(r, reqs[i].doc.name)
+        # Change the time limit to 100 days, so we should only get 3 completed
+        secretary_settings.days_to_show_in_reviewer_list = 100
+        secretary_settings.save()
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, review_req1.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        self.assertContains(r, review_req3.doc.name)
+        for i in range(10):
+            if i < 4:
+                self.assertContains(r, reqs[i].doc.name)
+            else:
+                self.assertNotContains(r, reqs[i].doc.name)
+        # Add assigned items, they should be visible as long as they
+        # are withing time period
+        review_req4 = ReviewRequestFactory(state_id='completed', team=team)
+        ReviewAssignmentFactory(
+            review_request__doc=review_req4.doc,
+            review_request__time=datetime.date.today() - datetime.timedelta(days=80),
+            review_request__team=review_req4.team,
+            review_request__type_id="lc",
+            review_request__deadline=datetime.date.today() - datetime.timedelta(days=60),
+            review_request__state_id="assigned",
+            review_request__requested_by=Person.objects.get(user__username="reviewer"),
+            state_id = "accepted",
+            reviewer=reviewer.email_set.first(),
+            assigned_on=datetime.date.today() - datetime.timedelta(days=80)
+        )
+        review_req5 = ReviewRequestFactory(state_id='completed', team=team)
+        ReviewAssignmentFactory(
+            review_request__doc=review_req5.doc,
+            review_request__time=datetime.date.today() - datetime.timedelta(days=120),
+            review_request__team=review_req5.team,
+            review_request__type_id="lc",
+            review_request__deadline=datetime.date.today() - datetime.timedelta(days=100),
+            review_request__state_id="assigned",
+            review_request__requested_by=Person.objects.get(user__username="reviewer"),
+            state_id = "accepted",
+            reviewer=reviewer.email_set.first(),
+            assigned_on=datetime.date.today() - datetime.timedelta(days=120)
+        )
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, review_req1.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        self.assertContains(r, review_req3.doc.name)
+        self.assertContains(r, review_req4.doc.name)
+        self.assertNotContains(r, review_req5.doc.name)
+        for i in range(10):
+            if i < 4:
+                self.assertContains(r, reqs[i].doc.name)
+            else:
+                self.assertNotContains(r, reqs[i].doc.name)
+        # Change the limit to be 1 instead of 10, so we should only one entry
+        secretary_settings.max_items_to_show_in_reviewer_list = 1
+        secretary_settings.save()
+        r = self.client.get(url)
+        self.assertEqual(r.status_code, 200)
+        self.assertContains(r, review_req1.doc.name)
+        self.assertContains(r, review_req2.doc.name)
+        self.assertNotContains(r, review_req3.doc.name)
+        for i in range(10):
+            self.assertNotContains(r, reqs[i].doc.name)
+        self.assertContains(r, review_req4.doc.name)
+        self.assertNotContains(r, review_req5.doc.name)
+
+        # print(r.content)
+
     def test_manage_review_requests(self):
         group = ReviewTeamFactory()
         RoleFactory(name_id='reviewer',group=group,person__user__username='reviewer').person
@@ -539,11 +690,15 @@ class ReviewTests(TestCase):
 
         # set settings
         r = self.client.post(url, {
-            "remind_days_before_deadline": "6"
+            "remind_days_before_deadline": "6",
+            "max_items_to_show_in_reviewer_list": 10,
+            "days_to_show_in_reviewer_list": 365
         })
         self.assertEqual(r.status_code, 302)
         settings = ReviewSecretarySettings.objects.get(person=secretary, team=review_req.team)
         self.assertEqual(settings.remind_days_before_deadline, 6)
+        self.assertEqual(settings.max_items_to_show_in_reviewer_list, 10)
+        self.assertEqual(settings.days_to_show_in_reviewer_list, 365)
 
     def test_review_reminders(self):
         review_req = ReviewRequestFactory(state_id='assigned')

ietf/group/views.py

@@ -1402,7 +1402,24 @@ def reviewer_overview(request, acronym, group_type=None):
 
     today = datetime.date.today()
 
-    req_data_for_reviewers = latest_review_assignments_for_reviewers(group)
+    max_closed_reqs = 10
+    days_back = 365
+    if can_manage:
+        secretary_settings = (ReviewSecretarySettings.objects.filter(person=
+                                                                     request.user.person,
+                                                                     team=group).first()
+                              or ReviewSecretarySettings(person=request.user.person,
+                                                         team=group))
+        if secretary_settings:
+            max_closed_reqs = secretary_settings.max_items_to_show_in_reviewer_list
+            days_back = secretary_settings.days_to_show_in_reviewer_list
+
+    if max_closed_reqs == None:
+        max_closed_reqs = 10
+
+    if days_back == None:
+        days_back = 365
+    req_data_for_reviewers = latest_review_assignments_for_reviewers(group, days_back)
     assignment_state_by_slug = { n.slug: n for n in ReviewAssignmentStateName.objects.all() }
 
     days_needed = days_needed_to_fulfill_min_interval_for_reviewers(group)
@@ -1424,13 +1441,14 @@ def reviewer_overview(request, acronym, group_type=None):
             person.busy = person.id in days_needed 
         
 
-        MAX_CLOSED_REQS = 10 # This keeps the overview page with being filled with too many closed requests, since the focus should be on open or recently closed per reviewer
         days_since = 9999
         req_data = req_data_for_reviewers.get(person.pk, [])
-        open_reqs = sum(1 for d in req_data if d.state in ["assigned", "accepted"])
+        closed_reqs = 0
         latest_reqs = []
         for d in req_data:
-            if d.state in ["assigned", "accepted"] or len(latest_reqs) < MAX_CLOSED_REQS + open_reqs:
+            if d.state in ["assigned", "accepted"] or closed_reqs < max_closed_reqs:
+                if not d.state in ["assigned", "accepted"]:
+                    closed_reqs += 1
                 latest_reqs.append((d.assignment_pk, d.request_pk, d.doc_name, d.reviewed_rev, d.assigned_time, d.deadline,
                                     assignment_state_by_slug.get(d.state),
                                     int(math.ceil(d.assignment_to_closure_days)) if d.assignment_to_closure_days is not None else None))

ietf/review/admin.py

@@ -23,7 +23,7 @@ class ReviewerSettingsAdmin(simple_history.admin.SimpleHistoryAdmin):
 admin.site.register(ReviewerSettings, ReviewerSettingsAdmin)
 
 class ReviewSecretarySettingsAdmin(admin.ModelAdmin):
-    list_display = ['id', 'team', 'person', 'remind_days_before_deadline']
+    list_display = ['id', 'team', 'person', 'remind_days_before_deadline', 'max_items_to_show_in_reviewer_list', 'days_to_show_in_reviewer_list']
     raw_id_fields = ['team', 'person']
 admin.site.register(ReviewSecretarySettings, ReviewSecretarySettingsAdmin)
 

ietf/review/migrations/0020_auto_20191115_2059.py

@@ -0,0 +1,26 @@
+# Copyright The IETF Trust 2019, All Rights Reserved
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.26 on 2019-11-15 20:59
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('review', '0019_auto_20191023_0829'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='reviewsecretarysettings',
+            name='days_to_show_in_reviewer_list',
+            field=models.IntegerField(blank=True, help_text='Maximum number of days to show in reviewer list for completed items.', null=True),
+        ),
+        migrations.AddField(
+            model_name='reviewsecretarysettings',
+            name='max_items_to_show_in_reviewer_list',
+            field=models.IntegerField(blank=True, help_text='Maximum number of completed items to show for one reviewer in the reviewer list view, the list is also filtered by the days to show in reviews list setting.', null=True),
+        ),
+    ]

ietf/review/models.py

@@ -52,6 +52,8 @@ class ReviewSecretarySettings(models.Model):
     team        = ForeignKey(Group, limit_choices_to=~models.Q(reviewteamsettings=None))
     person      = ForeignKey(Person)
     remind_days_before_deadline = models.IntegerField(null=True, blank=True, help_text="To get an email reminder in case a reviewer forgets to do an assigned review, enter the number of days before review deadline you want to receive it. Clear the field if you don't want a reminder.")
+    max_items_to_show_in_reviewer_list = models.IntegerField(null=True, blank=True, help_text="Maximum number of completed items to show for one reviewer in the reviewer list view, the list is also filtered by the days to show in reviews list setting.")
+    days_to_show_in_reviewer_list = models.IntegerField(null=True, blank=True, help_text="Maximum number of days to show in reviewer list for completed items.")
 
     def __str__(self):
         return "{} in {}".format(self.person, self.team)