django-analytical
django-analytical copied to clipboard
Fix #141 and add custom event trigger binding support
This should work to make Matomo respect consent.
I created two new utility functions, one builds paq commands, the other builds javascript to bind an event listener to elements with a certain class name. That will allow users to make their own custom template tags then use analytical.utils.get_event_bind_js() to bind events to their elements.
@bittner would you mind looking over this PR? It's pretty simple but can expose some extra options for the users of the package
@bittner This seems ready, would you mind looking it over?
- [ ] AFAICS, you're making unrelated changes to
analytical/utils.py. Would you mind moving that to a separate PR? - [ ] Tests are missing for your changes. Please supply them with your feature enhancement.
- [ ] Please squash your changes. If the change set is small enough the PR should have only one or two commits (e.g. one for the tests, one for the feature enhancement).
* [ ] AFAICS, you're making unrelated changes to `analytical/utils.py`. Would you mind moving that to a separate PR? * [ ] Tests are missing for your changes. Please supply them with your feature enhancement. * [ ] Please squash your changes. If the change set is small enough the PR should have only one or two commits (e.g. one for the tests, one for the feature enhancement).
I actuallly don't see that; I thought not having hard coded chunks of JS would be best. I figured dynamically creating the javascript for event binding could let people trigger custom events as well as keep the files relatively free of chunks of hard coded javascript. So, it just made sense to add them in and since I already had the code it made no sense to hard code the JS when what I already wrote makes doing so completely obsolete (for event binding and paq commands anyway).
It may take me a minute to figure out how to properly write tests for this. I'll dig in on my next break from my current project.
Squashing the changes means using git rebase right? I've never done this before so, sorry for the inexperience.
When it comes to splitting up the PRs, from what I understand and, forgive me if I'm misunderstanding, I need to rebase all of the changes out, take out the functions I added in analytics/utils.py then just hardcode the javascript into the context provider? Then on a separate PR introduce the two utility functions again? Wouldn't that be redundant considering it would go from dynamic to harcoded then back and we'd have to change the files back to their original state after the initial push.
It may take me a minute to figure out how to properly write tests for this.
Take a look at the tests that are there. The idea is simple:
- Your function generates output for some specific input. Hence, in your test code, call your function with some arguments.
- After that, in the line afterwards (in the same test function) verify the result using
assert.
That's it.
If there are various possible outcomes for different input, add more of those calls with different input, so that all of your if-else clauses are being traveled through. – Our test suite if full of examples. Take a look!