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
+
- 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, )