Skip to content

Commit 6d410ae

Browse files
authored
fix(integrations): do not set All by default when integrations are already set (#1902)
1 parent 8bfeb6c commit 6d410ae

File tree

3 files changed

+77
-91
lines changed

3 files changed

+77
-91
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@scaleway/cookie-consent": patch
3+
---
4+
5+
Remove the default object All: false of integrations when integrations are set. This will only apply on empty integrations array to protect users

packages/cookie-consent/src/CookieConsentProvider/CookieConsentProvider.tsx

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type Context = {
3333
needConsent: boolean
3434
isSegmentAllowed: boolean
3535
isSegmentIntegrationsLoaded: boolean
36-
segmentIntegrations: { All: boolean } & Record<string, boolean>
36+
segmentIntegrations: Record<string, boolean>
3737
categoriesConsent: Partial<Consent>
3838
saveConsent: (categoriesConsent: Partial<Consent>) => void
3939
}
@@ -51,6 +51,8 @@ export const useCookieConsent = (): Context => {
5151
return context
5252
}
5353

54+
const getCookies = () => cookie.parse(document.cookie)
55+
5456
export const CookieConsentProvider = ({
5557
children,
5658
isConsentRequired,
@@ -71,16 +73,11 @@ export const CookieConsentProvider = ({
7173
}>) => {
7274
const [needConsent, setNeedsConsent] = useState(false)
7375

74-
const [cookies, setCookies] = useState<Record<string, string>>()
7576
const {
7677
integrations: segmentIntegrations,
7778
isLoaded: isSegmentIntegrationsLoaded,
7879
} = useSegmentIntegrations(config)
7980

80-
useEffect(() => {
81-
setCookies(cookie.parse(document.cookie))
82-
}, [needConsent])
83-
8481
const integrations: Integrations = useMemo(
8582
() =>
8683
uniq([
@@ -104,7 +101,7 @@ export const CookieConsentProvider = ({
104101
...essentialIntegrations,
105102
])
106103
.sort()
107-
.join(),
104+
.join(undefined),
108105
),
109106
[segmentIntegrations, essentialIntegrations],
110107
)
@@ -115,17 +112,17 @@ export const CookieConsentProvider = ({
115112
// to false after receiving segment answer and flicker the UI
116113
setNeedsConsent(
117114
isConsentRequired &&
118-
cookies?.[HASH_COOKIE] !== integrationsHash.toString() &&
115+
getCookies()[HASH_COOKIE] !== integrationsHash.toString() &&
119116
segmentIntegrations !== undefined,
120117
)
121-
}, [isConsentRequired, cookies, integrationsHash, segmentIntegrations])
118+
}, [isConsentRequired, integrationsHash, segmentIntegrations])
122119

123120
// We store unique categories names in an array
124121
const categories = useMemo(
125122
() =>
126-
uniq([
127-
...(segmentIntegrations ?? []).map(({ category }) => category),
128-
]).sort(),
123+
uniq((segmentIntegrations ?? []).map(({ category }) => category)).sort(
124+
undefined,
125+
),
129126
[segmentIntegrations],
130127
)
131128

@@ -138,12 +135,12 @@ export const CookieConsentProvider = ({
138135
(acc, category) => ({
139136
...acc,
140137
[category]: isConsentRequired
141-
? cookies?.[`${cookiePrefix}_${category}`] === 'true'
138+
? getCookies()[`${cookiePrefix}_${category}`] === 'true'
142139
: true,
143140
}),
144141
{},
145142
),
146-
[isConsentRequired, categories, cookies, cookiePrefix],
143+
[isConsentRequired, categories, cookiePrefix],
147144
)
148145

149146
const saveConsent = useCallback(
@@ -205,18 +202,20 @@ export const CookieConsentProvider = ({
205202
[isConsentRequired, segmentIntegrations, cookieConsent, needConsent],
206203
)
207204

205+
// 'All': false tells Segment not to send data to any Destinations by default, unless they’re explicitly listed as true in the next lines.
206+
// 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.
208207
const segmentEnabledIntegrations = useMemo(
209-
() => ({
210-
All: false,
211-
...segmentIntegrations?.reduce(
212-
(acc, integration) => ({
213-
...acc,
214-
[integration.name]: cookieConsent[integration.category],
215-
}),
216-
{},
217-
),
218-
}),
219-
[cookieConsent, segmentIntegrations],
208+
() =>
209+
segmentIntegrations?.length === 0
210+
? { All: !isConsentRequired }
211+
: (segmentIntegrations ?? []).reduce<Record<string, boolean>>(
212+
(acc, integration) => ({
213+
...acc,
214+
[integration.name]: cookieConsent[integration.category] ?? false,
215+
}),
216+
{},
217+
),
218+
[cookieConsent, isConsentRequired, segmentIntegrations],
220219
)
221220

222221
const value = useMemo(

packages/cookie-consent/src/CookieConsentProvider/__tests__/index.tsx

Lines changed: 48 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
// useSegmentIntegrations tests have been splitted in multiple files because of https://github.com/facebook/jest/issues/8987
2-
import { afterEach, describe, expect, it, jest } from '@jest/globals'
2+
import { describe, expect, it, jest } from '@jest/globals'
33
import { act, renderHook } from '@testing-library/react'
44
import cookie from 'cookie'
55
import type { ComponentProps, ReactNode } from 'react'
66
import { CookieConsentProvider, useCookieConsent } from '..'
7+
import type { Integrations } from '../types'
8+
import type { useSegmentIntegrations } from '../useSegmentIntegrations'
79

810
const wrapper =
911
({
1012
isConsentRequired,
11-
}: Omit<ComponentProps<typeof CookieConsentProvider>, 'children'>) =>
13+
}: Omit<
14+
ComponentProps<typeof CookieConsentProvider>,
15+
'children' | 'essentialIntegrations' | 'config'
16+
>) =>
1217
({ children }: { children: ReactNode }) => (
1318
<CookieConsentProvider
1419
isConsentRequired={isConsentRequired}
@@ -24,7 +29,7 @@ const wrapper =
2429
</CookieConsentProvider>
2530
)
2631

27-
const integrations = [
32+
const integrations: Integrations = [
2833
{
2934
category: 'analytics',
3035
name: 'Google Universal Analytics',
@@ -38,26 +43,33 @@ const integrations = [
3843
name: 'Salesforce',
3944
},
4045
]
41-
const mockUseSegmentIntegrations = jest.fn().mockReturnValue({
46+
47+
const mockUseSegmentIntegrations = jest.fn<
48+
() => ReturnType<typeof useSegmentIntegrations>
49+
>(() => ({
4250
integrations,
4351
isLoaded: true,
44-
})
52+
}))
53+
4554
jest.mock('../useSegmentIntegrations', () => ({
46-
__esModule: true,
4755
useSegmentIntegrations: () => mockUseSegmentIntegrations(),
4856
}))
4957

5058
describe('CookieConsent - CookieConsentProvider', () => {
51-
afterEach(() => {
52-
document.cookie = ''
59+
beforeEach(() => {
60+
jest.clearAllMocks()
61+
})
62+
63+
afterAll(() => {
64+
jest.restoreAllMocks()
5365
})
5466

5567
it('useCookieConsent should throw without provider', () => {
5668
const spy = jest.spyOn(console, 'error')
5769
spy.mockImplementation(() => {})
5870

5971
expect(() => renderHook(() => useCookieConsent())).toThrow(
60-
Error('useCookieConsent must be used within a CookieConsentProvider'),
72+
new Error('useCookieConsent must be used within a CookieConsentProvider'),
6173
)
6274

6375
spy.mockRestore()
@@ -67,13 +79,6 @@ describe('CookieConsent - CookieConsentProvider', () => {
6779
const { result } = renderHook(() => useCookieConsent(), {
6880
wrapper: wrapper({
6981
isConsentRequired: false,
70-
essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'],
71-
config: {
72-
segment: {
73-
cdnURL: 'url',
74-
writeKey: 'key',
75-
},
76-
},
7782
}),
7883
})
7984

@@ -84,7 +89,6 @@ describe('CookieConsent - CookieConsentProvider', () => {
8489
marketing: true,
8590
})
8691
expect(result.current.segmentIntegrations).toStrictEqual({
87-
All: false,
8892
'Google Universal Analytics': true,
8993
Salesforce: true,
9094
'Salesforce custom destination (Scaleway)': true,
@@ -94,42 +98,24 @@ describe('CookieConsent - CookieConsentProvider', () => {
9498

9599
it('should know when integrations are loading', () => {
96100
// simulate that Segment is loading
97-
mockUseSegmentIntegrations.mockReturnValue({
101+
mockUseSegmentIntegrations.mockReturnValueOnce({
98102
integrations: undefined,
99103
isLoaded: false,
100104
})
101105
const { result } = renderHook(() => useCookieConsent(), {
102106
wrapper: wrapper({
103107
isConsentRequired: true,
104-
essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'],
105-
config: {
106-
segment: {
107-
cdnURL: 'url',
108-
writeKey: 'key',
109-
},
110-
},
111108
}),
112109
})
113-
expect(result.current.isSegmentIntegrationsLoaded).toBe(false)
114110

115-
// put mock back as if segment integrations are loaded
116-
mockUseSegmentIntegrations.mockReturnValue({
117-
integrations,
118-
isLoaded: true,
119-
})
111+
expect(mockUseSegmentIntegrations).toBeCalledTimes(1)
112+
expect(result.current.isSegmentIntegrationsLoaded).toBe(false)
120113
})
121114

122115
it('should know to ask for content when no cookie is set and consent is required', () => {
123116
const { result } = renderHook(() => useCookieConsent(), {
124117
wrapper: wrapper({
125118
isConsentRequired: true,
126-
essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'],
127-
config: {
128-
segment: {
129-
cdnURL: 'url',
130-
writeKey: 'key',
131-
},
132-
},
133119
}),
134120
})
135121

@@ -140,7 +126,6 @@ describe('CookieConsent - CookieConsentProvider', () => {
140126
analytics: false,
141127
})
142128
expect(result.current.segmentIntegrations).toStrictEqual({
143-
All: false,
144129
'Google Universal Analytics': false,
145130
Salesforce: false,
146131
'Salesforce custom destination (Scaleway)': false,
@@ -152,13 +137,6 @@ describe('CookieConsent - CookieConsentProvider', () => {
152137
const { result } = renderHook(() => useCookieConsent(), {
153138
wrapper: wrapper({
154139
isConsentRequired: true,
155-
essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'],
156-
config: {
157-
segment: {
158-
cdnURL: 'url',
159-
writeKey: 'key',
160-
},
161-
},
162140
}),
163141
})
164142

@@ -169,7 +147,6 @@ describe('CookieConsent - CookieConsentProvider', () => {
169147
marketing: false,
170148
})
171149
expect(result.current.segmentIntegrations).toStrictEqual({
172-
All: false,
173150
'Google Universal Analytics': false,
174151
Salesforce: false,
175152
'Salesforce custom destination (Scaleway)': false,
@@ -213,17 +190,12 @@ describe('CookieConsent - CookieConsentProvider', () => {
213190
})
214191

215192
it('should not need consent if hash cookie is set', () => {
216-
jest.spyOn(cookie, 'parse').mockReturnValue({ _scw_rgpd_hash: '913003917' })
193+
jest
194+
.spyOn(cookie, 'parse')
195+
.mockImplementation(() => ({ _scw_rgpd_hash: '913003917' }))
217196
const { result } = renderHook(() => useCookieConsent(), {
218197
wrapper: wrapper({
219198
isConsentRequired: true,
220-
essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'],
221-
config: {
222-
segment: {
223-
cdnURL: 'url',
224-
writeKey: 'key',
225-
},
226-
},
227199
}),
228200
})
229201

@@ -234,28 +206,21 @@ describe('CookieConsent - CookieConsentProvider', () => {
234206
marketing: false,
235207
})
236208
expect(result.current.segmentIntegrations).toStrictEqual({
237-
All: false,
238209
'Google Universal Analytics': false,
239210
Salesforce: false,
240211
'Salesforce custom destination (Scaleway)': false,
241212
})
242213
})
243214

244215
it('should not need consent if hash cookie is set and some categories already approved', () => {
245-
jest.spyOn(cookie, 'parse').mockReturnValue({
216+
jest.spyOn(cookie, 'parse').mockImplementation(() => ({
246217
_scw_rgpd_hash: '913003917',
247218
_scw_rgpd_marketing: 'true',
248-
})
219+
}))
220+
249221
const { result } = renderHook(() => useCookieConsent(), {
250222
wrapper: wrapper({
251223
isConsentRequired: true,
252-
essentialIntegrations: ['Deskpro', 'Stripe', 'Sentry'],
253-
config: {
254-
segment: {
255-
cdnURL: 'url',
256-
writeKey: 'key',
257-
},
258-
},
259224
}),
260225
})
261226

@@ -266,10 +231,27 @@ describe('CookieConsent - CookieConsentProvider', () => {
266231
marketing: true,
267232
})
268233
expect(result.current.segmentIntegrations).toStrictEqual({
269-
All: false,
270234
'Google Universal Analytics': false,
271235
Salesforce: true,
272236
'Salesforce custom destination (Scaleway)': true,
273237
})
274238
})
239+
240+
it('should return integration All: false in case there is no integrations', () => {
241+
mockUseSegmentIntegrations.mockReturnValue({
242+
integrations: [],
243+
isLoaded: true,
244+
})
245+
const { result } = renderHook(() => useCookieConsent(), {
246+
wrapper: wrapper({
247+
isConsentRequired: true,
248+
}),
249+
})
250+
251+
expect(mockUseSegmentIntegrations).toBeCalledTimes(2)
252+
253+
expect(result.current.segmentIntegrations).toStrictEqual({
254+
All: false,
255+
})
256+
})
275257
})

0 commit comments

Comments
 (0)