Skip to content

Conversation

HannesOberreiter
Copy link

@HannesOberreiter HannesOberreiter commented Jul 16, 2025

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

@sgiehl sgiehl requested a review from a team July 18, 2025 09:07
@nathangavin nathangavin self-assigned this Jul 22, 2025
Copy link
Contributor

@nathangavin nathangavin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minor change, all uses of hasConsent() are treating the output as boolean anyway so it is a suitable change.

Copy link
Contributor

github-actions bot commented Aug 7, 2025

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Aug 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Stale The label used by the Close Stale Issues action

Development

Successfully merging this pull request may close these issues.

2 participants