From f6463c507721090684e24a2a8fa017f2124fd6aa Mon Sep 17 00:00:00 2001 From: Greg Von Kuster Date: Thu, 23 Sep 2010 16:38:55 -0400 Subject: [PATCH] Remove the model from the grid framework and the UsesItemsRatings class. Use the new ItemRatingsColumn grid column in the tool shed, and fix several miscellaneous bugs and clean up some code related to the use of these modules. Fix a bug in ~/community/datatypes/verify when uploading a tool so that an error is displayed (rather than en excetpion thrown) when a tool congfig does not include required tag attributes. Some recent chage to the grid framework has broken the queries in the tool shed, but the fix will have to wait... --- lib/galaxy/model/item_attrs.py | 41 +++++++++------ lib/galaxy/web/framework/helpers/grids.py | 26 +++++----- .../webapps/community/controllers/admin.py | 8 --- .../webapps/community/controllers/common.py | 50 ++++++------------- .../webapps/community/controllers/tool.py | 4 -- .../webapps/community/datatypes/__init__.py | 6 +++ 6 files changed, 60 insertions(+), 75 deletions(-) diff --git a/lib/galaxy/model/item_attrs.py b/lib/galaxy/model/item_attrs.py index f3f46cfc047..e5d3d7f8746 100644 --- a/lib/galaxy/model/item_attrs.py +++ b/lib/galaxy/model/item_attrs.py @@ -1,6 +1,11 @@ from sqlalchemy.sql.expression import func # Cannot import galaxy.model b/c it creates a circular import graph. import galaxy +import logging +log = logging.getLogger( __name__ ) + +class RuntimeException( Exception ): + pass class UsesItemRatings: """ @@ -8,14 +13,14 @@ class UsesItemRatings: Class makes two assumptions: (1) item-rating association table is named RatingAssocation - and is in galaxy.model; (2) item-rating association table has a column with a foreign key referencing item table that contains the item's id. - """ - - def get_ave_item_rating_data( self, db_session, item ): + """ + def get_ave_item_rating_data( self, db_session, item, webapp_model=None ): """ Returns the average rating for an item.""" - item_rating_assoc_class = self._get_item_rating_assoc_class( item ) + if webapp_model is None: + webapp_model = galaxy.model + item_rating_assoc_class = self._get_item_rating_assoc_class( item, webapp_model=webapp_model ) if not item_rating_assoc_class: raise RuntimeException( "Item does not have ratings: %s" % item.__class__.__name__ ) item_id_filter = self._get_item_id_filter_str( item, item_rating_assoc_class ) @@ -28,12 +33,14 @@ class UsesItemRatings: num_ratings = int( db_session.query( func.count( item_rating_assoc_class.rating ) ).filter( item_id_filter ).scalar() ) return ( ave_rating, num_ratings ) - def rate_item( self, db_session, user, item, rating ): + def rate_item( self, db_session, user, item, rating, webapp_model=None ): """ Rate an item. Return type is RatingAssociation. """ - item_rating = self.get_user_item_rating( db_session, user, item ) + if webapp_model is None: + webapp_model = galaxy.model + item_rating = self.get_user_item_rating( db_session, user, item, webapp_model=webapp_model ) if not item_rating: # User has not yet rated item; create rating. - item_rating_assoc_class = self._get_item_rating_assoc_class( item ) + item_rating_assoc_class = self._get_item_rating_assoc_class( item, webapp_model=webapp_model ) item_rating = item_rating_assoc_class() item_rating.user = user item_rating.set_item( item ) @@ -46,9 +53,11 @@ class UsesItemRatings: db_session.flush() return item_rating - def get_user_item_rating( self, db_session, user, item ): + def get_user_item_rating( self, db_session, user, item, webapp_model=None ): """ Returns user's rating for an item. Return type is RatingAssociation. """ - item_rating_assoc_class = self._get_item_rating_assoc_class( item ) + if webapp_model is None: + webapp_model = galaxy.model + item_rating_assoc_class = self._get_item_rating_assoc_class( item, webapp_model=webapp_model ) if not item_rating_assoc_class: raise RuntimeException( "Item does not have ratings: %s" % item.__class__.__name__ ) @@ -56,13 +65,17 @@ class UsesItemRatings: item_id_filter = self._get_item_id_filter_str( item, item_rating_assoc_class ) return db_session.query( item_rating_assoc_class ).filter_by( user=user ).filter( item_id_filter ).first() - def _get_item_rating_assoc_class( self, item ): + def _get_item_rating_assoc_class( self, item, webapp_model=None ): """ Returns an item's item-rating association class. """ + if webapp_model is None: + webapp_model = galaxy.model item_rating_assoc_class = '%sRatingAssociation' % item.__class__.__name__ - return getattr( galaxy.model, item_rating_assoc_class, None ) - - def _get_item_id_filter_str( self, item, item_rating_assoc_class ): + return getattr( webapp_model, item_rating_assoc_class, None ) + + def _get_item_id_filter_str( self, item, item_rating_assoc_class, webapp_model=None ): # Get foreign key in item-rating association table that references item table. + if webapp_model is None: + webapp_model = galaxy.model item_fk = None for fk in item_rating_assoc_class.table.foreign_keys: if fk.references( item.table ): diff --git a/lib/galaxy/web/framework/helpers/grids.py b/lib/galaxy/web/framework/helpers/grids.py index cc70c27378f..a2d184c21d1 100644 --- a/lib/galaxy/web/framework/helpers/grids.py +++ b/lib/galaxy/web/framework/helpers/grids.py @@ -5,7 +5,6 @@ from galaxy.web import url_for from galaxy.util.json import from_json_string, to_json_string from galaxy.util.odict import odict from galaxy.web.framework.helpers import to_unicode -from galaxy import model from galaxy.model.item_attrs import * import sys, logging, math @@ -161,7 +160,7 @@ class Grid( object ): # Sort key is a column key. for column in self.columns: if column.key == column_key: - query = column.sort( query, ascending ) + query = column.sort( trans, query, ascending ) break extra_url_args['sort'] = sort_key # There might be a current row @@ -334,7 +333,7 @@ class GridColumn( object ): accepted_filters.append( GridColumnFilter( val, args) ) return accepted_filters - def sort( self, query, ascending ): + def sort( self, trans, query, ascending ): """ Sort query using this column. """ if ascending: query = query.order_by( self.model_class.table.c.get( self.key ).asc() ) @@ -344,7 +343,7 @@ class GridColumn( object ): class ReverseSortColumn( GridColumn ): """ Column that reverses sorting; this is useful when the natural sort is descending. """ - def sort( self, query, ascending ): + def sort( self, trans, query, ascending ): return GridColumn.sort( self, query, (not ascending) ) class TextColumn( GridColumn ): @@ -369,7 +368,7 @@ class TextColumn( GridColumn ): """ Returns a SQLAlchemy criterion derived for a single filter. Single filter is the most basic filter--usually a string--and cannot be a list. """ model_class_key_field = getattr( self.model_class, self.key ) return func.lower( model_class_key_field ).like( "%" + a_filter.lower() + "%" ) - def sort( self, query, ascending ): + def sort( self, trans, query, ascending ): """Sort column using case-insensitive alphabetical sorting.""" if ascending: query = query.order_by( func.lower( self.model_class.table.c.get( self.key ) ).asc() ) @@ -378,9 +377,9 @@ class TextColumn( GridColumn ): return query class DateTimeColumn( TextColumn ): - def sort( self, query, ascending ): + def sort( self, trans, query, ascending ): """Sort query using this column.""" - return GridColumn.sort( self, query, ascending ) + return GridColumn.sort( self, trans, query, ascending ) class IntegerColumn( TextColumn ): """ @@ -403,20 +402,19 @@ class IntegerColumn( TextColumn ): model_class_key_field = getattr( self.model_class, self.key ) assert int( a_filter ), "The search entry must be an integer" return model_class_key_field == int( a_filter ) - def sort( self, query, ascending ): + def sort( self, trans, query, ascending ): """Sort query using this column.""" - return GridColumn.sort( self, query, ascending ) + return GridColumn.sort( self, trans, query, ascending ) class CommunityRatingColumn( GridColumn, UsesItemRatings ): """ Column that displays community ratings for an item. """ - def get_value( self, trans, grid, item ): - ave_item_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, item ) + ave_item_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, item, webapp_model=trans.model ) return trans.fill_template( "community_rating.mako", ave_item_rating=ave_item_rating, num_ratings=num_ratings, item_id=trans.security.encode_id( item.id ) ) - def sort( self, query, ascending ): + def sort( self, trans, query, ascending ): def get_foreign_key( source_class, target_class ): """ Returns foreign key in source class that references target class. """ target_fk = None @@ -428,7 +426,7 @@ class CommunityRatingColumn( GridColumn, UsesItemRatings ): raise RuntimeException( "No foreign key found between objects: %s, %s" % source_class.table, target_class.table ) return target_fk # Get the columns that connect item's table and item's rating association table. - item_rating_assoc_class = getattr( model, '%sRatingAssociation' % self.model_class.__name__ ) + item_rating_assoc_class = getattr( trans.model, '%sRatingAssociation' % self.model_class.__name__ ) foreign_key = get_foreign_key( item_rating_assoc_class, self.model_class ) fk_col = foreign_key.parent referent_col = foreign_key.get_referent( self.model_class.table ) @@ -561,7 +559,7 @@ class OwnerColumn( TextColumn ): """ Column that lists item's owner. """ def get_value( self, trans, grid, item ): return item.user.username - def sort( self, query, ascending ): + def sort( self, trans, query, ascending ): """ Sort column using case-insensitive alphabetical sorting on item's username. """ if ascending: query = query.order_by( func.lower ( self.model_class.username ).asc() ) diff --git a/lib/galaxy/webapps/community/controllers/admin.py b/lib/galaxy/webapps/community/controllers/admin.py index 54d49e8e1f9..c80f629a7f3 100644 --- a/lib/galaxy/webapps/community/controllers/admin.py +++ b/lib/galaxy/webapps/community/controllers/admin.py @@ -62,13 +62,11 @@ class UserListGrid( grids.Grid ): columns = [ UserLoginColumn( "Email", key="email", - model_class=model.User, link=( lambda item: dict( operation="information", id=item.id, webapp="community" ) ), attach_popup=True, filterable="advanced" ), UserNameColumn( "User Name", key="username", - model_class=model.User, attach_popup=False, filterable="advanced" ), GroupsColumn( "Groups", attach_popup=False ), @@ -77,14 +75,12 @@ class UserListGrid( grids.Grid ): LastLoginColumn( "Last Login", format=time_ago ), StatusColumn( "Status", attach_popup=False ), ToolsColumn( "Uploaded Tools", - model_class=model.User, link=( lambda item: dict( operation="tools_by_user", id=item.id, webapp="community" ) ), attach_popup=False, filterable="advanced" ), # Columns that are valid for filtering but are not visible. EmailColumn( "Email", key="email", - model_class=model.User, visible=False ) ] columns.append( grids.MulticolFilterColumn( "Search", @@ -159,17 +155,14 @@ class RoleListGrid( grids.Grid ): NameColumn( "Name", key="name", link=( lambda item: dict( operation="Manage users and groups", id=item.id, webapp="community" ) ), - model_class=model.Role, attach_popup=True, filterable="advanced" ), DescriptionColumn( "Description", key='description', - model_class=model.Role, attach_popup=False, filterable="advanced" ), TypeColumn( "Type", key='type', - model_class=model.Role, attach_popup=False, filterable="advanced" ), GroupsColumn( "Groups", attach_popup=False ), @@ -249,7 +242,6 @@ class GroupListGrid( grids.Grid ): NameColumn( "Name", #key="name", link=( lambda item: dict( operation="Manage users and roles", id=item.id, webapp="community" ) ), - model_class=model.Group, attach_popup=True #filterable="advanced" ), diff --git a/lib/galaxy/webapps/community/controllers/common.py b/lib/galaxy/webapps/community/controllers/common.py index ef8dc234cdd..f856bcd6dd5 100644 --- a/lib/galaxy/webapps/community/controllers/common.py +++ b/lib/galaxy/webapps/community/controllers/common.py @@ -4,7 +4,7 @@ from galaxy.webapps.community import model from galaxy.model.orm import * from galaxy.web.framework.helpers import time_ago, iff, grids from galaxy.web.form_builder import SelectField -from galaxy.item_attrs.ratings import UsesItemRatings +from galaxy.model.item_attrs import UsesItemRatings import logging log = logging.getLogger( __name__ ) @@ -15,10 +15,10 @@ class ItemRatings( UsesItemRatings ): """Overrides rate_item method since we also allow for comments""" def rate_item( self, trans, user, item, rating, comment='' ): """ Rate an item. Return type is RatingAssociation. """ - item_rating = self.get_user_item_rating( trans.sa_session, user, item ) + item_rating = self.get_user_item_rating( trans.sa_session, user, item, webapp_model=trans.model ) if not item_rating: # User has not yet rated item; create rating. - item_rating_assoc_class = self._get_item_rating_assoc_class( trans, item ) + item_rating_assoc_class = self._get_item_rating_assoc_class( item, webapp_model=trans.model ) item_rating = item_rating_assoc_class() item_rating.user = trans.user item_rating.set_item( item ) @@ -32,25 +32,12 @@ class ItemRatings( UsesItemRatings ): item_rating.comment = comment trans.sa_session.flush() return item_rating - def get_avg_rating_html( self, avg_rating ): - # FIXME: the class="star" attribute in the input tag does not render correctly inside a table - # with the attribute class="grid-table", so just display the numerical avg_rating value until - # we can figure out why the styles don't work together. When this is fixed, eliminate the following - # line and return the html. - return int( avg_rating ) - html = '' - for index in range( 1, 6 ): - html += ' ( index - 0.5 ) and avg_rating < ( index + 0.5 ): - html += ' checked="checked"' - html += '/>' - return html class ToolListGrid( grids.Grid ): class NameColumn( grids.TextColumn ): def get_value( self, trans, grid, tool ): return tool.name - class TypeColumn( grids.GridColumn ): + class TypeColumn( grids.TextColumn ): def get_value( self, trans, grid, tool ): if tool.is_suite: return 'Suite' @@ -83,11 +70,7 @@ class ToolListGrid( grids.Grid ): if tool.user: return tool.user.username return 'no user' - class RatingColumn( grids.TextColumn, ItemRatings ): - def get_value( self, trans, grid, tool ): - avg_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, tool ) - return self.get_avg_rating_html( avg_rating ) - class EmailColumn( grids.GridColumn ): + class EmailColumn( grids.TextColumn ): def filter( self, trans, user, query, column_filter ): if column_filter == 'All': return query @@ -109,7 +92,6 @@ class ToolListGrid( grids.Grid ): attach_popup=False ), VersionColumn( "Version", key="version", - model_class=model.Tool, attach_popup=False, filterable="advanced" ), DescriptionColumn( "Description", @@ -121,19 +103,17 @@ class ToolListGrid( grids.Grid ): attach_popup=False, filterable="advanced" ), UserColumn( "Uploaded By", - model_class=model.User, - link=( lambda item: dict( operation="tools_by_user", id=item.id, webapp="community" ) ), - attach_popup=False, - filterable="advanced" ), - RatingColumn( "Average Rating", - attach_popup=False ), + model_class=model.User, + link=( lambda item: dict( operation="tools_by_user", id=item.id, webapp="community" ) ), + attach_popup=False, + filterable="advanced" ), + grids.CommunityRatingColumn( "Average Rating", + key="rating" ), # Columns that are valid for filtering but are not visible. EmailColumn( "Email", - key="email", model_class=model.User, visible=False ), ToolCategoryColumn( "Category", - key="category", model_class=model.Category, visible=False ) ] @@ -332,7 +312,7 @@ class CommonController( BaseController, ItemRatings ): action='browse_tools', message='You are not allowed to view this tool', status='error' ) ) - avg_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, tool ) + avg_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, tool, webapp_model=trans.model ) 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 ) @@ -343,7 +323,7 @@ class CommonController( BaseController, ItemRatings ): categories = [ tca.category for tca in tool.categories ] display_reviews = util.string_as_bool( params.get( 'display_reviews', False ) ) tool_file_contents = tarfile.open( tool.file_name, 'r' ).getnames() - tra = self.get_user_item_rating( trans.sa_session, trans.user, tool ) + tra = self.get_user_item_rating( trans.sa_session, trans.user, tool, webapp_model=trans.model ) visible_versions = trans.app.security_agent.get_visible_versions( trans.user, trans.user_is_admin(), cntrller, tool ) if tool.is_rejected: # Include the comments regarding the reason for rejection @@ -506,13 +486,13 @@ class CommonController( BaseController, ItemRatings ): rating = int( params.get( 'rating', '0' ) ) comment = util.restore_text( params.get( 'comment', '' ) ) rating = self.rate_item( trans, trans.user, tool, rating, comment ) - avg_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, tool ) + avg_rating, num_ratings = self.get_ave_item_rating_data( trans.sa_session, tool, webapp_model=trans.model ) 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 ) display_reviews = util.string_as_bool( params.get( 'display_reviews', False ) ) - tra = self.get_user_item_rating( trans.sa_session, trans.user, tool ) + tra = self.get_user_item_rating( trans.sa_session, trans.user, tool, webapp_model=trans.model ) return trans.fill_template( '/webapps/community/common/rate_tool.mako', cntrller=cntrller, tool=tool, diff --git a/lib/galaxy/webapps/community/controllers/tool.py b/lib/galaxy/webapps/community/controllers/tool.py index 3f6ddc890d9..2d3a7b78509 100644 --- a/lib/galaxy/webapps/community/controllers/tool.py +++ b/lib/galaxy/webapps/community/controllers/tool.py @@ -37,7 +37,6 @@ class ApprovedToolListGrid( ToolListGrid ): columns = [ col for col in ToolListGrid.columns ] columns.append( StateColumn( "Status", - model_class=model.Tool, link=( lambda item: dict( operation="tools_by_state", id=item.id, webapp="community" ) ), visible=False, attach_popup=False ) @@ -45,7 +44,6 @@ class ApprovedToolListGrid( ToolListGrid ): columns.append( ToolStateColumn( "State", key="state", - model_class=model.Tool, visible=False, filterable="advanced" ) ) @@ -54,7 +52,6 @@ class MyToolsListGrid( ApprovedToolListGrid ): columns = [ col for col in ToolListGrid.columns ] columns.append( StateColumn( "Status", - model_class=model.Tool, link=( lambda item: dict( operation="tools_by_state", id=item.id, webapp="community" ) ), visible=True, attach_popup=False ) @@ -62,7 +59,6 @@ class MyToolsListGrid( ApprovedToolListGrid ): columns.append( ToolStateColumn( "State", key="state", - model_class=model.Tool, visible=False, filterable="advanced" ) ) diff --git a/lib/galaxy/webapps/community/datatypes/__init__.py b/lib/galaxy/webapps/community/datatypes/__init__.py index 9982712315c..8da7949a782 100644 --- a/lib/galaxy/webapps/community/datatypes/__init__.py +++ b/lib/galaxy/webapps/community/datatypes/__init__.py @@ -82,6 +82,12 @@ class Tool( object ): log.exception( 'fail:' ) continue if root.tag == 'tool': + if 'id' not in root.keys(): + raise DatatypeVerificationError( "Tool xml file (%s) does not include the required 'id' attribute in the tag" % str( xml_file ) ) + if 'name' not in root.keys(): + raise DatatypeVerificationError( "Tool xml file (%s) does not include the required 'name' attribute in the tag" % str( xml_file ) ) + if 'version' not in root.keys(): + raise DatatypeVerificationError( "Tool xml file (%s) does not include the required 'version' attribute in the tag" % str( xml_file ) ) if tool_tags: # We are verifying the tools inside a tool suite, so the current tag should have been found in the suite_config.xml # file parsed in the ToolSuite verify() method. The tool_tags dictionary should include a key matching the current