From aa59c00f974dd4cb1c6ff00187ca975582fa19e3 Mon Sep 17 00:00:00 2001 From: Florent Chehab Date: Sun, 24 Mar 2019 18:47:35 +0100 Subject: [PATCH] fix(nb_versions): coherence * Added signal to catch version deletion and make sure nb_version in versioned models is coherent * Added test for this * Made sure pending moderation data (that was automatic) and versions are deleted when a model is deleted. * Added test for this. Fixes #84 --- backend/backend_app/apps.py | 1 + .../abstract/versionedEssentialModule.py | 11 +++ backend/backend_app/models/version.py | 5 +- .../backend_app/signals/update_nb_version.py | 15 ++++ .../backend_app/tests/test_delete_cascade.py | 80 +++++++++++++++++++ ...st_nb_version_updated_on_version_delete.py | 47 +++++++++++ backend/base_app/settings/app_settings.py | 10 ++- backend/base_app/settings/main.py | 2 +- 8 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 backend/backend_app/signals/update_nb_version.py create mode 100644 backend/backend_app/tests/test_delete_cascade.py create mode 100644 backend/backend_app/tests/test_nb_version_updated_on_version_delete.py diff --git a/backend/backend_app/apps.py b/backend/backend_app/apps.py index 1d83a1e4..d7b0cf51 100644 --- a/backend/backend_app/apps.py +++ b/backend/backend_app/apps.py @@ -7,3 +7,4 @@ class BackendAppConfig(AppConfig): def ready(self): import backend_app.signals.squash_revisions # noqa:F401 import backend_app.signals.auto_creation # noqa:F401 + import backend_app.signals.update_nb_version # noqa:F401 diff --git a/backend/backend_app/models/abstract/versionedEssentialModule.py b/backend/backend_app/models/abstract/versionedEssentialModule.py index 2322fc82..5fb74a33 100644 --- a/backend/backend_app/models/abstract/versionedEssentialModule.py +++ b/backend/backend_app/models/abstract/versionedEssentialModule.py @@ -8,6 +8,7 @@ from backend_app.models.abstract.essentialModule import ( ) from backend_app.signals.squash_revisions import new_revision_saved from rest_framework import serializers +from reversion.models import Version class VersionedEssentialModule(EssentialModule): @@ -27,6 +28,16 @@ class VersionedEssentialModule(EssentialModule): """ raise Exception("Get_serializer must be redefined in subclass") + def delete(self, using=None, keep_parents=False): + """ + Override the default delete behavior to make sure + versions instances corresponding to the + deleted instance are also deleted. + """ + # We need to delete the versions first. Otherwise for some reason it wouldn't work. + Version.objects.get_for_object(self).delete() + super().delete(using, keep_parents) + class Meta: abstract = True diff --git a/backend/backend_app/models/version.py b/backend/backend_app/models/version.py index 9ef38842..c275fa5f 100644 --- a/backend/backend_app/models/version.py +++ b/backend/backend_app/models/version.py @@ -32,9 +32,10 @@ class VersionSerializer(BaseModelSerializer): return obj_serializer(tmp.object, context=new_context).data except (DeserializationError, djangoSerializers.SerializerDoesNotExist): # The version is not valid regarding the model, we should delete it ! - # This might be due to an updated model. + # This might be due to an updated model structure at some point. obj.delete() - # Might result in inconsistent nb of versions but that's fine. + # We take care of the nb_versions field update with signals. + # So it will remain coherent. return None class Meta: diff --git a/backend/backend_app/signals/update_nb_version.py b/backend/backend_app/signals/update_nb_version.py new file mode 100644 index 00000000..2d097372 --- /dev/null +++ b/backend/backend_app/signals/update_nb_version.py @@ -0,0 +1,15 @@ +from reversion.models import Version +from django.db.models.signals import post_delete + + +def update_nb_version_on_version_delete(sender, instance, **kwargs): + pk = instance.object_id + ct = instance.content_type + + obj = ct.get_object_for_this_type(pk=pk) + # Make sure we have a new coherent value + obj.nb_versions = len(Version.objects.get_for_object(obj)) + obj.save() + + +post_delete.connect(update_nb_version_on_version_delete, sender=Version) diff --git a/backend/backend_app/tests/test_delete_cascade.py b/backend/backend_app/tests/test_delete_cascade.py new file mode 100644 index 00000000..063e1de5 --- /dev/null +++ b/backend/backend_app/tests/test_delete_cascade.py @@ -0,0 +1,80 @@ +from django.test import override_settings +from reversion.models import Version + +from backend_app.models.for_testing.versioning import ForTestingVersioning +from backend_app.tests.utils import WithUserTestCase +from backend_app.models.for_testing.moderation import ForTestingModeration +from backend_app.models.pendingModeration import PendingModeration +from django.contrib.contenttypes.models import ContentType + + +def retrieve_instance_in_moderation(instance, should_be_empty=False): + ct = ContentType.objects.get_for_model(ForTestingModeration) + pending = PendingModeration.objects.filter(content_type=ct, object_id=instance.pk) + + if should_be_empty: + assert len(pending) == 0 + return None + else: + assert len(pending) > 0 + return pending[0] + + +def get_object_moder(pk): + return ForTestingModeration.objects.filter(pk=pk)[0] + + +def get_object_versions(obj): + return Version.objects.get_for_object(obj) + + +class CascadeDeleteModerationTestCase(WithUserTestCase): + @classmethod + def setUpMoreTestData(cls): + cls.obj = ForTestingModeration.objects.create(aaa="v0") + cls.api_moderation = "/api/test/moderation/{}/".format(cls.obj.pk) + + @override_settings(MODERATION_ACTIVATED=True) + def test_delete_model_cascade_to_pending_moderation(self): + """ + Test to check that when a model instance is deleted, all pending moderation + data related to that instance is also deleted. + """ + + data_1 = {"aaa": "Test"} + self.authenticated_client.put(self.api_moderation, data_1) + + new_obj_in_db = get_object_moder(self.obj.pk) + self.assertTrue(new_obj_in_db.has_pending_moderation) + + retrieve_instance_in_moderation(new_obj_in_db, should_be_empty=False) + new_obj_in_db.delete() + retrieve_instance_in_moderation(new_obj_in_db, should_be_empty=True) + + # Really make sure there is nothing pending moderation + self.assertEqual(len(PendingModeration.objects.all()), 0) + + +class CascadeDeleteVersioningTestCase(WithUserTestCase): + @classmethod + def setUpMoreTestData(cls): + cls.obj = ForTestingVersioning.objects.create(bbb="v0") + cls.api_version = "/api/test/versioning/{}/".format(cls.obj.pk) + + def test_delete_model_cascade_to_versions(self): + """ + Test to check that when a versioned model instance is deleted, all + other versions of this model is also deleted. + """ + + data_1 = {"bbb": "Test"} + self.staff_client.put(self.api_version, data_1) + data_2 = {"bbb": "Test"} + self.moderator_client.put(self.api_version, data_2) + self.assertEqual(len(get_object_versions(self.obj)), 2) + + self.obj.delete() + self.assertEqual(len(get_object_versions(self.obj)), 0) + + # Really make sure all versions have been deleted + self.assertEqual(len(Version.objects.all()), 0) diff --git a/backend/backend_app/tests/test_nb_version_updated_on_version_delete.py b/backend/backend_app/tests/test_nb_version_updated_on_version_delete.py new file mode 100644 index 00000000..b65f9401 --- /dev/null +++ b/backend/backend_app/tests/test_nb_version_updated_on_version_delete.py @@ -0,0 +1,47 @@ +from reversion.models import Version + +from backend_app.models.for_testing.versioning import ForTestingVersioning +from backend_app.tests.utils import WithUserTestCase + + +def get_object_ver(obj): + return ForTestingVersioning.objects.filter(pk=obj.pk)[0] + + +def get_object_versions(obj): + return Version.objects.get_for_object(obj) + + +class CascadeDeleteVersionsTestCase(WithUserTestCase): + @classmethod + def setUpMoreTestData(cls): + cls.obj = ForTestingVersioning.objects.create(bbb="v0") + cls.api_version = "/api/test/versioning/{}/".format(cls.obj.pk) + + def test_delete_model_cascade_to_versions(self): + """ + Test to check that when a versioned model instance is deleted, all + other versions of this model is also deleted. + """ + + data_1 = {"bbb": "Test"} + self.staff_client.put(self.api_version, data_1) + + # Make sure we have a version + versions = get_object_versions(self.obj) + self.assertEqual(len(versions), 1) + + # Make sure we have the correct version number + new_obj = get_object_ver(self.obj) + self.assertEqual(new_obj.nb_versions, 1) + + versions.delete() + # Make sure they really have been deleted + versions = get_object_versions(self.obj) + self.assertEqual(len(versions), 0) + + # Re-get the object, to make sure we have updated fields + new_obj = get_object_ver(self.obj) + + # Final test + self.assertEqual(new_obj.nb_versions, 0) diff --git a/backend/base_app/settings/app_settings.py b/backend/base_app/settings/app_settings.py index f308a2ff..15d93f11 100644 --- a/backend/base_app/settings/app_settings.py +++ b/backend/base_app/settings/app_settings.py @@ -6,5 +6,11 @@ ALLOWED_PHOTOS_EXTENSION = ["jpg", "jpeg", "png", "svg"] # Configuration of the CAS server CAS_SERVER_URL = "https://cas.utc.fr/cas/" -CAS_APPLY_ATTRIBUTES_TO_USER = True # We want to map the attribute returned by the cas to the user model attribute -CAS_RENAME_ATTRIBUTES = {"mail": "email", "givenName": "first_name", "sn": "last_name"} # Mapping of the attribute +# We want to map the attribute returned by the cas to the user model attribute +CAS_APPLY_ATTRIBUTES_TO_USER = True +# Mapping of the attribute +CAS_RENAME_ATTRIBUTES = { + "mail": "email", + "givenName": "first_name", + "sn": "last_name", +} diff --git a/backend/base_app/settings/main.py b/backend/base_app/settings/main.py index 4e2358e8..d1938b8d 100644 --- a/backend/base_app/settings/main.py +++ b/backend/base_app/settings/main.py @@ -170,7 +170,7 @@ AUTHENTICATION_BACKENDS = [ AUTH_PASSWORD_VALIDATORS = [ { "NAME": "django.contrib.auth.password_validation." - "UserAttributeSimilarityValidator" + "UserAttributeSimilarityValidator" }, {"NAME": "django.contrib.auth.password_validation." "MinimumLengthValidator"}, {"NAME": "django.contrib.auth.password_validation." "CommonPasswordValidator"}, -- GitLab