galaxy-dist commit 8f2e44ed27f6: Community webapp features and fixes

16 Jul 2010

# HG changeset patch -- Bitbucket.org
# Project galaxy-dist
# URL http://bitbucket.org/galaxy/galaxy-dist/overview
# User Greg Von Kuster <greg@bx.psu.edu>
# Date 1278081407 14400
# Node ID 8f2e44ed27f6a17586854c8e423d2b7b42ca0ae1
# Parent  692458d15b9211ba7e6569e07b350ef2c3c9757a
Community webapp features and fixes

Fixes:
1. Can no longer upload a tool with a different tool_id than the one being replaced when uploading a new version of an existing tool
2. Fixed a bug in the base Admin controller that threw an exception rather than displaying a user's tool when clicking in grid links
3. Fixed bugs in the ToolListGrid class of the common controller that resulted in incorrect query filters being applied
4. Fixed a bug in the ToolListGrid initial query that did not return rows if tools were not associated with categories

New features:
0. Added stringent security to the templates and methods for performing actions
1. When an admin rejects a tool, they are required to add a reason for the rejection. ¬†The user that uploaded the tool can then make a correction and re-submit it for approval.
2. A new tool menu option is available for viewing the history of a tool.
3. Significantly improved GUI flow
4. An admin is now allowed to purge a tool, which removes records from the database, so should be used only when absolutely necessary.
5. Made the User Description field required when the tool is submitted for approval.
6. Tools can now be downloaded from the admin view as well as the tool view.

--- a/lib/galaxy/webapps/community/security/__init__.py
+++ b/lib/galaxy/webapps/community/security/__init__.py
@@ -6,6 +6,7 @@ from datetime import datetime, timedelta
 from galaxy.util.bunch import Bunch
 from galaxy.util import listify
 from galaxy.model.orm import *
+from galaxy.webapps.community.controllers.common import get_versions
 
 log = logging.getLogger(__name__)
 
@@ -163,16 +164,83 @@ class CommunityRBACAgent( RBACAgent ):
             self.sa_session.refresh( tool )
             for category in categories:
                 self.associate_components( tool=tool, category=category )
-    def can_edit_item( self, user, item ):
-        # 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 ):
+    def can_approve_or_reject( self, user, user_is_admin, cntrller, item ):
+        # The current user can approve or reject the item if the user
+        # is an admin, and the item's state is WAITING.
+        return user and user_is_admin and cntrller=='admin' and item.is_waiting()
+    def can_delete( self, user, user_is_admin, cntrller, item ):
+        # The current user can delete the item if they are an admin or if they uploaded the
+        # item and in either case the item's state is not DELETED.
+        if user and user_is_admin and cntrller == 'admin':
+            can_delete = not item.is_deleted()
+        elif cntrller in [ 'tool' ]:
+            can_delete = user==item.user and not item.is_deleted()
+        else:
+            can_delete = False
+        return can_delete
+    def can_download( self, user, user_is_admin, cntrller, item ):
+        # The current user can download the item if they are an admin or if the
+        # item's state is not one of: NEW, WAITING.
+        if user and user_is_admin and cntrller == 'admin':
+            return True
+        elif cntrller in [ 'tool' ]:
+            can_download = not( item.is_new() or item.is_waiting() )
+        else:
+            can_download = False
+        return can_download
+    def can_edit( self, user, user_is_admin, cntrller, item ):
+        # The current user can edit the item if they are an admin or if they uploaded the item
+        # and the item's state is one of: NEW, REJECTED.
+        if user and user_is_admin and cntrller == 'admin':
+            return True
+        if cntrller in [ 'tool' ]:
+            return user and user==item.user and ( item.is_new() or item.is_rejected() )
+        return False
+    def can_purge( self, user, user_is_admin, cntrller ):
+        # The current user can purge the item if they are an admin.
+        return user and user_is_admin and cntrller == 'admin'
+    def can_upload_new_version( self, user, item ):
+        # The current user can upload a new version as long as the item's state is not NEW or WAITING.
+        if not user:
+            return False
+        versions = get_versions( item )
         state_ok = True
         for version in versions:
             if version.is_new() or version.is_waiting():
                 state_ok = False
-        return user and user==item.user and state_ok
+                break
+        return state_ok
+    def can_view( self, user, user_is_admin, cntrller, item ):
+        # The current user can view the item if they are an admin or if they uploaded the item
+        # 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():
+            return True
+        return user and user==item.user
+    def get_all_action_permissions( self, user, user_is_admin, cntrller, item ):
+        """Get all permitted actions on item for the current user"""
+        can_edit = self.can_edit( cntrller, user, user_is_admin, item )
+        can_view = self.can_view( cntrller, user, user_is_admin, item )
+        can_upload_new_version = self.can_upload_new_version( user, item )
+        visible_versions = self.get_visible_versions( user, user_is_admin, cntrller, item )
+        can_approve_or_reject = self.can_approve_or_reject( user, user_is_admin, cntrller, item )
+        can_delete = self.can_delete( user, user_is_admin, cntrller, item )
+        return can_edit, can_view, can_upload_new_version, can_delete, visible_versions, can_approve_or_reject
+    def get_visible_versions( self, user, user_is_admin, cntrller, item ):
+        # All previous versions of item can be displayed if the current user is an admin
+        # or they uploaded item.  Otherwise, only versions whose state is APPROVED or 
+        # ARCHIVED will be displayed.
+        if user and user_is_admin and cntrller == 'admin':
+            visible_versions = get_versions( item )
+        elif cntrller in [ 'tool' ]:
+            visible_versions = []
+            for version in get_versions( item ):
+                if version.is_approved() or version.is_archived() or version.user == user:
+                    visible_versions.append( version )
+        else:
+           visible_versions = []
+        return visible_versions 
 
 def get_permitted_actions( filter=None ):
     '''Utility method to return a subset of RBACAgent's permitted actions'''

--- a/lib/galaxy/web/base/controller.py
+++ b/lib/galaxy/web/base/controller.py
@@ -1251,7 +1251,9 @@ class Admin( object ):
                 # sort filter instead of this class's.
                 kwargs[ 'user_id' ] = kwargs[ 'id' ]
                 kwargs[ 'sort' ] = 'name'
-                return self.browse_tools( trans, **kwargs )
+                return trans.response.send_redirect( web.url_for( controller='admin',
+                                                                  action='browse_tools',
+                                                                  **kwargs ) )
         # Render the list view
         return self.user_list_grid( trans, **kwargs )
     @web.expose

--- a/lib/galaxy/webapps/community/controllers/common.py
+++ b/lib/galaxy/webapps/community/controllers/common.py
@@ -22,14 +22,15 @@ class ToolListGrid( grids.Grid ):
             return tool.description
     class CategoryColumn( grids.TextColumn ):
         def get_value( self, trans, grid, tool ):
+            rval = '<ul>'
             if tool.categories:
-                rval = '<ul>'
                 for tca in tool.categories:
                     rval += '<li><a href="browse_tools?operation=tools_by_category&id=%s&webapp=community">%s</a></li>' \
                         % ( trans.security.encode_id( tca.category.id ), tca.category.name )
-                rval += '</ul>'
-                return rval
-            return 'not set'
+            else:
+                rval += '<li>not set</li>'
+            rval += '</ul>'
+            return rval
     class ToolCategoryColumn( grids.GridColumn ):
         def filter( self, trans, user, query, column_filter ):
             """Modify query to filter by category."""
@@ -55,9 +56,9 @@ class ToolListGrid( grids.Grid ):
     columns = [
         NameColumn( "Name",
                     key="name",
-                    link=( lambda item: dict( operation="View Tool", id=item.id, webapp="community" ) ),
+                    link=( lambda item: dict( operation="view_tool", id=item.id, webapp="community" ) ),
                     model_class=model.Tool,
-                    attach_popup=True
+                    attach_popup=False
                     ),
         VersionColumn( "Version",
                        key="version",
@@ -93,12 +94,7 @@ class ToolListGrid( grids.Grid ):
                                                 key="free-text-search",
                                                 visible=False,
                                                 filterable="standard" ) )
