step 5: select a license (bug 603124)

apps/devhub/templates/devhub/addons/owner.html

@@ -37,49 +37,8 @@ <h2>{{ title }}</h2>
           {% endfor %}
         </td>
       </tr>
-      {% if version %}
-        <tr>
-          <th>{{ _('License') }}</th>
-          <td>
-            {{ license_form.builtin.errors|safe }}
-            {{ license_form.builtin|safe }}
-            <div class="license-other {{ 'js-hidden' if not (license_form.initial.builtin == license_other_val or license_form.errors) }}"
-              data-val="{{ license_other_val }}">
-              {{ license_form.non_field_errors()|safe }}
-              {{ license_form.name.errors|safe }}
-              {{ license_form.name.label_tag()|safe }}
-              {{ license_form.name|safe }}
-              {{ license_form.text.errors|safe }}
-              {{ license_form.text.label_tag()|safe }}
-              {{ license_form.text|safe }}
-            </div>
-          </td>
-        </tr>
-      {% endif %}
-      <tr>
-        <th>{{ _('EULA') }}</th>
-        <td>
-          {{ policy_form.has_eula|safe }}
-          {{ policy_form.has_eula.label_tag()|safe }}
-          <div class="eula {{ 'hidden' if not policy_form.initial.has_eula }}">
-            {{ policy_form.eula.errors|safe }}
-            {{ policy_form.eula.label_tag()|safe }}
-            {{ policy_form.eula|safe }}
-          </div>
-        </td>
-      </tr>
-      <tr>
-        <th>{{ _('Privacy Policy') }}</th>
-        <td>
-          {{ policy_form.has_priv|safe }}
-          {{ policy_form.has_priv.label_tag()|safe }}
-          <div class="priv {{ 'hidden' if not policy_form.initial.has_priv }}">
-            {{ policy_form.privacy_policy.errors|safe }}
-            {{ policy_form.privacy_policy.label_tag()|safe }}
-            {{ policy_form.privacy_policy|safe }}
-          </div>
-        </td>
-      </tr>
+      {% include "devhub/includes/license_form.html" %}
+      {% include "devhub/includes/policy_form.html" %}
     </table>
   </div>
   <div class="listing-footer"><button type="submit">{{ _('Save Changes') }}</button></div>

apps/devhub/templates/devhub/includes/license_form.html

@@ -0,0 +1,19 @@
+{% if version %}
+  <tr>
+    <th>{{ _('License') }}</th>
+    <td>
+      {{ license_form.builtin.errors|safe }}
+      {{ license_form.builtin|safe }}
+      <div class="license-other {{ 'js-hidden' if not (license_form.initial.builtin == license_other_val or license_form.errors) }}"
+        data-val="{{ license_other_val }}">
+        {{ license_form.non_field_errors()|safe }}
+        {{ license_form.name.errors|safe }}
+        {{ license_form.name.label_tag()|safe }}
+        {{ license_form.name|safe }}
+        {{ license_form.text.errors|safe }}
+        {{ license_form.text.label_tag()|safe }}
+        {{ license_form.text|safe }}
+      </div>
+    </td>
+  </tr>
+{% endif %}

apps/devhub/templates/devhub/includes/policy_form.html

@@ -0,0 +1,24 @@
+<tr>
+  <th>{{ _('EULA') }}</th>
+  <td>
+    {{ policy_form.has_eula|safe }}
+    {{ policy_form.has_eula.label_tag()|safe }}
+    <div class="eula {{ 'hidden' if not policy_form.initial.has_eula }}">
+      {{ policy_form.eula.errors|safe }}
+      {{ policy_form.eula.label_tag()|safe }}
+      {{ policy_form.eula|safe }}
+    </div>
+  </td>
+</tr>
+<tr>
+  <th>{{ _('Privacy Policy') }}</th>
+  <td>
+    {{ policy_form.has_priv|safe }}
+    {{ policy_form.has_priv.label_tag()|safe }}
+    <div class="priv {{ 'hidden' if not policy_form.initial.has_priv }}">
+      {{ policy_form.privacy_policy.errors|safe }}
+      {{ policy_form.privacy_policy.label_tag()|safe }}
+      {{ policy_form.privacy_policy|safe }}
+    </div>
+  </td>
+</tr>

apps/devhub/tests/test_views.py

@@ -1673,6 +1673,9 @@ def setUp(self):
     def get_addon(self):
         return Addon.objects.no_cache().get(pk=3615)
 
