[extensions-web] extensions: Remove the extra "Submit" step in between uploading and reviewing

[Jasper St. Pierre <jstpierre src gnome org>]

commit c50647386d565415df36ad8b7c7fbbe0573493fc
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Tue Apr 24 04:41:37 2012 -0400

    extensions: Remove the extra "Submit" step in between uploading and reviewing
    
    It was visibly confusing users, for almost no gain, especially now that users
    can reject their own extensions. We're still keeping STATUS_NEW around, as it
    is required for backwards compatibility and all that. At some point, we may
    reject all STATUS_NEW versions of active extensions, and email the authors
    of extensions that never got past the dumb submit UI wall.
    
    While we're at it, let's remove the "step" numbers, and a lot of the
    instructions. The UI might need some more design in the future to be a bit
    more intuitive and featureful, but for now let's assume that it's good enough.

 .../extensions/templates/extensions/detail.html    |    3 -
 .../templates/extensions/detail_edit.html          |   79 ++++++--------------
 sweettooth/extensions/tests.py                     |   12 ++-
 sweettooth/extensions/urls.py                      |    1 -
 sweettooth/extensions/views.py                     |   26 +------
 sweettooth/static/css/sweettooth.css               |   34 ---------
 sweettooth/static/js/main.js                       |   12 ---
 7 files changed, 34 insertions(+), 133 deletions(-)
---
diff --git a/sweettooth/extensions/templates/extensions/detail.html b/sweettooth/extensions/templates/extensions/detail.html
index 2edab81..f335457 100644
--- a/sweettooth/extensions/templates/extensions/detail.html
+++ b/sweettooth/extensions/templates/extensions/detail.html
@@ -19,9 +19,6 @@
 {% endblock %}
 
 {% block body %}
-{% block extra-body-header %}
-{% endblock %}
-
     <div class="extension single-page {% block extra-ext-class %}{% endblock %}"
          data-epk="{{ extension.pk }}"
          data-uuid="{{ extension.uuid }}"
diff --git a/sweettooth/extensions/templates/extensions/detail_edit.html b/sweettooth/extensions/templates/extensions/detail_edit.html
index e405a11..14708b2 100644
--- a/sweettooth/extensions/templates/extensions/detail_edit.html
+++ b/sweettooth/extensions/templates/extensions/detail_edit.html
@@ -1,22 +1,5 @@
 {% extends "extensions/detail.html" %}
 
-{% block extra-body-header %}
-  {{ block.super }}
-  {% if is_preview %}
-  <div class="step">
-    <h2 class="steptitle">Step <span>2</span></h2>
-    <p>
-      Edit anything in yellow by clicking on it.
-    </p>
-
-    <p>
-      Give your extension some personality by uploading a screenshot
-      and icon. Make them identifiable!
-    </p>
-  </div>
-  {% endif %}
-{% endblock %}
-
 {% block comments %}
   {% if is_visible %}
     {% include "extensions/comments.html" %}
@@ -26,37 +9,25 @@
 {% endblock %}
 
 {% block extra %}
-  {% if is_preview %}
-  <div class="step lock">
-    <h2 class="steptitle">Step <span>3</span></h2>
-    <p>
-      After you're satisfied with how your extension looks, submit the extension for review.
+  <p class="step">
       A reviewer will review the extension you submitted to make sure there's nothing too
       dangerous. You'll be emailed the result of the review.
+  </p>
 
-      <button id="submit-ajax" data-url="{% url extensions-ajax-submit pk=version.pk %}">
-        Submit the extension for review.
-      </button>
-    </p>
-  </div>
-  {% endif %}
-
-  {% if not is_preview %}
-    <h2 class="expandy_header expanded"> Admin </h2>
-    <ul>
-      <li><a href="{% url extensions-upload-file %}">Upload a new version</a></li>
-      {% if version %}
-      <li><a href="{% url review-version pk=version.pk %}">View review and source</a></li>
-      {% endif %}
-    </ul>
+  <h2 class="expandy_header expanded"> Admin </h2>
+  <ul>
+    <li><a href="{% url extensions-upload-file %}">Upload a new version</a></li>
+    {% if version %}
+    <li><a href="{% url review-version pk=version.pk %}">View review and source</a></li>
+    {% endif %}
+  </ul>
 
-  {% if is_multiversion %}
-    <h2 class="expandy_header expanded"> Versions </h2>
-    <table>
-      <thead> <tr>
+  <h2 class="expandy_header expanded"> Versions </h2>
+  <table>
+    <thead> <tr>
         <th> Version </th> <th> Status </th> <th> Shell Versions </th> <th> Actions </th>
