From 0e79bf2632750c0b993c1c07d500fbcd88752703 Mon Sep 17 00:00:00 2001 From: Greg Von Kuster Date: Thu, 10 Mar 2011 15:48:00 -0500 Subject: [PATCH] Add a checkbox to the Create Group page that if checked will create a new Role with the same name. This provides a similar feature the the existing checkbox on the Create Role page. However, the behavior is now changed such that new associations are created when the checkbox is checked whereas before, only the Group or Role objects with the same name were created, but not associated with anything. This code was very old, so I did some cleanup / improvements as well. Functional tests enhanced to reflect changes. --- lib/galaxy/web/base/controller.py | 144 +++++++++++------- lib/galaxy/web/controllers/library_common.py | 6 - .../dataset_security/group/group_create.mako | 15 +- .../dataset_security/role/role_create.mako | 52 ++++--- test/base/twilltestcase.py | 27 ++-- test/functional/test_admin_features.py | 40 +++-- 6 files changed, 169 insertions(+), 115 deletions(-) diff --git a/lib/galaxy/web/base/controller.py b/lib/galaxy/web/base/controller.py index 3b133692064..0367a1079c5 100644 --- a/lib/galaxy/web/base/controller.py +++ b/lib/galaxy/web/base/controller.py @@ -1185,16 +1185,24 @@ class Admin( object ): webapp = params.get( 'webapp', 'galaxy' ) message = util.restore_text( params.get( 'message', '' ) ) status = params.get( 'status', 'done' ) + name = util.restore_text( params.get( 'name', '' ) ) + description = util.restore_text( params.get( 'description', '' ) ) + in_users = util.listify( params.get( 'in_users', [] ) ) + out_users = util.listify( params.get( 'out_users', [] ) ) + in_groups = util.listify( params.get( 'in_groups', [] ) ) + out_groups = util.listify( params.get( 'out_groups', [] ) ) + create_group_for_role = params.get( 'create_group_for_role', '' ) + create_group_for_role_checked = CheckboxField.is_checked( create_group_for_role ) + ok = True if params.get( 'create_role_button', False ): - name = util.restore_text( params.name ) - description = util.restore_text( params.description ) - in_users = util.listify( params.get( 'in_users', [] ) ) - in_groups = util.listify( params.get( 'in_groups', [] ) ) - create_group_for_role = params.get( 'create_group_for_role', 'no' ) if not name or not description: - message = "Enter a valid name and a description" + message = "Enter a valid name and a description." + status = 'error' + ok = False elif trans.sa_session.query( trans.app.model.Role ).filter( trans.app.model.Role.table.c.name==name ).first(): - message = "A role with that name already exists" + message = "Role names must be unique and a role with that name already exists, so choose another name." + status = 'error' + ok = False else: # Create the role role = trans.app.model.Role( name=name, description=description, type=trans.app.model.Role.types.ADMIN ) @@ -1207,41 +1215,44 @@ class Admin( object ): for group in [ trans.sa_session.query( trans.app.model.Group ).get( x ) for x in in_groups ]: gra = trans.app.model.GroupRoleAssociation( group, role ) trans.sa_session.add( gra ) - if create_group_for_role == 'yes': + if create_group_for_role_checked: # Create the group group = trans.app.model.Group( name=name ) trans.sa_session.add( group ) - message = "Group '%s' has been created, and role '%s' has been created with %d associated users and %d associated groups" % \ - ( group.name, role.name, len( in_users ), len( in_groups ) ) + # Associate the group with the role + gra = trans.model.GroupRoleAssociation( group, role ) + trans.sa_session.add( gra ) + num_in_groups = len( in_groups ) + 1 else: - message = "Role '%s' has been created with %d associated users and %d associated groups" % ( role.name, len( in_users ), len( in_groups ) ) + num_in_groups = len( in_groups ) trans.sa_session.flush() + message = "Role '%s' has been created with %d associated users and %d associated groups. " \ + % ( role.name, len( in_users ), num_in_groups ) + if create_group_for_role_checked: + message += 'One of the groups associated with this role is the newly created group with the same name.' trans.response.send_redirect( web.url_for( controller='admin', action='roles', webapp=webapp, message=util.sanitize_text( message ), status='done' ) ) - trans.response.send_redirect( web.url_for( controller='admin', - action='create_role', - webapp=webapp, - message=util.sanitize_text( message ), - status='error' ) ) - out_users = [] - for user in trans.sa_session.query( trans.app.model.User ) \ - .filter( trans.app.model.User.table.c.deleted==False ) \ - .order_by( trans.app.model.User.table.c.email ): - out_users.append( ( user.id, user.email ) ) - out_groups = [] - for group in trans.sa_session.query( trans.app.model.Group ) \ - .filter( trans.app.model.Group.table.c.deleted==False ) \ - .order_by( trans.app.model.Group.table.c.name ): - out_groups.append( ( group.id, group.name ) ) + if ok: + for user in trans.sa_session.query( trans.app.model.User ) \ + .filter( trans.app.model.User.table.c.deleted==False ) \ + .order_by( trans.app.model.User.table.c.email ): + out_users.append( ( user.id, user.email ) ) + for group in trans.sa_session.query( trans.app.model.Group ) \ + .filter( trans.app.model.Group.table.c.deleted==False ) \ + .order_by( trans.app.model.Group.table.c.name ): + out_groups.append( ( group.id, group.name ) ) return trans.fill_template( '/admin/dataset_security/role/role_create.mako', - in_users=[], - out_users=out_users, - in_groups=[], - out_groups=out_groups, webapp=webapp, + name=name, + description=description, + in_users=in_users, + out_users=out_users, + in_groups=in_groups, + out_groups=out_groups, + create_group_for_role_checked=create_group_for_role_checked, message=message, status=status ) @web.expose @@ -1617,14 +1628,23 @@ class Admin( object ): webapp = params.get( 'webapp', 'galaxy' ) message = util.restore_text( params.get( 'message', '' ) ) status = params.get( 'status', 'done' ) + name = util.restore_text( params.get( 'name', '' ) ) + in_users = util.listify( params.get( 'in_users', [] ) ) + out_users = util.listify( params.get( 'out_users', [] ) ) + in_roles = util.listify( params.get( 'in_roles', [] ) ) + out_roles = util.listify( params.get( 'out_roles', [] ) ) + create_role_for_group = params.get( 'create_role_for_group', '' ) + create_role_for_group_checked = CheckboxField.is_checked( create_role_for_group ) + ok = True if params.get( 'create_group_button', False ): - name = util.restore_text( params.name ) - in_users = util.listify( params.get( 'in_users', [] ) ) - in_roles = util.listify( params.get( 'in_roles', [] ) ) if not name: - message = "Enter a valid name" + message = "Enter a valid name." + status = 'error' + ok = False elif trans.sa_session.query( trans.app.model.Group ).filter( trans.app.model.Group.table.c.name==name ).first(): - message = "A group with that name already exists" + message = "Group names must be unique and a group with that name already exists, so choose another name." + status = 'error' + ok = False else: # Create the group group = trans.app.model.Group( name=name ) @@ -1634,39 +1654,49 @@ class Admin( object ): for user in [ trans.sa_session.query( trans.app.model.User ).get( x ) for x in in_users ]: uga = trans.app.model.UserGroupAssociation( user, group ) trans.sa_session.add( uga ) - trans.sa_session.flush() # Create the GroupRoleAssociations for role in [ trans.sa_session.query( trans.app.model.Role ).get( x ) for x in in_roles ]: gra = trans.app.model.GroupRoleAssociation( group, role ) trans.sa_session.add( gra ) - trans.sa_session.flush() - message = "Group '%s' has been created with %d associated users and %d associated roles" % ( name, len( in_users ), len( in_roles ) ) + if create_role_for_group_checked: + # Create the role + role = trans.app.model.Role( name=name, description='Role for group %s' % name ) + trans.sa_session.add( role ) + # Associate the role with the group + gra = trans.model.GroupRoleAssociation( group, role ) + trans.sa_session.add( gra ) + num_in_roles = len( in_roles ) + 1 + else: + num_in_roles = len( in_roles ) + trans.sa_session.flush() + message = "Group '%s' has been created with %d associated users and %d associated roles. " \ + % ( group.name, len( in_users ), num_in_roles ) + if create_role_for_group_checked: + message += 'One of the roles associated with this group is the newly created role with the same name.' trans.response.send_redirect( web.url_for( controller='admin', action='groups', webapp=webapp, message=util.sanitize_text( message ), status='done' ) ) - trans.response.send_redirect( web.url_for( controller='admin', - action='create_group', - webapp=webapp, - message=util.sanitize_text( message ), - status='error' ) ) - out_users = [] - for user in trans.sa_session.query( trans.app.model.User ) \ - .filter( trans.app.model.User.table.c.deleted==False ) \ - .order_by( trans.app.model.User.table.c.email ): - out_users.append( ( user.id, user.email ) ) - out_roles = [] - for role in trans.sa_session.query( trans.app.model.Role ) \ - .filter( trans.app.model.Role.table.c.deleted==False ) \ - .order_by( trans.app.model.Role.table.c.name ): - out_roles.append( ( role.id, role.name ) ) + + + if ok: + for user in trans.sa_session.query( trans.app.model.User ) \ + .filter( trans.app.model.User.table.c.deleted==False ) \ + .order_by( trans.app.model.User.table.c.email ): + out_users.append( ( user.id, user.email ) ) + for role in trans.sa_session.query( trans.app.model.Role ) \ + .filter( trans.app.model.Role.table.c.deleted==False ) \ + .order_by( trans.app.model.Role.table.c.name ): + out_roles.append( ( role.id, role.name ) ) return trans.fill_template( '/admin/dataset_security/group/group_create.mako', - in_users=[], - out_users=out_users, - in_roles=[], - out_roles=out_roles, webapp=webapp, + name=name, + in_users=in_users, + out_users=out_users, + in_roles=in_roles, + out_roles=out_roles, + create_role_for_group_checked=create_role_for_group_checked, message=message, status=status ) @web.expose diff --git a/lib/galaxy/web/controllers/library_common.py b/lib/galaxy/web/controllers/library_common.py index c65d70f00bb..2ab8c181d38 100644 --- a/lib/galaxy/web/controllers/library_common.py +++ b/lib/galaxy/web/controllers/library_common.py @@ -1986,13 +1986,7 @@ class LibraryCommon( BaseController, UsesFormDefinitions ): # We've been called from a menu option for a library dataset search result set move_ldda_ids = util.listify( item_id ) if move_ldda_ids: - # Checkboxes cause 2 copies of each id to be included in the request move_ldda_ids = map( trans.security.decode_id, move_ldda_ids ) - unique_ldda_ids = [] - for ldda_id in move_ldda_ids: - if ldda_id not in unique_ldda_ids: - unique_ldda_ids.append( ldda_id ) - move_ldda_ids = unique_ldda_ids elif item_type == 'folder': move_folder_id = item_id move_folder = trans.sa_session.query( trans.model.LibraryFolder ).get( trans.security.decode_id( move_folder_id ) ) diff --git a/templates/admin/dataset_security/group/group_create.mako b/templates/admin/dataset_security/group/group_create.mako index 6e382e453aa..45416927d38 100644 --- a/templates/admin/dataset_security/group/group_create.mako +++ b/templates/admin/dataset_security/group/group_create.mako @@ -42,7 +42,12 @@ $().ready(function() { }); }); - + +<% + from galaxy.web.form_builder import CheckboxField + create_role_for_group_checkbox = CheckboxField( 'create_role_for_group' ) +%> + %if message: ${render_msg( message, status )} %endif @@ -54,7 +59,7 @@ $().ready(function() {
- +
@@ -80,6 +85,12 @@ $().ready(function() {
+
+ %if create_role_for_group_checked: + <% create_role_for_group_checkbox.checked = True %> + %endif + ${create_role_for_group_checkbox.get_html()} Create a new role of the same name for this group +
diff --git a/templates/admin/dataset_security/role/role_create.mako b/templates/admin/dataset_security/role/role_create.mako index 4a46c476614..1866bb6d08e 100644 --- a/templates/admin/dataset_security/role/role_create.mako +++ b/templates/admin/dataset_security/role/role_create.mako @@ -19,30 +19,35 @@ +<% + from galaxy.web.form_builder import CheckboxField + create_group_for_role_checkbox = CheckboxField( 'create_group_for_role' ) +%> + %if message: ${render_msg( message, status )} %endif @@ -54,11 +59,11 @@ $().ready(function() {
- +
- +
@@ -85,7 +90,10 @@ $().ready(function() {
- Create a new group of the same name for this role + %if create_group_for_role_checked: + <% create_group_for_role_checkbox.checked = True %> + %endif + ${create_group_for_role_checkbox.get_html()} Create a new group of the same name for this role
diff --git a/test/base/twilltestcase.py b/test/base/twilltestcase.py index 3c5fd456d38..c40647ccc37 100644 --- a/test/base/twilltestcase.py +++ b/test/base/twilltestcase.py @@ -1229,25 +1229,22 @@ class TwillTestCase( unittest.TestCase ): description="This is Role One", in_user_ids=[], in_group_ids=[], - create_group_for_role='no', - private_role='' ): + create_group_for_role='', + private_role='', + strings_displayed=[] ): """Create a new role""" - url = "%s/admin/roles?operation=create&create_role_button=Save&name=%s&description=%s" % ( self.url, name.replace( ' ', '+' ), description.replace( ' ', '+' ) ) + url = "%s/admin/roles?operation=create&create_role_button=Save&name=%s&description=%s" % \ + ( self.url, name.replace( ' ', '+' ), description.replace( ' ', '+' ) ) if in_user_ids: url += "&in_users=%s" % ','.join( in_user_ids ) if in_group_ids: url += "&in_groups=%s" % ','.join( in_group_ids ) if create_group_for_role == 'yes': - url += '&create_group_for_role=yes' + url += '&create_group_for_role=yes&create_group_for_role=yes' self.home() self.visit_url( url ) - if create_group_for_role == 'yes': - check_str = "Group '%s' has been created, and role '%s' has been created with %d associated users and %d associated groups" % \ - ( name, name, len( in_user_ids ), len( in_group_ids ) ) - else: - check_str = "Role '%s' has been created with %d associated users and %d associated groups" % \ - ( name, len( in_user_ids ), len( in_group_ids ) ) - self.check_page_for_string( check_str ) + for check_str in strings_displayed: + self.check_page_for_string( check_str ) if private_role: # Make sure no private roles are displayed try: @@ -1304,17 +1301,19 @@ class TwillTestCase( unittest.TestCase ): self.home() # Tests associated with groups - def create_group( self, name='Group One', in_user_ids=[], in_role_ids=[] ): + def create_group( self, name='Group One', in_user_ids=[], in_role_ids=[], create_role_for_group='', strings_displayed=[] ): """Create a new group""" url = "%s/admin/groups?operation=create&create_group_button=Save&name=%s" % ( self.url, name.replace( ' ', '+' ) ) if in_user_ids: url += "&in_users=%s" % ','.join( in_user_ids ) if in_role_ids: url += "&in_roles=%s" % ','.join( in_role_ids ) + if create_role_for_group == 'yes': + url += '&create_role_for_group=yes&create_role_for_group=yes' self.home() self.visit_url( url ) - check_str = "Group '%s' has been created with %d associated users and %d associated roles" % ( name, len( in_user_ids ), len( in_role_ids ) ) - self.check_page_for_string( check_str ) + for check_str in strings_displayed: + self.check_page_for_string( check_str ) self.home() self.visit_url( "%s/admin/groups" % self.url ) self.check_page_for_string( name ) diff --git a/test/functional/test_admin_features.py b/test/functional/test_admin_features.py index ae381efb03d..14cf8bd9a31 100644 --- a/test/functional/test_admin_features.py +++ b/test/functional/test_admin_features.py @@ -126,21 +126,26 @@ class TestDataSecurity( TwillTestCase ): # Logged in as admin_user name = 'Role One' description = "This is Role Ones description" - user_ids=[ str( admin_user.id ), str( regular_user1.id ), str( regular_user3.id ) ] + in_user_ids = [ str( admin_user.id ), str( regular_user1.id ), str( regular_user3.id ) ] + in_group_ids = [] + # Add 1 to the number of associated groups since we are creating a new one with the same name as the role + num_gras = len( in_group_ids ) + 1 self.create_role( name=name, description=description, - in_user_ids=user_ids, - in_group_ids=[], + in_user_ids=in_user_ids, + in_group_ids=in_group_ids, create_group_for_role='yes', - private_role=admin_user.email ) + private_role=admin_user.email, + strings_displayed=[ "Role '%s' has been created with %d associated users and %d associated groups." % ( name, len( in_user_ids ), num_gras ), + "One of the groups associated with this role is the newly created group with the same name." ] ) # Get the role object for later tests global role_one role_one = sa_session.query( galaxy.model.Role ).filter( galaxy.model.Role.table.c.name==name ).first() assert role_one is not None, 'Problem retrieving role named "Role One" from the database' # Make sure UserRoleAssociations are correct - if len( role_one.users ) != len( user_ids ): + if len( role_one.users ) != len( in_user_ids ): raise AssertionError( '%d UserRoleAssociations were created for role id %d when it was created ( should have been %d )' \ - % ( len( role_one.users ), role_one.id, len( user_ids ) ) ) + % ( len( role_one.users ), role_one.id, len( in_user_ids ) ) ) # Each of the following users should now have 2 role associations, their private role and role_one for user in [ admin_user, regular_user1, regular_user3 ]: refresh( user ) @@ -162,29 +167,36 @@ class TestDataSecurity( TwillTestCase ): # Reset the role back to the original name and description self.rename_role( self.security.encode_id( role_one.id ), name=name, description=description ) def test_035_create_group( self ): - """Testing creating new group with 3 members and 1 associated role, then renaming it""" + """Testing creating new group with 3 members and 2 associated roles, then renaming it""" # Logged in as admin_user name = "Group One's Name" - user_ids=[ str( admin_user.id ), str( regular_user1.id ), str( regular_user3.id ) ] - role_ids=[ str( role_one.id ) ] - self.create_group( name=name, in_user_ids=user_ids, in_role_ids=role_ids ) + in_user_ids = [ str( admin_user.id ), str( regular_user1.id ), str( regular_user3.id ) ] + in_role_ids = [ str( role_one.id ) ] + # The number of GroupRoleAssociations should be 2, role_one and the newly created role named 'Group One's Name' + num_gras = len( in_role_ids ) + 1 + self.create_group( name=name, + in_user_ids=in_user_ids, + in_role_ids=in_role_ids, + create_role_for_group='yes', + strings_displayed=[ "Group '%s' has been created with %d associated users and %d associated roles." % ( name, len( in_user_ids ), num_gras ), + "One of the roles associated with this group is the newly created role with the same name." ] ) # Get the group object for later tests global group_one group_one = get_group_by_name( name ) assert group_one is not None, 'Problem retrieving group named "Group One" from the database' # Make sure UserGroupAssociations are correct - if len( group_one.users ) != len( user_ids ): + if len( group_one.users ) != len( in_user_ids ): raise AssertionError( '%d UserGroupAssociations were created for group id %d when it was created ( should have been %d )' \ - % ( len( group_one.users ), group_one.id, len( user_ids ) ) ) + % ( len( group_one.users ), group_one.id, len( in_user_ids ) ) ) # Each user should now have 1 group association, group_one for user in [ admin_user, regular_user1, regular_user3 ]: refresh( user ) if len( user.groups ) != 1: raise AssertionError( '%d UserGroupAssociations are associated with user %s ( should be 1 )' % ( len( user.groups ), user.email ) ) # Make sure GroupRoleAssociations are correct - if len( group_one.roles ) != len( role_ids ): + if len( group_one.roles ) != num_gras: raise AssertionError( '%d GroupRoleAssociations were created for group id %d when it was created ( should have been %d )' \ - % ( len( group_one.roles ), group_one.id, len( role_ids ) ) ) + % ( len( group_one.roles ), group_one.id, num_gras ) ) # Rename the group rename = "Group One's been Renamed" self.rename_group( self.security.encode_id( group_one.id ), name=rename, )