Split up open review requests on the review team page in assigned and
unassigned requests to make it easier to just work with the unassigned ones. Use same split on the manage reviews page which is now two pages. - Legacy-Id: 12266
This commit is contained in:
Ole Laursen
parent
19a3f10f69
commit
58dd25ff79
|
|
@ -148,11 +148,12 @@ class ReviewTests(TestCase):
|
|||
|
||||
group = review_req1.team
|
||||
|
||||
url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym })
|
||||
url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, "assignment_status": "assigned" })
|
||||
|
||||
login_testing_unauthorized(self, "secretary", url)
|
||||
|
||||
url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id })
|
||||
assigned_url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id, "assignment_status": "assigned" })
|
||||
unassigned_url = urlreverse(ietf.group.views_review.manage_review_requests, kwargs={ 'acronym': group.acronym, 'group_type': group.type_id, "assignment_status": "unassigned" })
|
||||
|
||||
review_req2 = ReviewRequest.objects.create(
|
||||
doc=review_req1.doc,
|
||||
|
|
@ -201,14 +202,14 @@ class ReviewTests(TestCase):
|
|||
)
|
||||
|
||||
# get
|
||||
r = self.client.get(url)
|
||||
r = self.client.get(assigned_url)
|
||||
self.assertEqual(r.status_code, 200)
|
||||
self.assertTrue(review_req1.doc.name in unicontent(r))
|
||||
|
||||
# can't save: conflict
|
||||
# can't save assigned: conflict
|
||||
new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman")
|
||||
# provoke conflict by posting bogus data
|
||||
r = self.client.post(url, {
|
||||
r = self.client.post(assigned_url, {
|
||||
"reviewrequest": [str(review_req1.pk), str(review_req2.pk), str(123456)],
|
||||
|
||||
# close
|
||||
|
|
@ -226,13 +227,21 @@ class ReviewTests(TestCase):
|
|||
self.assertEqual(r.status_code, 200)
|
||||
content = unicontent(r).lower()
|
||||
self.assertTrue("1 request closed" in content)
|
||||
self.assertTrue("1 request opened" in content)
|
||||
self.assertTrue("2 requests changed assignment" in content)
|
||||
|
||||
# close and assign
|
||||
# can't save unassigned: conflict
|
||||
r = self.client.post(unassigned_url, {
|
||||
"reviewrequest": [str(123456)],
|
||||
"action": "save-continue",
|
||||
})
|
||||
self.assertEqual(r.status_code, 200)
|
||||
content = unicontent(r).lower()
|
||||
self.assertTrue("1 request opened" in content)
|
||||
|
||||
# close and reassign assigned
|
||||
new_reviewer = Email.objects.get(role__name="reviewer", role__group=group, person__user__username="marschairman")
|
||||
r = self.client.post(url, {
|
||||
"reviewrequest": [str(review_req1.pk), str(review_req2.pk), str(review_req3.pk)],
|
||||
r = self.client.post(assigned_url, {
|
||||
"reviewrequest": [str(review_req1.pk), str(review_req2.pk)],
|
||||
|
||||
# close
|
||||
"r{}-existing_reviewer".format(review_req1.pk): review_req1.reviewer_id or "",
|
||||
|
|
@ -244,12 +253,20 @@ class ReviewTests(TestCase):
|
|||
"r{}-action".format(review_req2.pk): "assign",
|
||||
"r{}-reviewer".format(review_req2.pk): new_reviewer.pk,
|
||||
|
||||
"action": "save",
|
||||
})
|
||||
self.assertEqual(r.status_code, 302)
|
||||
|
||||
# no change on unassigned
|
||||
r = self.client.post(unassigned_url, {
|
||||
"reviewrequest": [str(review_req3.pk)],
|
||||
|
||||
# no change
|
||||
"r{}-existing_reviewer".format(review_req3.pk): review_req3.reviewer_id or "",
|
||||
"r{}-action".format(review_req3.pk): "",
|
||||
"r{}-close".format(review_req3.pk): "no-response",
|
||||
"r{}-reviewer".format(review_req3.pk): "",
|
||||
|
||||
|
||||
"action": "save",
|
||||
})
|
||||
self.assertEqual(r.status_code, 302)
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ urlpatterns = patterns('',
|
|||
(r'^archives/$', 'ietf.group.views.derived_archives'),
|
||||
(r'^photos/$', views.group_photos),
|
||||
(r'^reviews/$', views_review.review_requests),
|
||||
(r'^reviews/manage/$', views_review.manage_review_requests),
|
||||
(r'^reviews/manage/(?P<assignment_status>assigned|unassigned)/$', views_review.manage_review_requests),
|
||||
(r'^reviews/email-assignments/$', views_review.email_open_review_assignments),
|
||||
(r'^reviewers/$', views_review.reviewer_overview),
|
||||
(r'^reviewers/(?P<reviewer_email>[\w%+-.@]+)/settings/$', views_review.change_reviewer_settings),
|
||||
|
|
|
|||
|
|
@ -213,7 +213,8 @@ def construct_group_menu_context(request, group, selected, group_type, others):
|
|||
|
||||
if group.features.has_reviews and can_manage_review_requests_for_team(request.user, group):
|
||||
import ietf.group.views_review
|
||||
actions.append((u"Manage review requests", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=kwargs)))
|
||||
actions.append((u"Manage unassigned reviews", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=dict(assignment_status="unassigned", **kwargs))))
|
||||
actions.append((u"Manage assigned reviews", urlreverse(ietf.group.views_review.manage_review_requests, kwargs=dict(assignment_status="assigned", **kwargs))))
|
||||
|
||||
if group.state_id != "conclude" and (is_admin or can_manage):
|
||||
actions.append((u"Edit group", urlreverse("group_edit", kwargs=kwargs)))
|
||||
|
|
|
|||
|
|
@ -30,33 +30,55 @@ from ietf.utils.mail import send_mail_text
|
|||
from ietf.utils.fields import DatepickerDateField
|
||||
from ietf.ietfauth.utils import user_is_person
|
||||
|
||||
def get_open_review_requests_for_team(team, assignment_status=None):
|
||||
open_review_requests = ReviewRequest.objects.filter(
|
||||
team=team,
|
||||
state__in=("requested", "accepted")
|
||||
).prefetch_related(
|
||||
"reviewer__person", "type", "state"
|
||||
).order_by("-time", "-id")
|
||||
|
||||
if assignment_status == "unassigned":
|
||||
open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests.filter(reviewer=None))
|
||||
elif assignment_status == "assigned":
|
||||
open_review_requests = list(open_review_requests.exclude(reviewer=None))
|
||||
else:
|
||||
open_review_requests = suggested_review_requests_for_team(team) + list(open_review_requests)
|
||||
|
||||
today = datetime.date.today()
|
||||
unavailable_periods = current_unavailable_periods_for_reviewers(team)
|
||||
for r in open_review_requests:
|
||||
if r.reviewer:
|
||||
r.reviewer_unavailable = any(p.availability == "unavailable"
|
||||
for p in unavailable_periods.get(r.reviewer.person_id, []))
|
||||
r.due = max(0, (today - r.deadline).days)
|
||||
|
||||
return open_review_requests
|
||||
|
||||
def review_requests(request, acronym, group_type=None):
|
||||
group = get_group_or_404(acronym, group_type)
|
||||
if not group.features.has_reviews:
|
||||
raise Http404
|
||||
|
||||
open_review_requests = list(ReviewRequest.objects.filter(
|
||||
team=group, state__in=("requested", "accepted")
|
||||
).prefetch_related("reviewer", "type", "state").order_by("-time", "-id"))
|
||||
assigned_review_requests = []
|
||||
unassigned_review_requests = []
|
||||
|
||||
unavailable_periods = current_unavailable_periods_for_reviewers(group)
|
||||
for review_req in open_review_requests:
|
||||
if review_req.reviewer:
|
||||
review_req.reviewer_unavailable = any(p.availability == "unavailable"
|
||||
for p in unavailable_periods.get(review_req.reviewer.person_id, []))
|
||||
for r in get_open_review_requests_for_team(group):
|
||||
if r.reviewer:
|
||||
assigned_review_requests.append(r)
|
||||
else:
|
||||
unassigned_review_requests.append(r)
|
||||
|
||||
open_review_requests = suggested_review_requests_for_team(group) + open_review_requests
|
||||
|
||||
today = datetime.date.today()
|
||||
for r in open_review_requests:
|
||||
r.due = max(0, (today - r.deadline).days)
|
||||
open_review_requests = [
|
||||
("Unassigned", unassigned_review_requests),
|
||||
("Assigned", assigned_review_requests),
|
||||
]
|
||||
|
||||
closed_review_requests = ReviewRequest.objects.filter(
|
||||
team=group,
|
||||
).exclude(
|
||||
state__in=("requested", "accepted")
|
||||
).prefetch_related("reviewer", "type", "state", "doc").order_by("-time", "-id")
|
||||
).prefetch_related("reviewer__person", "type", "state", "doc", "result").order_by("-time", "-id")
|
||||
|
||||
since_choices = [
|
||||
(None, "1 month"),
|
||||
|
|
@ -192,7 +214,7 @@ class ManageReviewRequestForm(forms.Form):
|
|||
|
||||
|
||||
@login_required
|
||||
def manage_review_requests(request, acronym, group_type=None):
|
||||
def manage_review_requests(request, acronym, group_type=None, assignment_status=None):
|
||||
group = get_group_or_404(acronym, group_type)
|
||||
if not group.features.has_reviews:
|
||||
raise Http404
|
||||
|
|
@ -200,18 +222,7 @@ def manage_review_requests(request, acronym, group_type=None):
|
|||
if not can_manage_review_requests_for_team(request.user, group):
|
||||
return HttpResponseForbidden("You do not have permission to perform this action")
|
||||
|
||||
unavailable_periods = current_unavailable_periods_for_reviewers(group)
|
||||
|
||||
open_review_requests = list(ReviewRequest.objects.filter(
|
||||
team=group, state__in=("requested", "accepted")
|
||||
).prefetch_related("reviewer", "type", "state").order_by("-time", "-id"))
|
||||
|
||||
for review_req in open_review_requests:
|
||||
if review_req.reviewer:
|
||||
review_req.reviewer_unavailable = any(p.availability == "unavailable"
|
||||
for p in unavailable_periods.get(review_req.reviewer.person_id, []))
|
||||
|
||||
review_requests = suggested_review_requests_for_team(group) + open_review_requests
|
||||
review_requests = get_open_review_requests_for_team(group, assignment_status=assignment_status)
|
||||
|
||||
document_requests = extract_revision_ordered_review_requests_for_documents_and_replaced(
|
||||
ReviewRequest.objects.filter(state__in=("part-completed", "completed"), team=group).prefetch_related("result"),
|
||||
|
|
@ -221,10 +232,12 @@ def manage_review_requests(request, acronym, group_type=None):
|
|||
# we need a mutable query dict for resetting upon saving with
|
||||
# conflicts
|
||||
query_dict = request.POST.copy() if request.method == "POST" else None
|
||||
|
||||
for req in review_requests:
|
||||
# add previous requests
|
||||
l = []
|
||||
# take all on the latest reviewed rev
|
||||
for r in document_requests.get(req.doc_id, []):
|
||||
# take all on the latest reviewed rev
|
||||
if l and l[0].reviewed_rev:
|
||||
if r.doc_id == l[0].doc_id and r.reviewed_rev:
|
||||
if int(r.reviewed_rev) > int(l[0].reviewed_rev):
|
||||
|
|
@ -292,11 +305,19 @@ def manage_review_requests(request, acronym, group_type=None):
|
|||
kwargs["group_type"] = group_type
|
||||
|
||||
if form_action == "save-continue":
|
||||
if assignment_status:
|
||||
kwargs["assignment_status"] = assignment_status
|
||||
|
||||
return redirect(manage_review_requests, **kwargs)
|
||||
else:
|
||||
import ietf.group.views_review
|
||||
return redirect(ietf.group.views_review.review_requests, **kwargs)
|
||||
|
||||
other_assignment_status = {
|
||||
"unassigned": "assigned",
|
||||
"assigned": "unassigned",
|
||||
}.get(assignment_status)
|
||||
|
||||
return render(request, 'group/manage_review_requests.html', {
|
||||
'group': group,
|
||||
'review_requests': review_requests,
|
||||
|
|
@ -304,6 +325,8 @@ def manage_review_requests(request, acronym, group_type=None):
|
|||
'newly_opened': newly_opened,
|
||||
'newly_assigned': newly_assigned,
|
||||
'saving': saving,
|
||||
'assignment_status': assignment_status,
|
||||
'other_assignment_status': other_assignment_status,
|
||||
})
|
||||
|
||||
class EmailOpenAssignmentsForm(forms.Form):
|
||||
|
|
@ -327,16 +350,21 @@ def email_open_review_assignments(request, acronym, group_type=None):
|
|||
reviewer=None,
|
||||
).prefetch_related("reviewer", "type", "state", "doc").distinct().order_by("deadline", "reviewer"))
|
||||
|
||||
back_url = request.GET.get("next")
|
||||
if not back_url:
|
||||
kwargs = { "acronym": group.acronym }
|
||||
if group_type:
|
||||
kwargs["group_type"] = group_type
|
||||
|
||||
import ietf.group.views_review
|
||||
back_url = urlreverse(ietf.group.views_review.review_requests, kwargs=kwargs)
|
||||
|
||||
if request.method == "POST" and request.POST.get("action") == "email":
|
||||
form = EmailOpenAssignmentsForm(request.POST)
|
||||
if form.is_valid():
|
||||
send_mail_text(request, form.cleaned_data["to"], None, form.cleaned_data["subject"], form.cleaned_data["body"])
|
||||
|
||||
kwargs = { "acronym": group.acronym }
|
||||
if group_type:
|
||||
kwargs["group_type"] = group_type
|
||||
|
||||
return redirect(manage_review_requests, **kwargs)
|
||||
return HttpResponseRedirect(back_url)
|
||||
else:
|
||||
to = group.list_email
|
||||
subject = "Open review assignments in {}".format(group.acronym)
|
||||
|
|
@ -356,6 +384,7 @@ def email_open_review_assignments(request, acronym, group_type=None):
|
|||
'group': group,
|
||||
'review_requests': review_requests,
|
||||
'form': form,
|
||||
'back_url': back_url,
|
||||
})
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@
|
|||
{% bootstrap_form form %}
|
||||
|
||||
{% buttons %}
|
||||
<a href="{% url "ietf.group.views_review.manage_review_requests" group_type=group.type_id acronym=group.acronym %}" class="btn btn-default pull-right">Cancel</a>
|
||||
<a href="{{ back_url }}" class="btn btn-default pull-right">Cancel</a>
|
||||
<button class="btn btn-primary" type="submit" name="action" value="email">Send to team mailing list</button>
|
||||
{% endbuttons %}
|
||||
</form>
|
||||
|
|
|
|||
|
|
@ -13,11 +13,14 @@
|
|||
{% block content %}
|
||||
{% origin %}
|
||||
|
||||
<h1>Manage open review requests for {{ group.acronym }}</h1>
|
||||
<h1>Manage {{ assignment_status }} open review requests for {{ group.acronym }}</h1>
|
||||
|
||||
<p>Other options:
|
||||
<a href="{% url "ietf.group.views_review.reviewer_overview" group_type=group.type_id acronym=group.acronym %}">Reviewers in team</a>
|
||||
- <a href="{% url "ietf.group.views_review.email_open_review_assignments" group_type=group.type_id acronym=group.acronym %}">Email open assignments summary</a>
|
||||
- <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>
|
||||
{% if other_assignment_status %}
|
||||
- <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>
|
||||
{% endif %}
|
||||
</p>
|
||||
|
||||
{% if newly_closed > 0 or newly_opened > 0 or newly_assigned > 0 %}
|
||||
|
|
@ -136,7 +139,7 @@
|
|||
{% endbuttons %}
|
||||
</form>
|
||||
{% else %}
|
||||
<p>There are currently no open requests.</p>
|
||||
<p>There are currently no {{ assignment_status }} open requests.</p>
|
||||
{% endif %}
|
||||
{% endblock %}
|
||||
|
||||
|
|
|
|||
|
|
@ -17,46 +17,47 @@
|
|||
<h1 class="pull-right"><a href="{% url "ietf.stats.views.review_stats" %}" class="icon-link"> <span class="small fa fa-bar-chart"> </span></a></h1>
|
||||
{% endif %}
|
||||
|
||||
<h2>Open review requests</h2>
|
||||
{% for label, review_requests in open_review_requests %}
|
||||
{% if review_requests %}
|
||||
|
||||
{% if open_review_requests %}
|
||||
<table class="table table-condensed table-striped tablesorter">
|
||||
<thead>
|
||||
<tr>
|
||||
<th>Request</th>
|
||||
<th>Type</th>
|
||||
<th>Requested</th>
|
||||
<th>Deadline</th>
|
||||
<th>Reviewer</th>
|
||||
</tr>
|
||||
</thead>
|
||||
<tbody>
|
||||
{% for r in open_review_requests %}
|
||||
<h2>{{ label }} open review requests</h2>
|
||||
|
||||
<table class="table table-condensed table-striped tablesorter">
|
||||
<thead>
|
||||
<tr>
|
||||
<td>{% if r.pk != None %}<a href="{% url "ietf.doc.views_review.review_request" name=r.doc.name request_id=r.pk %}">{% endif %}{{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% if r.pk != None %}</a>{% endif %}</td>
|
||||
<td>{{ r.type.name }}</td>
|
||||
<td>{% if r.pk %}{{ r.time|date:"Y-m-d" }}{% else %}<em>auto-suggested</em>{% endif %}</td>
|
||||
<td>
|
||||
{{ r.deadline|date:"Y-m-d" }}
|
||||
{% if r.due %}<span class="label label-warning" title="{{ r.due }} day{{ r.due|pluralize }} past deadline">{{ r.due }} day{{ r.due|pluralize }}</span>{% endif %}
|
||||
</td>
|
||||
<td>
|
||||
{% if r.reviewer %}
|
||||
{{ r.reviewer.person }}
|
||||
{% if r.state_id == "accepted" %}<span class="label label-default">Accepted</span>{% endif %}
|
||||
{% if r.reviewer_unavailable %}<span class="label label-danger">Unavailable</span>{% endif %}
|
||||
{% elif r.pk != None %}
|
||||
<em>not yet assigned</em>
|
||||
{% endif %}
|
||||
</td>
|
||||
<th>Request</th>
|
||||
<th>Type</th>
|
||||
<th>Requested</th>
|
||||
<th>Deadline</th>
|
||||
{% if review_requests.0.reviewer %}
|
||||
<th>Reviewer</th>
|
||||
{% endif %}
|
||||
</tr>
|
||||
{% endfor %}
|
||||
</tbody>
|
||||
</table>
|
||||
|
||||
{% else %}
|
||||
<p>There are currently no open requests.</p>
|
||||
{% endif %}
|
||||
</thead>
|
||||
<tbody>
|
||||
{% for r in review_requests %}
|
||||
<tr>
|
||||
<td>{% if r.pk != None %}<a href="{% url "ietf.doc.views_review.review_request" name=r.doc.name request_id=r.pk %}">{% endif %}{{ r.doc.name }}-{% if r.requested_rev %}{{ r.requested_rev }}{% else %}{{ r.doc.rev }}{% endif %}{% if r.pk != None %}</a>{% endif %}</td>
|
||||
<td>{{ r.type.name }}</td>
|
||||
<td>{% if r.pk %}{{ r.time|date:"Y-m-d" }}{% else %}<em>auto-suggested</em>{% endif %}</td>
|
||||
<td>
|
||||
{{ r.deadline|date:"Y-m-d" }}
|
||||
{% if r.due %}<span class="label label-warning" title="{{ r.due }} day{{ r.due|pluralize }} past deadline">{{ r.due }} day{{ r.due|pluralize }}</span>{% endif %}
|
||||
</td>
|
||||
{% if r.reviewer %}
|
||||
<td>
|
||||
{{ r.reviewer.person }}
|
||||
{% if r.state_id == "accepted" %}<span class="label label-default">Accepted</span>{% endif %}
|
||||
{% if r.reviewer_unavailable %}<span class="label label-danger">Unavailable</span>{% endif %}
|
||||
</td>
|
||||
{% endif %}
|
||||
</tr>
|
||||
{% endfor %}
|
||||
</tbody>
|
||||
</table>
|
||||
|
||||
{% endif %}
|
||||
{% endfor %}
|
||||
|
||||
<h2 id="closed-review-requests">Closed review requests</h2>
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue