Skip to content

Commit ceec125

Browse files
committed
Add review team secretary reminders, like those for reviewers
- Legacy-Id: 12283
1 parent a5c70ff commit ceec125

11 files changed

Lines changed: 222 additions & 21 deletions
Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,18 @@ import django
1818
django.setup()
1919

2020
import datetime
21-
from ietf.review.utils import review_requests_needing_reviewer_reminder, email_reviewer_reminder
21+
from ietf.review.utils import (
22+
review_requests_needing_reviewer_reminder, email_reviewer_reminder,
23+
review_requests_needing_secretary_reminder, email_secretary_reminder,
24+
)
2225

23-
for review_req in review_requests_needing_reviewer_reminder(datetime.date.today()):
26+
today = datetime.date.today()
27+
28+
for review_req in review_requests_needing_reviewer_reminder(today):
2429
email_reviewer_reminder(review_req)
2530
print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_req.reviewer.address, review_req.doc_id, review_req.team.acronym, review_req.pk))
31+
32+
for review_req, secretary_role in review_requests_needing_secretary_reminder(today):
33+
email_secretary_reminder(review_req, secretary_role)
34+
print("Emailed reminder to {} for review of {} in {} (req. id {})".format(review_req.secretary_role.email.address, review_req.doc_id, review_req.team.acronym, review_req.pk))
35+

ietf/group/tests_review.py

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,15 @@
77
from ietf.utils.test_data import make_test_data, make_review_data
88
from ietf.utils.test_utils import login_testing_unauthorized, TestCase, unicontent, reload_db_objects
99
from ietf.doc.models import TelechatDocEvent
10+
from ietf.group.models import Role
1011
from ietf.iesg.models import TelechatDate
1112
from ietf.person.models import Email, Person
12-
from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod
13-
from ietf.review.utils import suggested_review_requests_for_team
14-
from ietf.review.utils import review_requests_needing_reviewer_reminder, email_reviewer_reminder
13+
from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings
14+
from ietf.review.utils import (
15+
suggested_review_requests_for_team,
16+
review_requests_needing_reviewer_reminder, email_reviewer_reminder,
17+
review_requests_needing_secretary_reminder, email_secretary_reminder,
18+
)
1519
from ietf.name.models import ReviewTypeName, ReviewResultName, ReviewRequestStateName
1620
import ietf.group.views_review
1721
from ietf.utils.mail import outbox, empty_outbox
@@ -403,23 +407,33 @@ def test_change_reviewer_settings(self):
403407
self.assertTrue(start_date.isoformat(), msg_content)
404408
self.assertTrue(end_date.isoformat(), msg_content)
405409

406-
def test_reviewer_reminders(self):
410+
def test_review_reminders(self):
407411
doc = make_test_data()
408412

409413
review_req = make_review_data(doc)
410414

415+
remind_days = 6
416+
411417
reviewer = Person.objects.get(user__username="reviewer")
412418

413-
settings = ReviewerSettings.objects.get(team=review_req.team, person=reviewer)
414-
settings.remind_days_before_deadline = 6
415-
settings.save()
419+
reviewer_settings = ReviewerSettings.objects.get(team=review_req.team, person=reviewer)
420+
reviewer_settings.remind_days_before_deadline = remind_days
421+
reviewer_settings.save()
422+
423+
secretary = Person.objects.get(user__username="reviewsecretary")
424+
secretary_role = Role.objects.get(group=review_req.team, name="secr", person=secretary)
425+
426+
secretary_settings = ReviewSecretarySettings(team=review_req.team, person=secretary)
427+
secretary_settings.remind_days_before_deadline = remind_days
428+
secretary_settings.save()
416429

417430
today = datetime.date.today()
418431

419432
review_req.reviewer = reviewer.email_set.first()
420-
review_req.deadline = today + datetime.timedelta(days=settings.remind_days_before_deadline)
433+
review_req.deadline = today + datetime.timedelta(days=remind_days)
421434
review_req.save()
422435

436+
# reviewer
423437
needing_reminders = review_requests_needing_reviewer_reminder(today - datetime.timedelta(days=1))
424438
self.assertEqual(list(needing_reminders), [])
425439

@@ -429,7 +443,25 @@ def test_reviewer_reminders(self):
429443
needing_reminders = review_requests_needing_reviewer_reminder(today + datetime.timedelta(days=1))
430444
self.assertEqual(list(needing_reminders), [])
431445

