[hg] galaxy 3726: Community: Add tool versioning and tighten up ...
details: http://www.bx.psu.edu/hg/galaxy/rev/42f3f5d78f9a changeset: 3726:42f3f5d78f9a user: Nate Coraor <nate@bx.psu.edu> date: Fri Apr 30 16:11:17 2010 -0400 description: Community: Add tool versioning and tighten up the requirements in the upload/approval process diffstat: lib/galaxy/web/framework/__init__.py | 4 +- lib/galaxy/webapps/community/controllers/admin.py | 19 +- lib/galaxy/webapps/community/controllers/common.py | 28 ++- lib/galaxy/webapps/community/controllers/upload.py | 68 +++++++++- lib/galaxy/webapps/community/model/mapping.py | 9 +- lib/galaxy/webapps/community/model/migrate/versions/0001_initial_tables.py | 3 +- lib/galaxy/webapps/community/security/__init__.py | 6 + templates/webapps/community/tool/view_tool.mako | 49 ++++++- templates/webapps/community/upload/upload.mako | 3 + 9 files changed, 160 insertions(+), 29 deletions(-) diffs (410 lines): diff -r e71a3d03a529 -r 42f3f5d78f9a lib/galaxy/web/framework/__init__.py --- a/lib/galaxy/web/framework/__init__.py Fri Apr 30 15:45:32 2010 -0400 +++ b/lib/galaxy/web/framework/__init__.py Fri Apr 30 16:11:17 2010 -0400 @@ -66,7 +66,7 @@ decorator.exposed = True return decorator -def require_login( verb="perform this action", use_panels=False ): +def require_login( verb="perform this action", use_panels=False, webapp='galaxy' ): def argcatcher( func ): def decorator( self, trans, *args, **kwargs ): if trans.get_user(): @@ -74,7 +74,7 @@ else: return trans.show_error_message( 'You must be <a target="_top" href="%s">logged in</a> to %s</div>.' - % ( url_for( controller='user', action='login' ), verb ), use_panels=use_panels ) + % ( url_for( controller='user', action='login', webapp=webapp ), verb ), use_panels=use_panels ) return decorator return argcatcher diff -r e71a3d03a529 -r 42f3f5d78f9a lib/galaxy/webapps/community/controllers/admin.py --- a/lib/galaxy/webapps/community/controllers/admin.py Fri Apr 30 15:45:32 2010 -0400 +++ b/lib/galaxy/webapps/community/controllers/admin.py Fri Apr 30 16:11:17 2010 -0400 @@ -2,7 +2,7 @@ from galaxy.webapps.community import model from galaxy.model.orm import * from galaxy.web.framework.helpers import time_ago, iff, grids -from common import get_categories, get_category, get_tools, get_event, get_tool +from common import get_categories, get_category, get_tools, get_event, get_tool, get_versions import logging log = logging.getLogger( __name__ ) @@ -670,12 +670,18 @@ webapp = params.get( 'webapp', 'galaxy' ) message = util.restore_text( params.get( 'message', '' ) ) status = params.get( 'status', 'done' ) + redirect = params.get( 'no_redirect', True ) id = params.get( 'id', None ) if not id: message = "No tool id received for setting status" status = 'error' else: tool = get_tool( trans, id ) + if state == trans.app.model.Tool.states.APPROVED: + # If we're approving a tool, all previous versions must be set to archived + for version in get_versions( trans, tool ): + if version != tool and version.is_approved(): + self.set_tool_state( trans, trans.app.model.Tool.states.ARCHIVED, id=trans.app.security.encode_id( version.id ), redirect='False' ) event = trans.model.Event( state ) # Flush so we an get an id trans.sa_session.add( event ) @@ -684,11 +690,12 @@ trans.sa_session.add( tea ) trans.sa_session.flush() message = "State of tool '%s' is now %s" % ( tool.name, state ) - trans.response.send_redirect( web.url_for( controller='admin', - action='browse_tools', - webapp=webapp, - message=message, - status=status ) ) + if redirect: + trans.response.send_redirect( web.url_for( controller='admin', + action='browse_tools', + webapp=webapp, + message=message, + status=status ) ) @web.expose @web.require_admin def edit_category( self, trans, **kwd ): diff -r e71a3d03a529 -r 42f3f5d78f9a lib/galaxy/webapps/community/controllers/common.py --- a/lib/galaxy/webapps/community/controllers/common.py Fri Apr 30 15:45:32 2010 -0400 +++ b/lib/galaxy/webapps/community/controllers/common.py Fri Apr 30 16:11:17 2010 -0400 @@ -81,9 +81,11 @@ tool = get_tool( trans, id ) categories = [ tca.category for tca in tool.categories ] tool_file_contents = tarfile.open( tool.file_name, 'r' ).getnames() + versions = get_versions( trans, tool ) return trans.fill_template( '/webapps/community/tool/view_tool.mako', tool=tool, tool_file_contents=tool_file_contents, + versions=versions, categories=categories, cntrller=cntrller, message=message, @@ -100,14 +102,11 @@ message='Select a tool to to upload a new version', status='error' ) ) tool = get_tool( trans, id ) - if params.save_button and ( params.file_data != '' or params.url != '' ): - # TODO: call the upload method in the upload controller. - message = 'Uploading new version not implemented' - status = 'error' - return trans.response.send_redirect( web.url_for( controller=cntrller, - action='browse_tools', - message='Not yet implemented, sorry...', - status='error' ) ) + return trans.response.send_redirect( web.url_for( controller='upload', + action='upload', + message=message, + status=status, + replace_id=id ) ) @web.expose def browse_category( self, trans, cntrller, **kwd ): params = util.Params( kwd ) @@ -159,11 +158,22 @@ ## ---- Utility methods ------------------------------------------------------- +def get_versions( trans, tool ): + versions = [tool] + this_tool = tool + while tool.newer_version: + versions.insert( 0, tool.newer_version ) + tool = tool.newer_version + tool = this_tool + while tool.older_version: + versions.append( tool.older_version[0] ) + tool = tool.older_version[0] + return versions def get_categories( trans ): """Get all categories from the database""" return trans.sa_session.query( trans.model.Category ) \ .filter( trans.model.Category.table.c.deleted==False ) \ - .order_by( trans.model.Category.table.c.name ) + .order_by( trans.model.Category.table.c.name ).all() def get_unassociated_categories( trans, obj ): """Get all categories from the database that are not associated with obj""" # TODO: we currently assume we are setting a tool category, so this method may need diff -r e71a3d03a529 -r 42f3f5d78f9a lib/galaxy/webapps/community/controllers/upload.py --- a/lib/galaxy/webapps/community/controllers/upload.py Fri Apr 30 15:45:32 2010 -0400 +++ b/lib/galaxy/webapps/community/controllers/upload.py Fri Apr 30 16:11:17 2010 -0400 @@ -3,23 +3,36 @@ from galaxy.web.framework.helpers import time_ago, iff, grids from galaxy.model.orm import * from galaxy.webapps.community import datatypes -from common import get_categories, get_category +from common import get_categories, get_category, get_versions log = logging.getLogger( __name__ ) # States for passing messages SUCCESS, INFO, WARNING, ERROR = "done", "info", "warning", "error" +class UploadError( Exception ): + pass + class UploadController( BaseController ): @web.expose + @web.require_login( 'upload', use_panels=True, webapp='community' ) def upload( self, trans, **kwd ): params = util.Params( kwd ) message = util.restore_text( params.get( 'message', '' ) ) status = params.get( 'status', 'done' ) category_ids = util.listify( params.get( 'category_id', '' ) ) + replace_id = params.get( 'replace_id', None ) + replace_version = None uploaded_file = None - if params.file_data == '' and params.url.strip() == '': + categories = get_categories( trans ) + if not get_categories( trans ): + return trans.response.send_redirect( web.url_for( controller='tool', + action='browse_tools', + cntrller='tool', + message='No categories have been configured in this instance of the Galaxy Community. An administrator needs to create some via the Administrator control panel before anything can be uploaded', + status='error' ) ) + elif params.file_data == '' and params.url.strip() == '': message = 'No files were entered on the upload form.' status = 'error' elif params.file_data == '': @@ -47,6 +60,29 @@ obj = datatype.create_model_object( meta ) trans.sa_session.add( obj ) if isinstance( obj, trans.app.model.Tool ): + existing = trans.sa_session.query( trans.app.model.Tool ).filter_by( tool_id = meta.id ).all() + if existing and replace_id is None: + raise UploadError( 'A tool with the same ID already exists. If you are trying to update this tool to a new version, please use the upload form on the "Edit Tool" page. Otherwise, please choose a new ID.' ) + elif existing: + replace_version = trans.sa_session.query( trans.app.model.Tool ).get( int( trans.app.security.decode_id( replace_id ) ) ) + if replace_version.newer_version: + # If the user has picked an old version, switch to the newest version + replace_version = get_versions( trans, replace_version )[0] + if trans.user != replace_version.user: + raise UploadError( 'You are not the owner of this tool and may not upload new versions of it.' ) + if replace_version.tool_id != meta.id: + raise UploadError( 'The new tool id (%s) does not match the old tool id (%s). Please check the tool XML file' % ( meta.id, replace_version.tool_id ) ) + for old_version in get_versions( trans, replace_version ): + if old_version.version == meta.version: + raise UploadError( 'The new version (%s) matches an old version. Please check your version in the tool XML file' % meta.version ) + if old_version.is_new(): + raise UploadError( 'There is an existing version of this tool which is unsubmitted. Please either <a href="%s">submit or delete it</a> before uploading a new version.' % url_for( controller='common', + action='view_tool', + cntrller='tool', + id=trans.app.security.encode_id( old_version.id ) ) ) + if old_version.is_waiting(): + raise UploadError( 'There is an existing version of this tool which is waiting for administrative approval. Please contact an administrator for help.' ) + # Defer setting the id since the newer version id doesn't exist until the new Tool object is flushed if category_ids: for category_id in category_ids: category = trans.app.model.Category.get( trans.security.decode_id( category_id ) ) @@ -58,6 +94,9 @@ tea = trans.app.model.ToolEventAssociation( obj, event ) trans.sa_session.add_all( ( event, tea ) ) trans.sa_session.flush() + if replace_version and replace_id: + replace_version.newer_version_id = obj.id + trans.sa_session.flush() try: os.link( uploaded_file.name, obj.file_name ) except OSError: @@ -69,13 +108,29 @@ id=trans.app.security.encode_id( obj.id ), message='Uploaded %s' % meta.message, status='done' ) ) - except datatypes.DatatypeVerificationError, e: + except ( datatypes.DatatypeVerificationError, UploadError ), e: message = str( e ) status = 'error' - except sqlalchemy.exc.IntegrityError: - message = 'A tool with the same ID already exists. If you are trying to update this tool to a new version, please use the upload form on the "Edit Tool" page. Otherwise, please choose a new ID.' - status = 'error' uploaded_file.close() + elif replace_id is not None: + replace_version = trans.sa_session.query( trans.app.model.Tool ).get( int( trans.app.security.decode_id( replace_id ) ) ) + old_version = None + for old_version in get_versions( trans, replace_version ): + if old_version.is_new(): + message = 'There is an existing version of this tool which is unsubmitted. Please either submit or delete it before uploading a new version.' + break + if old_version.is_waiting(): + message = 'There is an existing version of this tool which is waiting for administrative approval. Please contact an administrator for help.' + break + else: + old_version = None + if old_version is not None: + return trans.response.send_redirect( web.url_for( controller='common', + action='view_tool', + cntrller='tool', + id=trans.app.security.encode_id( old_version.id ), + message=message, + status='error' ) ) selected_upload_type = params.get( 'type', 'tool' ) selected_categories = [ trans.security.decode_id( id ) for id in category_ids ] return trans.fill_template( '/webapps/community/upload/upload.mako', @@ -83,5 +138,6 @@ status=status, selected_upload_type=selected_upload_type, upload_types=trans.app.datatypes_registry.get_datatypes_for_select_list(), + replace_id=replace_id, selected_categories=selected_categories, categories=get_categories( trans ) ) diff -r e71a3d03a529 -r 42f3f5d78f9a lib/galaxy/webapps/community/model/mapping.py --- a/lib/galaxy/webapps/community/model/mapping.py Fri Apr 30 15:45:32 2010 -0400 +++ b/lib/galaxy/webapps/community/model/mapping.py Fri Apr 30 16:11:17 2010 -0400 @@ -103,9 +103,10 @@ Tool.table = Table( "tool", metadata, Column( "id", Integer, primary_key=True ), Column( "guid", TrimmedString( 255 ), index=True, unique=True ), - Column( "tool_id", TrimmedString( 255 ), index=True, unique=True ), + Column( "tool_id", TrimmedString( 255 ), index=True ), Column( "create_time", DateTime, default=now ), Column( "update_time", DateTime, default=now, onupdate=now ), + Column( "newer_version_id", Integer, ForeignKey( "tool.id" ), nullable=True ), Column( "name", TrimmedString( 255 ), index=True ), Column( "description" , TEXT ), Column( "user_description" , TEXT ), @@ -215,7 +216,11 @@ properties = dict( categories=relation( ToolCategoryAssociation ), events=relation( ToolEventAssociation ), - user=relation( User.mapper ) + user=relation( User.mapper ), + older_version=relation( + Tool, + primaryjoin=( Tool.table.c.newer_version_id == Tool.table.c.id ), + backref=backref( "newer_version", primaryjoin=( Tool.table.c.newer_version_id == Tool.table.c.id ), remote_side=[Tool.table.c.id] ) ) ) ) assign_mapper( context, Event, Event.table, diff -r e71a3d03a529 -r 42f3f5d78f9a lib/galaxy/webapps/community/model/migrate/versions/0001_initial_tables.py --- a/lib/galaxy/webapps/community/model/migrate/versions/0001_initial_tables.py Fri Apr 30 15:45:32 2010 -0400 +++ b/lib/galaxy/webapps/community/model/migrate/versions/0001_initial_tables.py Fri Apr 30 16:11:17 2010 -0400 @@ -80,9 +80,10 @@ Tool_table = Table( "tool", metadata, Column( "id", Integer, primary_key=True ), Column( "guid", TrimmedString( 255 ), index=True, unique=True ), - Column( "tool_id", TrimmedString( 255 ), index=True, unique=True ), + Column( "tool_id", TrimmedString( 255 ), index=True ), Column( "create_time", DateTime, default=now ), Column( "update_time", DateTime, default=now, onupdate=now ), + Column( "newer_version_id", Integer, ForeignKey( "tool.id" ), nullable=True ), Column( "name", TrimmedString( 255 ), index=True ), Column( "description" , TEXT ), Column( "user_description" , TEXT ), diff -r e71a3d03a529 -r 42f3f5d78f9a lib/galaxy/webapps/community/security/__init__.py --- a/lib/galaxy/webapps/community/security/__init__.py Fri Apr 30 15:45:32 2010 -0400 +++ b/lib/galaxy/webapps/community/security/__init__.py Fri Apr 30 16:11:17 2010 -0400 @@ -167,6 +167,12 @@ # We currently assume the current user can edit the item if they are the owner (i.e., they # uploaded the item), and the item is in a NEW state. return user and user==item.user and item.is_new() + def can_upload_new_version( self, user, item, versions ): + state_ok = True + for version in versions: + if version.is_new() or version.is_approved(): + state_ok = False + return user and user==item.user and state_ok def get_permitted_actions( filter=None ): '''Utility method to return a subset of RBACAgent's permitted actions''' diff -r e71a3d03a529 -r 42f3f5d78f9a templates/webapps/community/tool/view_tool.mako --- a/templates/webapps/community/tool/view_tool.mako Fri Apr 30 15:45:32 2010 -0400 +++ b/templates/webapps/community/tool/view_tool.mako Fri Apr 30 16:11:17 2010 -0400 @@ -6,6 +6,12 @@ if cntrller in [ 'tool' ]: can_edit = trans.app.security_agent.can_edit_item( trans.user, tool ) + can_upload_new_version = trans.app.security_agent.can_upload_new_version( trans.user, tool, versions ) + + visible_versions = [] + for version in versions: + if version.is_approved() or version.is_archived() or version.user == trans.user: + visible_versions.append( version ) %> <%! @@ -49,10 +55,26 @@ <h2>View Tool: ${tool.name} <em>${tool.description}</em></h2> +%if tool.is_approved(): + <b><i>This is the latest approved version of this tool</i></b> +%elif tool.is_deleted(): + <font color="red"><b><i>This is a deleted version of this tool</i></b></font> +%elif tool.is_archived(): + <font color="red"><b><i>This is an archived version of this tool</i></b></font> +%elif tool.is_new(): + <font color="red"><b><i>This is an unsubmitted version of this tool</i></b></font> +%elif tool.is_waiting(): + <font color="red"><b><i>This version of this tool is awaiting administrative approval</i></b></font> +%elif tool.is_rejected(): + <font color="red"><b><i>This version of this tool has been rejected by an administrator</i></b></font> +%endif +<p/> + %if cntrller=='admin' and tool.is_waiting(): <p> <ul class="manage-table-actions"> <li><a class="action-button" href="${h.url_for( controller='admin', action='set_tool_state', state=trans.model.Tool.states.APPROVED, id=trans.security.encode_id( tool.id ), cntrller=cntrller )}"><span>Approve</span></a></li> + <li><a class="action-button" href="${h.url_for( controller='admin', action='set_tool_state', state=trans.model.Tool.states.REJECTED, id=trans.security.encode_id( tool.id ), cntrller=cntrller )}"><span>Reject</span></a></li> </ul> </p> %endif @@ -67,6 +89,8 @@ <div popupmenu="tool-${tool.id}-popup"> %if cntrller=='admin' or can_edit: <a class="action-button" href="${h.url_for( controller='common', action='edit_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Edit information</a> + %endif + %if cntrller=='admin' or can_upload_new_version: <a class="action-button" href="${h.url_for( controller='common', action='upload_new_tool_version', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Upload a new version</a> %endif <a class="action-button" href="${h.url_for( controller='tool', action='download_tool', id=trans.app.security.encode_id( tool.id ) )}">Download tool</a> @@ -100,11 +124,30 @@ </div> <div class="form-row"> <label>Categories:</label> - %for category in categories: - ${category.name} - %endfor + %if categories: + %for category in categories: + ${category.name} + %endfor + %else: + none set + %endif <div style="clear: both"></div> </div> + %if len( visible_versions ) > 1: + <div class="form-row"> + <label>All Versions:</label> + <ul> + %for version in visible_versions: + %if version == tool: + <li><strong>${version.version} (this version)</strong></li> + %else: + <li><a href="${h.url_for( controller='common', action='view_tool', id=trans.app.security.encode_id( version.id ), cntrller=cntrller )}">${version.version}</a></li> + %endif + %endfor + </ul> + <div style="clear: both"></div> + </div> + %endif </div> </div> diff -r e71a3d03a529 -r 42f3f5d78f9a templates/webapps/community/upload/upload.mako --- a/templates/webapps/community/upload/upload.mako Fri Apr 30 15:45:32 2010 -0400 +++ b/templates/webapps/community/upload/upload.mako Fri Apr 30 16:11:17 2010 -0400 @@ -22,6 +22,9 @@ <div class="toolFormBody"> ## TODO: nginx <form id="upload_form" name="upload_form" action="${h.url_for( controller='upload', action='upload' )}" enctype="multipart/form-data" method="post"> + %if replace_id is not None: + <input type='hidden' name="replace_id" value="${replace_id}"/> + %endif <div class="form-row"> <label>Upload Type</label> <div class="form-row-input">
participants (1)
-
Nate Coraor