From 3a5caa755cc712dcb8e4e407212091cb96929f35 Mon Sep 17 00:00:00 2001 From: Greg Von Kuster Date: Tue, 23 Nov 2010 15:09:49 -0500 Subject: [PATCH] Several miscellaneous fixes for the tool shed. --- .../webapps/community/controllers/common.py | 35 +++++++++++++------ .../webapps/community/controllers/tool.py | 13 ++++++- .../webapps/community/controllers/upload.py | 9 +++++ .../webapps/community/security/__init__.py | 2 +- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/webapps/community/controllers/common.py b/lib/galaxy/webapps/community/controllers/common.py index 5583613235a..43dec38e3fc 100644 --- a/lib/galaxy/webapps/community/controllers/common.py +++ b/lib/galaxy/webapps/community/controllers/common.py @@ -205,6 +205,7 @@ class CommonController( BaseController, ItemRatings ): if not id: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='Select a tool to edit', status='error' ) ) tool = get_tool( trans, id ) @@ -212,6 +213,7 @@ class CommonController( BaseController, ItemRatings ): if not can_edit: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='You are not allowed to edit this tool', status='error' ) ) if params.get( 'edit_tool_button', False ): @@ -306,6 +308,7 @@ class CommonController( BaseController, ItemRatings ): if not id: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='Select a tool to view', status='error' ) ) tool = get_tool( trans, id ) @@ -313,6 +316,7 @@ class CommonController( BaseController, ItemRatings ): if not can_view: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='You are not allowed to view this tool', status='error' ) ) avg_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, tool, webapp_model=trans.model ) @@ -368,6 +372,7 @@ class CommonController( BaseController, ItemRatings ): if not trans.app.security_agent.can_delete( trans.user, trans.user_is_admin(), cntrller, tool ): return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='You are not allowed to delete this tool', status='error' ) ) # Create a new event @@ -382,10 +387,11 @@ class CommonController( BaseController, ItemRatings ): trans.sa_session.add_all( ( tool, tea ) ) trans.sa_session.flush() # TODO: What if the tool has versions, should they all be deleted? - message = "Tool '%s' has been marked deleted" % tool.name + message = "Tool '%s' version %s has been marked deleted" % ( tool.name, tool.version ) status = 'done' return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message=message, status=status ) ) @web.expose @@ -395,12 +401,14 @@ class CommonController( BaseController, ItemRatings ): if not id: return trans.response.send_redirect( web.url_for( controller='tool', action='browse_tools', + cntrller=cntrller, message='Select a tool to download', status='error' ) ) tool = get_tool( trans, id ) if not trans.app.security_agent.can_download( trans.user, trans.user_is_admin(), cntrller, tool ): return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='You are not allowed to download this tool', status='error' ) ) trans.response.set_content_type( tool.mimetype ) @@ -416,12 +424,14 @@ class CommonController( BaseController, ItemRatings ): if not id: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='Select a tool to upload a new version', status='error' ) ) tool = get_tool( trans, id ) if not trans.app.security_agent.can_upload_new_version( trans.user, tool ): return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='You are not allowed to upload a new version of this tool', status='error' ) ) return trans.response.send_redirect( web.url_for( controller='upload', @@ -439,6 +449,7 @@ class CommonController( BaseController, ItemRatings ): if not id: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='Select a tool to view its history', status='error' ) ) tool = get_tool( trans, id ) @@ -446,6 +457,7 @@ class CommonController( BaseController, ItemRatings ): if not can_view: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message="You are not allowed to view this tool's history", status='error' ) ) can_approve_or_reject = trans.app.security_agent.can_approve_or_reject( trans.user, trans.user_is_admin(), cntrller, tool ) @@ -476,6 +488,7 @@ class CommonController( BaseController, ItemRatings ): if not id: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message='Select a tool to rate', status='error' ) ) tool = get_tool( trans, id ) @@ -483,6 +496,7 @@ class CommonController( BaseController, ItemRatings ): if not can_rate: return trans.response.send_redirect( web.url_for( controller=cntrller, action='browse_tools', + cntrller=cntrller, message="You are not allowed to rate this tool", status='error' ) ) if params.get( 'rate_button', False ): @@ -539,15 +553,16 @@ def get_tool( trans, id ): def get_latest_versions_of_tools( trans ): """Get only the latest version of each tool from the database""" return trans.sa_session.query( trans.model.Tool ) \ - .filter( trans.model.Tool.newer_version_id == None ) \ - .order_by( trans.model.Tool.name ) -def get_approved_tools( trans ): - """Get the tools from the database whose state is APPROVED""" - approved_tools = [] - for tool in get_latest_versions_of_tools( trans ): - if tool.state == trans.model.Tool.states.APPROVED: - approved_tools.append( tool ) - return approved_tools + .filter( trans.model.Tool.table.c.newer_version_id == None ) \ + .order_by( trans.model.Tool.table.c.name ) +def get_latest_versions_of_tools_by_state( trans, state ): + """Get only the latest version of each tool whose state is the received state from the database""" + tools = [] + for tool in trans.sa_session.query( trans.model.Tool ) \ + .order_by( trans.model.Tool.table.c.name ): + if tool.state == state: + tools.append( tool ) + return tools def get_event( trans, id ): """Get an event from the databse""" return trans.sa_session.query( trans.model.Event ).get( trans.security.decode_id( id ) ) diff --git a/lib/galaxy/webapps/community/controllers/tool.py b/lib/galaxy/webapps/community/controllers/tool.py index cb05a87b41f..0fd30c71b07 100644 --- a/lib/galaxy/webapps/community/controllers/tool.py +++ b/lib/galaxy/webapps/community/controllers/tool.py @@ -28,7 +28,7 @@ class ToolStateColumn( grids.StateColumn ): pass elif column_filter in [ v for k, v in self.model_class.states.items() ]: # Get all of the latest Events associated with the current version of each tool - latest_event_id_for_current_versions_of_tools = [ tool.latest_event.id for tool in get_latest_versions_of_tools( trans ) ] + latest_event_id_for_current_versions_of_tools = [ tool.latest_event.id for tool in get_latest_versions_of_tools_by_state( trans, column_filter ) ] # Filter query by the latest state for the current version of each tool return query.filter( and_( model.Event.table.c.state == column_filter, model.Event.table.c.id.in_( latest_event_id_for_current_versions_of_tools ) ) ) @@ -113,6 +113,7 @@ class ToolController( BaseController ): del kwd[ k ] return trans.response.send_redirect( web.url_for( controller='tool', action='browse_tools', + cntrller='tool', **kwd ) ) # Render the list view return self.category_list_grid( trans, **kwd ) @@ -121,6 +122,15 @@ class ToolController( BaseController ): # We add params to the keyword dict in this method in order to rename the param # with an "f-" prefix, simulating filtering by clicking a search link. We have # to take this approach because the "-" character is illegal in HTTP requests. + if 'operation' not in kwd: + # We may have been redirected here after performing an action. If we were + # redirected from the tool controller, we have to add the default tools_by_category + # operation to kwd so only tools we should see are displayed. This implies that + # all redirectes from the tool controller added the cntrller value to kwd when + # redirecting. + cntrller = kwd.get( 'cntrller', None ) + if cntrller == 'tool': + kwd[ 'operation' ] = 'approved_tools' if 'operation' in kwd: operation = kwd['operation'].lower() if operation == "view_tool": @@ -187,6 +197,7 @@ class ToolController( BaseController ): if not id: return trans.response.send_redirect( web.url_for( controller='tool', action='browse_tools', + cntrller='tool', message='Select a tool to download', status='error' ) ) tool = get_tool( trans, id ) diff --git a/lib/galaxy/webapps/community/controllers/upload.py b/lib/galaxy/webapps/community/controllers/upload.py index c40edbc3f14..3d3b3c94791 100644 --- a/lib/galaxy/webapps/community/controllers/upload.py +++ b/lib/galaxy/webapps/community/controllers/upload.py @@ -115,6 +115,15 @@ class UploadController( BaseController ): if replace_version and replace_id: replace_version.newer_version_id = obj.id trans.sa_session.add( replace_version ) + # TODO: should the state be changed to archived? We'll leave it alone for now + # because if the newer version is deleted, we'll need to add logic to reset the + # the older version back to it's previous state ( possible approved ). + comment = "Replaced by new version %s" % obj.version + event = trans.app.model.Event( state=replace_version.state, comment=comment ) + # Flush to get an event id + trans.sa_session.add( event ) + trans.sa_session.flush() + tea = trans.app.model.ToolEventAssociation( replace_version, event ) trans.sa_session.flush() try: os.link( uploaded_file.name, obj.file_name ) diff --git a/lib/galaxy/webapps/community/security/__init__.py b/lib/galaxy/webapps/community/security/__init__.py index f2babbdbb08..1ccfe6ccdc7 100644 --- a/lib/galaxy/webapps/community/security/__init__.py +++ b/lib/galaxy/webapps/community/security/__init__.py @@ -223,7 +223,7 @@ class CommunityRBACAgent( RBACAgent ): # or if the item's state is APPROVED. if user and user_is_admin and cntrller == 'admin': return True - if cntrller in [ 'tool' ] and item.is_approved: + if cntrller in [ 'tool' ] and item.is_approved or item.is_archived or item.is_deleted: return True return user and user==item.user def get_all_action_permissions( self, user, user_is_admin, cntrller, item ):