Merge branch 'delete'

apps/addons/models.py

@@ -261,33 +261,38 @@ def clean_slug(self):
 
     @transaction.commit_on_success
     def delete(self, msg):
-        log.debug('Adding guid to blacklist: %s' % self.guid)
-        BlacklistedGuid(guid=self.guid, comments=msg).save()
-        log.debug('Deleting add-on: %s' % self.id)
-
-        authors = [u.email for u in self.authors.all()]
-        to = [settings.FLIGTAR]
-        user = amo.get_user()
-        user_str = "%s, %s (%s)" % (user.display_name or user.username,
-                                    user.email, user.id) if user else "Unknown"
-
-        email_msg = u"""
-        The following add-on was deleted.
-        ADD-ON: %s
-        DELETED BY: %s
-        ID: %s
-        GUID: %s
-        AUTHORS: %s
-        TOTAL DOWNLOADS: %s
-        AVERAGE DAILY USERS: %s
-        NOTES: %s
-        """ % (self.name, user_str, self.id, self.guid, authors,
-               self.total_downloads, self.average_daily_users, msg)
-        log.debug('Sending delete email for add-on %s' % self.id)
-        subject = 'Deleting add-on %s' % self.id
+        if self.highest_status:
+            log.debug('Adding guid to blacklist: %s' % self.guid)
+            BlacklistedGuid(guid=self.guid, comments=msg).save()
+            log.debug('Deleting add-on: %s' % self.id)
+
+            authors = [u.email for u in self.authors.all()]
+            to = [settings.FLIGTAR]
+            user = amo.get_user()
+            user_str = "%s, %s (%s)" % (user.display_name or user.username,
+                    user.email, user.id) if user else "Unknown"
+
+            email_msg = u"""
+            The following add-on was deleted.
+            ADD-ON: %s
+            DELETED BY: %s
+            ID: %s
+            GUID: %s
+            AUTHORS: %s
+            TOTAL DOWNLOADS: %s
+            AVERAGE DAILY USERS: %s
+            NOTES: %s
+            """ % (self.name, user_str, self.id, self.guid, authors,
+                   self.total_downloads, self.average_daily_users, msg)
+            log.debug('Sending delete email for add-on %s' % self.id)
+            subject = 'Deleting add-on %s' % self.id
 
         rv = super(Addon, self).delete()
-        send_mail(subject, email_msg, recipient_list=to)
+
+        # We want to ensure deletion before notification.
+        if self.highest_status:
+            send_mail(subject, email_msg, recipient_list=to)
+
         return rv
 
     @classmethod

apps/addons/tests/test_models.py

@@ -136,6 +136,17 @@ def test_delete(self):
         a.name = u'é'
         a.delete('bye')
         eq_(len(mail.outbox), 1)
+        assert BlacklistedGuid.objects.filter(guid=a.guid)
+
+    def test_delete_incomplete(self):
+        """Test deleting incomplete add-ons."""
+        a = Addon.objects.get(pk=3615)
+        a.status = 0
+        a.highest_status = 0
+        a.save()
+        a.delete(None)
+        eq_(len(mail.outbox), 0)
+        assert not BlacklistedGuid.objects.filter(guid=a.guid)
 
     def test_incompatible_latest_apps(self):
         a = Addon.objects.get(pk=3615)
@@ -805,8 +816,8 @@ def setUp(self):
     def test_blacklisted_guid(self):
         BlacklistedGuid.objects.create(guid='guid@xpi')
         with self.assertRaises(forms.ValidationError) as e:
-            addon = Addon.from_upload(self.get_upload('extension.xpi'),
-                                      [self.platform])
+            Addon.from_upload(self.get_upload('extension.xpi'),
+                              [self.platform])
         eq_(e.exception.messages, ['Duplicate UUID found.'])
 
     def test_xpi_attributes(self):

apps/devhub/templates/devhub/addons/listing/delete_form.html

@@ -1,14 +1,16 @@
 <form method="post" action="{{ url('devhub.addons.delete', addon.slug) }}">
   {{ csrf() }}
   <h3>{{ _('Delete Add-on') }}</h3>
-  <p>
-    {% trans %}
-      Deleting your add-on will permanently remove it from the site and
-      prevent its GUID from being submitted ever again, even by you.  The
-      existing users of your add-on will remain on this update channel and
-      never receive updates again.
-    {% endtrans %}
-  </p>
+  {% if addon.highest_status %}
+    <p>
+      {% trans %}
+        Deleting your add-on will permanently remove it from the site and
+        prevent its GUID from being submitted ever again, even by you.  The
+        existing users of your add-on will remain on this update channel and
+        never receive updates again.
+      {% endtrans %}
+    </p>
+  {% endif %}
   <p>
     <label>
       {{ _('Enter your password to confirm your decision:') }}

apps/devhub/tests/test_views.py

@@ -1976,6 +1976,28 @@ def test_no_validation_results(self):
             reverse('devhub.file_validation',
                     args=[self.addon.slug, self.version.all_files[0].id]))
 
+    def test_delete_message(self):
+        """Make sure we warn our users of the pain they will feel."""
+        r = self.client.get(self.url)
+        doc = pq(r.content)
+        eq_(doc('#modal-delete p').eq(0).text(),
+            'Deleting your add-on will permanently remove it from the site '
+            'and prevent its GUID from being submitted ever again, even by '
+            'you. The existing users of your add-on will remain on this '
+            'update channel and never receive updates again.')
+
+    def test_delete_message_incomplete(self):
+        """
+        If an addon has highest_status = 0, they shouldn't be bothered with a
+        blacklisting threat if they hit delete.
+        """
+        self.addon.highest_status = amo.STATUS_NULL
+        self.addon.save()
+        r = self.client.get(self.url)
+        doc = pq(r.content)
+        # Normally 2 paragraphs, one is the warning which we should take out.
+        eq_(len(doc('#modal-delete p')), 1, 'We might be lying to our users.')
+
     def test_delete_version(self):
         self.client.post(self.delete_url, self.delete_data)
         assert not Version.objects.filter(pk=81551).exists()