From ec4c45d6147c423256dabec3c17b7e758cc92bdd Mon Sep 17 00:00:00 2001 From: Jeff Balogh Date: Tue, 24 Aug 2010 15:22:15 -0700 Subject: [PATCH] sharing for collections (bug 586444) --- apps/addons/models.py | 8 +++-- apps/addons/templates/addons/details.html | 2 +- .../templates/addons/personas_detail.html | 2 +- apps/amo/cron.py | 4 +-- apps/amo/fixtures/base/garbage.json | 2 +- apps/amo/tests/test_cron.py | 6 ++-- apps/bandwagon/models.py | 14 +++++++- .../bandwagon/collection_listing_items.html | 1 + apps/bandwagon/tests/test_views.py | 23 +++++++++++++ apps/bandwagon/urls.py | 1 + apps/bandwagon/views.py | 8 +++++ .../fixtures/sharing/share_counts.json | 10 +++--- apps/sharing/helpers.py | 24 +++++++------ apps/sharing/models.py | 20 +++++------ .../{addon_sharing.html => sharing_box.html} | 13 ++++--- apps/sharing/tests.py | 34 ++++++++++++++++--- apps/sharing/views.py | 19 +++++++++++ apps/stats/models.py | 16 +++++++-- migrations/68-collections-share.sql | 23 +++++++++++++ 19 files changed, 183 insertions(+), 47 deletions(-) rename apps/sharing/templates/sharing/{addon_sharing.html => sharing_box.html} (82%) create mode 100644 apps/sharing/views.py create mode 100644 migrations/68-collections-share.sql diff --git a/apps/addons/models.py b/apps/addons/models.py index d8af5ce141..5c78d142ac 100644 --- a/apps/addons/models.py +++ b/apps/addons/models.py @@ -15,8 +15,9 @@ import amo.models from amo.fields import DecimalCharField from amo.utils import urlparams, sorted_groupby, JSONEncoder from amo.urlresolvers import reverse +from cake.urlresolvers import remora_url from reviews.models import Review -from stats.models import Contribution as ContributionStats, ShareCountTotal +from stats.models import Contribution as ContributionStats, AddonShareCountTotal from translations.fields import (TranslatedField, PurifiedField, LinkifiedField, translations_with_fallback) from users.models import UserProfile, PersonaAuthor @@ -227,6 +228,9 @@ class Addon(amo.models.ModelBase): """The url for this add-on's AddonType.""" return AddonType(self.type).get_url_path() + def share_url(self): + return remora_url('/addon/share/%s' % self.id) + @amo.cached_property(writable=True) def listed_authors(self): return UserProfile.objects.filter(addons=self, @@ -469,7 +473,7 @@ class Addon(amo.models.ModelBase): @caching.cached_method def share_counts(self): rv = collections.defaultdict(int) - rv.update(ShareCountTotal.objects.filter(addon=self) + rv.update(AddonShareCountTotal.objects.filter(addon=self) .values_list('service', 'count')) return rv diff --git a/apps/addons/templates/addons/details.html b/apps/addons/templates/addons/details.html index 770e121772..fd93d342c1 100644 --- a/apps/addons/templates/addons/details.html +++ b/apps/addons/templates/addons/details.html @@ -114,7 +114,7 @@ {% include 'addons/includes/collection_add_widget.html' %} {% endif %} - {{ addon_sharing(addon) }} + {{ sharing_box(addon) }} {# /secondary #} diff --git a/apps/addons/templates/addons/personas_detail.html b/apps/addons/templates/addons/personas_detail.html index f164a36649..400d3dcfff 100644 --- a/apps/addons/templates/addons/personas_detail.html +++ b/apps/addons/templates/addons/personas_detail.html @@ -73,7 +73,7 @@ {% endif %} {# TODO(davedash): Remove until zamboni does sharing - {{ addon_sharing(addon) }} + {{ sharing_box(addon) }} #} {# /addon-summary and -wrapper #} diff --git a/apps/amo/cron.py b/apps/amo/cron.py index dee856425e..8991608fe7 100644 --- a/apps/amo/cron.py +++ b/apps/amo/cron.py @@ -13,7 +13,7 @@ from cake.models import Session from devhub.models import AddonLog, LOG as ADDONLOG from files.models import TestResult, TestResultCache from sharing.models import SERVICES -from stats.models import ShareCount, Contribution +from stats.models import AddonShareCount, Contribution log = commonware.log.getLogger('z.cron') @@ -33,7 +33,7 @@ def gc(test_result=True): Session.objects.filter(expires__lt=two_days_ago_unixtime).delete() log.debug('Cleaning up sharing services.') - ShareCount.objects.exclude( + AddonShareCount.objects.exclude( service__in=[s.shortname for s in SERVICES]).delete() # XXX(davedash): I can't seem to run this during testing without triggering diff --git a/apps/amo/fixtures/base/garbage.json b/apps/amo/fixtures/base/garbage.json index 3d2260af0f..7b9b458208 100644 --- a/apps/amo/fixtures/base/garbage.json +++ b/apps/amo/fixtures/base/garbage.json @@ -30,7 +30,7 @@ }, { "pk": 2567, - "model": "stats.sharecount", + "model": "stats.addonsharecount", "fields": { "count": 1, "date": "2009-07-27", diff --git a/apps/amo/tests/test_cron.py b/apps/amo/tests/test_cron.py index cd470cef2b..a33b2d9726 100644 --- a/apps/amo/tests/test_cron.py +++ b/apps/amo/tests/test_cron.py @@ -5,7 +5,7 @@ from bandwagon.models import Collection from cake.models import Session from devhub.models import AddonLog from files.models import TestResult, TestResultCache -from stats.models import ShareCount, Contribution +from stats.models import AddonShareCount, Contribution from amo.cron import gc @@ -19,7 +19,7 @@ class GarbageTest(test_utils.TestCase): eq_(AddonLog.objects.all().count(), 1) eq_(TestResult.objects.all().count(), 1) eq_(TestResultCache.objects.all().count(), 1) - eq_(ShareCount.objects.all().count(), 1) + eq_(AddonShareCount.objects.all().count(), 1) eq_(Contribution.objects.all().count(), 1) gc(test_result=False) eq_(Collection.objects.all().count(), 0) @@ -28,5 +28,5 @@ class GarbageTest(test_utils.TestCase): # XXX(davedash): this isn't working in testing. # eq_(TestResult.objects.all().count(), 0) eq_(TestResultCache.objects.all().count(), 0) - eq_(ShareCount.objects.all().count(), 0) + eq_(AddonShareCount.objects.all().count(), 0) eq_(Contribution.objects.all().count(), 0) diff --git a/apps/bandwagon/models.py b/apps/bandwagon/models.py index f098db171b..95ff1448d9 100644 --- a/apps/bandwagon/models.py +++ b/apps/bandwagon/models.py @@ -16,8 +16,9 @@ from amo.utils import sorted_groupby from amo.urlresolvers import reverse from addons.models import Addon, AddonRecommendation from applications.models import Application -from users.models import UserProfile +from stats.models import CollectionShareCountTotal from translations.fields import TranslatedField, LinkifiedField +from users.models import UserProfile SPECIAL_SLUGS = amo.COLLECTION_SPECIAL_SLUGS @@ -177,6 +178,10 @@ class Collection(amo.models.ModelBase): return reverse('collections.delete', args=[self.author_username, self.slug]) + def share_url(self): + return reverse('collections.share', + args=[self.author_username, self.slug]) + @property def author_username(self): return self.author.username if self.author else 'anonymous' @@ -203,6 +208,13 @@ class Collection(amo.models.ModelBase): else: return settings.MEDIA_URL + 'img/amo2009/icons/collection.png' + @caching.cached_method + def share_counts(self): + rv = collections.defaultdict(int) + rv.update(CollectionShareCountTotal.objects.filter(collection=self) + .values_list('service', 'count')) + return rv + def get_recommendations(self): """Get a collection of recommended add-ons for this collection.""" if self.recommended_collection: diff --git a/apps/bandwagon/templates/bandwagon/collection_listing_items.html b/apps/bandwagon/templates/bandwagon/collection_listing_items.html index 16a504c06f..a77a0fcc60 100644 --- a/apps/bandwagon/templates/bandwagon/collection_listing_items.html +++ b/apps/bandwagon/templates/bandwagon/collection_listing_items.html @@ -33,6 +33,7 @@ #} + {{ sharing_box(c, show_email=False) }} {% if request.check_ownership(c, require_owner=False) %} {% endif %} diff --git a/apps/bandwagon/tests/test_views.py b/apps/bandwagon/tests/test_views.py index 8acbbf8a74..1d0130de2f 100644 --- a/apps/bandwagon/tests/test_views.py +++ b/apps/bandwagon/tests/test_views.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- import json +import urlparse import django.test from django.utils.datastructures import MultiValueDict @@ -670,3 +671,25 @@ class TestWatching(test_utils.TestCase): HTTP_X_REQUESTED_WITH='XMLHttpRequest') eq_(r.status_code, 200) eq_(json.loads(r.content), {'watching': True}) + + +class TestSharing(test_utils.TestCase): + fixtures = ['base/collection_57181'] + + def test_twitter_share(self): + c = Collection.objects.get(id=57181) + r = self.client.get(c.share_url() + '?service=twitter') + eq_(r.status_code, 302) + loc = urlparse.urlparse(r['Location']) + query = dict(urlparse.parse_qsl(loc.query)) + eq_(loc.netloc, 'twitter.com') + status = 'Home Business Auto :: Add-ons for Firefox' + assert status in query['status'], query['status'] + + def test_404(self): + c = Collection.objects.get(id=57181) + url = reverse('collections.share', args=[c.author.username, c.slug]) + r = self.client.get(url) + eq_(r.status_code, 404) + r = self.client.get(url + '?service=xxx') + eq_(r.status_code, 404) diff --git a/apps/bandwagon/urls.py b/apps/bandwagon/urls.py index e3b82f1707..b93c6450e1 100644 --- a/apps/bandwagon/urls.py +++ b/apps/bandwagon/urls.py @@ -19,6 +19,7 @@ detail_urls = patterns('', url('^(?Padd|remove)$', views.collection_alter, name='collections.alter'), url('^watch$', views.watch, name='collections.watch'), + url('^share$', views.share, name='collections.share'), ) ajax_urls = patterns('', diff --git a/apps/bandwagon/views.py b/apps/bandwagon/views.py index 252fe2749e..7ff7cd99aa 100644 --- a/apps/bandwagon/views.py +++ b/apps/bandwagon/views.py @@ -11,6 +11,7 @@ import caching.base as caching from tower import ugettext_lazy as _lazy, ugettext as _ import amo.utils +import sharing.views from amo.decorators import login_required, post_required, json_view from amo.urlresolvers import reverse from access import acl @@ -455,3 +456,10 @@ def watch(request, username, slug): return {'watching': watching} else: return redirect(collection.get_url_path()) + + +def share(request, username, slug): + collection = get_collection(request, username, slug) + return sharing.views.share(request, collection, + name=collection.name, + description=collection.description) diff --git a/apps/sharing/fixtures/sharing/share_counts.json b/apps/sharing/fixtures/sharing/share_counts.json index 1adff7653c..01730d1fc2 100644 --- a/apps/sharing/fixtures/sharing/share_counts.json +++ b/apps/sharing/fixtures/sharing/share_counts.json @@ -1,7 +1,7 @@ [ { "pk": 177155738, - "model": "stats.sharecounttotal", + "model": "stats.addonsharecounttotal", "fields": { "count": 13, "service": "delicious", @@ -10,7 +10,7 @@ }, { "pk": 177155739, - "model": "stats.sharecounttotal", + "model": "stats.addonsharecounttotal", "fields": { "count": 29, "service": "digg", @@ -19,7 +19,7 @@ }, { "pk": 177155741, - "model": "stats.sharecounttotal", + "model": "stats.addonsharecounttotal", "fields": { "count": 4, "service": "friendfeed", @@ -28,7 +28,7 @@ }, { "pk": 177155742, - "model": "stats.sharecounttotal", + "model": "stats.addonsharecounttotal", "fields": { "count": 14, "service": "myspace", @@ -37,7 +37,7 @@ }, { "pk": 177155743, - "model": "stats.sharecounttotal", + "model": "stats.addonsharecounttotal", "fields": { "count": 4, "service": "twitter", diff --git a/apps/sharing/helpers.py b/apps/sharing/helpers.py index f809bacdda..54a419fd56 100644 --- a/apps/sharing/helpers.py +++ b/apps/sharing/helpers.py @@ -6,29 +6,33 @@ from amo.helpers import login_link from .models import ServiceBase, EMAIL -@register.inclusion_tag('sharing/addon_sharing.html') +@register.inclusion_tag('sharing/sharing_box.html') @jinja2.contextfunction -def addon_sharing(context, addon): - # prepare services +def sharing_box(context, obj, show_email=True): + request = context['request'] opts = {} + services = list(sharing.SERVICES_LIST) + if not show_email: + services.remove(EMAIL) for service in sharing.SERVICES_LIST: service_opts = {} - if service == EMAIL and not context['request'].user.is_authenticated(): + if service == EMAIL and not request.user.is_authenticated(): service_opts['url'] = login_link(context) service_opts['target'] = '_self' else: - service_opts['url'] = '/addon/share/{id}?service={name}'.format( - id=addon.id, name=service.shortname) + url = obj.share_url() + '?service=%s' % service.shortname + service_opts['url'] = url service_opts['target'] = '_blank' opts[service] = service_opts c = dict(context.items()) c.update({ - 'request': context['request'], - 'user': context['request'].user, - 'addon': addon, - 'services': sharing.SERVICES_LIST, + 'request': request, + 'user': request.user, + 'obj': obj, + 'services': services, 'service_opts': opts, 'email_service': EMAIL, + 'show_email': show_email, }) return c diff --git a/apps/sharing/models.py b/apps/sharing/models.py index 0e33a2dfcb..e95050c385 100644 --- a/apps/sharing/models.py +++ b/apps/sharing/models.py @@ -1,5 +1,5 @@ from tower import ugettext_lazy as _, ungettext as ngettext -from stats.models import ShareCountTotal +from stats.models import AddonShareCountTotal # string replacements in URLs are: url, title, description @@ -16,16 +16,16 @@ class DELICIOUS(ServiceBase): """see: http://delicious.com/help/savebuttons""" shortname = 'delicious' label = _(u'Add to Delicious') - url = ('http://delicious.com/save?url={url}&title={title}' - '¬es={description}') + url = (u'http://delicious.com/save?url={url}&title={title}' + '¬es={description}') class DIGG(ServiceBase): """see: http://digg.com/tools/integrate#3""" shortname = 'digg' label = _(u'Digg this!') - url = ('http://digg.com/submit?url={url}&title={title}&bodytext=' - '{description}&media=news&topic=tech_news') + url = (u'http://digg.com/submit?url={url}&title={title}&bodytext=' + '{description}&media=news&topic=tech_news') @staticmethod def count_term(count): @@ -36,14 +36,14 @@ class FACEBOOK(ServiceBase): """see: http://www.facebook.com/share_options.php""" shortname = 'facebook' label = _(u'Post to Facebook') - url = 'http://www.facebook.com/share.php?u={url}&t={title}' + url = u'http://www.facebook.com/share.php?u={url}&t={title}' class FRIENDFEED(ServiceBase): """see: http://friendfeed.com/embed/link""" shortname = 'friendfeed' label = _(u'Share on FriendFeed') - url = 'http://friendfeed.com/?url={url}&title={title}' + url = u'http://friendfeed.com/?url={url}&title={title}' @staticmethod def count_term(count): @@ -54,14 +54,14 @@ class MYSPACE(ServiceBase): """see: http://www.myspace.com/posttomyspace""" shortname = 'myspace' label = _(u'Post to MySpace') - url = ('http://www.myspace.com/index.cfm?fuseaction=postto&t={title}' - '&c={description}&u={url}&l=1') + url = (u'http://www.myspace.com/index.cfm?fuseaction=postto&t={title}' + '&c={description}&u={url}&l=1') class TWITTER(ServiceBase): shortname = 'twitter' label = _(u'Post to Twitter') - url = 'https://twitter.com/home?status={title}%20{title}' + url = u'https://twitter.com/home?status={title}%20{url}' @staticmethod def count_term(count): diff --git a/apps/sharing/templates/sharing/addon_sharing.html b/apps/sharing/templates/sharing/sharing_box.html similarity index 82% rename from apps/sharing/templates/sharing/addon_sharing.html rename to apps/sharing/templates/sharing/sharing_box.html index 94984c5ce4..9c29bcdcd5 100644 --- a/apps/sharing/templates/sharing/addon_sharing.html +++ b/apps/sharing/templates/sharing/sharing_box.html @@ -1,11 +1,16 @@