Merge pull request #3619 from open-formulieren/feature/3558-analytics…

…-csp

[#3558] Add an extra `identifier` field to account for analytics

src/openforms/analytics_tools/migrations/0003_cspsetting_identifier.py

@@ -0,0 +1,78 @@
+# Generated by Django 3.2.21 on 2023-11-22 17:27
+
+from django.db import migrations
+from django.db.migrations.state import StateApps
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+
+from openforms.analytics_tools.constants import AnalyticsTools
+
+SITEIMPROVE_VALUES = [
+    "https://siteimproveanalytics.com",
+    "https://siteimproveanalytics.com",
+    "https://*.siteimproveanalytics.io",
+]
+GA_VALUES = ["https://www.googleanalytics.com", "https://www.googletagmanager.com"]
+
+FIELD_TO_IDENTIFIER = {
+    "matomo_url": AnalyticsTools.matomo,
+    "piwik_pro_url": AnalyticsTools.piwik_pro,
+    "piwik_url": AnalyticsTools.piwik,
+}
+
+
+def set_identifier(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None:
+    """Set the corresponding analytic tool as the ``CSPSetting.identifier`` field.
+
+    Depending on the analytic tool used, the ``CSPSetting.value`` field can be fixed
+    (e.g. with GA or Siteimprove) or configured by the user. The latter requires more work,
+    as we need to iterate over the ``AnalyticsToolsConfiguration`` fields to find the right match.
+    """
+
+    AnalyticsToolsConfiguration = apps.get_model(
+        "analytics_tools", "AnalyticsToolsConfiguration"
+    )
+    try:
+        analytics_conf = AnalyticsToolsConfiguration.objects.get()
+    except AnalyticsToolsConfiguration.DoesNotExist:
+        return
+
+    ContentType = apps.get_model("contenttypes", "ContentType")
+    analytics_content_type = ContentType.objects.get_for_model(
+        AnalyticsToolsConfiguration
+    )
+
+    CSPSetting = apps.get_model("config", "CSPSetting")
+
+    set_content_type = False
+    for csp_setting in CSPSetting.objects.filter(identifier="").iterator():
+        if csp_setting.value in SITEIMPROVE_VALUES:
+            csp_setting.identifier = AnalyticsTools.siteimprove
+            set_content_type = True
+        elif csp_setting.value in GA_VALUES:
+            csp_setting.identifier = AnalyticsTools.google_analytics
+            set_content_type = True
+        else:
+            for field, identifier in FIELD_TO_IDENTIFIER.items():
+                if getattr(analytics_conf, field, None) == csp_setting.value:
+                    csp_setting.identifier = identifier
+                    set_content_type = True
+
+        if set_content_type:
+            # `content_object` is not available in migrations,
+            # so we set `content_type` and `object_id` instead:
+            csp_setting.content_type = analytics_content_type
+            csp_setting.object_id = analytics_conf.id
+
+        csp_setting.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("analytics_tools", "0002_auto_20230119_1500"),
+        ("config", "0063_auto_20231122_1816"),
+    ]
+
+    operations = [
+        migrations.RunPython(set_identifier, migrations.RunPython.noop),
+    ]

src/openforms/analytics_tools/models.py

@@ -240,6 +240,7 @@ def is_google_analytics_enabled(self) -> bool:
 
     def save(self, *args, **kwargs):
         # If instance is being created, we can't find original values
+        # (we use the _state API and not self.pk because `SingletonModel` hardcodes the PK).
         if self._state.adding:
             return super().save(*args, **kwargs)
 

src/openforms/analytics_tools/tests/test_cspsettings_identifier.py

@@ -0,0 +1,44 @@
+from django.contrib.admin.options import get_content_type_for_model
+from django.test import TestCase, override_settings
+
+from openforms.config.models import CSPSetting
+
+from ..constants import AnalyticsTools
+from .mixin import AnalyticsMixin
+
+
+@override_settings(SOLO_CACHE=None)
+class CSPIdentifierTests(AnalyticsMixin, TestCase):
+    def test_disabling_analytics_does_not_delete_unrelated_csp_settings(self):
+        self.config.gtm_code = "GTM-XXXX"
+        self.config.ga_code = "UA-XXXXX-Y"
+        self.config.enable_google_analytics = True
+
+        self.config.matomo_url = "https://example.com"
+        self.config.matomo_site_id = "1234"
+        self.config.enable_matomo_site_analytics = True
+        self.config.clean()
+        self.config.save()
+
+        csp_settings = CSPSetting.objects.filter(
+            content_type=get_content_type_for_model(self.config),
+            object_id=str(self.config.pk),
+        )
+
+        # GA sets two CSPSettings
+        assert csp_settings.count() == 3
+
+        self.config.enable_google_analytics = False
+        self.config.save()
+
+        try:
+            CSPSetting.objects.get(identifier=AnalyticsTools.matomo)
+        except CSPSetting.DoesNotExist:
+            self.fail("CSPSetting instance should exist")
+
+        other_settings = CSPSetting.objects.filter(
+            content_type=get_content_type_for_model(self.config),
+            object_id=str(self.config.pk),
+            identifier=AnalyticsTools.google_analytics,
+        )
+        self.assertFalse(other_settings.exists())

src/openforms/analytics_tools/tests/test_google_analytics.py

@@ -5,6 +5,7 @@
 
 from openforms.config.models import CSPSetting
 
+from ..constants import AnalyticsTools
 from .mixin import AnalyticsMixin
 
 
@@ -42,7 +43,9 @@ def test_google_analytics_properly_enabled(self):
             with self.subTest("Test creation of CSP"):
                 try:
                     CSPSetting.objects.get(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.google_analytics,
                     )
                 except CSPSetting.DoesNotExist as e:
                     self.fail(f"Unexpected exception : {e}")
@@ -69,7 +72,9 @@ def test_google_analytics_properly_disabled(self):
             with self.subTest("Test deletion of CSP"):
                 self.assertFalse(
                     CSPSetting.objects.filter(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.google_analytics,
                     ).exists()
                 )
 

src/openforms/analytics_tools/tests/test_matomo.py

@@ -5,6 +5,7 @@
 
 from openforms.config.models import CSPSetting
 
+from ..constants import AnalyticsTools
 from .mixin import AnalyticsMixin
 
 
@@ -42,7 +43,9 @@ def test_matomo_properly_enabled(self):
             with self.subTest("Test creation of CSP"):
                 try:
                     CSPSetting.objects.get(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.matomo,
                     )
                 except CSPSetting.DoesNotExist as e:
                     self.fail(f"Unexpected exception : {e}")
@@ -69,7 +72,9 @@ def test_matomo_properly_disabled(self):
             with self.subTest("Test deletion of CSP"):
                 self.assertFalse(
                     CSPSetting.objects.filter(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.matomo,
                     ).exists()
                 )
 

src/openforms/analytics_tools/tests/test_migrations.py

@@ -0,0 +1,77 @@
+from django.contrib.admin.options import get_content_type_for_model
+from django.db.migrations.state import StateApps
+
+from openforms.analytics_tools.constants import AnalyticsTools
+from openforms.config.constants import CSPDirective
+from openforms.utils.tests.test_migrations import TestMigrations
+
+
+class CSPSettingIdentifierMigrationTests(TestMigrations):
+    app = "analytics_tools"
+    migrate_from = "0002_auto_20230119_1500"
+    migrate_to = "0003_cspsetting_identifier"
+
+    def setUpBeforeMigration(self, apps: StateApps):
+        CSPSetting = apps.get_model("config", "CSPSetting")
+        AnalyticsToolsConfiguration = apps.get_model(
+            "analytics_tools", "AnalyticsToolsConfiguration"
+        )
+
+        AnalyticsToolsConfiguration.objects.create(
+            matomo_url="https://matomo.example.com",
+            piwik_url="https://piwik.example.com",
+            piwik_pro_url="https://your-instance-name.piwik.pro",
+        )
+
+        CSPSetting.objects.create(
+            directive=CSPDirective.DEFAULT_SRC, value="https://matomo.example.com"
+        )
+        CSPSetting.objects.create(
+            directive=CSPDirective.DEFAULT_SRC, value="https://piwik.example.com"
+        )
+        CSPSetting.objects.create(
+            directive=CSPDirective.DEFAULT_SRC,
+            value="https://your-instance-name.piwik.pro",
+        )
+        CSPSetting.objects.create(
+            directive=CSPDirective.DEFAULT_SRC, value="https://siteimproveanalytics.com"
+        )
+        CSPSetting.objects.create(
+            directive=CSPDirective.DEFAULT_SRC, value="https://www.googleanalytics.com"
+        )
+
+    def test_migration_sets_identifier_and_gfk(self):
+        CSPSetting = self.apps.get_model("config", "CSPSetting")
+        AnalyticsToolsConfiguration = self.apps.get_model(
+            "analytics_tools", "AnalyticsToolsConfiguration"
+        )
+        analytics_conf = AnalyticsToolsConfiguration.objects.get()
+
+        value_to_identifier = {
+            "https://matomo.example.com": AnalyticsTools.matomo,
+            "https://piwik.example.com": AnalyticsTools.piwik,
+            "https://your-instance-name.piwik.pro": AnalyticsTools.piwik_pro,
+            "https://siteimproveanalytics.com": AnalyticsTools.siteimprove,
+            "https://www.googleanalytics.com": AnalyticsTools.google_analytics,
+        }
+
+        self.assertFalse(CSPSetting.objects.filter(identifier="").exists())
+        print(type(get_content_type_for_model(analytics_conf)))
+
+        # We avoid using django.contrib.admin.options.get_content_type_for_model
+        # as it uses the "real" `ContentType` model. See:
+        # https://stackoverflow.com/q/51670468/#comment110467392_54357872
+        content_type = self.apps.get_model(
+            "contenttypes", "ContentType"
+        ).objects.get_for_model(analytics_conf)
+
+        for value, identifier in value_to_identifier.items():
+            self.assertEqual(
+                CSPSetting.objects.filter(
+                    value=value,
+                    identifier=identifier,
+                    content_type=content_type,
+                    object_id=str(analytics_conf.pk),
+                ).count(),
+                1,
+            )

src/openforms/analytics_tools/tests/test_piwik.py

@@ -5,6 +5,7 @@
 
 from openforms.config.models import CSPSetting
 
+from ..constants import AnalyticsTools
 from .mixin import AnalyticsMixin
 
 
@@ -40,7 +41,9 @@ def test_piwik_properly_enabled(self):
             with self.subTest("Test creation of CSP"):
                 try:
                     CSPSetting.objects.get(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.piwik,
                     )
                 except CSPSetting.DoesNotExist as e:
                     self.fail(f"Unexpected exception : {e}")
@@ -67,7 +70,9 @@ def test_piwik_properly_disabled(self):
             with self.subTest("Test deletion of CSP"):
                 self.assertFalse(
                     CSPSetting.objects.filter(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.piwik,
                     ).exists()
                 )
 

src/openforms/analytics_tools/tests/test_piwik_pro.py

@@ -10,6 +10,7 @@
 from openforms.forms.tests.factories import FormFactory
 from openforms.tests.test_csp import CSPMixin
 
+from ..constants import AnalyticsTools
 from .mixin import AnalyticsMixin
 
 
@@ -47,7 +48,9 @@ def test_piwik_pro_properly_enabled(self):
             with self.subTest("Test creation of CSP"):
                 try:
                     CSPSetting.objects.get(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.piwik_pro,
                     )
                 except CSPSetting.DoesNotExist as e:
                     self.fail(f"Unexpected exception : {e}")
@@ -74,7 +77,9 @@ def test_piwik_pro_properly_disabled(self):
             with self.subTest("Test deletion of CSP"):
                 self.assertFalse(
                     CSPSetting.objects.filter(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.piwik_pro,
                     ).exists()
                 )
 

src/openforms/analytics_tools/tests/test_site_improve.py

@@ -5,6 +5,7 @@
 
 from openforms.config.models import CSPSetting
 
+from ..constants import AnalyticsTools
 from .mixin import AnalyticsMixin
 
 
@@ -42,7 +43,9 @@ def test_site_improve_properly_enabled(self):
             with self.subTest("Test creation of CSP"):
                 try:
                     CSPSetting.objects.get(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.siteimprove,
                     )
                 except CSPSetting.DoesNotExist as e:
                     self.fail(f"Unexpected exception : {e}")
@@ -68,7 +71,9 @@ def test_site_improve_properly_disabled(self):
             with self.subTest("Test deletion of CSP"):
                 self.assertFalse(
                     CSPSetting.objects.filter(
-                        value=csp["value"], directive=csp["directive"]
+                        value=csp["value"],
+                        directive=csp["directive"],
+                        identifier=AnalyticsTools.siteimprove,
                     ).exists()
                 )