From 6e55f26dbd075c1b6318de8c5e160e3511bdc195 Mon Sep 17 00:00:00 2001 From: Sasha Romijn Date: Tue, 29 Oct 2019 16:27:56 +0000 Subject: [PATCH] Fix #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 --- ietf/community/utils.py | 14 ---- ietf/doc/tests_review.py | 51 +++++++++++++- ietf/doc/urls_review.py | 2 + ietf/doc/utils.py | 34 +++++++++- ietf/doc/utils_search.py | 4 +- ietf/doc/views_doc.py | 14 ++-- ietf/doc/views_review.py | 68 ++++++++++++++++++- ietf/static/ietf/js/ietf.js | 27 +++++--- ietf/templates/doc/document_draft.html | 4 ++ .../templates/doc/review/review_wish_add.html | 23 +++++++ .../doc/review/review_wishes_remove.html | 20 ++++++ .../doc/search/search_result_row.html | 10 +++ 12 files changed, 234 insertions(+), 37 deletions(-) create mode 100644 ietf/templates/doc/review/review_wish_add.html create mode 100644 ietf/templates/doc/review/review_wishes_remove.html diff --git a/ietf/community/utils.py b/ietf/community/utils.py index 2d7ed5ef5..f773839c7 100644 --- a/ietf/community/utils.py +++ b/ietf/community/utils.py @@ -58,20 +58,6 @@ def can_manage_community_list(user, clist): return False -def augment_docs_with_tracking_info(docs, user): - """Add attribute to each document with whether the document is tracked - by the user or not.""" - - tracked = set() - - if user and user.is_authenticated: - clist = CommunityList.objects.filter(user=user).first() - if clist: - tracked.update(docs_tracked_by_community_list(clist).filter(pk__in=[ d.pk for d in docs ]).values_list("pk", flat=True)) - - for d in docs: - d.tracked_in_personal_community_list = d.pk in tracked - def reset_name_contains_index_for_rule(rule): if not rule.rule_type == "name_contains": return diff --git a/ietf/doc/tests_review.py b/ietf/doc/tests_review.py index a72344e12..a1d8fb909 100644 --- a/ietf/doc/tests_review.py +++ b/ietf/doc/tests_review.py @@ -22,7 +22,8 @@ from pyquery import PyQuery import debug # pyflakes:ignore import ietf.review.mailarch -from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory, ReviewFactory +from ietf.doc.factories import NewRevisionDocEventFactory, WgDraftFactory, WgRfcFactory, \ + ReviewFactory, DocumentFactory from ietf.doc.models import DocumentAuthor, RelatedDocument, DocEvent, ReviewRequestDocEvent, ReviewAssignmentDocEvent from ietf.group.factories import RoleFactory, ReviewTeamFactory from ietf.group.models import Group @@ -1164,3 +1165,51 @@ class ReviewTests(TestCase): assignment = reload_db_objects(assignment) self.assertEqual(assignment.state_id, 'withdrawn') + def test_review_wish_add(self): + doc = DocumentFactory() + team = ReviewTeamFactory() + reviewer = RoleFactory(group=team, name_id='reviewer').person + url = urlreverse('ietf.doc.views_review.review_wish_add', kwargs={'name': doc.name}) + + login_testing_unauthorized(self, reviewer.user.username, url) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + # As this reviewer is only on a single team, posting without data should work + r = self.client.post(url + '?next=/redirect-url') + self.assertRedirects(r, '/redirect-url', fetch_redirect_response=False) + self.assertTrue(ReviewWish.objects.get(person=reviewer, doc=doc, team=team)) + + # Try again with a reviewer on multiple teams, requiring team selection. + # This also uses an invalid redirect URL that should be ignored. + ReviewWish.objects.all().delete() + team2 = ReviewTeamFactory() + RoleFactory(group=team2, person=reviewer, name_id='reviewer') + + r = self.client.post(url + '?next=http://example.com/') + self.assertEqual(r.status_code, 200) # Missing team parameter + + r = self.client.post(url + '?next=http://example.com/', data={'team': team2.pk}) + self.assertRedirects(r, doc.get_absolute_url(), fetch_redirect_response=False) + self.assertTrue(ReviewWish.objects.get(person=reviewer, doc=doc, team=team2)) + + def test_review_wishes_remove(self): + doc = DocumentFactory() + team = ReviewTeamFactory() + reviewer = RoleFactory(group=team, name_id='reviewer').person + ReviewWish.objects.create(person=reviewer, doc=doc, team=team) + url = urlreverse('ietf.doc.views_review.review_wishes_remove', kwargs={'name': doc.name}) + + login_testing_unauthorized(self, reviewer.user.username, url) + r = self.client.get(url) + self.assertEqual(r.status_code, 200) + + r = self.client.post(url + '?next=/redirect-url') + self.assertRedirects(r, '/redirect-url', fetch_redirect_response=False) + self.assertFalse(ReviewWish.objects.all()) + + # Try again with an invalid redirect URL that should be ignored. + ReviewWish.objects.create(person=reviewer, doc=doc, team=team) + r = self.client.post(url + '?next=http://example.com') + self.assertRedirects(r, doc.get_absolute_url(), fetch_redirect_response=False) + self.assertFalse(ReviewWish.objects.all()) diff --git a/ietf/doc/urls_review.py b/ietf/doc/urls_review.py index 7adce5145..3745332b9 100644 --- a/ietf/doc/urls_review.py +++ b/ietf/doc/urls_review.py @@ -20,4 +20,6 @@ urlpatterns = [ url(r'^team/%(acronym)s/searchmailarchive/$' % settings.URL_REGEXPS, views_review.search_mail_archive), url(r'^(?P[0-9]+)/editcomment/$', views_review.edit_comment), url(r'^(?P[0-9]+)/editdeadline/$', views_review.edit_deadline), + url(r'^addreviewwish/$' % settings.URL_REGEXPS, views_review.review_wish_add), + url(r'^removereviewwishes/$' % settings.URL_REGEXPS, views_review.review_wishes_remove), ] diff --git a/ietf/doc/utils.py b/ietf/doc/utils.py index 91a2e79be..30b38a2f4 100644 --- a/ietf/doc/utils.py +++ b/ietf/doc/utils.py @@ -23,14 +23,18 @@ from django.utils.html import escape from django.urls import reverse as urlreverse import debug # pyflakes:ignore +from ietf.community.models import CommunityList +from ietf.community.utils import docs_tracked_by_community_list from ietf.doc.models import Document, DocHistory, State, DocumentAuthor, DocHistoryAuthor from ietf.doc.models import DocAlias, RelatedDocument, RelatedDocHistory, BallotType, DocReminder from ietf.doc.models import DocEvent, ConsensusDocEvent, BallotDocEvent, NewRevisionDocEvent, StateDocEvent from ietf.doc.models import TelechatDocEvent from ietf.name.models import DocReminderTypeName, DocRelationshipName -from ietf.group.models import Role +from ietf.group.models import Role, Group from ietf.ietfauth.utils import has_role +from ietf.person.models import Person +from ietf.review.models import ReviewWish from ietf.utils import draft, text from ietf.utils.mail import send_mail from ietf.mailtrigger.utils import gather_address_lists @@ -903,3 +907,31 @@ def build_doc_meta_block(doc, path): # return block + +def augment_docs_and_user_with_user_info(docs, user): + """Add attribute to each document with whether the document is tracked + or has a review wish by the user or not, and the review teams the user is on.""" + + tracked = set() + review_wished = set() + + if user and user.is_authenticated: + user.review_teams = Group.objects.filter( + reviewteamsettings__isnull=False, role__person__user=user, role__name='reviewer') + + doc_pks = [d.pk for d in docs] + clist = CommunityList.objects.filter(user=user).first() + if clist: + tracked.update( + docs_tracked_by_community_list(clist).filter(pk__in=doc_pks).values_list("pk", flat=True)) + + try: + wishes = ReviewWish.objects.filter(person=Person.objects.get(user=user)) + wishes = wishes.filter(doc__pk__in=doc_pks).values_list("doc__pk", flat=True) + review_wished.update(wishes) + except Person.DoesNotExist: + pass + + for d in docs: + d.tracked_in_personal_community_list = d.pk in tracked + d.has_review_wish = d.pk in review_wished diff --git a/ietf/doc/utils_search.py b/ietf/doc/utils_search.py index 6864a67b5..47383a12d 100644 --- a/ietf/doc/utils_search.py +++ b/ietf/doc/utils_search.py @@ -7,9 +7,9 @@ from __future__ import absolute_import, print_function, unicode_literals import datetime import debug # pyflakes:ignore -from ietf.community.utils import augment_docs_with_tracking_info from ietf.doc.models import Document, DocAlias, RelatedDocument, DocEvent, TelechatDocEvent, BallotDocEvent from ietf.doc.expire import expirable_draft +from ietf.doc.utils import augment_docs_and_user_with_user_info from ietf.meeting.models import SessionPresentation, Meeting, Session def wrap_value(v): @@ -162,7 +162,7 @@ def prepare_document_table(request, docs, query=None, max_results=200): docs = list(docs) fill_in_document_table_attributes(docs) - augment_docs_with_tracking_info(docs, request.user) + augment_docs_and_user_with_user_info(docs, request.user) meta = {} diff --git a/ietf/doc/views_doc.py b/ietf/doc/views_doc.py index cffe273aa..0e7bf6e23 100644 --- a/ietf/doc/views_doc.py +++ b/ietf/doc/views_doc.py @@ -58,12 +58,12 @@ import debug # pyflakes:ignore from ietf.doc.models import ( Document, DocAlias, DocHistory, DocEvent, BallotDocEvent, ConsensusDocEvent, NewRevisionDocEvent, TelechatDocEvent, WriteupDocEvent, IESG_BALLOT_ACTIVE_STATES, STATUSCHANGE_RELATIONS ) -from ietf.doc.utils import ( add_links_in_new_revision_events, augment_events_with_revision, - can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, - needed_ballot_positions, nice_consensus, prettify_std_name, update_telechat, has_same_ballot, - get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, - add_events_message_info, get_unicode_document_content, build_doc_meta_block) -from ietf.community.utils import augment_docs_with_tracking_info +from ietf.doc.utils import (add_links_in_new_revision_events, augment_events_with_revision, + can_adopt_draft, can_unadopt_draft, get_chartering_type, get_tags_for_stream_id, + needed_ballot_positions, nice_consensus, prettify_std_name, update_telechat, has_same_ballot, + get_initial_notify, make_notify_changed_event, make_rev_history, default_consensus, + add_events_message_info, get_unicode_document_content, build_doc_meta_block, + augment_docs_and_user_with_user_info) from ietf.group.models import Role, Group from ietf.group.utils import can_manage_group_type, can_manage_materials, group_features_role_filter 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): elif can_edit_stream_info and (iesg_state.slug in ('idexists','watching')): actions.append(("Submit to IESG for Publication", urlreverse('ietf.doc.views_draft.to_iesg', kwargs=dict(name=doc.name)))) - augment_docs_with_tracking_info([doc], request.user) + augment_docs_and_user_with_user_info([doc], request.user) replaces = [d.name for d in doc.related_that_doc("replaces")] replaced_by = [d.name for d in doc.related_that("replaces")] diff --git a/ietf/doc/views_review.py b/ietf/doc/views_review.py index 33ced4049..1b6156a73 100644 --- a/ietf/doc/views_review.py +++ b/ietf/doc/views_review.py @@ -5,14 +5,17 @@ from __future__ import absolute_import, print_function, unicode_literals import io +import json import os import datetime import requests import email.utils +from django.utils.http import is_safe_url + import debug # pyflakes:ignore -from django.http import HttpResponseForbidden, JsonResponse +from django.http import HttpResponseForbidden, JsonResponse, HttpResponse, HttpResponseRedirect from django.shortcuts import render, get_object_or_404, redirect from django import forms from django.conf import settings @@ -27,7 +30,7 @@ from ietf.doc.models import (Document, NewRevisionDocEvent, State, DocAlias, from ietf.name.models import ReviewRequestStateName, ReviewAssignmentStateName, ReviewResultName, \ DocTypeName, ReviewTypeName from ietf.person.models import Person -from ietf.review.models import ReviewRequest, ReviewAssignment +from ietf.review.models import ReviewRequest, ReviewAssignment, ReviewWish from ietf.group.models import Group from ietf.ietfauth.utils import is_authorized_in_doc_stream, user_is_person, has_role from ietf.message.models import Message @@ -967,3 +970,64 @@ def edit_deadline(request, name, request_id): 'review_req': review_req, 'form' : form, }) + + +class ReviewWishAddForm(forms.Form): + team = forms.ModelChoiceField(queryset=Group.objects.filter(reviewteamsettings__isnull=False), + widget=forms.RadioSelect, empty_label=None, required=True) + + def __init__(self, user, doc, *args, **kwargs): + super(ReviewWishAddForm, self).__init__(*args, **kwargs) + self.person = get_object_or_404(Person, user=user) + self.doc = doc + self.fields['team'].queryset = self.fields['team'].queryset.filter(role__person=self.person, + role__name='reviewer') + if len(self.fields['team'].queryset) == 1: + self.team = self.fields['team'].queryset.get() + del self.fields['team'] + + def save(self): + team = self.team if hasattr(self, 'team') else self.cleaned_data['team'] + ReviewWish.objects.get_or_create(person=self.person, team=team, doc=self.doc) + +@login_required +def review_wish_add(request, name): + doc = get_object_or_404(Document, docalias__name=name) + + if request.method == "POST": + form = ReviewWishAddForm(request.user, doc, request.POST) + if form.is_valid(): + form.save() + return _generate_ajax_or_redirect_response(request, doc) + else: + form = ReviewWishAddForm(request.user, doc) + + return render(request, "doc/review/review_wish_add.html", { + "doc": doc, + "form": form, + }) + +@login_required +def review_wishes_remove(request, name): + doc = get_object_or_404(Document, docalias__name=name) + person = get_object_or_404(Person, user=request.user) + + if request.method == "POST": + ReviewWish.objects.filter(person=person, doc=doc).delete() + return _generate_ajax_or_redirect_response(request, doc) + + return render(request, "doc/review/review_wishes_remove.html", { + "name": doc.name, + }) + + +def _generate_ajax_or_redirect_response(request, doc): + redirect_url = request.GET.get('next') + url_is_safe = is_safe_url(url=redirect_url, allowed_hosts=request.get_host(), + require_https=request.is_secure()) + if request.is_ajax(): + return HttpResponse(json.dumps({'success': True}), content_type='application/json') + elif url_is_safe: + return HttpResponseRedirect(redirect_url) + else: + return HttpResponseRedirect(doc.get_absolute_url()) diff --git a/ietf/static/ietf/js/ietf.js b/ietf/static/ietf/js/ietf.js index f7f4b5a23..8d4372755 100644 --- a/ietf/static/ietf/js/ietf.js +++ b/ietf/static/ietf/js/ietf.js @@ -98,10 +98,9 @@ $(document).ready(function () { updateAdvanced(); } - - // search results - $('.track-untrack-doc').click(function(e) { - e.preventDefault(); + + $('.review-wish-add-remove-doc.ajax, .track-untrack-doc').click(function(e) { + e.preventDefault(); var trigger = $(this); $.ajax({ url: trigger.attr('href'), @@ -109,13 +108,21 @@ $(document).ready(function () { cache: false, dataType: 'json', success: function(response){ - if (response.success) { - trigger.parent().find(".tooltip").remove(); - trigger.addClass("hide"); - trigger.parent().find(".track-untrack-doc").not(trigger).removeClass("hide"); + if (response.success) { + trigger.parent().find(".tooltip").remove(); + trigger.addClass("hide"); + + var target_unhide = null; + if(trigger.hasClass('review-wish-add-remove-doc')) { + target_unhide = '.review-wish-add-remove-doc'; + } else if(trigger.hasClass('track-untrack-doc')) { + target_unhide = '.track-untrack-doc'; } - } - }); + if(target_unhide) {} + trigger.parent().find(target_unhide).not(trigger).removeClass("hide"); + } + } + }); }); }); diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index fc70217ea..1bae0cc82 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -587,6 +587,10 @@ Untrack Track {% endif %} + {% if user.review_teams %} + Remove review wishes + Add review wish + {% endif %} {% if can_edit and iesg_state.slug != 'idexists' %} Last call text diff --git a/ietf/templates/doc/review/review_wish_add.html b/ietf/templates/doc/review/review_wish_add.html new file mode 100644 index 000000000..e8898c51c --- /dev/null +++ b/ietf/templates/doc/review/review_wish_add.html @@ -0,0 +1,23 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Add {{ doc.name }} to your review wishes{% endblock %} + +{% block content %} + {% origin %} +

