GNOME.org
 

[extensions-web] review: Automatically approve harmless extension versions

commit d1b70ccd4c2a595d581c461376f9d3c3c4b5c8c6
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Mon Apr 2 00:05:09 2012 -0400

    review: Automatically approve harmless extension versions
    
    When a new extension version is uploaded, we diff it against the last
    approved version -- if the changes to its files are "safe", we automatically
    approve the new version.
    
    Currently, safe changes are defined by filenames and file types -- any
    updates to images, translation files, or metadata.json is considered safe,
    and more may be added to the list in the future.
    
    A similar feature is planned that checks for future 'safe' versions
    upon approval of other versions of the same extension.

 ...uto__add_field_changestatuslog_auto_approved.py |  110 ++++++++++++++++++++
 sweettooth/review/models.py                        |    1 +
 .../review/templates/review/auto_approved_mail.txt |   13 +++
 .../review/auto_approved_mail_subject.txt          |    1 +
 sweettooth/review/tests.py                         |   20 ++++-
 sweettooth/review/views.py                         |   89 ++++++++++++++--
 6 files changed, 222 insertions(+), 12 deletions(-)
---
diff --git a/sweettooth/review/migrations/0005_auto__add_field_changestatuslog_auto_approved.py b/sweettooth/review/migrations/0005_auto__add_field_changestatuslog_auto_approved.py
new file mode 100644
index 0000000..6eeb2da
--- /dev/null
+++ b/sweettooth/review/migrations/0005_auto__add_field_changestatuslog_auto_approved.py
@@ -0,0 +1,110 @@
+# 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):
+        
+        # Adding field 'ChangeStatusLog.auto_approved'
+        db.add_column('review_changestatuslog', 'auto_approved', self.gf('django.db.models.fields.BooleanField')(default=False), keep_default=False)
+
+
+    def backwards(self, orm):
+        
+        # Deleting field 'ChangeStatusLog.auto_approved'
+        db.delete_column('review_changestatuslog', '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_approved': ('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 2c49f7e..25e7be6 100644
--- a/sweettooth/review/models.py
+++ b/sweettooth/review/models.py
@@ -30,6 +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)
 
     def get_newstatus_class(self):
         return STATUSES[self.newstatus].lower()
diff --git a/sweettooth/review/templates/review/auto_approved_mail.txt b/sweettooth/review/templates/review/auto_approved_mail.txt
new file mode 100644
index 0000000..a11ea07
--- /dev/null
+++ b/sweettooth/review/templates/review/auto_approved_mail.txt
@@ -0,0 +1,13 @@
+A new extension version, "{{ extension.name }}", version {{ version.version }}, was auto-approved.
+
+The diff between this version and the last approved version contained these changed files:
+{% for file in changes %}
+  * {{ file }}
+{% endfor %}
+
+The full review details can be found at {{ review_url }}, and the extension's version page
+can be found at {{ version_url }}.
+
+â
+
+This email was sent automatically by GNOME Shell Extensions. Do not reply.
diff --git a/sweettooth/review/templates/review/auto_approved_mail_subject.txt b/sweettooth/review/templates/review/auto_approved_mail_subject.txt
new file mode 100644
index 0000000..48dc9d6
--- /dev/null
+++ b/sweettooth/review/templates/review/auto_approved_mail_subject.txt
@@ -0,0 +1 @@
+GNOME Shell Extensions â Auto-approved: "{{ extension.name }}", v{{ version.version }}
diff --git a/sweettooth/review/tests.py b/sweettooth/review/tests.py
index 7313da3..8ea8eb3 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
+from review.views import get_old_version, safe_to_auto_approve
 
 from testutils import BasicUserTestCase
 
@@ -28,3 +28,21 @@ class DiffViewTest(BasicUserTestCase, TestCase):
                                                           source=File(ContentFile("doot doo"), name="bb"),
                                                           status=models.STATUS_NEW)
         self.assertEquals(version1, get_old_version(version3))
