Skip to content

Commit 6e55f26

Browse files
author
Sasha Romijn
committed
Fix ietf-tools#2050 - Allow adding review wishes from document and search pages.
On the main page of a document and in document search results, a new button allows review team members to add a review wish for that document. For reviewers that are only on one team, this essentially works identical to tracking a document. Reviewers that are on multiple teams are lead through an intermediate step to select a review team, and then returned to their search or document page. Commit ready for merge. - Legacy-Id: 16939
1 parent f075ece commit 6e55f26

12 files changed

Lines changed: 234 additions & 37 deletions

File tree

ietf/community/utils.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -58,20 +58,6 @@ def can_manage_community_list(user, clist):
5858

5959
return False
6060

61-
def augment_docs_with_tracking_info(docs, user):
62-
"""Add attribute to each document with whether the document is tracked
63-
by the user or not."""
64-
65-
tracked = set()
66-
67-
if user and user.is_authenticated:
68-
clist = CommunityList.objects.filter(user=user).first()
69-
if clist:
70-
tracked.update(docs_tracked_by_community_list(clist).filter(pk__in=[ d.pk for d in docs ]).values_list("pk", flat=True))
71-
72-
for d in docs:
73-
d.tracked_in_personal_community_list = d.pk in tracked
74-
7561
def reset_name_contains_index_for_rule(rule):
7662
if not rule.rule_type == "name_contains":
7763
return

ietf/doc/tests_review.py

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@
2222
import debug # pyflakes:ignore
2323

2424
import ietf.review.mailarch
25-
from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory, ReviewFactory
25+
from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory, \
26+
ReviewFactory, DocumentFactory
2627
from ietf.doc.models import DocumentAuthor, RelatedDocument, DocEvent, ReviewRequestDocEvent, ReviewAssignmentDocEvent
2728
from ietf.group.factories import RoleFactory, ReviewTeamFactory
2829
from ietf.group.models import Group
@@ -1164,3 +1165,51 @@ def test_withdraw_assignment(self):
11641165
assignment = reload_db_objects(assignment)
11651166
self.assertEqual(assignment.state_id, 'withdrawn')
11661167

1168+
def test_review_wish_add(self):
1169+
doc = DocumentFactory()
1170+
team = ReviewTeamFactory()
1171+
reviewer = RoleFactory(group=team, name_id='reviewer').person
1172+
url = urlreverse('ietf.doc.views_review.review_wish_add', kwargs={'name': doc.name})
1173+
1174+
login_testing_unauthorized(self, reviewer.user.username, url)
1175+
r = self.client.get(url)
1176+
self.assertEqual(r.status_code, 200)
1177+
1178+
# As this reviewer is only on a single team, posting without data should work
1179+
r = self.client.post(url + '?next=/redirect-url')
1180+
self.assertRedirects(r, '/redirect-url', fetch_redirect_response=False)
1181+
self.assertTrue(ReviewWish.objects.get(person=reviewer, doc=doc, team=team))
1182+
1183+
# Try again with a reviewer on multiple teams, requiring team selection.
1184+
# This also uses an invalid redirect URL that should be ignored.
1185+
ReviewWish.objects.all().delete()
1186+
team2 = ReviewTeamFactory()
1187+
RoleFactory(group=team2, person=reviewer, name_id='reviewer')
1188+
1189+
r = self.client.post(url + '?next=http://example.com/')
1190+
self.assertEqual(r.status_code, 200) # Missing team parameter
1191+
1192+
r = self.client.post(url + '?next=http://example.com/', data={'team': team2.pk})
1193+
self.assertRedirects(r, doc.get_absolute_url(), fetch_redirect_response=False)
1194+
self.assertTrue(ReviewWish.objects.get(person=reviewer, doc=doc, team=team2))
1195+
1196+
def test_review_wishes_remove(self):
1197+
doc = DocumentFactory()
1198+
team = ReviewTeamFactory()
1199+
reviewer = RoleFactory(group=team, name_id='reviewer').person
1200+
ReviewWish.objects.create(person=reviewer, doc=doc, team=team)
1201+
url = urlreverse('ietf.doc.views_review.review_wishes_remove', kwargs={'name': doc.name})
1202+
1203+
login_testing_unauthorized(self, reviewer.user.username, url)
1204+
r = self.client.get(url)
1205+
self.assertEqual(r.status_code, 200)
1206+
1207+
r = self.client.post(url + '?next=/redirect-url')
1208+
self.assertRedirects(r, '/redirect-url', fetch_redirect_response=False)
1209+
self.assertFalse(ReviewWish.objects.all())
1210+
1211+
# Try again with an invalid redirect URL that should be ignored.
1212+
ReviewWish.objects.create(person=reviewer, doc=doc, team=team)
1213+
r = self.client.post(url + '?next=http://example.com')
1214+
self.assertRedirects(r, doc.get_absolute_url(), fetch_redirect_response=False)
1215+
self.assertFalse(ReviewWish.objects.all())

