Remove tags from edit app screen (bug 726763)

Author: kumar303

mkt/developers/forms.py

@@ -22,6 +22,7 @@
 
 import amo
 import addons.forms
+from addons.forms import clean_name, slug_validator
 import paypal
 from addons.models import (Addon, AddonDependency, AddonUpsell, AddonUser,
                            BlacklistedSlug, Charity, Preview)
@@ -1167,3 +1168,65 @@ def clean(self):
     FormSet = modelformset_factory(AddonDependency, formset=_FormSet,
                                    form=_Form, extra=0, can_delete=True)
     return FormSet(*args, **kw)
+
+
+class AppFormBasic(addons.forms.AddonFormBase):
+    """Form to edit basic app info."""
+    name = TransField(max_length=128)
+    slug = forms.CharField(max_length=30)
+    summary = TransField(widget=TransTextarea(attrs={'rows': 4}),
+                         max_length=250)
+
+    class Meta:
+        model = Addon
+        fields = ('name', 'slug', 'summary')
+
+    def __init__(self, *args, **kw):
+        # Force the form to use app_slug if this is a webapp. We want to keep
+        # this under "slug" so all the js continues to work.
+        if kw['instance'].is_webapp():
+            kw.setdefault('initial', {})['slug'] = kw['instance'].app_slug
+
+        super(AppFormBasic, self).__init__(*args, **kw)
+        # Do not simply append validators, as validators will persist between
+        # instances.
+        validate_name = lambda x: clean_name(x, self.instance)
+        name_validators = list(self.fields['name'].validators)
+        name_validators.append(validate_name)
+        self.fields['name'].validators = name_validators
+
+    def _post_clean(self):
+        # Switch slug to app_slug in cleaned_data and self._meta.fields so
+        # we can update the app_slug field for webapps.
+        try:
+            self._meta.fields = list(self._meta.fields)
+            slug_idx = self._meta.fields.index('slug')
+            data = self.cleaned_data
+            if 'slug' in data:
+                data['app_slug'] = data.pop('slug')
+            self._meta.fields[slug_idx] = 'app_slug'
+            super(AppFormBasic, self)._post_clean()
+        finally:
+            self._meta.fields[slug_idx] = 'slug'
+
+    def clean_slug(self):
+        target = self.cleaned_data['slug']
+        slug_validator(target, lower=False)
+        slug_field = 'app_slug' if self.instance.is_webapp() else 'slug'
+
+        if target != getattr(self.instance, slug_field):
+            if Addon.objects.filter(**{slug_field: target}).exists():
+                raise forms.ValidationError(_('This slug is already in use.'))
+
+            if BlacklistedSlug.blocked(target):
+                raise forms.ValidationError(_('The slug cannot be: %s.'
+                                              % target))
+        return target
+
+    def save(self, addon, commit=False):
+        # We ignore `commit`, since we need it to be `False` so we can save
+        # the ManyToMany fields on our own.
+        addonform = super(AppFormBasic, self).save(commit=False)
+        addonform.save()
+
+        return addonform

mkt/developers/templates/developers/addons/edit/basic.html

@@ -175,44 +175,6 @@ <h3>
             </td>
           </tr>
           {% endif %}
