details: http://www.bx.psu.edu/hg/galaxy/rev/6e0fc1622959 changeset: 3653:6e0fc1622959 user: Greg Von Kuster <greg@bx.psu.edu> date: Wed Apr 14 15:16:03 2010 -0400 description: Allow non-admin users that have the LIBRARY_MODIFY permission to delete / undelete library items as well as display / hide deleted library items. diffstat: lib/galaxy/web/controllers/library_admin.py | 74 -------------------------- lib/galaxy/web/controllers/library_common.py | 78 ++++++++++++++++++++++++++++ templates/library/common/browse_library.mako | 30 +++++----- templates/library/common/library_info.mako | 8 +- test/base/twilltestcase.py | 12 ++-- test/functional/test_library_features.py | 27 ++++++--- test/functional/test_library_security.py | 3 +- 7 files changed, 122 insertions(+), 110 deletions(-) diffs (388 lines): diff -r 1486617bfae8 -r 6e0fc1622959 lib/galaxy/web/controllers/library_admin.py --- a/lib/galaxy/web/controllers/library_admin.py Wed Apr 14 14:09:01 2010 -0400 +++ b/lib/galaxy/web/controllers/library_admin.py Wed Apr 14 15:16:03 2010 -0400 @@ -161,77 +161,3 @@ action='browse_libraries', message=util.sanitize_text( message ), status='done' ) ) - @web.expose - @web.require_admin - def delete_library_item( self, trans, library_id, item_id, item_type, **kwd ): - # This action will handle deleting all types of library items. State is saved for libraries and - # folders ( i.e., if undeleted, the state of contents of the library or folder will remain, so previously - # deleted / purged contents will have the same state ). When a library or folder has been deleted for - # the amount of time defined in the cleanup_datasets.py script, the library or folder and all of its - # contents will be purged. The association between this method and the cleanup_datasets.py script - # enables clean maintenance of libraries and library dataset disk files. This is also why the item_types - # are not any of the associations ( the cleanup_datasets.py script handles everything ). - show_deleted = util.string_as_bool( kwd.get( 'show_deleted', False ) ) - item_types = { 'library': trans.app.model.Library, - 'folder': trans.app.model.LibraryFolder, - 'library_dataset': trans.app.model.LibraryDataset } - if item_type not in item_types: - message = 'Bad item_type specified: %s' % str( item_type ) - status = 'error' - else: - if item_type == 'library_dataset': - item_desc = 'Dataset' - else: - item_desc = item_type.capitalize() - library_item = trans.sa_session.query( item_types[ item_type ] ).get( trans.security.decode_id( item_id ) ) - library_item.deleted = True - trans.sa_session.add( library_item ) - trans.sa_session.flush() - message = util.sanitize_text( "%s '%s' has been marked deleted" % ( item_desc, library_item.name ) ) - status = 'done' - if item_type == 'library': - return self.browse_libraries( trans, message=message, status=status ) - else: - return trans.response.send_redirect( web.url_for( controller='library_common', - action='browse_library', - cntrller='library_admin', - id=library_id, - show_deleted=show_deleted, - message=message, - status=status ) ) - @web.expose - @web.require_admin - def undelete_library_item( self, trans, library_id, item_id, item_type, **kwd ): - # This action will handle undeleting all types of library items - show_deleted = util.string_as_bool( kwd.get( 'show_deleted', False ) ) - item_types = { 'library': trans.app.model.Library, - 'folder': trans.app.model.LibraryFolder, - 'library_dataset': trans.app.model.LibraryDataset } - if item_type not in item_types: - message = 'Bad item_type specified: %s' % str( item_type ) - status = ERROR - else: - if item_type == 'library_dataset': - item_desc = 'Dataset' - else: - item_desc = item_type.capitalize() - library_item = trans.sa_session.query( item_types[ item_type ] ).get( trans.security.decode_id( item_id ) ) - if library_item.purged: - message = '%s %s has been purged, so it cannot be undeleted' % ( item_desc, library_item.name ) - status = ERROR - else: - library_item.deleted = False - trans.sa_session.add( library_item ) - trans.sa_session.flush() - message = util.sanitize_text( "%s '%s' has been marked undeleted" % ( item_desc, library_item.name ) ) - status = SUCCESS - if item_type == 'library': - return self.browse_libraries( trans, message=message, status=status ) - else: - return trans.response.send_redirect( web.url_for( controller='library_common', - action='browse_library', - cntrller='library_admin', - id=library_id, - show_deleted=show_deleted, - message=message, - status=status ) ) diff -r 1486617bfae8 -r 6e0fc1622959 lib/galaxy/web/controllers/library_common.py --- a/lib/galaxy/web/controllers/library_common.py Wed Apr 14 14:09:01 2010 -0400 +++ b/lib/galaxy/web/controllers/library_common.py Wed Apr 14 15:16:03 2010 -0400 @@ -1733,6 +1733,84 @@ show_deleted=show_deleted, message=util.sanitize_text( message ), status=status ) ) + @web.expose + def delete_library_item( self, trans, cntrller, library_id, item_id, item_type, **kwd ): + # This action will handle deleting all types of library items. State is saved for libraries and + # folders ( i.e., if undeleted, the state of contents of the library or folder will remain, so previously + # deleted / purged contents will have the same state ). When a library or folder has been deleted for + # the amount of time defined in the cleanup_datasets.py script, the library or folder and all of its + # contents will be purged. The association between this method and the cleanup_datasets.py script + # enables clean maintenance of libraries and library dataset disk files. This is also why the item_types + # are not any of the associations ( the cleanup_datasets.py script handles everything ). + show_deleted = util.string_as_bool( kwd.get( 'show_deleted', False ) ) + item_types = { 'library': trans.app.model.Library, + 'folder': trans.app.model.LibraryFolder, + 'library_dataset': trans.app.model.LibraryDataset } + if item_type not in item_types: + message = 'Bad item_type specified: %s' % str( item_type ) + status = 'error' + else: + if item_type == 'library_dataset': + item_desc = 'Dataset' + else: + item_desc = item_type.capitalize() + library_item = trans.sa_session.query( item_types[ item_type ] ).get( trans.security.decode_id( item_id ) ) + library_item.deleted = True + trans.sa_session.add( library_item ) + trans.sa_session.flush() + message = util.sanitize_text( "%s '%s' has been marked deleted" % ( item_desc, library_item.name ) ) + status = 'done' + if item_type == 'library': + return trans.response.send_redirect( web.url_for( controller=cntrller, + action='browse_libraries', + message=message, + status=status ) ) + else: + return trans.response.send_redirect( web.url_for( controller='library_common', + action='browse_library', + cntrller=cntrller, + id=library_id, + show_deleted=show_deleted, + message=message, + status=status ) ) + @web.expose + def undelete_library_item( self, trans, cntrller, library_id, item_id, item_type, **kwd ): + # This action will handle undeleting all types of library items + show_deleted = util.string_as_bool( kwd.get( 'show_deleted', False ) ) + item_types = { 'library': trans.app.model.Library, + 'folder': trans.app.model.LibraryFolder, + 'library_dataset': trans.app.model.LibraryDataset } + if item_type not in item_types: + message = 'Bad item_type specified: %s' % str( item_type ) + status = ERROR + else: + if item_type == 'library_dataset': + item_desc = 'Dataset' + else: + item_desc = item_type.capitalize() + library_item = trans.sa_session.query( item_types[ item_type ] ).get( trans.security.decode_id( item_id ) ) + if library_item.purged: + message = '%s %s has been purged, so it cannot be undeleted' % ( item_desc, library_item.name ) + status = ERROR + else: + library_item.deleted = False + trans.sa_session.add( library_item ) + trans.sa_session.flush() + message = util.sanitize_text( "%s '%s' has been marked undeleted" % ( item_desc, library_item.name ) ) + status = SUCCESS + if item_type == 'library': + return trans.response.send_redirect( web.url_for( controller=cntrller, + action='browse_libraries', + message=message, + status=status ) ) + else: + return trans.response.send_redirect( web.url_for( controller='library_common', + action='browse_library', + cntrller=cntrller, + id=library_id, + show_deleted=show_deleted, + message=message, + status=status ) ) # ---- Utility methods ------------------------------------------------------- diff -r 1486617bfae8 -r 6e0fc1622959 templates/library/common/browse_library.mako --- a/templates/library/common/browse_library.mako Wed Apr 14 14:09:01 2010 -0400 +++ b/templates/library/common/browse_library.mako Wed Apr 14 15:16:03 2010 -0400 @@ -232,11 +232,11 @@ <a class="action-button" href="${h.url_for( controller='library_common', action='act_on_multiple_datasets', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), ldda_ids=trans.security.encode_id( ldda.id ), do_action='import_to_history', use_panels=use_panels, show_deleted=show_deleted )}">Import this dataset into your current history</a> <a class="action-button" href="${h.url_for( controller='library_common', action='download_dataset_from_folder', cntrller=cntrller, id=trans.security.encode_id( ldda.id ), library_id=trans.security.encode_id( library.id ), use_panels=use_panels )}">Download this dataset</a> %endif - %if cntrller in [ 'library_admin', 'requests_admin' ]: + %if cntrller == 'library_admin' or can_modify: %if not library.deleted and not branch_deleted( folder ) and not ldda.library_dataset.deleted: - <a class="action-button" confirm="Click OK to delete dataset '${ldda.name}'." href="${h.url_for( controller='library_admin', action='delete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library_dataset.id ), item_type='library_dataset', show_deleted=show_deleted )}">Delete this dataset</a> + <a class="action-button" confirm="Click OK to delete dataset '${ldda.name}'." href="${h.url_for( controller='library_common', action='delete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library_dataset.id ), item_type='library_dataset', show_deleted=show_deleted )}">Delete this dataset</a> %elif not library.deleted and not branch_deleted( folder ) and not ldda.library_dataset.purged and ldda.library_dataset.deleted: - <a class="action-button" href="${h.url_for( controller='library_admin', action='undelete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library_dataset.id ), item_type='library_dataset', show_deleted=show_deleted )}">Undelete this dataset</a> + <a class="action-button" href="${h.url_for( controller='library_common', action='undelete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library_dataset.id ), item_type='library_dataset', show_deleted=show_deleted )}">Undelete this dataset</a> %endif %endif </div> @@ -328,11 +328,11 @@ %if not branch_deleted( folder ) and ( cntrller == 'library_admin' or can_manage ): <a class="action-button" href="${h.url_for( controller='library_common', action='folder_permissions', cntrller=cntrller, id=trans.security.encode_id( folder.id ), library_id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=show_deleted )}">Edit permissions</a> %endif - %if cntrller in [ 'library_admin', 'requests_admin' ]: + %if cntrller == 'library_admin' or can_modify: %if not library.deleted and not folder.deleted: - <a class="action-button" confirm="Click OK to delete the folder '${folder.name}.'" href="${h.url_for( controller='library_admin', action='delete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( folder.id ), item_type='folder', show_deleted=show_deleted )}">Delete this folder</a> + <a class="action-button" confirm="Click OK to delete the folder '${folder.name}.'" href="${h.url_for( controller='library_common', action='delete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( folder.id ), item_type='folder', show_deleted=show_deleted )}">Delete this folder</a> %elif not library.deleted and folder.deleted and not folder.purged: - <a class="action-button" href="${h.url_for( controller='library_admin', action='undelete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( folder.id ), item_type='folder', show_deleted=show_deleted )}">Undelete this folder</a> + <a class="action-button" href="${h.url_for( controller='library_common', action='undelete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( folder.id ), item_type='folder', show_deleted=show_deleted )}">Undelete this folder</a> %endif %endif </div> @@ -435,6 +435,12 @@ %if not library.deleted: %if cntrller == 'library_admin' or can_modify: <a class="action-button" href="${h.url_for( controller='library_common', action='library_info', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=show_deleted )}">Edit information</a> + <a class="action-button" confirm="Click OK to delete the library named '${library.name}'." href="${h.url_for( controller='library_common', action='delete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library' )}">Delete this data library</a> + %if show_deleted: + <a class="action-button" href="${h.url_for( controller='library_common', action='browse_library', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=False )}">Hide deleted items</a> + %else: + <a class="action-button" href="${h.url_for( controller='library_common', action='browse_library', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=True )}">Show deleted items</a> + %endif %endif %if ( cntrller == 'library_admin' or can_modify ) and not library.info_association: <a class="action-button" href="${h.url_for( controller='library_common', action='add_template', cntrller=cntrller, item_type='library', library_id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=show_deleted )}">Add template</a> @@ -446,16 +452,8 @@ %if cntrller == 'library_admin' or can_manage: <a class="action-button" href="${h.url_for( controller='library_common', action='library_permissions', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=show_deleted )}">Edit permissions</a> %endif - %if cntrller == 'library_admin': - <a class="action-button" confirm="Click OK to delete the library named '${library.name}'." href="${h.url_for( controller='library_admin', action='delete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library' )}">Delete this data library</a> - %if show_deleted: - <a class="action-button" href="${h.url_for( controller='library_common', action='browse_library', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=False )}">Hide deleted items</a> - %else: - <a class="action-button" href="${h.url_for( controller='library_common', action='browse_library', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=True )}">Show deleted items</a> - %endif - %endif - %elif cntrller == 'library_admin' and not library.purged: - <a class="action-button" href="${h.url_for( controller='library_admin', action='undelete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library', use_panels=use_panels )}">Undelete this data library</a> + %elif ( cntrller == 'library_admin' or can_modify ) and not library.purged: + <a class="action-button" href="${h.url_for( controller='library_common', action='undelete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library', use_panels=use_panels )}">Undelete this data library</a> %elif library.purged: <a class="action-button" href="${h.url_for( controller='library_common', action='browse_library', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=show_deleted )}">This data library has been purged</a> %endif diff -r 1486617bfae8 -r 6e0fc1622959 templates/library/common/library_info.mako --- a/templates/library/common/library_info.mako Wed Apr 14 14:09:01 2010 -0400 +++ b/templates/library/common/library_info.mako Wed Apr 14 15:16:03 2010 -0400 @@ -37,11 +37,11 @@ %if cntrller == 'library_admin' or can_manage: <a class="action-button" href="${h.url_for( controller='library_common', action='library_permissions', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=show_deleted )}">Edit permissions</a> %endif - %if cntrller == 'library_admin': - <a class="action-button" confirm="Click OK to delete the library named '${library.name}'." href="${h.url_for( controller='library_admin', action='delete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library' )}">Delete this data library</a> + %if cntrller == 'library_admin' or can_modify: + <a class="action-button" confirm="Click OK to delete the library named '${library.name}'." href="${h.url_for( controller='library_common', action='delete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library' )}">Delete this data library</a> %endif - %elif cntrller == 'library_admin' and not library.purged: - <a class="action-button" href="${h.url_for( controller='library_admin', action='undelete_library_item', library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library' )}">Undelete this data library</a> + %elif ( cntrller == 'library_admin' or can_modify ) and not library.purged: + <a class="action-button" href="${h.url_for( controller='library_common', action='undelete_library_item', cntrller=cntrller, library_id=trans.security.encode_id( library.id ), item_id=trans.security.encode_id( library.id ), item_type='library' )}">Undelete this data library</a> %elif library.purged: <a class="action-button" href="${h.url_for( controller='library_common', action='browse_library', cntrller=cntrller, id=trans.security.encode_id( library.id ), use_panels=use_panels, show_deleted=show_deleted )}">This data library has been purged</a> %endif diff -r 1486617bfae8 -r 6e0fc1622959 test/base/twilltestcase.py --- a/test/base/twilltestcase.py Wed Apr 14 14:09:01 2010 -0400 +++ b/test/base/twilltestcase.py Wed Apr 14 15:16:03 2010 -0400 @@ -2014,11 +2014,11 @@ errmsg += 'Unpacked archive remains in: %s\n' % tmpd raise AssertionError( errmsg ) shutil.rmtree( tmpd ) - def delete_library_item( self, library_id, item_id, item_name, item_type='library_dataset' ): + def delete_library_item( self, cntrller, library_id, item_id, item_name, item_type='library_dataset' ): """Mark a library item as deleted""" self.home() - self.visit_url( "%s/library_admin/delete_library_item?library_id=%s&item_id=%s&item_type=%s" \ - % ( self.url, library_id, item_id, item_type ) ) + self.visit_url( "%s/library_common/delete_library_item?cntrller=%s&library_id=%s&item_id=%s&item_type=%s" \ + % ( self.url, cntrller, library_id, item_id, item_type ) ) if item_type == 'library_dataset': item_desc = 'Dataset' else: @@ -2026,11 +2026,11 @@ check_str = "%s '%s' has been marked deleted" % ( item_desc, item_name ) self.check_page_for_string( check_str ) self.home() - def undelete_library_item( self, library_id, item_id, item_name, item_type='library_dataset' ): + def undelete_library_item( self, cntrller, library_id, item_id, item_name, item_type='library_dataset' ): """Mark a library item as deleted""" self.home() - self.visit_url( "%s/library_admin/undelete_library_item?library_id=%s&item_id=%s&item_type=%s" \ - % ( self.url, library_id, item_id, item_type ) ) + self.visit_url( "%s/library_common/undelete_library_item?cntrller=%s&library_id=%s&item_id=%s&item_type=%s" \ + % ( self.url, cntrller, library_id, item_id, item_type ) ) if item_type == 'library_dataset': item_desc = 'Dataset' else: diff -r 1486617bfae8 -r 6e0fc1622959 test/functional/test_library_features.py --- a/test/functional/test_library_features.py Wed Apr 14 14:09:01 2010 -0400 +++ b/test/functional/test_library_features.py Wed Apr 14 15:16:03 2010 -0400 @@ -476,7 +476,8 @@ def test_080_mark_dataset_deleted( self ): """Testing marking a library dataset as deleted""" # Logged in as admin_user - self.delete_library_item( self.security.encode_id( library_one.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library_one.id ), self.security.encode_id( ldda_two.library_dataset.id ), ldda_two.name, item_type='library_dataset' ) @@ -496,7 +497,8 @@ def test_090_mark_folder_deleted( self ): """Testing marking a library folder as deleted""" # Logged in as admin_user - self.delete_library_item( self.security.encode_id( library_one.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library_one.id ), self.security.encode_id( folder_two.id ), folder_two.name, item_type='folder' ) @@ -506,7 +508,8 @@ def test_095_mark_folder_undeleted( self ): """Testing marking a library folder as undeleted""" # Logged in as admin_user - self.undelete_library_item( self.security.encode_id( library_one.id ), + self.undelete_library_item( 'library_admin', + self.security.encode_id( library_one.id ), self.security.encode_id( folder_two.id ), folder_two.name, item_type='folder' ) @@ -520,11 +523,13 @@ """Testing marking a library as deleted""" # Logged in as admin_user # First mark folder_two as deleted to further test state saving when we undelete the library - self.delete_library_item( self.security.encode_id( library_one.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library_one.id ), self.security.encode_id( folder_two.id ), folder_two.name, item_type='folder' ) - self.delete_library_item( self.security.encode_id( library_one.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library_one.id ), self.security.encode_id( library_one.id ), library_one.name, item_type='library' ) @@ -533,7 +538,8 @@ def test_105_mark_library_undeleted( self ): """Testing marking a library as undeleted""" # Logged in as admin_user - self.undelete_library_item( self.security.encode_id( library_one.id ), + self.undelete_library_item( 'library_admin', + self.security.encode_id( library_one.id ), self.security.encode_id( library_one.id ), library_one.name, item_type='library' ) @@ -545,7 +551,8 @@ def test_110_purge_library( self ): """Testing purging a library""" # Logged in as admin_user - self.delete_library_item( self.security.encode_id( library_one.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library_one.id ), self.security.encode_id( library_one.id ), library_one.name, item_type='library' ) @@ -612,7 +619,8 @@ # TODO: add a functional test to cover adding a library dataset via url_paste here... # TODO: Add a functional test to cover checking the space_to_tab checkbox here... # Delete and purge the library - self.delete_library_item( self.security.encode_id( library_two.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library_two.id ), self.security.encode_id( library_two.id ), library_two.name, item_type='library' ) @@ -625,7 +633,8 @@ # Purge all libraries ################## for library in [ library_one, library_two ]: - self.delete_library_item( self.security.encode_id( library.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library.id ), self.security.encode_id( library.id ), library.name, item_type='library' ) diff -r 1486617bfae8 -r 6e0fc1622959 test/functional/test_library_security.py --- a/test/functional/test_library_security.py Wed Apr 14 14:09:01 2010 -0400 +++ b/test/functional/test_library_security.py Wed Apr 14 15:16:03 2010 -0400 @@ -569,7 +569,8 @@ # Purge all libraries ################## for library in [ library_one, library_two ]: - self.delete_library_item( self.security.encode_id( library.id ), + self.delete_library_item( 'library_admin', + self.security.encode_id( library.id ), self.security.encode_id( library.id ), library.name, item_type='library' )