From e807115e819de622ec99e015aa2b2232c8db08d9 Mon Sep 17 00:00:00 2001 From: Ole Laursen Date: Wed, 13 Jan 2016 10:15:38 +0000 Subject: [PATCH] Fix community list track/untrack to use POST rather than GET - Legacy-Id: 10660 --- ietf/community/tests.py | 30 +++++ ietf/community/urls.py | 7 +- ietf/community/views.py | 120 ++++++++---------- ietf/static/ietf/js/ietf.js | 16 +-- ietf/templates/community/track_document.html | 16 +++ .../templates/community/untrack_document.html | 16 +++ ietf/templates/doc/document_draft.html | 4 +- .../doc/search/search_result_row.html | 4 +- 8 files changed, 133 insertions(+), 80 deletions(-) create mode 100644 ietf/community/tests.py create mode 100644 ietf/templates/community/track_document.html create mode 100644 ietf/templates/community/untrack_document.html diff --git a/ietf/community/tests.py b/ietf/community/tests.py new file mode 100644 index 000000000..e5fae2c21 --- /dev/null +++ b/ietf/community/tests.py @@ -0,0 +1,30 @@ +import json + +from django.core.urlresolvers import reverse as urlreverse + +from ietf.community.models import CommunityList +from ietf.utils.test_data import make_test_data +from ietf.utils.test_utils import login_testing_unauthorized, TestCase + +class CommunityListTests(TestCase): + def test_track_untrack_document(self): + draft = make_test_data() + + url = urlreverse("community_track_document", kwargs={ "name": draft.name }) + login_testing_unauthorized(self, "plain", url) + + # track + r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + self.assertEqual(r.status_code, 200) + self.assertEqual(json.loads(r.content)["success"], True) + clist = CommunityList.objects.get(user__username="plain") + self.assertEqual(list(clist.added_ids.all()), [draft]) + + # untrack + url = urlreverse("community_untrack_document", kwargs={ "name": draft.name }) + r = self.client.post(url, HTTP_X_REQUESTED_WITH='XMLHttpRequest') + self.assertEqual(r.status_code, 200) + self.assertEqual(json.loads(r.content)["success"], True) + clist = CommunityList.objects.get(user__username="plain") + self.assertEqual(list(clist.added_ids.all()), []) + diff --git a/ietf/community/urls.py b/ietf/community/urls.py index 0f60ba63c..7c499d4f7 100644 --- a/ietf/community/urls.py +++ b/ietf/community/urls.py @@ -23,9 +23,10 @@ urlpatterns = patterns('ietf.community.views', url(r'^group/(?P[\w.@+-]+)/unsubscribe/$', 'unsubscribe_group_list', {'significant': False}, name='unsubscribe_group_list'), url(r'^group/(?P[\w.@+-]+)/unsubscribe/significant/$', 'unsubscribe_group_list', {'significant': True}, name='unsubscribe_significant_group_list'), - url(r'^add_track_document/(?P[^/]+)/$', 'add_track_document', name='community_add_track_document'), - url(r'^remove_track_document/(?P[^/]+)/$', 'remove_track_document', name='community_remove_track_document'), - url(r'^(?P[\d]+)/remove_document/(?P[^/]+)/$', 'remove_document', name='community_remove_document'), + url(r'^trackdocument/(?P[^/]+)/$', 'track_document', name='community_track_document'), + url(r'^untrackdocument/(?P[^/]+)/$', 'untrack_document', name='community_untrack_document'), + + url(r'^(?P[\d]+)/remove_document/(?P[^/]+)/$', 'remove_document', name='community_remove_document'), url(r'^(?P[\d]+)/remove_rule/(?P[^/]+)/$', 'remove_rule', name='community_remove_rule'), url(r'^(?P[\d]+)/subscribe/confirm/(?P[\w.@+-]+)/(?P[\d]+)/(?P[a-f0-9]+)/$', 'confirm_subscription', name='confirm_subscription'), url(r'^(?P[\d]+)/subscribe/significant/confirm/(?P[\w.@+-]+)/(?P[\d]+)/(?P[a-f0-9]+)/$', 'confirm_significant_subscription', name='confirm_significant_subscription'), diff --git a/ietf/community/views.py b/ietf/community/views.py index 87c2a9702..0b2f41a1c 100644 --- a/ietf/community/views.py +++ b/ietf/community/views.py @@ -7,15 +7,15 @@ import json from django.db import IntegrityError from django.conf import settings from django.contrib.auth import REDIRECT_FIELD_NAME -from django.http import HttpResponse, Http404, HttpResponseRedirect -from django.shortcuts import get_object_or_404, render_to_response -from django.template import RequestContext +from django.http import HttpResponse, HttpResponseForbidden, Http404, HttpResponseRedirect +from django.shortcuts import get_object_or_404, render, redirect from django.utils.http import urlquote +from django.contrib.auth.decorators import login_required from ietf.community.models import CommunityList, Rule, EmailSubscription from ietf.community.forms import RuleForm, DisplayForm, SubscribeForm, UnSubscribeForm from ietf.group.models import Group -from ietf.doc.models import DocEvent, DocAlias +from ietf.doc.models import DocEvent, Document def _manage_list(request, clist): @@ -41,12 +41,11 @@ def _manage_list(request, clist): rule_form = RuleForm(clist=clist) display_form = DisplayForm(instance=display_config) clist = CommunityList.objects.get(id=clist.id) - return render_to_response('community/manage_clist.html', + return render(request, 'community/manage_clist.html', {'cl': clist, 'dc': display_config, 'display_form': display_form, - 'rule_form': rule_form}, - context_instance=RequestContext(request)) + 'rule_form': rule_form}) def manage_personal_list(request): @@ -74,52 +73,49 @@ def manage_group_list(request, acronym): return HttpResponseRedirect('%s?%s=%s' % tup) return _manage_list(request, clist) +@login_required +def track_document(request, name): + doc = get_object_or_404(Document, docalias__name=name) -def add_track_document(request, document_name): - """supports the "Track this document" functionality - - This is exposed in the document view and in document search results.""" - if not request.user.is_authenticated(): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) - doc = get_object_or_404(DocAlias, name=document_name).document - clist = CommunityList.objects.get_or_create(user=request.user)[0] - clist.update() - return add_document_to_list(request, clist, doc) + if request.method == "POST": + clist = CommunityList.objects.get_or_create(user=request.user)[0] + clist.added_ids.add(doc) + if request.is_ajax(): + return HttpResponse(json.dumps({ 'success': True }), content_type='text/plain') + else: + return redirect("manage_personal_list") -def remove_track_document(request, document_name): - """supports the "Untrack this document" functionality - - This is exposed in the document view and in document search results.""" - clist = CommunityList.objects.get_or_create(user=request.user)[0] - if not clist.check_manager(request.user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) - doc = get_object_or_404(DocAlias, name=document_name).document - clist.added_ids.remove(doc) - clist.update() - return HttpResponse(json.dumps({'success': True}), content_type='text/plain') + return render(request, "community/track_document.html", { + "name": doc.name, + }) -def remove_document(request, list_id, document_name): +@login_required +def untrack_document(request, name): + doc = get_object_or_404(Document, docalias__name=name) + clist = get_object_or_404(CommunityList, user=request.user) + + if request.method == "POST": + clist = CommunityList.objects.get_or_create(user=request.user)[0] + clist.added_ids.remove(doc) + if request.is_ajax(): + return HttpResponse(json.dumps({ 'success': True }), content_type='text/plain') + else: + return redirect("manage_personal_list") + + return render(request, "community/untrack_document.html", { + "name": doc.name, + }) + +@login_required +def remove_document(request, list_id, name): clist = get_object_or_404(CommunityList, pk=list_id) if not clist.check_manager(request.user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) - doc = get_object_or_404(DocAlias, name=document_name).document - clist.added_ids.remove(doc) - clist.update() - return HttpResponseRedirect(clist.get_manage_url()) + return HttpResponseForbidden("You do not have permission to access this view") -def add_document_to_list(request, clist, doc): - if not clist.check_manager(request.user): - path = urlquote(request.get_full_path()) - tup = settings.LOGIN_URL, REDIRECT_FIELD_NAME, path - return HttpResponseRedirect('%s?%s=%s' % tup) - clist.added_ids.add(doc) - return HttpResponse(json.dumps({'success': True}), content_type='text/plain') + doc = get_object_or_404(Document, docalias__name=name) + clist.added_ids.remove(doc) + + return HttpResponseRedirect(clist.get_manage_url()) def remove_rule(request, list_id, rule_id): @@ -135,11 +131,10 @@ def remove_rule(request, list_id, rule_id): def _view_list(request, clist): display_config = clist.get_display_config() - return render_to_response('community/public/view_list.html', + return render(request, 'community/public/view_list.html', {'cl': clist, 'dc': display_config, - }, - context_instance=RequestContext(request)) + }) def view_personal_list(request, secret): @@ -172,7 +167,7 @@ def _atom_view(request, clist, significant=False): else: subtitle = 'Document changes' - return render_to_response('community/public/atom.xml', + return render(request, 'community/public/atom.xml', {'cl': clist, 'entries': notifications, 'title': title, @@ -180,8 +175,7 @@ def _atom_view(request, clist, significant=False): 'id': feed_id.get_urn(), 'updated': datetime.datetime.today(), }, - content_type='text/xml', - context_instance=RequestContext(request)) + content_type='text/xml') def changes_personal_list(request, secret): @@ -264,12 +258,11 @@ def _subscribe_list(request, clist, significant): success = True else: form = SubscribeForm(clist=clist, significant=significant) - return render_to_response('community/public/subscribe.html', + return render(request, 'community/public/subscribe.html', {'cl': clist, 'form': form, 'success': success, - }, - context_instance=RequestContext(request)) + }) def _unsubscribe_list(request, clist, significant): @@ -281,13 +274,12 @@ def _unsubscribe_list(request, clist, significant): success = True else: form = UnSubscribeForm(clist=clist, significant=significant) - return render_to_response('community/public/unsubscribe.html', + return render(request, 'community/public/unsubscribe.html', {'cl': clist, 'form': form, 'success': success, 'significant': significant, - }, - context_instance=RequestContext(request)) + }) def subscribe_personal_list(request, secret, significant=False): @@ -321,11 +313,10 @@ def confirm_subscription(request, list_id, email, date, confirm_hash, significan community_list=clist, email=email, significant=significant) - return render_to_response('community/public/subscription_confirm.html', + return render(request, 'community/public/subscription_confirm.html', {'cl': clist, 'significant': significant, - }, - context_instance=RequestContext(request)) + }) def confirm_significant_subscription(request, list_id, email, date, confirm_hash): @@ -341,11 +332,10 @@ def confirm_unsubscription(request, list_id, email, date, confirm_hash, signific community_list=clist, email=email, significant=significant).delete() - return render_to_response('community/public/unsubscription_confirm.html', + return render(request, 'community/public/unsubscription_confirm.html', {'cl': clist, 'significant': significant, - }, - context_instance=RequestContext(request)) + }) def confirm_significant_unsubscription(request, list_id, email, date, confirm_hash): diff --git a/ietf/static/ietf/js/ietf.js b/ietf/static/ietf/js/ietf.js index b40e1a24f..5ab14e5c0 100644 --- a/ietf/static/ietf/js/ietf.js +++ b/ietf/static/ietf/js/ietf.js @@ -105,19 +105,19 @@ $(document).ready(function () { var trigger = $(this); $.ajax({ url: trigger.attr('href'), - type: 'GET', + type: 'POST', cache: false, dataType: 'json', success: function(response){ if (response.success) { - trigger.parent().find(".tooltip").remove(); - trigger.find("span.fa").toggleClass("fa-bookmark fa-bookmark-o"); - if (trigger.hasClass('btn')) { - trigger.attr('disabled', true).blur(); - } else { - trigger.contents().unwrap().blur(); + trigger.parent().find(".tooltip").remove(); + trigger.find("span.fa").toggleClass("fa-bookmark fa-bookmark-o"); + if (trigger.hasClass('btn')) { + trigger.attr('disabled', true).blur(); + } else { + trigger.contents().unwrap().blur(); + } } - } } }); }); diff --git a/ietf/templates/community/track_document.html b/ietf/templates/community/track_document.html new file mode 100644 index 000000000..b592495f2 --- /dev/null +++ b/ietf/templates/community/track_document.html @@ -0,0 +1,16 @@ +{# Copyright The IETF Trust 2015, All Rights Reserved #} +{% load origin %}{% origin %} +{% load bootstrap3 %} + +{% block title %}Track document {{ name }}{% endblock %} + +{% bootstrap_messages %} + +
+ {% csrf_token %} +

