Removed name field from app edit form (bug 824734)

Author: Rob Hudson

mkt/developers/forms.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
-from datetime import datetime
 import json
 import os
+from datetime import datetime
 
 from django import forms
 from django.conf import settings
@@ -18,7 +18,7 @@
 import amo
 import addons.forms
 from access import acl
-from addons.forms import clean_name, icons, IconWidgetRenderer, slug_validator
+from addons.forms import icons, IconWidgetRenderer, slug_validator
 from addons.models import (Addon, AddonCategory, AddonUser, BlacklistedSlug,
                            Category, Preview)
 from addons.widgets import CategoriesSelectMultiple
@@ -29,12 +29,12 @@
 from translations.fields import TransField
 from translations.forms import TranslationFormMixin
 from translations.models import Translation
-from translations.widgets import TransInput, TransTextarea
+from translations.widgets import TransTextarea
 
 import mkt
 from mkt.constants import APP_IMAGE_SIZES, MAX_PACKAGED_APP_SIZE
-from mkt.constants.ratingsbodies import (RATINGS_BY_NAME, ALL_RATINGS,
-                                         RATINGS_BODIES)
+from mkt.constants.ratingsbodies import (ALL_RATINGS, RATINGS_BODIES,
+                                         RATINGS_BY_NAME)
 from mkt.site.forms import AddonChoiceField
 from mkt.webapps.models import (AddonExcludedRegion, ContentRating, ImageAsset,
                                 Webapp)
@@ -462,15 +462,14 @@ def clean_upload(self):
 
 class AppFormBasic(addons.forms.AddonFormBase):
     """Form to edit basic app info."""
-    name = TransField(max_length=128, widget=TransInput)
     slug = forms.CharField(max_length=30, widget=forms.TextInput)
     manifest_url = forms.URLField(verify_exists=False)
     summary = TransField(widget=TransTextarea(attrs={'rows': 4}),
                          max_length=250)
 
     class Meta:
         model = Addon
-        fields = ('name', 'slug', 'manifest_url', 'summary')
+        fields = ('slug', 'manifest_url', 'summary')
 
     def __init__(self, *args, **kw):
         # Force the form to use app_slug if this is a webapp. We want to keep
@@ -479,12 +478,6 @@ def __init__(self, *args, **kw):
             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
@@ -552,12 +545,12 @@ class Meta:
 
     def clean(self):
         # Make sure we have the required translations in the new locale.
-        required = 'name', 'summary', 'description'
+        required = ['name', 'summary', 'description']
         data = self.cleaned_data
         if not self.errors and 'default_locale' in self.changed_data:
             fields = dict((k, getattr(self.instance, k + '_id'))
                           for k in required)