+
+class TestAutoApproveLogic(TestCase):
+    def build_changeset(self, added=None, deleted=None, changed=None, unchanged=None):
+        return dict(added=added or [],
+                    deleted=deleted or [],
+                    changed=changed or [],
+                    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'])))
diff --git a/sweettooth/review/views.py b/sweettooth/review/views.py
index c26f2cd..a36ab02 100644
--- a/sweettooth/review/views.py
+++ b/sweettooth/review/views.py
@@ -1,5 +1,6 @@
 
 import base64
+import itertools
 import os.path
 
 import pygments
@@ -104,6 +105,16 @@ def get_old_version(version):
 
     return old_version
 
+def get_latest_active_version(version):
+    extension = version.extension
+
+    try:
+        old_version = extension.versions.filter(version__lt=version.version, status__in=models.VISIBLE_STATUSES).latest()
+    except models.ExtensionVersion.DoesNotExist:
+        return None
+
+    return old_version
+
 def get_zipfiles(*args):
     for version in args:
         if version is None:
@@ -146,13 +157,7 @@ def get_fake_chunks(numlines, tag):
 def get_file_list(zipfile):
     return set(n for n in zipfile.namelist() if not n.endswith('/'))
 
- ajax_view
- model_view(models.ExtensionVersion)
-def ajax_get_file_list_view(request, obj):
-    version, extension = obj, obj.extension
-
-    old_zipfile, new_zipfile = get_zipfiles(version, get_old_version(version))
-
+def get_file_changeset(old_zipfile, new_zipfile):
     new_filelist = get_file_list(new_zipfile)
 
     if old_zipfile is None:
@@ -186,9 +191,13 @@ def ajax_get_file_list_view(request, obj):
 
 @ajax_view
 @model_view(models.ExtensionVersion)
-def ajax_get_file_diff_view(request, obj):
-    version, extension = obj, obj.extension
+def ajax_get_file_list_view(request, version):
+    old_zipfile, new_zipfile = get_zipfiles(get_old_version(version), version)
+    return get_file_changeset(old_zipfile, new_zipfile)
 
+ ajax_view
+ model_view(models.ExtensionVersion)
+def ajax_get_file_diff_view(request, version):
     filename = request.GET['filename']
 
     file_base, file_extension = os.path.splitext(filename)
@@ -315,7 +324,7 @@ def render_mail(template, data):
 
     return subject.strip(), body.strip()
 
-def send_email_on_submitted(sender, request, version, **kwargs):
+def send_email_submitted(request, version):
     extension = version.extension
 
     url = request.build_absolute_uri(reverse('review-version',
@@ -335,7 +344,65 @@ def send_email_on_submitted(sender, request, version, **kwargs):
     message = EmailMessage(subject=subject, body=body, to=recipient_list, headers=extra_headers)
     message.send()
 
-models.submitted_for_review.connect(send_email_on_submitted)
+def send_email_auto_approved(request, version, changeset):
+    extension = version.extension
+
+    review_url = request.build_absolute_uri(reverse('review-version',
+                                                    kwargs=dict(pk=version.pk)))
+    version_url = request.build_absolute_uri(version.get_absolute_url())
+
+    data = dict(version=version,
+                extension=extension,
+                changes=changeset['changed'],
+                version_url=version_url)
+
+    subject, body = render_mail('auto_approved', data)
+
+    recipient_list = list(get_all_reviewers().values_list('email', flat=True))
+    recipient_list.append(extension.creator.email)
+
+    extra_headers = {'X-SweetTooth-Purpose': 'AutoApproved',
+                     'X-SweetTooth-ExtensionCreator': extension.creator.username}
+
+    message = EmailMessage(subject=subject, body=body, to=recipient_list, headers=extra_headers)
+    message.send()
+
+def safe_to_auto_approve(changes):
+    for filename in itertools.chain(changes['changed'], changes['added']):
+        # metadata.json updates are safe.
+        if filename == 'metadata.json':
+            continue
+
+        name, ext = os.path.splitext(filename)
+
+        # Translations and stylesheet updates are safe.
+        if ext in set(['.mo', '.po', '.css']):
+            continue
+
+        # Image updates are safe.
+        if ext in IMAGE_TYPES:
+            continue
+
+        return False
+
+    return True
+
+def extension_submitted(sender, request, version, **kwargs):
+    old_zipfile, new_zipfile = get_zipfiles(get_latest_active_version(version), version)
+    changeset = get_file_changeset(old_zipfile, new_zipfile)
+
+    if safe_to_auto_approve(changeset):
+        ChangeStatusLog.objects.create(user=request.user,
+                                       version=version,
+                                       newstatus=models.STATUS_ACTIVE,
+                                       auto_approved=True)
+        version.status = models.STATUS_ACTIVE
+        version.save()
+        send_email_auto_approved(request, version, changeset)
+    else:
+        send_email_submitted(request, version)
+
+models.submitted_for_review.connect(extension_submitted)
 
 def send_email_on_reviewed(sender, request, version, review, **kwargs):
     extension = version.extension
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]