Many signoff page improvements

Add a new 'SignoffSpecification' model which will capture metadata
regarding a specific package if it differs from the norm- e.g. more or
less than 2 required signoffs, is known to be bad, a comment from the
maintainer, etc. The groundwork is laid here; much of this will still
need to be wired up in the future.

Enhance the view with a lot more JS prettiness and add revoking of
signoffs. The signoff page can be filtered and the links and all the fun
stuff are totally dynamic now.

Signed-off-by: Dan McGee <dan@archlinux.org>

10 files changed, 379 insertions, 69 deletions

diff --git a/main/models.py b/main/models.py
index 780453c0..d55a9673 100644
--- a/main/models.py
+++ b/main/models.py
@@ -7,7 +7,6 @@ from django.forms import ValidationError
 
 from main.utils import cache_function, make_choice, set_created_field
 from packages.models import PackageRelation
-from packages.models import Signoff as PackageSignoff
 
 from datetime import datetime
 from itertools import groupby
@@ -213,16 +212,6 @@ class Package(models.Model):
                 package_relations__pkgbase=self.pkgbase,
                 package_relations__type=PackageRelation.MAINTAINER)
 
-    @property
-    def signoffs(self):
-        return PackageSignoff.objects.select_related('user').filter(
-            pkgbase=self.pkgbase, pkgver=self.pkgver, pkgrel=self.pkgrel,
-            epoch=self.epoch, arch=self.arch, repo=self.repo)
-
-    def approved_for_signoff(self):
-        count = self.signoffs.filter(revoked__isnull=True).count()
-        return count >= PackageSignoff.REQUIRED
-
     @cache_function(300)
     def applicable_arches(self):
         '''The list of (this arch) + (available agnostic arches).'''
diff --git a/media/archweb.css b/media/archweb.css
index ea2f3fb5..62dc4fbc 100644
--- a/media/archweb.css
+++ b/media/archweb.css
@@ -912,7 +912,6 @@ ul.admin-actions {
 
 #dev-signoffs .signed-username {
     color: #888;
-    margin-left: 0.5em;
 }
 
 /* iso testing feedback form */
