Remove duplicate methods to get a group's packages

Refactor active_packages() and get_package_revisions(), both of which
return a group's packages (but in slightly different ways), replace with
just one method packages().

We now have just one way to get a group's packages, the packages()
method of the group model. The group_package_show() action function
calls it.

ckan/controllers/group.py

@@ -342,7 +342,7 @@ def _force_reindex(self, grp):
         appearing on the read page for the group (as they're connected via
         the group name)'''
         group = model.Group.get(grp['name'])
-        for dataset in group.active_packages().all():
+        for dataset in group.packages():
             search.rebuild(dataset.name)
 
     def _save_edit(self, id, context):

ckan/lib/dictization/model_dictize.py

@@ -32,7 +32,7 @@ def group_list_dictize(obj_list, context,
         group_dict['display_name'] = obj.display_name
 
         group_dict['packages'] = \
-                len(obj.active_packages(with_private=with_private).all())
+                len(obj.packages(with_private=with_private))
 
         if context.get('for_view'):
             for item in plugins.PluginImplementations(

ckan/logic/action/get.py

@@ -764,7 +764,7 @@ def group_package_show(context, data_dict):
     _check_access('group_show', context, data_dict)
 
     result = []
-    for pkg_rev in group.get_package_revisions(limit=limit,
+    for pkg_rev in group.packages(limit=limit,
             return_query=context.get('return_query')):
         result.append(model_dictize.package_dictize(pkg_rev, context))
 

ckan/model/group.py

@@ -155,47 +155,48 @@ def get_children_groups(self, type='group'):
         return [{"id":idf, "name": name, "title": title}
                 for idf, name, title in results]
 
-    def active_packages(self, load_eager=True, with_private=False):
-        query = meta.Session.query(_package.Package).\
-            filter_by(state=vdm.sqlalchemy.State.ACTIVE).\
-            filter(group_table.c.id == self.id).\
-            filter(member_table.c.state == 'active')
+    def packages(self, with_private=False, limit=None,
+            return_query=False):
+        '''Return this group's active and pending packages.
 
-        if not with_private:
-            query = query.filter(member_table.c.capacity == 'public')
-
-        query = query.join(member_table, member_table.c.table_id ==
-                           _package.Package.id).\
-            join(group_table, group_table.c.id == member_table.c.group_id)
-
-        return query
+        Returns all packages in this group with VDM revision state ACTIVE or
+        PENDING.
 
-    def get_package_revisions(self, limit=None, return_query=False):
-        '''Return a group's packages.
+        :param with_private: if True, include the group's private packages
+        :type with_private: boolean
 
         :param limit: the maximum number of packages to return
         :type limit: int
 
-        :returns: a list of the group's packages, sorted by name
-        :rtype: list of PackageRevision objects
+        :param return_query: if True, return the SQLAlchemy query object
+            instead of the list of Packages resulting from the query
+        :type return_query: boolean
+
+        :returns: a list of this group's packages
+        :rtype: list of ckan.model.package.Package objects
 
         '''
-        import ckan.model as model
-        q = model.Session.query(model.PackageRevision)
-        q = q.filter(model.PackageRevision.state == 'active')
-        q = q.filter(model.PackageRevision.current == True)
-        q = q.join(model.Member,
-                model.Member.table_id == model.PackageRevision.id)
-        q = q.join(model.Group, model.Group.id == model.Member.group_id)
-        q = q.filter_by(id=self.id)
-        q = q.order_by(model.PackageRevision.name)
+        query = meta.Session.query(_package.Package)
+        query = query.filter(
+                or_(_package.Package.state == vdm.sqlalchemy.State.ACTIVE,
+                    _package.Package.state == vdm.sqlalchemy.State.PENDING))
+        query = query.filter(group_table.c.id == self.id)
+
+        if not with_private:
+            query = query.filter(member_table.c.capacity == 'public')
+
+        query = query.join(member_table,
+                member_table.c.table_id == _package.Package.id)
+        query = query.join(group_table,
+                group_table.c.id == member_table.c.group_id)
+
         if limit is not None:
-            q = q.limit(limit)
+            query = query.limit(limit)
+
         if return_query:
-            return q
+            return query
         else:
-            return q.all()
-
+            return query.all()
 
     @classmethod
     def search_by_name_or_title(cls, text_query, group_type=None):
@@ -212,7 +213,7 @@ def add_package_by_name(self, package_name):
             return
         package = _package.Package.by_name(package_name)
         assert package
