From a4fa71f66e12a41325e0f8d5fc924b0befb2eeec Mon Sep 17 00:00:00 2001 From: Dave Dash Date: Mon, 12 Jul 2010 11:45:19 -0700 Subject: [PATCH 1/3] bug 574290, add to collection --- apps/addons/templates/addons/details.html | 57 +++++++------- apps/addons/tests/test_views.py | 32 +++++--- apps/amo/fixtures/base/fixtures.json | 77 ++++++++++--------- apps/bandwagon/forms.py | 3 +- apps/bandwagon/models.py | 30 ++++++++ apps/bandwagon/sql/collectionuser.sql | 3 +- .../templates/bandwagon/ajax_list.html | 12 +++ .../templates/bandwagon/ajax_new.html | 29 +++++++ apps/bandwagon/tests/test_ajax.py | 65 ++++++++++++++++ apps/bandwagon/urls.py | 7 ++ apps/bandwagon/views.py | 68 +++++++++++++++- media/css/zamboni/zamboni.css | 12 +++ media/js/zamboni/addon_details.js | 62 +++++++++++++++ 13 files changed, 380 insertions(+), 77 deletions(-) create mode 100644 apps/bandwagon/templates/bandwagon/ajax_list.html create mode 100644 apps/bandwagon/templates/bandwagon/ajax_new.html create mode 100644 apps/bandwagon/tests/test_ajax.py diff --git a/apps/addons/templates/addons/details.html b/apps/addons/templates/addons/details.html index 3ef9637b43d..71e41332dff 100644 --- a/apps/addons/templates/addons/details.html +++ b/apps/addons/templates/addons/details.html @@ -26,7 +26,7 @@

{{ _('by') }} {{ users_list(addon.listed_authors) }}

