grrr-amsterdam / cookie-consent

Cookie consent with accessible dialog, agnostic tag triggers and conditional content, script and embed hooks.
MIT License
64 stars 11 forks source link

Feature proposal: include the previous version of the cookie in the on('update') callback #19

Open carlgieringer opened 3 years ago

carlgieringer commented 3 years ago

I am responding to cookieConsent.on('update') calls to modify my app's state to match the user's current preferences. E.g., if they enable ERROR_REPORTING, I can initialize sentry.

I found this difficult because in order to take the correct actions, I need to know if the cookie value is actually changing or not. Below I have some example code from a React app. My workaround is to persist the cookie state in a Redux store so that I am able to compare new values to old values.

I wanted to get initial feedback on this feature. Is there any apparent reason that it wouldn't be a good idea or would be difficult based upon the library's current implementation? Thanks!

class App extends Component {
  ...
  componentDidMount() {
    ...
    // cookieConsent.on(update) fires when it loads the cookies from storage, so persist them first.
    this.props.privacyConsent.update(cookieConsent.getPreferences())  // stores in redux
    // Load the cookie consent after the update to the Redux store has had a chance to occur.
    setTimeout(() => cookieConsent.on('update', this.onCookieConsentUpdate))
  }

 onCookieConsentUpdate = (cookies) => {
    let requiresReload = false
    const privacyConsentState = this.props.privacyConsentState  // was retrieved from redux elsewhere
    forEach(cookies, (cookie) => {
      const prevCookie = privacyConsentState[cookie.id]
      if (prevCookie && cookie.accepted === prevCookie.accepted) {
        // Only process differences
        return
      }
      let requestReload = false
      switch (cookie.id) {
        case REQUIRED_FUNCTIONALITY:
          // Required functionality can't be changed, so there's never anything to do.
          break
        case ERROR_REPORTING:
          if  (cookie.accepted) {
            sentryInit()
          } else {
            // Sentry's beforeSend checks this value before sending events
          }
          break
        case FULL_ERROR_REPORTING:
          requestReload = true
          break
        default:
          logger.error(`Unsupported cookie consent id: ${cookie.id}`)
          // Require reload just to be safe
          requestReload = true
          // It's possible that the user has an old version of the cookie IDs.
          fixConsentCookieIds()
          break
      }
      // Assume that functionality is disabled by default, and so if there is no previous cookie,
      // then we only need a reload if the functionality is now accepted.
      requiresReload = requiresReload || requestReload  && (isTruthy(prevCookie) || cookie.accepted)
    })
    if (requiresReload) {
      this.props.ui.addToast("Please reload the page for changes to take effect.")
    }
    this.props.privacyConsent.update(cookies)  // persist the new cookies
  }
harmenjanssen commented 3 years ago

Hmm, first of all: I think it's fine to pass along the old value to the callback, that shouldn't be a problem and can be handy. But secondly, your example looks fairly complex (especially requiresReload = requiresReload || requestReload && (isTruthy(prevCookie) || cookie.accepted)), and I would personally strife for an atomic function that would be able to respond to the new state regardless of what the old state was.

That would simplify your script I think. For example, maybe just write a sentryInit function that does nothing when already initialized.