Skip to content

Commit f2766e0

Browse files
apatasanne-san
andauthored
Allow creating and editing limited views (#5925)
* Add :limited_to_segment_id field to shared_links schema * Refactor column name, add FK, index, and on delete cascade * Format * Add static UI for adding a segment to a shared link - Update password UI to use a toogle switch to emphasize the optional nature of the feature - Add UI for limiting the link to a segment - Add toggle_field component for full-width label and toggle switch layout - Add optional link to the input component, showing below the input, which can be used for documentation links - Add eye icon to shared links list that indicates limited view - Add tooltips to icons in the shared links list * Allow creating and editing shared links with limited views * Clarify search function arg names * Unify function to set segment filter * Enforce limited view segment on dashboard * Clarify type * WIP * Disable clear filter button for limited view segment * Load related shared links WIP * Add warning about shared links WIP * Finalize delete modal * Unify shared link password protection checks * WIP * Give random name for shared links in tests * Add tests for related shared links endpoint * Split components and remove nesting * Change limited view copy * Fix search ranking, make segment search input DRYer, add search test * Fix url search params test * Update changelog * Fix invalid accessor * Format * Add test for stuffing limited view segment * Simplify preload syntax * Add tests for validating internal stats API requests * Remove id_suffix default values * Implement limited_to_segment?/1 * Refactor search_by_name * Clarify toggle components * Add tests for Shared Link form, remove unused slot --------- Co-authored-by: Sanne de Vries <[email protected]>
1 parent 9648296 commit f2766e0

File tree

39 files changed

+1587
-276
lines changed

39 files changed

+1587
-276
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file.
77
### Added
88

99
- A visitor percentage breakdown is now shown on all reports, both on the dashboard and in the detailed breakdown
10+
- Shared links can now be limited to a particular segment of the data
1011

1112
### Removed
1213

assets/js/dashboard.tsx

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { createAppRouter } from './dashboard/router'
77
import ErrorBoundary from './dashboard/error/error-boundary'
88
import * as api from './dashboard/api'
99
import * as timer from './dashboard/util/realtime-update-timer'
10-
import { redirectForLegacyParams } from './dashboard/util/url-search-params'
10+
import { maybeDoFERedirect } from './dashboard/util/url-search-params'
1111
import SiteContextProvider, {
1212
parseSiteFromDataset
1313
} from './dashboard/site-context'
@@ -19,6 +19,8 @@ import {
1919
SomethingWentWrongMessage
2020
} from './dashboard/error/something-went-wrong'
2121
import {
22+
getLimitedToSegment,
23+
parseLimitedToSegmentId,
2224
parsePreloadedSegments,
2325
SegmentsContextProvider
2426
} from './dashboard/filtering/segments-context'
@@ -39,8 +41,15 @@ if (container && container.dataset) {
3941
api.setSharedLinkAuth(sharedLinkAuth)
4042
}
4143

44+
const limitedToSegmentId = parseLimitedToSegmentId(container.dataset)
45+
const preloadedSegments = parsePreloadedSegments(container.dataset)
46+
const limitedToSegment = getLimitedToSegment(
47+
limitedToSegmentId,
48+
preloadedSegments
49+
)
50+
4251
try {
43-
redirectForLegacyParams(window.location, window.history)
52+
maybeDoFERedirect(window.location, window.history, limitedToSegment)
4453
} catch (e) {
4554
console.error('Error redirecting in a backwards compatible way', e)
4655
}
@@ -83,7 +92,8 @@ if (container && container.dataset) {
8392
}
8493
>
8594
<SegmentsContextProvider
86-
preloadedSegments={parsePreloadedSegments(container.dataset)}
95+
limitedToSegment={limitedToSegment}
96+
preloadedSegments={preloadedSegments}
8797
>
8898
<RouterProvider router={router} />
8999
</SegmentsContextProvider>

assets/js/dashboard/components/error-panel.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react'
1+
import React, { ReactNode } from 'react'
22
import classNames from 'classnames'
33
import {
44
ArrowPathIcon,
@@ -12,7 +12,7 @@ export const ErrorPanel = ({
1212
onClose,
1313
onRetry
1414
}: {
15-
errorMessage: string
15+
errorMessage: ReactNode
1616
className?: string
1717
onClose?: () => void
1818
onRetry?: () => void

assets/js/dashboard/filtering/segments-context.test.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ describe('SegmentsContext functions', () => {
3535
test('deleteOne works', () => {
3636
render(
3737
<SegmentsContextProvider
38+
limitedToSegment={null}
3839
preloadedSegments={[segmentOpenSource, segmentAPAC]}
3940
>
4041
<TestComponent />
@@ -51,7 +52,10 @@ describe('SegmentsContext functions', () => {
5152

5253
test('addOne adds to head of list', async () => {
5354
render(
54-
<SegmentsContextProvider preloadedSegments={[segmentAPAC]}>
55+
<SegmentsContextProvider
56+
limitedToSegment={null}
57+
preloadedSegments={[segmentAPAC]}
58+
>
5559
<TestComponent />
5660
</SegmentsContextProvider>
5761
)
@@ -68,6 +72,7 @@ describe('SegmentsContext functions', () => {
6872
test('updateOne works: updated segment is at head of list', () => {
6973
render(
7074
<SegmentsContextProvider
75+
limitedToSegment={null}
7176
preloadedSegments={[segmentOpenSource, segmentAPAC]}
7277
>
7378
<TestComponent />

assets/js/dashboard/filtering/segments-context.tsx

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,33 @@ export function parsePreloadedSegments(dataset: DOMStringMap): SavedSegments {
1717
return JSON.parse(dataset.segments!).map(handleSegmentResponse)
1818
}
1919

20+
export function parseLimitedToSegmentId(dataset: DOMStringMap): number | null {
21+
return JSON.parse(dataset.limitedToSegmentId!)
22+
}
23+
24+
export function getLimitedToSegment(
25+
limitedToSegmentId: number | null,
26+
preloadedSegments: SavedSegments
27+
): Pick<SavedSegment, 'id' | 'name'> | null {
28+
if (limitedToSegmentId !== null) {
29+
return preloadedSegments.find((s) => s.id === limitedToSegmentId) ?? null
30+
}
31+
return null
32+
}
33+
2034
type ChangeSegmentState = (
2135
segment: (SavedSegment | SavedSegmentPublic) & { segment_data: SegmentData }
2236
) => void
2337

2438
const initialValue: {
2539
segments: SavedSegments
40+
limitedToSegment: Pick<SavedSegment, 'id' | 'name'> | null
2641
updateOne: ChangeSegmentState
2742
addOne: ChangeSegmentState
2843
removeOne: ChangeSegmentState
2944
} = {
3045
segments: [],
46+
limitedToSegment: null,
3147
updateOne: () => {},
3248
addOne: () => {},
3349
removeOne: () => {}
@@ -41,9 +57,11 @@ export const useSegmentsContext = () => {
4157

4258
export const SegmentsContextProvider = ({
4359
preloadedSegments,
60+
limitedToSegment,
4461
children
4562
}: {
4663
preloadedSegments: SavedSegments
64+
limitedToSegment: Pick<SavedSegment, 'id' | 'name'> | null
4765
children: ReactNode
4866
}) => {
4967
const [segments, setSegments] = useState(preloadedSegments)
@@ -73,7 +91,13 @@ export const SegmentsContextProvider = ({
7391

7492
return (
7593
<SegmentsContext.Provider
76-
value={{ segments, removeOne, updateOne, addOne }}
94+
value={{
95+
segments,
96+
limitedToSegment,
97+
removeOne,
98+
updateOne,
99+
addOne
100+
}}
77101
>
78102
{children}
79103
</SegmentsContext.Provider>

assets/js/dashboard/filtering/segments.test.ts

Lines changed: 79 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { remapToApiFilters } from '../util/filters'
22
import {
33
formatSegmentIdAsLabelKey,
4-
getSearchToApplySingleSegmentFilter,
4+
getSearchToSetSegmentFilter,
55
getSegmentNamePlaceholder,
66
isSegmentIdLabelKey,
77
parseApiSegmentData,
@@ -64,9 +64,57 @@ describe(`${parseApiSegmentData.name}`, () => {
6464
})
6565
})
6666

67-
describe(`${getSearchToApplySingleSegmentFilter.name}`, () => {
68-
test('generated search function applies single segment correctly', () => {
69-
const searchFunction = getSearchToApplySingleSegmentFilter({
67+
describe(`${getSearchToSetSegmentFilter.name}`, () => {
68+
test('generated search function omits other filters segment correctly', () => {
69+
const searchFunction = getSearchToSetSegmentFilter(
70+
{
71+
name: 'APAC',
72+
id: 500
73+
},
74+
{ omitAllOtherFilters: true }
75+
)
76+
const existingSearch = {
77+
date: '2025-02-10',
78+
filters: [
79+
['is', 'country', ['US']],
80+
['is', 'page', ['/blog']]
81+
],
82+
labels: { US: 'United States' }
83+
}
84+
expect(searchFunction(existingSearch)).toEqual({
85+
date: '2025-02-10',
86+
filters: [['is', 'segment', [500]]],
87+
labels: { 'segment-500': 'APAC' }
88+
})
89+
})
90+
91+
test('generated search function replaces existing segment filter correctly', () => {
92+
const searchFunction = getSearchToSetSegmentFilter({
93+
name: 'APAC',
94+
id: 500
95+
})
96+
const existingSearch = {
97+
date: '2025-02-10',
98+
filters: [
99+
['is', 'segment', [100]],
100+
['is', 'country', ['US']],
101+
['is', 'page', ['/blog']]
102+
],
103+
labels: { US: 'United States', 'segment-100': 'Scandinavia' }
104+
}
105+
expect(searchFunction(existingSearch)).toEqual({
106+
date: '2025-02-10',
107+
filters: [
108+
['is', 'segment', [500]],
109+
['is', 'country', ['US']],
110+
['is', 'page', ['/blog']]
111+
],
112+
labels: { US: 'United States', 'segment-500': 'APAC' }
113+
})
114+
})
115+
116+
test('generated search function sets new segment filter correctly', () => {
117+
const searchFunction = getSearchToSetSegmentFilter({
70118
name: 'APAC',
71119
id: 500
72120
})
@@ -80,8 +128,12 @@ describe(`${getSearchToApplySingleSegmentFilter.name}`, () => {
80128
}
81129
expect(searchFunction(existingSearch)).toEqual({
82130
date: '2025-02-10',
83-
filters: [['is', 'segment', [500]]],
84-
labels: { 'segment-500': 'APAC' }
131+
filters: [
132+
['is', 'segment', [500]],
133+
['is', 'country', ['US']],
134+
['is', 'page', ['/blog']]
135+
],
136+
labels: { US: 'United States', 'segment-500': 'APAC' }
85137
})
86138
})
87139
})
@@ -187,7 +239,6 @@ describe(`${canExpandSegment.name}`, () => {
187239
it.each([[Role.admin], [Role.editor], [Role.owner]])(
188240
'allows expanding site segment if the user is logged in and in the role %p',
189241
(role) => {
190-
const site = { siteSegmentsAvailable: true }
191242
const user: UserContextValue = {
192243
loggedIn: true,
193244
role,
@@ -197,8 +248,7 @@ describe(`${canExpandSegment.name}`, () => {
197248
expect(
198249
canExpandSegment({
199250
segment: { id: 1, owner_id: 1, type: SegmentType.site },
200-
user,
201-
site
251+
user
202252
})
203253
).toBe(true)
204254
}
@@ -213,42 +263,11 @@ describe(`${canExpandSegment.name}`, () => {
213263
role: Role.owner,
214264
id: 111,
215265
team: { identifier: null, hasConsolidatedView: false }
216-
},
217-
site: { siteSegmentsAvailable: true }
266+
}
218267
})
219268
).toBe(true)
220269
})
221270

222-
it('forbids expanding site segment if site segments are not available', () => {
223-
expect(
224-
canExpandSegment({
225-
segment: { id: 1, owner_id: 1, type: SegmentType.site },
226-
user: {
227-
loggedIn: true,
228-
role: Role.owner,
229-
id: 1,
230-
team: { identifier: null, hasConsolidatedView: false }
231-
},
232-
site: { siteSegmentsAvailable: false }
233-
})
234-
).toBe(false)
235-
})
236-
237-
it('forbids public role from expanding site segments', () => {
238-
expect(
239-
canExpandSegment({
240-
segment: { id: 1, owner_id: null, type: SegmentType.site },
241-
user: {
242-
loggedIn: false,
243-
role: Role.public,
244-
id: null,
245-
team: { identifier: null, hasConsolidatedView: false }
246-
},
247-
site: { siteSegmentsAvailable: false }
248-
})
249-
).toBe(false)
250-
})
251-
252271
it.each([
253272
[Role.viewer],
254273
[Role.billing],
@@ -267,14 +286,13 @@ describe(`${canExpandSegment.name}`, () => {
267286
expect(
268287
canExpandSegment({
269288
segment: { id: 1, owner_id: 1, type: SegmentType.personal },
270-
user,
271-
site: { siteSegmentsAvailable: false }
289+
user
272290
})
273291
).toBe(true)
274292
}
275293
)
276294

277-
it('forbids expanding personal segment of other users', () => {
295+
it('forbids even site owners from expanding the personal segment of other users', () => {
278296
expect(
279297
canExpandSegment({
280298
segment: { id: 2, owner_id: 222, type: SegmentType.personal },
@@ -283,24 +301,25 @@ describe(`${canExpandSegment.name}`, () => {
283301
role: Role.owner,
284302
id: 111,
285303
team: { identifier: null, hasConsolidatedView: false }
286-
},
287-
site: { siteSegmentsAvailable: false }
304+
}
288305
})
289306
).toBe(false)
290307
})
291308

292-
it('forbids public role from expanding personal segments', () => {
293-
expect(
294-
canExpandSegment({
295-
segment: { id: 1, owner_id: 1, type: SegmentType.personal },
296-
user: {
297-
loggedIn: false,
298-
role: Role.public,
299-
id: null,
300-
team: { identifier: null, hasConsolidatedView: false }
301-
},
302-
site: { siteSegmentsAvailable: false }
303-
})
304-
).toBe(false)
305-
})
309+
it.each([[SegmentType.personal, SegmentType.site]])(
310+
'forbids public role from expanding %s segments',
311+
(segmentType) => {
312+
expect(
313+
canExpandSegment({
314+
segment: { id: 1, owner_id: 1, type: segmentType },
315+
user: {
316+
loggedIn: false,
317+
role: Role.public,
318+
id: null,
319+
team: { identifier: null, hasConsolidatedView: false }
320+
}
321+
})
322+
).toBe(false)
323+
}
324+
)
306325
})

0 commit comments

Comments
 (0)