ietf/doc/urls_review.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,6 @@
2020
url(r'^team/%(acronym)s/searchmailarchive/$' % settings.URL_REGEXPS, views_review.search_mail_archive),
2121
url(r'^(?P<request_id>[0-9]+)/editcomment/$', views_review.edit_comment),
2222
url(r'^(?P<request_id>[0-9]+)/editdeadline/$', views_review.edit_deadline),
23+
url(r'^addreviewwish/$' % settings.URL_REGEXPS, views_review.review_wish_add),
24+
url(r'^removereviewwishes/$' % settings.URL_REGEXPS, views_review.review_wishes_remove),
2325
]

ietf/doc/utils.py

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,18 @@
2323
from django.urls import reverse as urlreverse
2424

2525
import debug # pyflakes:ignore
26+
from ietf.community.models import CommunityList
27+
from ietf.community.utils import docs_tracked_by_community_list
2628

2729
from ietf.doc.models import Document, DocHistory, State, DocumentAuthor, DocHistoryAuthor
2830
from ietf.doc.models import DocAlias, RelatedDocument, RelatedDocHistory, BallotType, DocReminder
2931
from ietf.doc.models import DocEvent, ConsensusDocEvent, BallotDocEvent, NewRevisionDocEvent, StateDocEvent
3032
from ietf.doc.models import TelechatDocEvent
3133
from ietf.name.models import DocReminderTypeName, DocRelationshipName
32-
from ietf.group.models import Role
34+
from ietf.group.models import Role, Group
3335
from ietf.ietfauth.utils import has_role
36+
from ietf.person.models import Person
37+
from ietf.review.models import ReviewWish
3438
from ietf.utils import draft, text
3539
from ietf.utils.mail import send_mail
3640
from ietf.mailtrigger.utils import gather_address_lists
@@ -903,3 +907,31 @@ def add_markup(path, doc, lines):
903907
#
904908
return block
905909

910+
911+
def augment_docs_and_user_with_user_info(docs, user):
912+
"""Add attribute to each document with whether the document is tracked
913+
or has a review wish by the user or not, and the review teams the user is on."""
914+
915+
tracked = set()
916+
review_wished = set()
917+
918+
if user and user.is_authenticated:
919+
user.review_teams = Group.objects.filter(
920+
reviewteamsettings__isnull=False, role__person__user=user, role__name='reviewer')
921+
922+
doc_pks = [d.pk for d in docs]
923+
clist = CommunityList.objects.filter(user=user).first()
924+
if clist:
925+
tracked.update(
926+
docs_tracked_by_community_list(clist).filter(pk__in=doc_pks).values_list("pk", flat=True))
927+
928+
try:
929+
wishes = ReviewWish.objects.filter(person=Person.objects.get(user=user))
930+
wishes = wishes.filter(doc__pk__in=doc_pks).values_list("doc__pk", flat=True)
931+
review_wished.update(wishes)
932+
except Person.DoesNotExist:
933+
pass
934+
935+
for d in docs:
936+
d.tracked_in_personal_community_list = d.pk in tracked
937+
d.has_review_wish = d.pk in review_wished

ietf/doc/utils_search.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
import datetime
88
import debug # pyflakes:ignore
99

10-
from ietf.community.utils import augment_docs_with_tracking_info
1110
from ietf.doc.models import Document, DocAlias, RelatedDocument, DocEvent, TelechatDocEvent, BallotDocEvent
1211
from ietf.doc.expire import expirable_draft
12+
from ietf.doc.utils import augment_docs_and_user_with_user_info
1313
from ietf.meeting.models import SessionPresentation, Meeting, Session
1414

1515
def wrap_value(v):
@@ -162,7 +162,7 @@ def prepare_document_table(request, docs, query=None, max_results=200):
162162
docs = list(docs)
163163

164164
fill_in_document_table_attributes(docs)
165-
augment_docs_with_tracking_info(docs, request.user)
165+
augment_docs_and_user_with_user_info(docs, request.user)
166166

167167
meta = {}
168168

