[ckan-changes] commit/ckan: dread: [controllers, forms, templates]: #1323 Fix for authorization groups being created without names. Editing of authgroups already created also fixed, referring to them by id.

Bitbucket commits-noreply at bitbucket.org
Fri Sep 9 18:46:53 UTC 2011


1 new changeset in ckan:

http://bitbucket.org/okfn/ckan/changeset/bebd0a46ebc6/
changeset:   bebd0a46ebc6
branch:      release-v1.4.3
user:        dread
date:        2011-09-09 20:43:55
summary:     [controllers,forms,templates]: #1323 Fix for authorization groups being created without names. Editing of authgroups already created also fixed, referring to them by id.
affected #:  6 files (2.7 KB)

--- a/ckan/controllers/authorization_group.py	Fri Sep 09 17:40:06 2011 +0100
+++ b/ckan/controllers/authorization_group.py	Fri Sep 09 19:43:55 2011 +0100
@@ -27,8 +27,12 @@
         )
         return render('authorization_group/index.html')
 
+    def _get_authgroup_by_name_or_id(self, id):
+        return model.AuthorizationGroup.by_name(id) or\
+               model.Session.query(model.AuthorizationGroup).get(id)
+
     def read(self, id):
-        c.authorization_group = model.AuthorizationGroup.by_name(id)
+        c.authorization_group = self._get_authgroup_by_name_or_id(id)
         if c.authorization_group is None:
             abort(404)
         auth_for_read = self.authorizer.am_authorized(c, model.Action.READ, 
@@ -88,6 +92,7 @@
             h.redirect_to(action='read', id=c.authzgroupname)
 
         if request.params:
+            # when does this happen?
             data = ckan.forms.edit_group_dict(ckan.forms.get_authorization_group_dict(), request.params)
             fs = fs.bind(data=data, session=model.Session)
         c.form = self._render_edit_form(fs)
@@ -95,7 +100,7 @@
 
     def edit(self, id=None): # allow id=None to allow posting
         c.error = ''
-        authorization_group = model.AuthorizationGroup.by_name(id)
+        authorization_group = self._get_authgroup_by_name_or_id(id)
         if authorization_group is None:
             abort(404, '404 Not Found')
         am_authz = self.authorizer.am_authorized(c, model.Action.EDIT, authorization_group)
@@ -142,7 +147,7 @@
             h.redirect_to(action='read', id=c.authorization_group_name)
 
     def authz(self, id):
-        authorization_group = model.AuthorizationGroup.by_name(id)
+        authorization_group = self._get_authgroup_by_name_or_id(id)
         if authorization_group is None:
             abort(404, _('Group not found'))
 
@@ -156,7 +161,7 @@
 
         #see package.py for comments
         def get_userobjectroles():
-            authorization_group = model.AuthorizationGroup.by_name(id)
+            authorization_group = self._get_authgroup_by_name_or_id(id)
             uors = model.Session.query(model.AuthorizationGroupRole).join('authorization_group').filter_by(name=authorization_group.name).all()
             return uors
 


--- a/ckan/forms/authorization_group.py	Fri Sep 09 17:40:06 2011 +0100
+++ b/ckan/forms/authorization_group.py	Fri Sep 09 19:43:55 2011 +0100
@@ -45,6 +45,7 @@
     builder = FormBuilder(model.AuthorizationGroup)
     builder.set_field_text('name', 'Name', literal("<br/><strong>Unique identifier</strong> for group.<br/>2+ chars, lowercase, using only 'a-z0-9' and '-_'"))
     builder.set_field_option('name', 'validate', common.group_name_validator)
+    builder.set_field_option('name', 'required')
     displayed_fields = ['name']
     if with_users:
         builder.add_field(UsersField('users'))


--- a/ckan/templates/_util.html	Fri Sep 09 17:40:06 2011 +0100
+++ b/ckan/templates/_util.html	Fri Sep 09 19:43:55 2011 +0100
@@ -181,8 +181,8 @@
       <tr><th>Title</th><th>Number of members</th></tr><py:for each="authorization_group in authorization_groups"><tr>
-        <td><a href="${h.url_for(controller='authorization_group', action='read', id=authorization_group.name)}">
-            ${authorization_group.name}</a>
+        <td><a href="${h.url_for(controller='authorization_group', action='read', id=authorization_group.name or authorization_group.id)}">
+            ${authorization_group.name or authorization_group.id}</a></td><td>${len(authorization_group.users)}</td></tr>


--- a/ckan/templates/authorization_group/layout.html	Fri Sep 09 17:40:06 2011 +0100
+++ b/ckan/templates/authorization_group/layout.html	Fri Sep 09 19:43:55 2011 +0100
@@ -8,12 +8,12 @@
 
   <py:match path="minornavigation" py:if="c.authorization_group"><ul class="tabbed">
-    <li>${h.subnav_link(c, h.icon('authorization_group') + _('View'), controller='authorization_group', action='read', id=c.authorization_group.name)}</li>
+    <li>${h.subnav_link(c, h.icon('authorization_group') + _('View'), controller='authorization_group', action='read', id=c.authorization_group.name or c.authorization_group.id)}</li><li py:if="h.am_authorized(c, actions.EDIT, c.authorization_group)">
-      ${h.subnav_link(c, h.icon('authorization_group_edit') + _('Edit'), controller='authorization_group', action='edit', id=c.authorization_group.name)}
+      ${h.subnav_link(c, h.icon('authorization_group_edit') + _('Edit'), controller='authorization_group', action='edit', id=c.authorization_group.name or c.authorization_group.id)}
     </li><li py:if="h.am_authorized(c, actions.EDIT_PERMISSIONS, c.authorization_group)">
-      ${h.subnav_link(c, h.icon('lock') + _('Authorization'), controller='authorization_group', action='authz', id=c.authorization_group.name)}
+      ${h.subnav_link(c, h.icon('lock') + _('Authorization'), controller='authorization_group', action='authz', id=c.authorization_group.name or c.authorization_group.id)}
     </li></ul></py:match>


--- a/ckan/tests/functional/test_authorization_group.py	Fri Sep 09 17:40:06 2011 +0100
+++ b/ckan/tests/functional/test_authorization_group.py	Fri Sep 09 19:43:55 2011 +0100
@@ -1,4 +1,5 @@
 from nose.plugins.skip import SkipTest
+from nose.tools import assert_equal
 
 from ckan.tests import *
 from ckan.authz import Authorizer
@@ -127,6 +128,57 @@
         assert len(group.users) == 2
 
 
+class TestNew(FunctionalTestCase):
+    groupname = u'treasury'
+
+    @classmethod
+    def setup_class(self):
+        CreateTestData.create_user('tester1')
+        CreateTestData.create_user('tester2')
+        CreateTestData.create_user('tester3')
+
+        self.extra_environ = {'REMOTE_USER': 'tester1'}
+
+    @classmethod
+    def teardown_class(self):
+        model.repo.rebuild_db()
+
+    def test_0_new(self):
+        offset = url_for(controller='authorization_group', action='new', id=None)
+        res = self.app.get(offset, status=200, extra_environ=self.extra_environ)
+        assert 'New Authorization Group' in res, res
+
+        form = res.forms['group-edit']
+        form['AuthorizationGroup--name'] = 'testname'
+
+        # can't test users - needs javascript
+        #form['AuthorizationGroupUser--user_name'] = 'tester2' 
+        
+        res = form.submit('save', status=302, extra_environ=self.extra_environ)
+        res = res.follow()
+
+        # should be read page
+        main_res = self.main_div(res)
+        assert 'testname' in main_res, main_res
+
+        # test created object
+        auth_group = model.AuthorizationGroup.by_name('testname')
+        assert auth_group
+        assert_equal(auth_group.name, 'testname')
+
+    def test_0_new_without_name(self):
+        offset = url_for(controller='authorization_group', action='new', id=None)
+        res = self.app.get(offset, status=200, extra_environ=self.extra_environ)
+        assert 'New Authorization Group' in res, res
+
+        form = res.forms['group-edit']
+        # don't set name
+        
+        res = form.submit('save', status=200, extra_environ=self.extra_environ)
+        assert 'Error' in res, res
+        assert 'Name: Please enter a value' in res, res
+
+
 class TestAuthorizationGroupWalkthrough(FunctionalTestCase):
 
     @classmethod


--- a/ckan/tests/functional/test_package.py	Fri Sep 09 17:40:06 2011 +0100
+++ b/ckan/tests/functional/test_package.py	Fri Sep 09 19:43:55 2011 +0100
@@ -1165,6 +1165,23 @@
         assert 'Name must be at least 2 characters long' in res, res
         self._assert_form_errors(res)
 
+    def test_new_no_name(self):
+        offset = url_for(controller='package', action='new', id=None)
+        res = self.app.get(offset)
+        assert 'New - Data Packages' in res
+        fv = res.forms['package-edit']
+        prefix = ''
+        # don't set a name
+        res = fv.submit('preview')
+        assert 'Error' in res, res
+        assert 'Name: Missing value' in res, res
+        self._assert_form_errors(res)
+
+        res = fv.submit('save')
+        assert 'Error' in res, res
+        assert 'Name: Missing value' in res, res
+        self._assert_form_errors(res)
+
     def test_redirect_after_new_using_param(self):
         return_url = 'http://random.site.com/package/<NAME>?test=param'
         # It's useful to know that this url encodes to:

Repository URL: https://bitbucket.org/okfn/ckan/

--

This is a commit notification from bitbucket.org. You are receiving
this because you have the service enabled, addressing the recipient of
this email.




More information about the ckan-changes mailing list