feat(#78): allow cookieExpiresAfterDays as a function #79

gtnsimon · 2022-01-12T21:41:49Z

This allow to conditionally define cookie duration based on user consent.

Here a snippet function to control cookie duration using a function:

// orejime.config.js
const config = {
  /* ... */
  cookieExpiresAfterDays(consents, config) {
		if (consents) {
			// ignore required apps (true is enforced)
			const consentableApps = config.apps
				.filter((app) => app.required !== true)
				.map((app) => app.name);
			// get selected user apps
			const userConsents = Object.values(
				Object.fromEntries(
					Object.entries(consents).filter(([name]) =>
						consentableApps.includes(name)
					)
				)
			);

			// cookie will expire after 90 days for users who accepted all apps
			if (userConsents.every((value) => value === true)) {
				return 90;
			}

			// otherwise (all apps refused or not all accepted) cookie will expire after 30 days
			return 30;
		}
	},
  /* ... */
}

Resolves: #78

This allow to conditionally define expiration days of cookie based on user consent.

felixgirault · 2022-01-13T13:14:03Z

src/consent-manager.js

+				? configCookieExpiresAfterDays(this.consents, this.config)
+				: configCookieExpiresAfterDays;
+
+		if (


I would maybe remove this test and use a default value in setCookie like before.
This way any falsy or 0 value would be set to the default.

setCookie( this.cookieName, value, cookieExpiresAfterDays || 120, this.config.cookieDomain );

cookieExpiresAfterDays could then be a const var.

I agree with falsies values but 0 as an integer has a meaning for cookie expiration which ends when browser is closed. Using cookieExpiresAfterDays || 120 will replace 0 to 120 which may not be the expected behavior by the end developer.

To not break existing installations, the default could apply to non-functional cookieExpiresAfterDays (like before this feature). When used as a function it'll be up to the developer to explicitly return a value if he don't want a session cookie.

I didn't know about this behavior with 0, given the old implementation I thought it was kind of invalid.
I'm kind of torn here, it would be a breaking change to modify the current behavior, but it seems odd to have different behaviors for different methods...

src/consent-manager.js

This will prevent replacing a `return 0` from developer code land with the default `120` from orejime.

Allow cookieExpiresAfterDays to be a function

f960398

This allow to conditionally define expiration days of cookie based on user consent.

gtnsimon changed the title ~~Allow cookieExpiresAfterDays to be a function~~ feat(#78): allow cookieExpiresAfterDays as a function Jan 12, 2022

felixgirault requested changes Jan 13, 2022

View reviewed changes

gtnsimon added 2 commits January 14, 2022 00:23

fix: let user take care to return a value when using a function

908cafe

This will prevent replacing a `return 0` from developer code land with the default `120` from orejime.

chore: documentation to use it as a function

e027077

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(#78): allow cookieExpiresAfterDays as a function #79

feat(#78): allow cookieExpiresAfterDays as a function #79

gtnsimon commented Jan 12, 2022 •

edited

Loading

felixgirault Jan 13, 2022

felixgirault Jan 13, 2022

gtnsimon Jan 13, 2022

felixgirault Jan 14, 2022

feat(#78): allow cookieExpiresAfterDays as a function #79

Are you sure you want to change the base?

feat(#78): allow cookieExpiresAfterDays as a function #79

Conversation

gtnsimon commented Jan 12, 2022 • edited Loading

felixgirault Jan 13, 2022

Choose a reason for hiding this comment

felixgirault Jan 13, 2022

Choose a reason for hiding this comment

gtnsimon Jan 13, 2022

Choose a reason for hiding this comment

felixgirault Jan 14, 2022

Choose a reason for hiding this comment

gtnsimon commented Jan 12, 2022 •

edited

Loading