[extensions-web] review: Add auto-reject

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

commit 17be3abea3785e2e338856277095bb2af2e8ab19
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Wed Jun 6 01:13:51 2012 -0400

    review: Add auto-reject

 ...__rename_field_changestatuslog_auto_approved.py |  104 ++++++++++++++++++++
 sweettooth/review/models.py                        |    2 +-
 sweettooth/review/templates/review/review.html     |    2 +-
 sweettooth/review/tests.py                         |   59 +++++++++--
 sweettooth/review/views.py                         |   34 +++++-
 5 files changed, 183 insertions(+), 18 deletions(-)
---
diff --git a/sweettooth/review/migrations/0006_auto__rename_field_changestatuslog_auto_approved.py b/sweettooth/review/migrations/0006_auto__rename_field_changestatuslog_auto_approved.py
new file mode 100644
index 0000000..de4a13a
--- /dev/null
+++ b/sweettooth/review/migrations/0006_auto__rename_field_changestatuslog_auto_approved.py
@@ -0,0 +1,104 @@
+# encoding: utf-8
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        db.rename_column('review_changestatuslog', 'auto_approved', 'auto')
+
+    def backwards(self, orm):
+        db.rename_column('review_changestatuslog', 'auto', 'auto_approved')
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'extensions.extension': {
+            'Meta': {'object_name': 'Extension'},
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'creator': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}),
+            'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'downloads': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
+            'icon': ('django.db.models.fields.files.ImageField', [], {'default': "'/static/images/plugin.png'", 'max_length': '100', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '200'}),
+            'popularity': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'screenshot': ('sorl.thumbnail.fields.ImageField', [], {'max_length': '100', 'blank': 'True'}),
+            'slug': ('autoslug.fields.AutoSlugField', [], {'unique_with': '()', 'max_length': '50', 'populate_from': 'None', 'db_index': 'True'}),
+            'url': ('django.db.models.fields.URLField', [], {'max_length': '200', 'blank': 'True'}),
+            'uuid': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '200', 'db_index': 'True'})
+        },
+        'extensions.extensionversion': {
+            'Meta': {'unique_together': "(('extension', 'version'),)", 'object_name': 'ExtensionVersion'},
+            'extension': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'versions'", 'to': "orm['extensions.Extension']"}),
+            'extra_json_fields': ('django.db.models.fields.TextField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'shell_versions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['extensions.ShellVersion']", 'symmetrical': 'False'}),
+            'source': ('django.db.models.fields.files.FileField', [], {'max_length': '223'}),
+            'status': ('django.db.models.fields.PositiveIntegerField', [], {}),
+            'version': ('django.db.models.fields.IntegerField', [], {'default': '0'})
+        },
+        'extensions.shellversion': {
+            'Meta': {'object_name': 'ShellVersion'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'major': ('django.db.models.fields.PositiveIntegerField', [], {}),
+            'minor': ('django.db.models.fields.PositiveIntegerField', [], {}),
+            'point': ('django.db.models.fields.IntegerField', [], {})
+        },
+        'review.changestatuslog': {
+            'Meta': {'object_name': 'ChangeStatusLog'},
+            'auto': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'newstatus': ('django.db.models.fields.PositiveIntegerField', [], {}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}),
+            'version': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'status_log'", 'to': "orm['extensions.ExtensionVersion']"})
+        },
+        'review.codereview': {
+            'Meta': {'object_name': 'CodeReview'},
+            'changelog': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['review.ChangeStatusLog']", 'unique': 'True', 'null': 'True'}),
+            'comments': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'reviewer': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"}),
+            'version': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'reviews'", 'to': "orm['extensions.ExtensionVersion']"})
+        }
+    }
+
+    complete_apps = ['review']
diff --git a/sweettooth/review/models.py b/sweettooth/review/models.py
index 25e7be6..c6b80a5 100644
--- a/sweettooth/review/models.py
+++ b/sweettooth/review/models.py
@@ -30,7 +30,7 @@ class ChangeStatusLog(models.Model):
     date = models.DateTimeField(auto_now_add=True)
     newstatus = models.PositiveIntegerField(choices=STATUSES.items())
     version = models.ForeignKey(ExtensionVersion, related_name="status_log")
