Merge pull request #3549 from GeotrekCE/fix_cascading_deletions

Revise cascading deletions

docs/changelog.rst

@@ -16,6 +16,10 @@ CHANGELOG
 
 - Upgrade `django-large-image` and `pip-tools`
 
+**Improvements**
+
+- Improve cascading deletions logic, and log them to LogEntry model to maintain history of deletions
+
 
 2.98.0     (2023-03-27)
 -----------------------

docs/contribute/development.rst

@@ -72,7 +72,11 @@ When updating or adding a new field ``my_field`` to a model ``MyModel``, please
 
 **In** ``MyModel`` **class** :
 
-- If ``my_field`` is a ``ForeignKey``, make sure to override ``related_name`` with an explicit set name
+- If ``my_field`` is a ``ForeignKey``:
+
+    - make sure to override ``related_name`` with an explicit set name
+
+    - if ``my_field`` causes cascading deletion (argument ``on_delete=models.CASCADE``), make sure to log potential deletions (see example ``log_cascade_deletion_from_sector_practice`` in ``geotrek/outdoor/models.py``)
 
 - Make sure to set ``verbose_name`` on the field and add proper translations in ``.po`` files
 

geotrek/api/tests/test_v2.py

@@ -45,6 +45,8 @@
 from geotrek.zoning import models as zoning_models
 from geotrek.zoning.tests import factories as zoning_factory
 
+from mapentity.middleware import clear_internal_user_cache
+
 PAGINATED_JSON_STRUCTURE = sorted([
     'count', 'next', 'previous', 'results',
 ])
@@ -266,6 +268,10 @@
 class BaseApiTest(TestCase):
     """ Base TestCase for all API profiles """
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     @classmethod
     def setUpTestData(cls):
         # This prevents the test APIAccessAnonymousTestCase.test_hdviewpoint_detail_content_poi not passing on some environments.
@@ -2346,7 +2352,7 @@ class APIAccessAdministratorTestCase(BaseApiTest):
     def setUpTestData(cls):
         #  created user
         cls.administrator = SuperUserFactory()
-        BaseApiTest.setUpTestData()
+        super().setUpTestData()
 
     def test_path_list(self):
         self.client.force_login(self.administrator)
@@ -2375,10 +2381,6 @@ def test_path_list(self):
 class APISwaggerTestCase(BaseApiTest):
     """ TestCase API documentation """
 
-    @classmethod
-    def setUpTestData(cls):
-        BaseApiTest.setUpTestData()
-
     def test_schema_fields(self):
         response = self.client.get('/api/v2/', {'format': 'openapi'})
         self.assertContains(response, 'Filter by a bounding box formatted like W-lng,S-lat,E-lng,N-lat (WGS84).')
@@ -2461,6 +2463,10 @@ def setUpTestData(cls):
         cls.scale2 = outdoor_factory.RatingScaleFactory(name='AAA', practice=cls.practice2)
         cls.scale3 = outdoor_factory.RatingScaleFactory(name='BBB', practice=cls.practice2)
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def test_list(self):
         response = self.client.get('/api/v2/outdoor_ratingscale/')
         self.assertEqual(response.status_code, 200)
@@ -2519,6 +2525,10 @@ def setUpTestData(cls):
         cls.rating2.treks.set([trek_factory.TrekFactory()])
         cls.rating3.treks.set([trek_factory.TrekFactory()])
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def test_list(self):
         response = self.client.get('/api/v2/trek_rating/')
         self.assertEqual(response.status_code, 200)
@@ -2589,6 +2599,10 @@ def setUpTestData(cls):
         cls.rating2.sites.set([outdoor_factory.SiteFactory()])
         cls.rating3.sites.set([outdoor_factory.SiteFactory()])
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def test_list(self):
         response = self.client.get('/api/v2/outdoor_rating/')
         self.assertEqual(response.status_code, 200)
@@ -2661,6 +2675,10 @@ def setUpTestData(cls):
             title='BBB', published=True, order=1, target='mobile', content='Blbh'
         )
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def test_list(self):
         response = self.client.get('/api/v2/flatpage/')
         self.assertEqual(response.status_code, 200)
@@ -2746,6 +2764,10 @@ def setUpTestData(cls):
         cls.category1 = feedback_factory.ReportCategoryFactory(label="Conflict")
         cls.category2 = feedback_factory.ReportCategoryFactory(label="Literring")
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def test_status_list(self):
         response = self.client.get('/api/v2/feedback_status/')
         self.assertEqual(response.status_code, 200)
@@ -2843,6 +2865,10 @@ def setUpTestData(cls):
         cls.tc3 = tourism_factory.TouristicContentFactory(name_fr="BAA", name_en="AAA", published_fr=True, published_en=True)
         cls.tc4 = tourism_factory.TouristicContentFactory(name_fr="CCC", name_en="CCC", published_fr=True, published_en=True)
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def assert_ordered_by_language(self, endpoint, ordered_ids, language):
         # GET request on list with language param
         response = self.client.get(reverse(endpoint), {'language': language})