-      </tr> </thead>
-      <tbody>
+    </tr> </thead>
+    <tbody>
       {% for version in all_versions %}
       <tr data-pk="{{ version.pk }}">
         <td> <a href="{{ version.get_absolute_url }}"> {{ version.version }} </a> </td>
@@ -79,20 +50,18 @@
 
       </tr>
       {% endfor %}
+    </tbody>
+  </table>
+
+  <h2 class="expandy_header expanded"> Shell Versions </h2>
+  <div>
+    <p> Shell versions mapped to their associated extension versions </p>
+    <table id="extension_shell_versions_info">
+      <thead> <th> Shell Version </th> <th> Extension Version </th> </thead>
+      <tbody>
       </tbody>
     </table>
-
-    <h2 class="expandy_header expanded"> Shell Versions </h2>
-    <div>
-      <p> Shell versions mapped to their associated extension versions </p>
-      <table id="extension_shell_versions_info">
-        <thead> <th> Shell Version </th> <th> Extension Version </th> </thead>
-        <tbody>
-        </tbody>
-      </table>
-    </div>
-  {% endif %}
-  {% endif %}
+  </div>
 {% endblock %}
 
 {% block screenshot %}
diff --git a/sweettooth/extensions/tests.py b/sweettooth/extensions/tests.py
index 73b4039..6fb152c 100644
--- a/sweettooth/extensions/tests.py
+++ b/sweettooth/extensions/tests.py
@@ -191,7 +191,7 @@ class UploadTest(BasicUserTestCase, TransactionTestCase):
         extension = models.Extension.objects.get(uuid="test-extension mecheye net")
         version1 = extension.versions.order_by("-version")[0]
 
-        self.assertEquals(version1.status, models.STATUS_NEW)
+        self.assertEquals(version1.status, models.STATUS_UNREVIEWED)
         self.assertEquals(extension.creator, self.user)
         self.assertEquals(extension.name, "Test Extension")
         self.assertEquals(extension.description, "Simple test metadata")
@@ -210,7 +210,9 @@ class UploadTest(BasicUserTestCase, TransactionTestCase):
 
         version2 = extension.versions.order_by("-version")[0]
         self.assertNotEquals(version1, version2)
-        self.assertEquals(version2.status, models.STATUS_NEW)
+
+        # This should be auto-approved.
+        self.assertEquals(version2.status, models.STATUS_ACTIVE)
         self.assertEquals(version2.version, version1.version+1)
 
     def test_upload_large_uuid(self):
@@ -220,7 +222,7 @@ class UploadTest(BasicUserTestCase, TransactionTestCase):
         extension = models.Extension.objects.get(uuid=large_uuid)
         version1 = extension.versions.order_by("-version")[0]
 
-        self.assertEquals(version1.status, models.STATUS_NEW)
+        self.assertEquals(version1.status, models.STATUS_UNREVIEWED)
         self.assertEquals(extension.creator, self.user)
         self.assertEquals(extension.name, "Large UUID test")
         self.assertEquals(extension.description, "Simple test metadata")
@@ -569,7 +571,7 @@ class QueryExtensionsTest(BasicUserTestCase, TestCase):
         two = self.create_extension("two")
 
         models.ExtensionVersion.objects.create(extension=one, status=models.STATUS_ACTIVE)
-        models.ExtensionVersion.objects.create(extension=two, status=models.STATUS_NEW)
+        models.ExtensionVersion.objects.create(extension=two, status=models.STATUS_UNREVIEWED)
 
         # Since two is new, it shouldn't be visible.
         uuids = self.gather_uuids(dict(uuid=[one.uuid, two.uuid]))
@@ -608,7 +610,7 @@ class QueryExtensionsTest(BasicUserTestCase, TestCase):
         v = models.ExtensionVersion.objects.create(extension=one, status=models.STATUS_ACTIVE)
         v.parse_metadata_json({"shell-version": ["3.2"]})
 
-        v = models.ExtensionVersion.objects.create(extension=one, status=models.STATUS_NEW)
+        v = models.ExtensionVersion.objects.create(extension=one, status=models.STATUS_UNREVIEWED)
         v.parse_metadata_json({"shell-version": ["3.3.90"]})
 
         # Make sure that we don't see one, here - the version that