-          <tr>
-            <th>
-              {% if webapp %}
-                {# TODO(apps): Finalize copy. #}
-                {{ tip('Tags',
-                       "Tags help users find your app and should be short
-                        descriptors such as tabs, toolbar, or twitter. You
-                        may have a maximum of {0} tags.".format(amo.MAX_TAGS)) }}
-              {% else %}
-                {{ tip(_("Tags"),
-                       _("Tags help users find your add-on and should be short
-                          descriptors such as tabs, toolbar, or twitter.  You
-                          may have a maximum of {0} tags.").format(amo.MAX_TAGS)) }}
-              {% endif %}
-            </th>
-            <td id="addon_tags_edit">
-              {% if editable %}
-                {{ form.tags }}
-                {{ form.tags.errors }}
-                <div class="edit-addon-details">
-                  {{ ngettext('Comma-separated, minimum of {0} character.',
-                              'Comma-separated, minimum of {0} characters.',
-                              amo.MIN_TAG_LENGTH)|f(amo.MIN_TAG_LENGTH) }}
-                  {{ _('Example: pop, hen, yum.  Limit 20.') }}
-                </div>
-                {% if restricted_tags %}
-                <div class="edit-addon-details">
-                    {{ ngettext('Reserved tag:', 'Reserved tags:', restricted_tags|length) }}
-                    {{ restricted_tags|join(', ') }}
-                </div>
-                {% endif %}
-              {% else %}
-                {% call empty_unless(tags) %}
-                  {{ tags|join(', ') }}
-                {% endcall %}
-              {% endif %}
-            </td>
-          </tr>
         </tbody>
       </table>
     </div>

mkt/developers/tests/test_views_edit.py

@@ -23,7 +23,6 @@
                            AddonDeviceType, AddonUser, Category, DeviceType)
 from bandwagon.models import Collection, CollectionAddon, FeaturedCollection
 from mkt.developers.models import ActivityLog
-from tags.models import Tag, AddonTag
 from users.models import UserProfile
 
 
@@ -53,10 +52,6 @@ def setUp(self):
         self.url = addon.get_dev_url()
         self.user = UserProfile.objects.get(pk=55021)
 
-        self.tags = ['tag3', 'tag2', 'tag1']
-        for t in self.tags:
-            Tag(tag_text=t).save_tag(addon)
-
         self.addon = self.get_addon()
 
     def get_addon(self):
@@ -68,8 +63,7 @@ def get_url(self, section, edit=False):
     def get_dict(self, **kw):
         fs = formset(self.cat_initial, initial_count=1)
         result = {'name': 'new name', 'slug': 'test_slug',
-                  'summary': 'new summary',
-                  'tags': ', '.join(self.tags)}
+                  'summary': 'new summary'}
         result.update(**kw)
         result.update(fs)
         return result
@@ -266,8 +260,6 @@ def test_edit(self):
         eq_(unicode(addon.slug), data['slug'])
         eq_(unicode(addon.summary), data['summary'])
 
-        eq_([unicode(t) for t in addon.tags.all()], sorted(self.tags))
-
     def test_edit_check_description(self):
         # Make sure bug 629779 doesn't return.
         old_desc = self.addon.description
@@ -330,8 +322,6 @@ def test_edit_as_developer(self):
         eq_(unicode(addon.slug), data['slug'])
         eq_(unicode(addon.summary), data['summary'])
 
-        eq_([unicode(t) for t in addon.tags.all()], sorted(self.tags))
-
     def test_edit_name_required(self):
         data = self.get_dict(name='', slug='test_addon')
         r = self.client.post(self.basic_edit_url, data)
@@ -351,104 +341,6 @@ def test_edit_slugs_unique(self):
         eq_(r.status_code, 200)
         self.assertFormError(r, 'form', 'slug', 'This slug is already in use.')
 
-    def test_edit_add_tag(self):
-        count = ActivityLog.objects.all().count()
-        self.tags.insert(0, 'tag4')
-        data = self.get_dict()
-        r = self.client.post(self.basic_edit_url, data)
-        eq_(r.status_code, 200)
-
-        result = pq(r.content)('#addon_tags_edit').eq(0).text()
-
-        eq_(result, ', '.join(sorted(self.tags)))
-        eq_((ActivityLog.objects.for_addons(self.addon)
-             .get(action=amo.LOG.ADD_TAG.id)).to_string(),
-            '<a href="/en-US/firefox/tag/tag4">tag4</a> added to '
-            '<a href="/en-US/firefox/addon/test_slug/">new name</a>.')
-        eq_(ActivityLog.objects.filter(action=amo.LOG.ADD_TAG.id).count(),
-                                        count + 1)
-
-    def test_edit_blacklisted_tag(self):
-        Tag.objects.get_or_create(tag_text='blue', blacklisted=True)
-        data = self.get_dict(tags='blue')
-        r = self.client.post(self.basic_edit_url, data)
-        eq_(r.status_code, 200)
-
-        error = 'Invalid tag: blue'
-        self.assertFormError(r, 'form', 'tags', error)
-
-    def test_edit_blacklisted_tags_2(self):
-        Tag.objects.get_or_create(tag_text='blue', blacklisted=True)
-        Tag.objects.get_or_create(tag_text='darn', blacklisted=True)
-        data = self.get_dict(tags='blue, darn, swearword')
-        r = self.client.post(self.basic_edit_url, data)
-        eq_(r.status_code, 200)
-
-        error = 'Invalid tags: blue, darn'
-        self.assertFormError(r, 'form', 'tags', error)
-
-    def test_edit_blacklisted_tags_3(self):
-        Tag.objects.get_or_create(tag_text='blue', blacklisted=True)
-        Tag.objects.get_or_create(tag_text='darn', blacklisted=True)
-        Tag.objects.get_or_create(tag_text='swearword', blacklisted=True)
-        data = self.get_dict(tags='blue, darn, swearword')
-        r = self.client.post(self.basic_edit_url, data)
-        eq_(r.status_code, 200)
-
-        error = 'Invalid tags: blue, darn, swearword'
-        self.assertFormError(r, 'form', 'tags', error)
-
-    def test_edit_remove_tag(self):
-        self.tags.remove('tag2')
-
-        count = ActivityLog.objects.all().count()
-        data = self.get_dict()
-        r = self.client.post(self.basic_edit_url, data)
-        eq_(r.status_code, 200)
-
-        result = pq(r.content)('#addon_tags_edit').eq(0).text()
-
-        eq_(result, ', '.join(sorted(self.tags)))
-
-        eq_(ActivityLog.objects.filter(action=amo.LOG.REMOVE_TAG.id).count(),
-            count + 1)
-
-    def test_edit_minlength_tags(self):
-        tags = self.tags
-        tags.append('a' * (amo.MIN_TAG_LENGTH - 1))
-        data = self.get_dict()
-        r = self.client.post(self.basic_edit_url, data)
-        eq_(r.status_code, 200)
-
-        self.assertFormError(r, 'form', 'tags',
-                             'All tags must be at least %d characters.' %
-                             amo.MIN_TAG_LENGTH)
-
-    def test_edit_max_tags(self):
-        tags = self.tags
-
-        for i in range(amo.MAX_TAGS + 1):
-            tags.append('test%d' % i)
-
-        data = self.get_dict()
-        r = self.client.post(self.basic_edit_url, data)
-        self.assertFormError(r, 'form', 'tags', 'You have %d too many tags.' %
-                                                 (len(tags) - amo.MAX_TAGS))
-
-    def test_edit_tag_empty_after_slug(self):
-        start = Tag.objects.all().count()
-        data = self.get_dict(tags='>>')
-        self.client.post(self.basic_edit_url, data)
-
-        # Check that the tag did not get created.
-        eq_(start, Tag.objects.all().count())
-
-    def test_edit_tag_slugified(self):
-        data = self.get_dict(tags='<script>alert("foo")</script>')
-        self.client.post(self.basic_edit_url, data)
-        tag = Tag.objects.all().order_by('-pk')[0]
-        eq_(tag.tag_text, 'scriptalertfooscript')
-
     def test_edit_categories_add(self):
         eq_([c.id for c in self.get_addon().all_categories], [22])
         self.cat_initial['categories'] = [22, 23]
@@ -598,16 +490,6 @@ def test_edit_summary_max_length(self):
                              'Ensure this value has at most 250 '
                              'characters (it has 251).')
 
-    def test_edit_restricted_tags(self):
-        addon = self.get_addon()
-        tag = Tag.objects.create(tag_text='restartless', restricted=True)
-        AddonTag.objects.create(tag=tag, addon=addon)
-
-        res = self.client.get(self.basic_edit_url)
-        divs = pq(res.content)('#addon_tags_edit .edit-addon-details')
-        eq_(len(divs), 2)
-        assert 'restartless' in divs.eq(1).text()
-
     def test_text_not_none_when_has_flags(self):
         r = self.client.get(self.url)
         doc = pq(r.content)

mkt/developers/views.py

@@ -39,7 +39,8 @@
 from addons.models import Addon, AddonUser
 from addons.views import BaseFilter
 from mkt.developers.decorators import dev_required
-from mkt.developers.forms import CheckCompatibilityForm, InappConfigForm
+from mkt.developers.forms import (CheckCompatibilityForm, InappConfigForm,
+                                  AppFormBasic)
 from mkt.developers.models import ActivityLog, SubmitStep
 from mkt.developers import perf
 from editors.helpers import get_position
@@ -889,7 +890,7 @@ def ajax_dependencies(request, addon_id, addon):
 @dev_required(webapp=True)
 def addons_section(request, addon_id, addon, section, editable=False,
                    webapp=False):
-    basic = addon_forms.AppFormBasic if webapp else addon_forms.AddonFormBasic
+    basic = AppFormBasic if webapp else addon_forms.AddonFormBasic
     models = {'basic': basic,
               'media': addon_forms.AddonFormMedia,
               'details': addon_forms.AddonFormDetails,