diff --git a/media/archweb.js b/media/archweb.js
index a51ae460..43812b33 100644
--- a/media/archweb.js
+++ b/media/archweb.js
@@ -139,7 +139,7 @@ function ajaxifyFiles() {
 
 /* packages/differences.html */
 function filter_packages() {
-    // start with all rows, and then remove ones we shouldn't show
+    /* start with all rows, and then remove ones we shouldn't show */
     var rows = $('#tbody_differences').children();
     var all_rows = rows;
     if (!$('#id_multilib').is(':checked')) {
@@ -150,12 +150,12 @@ function filter_packages() {
         rows = rows.filter('.' + arch);
     }
     if (!$('#id_minor').is(':checked')) {
-        // this check is done last because it is the most expensive
+        /* this check is done last because it is the most expensive */
         var pat = /(.*)-(.+)/;
         rows = rows.filter(function(index) {
             var cells = $(this).children('td');
 
-            // all this just to get the split version out of the table cell
+            /* all this just to get the split version out of the table cell */
             var ver_a = cells.eq(2).find('span').text().match(pat);
             if (!ver_a) {
                 return true;
@@ -166,26 +166,26 @@ function filter_packages() {
                 return true;
             }
 
-            // first check pkgver
+            /* first check pkgver */
             if (ver_a[1] !== ver_b[1]) {
                 return true;
             }
-            // pkgver matched, so see if rounded pkgrel matches
+            /* pkgver matched, so see if rounded pkgrel matches */
             if (Math.floor(parseFloat(ver_a[2])) ===
                     Math.floor(parseFloat(ver_b[2]))) {
                 return false;
             }
-            // pkgrel didn't match, so keep the row
+            /* pkgrel didn't match, so keep the row */
             return true;
         });
     }
-    // hide all rows, then show the set we care about
+    /* hide all rows, then show the set we care about */
     all_rows.hide();
     rows.show();
-    // make sure we update the odd/even styling from sorting
+    /* make sure we update the odd/even styling from sorting */
     $('.results').trigger('applyWidgets');
 }
-function filter_reset() {
+function filter_packages_reset() {
     $('#id_archonly').val('both');
     $('#id_multilib').removeAttr('checked');
     $('#id_minor').removeAttr('checked');
@@ -213,26 +213,72 @@ function todolist_flag() {
 function signoff_package() {
     var link = this;
     $.getJSON(link.href, function(data) {
+        link = $(link);
+        var signoff = null;
         if (data.created) {
-            var signoff = $('<li>').addClass('signed-username').text(data.user);
-            $(link).append(signoff);
+            signoff = $('<li>').addClass('signed-username').text(data.user);
+            link.closest('td').children('ul').append(signoff);
+        } else if(data.user) {
+            signoff = link.closest('td').find('li').filter(function(index) {
+                return $(this).text() == data.user;
+            });
+        }
+        console.log(signoff, data.revoked, data.user);
+        if (signoff && data.revoked) {
+            signoff.text(signoff.text() + ' (revoked)');
         }
         /* update the approved column to reflect reality */
         var approved;
         if (data.approved) {
-            approved = $(link).closest('tr').children('.signoff-no');
+            approved = link.closest('tr').children('.signoff-no');
             approved.text('Yes').addClass(
                 'signoff-yes').removeClass('signoff-no');
         } else {
-            approved = $(link).closest('tr').children('.signoff-yes');
+            approved = link.closest('tr').children('.signoff-yes');
             approved.text('No').addClass(
                 'signoff-no').removeClass('signoff-yes');
         }
+        link.removeAttr('title');
+        /* Form our new link. The current will be something like
+         * '/packages/repo/arch/package/...' */
+        var base_href = link.attr('href').split('/').slice(0, 5).join('/');
+        if (data.revoked) {
+            link.text('Signoff');
+            link.attr('href', base_href + '/signoff/');
+        } else {
+            link.text('Revoke Signoff');
+            link.attr('href', base_href + '/signoff/revoke/');
+        }
         $('.results').trigger('updateCell', approved);
     });
     return false;
 }
 
+function filter_signoffs() {
+    /* start with all rows, and then remove ones we shouldn't show */
+    var rows = $('#tbody_signoffs').children();
+    var all_rows = rows;
+    $('#signoffs_filter .arch_filter').each(function() {
+        if (!$(this).is(':checked')) {
+            console.log($(this).val());
+            rows = rows.not('.' + $(this).val());
+        }
+    });
+    if ($('#id_pending').is(':checked')) {
+        rows = rows.has('td.signoff-no');
+    }
+    /* hide all rows, then show the set we care about */
+    all_rows.hide();
+    rows.show();
+    /* make sure we update the odd/even styling from sorting */
+    $('.results').trigger('applyWidgets');
+}
+function filter_signoffs_reset() {
+    $('#signoffs_filter .arch_filter').attr('checked', 'checked');
+    $('#id_pending').removeAttr('checked');
+    filter_signoffs();
+}
+
 /* visualizations */
 function format_filesize(size, decimals) {
     /*var labels = ['B', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB'];*/
diff --git a/packages/migrations/0010_auto__add_signoffspecification.py b/packages/migrations/0010_auto__add_signoffspecification.py
new file mode 100644
index 00000000..da24824e
--- /dev/null
+++ b/packages/migrations/0010_auto__add_signoffspecification.py
@@ -0,0 +1,183 @@
+# 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.create_table('packages_signoffspecification', (
+            ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)),
+            ('pkgbase', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)),
+            ('pkgver', self.gf('django.db.models.fields.CharField')(max_length=255)),
+            ('pkgrel', self.gf('django.db.models.fields.CharField')(max_length=255)),
+            ('epoch', self.gf('django.db.models.fields.PositiveIntegerField')(default=0)),
+            ('arch', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['main.Arch'])),
+            ('repo', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['main.Repo'])),
+            ('user', self.gf('django.db.models.fields.related.ForeignKey')(to=orm['auth.User'])),
+            ('created', self.gf('django.db.models.fields.DateTimeField')()),
+            ('required', self.gf('django.db.models.fields.PositiveIntegerField')(default=2)),
+            ('enabled', self.gf('django.db.models.fields.BooleanField')(default=True)),
+            ('known_bad', self.gf('django.db.models.fields.BooleanField')(default=False)),
+            ('comments', self.gf('django.db.models.fields.TextField')(null=True, blank=True)),
+        ))
+        db.send_create_signal('packages', ['SignoffSpecification'])
+
+
+    def backwards(self, orm):
+        db.delete_table('packages_signoffspecification')
+
+
+    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'})
+        },
+        'main.arch': {
+            'Meta': {'ordering': "['name']", 'object_name': 'Arch', 'db_table': "'arches'"},
+            'agnostic': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'})
+        },
+        'main.package': {
+            'Meta': {'ordering': "('pkgname',)", 'object_name': 'Package', 'db_table': "'packages'"},
+            'arch': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'packages'", 'to': "orm['main.Arch']"}),
+            'build_date': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'compressed_size': ('main.models.PositiveBigIntegerField', [], {}),
+            'epoch': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
+            'filename': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'files_last_update': ('django.db.models.fields.DateTimeField', [], {'null': 'True', 'blank': 'True'}),
+            'flag_date': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'installed_size': ('main.models.PositiveBigIntegerField', [], {}),
+            'last_update': ('django.db.models.fields.DateTimeField', [], {}),
+            'packager': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True'}),
+            'packager_str': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'pgp_signature': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'pkgbase': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkgdesc': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True'}),
+            'pkgname': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkgrel': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'pkgver': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'repo': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'packages'", 'to': "orm['main.Repo']"}),
+            'url': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True'})
+        },
+        'main.repo': {
+            'Meta': {'ordering': "['name']", 'object_name': 'Repo', 'db_table': "'repos'"},
+            'bugs_category': ('django.db.models.fields.SmallIntegerField', [], {'default': '0'}),
+            'bugs_project': ('django.db.models.fields.SmallIntegerField', [], {'default': '1'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
+            'staging': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'svn_root': ('django.db.models.fields.CharField', [], {'max_length': '64'}),
+            'testing': ('django.db.models.fields.BooleanField', [], {'default': 'False'})
+        },
+        'packages.conflict': {
+            'Meta': {'ordering': "['name']", 'object_name': 'Conflict'},
+            'comparison': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkg': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'conflicts'", 'to': "orm['main.Package']"}),
+            'version': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'})
+        },
+        'packages.license': {
+            'Meta': {'ordering': "['name']", 'object_name': 'License'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'pkg': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'licenses'", 'to': "orm['main.Package']"})
+        },
+        'packages.packagegroup': {
+            'Meta': {'object_name': 'PackageGroup'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkg': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'groups'", 'to': "orm['main.Package']"})
+        },
+        'packages.packagerelation': {
+            'Meta': {'unique_together': "(('pkgbase', 'user', 'type'),)", 'object_name': 'PackageRelation'},
+            'created': ('django.db.models.fields.DateTimeField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'pkgbase': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'type': ('django.db.models.fields.PositiveIntegerField', [], {'default': '1'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'package_relations'", 'to': "orm['auth.User']"})
+        },
+        'packages.provision': {
+            'Meta': {'ordering': "['name']", 'object_name': 'Provision'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkg': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'provides'", 'to': "orm['main.Package']"}),
+            'version': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'})
+        },
+        'packages.replacement': {
+            'Meta': {'ordering': "['name']", 'object_name': 'Replacement'},
+            'comparison': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkg': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'replaces'", 'to': "orm['main.Package']"}),
+            'version': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '255'})
+        },
+        'packages.signoff': {
+            'Meta': {'object_name': 'Signoff'},
+            'arch': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['main.Arch']"}),
+            'comments': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {}),
+            'epoch': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'pkgbase': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkgrel': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'pkgver': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'repo': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['main.Repo']"}),
+            'revoked': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'package_signoffs'", 'to': "orm['auth.User']"})
+        },
+        'packages.signoffspecification': {
+            'Meta': {'object_name': 'SignoffSpecification'},
+            'arch': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['main.Arch']"}),
+            'comments': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {}),
+            'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'epoch': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'known_bad': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'pkgbase': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'pkgrel': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'pkgver': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'repo': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['main.Repo']"}),
+            'required': ('django.db.models.fields.PositiveIntegerField', [], {'default': '2'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        }
+    }
+
+    complete_apps = ['packages']
diff --git a/packages/models.py b/packages/models.py
index 4cd3b1b5..ad082501 100644
--- a/packages/models.py
+++ b/packages/models.py
@@ -38,6 +38,49 @@ class PackageRelation(models.Model):
     class Meta:
         unique_together = (('pkgbase', 'user', 'type'),)
 
+class SignoffSpecification(models.Model):
+    '''
+    A specification for the signoff policy for this particular revision of a
+    pakcage. The default is requiring two signoffs for a given package. These
+    are created only if necessary; e.g., if one wanted to override the
+    required=2 attribute, otherwise a sane default object is used.
+    '''
+    pkgbase = models.CharField(max_length=255, db_index=True)
+    pkgver = models.CharField(max_length=255)
+    pkgrel = models.CharField(max_length=255)
+    epoch = models.PositiveIntegerField(default=0)
+    arch = models.ForeignKey('main.Arch')
+    repo = models.ForeignKey('main.Repo')
+    user = models.ForeignKey(User)
+    created = models.DateTimeField(editable=False)
+    required = models.PositiveIntegerField(default=2)
+    enabled = models.BooleanField(default=True)
+    known_bad = models.BooleanField(default=False)
+    comments = models.TextField(null=True, blank=True)
+
+class SignoffManager(models.Manager):
+    def get_from_package(self, pkg, user, revoked=False):
+        '''Utility method to pull all relevant name-version fields from a
+        package and create a matching signoff.'''
+        not_revoked = not revoked
+        return Signoff.objects.get(
+                pkgbase=pkg.pkgbase, pkgver=pkg.pkgver, pkgrel=pkg.pkgrel,
+                epoch=pkg.epoch, arch=pkg.arch, repo=pkg.repo,
+                revoked__isnull=not_revoked, user=user)
+
+    def get_or_create_from_package(self, pkg, user):
+        '''Utility method to pull all relevant name-version fields from a
+        package and create a matching signoff.'''
+        return Signoff.objects.get_or_create(
+                pkgbase=pkg.pkgbase, pkgver=pkg.pkgver, pkgrel=pkg.pkgrel,
+                epoch=pkg.epoch, arch=pkg.arch, repo=pkg.repo,
+                revoked=None, user=user)
+
+    def for_package(self, pkg):
+        return self.select_related('user').filter(
+                pkgbase=pkg.pkgbase, pkgver=pkg.pkgver, pkgrel=pkg.pkgrel,
+                epoch=pkg.epoch, arch=pkg.arch, repo=pkg.repo)
+
 class Signoff(models.Model):
     '''
     A signoff for a package (by pkgbase) at a given point in time. These are
@@ -55,7 +98,7 @@ class Signoff(models.Model):
     revoked = models.DateTimeField(null=True)
     comments = models.TextField(null=True, blank=True)
 
-    REQUIRED = 2
+    objects = SignoffManager()
 
     @property
     def packages(self):
diff --git a/packages/urls.py b/packages/urls.py
index d7d01170..576e3279 100644
--- a/packages/urls.py
+++ b/packages/urls.py
@@ -10,6 +10,7 @@ package_patterns = patterns('packages.views',
     (r'^unflag/$',     'unflag'),
     (r'^unflag/all/$', 'unflag_all'),
     (r'^signoff/$',    'signoff_package'),
+    (r'^signoff/revoke/$', 'signoff_package', {'revoke': True}),
     (r'^download/$',   'download'),
 )
 
diff --git a/packages/views.py b/packages/views.py
index 5114c87f..035d51cb 100644
--- a/packages/views.py
+++ b/packages/views.py
@@ -25,7 +25,7 @@ from urllib import urlencode
 from main.models import Package, PackageFile, Arch, Repo
 from main.utils import make_choice, groupby_preserve_order, PackageStandin
 from mirrors.models import MirrorUrl
-from .models import PackageRelation, PackageGroup, Signoff
+from .models import PackageRelation, PackageGroup, SignoffSpecification, Signoff
 from .utils import (get_group_info, get_differences_info,
         get_wrong_permissions, get_current_signoffs)
 
@@ -369,14 +369,24 @@ def unflag_all(request, name, repo, arch):
     pkgs.update(flag_date=None)
     return redirect(pkg)
 
+DEFAULT_SIGNOFF_SPEC = SignoffSpecification(required=2)
+
+def approved_by_signoffs(signoffs, spec=DEFAULT_SIGNOFF_SPEC):
+    if signoffs:
+        good_signoffs = sum(1 for s in signoffs if not s.revoked)
+        return good_signoffs >= spec.required
+    return False
+
 class PackageSignoffGroup(object):
     '''Encompasses all packages in testing with the same pkgbase.'''
-    def __init__(self, packages, target_repo=None, signoffs=None):
+    def __init__(self, packages, user=None):
         if len(packages) == 0:
             raise Exception
         self.packages = packages
-        self.target_repo = target_repo
-        self.signoffs = signoffs
+        self.user = user
+        self.target_repo = None
+        self.signoffs = set()
+        self.specification = DEFAULT_SIGNOFF_SPEC
 
         first = packages[0]
         self.pkgbase = first.pkgbase
@@ -406,21 +416,24 @@ class PackageSignoffGroup(object):
     def find_signoffs(self, all_signoffs):
         '''Look through a list of Signoff objects for ones matching this
         particular group and store them on the object.'''
-        if self.signoffs is None:
-            self.signoffs = []
         for s in all_signoffs:
             if s.pkgbase != self.pkgbase:
                 continue
             if self.version and not s.full_version == self.version:
                 continue
             if s.arch_id == self.arch.id and s.repo_id == self.repo.id:
-                self.signoffs.append(s)
+                self.signoffs.add(s)
 
     def approved(self):
-        if self.signoffs:
-            good_signoffs = [s for s in self.signoffs if not s.revoked]
-            return len(good_signoffs) >= Signoff.REQUIRED
-        return False
+        return approved_by_signoffs(self.signoffs, self.specification)
+
+    def user_signed_off(self, user=None):
+        '''Did a given user signoff on this package? user can be passed as an
+        argument, or attached to the group object itself so this can be called
+        from a template.'''
+        if user is None:
+            user = self.user
+        return user in (s.user for s in self.signoffs if not s.revoked)
 
 @permission_required('main.change_package')
 @never_cache
@@ -443,7 +456,7 @@ def signoffs(request):
     grouped = groupby_preserve_order(packages, same_pkgbase_key)
     signoff_groups = []
     for group in grouped:
-        signoff_group = PackageSignoffGroup(group)
+        signoff_group = PackageSignoffGroup(group, user=request.user)
         signoff_group.target_repo = pkgtorepo.get(signoff_group.pkgbase,
                 "Unknown")
         signoff_group.find_signoffs(signoffs)
@@ -451,27 +464,43 @@ def signoffs(request):
 
     signoff_groups.sort(key=attrgetter('pkgbase'))
 
-    return direct_to_template(request, 'packages/signoffs.html',
-            {'signoff_groups': signoff_groups})
+    context = {
+        'signoff_groups': signoff_groups,
+        'arches': Arch.objects.all(),
+    }
+    return direct_to_template(request, 'packages/signoffs.html', context)
 
 @permission_required('main.change_package')
 @never_cache
-def signoff_package(request, name, repo, arch):
+def signoff_package(request, name, repo, arch, revoke=False):
     packages = get_list_or_404(Package, pkgbase=name,
             arch__name=arch, repo__name__iexact=repo, repo__testing=True)
 
-    pkg = packages[0]
-    signoff, created = Signoff.objects.get_or_create(
-            pkgbase=pkg.pkgbase, pkgver=pkg.pkgver, pkgrel=pkg.pkgrel,
-            epoch=pkg.epoch, arch=pkg.arch, repo=pkg.repo, user=request.user)
+    package = packages[0]
+
+    if revoke:
+        try:
+            signoff = Signoff.objects.get_from_package(
+                    package, request.user, False)
+        except Signoff.DoesNotExist:
+            raise Http404
+        signoff.revoked = datetime.utcnow()
+        signoff.save()
+        created = False
+    else:
+        signoff, created = Signoff.objects.get_or_create_from_package(
+                package, request.user)
+
+    all_signoffs = Signoff.objects.for_package(package)
 
     if request.is_ajax():
         data = {
             'created': created,
-            'approved': pkg.approved_for_signoff(),
+            'revoked': bool(signoff.revoked),
+            'approved': approved_by_signoffs(all_signoffs),
             'user': str(request.user),
         }
-        return HttpResponse(simplejson.dumps(data),
+        return HttpResponse(simplejson.dumps(data, ensure_ascii=False),
                 mimetype='application/json')
 
     return redirect('package-signoffs')
diff --git a/templates/packages/differences.html b/templates/packages/differences.html
index dd1046bc..0400ea37 100644
--- a/templates/packages/differences.html
+++ b/templates/packages/differences.html
@@ -65,7 +65,7 @@ $(document).ready(function() {
     $('.results').tablesorter({widgets: ['zebra'], sortList: [[1,0], [0,0]]});
     $('#diff_filter select').change(filter_packages);
     $('#diff_filter input').change(filter_packages);
-    $('#criteria_reset').click(filter_reset);
+    $('#criteria_reset').click(filter_differences_reset);
     // fire function on page load to ensure the current form selections take effect
     filter_packages();
 });
diff --git a/templates/packages/signoff_cell.html b/templates/packages/signoff_cell.html
new file mode 100644
index 00000000..fce5d551
--- /dev/null
+++ b/templates/packages/signoff_cell.html
@@ -0,0 +1,12 @@
+<ul>
+    {% for signoff in group.signoffs %}
+    <li class="signed-username" title="Signed off by {{ signoff.user }}">{{ signoff.user }}{% if signoff.revoked %} (revoked){% endif %}</li>
+    {% endfor %}
+</ul>
+{% if group.user_signed_off %}
+<div><a class="signoff-link" href="{{ group.package.get_absolute_url }}signoff/revoke/"
+        title="Revoke signoff {{ group.package.pkgname }} for {{ group.package.arch }}">Revoke Signoff</a></div>
+{% else %}
+<div><a class="signoff-link" href="{{ group.package.get_absolute_url }}signoff/"
+        title="Signoff {{ group.package.pkgname }} for {{ group.package.arch }}">Signoff</a></div>
+{% endif %}
diff --git a/templates/packages/signoffs.html b/templates/packages/signoffs.html
index a8aa4de2..4a2f6c99 100644
--- a/templates/packages/signoffs.html
+++ b/templates/packages/signoffs.html
@@ -12,42 +12,46 @@
     <p>{{ signoff_groups|length }} signoff group{{ signoff_groups|pluralize }} found.
     A "signoff group" consists of packages grouped by pkgbase, architecture, and repository.</p>
 
+    <div class="box filter-criteria">
+        <h3>Filter Displayed Signoffs</h3>
+        <form id="signoffs_filter" method="post" action=".">
+        <fieldset>
+            <legend>Select filter criteria</legend>
+            {% for arch in arches %}
+            <div><label for="id_arch_{{ arch.name }}" title="Architecture {{ arch.name }}">Arch {{ arch.name }}</label>
+                <input type="checkbox" name="arch_{{ arch.name }}" id="id_arch_{{ arch.name }}" class="arch_filter" value="{{ arch.name }}" checked="checked"/></div>
+            {% endfor %}
+            <div><label for="id_pending" title="Packages with not enough signoffs">Only Pending Approval</label>
+                <input type="checkbox" name="pending" id="id_pending" value="pending"/></div>
+            <div ><label>&nbsp;</label><input title="Reset search criteria" type="button" id="criteria_reset" value="Reset"/></div>
+        </fieldset>
+        </form>
+    </div>
+
     <table id="signoffs" class="results">
         <thead>
             <tr>
+                <th>Package Base/Version</th>
                 <th>Arch</th>
-                <th>Package Base</th>
+                <th>Target Repo</th>
                 <th># of Packages</th>
-                <th>Version</th>
                 <th>Last Updated</th>
-                <th>Target Repo</th>
                 <th>Approved</th>
-                <th>Signoff</th>
+                <th>Signoffs</th>
             </tr>
         </thead>
-        <tbody>
+        <tbody id="tbody_signoffs">
             {% for group in signoff_groups %}
             {% with group.package as pkg %}
-            <tr class="{% cycle 'odd' 'even' %}">
+            <tr class="{% cycle 'odd' 'even' %} {{ pkg.arch.name }}">
+                <td>{% pkg_details_link pkg %} {{ pkg.full_version }}</td>
                 <td>{{ pkg.arch.name }}</td>
-                <td>{% pkg_details_link pkg %}</td>
+                <td>{{ group.target_repo }}</td>
                 <td>{{ group.packages|length }}</td>
-                <td>{{ pkg.full_version }}</td>
                 <td>{{ pkg.last_update|date }}</td>
-                <td>{{ group.target_repo }}</td>
                 <td class="signoff-{{ group.approved|yesno }}">
                     {{ group.approved|yesno|capfirst }}</td>
-                <td>
-                    <ul>
-                        <li><a class="signoff-link" href="{{ pkg.get_absolute_url }}signoff/"
-                            title="Signoff {{ pkg.pkgname }} for {{ pkg.arch }}">Signoff</a>
-                        </li>
-                        {% for signoff in group.signoffs %}
-                        <li class="signed-username" title="Signed off by {{ signoff.user }}">
-                        {{ signoff.user }}{% if signoff.revoked %} (revoked){% endif %}</li>
-                        {% endfor %}
-                    </ul>
-                </td>
+                <td>{% include "packages/signoff_cell.html" %}</td>
             </tr>
             {% endwith %}
             {% endfor %}
@@ -60,8 +64,12 @@
 <script type="text/javascript">
 $(document).ready(function() {
     $('a.signoff-link').click(signoff_package);
-    $(".results").tablesorter({widgets: ['zebra'], sortList: [[1,0]],
+    $(".results").tablesorter({widgets: ['zebra'], sortList: [[0,0]],
         headers: { 6: { sorter: false } } });
+    $('#signoffs_filter input').change(filter_signoffs);
+    $('#criteria_reset').click(filter_signoffs_reset);
+    // fire function on page load to ensure the current form selections take effect
+    filter_signoffs();
 });
 </script>
 {% endblock %}