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

Restrict editing ReviewSettings.skip_next to team secretaries and the secretariat. Improve validation of skip_next value. Fixes #2149. Commit ready for merge.

Browse source 
- Legacy-Id: 12706
This commit is contained in:
Robert Sparks 2017-01-19 19:17:29 +00:00
parent 4f2f691de6
commit 518f7a1d65
2 changed files with 47 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
ietf/group/tests_review.py
View file

@ -373,14 +373,13 @@ class ReviewTests(TestCase):
"action": "change_settings",
"min_interval": "7",
"filter_re": "test-[regexp]",
"skip_next": "2",
"remind_days_before_deadline": "6"
})
self.assertEqual(r.status_code, 302)
settings = ReviewerSettings.objects.get(person=reviewer, team=review_req.team)
self.assertEqual(settings.min_interval, 7)
self.assertEqual(settings.filter_re, "test-[regexp]")
self.assertEqual(settings.skip_next, 2)
self.assertEqual(settings.skip_next, 0)
self.assertEqual(settings.remind_days_before_deadline, 6)
self.assertEqual(len(outbox), 1)
self.assertTrue("reviewer availability" in outbox[0]["subject"].lower())
@ -388,6 +387,18 @@ class ReviewTests(TestCase):
self.assertTrue("frequency changed", msg_content)
self.assertTrue("skip next", msg_content)
# Normal reviewer should not be able to change skip_next
r = self.client.post(url, {
"action": "change_settings",
"min_interval": "7",
"filter_re": "test-[regexp]",
"remind_days_before_deadline": "6",
"skip_next" : "2",
})
self.assertEqual(r.status_code, 302)
settings = ReviewerSettings.objects.get(person=reviewer, team=review_req.team)
self.assertEqual(settings.skip_next, 0)
# add unavailable period
start_date = datetime.date.today() + datetime.timedelta(days=10)
empty_outbox()
@ -435,6 +446,22 @@ class ReviewTests(TestCase):
self.assertTrue(start_date.isoformat(), msg_content)
self.assertTrue(end_date.isoformat(), msg_content)
# secretaries and the secretariat should be able to change skip_next
for username in ["secretary","reviewsecretary"]:
skip_next_val = {'secretary':'3','reviewsecretary':'4'}[username]
self.client.login(username=username,password=username+"+password")
r = self.client.post(url, {
"action": "change_settings",
"min_interval": "7",
"filter_re": "test-[regexp]",
"remind_days_before_deadline": "6",
"skip_next" : skip_next_val,
})
self.assertEqual(r.status_code, 302)
settings = ReviewerSettings.objects.get(person=reviewer, team=review_req.team)
self.assertEqual(settings.skip_next, int(skip_next_val))
def test_change_review_secretary_settings(self):
doc = make_test_data()

20
ietf/group/views_review.py
View file

@ -448,6 +448,18 @@ class ReviewerSettingsForm(forms.ModelForm):
model = ReviewerSettings
fields = ['min_interval', 'filter_re', 'skip_next', 'remind_days_before_deadline']
def __init__(self, *args, **kwargs):
exclude_fields = kwargs.pop('exclude_fields', [])
super(ReviewerSettingsForm, self).__init__(*args, **kwargs)
for field_name in exclude_fields:
self.fields.pop(field_name)
def clean_skip_next(self):
skip_next = self.cleaned_data.get('skip_next')
if skip_next < 0:
raise forms.ValidationError("Skip next must not be negative")
return skip_next
class AddUnavailablePeriodForm(forms.ModelForm):
class Meta:
model = UnavailablePeriod
@ -496,6 +508,10 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None):
or can_manage_review_requests_for_team(request.user, group)):
return HttpResponseForbidden("You do not have permission to perform this action")
exclude_fields = []
if not can_manage_review_requests_for_team(request.user, group):
exclude_fields.append('skip_next')
settings = ReviewerSettings.objects.filter(person=reviewer, team=group).first()
if not settings:
settings = ReviewerSettings(person=reviewer, team=group)
@ -513,7 +529,7 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None):
if request.method == "POST" and request.POST.get("action") == "change_settings":
prev_min_interval = settings.get_min_interval_display()
prev_skip_next = settings.skip_next
settings_form = ReviewerSettingsForm(request.POST, instance=settings)
settings_form = ReviewerSettingsForm(request.POST, instance=settings, exclude_fields=exclude_fields)
if settings_form.is_valid():
settings = settings_form.save()
@ -528,7 +544,7 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None):
return HttpResponseRedirect(back_url)
else:
settings_form = ReviewerSettingsForm(instance=settings)
settings_form = ReviewerSettingsForm(instance=settings,exclude_fields=exclude_fields)
# periods
unavailable_periods = unavailable_periods_to_list().filter(person=reviewer, team=group)