have combined name and summary length restriction (#10059)

src/olympia/devhub/forms.py

@@ -32,7 +32,8 @@ from olympia.constants.categories import CATEGORIES, CATEGORIES_NO_APP
 from olympia.files.models import FileUpload
 from olympia.files.utils import (
     archive_member_validator, parse_addon, SafeZip)
-from olympia.translations.fields import TransField, TransTextarea
+from olympia.translations.fields import (
+    LocaleErrorMessage, TransField, TransTextarea)
 from olympia.translations.forms import TranslationFormMixin
 from olympia.translations.models import Translation, delete_translation
 from olympia.translations.widgets import (
@@ -657,6 +658,35 @@ class DescribeForm(AkismetSpamCheckFormMixin, AddonFormBase):
         return obj
 
 
+class CombinedNameSummaryCleanMixin(object):
+    def clean(self):
+        message = _('Ensure name and summary combined are at most 70 '
+                    'characters (they have {0}).')
+        super(CombinedNameSummaryCleanMixin, self).clean()
+        name_summary_locales = set(
+            self.cleaned_data.get('name', {}).keys() +
+            self.cleaned_data.get('summary', {}).keys())
+        default_locale = self.instance.default_locale
+        name_values = self.cleaned_data.get('name') or {}
+        name_default = name_values.get(default_locale) or ''
+        summary_values = self.cleaned_data.get('summary') or {}
+        summary_default = summary_values.get(default_locale) or ''
+        for locale in name_summary_locales:
+            val_len = len(name_values.get(locale, name_default) +
+                          summary_values.get(locale, summary_default))
+            if val_len > 70:
+                self.add_error(
+                    'name', LocaleErrorMessage(
+                        message=message.format(val_len), locale=locale))
+        return self.cleaned_data
+
+
+class DescribeFormContentOptimization(CombinedNameSummaryCleanMixin,
+                                      DescribeForm):
+    name = TransField(max_length=68, min_length=2)
+    summary = TransField(max_length=68, min_length=2)
+
+
 class DescribeFormUnlisted(AkismetSpamCheckFormMixin, AddonFormBase):
     name = TransField(max_length=50)
     slug = forms.CharField(max_length=30)
@@ -672,6 +702,12 @@ class DescribeFormUnlisted(AkismetSpamCheckFormMixin, AddonFormBase):
         fields = ('name', 'slug', 'summary', 'description')
 
 
+class DescribeFormUnlistedContentOptimization(CombinedNameSummaryCleanMixin,
+                                              DescribeFormUnlisted):
+    name = TransField(max_length=68, min_length=2)
+    summary = TransField(max_length=68, min_length=2)
+
+
 class PreviewForm(forms.ModelForm):
     caption = TransField(widget=TransTextarea, required=False)
     file_upload = forms.FileField(required=False)

src/olympia/devhub/templates/devhub/addons/edit/describe.html

@@ -17,6 +17,60 @@
         {# L10n: {0} is the addon name #}
         <caption>{{ _('Describe {0}')|format_html(addon.name) }}</caption>
         <tbody>
+          {% if waffle.switch('content-optimization') %}
+          <tr>
+            <th><label data-for="name">{{ _("Name and Summary") }}
+                {{ tip(None,
+                       _("The summary should explain what your add-on does, "
+                         "clearly and concisely. Both the name and summary will "
+                         "appear on your product page and search results. They "
+                         "have a combined maximum length of 70 characters and a "
+                         "minimum length of 2 characters for each.")) }}
+            </label></th>
+            <td>
+              {% if editable %}
+                <div class="combine-name-summary">
+                  {{ form.name }}&nbsp;{{ form.summary }}
+                  {{ form.name.errors }}
+                  {{ form.summary.errors }}
+                </div>
+                <div class="char-count"
+                  data-text-prefix="{{ _('Name') }}&nbsp;"
+                  data-text-postfix=";&nbsp;"
+                  data-for-startswith="{{ form.name.auto_id }}_"
+                  data-minlength="{{ form.name.field.min_length }}"></div>
+                <div class="char-count"
+                  data-text-prefix="{{ _('Summary') }}&nbsp;"
+                  data-text-postfix=";&nbsp;"
+                  data-for-startswith="{{ form.summary.auto_id }}_"
+                  data-minlength="{{ form.summary.field.min_length }}"></div>
+                <div class="char-count"
+                  data-for-names="name,summary"
+                  data-maxlength="{{ form.name.field.min_length + form.summary.field.max_length }}"></div>
+              {% else %}
+                {{ addon|all_locales('name') }}
+                {{ addon|all_locales('summary') }}
+              {% endif %}
+            </td>
+          </tr>
+          <tr>
+            <th>
+              {{ tip(_("Add-on URL"),
+                 _("Choose a short, unique URL slug for your add-on.")) }}
+            </th>
+            <td id="slug_edit">
+              {% if editable %}
+                <div class="edit_with_prefix">
+                  <span>{{ settings.SITE_URL }}/&hellip;/</span>{{ form.slug }}
+                </div>
+                {{ form.slug.errors }}
+              {% else %}
+                {{ settings.SITE_URL }}/&hellip;/{{ addon.slug }}
+                {% if addon.has_listed_versions() %}<a href="{{ addon.get_url_path() }}">{{ _('View Listing') }}</a>{% endif %}
+              {% endif %}
+            </td>
+          </tr>
+          {% else %}
           <tr>
             <th><label data-for="name">{{ _("Name") }}</label></th>
             <td>
@@ -67,6 +121,7 @@
               {% endif %}
             </td>
           </tr>
+          {% endif %}
           <tr>
             <th>
               <label data-for="description">

src/olympia/devhub/templates/devhub/addons/submit/describe.html

@@ -9,6 +9,49 @@
   <form method="post" id="submit-describe" class="item">
     {{ form.non_field_errors() }}
     {% csrf_token %}
+    {% if waffle.switch('content-optimization') %}
+    <div class="addon-submission-field">
+      <div class="combine-name-summary">
+        <label for="id_name">{{ _("Name and Summary:") }}<span class="req" title="{{ _('required') }}">*</span></label>
+        {{ form.name }}&nbsp;{{ form.summary }}
+        {{ form.name.errors }}
+        {{ form.summary.errors }}
+      </div>
+      <div class="edit-addon-details">
+        {{ _('The summary should explain what your add-on does, clearly and '
+             'concisely. Both the name and summary will appear on your product '
+             'page and search results. They have a combined maximum length of '
+             '70 characters and a minimum length of 2 characters for each.') }}
+        <div class="char-count"
+             data-for-names="name,summary"
+             data-maxlength="{{ form.name.field.min_length + form.summary.field.max_length }}"></div>
+        <div class="char-count"
+             data-text-prefix="{{ _('Summary') }}&nbsp;"
+             data-text-postfix=";&nbsp;"
+             data-for-startswith="{{ form.summary.auto_id }}_"
+             data-minlength="{{ form.summary.field.min_length }}"></div>
+        <div class="char-count"
+             data-text-prefix="{{ _('Name') }}&nbsp;"
+             data-text-postfix=";&nbsp;"
+             data-for-startswith="{{ form.name.auto_id }}_"
+             data-minlength="{{ form.name.field.min_length }}"></div>
+      </div>
+    </div>
+    <div class="addon-submission-field slug-edit">
+      <label>{{ _("Add-on URL:") }}</label>
+      <div id="slug_edit" class="edit_with_prefix edit_initially_hidden">
+        <span>{{ settings.SITE_URL }}</span>{{ form.slug }}
+        <div class="edit-addon-details">
+          {{ _('Please use only letters, numbers, underscores, and dashes in your URL.') }}
+        </div>
+      </div>
+      <span id="slug_readonly">
+        {{ settings.SITE_URL }}/&hellip;/<span id="slug_value"></span>
+        <a id="edit_slug" href="#">{{ _('Edit') }}</a>
+      </span>
+      {{ form.slug.errors }}
+    </div>
+    {% else %}
     <div class="addon-submission-field">
       <label for="id_name">{{ _("Name:") }}</label>
       <span class="tip tooltip" title="{{ _('Name on listing on this site. May be different to the name inside the add-on, which is shown inside Firefox') }}" data-oldtitle="">?</span>
@@ -40,6 +83,7 @@
              data-maxlength="{{ form.summary.field.max_length }}"></div>
       </div>
     </div>
+    {% endif %}
     {% if addon.type == amo.ADDON_EXTENSION %}
     <div class="addon-submission-field">
       <label>{{ _('Description:') }}

src/olympia/devhub/tests/test_forms.py

@@ -852,7 +852,7 @@ class TestDescribeForm(TestCase):
             request=self.request, instance=Addon.objects.get())
         assert form.is_valid(), form.errors
 
-    def test_summary_optional(self):
+    def test_description_optional(self):
         delicious = Addon.objects.get()
         assert delicious.type == amo.ADDON_EXTENSION
 
@@ -887,7 +887,7 @@ class TestDescribeForm(TestCase):
                 instance=delicious)
             assert form.is_valid(), form.errors
 
-    def test_summary_min_length(self):
+    def test_description_min_length(self):
         delicious = Addon.objects.get()
         assert delicious.type == amo.ADDON_EXTENSION
 
@@ -913,7 +913,7 @@ class TestDescribeForm(TestCase):
                 request=self.request, instance=delicious)
             assert form.is_valid()
 
