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

Change default of ReviewerSettings.min_interval to null - if it's not

Browse source 
specified for a reviewer, we don't take it into account
 - Legacy-Id: 12168
This commit is contained in:
Ole Laursen 2016-10-18 10:36:42 +00:00
parent 78e4fa623e
commit d0877a0aa9
7 changed files with 43 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

32
ietf/doc/tests_review.py
View file

@ -136,7 +136,9 @@ class ReviewTests(TestCase):
self.assertEqual(len(outbox), 1)
self.assertTrue("closed" in unicode(outbox[0]).lower())
def make_data_for_rotation_tests(self, doc):
def test_possibly_advance_next_reviewer_for_team(self):
doc = make_test_data()
team = Group.objects.create(state_id="active", acronym="rotationteam", name="Review Team", type_id="dir",
list_email="rotationteam@ietf.org", parent=Group.objects.get(acronym="farfut"))
@ -148,28 +150,21 @@ class ReviewTests(TestCase):
self.assertEqual(reviewers, reviewer_rotation_list(team))
return team, reviewers
def test_possibly_advance_next_reviewer_for_team(self):
doc = make_test_data()
team, reviewers = self.make_data_for_rotation_tests(doc)
def get_skip_next(person):
settings = (ReviewerSettings.objects.filter(team=team, person=person).first()
or ReviewerSettings(team=team))
return settings.skip_next
possibly_advance_next_reviewer_for_team(team, reviewers[0].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[0].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[1])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
possibly_advance_next_reviewer_for_team(team, reviewers[1].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[1].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
# skip reviewer 2
possibly_advance_next_reviewer_for_team(team, reviewers[3].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[3].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
@ -177,7 +172,7 @@ class ReviewTests(TestCase):
self.assertEqual(get_skip_next(reviewers[3]), 1)
# pick reviewer 2, use up reviewer 3's skip_next
possibly_advance_next_reviewer_for_team(team, reviewers[2].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[2].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
@ -186,7 +181,7 @@ class ReviewTests(TestCase):
self.assertEqual(get_skip_next(reviewers[4]), 0)
# check wrap-around
possibly_advance_next_reviewer_for_team(team, reviewers[4].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[4].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[0])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
@ -197,7 +192,7 @@ class ReviewTests(TestCase):
# unavailable
today = datetime.date.today()
UnavailablePeriod.objects.create(team=team, person=reviewers[1], start_date=today, end_date=today, availability="unavailable")
possibly_advance_next_reviewer_for_team(team, reviewers[0].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[0].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 0)
@ -206,7 +201,7 @@ class ReviewTests(TestCase):
self.assertEqual(get_skip_next(reviewers[4]), 0)
# pick unavailable anyway
possibly_advance_next_reviewer_for_team(team, reviewers[1].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[1].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[2])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1)
@ -214,9 +209,12 @@ class ReviewTests(TestCase):
self.assertEqual(get_skip_next(reviewers[3]), 0)
self.assertEqual(get_skip_next(reviewers[4]), 0)
# not through min_interval
# not through min_interval so advance past reviewer[2]
settings, _ = ReviewerSettings.objects.get_or_create(team=team, person=reviewers[2])
settings.min_interval = 30
settings.save()
ReviewRequest.objects.create(team=team, doc=doc, type_id="early", state_id="accepted", deadline=today, requested_by=reviewers[0], reviewer=reviewers[2].email_set.first())
possibly_advance_next_reviewer_for_team(team, reviewers[3].pk)
possibly_advance_next_reviewer_for_team(team, assigned_review_to_person_id=reviewers[3].pk)
self.assertEqual(NextReviewerInTeam.objects.get(team=team).next_reviewer, reviewers[4])
self.assertEqual(get_skip_next(reviewers[0]), 0)
self.assertEqual(get_skip_next(reviewers[1]), 1)

2
ietf/group/views_review.py
View file

@ -429,7 +429,7 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None):
changes = []
if settings.get_min_interval_display() != prev_min_interval:
changes.append("Frequency changed to \"{}\" from \"{}\".".format(settings.get_min_interval_display(), prev_min_interval))
changes.append("Frequency changed to \"{}\" from \"{}\".".format(settings.get_min_interval_display() or "Not specified", prev_min_interval or "Not specified"))
if settings.skip_next != prev_skip_next:
changes.append("Skip next assignments changed to {} from {}.".format(settings.skip_next, prev_skip_next))

13
ietf/review/import_from_review_tool.py
View file

@ -114,22 +114,23 @@ with db_con.cursor() as c:
if created:
print "created role", unicode(role).encode("utf-8")
reviewer, created = ReviewerSettings.objects.get_or_create(
reviewer_settings, created = ReviewerSettings.objects.get_or_create(
team=team,
person=email.person,
)
if created:
print "created reviewer", reviewer.pk, unicode(reviewer).encode("utf-8")
print "created reviewer settings", reviewer_settings.pk, unicode(reviewer_settings).encode("utf-8")
reviewer_settings.min_interval = None
if autopolicy_days.get(row.autopolicy):
reviewer.min_interval = autopolicy_days.get(row.autopolicy)
reviewer_settings.min_interval = autopolicy_days.get(row.autopolicy)
reviewer.filter_re = row.donotassign
reviewer_settings.filter_re = row.donotassign
try:
reviewer.skip_next = int(row.autopolicy)
reviewer_settings.skip_next = int(row.autopolicy)
except ValueError:
pass
reviewer.save()
reviewer_settings.save()
unavailable_until = parse_timestamp(row.available)
if unavailable_until:

2
ietf/review/models.py
View file

@ -20,7 +20,7 @@ class ReviewerSettings(models.Model):
(61, "Once per two months"),
(91, "Once per quarter"),
]
min_interval = models.IntegerField(default=30, verbose_name="Can review at most", choices=INTERVALS)
min_interval = models.IntegerField(verbose_name="Can review at most", choices=INTERVALS, blank=True, null=True)
filter_re = models.CharField(max_length=255, verbose_name="Filter regexp", blank=True, help_text="Draft names matching regular expression should not be assigned")
skip_next = models.IntegerField(default=0, verbose_name="Skip next assignments")
remind_days_before_deadline = models.IntegerField(null=True, blank=True, help_text="To get an email reminder in case you forget to do an assigned review, enter the number of days before a review deadline you want to receive it. Clear the field if you don't want a reminder.")

14
ietf/review/utils.py
View file

@ -110,7 +110,7 @@ def reviewer_rotation_list(team, skip_unavailable=False, dont_skip=[]):
days_needed_for_reviewers = days_needed_to_fulfill_min_interval_for_reviewers(team)
for person_id, days_needed in days_needed_for_reviewers.iteritems():
if days_needed > 0 and person_id not in dont_skip:
if person_id not in dont_skip:
reviewers_to_skip.add(person_id)
rotation_list = [p.pk for p in rotation_list if p.pk not in reviewers_to_skip]
@ -118,25 +118,25 @@ def reviewer_rotation_list(team, skip_unavailable=False, dont_skip=[]):
return rotation_list
def days_needed_to_fulfill_min_interval_for_reviewers(team):
"""Returns person_id -> days needed until min_interval is fulfilled for
reviewer."""
"""Returns person_id -> days needed until min_interval is fulfilled
for reviewer (in case it is necessary to wait, otherwise reviewer
is absent in result)."""
latest_assignments = dict(ReviewRequest.objects.filter(
team=team,
).values_list("reviewer__person").annotate(Max("time")))
min_intervals = dict(ReviewerSettings.objects.filter(team=team).values_list("person_id", "min_interval"))
default_min_interval = ReviewerSettings(team=team).min_interval
now = datetime.datetime.now()
res = {}
for person_id, latest_assignment_time in latest_assignments.iteritems():
if latest_assignment_time is not None:
min_interval = min_intervals.get(person_id, default_min_interval)
min_interval = min_intervals.get(person_id)
if min_interval is None:
continue
days_needed = max(0, min_interval - (now - latest_assignment_time).days)
if days_needed > 0:
res[person_id] = days_needed

13
ietf/templates/group/reviewer_overview.html
View file

@ -41,8 +41,17 @@
</table>
</td>
<td>
{{ person.settings.get_min_interval_display }} {% if person.settings.skip_next %}(skip: {{ person.settings.skip_next }}){% endif %}<br>
{% if person.settings.filter_re %}Filter: <code title="{{ person.settings.filter_re }}">{{ person.settings.filter_re|truncatechars:15 }}</code><br>{% endif %}
{% if person.settings.min_interval %}
{{ person.settings.get_min_interval_display }}<br>
{% endif %}
{% if person.settings.skip_next %}
Skip: {{ person.settings.skip_next }}<br>
{% endif %}
{% if person.settings.filter_re %}
Filter: <code title="{{ person.settings.filter_re }}">{{ person.settings.filter_re|truncatechars:15 }}</code><br>
{% endif %}
{% if person.unavailable_periods %}
{% include "review/unavailable_table.html" with unavailable_periods=person.unavailable_periods %}

2
ietf/templates/ietfauth/review_overview.html
View file

@ -126,7 +126,7 @@
<table class="table">
<tr>
<th>Can review</th>
<td>{{ t.reviewer_settings.get_min_interval_display }}</td>
<td>{{ t.reviewer_settings.get_min_interval_display|default:"No max frequency set" }}</td>
</tr>
<tr>
<th>Skip next assignments</th>