Merge branch 'feature-1478-change-package-group-to-members-table' of h…

…ttps://github.com/okfn/ckan into feature-1485-package-form-extension

ckan/controllers/authorization_group.py

@@ -143,8 +143,6 @@ def edit(self, id=None): # allow id=None to allow posting
                     if usr and usr not in authorization_group.users:
                         model.add_user_to_authorization_group(usr, authorization_group, model.Role.READER)
             model.repo.commit_and_remove()
-            from nose.tools import set_trace; set_trace()
-
             h.redirect_to(controller='authorization_group', action='read', id=c.authorization_group_name)
 
     def authz(self, id):

ckan/controllers/group_formalchemy.py

@@ -63,7 +63,7 @@ def new(self):
             group = model.Group.get(c.groupname)
             pkgs = [model.Package.by_name(name) for name in request.params.getall('Group-packages-current')]
             group.packages = pkgs
-            pkgnames = request.params.getall('PackageGroup--package_name')
+            pkgnames = request.params.getall('Member--package_name')
             for pkgname in pkgnames:
                 if pkgname:
                     package = model.Package.by_name(pkgname)
@@ -126,7 +126,7 @@ def edit(self, id=None): # allow id=None to allow posting
                 return render('group/edit.html')
             pkgs = [model.Package.by_name(name) for name in request.params.getall('Group-packages-current')]
             group.packages = pkgs
-            pkgnames = request.params.getall('PackageGroup--package_name')
+            pkgnames = request.params.getall('Member--package_name')
             for pkgname in pkgnames:
                 if pkgname:
                     package = model.Package.by_name(pkgname)

ckan/controllers/home.py

@@ -14,6 +14,7 @@
 from ckan.lib.base import *
 import ckan.lib.stats
 from ckan.lib.hash import get_redirect
+from ckan.lib.helpers import url_for
 
 class HomeController(BaseController):
     repo = model.repo
@@ -60,6 +61,27 @@ def index(self):
             c.package_count = 0
             c.groups = []
 
+        if c.userobj is not None:
+            msg = None
+            url = url_for(controller='user', action='edit')
+            is_google_id = \
+                c.userobj.name.startswith('https://www.google.com/accounts/o8/id')
+            if not c.userobj.email and (is_google_id and not c.userobj.fullname):
+                msg = _('Please <a href="%s">update your profile</a>'
+                    ' and add your email address and your full name. %s uses'
+                    ' your email address if you need to reset your'
+                    ' password.''') % (url, g.site_title)
+            elif not c.userobj.email:
+                msg = _('Please <a href="%s">update your profile</a>'
+                    ' and add your email address. %s uses your email address'
+                    ' if you need to reset your password.') \
+                    % (url, g.site_title)
+            elif is_google_id and not c.userobj.fullname:
+                msg = _('Please <a href="%s">update your profile</a>'
+                    ' and add your full name.') % (url)
+            if msg:
+                h.flash_notice(msg, allow_html=True)
+
         return render('home/index.html')
 
     def license(self):

ckan/controllers/package.py

@@ -457,6 +457,8 @@ def edit(self, id, data=None, errors=None, error_summary=None):
             if schema and not data:
                 old_data, errors = validate(old_data, schema, context=context)
             data = data or old_data
+            # Merge all elements for the complete package dictionary
+            c.pkg_dict = dict(old_data.items() + data.items())
         except NotAuthorized:
             abort(401, _('Unauthorized to read package %s') % '')
         except NotFound:
@@ -656,6 +658,7 @@ def authz(self, id):
             context = {'model':model,'user':c.user or c.author, 'package':pkg}
             check_access('package_edit_permissions',context)
             c.authz_editable = True
+            c.pkg_dict = get_action('package_show')(context, {'id': id})
         except NotAuthorized:
             c.authz_editable = False
         if not c.authz_editable:
@@ -752,18 +755,15 @@ def resource_read(self, id, resource_id):
 
         try:
             c.resource = get_action('resource_show')(context, {'id': resource_id})
