Remove ability to edit name on app submission flow (bug 824734)

Author: Rob Hudson

mkt/site/fixtures/data/user_10482.json

@@ -0,0 +1,50 @@
+[
+    {
+        "pk": 10482,
+        "model": "auth.user",
+        "fields": {
+            "username": "clouserw@gmail.com",
+            "first_name": "Wil",
+            "last_name": "Clouser",
+            "is_active": 1,
+            "is_superuser": 1,
+            "is_staff": 1,
+            "last_login": "2010-04-23 10:33:53",
+            "groups": [],
+            "user_permissions": [],
+            "password": "sha1$a04e0$0512298efb3e6e7dbace3976474151d396078fdd",
+            "email": "clouserw@gmail.com",
+            "date_joined": "2007-03-05 13:09:38"
+        }
+    },
+    {
+        "pk": 10482,
+        "model": "users.userprofile",
+        "fields": {
+            "display_collections_fav": 1,
+            "display_collections": 1,
+            "averagerating": "3.39",
+            "confirmationcode": "",
+            "notifycompat": 1,
+            "picture_type": "image/jpeg",
+            "occupation": "Internets",
+            "homepage": "http://micropipes.com/blog/",
+            "email": "clouserw@gmail.com",
+            "location": "Portland, OR",
+            "bio": 480997,
+            "deleted": 0,
+            "emailhidden": 0,
+            "user": 10482,
+            "password": "sha512$7b5436061f8c0902088c292c057be69fdb17312e2f71607c9c51641f5d876522$08d1d370d89e2ae92755fd03464a7276ca607c431d04a52d659f7a184f3f9918073637d82fc88981c7099c7c46a1137b9fdeb675304eb98801038905a9ee0600",
+            "username": "clouserw",
+            "display_name": "clouserw \u0627\u0644\u062a\u0637\u0628",
+            "resetcode_expires": "2010-01-01 00:00:00",
+            "resetcode": "",
+            "created": "2007-03-05 13:09:38",
+            "notes": "",
+            "modified": "2010-03-19 13:50:38",
+            "notifyevents": 1,
+            "read_dev_agreement": "2012-08-20 00:00:00"
+        }
+    }
+]

mkt/submit/forms.py

@@ -7,18 +7,19 @@
 from tower import ugettext as _, ugettext_lazy as _lazy
 
 import amo
-from addons.forms import AddonFormBasic
-from addons.models import Addon, AddonDeviceType, AddonUpsell
-from apps.users.notifications import app_surveys
+from addons.models import Addon, AddonUpsell, BlacklistedSlug
+from amo.utils import slug_validator
 from apps.users.models import UserNotification
+from apps.users.notifications import app_surveys
 from editors.models import RereviewQueue
 from files.models import FileUpload
 from files.utils import parse_addon
 from market.models import AddonPremium, Price
-from translations.widgets import TransInput, TransTextarea
 from translations.fields import TransField
+from translations.forms import TranslationFormMixin
+from translations.widgets import TransInput, TransTextarea
 
-from mkt.constants import DEVICE_LOOKUP, FREE_PLATFORMS, PAID_PLATFORMS
+from mkt.constants import FREE_PLATFORMS, PAID_PLATFORMS
 from mkt.site.forms import AddonChoiceField, APP_PUBLIC_CHOICES
 
 
@@ -309,11 +310,14 @@ def save(self):
         self.addon.update(make_public=self.cleaned_data['make_public'])
 
 
-class AppDetailsBasicForm(AddonFormBasic):
+class AppDetailsBasicForm(TranslationFormMixin, happyforms.ModelForm):
+
+    def __init__(self, *args, **kw):
+        self.request = kw.pop('request')
+        super(AppDetailsBasicForm, self).__init__(*args, **kw)
+
     """Form for "Details" submission step."""
