From 695914797dad0bdd8f0e39ab89bfb7dc99405650 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CE=9Blisue?= Date: Tue, 14 Oct 2014 09:49:07 +0900 Subject: [PATCH] Fix content_object issue - Remove `content_object` attribute which can be `None` for deleted object - Use `snapshot` instead of `content_object` which would not be `None` - Stop defering `snapshot` to prevent N+1 problem --- src/kawaz/apps/activities/mediator.py | 19 +++++++------------ src/kawaz/apps/activities/models.py | 5 ----- src/kawaz/apps/activities/registry.py | 3 ++- .../apps/activities/tests/test_mediator.py | 2 +- .../apps/activities/tests/test_models.py | 2 +- .../apps/activities/tests/test_registry.py | 2 +- 6 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/kawaz/apps/activities/mediator.py b/src/kawaz/apps/activities/mediator.py index b350c521..5bff1ff8 100644 --- a/src/kawaz/apps/activities/mediator.py +++ b/src/kawaz/apps/activities/mediator.py @@ -16,8 +16,6 @@ class ActivityMediator(object): An ActivityMediator class which has responsivilities to controll automatic activity creation (with signal handling) or rendering. """ - use_snapshot = False - def _pre_delete_receiver(self, sender, instance, **kwargs): ct = ContentType.objects.get_for_model(instance) activity = Activity(content_type=ct, @@ -26,9 +24,8 @@ def _pre_delete_receiver(self, sender, instance, **kwargs): # call user defined alternation code activity = self.alter(instance, activity, **kwargs) if activity: - # save current instance snapshot if 'use_snapshot' - if self.use_snapshot: - activity.snapshot = instance + # save current instance as a snapshot + activity.snapshot = instance activity.save() def _post_save_receiver(self, sender, instance, created, **kwargs): @@ -39,9 +36,8 @@ def _post_save_receiver(self, sender, instance, created, **kwargs): # call user defined alternation code activity = self.alter(instance, activity, **kwargs) if activity: - # save current instance snapshot if 'use_snapshot' - if self.use_snapshot: - activity.snapshot = instance + # save current instance as a snapshot + activity.snapshot = instance activity.save() def _m2m_changed_receiver(self, sender, instance, **kwargs): @@ -49,9 +45,8 @@ def _m2m_changed_receiver(self, sender, instance, **kwargs): # user need to create activity instance activity = self.alter(instance, None, **kwargs) if activity: - # save current instance snapshot if 'use_snapshot' - if self.use_snapshot: - activity.snapshot = instance + # save current instance as a snapshot + activity.snapshot = instance activity.save() def connect(self, model): @@ -109,7 +104,7 @@ def prepare_context(self, activity, context): """ context.update({ 'activity': activity, - 'object': activity.content_object + 'object': activity.snapshot }) return context diff --git a/src/kawaz/apps/activities/models.py b/src/kawaz/apps/activities/models.py index 1ee7c772..bb71924d 100644 --- a/src/kawaz/apps/activities/models.py +++ b/src/kawaz/apps/activities/models.py @@ -14,10 +14,6 @@ class ActivityManager(models.Manager): - def get_queryset(self): - # defer '_snapshot' column which only required during signal handling - # to accelerate database access - return super().get_queryset().defer('_snapshot') def latests(self): """ @@ -43,7 +39,6 @@ class Activity(models.Model): content_type = models.ForeignKey(ContentType) object_id = models.PositiveIntegerField('Object ID') - content_object = GenericForeignKey() _snapshot = models.BinaryField(default=None, null=True) created_at = models.DateTimeField(auto_now_add=True) diff --git a/src/kawaz/apps/activities/registry.py b/src/kawaz/apps/activities/registry.py index b15e2df4..be0b4b0a 100644 --- a/src/kawaz/apps/activities/registry.py +++ b/src/kawaz/apps/activities/registry.py @@ -2,6 +2,7 @@ """ """ __author__ = 'Alisue ' +from django.contrib.contenttypes.models import ContentType from .mediator import ActivityMediator @@ -38,7 +39,7 @@ def get(self, activity): """ Get connected activity mediator of a model which the activity has. """ - model = activity.content_object._meta.model + model = activity.content_type.model_class() return self._registry[model] diff --git a/src/kawaz/apps/activities/tests/test_mediator.py b/src/kawaz/apps/activities/tests/test_mediator.py index c336b60a..d747247c 100644 --- a/src/kawaz/apps/activities/tests/test_mediator.py +++ b/src/kawaz/apps/activities/tests/test_mediator.py @@ -137,7 +137,7 @@ def test_prepare_context(self): context.update.assert_called_with({ 'activity': activity, - 'object': activity.content_object, + 'object': activity.snapshot, }) self.assertEqual(c, context) diff --git a/src/kawaz/apps/activities/tests/test_models.py b/src/kawaz/apps/activities/tests/test_models.py index 20ee66d2..d8073034 100644 --- a/src/kawaz/apps/activities/tests/test_models.py +++ b/src/kawaz/apps/activities/tests/test_models.py @@ -66,10 +66,10 @@ def test_attributes(self): activity = Activity() self.assertTrue(hasattr(activity, 'status')) self.assertTrue(hasattr(activity, 'remarks')) - self.assertTrue(hasattr(activity, 'content_object')) self.assertTrue(hasattr(activity, 'object_id')) self.assertTrue(hasattr(activity, 'created_at')) self.assertTrue(hasattr(activity, '_snapshot')) + self.assertTrue(hasattr(activity, 'snapshot')) def test_snapshot(self): model = self.models[0] diff --git a/src/kawaz/apps/activities/tests/test_registry.py b/src/kawaz/apps/activities/tests/test_registry.py index c83e8de8..443196b0 100644 --- a/src/kawaz/apps/activities/tests/test_registry.py +++ b/src/kawaz/apps/activities/tests/test_registry.py @@ -37,6 +37,6 @@ def test_get(self): registry.register(model, mediator) activity = MagicMock() - activity.content_object._meta.model = model + activity.content_type.model_class = MagicMock(return_value=model) self.assertEqual(registry.get(activity), mediator)