MatomoConsent.hasConsent return boolean
Description:
Currently the fallback local cookie implementation behaves differently than the tracker for the hasConsent. The tracker always returns a boolean and the fallback a string or boolean.
This difference can also be seen in the test case, where a double bang is used to get the same results.
I would expect to behave them the same and therefore to always return a boolean for the MatomoConsent.hasConsent function.
Before
consentCookie || consentCookie !== 0
If consentCookie is a string (which it is when there is one set): consentCookie is truthy, so || short-circuits and returns the string value (e.g. "1234567890").
consentCookie = 0
consentCookie || consentCookie !== 0
# false
consentCookie = "123123123"
consentCookie || consentCookie !== 0
# '123123123'
After
!!(consentCookie && consentCookie !== 0)
If consentCookie is a string (e.g. "1234567890"), therefore consentCookie is truthy, so && evaluates the right side consentCookie !== 0 is true. Result: true (boolean)
If consentCookie is 0 it would short-circuit again, therefore the boolean transform !! is needed.
consentCookie = 0
!!(consentCookie && consentCookie !== 0)
# false
consentCookie = "123123123"
!!(consentCookie && consentCookie !== 0)
# true
Alternative
We could also add a double bang in front of the current implementation, but I think the && is more logical than the || in this case.
Review
- [x] Functional review done
- [x] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- [ ] Security review done
- [ ] Wording review done
- [ ] Code review done
- [ ] Tests were added if useful/possible
- [x] Reviewed for breaking changes
- [ ] Developer changelog updated if needed
- [ ] Documentation added if needed
- [ ] Existing documentation updated if needed
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.
This PR was last updated more than one month ago, maybe it's time to close it. Please check if there is anything we still can do or close this PR. ping @matomo-org/core-reviewers