-            locale = self.cleaned_data['default_locale']
+            locale = data['default_locale']
             ids = filter(None, fields.values())
             qs = (Translation.objects.filter(locale=locale, id__in=ids,
                                              localized_string__isnull=False)

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

@@ -17,15 +17,15 @@ <h2>
       <tbody>
         <tr>
           <th>
-            <label data-for="name">{{ _('Name') }}</label> {{ req_if_edit }}
+            <label data-for="name">
+              {{ _('Name') }}
+              {{ tip(None,
+                     _('This field can only be edited by updating it in the '
+                       'manifest.')) }}
+            </label>
           </th>
           <td>
-            {% if editable %}
-              {{ form.name }}
-              {{ form.name.errors }}
-            {% else %}
-              {{ addon|all_locales('name') }}
-            {% endif %}
+            {{ addon|all_locales('name') }}
           </td>
         </tr>
         <tr>

mkt/developers/tests/test_views_edit.py

@@ -11,7 +11,6 @@
 from nose.tools import eq_
 from PIL import Image
 from pyquery import PyQuery as pq
-from tower import strip_whitespace
 from waffle.models import Switch
 
 import amo
@@ -32,6 +31,7 @@
 from mkt.constants import APP_IMAGE_SIZES
 from mkt.constants.ratingsbodies import RATINGS_BODIES
 from mkt.developers.models import ActivityLog
+from mkt.site.fixtures import fixture
 from mkt.webapps.models import (AddonExcludedRegion as AER, ContentRating,
                                 ImageAsset)
 
@@ -63,7 +63,8 @@ def get_section_url(addon, section, edit=False):
 
 
 class TestEdit(amo.tests.TestCase):
-    fixtures = ['base/apps', 'base/users', 'webapps/337141-steamcube']
+    fixtures = fixture('group_admin', 'user_999', 'user_admin',
+                       'user_admin_group', 'webapp_337141')
 
     def setUp(self):
         self.webapp = self.get_webapp()
@@ -116,7 +117,7 @@ def check_form_url(self, section):
 
 
 class TestEditListingWebapp(TestEdit):
-    fixtures = TestEdit.fixtures
+    fixtures = fixture('webapp_337141')
 
     @mock.patch.object(settings, 'APP_PREVIEW', False)
     def test_apps_context(self):
@@ -138,7 +139,6 @@ class TestEditBasic(TestEdit):
 
     def setUp(self):
         super(TestEditBasic, self).setUp()
-        Switch.objects.create(name='marketplace', active=True)
         self.cat = Category.objects.create(name='Games', type=amo.ADDON_WEBAPP)
         self.dtype = DEVICE_TYPES.keys()[0]
         AddonCategory.objects.create(addon=self.webapp, category=self.cat)
@@ -151,8 +151,8 @@ def get_webapp(self):
         return Addon.objects.get(id=337141)
 
     def get_dict(self, **kw):
-        result = {'device_types': self.dtype, 'name': 'new name',
-                  'slug': 'test_slug', 'summary': 'new summary',
+        result = {'device_types': self.dtype, 'slug': 'test_slug',
+                  'summary': 'new summary',
                   'manifest_url': self.get_webapp().manifest_url,
                   'categories': [self.cat.id]}
         result.update(**kw)
@@ -170,20 +170,6 @@ def test_appslug_visible(self):
         eq_(pq(r.content)('#slug_edit').remove('a, em').text(),
             u'/\u2026/%s' % self.webapp.app_slug)
 
-    def test_edit_name_required(self):
-        r = self.client.post(self.edit_url, self.get_dict(name=''))
-        self.assertFormError(r, 'form', 'name', 'This field is required.')
-
-    def test_edit_name_required_for_all_whitespace(self):
-        r = self.client.post(self.edit_url, self.get_dict(name='  \t  \n  '))
-        self.assertFormError(r, 'form', 'name', 'This field is required.')
-
-    def test_edit_name_max_length(self):
-        r = self.client.post(self.edit_url, self.get_dict(name='x' * 129))
-        eq_(list(r.context['form'].errors['name']),
-            [('en-us',
-              'Ensure this value has at most 128 characters (it has 129).')])
-
     def test_edit_slug_success(self):
         data = self.get_dict()
         r = self.client.post(self.edit_url, data)
@@ -233,8 +219,7 @@ def test_view_manifest_url_default(self, mock_urlopen):
 
     def test_view_edit_manifest_url_empty(self):
         # Empty manifest should throw an error.
-        url = ''
-        r = self.client.post(self.edit_url, self.get_dict(manifest_url=url))
+        r = self.client.post(self.edit_url, self.get_dict(manifest_url=''))
         form = r.context['form']
         assert 'manifest_url' in form.errors
         assert 'This field is required' in form.errors['manifest_url'][0]
@@ -362,31 +347,13 @@ def test_edit_summary_max_length(self):
             [('en-us',
               'Ensure this value has at most 250 characters (it has 251).')])
 
-    def test_edit(self):
-        old_name = self.webapp.name
-        data = self.get_dict()
-
-        r = self.client.post(self.edit_url, data)
-        eq_(r.status_code, 200)
-        webapp = self.get_webapp()
-
-        eq_(unicode(webapp.name), data['name'])
-        eq_(webapp.name.id, old_name.id)
-        eq_(unicode(webapp.app_slug), data['slug'])
-        eq_(unicode(webapp.summary), data['summary'])
-
     def test_edit_check_description(self):
         # Make sure bug 629779 doesn't return.
         r = self.client.post(self.edit_url, self.get_dict())
         eq_(r.status_code, 200)
 
         eq_(self.get_webapp().description, self.webapp.description)
 
-    def test_edit_slug_invalid(self):
-        r = self.client.post(self.edit_url,
-                             self.get_dict(name='', slug='invalid'))
-        eq_(pq(r.content)('form').attr('action'), self.edit_url)
-
     def test_edit_slug_valid(self):
         old_edit = self.edit_url
         data = self.get_dict(slug='valid')
@@ -423,7 +390,6 @@ def test_edit_as_developer(self):
         eq_(r.status_code, 200)
         webapp = self.get_webapp()
 
-        eq_(unicode(webapp.name), data['name'])
         eq_(unicode(webapp.app_slug), data['slug'])
         eq_(unicode(webapp.summary), data['summary'])
 
@@ -473,6 +439,7 @@ def test_refresh_dev_only(self, fetch):
 
 
 class TestEditMedia(TestEdit):
+    fixtures = fixture('webapp_337141')
 
     def setUp(self):
         super(TestEditMedia, self).setUp()
@@ -501,7 +468,7 @@ def formset_media(self, prev_blank=None, *args, **kw):
         kw.setdefault('initial_count', 0)
         kw.setdefault('prefix', 'files')
 
-        # Preview formset
+        # Preview formset.
         fs = formset(*list(args) + [self.formset_new_form(**prev_blank)], **kw)
 
         # Image asset formset
@@ -570,14 +537,14 @@ def test_edit_uploadedicon(self):
         self.assertNoFormErrors(r)
         webapp = self.get_webapp()
 
-        # Unfortunate hardcoding of URL
+        # Unfortunate hardcoding of URL.
         url = webapp.get_icon_url(64)
         assert ('addon_icons/%s/%s' % (webapp.id / 1000, webapp.id)) in url, (
             'Unexpected path: %r' % url)
 
         eq_(data['icon_type'], 'image/png')
 
-        # Check that it was actually uploaded
+        # Check that it was actually uploaded.
         dirname = os.path.join(settings.ADDON_ICONS_PATH,
                                '%s' % (webapp.id / 1000))
         dest = os.path.join(dirname, '%s-32.png' % webapp.id)
@@ -612,14 +579,14 @@ def test_edit_uploadedicon_noresize(self):
         self.assertNoFormErrors(r)
         webapp = self.get_webapp()
 
-        # Unfortunate hardcoding of URL
+        # Unfortunate hardcoding of URL.
         addon_url = webapp.get_icon_url(64).split('?')[0]
         end = 'addon_icons/%s/%s-64.png' % (webapp.id / 1000, webapp.id)
         assert addon_url.endswith(end), 'Unexpected path: %r' % addon_url
 
         eq_(data['icon_type'], 'image/png')
 
-        # Check that it was actually uploaded
+        # Check that it was actually uploaded.
         dirname = os.path.join(settings.ADDON_ICONS_PATH,
                                '%s' % (webapp.id / 1000))
         dest = os.path.join(dirname, '%s-64.png' % webapp.id)
@@ -941,6 +908,7 @@ def test_screenshot_with_icon(self):
 
 
 class TestEditDetails(TestEdit):
+    fixtures = fixture('webapp_337141')
 
     def setUp(self):
         super(TestEditDetails, self).setUp()
@@ -1028,7 +996,7 @@ def test_edit_default_locale_required_trans(self):
         r = self.client.post(self.edit_url, data)
         self.assertFormError(r, 'form', None, missing(fields))
 
-        # Now we're sending an fr description with the form.
+        # Now we're sending an pt-BR description with the form.
         data['description_pt-BR'] = 'pt-BR description'
         r = self.client.post(self.edit_url, data)
         self.assertNoFormErrors(r)
@@ -1069,6 +1037,7 @@ def test_games_already_excluded_in_brazil(self):
 
 
 class TestEditSupport(TestEdit):
+    fixtures = fixture('webapp_337141')
 
     def setUp(self):
         super(TestEditSupport, self).setUp()
@@ -1113,7 +1082,7 @@ def test_edit_support_url_optional(self):
 
 
 class TestEditTechnical(TestEdit):
-    fixtures = TestEdit.fixtures
+    fixtures = fixture('webapp_337141')
 
     def setUp(self):
         super(TestEditTechnical, self).setUp()
@@ -1170,6 +1139,7 @@ def test_public_stats(self):
 
 
 class TestAdmin(TestEdit):
+    fixtures = TestEdit.fixtures
 
     def setUp(self):
         super(TestAdmin, self).setUp()
@@ -1189,6 +1159,7 @@ def log_in_with(self, rules):
 
 
 class TestAdminSettings(TestAdmin):
+    fixtures = TestEdit.fixtures
 
     def test_form_url(self):
         self.check_form_url('admin')
@@ -1333,6 +1304,7 @@ def test_ratings_view(self):
 
 
 class TestPromoUpload(TestAdmin):
+    fixtures = TestEdit.fixtures
 
     def post(self, **kw):
         data = {'caption': 'ball so hard that ish cray',

mkt/site/fixtures/data/webapp_337141.json

@@ -57,7 +57,7 @@
             "icon_type": "image/png",
             "annoying": 0,
             "modified": "2011-10-18 16:29:46",
-            "summary": 2425897,
+            "summary": 2425898,
             "nomination_message": null,
             "site_specific": false,
             "charity": null,
@@ -120,6 +120,18 @@
             "localized_string": "Something Something Steamcube!"
         }
     },
+    {
+        "pk": 2527086,
+        "model": "translations.translation",
+        "fields": {
+            "localized_string_clean": null,
+            "created": "2011-10-18 16:28:24",
+            "locale": "en-US",
+            "modified": "2011-10-18 16:28:57",
+            "id": 2425898,
+            "localized_string": "Something Something Steamcube Summary!"
+        }
+    },
     {
         "pk": 31337,
         "model": "auth.user",