-    auto_approved = models.BooleanField(default=False)
+    auto = models.BooleanField(default=False)
 
     def get_newstatus_class(self):
         return STATUSES[self.newstatus].lower()
diff --git a/sweettooth/review/templates/review/review.html b/sweettooth/review/templates/review/review.html
index ca59090..83a4cf9 100644
--- a/sweettooth/review/templates/review/review.html
+++ b/sweettooth/review/templates/review/review.html
@@ -96,7 +96,7 @@
     <a class="review-author" href="{% url auth-profile user=review.reviewer.username %}">{{ review.reviewer }}</a>
     {% if review.changelog %}
     <span class="{{ review.changelog.get_newstatus_class }}">
-      {% if review.changelog.auto_approved %}auto-{% endif%}
+      {% if review.changelog.auto %}auto-{% endif%}
       {{ review.changelog.get_newstatus_display|lower }}
     </span>
     {% else %}
diff --git a/sweettooth/review/tests.py b/sweettooth/review/tests.py
index 8ea8eb3..5758a51 100644
--- a/sweettooth/review/tests.py
+++ b/sweettooth/review/tests.py
@@ -3,7 +3,7 @@ from django.test import TestCase
 from django.core.files.base import File, ContentFile, StringIO
 
 from extensions import models
-from review.views import get_old_version, safe_to_auto_approve
+from review.views import get_old_version, should_auto_approve, should_auto_reject
 
 from testutils import BasicUserTestCase
 
@@ -37,12 +37,51 @@ class TestAutoApproveLogic(TestCase):
                     unchanged=unchanged or [])
 
     def test_auto_approve_logic(self):
-        self.assertTrue(safe_to_auto_approve(self.build_changeset()))
-        self.assertTrue(safe_to_auto_approve(self.build_changeset(changed=['metadata.json'])))
-        self.assertTrue(safe_to_auto_approve(self.build_changeset(changed=['metadata.json', 'po/en_GB.po', 'images/new_fedora.png', 'stylesheet.css'])))
-        self.assertTrue(safe_to_auto_approve(self.build_changeset(changed=['stylesheet.css'], added=['po/zn_CH.po'])))
-
-        self.assertFalse(safe_to_auto_approve(self.build_changeset(changed=['extension.js'])))
-        self.assertFalse(safe_to_auto_approve(self.build_changeset(changed=['secret_keys.json'])))
-        self.assertFalse(safe_to_auto_approve(self.build_changeset(changed=['libbignumber/BigInteger.js'])))
-        self.assertFalse(safe_to_auto_approve(self.build_changeset(added=['libbignumber/BigInteger.js'])))
+        self.assertTrue(should_auto_approve(self.build_changeset()))
+        self.assertTrue(should_auto_approve(self.build_changeset(changed=['metadata.json'])))
+        self.assertTrue(should_auto_approve(self.build_changeset(changed=['metadata.json', 'po/en_GB.po', 'images/new_fedora.png', 'stylesheet.css'])))
+        self.assertTrue(should_auto_approve(self.build_changeset(changed=['stylesheet.css'], added=['po/zn_CH.po'])))
+
+        self.assertFalse(should_auto_approve(self.build_changeset(changed=['extension.js'])))
+        self.assertFalse(should_auto_approve(self.build_changeset(changed=['secret_keys.json'])))
+        self.assertFalse(should_auto_approve(self.build_changeset(changed=['libbignumber/BigInteger.js'])))
+        self.assertFalse(should_auto_approve(self.build_changeset(added=['libbignumber/BigInteger.js'])))
+
+class TestAutoReject(BasicUserTestCase, TestCase):
+    def test_shell_version_auto_reject(self):
+        metadata = {"name": "Test Metadata 10",
+                    "uuid": "test-10 mecheye net",
+                    "description": "Simple test metadata",
+                    "url": "http://test-metadata.gnome.org"}
+
+        extension = models.Extension.objects.create_from_metadata(metadata, creator=self.user)
+        version1 = models.ExtensionVersion.objects.create(extension=extension, status=models.STATUS_UNREVIEWED)
+        version1.parse_metadata_json({'shell-version': ["3.4", "3.2.1"]})
+
+        version2 = models.ExtensionVersion.objects.create(extension=extension, status=models.STATUS_UNREVIEWED)
+
+        version2.parse_metadata_json({'shell-version': ["3.4"]})
+        self.assertFalse(should_auto_reject(version1, version2))
+
+        version2.parse_metadata_json({'shell-version': ["3.2.1"]})
+        self.assertTrue(should_auto_reject(version1, version2))
+
+        # As this safely covers all of version 1's shell versions,
+        # it should be rejected
+        version2.parse_metadata_json({'shell-version': ["3.6"]})
+        self.assertTrue(should_auto_reject(version1, version2))
+
+    def test_existing_version_auto_reject(self):
+        metadata = {"name": "Test Metadata 11",
+                    "uuid": "test-11 mecheye net",
+                    "description": "Simple test metadata",
+                    "url": "http://test-metadata.gnome.org"}
+
+        extension = models.Extension.objects.create_from_metadata(metadata, creator=self.user)
+        version1 = models.ExtensionVersion.objects.create(extension=extension, status=models.STATUS_ACTIVE)
+        version1.parse_metadata_json({'shell-version': ["3.4", "3.2.1"]})
+
+        version2 = models.ExtensionVersion.objects.create(extension=extension, status=models.STATUS_UNREVIEWED)
+        version2.parse_metadata_json({'shell-version': ["3.4", "3.2.1"]})
+
+        self.assertFalse(should_auto_reject(version1, version2))
diff --git a/sweettooth/review/views.py b/sweettooth/review/views.py
index 51a35ac..53fe3a9 100644
--- a/sweettooth/review/views.py
+++ b/sweettooth/review/views.py
@@ -369,7 +369,7 @@ def send_email_auto_approved(request, version, changeset):
                                   'X-SweetTooth-ExtensionCreator': extension.creator.username})
     message.send()
 
