Simplify rating classes initialization (see note)

1. Remove id parameter from constructor (this is created by the db).
2. Remove `item` instance attribute: it's not used and is meaningless.
3. Make user and item parameters in the constructor required: a rating
   cannot exist without a rater or the item being rated.
4. For the same reason as in #2, make the `set_item()` method private
   and only call it from the constructor.
5. Simplify mapping test: just use the constructor.

lib/galaxy/managers/ratable.py

@@ -49,7 +49,7 @@ def rate(self, item, user, value, flush=True):
         # TODO?: update and create to RatingsManager (if not overkill)
         rating = self.rating(item, user, as_int=False)
         if not rating:
-            rating = self.rating_assoc(user=user)
+            rating = self.rating_assoc(user, item)
             self.associate(rating, item)
         rating.rating = value
 

lib/galaxy/model/__init__.py

@@ -6874,48 +6874,48 @@ class LibraryDatasetCollectionAnnotationAssociation(RepresentById):
 
 # Item rating classes.
 class ItemRatingAssociation:
-    def __init__(self, id=None, user=None, item=None, rating=0):
-        self.id = id
+    def __init__(self, user, item, rating=0):
         self.user = user
-        self.item = item
         self.rating = rating
+        self._set_item(item)
 
-    def set_item(self, item):
+    def _set_item(self, item):
         """ Set association's item. """
+        raise NotImplementedError()
 
 
 class HistoryRatingAssociation(ItemRatingAssociation, RepresentById):
-    def set_item(self, history):
+    def _set_item(self, history):
         self.history = history
 
 
 class HistoryDatasetAssociationRatingAssociation(ItemRatingAssociation, RepresentById):
-    def set_item(self, history_dataset_association):
+    def _set_item(self, history_dataset_association):
         self.history_dataset_association = history_dataset_association
 
 
 class StoredWorkflowRatingAssociation(ItemRatingAssociation, RepresentById):
-    def set_item(self, stored_workflow):
+    def _set_item(self, stored_workflow):
         self.stored_workflow = stored_workflow
 
 
 class PageRatingAssociation(ItemRatingAssociation, RepresentById):
-    def set_item(self, page):
+    def _set_item(self, page):
         self.page = page
 
 
 class VisualizationRatingAssociation(ItemRatingAssociation, RepresentById):
-    def set_item(self, visualization):
+    def _set_item(self, visualization):
         self.visualization = visualization
 
 
 class HistoryDatasetCollectionRatingAssociation(ItemRatingAssociation, RepresentById):
-    def set_item(self, dataset_collection):
+    def _set_item(self, dataset_collection):
         self.dataset_collection = dataset_collection
 
 
 class LibraryDatasetCollectionRatingAssociation(ItemRatingAssociation, RepresentById):
-    def set_item(self, dataset_collection):
+    def _set_item(self, dataset_collection):
         self.dataset_collection = dataset_collection
 
 

lib/galaxy/model/item_attrs.py

@@ -43,10 +43,7 @@ def rate_item(self, db_session, user, item, rating, webapp_model=None):
         if not item_rating:
             # User has not yet rated item; create rating.
             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)
-            item_rating.rating = rating
+            item_rating = item_rating_assoc_class(user, item, rating)
             db_session.add(item_rating)
             db_session.flush()
         elif item_rating.rating != rating:

test/unit/data/test_galaxy_mapping.py

@@ -130,52 +130,50 @@ def persist_and_check_annotation(annotation_class, **kwds):
     def test_ratings(self):
         model = self.model
 
-        u = model.User(email="[email protected]", password="password")
+        user_email = "[email protected]"
+        u = model.User(email=user_email, password="password")
         self.persist(u)
 
-        def persist_and_check_rating(rating_class, **kwds):
-            rating_association = rating_class()
-            rating_association.rating = 5
-            rating_association.user = u
-            for key, value in kwds.items():
-                setattr(rating_association, key, value)
+        def persist_and_check_rating(rating_class, item):
+            rating = 5
+            rating_association = rating_class(u, item, rating)
             self.persist(rating_association)
             self.expunge()
-            stored_annotation = self.query(rating_class).all()[0]
-            assert stored_annotation.rating == 5
-            assert stored_annotation.user.email == "[email protected]"
+            stored_rating = self.query(rating_class).all()[0]
+            assert stored_rating.rating == rating
+            assert stored_rating.user.email == user_email
 
         sw = model.StoredWorkflow()
         sw.user = u
         self.persist(sw)
-        persist_and_check_rating(model.StoredWorkflowRatingAssociation, stored_workflow=sw)
+        persist_and_check_rating(model.StoredWorkflowRatingAssociation, sw)
 
         h = model.History(name="History for Rating", user=u)
         self.persist(h)
-        persist_and_check_rating(model.HistoryRatingAssociation, history=h)
+        persist_and_check_rating(model.HistoryRatingAssociation, h)
 
         d1 = model.HistoryDatasetAssociation(extension="txt", history=h, create_dataset=True, sa_session=model.session)
         self.persist(d1)
-        persist_and_check_rating(model.HistoryDatasetAssociationRatingAssociation, hda=d1)
+        persist_and_check_rating(model.HistoryDatasetAssociationRatingAssociation, d1)
 
         page = model.Page()
         page.user = u
         self.persist(page)
-        persist_and_check_rating(model.PageRatingAssociation, page=page)
+        persist_and_check_rating(model.PageRatingAssociation, page)
 
         visualization = model.Visualization()
         visualization.user = u
         self.persist(visualization)
-        persist_and_check_rating(model.VisualizationRatingAssociation, visualization=visualization)
+        persist_and_check_rating(model.VisualizationRatingAssociation, visualization)
 
         dataset_collection = model.DatasetCollection(collection_type="paired")
         history_dataset_collection = model.HistoryDatasetCollectionAssociation(collection=dataset_collection)
         self.persist(history_dataset_collection)
-        persist_and_check_rating(model.HistoryDatasetCollectionRatingAssociation, history_dataset_collection=history_dataset_collection)
+        persist_and_check_rating(model.HistoryDatasetCollectionRatingAssociation, history_dataset_collection)
 
         library_dataset_collection = model.LibraryDatasetCollectionAssociation(collection=dataset_collection)
         self.persist(library_dataset_collection)
-        persist_and_check_rating(model.LibraryDatasetCollectionRatingAssociation, library_dataset_collection=library_dataset_collection)
+        persist_and_check_rating(model.LibraryDatasetCollectionRatingAssociation, library_dataset_collection)
 
     def test_display_name(self):