Add {{ name }} to the list?

+ + {% buttons %} + + {% endbuttons %} +
diff --git a/ietf/templates/community/untrack_document.html b/ietf/templates/community/untrack_document.html new file mode 100644 index 000000000..ef5156363 --- /dev/null +++ b/ietf/templates/community/untrack_document.html @@ -0,0 +1,16 @@ +{# Copyright The IETF Trust 2015, All Rights Reserved #} +{% load origin %}{% origin %} +{% load bootstrap3 %} + +{% block title %}Remove tracking of document {{ name }}{% endblock %} + +{% bootstrap_messages %} + +
+ {% csrf_token %} +

Remove {{ name }} from the list?

+ + {% buttons %} + + {% endbuttons %} +
diff --git a/ietf/templates/doc/document_draft.html b/ietf/templates/doc/document_draft.html index 7188cb557..85d26bb4e 100644 --- a/ietf/templates/doc/document_draft.html +++ b/ietf/templates/doc/document_draft.html @@ -435,9 +435,9 @@ {% if user.is_authenticated %} {% if tracking_document %} - Untrack + Untrack {% else %} - Track + Track {% endif %} {% endif %} diff --git a/ietf/templates/doc/search/search_result_row.html b/ietf/templates/doc/search/search_result_row.html index 893251326..343c3da14 100644 --- a/ietf/templates/doc/search/search_result_row.html +++ b/ietf/templates/doc/search/search_result_row.html @@ -14,11 +14,11 @@ {% if user.is_authenticated %} {% if doc.name in doc_is_tracked %} - + {% else %} - + {% endif %}