diff --git a/sweettooth/extensions/urls.py b/sweettooth/extensions/urls.py
index d40f0d0..bb2b675 100644
--- a/sweettooth/extensions/urls.py
+++ b/sweettooth/extensions/urls.py
@@ -6,7 +6,6 @@ from extensions import views, models, feeds
 
 ajax_patterns = patterns('',
     url(r'^edit/(?P<pk>\d+)', views.ajax_inline_edit_view, name='extensions-ajax-inline'),
-    url(r'^submit/(?P<pk>\d+)', views.ajax_submit_extension_view, name='extensions-ajax-submit'),
     url(r'^upload/screenshot/(?P<pk>\d+)', views.ajax_upload_screenshot_view, name='extensions-ajax-screenshot'),
     url(r'^upload/icon/(?P<pk>\d+)', views.ajax_upload_icon_view, name='extensions-ajax-icon'),
     url(r'^detail/', views.ajax_details_view, name='extensions-ajax-details'),
diff --git a/sweettooth/extensions/views.py b/sweettooth/extensions/views.py
index aa255b9..41814a9 100644
--- a/sweettooth/extensions/views.py
+++ b/sweettooth/extensions/views.py
@@ -230,12 +230,7 @@ def extension_view(request, obj, **kwargs):
 def extension_version_view(request, obj, **kwargs):
     extension, version = obj.extension, obj
 
-    is_preview = False
-
     status = version.status
-    if status == models.STATUS_NEW:
-        # If it's new, this is a preview before it's submitted
-        is_preview = True
 
     # Redirect if we don't match the slug or extension PK.
     slug = kwargs.get('slug')
@@ -260,7 +255,6 @@ def extension_version_view(request, obj, **kwargs):
     context = dict(version = version,
                    extension = extension,
                    shell_version_map = json.dumps(shell_version_map),
-                   is_preview = is_preview,
                    is_visible = status in models.VISIBLE_STATUSES,
                    is_rejected = status in models.REJECTED_STATUSES,
                    status = status)
@@ -289,21 +283,6 @@ def ajax_adjust_popularity_view(request):
 
 @ajax_view
 @require_POST
- model_view(models.ExtensionVersion)
-def ajax_submit_extension_view(request, obj):
-    if not obj.extension.user_can_edit(request.user):
-        return HttpResponseForbidden()
-
-    if obj.status != models.STATUS_NEW:
-        return HttpResponseForbidden()
-
-    obj.status = models.STATUS_UNREVIEWED
-    obj.save()
-
-    models.submitted_for_review.send(sender=request, request=request, version=obj)
-
- ajax_view
- require_POST
 @model_view(models.Extension)
 def ajax_inline_edit_view(request, obj):
     if not obj.user_can_edit(request.user):
@@ -428,15 +407,16 @@ def upload_file(request):
             else:
                 version = models.ExtensionVersion.objects.create(extension=extension,
                                                                  source=file_source,
-                                                                 status=models.STATUS_NEW)
+                                                                 status=models.STATUS_UNREVIEWED)
                 version.parse_metadata_json(metadata)
                 version.replace_metadata_json()
                 version.save()
 
+                models.submitted_for_review.send(sender=request, request=request, version=version)
+
                 transaction.commit()
 
                 return redirect(version)
-
     else:
         form = UploadForm()
 
diff --git a/sweettooth/static/css/sweettooth.css b/sweettooth/static/css/sweettooth.css
index 51b750c..0aee09e 100644
--- a/sweettooth/static/css/sweettooth.css
+++ b/sweettooth/static/css/sweettooth.css
@@ -707,40 +707,6 @@ input[type=submit], button {
     box-shadow: 0 2px 2px rgba(0, 0, 0, 0.5);
 }
 
-.step p {
-    line-height: 1.8em;
-}
-
-.step .steptitle {
-    color: #666;
-    margin: 0;
-}
-
-.step .steptitle span {
-    display: inline-block;
-
-    border-radius: 0.7em;
-    width: 1.4em;
-    height: 1.4em;
-    line-height: 1.4em;
-
-    color: #fff;
-    background-color: #666;
-    text-shadow: 1px 1px 0 #333;
-    text-align: center;
-    box-shadow: 1px 1px 3px rgba(0, 0, 0, 0.5);
-}
-
-.step .warning {
-    color: #cc0000;
-    font-weight: bold;
-}
-
-.step.lock button {
-    display: block;
-    margin: 10px auto 0;
-}
-
 .expandy_header {
     cursor: pointer;
     background-color: #336C84;
diff --git a/sweettooth/static/js/main.js b/sweettooth/static/js/main.js
index ce842fd..7c6732f 100644
--- a/sweettooth/static/js/main.js
+++ b/sweettooth/static/js/main.js
@@ -68,18 +68,6 @@ function($, messages, modal) {
             return false;
         });
 
-        $('#submit-ajax').click(function() {
-            var $elem = $(this);
-
-            var df = $.ajax({ url: $elem.data('url'),
-                              type: 'POST' });
-            df.done(function() {
-                messages.addInfo("Your extension has been submitted.").hide().slideDown();
-            });
-
-            return false;
-        });
-
         $('#local_extensions').addLocalExtensions();
         $('.extension.single-page').addExtensionSwitch();