-    operations = [
-        grids.GridOperation( "Download tool",
-                             condition=( lambda item: not item.deleted ),
-                             allow_multiple=False,
-                             url_args=dict( controller="tool", action="download_tool", cntrller="tool", webapp="community" ) )
-        ]
+    operations = []
     standard_filters = []
     default_filter = {}
     num_rows_per_page = 50
@@ -108,8 +104,8 @@ class ToolListGrid( grids.Grid ):
         return trans.sa_session.query( self.model_class ) \
                                .join( model.ToolEventAssociation.table ) \
                                .join( model.Event.table ) \
-                               .join( model.ToolCategoryAssociation.table ) \
-                               .join( model.Category.table )
+                               .outerjoin( model.ToolCategoryAssociation.table ) \
+                               .outerjoin( model.Category.table )
 
 class CategoryListGrid( grids.Grid ):
     class NameColumn( grids.TextColumn ):
@@ -171,6 +167,12 @@ class CommonController( BaseController )
                                                               message='Select a tool to edit',
                                                               status='error' ) )
         tool = get_tool( trans, id )
+        can_edit = trans.app.security_agent.can_edit( trans.user, trans.user_is_admin(), cntrller, tool )
+        if not can_edit:
+            return trans.response.send_redirect( web.url_for( controller=cntrller,
+                                                              action='browse_tools',
+                                                              message='You are not allowed to edit this tool',
+                                                              status='error' ) )
         if params.get( 'edit_tool_button', False ):
             if params.get( 'in_categories', False ):
                 in_categories = [ trans.sa_session.query( trans.app.model.Category ).get( x ) for x in util.listify( params.in_categories ) ]
@@ -185,7 +187,7 @@ class CommonController( BaseController )
                 tool.user_description = ''
             trans.sa_session.add( tool )
             trans.sa_session.flush()
-            message="Tool '%s' description and category associations have been saved" % tool.name
+            message = "Tool '%s' description and category associations have been saved" % tool.name
             return trans.response.send_redirect( web.url_for( controller='common',
                                                               action='edit_tool',
                                                               cntrller=cntrller,
@@ -193,31 +195,33 @@ class CommonController( BaseController )
                                                               message=message,
                                                               status='done' ) )
         elif params.get( 'approval_button', False ):
-            if params.get( 'in_categories', False ):
-                in_categories = [ trans.sa_session.query( trans.app.model.Category ).get( x ) for x in util.listify( params.in_categories ) ]
-                trans.app.security_agent.set_entity_category_associations( tools=[ tool ], categories=in_categories )
-            else:
-                # There must not be any categories associated with the tool
-                trans.app.security_agent.set_entity_category_associations( tools=[ tool ], categories=[] )
             user_description = util.restore_text( params.get( 'user_description', '' ) )
             if user_description:
                 tool.user_description = user_description
+                if params.get( 'in_categories', False ):
+                    in_categories = [ trans.sa_session.query( trans.app.model.Category ).get( x ) for x in util.listify( params.in_categories ) ]
+                    trans.app.security_agent.set_entity_category_associations( tools=[ tool ], categories=in_categories )
+                else:
+                    # There must not be any categories associated with the tool
+                    trans.app.security_agent.set_entity_category_associations( tools=[ tool ], categories=[] )
+                trans.sa_session.add( tool )
+                trans.sa_session.flush()
+                # Move the state from NEW to WAITING
+                event = trans.app.model.Event( state=trans.app.model.Tool.states.WAITING )
+                tea = trans.app.model.ToolEventAssociation( tool, event )
+                trans.sa_session.add_all( ( event, tea ) )
+                trans.sa_session.flush()
+                message = "Tool '%s' has been submitted for approval and can no longer be modified" % ( tool.name )
+                return trans.response.send_redirect( web.url_for( controller='common',
+                                                                  action='view_tool',
+                                                                  cntrller=cntrller,
+                                                                  id=id,
+                                                                  message=message,
+                                                                  status='done' ) )
             else:
-                tool.user_description = ''
-            trans.sa_session.add( tool )
-            trans.sa_session.flush()
-            # Move the state from NEW to WAITING
-            event = trans.app.model.Event( state=trans.app.model.Tool.states.WAITING )
-            tea = trans.app.model.ToolEventAssociation( tool, event )
-            trans.sa_session.add_all( ( event, tea ) )
-            trans.sa_session.flush()
-            message = "Tool '%s' has been submitted for approval and can no longer be modified" % ( tool.name )
-            return trans.response.send_redirect( web.url_for( controller='common',
-                                                              action='view_tool',
-                                                              cntrller=cntrller,
-                                                              id=id,
-                                                              message=message,
-                                                              status='done' ) )
+                # The user_description field is required when submitting for approval
+                message = 'A user description is required prior to approval.'
+                status = 'error'
         in_categories = []
         out_categories = []
         for category in get_categories( trans ):
@@ -225,12 +229,31 @@ class CommonController( BaseController )
                 in_categories.append( ( category.id, category.name ) )
             else:
                 out_categories.append( ( category.id, category.name ) )