+    def get_step(self):
+        return SubmitStep.objects.get(addon=self.get_addon())
+
 
 class TestSubmitStep1(TestSubmitBase):
 
@@ -1769,6 +1772,39 @@ def test_submit_summary_length(self):
         error = 'Ensure this value has at most 250 characters (it has 251).'
         self.assertFormError(r, 'form', 'summary', error)
 
+class TestSubmitStep5(TestSubmitBase):
+
+    def setUp(self):
+        super(TestSubmitStep5, self).setUp()
+        SubmitStep.objects.create(addon_id=3615, step=5)
+        self.url = reverse('devhub.submit.5', args=[3615])
+        self.next_step = reverse('devhub.submit.6', args=[3615])
+        License.objects.create(builtin=3, on_form=True)
+
+    def test_get(self):
+        eq_(self.client.get(self.url).status_code, 200)
+
+    def test_set_license(self):
+        r = self.client.post(self.url, {'builtin': 3})
+        self.assertRedirects(r, self.next_step)
+        eq_(self.get_addon().current_version.license.builtin, 3)
+        eq_(self.get_step().step, 6)
+
+    def test_license_error(self):
+        r = self.client.post(self.url, {'builtin': 4})
+        eq_(r.status_code, 200)
+        self.assertFormError(r, 'license_form', 'builtin',
+                             'Select a valid choice. 4 is not one of '
+                             'the available choices.')
+        eq_(self.get_step().step, 5)
+
+    def test_set_eula(self):
+        r = self.client.post(self.url, dict(builtin=3, has_eula=True,
+                                            eula='xxx'))
+        self.assertRedirects(r, self.next_step)
+        eq_(unicode(self.get_addon().eula), 'xxx')
+        eq_(self.get_step().step, 6)
+
 
 class TestSubmitStep6(TestSubmitBase):
 
@@ -1800,16 +1836,14 @@ def test_prelim_review(self):
         r = self.client.post(self.url, d)
         eq_(r.status_code, 302)
         eq_(self.get_addon().status, amo.STATUS_UNREVIEWED)
-        assert_raises(SubmitStep.DoesNotExist,
-                      SubmitStep.objects.get, addon=3615)
+        assert_raises(SubmitStep.DoesNotExist, self.get_step)
 
     def test_full_review(self):
         d = dict(review_type=amo.STATUS_NOMINATED)
         r = self.client.post(self.url, d)
         eq_(r.status_code, 302)
         eq_(self.get_addon().status, amo.STATUS_NOMINATED)
-        assert_raises(SubmitStep.DoesNotExist,
-                      SubmitStep.objects.get, addon=3615)
+        assert_raises(SubmitStep.DoesNotExist, self.get_step)
 
 
 class TestSubmitStep7(TestSubmitBase):

apps/devhub/urls.py

@@ -11,7 +11,7 @@
 submit_patterns = patterns('',
     url('^3$', views.submit_describe, name='devhub.submit.3'),
     url('^4$', views.submit_describe, name='devhub.submit.4'),
-    url('^5$', views.submit_describe, name='devhub.submit.5'),
+    url('^5$', views.submit_license, name='devhub.submit.5'),
     url('^6$', views.submit_select_review, name='devhub.submit.6'),
     url('^7$', views.submit_done, name='devhub.submit.7'),
 )

apps/devhub/views.py

@@ -256,57 +256,71 @@ def delete(request, addon_id, addon):
         return redirect(reverse('devhub.versions', args=(addon_id,)))
 
 
-@dev_required
-@owner_for_post_required
-def ownership(request, addon_id, addon):
-    fs = []
-    # Authors.
-    qs = AddonUser.objects.filter(addon=addon).order_by('position')
-    user_form = forms.AuthorFormSet(request.POST or None, queryset=qs)
-    fs.append(user_form)
-    # License. Clear out initial data if it's a builtin license.
+def _license_form(request, addon, save=False):
     qs = addon.versions.order_by('-version')[:1]
     version = qs[0] if qs else None
     if version:
         instance, initial = version.license, None
+        # Clear out initial data if it's a builtin license.
         if getattr(instance, 'builtin', None):
             instance, initial = None, {'builtin': instance.builtin}
         license_form = forms.LicenseForm(request.POST or None, initial=initial,
                                          instance=instance)