-    name = TransField(max_length=128,
-                      widget=TransInput(attrs={'class': 'name l'}))
-    slug = forms.CharField(max_length=30,
+    app_slug = forms.CharField(max_length=30,
                            widget=forms.TextInput(attrs={'class': 'm'}))
     summary = TransField(max_length=250,
         label=_lazy(u"Brief Summary:"),
@@ -325,10 +329,10 @@ class AppDetailsBasicForm(AddonFormBasic):
         help_text=_lazy(u'This description will appear on the details page.'),
         widget=TransTextarea(attrs={'rows': 4}))
     privacy_policy = TransField(widget=TransTextarea(attrs={'rows': 6}),
-         label=_lazy(u'Privacy Policy:'),
-         help_text=_lazy(u"A privacy policy that explains what "
-                          "data is transmitted from a user's computer and how "
-                          "it is used is required."))
+        label=_lazy(u'Privacy Policy:'),
+        help_text=_lazy(u"A privacy policy that explains what "
+                         "data is transmitted from a user's computer and how "
+                         "it is used is required."))
     homepage = TransField.adapt(forms.URLField)(required=False,
         verify_exists=False, label=_lazy(u'Homepage:'),
         help_text=_lazy(u'If your app has another homepage, enter its address '
@@ -364,13 +368,30 @@ class AppDetailsBasicForm(AddonFormBasic):
 
     class Meta:
         model = Addon
-        fields = ('flash', 'name', 'slug', 'summary', 'tags', 'description',
-                  'privacy_policy', 'homepage', 'support_url', 'support_email')
+        fields = ('app_slug', 'summary', 'description', 'privacy_policy',
+                  'homepage', 'support_url', 'support_email')
+
+    def clean_app_slug(self):
+        slug_field = 'app_slug'
+        target = self.cleaned_data[slug_field]
+        slug_validator(target, lower=False)
+
+        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, *args, **kw):
         uses_flash = self.cleaned_data.get('flash')
         af = self.instance.get_latest_file()
         if af is not None:
             af.update(uses_flash=bool(uses_flash))
 
-        return super(AppDetailsBasicForm, self).save(*args, **kw)
+        form = super(AppDetailsBasicForm, self).save(commit=False)
+        form.save()
+
+        return form

mkt/submit/templates/submit/details.html

@@ -36,8 +36,7 @@ <h2>{{ title }}</h2>
                    data-upload-url="{{ icon_upload_url }}">
           </div>
         </div>
-        <div class="houdini{{ ' ready' if not (form_basic.name.errors or
-                                               form_basic.slug.errors) }}">
+        <div class="houdini{{ ' ready' if not form_basic.app_slug.errors }}">
           <div class="before">
             <!-- {{ addon.name }} -->
             <h2 class="output">{{ addon.name }}</h2>
@@ -50,20 +49,19 @@ <h2 class="output">{{ addon.name }}</h2>
           </div>
           <div class="after">
             <table>
-              <tr{% if form_basic.name.errors %} class="error"{% endif %}>
+              <tr>
                 <th>
-                  <label for="{{ form_basic.name.auto_id }}">
+                  <label>
                     {{ _('Name') }}
                   </label>
                 </th>
                 <td>
-                  {{ form_basic.name }}
-                  {{ form_basic.name.errors }}
+                  {{ addon.name }}
                 </td>
               </tr>
-              <tr{% if form_basic.slug.errors %} class="error"{% endif %}>
+              <tr{% if form_basic.app_slug.errors %} class="error"{% endif %}>
                 <th>