+        if tool.is_rejected():
+            # Include the comments regarding the reason for rejection
+            reason_for_rejection = get_most_recent_event( tool ).comment
+        else:
+            reason_for_rejection = ''
+        can_approve_or_reject = trans.app.security_agent.can_approve_or_reject( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_delete = trans.app.security_agent.can_delete( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_download = trans.app.security_agent.can_download( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_purge = trans.app.security_agent.can_purge( trans.user, trans.user_is_admin(), cntrller )
+        can_upload_new_version = trans.app.security_agent.can_upload_new_version( trans.user, tool )
+        can_view = trans.app.security_agent.can_view( trans.user, trans.user_is_admin(), cntrller, tool )
         return trans.fill_template( '/webapps/community/tool/edit_tool.mako',
                                     cntrller=cntrller,
                                     tool=tool,
                                     id=id,
                                     in_categories=in_categories,
                                     out_categories=out_categories,
+                                    can_approve_or_reject=can_approve_or_reject,
+                                    can_delete=can_delete,
+                                    can_download=can_download,
+                                    can_edit=can_edit,
+                                    can_purge=can_purge,
+                                    can_upload_new_version=can_upload_new_version,
+                                    can_view=can_view,
+                                    reason_for_rejection=reason_for_rejection,
                                     message=message,
                                     status=status )
     @web.expose
@@ -245,15 +268,40 @@ class CommonController( BaseController )
                                                               message='Select a tool to view',
                                                               status='error' ) )
         tool = get_tool( trans, id )
+        can_view = trans.app.security_agent.can_view( trans.user, trans.user_is_admin(), cntrller, tool )
+        if not can_view:
+            return trans.response.send_redirect( web.url_for( controller=cntrller,
+                                                              action='browse_tools',
+                                                              message='You are not allowed to view this tool',
+                                                              status='error' ) )
+        can_approve_or_reject = trans.app.security_agent.can_approve_or_reject( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_delete = trans.app.security_agent.can_delete( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_download = trans.app.security_agent.can_download( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_edit = trans.app.security_agent.can_edit( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_purge = trans.app.security_agent.can_purge( trans.user, trans.user_is_admin(), cntrller )
+        can_upload_new_version = trans.app.security_agent.can_upload_new_version( trans.user, tool )
+        visible_versions = trans.app.security_agent.get_visible_versions( trans.user, trans.user_is_admin(), cntrller, tool )
         categories = [ tca.category for tca in tool.categories ]
         tool_file_contents = tarfile.open( tool.file_name, 'r' ).getnames()
-        versions = get_versions( trans, tool )
+        if tool.is_rejected():
+            # Include the comments regarding the reason for rejection
+            reason_for_rejection = get_most_recent_event( tool ).comment
+        else:
+            reason_for_rejection = ''
         return trans.fill_template( '/webapps/community/tool/view_tool.mako',
                                     tool=tool,
                                     tool_file_contents=tool_file_contents,
-                                    versions=versions,
                                     categories=categories,
                                     cntrller=cntrller,
+                                    can_approve_or_reject=can_approve_or_reject,
+                                    can_delete=can_delete,
+                                    can_download=can_download,
+                                    can_edit=can_edit,
+                                    can_purge=can_purge,
+                                    can_upload_new_version=can_upload_new_version,
+                                    can_view=can_view,
+                                    visible_versions=visible_versions,
+                                    reason_for_rejection=reason_for_rejection,
                                     message=message,
                                     status=status )
     @web.expose
@@ -267,6 +315,11 @@ class CommonController( BaseController )
             status='error'
         else:
             tool = get_tool( trans, id )
+            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',
+                                                                  message='You are not allowed to delete this tool',
+                                                                  status='error' ) )
             # Create a new event
             event = trans.model.Event( state=trans.model.Tool.states.DELETED )
             # Flush so we can get an event id
@@ -279,13 +332,32 @@ class CommonController( BaseController )
             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"
+            message = "Tool '%s' has been marked deleted" % tool.name
             status = 'done'
         return trans.response.send_redirect( web.url_for( controller=cntrller,
                                                           action='browse_tools',
                                                           message=message,
                                                           status=status ) )
     @web.expose
+    def download_tool( self, trans, cntrller, **kwd ):
+        params = util.Params( kwd )
+        id = params.get( 'id', None )
+        if not id:
+            return trans.response.send_redirect( web.url_for( controller='tool',
+                                                              action='browse_tools',
+                                                              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',
+                                                              message='You are not allowed to download this tool',
+                                                              status='error' ) )
+        trans.response.set_content_type( tool.mimetype )
+        trans.response.headers['Content-Length'] = int( os.stat( tool.file_name ).st_size )
+        trans.response.headers['Content-Disposition'] = 'attachment; filename=%s' % tool.download_file_name
+        return open( tool.file_name )
+    @web.expose
     def upload_new_tool_version( self, trans, cntrller, **kwd ):
         params = util.Params( kwd )
         message = util.restore_text( params.get( 'message', ''  ) )
@@ -294,27 +366,69 @@ class CommonController( BaseController )
         if not id:
             return trans.response.send_redirect( web.url_for( controller=cntrller,
                                                               action='browse_tools',
-                                                              message='Select a tool to to upload a new version',
+                                                              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',
+                                                              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',
                                                           action='upload',
                                                           message=message,
                                                           status=status,
                                                           replace_id=id ) )
+    @web.expose
+    @web.require_login( "view tool history" )
+    def view_tool_history( self, trans, cntrller, **kwd ):
+        params = util.Params( kwd )
+        message = util.restore_text( params.get( 'message', ''  ) )
+        status = params.get( 'status', 'done' )
+        id = params.get( 'id', None )
+        if not id:
+            return trans.response.send_redirect( web.url_for( controller=cntrller,
+                                                              action='browse_tools',
+                                                              message='Select a tool to view events',
+                                                              status='error' ) )
+        tool = get_tool( trans, id )
+        can_view = trans.app.security_agent.can_view( trans.user, trans.user_is_admin(), cntrller, tool )
+        if not can_view:
+            return trans.response.send_redirect( web.url_for( controller=cntrller,
+                                                              action='browse_tools',
+                                                              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 )
+        can_edit = trans.app.security_agent.can_edit( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_delete = trans.app.security_agent.can_delete( trans.user, trans.user_is_admin(), cntrller, tool )
+        can_download = trans.app.security_agent.can_download( trans.user, trans.user_is_admin(), cntrller, tool )
+        events = [ tea.event for tea in tool.events ]
+        events = [ ( event.state, time_ago( event.update_time ), event.comment ) for event in events ]
+        return trans.fill_template( '/webapps/community/common/view_tool_history.mako', 
+                                    cntrller=cntrller,
+                                    events=events,
+                                    tool=tool,
+                                    can_approve_or_reject=can_approve_or_reject,
+                                    can_edit=can_edit,
+                                    can_delete=can_delete,
+                                    can_download=can_download,
+                                    can_view=can_view,
+                                    message=message,
+                                    status=status )
 
 ## ---- 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]
+def get_versions( item ):
+    """Get all versions of item"""
+    versions = [item]
+    this_item = item
+    while item.newer_version:
+        versions.insert( 0, item.newer_version )
+        item = item.newer_version
+    item = this_item
+    while item.older_version:
+        versions.append( item.older_version[0] )
+        item = item.older_version[0]
     return versions
 def get_categories( trans ):
     """Get all categories from the database"""
@@ -322,15 +436,27 @@ def get_categories( trans ):
                            .filter( trans.model.Category.table.c.deleted==False ) \
                            .order_by( trans.model.Category.table.c.name ).all()
 def get_category( trans, id ):
+    """Get a category from the database"""
     return trans.sa_session.query( trans.model.Category ).get( trans.security.decode_id( id ) )
 def get_tool( trans, id ):
+    """Get a tool from the database"""
     return trans.sa_session.query( trans.model.Tool ).get( trans.app.security.decode_id( id ) )
 def get_tools( trans ):
-    # Return only the latest version of each tool
+    """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_event( trans, id ):
+    """Get an event from the databse"""
     return trans.sa_session.query( trans.model.Event ).get( trans.security.decode_id( id ) )
+def get_most_recent_event( item ):
+    """Get the most recent event for item"""
+    if item.events:
+        # Sort the events in ascending order by update_time
+        events = model.sort_by_attr( [ item_event_assoc.event for item_event_assoc in item.events ], 'update_time' )
+        # Get the last event that occurred
+        return events[-1]
+    return None
 def get_user( trans, id ):
+    """Get a user from the database"""
     return trans.sa_session.query( trans.model.User ).get( trans.security.decode_id( id ) )

--- /dev/null
+++ b/templates/webapps/community/admin/reject_tool.mako
@@ -0,0 +1,70 @@
+<%namespace file="/message.mako" import="render_msg" />
+
+<%!
+   def inherit(context):
+       if context.get('use_panels'):
+           return '/webapps/community/base_panels.mako'
+       else:
+           return '/base.mako'
+%>
+<%inherit file="${inherit(context)}"/>
+
+<%def name="title()">Reject Tool</%def>
+
+<h2>Reject Tool</h2>
+
+<ul class="manage-table-actions">
+    <li><a class="action-button" id="tool-${tool.id}-popup" class="menubutton">Tool Actions</a></li>
+    <div popupmenu="tool-${tool.id}-popup">
+        <a class="action-button" href="${h.url_for( controller='common', action='view_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">View tool</a>
+        <a class="action-button" href="${h.url_for( controller='common', action='view_tool_history', id=trans.security.encode_id( tool.id ), cntrller=cntrller )}">Tool history</a>
+        <a class="action-button" href="${h.url_for( controller='common', action='download_tool', id=trans.security.encode_id( tool.id ), cntrller=cntrller )}">Download tool</a>
+    </div>
+</ul>
+
+%if message:
+    ${render_msg( message, status )}
+%endif
+
+<div class="toolForm">
+    <div class="toolFormTitle">${tool.name}</div>
+        <form name="reject_tool" action="${h.url_for( controller='admin', action='reject_tool', id=trans.security.encode_id( tool.id ) )}" method="post" >
+            <div class="form-row">
+                <label>Tool id:</label>
+                ${tool.tool_id}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Version:</label>
+                ${tool.version}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Description:</label>
+                ${tool.description}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>User description:</label>
+                ${tool.user_description}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Uploaded by:</label>
+                ${tool.user.username}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Reason for rejection</label>
+                <textarea name="comments" rows="5" cols="40"></textarea>
+                <div class="toolParamHelp" style="clear: both;">
+                    Required
+                </div>
+            </div>
+            <div class="form-row">
+                <input type="submit" name="reject_button" value="Reject"/>
+                <input type="submit" name="cancel_reject_button" value="Cancel"/>
+            </div>
+        </form>
+    </div>
+</div>

--- a/lib/galaxy/webapps/community/controllers/upload.py
+++ b/lib/galaxy/webapps/community/controllers/upload.py
@@ -33,7 +33,6 @@ class UploadController( BaseController )
                                                               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' ) )
         if params.get( 'upload_button', False ):
-
             url_paste = params.get( 'url', '' ).strip()
             file_data = params.get( 'file_data', '' )
             if file_data == '' and url_paste == '':
@@ -66,19 +65,24 @@ class UploadController( BaseController )
                         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 ).first()
-                            if existing and replace_id is None:
+                            existing = trans.sa_session.query( trans.app.model.Tool ) \
+                                                       .filter_by( tool_id = meta.id ) \
+                                                       .first()
+                            if replace_id:
+                                replace_version = trans.sa_session.query( trans.app.model.Tool ).get( trans.security.decode_id( replace_id ) )
+                            if existing and not replace_id:
                                 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( trans.security.decode_id( replace_id ) )
+                            elif replace_id and not existing:
+                                raise UploadError( 'Tool ids must match when uploading a new version of a tool.  The new tool id does not match the old tool id (%s).  Check the tool XML files.' % str( replace_version.tool_id ) )
+                            elif existing and 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]
+                                    replace_version = get_versions( replace_version )[0]
                                 if replace_version.tool_id != meta.id:
-                                    raise UploadError( 'The new tool id (%s) does not match the old tool id (%s).  Check the tool XML file' % ( meta.id, replace_version.tool_id ) )
-                                for old_version in get_versions( trans, replace_version ):
+                                    raise UploadError( 'Tool ids must match when uploading a new version of a tool.  The new tool id (%s) does not match the old tool id (%s).  Check the tool XML files.' % ( str( meta.id ), str( replace_version.tool_id ) ) )
+                                for old_version in get_versions( replace_version ):
                                     if old_version.version == meta.version:
-                                        raise UploadError( 'The new version (%s) matches an old version.  Check your version in the tool XML file' % meta.version )
+                                        raise UploadError( 'The new version (%s) matches an old version.  Check your version in the tool XML file.' % str( meta.version ) )
                                     if old_version.is_new():
                                         raise UploadError( 'There is an existing version of this tool which has not yet been submitted for approval, so either <a href="%s">submit or delete it</a> before uploading a new version.' % url_for( controller='common',
                                                                                                                                                                                                                                                 action='view_tool',
@@ -86,7 +90,7 @@ class UploadController( BaseController )
                                                                                                                                                                                                                                                 id=trans.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, so contact an administrator for help.' )
-                                # Defer setting the id since the newer version id doesn't exist until the new Tool object is flushed
+                                    # 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 ) )
@@ -123,7 +127,7 @@ class UploadController( BaseController )
             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 ):
+                for old_version in get_versions( replace_version ):
                     if old_version.is_new():
                         message = 'There is an existing version of this tool which has not been submitted for approval, so either submit or delete it before uploading a new version.'
                         break

--- a/templates/webapps/community/category/create_category.mako
+++ b/templates/webapps/community/category/create_category.mako
@@ -19,13 +19,12 @@
     <div class="toolFormBody"><form name="create_category_form" id="create_category_form" action="${h.url_for( action='create_category' )}" method="post" ><div class="form-row">
-                <input  name="webapp" type="hidden" value="${webapp}" size=40"/><label>Name:</label>
-                <input  name="name" type="textfield" value="" size=40"/>
+                <input  name="name" type="textfield" value="${name}" size=40"/></div><div class="form-row"><label>Description:</label>
-                <input  name="description" type="textfield" value="" size=40"/>
+                <input  name="description" type="textfield" value="${description}" size=40"/></div><div class="form-row"><input type="submit" name="create_category_button" value="Save"/>

--- a/templates/webapps/community/tool/edit_tool.mako
+++ b/templates/webapps/community/tool/edit_tool.mako
@@ -1,6 +1,10 @@
 <%inherit file="/base.mako"/><%namespace file="/message.mako" import="render_msg" />
 
+<%
+    from galaxy.web.framework.helpers import time_ago
+%>
+
 <%!
    def inherit(context):
        if context.get('use_panels'):
@@ -17,7 +21,7 @@
             $("input:text:first").focus();
         })
         function confirmSubmit() {
-            if ( confirm( "After you have submitted your tool to be published, you will no longer be able to modify it.  Click OK to submit it." ) ) {
+            if ( confirm( "Make sure you have filled in the User Description field.  After you have submitted your tool to be published, you will no longer be able to modify it.  Click OK to submit it." ) ) {
                 return true;
             } else {
                 return false;
@@ -52,28 +56,57 @@
 
 <%def name="title()">Edit Tool</%def>
 
-<h2>Edit Tool: ${tool.name} <em>${tool.description}</em></h2>
+<h2>Edit Tool</h2>
+
+${tool.get_state_message()}
+<p/>
+
+<ul class="manage-table-actions">
+    %if can_approve_or_reject:
+        <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 )}">Approve</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 )}">Reject</a></li>
+    %endif
+    <li><a class="action-button" id="tool-${tool.id}-popup" class="menubutton">Tool Actions</a></li>
+    <div popupmenu="tool-${tool.id}-popup">
+        %if can_view:
+            <a class="action-button" href="${h.url_for( controller='common', action='view_tool_history', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Tool history</a>
+            <a class="action-button" href="${h.url_for( controller='common', action='view_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">View tool</a>
+        %endif
+        %if can_download:
+            <a class="action-button" href="${h.url_for( controller='common', action='download_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Download tool</a>
+        %endif
+        %if can_delete:
+            <a class="action-button" href="${h.url_for( controller='common', action='delete_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}" confirm="Are you sure you want to delete this tool?">Delete tool</a>
+        %endif
+        %if 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
+        %if can_purge:
+            <li><a class="action-button" href="${h.url_for( controller='admin', action='purge_tool', id=trans.security.encode_id( tool.id ), cntrller=cntrller )}" confirm="Purging removes records from the database, are you sure you want to purge this tool?">Purge tool</a></li>
+        %endif
+    </div>
+</ul>
 
 %if message:
     ${render_msg( message, status )}
 %endif
 
-%if cntrller == 'admin' or trans.user == tool.user:
+%if can_edit:
     <form id="edit_tool" name="edit_tool" action="${h.url_for( controller='common', action='edit_tool' )}" method="post">
+        %if tool.is_rejected():
+            <div class="toolForm">
+                <div class="toolFormTitle">Reason for rejection</div>
+                <div class="toolFormBody">
+                    <div class="form-row">
+                        ${reason_for_rejection}
+                        <div style="clear: both"></div>
+                    </div>
+                </div>
+            </div>
+            <p/>
+        %endif
         <div class="toolForm">
-            <div class="toolFormTitle">${tool.name}
-                %if not tool.deleted:
-                    <a id="tool-${tool.id}-popup" class="popup-arrow" style="display: none;">▼</a>
-                    <div popupmenu="tool-${tool.id}-popup">
-                        <a class="action-button" href="${h.url_for( controller='common', action='view_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">View information</a>
-                        <a class="action-button" href="${h.url_for( controller='tool', action='download_tool', id=trans.app.security.encode_id( tool.id ) )}">Download tool</a>
-                        <a class="action-button" href="${h.url_for( controller='common', action='delete_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Delete tool</a>
-                        %if not tool.is_new() and not tool.is_waiting():
-                            <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
-                    </div>
-                %endif
-            </div>
+            <div class="toolFormTitle">${tool.name}</div><div class="toolFormBody"><input type="hidden" name="id" value="${trans.app.security.encode_id( tool.id )}"/><input type="hidden" name="cntrller" value="${cntrller}"/>
@@ -89,11 +122,27 @@
                 </div><div class="form-row"><label>Description:</label>
+                    ${tool.description}
+                    <div style="clear: both"></div>
+                </div>
+                <div class="form-row">
+                    <label>User Description:</label>
                     %if tool.user_description:
                         <div class="form-row-input"><pre><textarea name="user_description" rows="5" cols="35">${tool.user_description}</textarea></pre></div>
                     %else:
                         <div class="form-row-input"><textarea name="user_description" rows="5" cols="35"></textarea></div>
                     %endif
+                    <div class="toolParamHelp" style="clear: both;">Required when submitting for approval</div>
+                    <div style="clear: both"></div>
+                </div>
+                <div class="form-row">
+                    <label>Uploaded by:</label>
+                    ${tool.user.username}
+                    <div style="clear: both"></div>
+                </div>
+                <div class="form-row">
+                    <label>Date uploaded:</label>
+                    ${time_ago( tool.create_time )}
                     <div style="clear: both"></div></div></div>
@@ -120,7 +169,7 @@
             </div></div><p/>
-        %if tool.is_new():
+        %if tool.is_new() or tool.is_rejected():
             <div class="toolForm"><div class="toolFormTitle">Get approval for publishing</div><div class="toolFormBody">

--- /dev/null
+++ b/templates/webapps/community/common/view_tool_history.mako
@@ -0,0 +1,92 @@
+<%namespace file="/message.mako" import="render_msg" />
+
+<%
+    if cntrller in [ 'tool' ] and can_edit:
+        menu_label = 'Edit information or submit for approval'
+    else:
+        menu_label = 'Edit information'
+%>
+
+<%!
+   def inherit(context):
+       if context.get('use_panels'):
+           return '/webapps/community/base_panels.mako'
+       else:
+           return '/base.mako'
+%>
+<%inherit file="${inherit(context)}"/>
+
+<h2>Tool history</h2>
+<ul class="manage-table-actions">
+    %if can_approve_or_reject:
+        <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 )}">Approve</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 )}">Reject</a></li>
+    %endif
+    <li><a class="action-button" id="tool-${tool.id}-popup" class="menubutton">Tool Actions</a></li>
+    <div popupmenu="tool-${tool.id}-popup">
+        %if 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 )}">${menu_label}</a>
+        %endif
+        %if can_view:
+            <a class="action-button" href="${h.url_for( controller='common', action='view_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">View tool</a>
+        %endif
+        %if can_delete:
+            <a class="action-button" href="${h.url_for( controller='common', action='delete_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}" confirm="Are you sure you want to delete this tool?">Delete tool</a>
+        %endif
+        %if can_download:
+            <a class="action-button" href="${h.url_for( controller='common', action='download_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Download tool</a>
+        %endif
+    </div>
+</ul>
+
+%if message:
+    ${render_msg( message, status )}
+%endif
+
+<div class="toolForm">
+    <div class="toolFormTitle">${tool.name}</div>
+    <div class="form-row">
+        <label>Tool id:</label>
+        ${tool.tool_id}
+        <div style="clear: both"></div>
+    </div>
+    <div class="form-row">
+        <label>Version:</label>
+        ${tool.version}
+        <div style="clear: both"></div>
+    </div>
+    <div class="form-row">
+        <label>Description:</label>
+        ${tool.description}
+        <div style="clear: both"></div>
+    </div>
+    <div class="form-row">
+        <label>User description:</label>
+        ${tool.user_description}
+        <div style="clear: both"></div>
+    </div>
+    <div class="form-row">
+        <label>Uploaded by:</label>
+        ${tool.user.username}
+        <div style="clear: both"></div>
+    </div>
+</div>
+<p/>
+<table class="grid">
+    <thead>
+        <tr>
+            <th>State</th>
+            <th>Last Update</th>
+            <th>Comments</th>
+        </tr>
+    </thead>
+    <tbody>
+        %for state, updated, comments in events:    
+            <tr class="libraryRow libraryOrFolderRow" id="libraryRow">
+                <td><b><a>${state}</a></b></td>
+                <td><a>${updated}</a></td>
+                <td><a>${comments}</a></td>
+            </tr>             
+        %endfor
+    </tbody>
+</table>

--- a/lib/galaxy/webapps/community/model/__init__.py
+++ b/lib/galaxy/webapps/community/model/__init__.py
@@ -164,6 +164,21 @@ class Tool( object ):
         return self.state() == self.states.REJECTED
     def is_archived( self ):
         return self.state() == self.states.ARCHIVED
+    def get_state_message( self ):
+        if self.is_new():
+            return '<font color="red"><b><i>This is an unsubmitted version of this tool</i></b></font>'
+        if self.is_error():
+            return '<font color="red"><b><i>This tool is in an error state</i></b></font>'
+        if self.is_deleted():
+            return '<font color="red"><b><i>This is a deleted version of this tool</i></b></font>'
+        if self.is_waiting():
+            return '<font color="red"><b><i>This version of this tool is awaiting administrative approval</i></b></font>'
+        if self.is_approved():
+            return '<b><i>This is the latest approved version of this tool</i></b>'
+        if self.is_rejected():
+            return '<font color="red"><b><i>This version of this tool has been rejected by an administrator</i></b></font>'
+        if self.is_archived():
+            return '<font color="red"><b><i>This is an archived version of this tool</i></b></font>'
     @property
     def extension( self ):
         # if instantiated via a query, this unmapped property won't exist
@@ -240,7 +255,6 @@ class ToolAnnotationAssociation( object 
     pass
 
 ## ---- Utility methods -------------------------------------------------------
-
 def sort_by_attr( seq, attr ):
     """
     Sort the sequence of objects by object's attribute
@@ -256,7 +270,6 @@ def sort_by_attr( seq, attr ):
     intermed = map( None, map( getattr, seq, ( attr, ) * len( seq ) ), xrange( len( seq ) ), seq )
     intermed.sort()
     return map( operator.getitem, intermed, ( -1, ) * len( intermed ) )
-
 def directory_hash_id( id ):
     s = str( id )
     l = len( s )
@@ -269,5 +282,3 @@ def directory_hash_id( id ):
     padded = padded[:-3]
     # Break into chunks of three
     return [ padded[i*3:(i+1)*3] for i in range( len( padded ) // 3 ) ]
-
-

--- a/templates/webapps/community/tool/view_tool.mako
+++ b/templates/webapps/community/tool/view_tool.mako
@@ -3,19 +3,11 @@
 <%
     from galaxy.web.framework.helpers import time_ago
     from urllib import quote_plus
-    
-    menu_label = 'Edit information'
-    
-    if cntrller in [ 'tool' ]:
-        can_edit = trans.app.security_agent.can_edit_item( trans.user, tool )
-        if can_edit:
-            menu_label = 'Edit information or submit for approval'
-        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 )
+    if cntrller in [ 'tool' ] and can_edit:
+        menu_label = 'Edit information or submit for approval'
+    else:
+        menu_label = 'Edit information'
 %><%!
@@ -57,127 +49,133 @@
 
 <%def name="title()">View Tool</%def>
 
-<h2>View Tool: ${tool.name} <em>${tool.description}</em></h2>
+<h2>View Tool</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
+${tool.get_state_message()}
 <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
+<ul class="manage-table-actions">
+    %if can_approve_or_reject:
+        <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 )}">Approve</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 )}">Reject</a></li>
+    %endif
+    <li><a class="action-button" id="tool-${tool.id}-popup" class="menubutton">Tool Actions</a></li>
+    <div popupmenu="tool-${tool.id}-popup">
+        %if 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 )}">${menu_label}</a>
+        %endif
+        <a class="action-button" href="${h.url_for( controller='common', action='view_tool_history', id=trans.security.encode_id( tool.id ), cntrller=cntrller )}">Tool history</a>
+        %if can_download:
+            <a class="action-button" href="${h.url_for( controller='common', action='download_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Download tool</a>
+        %endif
+        %if can_delete:
+            <a class="action-button" href="${h.url_for( controller='common', action='delete_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}" confirm="Are you sure you want to delete this tool?">Delete tool</a>
+        %endif
+        %if 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
+        %if can_purge:
+            <li><a class="action-button" href="${h.url_for( controller='admin', action='purge_tool', id=trans.security.encode_id( tool.id ), cntrller=cntrller )}" confirm="Purging removes records from the database, are you sure you want to purge this tool?">Purge tool</a></li>
+        %endif
+    </div>
+</ul>
 
 %if message:
     ${render_msg( message, status )}
 %endif
 
-<div class="toolForm">
-    <div class="toolFormTitle">${tool.name}
-        %if not tool.deleted:
-            <a id="tool-${tool.id}-popup" class="popup-arrow" style="display: none;">▼</a>
-            <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 )}">${menu_label}</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>
-                %if cntrller=='admin' or trans.user==tool.user:
-                    <a class="action-button" href="${h.url_for( controller='common', action='delete_tool', id=trans.app.security.encode_id( tool.id ), cntrller=cntrller )}">Delete tool</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
+%if can_view:
+    %if tool.is_rejected():
+        <div class="toolForm">
+            <div class="toolFormTitle">Reason for rejection</div>
+            <div class="toolFormBody">
+                <div class="form-row">
+                    ${reason_for_rejection}
+                    <div style="clear: both"></div>
+                </div></div>
-        %endif
-    </div>
-    <div class="toolFormBody">
-        <div class="form-row">
-            <label>Tool Id:</label>
-            ${tool.tool_id}
-            <div style="clear: both"></div></div>
-        <div class="form-row">
-            <label>Version:</label>
-            ${tool.version}
-            <div style="clear: both"></div>
-        </div>
-        <div class="form-row">
-            <label>Description:</label>
-            %if tool.user_description:
-                <pre>${tool.user_description}</pre>
-            %endif
-            <div style="clear: both"></div>
-        </div>
-        <div class="form-row">
-            <label>Uploaded by:</label>
-            ${tool.user.username}
-            <div style="clear: both"></div>
-        </div>
-        <div class="form-row">
-            <label>Date uploaded:</label>
-            ${time_ago( tool.create_time )}
-            <div style="clear: both"></div>
-        </div>
-        <div class="form-row">
-            <label>Categories:</label>
-            %if categories:
-                <ul>
-                    %for category in categories:
-                        <li>${category.name}</li>
-                    %endfor
-                </ul>
-            %else:
-                none set
-            %endif
-            <div style="clear: both"></div>
-        </div>
-        %if len( visible_versions ) > 1:
+        <p/>
+    %endif
+    <div class="toolForm">
+        <div class="toolFormTitle">${tool.name}</div>
+        <div class="toolFormBody"><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>
+                <label>Tool Id:</label>
+                ${tool.tool_id}
                 <div style="clear: both"></div></div>
