Add admin tool to enter custom heading/description for disco pane recommendations

src/olympia/addons/admin.py

@@ -26,12 +26,13 @@ class AddonAdmin(admin.ModelAdmin):
     list_display = ('__unicode__', 'type', 'guid',
                     'status_with_admin_manage_link', 'average_rating')
     list_filter = ('type', 'status')
+    search_fields = ('id', '^guid', '^slug')
 
-    readonly_fields = ('status_with_admin_manage_link',)
+    readonly_fields = ('id', 'status_with_admin_manage_link',)
 
     fieldsets = (
         (None, {
-            'fields': ('name', 'slug', 'guid', 'default_locale', 'type',
+            'fields': ('id', 'name', 'slug', 'guid', 'default_locale', 'type',
                        'status_with_admin_manage_link'),
         }),
         ('Details', {

src/olympia/constants/permissions.py

@@ -73,6 +73,9 @@ REVIEWS_ADMIN = AclPermission('Reviews', 'Admin')
 # Can access advanced admin features, like deletion.
 ADMIN_ADVANCED = AclPermission('Admin', 'Advanced')
 
+# Can add/edit/delete DiscoveryItems.
+DISCOVERY_EDIT = AclPermission('Discovery', 'Edit')
+
 # All permissions, for easy introspection
 PERMISSIONS_LIST = [
     x for x in vars().values() if isinstance(x, AclPermission)]
@@ -81,11 +84,13 @@ PERMISSIONS_LIST = [
 # require superuser admins (which also have all other permissions anyway) to do
 # something, and then add some custom ones.
 DJANGO_PERMISSIONS_MAPPING = defaultdict(lambda: SUPERPOWERS)
-# Curators can do anything to ReplacementAddon. In addition, the modeladmin
-# will also check for addons:edit and give them read-only access to the
-# changelist (obj=None passed to the has_change_permission() method)
+
 DJANGO_PERMISSIONS_MAPPING.update({
     'addons.change_addon': ADDONS_EDIT,
+    # Users with Admin:Curation can do anything to ReplacementAddon.
+    # In addition, the modeladmin will also check for Addons:Edit and give them
+    # read-only access to the changelist (obj=None passed to the
+    # has_change_permission() method)
     'addons.change_replacementaddon': ADMIN_CURATION,
     'addons.add_replacementaddon': ADMIN_CURATION,
     'addons.delete_replacementaddon': ADMIN_CURATION,
@@ -93,6 +98,10 @@ DJANGO_PERMISSIONS_MAPPING.update({
     'bandwagon.change_collection': COLLECTIONS_EDIT,
     'bandwagon.delete_collection': COLLECTIONS_EDIT,
 
+    'discovery.add_discoveryitem': DISCOVERY_EDIT,
+    'discovery.change_discoveryitem': DISCOVERY_EDIT,
+    'discovery.delete_discoveryitem': DISCOVERY_EDIT,
+
     'users.change_userprofile': USERS_EDIT,
     'users.delete_userprofile': ADMIN_ADVANCED,
 

src/olympia/discovery/admin.py

@@ -0,0 +1,42 @@
+from django import forms
+from django.contrib import admin
+from django.contrib.admin.widgets import ForeignKeyRawIdWidget
+
+from olympia.addons.models import Addon
+from olympia.discovery.models import DiscoveryItem
+
+
+class SlugOrPkChoiceField(forms.ModelChoiceField):
+    """A ModelChoiceField that supports entering slugs instead of PKs for
+    convenience."""
+    def clean(self, value):
+        if value and isinstance(value, basestring) and not value.isdigit():
+            try:
+                value = self.queryset.values_list(
+                    'pk', flat=True).get(slug=value)
+            except self.queryset.model.DoesNotExist:
+                value = value
+        return super(SlugOrPkChoiceField, self).clean(value)
+
+
+class DiscoveryItemAdmin(admin.ModelAdmin):
+    class Media:
+        css = {
+            'all': ('css/admin/larger_raw_id.css',)
+        }
+    raw_id_fields = ('addon',)
+    list_display = ('__unicode__', 'custom_addon_name', 'custom_heading',)
+    view_on_site = False
+
+    def formfield_for_foreignkey(self, db_field, request=None, **kwargs):
+        if db_field.name == 'addon':
+            kwargs['widget'] = ForeignKeyRawIdWidget(
+                db_field.rel, self.admin_site, using=kwargs.get('using'))
+            kwargs['queryset'] = Addon.objects.public()
+            kwargs['help_text'] = db_field.help_text
+            return SlugOrPkChoiceField(**kwargs)
+        return super(DiscoveryItemAdmin, self).formfield_for_foreignkey(
+            db_field, request, **kwargs)
+
+
+admin.site.register(DiscoveryItem, DiscoveryItemAdmin)

src/olympia/discovery/models.py

@@ -0,0 +1,31 @@
+from django.db import models
+
+from olympia.addons.models import Addon
+from olympia.amo.models import ModelBase
+
+
+class DiscoveryItem(ModelBase):
+    addon = models.OneToOneField(
+        Addon, on_delete=models.CASCADE,
+        help_text='Add-on id this item will point to (If you do not know the '
+                  'id, paste the slug instead and it will be transformed '
+                  'automatically for you. If you have access to the add-on '
+                  'admin page, you can use the magnifying glass to see '
+                  'all available add-ons.')
+    custom_addon_name = models.CharField(
+        max_length=255, blank=True,
+        help_text='Custom add-on name, if needed for space constraints. '
+                  'Will be used in the heading if present, but will *not* be '
+                  'translated.')
+    custom_heading = models.CharField(
+        max_length=255, blank=True,
+        help_text='Short text used in the header. Can contain the following '
+                  'special tags: {start_sub_heading}, {addon_name}, '
+                  '{end_sub_heading}. Will be translated.')
+    custom_description = models.TextField(
+        blank=True, help_text='Longer text used to describe an add-on. Should '
+                              'not contain any HTML or special tags. Will be '
+                              'translated.')
+
+    def __unicode__(self):
+        return unicode(self.addon)

src/olympia/discovery/tests/test_admin.py

@@ -0,0 +1,255 @@
+# -*- coding: utf-8 -*-
+from olympia import amo
+from olympia.discovery.models import DiscoveryItem
+from olympia.amo.tests import TestCase, addon_factory, user_factory
+from olympia.amo.urlresolvers import django_reverse, reverse
+
+
+class TestDiscoveryAdmin(TestCase):
+    def setUp(self):
+        self.list_url = reverse('admin:discovery_discoveryitem_changelist')
+
+    def test_can_see_discovery_module_in_admin_with_discovery_edit(self):
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.grant_permission(user, 'Discovery:Edit')
+        self.client.login(email=user.email)
+        url = reverse('admin:index')
+        response = self.client.get(url)
+        assert response.status_code == 200
+
+        # Use django's reverse, since that's what the admin will use. Using our
+        # own would fail the assertion because of the locale that gets added.
+        self.list_url = django_reverse(
+            'admin:discovery_discoveryitem_changelist')
+        assert self.list_url in response.content.decode('utf-8')
+
+    def test_can_list_with_discovery_edit_permission(self):
+        DiscoveryItem.objects.create(addon=addon_factory(name=u'FooBâr'))
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.grant_permission(user, 'Discovery:Edit')
+        self.client.login(email=user.email)
+        response = self.client.get(self.list_url, follow=True)
+        assert response.status_code == 200
+        assert u'FooBâr' in response.content.decode('utf-8')
+
+    def test_can_edit_with_discovery_edit_permission(self):
+        addon = addon_factory(name=u'BarFöo')
+        item = DiscoveryItem.objects.create(addon=addon)
+        self.detail_url = reverse(
+            'admin:discovery_discoveryitem_change', args=(item.pk,)
+        )
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.grant_permission(user, 'Discovery:Edit')
+        self.client.login(email=user.email)
+        response = self.client.get(self.detail_url, follow=True)
+        assert response.status_code == 200
+        content = response.content.decode('utf-8')
+        assert u'BarFöo' in content
+        assert DiscoveryItem._meta.get_field('addon').help_text in content
+
+        response = self.client.post(
+            self.detail_url, {
+                'addon': unicode(addon.pk),
+                'custom_addon_name': u'Xäxâxàxaxaxa !',
+                'custom_heading': u'This heading is totally custom.',
+                'custom_description': u'This description is as well!',
+            }, follow=True)
+        assert response.status_code == 200
+        item.reload()
+        assert DiscoveryItem.objects.count() == 1
+        assert item.addon == addon
+        assert item.custom_addon_name == u'Xäxâxàxaxaxa !'
+        assert item.custom_heading == u'This heading is totally custom.'
+        assert item.custom_description == u'This description is as well!'
+
+    def test_can_change_addon_with_discovery_edit_permission(self):
+        addon = addon_factory(name=u'BarFöo')
+        addon2 = addon_factory(name=u'Another ône', slug='another-addon')
+        item = DiscoveryItem.objects.create(addon=addon)
+        self.detail_url = reverse(
+            'admin:discovery_discoveryitem_change', args=(item.pk,)
+        )
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.grant_permission(user, 'Discovery:Edit')
+        self.client.login(email=user.email)
+        response = self.client.get(self.detail_url, follow=True)
+        assert response.status_code == 200
+        assert u'BarFöo' in response.content.decode('utf-8')
+
+        # Change add-on using the slug.
+        response = self.client.post(
+            self.detail_url, {'addon': unicode(addon2.slug)}, follow=True)
+        assert response.status_code == 200
+        item.reload()
+        assert DiscoveryItem.objects.count() == 1
+        # assert item.addon == addon2
+
+        # Change add-on using the id.
+        response = self.client.post(
+            self.detail_url, {'addon': unicode(addon.pk)}, follow=True)
+        assert response.status_code == 200
+        item.reload()
+        assert DiscoveryItem.objects.count() == 1
+        assert item.addon == addon
+
+    def test_change_addon_errors(self):
+        addon = addon_factory(name=u'BarFöo')
+        addon2 = addon_factory(name=u'Another ône', slug='another-addon')
+        item = DiscoveryItem.objects.create(addon=addon)
+        self.detail_url = reverse(
+            'admin:discovery_discoveryitem_change', args=(item.pk,)
+        )
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.grant_permission(user, 'Discovery:Edit')
+        self.client.login(email=user.email)
+
+        # Try changing using an unknown slug.
+        response = self.client.post(
+            self.detail_url, {'addon': u'gârbage'}, follow=True)
+        assert response.status_code == 200
+        assert not response.context_data['adminform'].form.is_valid()
+        assert 'addon' in response.context_data['adminform'].form.errors
+        item.reload()
+        assert item.addon == addon
+
+        # Try changing using an unknown id.
+        response = self.client.post(
+            self.detail_url, {'addon': unicode(addon2.pk + 666)}, follow=True)
+        assert response.status_code == 200
+        assert not response.context_data['adminform'].form.is_valid()
+        assert 'addon' in response.context_data['adminform'].form.errors
+        item.reload()
+        assert item.addon == addon
+
+        # Try changing using a non-public add-on id.
+        addon3 = addon_factory(status=amo.STATUS_DISABLED)
+        response = self.client.post(
+            self.detail_url, {'addon': unicode(addon3.pk)}, follow=True)
+        assert response.status_code == 200
+        assert not response.context_data['adminform'].form.is_valid()
+        assert 'addon' in response.context_data['adminform'].form.errors
+        item.reload()
+        assert item.addon == addon
+
+        # Try changing to an add-on that is already used by another item.
+        item2 = DiscoveryItem.objects.create(addon=addon2)
+        response = self.client.post(
+            self.detail_url, {'addon': unicode(addon2.pk)}, follow=True)
+        assert response.status_code == 200
+        assert not response.context_data['adminform'].form.is_valid()
+        assert 'addon' in response.context_data['adminform'].form.errors
+        item.reload()
+        item2.reload()
+        assert item.addon == addon
+        assert item2.addon == addon2
+
+    def test_can_delete_with_discovery_edit_permission(self):
+        item = DiscoveryItem.objects.create(addon=addon_factory())
+        self.delete_url = reverse(
+            'admin:discovery_discoveryitem_delete', args=(item.pk,)
+        )
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.grant_permission(user, 'Discovery:Edit')
+        self.client.login(email=user.email)
+        # Can access delete confirmation page.
+        response = self.client.get(self.delete_url, follow=True)
+        assert response.status_code == 200
+        assert DiscoveryItem.objects.filter(pk=item.pk).exists()
+
+        # Can actually delete.
+        response = self.client.post(
+            self.delete_url, data={'post': 'yes'}, follow=True)
+        assert response.status_code == 200
+        assert not DiscoveryItem.objects.filter(pk=item.pk).exists()
+
+    def test_can_add_with_discovery_edit_permission(self):
+        addon = addon_factory(name=u'BarFöo')
+        self.add_url = reverse('admin:discovery_discoveryitem_add')
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.grant_permission(user, 'Discovery:Edit')
+        self.client.login(email=user.email)
+        response = self.client.get(self.add_url, follow=True)
+        assert response.status_code == 200
+        assert DiscoveryItem.objects.count() == 0
+        response = self.client.post(
+            self.add_url, {
+                'addon': unicode(addon.pk),
+                'custom_addon_name': u'Xäxâxàxaxaxa !',
+                'custom_heading': u'This heading is totally custom.',
+                'custom_description': u'This description is as well!',
+            }, follow=True)
+        assert response.status_code == 200
+        assert DiscoveryItem.objects.count() == 1
+        item = DiscoveryItem.objects.get()
+        assert item.addon == addon
+        assert item.custom_addon_name == u'Xäxâxàxaxaxa !'
+        assert item.custom_heading == u'This heading is totally custom.'
+        assert item.custom_description == u'This description is as well!'
+
+    def test_can_not_add_without_discovery_edit_permission(self):
+        addon = addon_factory(name=u'BarFöo')
+        self.add_url = reverse('admin:discovery_discoveryitem_add')
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.client.login(email=user.email)
+        response = self.client.get(self.add_url, follow=True)
+        assert response.status_code == 403
+        response = self.client.post(
+            self.add_url, {
+                'addon': unicode(addon.pk),
+            }, follow=True)
+        assert response.status_code == 403
+        assert DiscoveryItem.objects.count() == 0
+
+    def test_can_not_edit_without_discovery_edit_permission(self):
+        addon = addon_factory(name=u'BarFöo')
+        item = DiscoveryItem.objects.create(addon=addon)
+        self.detail_url = reverse(
+            'admin:discovery_discoveryitem_change', args=(item.pk,)
+        )
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.client.login(email=user.email)
+        response = self.client.get(self.detail_url, follow=True)
+        assert response.status_code == 403
+
+        response = self.client.post(
+            self.detail_url, {
+                'addon': unicode(addon.pk),
+                'custom_addon_name': u'Noooooô !',
+                'custom_heading': u'I should not be able to do this.',
+                'custom_description': u'This is wrong.',
+            }, follow=True)
+        assert response.status_code == 403
+        item.reload()
+        assert DiscoveryItem.objects.count() == 1
+        assert item.addon == addon
+        assert item.custom_addon_name == u''
+        assert item.custom_heading == u''
+        assert item.custom_description == u''
+
+    def test_can_not_delete_without_discovery_edit_permission(self):
+        item = DiscoveryItem.objects.create(addon=addon_factory())
+        self.delete_url = reverse(
+            'admin:discovery_discoveryitem_delete', args=(item.pk,)
+        )
+        user = user_factory()
+        self.grant_permission(user, 'Admin:Tools')
+        self.client.login(email=user.email)
+        # Can not access delete confirmation page.
+        response = self.client.get(self.delete_url, follow=True)
+        assert response.status_code == 403
+        assert DiscoveryItem.objects.filter(pk=item.pk).exists()
+
+        # Can not actually delete either.
+        response = self.client.post(
+            self.delete_url, data={'post': 'yes'}, follow=True)
+        assert response.status_code == 403
+        assert DiscoveryItem.objects.filter(pk=item.pk).exists()

src/olympia/migrations/1032-add-discovery-items.sql

@@ -0,0 +1,16 @@
+-- Note: if the migration fails for you locally, remove the 'unsigned' next to addon_id below.
+CREATE TABLE `discovery_discoveryitem` (
+    `id` integer AUTO_INCREMENT NOT NULL PRIMARY KEY,
+    `created` datetime(6) NOT NULL,
+    `modified` datetime(6) NOT NULL,
+    `addon_id` integer UNSIGNED NOT NULL UNIQUE,
+    `custom_addon_name` varchar(255) NOT NULL,
+    `custom_heading` varchar(255) NOT NULL,
+    `custom_description` longtext NOT NULL
+)
+;
+ALTER TABLE `discovery_discoveryitem` ADD CONSTRAINT `addon_id_refs_id_93b5ecf8` FOREIGN KEY (`addon_id`) REFERENCES `addons` (`id`);
+
+-- Create group allowing users to edit discovery items in the admin.
+INSERT INTO `groups` (name, rules, notes, created, modified)
+    VALUES ('Discovery Recommendations Editors', 'Admin:Tools,Discovery:Edit', '', NOW(), NOW());

src/olympia/zadmin/tests/test_views.py

@@ -76,7 +76,7 @@ class TestHomeAndIndex(TestCase):
         assert response.status_code == 200
         doc = pq(response.content)
         modules = [x.text for x in doc('a.section')]
-        assert len(modules) == 16  # Increment as we add new admin modules.
+        assert len(modules) == 17  # Increment as we add new admin modules.
 
         # Redirected because no permissions if not logged in.
         self.client.logout()

static/css/admin/larger_raw_id.css

@@ -0,0 +1,3 @@
+.vForeignKeyRawIdAdminField {
+    width: 13em;
+}