+            c.package = get_action('package_show')(context, {'id': id})
+            # required for nav menu
+            c.pkg = context['package']
+            c.resource_json = json.dumps(c.resource)
+            c.pkg_dict = c.package
         except NotFound:
             abort(404, _('Resource not found'))
         except NotAuthorized:
             abort(401, _('Unauthorized to read resource %s') % id)
-
-        try:
-            c.package = get_action('package_show')(context, {'id': id})
-        except NotFound:
-            abort(404, _('Package not found'))
-        except NotAuthorized:
-            abort(401, _('Unauthorized to read package %s') % id)
-
         # get package license info
         license_id = c.package.get('license_id')
         try:

ckan/forms/common.py

@@ -14,6 +14,7 @@
 import ckan.lib.helpers as h
 import ckan.lib.field_types as field_types
 import ckan.misc
+import ckan.lib.dictization.model_save as model_save
 
 log = logging.getLogger(__name__)
 
@@ -701,36 +702,21 @@ def sync(self):
 
         def _update_groups(self):
             new_group_ids = self._deserialize() or []
-
-            # Get groups which have alread been associated.
-            old_groups = self.parent.model.groups
-
-            # Calculate which to append and which to remove.
-            editable_set = set([g.id for g in self.user_editable_groups])
-            old_group_ids = [g.id for g in old_groups]
-            new_set = set(new_group_ids)
-            old_set = set(old_group_ids)
-            append_set = (new_set - old_set).intersection(editable_set)
-            remove_set = (old_set - new_set).intersection(editable_set)
-
-            # Create dataset group associations.
-            for id in append_set:
-                group = model.Session.query(model.Group).autoflush(False).get(id)
-                if group:
-                    self.parent.model.groups.append(group)
-
-            # Delete dataset group associations.
-            for group in self.parent.model.groups:
-                if group.id in remove_set:
-                    self.parent.model.groups.remove(group)
+
+            group_dicts = [dict(id = group_id) for 
+                           group_id in new_group_ids]
+
+            context = {'model': model, 'session': model.Session}
+            model_save.package_membership_list_save(
+                group_dicts, self.parent.model, context)
 
         def requires_label(self):
             return False
         requires_label = property(requires_label)
 
     class GroupSelectEditRenderer(formalchemy.fields.FieldRenderer):
         def _get_value(self, **kwargs):
-            return self.field.parent.model.groups
+            return self.field.parent.model.get_groups()
 
         def _get_user_editable_groups(self):
             return self.field.user_editable_groups

ckan/forms/group.py

@@ -81,7 +81,7 @@ def get_group_fieldset(combined=False, is_admin=False):
 def get_package_group_fieldset():
     if not 'new_package_group_fs' in fieldsets:
         # new_package_group_fs is the packages for the WUI form
-        builder = FormBuilder(model.PackageGroup)
+        builder = FormBuilder(model.Member)
         builder.add_field(PackageNameField('package_name'))
         builder.set_field_option('package_name', 'with_renderer', PackagesRenderer)
         builder.set_field_text('package_name', _('Package'))

ckan/lib/create_test_data.py