-        %endif
-    </div>
-</div>
-
-<p/>
-
-<div class="toolForm">
-    <div class="toolFormTitle">Tool Contents</div>
-    <div class="toolFormBody">
-        <div class="form-row">
-            <ul class="toolFile">
-                <li><a href="${h.url_for( controller='tool', action='download_tool', id=trans.app.security.encode_id( tool.id ) )}">${tool.download_file_name}</a></li>
-                <ul class="fileBrowser">
-                    %for name in tool_file_contents:
-                        <li><a href="${h.url_for( controller='tool', action='view_tool_file', id=trans.app.security.encode_id( tool.id ), file_name=quote_plus( name ) )}">${name}</a></li>
-                    %endfor
-                </ul>
-            </ul>
+            <div class="form-row">
+                <label>Version:</label>
+                ${tool.version}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Description:</label>
+                ${tool.description}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>User Description:</label>
+                %if tool.user_description:
+                    <pre>${tool.user_description}</pre>
+                %endif
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Uploaded by:</label>
+                ${tool.user.username}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Date uploaded:</label>
+                ${time_ago( tool.create_time )}
+                <div style="clear: both"></div>
+            </div>
+            <div class="form-row">
+                <label>Categories:</label>
+                %if categories:
+                    <ul>
+                        %for category in categories:
+                            <li>${category.name}</li>
+                        %endfor
+                    </ul>
+                %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>
-</div>
+    <p/>
+    <div class="toolForm">
+        <div class="toolFormTitle">Tool Contents</div>
+        <div class="toolFormBody">
+            <div class="form-row">
+                <ul class="toolFile">
+                    <li><a href="${h.url_for( controller='tool', action='download_tool', id=trans.app.security.encode_id( tool.id ) )}">${tool.download_file_name}</a></li>
+                    <ul class="fileBrowser">
+                        %for name in tool_file_contents:
+                            <li><a href="${h.url_for( controller='tool', action='view_tool_file', id=trans.app.security.encode_id( tool.id ), file_name=quote_plus( name ) )}">${name}</a></li>
+                        %endfor
+                    </ul>
+                </ul>
+            </div>
+        </div>
+    </div>
+%endif