446+
# secretary
447+
needing_reminders = review_requests_needing_secretary_reminder(today - datetime.timedelta(days=1))
448+
self.assertEqual(list(needing_reminders), [])
449+
450+
needing_reminders = review_requests_needing_secretary_reminder(today)
451+
self.assertEqual(list(needing_reminders), [(review_req, secretary_role)])
452+
453+
needing_reminders = review_requests_needing_secretary_reminder(today + datetime.timedelta(days=1))
454+
self.assertEqual(list(needing_reminders), [])
455+
456+
# email reviewer
432457
empty_outbox()
433458
email_reviewer_reminder(review_req)
434459
self.assertEqual(len(outbox), 1)
435460
self.assertTrue(review_req.doc_id in outbox[0].get_payload(decode=True).decode("utf-8"))
461+
462+
# email secretary
463+
empty_outbox()
464+
email_secretary_reminder(review_req, secretary_role)
465+
self.assertEqual(len(outbox), 1)
466+
self.assertTrue(review_req.doc_id in outbox[0].get_payload(decode=True).decode("utf-8"))
467+

ietf/group/urls_info_details.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,5 +35,6 @@
3535
(r'^reviews/email-assignments/$', views_review.email_open_review_assignments),
3636
(r'^reviewers/$', views_review.reviewer_overview),
3737
(r'^reviewers/(?P<reviewer_email>[\w%+-.@]+)/settings/$', views_review.change_reviewer_settings),
38+
(r'^secretarysettings/$', views_review.change_secretary_settings),
3839
url(r'^email-aliases/$', RedirectView.as_view(pattern_name='ietf.group.views.email',permanent=False),name='old_group_email_aliases'),
3940
)

ietf/group/utils.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import debug # pyflakes:ignore
88

9-
from ietf.group.models import Group, RoleHistory
9+
from ietf.group.models import Group, RoleHistory, Role
1010
from ietf.person.models import Email
1111
from ietf.utils.history import get_history_object_for, copy_many_to_many_for_history
1212
from ietf.ietfauth.utils import has_role
@@ -216,6 +216,10 @@ def construct_group_menu_context(request, group, selected, group_type, others):
216216
actions.append((u"Manage unassigned reviews", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=dict(assignment_status="unassigned", **kwargs))))
217217
actions.append((u"Manage assigned reviews", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=dict(assignment_status="assigned", **kwargs))))
218218

219+
if Role.objects.filter(name="secr", group=group, person__user=request.user).exists():
220+
actions.append((u"Secretary settings", urlreverse(ietf.group.views_review.change_secretary_settings, kwargs=kwargs)))
221+
222+
219223
if group.state_id != "conclude" and (is_admin or can_manage):
220224
actions.append((u"Edit group", urlreverse("group_edit", kwargs=kwargs)))
221225

ietf/group/views_review.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from django import forms
99
from django.template.loader import render_to_string
1010

11-
from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod
11+
from ietf.review.models import ReviewRequest, ReviewerSettings, UnavailablePeriod, ReviewSecretarySettings
1212
from ietf.review.utils import (can_manage_review_requests_for_team,
1313
can_access_review_stats_for_team,
1414
close_review_request_states,
@@ -563,3 +563,41 @@ def change_reviewer_settings(request, acronym, reviewer_email, group_type=None):
563563
'period_form': period_form,
564564
'unavailable_periods': unavailable_periods,
565565
})
566+
567+
568+
class ReviewSecretarySettingsForm(forms.ModelForm):
569+
class Meta:
570+
model = ReviewSecretarySettings
571+
fields = ['remind_days_before_deadline']
572+
573+
574+
@login_required
575+
def change_secretary_settings(request, acronym, group_type=None):
576+
group = get_group_or_404(acronym, group_type)
577+
if not group.features.has_reviews:
578+
raise Http404
579+
if not Role.objects.filter(name="secr", group=group, person__user=request.user).exists():
580+
raise Http404
581+
582+
person = request.user.person
583+
584+
settings = (ReviewSecretarySettings.objects.filter(person=person, team=group).first()
585+
or ReviewSecretarySettings(person=person, team=group))
586+
587+
import ietf.group.views_review
588+
back_url = urlreverse(ietf.group.views_review.review_requests, kwargs={ "acronym": acronym, "group_type": group.type_id })
589+
590+
# settings
591+
if request.method == "POST":
592+
settings_form = ReviewSecretarySettingsForm(request.POST, instance=settings)
593+
if settings_form.is_valid():
594+
settings_form.save()
595+
return HttpResponseRedirect(back_url)
596+
else:
597+
settings_form = ReviewSecretarySettingsForm(instance=settings)
598+
599+
return render(request, 'group/change_review_secretary_settings.html', {
600+
'group': group,
601+
'back_url': back_url,
602+
'settings_form': settings_form,
603+
})
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# -*- coding: utf-8 -*-
2+
from __future__ import unicode_literals
3+
4+
from django.db import models, migrations
5+
6+
7+
class Migration(migrations.Migration):
8+
9+
dependencies = [
10+
('person', '0014_auto_20160613_0751'),
11+
('group', '0009_auto_20150930_0758'),
12+
('review', '0003_auto_20161018_0254'),
13+
]
14+
15+
operations = [
16+
migrations.CreateModel(
17+
name='ReviewSecretarySettings',
18+
fields=[
19+
('id', models.AutoField(verbose_name='ID', serialize=False, auto_created=True, primary_key=True)),
20+
('remind_days_before_deadline', models.IntegerField(help_text=b"To get an email reminder in case an assigned review gets near its deadline, enter the number of days before a review deadline you want to receive it. Clear the field if you don't want a reminder.", null=True, blank=True)),
21+
('person', models.ForeignKey(to='person.Person')),
22+
('team', models.ForeignKey(to='group.Group')),
23+
],
24+
options={
25+
'verbose_name_plural': 'review secretary settings',
26+
},
27+
bases=(models.Model,),
28+
),
29+
]

