Ask for consent - Matomo (Piwik) #142
Conversation
jayhaluska
changed the title
|
Fixes #141 |
|
Merged the newest version of django-analytical into my fork. I think this PR was forgotten, can we get it merged, @bittner? |
|
Probably it makes more sense now to add these changes to the newly created Matomo tag. |
|
I added the 'ask for consent' logic and documentation to the new matomo files and renamed everything accordingly. But now the Travis CI check fails and I don't know why. Any suggestions? |
Flake8 complains about a missing newline in the test file:
It's a good idea to configure your IDE in a way that it ensures a final newline in every file you touch. Most modern IDEs have such a setting now. |
| @@ -98,6 +122,9 @@ def render(self, context): | |||
| if getattr(settings, 'PIWIK_DISABLE_COOKIES', False): | |||
| commands.append(DISABLE_COOKIES_CODE) | |||
|
|
|||
| if getattr(settings, 'PIWIK_ASK_FOR_CONSENT', False): | |||
There was a problem hiding this comment.
This still says, PIWIK in the constant name. Shouldn't it say MATOMO?
There was a problem hiding this comment.
Actually, this code shouldn't go here at all. As mentioned by my other comments.
| <button class="piwik_remove_consent">Don't track me anymore!</button> | ||
|
|
||
| .. _`asking for consent`: https://developer.matomo.org/guides/tracking-javascript-guide#asking-for-consent | ||
|
|
There was a problem hiding this comment.
Can't we remove the documentation for Piwiki all together, maybe just showing a hint on the new location and the reason for it? I don't think it makes sense to continue to maintain, let alone extend, it.
| @override_settings(MATOMO_ASK_FOR_CONSENT=True) | ||
| def test_ask_for_consent(self): | ||
| r = MatomoNode().render(Context({})) | ||
| self.assertTrue("_paq.push(['requireConsent']);" in r, r) |
There was a problem hiding this comment.
Here is the missing newline that causes Flake8 to fail.
| }} | ||
| ); | ||
| }} | ||
| """.format(REMOVE_CONSENT_CLASS, GIVE_CONSENT_CLASS) |
There was a problem hiding this comment.
We should really just keep the Piwik implementation as is. Not extend it.
I'm not even sure if it would work. There is no Piwik that does consent. It's only Matomo that does it.
| @override_settings(PIWIK_ASK_FOR_CONSENT=True) | ||
| def test_ask_for_consent(self): | ||
| r = PiwikNode().render(Context({})) | ||
| self.assertTrue("_paq.push(['requireConsent']);" in r, r) |
There was a problem hiding this comment.
Same here: Why add code and tests for an obsolete integration?
|
This PR needs to be rebased and its conflicts resolved. In addition, some review comments may still need to be addressed. But then it would be ready to be merged. Want to give it another try? |
This is a pull request to add the "Ask for consent" functionality from Matomo (Piwik). Especially relevant in the context of the new GDPR in Europe.
See: https://developer.matomo.org/guides/tracking-javascript-guide#asking-for-consent