@@ -164,6 +164,7 @@ def create_arbitrary(cls, package_dicts, relationships=[],
                             pkg.tags.append(tag)
                             model.Session.flush()
                     elif attr == 'groups':
+                        model.Session.flush()
                         if isinstance(val, (str, unicode)):
                             group_names = val.split()
                         elif isinstance(val, list):
@@ -184,7 +185,8 @@ def create_arbitrary(cls, package_dicts, relationships=[],
                                     # session has not yet been committed at this point.
                                     # Fetch from the new_groups dict instead.
                                     group = new_groups[group_name]
-                            pkg.groups.append(group)
+                            member = model.Member(group=group, table_id=pkg.id, table_name='package')
+                            model.Session.add(member)
                     elif attr == 'license':
                         pkg.license_id = val
                     elif attr == 'license_id':
@@ -297,7 +299,8 @@ def create_groups(cls, group_dicts, admin_user_name=None):
             for pkg_name in pkg_names:
                 pkg = model.Package.by_name(unicode(pkg_name))
                 assert pkg, pkg_name
-                pkg.groups.append(group)
+                member = model.Member(group=group, table_id=pkg.id, table_name='package')
+                model.Session.add(member)
             model.Session.add(group)
             model.setup_default_user_roles(group, admin_users)
             cls.group_names.add(group_dict['name'])
@@ -396,9 +399,12 @@ def create(cls):
 
         cls.group_names.add(u'david')
         cls.group_names.add(u'roger')
-        model.Session.add(model.PackageGroup(package=pkg1, group=david))
-        model.Session.add(model.PackageGroup(package=pkg2, group=david))
-        model.Session.add(model.PackageGroup(package=pkg1, group=roger))
+
+        model.Session.flush()
+
+        model.Session.add(model.Member(table_id=pkg1.id, table_name='package', group=david))
+        model.Session.add(model.Member(table_id=pkg2.id, table_name='package', group=david))
+        model.Session.add(model.Member(table_id=pkg1.id, table_name='package', group=roger))
         # authz
         model.Session.add_all([
             model.User(name=u'tester', apikey=u'tester', password=u'tester'),

ckan/lib/dictization/model_dictize.py

@@ -26,7 +26,7 @@ def group_list_dictize(obj_list, context,
 
         group_dict['display_name'] = obj.display_name
 
-        group_dict['packages'] = len(obj.packages)
+        group_dict['packages'] = len(obj.active_packages().all())
 
         result_list.append(group_dict)
     return sorted(result_list, key=sort_key, reverse=reverse)
@@ -140,12 +140,12 @@ def package_dictize(pkg, context):
     result = _execute_with_revision(q, extra_rev, context)
     result_dict["extras"] = extras_list_dictize(result, context)
     #groups
-    group_rev = model.package_group_revision_table
+    member_rev = model.member_revision_table
     group = model.group_table
     q = select([group],
-               from_obj=group_rev.join(group, group.c.id == group_rev.c.group_id)
-               ).where(group_rev.c.package_id == pkg.id)
-    result = _execute_with_revision(q, group_rev, context)
+               from_obj=member_rev.join(group, group.c.id == member_rev.c.group_id)
+               ).where(member_rev.c.table_id == pkg.id)
+    result = _execute_with_revision(q, member_rev, context)
     result_dict["groups"] = obj_list_dictize(result, context)
     #relations
     rel_rev = model.package_relationship_revision_table
@@ -163,16 +163,21 @@ def package_dictize(pkg, context):
     return result_dict
 
 def group_dictize(group, context):
-
+    model = context['model']
     result_dict = table_dictize(group, context)
-    
+
     result_dict['display_name'] = group.display_name
 
     result_dict['extras'] = extras_dict_dictize(
         group._extras, context)
 
+    packages = model.Session.query(model.Package).\
+               join(model.Member, model.Member.table_id == model.Package.id).\
+               filter(model.Member.group_id == group.id).\
+               filter(model.Member.state == 'active').all()
+
     result_dict['packages'] = obj_list_dictize(
-        group.packages, context)
+        packages, context)
 
     return result_dict
 

ckan/lib/dictization/model_save.py

@@ -182,7 +182,7 @@ def package_tag_list_save(tag_dicts, package, context):
 
     package.package_tag_all[:] = tag_package_tag.values()
 
-def package_group_list_save(group_dicts, package, context):
+def package_membership_list_save(group_dicts, package, context):
 
     allow_partial_update = context.get("allow_partial_update", False)
     if not group_dicts and allow_partial_update:
@@ -192,9 +192,11 @@ def package_group_list_save(group_dicts, package, context):
     session = context["session"]
     pending = context.get('pending')
 
-    group_package_group = dict((package_group.group, package_group) 
-                               for package_group in
-                               package.package_group_all)
+    members = session.query(model.Member).filter_by(table_id = package.id)
+
+    group_member = dict((member.group, member) 
+                         for member in
+                         members)
     groups = set()
     for group_dict in group_dicts:
         id = group_dict.get("id")
@@ -205,25 +207,25 @@ def package_group_list_save(group_dicts, package, context):
             group = session.query(model.Group).filter_by(name=name).first()
         groups.add(group)
 
-    for group in groups - set(group_package_group.keys()):
-        package_group_obj = model.PackageGroup(package = package,
-                                               group = group,
-                                               state = 'active')
-        session.add(package_group_obj)
-        group_package_group[group] = package_group_obj
-
-    for group in set(group_package_group.keys()) - groups:
-        group_package_group.pop(group)
-        continue
-        ### this is alternate behavioiur below which is correct
-        ### but not compatible with old behaviour
-        package_group = group_package_group[group]
-        if pending and package_group.state <> 'deleted':
-            package_group.state = 'pending-deleted'
-        else:
-            package_group.state = 'deleted'
+    ## need to flush so we can get out the package id
+    model.Session.flush()
+    for group in groups - set(group_member.keys()):
+        member_obj = model.Member(table_id = package.id,
+                                  table_name = 'package',
+                                  group = group,
+                                  state = 'active')
+        session.add(member_obj)
+
 
-    package.package_group_all[:] = group_package_group.values()
+    for group in set(group_member.keys()) - groups:
+        member_obj = group_member[group]
+        member_obj.state = 'deleted'
+        session.add(member_obj)
+
+    for group in set(group_member.keys()) & groups:
+        member_obj = group_member[group]
+        member_obj.state = 'active'
+        session.add(member_obj)
 
 
 def relationship_list_save(relationship_dicts, package, attr, context):
@@ -266,7 +268,7 @@ def package_dict_save(pkg_dict, context):
 
     package_resource_list_save(pkg_dict.get("resources", []), pkg, context)
     package_tag_list_save(pkg_dict.get("tags", []), pkg, context)
-    package_group_list_save(pkg_dict.get("groups", []), pkg, context)
+    package_membership_list_save(pkg_dict.get("groups", []), pkg, context)
 
     subjects = pkg_dict.get('relationships_as_subject', [])
     relationship_list_save(subjects, pkg, 'relationships_as_subject', context)
@@ -287,6 +289,7 @@ def group_dict_save(group_dict, context):
 
     Group = model.Group
     Package = model.Package
+    Member = model.Member
     if group:
         group_dict["id"] = group.id 
 
@@ -302,7 +305,7 @@ def group_dict_save(group_dict, context):
 
     package_dicts = group_dict.get("packages", [])
 
-    packages = []
+    packages = {}
 
     for package in package_dicts:
         pkg = None
@@ -311,14 +314,31 @@ def group_dict_save(group_dict, context):
             pkg = session.query(Package).get(id)
         if not pkg:
             pkg = session.query(Package).filter_by(name=package["name"]).first()
-        if pkg and pkg not in packages:
-            packages.append(pkg)
+        if pkg and pkg not in packages.values():
+            packages[pkg.id] = pkg
+
+    members = session.query(Member).filter_by(
+        table_name='package',
+        group_id=group.id,
+    ).all()
 
-    if packages or not allow_partial_update:
-        group.packages[:] = packages
+    package_member = dict((member.table_id, member) for member in members)
+
+    for package_id in set(package_member.keys()) - set(packages.keys()):
+        package_member[package_id].state = 'deleted'
+        session.add(package_member[package_id])
+
+    for package_id in set(package_member.keys()) & set(packages.keys()):
+        package_member[package_id].state = 'active'
+        session.add(package_member[package_id])
+
+    for package_id in set(packages.keys()) - set(package_member.keys()):
+        member = Member(group=group, table_id=package_id, table_name='package')
+        session.add(member)
 
     return group
 
+
 def user_dict_save(user_dict, context):
 
     model = context['model']

ckan/lib/dumper.py

@@ -61,7 +61,7 @@ class Dumper(object):
         ckan.model.PackageRevision,
         ckan.model.PackageTagRevision,
         ckan.model.Group,
-        ckan.model.PackageGroup,
+        ckan.model.Member,
         ckan.model.PackageExtra,
         ]
     # TODO Bring this list of classes up to date. In the meantime,