ietf/review/models.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
from ietf.name.models import ReviewTypeName, ReviewRequestStateName, ReviewResultName
99

1010
class ReviewerSettings(models.Model):
11-
"""Keeps track of admin data associated with the reviewer in the
12-
particular team. There will be one record for each combination of
13-
reviewer and team."""
11+
"""Keeps track of admin data associated with a reviewer in a team."""
1412
team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None))
1513
person = models.ForeignKey(Person)
1614
INTERVALS = [
@@ -23,14 +21,26 @@ class ReviewerSettings(models.Model):
2321
min_interval = models.IntegerField(verbose_name="Can review at most", choices=INTERVALS, blank=True, null=True)
2422
filter_re = models.CharField(max_length=255, verbose_name="Filter regexp", blank=True, help_text="Draft names matching regular expression should not be assigned")
2523
skip_next = models.IntegerField(default=0, verbose_name="Skip next assignments")
26-
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.")
24+
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 review deadline you want to receive it. Clear the field if you don't want a reminder.")
2725

2826
def __unicode__(self):
2927
return u"{} in {}".format(self.person, self.team)
3028

3129
class Meta:
3230
verbose_name_plural = "reviewer settings"
3331

32+
class ReviewSecretarySettings(models.Model):
33+
"""Keeps track of admin data associated with a secretary in a team."""
34+
team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None))
35+
person = models.ForeignKey(Person)
36+
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.")
37+
38+
def __unicode__(self):
39+
return u"{} in {}".format(self.person, self.team)
40+
41+
class Meta:
42+
verbose_name_plural = "review secretary settings"
43+
3444
class UnavailablePeriod(models.Model):
3545
team = models.ForeignKey(Group, limit_choices_to=~models.Q(resultusedinreviewteam=None))
3646
person = models.ForeignKey(Person)

ietf/review/utils.py

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
from ietf.person.models import Person
1414
from ietf.ietfauth.utils import has_role, is_authorized_in_doc_stream
1515
from ietf.review.models import (ReviewRequest, ReviewRequestStateName, ReviewTypeName, TypeUsedInReviewTeam,
16-
ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam)
16+
ReviewerSettings, UnavailablePeriod, ReviewWish, NextReviewerInTeam,
17+
ReviewSecretarySettings)
1718
from ietf.utils.mail import send_mail
1819
from ietf.doc.utils import extract_complete_replaces_ancestor_mapping_for_docs
1920

@@ -840,8 +841,10 @@ def email_reviewer_reminder(review_request):
840841

841842
subject = "Reminder: deadline for review of {} in {} is {}".format(review_request.doc_id, team.acronym, review_request.deadline.isoformat())
842843

843-
overview_url = urlreverse("ietf.ietfauth.views.review_overview")
844-
request_url = urlreverse("ietf.doc.views_review.review_request", kwargs={ "name": review_request.doc_id, "request_id": review_request.pk })
844+
import ietf.ietfauth.views
845+
overview_url = urlreverse(ietf.ietfauth.views.review_overview)
846+
import ietf.doc.views_review
847+
request_url = urlreverse(ietf.doc.views_review.review_request, kwargs={ "name": review_request.doc_id, "request_id": review_request.pk })
845848

846849
domain = Site.objects.get_current().domain
847850