-                  <label for="{{ form_basic.slug.auto_id }}">
+                  <label for="{{ form_basic.app_slug.auto_id }}">
                     {{ _('App URL') }}
                   </label>
                   {{ tip(None, _('Select a unique URL for your detail page. '
@@ -72,9 +70,9 @@ <h2 class="output">{{ addon.name }}</h2>
                 </th>
                 <td>
                   <div class="edit_with_prefix c">
-                    <span>{{ settings.SITE_URL }}/&hellip;/</span>{{ form_basic.slug }}
+                    <span>{{ settings.SITE_URL }}/&hellip;/</span>{{ form_basic.app_slug }}
                   </div>
-                  {{ form_basic.slug.errors }}
+                  {{ form_basic.app_slug.errors }}
                 </td>
               </tr>
             </table>

mkt/submit/tests/test_views.py

@@ -28,14 +28,15 @@
 from users.models import UserProfile
 
 import mkt
+from mkt.site.fixtures import fixture
 from mkt.submit.forms import NewWebappVersionForm
 from mkt.submit.models import AppSubmissionChecklist
 from mkt.submit.decorators import read_dev_agreement_required
 from mkt.webapps.models import AddonExcludedRegion as AER, Webapp
 
 
 class TestSubmit(amo.tests.TestCase):
-    fixtures = ['base/users']
+    fixtures = fixture('user_999')
 
     def setUp(self):
         self.user = self.get_user()
@@ -77,7 +78,7 @@ def _test_progress_display(self, completed, current):
 
 
 class TestProceed(TestSubmit):
-    fixtures = ['base/users']
+    fixtures = TestSubmit.fixtures
 
     def setUp(self):
         super(TestProceed, self).setUp()
@@ -98,7 +99,7 @@ def test_is_anonymous(self):
 
 
 class TestTerms(TestSubmit):
-    fixtures = ['base/users']
+    fixtures = TestSubmit.fixtures
 
     def setUp(self):
         super(TestTerms, self).setUp()
@@ -164,7 +165,7 @@ def test_read_dev_agreement_required(self):
 
 
 class TestManifest(TestSubmit):
-    fixtures = ['base/users']
+    fixtures = TestSubmit.fixtures
 
     def setUp(self):
         super(TestManifest, self).setUp()
@@ -216,7 +217,7 @@ def post(self, expect_errors=False, data=None):
 
 
 class BaseWebAppTest(BaseUploadTest, UploadAddon, amo.tests.TestCase):
-    fixtures = ['base/apps', 'base/users', 'base/platforms']
+    fixtures = fixture('app_firefox', 'platform_all', 'user_999', 'user_10482')
 
     def setUp(self):
         super(BaseWebAppTest, self).setUp()
@@ -394,8 +395,7 @@ def test_allow_duplicate_domains_from_js(self):
 
 
 class BasePackagedAppTest(BaseUploadTest, UploadAddon, amo.tests.TestCase):
-    fixtures = ['base/apps', 'base/users', 'base/platforms',
-                'webapps/337141-steamcube']
+    fixtures = fixture('webapp_337141', 'user_999')
 
     def setUp(self):
         super(BasePackagedAppTest, self).setUp()
@@ -479,7 +479,7 @@ def test_packaged_app_has_ids_file(self):
 
 
 class TestDetails(TestSubmit):
-    fixtures = ['base/apps', 'base/users', 'webapps/337141-steamcube']
+    fixtures = fixture('webapp_337141', 'user_999', 'user_10482')
 
     def setUp(self):
         super(TestDetails, self).setUp()
@@ -570,8 +570,7 @@ def preview_formset(self, *args, **kw):
 
     def get_dict(self, **kw):
         data = {
-            'name': 'Test name',
-            'slug': 'testname',
+            'app_slug': 'testname',
             'summary': 'Hello!',
             'description': 'desc',
             'privacy_policy': 'XXX <script>alert("xss")</script>',
@@ -599,7 +598,6 @@ def check_dict(self, data=None, expected=None):
 
         # Build a dictionary of expected results.
         expected_data = {
-            'name': 'Test name',
             'app_slug': 'testname',
             'summary': 'Hello!',
             'description': 'desc',
@@ -711,41 +709,14 @@ def test_icon(self):
         for size in amo.ADDON_ICON_SIZES:
             fn = '%s-%s.png' % (ad.id, size)
             assert os.path.exists(os.path.join(ad.get_icon_dir(), fn)), (
-                    'Expected %s in %s' % (fn, os.listdir(ad.get_icon_dir())))
+                'Expected %s in %s' % (fn, os.listdir(ad.get_icon_dir())))
 
     def _setup_other_webapp(self):
         self._step()
         # Generate another webapp to test name uniqueness.
         app = amo.tests.addon_factory(type=amo.ADDON_WEBAPP, name='Cool App')
         eq_(reverse_name_lookup(app.name, webapp=True), app.id)
 
-    def test_name_unique(self):
-        self._setup_other_webapp()
-        r = self.client.post(self.url, self.get_dict(name='Cool App'))
-        error = 'This name is already in use. Please choose another.'
-        self.assertFormError(r, 'form_basic', 'name', error)
-
-    def test_name_unique_strip(self):
-        # Make sure we can't sneak in a name by adding a space or two.
-        self._setup_other_webapp()
-        r = self.client.post(self.url, self.get_dict(name='  Cool App  '))
-        error = 'This name is already in use. Please choose another.'
-        self.assertFormError(r, 'form_basic', 'name', error)
-
-    def test_name_unique_case(self):
-        # Make sure unique names aren't case sensitive.
-        self._setup_other_webapp()
-        r = self.client.post(self.url, self.get_dict(name='cool app'))
-        error = 'This name is already in use. Please choose another.'
-        self.assertFormError(r, 'form_basic', 'name', error)
-
-    def test_name_required(self):
-        self._step()
-        r = self.client.post(self.url, self.get_dict(name=''))
-        eq_(r.status_code, 200)
-        self.assertFormError(r, 'form_basic', 'name',
-                             'This field is required.')
-
     def test_screenshot_or_video_required(self):
         self._step()
         data = self.get_dict()
@@ -775,28 +746,21 @@ def test_unsaved_screenshot(self):
         eq_(form.find('input[name=files-0-unsaved_image_data]').val(),
             preview_uri)
 
-    def test_name_length(self):
-        self._step()
-        r = self.client.post(self.url, self.get_dict(name='a' * 129))
-        eq_(r.status_code, 200)
-        self.assertFormError(r, 'form_basic', 'name',
-            'Ensure this value has at most 128 characters (it has 129).')
-
     def test_slug_invalid(self):
         self._step()
         # Submit an invalid slug.
-        d = self.get_dict(slug='slug!!! aksl23%%')
+        d = self.get_dict(app_slug='slug!!! aksl23%%')
         r = self.client.post(self.url, d)
         eq_(r.status_code, 200)
-        self.assertFormError(r, 'form_basic', 'slug',
+        self.assertFormError(r, 'form_basic', 'app_slug',
             "Enter a valid 'slug' consisting of letters, numbers, underscores "
             "or hyphens.")
 
     def test_slug_required(self):
         self._step()
-        r = self.client.post(self.url, self.get_dict(slug=''))
+        r = self.client.post(self.url, self.get_dict(app_slug=''))
         eq_(r.status_code, 200)
-        self.assertFormError(r, 'form_basic', 'slug',
+        self.assertFormError(r, 'form_basic', 'app_slug',
                              'This field is required.')
 
     def test_summary_required(self):