altf4arnold/datatracker
1
0
Fork 0
Code Issues Pull requests Actions 3 Packages Projects Releases Wiki Activity

Added two new configuration settings for the review team secretary,

Browse source 
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.
Commit ready for merge.
 - Legacy-Id: 17034
This commit is contained in:
Tero Kivinen 2019-11-16 09:41:55 +00:00
parent 996fcf8a54
commit b5d864499f
6 changed files with 209 additions and 7 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
ietf/group/forms.py
View file

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

157
ietf/group/tests_review.py
View file

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

26
ietf/group/views.py
View file

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

2
ietf/review/admin.py
View file

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

26
ietf/review/migrations/0020_auto_20191115_2059.py Normal file
View file

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

2
ietf/review/models.py
View file

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