Refactor more package signoff stuff

This sets up some shared utility code for use in a later package signoff
email report command.

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

5 files changed, 144 insertions, 113 deletions

diff --git a/packages/models.py b/packages/models.py
index ad082501..3c319fe7 100644
--- a/packages/models.py
+++ b/packages/models.py
@@ -115,8 +115,11 @@ class Signoff(models.Model):
         return u'%s-%s' % (self.pkgver, self.pkgrel)
 
     def __unicode__(self):
-        return u'%s-%s: %s' % (
-                self.pkgbase, self.full_version, self.user)
+        revoked = u''
+        if self.revoked:
+            revoked = u' (revoked)'
+        return u'%s-%s: %s%s' % (
+                self.pkgbase, self.full_version, self.user, revoked)
 
 class PackageGroup(models.Model):
     '''
diff --git a/packages/utils.py b/packages/utils.py
index c8c1f8a6..42cfbe0f 100644
--- a/packages/utils.py
+++ b/packages/utils.py
@@ -1,12 +1,11 @@
-from collections import defaultdict
 from operator import itemgetter
 
 from django.db import connection
 from django.db.models import Count, Max
 
-from main.models import Package
-from main.utils import cache_function
-from .models import PackageGroup, PackageRelation, Signoff
+from main.models import Package, Repo
+from main.utils import cache_function, groupby_preserve_order, PackageStandin
+from .models import PackageGroup, PackageRelation, SignoffSpecification, Signoff
 
 @cache_function(300)
 def get_group_info(include_arches=None):
@@ -148,8 +147,90 @@ SELECT DISTINCT id
             id__in=to_fetch)
     return relations
 
-def get_current_signoffs():
-    '''Returns a mapping of pkgbase -> signoff objects.'''
+
+DEFAULT_SIGNOFF_SPEC = SignoffSpecification()
+
+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, user=None):
+        if len(packages) == 0:
+            raise Exception
+        self.packages = packages
+        self.user = user
+        self.target_repo = None
+        self.signoffs = set()
+        self.specification = DEFAULT_SIGNOFF_SPEC
+
+        first = packages[0]
+        self.pkgbase = first.pkgbase
+        self.arch = first.arch
+        self.repo = first.repo
+        self.version = ''
+        self.last_update = first.last_update
+        self.packager = first.packager
+
+        version = first.full_version
+        if all(version == pkg.full_version for pkg in packages):
+            self.version = version
+
+    @property
+    def package(self):
+        '''Try and return a relevant single package object representing this
+        group. Start by seeing if there is only one package, then look for the
+        matching package by name, finally falling back to a standin package
+        object.'''
+        if len(self.packages) == 1:
+            return self.packages[0]
+
+        same_pkgs = [p for p in self.packages if p.pkgname == p.pkgbase]
+        if same_pkgs:
+            return same_pkgs[0]
+
+        return PackageStandin(self.packages[0])
+
+    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.'''
+        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.add(s)
+
+    def approved(self):
+        return approved_by_signoffs(self.signoffs, self.specification)
+
+    @property
+    def completed(self):
+        return sum(1 for s in self.signoffs if not s.revoked)
+
+    @property
+    def required(self):
+        return self.specification.required
+
+    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)
+
+    def __unicode__(self):
+        return u'%s-%s (%s): %d' % (
+                self.pkgbase, self.version, self.arch, len(self.signoffs))
+
+def get_current_signoffs(repos):
+    '''Returns a mapping of pkgbase -> signoff objects for the given repos.'''
+    cursor = connection.cursor()
     sql = """
 SELECT DISTINCT s.id
     FROM packages_signoff s
@@ -162,14 +243,49 @@ SELECT DISTINCT s.id
         AND s.repo_id = p.repo_id
     )
     JOIN repos r ON p.repo_id = r.id
-    WHERE r.testing = %s
+    WHERE r.id IN (
 """
-    cursor = connection.cursor()
-    cursor.execute(sql, [True])
+    sql += ", ".join("%s" for r in repos)
+    sql += ")"
+    cursor.execute(sql, [r.id for r in repos])
+
     results = cursor.fetchall()
     # fetch all of the returned signoffs by ID
     to_fetch = [row[0] for row in results]
     signoffs = Signoff.objects.select_related('user').in_bulk(to_fetch)
     return signoffs.values()
 
+def get_target_repo_map(pkgbases):
+    package_repos = Package.objects.order_by().values_list(
+            'pkgbase', 'repo__name').filter(
+            repo__testing=False, repo__staging=False,
+            pkgbase__in=pkgbases).distinct()
+    return dict(package_repos)
+
+def get_signoff_groups(repos=None):
+    if repos is None:
+        repos = Repo.objects.filter(testing=True)
+
+    test_pkgs = Package.objects.normal().filter(repo__in=repos)
+    packages = test_pkgs.order_by('pkgname')
+
+    # Collect all pkgbase values in testing repos
+    q_pkgbase = test_pkgs.values('pkgbase')
+    pkgtorepo = get_target_repo_map(q_pkgbase)
+
+    # Collect all existing signoffs for these packages
+    signoffs = get_current_signoffs(repos)
+
+    same_pkgbase_key = lambda x: (x.repo.name, x.arch.name, x.pkgbase)
+    grouped = groupby_preserve_order(packages, same_pkgbase_key)
+    signoff_groups = []
+    for group in grouped:
+        signoff_group = PackageSignoffGroup(group)
+        signoff_group.target_repo = pkgtorepo.get(signoff_group.pkgbase,
+                "Unknown")
+        signoff_group.find_signoffs(signoffs)
+        signoff_groups.append(signoff_group)
+
+    return signoff_groups
+
 # vim: set ts=4 sw=4 et:
