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

@@ -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 <item_class>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 <item_class>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 <item_class>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 ):

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() ) 

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"
                     ),

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 <item_class>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 += '<input name="avg_rating" type="radio" class="star" value="%s" disabled="disabled"' % str( index )
-            if avg_rating > ( 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,

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" )
     )

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 <tool> 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 <tool> 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 <tool> 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