{# TODO(fwenzel): "add-on has been added to collection" notification #} -
+
{# /collections #} diff --git a/apps/addons/tests/test_views.py b/apps/addons/tests/test_views.py index d9fded32b85..127790049ca 100644 --- a/apps/addons/tests/test_views.py +++ b/apps/addons/tests/test_views.py @@ -199,8 +199,10 @@ def test_ppal_return_url_not_relative(self): response = self.client.get(reverse('addons.contribute', args=[592]), follow=True) redirect_url = response.redirect_chain[0][0] + assert(re.search('\?|&return=https?%3A%2F%2F', redirect_url)), \ - "return URL param did not start w/ http%3A%2F%2F (http://) [%s]" % redirect_url + ("return URL param did not start w/ " + "http%3A%2F%2F (http://) [%s]" % redirect_url) def test_redirect_params_type_monthly(self): """Test that we have the required ppal param when @@ -440,13 +442,18 @@ def test_invalid_version(self): eq_(response.status_code, 404) def test_login_links(self): - """Make sure the login links on this page, redirect back to itself.""" + """ + Make sure the login links on this page, redirect back to itself. + + Note: This test needs to be changed if you add/remove a login link from + the detail page. + """ url = reverse('addons.detail', args=[3615]) resp = self.client.get(url, follow=True) sel = 'a[href$="%s"]' % urlparams(reverse('users.login'), to=url) doc = pq(resp.content) - eq_(len(doc(sel)), 4) # 4 login links + eq_(len(doc(sel)), 5) # 5 login links def test_other_author_addons(self): """ @@ -474,7 +481,7 @@ def test_other_author_addons(self): response = forward_to(u'\u271D') eq_(response.status_code, 400) - def test_details_collections_dropdown(self): + def test_collections_dropdown(self): request = Mock() request.APP.id = 1 @@ -518,12 +525,6 @@ def test_no_listed_authors(self): doc = pq(r.content) eq_(0, len(doc('.avatar'))) - def test_collection_detal_url(self): - self.client.login(username='regular@mozilla.com', password='password') - r = self.client.get(reverse('addons.detail', args=[3615])) - url = pq(r.content)('[data-detail-url]').attr('data-detail-url') - eq_(url, '/en-US/firefox/collection/') - def test_search_engine_works_with(self): """We don't display works-with info for search engines.""" addon = Addon.objects.filter(type=amo.ADDON_SEARCH)[0] @@ -538,6 +539,17 @@ def test_search_engine_works_with(self): assert any(th.text.strip().lower() == 'works with' for th in headings) + def test_collections_login_form(self): + # logged out + r = self.client.get(reverse('addons.detail', args=[3615])) + doc = (pq(r.content)) + eq_(len(doc('.collection-add-login')), 1) + # logged in + self.client.login(username='regular@mozilla.com', password='password') + r = self.client.get(reverse('addons.detail', args=[3615])) + doc = (pq(r.content)) + eq_(len(doc('.collection-add-login')), 0) + class TestTagsBox(amo.test_utils.ExtraSetup, test_utils.TestCase): fixtures = ['base/addontag', 'base/apps'] diff --git a/apps/amo/fixtures/base/fixtures.json b/apps/amo/fixtures/base/fixtures.json index 2c495c7a62b..9a2eafee4a8 100644 --- a/apps/amo/fixtures/base/fixtures.json +++ b/apps/amo/fixtures/base/fixtures.json @@ -8818,36 +8818,11 @@ "last_login": "2010-04-23 10:33:53", "groups": [], "user_permissions": [], - "password": "yermom", + "password": "sha1$a04e0$0512298efb3e6e7dbace3976474151d396078fdd", "email": "clouserw@gmail.com", "date_joined": "2007-03-05 13:09:38" } }, - { - "pk": 80, - "model": "bandwagon.collection", - "fields": { - "description": 394558, - "rating": 0.0, - "uuid": "a5f1b177-c01d-c306-3030-e01598892405", - "default_locale": "en-US", - "created": "2009-06-09 23:35:10", - "monthly_subscribers": 0, - "nickname": "webdev", - "downvotes": 0, - "weekly_subscribers": 0, - "modified": "2009-06-09 23:37:36", - "addon_count": 2, - "application": 1, - "listed": 1, - "subscribers": 1, - "upvotes": 0, - "icontype": "", - "type": 0, - "downloads": 46, - "name": 394557 - } - }, { "pk": 50, "model": "bandwagon.collection", @@ -8873,17 +8848,6 @@ "name": 394487 } }, - { - "pk": 47, - "model": "bandwagon.collectionpromo", - "fields": { - "locale": "", - "collection_feature": 1, - "modified": null, - "collection": 80, - "created": "2010-01-05 18:16:11" - } - }, { "pk": 10482, "model": "users.userprofile", @@ -8905,7 +8869,7 @@ "lastname": "Clouser", "emailhidden": 0, "user": 10482, - "password": "yermom", + "password": "sha1$a04e0$0512298efb3e6e7dbace3976474151d396078fdd", "nickname": "clouserw", "resetcode_expires": "2010-01-01 00:00:00", "resetcode": "", @@ -8915,6 +8879,43 @@ "notifyevents": 1 } }, + { + "pk": 80, + "model": "bandwagon.collection", + "fields": { + "description": 394558, + "rating": 0.0, + "uuid": "a5f1b177-c01d-c306-3030-e01598892405", + "default_locale": "en-US", + "created": "2009-06-09 23:35:10", + "monthly_subscribers": 0, + "nickname": "webdev", + "downvotes": 0, + "weekly_subscribers": 0, + "modified": "2009-06-09 23:37:36", + "addon_count": 2, + "application": 1, + "listed": 1, + "subscribers": 1, + "upvotes": 0, + "icontype": "", + "type": 0, + "downloads": 46, + "name": 394557, + "author": 10482 + } + }, + { + "pk": 47, + "model": "bandwagon.collectionpromo", + "fields": { + "locale": "", + "collection_feature": 1, + "modified": null, + "collection": 80, + "created": "2010-01-05 18:16:11" + } + }, { "pk": 5, "model": "bandwagon.collection", diff --git a/apps/bandwagon/forms.py b/apps/bandwagon/forms.py index 8cc0410d12e..b952cad360f 100644 --- a/apps/bandwagon/forms.py +++ b/apps/bandwagon/forms.py @@ -23,7 +23,8 @@ class CollectionForm(forms.ModelForm): label=_('Give your collection a name.')) slug = forms.CharField(label=_('URL:')) description = forms.CharField(label=_('Describe your collections.'), - widget=forms.Textarea, required=False) + widget=forms.Textarea(attrs={'rows': 3}), + required=False) listed = forms.ChoiceField( label=_('Who can view your collection?'), widget=forms.RadioSelect, diff --git a/apps/bandwagon/models.py b/apps/bandwagon/models.py index 3ed8fbfbe8c..a211ebc8ff9 100644 --- a/apps/bandwagon/models.py +++ b/apps/bandwagon/models.py @@ -8,6 +8,8 @@ from django.core.cache import cache from django.db import models, connection +from MySQLdb import IntegrityError + import amo import amo.models from amo.utils import sorted_groupby @@ -36,6 +38,14 @@ def __set__(self, obj, value): class CollectionManager(amo.models.ManagerBase): + def manual(self): + """Only hand-crafted, favorites, and featured collections should appear + in this filter.""" + types = (amo.COLLECTION_NORMAL, amo.COLLECTION_FAVORITE, + amo.COLLECTION_FEATURED, ) + + return self.filter(type__in=types) + def listed(self): """Return public collections only.""" return self.filter(listed=True) @@ -187,6 +197,26 @@ def is_subscribed(self, user): """Determines if the user is subscribed to this collection.""" return self.subscriptions.filter(user=user).exists() + # TODO(davedash): use this when we're on 1.3: + # http://code.djangoproject.com/ticket/13240 + def add_addon(self, addon): + "Adds an addon to the collection." + ca = CollectionAddon() + ca.addon = addon + ca.collection = self + try: + ca.save() + except IntegrityError: + pass + self.save() # To invalidate Collection. + + def remove_addon(self, addon): + CollectionAddon.objects.filter(addon=addon, collection=self).delete() + self.save() # To invalidate Collection. + + def is_owner(self, user): + return (user.id == self.author_id) + class CollectionAddon(amo.models.ModelBase): addon = models.ForeignKey(Addon) diff --git a/apps/bandwagon/sql/collectionuser.sql b/apps/bandwagon/sql/collectionuser.sql index 66cad10fdd2..0ffa5bde3b4 100644 --- a/apps/bandwagon/sql/collectionuser.sql +++ b/apps/bandwagon/sql/collectionuser.sql @@ -1,4 +1,5 @@ -ALTER TABLE `collections_users` CHANGE COLUMN `id` `id` int(11) NOT NULL; +ALTER TABLE `collections_users` + CHANGE COLUMN `id` `id` int(11) NOT NULL DEFAULT 0; ALTER TABLE `collections_users` DROP PRIMARY KEY, ADD PRIMARY KEY(`collection_id`, `user_id`); diff --git a/apps/bandwagon/templates/bandwagon/ajax_list.html b/apps/bandwagon/templates/bandwagon/ajax_list.html new file mode 100644 index 00000000000..c05fb1d82c7 --- /dev/null +++ b/apps/bandwagon/templates/bandwagon/ajax_list.html @@ -0,0 +1,12 @@ +{% if collections %} +
    + {% for collection in collections %} +
  • + {{ collection.name }} +
  • + {% endfor %} +
+ +

{{ _('Start a new collection...') }}

+{% endif %} diff --git a/apps/bandwagon/templates/bandwagon/ajax_new.html b/apps/bandwagon/templates/bandwagon/ajax_new.html new file mode 100644 index 00000000000..d9ef732440c --- /dev/null +++ b/apps/bandwagon/templates/bandwagon/ajax_new.html @@ -0,0 +1,29 @@ +
+

{{ _('Start a New Collection') }}

+ +
+
+

+ {{ form.errors['name']|safe }} + {{ form.name.label|safe }} + {{ form.name|safe }} +

+

+ {{ form.errors['slug']|safe }} + {{ form.slug.label|safe }} + {{ url('collections.user', user.get_profile().nickname)|absolutify -}} + {{ form.slug|safe }} +

+

+ {{ form.description.label|safe }} {{ _('(optional)') }} + {{ form.description|safe }} +

+ + {{ form.listed|safe }} +
+

+ + {% trans %}or Cancel{% endtrans %} +

+
+
diff --git a/apps/bandwagon/tests/test_ajax.py b/apps/bandwagon/tests/test_ajax.py new file mode 100644 index 00000000000..a49ac0053bd --- /dev/null +++ b/apps/bandwagon/tests/test_ajax.py @@ -0,0 +1,65 @@ +import test_utils +from nose.tools import eq_ +from pyquery import PyQuery as pq + +from amo.urlresolvers import reverse +from bandwagon.models import Collection + + +class AjaxTest(test_utils.TestCase): + fixtures = ['base/fixtures'] + + def test_list_collections(self): + self.client.login(username='clouserw@gmail.com', password='yermom') + r = self.client.get(reverse('collections.ajax_list') + + '?addon_id=1843',) + doc = pq(r.content) + eq_(doc('li.selected').attr('data-id'), '80') + + def test_add_collection(self): + self.client.login(username='clouserw@gmail.com', password='yermom') + r = self.client.post(reverse('collections.ajax_add'), + {'addon_id': 3615, 'id': 80}, follow=True) + doc = pq(r.content) + eq_(doc('li.selected').attr('data-id'), '80') + + def test_remove_collection(self): + self.client.login(username='clouserw@gmail.com', password='yermom') + r = self.client.post(reverse('collections.ajax_remove'), + {'addon_id': 1843, 'id': 80}, follow=True) + doc = pq(r.content) + eq_(len(doc('li.selected')), 0) + + def test_new_collection(self): + num_collections = Collection.objects.all().count() + self.client.login(username='clouserw@gmail.com', password='yermom') + r = self.client.post(reverse('collections.ajax_new'), + {'addon_id': 3615, + 'name': 'foo', + 'slug': 'auniqueone', + 'description': 'yermom', + 'listed': True}, + follow=True) + doc = pq(r.content) + eq_(len(doc('li.selected')), 1, "The new collection is not selected.") + eq_(Collection.objects.all().count(), num_collections + 1) + + def test_add_other_collection(self): + "403 when you try to add to a collection that isn't yours." + c = Collection() + c.save() + + self.client.login(username='clouserw@gmail.com', password='yermom') + r = self.client.post(reverse('collections.ajax_add'), + {'addon_id': 3615, 'id': c.id}, follow=True) + eq_(r.status_code, 403) + + def test_remove_other_collection(self): + "403 when you try to add to a collection that isn't yours." + c = Collection() + c.save() + + self.client.login(username='clouserw@gmail.com', password='yermom') + r = self.client.post(reverse('collections.ajax_remove'), + {'addon_id': 3615, 'id': c.id}, follow=True) + eq_(r.status_code, 403) diff --git a/apps/bandwagon/urls.py b/apps/bandwagon/urls.py index 2b66e4142ad..d3f974f98e4 100644 --- a/apps/bandwagon/urls.py +++ b/apps/bandwagon/urls.py @@ -9,6 +9,12 @@ name='collections.vote'), ) +ajax_urls = patterns('', + url('^list$', views.ajax_list, name='collections.ajax_list'), + url('^add$', views.ajax_add, name='collections.ajax_add'), + url('^remove$', views.ajax_remove, name='collections.ajax_remove'), + url('^new$', views.ajax_new, name='collections.ajax_new'), +) urlpatterns = patterns('', url('^collection/(?P[^/]+)/?$', views.legacy_redirect), @@ -19,4 +25,5 @@ url('^collections/(?P[^/]+)/(?P[^/]+)/', include(detail_urls)), url('^collections/add$', views.add, name='collections.add'), + url('^collections/ajax/', include(ajax_urls)), ) diff --git a/apps/bandwagon/views.py b/apps/bandwagon/views.py index e65d0e4c3e8..e85698bfa59 100644 --- a/apps/bandwagon/views.py +++ b/apps/bandwagon/views.py @@ -6,12 +6,13 @@ import amo.utils from amo.decorators import login_required +from amo.urlresolvers import reverse from access import acl from addons.models import Addon from addons.views import BaseFilter from tags.models import Tag from translations.query import order_by_translation -from .models import Collection, CollectionAddon, CollectionVote +from .models import Collection, CollectionAddon, CollectionUser, CollectionVote from . import forms @@ -130,3 +131,68 @@ def add(request): data['form'] = form return jingo.render(request, 'bandwagon/add.html', data) + + +def ajax_new(request): + form = forms.CollectionForm(request.POST or None, + initial={'author': request.amo_user, + 'application_id': request.APP.id}, + ) + + if request.method == 'POST': + + if form.is_valid(): + collection = form.save() + CollectionUser(collection=collection, user=request.amo_user).save() + addon_id = request.REQUEST['addon_id'] + a = Addon.objects.get(pk=addon_id) + collection.add_addon(a) + + return http.HttpResponseRedirect(reverse('collections.ajax_list') + + '?addon_id=%s' % addon_id) + + return jingo.render(request, 'bandwagon/ajax_new.html', {'form': form}) + + +@login_required +def ajax_list(request): + # Get collections associated with this user + collections = request.amo_user.collections.manual() + addon_id = int(request.GET['addon_id']) + + for collection in collections: + # See if the collections contains the addon + if addon_id in collection.addons.values_list('id', flat=True): + collection.has_addon = True + + return jingo.render(request, 'bandwagon/ajax_list.html', + {'collections': collections}) + + +def _ajax_add_remove(request, op): + id = request.POST['id'] + addon_id = request.POST['addon_id'] + + c = Collection.objects.get(pk=id) + + if not c.is_owner(request.amo_user): + return http.HttpResponseForbidden() + + a = Addon.objects.get(pk=addon_id) + + if op == 'add': + c.add_addon(a) + else: + c.remove_addon(a) + + # redirect + return http.HttpResponseRedirect(reverse('collections.ajax_list') + + '?addon_id=%s' % addon_id) + + +def ajax_add(request): + return _ajax_add_remove(request, 'add') + + +def ajax_remove(request): + return _ajax_add_remove(request, 'remove') diff --git a/media/css/zamboni/zamboni.css b/media/css/zamboni/zamboni.css index 02d401d9de0..a0721c8c3a9 100644 --- a/media/css/zamboni/zamboni.css +++ b/media/css/zamboni/zamboni.css @@ -2375,3 +2375,15 @@ h6.author, .author a { .reply-form input[type='text'] { width: 60%; } + +.collection-add { + display: none; +} + +.collection-add-dropdown { + display: none; +} + +.collection-add-dropdown .selected { + background: red; +} diff --git a/media/js/zamboni/addon_details.js b/media/js/zamboni/addon_details.js index 36957afee57..e07ea8751ca 100644 --- a/media/js/zamboni/addon_details.js +++ b/media/js/zamboni/addon_details.js @@ -100,3 +100,65 @@ $(document).ready(function () { feedback_widget.show(); }); }); + +/* Add to collection initialization */ +$(document).ready(function () { + var btn = $('.collection-add'); + var dropdown = $('.collection-add-dropdown'); + if (!btn.length) return; + + btn.show(); + + var refreshHandlers = function() { + $('#collections-new button').click(handleSubmit); + $('#ajax_collections_list li').click(handleToggle); + $('#ajax_new_collection').click(handleNew); + $('#collections-new-cancel').click(handleClick); + }; + + var list_url = btn.attr('data-listurl'); + var remove_url = btn.attr('data-removeurl'); + var add_url = btn.attr('data-addurl'); + var form_url = btn.attr('data-newurl'); + var addon_id = $('#addon').attr('data-id'); + + var handleToggle = function() { + var data = {'addon_id': addon_id, + 'id': this.getAttribute('data-id')}; + var url = this.className == "selected" ? remove_url + : add_url; + $.post(url, data, handleClick); + } + + var handleSubmit = function(e) { + e.preventDefault() + form_data = $('#collections-new form').serialize(); + $.post(form_url + '?addon_id=' + addon_id, form_data, function(d) { + dropdown.html(d); + refreshHandlers(); + }); + } + + var handleNew = function(e) { + $.get(form_url, {'addon_id': addon_id}, function(d) { + dropdown.html(d); + refreshHandlers(); + }); + } + + var handleClick = function(e) { + // If anonymous, show login overlay. + dropdown.show(); + // Make a call to /collections/ajax/list with addon_id + if (!z.anonymous) { + $.get(list_url, {'addon_id': addon_id}, function(data) { + dropdown.html(data); + refreshHandlers(); + }, 'html'); + } + }; + btn.click(handleClick); + + + +}); From 38a62ac8976e0152598c601134c1f9d406a3e9c3 Mon Sep 17 00:00:00 2001 From: Matt Claypotch Date: Fri, 30 Jul 2010 16:17:29 -0700 Subject: [PATCH 2/3] Bug 574290, initial prettification --- apps/addons/templates/addons/details.html | 6 +++--- apps/bandwagon/templates/bandwagon/ajax_list.html | 4 ++-- media/css/main-mozilla.css | 2 +- media/css/zamboni/zamboni.css | 1 - 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/apps/addons/templates/addons/details.html b/apps/addons/templates/addons/details.html index 71e41332dff..649c8a9e846 100644 --- a/apps/addons/templates/addons/details.html +++ b/apps/addons/templates/addons/details.html @@ -115,9 +115,9 @@

{{ _('by') }} {{ users_list(addon.listed_authors) }}

data-removeurl="{{ url('collections.ajax_remove') }}" data-newurl="{{ url('collections.ajax_new') }}" > - Add to collection + Add to collection
-