diff --git a/assets/js/dashboard.tsx b/assets/js/dashboard.tsx index 227ba57c924b..ed227c6f8c58 100644 --- a/assets/js/dashboard.tsx +++ b/assets/js/dashboard.tsx @@ -57,8 +57,15 @@ if (container && container.dataset) { diff --git a/assets/js/dashboard/api.js b/assets/js/dashboard/api.js deleted file mode 100644 index bb1749ddfaff..000000000000 --- a/assets/js/dashboard/api.js +++ /dev/null @@ -1,76 +0,0 @@ -import { formatISO } from './util/date' -import { serializeApiFilters } from './util/filters' - -let abortController = new AbortController() -let SHARED_LINK_AUTH = null - -class ApiError extends Error { - constructor(message, payload) { - super(message) - this.name = "ApiError" - this.payload = payload - } -} - -function serialize(obj) { - var str = []; - for (var p in obj) - /* eslint-disable-next-line no-prototype-builtins */ - if (obj.hasOwnProperty(p)) { - str.push(encodeURIComponent(p) + "=" + encodeURIComponent(obj[p])); - } - return str.join("&"); -} - -export function setSharedLinkAuth(auth) { - SHARED_LINK_AUTH = auth -} - -export function cancelAll() { - abortController.abort() - abortController = new AbortController() -} - -export function serializeQuery(query, extraQuery = []) { - const queryObj = {} - if (query.period) { queryObj.period = query.period } - if (query.date) { queryObj.date = formatISO(query.date) } - if (query.from) { queryObj.from = formatISO(query.from) } - if (query.to) { queryObj.to = formatISO(query.to) } - if (query.filters) { queryObj.filters = serializeApiFilters(query.filters) } - if (query.with_imported) { queryObj.with_imported = query.with_imported } - if (SHARED_LINK_AUTH) { queryObj.auth = SHARED_LINK_AUTH } - - if (query.comparison) { - queryObj.comparison = query.comparison - queryObj.compare_from = query.compare_from ? formatISO(query.compare_from) : undefined - queryObj.compare_to = query.compare_to ? formatISO(query.compare_to) : undefined - queryObj.match_day_of_week = query.match_day_of_week - } - - Object.assign(queryObj, ...extraQuery) - - return '?' + serialize(queryObj) -} - -export function get(url, query = {}, ...extraQuery) { - const headers = SHARED_LINK_AUTH ? { 'X-Shared-Link-Auth': SHARED_LINK_AUTH } : {} - url = url + serializeQuery(query, extraQuery) - return fetch(url, { signal: abortController.signal, headers: headers }) - .then(response => { - if (!response.ok) { - return response.json().then((payload) => { - throw new ApiError(payload.error, payload) - }) - } - return response.json() - }) -} - -export function put(url, body) { - return fetch(url, { - method: "PUT", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify(body) - }) -} diff --git a/assets/js/dashboard/api.ts b/assets/js/dashboard/api.ts new file mode 100644 index 000000000000..1bc8e50ec144 --- /dev/null +++ b/assets/js/dashboard/api.ts @@ -0,0 +1,134 @@ +/** @format */ +import { DashboardQuery } from './query' +import { formatISO } from './util/date' +import { serializeApiFilters } from './util/filters' + +let abortController = new AbortController() +let SHARED_LINK_AUTH: null | string = null + +export class ApiError extends Error { + payload: unknown + constructor(message: string, payload: unknown) { + super(message) + this.name = 'ApiError' + this.payload = payload + } +} + +function serialize(obj: Record) { + const str: string[] = [] + /* eslint-disable-next-line no-prototype-builtins */ + for (const p in obj) + if (obj.hasOwnProperty(p)) { + str.push(`${encodeURIComponent(p)}=${encodeURIComponent(obj[p])}`) + } + return str.join('&') +} + +export function setSharedLinkAuth(auth: string) { + SHARED_LINK_AUTH = auth +} + +export function cancelAll() { + abortController.abort() + abortController = new AbortController() +} + +export function serializeQuery( + query: DashboardQuery, + extraQuery: unknown[] = [] +) { + const queryObj: Record = {} + if (query.period) { + queryObj.period = query.period + } + if (query.date) { + queryObj.date = formatISO(query.date) + } + if (query.from) { + queryObj.from = formatISO(query.from) + } + if (query.to) { + queryObj.to = formatISO(query.to) + } + if (query.filters) { + queryObj.filters = serializeApiFilters(query.filters) + } + if (query.with_imported) { + queryObj.with_imported = String(query.with_imported) + } + if (SHARED_LINK_AUTH) { + queryObj.auth = SHARED_LINK_AUTH + } + + if (query.comparison) { + queryObj.comparison = query.comparison + queryObj.compare_from = query.compare_from + ? formatISO(query.compare_from) + : undefined + queryObj.compare_to = query.compare_to + ? formatISO(query.compare_to) + : undefined + queryObj.match_day_of_week = String(query.match_day_of_week) + } + + Object.assign(queryObj, ...extraQuery) + + return '?' + serialize(queryObj) +} + +function getHeaders(): Record { + return SHARED_LINK_AUTH ? { 'X-Shared-Link-Auth': SHARED_LINK_AUTH } : {} +} + +async function handleApiResponse(response: Response) { + const payload = await response.json() + if (!response.ok) { + throw new ApiError(payload.error, payload) + } + + return payload +} + +export async function get( + url: string, + query?: DashboardQuery, + ...extraQuery: unknown[] +) { + const response = await fetch( + query ? url + serializeQuery(query, extraQuery) : url, + { + signal: abortController.signal, + headers: { ...getHeaders(), Accept: 'application/json' } + } + ) + return handleApiResponse(response) +} + +export const mutation = async < + TBody extends Record = Record +>( + url: string, + options: + | { body: TBody; method: 'PATCH' | 'PUT' | 'POST' } + | { method: 'DELETE' } +) => { + const fetchOptions = + options.method === 'DELETE' + ? {} + : { + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(options.body) + } + const response = await fetch(url, { + method: options.method, + headers: { + ...getHeaders(), + ...fetchOptions.headers, + Accept: 'application/json' + }, + body: fetchOptions.body, + signal: abortController.signal + }) + return handleApiResponse(response) +} diff --git a/assets/js/dashboard/components/error-panel.tsx b/assets/js/dashboard/components/error-panel.tsx new file mode 100644 index 000000000000..39832ae11569 --- /dev/null +++ b/assets/js/dashboard/components/error-panel.tsx @@ -0,0 +1,51 @@ +/** @format */ + +import React from 'react' +import classNames from 'classnames' +import { + ArrowPathIcon, + ExclamationTriangleIcon, + XMarkIcon +} from '@heroicons/react/24/outline' + +export const ErrorPanel = ({ + errorMessage, + className, + onClose, + onRetry +}: { + errorMessage: string + className?: string + onClose?: () => void + onRetry?: () => void +}) => ( +
+
+ +
+
{errorMessage}
+ {typeof onClose === 'function' && ( + + )} + {typeof onRetry === 'function' && ( + + )} +
+) diff --git a/assets/js/dashboard/components/notice.js b/assets/js/dashboard/components/notice.js index 1f129224269f..9a90a16b9006 100644 --- a/assets/js/dashboard/components/notice.js +++ b/assets/js/dashboard/components/notice.js @@ -9,10 +9,10 @@ export function FeatureSetupNotice({ feature, title, info, callToAction, onHideA const requestHideSection = () => { if (window.confirm(`Are you sure you want to hide ${sectionTitle}? You can make it visible again in your site settings later.`)) { - api.put(`/api/${encodeURIComponent(site.domain)}/disable-feature`, { feature: feature }) - .then(response => { - if (response.ok) { onHideAction() } - }) + api.mutation(`/api/${encodeURIComponent(site.domain)}/disable-feature`, { method: 'PUT', body: { feature: feature } }) + .then(() => onHideAction()) + .catch((error) => {if (!(error instanceof api.ApiError)) {throw error}}) + } } diff --git a/assets/js/dashboard/components/popover.tsx b/assets/js/dashboard/components/popover.tsx index 81a41811c6f4..5ceac56d88f3 100644 --- a/assets/js/dashboard/components/popover.tsx +++ b/assets/js/dashboard/components/popover.tsx @@ -55,6 +55,10 @@ const items = { roundedStartEnd: classNames( 'first-of-type:rounded-t-md', 'last-of-type:rounded-b-md' + ), + groupRoundedStartEnd: classNames( + 'group-first-of-type:rounded-t-md', + 'group-last-of-type:rounded-b-md' ) } } diff --git a/assets/js/dashboard/components/search-input.tsx b/assets/js/dashboard/components/search-input.tsx index 1c7b6e8fbced..245dd8533eab 100644 --- a/assets/js/dashboard/components/search-input.tsx +++ b/assets/js/dashboard/components/search-input.tsx @@ -59,7 +59,7 @@ export const SearchInput = ({ type="text" placeholder={isFocused ? placeholderFocused : placeholderUnfocused} className={classNames( - 'shadow-sm dark:bg-gray-900 dark:text-gray-100 focus:ring-indigo-500 focus:border-indigo-500 block sm:text-sm border-gray-300 dark:border-gray-500 rounded-md dark:bg-gray-800 w-48', + 'shadow-sm dark:bg-gray-900 dark:text-gray-100 focus:ring-indigo-500 focus:border-indigo-500 block border-gray-300 dark:border-gray-500 rounded-md dark:bg-gray-800 w-48', className )} onChange={debouncedOnSearchInputChange} diff --git a/assets/js/dashboard/dashboard-keybinds.tsx b/assets/js/dashboard/dashboard-keybinds.tsx index f40b244df05f..b4d3ceb46801 100644 --- a/assets/js/dashboard/dashboard-keybinds.tsx +++ b/assets/js/dashboard/dashboard-keybinds.tsx @@ -1,6 +1,7 @@ /* @format */ import React from 'react' import { NavigateKeybind } from './keybinding' +import { useRoutelessModalsContext } from './navigation/routeless-modals-context' const ClearFiltersKeybind = () => ( ( ) export function DashboardKeybinds() { - return ( - <> - - - ) + const { modal } = useRoutelessModalsContext() + return modal === null && } diff --git a/assets/js/dashboard/filtering/segments.test.ts b/assets/js/dashboard/filtering/segments.test.ts new file mode 100644 index 000000000000..c45ee406f79d --- /dev/null +++ b/assets/js/dashboard/filtering/segments.test.ts @@ -0,0 +1,111 @@ +/** @format */ + +import { remapToApiFilters } from '../util/filters' +import { + formatSegmentIdAsLabelKey, + getFilterSegmentsByNameInsensitive, + getSearchToApplySingleSegmentFilter, + getSegmentNamePlaceholder, + isSegmentIdLabelKey, + parseApiSegmentData +} from './segments' + +describe(`${getFilterSegmentsByNameInsensitive.name}`, () => { + const unfilteredSegments = [ + { name: 'APAC Region' }, + { name: 'EMEA Region' }, + { name: 'Scandinavia' } + ] + it('generates insensitive filter function', () => { + expect( + unfilteredSegments.filter(getFilterSegmentsByNameInsensitive('region')) + ).toEqual([{ name: 'APAC Region' }, { name: 'EMEA Region' }]) + }) + + it('ignores preceding and following whitespace', () => { + expect( + unfilteredSegments.filter(getFilterSegmentsByNameInsensitive(' scandi ')) + ).toEqual([{ name: 'Scandinavia' }]) + }) + + it.each([[undefined], [''], [' '], ['\n\n']])( + 'generates always matching filter for search value %p', + (searchValue) => { + expect( + unfilteredSegments.filter( + getFilterSegmentsByNameInsensitive(searchValue) + ) + ).toEqual(unfilteredSegments) + } + ) +}) + +describe(`${getSegmentNamePlaceholder.name}`, () => { + it('gives readable result', () => { + const placeholder = getSegmentNamePlaceholder({ + labels: { US: 'United States' }, + filters: [ + ['is', 'country', ['US']], + ['contains', 'page', ['/blog', `${new Array(250).fill('c').join('')}`]] + ] + }) + + expect(placeholder).toEqual( + 'Country is United States and Page contains /blog or ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc' + ) + expect(placeholder).toHaveLength(255) + }) +}) + +describe('segment labels in URL search params', () => { + test(`${formatSegmentIdAsLabelKey.name} and ${isSegmentIdLabelKey.name} work`, () => { + const formatted = formatSegmentIdAsLabelKey(5) + expect(formatted).toEqual('segment-5') + expect(isSegmentIdLabelKey(formatted)).toEqual(true) + }) +}) + +describe(`${parseApiSegmentData.name}`, () => { + it('correctly formats values stored as API filters in their dashboard format', () => { + const apiFormatFilters = [ + ['is', 'visit:country', ['PL']], + ['is', 'event:props:logged_in', ['true']], + ['has_not_done', ['is', 'event:goal', ['Signup']]] + ] + const dashboardFormat = parseApiSegmentData({ + filters: apiFormatFilters, + labels: { PL: 'Poland' } + }) + expect(dashboardFormat).toEqual({ + filters: [ + ['is', 'country', ['PL']], + ['is', 'props:logged_in', ['true']], + ['has_not_done', 'goal', ['Signup']] + ], + labels: { PL: 'Poland' } + }) + expect(remapToApiFilters(dashboardFormat.filters)).toEqual(apiFormatFilters) + }) +}) + +describe(`${getSearchToApplySingleSegmentFilter.name}`, () => { + test('generated search function applies single segment correctly', () => { + const searchFunction = getSearchToApplySingleSegmentFilter({ + name: 'APAC', + id: 500 + }) + 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' } + }) + }) +}) diff --git a/assets/js/dashboard/filtering/segments.ts b/assets/js/dashboard/filtering/segments.ts new file mode 100644 index 000000000000..f480cbbd8313 --- /dev/null +++ b/assets/js/dashboard/filtering/segments.ts @@ -0,0 +1,103 @@ +/** @format */ + +import { DashboardQuery, Filter } from '../query' +import { cleanLabels, remapFromApiFilters } from '../util/filters' +import { plainFilterText } from '../util/filter-text' +import { AppNavigationTarget } from '../navigation/use-app-navigate' + +export enum SegmentType { + personal = 'personal', + site = 'site' +} + +export type SavedSegment = { + id: number + name: string + type: SegmentType + owner_id: number + inserted_at: string + updated_at: string +} + +export type SegmentDataFromApi = { + filters: unknown[] + labels: Record +} + +/** In this type, filters are parsed to dashboard format */ +export type SegmentData = { + filters: Filter[] + labels: Record +} + +const SEGMENT_LABEL_KEY_PREFIX = 'segment-' + +export function handleSegmentResponse( + segment: SavedSegment & { + segment_data: SegmentDataFromApi + } +): SavedSegment & { segment_data: SegmentData } { + return { + ...segment, + segment_data: parseApiSegmentData(segment.segment_data) + } +} + +export function getFilterSegmentsByNameInsensitive( + search?: string +): (s: Pick) => boolean { + return (s) => + search?.trim().length + ? s.name.toLowerCase().includes(search.trim().toLowerCase()) + : true +} + +export const getSegmentNamePlaceholder = ( + query: Pick +) => + query.filters + .reduce( + (combinedName, filter) => + combinedName.length > 100 + ? combinedName + : `${combinedName}${combinedName.length ? ' and ' : ''}${plainFilterText(query, filter)}`, + '' + ) + .slice(0, 255) + +export function isSegmentIdLabelKey(labelKey: string): boolean { + return labelKey.startsWith(SEGMENT_LABEL_KEY_PREFIX) +} + +export function formatSegmentIdAsLabelKey(id: number): string { + return `${SEGMENT_LABEL_KEY_PREFIX}${id}` +} + +export const isSegmentFilter = (f: Filter): boolean => f[1] === 'segment' + +export const parseApiSegmentData = ({ + filters, + ...rest +}: { + filters: unknown[] + labels: Record +}): SegmentData => ({ + filters: remapFromApiFilters(filters), + ...rest +}) + +export function getSearchToApplySingleSegmentFilter( + segment: Pick +): Required['search'] { + return (search) => { + const filters = [['is', 'segment', [segment.id]]] + const labels = cleanLabels(filters, {}, 'segment', { + [formatSegmentIdAsLabelKey(segment.id)]: segment.name + }) + return { + ...search, + filters, + labels + } + } +} diff --git a/assets/js/dashboard/filters.js b/assets/js/dashboard/filters.js index 31d5d56edd4e..6b4ef30ddd3d 100644 --- a/assets/js/dashboard/filters.js +++ b/assets/js/dashboard/filters.js @@ -10,9 +10,9 @@ import { Menu, Transition } from '@headlessui/react'; import { FILTER_GROUP_TO_MODAL_TYPE, cleanLabels, - FILTER_MODAL_TO_FILTER_GROUP, formatFilterGroup, - EVENT_PROPS_PREFIX + EVENT_PROPS_PREFIX, + getAvailableFilterModals } from "./util/filters" import { plainFilterText, styledFilterText } from "./util/filter-text" @@ -101,10 +101,7 @@ function DropdownContent({ wrapped }) { const [addingFilter, setAddingFilter] = useState(false); if (wrapped === WRAPSTATE.unwrapped || addingFilter) { - let filterModals = { ...FILTER_MODAL_TO_FILTER_GROUP } - if (!site.propsAvailable) delete filterModals.props - - return <>{Object.keys(filterModals).map((option) => )} + return <>{Object.keys(getAvailableFilterModals(site)).map((option) => )} } return ( diff --git a/assets/js/dashboard/hooks/api-client.ts b/assets/js/dashboard/hooks/api-client.ts index b4472fa5cad9..628d43a19f2a 100644 --- a/assets/js/dashboard/hooks/api-client.ts +++ b/assets/js/dashboard/hooks/api-client.ts @@ -18,7 +18,7 @@ type PaginatedQueryKeyBase = [Endpoint, { query: DashboardQuery }] type GetRequestParams = ( k: TKey -) => [Record, Record] +) => [DashboardQuery, Record] /** * Hook that fetches the first page from the defined GET endpoint on mount, diff --git a/assets/js/dashboard/nav-menu/filter-menu.tsx b/assets/js/dashboard/nav-menu/filter-menu.tsx index 6b88f6605a89..f3fd7455e12f 100644 --- a/assets/js/dashboard/nav-menu/filter-menu.tsx +++ b/assets/js/dashboard/nav-menu/filter-menu.tsx @@ -13,6 +13,7 @@ import { popover } from '../components/popover' import classNames from 'classnames' import { AppNavigationLink } from '../navigation/use-app-navigate' import { BlurMenuButtonOnEscape } from '../keybinding' +import { SearchableSegmentsSection } from './segments/searchable-segments-section' export function getFilterListItems({ propsAvailable @@ -50,6 +51,7 @@ const FilterMenuItems = ({ closeDropdown }: { closeDropdown: () => void }) => { const site = useSiteContext() const columns = useMemo(() => getFilterListItems(site), [site]) const buttonRef = useRef(null) + return ( <> @@ -57,13 +59,13 @@ const FilterMenuItems = ({ closeDropdown }: { closeDropdown: () => void }) => { ref={buttonRef} className={classNames( popover.toggleButton.classNames.rounded, - popover.toggleButton.classNames.ghost, + popover.toggleButton.classNames.shadow, 'justify-center gap-1 px-3' )} > - Add filter + Filter void }) => { )} > - {columns.map((filterGroups, index) => ( -
- {filterGroups.map(({ title, modals }) => ( -
-
- {title} +
+ {columns.map((filterGroups, index) => ( +
+ {filterGroups.map(({ title, modals }) => ( +
+
{title}
+ {modals + .filter((m) => !!m) + .map((modalKey) => ( + closeDropdown()} + key={modalKey} + path={filterRoute.path} + params={{ field: modalKey }} + search={(s) => s} + > + {formatFilterGroup(modalKey)} + + ))}
- {modals - .filter((m) => !!m) - .map((modalKey) => ( - closeDropdown()} - key={modalKey} - path={filterRoute.path} - params={{ field: modalKey }} - search={(s) => s} - > - {formatFilterGroup(modalKey)} - - ))} -
- ))} -
- ))} + ))} +
+ ))} +
+ @@ -117,3 +119,6 @@ export const FilterMenu = () => ( {({ close }) => } ) + +const titleClassName = + 'text-sm pb-1 px-4 pt-2 font-bold uppercase text-indigo-500 dark:text-indigo-400' diff --git a/assets/js/dashboard/nav-menu/filters-bar.test.tsx b/assets/js/dashboard/nav-menu/filters-bar.test.tsx index f9bae645f71b..f71097228b2e 100644 --- a/assets/js/dashboard/nav-menu/filters-bar.test.tsx +++ b/assets/js/dashboard/nav-menu/filters-bar.test.tsx @@ -115,7 +115,8 @@ describe(`${handleVisibility.name}`, () => { leftoverWidth: 1000, seeMoreWidth: 100, pillWidths: [200, 200, 200, 200], - pillGap: 25 + pillGap: 25, + mustShowSeeMoreMenu: true } handleVisibility(input) expect(setVisibility).toHaveBeenCalledTimes(1) @@ -148,7 +149,8 @@ describe(`${handleVisibility.name}`, () => { leftoverWidth: 300, seeMoreWidth: 50, pillWidths: [250], - pillGap: 25 + pillGap: 25, + mustShowSeeMoreMenu: false } handleVisibility(input) expect(setVisibility).toHaveBeenCalledTimes(1) @@ -165,7 +167,8 @@ describe(`${handleVisibility.name}`, () => { leftoverWidth: 300, seeMoreWidth: 50, pillWidths: [250, 200], - pillGap: 25 + pillGap: 25, + mustShowSeeMoreMenu: true } handleVisibility(input) expect(setVisibility).toHaveBeenCalledTimes(1) diff --git a/assets/js/dashboard/nav-menu/filters-bar.tsx b/assets/js/dashboard/nav-menu/filters-bar.tsx index 27155605af04..b5ca5b0140df 100644 --- a/assets/js/dashboard/nav-menu/filters-bar.tsx +++ b/assets/js/dashboard/nav-menu/filters-bar.tsx @@ -9,6 +9,9 @@ import { AppNavigationLink } from '../navigation/use-app-navigate' import { Popover, Transition } from '@headlessui/react' import { popover } from '../components/popover' import { BlurMenuButtonOnEscape } from '../keybinding' +import { isSegmentFilter } from '../filtering/segments' +import { useRoutelessModalsContext } from '../navigation/routeless-modals-context' +import { DashboardQuery } from '../query' // Component structure is // `..[ filter (x) ]..[ filter (x) ]..[ three dot menu ]..` @@ -25,13 +28,15 @@ export const handleVisibility = ({ leftoverWidth, seeMoreWidth, pillWidths, - pillGap + pillGap, + mustShowSeeMoreMenu }: { setVisibility: (v: VisibilityState) => void leftoverWidth: number | null pillWidths: (number | null)[] | null seeMoreWidth: number pillGap: number + mustShowSeeMoreMenu: boolean }): void => { if (leftoverWidth === null || pillWidths === null) { return @@ -56,7 +61,7 @@ export const handleVisibility = ({ const fits = fitToWidth(leftoverWidth) const seeMoreWillBePresent = - fits.visibleCount < pillWidths.length || pillWidths.length > 1 + fits.visibleCount < pillWidths.length || mustShowSeeMoreMenu // Check if the appearance of "See more" would cause overflow if (seeMoreWillBePresent) { @@ -104,13 +109,30 @@ interface FiltersBarProps { } } +const canShowClearAllAction = ({ filters }: Pick) => + filters.length >= 2 + +const canShowSaveAsSegmentAction = ({ + filters, + isEditingSegment +}: Pick & { isEditingSegment: boolean }) => + filters.length >= 1 && !filters.some(isSegmentFilter) && !isEditingSegment + export const FiltersBar = ({ accessors }: FiltersBarProps) => { const containerRef = useRef(null) const pillsRef = useRef(null) const [visibility, setVisibility] = useState(null) - const { query } = useQueryContext() + const { query, expandedSegment } = useQueryContext() const seeMoreRef = useRef(null) + const showingClearAll = canShowClearAllAction({ filters: query.filters }) + const showingSaveAsSegment = canShowSaveAsSegmentAction({ + filters: query.filters, + isEditingSegment: !!expandedSegment + }) + + const mustShowSeeMoreMenu = showingClearAll || showingSaveAsSegment + useLayoutEffect(() => { const topBar = accessors.topBar(containerRef.current) const leftSection = accessors.leftSection(containerRef.current) @@ -135,7 +157,10 @@ export const FiltersBar = ({ accessors }: FiltersBarProps) => { BUFFER_RIGHT_PX : null, seeMoreWidth: - SEE_MORE_LEFT_MARGIN_PX + SEE_MORE_WIDTH_PX + SEE_MORE_RIGHT_MARGIN_PX + SEE_MORE_LEFT_MARGIN_PX + + SEE_MORE_WIDTH_PX + + SEE_MORE_RIGHT_MARGIN_PX, + mustShowSeeMoreMenu }) }) @@ -146,15 +171,13 @@ export const FiltersBar = ({ accessors }: FiltersBarProps) => { return () => { resizeObserver.disconnect() } - }, [accessors, query.filters]) + }, [accessors, query.filters, mustShowSeeMoreMenu]) if (!query.filters.length) { // functions as spacer between elements.leftSection and elements.rightSection return
} - const canClear = query.filters.length > 1 - return (
{ />
{visibility !== null && - (query.filters.length !== visibility.visibleCount || canClear) && ( + (query.filters.length !== visibility.visibleCount || + mustShowSeeMoreMenu) && ( { }} /> )} - {canClear && } + {showingClearAll && } + {showingSaveAsSegment && } @@ -232,9 +257,8 @@ export const FiltersBar = ({ accessors }: FiltersBarProps) => { const ClearAction = () => ( ({ ...search, @@ -245,3 +269,20 @@ const ClearAction = () => ( Clear all filters ) + +const SaveAsSegmentAction = () => { + const { setModal } = useRoutelessModalsContext() + + return ( + s} + onClick={() => setModal('create')} + state={{ expandedSegment: null }} + > + Save as segment + + ) +} diff --git a/assets/js/dashboard/nav-menu/nav-menu-components.tsx b/assets/js/dashboard/nav-menu/nav-menu-components.tsx new file mode 100644 index 000000000000..71607ab14706 --- /dev/null +++ b/assets/js/dashboard/nav-menu/nav-menu-components.tsx @@ -0,0 +1,7 @@ +/** @format */ + +import React from 'react' + +export const MenuSeparator = () => ( +
+) diff --git a/assets/js/dashboard/nav-menu/query-periods/comparison-period-menu.tsx b/assets/js/dashboard/nav-menu/query-periods/comparison-period-menu.tsx index 1cdb7143b2f8..875a8a365df0 100644 --- a/assets/js/dashboard/nav-menu/query-periods/comparison-period-menu.tsx +++ b/assets/js/dashboard/nav-menu/query-periods/comparison-period-menu.tsx @@ -26,12 +26,12 @@ import { DateMenuChevron, PopoverMenuProps, linkClassName, - MenuSeparator, CalendarPanel, hiddenCalendarButtonClassName } from './shared-menu-items' import { DateRangeCalendar } from './date-range-calendar' import { formatISO, nowForSite } from '../../util/date' +import { MenuSeparator } from '../nav-menu-components' export const ComparisonPeriodMenuItems = ({ closeDropdown, diff --git a/assets/js/dashboard/nav-menu/query-periods/move-period-arrows.tsx b/assets/js/dashboard/nav-menu/query-periods/move-period-arrows.tsx index 3fda3fe0138c..7df76136994d 100644 --- a/assets/js/dashboard/nav-menu/query-periods/move-period-arrows.tsx +++ b/assets/js/dashboard/nav-menu/query-periods/move-period-arrows.tsx @@ -82,14 +82,14 @@ export function MovePeriodArrows({ className }: { className?: string }) { return (
void }) => { const site = useSiteContext() - const { query } = useQueryContext() + const { query, expandedSegment } = useQueryContext() const groups = useMemo(() => { const compareLink = getCompareLinkItem({ site, query }) @@ -138,11 +138,14 @@ const QueryPeriodMenuInner = ({ } ] ], - extraGroups: COMPARISON_DISABLED_PERIODS.includes(query.period) + extraGroups: isComparisonForbidden({ + period: query.period, + segmentIsExpanded: !!expandedSegment + }) ? [] : [[compareLink]] }) - }, [site, query, closeDropdown, toggleCalendar]) + }, [site, query, closeDropdown, toggleCalendar, expandedSegment]) return ( <> diff --git a/assets/js/dashboard/nav-menu/query-periods/query-periods-picker.tsx b/assets/js/dashboard/nav-menu/query-periods/query-periods-picker.tsx index 12f69b9912dc..684f4a12f528 100644 --- a/assets/js/dashboard/nav-menu/query-periods/query-periods-picker.tsx +++ b/assets/js/dashboard/nav-menu/query-periods/query-periods-picker.tsx @@ -40,7 +40,7 @@ export function QueryPeriodsPicker({ className }: { className?: string }) { {isComparing && ( <>
- vs. + vs.
{({ close }) => ( diff --git a/assets/js/dashboard/nav-menu/query-periods/shared-menu-items.tsx b/assets/js/dashboard/nav-menu/query-periods/shared-menu-items.tsx index f1b14e1e9477..a7844091d6c8 100644 --- a/assets/js/dashboard/nav-menu/query-periods/shared-menu-items.tsx +++ b/assets/js/dashboard/nav-menu/query-periods/shared-menu-items.tsx @@ -25,29 +25,11 @@ export const DateMenuChevron = () => ( ) -export const MenuSeparator = () => ( -
-) - export interface PopoverMenuProps { closeDropdown: () => void calendarButtonRef: RefObject } -export enum DropdownState { - CLOSED = 'CLOSED', - MENU = 'MENU', - CALENDAR = 'CALENDAR' -} - -export interface DropdownWithCalendarState { - closeDropdown: () => void - toggleDropdown: (mode: 'menu' | 'calendar') => void - dropdownState: DropdownState - buttonRef: RefObject - toggleCalendar: () => void -} - const calendarPositionClassName = '*:!top-auto *:!right-0 *:!absolute' type CalendarPanelProps = { diff --git a/assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx b/assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx new file mode 100644 index 000000000000..6293ebde53d5 --- /dev/null +++ b/assets/js/dashboard/nav-menu/segments/searchable-segments-section.tsx @@ -0,0 +1,271 @@ +/** @format */ + +import React, { useCallback, useEffect, useMemo, useState } from 'react' +import { useQueryContext } from '../../query-context' +import { useSiteContext } from '../../site-context' +import { + formatSegmentIdAsLabelKey, + getFilterSegmentsByNameInsensitive, + handleSegmentResponse, + isSegmentFilter, + SavedSegment, + SegmentData, + SegmentDataFromApi +} from '../../filtering/segments' +import { QueryFunction, useQuery, useQueryClient } from '@tanstack/react-query' +import { cleanLabels } from '../../util/filters' +import classNames from 'classnames' +import { Tooltip } from '../../util/tooltip' +import { SegmentAuthorship } from '../../segments/segment-authorship' +import { SearchInput } from '../../components/search-input' +import { EllipsisHorizontalIcon } from '@heroicons/react/24/solid' +import { popover } from '../../components/popover' +import { AppNavigationLink } from '../../navigation/use-app-navigate' +import { MenuSeparator } from '../nav-menu-components' +import { ErrorPanel } from '../../components/error-panel' +import { get } from '../../api' + +const useSegmentsListQuery = () => { + const site = useSiteContext() + return useQuery({ + queryKey: ['segments'], + placeholderData: (previousData) => previousData, + queryFn: async () => { + const response: SavedSegment[] = await get( + `/api/${encodeURIComponent(site.domain)}/segments` + ) + return response + } + }) +} + +const linkClassName = classNames( + popover.items.classNames.navigationLink, + popover.items.classNames.selectedOption, + popover.items.classNames.hoverLink, + popover.items.classNames.groupRoundedStartEnd +) + +const initialSliceLength = 5 + +export const SearchableSegmentsSection = ({ + closeList +}: { + closeList: () => void +}) => { + const { query, expandedSegment } = useQueryContext() + const segmentFilter = query.filters.find(isSegmentFilter) + const appliedSegmentIds = (segmentFilter ? segmentFilter[2] : []) as number[] + + const { data, ...listQuery } = useSegmentsListQuery() + const [searchValue, setSearch] = useState() + const [showAll, setShowAll] = useState(false) + + const searching = !searchValue?.trim().length + + useEffect(() => { + setShowAll(false) + }, [searching]) + + const filteredData = data?.filter( + getFilterSegmentsByNameInsensitive(searchValue) + ) + + const showableSlice = showAll + ? filteredData + : filteredData?.slice(0, initialSliceLength) + + if (expandedSegment) { + return null + } + + return ( + <> + {!!data?.length && ( + <> + +
+
+ Segments +
+ {data.length > initialSliceLength && ( + + )} +
+ + {showableSlice!.map((s) => { + return ( + +
{s.name}
+
+ { + { + personal: 'Personal segment', + site: 'Site segment' + }[s.type] + } +
+ + +
+ } + > + + + ) + })} + {!!filteredData?.length && + !!showableSlice?.length && + filteredData?.length > showableSlice?.length && + showAll === false && ( + + s} + onClick={() => setShowAll(true)} + > + {`Show ${filteredData.length - showableSlice.length} more`} + + + + )} + + )} + {!!data?.length && searchValue && !showableSlice?.length && ( + +
+ No segments found. Clear search to show all. +
+ + )} + + {listQuery.status === 'pending' && ( +
+
+
+
+
+ )} + {listQuery.error && ( +
+ listQuery.refetch()} + /> +
+ )} + + ) +} + +export const useSegmentPrefetch = ({ id }: { id: string }) => { + const site = useSiteContext() + const queryClient = useQueryClient() + const queryKey = useMemo(() => ['segments', id] as const, [id]) + + const getSegmentFn: QueryFunction< + SavedSegment & { segment_data: SegmentData }, + typeof queryKey + > = useCallback( + async ({ queryKey: [_, id] }) => { + const response: SavedSegment & { segment_data: SegmentDataFromApi } = + await get(`/api/${encodeURIComponent(site.domain)}/segments/${id}`) + return handleSegmentResponse(response) + }, + [site] + ) + + const getSegment = useQuery({ + enabled: false, + queryKey: queryKey, + queryFn: getSegmentFn + }) + + const prefetchSegment = useCallback( + () => + queryClient.prefetchQuery({ + queryKey, + queryFn: getSegmentFn, + staleTime: 120_000 + }), + [queryClient, getSegmentFn, queryKey] + ) + + const fetchSegment = useCallback( + () => + queryClient.fetchQuery({ + queryKey, + queryFn: getSegmentFn + }), + [queryClient, getSegmentFn, queryKey] + ) + + return { + prefetchSegment, + data: getSegment.data, + fetchSegment, + error: getSegment.error, + status: getSegment.status + } +} + +const SegmentLink = ({ + id, + name, + appliedSegmentIds, + closeList +}: SavedSegment & { appliedSegmentIds: number[]; closeList: () => void }) => { + const { query } = useQueryContext() + + const { prefetchSegment } = useSegmentPrefetch({ id: String(id) }) + + return ( + { + const otherFilters = query.filters.filter((f) => !isSegmentFilter(f)) + const updatedSegmentIds = appliedSegmentIds.includes(id) ? [] : [id] + if (!updatedSegmentIds.length) { + return { + ...search, + filters: otherFilters, + labels: cleanLabels(otherFilters, query.labels) + } + } + + const updatedFilters = [ + ['is', 'segment', updatedSegmentIds], + ...otherFilters + ] + + return { + ...search, + filters: updatedFilters, + labels: cleanLabels(updatedFilters, query.labels, 'segment', { + [formatSegmentIdAsLabelKey(id)]: name + }) + } + }} + > +
{name}
+ + ) +} diff --git a/assets/js/dashboard/nav-menu/segments/segment-menu.tsx b/assets/js/dashboard/nav-menu/segments/segment-menu.tsx new file mode 100644 index 000000000000..187cc7441afe --- /dev/null +++ b/assets/js/dashboard/nav-menu/segments/segment-menu.tsx @@ -0,0 +1,159 @@ +/** @format */ + +import React, { useEffect } from 'react' +import classNames from 'classnames' +import { Popover, Transition } from '@headlessui/react' +import { popover } from '../../components/popover' +import { + AppNavigationLink, + useAppNavigate +} from '../../navigation/use-app-navigate' +import { + Square2StackIcon, + TrashIcon, + XMarkIcon +} from '@heroicons/react/24/outline' +import { ChevronDownIcon } from '@heroicons/react/20/solid' +import { useQueryContext } from '../../query-context' +import { useRoutelessModalsContext } from '../../navigation/routeless-modals-context' +import { SavedSegment } from '../../filtering/segments' +import { DashboardQuery } from '../../query' + +const linkClassName = classNames( + popover.items.classNames.navigationLink, + popover.items.classNames.selectedOption, + popover.items.classNames.hoverLink, + popover.items.classNames.roundedStartEnd +) +const buttonClassName = classNames( + 'text-white font-medium bg-indigo-600 hover:bg-indigo-700' +) + +export const useClearExpandedSegmentModeOnFilterClear = ({ + expandedSegment, + query +}: { + expandedSegment: SavedSegment | null + query: DashboardQuery +}) => { + const navigate = useAppNavigate() + useEffect(() => { + // clear edit mode on clearing all filters or removing last filter + if (!!expandedSegment && !query.filters.length) { + navigate({ + search: (s) => s, + state: { + expandedSegment: null + }, + replace: true + }) + } + }, [query.filters, expandedSegment, navigate]) +} + +export const SegmentMenu = () => { + const { setModal } = useRoutelessModalsContext() + const { expandedSegment } = useQueryContext() + + if (!expandedSegment) { + return null + } + + return ( +
+ s} + state={{ expandedSegment }} + onClick={() => { + setModal('update') + }} + > + Update segment + + + {({ close: closeDropdown }) => ( + <> + + + + + s} + state={{ expandedSegment }} + onClick={() => { + closeDropdown() + setModal('create') + }} + > +
+ + + Save as a new segment + +
+ + s} + state={{ expandedSegment }} + onClick={() => { + closeDropdown() + setModal('delete') + }} + > +
+ + Delete segment +
+ + ({ + ...s, + filters: [], + labels: {} + })} + state={{ expandedSegment: null }} + onClick={closeDropdown} + > +
+ + + Close without saving + +
+ + + + + )} + +
+ ) +} diff --git a/assets/js/dashboard/nav-menu/top-bar.test.tsx b/assets/js/dashboard/nav-menu/top-bar.test.tsx index 832bcdf1e5a6..89d7559cdb52 100644 --- a/assets/js/dashboard/nav-menu/top-bar.test.tsx +++ b/assets/js/dashboard/nav-menu/top-bar.test.tsx @@ -80,7 +80,7 @@ test('user can open and close filters dropdown', async () => { ) }) - const toggleFilters = screen.getByRole('button', { name: /Add filter/ }) + const toggleFilters = screen.getByRole('button', { name: 'Filter' }) await userEvent.click(toggleFilters) expect(screen.queryAllByRole('link').map((el) => el.textContent)).toEqual([ 'Page', @@ -98,7 +98,7 @@ test('user can open and close filters dropdown', async () => { }) test('current visitors renders when visitors are present and disappears after visitors are null', async () => { - mockAPI.get(`/api/stats/${domain}/current-visitors?`, 500) + mockAPI.get(`/api/stats/${domain}/current-visitors`, 500) render(, { wrapper: (props) => ( @@ -111,7 +111,7 @@ test('current visitors renders when visitors are present and disappears after vi ).toBeVisible() }) - mockAPI.get(`/api/stats/${domain}/current-visitors?`, null) + mockAPI.get(`/api/stats/${domain}/current-visitors`, null) fireEvent(document, new CustomEvent('tick')) await waitForElementToBeRemoved(() => screen.queryByRole('link', { name: /current visitors/ }) diff --git a/assets/js/dashboard/nav-menu/top-bar.tsx b/assets/js/dashboard/nav-menu/top-bar.tsx index f6b6e43dc413..fa6dd7f7e674 100644 --- a/assets/js/dashboard/nav-menu/top-bar.tsx +++ b/assets/js/dashboard/nav-menu/top-bar.tsx @@ -11,6 +11,7 @@ import { useInView } from 'react-intersection-observer' import { FilterMenu } from './filter-menu' import { FiltersBar } from './filters-bar' import { QueryPeriodsPicker } from './query-periods/query-periods-picker' +import { SegmentMenu } from './segments/segment-menu' interface TopBarProps { showCurrentVisitors: boolean @@ -84,6 +85,7 @@ function TopBarInner({ showCurrentVisitors }: TopBarProps) {
+
diff --git a/assets/js/dashboard/navigation/routeless-modals-context.tsx b/assets/js/dashboard/navigation/routeless-modals-context.tsx new file mode 100644 index 000000000000..32e43aef35ec --- /dev/null +++ b/assets/js/dashboard/navigation/routeless-modals-context.tsx @@ -0,0 +1,42 @@ +/* @format */ +import React, { createContext, ReactNode, useContext, useState } from 'react' +import { RoutelessSegmentModal } from '../segments/routeless-segment-modals' + +type ActiveModal = null | RoutelessSegmentModal + +const routelessModalsContextDefaultValue: { + modal: ActiveModal + setModal: (modal: ActiveModal) => void +} = { + modal: null, + setModal: () => {} +} + +const RoutelessModalsContext = createContext< + typeof routelessModalsContextDefaultValue +>(routelessModalsContextDefaultValue) + +export const useRoutelessModalsContext = () => { + return useContext(RoutelessModalsContext) +} + +export function RoutelessModalsContextProvider({ + children +}: { + children: ReactNode +}) { + const [modal, setModal] = useState( + routelessModalsContextDefaultValue.modal + ) + + return ( + + {children} + + ) +} diff --git a/assets/js/dashboard/navigation/use-definite-location-state.tsx b/assets/js/dashboard/navigation/use-definite-location-state.tsx new file mode 100644 index 000000000000..db66d6e5f5fe --- /dev/null +++ b/assets/js/dashboard/navigation/use-definite-location-state.tsx @@ -0,0 +1,44 @@ +/** @format */ + +import { useEffect, useMemo, useState } from 'react' +import { useLocation } from 'react-router-dom' +import { useAppNavigate } from './use-app-navigate' + +/** + * This hook should be rendered once in the app, since it hooks into each navigation + * (re-navigating with replace: true on most navigations). It does so to + * replace undefined `location.state[key]` with a known definite value. + * The way to unset `location.state[key]` is to set it to null in navigation. + * This is to make sure that normal navigations (open Details etc) don't need to declare + * that they want to keep previous location.state. + */ +export function useDefiniteLocationState(key: string): { + definiteValue: T | null +} { + const location = useLocation() + const rawState = useMemo(() => location.state ?? {}, [location.state]) + + const navigate = useAppNavigate() + + const [definiteValue, setDefiniteValue] = useState( + rawState[key] === undefined ? null : (rawState[key] as T) + ) + + useEffect(() => { + if (rawState[key] === undefined) { + navigate({ + search: (s) => s, + state: { ...rawState, [key]: definiteValue }, + replace: true + }) + } + }, [definiteValue, rawState, navigate, key]) + + useEffect(() => { + if (rawState[key] !== undefined && rawState[key] !== definiteValue) { + setDefiniteValue(rawState[key] as T) + } + }, [rawState, definiteValue, key]) + + return { definiteValue } +} diff --git a/assets/js/dashboard/query-context.tsx b/assets/js/dashboard/query-context.tsx index 8acf552e12ee..d4987ff10d45 100644 --- a/assets/js/dashboard/query-context.tsx +++ b/assets/js/dashboard/query-context.tsx @@ -19,10 +19,14 @@ import { queryDefaultValue, postProcessFilters } from './query' +import { SavedSegment, SegmentData } from './filtering/segments' +import { useDefiniteLocationState } from './navigation/use-definite-location-state' +import { useClearExpandedSegmentModeOnFilterClear } from './nav-menu/segments/segment-menu' const queryContextDefaultValue = { query: queryDefaultValue, - otherSearch: {} as Record + otherSearch: {} as Record, + expandedSegment: null as (SavedSegment & { segment_data: SegmentData }) | null } export type QueryContextValue = typeof queryContextDefaultValue @@ -39,7 +43,11 @@ export default function QueryContextProvider({ children: ReactNode }) { const location = useLocation() + const { definiteValue: expandedSegment } = useDefiniteLocationState< + SavedSegment & { segment_data: SegmentData } + >('expandedSegment') const site = useSiteContext() + const { compare_from, compare_to, @@ -58,11 +66,11 @@ export default function QueryContextProvider({ const query = useMemo(() => { const defaultValues = queryDefaultValue const storedValues = getSavedTimePreferencesFromStorage({ site }) - const timeQuery = getDashboardTimeSettings({ searchValues: { period, comparison, match_day_of_week }, storedValues, - defaultValues + defaultValues, + segmentIsExpanded: !!expandedSegment }) return { @@ -111,9 +119,11 @@ export default function QueryContextProvider({ period, to, with_imported, - site + site, + expandedSegment ]) + useClearExpandedSegmentModeOnFilterClear({ expandedSegment, query }) useSaveTimePreferencesToStorage({ site, period, @@ -126,7 +136,13 @@ export default function QueryContextProvider({ }, []) return ( - + {children} ) diff --git a/assets/js/dashboard/query-time-periods.test.ts b/assets/js/dashboard/query-time-periods.test.ts index b2361092d11d..a8244c6b0b4e 100644 --- a/assets/js/dashboard/query-time-periods.test.ts +++ b/assets/js/dashboard/query-time-periods.test.ts @@ -45,7 +45,8 @@ describe(`${getDashboardTimeSettings.name}`, () => { getDashboardTimeSettings({ searchValues: emptySearchValues, storedValues: emptyStoredValues, - defaultValues + defaultValues, + segmentIsExpanded: false }) ).toEqual(defaultValues) }) @@ -59,7 +60,8 @@ describe(`${getDashboardTimeSettings.name}`, () => { comparison: ComparisonMode.year_over_year, match_day_of_week: false }, - defaultValues + defaultValues, + segmentIsExpanded: false }) ).toEqual({ period: QueryPeriod['12mo'], @@ -81,7 +83,8 @@ describe(`${getDashboardTimeSettings.name}`, () => { comparison: ComparisonMode.year_over_year, match_day_of_week: false }, - defaultValues + defaultValues, + segmentIsExpanded: false }) ).toEqual({ period: QueryPeriod['year'], @@ -89,4 +92,23 @@ describe(`${getDashboardTimeSettings.name}`, () => { match_day_of_week: true }) }) + + it('respects segmentIsExpanded: true option: comparison and edit segment mode are mutually exclusive', () => { + expect( + getDashboardTimeSettings({ + searchValues: { + period: QueryPeriod['custom'], + comparison: ComparisonMode.previous_period, + match_day_of_week: true + }, + storedValues: emptyStoredValues, + defaultValues, + segmentIsExpanded: true + }) + ).toEqual({ + period: QueryPeriod['custom'], + comparison: null, + match_day_of_week: true + }) + }) }) diff --git a/assets/js/dashboard/query-time-periods.ts b/assets/js/dashboard/query-time-periods.ts index d362df647417..cdc2d25fb85a 100644 --- a/assets/js/dashboard/query-time-periods.ts +++ b/assets/js/dashboard/query-time-periods.ts @@ -64,10 +64,15 @@ export const COMPARISON_MATCH_MODE_LABELS = { export const DEFAULT_COMPARISON_MODE = ComparisonMode.previous_period -export const COMPARISON_DISABLED_PERIODS = [ - QueryPeriod.realtime, - QueryPeriod.all -] +const COMPARISON_DISABLED_PERIODS = [QueryPeriod.realtime, QueryPeriod.all] + +export const isComparisonForbidden = ({ + period, + segmentIsExpanded +}: { + period: QueryPeriod + segmentIsExpanded: boolean +}) => COMPARISON_DISABLED_PERIODS.includes(period) || segmentIsExpanded export const DEFAULT_COMPARISON_MATCH_MODE = ComparisonMatchMode.MatchDayOfWeek @@ -501,7 +506,8 @@ export function getSavedTimePreferencesFromStorage({ export function getDashboardTimeSettings({ searchValues, storedValues, - defaultValues + defaultValues, + segmentIsExpanded }: { searchValues: Record<'period' | 'comparison' | 'match_day_of_week', unknown> storedValues: ReturnType @@ -509,6 +515,7 @@ export function getDashboardTimeSettings({ DashboardQuery, 'period' | 'comparison' | 'match_day_of_week' > + segmentIsExpanded: boolean }): Pick { let period: QueryPeriod if (isValidPeriod(searchValues.period)) { @@ -521,7 +528,7 @@ export function getDashboardTimeSettings({ let comparison: ComparisonMode | null - if ([QueryPeriod.realtime, QueryPeriod.all].includes(period)) { + if (isComparisonForbidden({ period, segmentIsExpanded })) { comparison = null } else { comparison = isValidComparison(searchValues.comparison) diff --git a/assets/js/dashboard/router.tsx b/assets/js/dashboard/router.tsx index af2a67505ea0..95136faed703 100644 --- a/assets/js/dashboard/router.tsx +++ b/assets/js/dashboard/router.tsx @@ -27,6 +27,8 @@ import FilterModal from './stats/modals/filter-modal' import QueryContextProvider from './query-context' import { DashboardKeybinds } from './dashboard-keybinds' import LastLoadContextProvider from './last-load-context' +import { RoutelessModalsContextProvider } from './navigation/routeless-modals-context' +import { RoutelessSegmentModals } from './segments/routeless-segment-modals' const queryClient = new QueryClient({ defaultOptions: { @@ -39,13 +41,16 @@ const queryClient = new QueryClient({ function DashboardElement() { return ( - - - - {/** render any children of the root route below */} - - - + + + + + {/** render any children of the root route below */} + + + + + ) } diff --git a/assets/js/dashboard/segments/routeless-segment-modals.tsx b/assets/js/dashboard/segments/routeless-segment-modals.tsx new file mode 100644 index 000000000000..7b2ecd2cbef5 --- /dev/null +++ b/assets/js/dashboard/segments/routeless-segment-modals.tsx @@ -0,0 +1,213 @@ +/** @format */ + +import React from 'react' +import { + CreateSegmentModal, + DeleteSegmentModal, + UpdateSegmentModal +} from './segment-modals' +import { + getSearchToApplySingleSegmentFilter, + getSegmentNamePlaceholder, + handleSegmentResponse, + SavedSegment, + SegmentData, + SegmentDataFromApi +} from '../filtering/segments' +import { useMutation, useQueryClient } from '@tanstack/react-query' +import { useSiteContext } from '../site-context' +import { cleanLabels, remapToApiFilters } from '../util/filters' +import { useAppNavigate } from '../navigation/use-app-navigate' +import { useQueryContext } from '../query-context' +import { Role, useUserContext } from '../user-context' +import { mutation } from '../api' +import { useRoutelessModalsContext } from '../navigation/routeless-modals-context' + +export type RoutelessSegmentModal = 'create' | 'update' | 'delete' + +export const RoutelessSegmentModals = () => { + const navigate = useAppNavigate() + const queryClient = useQueryClient() + const site = useSiteContext() + const { modal, setModal } = useRoutelessModalsContext() + const { query, expandedSegment } = useQueryContext() + const user = useUserContext() + + const patchSegment = useMutation({ + mutationFn: async ({ + id, + name, + type, + segment_data + }: Pick & + Partial> & { + segment_data?: SegmentData + }) => { + const response: SavedSegment & { segment_data: SegmentDataFromApi } = + await mutation( + `/api/${encodeURIComponent(site.domain)}/segments/${id}`, + { + method: 'PATCH', + body: { + name, + type, + ...(segment_data && { + segment_data: { + filters: remapToApiFilters(segment_data.filters), + labels: cleanLabels(segment_data.filters, segment_data.labels) + } + }) + } + } + ) + + return handleSegmentResponse(response) + }, + onSuccess: async (segment) => { + queryClient.invalidateQueries({ queryKey: ['segments'] }) + navigate({ + search: getSearchToApplySingleSegmentFilter(segment), + state: { + expandedSegment: null + } + }) + setModal(null) + } + }) + + const createSegment = useMutation({ + mutationFn: async ({ + name, + type, + segment_data + }: { + name: string + type: 'personal' | 'site' + segment_data: SegmentData + }) => { + const response: SavedSegment & { segment_data: SegmentDataFromApi } = + await mutation(`/api/${encodeURIComponent(site.domain)}/segments`, { + method: 'POST', + body: { + name, + type, + segment_data: { + filters: remapToApiFilters(segment_data.filters), + labels: cleanLabels(segment_data.filters, segment_data.labels) + } + } + }) + return handleSegmentResponse(response) + }, + onSuccess: async (segment) => { + queryClient.invalidateQueries({ queryKey: ['segments'] }) + navigate({ + search: getSearchToApplySingleSegmentFilter(segment), + state: { + expandedSegment: null + } + }) + setModal(null) + } + }) + + const deleteSegment = useMutation({ + mutationFn: async (data: Pick) => { + const response: SavedSegment & { segment_data: SegmentDataFromApi } = + await mutation( + `/api/${encodeURIComponent(site.domain)}/segments/${data.id}`, + { + method: 'DELETE' + } + ) + return handleSegmentResponse(response) + }, + onSuccess: (_segment): void => { + queryClient.invalidateQueries({ queryKey: ['segments'] }) + navigate({ + search: (s) => { + return { + ...s, + filters: null, + labels: null + } + }, + state: { + expandedSegment: null + } + }) + setModal(null) + } + }) + + if (!user.loggedIn || !site.flags.saved_segments) { + return null + } + + const userCanSelectSiteSegment = [ + Role.admin, + Role.owner, + Role.editor, + 'super_admin' + ].includes(user.role) + + return ( + <> + {modal === 'update' && expandedSegment && ( + setModal(null)} + onSave={({ id, name, type }) => + patchSegment.mutate({ + id, + name, + type, + segment_data: { + filters: query.filters, + labels: query.labels + } + }) + } + status={patchSegment.status} + error={patchSegment.error} + reset={patchSegment.reset} + /> + )} + {modal === 'create' && ( + setModal(null)} + onSave={({ name, type }) => + createSegment.mutate({ + name, + type, + segment_data: { + filters: query.filters, + labels: query.labels + } + }) + } + status={createSegment.status} + error={createSegment.error} + reset={createSegment.reset} + /> + )} + {modal === 'delete' && expandedSegment && ( + setModal(null)} + onSave={({ id }) => deleteSegment.mutate({ id })} + status={deleteSegment.status} + error={deleteSegment.error} + reset={deleteSegment.reset} + /> + )} + + ) +} diff --git a/assets/js/dashboard/segments/segment-authorship.tsx b/assets/js/dashboard/segments/segment-authorship.tsx new file mode 100644 index 000000000000..7f405c471f59 --- /dev/null +++ b/assets/js/dashboard/segments/segment-authorship.tsx @@ -0,0 +1,55 @@ +/** @format */ + +import React from 'react' +import { SavedSegment } from '../filtering/segments' +import { PlausibleSite, useSiteContext } from '../site-context' +import { dateForSite, formatDayShort } from '../util/date' + +const getAuthorLabel = ( + site: Pick, + owner_id: number +) => { + if (!site.members) { + return '' + } + + if (!owner_id || !site.members[owner_id]) { + return '(Removed User)' + } + + // if (owner_id === user.id) { + // return 'You' + // } + + return site.members[owner_id] +} + +export const SegmentAuthorship = ({ + className, + owner_id, + inserted_at, + updated_at +}: Pick & { + className?: string +}) => { + const site = useSiteContext() + + const authorLabel = getAuthorLabel(site, owner_id) + + const showUpdatedAt = updated_at !== inserted_at + + return ( +
+
+ {`Created at ${formatDayShort(dateForSite(inserted_at, site))}`} + {!showUpdatedAt && !!authorLabel && ` by ${authorLabel}`} +
+ {showUpdatedAt && ( +
+ {`Last updated at ${formatDayShort(dateForSite(updated_at, site))}`} + {!!authorLabel && ` by ${authorLabel}`} +
+ )} +
+ ) +} diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx new file mode 100644 index 000000000000..771f29adaf08 --- /dev/null +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -0,0 +1,534 @@ +/** @format */ + +import React, { ReactNode, useEffect, useState } from 'react' +import ModalWithRouting from '../stats/modals/modal' +import { + isSegmentFilter, + SavedSegment, + SegmentData, + SegmentType +} from '../filtering/segments' +import { useSegmentPrefetch } from '../nav-menu/segments/searchable-segments-section' +import { useQueryContext } from '../query-context' +import { AppNavigationLink } from '../navigation/use-app-navigate' +import { cleanLabels } from '../util/filters' +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, TrashIcon } from '@heroicons/react/24/outline' +import { MutationStatus } from '@tanstack/react-query' +import { ApiError } from '../api' +import { ErrorPanel } from '../components/error-panel' + +interface ApiRequestProps { + status: MutationStatus + error?: unknown + reset: () => void +} + +interface SegmentTypeSelectorProps { + siteSegmentsAvailable: boolean + userCanSelectSiteSegment: boolean +} + +interface SharedSegmentModalProps { + onClose: () => void + namePlaceholder: string +} + +const primaryNeutralButtonClassName = 'button' + +const primaryNegativeButtonClassName = classNames( + 'button', + 'items-center !bg-red-500 dark:!bg-red-500 hover:!bg-red-600 dark:hover:!bg-red-700 whitespace-nowrap' +) + +const secondaryButtonClassName = classNames( + 'button', + 'border !border-gray-300 dark:!border-gray-500 !text-gray-700 dark:!text-gray-300 !bg-transparent hover:!bg-gray-100 dark:hover:!bg-gray-850' +) + +const SegmentActionModal = ({ + children, + onClose +}: { + children: ReactNode + onClose: () => void +}) => { + return ( + +
{children}
+ + ) +} + +export const CreateSegmentModal = ({ + segment, + onClose, + onSave, + siteSegmentsAvailable: siteSegmentsAvailable, + userCanSelectSiteSegment, + namePlaceholder, + error, + reset, + status +}: SharedSegmentModalProps & + ApiRequestProps & + SegmentTypeSelectorProps & { + segment?: SavedSegment + onSave: (input: Pick) => void + }) => { + const defaultName = segment?.name + ? `Copy of ${segment.name}`.slice(0, 255) + : '' + const [name, setName] = useState(defaultName) + const defaultType = + segment?.type === SegmentType.site && + siteSegmentsAvailable && + userCanSelectSiteSegment + ? SegmentType.site + : SegmentType.personal + + const [type, setType] = useState(defaultType) + + return ( + + Create segment + + + + + + + {error !== null && ( + + )} + + ) +} + +export const DeleteSegmentModal = ({ + onClose, + onSave, + segment, + status, + error, + reset +}: { + onClose: () => void + onSave: (input: Pick) => void + segment: SavedSegment & { segment_data?: SegmentData } +} & ApiRequestProps) => { + return ( + + + { + { personal: 'Delete personal segment', site: 'Delete site segment' }[ + segment.type + ] + } + {` "${segment.name}"?`} + + {!!segment.segment_data && ( + + )} + + + + + + {error !== null && ( + + )} + + ) +} + +const FormTitle = ({ children }: { children?: ReactNode }) => ( +

{children}

+) + +const ButtonsRow = ({ + className, + children +}: { + className?: string + children?: ReactNode +}) => ( +
+ {children} +
+) + +const SegmentNameInput = ({ + namePlaceholder, + value, + onChange +}: { + namePlaceholder: string + value: string + onChange: (value: string) => void +}) => { + return ( + <> + + onChange(e.target.value)} + placeholder={namePlaceholder} + id="name" + className="block mt-2 p-2 w-full dark:bg-gray-900 dark:text-gray-300 rounded-md shadow-sm border border-gray-300 dark:border-gray-700 focus-within:border-indigo-500 focus-within:ring-1 focus-within:ring-indigo-500" + /> + + ) +} + +const SegmentTypeSelector = ({ + value, + onChange, + siteSegmentsAvailable, + userCanSelectSiteSegment +}: SegmentTypeSelectorProps & { + value: SegmentType + onChange: (value: SegmentType) => void +}) => { + const options = [ + { + type: SegmentType.personal, + name: 'Personal segment', + description: 'Visible only to you', + disabled: false + }, + { + type: SegmentType.site, + name: 'Site segment', + description: 'Visible to others on the site', + disabled: !userCanSelectSiteSegment || !siteSegmentsAvailable, + disabledReason: !userCanSelectSiteSegment ? ( + <> + {"You don't have enough permissions to change segment to this type."} + + ) : !siteSegmentsAvailable ? ( + <> + {`Your account does not have access to site segments. To use this + segment type, `} + + please upgrade your subscription + + . + + ) : null + } + ] + + return ( +
+ {options.map(({ type, name, description, disabled, disabledReason }) => ( +
+
+ onChange(type)} + className="mt-4 w-4 h-4 text-indigo-600 bg-gray-100 border-gray-300 focus:ring-2 focus:ring-offset-2 focus:ring-indigo-500 dark:border-gray-600" + /> + +
+ {value === type && disabled && !!disabledReason && ( +
+ +
{disabledReason}
+
+ )} +
+ ))} +
+ ) +} + +export const UpdateSegmentModal = ({ + onClose, + onSave, + segment, + siteSegmentsAvailable, + userCanSelectSiteSegment, + namePlaceholder, + status, + error, + reset +}: SharedSegmentModalProps & + ApiRequestProps & + SegmentTypeSelectorProps & { + onSave: (input: Pick) => void + segment: SavedSegment + }) => { + const [name, setName] = useState(segment.name) + const [type, setType] = useState(segment.type) + + return ( + + Update segment + + + + + + + {error !== null && ( + + )} + + ) +} + +const FiltersInSegment = ({ segment_data }: { segment_data: SegmentData }) => { + return ( + <> +

Filters in segment

+
+ ({ + plainText: plainFilterText({ labels: segment_data.labels }, filter), + children: styledFilterText({ labels: segment_data.labels }, filter), + interactive: false + }))} + /> +
+ + ) +} + +const Placeholder = ({ + children, + placeholder +}: { + children: ReactNode | false + placeholder: ReactNode +}) => ( + + {children === false ? placeholder : children} + +) + +export const SegmentModal = ({ id }: { id: SavedSegment['id'] }) => { + const { query } = useQueryContext() + + const { data, fetchSegment, status, error } = useSegmentPrefetch({ + id: String(id) + }) + + useEffect(() => { + fetchSegment() + }, [fetchSegment]) + + return ( + +
+
+
+

+ {data ? data.name : 'Segment details'} +

+
+
+ +
+ + {data?.segment_data + ? { + [SegmentType.personal]: 'Personal segment', + [SegmentType.site]: 'Site segment' + }[data.type] + : false} + +
+
+ {!!data?.segment_data && ( + <> + + + +
+ + ({ + ...s, + filters: data.segment_data.filters, + labels: data.segment_data.labels + })} + state={{ + expandedSegment: data + }} + > + Edit segment + + + { + const nonSegmentFilters = query.filters.filter( + (f) => !isSegmentFilter(f) + ) + return { + ...s, + filters: nonSegmentFilters, + labels: cleanLabels( + nonSegmentFilters, + query.labels, + 'segment', + {} + ) + } + }} + > + + Remove filter + + +
+ + )} + {status === 'pending' && ( +
+
+
+
+
+ )} + {error !== null && ( + fetchSegment()} + /> + )} +
+ + ) +} diff --git a/assets/js/dashboard/site-context.test.tsx b/assets/js/dashboard/site-context.test.tsx index 9c4109bee1e8..1ad2701104c4 100644 --- a/assets/js/dashboard/site-context.test.tsx +++ b/assets/js/dashboard/site-context.test.tsx @@ -16,6 +16,7 @@ describe('parseSiteFromDataset', () => { data-funnels-opted-out="false" data-props-opted-out="false" data-funnels-available="true" + data-site-segments-available="true" data-props-available="true" data-revenue-goals='[{"currency":"USD","display_name":"Purchase"}]' data-funnels='[{"id":1,"name":"From homepage to login","steps_count":3}]' @@ -27,6 +28,8 @@ describe('parseSiteFromDataset', () => { data-embedded="" data-is-dbip="false" data-current-user-role="owner" + data-current-user-id="1" + data-members='{"1":"Test User"}' data-flags="{}" data-valid-intervals-by-period='{"12mo":["day","week","month"],"30d":["day","week"],"6mo":["day","week","month"],"7d":["hour","day"],"all":["week","month"],"custom":["day","week","month"],"day":["minute","hour"],"month":["day","week"],"realtime":["minute"],"year":["day","week","month"]}' {...attrs} @@ -41,6 +44,7 @@ describe('parseSiteFromDataset', () => { propsOptedOut: false, funnelsAvailable: true, propsAvailable: true, + siteSegmentsAvailable: true, revenueGoals: [{ currency: 'USD', display_name: 'Purchase' }], funnels: [{ id: 1, name: 'From homepage to login', steps_count: 3 }], hasProps: true, @@ -63,7 +67,8 @@ describe('parseSiteFromDataset', () => { realtime: ['minute'], year: ['day', 'week', 'month'] }, - shared: false + shared: false, + members: { '1': 'Test User' } } it('parses from dom string map correctly', () => { diff --git a/assets/js/dashboard/site-context.tsx b/assets/js/dashboard/site-context.tsx index 215bcba48db3..fbd03c3f5fde 100644 --- a/assets/js/dashboard/site-context.tsx +++ b/assets/js/dashboard/site-context.tsx @@ -10,6 +10,7 @@ export function parseSiteFromDataset(dataset: DOMStringMap): PlausibleSite { scrollDepthVisible: dataset.scrollDepthVisible === 'true', funnelsAvailable: dataset.funnelsAvailable === 'true', propsAvailable: dataset.propsAvailable === 'true', + siteSegmentsAvailable: dataset.siteSegmentsAvailable === 'true', conversionsOptedOut: dataset.conversionsOptedOut === 'true', funnelsOptedOut: dataset.funnelsOptedOut === 'true', propsOptedOut: dataset.propsOptedOut === 'true', @@ -22,7 +23,8 @@ export function parseSiteFromDataset(dataset: DOMStringMap): PlausibleSite { isDbip: dataset.isDbip === 'true', flags: JSON.parse(dataset.flags!), validIntervalsByPeriod: JSON.parse(dataset.validIntervalsByPeriod!), - shared: !!dataset.sharedLinkAuth + shared: !!dataset.sharedLinkAuth, + members: JSON.parse(dataset.members!) } } @@ -40,6 +42,7 @@ const siteContextDefaultValue = { scrollDepthVisible: false, funnelsAvailable: false, propsAvailable: false, + siteSegmentsAvailable: false, conversionsOptedOut: false, funnelsOptedOut: false, propsOptedOut: false, @@ -54,7 +57,8 @@ const siteContextDefaultValue = { isDbip: false, flags: {} as FeatureFlags, validIntervalsByPeriod: {} as Record>, - shared: false + shared: false, + members: null as null | Record } export type PlausibleSite = typeof siteContextDefaultValue diff --git a/assets/js/dashboard/site-switcher.js b/assets/js/dashboard/site-switcher.js index 4919265dfd94..2612bc23d712 100644 --- a/assets/js/dashboard/site-switcher.js +++ b/assets/js/dashboard/site-switcher.js @@ -249,13 +249,13 @@ export default class SiteSwitcher extends React.Component { > - + {this.props.site.domain} {this.props.loggedIn && ( - + )} diff --git a/assets/js/dashboard/stats/modals/filter-modal.js b/assets/js/dashboard/stats/modals/filter-modal.js index 6762b3bee36d..ea381637096c 100644 --- a/assets/js/dashboard/stats/modals/filter-modal.js +++ b/assets/js/dashboard/stats/modals/filter-modal.js @@ -2,13 +2,16 @@ import React from 'react' import { useParams } from 'react-router-dom'; import Modal from './modal'; -import { EVENT_PROPS_PREFIX, FILTER_GROUP_TO_MODAL_TYPE, formatFilterGroup, FILTER_OPERATIONS, getFilterGroup, FILTER_MODAL_TO_FILTER_GROUP, cleanLabels } from '../../util/filters'; +import { EVENT_PROPS_PREFIX, FILTER_GROUP_TO_MODAL_TYPE, formatFilterGroup, FILTER_OPERATIONS, getFilterGroup, FILTER_MODAL_TO_FILTER_GROUP, cleanLabels, getAvailableFilterModals } from '../../util/filters'; import { useQueryContext } from '../../query-context'; import { useSiteContext } from '../../site-context'; import { isModifierPressed, isTyping } from '../../keybinding'; import FilterModalGroup from "./filter-modal-group"; import { rootRoute } from '../../router'; import { useAppNavigate } from '../../navigation/use-app-navigate'; +import { SegmentModal } from '../../segments/segment-modals'; +import { TrashIcon } from '@heroicons/react/24/outline'; +import { isSegmentFilter } from '../../filtering/segments'; function partitionFilters(modalType, filters) { const otherFilters = [] @@ -171,18 +174,18 @@ class FilterModal extends React.Component { className="button" disabled={this.isDisabled()} > - Apply Filter + Apply filter {this.state.hasRelevantFilters && ( )} @@ -199,6 +202,14 @@ export default function FilterModalWithRouter(props) { const { field } = useParams() const { query } = useQueryContext() const site = useSiteContext() + if (!Object.keys(getAvailableFilterModals(site)).includes(field)) { + return null + } + const firstSegmentFilter = field === 'segment' ? query.filters?.find(isSegmentFilter) : null + if (firstSegmentFilter) { + const firstSegmentId = firstSegmentFilter[2][0] + return + } return ( (userContextDefaultValue) export const useUserContext = () => { return useContext(UserContext) } export default function UserContextProvider({ - role, - loggedIn, + user, children }: { - role: Role - loggedIn: boolean + user: UserContextValue children: ReactNode }) { - return ( - - {children} - - ) + return {children} } diff --git a/assets/js/dashboard/util/date.js b/assets/js/dashboard/util/date.js index e0519406cfba..1430c22c281e 100644 --- a/assets/js/dashboard/util/date.js +++ b/assets/js/dashboard/util/date.js @@ -1,5 +1,7 @@ -import dayjs from 'dayjs'; -import utc from 'dayjs/plugin/utc'; +/** @format */ + +import dayjs from 'dayjs' +import utc from 'dayjs/plugin/utc' dayjs.extend(utc) @@ -21,11 +23,11 @@ export function formatMonthYYYY(date) { } export function formatYear(date) { - return `Year of ${date.year()}`; + return `Year of ${date.year()}` } export function formatYearShort(date) { - return date.getUTCFullYear().toString().substring(2) + return date.getUTCFullYear().toString().substring(2) } export function formatDay(date) { @@ -67,6 +69,10 @@ export function parseNaiveDate(dateString) { return dayjs(dateString) } +export function dateForSite(utcDateString, site) { + return dayjs.utc(utcDateString).utcOffset(site.offset / 60) +} + export function nowForSite(site) { return dayjs.utc().utcOffset(site.offset / 60) } @@ -102,16 +108,16 @@ export function isThisYear(site, date) { export function isBefore(date1, date2, period) { /* assumes 'day' and 'month' are the only valid periods */ if (date1.year() !== date2.year()) { - return date1.year() < date2.year(); + return date1.year() < date2.year() } - if (period === "year") { - return false; + if (period === 'year') { + return false } if (date1.month() !== date2.month()) { - return date1.month() < date2.month(); + return date1.month() < date2.month() } - if (period === "month") { - return false; + if (period === 'month') { + return false } return date1.date() < date2.date() } @@ -119,16 +125,16 @@ export function isBefore(date1, date2, period) { export function isAfter(date1, date2, period) { /* assumes 'day' and 'month' are the only valid periods */ if (date1.year() !== date2.year()) { - return date1.year() > date2.year(); + return date1.year() > date2.year() } - if (period === "year") { - return false; + if (period === 'year') { + return false } if (date1.month() !== date2.month()) { - return date1.month() > date2.month(); + return date1.month() > date2.month() } - if (period === "month") { - return false; + if (period === 'month') { + return false } return date1.date() > date2.date() } diff --git a/assets/js/dashboard/util/date.test.ts b/assets/js/dashboard/util/date.test.ts index 99a2085fa70f..ba085e7fce73 100644 --- a/assets/js/dashboard/util/date.test.ts +++ b/assets/js/dashboard/util/date.test.ts @@ -1,6 +1,13 @@ /** @format */ -import { formatISO, nowForSite, shiftMonths, yesterday } from './date' +import { + dateForSite, + formatDayShort, + formatISO, + nowForSite, + shiftMonths, + yesterday +} from './date' jest.useFakeTimers() @@ -102,3 +109,13 @@ for (const [timezone, suite] of sets) { ) }) } + +describe('formatting UTC dates from database', () => { + it('is able to enrich UTC date string with site timezone, formatting the value correctly', () => { + expect( + formatDayShort( + dateForSite('2025-01-01T14:00:00', { offset: 60 * 60 * 11 }) + ) + ).toEqual('2 Jan') + }) +}) diff --git a/assets/js/dashboard/util/filters.js b/assets/js/dashboard/util/filters.js index d59b5f7378ba..4f6714d1d3c7 100644 --- a/assets/js/dashboard/util/filters.js +++ b/assets/js/dashboard/util/filters.js @@ -1,6 +1,7 @@ /** @format */ import * as api from '../api' +import { formatSegmentIdAsLabelKey } from '../filtering/segments' export const FILTER_MODAL_TO_FILTER_GROUP = { page: ['page', 'entry_page', 'exit_page'], @@ -12,7 +13,17 @@ export const FILTER_MODAL_TO_FILTER_GROUP = { utm: ['utm_medium', 'utm_source', 'utm_campaign', 'utm_term', 'utm_content'], goal: ['goal'], props: ['props'], - hostname: ['hostname'] + hostname: ['hostname'], + segment: ['segment'] +} + +export function getAvailableFilterModals(site) { + const { props, segment, ...rest } = FILTER_MODAL_TO_FILTER_GROUP + return { + ...rest, + ...(site.propsAvailable && { props }), + ...(site.flags.saved_segments && { segment }) + } } export const FILTER_GROUP_TO_MODAL_TYPE = Object.fromEntries( @@ -65,9 +76,13 @@ export function isFreeChoiceFilterOperation(operation) { export function getLabel(labels, filterKey, value) { if (['country', 'region', 'city'].includes(filterKey)) { return labels[value] - } else { - return value } + + if (filterKey === 'segment') { + return labels[formatSegmentIdAsLabelKey(value)] + } + + return value } export function getPropertyKeyFromFilterKey(filterKey) { @@ -135,11 +150,18 @@ export function formatFilterGroup(filterGroup) { export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) { const filteredBy = Object.fromEntries( filters - .flatMap(([_operation, filterKey, clauses]) => - ['country', 'region', 'city'].includes(filterKey) ? clauses : [] - ) + .flatMap(([_operation, filterKey, clauses]) => { + if (filterKey === 'segment') { + return clauses.map(formatSegmentIdAsLabelKey) + } + if (['country', 'region', 'city'].includes(filterKey)) { + return clauses + } + return [] + }) .map((value) => [value, true]) ) + let result = { ...labels } for (const value in labels) { if (!filteredBy[value]) { @@ -149,7 +171,7 @@ export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) { if ( mergedFilterKey && - ['country', 'region', 'city'].includes(mergedFilterKey) + ['country', 'region', 'city', 'segment'].includes(mergedFilterKey) ) { result = { ...result, @@ -160,20 +182,72 @@ export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) { return result } +const NO_PREFIX_KEYS = new Set(['segment']) const EVENT_FILTER_KEYS = new Set(['name', 'page', 'goal', 'hostname']) +const EVENT_PREFIX = 'event:' +const VISIT_PREFIX = 'visit:' + +function remapFilterKey(filterKey) { + if (NO_PREFIX_KEYS.has(filterKey)) { + return filterKey + } + if ( + EVENT_FILTER_KEYS.has(filterKey) || + filterKey.startsWith(EVENT_PROPS_PREFIX) + ) { + return `${EVENT_PREFIX}${filterKey}` + } + return `${VISIT_PREFIX}${filterKey}` +} + +function remapApiFilterKey(apiFilterKey) { + const isNoPrefixKey = NO_PREFIX_KEYS.has(apiFilterKey) + + if (isNoPrefixKey) { + return apiFilterKey + } + + const isEventKey = apiFilterKey.startsWith(EVENT_PREFIX) + const isVisitKey = apiFilterKey.startsWith(VISIT_PREFIX) + + if (isEventKey) { + return apiFilterKey.substring(EVENT_PREFIX.length) + } + if (isVisitKey) { + return apiFilterKey.substring(VISIT_PREFIX.length) + } + + return apiFilterKey // maybe throw? +} + +export function remapToApiFilters(filters) { + return filters.map(remapToApiFilter) +} + +export function remapFromApiFilters(apiFilters) { + return apiFilters.map((apiFilter) => { + const [operation, ...rest] = apiFilter + if (operation === 'has_not_done') { + const [[_, apiFilterKey, clauses]] = rest + return [ + FILTER_OPERATIONS.has_not_done, + remapApiFilterKey(apiFilterKey), + clauses + ] + } + const [apiFilterKey, clauses] = rest + return [operation, remapApiFilterKey(apiFilterKey), clauses] + }) +} export function serializeApiFilters(filters) { - const apiFilters = filters.map(serializeFilter) - return JSON.stringify(apiFilters) + return JSON.stringify(remapToApiFilters(filters)) } -function serializeFilter([operation, filterKey, clauses, ...modifiers]) { - let apiFilterKey = `visit:${filterKey}` - if ( - filterKey.startsWith(EVENT_PROPS_PREFIX) || - EVENT_FILTER_KEYS.has(filterKey) - ) { - apiFilterKey = `event:${filterKey}` +function remapToApiFilter([operation, filterKey, clauses, ...modifiers]) { + const apiFilterKey = remapFilterKey(filterKey) + if (apiFilterKey === 'segment') { + return [operation, apiFilterKey, clauses.map((v) => parseInt(v, 10))] } if (operation === FILTER_OPERATIONS.has_not_done) { // :NOTE: Frontend does not support advanced query building that's used in the backend. @@ -232,5 +306,6 @@ export const formattedFilters = { page: 'Page', hostname: 'Hostname', entry_page: 'Entry Page', - exit_page: 'Exit Page' + exit_page: 'Exit Page', + segment: 'Segment' } diff --git a/assets/js/dashboard/util/filters.test.ts b/assets/js/dashboard/util/filters.test.ts index 10661dc82d9b..cd774b30f27a 100644 --- a/assets/js/dashboard/util/filters.test.ts +++ b/assets/js/dashboard/util/filters.test.ts @@ -1,6 +1,62 @@ -import { serializeApiFilters } from './filters' +/** @format */ -describe('serializeApiFilters', () => { +import { getAvailableFilterModals, serializeApiFilters } from './filters' + +describe(`${getAvailableFilterModals.name}`, () => { + it('gives limited object when props and segments are not available', () => { + expect( + getAvailableFilterModals({ + propsAvailable: false, + flags: { saved_segments: null } + }) + ).toEqual({ + browser: ['browser', 'browser_version'], + goal: ['goal'], + hostname: ['hostname'], + location: ['country', 'region', 'city'], + os: ['os', 'os_version'], + page: ['page', 'entry_page', 'exit_page'], + screen: ['screen'], + source: ['source', 'channel', 'referrer'], + utm: [ + 'utm_medium', + 'utm_source', + 'utm_campaign', + 'utm_term', + 'utm_content' + ] + }) + }) + + it('gives full object when props and segments are available', () => { + expect( + getAvailableFilterModals({ + propsAvailable: true, + flags: { saved_segments: true } + }) + ).toEqual({ + browser: ['browser', 'browser_version'], + goal: ['goal'], + hostname: ['hostname'], + location: ['country', 'region', 'city'], + os: ['os', 'os_version'], + page: ['page', 'entry_page', 'exit_page'], + screen: ['screen'], + source: ['source', 'channel', 'referrer'], + utm: [ + 'utm_medium', + 'utm_source', + 'utm_campaign', + 'utm_term', + 'utm_content' + ], + props: ['props'], + segment: ['segment'] + }) + }) +}) + +describe(`${serializeApiFilters.name}`, () => { it('should prefix filter keys with event: or visit: when appropriate', () => { const filters = [ ['is', 'page', ['/docs', '/blog']], @@ -9,21 +65,21 @@ describe('serializeApiFilters', () => { ['is', 'country', ['US']], ['is_not', 'utm_source', ['google']] ] - expect(serializeApiFilters(filters)).toEqual(JSON.stringify([ - ['is', 'event:page', ['/docs', '/blog']], - ['contains', 'event:goal', ['Signup']], - ['contains_not', 'visit:browser', ['chrom'], { case_sensitive: false }], - ['is', 'visit:country', ['US']], - ['is_not', 'visit:utm_source', ['google']] - ])) + expect(serializeApiFilters(filters)).toEqual( + JSON.stringify([ + ['is', 'event:page', ['/docs', '/blog']], + ['contains', 'event:goal', ['Signup']], + ['contains_not', 'visit:browser', ['chrom'], { case_sensitive: false }], + ['is', 'visit:country', ['US']], + ['is_not', 'visit:utm_source', ['google']] + ]) + ) }) it('wraps has_not_done goal filters in API format', () => { - const filters = [ - ['has_not_done', 'goal', ['Signup']] - ] - expect(serializeApiFilters(filters)).toEqual(JSON.stringify([ - ['has_not_done', ['is', 'event:goal', ['Signup']]] - ])) + const filters = [['has_not_done', 'goal', ['Signup']]] + expect(serializeApiFilters(filters)).toEqual( + JSON.stringify([['has_not_done', ['is', 'event:goal', ['Signup']]]]) + ) }) }) diff --git a/assets/test-utils/app-context-providers.tsx b/assets/test-utils/app-context-providers.tsx index 96819be93934..012d5391bc21 100644 --- a/assets/test-utils/app-context-providers.tsx +++ b/assets/test-utils/app-context-providers.tsx @@ -9,6 +9,7 @@ import { MemoryRouter, MemoryRouterProps } from 'react-router-dom' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import QueryContextProvider from '../js/dashboard/query-context' import { getRouterBasepath } from '../js/dashboard/router' +import { RoutelessModalsContextProvider } from '../js/dashboard/navigation/routeless-modals-context' type TestContextProvidersProps = { children: ReactNode @@ -29,6 +30,7 @@ export const TestContextProviders = ({ scrollDepthVisible: false, funnelsAvailable: false, propsAvailable: false, + siteSegmentsAvailable: false, conversionsOptedOut: false, funnelsOptedOut: false, propsOptedOut: false, @@ -41,7 +43,8 @@ export const TestContextProviders = ({ isDbip: false, flags: {}, validIntervalsByPeriod: {}, - shared: false + shared: false, + members: { 1: 'Test User' } } const site = { ...defaultSite, ...siteOptions } @@ -59,14 +62,16 @@ export const TestContextProviders = ({ return ( // not interactive component, default value is suitable - + - {children} + + {children} + diff --git a/lib/plausible/segments/filters.ex b/lib/plausible/segments/filters.ex index 132ce2bd55bc..bd62b8dd58b7 100644 --- a/lib/plausible/segments/filters.ex +++ b/lib/plausible/segments/filters.ex @@ -63,8 +63,15 @@ defmodule Plausible.Segments.Filters do end end + defp expand_first_level_and_filters(filter) do + case filter do + [:and, clauses] -> clauses + filter -> [filter] + end + end + defp replace_segment_with_filter_tree([_, "segment", clauses], preloaded_segments) do - if length(clauses) === 1 do + if length(clauses) == 1 do [[:and, Map.get(preloaded_segments, Enum.at(clauses, 0))]] else [[:or, Enum.map(clauses, fn id -> [:and, Map.get(preloaded_segments, id)] end)]] @@ -84,9 +91,18 @@ defmodule Plausible.Segments.Filters do iex> resolve_segments([[:is, "visit:entry_page", ["/home"]], [:is, "segment", [1]]], %{1 => [[:contains, "visit:entry_page", ["blog"]], [:is, "visit:country", ["PL"]]]}) {:ok, [ [:is, "visit:entry_page", ["/home"]], - [:and, [[:contains, "visit:entry_page", ["blog"]], [:is, "visit:country", ["PL"]]]] + [:contains, "visit:entry_page", ["blog"]], + [:is, "visit:country", ["PL"]] + ]} + + iex> resolve_segments([[:is, "visit:entry_page", ["/home"]], [:is, "segment", [1]], [:is, "segment", [2]]], %{1 => [[:is, "visit:country", ["PL"]]], 2 => [[:is, "event:goal", ["Signup"]]]}) + {:ok, [ + [:is, "visit:entry_page", ["/home"]], + [:is, "visit:country", ["PL"]], + [:is, "event:goal", ["Signup"]] ]} + iex> resolve_segments([[:is, "segment", [1, 2]]], %{1 => [[:contains, "event:goal", ["Singup"]], [:is, "visit:country", ["PL"]]], 2 => [[:contains, "event:goal", ["Sauna"]], [:is, "visit:country", ["EE"]]]}) {:ok, [ [:or, [ @@ -98,9 +114,11 @@ defmodule Plausible.Segments.Filters do def resolve_segments(original_filters, preloaded_segments) do if map_size(preloaded_segments) > 0 do {:ok, - Filters.transform_filters(original_filters, fn f -> + original_filters + |> Filters.transform_filters(fn f -> replace_segment_with_filter_tree(f, preloaded_segments) - end)} + end) + |> Filters.transform_filters(&expand_first_level_and_filters/1)} else {:ok, original_filters} end diff --git a/lib/plausible/segments/segment.ex b/lib/plausible/segments/segment.ex index d2edfe2d7aed..43978b7cc315 100644 --- a/lib/plausible/segments/segment.ex +++ b/lib/plausible/segments/segment.ex @@ -197,9 +197,10 @@ defmodule Plausible.Segments.Segment do end defp dashboard_compatible_filter?(filter) do - is_list(filter) and length(filter) === 3 and - is_atom(Enum.at(filter, 0)) and - is_binary(Enum.at(filter, 1)) and - is_list(Enum.at(filter, 2)) + case filter do + [operation, dimension, _clauses] when is_atom(operation) and is_binary(dimension) -> true + [:has_not_done, _] -> true + _ -> false + end end end diff --git a/lib/plausible/segments/segments.ex b/lib/plausible/segments/segments.ex index 63ac290244d8..4a52a60d8a9c 100644 --- a/lib/plausible/segments/segments.ex +++ b/lib/plausible/segments/segments.ex @@ -297,4 +297,19 @@ defmodule Plausible.Segments do Repo.all(query) end + + @doc """ + iex> serialize_first_error([{"name", {"should be at most %{count} byte(s)", [count: 255]}}]) + "name should be at most 255 byte(s)" + """ + def serialize_first_error(errors) do + {field, {message, opts}} = List.first(errors) + + formatted_message = + Enum.reduce(opts, message, fn {key, value}, acc -> + String.replace(acc, "%{#{key}}", to_string(value)) + end) + + "#{field} #{formatted_message}" + end end diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index 38fd78fbe570..392e37528743 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -566,7 +566,7 @@ defmodule Plausible.Stats.Filters.QueryParser do :ok else {:error, - "The goal `#{clause}` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals"} + "Invalid filters. The goal `#{clause}` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals"} end end diff --git a/lib/plausible_web/controllers/api/internal/segments_controller.ex b/lib/plausible_web/controllers/api/internal/segments_controller.ex index a1df13bff790..b28db63f5d39 100644 --- a/lib/plausible_web/controllers/api/internal/segments_controller.ex +++ b/lib/plausible_web/controllers/api/internal/segments_controller.ex @@ -76,8 +76,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsController do conn |> put_status(400) |> json(%{ - errors: - Enum.map(errors, fn {field_key, {message, opts}} -> [field_key, message, opts] end) + error: Segments.serialize_first_error(errors) }) {:ok, segment} -> @@ -111,8 +110,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsController do conn |> put_status(400) |> json(%{ - errors: - Enum.map(errors, fn {field_key, {message, opts}} -> [field_key, message, opts] end) + error: Segments.serialize_first_error(errors) }) {:ok, segment} -> diff --git a/lib/plausible_web/controllers/stats_controller.ex b/lib/plausible_web/controllers/stats_controller.ex index 5023fccfbc94..713cceeee329 100644 --- a/lib/plausible_web/controllers/stats_controller.ex +++ b/lib/plausible_web/controllers/stats_controller.ex @@ -71,6 +71,8 @@ defmodule PlausibleWeb.StatsController do cond do (stats_start_date && can_see_stats?) || (can_see_stats? && skip_to_dashboard?) -> + flags = get_flags(current_user, site) + conn |> put_resp_header("x-robots-tag", "noindex, nofollow") |> render("stats.html", @@ -84,7 +86,12 @@ defmodule PlausibleWeb.StatsController do native_stats_start_date: NaiveDateTime.to_date(site.native_stats_start_at), title: title(conn, site), demo: demo, - flags: get_flags(current_user, site), + flags: flags, + members: + if(flags.saved_segments, + do: get_members(conn.assigns[:site_role], site), + else: nil + ), is_dbip: is_dbip(), dogfood_page_path: dogfood_page_path, load_dashboard_js: true @@ -352,6 +359,8 @@ defmodule PlausibleWeb.StatsController do scroll_depth_visible? = Plausible.Stats.ScrollDepth.check_feature_visible!(shared_link.site, current_user) + flags = get_flags(current_user, shared_link.site) + conn |> put_resp_header("x-robots-tag", "noindex, nofollow") |> delete_resp_header("x-frame-options") @@ -371,7 +380,12 @@ defmodule PlausibleWeb.StatsController do embedded: conn.params["embed"] == "true", background: conn.params["background"], theme: conn.params["theme"], - flags: get_flags(current_user, shared_link.site), + flags: flags, + members: + if(flags.saved_segments, + do: get_members(conn.assigns[:site_role], shared_link.site), + else: nil + ), is_dbip: is_dbip(), load_dashboard_js: true ) @@ -397,6 +411,30 @@ defmodule PlausibleWeb.StatsController do end) |> Map.new() + defp get_members(site_role, %Plausible.Site{} = site) + when site_role in [:viewer, :editor, :owner, :admin, :super_admin] do + site = + site + |> Plausible.Repo.preload( + team: [team_memberships: [:user]], + guest_memberships: [team_membership: [:user]] + ) + + site.guest_memberships + |> Enum.map(fn i = %Plausible.Teams.GuestMembership{} -> + i.team_membership + end) + |> Enum.concat(site.team.team_memberships) + |> Enum.map(fn i = %Plausible.Teams.Membership{} -> + {i.user.id, i.user.name} + end) + |> Map.new() + end + + defp get_members(_site_role, _site) do + nil + end + defp is_dbip() do on_ee do false diff --git a/lib/plausible_web/templates/stats/stats.html.heex b/lib/plausible_web/templates/stats/stats.html.heex index 8bbecc573aa8..d729e115b6cb 100644 --- a/lib/plausible_web/templates/stats/stats.html.heex +++ b/lib/plausible_web/templates/stats/stats.html.heex @@ -27,6 +27,9 @@ data-props-available={ to_string(Plausible.Billing.Feature.Props.check_availability(@site.team) == :ok) } + data-site-segments-available={ + to_string(Plausible.Billing.Feature.Props.check_availability(@site.team) == :ok) + } data-revenue-goals={Jason.encode!(@revenue_goals)} data-funnels={Jason.encode!(@funnels)} data-has-props={to_string(@has_props)} @@ -39,6 +42,10 @@ data-background={@conn.assigns[:background]} data-is-dbip={to_string(@is_dbip)} data-current-user-role={@conn.assigns[:site_role]} + data-current-user-id={ + if user = @conn.assigns[:current_user], do: user.id, else: Jason.encode!(nil) + } + data-members={Jason.encode!(@members)} data-flags={Jason.encode!(@flags)} data-valid-intervals-by-period={ Plausible.Stats.Interval.valid_by_period(site: @site) |> Jason.encode!() diff --git a/test/plausible/segments/segments_test.exs b/test/plausible/segments/segments_test.exs new file mode 100644 index 000000000000..4fc53ac01bbd --- /dev/null +++ b/test/plausible/segments/segments_test.exs @@ -0,0 +1,4 @@ +defmodule Plausible.SegmentsTest do + use ExUnit.Case + doctest Plausible.Segments, import: true +end diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index a523dc7e8708..5615a0ebd567 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -1137,7 +1137,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } |> check_error( site, - "The goal `Signup` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals" + "Invalid filters. The goal `Signup` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals" ) end @@ -1152,7 +1152,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } |> check_error( site, - "The goal `Visit /thank-you` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals" + "Invalid filters. The goal `Visit /thank-you` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals" ) end @@ -1255,7 +1255,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } |> check_error( site, - "The goal `Unknown` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals", + "Invalid filters. The goal `Unknown` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals", :internal ) end @@ -2440,23 +2440,18 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors, :events], utc_time_range: @date_range_day, filters: [ + [ + :or, + [ + [:and, [[:is, "visit:country", ["AU", "NZ"]]]], + [:and, [[:is, "visit:country", ["FR", "DE"]]]] + ] + ], [ :and, [ - [ - :or, - [ - [:and, [[:is, "visit:country", ["AU", "NZ"]]]], - [:and, [[:is, "visit:country", ["FR", "DE"]]]] - ] - ], - [ - :and, - [ - [:is, "visit:browser", ["Firefox"]], - [:is, "visit:os", ["Linux"]] - ] - ] + [:is, "visit:browser", ["Firefox"]], + [:is, "visit:os", ["Linux"]] ] ] ], diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs index a5acd797b727..104fa08176be 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs @@ -4959,7 +4959,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do site: site, name: "Signups", segment_data: %{ - "filters" => [["is", "event:name", ["Signup"]]] + "filters" => [["is", "event:goal", ["Signup"]]] } ) @@ -4998,7 +4998,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do # response shows what filters the segment was resolved to assert json_response(conn, 200)["query"]["filters"] == [ - ["and", [["is", "event:name", ["Signup"]]]] + ["is", "event:goal", ["Signup"]] ] end end diff --git a/test/plausible_web/controllers/api/internal_controller/segments_controller_test.exs b/test/plausible_web/controllers/api/internal_controller/segments_controller_test.exs index d6269ebee005..375a7a31fe06 100644 --- a/test/plausible_web/controllers/api/internal_controller/segments_controller_test.exs +++ b/test/plausible_web/controllers/api/internal_controller/segments_controller_test.exs @@ -327,13 +327,8 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do }) assert json_response(conn, 400) == %{ - "errors" => [ - [ - "segment_data", - "#/filters/0: Invalid filter [\"is\", \"entry_page\", [\"/blog\"]]", - [] - ] - ] + "error" => + "segment_data #/filters/0: Invalid filter [\"is\", \"entry_page\", [\"/blog\"]]" } end @@ -346,12 +341,18 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do %{conn: conn, user: user} do site = new_site() add_guest(site, user: user, role: unquote(role)) + insert(:goal, site: site, event_name: "Conversion") response = post(conn, "/api/#{site.domain}/segments", %{ "name" => "Some segment", "type" => "#{unquote(type)}", - "segment_data" => %{"filters" => [["is", "visit:entry_page", ["/blog"]]]} + "segment_data" => %{ + "filters" => [ + ["is", "visit:entry_page", ["/blog"]], + ["has_not_done", ["is", "event:goal", ["Conversion"]]] + ] + } }) |> json_response(200) @@ -360,7 +361,12 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do "name" => "Some segment", "type" => ^"#{unquote(type)}", "segment_data" => - ^strict_map(%{"filters" => [["is", "visit:entry_page", ["/blog"]]]}), + ^strict_map(%{ + "filters" => [ + ["is", "visit:entry_page", ["/blog"]], + ["has_not_done", ["is", "event:goal", ["Conversion"]]] + ] + }), "owner_id" => ^user.id, "inserted_at" => ^any(:iso8601_naive_datetime), "updated_at" => ^any(:iso8601_naive_datetime) @@ -417,19 +423,11 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do end end - for {filters, expected_errors} <- [ - {[], - [ - [ - "segment_data", - "property \"filters\" must be an array with at least one member", - [] - ] - ]}, - {[["foo", "bar"]], - [["segment_data", "#/filters/0: Invalid filter [\"foo\", \"bar\"]", []]]}, + for {filters, expected_error} <- [ + {[], "segment_data property \"filters\" must be an array with at least one member"}, + {[["foo", "bar"]], "segment_data #/filters/0: Invalid filter [\"foo\", \"bar\"]"}, {[["not", ["is", "visit:entry_page", ["/campaigns/:campaign_name"]]]], - [["segment_data", "Invalid filters. Deep filters are not supported.", []]]}, + "segment_data Invalid filters. Deep filters are not supported."}, {[ [ "or", @@ -439,13 +437,7 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do ] ] ], - [ - [ - "segment_data", - "Invalid filters. Dimension `event:goal` can only be filtered at the top level.", - [] - ] - ]} + "segment_data Invalid filters. Dimension `event:goal` can only be filtered at the top level."} ] do test "prevents owners from updating segments to invalid filters #{inspect(filters)} with error 400", %{ @@ -467,11 +459,104 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do }) assert json_response(conn, 400) == %{ - "errors" => unquote(expected_errors) + "error" => unquote(expected_error) } end end + test "prevents editors from updating segments name beyond 255 characters with error 400", + %{ + conn: conn, + user: user, + site: site + } do + segment = + insert(:segment, + site: site, + name: "any name", + type: :personal, + owner: user + ) + + conn = + patch(conn, "/api/#{site.domain}/segments/#{segment.id}", %{ + "name" => String.duplicate("a", 256) + }) + + assert json_response(conn, 400) == %{ + "error" => "name should be at most 255 byte(s)" + } + end + + test "updating a segment containing a goal that has been deleted, with the deleted goal still in filters, fails", + %{ + conn: conn, + user: user, + site: site + } do + segment = + insert(:segment, + site: site, + name: "any name", + type: :personal, + owner: user, + segment_data: %{"filters" => [["is", "event:goal", ["Signup"]]]} + ) + + conn = + patch(conn, "/api/#{site.domain}/segments/#{segment.id}", %{ + "segment_data" => %{ + "filters" => [["is", "event:goal", ["Signup"]], ["is", "event:page", ["/register"]]] + } + }) + + assert json_response(conn, 400) == %{ + "error" => + "segment_data Invalid filters. The goal `Signup` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals" + } + end + + test "a segment containing a goal that has been deleted can be updated to not contain the goal", + %{ + conn: conn, + user: user, + site: site + } do + segment = + insert(:segment, + site: site, + name: "any name", + type: :personal, + owner: user, + segment_data: %{"filters" => [["is", "event:goal", ["Signup"]]]} + ) + + insert(:goal, site: site, event_name: "a new goal") + + response = + patch(conn, "/api/#{site.domain}/segments/#{segment.id}", %{ + "segment_data" => %{ + "filters" => [["is", "event:goal", ["a new goal"]]] + } + }) + |> json_response(200) + + assert_matches ^strict_map(%{ + "id" => ^segment.id, + "name" => ^segment.name, + "type" => ^any(:string, ~r/#{segment.type}/), + "segment_data" => + ^strict_map(%{ + "filters" => [ + ["is", "event:goal", ["a new goal"]] + ] + }), + "owner_id" => ^user.id, + "inserted_at" => ^any(:iso8601_naive_datetime), + "updated_at" => ^any(:iso8601_naive_datetime) + }) = response + end + test "editors can update a segment", %{conn: conn, user: user} do site = new_site() add_guest(site, user: user, role: :editor) @@ -531,12 +616,33 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do verify_segment_in_db(segment) end + test "even site owners can't delete personal segments of other users", + %{conn: conn, site: site} do + other_user = add_guest(site, role: :editor) + + segment = + insert(:segment, + site: site, + owner_id: other_user.id, + name: "any", + type: :personal + ) + + conn = + delete(conn, "/api/#{site.domain}/segments/#{segment.id}") + + assert %{"error" => "Segment not found with ID \"#{segment.id}\""} == + json_response(conn, 404) + + verify_segment_in_db(segment) + end + for %{role: role, type: type} <- [ %{role: :viewer, type: :personal}, %{role: :editor, type: :personal}, %{role: :editor, type: :site} ] do - test "#{role} can delete segment with type \"#{type}\" successfully", + test "#{role} can delete their own segment with type \"#{type}\" successfully", %{conn: conn, user: user} do site = new_site() add_guest(site, user: user, role: unquote(role)) @@ -566,6 +672,36 @@ defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do verify_no_segment_in_db(segment) end end + + test "site owner can delete a site segment owned by someone else, even if it contains a non-existing goal", + %{conn: conn, site: site} do + other_user = add_guest(site, role: :editor) + + segment = + insert(:segment, + site: site, + owner: other_user, + name: "any", + type: :site, + segment_data: %{"filters" => [["is", "event:goal", ["non-existing goal"]]]} + ) + + response = + delete(conn, "/api/#{site.domain}/segments/#{segment.id}") + |> json_response(200) + + assert %{ + "owner_id" => other_user.id, + "id" => segment.id, + "name" => segment.name, + "segment_data" => segment.segment_data, + "type" => "site", + "inserted_at" => NaiveDateTime.to_iso8601(segment.inserted_at), + "updated_at" => NaiveDateTime.to_iso8601(segment.updated_at) + } == response + + verify_no_segment_in_db(segment) + end end defp verify_segment_in_db(segment) do diff --git a/test/plausible_web/controllers/stats_controller_test.exs b/test/plausible_web/controllers/stats_controller_test.exs index 52ecadeed5b0..c94a5a27b6fb 100644 --- a/test/plausible_web/controllers/stats_controller_test.exs +++ b/test/plausible_web/controllers/stats_controller_test.exs @@ -22,10 +22,12 @@ defmodule PlausibleWeb.StatsControllerTest do assert text_of_attr(resp, @react_container, "data-funnels-opted-out") == "false" assert text_of_attr(resp, @react_container, "data-props-opted-out") == "false" assert text_of_attr(resp, @react_container, "data-props-available") == "true" + assert text_of_attr(resp, @react_container, "data-site-segments-available") == "true" assert text_of_attr(resp, @react_container, "data-funnels-available") == "true" assert text_of_attr(resp, @react_container, "data-has-props") == "false" assert text_of_attr(resp, @react_container, "data-logged-in") == "false" assert text_of_attr(resp, @react_container, "data-embedded") == "" + assert text_of_attr(resp, @react_container, "data-members") == "null" [{"div", attrs, _}] = find(resp, @react_container) assert Enum.all?(attrs, fn {k, v} -> is_binary(k) and is_binary(v) end) @@ -115,11 +117,14 @@ defmodule PlausibleWeb.StatsControllerTest do describe "GET /:domain - as a logged in user" do setup [:create_user, :log_in, :create_site] - test "can view stats of a website I've created", %{conn: conn, site: site} do + test "can view stats of a website I've created", %{conn: conn, site: site, user: user} do populate_stats(site, [build(:pageview)]) conn = get(conn, "/" <> site.domain) resp = html_response(conn, 200) assert text_of_attr(resp, @react_container, "data-logged-in") == "true" + + assert text_of_attr(resp, @react_container, "data-members") == + "{\"#{user.id}\":\"#{user.name}\"}" end test "can view stats of a website I've created, enforcing pageviews check skip", %{