--- a/templates/webapps/community/category/edit_category.mako
+++ b/templates/webapps/community/category/edit_category.mako
@@ -10,7 +10,6 @@
     <div class="toolFormBody"><form name="library" action="${h.url_for( controller='admin', action='edit_category' )}" method="post" ><div class="form-row">
-                <input  name="webapp" type="hidden" value="${webapp}" size=40"/><label>Name:</label><div style="float: left; width: 250px; margin-right: 10px;"><input type="text" name="name" value="${category.name}" size="40"/>

--- a/lib/galaxy/webapps/community/controllers/tool.py
+++ b/lib/galaxy/webapps/community/controllers/tool.py
@@ -97,7 +97,9 @@ class ToolController( BaseController ):
                 for k, v in kwd.items():
                     if k.startswith( 'f-' ):
                         del kwd[ k ]
-                return self.browse_tools( trans, **kwd )
+                return trans.response.send_redirect( web.url_for( controller='tool',
+                                                                  action='browse_tools',
+                                                                  **kwd ) )
         # Render the list view
         return self.category_list_grid( trans, **kwd )
     @web.expose
@@ -107,19 +109,20 @@ class ToolController( BaseController ):
         # to take this approach because the "-" character is illegal in HTTP requests.
         if 'operation' in kwd:
             operation = kwd['operation'].lower()