@@ -2883,6 +2909,10 @@ def setUpTestData(cls):
         cls.web_link_cat2 = trek_factory.WebLinkCategoryFactory(pictogram='dummy_picto2.png', label="To see")
         cls.web_link_cat3 = trek_factory.WebLinkCategoryFactory(pictogram='dummy_picto3.png', label="To eat")
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def test_web_links_category_list(self):
         response = self.client.get(reverse('apiv2:weblink-category-list'))
         self.assertEqual(response.status_code, 200)
@@ -2926,6 +2956,10 @@ def setUpTestData(cls):
         cls.web_link2 = trek_factory.WebLinkFactory(category=cls.web_link_cat, name="Web link", name_en="Web link", url="http://dummy.url")
         cls.trek1 = trek_factory.TrekFactory(web_links=[cls.web_link1, cls.web_link2])
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def test_web_links_in_trek_list(self):
         response = self.client.get(reverse('apiv2:trek-list'))
         self.assertEqual(response.status_code, 200)
@@ -2968,6 +3002,10 @@ def setUpTestData(cls):
         cls.trek_hard = trek_factory.TrekFactory(difficulty=cls.hard)
         cls.trek_v_hard = trek_factory.TrekFactory(difficulty=cls.v_hard)
 
+    def tearDown(self):
+        clear_internal_user_cache()
+        super().tearDown()
+
     def assert_trek_is_in_reponse(self, response, expected_trek):
         found = list(filter(lambda trek: trek['id'] == expected_trek.pk, response.json()['results']))
         self.assertTrue(found)

geotrek/authent/migrations/0011_alter_userprofile_structure.py

@@ -0,0 +1,20 @@
+# Generated by Django 3.2.18 on 2023-04-07 08:15
+
+from django.db import migrations, models
+import django.db.models.deletion
+import geotrek.authent.models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('authent', '0010_auto_20221110_1128'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='userprofile',
+            name='structure',
+            field=models.ForeignKey(default=geotrek.authent.models.default_structure_pk, on_delete=django.db.models.deletion.PROTECT, to='authent.structure', verbose_name='Related structure'),
+        ),
+    ]

geotrek/authent/models.py

@@ -49,7 +49,7 @@ class StructureRelated(models.Model):
     """
     A mixin used for any entities that belong to a structure
     """
-    structure = models.ForeignKey(Structure, default=default_structure_pk, on_delete=models.CASCADE,
+    structure = models.ForeignKey(Structure, default=default_structure_pk, on_delete=models.PROTECT,
                                   verbose_name=_("Related structure"))
 
     check_structure_in_forms = True
@@ -71,7 +71,7 @@ class StructureOrNoneRelated(models.Model):
     """
     A mixin used for any entities that belong to a structure or None entity
     """
-    structure = models.ForeignKey(Structure, on_delete=models.CASCADE,
+    structure = models.ForeignKey(Structure, on_delete=models.PROTECT,
                                   verbose_name=_("Related structure"), blank=True, null=True)
 
     check_structure_in_forms = True

geotrek/common/migrations/0031_auto_20230407_0947.py

@@ -0,0 +1,64 @@
+# Generated by Django 3.2.18 on 2023-04-07 09:47
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('contenttypes', '0002_remove_content_type_name'),
+        ('cirkwi', '0003_auto_20200406_1353'),
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('authent', '0011_alter_userprofile_structure'),
+        ('common', '0030_auto_20230202_1535'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='accessibilityattachment',
+            name='content_type',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, to='contenttypes.contenttype'),
+        ),
+        migrations.AlterField(
+            model_name='accessibilityattachment',
+            name='creator',
+            field=models.ForeignKey(help_text='User that uploaded', null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='created_attachments_accessibility', to=settings.AUTH_USER_MODEL, verbose_name='Creator'),
+        ),
+        migrations.AlterField(
+            model_name='accessibilityattachment',
+            name='license',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to='common.license', verbose_name='License'),
+        ),
+        migrations.AlterField(
+            model_name='filetype',
+            name='structure',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='authent.structure', verbose_name='Related structure'),
+        ),
+        migrations.AlterField(
+            model_name='hdviewpoint',
+            name='content_type',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.DO_NOTHING, to='contenttypes.contenttype'),
+        ),
+        migrations.AlterField(
+            model_name='hdviewpoint',
+            name='license',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to='common.license', verbose_name='License'),
+        ),
+        migrations.AlterField(
+            model_name='license',
+            name='structure',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='authent.structure', verbose_name='Related structure'),
+        ),
+        migrations.AlterField(
+            model_name='organism',
+            name='structure',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='authent.structure', verbose_name='Related structure'),
+        ),
+        migrations.AlterField(
+            model_name='theme',
+            name='cirkwi',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='cirkwi.cirkwitag', verbose_name='Cirkwi tag'),
+        ),
+    ]