@@ -855,3 +858,48 @@ def email_reviewer_reminder(review_request):
855858
"deadline_days": deadline_days,
856859
"remind_days": remind_days,
857860
})
861+
862+
def review_requests_needing_secretary_reminder(remind_date):
863+
reqs_qs = ReviewRequest.objects.filter(
864+
state__in=("requested", "accepted"),
865+
team__role__person__reviewsecretarysettings__remind_days_before_deadline__isnull=False,
866+
team__role__person__reviewsecretarysettings__team=F("team"),
867+
).exclude(
868+
reviewer=None
869+
).values_list("pk", "deadline", "team__role", "team__role__person__reviewsecretarysettings__remind_days_before_deadline").distinct()
870+
871+
req_pks = {}
872+
for r_pk, deadline, secretary_role_pk, remind_days in reqs_qs:
873+
if (deadline - remind_date).days == remind_days:
874+
req_pks[r_pk] = secretary_role_pk
875+
876+
review_reqs = { r.pk: r for r in ReviewRequest.objects.filter(pk__in=req_pks.keys()).select_related("reviewer", "reviewer__person", "state", "team") }
877+
secretary_roles = { r.pk: r for r in Role.objects.filter(pk__in=req_pks.values()).select_related("email", "person") }
878+
879+
return [ (review_reqs[req_pk], secretary_roles[secretary_role_pk]) for req_pk, secretary_role_pk in req_pks.iteritems() ]
880+
881+
def email_secretary_reminder(review_request, secretary_role):
882+
team = review_request.team
883+
884+
deadline_days = (review_request.deadline - datetime.date.today()).days
885+
886+
subject = "Reminder: deadline for review of {} in {} is {}".format(review_request.doc_id, team.acronym, review_request.deadline.isoformat())
887+
888+
import ietf.group.views_review
889+
settings_url = urlreverse(ietf.group.views_review.change_secretary_settings, kwargs={ "acronym": team.acronym, "group_type": team.type_id })
890+
import ietf.doc.views_review
891+
request_url = urlreverse(ietf.doc.views_review.review_request, kwargs={ "name": review_request.doc_id, "request_id": review_request.pk })
892+
893+
domain = Site.objects.get_current().domain
894+
895+
settings = ReviewSecretarySettings.objects.filter(person=secretary_role.person_id, team=team).first()
896+
remind_days = settings.remind_days_before_deadline if settings else 0
897+
898+
send_mail(None, [review_request.reviewer.formatted_email()], None, subject, "review/secretary_reminder.txt", {
899+
"review_request_url": "https://{}{}".format(domain, request_url),
900+
"settings_url": "https://{}{}".format(domain, settings_url),
901+
"review_request": review_request,
902+
"deadline_days": deadline_days,
903+
"remind_days": remind_days,
904+
})
905+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{% extends "base.html" %}
2+
{# Copyright The IETF Trust 2015, All Rights Reserved #}
3+
{% load origin %}{% origin %}
4+
5+
{% load ietf_filters staticfiles bootstrap3 %}
6+
7+
{% block content %}
8+
{% origin %}
9+
10+
<h1>{% block title %}Change your review secretary settings for {{ group.acronym }}{% endblock %}</h1>
11+
12+
<form class="change-review-secretary-settings" method="post">{% csrf_token %}
13+
{% bootstrap_form settings_form %}
14+
15+
{% buttons %}
16+
<a href="{{ back_url }}" class="btn btn-default pull-right">Cancel</a>
17+
<button class="btn btn-primary" type="submit" name="action" value="change_settings">Save</button>
18+
{% endbuttons %}
19+
</form>
20+
{% endblock %}

ietf/templates/group/manage_review_requests.html

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
<h1>Manage {{ assignment_status }} open review requests for {{ group.acronym }}</h1>
1717

1818
<p>Other options:
19-
<a href="{% url "ietf.group.views_review.reviewer_overview" group_type=group.type_id acronym=group.acronym %}">Reviewers in team</a>
19+
<a href="{% url "ietf.group.views_review.review_requests" group_type=group.type_id acronym=group.acronym %}">All review requests</a>
20+
- <a href="{% url "ietf.group.views_review.reviewer_overview" group_type=group.type_id acronym=group.acronym %}">Reviewers</a>
2021
- <a href="{% url "ietf.group.views_review.email_open_review_assignments" group_type=group.type_id acronym=group.acronym %}?next={{ request.get_full_path|urlencode }}">Email open assignments summary</a>
2122
{% if other_assignment_status %}
2223
- <a href="{% url "ietf.group.views_review.manage_review_requests" group_type=group.type_id acronym=group.acronym assignment_status=other_assignment_status %}">Manage {{ other_assignment_status }} reviews</a>
@@ -46,7 +47,7 @@ <h1>Manage {{ assignment_status }} open review requests for {{ group.acronym }}<
4647
<h3 class="panel-title">
4748
<span class="pull-right">
4849
{{ r.type.name }}
49-
- deadline: {{ r.deadline|date:"Y-m-d" }}
50+
- deadline {{ r.deadline|date:"Y-m-d" }}
5051
{% if r.due %}<span class="label label-warning">{{ r.due }} day{{ r.due|pluralize }}</span>{% endif %}
5152
</span>
5253

0 commit comments

Comments
 (0)