Add {{ doc.name }} to your review wishes +

+ +

You are a reviewer for multiple teams, and need to select a team first.

+
+ {% csrf_token %} + + {% bootstrap_form form layout="horizontal" %} + + {% buttons %} + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/review/review_wishes_remove.html b/ietf/templates/doc/review/review_wishes_remove.html new file mode 100644 index 000000000..b9e203fe6 --- /dev/null +++ b/ietf/templates/doc/review/review_wishes_remove.html @@ -0,0 +1,20 @@ +{% extends "base.html" %} +{# Copyright The IETF Trust 2016, All Rights Reserved #} +{% load origin bootstrap3 static %} + +{% block title %}Remove {{ doc.name }} from your review wishes{% endblock %} + +{% block content %} + {% origin %} +

Remove {{ doc.name }} from your review wishes +

+ +
+ {% csrf_token %} + + {% buttons %} + + {% endbuttons %} +
+ +{% endblock %} diff --git a/ietf/templates/doc/search/search_result_row.html b/ietf/templates/doc/search/search_result_row.html index 773bcbaaa..44ccb1223 100644 --- a/ietf/templates/doc/search/search_result_row.html +++ b/ietf/templates/doc/search/search_result_row.html @@ -21,6 +21,16 @@
{% endif %} + {% if user.review_teams %} + + + + + + +
+ {% endif %} + {% for session in doc.sessions %}