campaign parameters now masked with CNIL active#213
Conversation
|
Hi @nathangavin |
| { | ||
| $campaignDetector = StaticContainer::get('advanced_campaign_reporting.campaign_detector'); | ||
| $campaignParameters = MarketingCampaignsReporting::getCampaignParameters(); | ||
| $campaignValuesMasked = MarketingCampaignsReporting::isCampaignValuesMaskingEnabled((int) $request->getIdSiteIfExists()); |
There was a problem hiding this comment.
| $campaignValuesMasked = MarketingCampaignsReporting::isCampaignValuesMaskingEnabled((int) $request->getIdSiteIfExists()); | |
| $isCampaignValuesMasked = MarketingCampaignsReporting::isCampaignValuesMaskingEnabled((int) $request->getIdSiteIfExists()); |
| $request, | ||
| $campaignParameters | ||
| ); | ||
| $campaignDimensions = $this->maskDetectedCampaignDimensions($campaignDimensions, $campaignValuesMasked); |
There was a problem hiding this comment.
| $campaignDimensions = $this->maskDetectedCampaignDimensions($campaignDimensions, $campaignValuesMasked); | |
| $campaignDimensions = $this->maskDetectedCampaignDimensions($campaignDimensions, $isCampaignValuesMasked); |
AltamashShaikh
left a comment
There was a problem hiding this comment.
@nathangavin More from AI
- The combined Source - Medium report can still expose raw placeholder labels. The new formatting hook only applies to dimensions inheriting this plugin’s Columns\Base, but CampaignSourceMedium
extends plain Dimension and has no formatValue() override in Columns/CampaignSourceMedium.php:16. Its labels are assembled directly from archived raw values in RecordBuilders/
CampaignReporting.php:145. With CNIL active, that likely yields labels like __discarded_by_policy__ - __discarded_by_policy__ instead of the translated discard label.
- Live.getLastVisitsDetails still appears to return the raw placeholder sentinel for marketing campaign fields. renderVisitorDetails() formats values for the HTML block in VisitorDetails.php:37,
but extendVisitorDetails() copies raw DB values straight into the visitor payload in VisitorDetails.php:19. Core Referrers formats equivalent fields before exposing them, so this branch leaves
API consumers with inconsistent output.
|
@Chardonneaur Thats a good suggestion, I am unaware if allowing the end user to explicitly allow consent is permitted within the CNIL guidelines. We can address that in a followup feature. |
|
@AltamashShaikh I've fixed the formatting in a couple of places such that the translated text should now be retrieved correctly. |
AltamashShaikh
left a comment
There was a problem hiding this comment.
High-risk issues (blocking)
- Columns/CampaignName.php:49 compares raw incoming campaign params against masked stored visit columns.
Base::onNewVisit() now masks stored campaign fields under CNIL (Columns/Base.php:54, Columns/Base.php:125), but CampaignName::shouldForceNewVisit() still calls detectCampaignFromRequest() and
compares the unmasked result to the existing visit data (Columns/CampaignName.php:71, Columns/CampaignName.php:95). With privacy masking enabled, an existing visit will hold placeholder values
while the next request still carries raw utm_* values, so the plugin will treat the campaign as “changed” and can force a new visit repeatedly for the same campaign. This is a behavioral
regression, not just missing coverage.
Medium-risk issues
- The new tests do not cover the regression above.
tests/Integration/CorrectCampaignDetectionTest.php:176 verifies masked storage, and tests/Integration/CorrectCampaignDetectionTest.php:217 / tests/Integration/
CorrectCampaignDetectionTest.php:242 verify formatter-style behavior, but there is no test for shouldForceNewVisit() when CNIL masking is active. That is the exact path most likely to regress
from this change.
|
@AltamashShaikh Fixed the force-new-visit path so that both ends of the campaign values check are normalised when trying to determine if its a new visit. Also added test coverage for this specific scenario. |
AltamashShaikh
left a comment
There was a problem hiding this comment.
Can you also update plugin version - https://github.com/matomo-org/plugin-MarketingCampaignsReporting/blob/5.x-dev/plugin.json#L4
5.2.0, should be good and add a Changelog entry CHANGELOG.md, next release date is 2026-05-11
Also a minor fix
- The helper getCampaignValuesMaskedSettingClass() is duplicated in both MarketingCampaignsReporting.php:152 and tests/Integration/CorrectCampaignDetectionTest.php:277. That is harmless, but the
test could call the production helper indirectly to avoid drift.
- formatCampaignValue($value) in MarketingCampaignsReporting.php:142 is intentionally untyped. That matches current usage, but a short docblock would make the mixed input expectation clearer.
Description
Masks campaign parameter values when the CNIL policy is enabled. Refer to matomo-org/matomo#24416 for more information.
Backwards compatibility is maintained, if the setting is not available then all new functionality is disabled.
Checklist