Fix some problems with activity mixins and migrations

mediagoblin/db/migrations.py

@@ -36,7 +36,7 @@ from mediagoblin.db.extratypes import JSONEncoded, MutationDict
 from mediagoblin.db.migration_tools import (
     RegisterMigration, inspect_table, replace_table_hack)
 from mediagoblin.db.models import (MediaEntry, Collection, MediaComment, User,
-    Privilege, Generator, GenericForeignKey)
+    Privilege, Generator)
 from mediagoblin.db.extratypes import JSONEncoded, MutationDict
 
 
@@ -1289,10 +1289,10 @@ def add_foreign_key_fields(db):
     activity_table = inspect_table(metadata, "core__activities")
 
     # Create column and add to model.
-    object_column = Column("temp_object", Integer, GenericForeignKey())
+    object_column = Column("temp_object", Integer, ForeignKey(GenericModelReference_V0))
     object_column.create(activity_table)
 
-    target_column = Column("temp_target", Integer, GenericForeignKey())
+    target_column = Column("temp_target", Integer, ForeignKey(GenericModelReference_V0))
     target_column.create(activity_table)
 
     # Commit this to the database

mediagoblin/db/mixin.py

@@ -432,13 +432,12 @@ class ActivityMixin(object):
             "audio": _("audio"),
             "person": _("a person"),
         }
-
+        obj = self.object_helper.get_object()
-        obj = self.get_object
+        target = None if self.target_helper is None else self.target_helper.get_object()
-        target = self.get_target
         actor = self.get_actor
         content = verb_to_content.get(self.verb, None)
 
-        if content is None or obj is None:
+        if content is None or self.object is None:
             return
 
         # Decide what to fill the object with
@@ -452,7 +451,7 @@ class ActivityMixin(object):
         # Do we want to add a target (indirect object) to content?
         if target is not None and "targetted" in content:
             if hasattr(target, "title") and target.title.strip(" "):
-                target_value = target.title
+                target_value = terget.title
             elif target.object_type in object_map:
                 target_value = object_map[target.object_type]
             else:

mediagoblin/db/models.py

@@ -61,8 +61,10 @@ class GenericModelReference(Base):
     model_type = Column(Unicode, nullable=False)
 
     # Constrain it so obj_pk and model_type have to be unique
+    # They should be this order as the index is generated, "model_type" will be
+    # the major order as it's put first.
     __table_args__ = (
-        UniqueConstraint("obj_pk", "model_type"),
+        UniqueConstraint("model_type", "obj_pk"),
         {})
 
     def get_object(self):
@@ -108,11 +110,11 @@ class GenericModelReference(Base):
             # the class so it can be shared between all instances
 
             # to prevent circular imports do import here
-            registry = Base._decl_class_registry
+            registry = dict(Base._decl_class_registry).values()
             self._TYPE_MAP = dict(
-                ((m.__tablename__, m) for m in six.itervalues(registry))
+                ((m.__tablename__, m) for m in registry if hasattr(m, "__tablename__"))
             )
-            setattr(self.__class__, "_TYPE_MAP",  self._TYPE_MAP)
+            setattr(type(self), "_TYPE_MAP",  self._TYPE_MAP)
 
         return self.__class__._TYPE_MAP[model_type]
 
@@ -133,16 +135,16 @@ class GenericModelReference(Base):
     @classmethod
     def find_or_new(cls, obj):
         """ Finds an existing GMR or creates a new one for the object """
-        obj = cls.find_for_obj(obj)
+        gmr = cls.find_for_obj(obj)
 
         # If there isn't one already create one
-        if obj is None:
+        if gmr is None:
-            obj = cls(
+            gmr = cls(
                 obj_pk=obj.id,
                 model_type=type(obj).__tablename__
             )
 
-        return obj
+        return gmr
 
 class Location(Base):
     """ Represents a physical location """
@@ -582,7 +584,6 @@ class MediaEntry(Base, MediaEntryMixin):
         return import_component(self.media_type + '.models:BACKREF_NAME')
 
     def __repr__(self):
-        return "<MediaEntry DEBUG>"
         if six.PY2:
             # obj.__repr__() should return a str on Python 2
             safe_title = self.title.encode('utf-8', 'replace')
@@ -715,7 +716,7 @@ class MediaEntry(Base, MediaEntryMixin):
             self.license = data["license"]
 
         if "location" in data:
-            Licence.create(data["location"], self)
+            License.create(data["location"], self)
 
         return True
 
@@ -1373,13 +1374,13 @@ class Activity(Base, ActivityMixin):
 
     # Create the generic foreign keys for the object
     object_id = Column(Integer, ForeignKey(GenericModelReference.id), nullable=False)
-    object_helper = relationship(GenericModelReference)
+    object_helper = relationship(GenericModelReference, foreign_keys=[object_id])
     object = association_proxy("object_helper", "get_object",
                                 creator=GenericModelReference.find_or_new)
 
     # Create the generic foreign Key for the target
-    target_id = Column(Integer, ForeignKey(GenericModelReference), nullable=True)
+    target_id = Column(Integer, ForeignKey(GenericModelReference.id), nullable=True)
-    target_helper = relationship(GenericModelReference)
+    target_helper = relationship(GenericModelReference, foreign_keys=[target_id])
     taget = association_proxy("target_helper", "get_target",
                               creator=GenericModelReference.find_or_new)
 

mediagoblin/tests/test_modelmethods.py

@@ -232,55 +232,3 @@ class TestUserUrlForSelf(MGClientTestCase):
             self.user(u'lindsay').url_for_self(fake_urlgen())
         assert excinfo.errisinstance(TypeError)
         assert 'object is not callable' in str(excinfo)
-
-class TestActivitySetGet(object):
-    """ Test methods on the Activity and ActivityIntermediator models """
-
-    @pytest.fixture(autouse=True)
-    def setup(self, test_app):
-        self.app = test_app
-        self.user = fixture_add_user()
-        self.obj = fixture_media_entry()
-        self.target = fixture_media_entry()
-
-    def test_set_activity_object(self):
-        """ Activity.set_object should produce ActivityIntermediator """
-        # The fixture will set self.obj as the object on the activity.
-        activity = fixture_add_activity(self.obj, actor=self.user)
-
-        # Assert the media has been associated with an AI
-        assert self.obj.activity is not None
-
-        # Assert the AI on the media and object are the same
-        assert activity.object == self.obj.activity
-
-    def test_activity_set_target(self):
-        """ Activity.set_target should produce ActivityIntermediator """
-        # This should set everything needed on the target
-        activity = fixture_add_activity(self.obj, actor=self.user)
-        activity.set_target(self.target)
-
-        # Assert the media has been associated with the AI
-        assert self.target.activity is not None
-
-        # assert the AI on the media and target are the same
-        assert activity.target == self.target.activity
-
-    def test_get_activity_object(self):
-        """ Activity.get_object should return a set object """
-        activity = fixture_add_activity(self.obj, actor=self.user)
-
-        print("self.obj.activity = {0}".format(self.obj.activity))
-
-        # check we now can get the object
-        assert activity.get_object is not None
-        assert activity.get_object.id == self.obj.id
-
-    def test_get_activity_target(self):
-        """ Activity.set_target should return a set target """
-        activity = fixture_add_activity(self.obj, actor=self.user)
-        activity.set_target(self.target)
-
-        # check we can get the target
-        assert activity.get_target is not None
-        assert activity.get_target.id == self.target.id