-            #  Do it again, but this time with a description
+            #  Do it again, but this time with a longer description
             delicious.update(type=amo.ADDON_EXTENSION)
             form = forms.DescribeForm(
                 {'name': 'Delicious for everyone', 'summary': 'foo',
@@ -921,3 +921,56 @@ class TestDescribeForm(TestCase):
                 request=self.request,
                 instance=delicious)
             assert form.is_valid(), form.errors
+
+    def test_name_summary_lengths(self):
+        delicious = Addon.objects.get()
+        short_data = {
+            'name': 'n', 'summary': 's', 'slug': 'bar',
+            'description': '1234567890'}
+        over_70_data = {
+            'name': 'this is a name that hits the 50 char limit almost',
+            'summary': 'this is a summary that doesn`t get close to the '
+                       'existing 250 limit but is over 70',
+            'slug': 'bar', 'description': '1234567890'}
+        under_70_data = {
+            'name': 'this is a name that is over the 50 char limit by a few',
+            'summary': 'ab',
+            'slug': 'bar', 'description': '1234567890'}
+
+        # short name and summary - both allowed with DescribeForm
+        form = forms.DescribeForm(
+            short_data, request=self.request, instance=delicious)
+        assert form.is_valid()
+        # but not with DescribeFormContentOptimization
+        form = forms.DescribeFormContentOptimization(
+            short_data, request=self.request, instance=delicious)
+        assert not form.is_valid()
+        assert form.errors['name'] == [
+            u'Ensure this value has at least 2 characters (it has 1).']
+        assert form.errors['summary'] == [
+            u'Ensure this value has at least 2 characters (it has 1).']
+
+        # As are long names and summaries
+        form = forms.DescribeForm(
+            over_70_data, request=self.request, instance=delicious)
+        assert form.is_valid()
+        # but together are over 70 chars so no longer allowed
+        form = forms.DescribeFormContentOptimization(
+            over_70_data, request=self.request, instance=delicious)
+        assert not form.is_valid()
+        assert len(over_70_data['name']) + len(over_70_data['summary']) == 130
+        assert form.errors['name'] == [
+            u'Ensure name and summary combined are at most 70 characters '
+            u'(they have 130).']
+
+        # DescribeForm has a lower limit for name length
+        form = forms.DescribeForm(
+            under_70_data, request=self.request, instance=delicious)
+        assert not form.is_valid()
+        assert form.errors['name'] == [
+            u'Ensure this value has at most 50 characters (it has 54).']
+        # DescribeFormContentOptimization only cares that the total is <= 70
+        form = forms.DescribeFormContentOptimization(
+            under_70_data, request=self.request, instance=delicious)
+        assert form.is_valid()
+        assert len(under_70_data['name']) + len(under_70_data['summary']) == 56

src/olympia/devhub/tests/test_views_edit.py

@@ -418,6 +418,65 @@ class BaseTestEditDescribe(BaseTestEdit):
         doc = pq(response.content)
         assert not doc('#trans-description textarea').attr('minlength')
 
+    @override_switch('content-optimization', active=False)
+    def test_name_summary_lengths_short(self):
+        # check the separate name and summary labels, etc are served
+        response = self.client.get(self.url)
+        assert 'Name and Summary' not in response.content
+        assert 'It will be shown in listings and searches' in response.content
+
+        self.client.post(
+            self.describe_edit_url, self.get_dict(name='a', summary='b'))
+        assert self.get_addon().name == 'a'
+        assert self.get_addon().summary == 'b'
+
+    @override_switch('content-optimization', active=False)
+    def test_name_summary_lengths_long(self):
+        self.client.post(
+            self.describe_edit_url, self.get_dict(
+                name='a' * 50, summary='b' * 50))
+        assert self.get_addon().name == 'a' * 50
+        assert self.get_addon().summary == 'b' * 50
+
+    @override_switch('content-optimization', active=True)
+    def test_name_summary_lengths_content_optimization(self):
+        # check the combined name and summary label, etc are served
+        response = self.client.get(self.url)
+        assert 'Name and Summary' in response.content
+
+        # name and summary are too short
+        response = self.client.post(
+            self.describe_edit_url, self.get_dict(name='a', summary='b'))
+        assert self.get_addon().name != 'a'
+        assert self.get_addon().summary != 'b'
+        assert response.status_code == 200
+        self.assertFormError(
+            response, 'form', 'name',
+            'Ensure this value has at least 2 characters (it has 1).')
+        self.assertFormError(
+            response, 'form', 'summary',
+            'Ensure this value has at least 2 characters (it has 1).')
+
+        # name and summary individually are okay, but together are too long
+        response = self.client.post(
+            self.describe_edit_url, self.get_dict(
+                name='a' * 50, summary='b' * 50))
+        assert self.get_addon().name != 'a' * 50
+        assert self.get_addon().summary != 'b' * 50
+        assert response.status_code == 200
+        self.assertFormError(
+            response, 'form', 'name',
+            'Ensure name and summary combined are at most 70 characters '
+            u'(they have 100).')
+
+        # success: together name and summary are 70 characters.
+        response = self.client.post(
+            self.describe_edit_url, self.get_dict(
+                name='a' * 2, summary='b' * 68))
+        assert self.get_addon().name == 'a' * 2
+        assert self.get_addon().summary == 'b' * 68
+        assert response.status_code == 200
+
 
 class L10nTestsMixin(object):
     def get_l10n_urls(self):

src/olympia/devhub/tests/test_views_submit.py

@@ -987,6 +987,58 @@ class DetailsPageMixin(object):
         comment_check_mock.assert_not_called()
         assert AkismetReport.objects.count() == 0
 
+    @override_switch('content-optimization', active=False)
+    def test_name_summary_lengths_short(self):
+        # check the separate name and summary labels, etc are served
+        response = self.client.get(self.url)
+        assert 'Name and Summary' not in response.content
+        assert 'It will be shown in listings and searches' in response.content
+
+        data = self.get_dict(name='a', summary='b')
+        self.is_success(data)
+
+    @override_switch('content-optimization', active=False)
+    def test_name_summary_lengths_long(self):
+        data = self.get_dict(name='a' * 50, summary='b' * 50)
+        self.is_success(data)
+
+    @override_switch('content-optimization', active=True)
+    def test_name_summary_lengths_content_optimization(self):
+        # check the combined name and summary label, etc are served
+        response = self.client.get(self.url)
+        assert 'Name and Summary' in response.content
+
+        # name and summary are too short
+        response = self.client.post(
+            self.url, self.get_dict(
+                name='a', summary='b', description='c' * 10))
+        assert self.get_addon().name != 'a'
+        assert self.get_addon().summary != 'b'
+        assert response.status_code == 200
+        self.assertFormError(
+            response, 'form', 'name',
+            'Ensure this value has at least 2 characters (it has 1).')
+        self.assertFormError(
+            response, 'form', 'summary',
+            'Ensure this value has at least 2 characters (it has 1).')
+
+        # name and summary individually are okay, but together are too long
+        response = self.client.post(
+            self.url, self.get_dict(
+                name='a' * 50, summary='b' * 50, description='c' * 10))
+        assert self.get_addon().name != 'a' * 50
+        assert self.get_addon().summary != 'b' * 50
+        assert response.status_code == 200
+        self.assertFormError(
+            response, 'form', 'name',
+            'Ensure name and summary combined are at most 70 characters '
+            u'(they have 100).')
+
+        # success: together name and summary are 70 characters.
+        data = self.get_dict(
+            name='a' * 2, summary='b' * 68, description='c' * 10)
+        self.is_success(data)
+
 
 class TestAddonSubmitDetails(DetailsPageMixin, TestSubmitBase):
 

src/olympia/devhub/views.py

@@ -760,9 +760,11 @@ def addons_section(request, addon_id, addon, section, editable=False):
     show_listed = addon.has_listed_versions()
     static_theme = addon.type == amo.ADDON_STATICTHEME
     models = {}
+    content_waffle = waffle.switch_is_active('content-optimization')
     if show_listed:
         models.update({
-            'describe': forms.DescribeForm,
+            'describe': (forms.DescribeForm if not content_waffle
+                         else forms.DescribeFormContentOptimization),
             'additional_details': addon_forms.AdditionalDetailsForm,
             'technical': addon_forms.AddonFormTechnical
         })
@@ -770,7 +772,8 @@ def addons_section(request, addon_id, addon, section, editable=False):
             models.update({'media': addon_forms.AddonFormMedia})
     else:
         models.update({
-            'describe': forms.DescribeFormUnlisted,
+            'describe': (forms.DescribeFormUnlisted if not content_waffle
+                         else forms.DescribeFormUnlistedContentOptimization),
             'additional_details': addon_forms.AdditionalDetailsFormUnlisted,
             'technical': addon_forms.AddonFormTechnicalUnlisted
         })
@@ -1496,7 +1499,10 @@ def _submit_details(request, addon, version):
     show_all_fields = not version or not addon.has_complete_metadata()
 
     if show_all_fields:
-        describe_form = forms.DescribeForm(
+        content_waffle = waffle.switch_is_active('content-optimization')
+        describe_form_cls = (forms.DescribeForm if not content_waffle
+                             else forms.DescribeFormContentOptimization)
+        describe_form = describe_form_cls(
             post_data, instance=addon, request=request, version=version)
         cat_form_class = (addon_forms.CategoryFormSet if not static_theme
                           else forms.SingleCategoryForm)

static/css/zamboni/developers.css

@@ -86,6 +86,29 @@ ul.refinements:last-child {
     width: 370px;
 }
 
+#submit-describe .combine-name-summary,
+#addon-edit-describe .combine-name-summary {
+    display: flex;
+    margin-right: 2px;
+}
+
+#submit-describe .combine-name-summary #trans-name,
+#addon-edit-describe .combine-name-summary #trans-name {
+    flex-grow: 20;
+}
+
+#submit-describe .combine-name-summary #trans-summary,
+#addon-edit-describe .combine-name-summary #trans-summary {
+    flex-grow: 50;
+}
+
+#submit-describe .combine-name-summary #trans-name input,
+#addon-edit-describe .combine-name-summary #trans-name input,
+#submit-describe .combine-name-summary #trans-summary input,
+#addon-edit-describe .combine-name-summary #trans-summary input {
+    width: 95%;
+}
+
 /*#id_homepage, #id_support_url, #id_support_email {*/
 #trans-support_email input,
 #trans-homepage input,

