diff --git a/apps/devhub/models.py b/apps/devhub/models.py index 8448edf46b..b60f3a65d8 100644 --- a/apps/devhub/models.py +++ b/apps/devhub/models.py @@ -220,8 +220,11 @@ class ActivityLog(amo.models.ModelBase): arg.get_url_path(), arg.name) arguments.remove(arg) if isinstance(arg, Tag) and not tag: - tag = self.f(u'{1}', - arg.get_url_path(), arg.tag_text) + if arg.can_reverse(): + tag = self.f(u'{1}', + arg.get_url_path(), arg.tag_text) + else: + tag = self.f('{0}', arg.tag_text) try: kw = dict(addon=addon, review=review, version=version, collection=collection, tag=tag) diff --git a/apps/devhub/tests/test_models.py b/apps/devhub/tests/test_models.py index 88e07e5ca0..4ec77c4e51 100644 --- a/apps/devhub/tests/test_models.py +++ b/apps/devhub/tests/test_models.py @@ -2,11 +2,13 @@ import jingo import test_utils from nose.tools import eq_ from mock import Mock +from pyquery import PyQuery as pq import amo from addons.models import Addon, AddonUser from bandwagon.models import Collection from devhub.models import ActivityLog +from tags.models import Tag from files.models import File from reviews.models import Review from users.models import UserProfile @@ -125,6 +127,15 @@ class TestActivityLog(test_utils.TestCase): '

<script src="x.js"> role changed to Owner for Delicious Bookmarks.

') + def test_tag_no_match(self): + addon = Addon.objects.get() + tag = Tag.objects.create(tag_text='http://foo.com') + amo.log(amo.LOG.ADD_TAG, addon, tag) + log = ActivityLog.objects.get() + text = jingo.env.from_string('

{{ log }}

').render(log=log) + # There should only be one a, the link to the addon, but no tag link. + eq_(len(pq(text)('a')), 1) + class TestVersion(test_utils.TestCase): fixtures = ['base/apps', 'base/users', 'base/addon_3615', diff --git a/apps/tags/models.py b/apps/tags/models.py index 520b3eee5c..57140af74b 100644 --- a/apps/tags/models.py +++ b/apps/tags/models.py @@ -1,4 +1,5 @@ from django.db import models +from django.core.urlresolvers import NoReverseMatch import amo.models from amo.urlresolvers import reverse @@ -30,6 +31,13 @@ class Tag(amo.models.ModelBase): def popularity(self): return self.tagstat.num_addons + def can_reverse(self): + try: + self.get_url_path() + return True + except NoReverseMatch: + return False + def get_url_path(self): return reverse('tags.detail', args=[self.tag_text]) diff --git a/apps/tags/templates/tags/tag_link.html b/apps/tags/templates/tags/tag_link.html index c049ef2bdb..241cac044b 100644 --- a/apps/tags/templates/tags/tag_link.html +++ b/apps/tags/templates/tags/tag_link.html @@ -1 +1,5 @@ -{{ tag.tag_text }} +{% if tag.can_reverse() %} + {{ tag.tag_text }} +{% else %} + {{ tag.tag_text }} +{% endif %} diff --git a/apps/tags/templates/tags/tag_list.html b/apps/tags/templates/tags/tag_list.html index 77a3f5cb35..57dc743c5f 100644 --- a/apps/tags/templates/tags/tag_list.html +++ b/apps/tags/templates/tags/tag_list.html @@ -1,8 +1,12 @@ {% macro tag_li(tag, addon=None, css_class='') -%} {%- endmacro %} diff --git a/apps/tags/tests/test_helpers.py b/apps/tags/tests/test_helpers.py index 4e97d98f51..5ffab8cb1c 100644 --- a/apps/tags/tests/test_helpers.py +++ b/apps/tags/tests/test_helpers.py @@ -11,7 +11,6 @@ from addons.models import Addon from tags.models import AddonTag, Tag, TagStat from tags.helpers import tag_link -from django.core.urlresolvers import NoReverseMatch xss = "" @@ -34,8 +33,7 @@ class TestHelpers(test.TestCase): 'LANG': 'en-us', 'request': request, 'addon': addon, - 'tags': tags - } + 'tags': tags} # initialize jingo jingo.load_helpers() @@ -59,7 +57,8 @@ class TestHelpers(test.TestCase): tag.save() TagStat.objects.create(tag=tag, num_addons=1) - self.assertRaises(NoReverseMatch, tag_link, tag, 1, 1) + doc = pq(tag_link(tag, 1, 1)) + assert not doc('a') def create_tags(addon, author, number): diff --git a/apps/tags/tests/test_views.py b/apps/tags/tests/test_views.py index 725d972f90..8d9f391caf 100644 --- a/apps/tags/tests/test_views.py +++ b/apps/tags/tests/test_views.py @@ -87,6 +87,9 @@ class TestXSSURLFail(test_utils.TestCase): self.assertRaises(NoReverseMatch, reverse, 'tags.top_cloud', args=[self.xss]) + def test_no_reverse(self): + assert not self.tag.can_reverse() + class TestNoTags(test_utils.TestCase): fixtures = ['base/addon_3615']