-            if operation == "view tool":
+            if operation == "view_tool":
                 return trans.response.send_redirect( web.url_for( controller='common',
                                                                   action='view_tool',
                                                                   cntrller='tool',
                                                                   **kwd ) )
-            elif operation == "edit tool":
+            elif operation == "edit_tool":
                 return trans.response.send_redirect( web.url_for( controller='common',
                                                                   action='edit_tool',
                                                                   cntrller='tool',
                                                                   **kwd ) )
             elif operation == "download tool":
-                return trans.response.send_redirect( web.url_for( controller='tool',
+                return trans.response.send_redirect( web.url_for( controller='common',
                                                                   action='download_tool',
+                                                                  cntrller='tool',
                                                                   **kwd ) )
             elif operation == "tools_by_user":
                 # Eliminate the current filters if any exist.
@@ -154,20 +157,6 @@ class ToolController( BaseController ):
         # Render the list view
         return self.tool_list_grid( trans, **kwd )
     @web.expose
-    def download_tool( self, trans, **kwd ):
-        params = util.Params( kwd )
-        id = params.get( 'id', None )
-        if not id:
-            return trans.response.send_redirect( web.url_for( controller='tool',
-                                                              action='browse_tools',
-                                                              message='Select a tool to download',
-                                                              status='error' ) )
-        tool = get_tool( trans, id )
-        trans.response.set_content_type( tool.mimetype )
-        trans.response.headers['Content-Length'] = int( os.stat( tool.file_name ).st_size )
-        trans.response.headers['Content-Disposition'] = 'attachment; filename=%s' % tool.download_file_name
-        return open( tool.file_name )
-    @web.expose
     def view_tool_file( self, trans, **kwd ):
         params = util.Params( kwd )
         id = params.get( 'id', None )

--- a/lib/galaxy/webapps/community/controllers/admin.py
+++ b/lib/galaxy/webapps/community/controllers/admin.py
@@ -399,16 +399,21 @@ class AdminController( BaseController, A
         # to take this approach because the "-" character is illegal in HTTP requests.
         if 'operation' in kwd:
             operation = kwd['operation'].lower()
-            if operation == "edit tool":
+            if operation == "edit_tool":
                 return trans.response.send_redirect( web.url_for( controller='common',
                                                                   action='edit_tool',
                                                                   cntrller='admin',
                                                                   **kwd ) )
-            elif operation == "view tool":
+            elif operation == "view_tool":
                 return trans.response.send_redirect( web.url_for( controller='common',
                                                                   action='view_tool',
                                                                   cntrller='admin',
                                                                   **kwd ) )
+            elif operation == 'tool_history':
+                return trans.response.send_redirect( web.url_for( controller='common',
+                                                                  cntrller='admin',
+                                                                  action='events',
+                                                                  **kwd ) )
             elif operation == "tools_by_user":
                 # Eliminate the current filters if any exist.
                 for k, v in kwd.items():
@@ -457,7 +462,9 @@ class AdminController( BaseController, A
                 for k, v in kwd.items():
                     if k.startswith( 'f-' ):
                         del kwd[ k ]
-                return self.browse_tools( trans, **kwd )
+                return trans.response.send_redirect( web.url_for( controller='admin',
+                                                                  action='browse_tools',
+                                                                  **kwd ) )
         # Render the list view
         return self.category_list_grid( trans, **kwd )
     @web.expose
@@ -481,16 +488,26 @@ class AdminController( BaseController, A
     @web.require_admin
     def create_category( self, trans, **kwd ):
         params = util.Params( kwd )
-        webapp = params.get( 'webapp', 'community' )
         message = util.restore_text( params.get( 'message', ''  ) )
         status = params.get( 'status', 'done' )
         if params.get( 'create_category_button', False ):
             name = util.restore_text( params.name )
             description = util.restore_text( params.description )
+            error = False
             if not name or not description:
-                message = "Enter a valid name and a description"
-            elif trans.sa_session.query( trans.app.model.Category ).filter( trans.app.model.Category.table.c.name==name ).first():
-                message = "A category with that name already exists"
+                message = 'Enter a valid name and a description'
+                error = True
+            elif trans.sa_session.query( trans.app.model.Category ) \
+                                 .filter( trans.app.model.Category.table.c.name==name ) \
+                                 .first():
+                message = 'A category with that name already exists'
+                error = True
+            if error:
+                return trans.fill_template( '/webapps/community/category/create_category.mako',
+                                            name=name,
+                                            description=description,
+                                            message=message,
+                                            status='error' )
             else:
                 # Create the category
                 category = trans.app.model.Category( name=name, description=description )
@@ -499,26 +516,27 @@ class AdminController( BaseController, A
                 trans.sa_session.flush()
                 trans.response.send_redirect( web.url_for( controller='admin',
                                                            action='manage_categories',
-                                                           webapp=webapp,
                                                            message=util.sanitize_text( message ),
                                                            status='done' ) )
             trans.response.send_redirect( web.url_for( controller='admin',
                                                        action='create_category',
-                                                       webapp=webapp,
                                                        message=util.sanitize_text( message ),
                                                        status='error' ) )
+        else:
+            name = ''
+            description = ''
         return trans.fill_template( '/webapps/community/category/create_category.mako',
-                                    webapp=webapp,
+                                    name=name,
+                                    description=description,
                                     message=message,
                                     status=status )
     @web.expose
     @web.require_admin
     def set_tool_state( self, trans, state, **kwd ):
         params = util.Params( kwd )
-        webapp = params.get( 'webapp', 'galaxy' )
         message = util.restore_text( params.get( 'message', ''  ) )
         status = params.get( 'status', 'done' )
-        redirect = params.get( 'no_redirect', True )
+        comments = util.restore_text( params.get( 'comments', '' ) )
         id = params.get( 'id', None )
         if not id:
             message = "No tool id received for setting status"
@@ -526,32 +544,130 @@ class AdminController( BaseController, A
         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 we're approving a tool, all previously approved versions must be set to archived
+                for version in get_versions( tool ):
+                    # TODO: get latest approved version instead of all versions
                     if version != tool and version.is_approved():
-                        self.set_tool_state( trans,
-                                             trans.app.model.Tool.states.ARCHIVED,
-                                             id=trans.security.encode_id( version.id ),
-                                             redirect='False' )
-            event = trans.model.Event( state )
-            # Flush so we an get an id
-            trans.sa_session.add( event )
-            trans.sa_session.flush()
-            tea = trans.model.ToolEventAssociation( tool, event )
-            trans.sa_session.add( tea )
-            trans.sa_session.flush()
+                        # Create an event with state ARCHIVED for the previously approved version of this tool
+                        self.__create_tool_event( trans,
+                                                  version,
+                                                  trans.app.model.Tool.states.ARCHIVED )
+                # Create an event with state APPROVED for this tool
+                self.__create_tool_event( trans, tool, state, comments )
+            elif state == trans.app.model.Tool.states.REJECTED:
+                # If we're rejecting a tool, comments about why are necessary.
+                return trans.fill_template( '/webapps/community/admin/reject_tool.mako',
+                                            tool=tool,
+                                            cntrller='admin' )
             message = "State of tool '%s' is now %s" % ( tool.name, state )
-        if redirect:
+        trans.response.send_redirect( web.url_for( controller='admin',
+                                                   action='browse_tools',
+                                                   message=message,
+                                                   status=status ) )
+    @web.expose
+    @web.require_admin
+    def reject_tool( self, trans, **kwd ):
+        params = util.Params( kwd )
+        if params.get( 'cancel_reject_button', False ):
+            # Fix up the keyword dict to include params to view the current tool
+            # since that is the page from which we originated.
+            del kwd[ 'cancel_reject_button' ]
+            del kwd[ 'comments' ]
+            kwd[ 'webapp' ] = 'community'
+            kwd[ 'operation' ] = 'view_tool'
+            message = 'Tool rejection cancelled'
+            status = 'done'
+            return trans.response.send_redirect( web.url_for( controller='admin',
+                                                              action='browse_tools',
+                                                              message=message,
+                                                              status=status,
+                                                              **kwd ) )
+        id = params.get( 'id', None )
+        if not id:
+            return trans.response.send_redirect( web.url_for( controller=cntrller,
+                                                              action='browse_tools',
+                                                              message='No tool id received for rejecting',
+                                                              status='error' ) )
+        tool = get_tool( trans, id )
+        if not trans.app.security_agent.can_approve_or_reject( trans.user, trans.user_is_admin(), 'admin', tool ):
+            return trans.response.send_redirect( web.url_for( controller='admin',
+                                                              action='browse_tools',
+                                                              message='You are not allowed to reject this tool',
+                                                              status='error' ) )
+        # Comments are required when rejecting a tool.
+        comments = util.restore_text( params.get( 'comments', '' ) )
+        if not comments:
+            message = 'The reason for rejection is required when rejecting a tool.'
+            return trans.fill_template( '/webapps/community/admin/reject_tool.mako',
+                                        tool=tool,
+                                        cntrller='admin',
+                                        message=message,
+                                        status='error' )
+        # Create an event with state REJECTED for this tool
+        self.__create_tool_event( trans, tool, trans.app.model.Tool.states.REJECTED, comments )
+        message = 'The tool "%s" has been rejected.' % tool.name
+        return trans.response.send_redirect( web.url_for( controller='admin',
+                                                          action='browse_tools',
+                                                          operation='tools_by_state',
+                                                          state='rejected',
+                                                          message=message,
+                                                          status='done' ) )
+    def __create_tool_event( self, trans, tool, state, comments='' ):
+        event = trans.model.Event( state, comments )
+        # Flush so we can get an id
+        trans.sa_session.add( event )
+        trans.sa_session.flush()
+        tea = trans.model.ToolEventAssociation( tool, event )
+        trans.sa_session.add( tea )
+        trans.sa_session.flush()
+    @web.expose
+    @web.require_admin
+    def purge_tool( self, trans, **kwd ):
+        # This method completely removes a tool record and all associated foreign key rows
+        # from the database, so it must be used carefully.
+        # This method should only be called for a tool that has previously been deleted.
+        # Purging a deleted tool deletes all of the following from the database:
+        # - ToolCategoryAssociations
+        # - ToolEventAssociations and associated Events
+        # TODO: when we add tagging for tools, we'll have to purge them as well
+        params = util.Params( kwd )
+        id = kwd.get( 'id', None )
+        if not id:
+            message = "No tool ids received for purging"
             trans.response.send_redirect( web.url_for( controller='admin',
                                                        action='browse_tools',
-                                                       webapp=webapp,
-                                                       message=message,
-                                                       status=status ) )
+                                                       message=util.sanitize_text( message ),
+                                                       status='error' ) )
+        ids = util.listify( id )
+        message = "Purged %d tools: " % len( ids )
+        for tool_id in ids:
+            tool = get_tool( trans, tool_id )
+            message += " %s " % tool.name
+            if not tool.deleted:
+                message = "Tool '%s' has not been deleted, so it cannot be purged." % tool.name
+                trans.response.send_redirect( web.url_for( controller='admin',
+                                                           action='browse_tools',
+                                                           message=util.sanitize_text( message ),
+                                                           status='error' ) )
+            # Delete ToolCategoryAssociations
+            for tca in tool.categories:
+                trans.sa_session.delete( tca )
+            # Delete ToolEventAssociations and associated events
+            for tea in tool.events:
+                event = tea.event
+                trans.sa_session.delete( event )
+                trans.sa_session.delete( tea )
+            # Delete the tool
+            trans.sa_session.delete( tool )
+            trans.sa_session.flush()
+        trans.response.send_redirect( web.url_for( controller='admin',
+                                                   action='browse_tools',
+                                                   message=util.sanitize_text( message ),
+                                                   status='done' ) )
     @web.expose
     @web.require_admin
     def edit_category( self, trans, **kwd ):
         params = util.Params( kwd )
-        webapp = params.get( 'webapp', 'galaxy' )
         message = util.restore_text( params.get( 'message', ''  ) )
         status = params.get( 'status', 'done' )
         id = params.get( 'id', None )
@@ -559,7 +675,6 @@ class AdminController( BaseController, A
             message = "No category ids received for editing"
             trans.response.send_redirect( web.url_for( controller='admin',
                                                        action='manage_categories',
-                                                       webapp=webapp,
                                                        message=message,
                                                        status='error' ) )
         category = get_category( trans, id )
@@ -582,25 +697,21 @@ class AdminController( BaseController, A
                     message = "The information has been saved for category '%s'" % ( category.name )
                     return trans.response.send_redirect( web.url_for( controller='admin',
                                                                       action='manage_categories',
-                                                                      webapp=webapp,
                                                                       message=util.sanitize_text( message ),
                                                                       status='done' ) )
         return trans.fill_template( '/webapps/community/category/edit_category.mako',
                                     category=category,
-                                    webapp=webapp,
                                     message=message,
                                     status=status )
     @web.expose
     @web.require_admin
     def mark_category_deleted( self, trans, **kwd ):
         params = util.Params( kwd )
-        webapp = params.get( 'webapp', 'galaxy' )
         id = kwd.get( 'id', None )
         if not id:
             message = "No category ids received for deleting"
             trans.response.send_redirect( web.url_for( controller='admin',
                                                        action='manage_categories',
-                                                       webapp=webapp,
                                                        message=message,
                                                        status='error' ) )
         ids = util.listify( id )
@@ -613,20 +724,17 @@ class AdminController( BaseController, A
             message += " %s " % category.name
         trans.response.send_redirect( web.url_for( controller='admin',
                                                    action='manage_categories',
-                                                   webapp=webapp,
                                                    message=util.sanitize_text( message ),
                                                    status='done' ) )
     @web.expose
     @web.require_admin
     def undelete_category( self, trans, **kwd ):
         params = util.Params( kwd )
-        webapp = params.get( 'webapp', 'galaxy' )
         id = kwd.get( 'id', None )
         if not id:
             message = "No category ids received for undeleting"
             trans.response.send_redirect( web.url_for( controller='admin',
                                                        action='manage_categories',
-                                                       webapp=webapp,
                                                        message=message,
                                                        status='error' ) )
         ids = util.listify( id )
@@ -638,7 +746,6 @@ class AdminController( BaseController, A
                 message = "Category '%s' has not been deleted, so it cannot be undeleted." % category.name
                 trans.response.send_redirect( web.url_for( controller='admin',
                                                            action='manage_categories',
-                                                           webapp=webapp,
                                                            message=util.sanitize_text( message ),
                                                            status='error' ) )
             category.deleted = False
@@ -649,7 +756,6 @@ class AdminController( BaseController, A
         message = "Undeleted %d categories: %s" % ( count, undeleted_categories )
         trans.response.send_redirect( web.url_for( controller='admin',
                                                    action='manage_categories',
-                                                   webapp=webapp,
                                                    message=util.sanitize_text( message ),
                                                    status='done' ) )
     @web.expose
@@ -659,13 +765,11 @@ class AdminController( BaseController, A
         # Purging a deleted Category deletes all of the following from the database:
         # - ToolCategoryAssociations where category_id == Category.id
         params = util.Params( kwd )
-        webapp = params.get( 'webapp', 'galaxy' )
         id = kwd.get( 'id', None )
         if not id:
             message = "No category ids received for purging"
             trans.response.send_redirect( web.url_for( controller='admin',
                                                        action='manage_categories',
-                                                       webapp=webapp,
                                                        message=util.sanitize_text( message ),
                                                        status='error' ) )
         ids = util.listify( id )
@@ -676,7 +780,6 @@ class AdminController( BaseController, A
                 message = "Category '%s' has not been deleted, so it cannot be purged." % category.name
                 trans.response.send_redirect( web.url_for( controller='admin',
                                                            action='manage_categories',
-                                                           webapp=webapp,
                                                            message=util.sanitize_text( message ),
                                                            status='error' ) )
             # Delete ToolCategoryAssociations
@@ -686,7 +789,6 @@ class AdminController( BaseController, A
             message += " %s " % category.name
         trans.response.send_redirect( web.url_for( controller='admin',
                                                    action='manage_categories',
-                                                   webapp=webapp,
                                                    message=util.sanitize_text( message ),
                                                    status='done' ) )

    

commits-noreply＠bitbucket.org

tags

participants (1)