diff --git a/ietf/group/forms.py b/ietf/group/forms.py index ead477ac5..ac486aefa 100644 --- a/ietf/group/forms.py +++ b/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'] diff --git a/ietf/group/tests_review.py b/ietf/group/tests_review.py index 296f99e7d..fb45a3862 100644 --- a/ietf/group/tests_review.py +++ b/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') diff --git a/ietf/group/views.py b/ietf/group/views.py index e757709f1..ffe8cdb28 100644 --- a/ietf/group/views.py +++ b/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)) diff --git a/ietf/review/admin.py b/ietf/review/admin.py index b540e75cd..290556833 100644 --- a/ietf/review/admin.py +++ b/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) diff --git a/ietf/review/migrations/0020_auto_20191115_2059.py b/ietf/review/migrations/0020_auto_20191115_2059.py new file mode 100644 index 000000000..ccf041800 --- /dev/null +++ b/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), + ), + ] diff --git a/ietf/review/models.py b/ietf/review/models.py index a701e6f0d..d57c14da4 100644 --- a/ietf/review/models.py +++ b/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)