ietf/doc/views_doc.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,12 +58,12 @@
5858
from ietf.doc.models import ( Document, DocAlias, DocHistory, DocEvent, BallotDocEvent,
5959
ConsensusDocEvent, NewRevisionDocEvent, TelechatDocEvent, WriteupDocEvent,
6060
IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS )
61-
from ietf.doc.utils import ( add_links_in_new_revision_events, augment_events_with_revision,
62-
can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id,
63-
needed_ballot_positions, nice_consensus, prettify_std_name, update_telechat, has_same_ballot,
64-
get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus,
65-
add_events_message_info, get_unicode_document_content, build_doc_meta_block)
66-
from ietf.community.utils import augment_docs_with_tracking_info
61+
from ietf.doc.utils import (add_links_in_new_revision_events, augment_events_with_revision,
62+
can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id,
63+
needed_ballot_positions, nice_consensus, prettify_std_name, update_telechat, has_same_ballot,
64+
get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus,
65+
add_events_message_info, get_unicode_document_content, build_doc_meta_block,
66+
augment_docs_and_user_with_user_info)
6767
from ietf.group.models import Role, Group
6868
from ietf.group.utils import can_manage_group_type, can_manage_materials, group_features_role_filter
6969
from ietf.ietfauth.utils import ( has_role, is_authorized_in_doc_stream, user_is_person,
@@ -388,7 +388,7 @@ def document_main(request, name, rev=None):
388388
elif can_edit_stream_info and (iesg_state.slug in ('idexists','watching')):
389389
actions.append(("Submit to IESG for Publication", urlreverse('ietf.doc.views_draft.to_iesg', kwargs=dict(name=doc.name))))
390390

391-
augment_docs_with_tracking_info([doc], request.user)
391+
augment_docs_and_user_with_user_info([doc], request.user)
392392

393393
replaces = [d.name for d in doc.related_that_doc("replaces")]
394394
replaced_by = [d.name for d in doc.related_that("replaces")]

ietf/doc/views_review.py

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,17 @@
55
from __future__ import absolute_import, print_function, unicode_literals
66

77
import io
8+
import json
89
import os
910
import datetime
1011
import requests
1112
import email.utils
1213

14+
from django.utils.http import is_safe_url
15+
1316
import debug # pyflakes:ignore
1417

15-
from django.http import HttpResponseForbidden, JsonResponse
18+
from django.http import HttpResponseForbidden, JsonResponse, HttpResponse, HttpResponseRedirect
1619
from django.shortcuts import render, get_object_or_404, redirect
1720
from django import forms
1821
from django.conf import settings
@@ -27,7 +30,7 @@
2730
from ietf.name.models import ReviewRequestStateName, ReviewAssignmentStateName, ReviewResultName, \
2831
DocTypeName, ReviewTypeName
2932
from ietf.person.models import Person
30-
from ietf.review.models import ReviewRequest, ReviewAssignment
33+
from ietf.review.models import ReviewRequest, ReviewAssignment, ReviewWish
3134
from ietf.group.models import Group
3235
from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has_role
3336
from ietf.message.models import Message
@@ -967,3 +970,64 @@ def edit_deadline(request, name, request_id):
967970
'review_req': review_req,
968971
'form' : form,
969972
})
973+
974+
975+
class ReviewWishAddForm(forms.Form):
976+
team = forms.ModelChoiceField(queryset=Group.objects.filter(reviewteamsettings__isnull=False),
977+
widget=forms.RadioSelect, empty_label=None, required=True)
978+
979+
def __init__(self, user, doc, *args, **kwargs):
980+
super(ReviewWishAddForm, self).__init__(*args, **kwargs)
981+
self.person = get_object_or_404(Person, user=user)
982+
self.doc = doc
983+
self.fields['team'].queryset = self.fields['team'].queryset.filter(role__person=self.person,
984+
role__name='reviewer')
985+
if len(self.fields['team'].queryset) == 1:
986+
self.team = self.fields['team'].queryset.get()
987+
del self.fields['team']
988+
989+
def save(self):
990+
team = self.team if hasattr(self, 'team') else self.cleaned_data['team']
991+
ReviewWish.objects.get_or_create(person=self.person, team=team, doc=self.doc)
992+
993+
@login_required
994+
def review_wish_add(request, name):
995+
doc = get_object_or_404(Document, docalias__name=name)
996+
997+
if request.method == "POST":
998+
form = ReviewWishAddForm(request.user, doc, request.POST)
999+
if form.is_valid():
1000+
form.save()
1001+
return _generate_ajax_or_redirect_response(request, doc)
1002+
else:
1003+
form = ReviewWishAddForm(request.user, doc)
1004+
1005+
return render(request, "doc/review/review_wish_add.html", {
1006+
"doc": doc,
1007+
"form": form,
1008+
})
1009+
1010+
@login_required
1011+
def review_wishes_remove(request, name):
1012+
doc = get_object_or_404(Document, docalias__name=name)
1013+
person = get_object_or_404(Person, user=request.user)
1014+
1015+
if request.method == "POST":
1016+
ReviewWish.objects.filter(person=person, doc=doc).delete()
1017+
return _generate_ajax_or_redirect_response(request, doc)
1018+
1019+
return render(request, "doc/review/review_wishes_remove.html", {
1020+
"name": doc.name,
1021+
})
1022+
1023+
1024+
def _generate_ajax_or_redirect_response(request, doc):
1025+
redirect_url = request.GET.get('next')
1026+
url_is_safe = is_safe_url(url=redirect_url, allowed_hosts=request.get_host(),
1027+
require_https=request.is_secure())
1028+
if request.is_ajax():
1029+
return HttpResponse(json.dumps({'success': True}), content_type='application/json')
1030+
elif url_is_safe:
1031+
return HttpResponseRedirect(redirect_url)
1032+
else:
1033+
return HttpResponseRedirect(doc.get_absolute_url())