-def safe_to_auto_approve(changes, extension=None):
+def should_auto_approve(changes, extension=None):
     # If a user can approve extensions, don't bother making him do so.
     if extension is not None and can_approve_extension(extension.creator, extension):
         return True
@@ -397,15 +397,24 @@ def safe_to_auto_approve(changes, extension=None):
 
     return True
 
+def should_auto_reject(old_version, new_version):
+    if old_version.status != models.STATUS_UNREVIEWED:
+        return False
+
+    old_svs = set(old_version.shell_versions.all())
+    new_svs = set(new_version.shell_versions.all())
+    return new_svs.issuperset(old_svs)
+
 def extension_submitted(sender, request, version, **kwargs):
-    old_zipfile, new_zipfile = get_zipfiles(get_latest_active_version(version), version)
+    old_version = get_latest_active_version(version)
+    old_zipfile, new_zipfile = get_zipfiles(old_version, version)
     changeset = get_file_changeset(old_zipfile, new_zipfile)
 
-    if old_zipfile is not None and safe_to_auto_approve(changeset, extension=version.extension):
+    if old_zipfile is not None and should_auto_approve(changeset, version.extension):
         log = ChangeStatusLog.objects.create(user=request.user,
                                              version=version,
                                              newstatus=models.STATUS_ACTIVE,
-                                             auto_approved=True)
+                                             auto=True)
         CodeReview.objects.create(version=version,
                                   reviewer=request.user,
                                   comments="",
@@ -413,8 +422,21 @@ def extension_submitted(sender, request, version, **kwargs):
         version.status = models.STATUS_ACTIVE
         version.save()
         send_email_auto_approved(request, version, changeset)
-    else:
-        send_email_submitted(request, version)
+        return
+
+    if should_auto_reject(old_version, version):
+        log = ChangeStatusLog.objects.create(user=request.user,
+                                             version=old_version,
+                                             newstatus=models.STATUS_REJECTED,
+                                             auto=True)
+        CodeReview.objects.create(version=old_version,
+                                  reviewer=request.user,
+                                  comments="",
+                                  changelog=log)
+        old_version.status = models.STATUS_REJECTED
+        version.save()
+
+    send_email_submitted(request, version)
 
 models.submitted_for_review.connect(extension_submitted)