-        if not package in self.active_packages().all():
+        if not package in self.packages():
             member = Member(group=self, table_id=package.id,
                             table_name='package')
             meta.Session.add(member)

ckan/tests/functional/test_group.py

@@ -187,7 +187,7 @@ def test_index(self):
         groupname = 'david'
         group = model.Group.by_name(unicode(groupname))
         group_title = group.title
-        group_packages_count = len(group.active_packages().all())
+        group_packages_count = len(group.packages())
         group_description = group.description
         self.check_named_element(res, 'tr', group_title,
                                  group_packages_count,
@@ -321,7 +321,7 @@ def test_2_edit(self):
         assert group.description == newdesc, group
 
         # now look at datasets
-        assert len(group.active_packages().all()) == 3
+        assert len(group.packages()) == 3
 
     def test_3_edit_form_has_new_package(self):
         # check for dataset in autocomplete
@@ -369,7 +369,7 @@ def test_4_new_duplicate_package(self):
 
         # check package only added to the group once
         group = model.Group.by_name(group_name)
-        pkg_names = [pkg.name for pkg in group.active_packages().all()]
+        pkg_names = [pkg.name for pkg in group.packages()]
         assert_equal(pkg_names, [self.packagename])
 
     def test_edit_plugin_hook(self):
@@ -435,7 +435,7 @@ def update_group(res, name, with_pkg=True):
 
         # We have the datasets in the DB, but we should also see that many
         # on the group read page.
-        assert len(group.active_packages().all()) == 3
+        assert len(group.packages()) == 3
 
         offset = url_for(controller='group', action='read', id='newname')
         res = self.app.get(offset, status=200,
@@ -543,9 +543,9 @@ def test_2_new(self):
         group = model.Group.by_name(group_name)
         assert group.title == group_title, group
         assert group.description == group_description, group
-        assert len(group.active_packages().all()) == 1
+        assert len(group.packages()) == 1
         pkg = model.Package.by_name(self.packagename)
-        assert group.active_packages().all() == [pkg]
+        assert group.packages() == [pkg]
 
     def test_3_new_duplicate_group(self):
         prefix = ''
@@ -702,7 +702,7 @@ def test_index(self):
         groupname = 'david'
         group = model.Group.by_name(unicode(groupname))
         group_title = group.title
-        group_packages_count = len(group.active_packages().all())
+        group_packages_count = len(group.packages())
         group_description = group.description
         self.check_named_element(res, 'tr', group_title,
                                  group_packages_count,
@@ -820,7 +820,7 @@ def test_2_edit(self):
         assert group.description == newdesc, group
 
         # now look at datasets
-        assert len(group.active_packages().all()) == 3
+        assert len(group.packages()) == 3
 
     def test_3_edit_form_has_new_package(self):
         # check for dataset in autocomplete
@@ -868,7 +868,7 @@ def test_4_new_duplicate_package(self):
 
         # check package only added to the group once
         group = model.Group.by_name(group_name)
-        pkg_names = [pkg.name for pkg in group.active_packages().all()]
+        pkg_names = [pkg.name for pkg in group.packages()]
         assert_equal(pkg_names, [self.packagename])
 
     def test_edit_plugin_hook(self):

ckan/tests/lib/test_dictization_schema.py

@@ -148,7 +148,7 @@ def test_2_group_schema(self):
         del data['extras']
 
         converted_data, errors = validate(data, default_group_schema(), self.context)
-        group_pack = sorted(group.active_packages().all(), key=lambda x:x.id)
+        group_pack = sorted(group.packages(), key=lambda x:x.id)
 
         converted_data["packages"] = sorted(converted_data["packages"], key=lambda x:x["id"])
 

ckan/tests/models/test_group.py

@@ -25,7 +25,7 @@ def test_1_basic(self):
         grp = model.Group.by_name(u'group1')
         assert grp.title == u'Test Group'
         assert grp.description == u'This is a test group'
-        assert grp.active_packages().all() == []
+        assert grp.packages() == []
 
     def test_2_add_packages(self):
         model.repo.new_revision()
@@ -50,7 +50,7 @@ def test_2_add_packages(self):
         assert grp.title == u'Russian Group'
         anna = model.Package.by_name(u'annakarenina')
         war = model.Package.by_name(u'warandpeace')
-        assert set(grp.active_packages().all()) == set((anna, war)), grp.active_packages().all()
+        assert set(grp.packages()) == set((anna, war)), grp.packages()
         assert grp in anna.get_groups()
 
     def test_3_search(self):