diff --git a/packages/views.py b/packages/views.py
index 035d51cb..e102760b 100644
--- a/packages/views.py
+++ b/packages/views.py
@@ -23,11 +23,11 @@ from string import Template
 from urllib import urlencode
 
 from main.models import Package, PackageFile, Arch, Repo
-from main.utils import make_choice, groupby_preserve_order, PackageStandin
+from main.utils import make_choice
 from mirrors.models import MirrorUrl
-from .models import PackageRelation, PackageGroup, SignoffSpecification, Signoff
+from .models import PackageRelation, PackageGroup, Signoff
 from .utils import (get_group_info, get_differences_info,
-        get_wrong_permissions, get_current_signoffs)
+        get_wrong_permissions, get_signoff_groups, approved_by_signoffs)
 
 class PackageJSONEncoder(DjangoJSONEncoder):
     pkg_attributes = [ 'pkgname', 'pkgbase', 'repo', 'arch', 'pkgver',
@@ -369,100 +369,12 @@ 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, user=None):
-        if len(packages) == 0:
-            raise Exception
-        self.packages = packages
-        self.user = user
-        self.target_repo = None
-        self.signoffs = set()
-        self.specification = DEFAULT_SIGNOFF_SPEC
-
-        first = packages[0]
-        self.pkgbase = first.pkgbase
-        self.arch = first.arch
-        self.repo = first.repo
-        self.version = ''
-
-        version = first.full_version
-        if all(version == pkg.full_version for pkg in packages):
-            self.version = version
-
-    @property
-    def package(self):
-        '''Try and return a relevant single package object representing this
-        group. Start by seeing if there is only one package, then look for the
-        matching package by name, finally falling back to a standin package
-        object.'''
-        if len(self.packages) == 1:
-            return self.packages[0]
-
-        same_pkgs = [p for p in self.packages if p.pkgname == p.pkgbase]
-        if same_pkgs:
-            return same_pkgs[0]
-
-        return PackageStandin(self.packages[0])
-
-    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.'''
-        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.add(s)
-
-    def approved(self):
-        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
 def signoffs(request):
-    test_pkgs = Package.objects.normal().filter(repo__testing=True)
-    packages = test_pkgs.order_by('pkgname')
-
-    # Collect all pkgbase values in testing repos
-    q_pkgbase = test_pkgs.values('pkgbase')
-    package_repos = Package.objects.order_by().values_list(
-            'pkgbase', 'repo__name').filter(
-            repo__testing=False, repo__staging=False,
-            pkgbase__in=q_pkgbase).distinct()
-    pkgtorepo = dict(package_repos)
-
-    # Collect all existing signoffs for these packages
-    signoffs = get_current_signoffs()
-
-    same_pkgbase_key = lambda x: (x.repo.name, x.arch.name, x.pkgbase)
-    grouped = groupby_preserve_order(packages, same_pkgbase_key)
-    signoff_groups = []
-    for group in grouped:
-        signoff_group = PackageSignoffGroup(group, user=request.user)
-        signoff_group.target_repo = pkgtorepo.get(signoff_group.pkgbase,
-                "Unknown")
-        signoff_group.find_signoffs(signoffs)
-        signoff_groups.append(signoff_group)
-
-    signoff_groups.sort(key=attrgetter('pkgbase'))
+    signoff_groups = sorted(get_signoff_groups(), key=attrgetter('pkgbase'))
+    for group in signoff_groups:
+        group.user = request.user
 
     context = {
         'signoff_groups': signoff_groups,
diff --git a/templates/packages/signoff_cell.html b/templates/packages/signoff_cell.html
index fce5d551..87216193 100644
--- a/templates/packages/signoff_cell.html
+++ b/templates/packages/signoff_cell.html
@@ -5,8 +5,8 @@
 </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>
+        title="Revoke signoff {{ group.pkgbase }} for {{ group.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>
+        title="Signoff {{ group.pkgbase }} for {{ group.arch }}">Signoff</a></div>
 {% endif %}
diff --git a/templates/packages/signoffs.html b/templates/packages/signoffs.html
index 4a2f6c99..8d57a8c5 100644
--- a/templates/packages/signoffs.html
+++ b/templates/packages/signoffs.html
@@ -34,6 +34,7 @@
                 <th>Package Base/Version</th>
                 <th>Arch</th>
                 <th>Target Repo</th>
+                <th>Packager</th>
                 <th># of Packages</th>
                 <th>Last Updated</th>
                 <th>Approved</th>
@@ -42,18 +43,17 @@
         </thead>
         <tbody id="tbody_signoffs">
             {% for group in signoff_groups %}
-            {% with group.package as pkg %}
-            <tr class="{% cycle 'odd' 'even' %} {{ pkg.arch.name }}">
-                <td>{% pkg_details_link pkg %} {{ pkg.full_version }}</td>
-                <td>{{ pkg.arch.name }}</td>
+            <tr class="{% cycle 'odd' 'even' %} {{ group.arch.name }}">
+                <td>{% pkg_details_link group.package %} {{ group.version }}</td>
+                <td>{{ group.arch.name }}</td>
                 <td>{{ group.target_repo }}</td>
+                <td>{{ group.packager|default:"Unknown" }}</td>
                 <td>{{ group.packages|length }}</td>
-                <td>{{ pkg.last_update|date }}</td>
+                <td>{{ group.last_update|date }}</td>
                 <td class="signoff-{{ group.approved|yesno }}">
                     {{ group.approved|yesno|capfirst }}</td>
                 <td>{% include "packages/signoff_cell.html" %}</td>
             </tr>
-            {% endwith %}
             {% endfor %}
         </tbody>
     </table>