-        fs.append(license_form)
-    # Policy.
+
+    if save and version and license_form.is_valid():
+        license = license_form.save()
+        version.update(license=license)
+        amo.log(amo.LOG.CHANGE_LICENSE, license, addon)
+
+    license_urls = dict(License.objects.builtins()
+                        .values_list('builtin', 'url'))
+    return dict(license_urls=license_urls, version=version,
+                license_form=version and license_form,
+                license_other_val=License.OTHER)
+
+
+def _policy_form(request, addon, save=False):
     policy_form = forms.PolicyForm(
         request.POST or None, instance=addon,
         initial=dict(has_priv=bool(addon.privacy_policy),
                      has_eula=bool(addon.eula)))
+    if save and policy_form.is_valid():
+        policy_form.save(addon=addon)
+        amo.log(amo.LOG.CHANGE_POLICY, addon, policy_form.instance)
+    return policy_form
+
+
+@dev_required
+@owner_for_post_required
+def ownership(request, addon_id, addon):
+    fs, ctx = [], {}
+    # Authors.
+    qs = AddonUser.objects.filter(addon=addon).order_by('position')
+    user_form = forms.AuthorFormSet(request.POST or None, queryset=qs)
+    fs.append(user_form)
+    # Versions.
+    ctx.update(_license_form(request, addon))
+    if ctx['license_form']:
+        fs.append(ctx['license_form'])
+    # Policy.
+    policy_form = _policy_form(request, addon)
     fs.append(policy_form)
 
     if request.method == 'POST' and all([form.is_valid() for form in fs]):
         # Authors.
         authors = user_form.save(commit=False)
         for author in authors:
-            action = amo.LOG.CHANGE_USER_WITH_ROLE if author.id \
-                     else amo.LOG.REMOVE_USER_WITH_ROLE
+            action = (amo.LOG.CHANGE_USER_WITH_ROLE if author.id
+                      else amo.LOG.REMOVE_USER_WITH_ROLE)
             author.addon = addon
             author.save()
             amo.log(action, author.user, author.get_role_display(), addon)
-        # License.
-        if version:
-            license = license_form.save()
-            addon.current_version.update(license=license)
-            amo.log(amo.LOG.CHANGE_LICENSE, license, addon)
-        # Policy.
-        policy_form.save(addon=addon)
-        amo.log(amo.LOG.CHANGE_POLICY, addon, policy_form.instance)
-
+        _license_form(request, addon, save=True)
+        _policy_form(request, addon, save=True)
         return redirect('devhub.addons.owner', addon_id)
 
-    license_urls = dict(License.objects.builtins()
-                        .values_list('builtin', 'url'))
-    return jingo.render(request, 'devhub/addons/owner.html',
-        dict(addon=addon, user_form=user_form, version=version,
-             license_form=version and license_form, license_urls=license_urls,
-             policy_form=policy_form, license_other_val=License.OTHER))
+    ctx.update(addon=addon, user_form=user_form, policy_form=policy_form)
+    return jingo.render(request, 'devhub/addons/owner.html', ctx)
 
 
 @dev_required
@@ -625,13 +639,32 @@ def submit_describe(request, addon_id, addon, step):
     if request.method == 'POST' and form.is_valid():
         addon = form.save(addon)
         amo.log(amo.LOG.EDIT_DESCRIPTIONS, addon)
-        SubmitStep.objects.filter(addon=addon).update(step=F('step') + 1)
-        return redirect('devhub.submit.add_media')
+        SubmitStep.objects.filter(addon=addon).update(step=4)
+        return redirect('devhub.submit.4', addon.id)
 
     return jingo.render(request, 'devhub/addons/submit/describe.html',
                         {'form': form, 'addon': addon, 'step': step})
 
 
+@dev_required
+@submit_step(5)
+def submit_license(request, addon_id, addon, step):
+    fs, ctx = [], {}
+    # Versions.
+    ctx.update(_license_form(request, addon))
+    fs.append(ctx['license_form'])
+    # Policy.
+    policy_form = _policy_form(request, addon)
+    fs.append(policy_form)
+    if request.method == 'POST' and all([form.is_valid() for form in fs]):
+        _license_form(request, addon, save=True)
+        _policy_form(request, addon, save=True)
+        SubmitStep.objects.filter(addon=addon).update(step=6)
+        return redirect('devhub.submit.6', addon.id)
+    ctx.update(addon=addon, policy_form=policy_form, step=step)
+    return jingo.render(request, 'devhub/addons/submit/license.html', ctx)
+
+
 @dev_required
 @submit_step(6)
 def submit_select_review(request, addon_id, addon, step):