static/js/zamboni/global.js

@@ -58,7 +58,7 @@ jQuery.fn.tooltip = function(tip_el) {
 
     $(document.body).on("tooltip_change", setTip);
 
-    function mouseover(e) {
+    function mouseover() {
         $tgt = $(this);
         if ($tgt.hasClass("formerror")) $tip.addClass("error");
         $title = $tgt.attr('title') ? $tgt : $("[title]", $tgt).first();
@@ -67,7 +67,7 @@ jQuery.fn.tooltip = function(tip_el) {
         }
     }
 
-    function mouseout(e) {
+    function mouseout() {
         clearTimeout(timeout);
         $tip.hide()
             .removeClass("error");
@@ -395,13 +395,13 @@ $.fn.modal = function(click_target, o) {
 
 // Modal from URL. Pass in a URL, and load it in a modal.
 function modalFromURL(url, settings) {
+    settings = settings || {};
     var a = $('<a>'),
         defaults = {'deleteme': true, 'close': true},
-        settings = settings || {},
-        data = settings['data'] || {},
-        callback = settings['callback'];
+        data = settings.data || {},
+        callback = settings.callback;
 
-    delete settings['callback'];
+    delete settings.callback;
     settings = $.extend(defaults, settings);
 
     var inside = $('<div>', {'class': 'modal-inside', 'text': gettext('Loading...')}),
@@ -460,10 +460,8 @@ function slugify() {
 
 // Initializes character counters for textareas.
 function initCharCount() {
-    var countChars = function(el, cc) {
-        var $el = $(el),
-            val = $el.val(),
-            max = parseInt(cc.attr('data-maxlength'), 10),
+    var countChars = function(val, cc) {
+        var max = parseInt(cc.attr('data-maxlength'), 10),
             min = parseInt(cc.attr('data-minlength'), 10) || 0,
             // Count \r\n as one character, not two.
             lineBreaks = val.split('\n').length - 1,
@@ -480,19 +478,36 @@ function initCharCount() {
             output.push(format(ngettext('<b>{0}</b> character left',
                                         '<b>{0}</b> characters left', left), [left]));
         }
-        cc.html(output.join('; ') + '.')
+        cc.html((cc.attr('data-text-prefix') || '') + output.join('; ') + (cc.attr('data-text-postfix') || '.'))
           .toggleClass('error', left < 0 || count < min);
     };
     $('.char-count').each(function() {
         var $cc = $(this),
             $form = $(this).closest('form'),
-            $el;
-        if ($cc.attr('data-for-startswith') !== undefined) {
-            $el = $('textarea[id^="' + $cc.attr('data-for-startswith') + '"]:visible', $form);
+            $el,
+            multi=false;
+        if ($cc.data('for-names') !== undefined) {
+            multi=true;
+            var query_string = $cc.data('for-names').split(',').map(function(field_name) {
+                return 'textarea[name^="' + field_name + '"]:visible, input[name^="' + field_name + '"]:visible';
+            }).join(', ');
+            $el = $(query_string, $form);
+        } else if ($cc.attr('data-for-startswith') !== undefined) {
+            $el = $('textarea[id^="' + $cc.attr('data-for-startswith') + '"]:visible, input[id^="' + $cc.attr('data-for-startswith') + '"]:visible', $form);
         } else {
             $el = $('textarea#' + $cc.attr('data-for'), $form);
         }
-        $el.on('keyup blur', function() { countChars(this, $cc); }).trigger('blur');
+        $el.on('keyup blur', function() {
+            var $this=$(this),
+                val;
+            if (multi) {
+                val = $el.filter('[name$="' + $this.attr('lang') + '"]').map(function() {
+                        return $(this).val();
+                    }).get().join('');
+            } else {
+                val = $this.val();
+            }
+            countChars(val, $cc); }).trigger('blur');
     });
 }