geotrek/common/migrations/0032_auto_20230407_1337.py

@@ -0,0 +1,25 @@
+# Generated by Django 3.2.18 on 2023-04-07 13:37
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('contenttypes', '0002_remove_content_type_name'),
+        ('common', '0031_auto_20230407_0947'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='accessibilityattachment',
+            name='content_type',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='contenttypes.contenttype'),
+        ),
+        migrations.AlterField(
+            model_name='hdviewpoint',
+            name='content_type',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='contenttypes.contenttype'),
+        ),
+    ]

geotrek/common/migrations/0033_auto_20230503_0837.py

@@ -0,0 +1,30 @@
+# Generated by Django 3.2.18 on 2023-05-03 08:37
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('authent', '0011_alter_userprofile_structure'),
+        ('common', '0032_auto_20230407_1337'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='filetype',
+            name='structure',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to='authent.structure', verbose_name='Related structure'),
+        ),
+        migrations.AlterField(
+            model_name='license',
+            name='structure',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to='authent.structure', verbose_name='Related structure'),
+        ),
+        migrations.AlterField(
+            model_name='organism',
+            name='structure',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to='authent.structure', verbose_name='Related structure'),
+        ),
+    ]

geotrek/common/migrations/0034_alter_accessibilityattachment_creator.py

@@ -0,0 +1,21 @@
+# Generated by Django 3.2.18 on 2023-05-04 13:10
+
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('common', '0033_auto_20230503_0837'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='accessibilityattachment',
+            name='creator',
+            field=models.ForeignKey(help_text='User that uploaded', on_delete=django.db.models.deletion.PROTECT, related_name='created_attachments_accessibility', to='auth.user', verbose_name='Creator'),
+        ),
+    ]

geotrek/common/models.py

@@ -56,7 +56,7 @@ class InfoAccessibilityChoices(models.TextChoices):
 
     objects = AccessibilityAttachmentManager()
 
-    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
+    content_type = models.ForeignKey(ContentType, on_delete=models.PROTECT)
     object_id = models.PositiveIntegerField()
     content_object = GenericForeignKey('content_type', 'object_id')
 
@@ -70,13 +70,13 @@ class InfoAccessibilityChoices(models.TextChoices):
     license = models.ForeignKey(settings.PAPERCLIP_LICENSE_MODEL,
                                 verbose_name=_("License"),
                                 null=True, blank=True,
-                                on_delete=models.SET_NULL)
+                                on_delete=models.PROTECT)
     creation_date = models.DateField(verbose_name=_("Creation Date"), null=True, blank=True)
     uuid = models.UUIDField(default=uuid.uuid4, editable=False, unique=True)
     creator = models.ForeignKey(settings.AUTH_USER_MODEL,
                                 related_name="created_attachments_accessibility",
                                 verbose_name=_('Creator'),
-                                help_text=_("User that uploaded"), on_delete=models.CASCADE)
+                                help_text=_("User that uploaded"), on_delete=models.PROTECT)
     author = models.CharField(blank=True, default='', max_length=128,
                               verbose_name=_('Author'),
                               help_text=_("Original creator"))
@@ -189,7 +189,7 @@ class Attachment(BaseAttachment):
 
 class Theme(TimeStampedModelMixin, PictogramMixin):
     label = models.CharField(verbose_name=_("Name"), max_length=128)
-    cirkwi = models.ForeignKey('cirkwi.CirkwiTag', verbose_name=_("Cirkwi tag"), null=True, blank=True, on_delete=models.CASCADE)
+    cirkwi = models.ForeignKey('cirkwi.CirkwiTag', verbose_name=_("Cirkwi tag"), null=True, blank=True, on_delete=models.SET_NULL)
 
     class Meta:
         verbose_name = _("Theme")
@@ -323,7 +323,7 @@ class HDViewPoint(TimeStampedModelMixin, MapEntityMixin):
     geom = gis_models.PointField(verbose_name=_("Location"),
                                  srid=settings.SRID)
     object_id = models.PositiveIntegerField()
-    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE)
+    content_type = models.ForeignKey(ContentType, on_delete=models.PROTECT)
     content_object = GenericForeignKey('content_type', 'object_id')
     annotations = models.JSONField(verbose_name=_("Annotations"), blank=True, default=dict)
     uuid = models.UUIDField(default=uuid.uuid4, editable=False, unique=True)
@@ -339,7 +339,7 @@ class HDViewPoint(TimeStampedModelMixin, MapEntityMixin):
     license = models.ForeignKey(settings.PAPERCLIP_LICENSE_MODEL,
                                 verbose_name=_("License"),
                                 null=True, blank=True,
-                                on_delete=models.SET_NULL)
+                                on_delete=models.PROTECT)
 
     class Meta:
         verbose_name = _("HD View")