ietf/static/ietf/js/ietf.js

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -98,24 +98,31 @@ $(document).ready(function () {
9898

9999
updateAdvanced();
100100
}
101-
102-
// search results
103-
$('.track-untrack-doc').click(function(e) {
104-
e.preventDefault();
101+
102+
$('.review-wish-add-remove-doc.ajax, .track-untrack-doc').click(function(e) {
103+
e.preventDefault();
105104
var trigger = $(this);
106105
$.ajax({
107106
url: trigger.attr('href'),
108107
type: 'POST',
109108
cache: false,
110109
dataType: 'json',
111110
success: function(response){
112-
if (response.success) {
113-
trigger.parent().find(".tooltip").remove();
114-
trigger.addClass("hide");
115-
trigger.parent().find(".track-untrack-doc").not(trigger).removeClass("hide");
111+
if (response.success) {
112+
trigger.parent().find(".tooltip").remove();
113+
trigger.addClass("hide");
114+
115+
var target_unhide = null;
116+
if(trigger.hasClass('review-wish-add-remove-doc')) {
117+
target_unhide = '.review-wish-add-remove-doc';
118+
} else if(trigger.hasClass('track-untrack-doc')) {
119+
target_unhide = '.track-untrack-doc';
116120
}
117-
}
118-
});
121+
if(target_unhide) {}
122+
trigger.parent().find(target_unhide).not(trigger).removeClass("hide");
123+
}
124+
}
125+
});
119126
});
120127
});
121128

ietf/templates/doc/document_draft.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,10 @@
587587
<a class="btn btn-default btn-xs track-untrack-doc {% if not doc.tracked_in_personal_community_list %}hide{% endif %}" href="{% url "ietf.community.views.untrack_document" username=user.username name=doc.name %}" title="Remove from your personal ID list"><span class="fa fa-bookmark"></span> Untrack</a>
588588
<a class="btn btn-default btn-xs track-untrack-doc {% if doc.tracked_in_personal_community_list %}hide{% endif %}" href="{% url "ietf.community.views.track_document" username=user.username name=doc.name %}" title="Add to your personal ID list"><span class="fa fa-bookmark-o"></span> Track</a>
589589
{% endif %}
590+
{% if user.review_teams %}
591+
<a class="btn btn-default btn-xs review-wish-add-remove-doc ajax {% if not doc.has_review_wish %}hide{% endif %}" href="{% url "ietf.doc.views_review.review_wishes_remove" name=doc.name %}?next={{ request.get_full_path|urlencode }}" title="Remove from your review wishes for all teams"><span class="fa fa-comments"></span> Remove review wishes</a>
592+
<a class="btn btn-default btn-xs review-wish-add-remove-doc {% if user.review_teams|length_is:"1" %}ajax {% endif %}{% if doc.has_review_wish %}hide{% endif %}" href="{% url "ietf.doc.views_review.review_wish_add" name=doc.name %}?next={{ request.get_full_path|urlencode }}" title="Add to your review wishes"><span class="fa fa-comments-o"></span> Add review wish</a>
593+
{% endif %}
590594

591595
{% if can_edit and iesg_state.slug != 'idexists' %}
592596
<a class="btn btn-default btn-xs" href="{% url 'ietf.doc.views_ballot.lastcalltext' name=doc.name %}">Last call text</a>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
{% extends "base.html" %}
2+
{# Copyright The IETF Trust 2016, All Rights Reserved #}
3+
{% load origin bootstrap3 static %}
4+
5+
{% block title %}Add {{ doc.name }} to your review wishes{% endblock %}
6+
7+
{% block content %}
8+
{% origin %}
9+
<h1>Add {{ doc.name }} to your review wishes
10+
</h1>
11+
12+
<p>You are a reviewer for multiple teams, and need to select a team first.</p>
13+
<form class="form-horizontal" method="post">
14+
{% csrf_token %}
15+
16+
{% bootstrap_form form layout="horizontal" %}
17+
18+
{% buttons %}
19+
<button type="submit" class="btn btn-primary">Add to review wishes</button>
20+
{% endbuttons %}
21+
</form>
22+
23+
{% endblock %}

0 commit comments

Comments
 (0)