diff --git a/CHANGELOG.md b/CHANGELOG.md index eee1c4a618de..9582bf74041b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to this project will be documented in this file. ### Added - A visitor percentage breakdown is now shown on all reports, both on the dashboard and in the detailed breakdown +- Shared links can now be limited to a particular segment of the data ### Removed diff --git a/assets/js/dashboard.tsx b/assets/js/dashboard.tsx index c4451df210d9..b0871d687397 100644 --- a/assets/js/dashboard.tsx +++ b/assets/js/dashboard.tsx @@ -7,7 +7,7 @@ import { createAppRouter } from './dashboard/router' import ErrorBoundary from './dashboard/error/error-boundary' import * as api from './dashboard/api' import * as timer from './dashboard/util/realtime-update-timer' -import { redirectForLegacyParams } from './dashboard/util/url-search-params' +import { maybeDoFERedirect } from './dashboard/util/url-search-params' import SiteContextProvider, { parseSiteFromDataset } from './dashboard/site-context' @@ -19,6 +19,8 @@ import { SomethingWentWrongMessage } from './dashboard/error/something-went-wrong' import { + getLimitedToSegment, + parseLimitedToSegmentId, parsePreloadedSegments, SegmentsContextProvider } from './dashboard/filtering/segments-context' @@ -39,8 +41,15 @@ if (container && container.dataset) { api.setSharedLinkAuth(sharedLinkAuth) } + const limitedToSegmentId = parseLimitedToSegmentId(container.dataset) + const preloadedSegments = parsePreloadedSegments(container.dataset) + const limitedToSegment = getLimitedToSegment( + limitedToSegmentId, + preloadedSegments + ) + try { - redirectForLegacyParams(window.location, window.history) + maybeDoFERedirect(window.location, window.history, limitedToSegment) } catch (e) { console.error('Error redirecting in a backwards compatible way', e) } @@ -83,7 +92,8 @@ if (container && container.dataset) { } > diff --git a/assets/js/dashboard/components/error-panel.tsx b/assets/js/dashboard/components/error-panel.tsx index f010aacc603e..1ad5c68e8264 100644 --- a/assets/js/dashboard/components/error-panel.tsx +++ b/assets/js/dashboard/components/error-panel.tsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, { ReactNode } from 'react' import classNames from 'classnames' import { ArrowPathIcon, @@ -12,7 +12,7 @@ export const ErrorPanel = ({ onClose, onRetry }: { - errorMessage: string + errorMessage: ReactNode className?: string onClose?: () => void onRetry?: () => void diff --git a/assets/js/dashboard/filtering/segments-context.test.tsx b/assets/js/dashboard/filtering/segments-context.test.tsx index 01889aa5d396..3925e6d39b6c 100644 --- a/assets/js/dashboard/filtering/segments-context.test.tsx +++ b/assets/js/dashboard/filtering/segments-context.test.tsx @@ -35,6 +35,7 @@ describe('SegmentsContext functions', () => { test('deleteOne works', () => { render( @@ -51,7 +52,10 @@ describe('SegmentsContext functions', () => { test('addOne adds to head of list', async () => { render( - + ) @@ -68,6 +72,7 @@ describe('SegmentsContext functions', () => { test('updateOne works: updated segment is at head of list', () => { render( diff --git a/assets/js/dashboard/filtering/segments-context.tsx b/assets/js/dashboard/filtering/segments-context.tsx index cbc15233e303..aacebb4bdac8 100644 --- a/assets/js/dashboard/filtering/segments-context.tsx +++ b/assets/js/dashboard/filtering/segments-context.tsx @@ -17,17 +17,33 @@ export function parsePreloadedSegments(dataset: DOMStringMap): SavedSegments { return JSON.parse(dataset.segments!).map(handleSegmentResponse) } +export function parseLimitedToSegmentId(dataset: DOMStringMap): number | null { + return JSON.parse(dataset.limitedToSegmentId!) +} + +export function getLimitedToSegment( + limitedToSegmentId: number | null, + preloadedSegments: SavedSegments +): Pick | null { + if (limitedToSegmentId !== null) { + return preloadedSegments.find((s) => s.id === limitedToSegmentId) ?? null + } + return null +} + type ChangeSegmentState = ( segment: (SavedSegment | SavedSegmentPublic) & { segment_data: SegmentData } ) => void const initialValue: { segments: SavedSegments + limitedToSegment: Pick | null updateOne: ChangeSegmentState addOne: ChangeSegmentState removeOne: ChangeSegmentState } = { segments: [], + limitedToSegment: null, updateOne: () => {}, addOne: () => {}, removeOne: () => {} @@ -41,9 +57,11 @@ export const useSegmentsContext = () => { export const SegmentsContextProvider = ({ preloadedSegments, + limitedToSegment, children }: { preloadedSegments: SavedSegments + limitedToSegment: Pick | null children: ReactNode }) => { const [segments, setSegments] = useState(preloadedSegments) @@ -73,7 +91,13 @@ export const SegmentsContextProvider = ({ return ( {children} diff --git a/assets/js/dashboard/filtering/segments.test.ts b/assets/js/dashboard/filtering/segments.test.ts index e896ac3d02b0..932d9eb79fbf 100644 --- a/assets/js/dashboard/filtering/segments.test.ts +++ b/assets/js/dashboard/filtering/segments.test.ts @@ -1,7 +1,7 @@ import { remapToApiFilters } from '../util/filters' import { formatSegmentIdAsLabelKey, - getSearchToApplySingleSegmentFilter, + getSearchToSetSegmentFilter, getSegmentNamePlaceholder, isSegmentIdLabelKey, parseApiSegmentData, @@ -64,9 +64,57 @@ describe(`${parseApiSegmentData.name}`, () => { }) }) -describe(`${getSearchToApplySingleSegmentFilter.name}`, () => { - test('generated search function applies single segment correctly', () => { - const searchFunction = getSearchToApplySingleSegmentFilter({ +describe(`${getSearchToSetSegmentFilter.name}`, () => { + test('generated search function omits other filters segment correctly', () => { + const searchFunction = getSearchToSetSegmentFilter( + { + name: 'APAC', + id: 500 + }, + { omitAllOtherFilters: true } + ) + const existingSearch = { + date: '2025-02-10', + filters: [ + ['is', 'country', ['US']], + ['is', 'page', ['/blog']] + ], + labels: { US: 'United States' } + } + expect(searchFunction(existingSearch)).toEqual({ + date: '2025-02-10', + filters: [['is', 'segment', [500]]], + labels: { 'segment-500': 'APAC' } + }) + }) + + test('generated search function replaces existing segment filter correctly', () => { + const searchFunction = getSearchToSetSegmentFilter({ + name: 'APAC', + id: 500 + }) + const existingSearch = { + date: '2025-02-10', + filters: [ + ['is', 'segment', [100]], + ['is', 'country', ['US']], + ['is', 'page', ['/blog']] + ], + labels: { US: 'United States', 'segment-100': 'Scandinavia' } + } + expect(searchFunction(existingSearch)).toEqual({ + date: '2025-02-10', + filters: [ + ['is', 'segment', [500]], + ['is', 'country', ['US']], + ['is', 'page', ['/blog']] + ], + labels: { US: 'United States', 'segment-500': 'APAC' } + }) + }) + + test('generated search function sets new segment filter correctly', () => { + const searchFunction = getSearchToSetSegmentFilter({ name: 'APAC', id: 500 }) @@ -80,8 +128,12 @@ describe(`${getSearchToApplySingleSegmentFilter.name}`, () => { } expect(searchFunction(existingSearch)).toEqual({ date: '2025-02-10', - filters: [['is', 'segment', [500]]], - labels: { 'segment-500': 'APAC' } + filters: [ + ['is', 'segment', [500]], + ['is', 'country', ['US']], + ['is', 'page', ['/blog']] + ], + labels: { US: 'United States', 'segment-500': 'APAC' } }) }) }) @@ -187,7 +239,6 @@ describe(`${canExpandSegment.name}`, () => { it.each([[Role.admin], [Role.editor], [Role.owner]])( 'allows expanding site segment if the user is logged in and in the role %p', (role) => { - const site = { siteSegmentsAvailable: true } const user: UserContextValue = { loggedIn: true, role, @@ -197,8 +248,7 @@ describe(`${canExpandSegment.name}`, () => { expect( canExpandSegment({ segment: { id: 1, owner_id: 1, type: SegmentType.site }, - user, - site + user }) ).toBe(true) } @@ -213,42 +263,11 @@ describe(`${canExpandSegment.name}`, () => { role: Role.owner, id: 111, team: { identifier: null, hasConsolidatedView: false } - }, - site: { siteSegmentsAvailable: true } + } }) ).toBe(true) }) - it('forbids expanding site segment if site segments are not available', () => { - expect( - canExpandSegment({ - segment: { id: 1, owner_id: 1, type: SegmentType.site }, - user: { - loggedIn: true, - role: Role.owner, - id: 1, - team: { identifier: null, hasConsolidatedView: false } - }, - site: { siteSegmentsAvailable: false } - }) - ).toBe(false) - }) - - it('forbids public role from expanding site segments', () => { - expect( - canExpandSegment({ - segment: { id: 1, owner_id: null, type: SegmentType.site }, - user: { - loggedIn: false, - role: Role.public, - id: null, - team: { identifier: null, hasConsolidatedView: false } - }, - site: { siteSegmentsAvailable: false } - }) - ).toBe(false) - }) - it.each([ [Role.viewer], [Role.billing], @@ -267,14 +286,13 @@ describe(`${canExpandSegment.name}`, () => { expect( canExpandSegment({ segment: { id: 1, owner_id: 1, type: SegmentType.personal }, - user, - site: { siteSegmentsAvailable: false } + user }) ).toBe(true) } ) - it('forbids expanding personal segment of other users', () => { + it('forbids even site owners from expanding the personal segment of other users', () => { expect( canExpandSegment({ segment: { id: 2, owner_id: 222, type: SegmentType.personal }, @@ -283,24 +301,25 @@ describe(`${canExpandSegment.name}`, () => { role: Role.owner, id: 111, team: { identifier: null, hasConsolidatedView: false } - }, - site: { siteSegmentsAvailable: false } + } }) ).toBe(false) }) - it('forbids public role from expanding personal segments', () => { - expect( - canExpandSegment({ - segment: { id: 1, owner_id: 1, type: SegmentType.personal }, - user: { - loggedIn: false, - role: Role.public, - id: null, - team: { identifier: null, hasConsolidatedView: false } - }, - site: { siteSegmentsAvailable: false } - }) - ).toBe(false) - }) + it.each([[SegmentType.personal, SegmentType.site]])( + 'forbids public role from expanding %s segments', + (segmentType) => { + expect( + canExpandSegment({ + segment: { id: 1, owner_id: 1, type: segmentType }, + user: { + loggedIn: false, + role: Role.public, + id: null, + team: { identifier: null, hasConsolidatedView: false } + } + }) + ).toBe(false) + } + ) }) diff --git a/assets/js/dashboard/filtering/segments.ts b/assets/js/dashboard/filtering/segments.ts index c1516138a81e..ff3e324cd353 100644 --- a/assets/js/dashboard/filtering/segments.ts +++ b/assets/js/dashboard/filtering/segments.ts @@ -115,16 +115,44 @@ export const parseApiSegmentData = ({ ...rest }) -export function getSearchToApplySingleSegmentFilter( - segment: Pick +export function getSearchToRemoveSegmentFilter(): Required['search'] { + return (searchRecord) => { + const updatedFilters = ( + (Array.isArray(searchRecord.filters) + ? searchRecord.filters + : []) as Filter[] + ).filter((f) => !isSegmentFilter(f)) + const currentLabels = searchRecord.labels ?? {} + return { + ...searchRecord, + filters: updatedFilters, + labels: cleanLabels(updatedFilters, currentLabels) + } + } +} + +export function getSearchToSetSegmentFilter( + segment: Pick, + options: { omitAllOtherFilters?: boolean } = {} ): Required['search'] { - return (search) => { - const filters = [['is', 'segment', [segment.id]]] - const labels = cleanLabels(filters, {}, 'segment', { + return (searchRecord) => { + const otherFilters = ( + (Array.isArray(searchRecord.filters) + ? searchRecord.filters + : []) as Filter[] + ).filter((f) => !isSegmentFilter(f)) + const currentLabels = searchRecord.labels ?? {} + + const filters = [ + ['is', 'segment', [segment.id]], + ...(options.omitAllOtherFilters ? [] : otherFilters) + ] + + const labels = cleanLabels(filters, currentLabels, 'segment', { [formatSegmentIdAsLabelKey(segment.id)]: segment.name }) return { - ...search, + ...searchRecord, filters, labels } @@ -160,16 +188,13 @@ export function resolveFilters( export function canExpandSegment({ segment, - site, user }: { segment: Pick - site: Pick user: UserContextValue }) { if ( segment.type === SegmentType.site && - site.siteSegmentsAvailable && user.loggedIn && ROLES_WITH_MAYBE_SITE_SEGMENTS.includes(user.role) ) { @@ -213,6 +238,23 @@ export function isListableSegment({ return false } +export function canSeeSegmentDetails({ user }: { user: UserContextValue }) { + return user.loggedIn && user.role !== Role.public +} + +export function canRemoveFilter( + filter: Filter, + limitedToSegment: Pick | null +) { + if (isSegmentFilter(filter) && limitedToSegment) { + const [_operation, _dimension, clauses] = filter + return ( + clauses.length === 1 && String(limitedToSegment.id) === String(clauses[1]) + ) + } + return true +} + export function findAppliedSegmentFilter({ filters }: { filters: Filter[] }) { const segmentFilter = filters.find(isSegmentFilter) if (!segmentFilter) { diff --git a/assets/js/dashboard/nav-menu/filter-menu.tsx b/assets/js/dashboard/nav-menu/filter-menu.tsx index 12fa7c999737..ba1c6178a71c 100644 --- a/assets/js/dashboard/nav-menu/filter-menu.tsx +++ b/assets/js/dashboard/nav-menu/filter-menu.tsx @@ -11,6 +11,7 @@ import { popover, BlurMenuButtonOnEscape } from '../components/popover' import classNames from 'classnames' import { AppNavigationLink } from '../navigation/use-app-navigate' import { SearchableSegmentsSection } from './segments/searchable-segments-section' +import { useSegmentsContext } from '../filtering/segments-context' export function getFilterListItems({ propsAvailable @@ -49,6 +50,7 @@ const FilterMenuItems = ({ closeDropdown }: { closeDropdown: () => void }) => { const columns = useMemo(() => getFilterListItems(site), [site]) const buttonRef = useRef(null) const panelRef = useRef(null) + const { limitedToSegment } = useSegmentsContext() return ( <> @@ -107,10 +109,12 @@ const FilterMenuItems = ({ closeDropdown }: { closeDropdown: () => void }) => { ))} - + {limitedToSegment === null && ( + + )} diff --git a/assets/js/dashboard/nav-menu/filter-pills-list.tsx b/assets/js/dashboard/nav-menu/filter-pills-list.tsx index 6ca03290aa0a..307eae994ea2 100644 --- a/assets/js/dashboard/nav-menu/filter-pills-list.tsx +++ b/assets/js/dashboard/nav-menu/filter-pills-list.tsx @@ -10,6 +10,8 @@ import { styledFilterText, plainFilterText } from '../util/filter-text' import { useAppNavigate } from '../navigation/use-app-navigate' import classNames from 'classnames' import { filterRoute } from '../router' +import { canRemoveFilter } from '../filtering/segments' +import { useSegmentsContext } from '../filtering/segments-context' export const PILL_X_GAP_PX = 16 export const PILL_Y_GAP_PX = 8 @@ -48,6 +50,7 @@ export const AppliedFilterPillsList = React.forwardRef< AppliedFilterPillsListProps >(({ className, style, slice, direction, pillClassName }, ref) => { const { query } = useQueryContext() + const { limitedToSegment } = useSegmentsContext() const navigate = useAppNavigate() const renderableFilters = @@ -82,19 +85,21 @@ export const AppliedFilterPillsList = React.forwardRef< ] } }, - onRemoveClick: () => { - const newFilters = query.filters.filter( - (_, i) => i !== index + indexAdjustment - ) + onRemoveClick: canRemoveFilter(filter, limitedToSegment) + ? () => { + const newFilters = query.filters.filter( + (_, i) => i !== index + indexAdjustment + ) - navigate({ - search: (search) => ({ - ...search, - filters: newFilters, - labels: cleanLabels(newFilters, query.labels) - }) - }) - } + navigate({ + search: (searchRecord) => ({ + ...searchRecord, + filters: newFilters, + labels: cleanLabels(newFilters, query.labels) + }) + }) + } + : undefined } }))} className={className} diff --git a/assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx b/assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx index fbbaaf216823..810553a420c7 100644 --- a/assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx +++ b/assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx @@ -2,14 +2,12 @@ import React, { RefObject } from 'react' import { useQueryContext } from '../../query-context' import { useSiteContext } from '../../site-context' import { - formatSegmentIdAsLabelKey, - isSegmentFilter, SavedSegmentPublic, SavedSegment, SEGMENT_TYPE_LABELS, - isListableSegment + isListableSegment, + getSearchToSetSegmentFilter } from '../../filtering/segments' -import { cleanLabels } from '../../util/filters' import classNames from 'classnames' import { Tooltip } from '../../util/tooltip' import { SegmentAuthorship } from '../../segments/segment-authorship' @@ -170,26 +168,12 @@ const SegmentLink = ({ }: Pick & { closeList: () => void }) => { - const { query } = useQueryContext() - return ( { - const otherFilters = query.filters.filter((f) => !isSegmentFilter(f)) - - const updatedFilters = [['is', 'segment', [id]], ...otherFilters] - - return { - ...search, - filters: updatedFilters, - labels: cleanLabels(updatedFilters, query.labels, 'segment', { - [formatSegmentIdAsLabelKey(id)]: name - }) - } - }} + search={getSearchToSetSegmentFilter({ id, name })} >
{name}
diff --git a/assets/js/dashboard/navigation/use-app-navigate.tsx b/assets/js/dashboard/navigation/use-app-navigate.tsx index c8342c9d6ce5..c3d171ac818c 100644 --- a/assets/js/dashboard/navigation/use-app-navigate.tsx +++ b/assets/js/dashboard/navigation/use-app-navigate.tsx @@ -9,6 +9,8 @@ import { LinkProps } from 'react-router-dom' import { parseSearch, stringifySearch } from '../util/url-search-params' +import { useSegmentsContext } from '../filtering/segments-context' +import { getSearchToSetSegmentFilter } from '../filtering/segments' export type AppNavigationTarget = { /** @@ -24,7 +26,7 @@ export type AppNavigationTarget = { * - `(s) => s` preserves current search value, * - `(s) => ({ ...s, calendar: !s.calendar })` toggles the value for calendar search parameter, * - `() => ({ page: 5 })` sets the search to `?page=5`, - * - `undefined` empties the search + * - `undefined` empties the search, except for the enforced segment */ search?: (search: Record) => Record } @@ -44,11 +46,24 @@ const getNavigateToOptions = ( export const useGetNavigateOptions = () => { const location = useLocation() + const { limitedToSegment } = useSegmentsContext() + const getToOptions = useCallback( ({ path, params, search }: AppNavigationTarget) => { - return getNavigateToOptions(location.search, { path, params, search }) + const wrappedSearch: typeof search = (searchRecord) => { + const updatedSearchRecord = + typeof search === 'function' ? search(searchRecord) : searchRecord + return limitedToSegment + ? getSearchToSetSegmentFilter(limitedToSegment)(updatedSearchRecord) + : updatedSearchRecord + } + return getNavigateToOptions(location.search, { + path, + params, + search: wrappedSearch + }) }, - [location.search] + [location.search, limitedToSegment] ) return getToOptions } diff --git a/assets/js/dashboard/segments/routeless-segment-modals.tsx b/assets/js/dashboard/segments/routeless-segment-modals.tsx index 024a9d4b8ff8..6541b5d93ab8 100644 --- a/assets/js/dashboard/segments/routeless-segment-modals.tsx +++ b/assets/js/dashboard/segments/routeless-segment-modals.tsx @@ -5,7 +5,7 @@ import { UpdateSegmentModal } from './segment-modals' import { - getSearchToApplySingleSegmentFilter, + getSearchToSetSegmentFilter, getSegmentNamePlaceholder, handleSegmentResponse, SavedSegment, @@ -67,7 +67,9 @@ export const RoutelessSegmentModals = () => { updateOne(segment) queryClient.invalidateQueries({ queryKey: ['segments'] }) navigate({ - search: getSearchToApplySingleSegmentFilter(segment), + search: getSearchToSetSegmentFilter(segment, { + omitAllOtherFilters: true + }), state: { expandedSegment: null } @@ -104,7 +106,9 @@ export const RoutelessSegmentModals = () => { addOne(segment) queryClient.invalidateQueries({ queryKey: ['segments'] }) navigate({ - search: getSearchToApplySingleSegmentFilter(segment), + search: getSearchToSetSegmentFilter(segment, { + omitAllOtherFilters: true + }), state: { expandedSegment: null } diff --git a/assets/js/dashboard/segments/segment-modals.test.tsx b/assets/js/dashboard/segments/segment-modals.test.tsx index 92535aaad72a..a22160928ee2 100644 --- a/assets/js/dashboard/segments/segment-modals.test.tsx +++ b/assets/js/dashboard/segments/segment-modals.test.tsx @@ -4,6 +4,7 @@ import { SegmentModal } from './segment-modals' import { TestContextProviders } from '../../../test-utils/app-context-providers' import { SavedSegment, + SavedSegmentPublic, SavedSegments, SegmentData, SegmentType @@ -18,7 +19,7 @@ beforeEach(() => { }) describe('Segment details modal - errors', () => { - const anySiteSegment: SavedSegment & { segment_data: SegmentData } = { + const anySegment: SavedSegment & { segment_data: SegmentData } = { id: 1, type: SegmentType.site, owner_id: 1, @@ -33,7 +34,7 @@ describe('Segment details modal - errors', () => { } const anyPersonalSegment: SavedSegment & { segment_data: SegmentData } = { - ...anySiteSegment, + ...anySegment, id: 2, type: SegmentType.personal } @@ -48,7 +49,7 @@ describe('Segment details modal - errors', () => { }[] = [ { case: 'segment is not in list', - segments: [anyPersonalSegment, anySiteSegment], + segments: [anyPersonalSegment, anySegment], segmentId: 202020, user: { loggedIn: true, @@ -81,12 +82,125 @@ describe('Segment details modal - errors', () => { }) describe('Segment details modal - other cases', () => { - it('displays site segment correctly', () => { - const anySiteSegment: SavedSegment & { segment_data: SegmentData } = { + it.each([ + [SegmentType.site, 'Site segment'], + [SegmentType.personal, 'Personal segment'] + ])( + 'displays segment with type %s correctly for logged in user', + (segmentType, expectedSegmentTypeText) => { + const user: UserContextValue = { + loggedIn: true, + role: Role.editor, + id: 1, + team: { identifier: null, hasConsolidatedView: false } + } + const anySegment: SavedSegment & { segment_data: SegmentData } = { + id: 100, + type: segmentType, + owner_id: user.id, + owner_name: 'Jane Smith', + name: 'Blog or About', + segment_data: { + filters: [['is', 'page', ['/blog', '/about']]], + labels: {} + }, + inserted_at: '2025-03-13T13:00:00', + updated_at: '2025-03-13T16:00:00' + } + + render(, { + wrapper: (props) => ( + + ) + }) + expect(screen.getByText(anySegment.name)).toBeVisible() + expect(screen.getByText(expectedSegmentTypeText)).toBeVisible() + + expect(screen.getByText('Filters in segment')).toBeVisible() + expect(screen.getByTitle('Page is /blog or /about')).toBeVisible() + + expect( + screen.getByText(`Last updated at 13 Mar by ${anySegment.owner_name}`) + ).toBeVisible() + expect(screen.getByText(`Created at 13 Mar`)).toBeVisible() + + expect(screen.getByText('Edit segment')).toBeVisible() + expect(screen.getByText('Remove filter')).toBeVisible() + } + ) + + it.each([ + [SegmentType.site, 'Site segment'], + [SegmentType.personal, 'Personal segment'] + ])( + 'displays segment with type %s correctly for public role', + (segmentType, expectedSegmentTypeText) => { + const user: UserContextValue = { + loggedIn: false, + role: Role.public, + id: null, + team: { identifier: null, hasConsolidatedView: false } + } + const anySegment: SavedSegment & { segment_data: SegmentData } = { + id: 100, + type: segmentType, + owner_id: null, + owner_name: null, + name: 'Blog or About', + segment_data: { + filters: [['is', 'page', ['/blog', '/about']]], + labels: {} + }, + inserted_at: '2025-03-13T13:00:00', + updated_at: '2025-03-13T16:00:00' + } + + render(, { + wrapper: (props) => ( + + ) + }) + expect(screen.getByText(anySegment.name)).toBeVisible() + expect(screen.getByText(expectedSegmentTypeText)).toBeVisible() + + expect(screen.getByText('Filters in segment')).toBeVisible() + expect(screen.getByTitle('Page is /blog or /about')).toBeVisible() + + expect(screen.getByText(`Last updated at 13 Mar`)).toBeVisible() + expect(screen.queryByText('by ')).toBeNull() // no segment author is shown to public role + expect(screen.getByText(`Created at 13 Mar`)).toBeVisible() + + expect(screen.getByText('Remove filter')).toBeVisible() + expect(screen.queryByText('Edit segment')).toBeNull() + } + ) + + it('allows elevated roles to expand site segments even if site segments are not available on their plan (to update type to personal segment)', () => { + const user: UserContextValue = { + loggedIn: true, + role: Role.owner, + id: 1, + team: { identifier: null, hasConsolidatedView: false } + } + const anySegment: SavedSegmentPublic & { segment_data: SegmentData } = { id: 100, type: SegmentType.site, - owner_id: 100100, - owner_name: 'Test User', + owner_id: null, + owner_name: null, name: 'Blog or About', segment_data: { filters: [['is', 'page', ['/blog', '/about']]], @@ -96,35 +210,77 @@ describe('Segment details modal - other cases', () => { updated_at: '2025-03-13T16:00:00' } - render(, { + render(, { wrapper: (props) => ( ) }) - expect(screen.getByText(anySiteSegment.name)).toBeVisible() + expect(screen.getByText(anySegment.name)).toBeVisible() expect(screen.getByText('Site segment')).toBeVisible() expect(screen.getByText('Filters in segment')).toBeVisible() expect(screen.getByTitle('Page is /blog or /about')).toBeVisible() expect( - screen.getByText(`Last updated at 13 Mar by ${anySiteSegment.owner_name}`) + screen.getByText(`Last updated at 13 Mar by (Removed User)`) ).toBeVisible() expect(screen.getByText(`Created at 13 Mar`)).toBeVisible() - expect(screen.getByText('Edit segment')).toBeVisible() expect(screen.getByText('Remove filter')).toBeVisible() + expect(screen.getByText('Edit segment')).toBeVisible() + }) + + it('does not display clear filter button if the dashboard is limited to this segment', () => { + const user: UserContextValue = { + loggedIn: false, + role: Role.public, + id: null, + team: { identifier: null, hasConsolidatedView: false } + } + const anySegment: SavedSegmentPublic & { segment_data: SegmentData } = { + id: 100, + type: SegmentType.site, + owner_id: null, + owner_name: null, + name: 'Blog or About', + segment_data: { + filters: [['is', 'page', ['/blog', '/about']]], + labels: {} + }, + inserted_at: '2025-03-13T13:00:00', + updated_at: '2025-03-13T16:00:00' + } + + render(, { + wrapper: (props) => ( + + ) + }) + expect(screen.getByText(anySegment.name)).toBeVisible() + expect(screen.getByText('Site segment')).toBeVisible() + + expect(screen.getByText('Filters in segment')).toBeVisible() + expect(screen.getByTitle('Page is /blog or /about')).toBeVisible() + + expect(screen.getByText(`Last updated at 13 Mar`)).toBeVisible() + expect(screen.getByText(`Created at 13 Mar`)).toBeVisible() + + expect(screen.queryByText('Remove filter')).toBeNull() + expect(screen.queryByText('Edit segment')).toBeNull() }) }) diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx index 1261a24a0c30..92c18f1b69a2 100644 --- a/assets/js/dashboard/segments/segment-modals.tsx +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -1,24 +1,26 @@ import React, { ReactNode, useCallback, useState } from 'react' import ModalWithRouting from '../stats/modals/modal' import { + canRemoveFilter, + getSearchToRemoveSegmentFilter, canExpandSegment, - isSegmentFilter, SavedSegment, SEGMENT_TYPE_LABELS, SegmentData, SegmentType } from '../filtering/segments' -import { useQueryContext } from '../query-context' -import { AppNavigationLink } from '../navigation/use-app-navigate' -import { cleanLabels } from '../util/filters' +import { + AppNavigationLink, + useAppNavigate +} from '../navigation/use-app-navigate' import { plainFilterText, styledFilterText } from '../util/filter-text' import { rootRoute } from '../router' import { FilterPillsList } from '../nav-menu/filter-pills-list' import classNames from 'classnames' import { SegmentAuthorship } from './segment-authorship' import { ExclamationTriangleIcon } from '@heroicons/react/24/outline' -import { MutationStatus } from '@tanstack/react-query' -import { ApiError } from '../api' +import { MutationStatus, useQuery } from '@tanstack/react-query' +import { ApiError, get } from '../api' import { ErrorPanel } from '../components/error-panel' import { useSegmentsContext } from '../filtering/segments-context' import { Role, UserContextValue, useUserContext } from '../user-context' @@ -42,7 +44,8 @@ const primaryNeutralButtonClassName = 'button !px-3' const primaryNegativeButtonClassName = classNames( 'button !px-3.5', - 'items-center !bg-red-500 dark:!bg-red-500 hover:!bg-red-600 dark:hover:!bg-red-700 whitespace-nowrap' + 'items-center !bg-red-500 dark:!bg-red-500 hover:!bg-red-600 dark:hover:!bg-red-700 whitespace-nowrap', + 'disabled:!bg-red-400 disabled:cursor-not-allowed' ) const secondaryButtonClassName = classNames( @@ -105,7 +108,7 @@ export const CreateSegmentModal = ({ return ( - Create segment + Create segment ) => void segment: SavedSegment & { segment_data?: SegmentData } } & ApiRequestProps) => { + const site = useSiteContext() + const [confirmed, setConfirmed] = useState(false) + + const linksQuery = useQuery({ + queryKey: [segment.id], + queryFn: async () => { + const response: string[] = await get( + `/api/${encodeURIComponent(site.domain)}/segments/${segment.id}/shared-links` + ) + return response + } + }) + + const deleteDisabled = + status === 'pending' || + linksQuery.status !== 'success' || + (!!linksQuery.data?.length && !confirmed) + return ( - + Delete {SEGMENT_TYPE_LABELS[segment.type].toLowerCase()} {` "${segment.name}"?`} + {linksQuery.status === 'pending' && ( +
+
+
+ )} + {linksQuery.status === 'success' && !!linksQuery.data?.length && ( + + {getLinksDeleteNotice(linksQuery.data)} + + } + /> + )} + {linksQuery.status === 'error' && ( + + )} {!!segment.segment_data && ( - +
+ +
+ )} + {!!linksQuery.data?.length && ( + <> +
+ +
+
+ setConfirmed(e.currentTarget.checked)} + > + Yes, delete the associated shared links + +
+ )} - + )}
diff --git a/assets/js/dashboard/stats/graph/line-graph.js b/assets/js/dashboard/stats/graph/line-graph.js index ef45b2fa4acd..4b10e734091d 100644 --- a/assets/js/dashboard/stats/graph/line-graph.js +++ b/assets/js/dashboard/stats/graph/line-graph.js @@ -247,11 +247,11 @@ class LineGraph extends React.Component { if (this.props.graphData.interval === 'month') { this.props.navigate({ - search: (search) => ({ ...search, period: 'month', date }) + search: (searchRecord) => ({ ...searchRecord, period: 'month', date }) }) } else if (this.props.graphData.interval === 'day') { this.props.navigate({ - search: (search) => ({ ...search, period: 'day', date }) + search: (searchRecord) => ({ ...searchRecord, period: 'day', date }) }) } } diff --git a/assets/js/dashboard/stats/locations/map.tsx b/assets/js/dashboard/stats/locations/map.tsx index 593a5747d8f0..5ef90cf0dd2c 100644 --- a/assets/js/dashboard/stats/locations/map.tsx +++ b/assets/js/dashboard/stats/locations/map.tsx @@ -106,7 +106,9 @@ const WorldMap = ({ [country.code]: country.name }) onCountrySelect() - navigate({ search: (search) => ({ ...search, filters, labels }) }) + navigate({ + search: (searchRecord) => ({ ...searchRecord, filters, labels }) + }) } }, [navigate, query, dataByCountryCode, onCountrySelect] diff --git a/assets/js/dashboard/stats/modals/filter-modal.js b/assets/js/dashboard/stats/modals/filter-modal.js index c25cf483b4be..3864ac455007 100644 --- a/assets/js/dashboard/stats/modals/filter-modal.js +++ b/assets/js/dashboard/stats/modals/filter-modal.js @@ -120,8 +120,8 @@ class FilterModal extends React.Component { selectFiltersAndCloseModal(filters) { this.props.navigate({ path: rootRoute.path, - search: (search) => ({ - ...search, + search: (searchRecord) => ({ + ...searchRecord, filters: filters, labels: cleanLabels(filters, this.state.labelState) }), diff --git a/assets/js/dashboard/util/url-search-params.test.ts b/assets/js/dashboard/util/url-search-params.test.ts index 91ecbb6b2894..6f99dfa4c143 100644 --- a/assets/js/dashboard/util/url-search-params.test.ts +++ b/assets/js/dashboard/util/url-search-params.test.ts @@ -1,8 +1,9 @@ import { Filter } from '../query' import { encodeURIComponentPermissive, + getSearchWithEnforcedSegment, isSearchEntryDefined, - getRedirectTarget, + maybeGetLatestReadableSearch, parseFilter, parseLabelsEntry, parseSearch, @@ -206,61 +207,55 @@ describe(`${stringifySearch.name}`, () => { }) }) -describe(`${getRedirectTarget.name}`, () => { +describe(`${maybeGetLatestReadableSearch.name}`, () => { it.each([ [''], ['?auth=_Y6YOjUl2beUJF_XzG1hk&theme=light&background=%23ee00ee'], ['?keybindHint=Escape&with_imported=true'], ['?f=is,page,/blog/:category/:article-name&date=2024-10-10&period=day'], ['?f=is,country,US&l=US,United%20States'] - ])('for modern search %p returns null', (search) => { - expect( - getRedirectTarget({ - pathname: '/example.com%2Fdeep%2Fpath', - search - } as Location) - ).toBeNull() + ])('for modern search string %p returns null', (search) => { + expect(maybeGetLatestReadableSearch(search)).toBeNull() }) - it('returns updated URL for jsonurl style filters (v2), and running the updated value through the function again returns null (no redirect loop)', () => { - const pathname = '/' + it('returns updated search string for jsonurl style filters (v2), and running the updated value through the function again returns null (no redirect loop)', () => { const search = '?filters=((is,exit_page,(/plausible.io)),(is,source,(Brave)),(is,city,(993800)))&labels=(993800:Johannesburg)' const expectedUpdatedSearch = '?f=is,exit_page,/plausible.io&f=is,source,Brave&f=is,city,993800&l=993800,Johannesburg&r=v2' - expect( - getRedirectTarget({ - pathname, - search - } as Location) - ).toEqual(`${pathname}${expectedUpdatedSearch}`) - expect( - getRedirectTarget({ - pathname, - search: expectedUpdatedSearch - } as Location) - ).toBeNull() + expect(maybeGetLatestReadableSearch(search)).toEqual(expectedUpdatedSearch) + expect(maybeGetLatestReadableSearch(expectedUpdatedSearch)).toBeNull() }) it.each([ ['?page=/docs', '?f=is,page,/docs&r=v1'], - ['?page=%C3%AA&embed=true', '?f=is,page,%C3%AA&embed=true&r=v1'] + ['?page=%C3%AA&embed=true', '?f=is,page,%C3%AA&embed=true&r=v1'], + [ + '?page=/|/foo&goal=~Signup&source=!Facebook|Instagram', + '?f=is,page,/,/foo&f=contains,goal,Signup&f=is_not,source,Facebook,Instagram&r=v1' + ] ])( - 'returns updated URL v1 style filter %s, and running the updated value through the function again returns null (no redirect loop)', + 'returns updated search string v1 style filter %s, and running the updated value through the function again returns null (no redirect loop)', (searchString, expectedSearchString) => { - const pathname = '/' - expect( - getRedirectTarget({ - pathname, - search: searchString - } as Location) - ).toEqual(`${pathname}${expectedSearchString}`) - expect( - getRedirectTarget({ - pathname, - search: expectedSearchString - } as Location) - ).toBeNull() + expect(maybeGetLatestReadableSearch(searchString)).toEqual( + expectedSearchString + ) + expect(maybeGetLatestReadableSearch(expectedSearchString)).toBeNull() } ) }) + +describe(`${getSearchWithEnforcedSegment.name}`, () => { + it('adds enforced segment appropriately, and running the updated value through the function again returns the same value', () => { + const segment = { id: 100, name: 'Eastern Europe' } + const search = '?auth=foo&embed=true' + const expectedUpdatedSearch = + '?f=is,segment,100&l=segment-100,Eastern%20Europe&auth=foo&embed=true' + expect(getSearchWithEnforcedSegment(search, segment)).toEqual( + expectedUpdatedSearch + ) + expect( + getSearchWithEnforcedSegment(expectedUpdatedSearch, segment) + ).toEqual(expectedUpdatedSearch) + }) +}) diff --git a/assets/js/dashboard/util/url-search-params.ts b/assets/js/dashboard/util/url-search-params.ts index be8ce5d9a0df..ab9bbfa996a0 100644 --- a/assets/js/dashboard/util/url-search-params.ts +++ b/assets/js/dashboard/util/url-search-params.ts @@ -1,3 +1,7 @@ +import { + getSearchToSetSegmentFilter, + SavedSegment +} from '../filtering/segments' import { Filter, FilterClauseLabels } from '../query' import { v1 } from './url-search-params-v1' import { v2 } from './url-search-params-v2' @@ -230,8 +234,10 @@ function isAlreadyRedirected(searchParams: URLSearchParams) { The purpose of this function is to redirect users from one of the previous versions to the current version, so previous dashboard links still work. */ -export function getRedirectTarget(windowLocation: Location): null | string { - const searchParams = new URLSearchParams(windowLocation.search) +export function maybeGetLatestReadableSearch( + searchString: string +): null | string { + const searchParams = new URLSearchParams(searchString) if (isAlreadyRedirected(searchParams)) { return null } @@ -244,24 +250,64 @@ export function getRedirectTarget(windowLocation: Location): null | string { const isV1 = v1.isV1(searchParams) if (isV2) { - return `${windowLocation.pathname}${stringifySearch({ ...v2.parseSearch(windowLocation.search), [REDIRECTED_SEARCH_PARAM_NAME]: 'v2' })}` + return stringifySearch({ + ...v2.parseSearch(searchString), + [REDIRECTED_SEARCH_PARAM_NAME]: 'v2' + }) } if (isV1) { - return `${windowLocation.pathname}${stringifySearch({ ...v1.parseSearch(windowLocation.search), [REDIRECTED_SEARCH_PARAM_NAME]: 'v1' })}` + return stringifySearch({ + ...v1.parseSearch(searchString), + [REDIRECTED_SEARCH_PARAM_NAME]: 'v1' + }) } return null } +/** + * It's possible to set a particular segment to be always applied on the data on dashboards accessed with a shared link. + * This function ensures that the particular segment filter is set to the URL string on initial page load. + * Other functions ensure that it can't be removed. + */ +export function getSearchWithEnforcedSegment( + searchString: string, + enforcedSegment: Pick +): string { + const searchRecord = parseSearch(searchString) + return stringifySearch( + getSearchToSetSegmentFilter(enforcedSegment)(searchRecord) + ) +} + /** Called once before React app mounts. If legacy url search params are present, does a redirect to new format. */ -export function redirectForLegacyParams( +export function maybeDoFERedirect( windowLocation: Location, - windowHistory: History + windowHistory: History, + enforcedSegment: Pick | null ) { - const redirectTargetURL = getRedirectTarget(windowLocation) - if (redirectTargetURL === null) { + const originalSearchString = windowLocation.search + + let updatedSearchString = maybeGetLatestReadableSearch(originalSearchString) + + if (enforcedSegment) { + updatedSearchString = getSearchWithEnforcedSegment( + updatedSearchString ?? originalSearchString, + enforcedSegment + ) + } + + if ( + updatedSearchString === null || + updatedSearchString === originalSearchString + ) { return } - windowHistory.pushState({}, '', redirectTargetURL) + + windowHistory.pushState( + {}, + '', + `${windowLocation.pathname}${updatedSearchString}` + ) } diff --git a/assets/test-utils/app-context-providers.tsx b/assets/test-utils/app-context-providers.tsx index 17f4bf4b3bfd..d5e059a11282 100644 --- a/assets/test-utils/app-context-providers.tsx +++ b/assets/test-utils/app-context-providers.tsx @@ -12,7 +12,7 @@ import QueryContextProvider from '../js/dashboard/query-context' import { getRouterBasepath } from '../js/dashboard/router' import { RoutelessModalsContextProvider } from '../js/dashboard/navigation/routeless-modals-context' import { SegmentsContextProvider } from '../js/dashboard/filtering/segments-context' -import { SavedSegments } from '../js/dashboard/filtering/segments' +import { SavedSegment, SavedSegments } from '../js/dashboard/filtering/segments' type TestContextProvidersProps = { children: ReactNode @@ -20,6 +20,7 @@ type TestContextProvidersProps = { siteOptions?: Partial user?: UserContextValue preloaded?: { segments?: SavedSegments } + limitedToSegment?: SavedSegment | null } export const DEFAULT_SITE: PlausibleSite = { @@ -51,6 +52,7 @@ export const TestContextProviders = ({ routerProps, siteOptions, preloaded, + limitedToSegment, user }: TestContextProvidersProps) => { const site = { ...DEFAULT_SITE, ...siteOptions } @@ -78,7 +80,10 @@ export const TestContextProviders = ({ } } > - + cast(attrs, [:slug, :password, :name]) + |> cast(attrs, [:slug, :password, :name, :segment_id]) |> validate_required([:slug, :name]) |> validate_special_name(opts) |> unique_constraint(:slug) |> unique_constraint(:name, name: :shared_links_site_id_name_index) + |> foreign_key_constraint(:segment_id) |> hash_password() end @@ -48,4 +50,9 @@ defmodule Plausible.Site.SharedLink do def password_protected?(%__MODULE__{password_hash: hash}) when not is_nil(hash), do: true def password_protected?(%__MODULE__{}), do: false + + def limited_to_segment?(%__MODULE__{segment_id: segment_id}) when is_integer(segment_id), + do: true + + def limited_to_segment?(%__MODULE__{}), do: false end diff --git a/lib/plausible/sites.ex b/lib/plausible/sites.ex index f0f04d1f5d9e..98b0ffb709f1 100644 --- a/lib/plausible/sites.ex +++ b/lib/plausible/sites.ex @@ -415,6 +415,8 @@ defmodule Plausible.Sites do def create_shared_link(site, name, opts \\ []) do password = Keyword.get(opts, :password) + segment_id = Keyword.get(opts, :segment_id) + site = Plausible.Repo.preload(site, :team) skip_feature_check? = Keyword.get(opts, :skip_feature_check?, false) @@ -423,7 +425,7 @@ defmodule Plausible.Sites do else %SharedLink{site_id: site.id, slug: Nanoid.generate()} |> SharedLink.changeset( - %{name: name, password: password}, + %{name: name, password: password, segment_id: segment_id}, Keyword.take(opts, [:skip_special_name_check?]) ) |> Repo.insert() diff --git a/lib/plausible_web/components/generic.ex b/lib/plausible_web/components/generic.ex index a1aac36cdffa..696195bdcaf6 100644 --- a/lib/plausible_web/components/generic.ex +++ b/lib/plausible_web/components/generic.ex @@ -393,7 +393,7 @@ defmodule PlausibleWeb.Components.Generic do ~H""" <.link class={[ - "inline-flex items-center gap-x-0.5", + "inline-flex items-center gap-x-1", @class ]} href={@href} @@ -403,7 +403,7 @@ defmodule PlausibleWeb.Components.Generic do {@rest} > {render_slot(@inner_block)} - + """ else @@ -444,6 +444,18 @@ defmodule PlausibleWeb.Components.Generic do attr(:rest, :global) + @doc """ + Renders toggle input. + Needs `:js_active_var` that controls toggle state. + Set this outside this component with `x-data="{ : }"`. + + ### Examples + ``` +
+
+ ``` + """ def toggle_switch(assigns) do ~H"""