From b497c09622ccbc21e9960e9e86c0125e38175a53 Mon Sep 17 00:00:00 2001 From: Alexandre Philibeaux Date: Mon, 1 Apr 2024 21:37:36 +0000 Subject: [PATCH] fix(integrations): do not set All by default when integrations are already set --- .changeset/swift-countries-worry.md | 5 + .../CookieConsentProvider.tsx | 49 ++++---- .../CookieConsentProvider/__tests__/index.tsx | 114 ++++++++---------- 3 files changed, 77 insertions(+), 91 deletions(-) create mode 100644 .changeset/swift-countries-worry.md diff --git a/.changeset/swift-countries-worry.md b/.changeset/swift-countries-worry.md new file mode 100644 index 000000000..d9f047064 --- /dev/null +++ b/.changeset/swift-countries-worry.md @@ -0,0 +1,5 @@ +--- +"@scaleway/cookie-consent": patch +--- + +Remove the default object All: false of integrations when integrations are set. This will only apply on empty integrations array to protect users diff --git a/packages/cookie-consent/src/CookieConsentProvider/CookieConsentProvider.tsx b/packages/cookie-consent/src/CookieConsentProvider/CookieConsentProvider.tsx index d35578ed3..2623e691f 100644 --- a/packages/cookie-consent/src/CookieConsentProvider/CookieConsentProvider.tsx +++ b/packages/cookie-consent/src/CookieConsentProvider/CookieConsentProvider.tsx @@ -33,7 +33,7 @@ type Context = { needConsent: boolean isSegmentAllowed: boolean isSegmentIntegrationsLoaded: boolean - segmentIntegrations: { All: boolean } & Record + segmentIntegrations: Record categoriesConsent: Partial saveConsent: (categoriesConsent: Partial) => void } @@ -51,6 +51,8 @@ export const useCookieConsent = (): Context => { return context } +const getCookies = () => cookie.parse(document.cookie) + export const CookieConsentProvider = ({ children, isConsentRequired, @@ -71,16 +73,11 @@ export const CookieConsentProvider = ({ }>) => { const [needConsent, setNeedsConsent] = useState(false) - const [cookies, setCookies] = useState>() const { integrations: segmentIntegrations, isLoaded: isSegmentIntegrationsLoaded, } = useSegmentIntegrations(config) - useEffect(() => { - setCookies(cookie.parse(document.cookie)) - }, [needConsent]) - const integrations: Integrations = useMemo( () => uniq([ @@ -104,7 +101,7 @@ export const CookieConsentProvider = ({ ...essentialIntegrations, ]) .sort() - .join(), + .join(undefined), ), [segmentIntegrations, essentialIntegrations], ) @@ -115,17 +112,17 @@ export const CookieConsentProvider = ({ // to false after receiving segment answer and flicker the UI setNeedsConsent( isConsentRequired && - cookies?.[HASH_COOKIE] !== integrationsHash.toString() && + getCookies()[HASH_COOKIE] !== integrationsHash.toString() && segmentIntegrations !== undefined, ) - }, [isConsentRequired, cookies, integrationsHash, segmentIntegrations]) + }, [isConsentRequired, integrationsHash, segmentIntegrations]) // We store unique categories names in an array const categories = useMemo( () => - uniq([ - ...(segmentIntegrations ?? []).map(({ category }) => category), - ]).sort(), + uniq((segmentIntegrations ?? []).map(({ category }) => category)).sort( + undefined, + ), [segmentIntegrations], ) @@ -138,12 +135,12 @@ export const CookieConsentProvider = ({ (acc, category) => ({ ...acc, [category]: isConsentRequired - ? cookies?.[`${cookiePrefix}_${category}`] === 'true' + ? getCookies()[`${cookiePrefix}_${category}`] === 'true' : true, }), {}, ), - [isConsentRequired, categories, cookies, cookiePrefix], + [isConsentRequired, categories, cookiePrefix], ) const saveConsent = useCallback( @@ -205,18 +202,20 @@ export const CookieConsentProvider = ({ [isConsentRequired, segmentIntegrations, cookieConsent, needConsent], ) + // 'All': false tells Segment not to send data to any Destinations by default, unless they’re explicitly listed as true in the next lines. + // In this case we should not have any integration, so we protect the user. Maybe unecessary as we always set true of false for an integration. const segmentEnabledIntegrations = useMemo( - () => ({ - All: false, - ...segmentIntegrations?.reduce( - (acc, integration) => ({ - ...acc, - [integration.name]: cookieConsent[integration.category], - }), - {}, - ), - }), - [cookieConsent, segmentIntegrations], + () => + segmentIntegrations?.length === 0 + ? { All: !isConsentRequired } + : (segmentIntegrations ?? []).reduce>( + (acc, integration) => ({ + ...acc, + [integration.name]: cookieConsent[integration.category] ?? false, + }), + {}, + ), + [cookieConsent, isConsentRequired, segmentIntegrations], ) const value = useMemo( diff --git a/packages/cookie-consent/src/CookieConsentProvider/__tests__/index.tsx b/packages/cookie-consent/src/CookieConsentProvider/__tests__/index.tsx index 90c536351..c9699ab49 100644 --- a/packages/cookie-consent/src/CookieConsentProvider/__tests__/index.tsx +++ b/packages/cookie-consent/src/CookieConsentProvider/__tests__/index.tsx @@ -1,14 +1,19 @@ // useSegmentIntegrations tests have been splitted in multiple files because of https://github.com/facebook/jest/issues/8987 -import { afterEach, describe, expect, it, jest } from '@jest/globals' +import { describe, expect, it, jest } from '@jest/globals' import { act, renderHook } from '@testing-library/react' import cookie from 'cookie' import type { ComponentProps, ReactNode } from 'react' import { CookieConsentProvider, useCookieConsent } from '..' +import type { Integrations } from '../types' +import type { useSegmentIntegrations } from '../useSegmentIntegrations' const wrapper = ({ isConsentRequired, - }: Omit, 'children'>) => + }: Omit< + ComponentProps, + 'children' | 'essentialIntegrations' | 'config' + >) => ({ children }: { children: ReactNode }) => ( ) -const integrations = [ +const integrations: Integrations = [ { category: 'analytics', name: 'Google Universal Analytics', @@ -38,18 +43,25 @@ const integrations = [ name: 'Salesforce', }, ] -const mockUseSegmentIntegrations = jest.fn().mockReturnValue({ + +const mockUseSegmentIntegrations = jest.fn< + () => ReturnType +>(() => ({ integrations, isLoaded: true, -}) +})) + jest.mock('../useSegmentIntegrations', () => ({ - __esModule: true, useSegmentIntegrations: () => mockUseSegmentIntegrations(), })) describe('CookieConsent - CookieConsentProvider', () => { - afterEach(() => { - document.cookie = '' + beforeEach(() => { + jest.clearAllMocks() + }) + + afterAll(() => { + jest.restoreAllMocks() }) it('useCookieConsent should throw without provider', () => { @@ -57,7 +69,7 @@ describe('CookieConsent - CookieConsentProvider', () => { spy.mockImplementation(() => {}) expect(() => renderHook(() => useCookieConsent())).toThrow( - Error('useCookieConsent must be used within a CookieConsentProvider'), + new Error('useCookieConsent must be used within a CookieConsentProvider'), ) spy.mockRestore() @@ -67,13 +79,6 @@ describe('CookieConsent - CookieConsentProvider', () => { const { result } = renderHook(() => useCookieConsent(), { wrapper: wrapper({ isConsentRequired: false, - essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'], - config: { - segment: { - cdnURL: 'url', - writeKey: 'key', - }, - }, }), }) @@ -84,7 +89,6 @@ describe('CookieConsent - CookieConsentProvider', () => { marketing: true, }) expect(result.current.segmentIntegrations).toStrictEqual({ - All: false, 'Google Universal Analytics': true, Salesforce: true, 'Salesforce custom destination (Scaleway)': true, @@ -94,42 +98,24 @@ describe('CookieConsent - CookieConsentProvider', () => { it('should know when integrations are loading', () => { // simulate that Segment is loading - mockUseSegmentIntegrations.mockReturnValue({ + mockUseSegmentIntegrations.mockReturnValueOnce({ integrations: undefined, isLoaded: false, }) const { result } = renderHook(() => useCookieConsent(), { wrapper: wrapper({ isConsentRequired: true, - essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'], - config: { - segment: { - cdnURL: 'url', - writeKey: 'key', - }, - }, }), }) - expect(result.current.isSegmentIntegrationsLoaded).toBe(false) - // put mock back as if segment integrations are loaded - mockUseSegmentIntegrations.mockReturnValue({ - integrations, - isLoaded: true, - }) + expect(mockUseSegmentIntegrations).toBeCalledTimes(1) + expect(result.current.isSegmentIntegrationsLoaded).toBe(false) }) it('should know to ask for content when no cookie is set and consent is required', () => { const { result } = renderHook(() => useCookieConsent(), { wrapper: wrapper({ isConsentRequired: true, - essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'], - config: { - segment: { - cdnURL: 'url', - writeKey: 'key', - }, - }, }), }) @@ -140,7 +126,6 @@ describe('CookieConsent - CookieConsentProvider', () => { analytics: false, }) expect(result.current.segmentIntegrations).toStrictEqual({ - All: false, 'Google Universal Analytics': false, Salesforce: false, 'Salesforce custom destination (Scaleway)': false, @@ -152,13 +137,6 @@ describe('CookieConsent - CookieConsentProvider', () => { const { result } = renderHook(() => useCookieConsent(), { wrapper: wrapper({ isConsentRequired: true, - essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'], - config: { - segment: { - cdnURL: 'url', - writeKey: 'key', - }, - }, }), }) @@ -169,7 +147,6 @@ describe('CookieConsent - CookieConsentProvider', () => { marketing: false, }) expect(result.current.segmentIntegrations).toStrictEqual({ - All: false, 'Google Universal Analytics': false, Salesforce: false, 'Salesforce custom destination (Scaleway)': false, @@ -213,17 +190,12 @@ describe('CookieConsent - CookieConsentProvider', () => { }) it('should not need consent if hash cookie is set', () => { - jest.spyOn(cookie, 'parse').mockReturnValue({ _scw_rgpd_hash: '913003917' }) + jest + .spyOn(cookie, 'parse') + .mockImplementation(() => ({ _scw_rgpd_hash: '913003917' })) const { result } = renderHook(() => useCookieConsent(), { wrapper: wrapper({ isConsentRequired: true, - essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'], - config: { - segment: { - cdnURL: 'url', - writeKey: 'key', - }, - }, }), }) @@ -234,7 +206,6 @@ describe('CookieConsent - CookieConsentProvider', () => { marketing: false, }) expect(result.current.segmentIntegrations).toStrictEqual({ - All: false, 'Google Universal Analytics': false, Salesforce: false, 'Salesforce custom destination (Scaleway)': false, @@ -242,20 +213,14 @@ describe('CookieConsent - CookieConsentProvider', () => { }) it('should not need consent if hash cookie is set and some categories already approved', () => { - jest.spyOn(cookie, 'parse').mockReturnValue({ + jest.spyOn(cookie, 'parse').mockImplementation(() => ({ _scw_rgpd_hash: '913003917', _scw_rgpd_marketing: 'true', - }) + })) + const { result } = renderHook(() => useCookieConsent(), { wrapper: wrapper({ isConsentRequired: true, - essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'], - config: { - segment: { - cdnURL: 'url', - writeKey: 'key', - }, - }, }), }) @@ -266,10 +231,27 @@ describe('CookieConsent - CookieConsentProvider', () => { marketing: true, }) expect(result.current.segmentIntegrations).toStrictEqual({ - All: false, 'Google Universal Analytics': false, Salesforce: true, 'Salesforce custom destination (Scaleway)': true, }) }) + + it('should return integration All: false in case there is no integrations', () => { + mockUseSegmentIntegrations.mockReturnValue({ + integrations: [], + isLoaded: true, + }) + const { result } = renderHook(() => useCookieConsent(), { + wrapper: wrapper({ + isConsentRequired: true, + }), + }) + + expect(mockUseSegmentIntegrations).toBeCalledTimes(2) + + expect(result.current.segmentIntegrations).toStrictEqual({ + All: false, + }) + }) })