From d0a1827e0cd679fdedb5538a9f4899c8bc07d3bf Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Thu, 14 Nov 2024 16:09:11 +0200 Subject: [PATCH 01/18] Squashed saved segments --- assets/js/dashboard.tsx | 11 +- assets/js/dashboard/components/dropdown.tsx | 44 +- assets/js/dashboard/dashboard-keybinds.tsx | 6 +- assets/js/dashboard/datepicker.tsx | 4 +- assets/js/dashboard/nav-menu/filter-menu.tsx | 5 +- .../dashboard/nav-menu/filter-pills-list.tsx | 13 +- assets/js/dashboard/nav-menu/filters-bar.tsx | 57 ++- .../js/dashboard/segments/segment-actions.tsx | 253 +++++++++++ .../js/dashboard/segments/segment-modals.tsx | 194 ++++++++ .../dashboard/segments/segments-dropdown.tsx | 376 +++++++++++++++ assets/js/dashboard/segments/segments.ts | 46 ++ assets/js/dashboard/site-context.tsx | 6 +- assets/js/dashboard/site-switcher.js | 2 +- .../stats/modals/filter-modal-row.js | 15 +- assets/js/dashboard/stats/modals/modal.js | 51 ++- assets/js/dashboard/user-context.tsx | 23 +- assets/js/dashboard/util/filters.js | 89 +++- assets/js/types/query-api.d.ts | 11 +- assets/test-utils/app-context-providers.tsx | 5 +- lib/plausible/helpers/list-traverse.ex | 56 +++ lib/plausible/segment.ex | 95 ++++ lib/plausible/segment/segments.md | 70 +++ lib/plausible/site.ex | 1 + lib/plausible/stats/filter_suggestions.ex | 6 + lib/plausible/stats/filters/filters.ex | 8 +- lib/plausible/stats/filters/filters_parser.ex | 134 ++++++ lib/plausible/stats/filters/query_parser.ex | 171 ++----- lib/plausible/stats/filters/segments.ex | 72 +++ .../stats/legacy/legacy_query_builder.ex | 11 + lib/plausible/stats/query.ex | 1 + lib/plausible/stats/query_optimizer.ex | 17 +- .../api/external_stats_controller.ex | 10 +- lib/plausible_web/controllers/api/helpers.ex | 7 + .../api/internal/segments_controller.ex | 146 ++++++ .../controllers/api/stats_controller.ex | 46 +- .../controllers/stats_controller.ex | 13 +- lib/plausible_web/router.ex | 11 + .../templates/stats/stats.html.heex | 2 + priv/json-schemas/query-api-schema.json | 26 +- test/plausible/segment_schema_test.exs | 92 ++++ test/plausible/stats/query_parser_test.exs | 4 +- .../segments_controller_test.exs | 428 ++++++++++++++++++ .../api/stats_controller/countries_test.exs | 68 +++ test/support/factory.ex | 6 + 44 files changed, 2442 insertions(+), 270 deletions(-) create mode 100644 assets/js/dashboard/segments/segment-actions.tsx create mode 100644 assets/js/dashboard/segments/segment-modals.tsx create mode 100644 assets/js/dashboard/segments/segments-dropdown.tsx create mode 100644 assets/js/dashboard/segments/segments.ts create mode 100644 lib/plausible/helpers/list-traverse.ex create mode 100644 lib/plausible/segment.ex create mode 100644 lib/plausible/segment/segments.md create mode 100644 lib/plausible/stats/filters/filters_parser.ex create mode 100644 lib/plausible/stats/filters/segments.ex create mode 100644 lib/plausible_web/controllers/api/internal/segments_controller.ex create mode 100644 test/plausible/segment_schema_test.exs create mode 100644 test/plausible_web/controllers/api/internal_controller/segments_controller_test.exs diff --git a/assets/js/dashboard.tsx b/assets/js/dashboard.tsx index ad44acc74dfc..0498b63bf6b2 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/components/dropdown.tsx b/assets/js/dashboard/components/dropdown.tsx index c3397211f192..9941ee169fad 100644 --- a/assets/js/dashboard/components/dropdown.tsx +++ b/assets/js/dashboard/components/dropdown.tsx @@ -13,6 +13,7 @@ import { AppNavigationLink, AppNavigationTarget } from '../navigation/use-app-navigate' +import { NavigateOptions } from 'react-router-dom' export const ToggleDropdownButton = forwardRef< HTMLDivElement, @@ -129,22 +130,43 @@ export const DropdownNavigationLink = ({ children, active, className, + actions, + path, + params, + search, + navigateOptions, + onLinkClick, ...props }: AppNavigationTarget & { - active?: boolean - children: ReactNode - className?: string - onClick?: () => void -}) => ( - , HTMLDivElement> & { + active?: boolean + onLinkClick?: () => void + actions?: ReactNode + }) => ( +
- {children} - + + {children} + + {!!actions && actions} +
) diff --git a/assets/js/dashboard/dashboard-keybinds.tsx b/assets/js/dashboard/dashboard-keybinds.tsx index f40b244df05f..1d6191728a03 100644 --- a/assets/js/dashboard/dashboard-keybinds.tsx +++ b/assets/js/dashboard/dashboard-keybinds.tsx @@ -21,9 +21,5 @@ const ClearFiltersKeybind = () => ( ) export function DashboardKeybinds() { - return ( - <> - - - ) + return <>{false && } // temp disable } diff --git a/assets/js/dashboard/datepicker.tsx b/assets/js/dashboard/datepicker.tsx index ae5fa74a98ab..8d6cf6442a09 100644 --- a/assets/js/dashboard/datepicker.tsx +++ b/assets/js/dashboard/datepicker.tsx @@ -201,7 +201,7 @@ function ComparisonMenu({ s} - onClick={toggleCompareMenuCalendar} + onLinkClick={toggleCompareMenuCalendar} > {COMPARISON_MODES[ComparisonMode.custom]} @@ -250,7 +250,7 @@ function QueryPeriodsMenu({ key={label} active={isActive({ site, query })} search={search} - onClick={onClick || closeMenu} + onLinkClick={onClick || closeMenu} > {label} {!!keyboardKey && {keyboardKey}} diff --git a/assets/js/dashboard/nav-menu/filter-menu.tsx b/assets/js/dashboard/nav-menu/filter-menu.tsx index 63bc7cd67b79..0ead3c6893ee 100644 --- a/assets/js/dashboard/nav-menu/filter-menu.tsx +++ b/assets/js/dashboard/nav-menu/filter-menu.tsx @@ -15,6 +15,7 @@ import { import { PlausibleSite, useSiteContext } from '../site-context' import { filterRoute } from '../router' import { useOnClickOutside } from '../util/use-on-click-outside' +import { SegmentsList } from '../segments/segments-dropdown' export function getFilterListItems({ propsAvailable @@ -26,7 +27,7 @@ export function getFilterListItems({ keyof typeof FILTER_MODAL_TO_FILTER_GROUP > const keysToOmit: Array = - propsAvailable ? [] : ['props'] + propsAvailable ? ['segment'] : ['segment', 'props'] return allKeys .filter((k) => !keysToOmit.includes(k)) .map((modalKey) => ({ modalKey, label: formatFilterGroup(modalKey) })) @@ -63,9 +64,11 @@ export const FilterMenu = () => { > {opened && ( + setOpened(false)} /> {filterListItems.map(({ modalKey, label }) => ( setOpened(false)} active={false} key={modalKey} path={filterRoute.path} diff --git a/assets/js/dashboard/nav-menu/filter-pills-list.tsx b/assets/js/dashboard/nav-menu/filter-pills-list.tsx index 830cae556fc7..95087cd87b3e 100644 --- a/assets/js/dashboard/nav-menu/filter-pills-list.tsx +++ b/assets/js/dashboard/nav-menu/filter-pills-list.tsx @@ -76,17 +76,18 @@ export const FilterPillsList = React.forwardRef< } plainText={plainFilterText(query, filter)} key={index} - onRemoveClick={() => + onRemoveClick={() => { + const newFilters = query.filters.filter( + (_, i) => i !== index + indexAdjustment + ) navigate({ search: (search) => ({ ...search, - filters: query.filters.filter( - (_, i) => i !== index + indexAdjustment - ), - labels: cleanLabels(query.filters, query.labels) + filters: newFilters, + labels: cleanLabels(newFilters, query.labels) }) }) - } + }} > {styledFilterText(query, filter)} diff --git a/assets/js/dashboard/nav-menu/filters-bar.tsx b/assets/js/dashboard/nav-menu/filters-bar.tsx index e0f48e342611..cb2c0af8512b 100644 --- a/assets/js/dashboard/nav-menu/filters-bar.tsx +++ b/assets/js/dashboard/nav-menu/filters-bar.tsx @@ -11,6 +11,10 @@ import { } from '../components/dropdown' import { FilterPillsList, PILL_X_GAP } from './filter-pills-list' import { useQueryContext } from '../query-context' +import { SaveSegmentAction } from '../segments/segment-actions' +import { EditingSegmentState, isSegmentFilter } from '../segments/segments' +import { useLocation } from 'react-router-dom' +import { useUserContext } from '../user-context' const SEE_MORE_GAP_PX = 16 const SEE_MORE_WIDTH_PX = 36 @@ -91,12 +95,34 @@ type VisibilityState = { } export const FiltersBar = () => { + const user = useUserContext(); const containerRef = useRef(null) const pillsRef = useRef(null) const actionsRef = useRef(null) const seeMoreRef = useRef(null) const [visibility, setVisibility] = useState(null) const { query } = useQueryContext() + const { state: locationState } = useLocation() as { + state?: EditingSegmentState + } + const [editingSegment, setEditingSegment] = useState< + null | EditingSegmentState['editingSegment'] + >(null) + + useLayoutEffect(() => { + if (locationState?.editingSegment) { + setEditingSegment(locationState?.editingSegment) + } + if (locationState?.editingSegment === null) { + setEditingSegment(null) + } + }, [locationState?.editingSegment]) + + useLayoutEffect(() => { + if (!query.filters.length) { + setEditingSegment(null) + } + }, [query.filters.length]) const [opened, setOpened] = useState(false) @@ -146,7 +172,7 @@ export const FiltersBar = () => { return (
{ )} + {user.loggedIn && editingSegment === null && + !query.filters.some((f) => isSegmentFilter(f)) && ( + <> + + + + )} + {user.loggedIn && editingSegment !== null && ( + <> + + + + )}
) @@ -206,7 +253,7 @@ export const FiltersBar = () => { export const ClearAction = () => ( ({ ...search, filters: null, @@ -216,3 +263,9 @@ export const ClearAction = () => ( ) + +const VerticalSeparator = () => { + return ( +
+ ) +} diff --git a/assets/js/dashboard/segments/segment-actions.tsx b/assets/js/dashboard/segments/segment-actions.tsx new file mode 100644 index 000000000000..bb83d5b1546b --- /dev/null +++ b/assets/js/dashboard/segments/segment-actions.tsx @@ -0,0 +1,253 @@ +/** @format */ + +import React, { useState, useCallback } from 'react' +import { useAppNavigate } from '../navigation/use-app-navigate' +import { useQueryContext } from '../query-context' +import { useMutation, useQueryClient } from '@tanstack/react-query' +import { DashboardQuery } from '../query' +import { useSiteContext } from '../site-context' +import { + cleanLabels, + plainFilterText, + remapToApiFilters +} from '../util/filters' +import { + EditingSegmentState, + formatSegmentIdAsLabelKey, + parseApiSegmentData, + SavedSegment, + SegmentType +} from './segments' +import { CreateSegmentModal, UpdateSegmentModal } from './segment-modals' +import { useUserContext } from '../user-context' + +type M = 'create segment' | 'update segment' +type O = + | { type: 'create segment' } + | { type: 'update segment'; segment: SavedSegment } + +export const SaveSegmentAction = ({ options }: { options: O[] }) => { + const user = useUserContext() + const site = useSiteContext() + const { query } = useQueryContext() + const [modal, setModal] = useState(null) + const navigate = useAppNavigate() + const openCreateSegment = useCallback(() => { + return setModal('create segment') + }, []) + const openUpdateSegment = useCallback(() => { + return setModal('update segment') + }, []) + const close = useCallback(() => { + return setModal(null) + }, []) + const queryClient = useQueryClient() + const createSegment = useMutation({ + mutationFn: ({ + name, + type, + segment_data + }: { + name: string + type: 'personal' | 'site' + segment_data: { + filters: DashboardQuery['filters'] + labels: DashboardQuery['labels'] + } + }) => { + return fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments`, + { + method: 'POST', + body: JSON.stringify({ + name, + type, + segment_data: { + filters: remapToApiFilters(segment_data.filters), + labels: cleanLabels(segment_data.filters, segment_data.labels) + } + }), + headers: { 'content-type': 'application/json' } + } + ) + .then((res) => res.json()) + .then((d) => ({ + ...d, + segment_data: parseApiSegmentData(d.segment_data) + })) + }, + onSuccess: async (d) => { + navigate({ + search: (search) => { + const filters = [['is', 'segment', [d.id]]] + const labels = cleanLabels(filters, {}, 'segment', { + [formatSegmentIdAsLabelKey(d.id)]: d.name + }) + return { + ...search, + filters, + labels + } + }, + state: { editingSegment: null } as EditingSegmentState + }) + close() + queryClient.invalidateQueries({ queryKey: ['segments'] }) + } + }) + + const patchSegment = useMutation({ + mutationFn: ({ + id, + name, + type, + segment_data + }: { + id: number + name?: string + type?: SegmentType + segment_data?: { + filters: DashboardQuery['filters'] + labels: DashboardQuery['labels'] + } + }) => { + return fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments/${id}`, + { + method: 'PATCH', + body: JSON.stringify({ + name, + type, + ...(segment_data && { + segment_data: { + filters: remapToApiFilters(segment_data.filters), + labels: cleanLabels(segment_data.filters, segment_data.labels) + } + }) + }), + headers: { + 'content-type': 'application/json', + accept: 'application/json' + } + } + ) + .then((res) => res.json()) + .then((d) => ({ + ...d, + segment_data: parseApiSegmentData(d.segment_data) + })) + }, + onSuccess: async (d) => { + navigate({ + search: (search) => { + const filters = [['is', 'segment', [d.id]]] + const labels = cleanLabels(filters, {}, 'segment', { + [formatSegmentIdAsLabelKey(d.id)]: d.name + }) + return { + ...search, + filters, + labels + } + }, + state: { editingSegment: null } as EditingSegmentState + }) + close() + queryClient.invalidateQueries({ queryKey: ['segments'] }) + } + }) + + if (!user.loggedIn) { + return null + } + + const segmentNamePlaceholder = query.filters.reduce( + (combinedName, filter) => + combinedName.length > 100 + ? combinedName + : `${combinedName}${combinedName.length ? ' and ' : ''}${plainFilterText(query, filter)}`, + '' + ) + + const option = options.find((o) => o.type === modal) + const buttonClass = + 'whitespace-nowrap rounded font-semibold text-sm leading-tight p-2 h-9 text-gray-500 hover:text-indigo-700 dark:hover:text-indigo-500 disabled:cursor-not-allowed' + return ( +
+ {options.map((o) => { + if (o.type === 'create segment') { + return ( + + ) + } + if (o.type === 'update segment') { + const canEdit = + (o.segment.type === SegmentType.personal && + o.segment.owner_id === user.id) || + (o.segment.type === SegmentType.site && + ['admin', 'owner', 'super_admin'].includes(user.role)) + + return ( + + ) + } + })} + {modal === 'create segment' && ( + o.type === 'update segment')?.segment} + namePlaceholder={segmentNamePlaceholder} + close={close} + onSave={({ name, type }) => + createSegment.mutate({ + name, + type, + segment_data: { + filters: query.filters, + labels: query.labels + } + }) + } + /> + )} + {option?.type === 'update segment' && ( + + patchSegment.mutate({ + id, + name, + type, + segment_data: { + filters: query.filters, + labels: query.labels + } + }) + } + /> + )} +
+ ) +} diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx new file mode 100644 index 000000000000..20060952c8e4 --- /dev/null +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -0,0 +1,194 @@ +/** @format */ + +import React, { useState } from 'react' +import ModalWithRouting from '../stats/modals/modal' +import classNames from 'classnames' +import { SavedSegment, SegmentType } from './segments' + +const buttonClass = + 'h-12 text-md font-medium py-2 px-3 rounded border dark:border-gray-100 dark:text-gray-100' + +export const CreateSegmentModal = ({ + segment, + close, + onSave, + canTogglePersonal, + namePlaceholder +}: { + segment?: SavedSegment + close: () => void + onSave: (input: Pick) => void + canTogglePersonal: boolean + namePlaceholder: string +}) => { + const [name, setName] = useState( + segment?.name ? `Copy of ${segment.name}` : '' + ) + const [type, setType] = useState(SegmentType.personal) + + return ( + +

+ Create segment +

+ + setName(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" + /> +
+ Add a name to your segment to make it easier to find +
+
+ + + Show this segment for all site users + +
+
+ + +
+ + ) +} + +export const UpdateSegmentModal = ({ + close, + onSave, + segment, + canTogglePersonal, + namePlaceholder +}: { + close: () => void + onSave: (input: Pick) => void + segment: SavedSegment + canTogglePersonal: boolean + namePlaceholder: string +}) => { + const [name, setName] = useState(segment.name) + const [type, setType] = useState(segment.type) + + return ( + +

+ Update segment +

+ + setName(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" + /> +
+ Add a name to your segment to make it easier to find +
+
+ + + Show this segment for all site users + +
+
+ + +
+ + ) +} diff --git a/assets/js/dashboard/segments/segments-dropdown.tsx b/assets/js/dashboard/segments/segments-dropdown.tsx new file mode 100644 index 000000000000..3d4f52d0e666 --- /dev/null +++ b/assets/js/dashboard/segments/segments-dropdown.tsx @@ -0,0 +1,376 @@ +/** @format */ + +import React from 'react' +import { + DropdownLinkGroup, + DropdownNavigationLink +} from '../components/dropdown' +import { useQueryContext } from '../query-context' +import { useSiteContext } from '../site-context' +import { + EditingSegmentState, + formatSegmentIdAsLabelKey, + isSegmentFilter, + parseApiSegmentData, + SavedSegment, + SegmentData, + SegmentType +} from './segments' +import { + QueryFunction, + useMutation, + useQuery, + useQueryClient +} from '@tanstack/react-query' +import { cleanLabels } from '../util/filters' +import { useAppNavigate } from '../navigation/use-app-navigate' +import classNames from 'classnames' +import { Tooltip } from '../util/tooltip' +import { formatDayShort, parseUTCDate } from '../util/date' +import { useUserContext } from '../user-context' + +export const SegmentsList = ({ closeList }: { closeList: () => void }) => { + // const user = useUserContext(); + const { query } = useQueryContext() + const site = useSiteContext() + const { data } = useQuery({ + queryKey: ['segments'], + placeholderData: (previousData) => previousData, + queryFn: async () => { + const response = await fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments`, + { + method: 'GET', + headers: { + 'content-type': 'application/json', + accept: 'application/json' + } + } + ).then( + ( + res + ): Promise< + (SavedSegment & { + owner_id: number + inserted_at: string + updated_at: string + })[] + > => res.json() + ) + return response + } + }) + + const segmentFilter = query.filters.find(isSegmentFilter) + const appliedSegmentIds = (segmentFilter ? segmentFilter[2] : []) as number[] + + return ( + !!data?.length && ( + + {data.map((s) => { + const authorLabel = (() => { + if (!site.members) { + return '' + } + + if (!s.owner_id || !site.members[s.owner_id]) { + return '(Deleted User)' + } + + // if (s.owner_id === user.id) { + // return 'You' + // } + + return site.members[s.owner_id] + })() + + const showUpdatedAt = s.updated_at !== s.inserted_at + + return ( + +
+ { + { + [SegmentType.personal]: 'Personal segment', + [SegmentType.site]: 'Segment' + }[s.type] + } +
+
+ {`Created at ${formatDayShort(parseUTCDate(s.inserted_at))}`} + {!showUpdatedAt && !!authorLabel && ` by ${authorLabel}`} +
+ {showUpdatedAt && ( +
+ {`Last updated at ${formatDayShort(parseUTCDate(s.updated_at))}`} + {!!authorLabel && ` by ${authorLabel}`} +
+ )} + + } + > + + + ) + })} + + ) + ) +} + +const SegmentLink = ({ + id, + name, + type, + owner_id, + appliedSegmentIds, + closeList +}: SavedSegment & { appliedSegmentIds: number[]; closeList: () => void }) => { + const user = useUserContext() + const canSeeActions = user.loggedIn + const canDeleteSegment = + user.loggedIn && + ((owner_id === user.id && type === SegmentType.personal) || + (type === SegmentType.site && + ['admin', 'owner', 'super_admin'].includes(user.role))) + const site = useSiteContext() + const { query } = useQueryContext() + const queryClient = useQueryClient() + + const queryKey = ['segments', id] as const + + const getSegmentFn: QueryFunction< + { segment_data: SegmentData } & SavedSegment, + typeof queryKey + > = async ({ queryKey: [_, id] }) => { + const res = await fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments/${id}`, + { + method: 'GET', + headers: { + 'content-type': 'application/json', + accept: 'application/json' + } + } + ) + const d = await res.json() + return { + ...d, + segment_data: parseApiSegmentData(d.segment_data) + } + } + + const navigate = useAppNavigate() + + const getSegment = useQuery({ + enabled: false, + queryKey: queryKey, + queryFn: getSegmentFn + }) + + const prefetchSegment = () => + queryClient.prefetchQuery({ + queryKey, + queryFn: getSegmentFn, + staleTime: 120_000 + }) + + const fetchSegment = () => + queryClient.fetchQuery({ + queryKey, + queryFn: getSegmentFn + }) + + const editSegment = async () => { + try { + const data = getSegment.data ?? (await fetchSegment()) + navigate({ + search: (search) => ({ + ...search, + filters: data.segment_data.filters, + labels: data.segment_data.labels + }), + state: { + editingSegment: { + id: data.id, + name: data.name, + type: data.type, + owner_id: data.owner_id + } + } as EditingSegmentState + }) + closeList() + } catch (_error) { + return + } + } + + return ( + { + const otherFilters = query.filters.filter((f) => !isSegmentFilter(f)) + const updatedSegmentIds = appliedSegmentIds.includes(id) + ? appliedSegmentIds.filter((i) => i !== id) + : [...appliedSegmentIds, 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 + }) + } + }} + actions={ + !canSeeActions ? null : ( + <> + + + + ) + } + > + {name} + + ) +} + +const EditSegment = ({ + className, + onClick +}: { + onClick: () => Promise + className?: string +}) => { + return ( + + ) +} + +const DeleteSegment = ({ + disabled, + className, + segment +}: { + disabled?: boolean + className?: string + segment: SavedSegment +}) => { + const queryClient = useQueryClient() + const site = useSiteContext() + const navigate = useAppNavigate() + const { query } = useQueryContext() + const deleteSegment = useMutation({ + mutationFn: (data: SavedSegment) => { + return fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments/${data.id}`, + { + method: 'DELETE' + } + ) + .then((res) => res.json()) + .then((d) => ({ + ...d, + segment_data: parseApiSegmentData(d.segment_data) + })) + }, + onSuccess: (_d): void => { + queryClient.invalidateQueries({ queryKey: ['segments'] }) + + const segmentFilterIndex = query.filters.findIndex(isSegmentFilter) + if (segmentFilterIndex < 0) { + return + } + + const filter = query.filters[segmentFilterIndex] + const clauses = filter[2] + const updatedSegmentIds = clauses.filter((c) => c !== segment.id) + + if (updatedSegmentIds.length === clauses.length) { + return + } + + const newFilters = !updatedSegmentIds.length + ? query.filters.filter((_f, index) => index !== segmentFilterIndex) + : [ + ...query.filters.slice(0, segmentFilterIndex), + [filter[0], filter[1], updatedSegmentIds], + ...query.filters.slice(segmentFilterIndex + 1) + ] + + navigate({ + search: (s) => { + return { + ...s, + filters: newFilters, + labels: cleanLabels(newFilters, query.labels) + } + } + }) + } + }) + + return ( + + ) +} + +const DeleteSegmentIcon = () => ( + + + +) + +const EditSegmentIcon = () => ( + + + +) diff --git a/assets/js/dashboard/segments/segments.ts b/assets/js/dashboard/segments/segments.ts new file mode 100644 index 000000000000..c782e056d9ef --- /dev/null +++ b/assets/js/dashboard/segments/segments.ts @@ -0,0 +1,46 @@ +/** @format */ + +import { Filter } from '../query' +import { remapFromApiFilters } from '../util/filters' + +export enum SegmentType { + personal = 'personal', + site = 'site' +} + +export type SavedSegment = { + id: number + name: string + type: SegmentType + owner_id: number +} + +export type SegmentData = { + filters: Filter[] + labels: Record +} + +export type EditingSegmentState = { + /** null means to definitively close the edit mode */ + editingSegment: SavedSegment | null +} + +const SEGMENT_LABEL_KEY_PREFIX = 'segment-' + +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 +}: SegmentData): SegmentData => ({ + filters: remapFromApiFilters(filters), + ...rest +}) diff --git a/assets/js/dashboard/site-context.tsx b/assets/js/dashboard/site-context.tsx index 80e6f2ba5219..6d781ea36bc5 100644 --- a/assets/js/dashboard/site-context.tsx +++ b/assets/js/dashboard/site-context.tsx @@ -21,7 +21,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!) } } @@ -52,7 +53,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 4a94245994ae..06c5258741d7 100644 --- a/assets/js/dashboard/site-switcher.js +++ b/assets/js/dashboard/site-switcher.js @@ -261,7 +261,7 @@ export default class SiteSwitcher extends React.Component { leaveTo="opacity-0 scale-95" >
(this.dropDownNode = node)} >
diff --git a/assets/js/dashboard/stats/modals/filter-modal-row.js b/assets/js/dashboard/stats/modals/filter-modal-row.js index 03b171022ae3..b1073587e0b7 100644 --- a/assets/js/dashboard/stats/modals/filter-modal-row.js +++ b/assets/js/dashboard/stats/modals/filter-modal-row.js @@ -15,6 +15,10 @@ import { import { apiPath } from '../../util/url' import { useQueryContext } from '../../query-context' import { useSiteContext } from '../../site-context' +import { + formatSegmentIdAsLabelKey, + isSegmentFilter +} from '../../segments/segments' export default function FilterModalRow({ filter, labels, onUpdate }) { const { query } = useQueryContext() @@ -27,16 +31,19 @@ export default function FilterModalRow({ filter, labels, onUpdate }) { value, label: getLabel(labels, filterKey, value) })), - // eslint-disable-next-line react-hooks/exhaustive-deps - [filter, labels] + [clauses, labels, filterKey] ) function onComboboxSelect(selection) { const newClauses = selection.map(({ value }) => value) const newLabels = Object.fromEntries( - selection.map(({ label, value }) => [value, label]) + selection.map(({ label, value }) => { + if (isSegmentFilter(filter)) { + return [formatSegmentIdAsLabelKey(value), label] + } + return [value, label] + }) ) - onUpdate([operation, filterKey, newClauses], newLabels) } diff --git a/assets/js/dashboard/stats/modals/modal.js b/assets/js/dashboard/stats/modals/modal.js index 2b7114cb83cf..38153369593d 100644 --- a/assets/js/dashboard/stats/modals/modal.js +++ b/assets/js/dashboard/stats/modals/modal.js @@ -1,8 +1,9 @@ import React from "react"; import { createPortal } from "react-dom"; -import { NavigateKeybind } from '../../keybinding' +import { Keybind } from '../../keybinding' import { rootRoute } from "../../router"; import { useAppNavigate } from "../../navigation/use-app-navigate"; +import classNames from "classnames"; // This corresponds to the 'md' breakpoint on TailwindCSS. const MD_WIDTH = 768; @@ -41,20 +42,13 @@ class Modal extends React.Component { return; } - this.close() + this.props.close() } handleResize() { this.setState({ viewport: window.innerWidth }); } - close() { - this.props.navigate({ - path: rootRoute.path, - search: (search) => search, - }) - } - /** * @description * Decide whether to set max-width, and if so, to what. @@ -76,22 +70,18 @@ class Modal extends React.Component { render() { return createPortal( - <> - search }} /> -
-
- -
- {this.props.children} -
+
+
+ +
+ {this.props.children}
- -, +
, document.getElementById("modal_root"), ); } @@ -99,5 +89,18 @@ class Modal extends React.Component { export default function ModalWithRouting(props) { const navigate = useAppNavigate() - return + const defaultCloseHandler = () => + navigate({ path: rootRoute.path, search: (s) => s }) + const closeHandler = props.close ?? defaultCloseHandler + return ( + <> + + + navigate({ path: rootRoute.path, search: (search) => search }) + } + {...props} + /> + + ) } diff --git a/assets/js/dashboard/user-context.tsx b/assets/js/dashboard/user-context.tsx index 0ff62b395c05..bf3e4e0f8c80 100644 --- a/assets/js/dashboard/user-context.tsx +++ b/assets/js/dashboard/user-context.tsx @@ -8,28 +8,27 @@ export enum Role { } const userContextDefaultValue = { - role: Role.viewer, + id: null, + role: null, loggedIn: false -} +} as + | { loggedIn: false; id: null; role: null } + | { loggedIn: true; id: number; role: Role } + +type UserContextValue = typeof userContextDefaultValue -const UserContext = createContext(userContextDefaultValue) +const UserContext = createContext(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/filters.js b/assets/js/dashboard/util/filters.js index d8d13a70dab5..564681e8df5f 100644 --- a/assets/js/dashboard/util/filters.js +++ b/assets/js/dashboard/util/filters.js @@ -3,6 +3,10 @@ import React, { useMemo } from 'react' import * as api from '../api' import { useQueryContext } from '../query-context' +import { + formatSegmentIdAsLabelKey, + isSegmentFilter +} from '../segments/segments' export const FILTER_MODAL_TO_FILTER_GROUP = { page: ['page', 'entry_page', 'exit_page'], @@ -14,7 +18,8 @@ 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 const FILTER_GROUP_TO_MODAL_TYPE = Object.fromEntries( @@ -76,9 +81,13 @@ const ESCAPED_PIPE = '\\|' export function getLabel(labels, filterKey, value) { if (['country', 'region', 'city'].includes(filterKey)) { return labels[value] - } else { - return value } + + if (isSegmentFilter(['is', filterKey, []])) { + return labels[formatSegmentIdAsLabelKey(value)] + } + + return value } export function getPropertyKeyFromFilterKey(filterKey) { @@ -201,11 +210,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]) { @@ -215,7 +231,7 @@ export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) { if ( mergedFilterKey && - ['country', 'region', 'city'].includes(mergedFilterKey) + ['country', 'region', 'city', 'segment'].includes(mergedFilterKey) ) { result = { ...result, @@ -226,21 +242,55 @@ 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:' -export function serializeApiFilters(filters) { - const apiFilters = filters.map(([operation, filterKey, clauses]) => { - let apiFilterKey = `visit:${filterKey}` - if ( - filterKey.startsWith(EVENT_PROPS_PREFIX) || - EVENT_FILTER_KEYS.has(filterKey) - ) { - apiFilterKey = `event:${filterKey}` - } - return [operation, apiFilterKey, clauses] +function remapFilterKey(filterKey) { + if (NO_PREFIX_KEYS.has(filterKey)) { + return filterKey + } + if (EVENT_FILTER_KEYS.has(filterKey)) { + 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(([operation, filterKey, clauses]) => { + return [operation, remapFilterKey(filterKey), clauses] + }) +} + +export function remapFromApiFilters(apiFilters) { + return apiFilters.map(([operation, apiFilterKey, clauses]) => { + return [operation, remapApiFilterKey(apiFilterKey), clauses] }) +} - return JSON.stringify(apiFilters) +export function serializeApiFilters(filters) { + return JSON.stringify(remapToApiFilters(filters)) } export function fetchSuggestions(apiPath, query, input, additionalFilter) { @@ -291,7 +341,8 @@ export const formattedFilters = { page: 'Page', hostname: 'Hostname', entry_page: 'Entry Page', - exit_page: 'Exit Page' + exit_page: 'Exit Page', + segment: 'Segment' } export function parseLegacyFilter(filterKey, rawValue) { diff --git a/assets/js/types/query-api.d.ts b/assets/js/types/query-api.d.ts index 06121a8166e3..4f11ad048329 100644 --- a/assets/js/types/query-api.d.ts +++ b/assets/js/types/query-api.d.ts @@ -64,7 +64,7 @@ export type CustomPropertyFilterDimensions = string; export type GoalDimension = "event:goal"; export type TimeDimensions = "time" | "time:month" | "time:week" | "time:day" | "time:hour"; export type FilterTree = FilterEntry | FilterAndOr | FilterNot; -export type FilterEntry = FilterWithoutGoals | FilterWithGoals; +export type FilterEntry = FilterWithoutGoals | FilterWithGoals | FilterForSegment; /** * @minItems 3 * @maxItems 3 @@ -92,6 +92,15 @@ export type FilterWithGoals = [ * filter operation */ export type FilterOperationWithGoals = "is" | "contains"; +/** + * @minItems 3 + * @maxItems 3 + */ +export type FilterForSegment = [FilterOperationForSegments, "segment", Clauses]; +/** + * filter operation + */ +export type FilterOperationForSegments = "is"; /** * @minItems 2 * @maxItems 2 diff --git a/assets/test-utils/app-context-providers.tsx b/assets/test-utils/app-context-providers.tsx index 5d26d3f2f5cb..f3558309c619 100644 --- a/assets/test-utils/app-context-providers.tsx +++ b/assets/test-utils/app-context-providers.tsx @@ -40,7 +40,8 @@ export const TestContextProviders = ({ isDbip: false, flags: {}, validIntervalsByPeriod: {}, - shared: false + shared: false, + members: {1: "Test User"} } const site = { ...defaultSite, ...siteOptions } @@ -58,7 +59,7 @@ export const TestContextProviders = ({ return ( // not interactive component, default value is suitable - + parse_list(["1", "2", "3"], &Integer.parse/1) + {:ok, [1, 2, 3]} + + iex> parse_list(["1", "not_a_number", "3"], &Integer.parse/1) + {:error, :invalid} + + """ + @spec parse_list(list(), (any() -> {:ok, any()} | {:error, any()})) :: + {:ok, list()} | {:error, any()} + def parse_list(list, parser_function) do + Enum.reduce_while(list, {:ok, []}, fn value, {:ok, results} -> + case parser_function.(value) do + {:ok, result} -> {:cont, {:ok, results ++ [result]}} + {:error, _} = error -> {:halt, error} + end + end) + end + + @doc """ + Validates a list of values using a provided parser function. + + Returns `:ok` if all values are valid, or `{:error, reason}` on first invalid value. + """ + @spec validate_list(list(), (any() -> :ok | {:error, any()})) :: :ok | {:error, any()} + def validate_list(list, parser_function) do + Enum.reduce_while(list, :ok, fn value, :ok -> + case parser_function.(value) do + :ok -> {:cont, :ok} + {:error, _} = error -> {:halt, error} + end + end) + end +end diff --git a/lib/plausible/segment.ex b/lib/plausible/segment.ex new file mode 100644 index 000000000000..fc5809e15b19 --- /dev/null +++ b/lib/plausible/segment.ex @@ -0,0 +1,95 @@ +defmodule Plausible.Segment do + @moduledoc """ + Schema for segments. Segments are saved filter combinations. + """ + use Plausible + use Ecto.Schema + import Ecto.Changeset + + @segment_types [:personal, :site] + + @type t() :: %__MODULE__{} + + @derive {Jason.Encoder, + only: [ + :id, + :name, + :type, + :segment_data, + :owner_id, + :inserted_at, + :updated_at + ]} + + schema "segments" do + field :name, :string + field :type, Ecto.Enum, values: @segment_types + field :segment_data, :map + + # owner ID can be null (aka segment is dangling) when the original owner is deassociated from the site + # the segment is dangling until another user edits it: the editor becomes the new owner + belongs_to :owner, Plausible.Auth.User, foreign_key: :owner_id + belongs_to :site, Plausible.Site + + timestamps() + end + + def changeset(segment, attrs) do + segment + |> cast(attrs, [ + :name, + :segment_data, + :site_id, + :type, + :owner_id + ]) + |> validate_required([:name, :segment_data, :site_id, :type, :owner_id]) + |> foreign_key_constraint(:site_id) + |> foreign_key_constraint(:owner_id) + |> validate_only_known_properties_present() + |> validate_segment_data_filters() + |> validate_segment_data_labels() + end + + defp validate_only_known_properties_present(changeset) do + case get_field(changeset, :segment_data) do + segment_data when is_map(segment_data) -> + if Enum.any?(Map.keys(segment_data) -- ["filters", "labels"]) do + add_error( + changeset, + :segment_data, + "must not contain any other property except \"filters\" and \"labels\"" + ) + else + changeset + end + + _ -> + changeset + end + end + + defp validate_segment_data_filters(changeset) do + case get_field(changeset, :segment_data) do + %{"filters" => filters} when is_list(filters) and length(filters) > 0 -> + changeset + + _ -> + add_error( + changeset, + :segment_data, + "property \"filters\" must be an array with at least one member" + ) + end + end + + defp validate_segment_data_labels(changeset) do + case get_field(changeset, :segment_data) do + %{"labels" => labels} when not is_map(labels) -> + add_error(changeset, :segment_data, "property \"labels\" must be map or nil") + + _ -> + changeset + end + end +end diff --git a/lib/plausible/segment/segments.md b/lib/plausible/segment/segments.md new file mode 100644 index 000000000000..4c6a203450ce --- /dev/null +++ b/lib/plausible/segment/segments.md @@ -0,0 +1,70 @@ +# Saved segments + +## Definitions + +| Term | Definition | +|------|------------| +| **Segment Owner** | Usually the user who authored the segment | +| **Personal Segment** | A segment that has personal flag set as true and the user is the segment owner | +| **Personal Segments of Other Users** | A segment that has personal flag set as true and the user is not the segment owner | +| **Site Segment** | A segment that has personal flag set to false | +| **Segment Contents** | A list of filters | + +## Capabilities + +| Capability | Public | Viewer | Admin | Owner | Super Admin | +|------------|--------|--------|-------|-------|-------------| +| Can view data filtered by any segment they know the ID of | ✅ | ✅ | ✅ | ✅ | ✅ | +| Can see contents of any segment they know the ID of | | ✅ | ✅ | ✅ | ✅ | +| Can make API requests filtered by any segment they know the ID of | | ✅ | ✅ | ✅ | ✅ | +| Can create personal segments | | ✅ | ✅ | ✅ | ✅ | +| Can see list of personal segments | | ✅ | ✅ | ✅ | ✅ | +| Can edit personal segments | | ✅ | ✅ | ✅ | ✅ | +| Can delete personal segments | | ✅ | ✅ | ✅ | ✅ | +| Can set personal segments to be site segments [$] | | | ✅ | ✅ | ✅ | +| Can set site segments to be personal segments [$] | | | ✅ | ✅ | ✅ | +| Can see list of site segments [$] | ✅ | ✅ | ✅ | ✅ | ✅ | +| Can edit site segments [$] | | | ✅ | ✅ | ✅ | +| Can delete site segments [$] | | | ✅ | ✅ | ✅ | +| Can list personal segments of other users | | | | | | +| Can edit personal segments of other users | | | | | | +| Can delete personal segments of other users | | | | | | + +### Notes + +* __[$]__: functionality available on Business plan or above + +## Segment lifecycle + +| Action | Outcome | +|--------|---------| +| A user* selects filters that constitute the segment, chooses name, chooses whether it's site segment or not*, clicks "update segment" | Segment created (with user as segment owner) | +| A user* views the contents of an existing segment, chooses name, chooses whether it's site segment or not*, clicks "save as new segment" | Segment created (with user as segment owner) | +| Segment owner* clicks edit segment, changes segment name or adds/removes/edits filters, chooses whether it's site segment or not*, clicks "update segment" | Segment updated | +| Any user* except the segment owner opens the segment for editing and clicks save, with or without changes | Segment updated (with the user becoming the new segment owner) | +| Segment owner* deletes segment | Segment deleted | +| Any user* except the segment owner deletes segment | Segment deleted | +| Site deleted | Segment deleted | +| Segment owner is removed from site or deleted from Plausible | If personal segment, segment deleted; if site segment, nothing happens | +| Any user* updates goal name, if site has any segments with "is goal ..." filters for that goal | Segment updated | +| Plausible engineer updates filters schema in backwards incompatible way | Segment updated | + +### Notes + +__*__: if the user has that particular capability + +## Schema + +| Field | Type | Constraints | Comment | +|-------|------|-------------|---------| +| :id | :bigint | null: false | | +| :name | :string | null: false | | +| :type | :enum | default: :personal, null: false | Possible values are :site, :personal. Needed to distinguish between segments that are supposed to be listed site-wide and ones that are listed only for author | +| :segment_data | :map | null: false | Contains the filters array at "filters" key and the labels record at "labels" key | +| :site_id | references(:sites) | on_delete: :delete_all, null: false | | +| :owner_id | references(:users) | on_delete: :nothing, null: false | Used to display author info without repeating author name and email in the database | +| timestamps() | | | Provides inserted_at, updated_at fields | + +## API + +[lib/plausible_web/router.ex](../../plausible_web/router.ex) \ No newline at end of file diff --git a/lib/plausible/site.ex b/lib/plausible/site.ex index 20915c0a37a6..95cb5b29b468 100644 --- a/lib/plausible/site.ex +++ b/lib/plausible/site.ex @@ -47,6 +47,7 @@ defmodule Plausible.Site do has_many :invitations, Plausible.Auth.Invitation has_many :goals, Plausible.Goal, preload_order: [desc: :id] has_many :revenue_goals, Plausible.Goal, where: [currency: {:not, nil}] + has_many :segments, Plausible.Segment, preload_order: [asc: :name] has_one :google_auth, GoogleAuth has_one :weekly_report, Plausible.Site.WeeklyReport has_one :monthly_report, Plausible.Site.MonthlyReport diff --git a/lib/plausible/stats/filter_suggestions.ex b/lib/plausible/stats/filter_suggestions.ex index f3134f198f4e..10b086206a3f 100644 --- a/lib/plausible/stats/filter_suggestions.ex +++ b/lib/plausible/stats/filter_suggestions.ex @@ -160,6 +160,12 @@ defmodule Plausible.Stats.FilterSuggestions do |> wrap_suggestions() end + def filter_suggestions(site, _query, "segment", _filter_search) do + Enum.map(Repo.preload(site, :segments).segments, fn segment -> + %{value: segment.id, label: segment.name} + end) + end + def filter_suggestions(site, query, "prop_key", filter_search) do filter_query = if filter_search == nil, do: "%", else: "%#{filter_search}%" diff --git a/lib/plausible/stats/filters/filters.ex b/lib/plausible/stats/filters/filters.ex index c68e2b3f8651..a72952841d3d 100644 --- a/lib/plausible/stats/filters/filters.ex +++ b/lib/plausible/stats/filters/filters.ex @@ -3,7 +3,7 @@ defmodule Plausible.Stats.Filters do A module for parsing filters used in stat queries. """ - alias Plausible.Stats.Filters.QueryParser + alias Plausible.Stats.Filters.{FiltersParser} alias Plausible.Stats.Filters.{LegacyDashboardFilterParser, StatsAPIFilterParser} @visit_props [ @@ -81,7 +81,7 @@ defmodule Plausible.Stats.Filters do do: LegacyDashboardFilterParser.parse_and_prefix(filters) def parse(filters) when is_list(filters) do - {:ok, parsed_filters} = QueryParser.parse_filters(filters) + {:ok, parsed_filters} = FiltersParser.parse_filters(filters) parsed_filters end @@ -103,8 +103,8 @@ defmodule Plausible.Stats.Filters do |> Enum.map(fn {[_operator, dimension | _rest], _depth} -> dimension end) end - def filtering_on_dimension?(query, dimension) do - dimension in dimensions_used_in_filters(query.filters) + def filtering_on_dimension?(filters, dimension) do + dimension in dimensions_used_in_filters(filters) end @doc """ diff --git a/lib/plausible/stats/filters/filters_parser.ex b/lib/plausible/stats/filters/filters_parser.ex new file mode 100644 index 000000000000..5d5187f77ca4 --- /dev/null +++ b/lib/plausible/stats/filters/filters_parser.ex @@ -0,0 +1,134 @@ +defmodule Plausible.Stats.Filters.FiltersParser do + @moduledoc """ + FiltersParser is the module to verify that filters array is in the expected format. + """ + + alias Plausible.Stats.Filters + alias Plausible.Helpers.ListTraverse + + @segment_filter_key "segment" + def segment_filter_key(), do: @segment_filter_key + + @filter_entry_operators [ + :is, + :is_not, + :matches, + :matches_not, + :matches_wildcard, + :matches_wildcard_not, + :contains, + :contains_not + ] + + @filter_tree_operators [:not, :and, :or] + + def parse_filters(filters) when is_list(filters) do + ListTraverse.parse_list(filters, &parse_filter/1) + end + + def parse_filters(_invalid_metrics), do: {:error, "Invalid filters passed."} + + defp parse_filter(filter) do + with {:ok, operator} <- parse_operator(filter), + {:ok, second} <- parse_filter_second(operator, filter), + {:ok, rest} <- parse_filter_rest(operator, filter) do + {:ok, [operator, second | rest]} + end + end + + defp parse_operator(["is" | _rest]), do: {:ok, :is} + defp parse_operator(["is_not" | _rest]), do: {:ok, :is_not} + defp parse_operator(["matches" | _rest]), do: {:ok, :matches} + defp parse_operator(["matches_not" | _rest]), do: {:ok, :matches_not} + defp parse_operator(["matches_wildcard" | _rest]), do: {:ok, :matches_wildcard} + defp parse_operator(["matches_wildcard_not" | _rest]), do: {:ok, :matches_wildcard_not} + defp parse_operator(["contains" | _rest]), do: {:ok, :contains} + defp parse_operator(["contains_not" | _rest]), do: {:ok, :contains_not} + defp parse_operator(["not" | _rest]), do: {:ok, :not} + defp parse_operator(["and" | _rest]), do: {:ok, :and} + defp parse_operator(["or" | _rest]), do: {:ok, :or} + defp parse_operator(filter), do: {:error, "Unknown operator for filter '#{i(filter)}'."} + + def parse_filter_second(:not, [_, filter | _rest]), do: parse_filter(filter) + + def parse_filter_second(operator, [_, filters | _rest]) when operator in [:and, :or], + do: parse_filters(filters) + + def parse_filter_second(_operator, filter), do: parse_filter_key(filter) + + defp parse_filter_key([_operator, filter_key | _rest] = filter) do + parse_filter_key_string(filter_key, "Invalid filter '#{i(filter)}") + end + + defp parse_filter_key(filter), do: {:error, "Invalid filter '#{i(filter)}'."} + + defp parse_filter_rest(operator, filter) + when operator in @filter_entry_operators, + do: parse_clauses_list(filter) + + defp parse_filter_rest(operator, _filter) + when operator in @filter_tree_operators, + do: {:ok, []} + + defp parse_clauses_list([operation, filter_key, list] = filter) when is_list(list) do + all_strings? = Enum.all?(list, &is_binary/1) + all_integers? = Enum.all?(list, &is_integer/1) + + case {filter_key, all_strings?} do + {"visit:city", false} when all_integers? -> + {:ok, [list]} + + {"visit:country", true} when operation in ["is", "is_not"] -> + if Enum.all?(list, &(String.length(&1) == 2)) do + {:ok, [list]} + else + {:error, + "Invalid visit:country filter, visit:country needs to be a valid 2-letter country code."} + end + + {@segment_filter_key, false} when all_integers? -> + {:ok, [list]} + + {_, true} -> + {:ok, [list]} + + _ -> + {:error, "Invalid filter '#{i(filter)}'."} + end + end + + defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{i(filter)}'"} + + def parse_filter_key_string(filter_key, error_message \\ "") do + case filter_key do + "event:props:" <> property_name -> + if String.length(property_name) > 0 do + {:ok, filter_key} + else + {:error, error_message} + end + + "event:" <> key -> + if key in Filters.event_props() do + {:ok, filter_key} + else + {:error, error_message} + end + + "visit:" <> key -> + if key in Filters.visit_props() do + {:ok, filter_key} + else + {:error, error_message} + end + + @segment_filter_key -> + {:ok, filter_key} + + _ -> + {:error, error_message} + end + end + + defp i(value), do: inspect(value, charlists: :as_lists) +end diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index b7410964b854..5ad6459ac8f6 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -4,6 +4,8 @@ defmodule Plausible.Stats.Filters.QueryParser do use Plausible alias Plausible.Stats.{TableDecider, Filters, Metrics, DateTimeRange, JSONSchema, Time} + alias Plausible.Stats.Filters.FiltersParser + alias Plausible.Helpers.ListTraverse @default_include %{ imports: false, @@ -33,13 +35,14 @@ defmodule Plausible.Stats.Filters.QueryParser do parse_time_range(site, Map.get(params, "date_range"), date, now), utc_time_range = raw_time_range |> DateTimeRange.to_timezone("Etc/UTC"), {:ok, metrics} <- parse_metrics(Map.get(params, "metrics", [])), - {:ok, filters} <- parse_filters(Map.get(params, "filters", [])), + {:ok, filters} <- FiltersParser.parse_filters(Map.get(params, "filters", [])), {:ok, dimensions} <- parse_dimensions(Map.get(params, "dimensions", [])), {:ok, order_by} <- parse_order_by(Map.get(params, "order_by")), {:ok, include} <- parse_include(site, Map.get(params, "include", %{})), {:ok, pagination} <- parse_pagination(Map.get(params, "pagination", %{})), {preloaded_goals, revenue_currencies} <- preload_needed_goals(site, metrics, filters, dimensions), + preloaded_segments = preload_needed_segments(site, filters), query = %{ metrics: metrics, filters: filters, @@ -50,13 +53,15 @@ defmodule Plausible.Stats.Filters.QueryParser do include: include, pagination: pagination, preloaded_goals: preloaded_goals, - revenue_currencies: revenue_currencies + revenue_currencies: revenue_currencies, + preloaded_segments: preloaded_segments }, :ok <- validate_order_by(query), :ok <- validate_custom_props_access(site, query), :ok <- validate_toplevel_only_filter_dimension(query), :ok <- validate_special_metrics_filters(query), :ok <- validate_filtered_goals_exist(query), + :ok <- validate_segments_allowed(site, query, %{}), :ok <- validate_revenue_metrics_access(site, query), :ok <- validate_metrics(query), :ok <- validate_include(query) do @@ -73,7 +78,7 @@ defmodule Plausible.Stats.Filters.QueryParser do def parse_date_range_pair(_site, unknown), do: {:error, "Invalid date_range '#{i(unknown)}'."} defp parse_metrics(metrics) when is_list(metrics) do - parse_list(metrics, &parse_metric/1) + ListTraverse.parse_list(metrics, &parse_metric/1) end defp parse_metric(metric_str) do @@ -83,89 +88,6 @@ defmodule Plausible.Stats.Filters.QueryParser do end end - def parse_filters(filters) when is_list(filters) do - parse_list(filters, &parse_filter/1) - end - - def parse_filters(_invalid_metrics), do: {:error, "Invalid filters passed."} - - defp parse_filter(filter) do - with {:ok, operator} <- parse_operator(filter), - {:ok, second} <- parse_filter_second(operator, filter), - {:ok, rest} <- parse_filter_rest(operator, filter) do - {:ok, [operator, second | rest]} - end - end - - defp parse_operator(["is" | _rest]), do: {:ok, :is} - defp parse_operator(["is_not" | _rest]), do: {:ok, :is_not} - defp parse_operator(["matches" | _rest]), do: {:ok, :matches} - defp parse_operator(["matches_not" | _rest]), do: {:ok, :matches_not} - defp parse_operator(["matches_wildcard" | _rest]), do: {:ok, :matches_wildcard} - defp parse_operator(["matches_wildcard_not" | _rest]), do: {:ok, :matches_wildcard_not} - defp parse_operator(["contains" | _rest]), do: {:ok, :contains} - defp parse_operator(["contains_not" | _rest]), do: {:ok, :contains_not} - defp parse_operator(["not" | _rest]), do: {:ok, :not} - defp parse_operator(["and" | _rest]), do: {:ok, :and} - defp parse_operator(["or" | _rest]), do: {:ok, :or} - defp parse_operator(filter), do: {:error, "Unknown operator for filter '#{i(filter)}'."} - - def parse_filter_second(:not, [_, filter | _rest]), do: parse_filter(filter) - - def parse_filter_second(operator, [_, filters | _rest]) when operator in [:and, :or], - do: parse_filters(filters) - - def parse_filter_second(_operator, filter), do: parse_filter_key(filter) - - defp parse_filter_key([_operator, filter_key | _rest] = filter) do - parse_filter_key_string(filter_key, "Invalid filter '#{i(filter)}") - end - - defp parse_filter_key(filter), do: {:error, "Invalid filter '#{i(filter)}'."} - - defp parse_filter_rest(operator, filter) - when operator in [ - :is, - :is_not, - :matches, - :matches_not, - :matches_wildcard, - :matches_wildcard_not, - :contains, - :contains_not - ], - do: parse_clauses_list(filter) - - defp parse_filter_rest(operator, _filter) - when operator in [:not, :and, :or], - do: {:ok, []} - - defp parse_clauses_list([operation, filter_key, list] = filter) when is_list(list) do - all_strings? = Enum.all?(list, &is_binary/1) - all_integers? = Enum.all?(list, &is_integer/1) - - case {filter_key, all_strings?} do - {"visit:city", false} when all_integers? -> - {:ok, [list]} - - {"visit:country", true} when operation in ["is", "is_not"] -> - if Enum.all?(list, &(String.length(&1) == 2)) do - {:ok, [list]} - else - {:error, - "Invalid visit:country filter, visit:country needs to be a valid 2-letter country code."} - end - - {_, true} -> - {:ok, [list]} - - _ -> - {:error, "Invalid filter '#{i(filter)}'."} - end - end - - defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{i(filter)}'"} - defp parse_date(_site, date_string, _date) when is_binary(date_string) do case Date.from_iso8601(date_string) do {:ok, date} -> {:ok, date} @@ -273,14 +195,14 @@ defmodule Plausible.Stats.Filters.QueryParser do defp today(site), do: DateTime.now!(site.timezone) |> DateTime.to_date() defp parse_dimensions(dimensions) when is_list(dimensions) do - parse_list( + ListTraverse.parse_list( dimensions, &parse_dimension_entry(&1, "Invalid dimensions '#{i(dimensions)}'") ) end defp parse_order_by(order_by) when is_list(order_by) do - parse_list(order_by, &parse_order_by_entry/1) + ListTraverse.parse_list(order_by, &parse_order_by_entry/1) end defp parse_order_by(nil), do: {:ok, nil} @@ -296,7 +218,7 @@ defmodule Plausible.Stats.Filters.QueryParser do defp parse_dimension_entry(key, error_message) do case { parse_time(key), - parse_filter_key_string(key) + FiltersParser.parse_filter_key_string(key) } do {{:ok, time}, _} -> {:ok, time} {_, {:ok, filter_key}} -> {:ok, filter_key} @@ -308,7 +230,7 @@ defmodule Plausible.Stats.Filters.QueryParser do case { parse_time(value), parse_metric(value), - parse_filter_key_string(value) + FiltersParser.parse_filter_key_string(value) } do {{:ok, time}, _, _} -> {:ok, time} {_, {:ok, metric}, _} -> {:ok, metric} @@ -360,34 +282,6 @@ defmodule Plausible.Stats.Filters.QueryParser do defp atomize_keys(value), do: value - defp parse_filter_key_string(filter_key, error_message \\ "") do - case filter_key do - "event:props:" <> property_name -> - if String.length(property_name) > 0 do - {:ok, filter_key} - else - {:error, error_message} - end - - "event:" <> key -> - if key in Filters.event_props() do - {:ok, filter_key} - else - {:error, error_message} - end - - "visit:" <> key -> - if key in Filters.visit_props() do - {:ok, filter_key} - else - {:error, error_message} - end - - _ -> - {:error, error_message} - end - end - defp validate_order_by(query) do if query.order_by do valid_values = query.metrics ++ query.dimensions @@ -410,6 +304,14 @@ defmodule Plausible.Stats.Filters.QueryParser do end end + def preload_needed_segments(site, filters) do + if Plausible.Stats.Filters.Segments.has_segment_filters?(filters) do + Plausible.Repo.preload(site, :segments).segments + else + [] + end + end + def preload_needed_goals(site, metrics, filters, dimensions) do goal_filters? = Enum.any?(filters, fn [_, filter_key | _rest] -> filter_key == "event:goal" end) @@ -455,6 +357,10 @@ defmodule Plausible.Stats.Filters.QueryParser do end end + defp validate_segments_allowed(_site, _query, _available_segments) do + :ok + end + defp validate_filtered_goals_exist(query) do # Note: Only works since event:goal is allowed as a top level filter goal_filter_clauses = @@ -464,7 +370,10 @@ defmodule Plausible.Stats.Filters.QueryParser do end) if length(goal_filter_clauses) > 0 do - validate_list(goal_filter_clauses, &validate_goal_filter(&1, query.preloaded_goals)) + ListTraverse.validate_list( + goal_filter_clauses, + &validate_goal_filter(&1, query.preloaded_goals) + ) else :ok end @@ -527,14 +436,14 @@ defmodule Plausible.Stats.Filters.QueryParser do end defp validate_metrics(query) do - with :ok <- validate_list(query.metrics, &validate_metric(&1, query)) do + with :ok <- ListTraverse.validate_list(query.metrics, &validate_metric(&1, query)) do validate_no_metrics_filters_conflict(query) end end defp validate_metric(metric, query) when metric in [:conversion_rate, :group_conversion_rate] do if Enum.member?(query.dimensions, "event:goal") or - Filters.filtering_on_dimension?(query, "event:goal") do + Filters.filtering_on_dimension?(query.filters, "event:goal") do :ok else {:error, "Metric `#{metric}` can only be queried with event:goal filters or dimensions."} @@ -554,7 +463,7 @@ defmodule Plausible.Stats.Filters.QueryParser do defp validate_metric(:views_per_visit = metric, query) do cond do - Filters.filtering_on_dimension?(query, "event:page") -> + Filters.filtering_on_dimension?(query.filters, "event:page") -> {:error, "Metric `#{metric}` cannot be queried with a filter on `event:page`."} length(query.dimensions) > 0 -> @@ -599,22 +508,4 @@ defmodule Plausible.Stats.Filters.QueryParser do end defp i(value), do: inspect(value, charlists: :as_lists) - - defp parse_list(list, parser_function) do - Enum.reduce_while(list, {:ok, []}, fn value, {:ok, results} -> - case parser_function.(value) do - {:ok, result} -> {:cont, {:ok, results ++ [result]}} - {:error, _} = error -> {:halt, error} - end - end) - end - - defp validate_list(list, parser_function) do - Enum.reduce_while(list, :ok, fn value, :ok -> - case parser_function.(value) do - :ok -> {:cont, :ok} - {:error, _} = error -> {:halt, error} - end - end) - end end diff --git a/lib/plausible/stats/filters/segments.ex b/lib/plausible/stats/filters/segments.ex new file mode 100644 index 000000000000..a55dd7279110 --- /dev/null +++ b/lib/plausible/stats/filters/segments.ex @@ -0,0 +1,72 @@ +defmodule Plausible.Stats.Filters.Segments do + @moduledoc """ + Module containing the business logic of segments + """ + alias Plausible.Stats.Filters + alias Plausible.Stats.Filters.FiltersParser + + @spec has_segment_filters?(list()) :: boolean() + def has_segment_filters?(filters), + do: Filters.filtering_on_dimension?(filters, FiltersParser.segment_filter_key()) + + @spec expand_segments_to_constituent_filters(list(), list()) :: + list() + def expand_segments_to_constituent_filters(filters, segments) do + case segment_filter_index = find_top_level_segment_filter_index(filters) do + nil -> + filters + + _ -> + {head, [segment_filter | tail]} = Enum.split(filters, segment_filter_index) + [_operator, _filter_key, segment_id_clauses] = segment_filter + + expanded_filters = + Enum.concat( + Enum.map(segment_id_clauses, fn segment_id -> + with {:ok, segment_data} <- get_segment_data(segments, segment_id), + {:ok, %{filters: filters}} <- + validate_segment_data(segment_data) do + filters + else + {:error, :segment_not_found} -> + raise "Segment not found with id #{inspect(segment_id)}." + + {:error, :segment_invalid} -> + raise "Segment invalid with id #{inspect(segment_id)}." + end + end) + ) + + head ++ expanded_filters ++ tail + end + end + + @spec find_top_level_segment_filter_index(list()) :: non_neg_integer() | nil + defp find_top_level_segment_filter_index(filters) do + Enum.find_index(filters, fn filter -> + case filter do + [_first, second, _third] -> second == FiltersParser.segment_filter_key() + _ -> false + end + end) + end + + @spec get_segment_data(list(), integer()) :: {:ok, map()} | {:error, :segment_not_found} + defp get_segment_data(segments, segment_id) do + case Enum.find(segments, fn segment -> segment.id == segment_id end) do + nil -> {:error, :segment_not_found} + %Plausible.Segment{segment_data: segment_data} -> {:ok, segment_data} + end + end + + @spec validate_segment_data(map()) :: {:ok, list()} | {:error, :segment_invalid} + def validate_segment_data(segment_data) do + with {:ok, filters} <- FiltersParser.parse_filters(segment_data["filters"]), + # segments are not permitted within segments + false <- has_segment_filters?(filters) do + {:ok, %{filters: filters}} + else + _ -> {:error, :segment_invalid} + end + end +end diff --git a/lib/plausible/stats/legacy/legacy_query_builder.ex b/lib/plausible/stats/legacy/legacy_query_builder.ex index f555fa491e98..84696e5f73fd 100644 --- a/lib/plausible/stats/legacy/legacy_query_builder.ex +++ b/lib/plausible/stats/legacy/legacy_query_builder.ex @@ -19,6 +19,7 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do |> put_dimensions(params) |> put_interval(params) |> put_parsed_filters(params) + |> put_preloaded_segments(site) |> put_preloaded_goals(site) |> put_order_by(params) |> put_include_comparisons(site, params) @@ -31,6 +32,16 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do query end + defp put_preloaded_segments(query, site) do + preloaded_segments = + Plausible.Stats.Filters.QueryParser.preload_needed_segments( + site, + query.filters + ) + + struct!(query, preloaded_segments: preloaded_segments) + end + defp put_preloaded_goals(query, site) do {preloaded_goals, revenue_currencies} = Plausible.Stats.Filters.QueryParser.preload_needed_goals( diff --git a/lib/plausible/stats/query.ex b/lib/plausible/stats/query.ex index c4e18a01ec4c..b1a96dad168d 100644 --- a/lib/plausible/stats/query.ex +++ b/lib/plausible/stats/query.ex @@ -17,6 +17,7 @@ defmodule Plausible.Stats.Query do legacy_breakdown: false, remove_unavailable_revenue_metrics: false, preloaded_goals: [], + preloaded_segments: [], revenue_currencies: %{}, include: Plausible.Stats.Filters.QueryParser.default_include(), debug_metadata: %{}, diff --git a/lib/plausible/stats/query_optimizer.ex b/lib/plausible/stats/query_optimizer.ex index 827dfebb169c..b8601b69621d 100644 --- a/lib/plausible/stats/query_optimizer.ex +++ b/lib/plausible/stats/query_optimizer.ex @@ -7,7 +7,7 @@ defmodule Plausible.Stats.QueryOptimizer do alias Plausible.Stats.{DateTimeRange, Filters, Query, TableDecider, Util, Time} @doc """ - This module manipulates an existing query, updating it according to business logic. + This method manipulates an existing query, updating it according to business logic. For example, it: 1. Figures out what the right granularity to group by time is @@ -46,6 +46,7 @@ defmodule Plausible.Stats.QueryOptimizer do defp pipeline() do [ + &expand_segments_to_filters/1, &update_group_by_time/1, &add_missing_order_by/1, &update_time_in_order_by/1, @@ -176,6 +177,20 @@ defmodule Plausible.Stats.QueryOptimizer do ) end + defp expand_segments_to_filters(query) do + if length(query.preloaded_segments) > 0 do + filters = + Filters.Segments.expand_segments_to_constituent_filters( + query.filters, + query.preloaded_segments + ) + + %Query{query | filters: filters} + else + query + end + end + on_ee do defp remove_revenue_metrics_if_unavailable(query) do if query.remove_unavailable_revenue_metrics and map_size(query.revenue_currencies) == 0 do diff --git a/lib/plausible_web/controllers/api/external_stats_controller.ex b/lib/plausible_web/controllers/api/external_stats_controller.ex index 0adebbd506d0..d50ab685fb3b 100644 --- a/lib/plausible_web/controllers/api/external_stats_controller.ex +++ b/lib/plausible_web/controllers/api/external_stats_controller.ex @@ -165,10 +165,10 @@ defmodule PlausibleWeb.Api.ExternalStatsController do defp validate_metric("time_on_page" = metric, query) do cond do - Filters.filtering_on_dimension?(query, "event:goal") -> + Filters.filtering_on_dimension?(query.filters, "event:goal") -> {:error, "Metric `#{metric}` cannot be queried when filtering by `event:goal`"} - Filters.filtering_on_dimension?(query, "event:name") -> + Filters.filtering_on_dimension?(query.filters, "event:name") -> {:error, "Metric `#{metric}` cannot be queried when filtering by `event:name`"} query.dimensions == ["event:page"] -> @@ -178,7 +178,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do {:error, "Metric `#{metric}` is not supported in breakdown queries (except `event:page` breakdown)"} - Filters.filtering_on_dimension?(query, "event:page") -> + Filters.filtering_on_dimension?(query.filters, "event:page") -> {:ok, metric} true -> @@ -192,7 +192,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do query.dimensions == ["event:goal"] -> {:ok, metric} - Filters.filtering_on_dimension?(query, "event:goal") -> + Filters.filtering_on_dimension?(query.filters, "event:goal") -> {:ok, metric} true -> @@ -207,7 +207,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do defp validate_metric("views_per_visit" = metric, query) do cond do - Filters.filtering_on_dimension?(query, "event:page") -> + Filters.filtering_on_dimension?(query.filters, "event:page") -> {:error, "Metric `#{metric}` cannot be queried with a filter on `event:page`."} not Enum.empty?(query.dimensions) -> diff --git a/lib/plausible_web/controllers/api/helpers.ex b/lib/plausible_web/controllers/api/helpers.ex index 1c3a9e98bc59..109e3a1263dc 100644 --- a/lib/plausible_web/controllers/api/helpers.ex +++ b/lib/plausible_web/controllers/api/helpers.ex @@ -8,6 +8,13 @@ defmodule PlausibleWeb.Api.Helpers do |> halt() end + def not_enough_permissions(conn, msg) do + conn + |> put_status(403) + |> Phoenix.Controller.json(%{error: msg}) + |> halt() + end + def bad_request(conn, msg) do conn |> put_status(400) diff --git a/lib/plausible_web/controllers/api/internal/segments_controller.ex b/lib/plausible_web/controllers/api/internal/segments_controller.ex new file mode 100644 index 000000000000..1adf409b1e74 --- /dev/null +++ b/lib/plausible_web/controllers/api/internal/segments_controller.ex @@ -0,0 +1,146 @@ +defmodule PlausibleWeb.Api.Internal.SegmentsController do + use Plausible + use PlausibleWeb, :controller + use Plausible.Repo + use PlausibleWeb.Plugs.ErrorHandler + alias PlausibleWeb.Api.Helpers, as: H + + defp normalize_segment_id_param(input) do + case Integer.parse(input) do + {int_value, ""} -> int_value + _ -> nil + end + end + + defp get_one_segment(_user_id, _site_id, nil) do + nil + end + + defp get_one_segment(user_id, site_id, segment_id) do + query = + from(segment in Plausible.Segment, + where: segment.site_id == ^site_id, + where: segment.id == ^segment_id, + where: segment.type == :site or segment.owner_id == ^user_id + ) + + Repo.one(query) + end + + defp get_index_query(user_id, site_id) do + fields_in_index = [ + :id, + :name, + :type, + :inserted_at, + :updated_at, + :owner_id + ] + + from(segment in Plausible.Segment, + select: ^fields_in_index, + where: segment.site_id == ^site_id, + where: segment.type == :site or segment.owner_id == ^user_id, + order_by: [asc: segment.name] + ) + end + + defp has_capability_to_toggle_site_segment?(current_user_role) do + current_user_role in [:admin, :owner, :super_admin] + end + + def get_all_segments(conn, _params) do + site_id = conn.assigns.site.id + user_id = if is_nil(conn.assigns[:current_user]) do 0 else conn.assigns.current_user.id end + + result = Repo.all(get_index_query(user_id, site_id)) + + json(conn, result) + end + + def get_segment(conn, params) do + site_id = conn.assigns.site.id + user_id = if is_nil(conn.assigns[:current_user]) do 0 else conn.assigns.current_user.id end + segment_id = normalize_segment_id_param(params["segment_id"]) + + result = get_one_segment(user_id, site_id, segment_id) + + case result do + nil -> H.not_found(conn, "Segment not found with ID #{inspect(params["segment_id"])}") + %{} -> json(conn, result) + end + end + + def create_segment(conn, params) do + user_id = conn.assigns.current_user.id + site_id = conn.assigns.site.id + + segment_definition = + Map.merge(params, %{"site_id" => site_id, "owner_id" => user_id}) + + changeset = Plausible.Segment.changeset(%Plausible.Segment{}, segment_definition) + + if changeset.changes.type == :site and + not has_capability_to_toggle_site_segment?(conn.assigns.current_user_role) do + H.not_enough_permissions(conn, "Not enough permissions to create site segments") + else + result = Repo.insert(changeset) + + case result do + {:ok, segment} -> + json(conn, segment) + + {:error, _} -> + H.bad_request(conn, "Failed to create segment") + end + end + end + + def update_segment(conn, params) do + user_id = conn.assigns.current_user.id + site_id = conn.assigns.site.id + segment_id = normalize_segment_id_param(params["segment_id"]) + + if not is_nil(params["type"]) and + not has_capability_to_toggle_site_segment?(conn.assigns.current_user_role) do + H.not_enough_permissions(conn, "Not enough permissions to set segment visibility") + else + existing_segment = get_one_segment(user_id, site_id, segment_id) + + case existing_segment do + nil -> + H.not_found(conn, "Segment not found with ID #{inspect(params["segment_id"])}") + + %{} -> + updated_segment = + Repo.update!(Plausible.Segment.changeset(existing_segment, params), + returning: true + ) + + json(conn, updated_segment) + end + end + end + + def delete_segment(conn, params) do + user_id = conn.assigns.current_user.id + site_id = conn.assigns.site.id + segment_id = normalize_segment_id_param(params["segment_id"]) + + existing_segment = get_one_segment(user_id, site_id, segment_id) + + if existing_segment.type == :site and + not has_capability_to_toggle_site_segment?(conn.assigns.current_user_role) do + H.not_enough_permissions(conn, "Not enough permissions to delete site segments") + else + case existing_segment do + nil -> + H.not_found(conn, "Segment not found with ID #{inspect(params["segment_id"])}") + + %{} -> + Repo.delete!(existing_segment) + json(conn, existing_segment) + end + end + end +end diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 89b73d5caed6..a2fca2c012c4 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -474,7 +474,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{source: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -561,10 +561,10 @@ defmodule PlausibleWeb.Api.StatsController do defp validate_funnel_query(query) do cond do - Filters.filtering_on_dimension?(query, "event:goal") -> + Filters.filtering_on_dimension?(query.filters, "event:goal") -> {:error, {:invalid_funnel_query, "goals"}} - Filters.filtering_on_dimension?(query, "event:page") -> + Filters.filtering_on_dimension?(query.filters, "event:page") -> {:error, {:invalid_funnel_query, "pages"}} query.period == "realtime" -> @@ -588,7 +588,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_medium: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -616,7 +616,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_campaign: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -644,7 +644,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_content: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -672,7 +672,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_term: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -700,7 +700,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_source: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -728,7 +728,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{referrer: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -854,7 +854,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{page: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do pages |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -882,7 +882,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{entry_page: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do to_csv(entry_pages, [:name, :visitors, :conversion_rate], [ :name, :conversions, @@ -918,7 +918,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{exit_page: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do to_csv(exit_pages, [:name, :visitors, :conversion_rate], [ :name, :conversions, @@ -991,7 +991,7 @@ defmodule PlausibleWeb.Api.StatsController do Map.put(country, :name, country_info.name) end) - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do countries |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1051,7 +1051,7 @@ defmodule PlausibleWeb.Api.StatsController do end) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do regions |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1095,7 +1095,7 @@ defmodule PlausibleWeb.Api.StatsController do end) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do cities |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1127,7 +1127,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{browser: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do browsers |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1159,7 +1159,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{browser_version: :version}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do results |> transform_keys(%{browser: :name, visitors: :conversions}) |> to_csv([:name, :version, :conversions, :conversion_rate]) @@ -1200,7 +1200,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{os: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do systems |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1232,7 +1232,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{os_version: :version}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do results |> transform_keys(%{os: :name, visitors: :conversions}) |> to_csv([:name, :version, :conversions, :conversion_rate]) @@ -1273,7 +1273,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{device: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do sizes |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1363,7 +1363,7 @@ defmodule PlausibleWeb.Api.StatsController do |> Enum.concat() percent_or_cr = - if Filters.filtering_on_dimension?(query, "event:goal"), + if Filters.filtering_on_dimension?(query.filters, "event:goal"), do: :conversion_rate, else: :percentage @@ -1380,7 +1380,7 @@ defmodule PlausibleWeb.Api.StatsController do query = Query.from(site, params, debug_metadata(conn)) metrics = - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do [:visitors, :events, :conversion_rate] ++ @revenue_metrics else [:visitors, :events, :percentage] ++ @revenue_metrics @@ -1592,7 +1592,7 @@ defmodule PlausibleWeb.Api.StatsController do end defp breakdown_metrics(query, extra_metrics \\ []) do - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do [:visitors, :conversion_rate, :total_visitors] else [:visitors] ++ extra_metrics diff --git a/lib/plausible_web/controllers/stats_controller.ex b/lib/plausible_web/controllers/stats_controller.ex index c040f5c6a774..1de4ace5d3f0 100644 --- a/lib/plausible_web/controllers/stats_controller.ex +++ b/lib/plausible_web/controllers/stats_controller.ex @@ -73,6 +73,7 @@ defmodule PlausibleWeb.StatsController do title: title(conn, site), demo: demo, flags: get_flags(conn.assigns[:current_user], site), + members: get_members(conn.assigns[:current_user], site), is_dbip: is_dbip(), dogfood_page_path: dogfood_page_path, load_dashboard_js: true @@ -192,7 +193,7 @@ defmodule PlausibleWeb.StatsController do defp csv_graph_metrics(query) do {metrics, column_headers} = - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query.filters, "event:goal") do { [:visitors, :events, :conversion_rate], [:date, :unique_conversions, :total_conversions, :conversion_rate] @@ -356,6 +357,7 @@ defmodule PlausibleWeb.StatsController do background: conn.params["background"], theme: conn.params["theme"], flags: get_flags(conn.assigns[:current_user], shared_link.site), + members: get_members(conn.assigns[:current_user], shared_link.site), is_dbip: is_dbip(), load_dashboard_js: true ) @@ -381,6 +383,15 @@ defmodule PlausibleWeb.StatsController do end) |> Map.new() + defp get_members(nil, _site) do + nil + end + + defp get_members(_user, site) do + s = Plausible.Repo.preload(site, :members) + s.members |> Enum.map(fn member -> {member.id, member.name} end) |> Map.new() + end + defp is_dbip() do on_ee do false diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index 335fe37ca39b..bc42d63281f7 100644 --- a/lib/plausible_web/router.ex +++ b/lib/plausible_web/router.ex @@ -165,6 +165,17 @@ defmodule PlausibleWeb.Router do end end + # This scope indicates routes changeable without notice. + scope "/internal-api", PlausibleWeb.Api.Internal do + pipe_through [:internal_stats_api] + + get "/:domain/segments", SegmentsController, :get_all_segments + get "/:domain/segments/:segment_id", SegmentsController, :get_segment + post "/:domain/segments", SegmentsController, :create_segment + patch "/:domain/segments/:segment_id", SegmentsController, :update_segment + delete "/:domain/segments/:segment_id", SegmentsController, :delete_segment + end + scope "/api/stats", PlausibleWeb.Api do pipe_through :internal_stats_api diff --git a/lib/plausible_web/templates/stats/stats.html.heex b/lib/plausible_web/templates/stats/stats.html.heex index 4a2d69ab5b05..5b004cb4d8d0 100644 --- a/lib/plausible_web/templates/stats/stats.html.heex +++ b/lib/plausible_web/templates/stats/stats.html.heex @@ -38,6 +38,8 @@ data-background={@conn.assigns[:background]} data-is-dbip={to_string(@is_dbip)} data-current-user-role={@conn.assigns[:current_user_role]} + data-current-user-id={if is_nil(@conn.assigns[:current_user]) do nil else @conn.assigns[:current_user].id end} + 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/priv/json-schemas/query-api-schema.json b/priv/json-schemas/query-api-schema.json index e715c5957184..d6853b19277f 100644 --- a/priv/json-schemas/query-api-schema.json +++ b/priv/json-schemas/query-api-schema.json @@ -351,6 +351,11 @@ "enum": ["is", "contains"], "description": "filter operation" }, + "filter_operation_for_segments": { + "type": "string", + "enum": ["is"], + "description": "filter operation" + }, "filter_without_goals": { "type": "array", "additionalItems": false, @@ -396,10 +401,29 @@ } ] }, + "filter_for_segment": { + "type": "array", + "additionalItems": false, + "minItems": 3, + "maxItems": 3, + "items": [ + { + "$ref": "#/definitions/filter_operation_for_segments" + }, + { + "type": "string", + "const": "segment" + }, + { + "$ref": "#/definitions/clauses" + } + ] + }, "filter_entry": { "oneOf": [ { "$ref": "#/definitions/filter_without_goals" }, - { "$ref": "#/definitions/filter_with_goals" } + { "$ref": "#/definitions/filter_with_goals" }, + { "$ref": "#/definitions/filter_for_segment" } ] }, "filter_tree": { diff --git a/test/plausible/segment_schema_test.exs b/test/plausible/segment_schema_test.exs new file mode 100644 index 000000000000..93622b631eca --- /dev/null +++ b/test/plausible/segment_schema_test.exs @@ -0,0 +1,92 @@ +defmodule Plausible.SegmentSchemaTest do + use ExUnit.Case + + setup do + segment = %Plausible.Segment{ + name: "any name", + type: :personal, + segment_data: %{"filters" => ["is", "visit:page", ["/blog"]]}, + owner_id: 1, + site_id: 100 + } + + {:ok, segment: segment} + end + + test "changeset has required fields" do + assert Plausible.Segment.changeset(%Plausible.Segment{}, %{}).errors == [ + segment_data: {"property \"filters\" must be an array with at least one member", []}, + name: {"can't be blank", [validation: :required]}, + segment_data: {"can't be blank", [validation: :required]}, + site_id: {"can't be blank", [validation: :required]}, + type: {"can't be blank", [validation: :required]}, + owner_id: {"can't be blank", [validation: :required]} + ] + end + + test "changeset does not allow setting owner_id to nil (setting to nil happens with database triggers)", + %{segment: valid_segment} do + assert Plausible.Segment.changeset( + valid_segment, + %{ + owner_id: nil + } + ).errors == [ + owner_id: {"can't be blank", [validation: :required]} + ] + end + + test "changeset allows setting nil owner_id to a user id (to be able to recover dangling site segments)", + %{segment: valid_segment} do + assert Plausible.Segment.changeset( + %Plausible.Segment{ + valid_segment + | owner_id: nil + }, + %{ + owner_id: 100_100 + } + ).valid? == true + end + + test "changeset requires segment_data to be structured as expected", %{segment: valid_segment} do + assert Plausible.Segment.changeset( + valid_segment, + %{ + segment_data: %{"filters" => 1, "labels" => true, "other" => []} + } + ).errors == [ + {:segment_data, {"property \"labels\" must be map or nil", []}}, + {:segment_data, + {"property \"filters\" must be an array with at least one member", []}}, + {:segment_data, + {"must not contain any other property except \"filters\" and \"labels\"", []}} + ] + end + + test "changeset forbids empty filters list", %{segment: valid_segment} do + assert Plausible.Segment.changeset( + valid_segment, + %{ + segment_data: %{ + "filters" => [] + } + } + ).errors == [ + {:segment_data, + {"property \"filters\" must be an array with at least one member", []}} + ] + end + + test "changeset permits well-structured segment data", %{segment: valid_segment} do + assert Plausible.Segment.changeset( + valid_segment, + %{ + segment_data: %{ + "filters" => [["is", "visit:country", ["DE"]]], + "labels" => %{"DE" => "Germany"} + } + } + ).valid? == true + end +end diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index 53e998d23a99..8fc156525658 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -50,7 +50,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do assert {:ok, result} = parse(site, schema_type, params, @now) return_value = Map.take(result, [:preloaded_goals, :revenue_currencies]) - result = Map.drop(result, [:preloaded_goals, :revenue_currencies]) + result = Map.drop(result, [:preloaded_goals, :preloaded_segments, :revenue_currencies]) assert result == expected_result return_value @@ -498,6 +498,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do "metrics" => ["visitors"], "date_range" => "all", "filters" => [ + ["is", "segment", [200]], [ "or", [ @@ -517,6 +518,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ + [:is, "segment", [200]], [ :or, [ 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 new file mode 100644 index 000000000000..9d193be380dd --- /dev/null +++ b/test/plausible_web/controllers/api/internal_controller/segments_controller_test.exs @@ -0,0 +1,428 @@ +defmodule PlausibleWeb.Api.Internal.SegmentsControllerTest do + use PlausibleWeb.ConnCase, async: true + use Plausible.Repo + + describe "GET /internal-api/:domain/segments" do + setup [:create_user, :create_new_site, :log_in] + + test "returns empty list when no segments", %{conn: conn, site: site} do + conn = + get(conn, "/internal-api/#{site.domain}/segments") + + assert json_response(conn, 200) == [] + end + + for role <- [:viewer, :owner] do + test "returns list with personal and site segments for #{role}", %{conn: conn, user: user} do + other_site = insert(:site, owner: user) + other_user = insert(:user) + + site = + insert(:site, + memberships: [ + build(:site_membership, user: user, role: unquote(role)), + build(:site_membership, user: other_user, role: :admin) + ] + ) + + insert_list(2, :segment, + site: other_site, + owner_id: user.id, + type: :site, + name: "other site segment" + ) + + insert_list(10, :segment, + site: site, + owner_id: other_user.id, + type: :personal, + name: "other user personal segment" + ) + + inserted_at = "2024-10-04T12:00:00" + + personal_segment = + insert(:segment, + site: site, + owner_id: user.id, + type: :personal, + name: "a personal segment", + inserted_at: inserted_at, + updated_at: inserted_at + ) + + emea_site_segment = + insert(:segment, + site: site, + owner_id: other_user.id, + type: :site, + name: "EMEA region", + inserted_at: inserted_at, + updated_at: inserted_at + ) + + apac_site_segment = + insert(:segment, + site: site, + owner_id: user.id, + type: :site, + name: "APAC region", + inserted_at: inserted_at, + updated_at: inserted_at + ) + + conn = + get(conn, "/internal-api/#{site.domain}/segments") + + assert json_response(conn, 200) == + Enum.map([apac_site_segment, personal_segment, emea_site_segment], fn s -> + %{ + "id" => s.id, + "name" => s.name, + "type" => Atom.to_string(s.type), + "owner_id" => s.owner_id, + "inserted_at" => inserted_at, + "updated_at" => inserted_at, + "segment_data" => nil + } + end) + end + end + end + + describe "GET /internal-api/:domain/segments/:segment_id" do + setup [:create_user, :create_new_site, :log_in] + + test "serves 404 when invalid segment key used", %{conn: conn, site: site} do + conn = + get(conn, "/internal-api/#{site.domain}/segments/any-id") + + assert json_response(conn, 404) == %{"error" => "Segment not found with ID \"any-id\""} + end + + test "serves 404 when no segment found", %{conn: conn, site: site} do + conn = + get(conn, "/internal-api/#{site.domain}/segments/100100") + + assert json_response(conn, 404) == %{"error" => "Segment not found with ID \"100100\""} + end + + test "serves 404 when segment is for another site", %{conn: conn, site: site, user: user} do + other_site = insert(:site, owner: user) + + %{id: segment_id} = + insert(:segment, + site: other_site, + owner_id: user.id, + type: :site, + name: "any" + ) + + conn = + get(conn, "/internal-api/#{site.domain}/segments/#{segment_id}") + + assert json_response(conn, 404) == %{ + "error" => "Segment not found with ID \"#{segment_id}\"" + } + end + + test "serves 404 when user is not the segment owner and segment is personal", + %{ + conn: conn, + site: site + } do + other_user = insert(:user) + + inserted_at = "2024-10-01T10:00:00" + updated_at = inserted_at + + %{ + id: segment_id + } = + insert(:segment, + type: :personal, + owner_id: other_user.id, + site: site, + name: "any", + inserted_at: inserted_at, + updated_at: updated_at + ) + + conn = + get(conn, "/internal-api/#{site.domain}/segments/#{segment_id}") + + assert json_response(conn, 404) == %{ + "error" => "Segment not found with ID \"#{segment_id}\"" + } + end + + test "serves 200 with segment when user is not the segment owner and segment is not personal", + %{ + conn: conn, + site: site + } do + other_user = insert(:user) + + inserted_at = "2024-10-01T10:00:00" + updated_at = inserted_at + + %{ + id: segment_id, + segment_data: segment_data + } = + insert(:segment, + type: :site, + owner_id: other_user.id, + site: site, + name: "any", + inserted_at: inserted_at, + updated_at: updated_at + ) + + conn = + get(conn, "/internal-api/#{site.domain}/segments/#{segment_id}") + + assert json_response(conn, 200) == %{ + "id" => segment_id, + "owner_id" => other_user.id, + "name" => "any", + "type" => "site", + "segment_data" => segment_data, + "inserted_at" => inserted_at, + "updated_at" => updated_at + } + end + + test "serves 200 with segment when user is segment owner", %{ + conn: conn, + site: site, + user: user + } do + inserted_at = "2024-09-01T10:00:00" + updated_at = inserted_at + + %{id: segment_id, segment_data: segment_data} = + insert(:segment, + site: site, + name: "any", + owner_id: user.id, + type: :personal, + inserted_at: inserted_at, + updated_at: updated_at + ) + + conn = + get(conn, "/internal-api/#{site.domain}/segments/#{segment_id}") + + assert json_response(conn, 200) == %{ + "id" => segment_id, + "owner_id" => user.id, + "name" => "any", + "type" => "personal", + "segment_data" => segment_data, + "inserted_at" => inserted_at, + "updated_at" => updated_at + } + end + end + + describe "POST /internal-api/:domain/segments" do + setup [:create_user, :create_new_site, :log_in] + + test "forbids viewers from creating site segments", %{conn: conn, user: user} do + site = + insert(:site, memberships: [build(:site_membership, user: user, role: :viewer)]) + + conn = + post(conn, "/internal-api/#{site.domain}/segments", %{ + "type" => "site", + "segment_data" => %{"filters" => [["is", "visit:entry_page", ["/blog"]]]}, + "name" => "any name" + }) + + assert json_response(conn, 403) == %{ + "error" => "Not enough permissions to create site segments" + } + end + + for %{role: role, type: type} <- [ + %{role: :viewer, type: :personal}, + %{role: :admin, type: :personal}, + %{role: :admin, type: :site} + ] do + test "#{role} can create segment with type \"#{type}\" successfully", + %{conn: conn, user: user} do + site = + insert(:site, memberships: [build(:site_membership, user: user, role: unquote(role))]) + t = Atom.to_string(unquote(type)) + name = "any name" + + conn = + post(conn, "/internal-api/#{site.domain}/segments", %{ + "type" => t, + "segment_data" => %{"filters" => [["is", "visit:entry_page", ["/blog"]]]}, + "name" => name + }) + + response = json_response(conn, 200) + + assert %{ + "name" => ^name, + "segment_data" => %{"filters" => [["is", "visit:entry_page", ["/blog"]]]}, + "type" => ^t + } = response + + %{ + "id" => id, + "owner_id" => owner_id, + "updated_at" => updated_at, + "inserted_at" => inserted_at + } = + response + + assert is_integer(id) + assert ^owner_id = user.id + assert is_binary(inserted_at) + assert is_binary(updated_at) + assert ^inserted_at = updated_at + end + end + end + + describe "PATCH /internal-api/:domain/segments/:segment_id" do + setup [:create_user, :create_new_site, :log_in] + + for {current_type, patch_type} <- [ + {:personal, :site}, + {:site, :personal}, + ] do + test "prevents viewers from updating segments with current type #{current_type} with #{patch_type}", + %{ + conn: conn, + user: user + } do + site = insert(:site, memberships: [build(:site_membership, user: user, role: :viewer)]) + inserted_at = "2024-09-01T10:00:00" + updated_at = inserted_at + + %{id: segment_id} = + insert(:segment, + site: site, + name: "foo", + type: unquote(current_type), + owner_id: user.id, + inserted_at: inserted_at, + updated_at: updated_at + ) + + conn = + patch(conn, "/internal-api/#{site.domain}/segments/#{segment_id}", %{ + "name" => "updated name", + "type" => unquote(patch_type) + }) + + assert json_response(conn, 403) == %{ + "error" => "Not enough permissions to set segment visibility" + } + end + end + + test "updates segment successfully", %{conn: conn, user: user} do + site = insert(:site, memberships: [build(:site_membership, user: user, role: :admin)]) + + name = "foo" + inserted_at = "2024-09-01T10:00:00" + updated_at = inserted_at + type = :site + updated_type = :personal + + %{id: segment_id, owner_id: owner_id, segment_data: segment_data} = + insert(:segment, + site: site, + name: name, + type: type, + owner_id: user.id, + inserted_at: inserted_at, + updated_at: updated_at + ) + + conn = + patch(conn, "/internal-api/#{site.domain}/segments/#{segment_id}", %{ + "name" => "updated name", + "type" => updated_type + }) + + response = json_response(conn, 200) + + assert %{ + "owner_id" => ^owner_id, + "inserted_at" => ^inserted_at, + "id" => ^segment_id, + "segment_data" => ^segment_data + } = response + + assert response["name"] == "updated name" + assert response["type"] == Atom.to_string(updated_type) + assert response["updated_at"] != inserted_at + end + end + + describe "DELETE /internal-api/:domain/segments/:segment_id" do + setup [:create_user, :create_new_site, :log_in] + + test "forbids viewers from deleting site segments", %{conn: conn, user: user} do + site = insert(:site, memberships: [build(:site_membership, user: user, role: :viewer)]) + + %{id: segment_id} = + insert(:segment, + site: site, + name: "any", + type: :site, + owner_id: user.id + ) + + conn = + delete(conn, "/internal-api/#{site.domain}/segments/#{segment_id}") + + assert json_response(conn, 403) == %{ + "error" => "Not enough permissions to delete site segments" + } + end + + for %{role: role, type: type} <- [ + %{role: :viewer, type: :personal}, + %{role: :admin, type: :personal}, + %{role: :admin, type: :site} + ] do + test "#{role} can delete segment with type \"#{type}\" successfully", + %{conn: conn, user: user} do + site = + insert(:site, memberships: [build(:site_membership, user: user, role: unquote(role))]) + t = Atom.to_string(unquote(type)) + + user_id = user.id + + %{id: segment_id, segment_data: segment_data} = + insert(:segment, + site: site, + name: "any", + type: t, + owner_id: user_id + ) + + conn = + delete(conn, "/internal-api/#{site.domain}/segments/#{segment_id}") + + response = json_response(conn, 200) + + assert %{ + "id" => ^segment_id, + "owner_id" => ^user_id, + "name" => "any", + "segment_data" => ^segment_data, + "type" => ^t + } = response + end + end + end +end diff --git a/test/plausible_web/controllers/api/stats_controller/countries_test.exs b/test/plausible_web/controllers/api/stats_controller/countries_test.exs index db3b2f06972b..c6ca8142f202 100644 --- a/test/plausible_web/controllers/api/stats_controller/countries_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/countries_test.exs @@ -342,5 +342,73 @@ defmodule PlausibleWeb.Api.StatsController.CountriesTest do } ] end + + test "handles multiple segment filters", %{conn: conn, site: site, user: user} do + %{id: segment_alfa} = + insert(:segment, + site_id: site.id, + owner_id: user.id, + name: "Ireland and Britain (excl London)", + type: :site, + segment_data: %{ + "filters" => [ + ["is", "visit:country", ["IE", "GB"]], + ["is_not", "visit:city", [2_643_743]] + ] + } + ) + + %{id: segment_beta} = + insert(:segment, + site_id: site.id, + owner_id: user.id, + name: "Entered on root or Blog", + type: :personal, + segment_data: %{ + "filters" => [ + ["is", "visit:entry_page", ["/", "/blog"]] + ] + } + ) + + populate_stats(site, [ + build(:pageview, country_code: "EE"), + build(:pageview, + country_code: "GB", + # London + city_geoname_id: 2_643_743 + ), + build(:pageview, + country_code: "GB", + # London + city_geoname_id: 2_643_743 + ), + build(:pageview, country_code: "GB"), + build(:pageview, country_code: "IE", pathname: "/other"), + build(:pageview, country_code: "IE") + ]) + + filters = Jason.encode!([["is", "segment", [segment_alfa, segment_beta]]]) + conn = get(conn, "/api/stats/#{site.domain}/countries?period=day&filters=#{filters}") + + assert json_response(conn, 200)["results"] == [ + %{ + "code" => "GB", + "alpha_3" => "GBR", + "name" => "United Kingdom", + "flag" => "🇬🇧", + "visitors" => 1, + "percentage" => 50 + }, + %{ + "code" => "IE", + "alpha_3" => "IRL", + "name" => "Ireland", + "flag" => "🇮🇪", + "visitors" => 1, + "percentage" => 50 + } + ] + end end end diff --git a/test/support/factory.ex b/test/support/factory.ex index 62de00560b30..0be437e7ac55 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -402,6 +402,12 @@ defmodule Plausible.Factory do } end + def segment_factory do + %Plausible.Segment{ + segment_data: %{"filters" => [["is", "visit:entry_page", ["/blog"]]]} + } + end + defp hash_key() do Keyword.fetch!( Application.get_env(:plausible, PlausibleWeb.Endpoint), From 05822c5700e679bd274301e0e46682c1989e58b5 Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Mon, 18 Nov 2024 13:34:39 +0200 Subject: [PATCH 02/18] Fix site-context text --- assets/js/dashboard/site-context.test.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/assets/js/dashboard/site-context.test.tsx b/assets/js/dashboard/site-context.test.tsx index 012ca3371494..67323ddfe1f2 100644 --- a/assets/js/dashboard/site-context.test.tsx +++ b/assets/js/dashboard/site-context.test.tsx @@ -26,6 +26,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} @@ -61,7 +63,8 @@ describe('parseSiteFromDataset', () => { realtime: ['minute'], year: ['day', 'week', 'month'] }, - shared: false + shared: false, + members: {"1": "Test User"} } it('parses from dom string map correctly', () => { From cff3bbd3b93a213221d3cf18e9b50e3179b61ef4 Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Mon, 18 Nov 2024 19:12:58 +0200 Subject: [PATCH 03/18] Segment related actions are in the filters menu --- assets/js/dashboard/components/dropdown.tsx | 7 +- assets/js/dashboard/datepicker.tsx | 2 +- assets/js/dashboard/nav-menu/filter-menu.tsx | 341 ++++++++++++++++-- .../dashboard/nav-menu/filters-bar.test.tsx | 6 +- assets/js/dashboard/nav-menu/filters-bar.tsx | 115 ++---- assets/js/dashboard/nav-menu/top-bar.tsx | 20 +- .../js/dashboard/segments/segment-actions.tsx | 253 ------------- .../segments/segment-expanded-context.tsx | 68 ++++ .../js/dashboard/segments/segment-modals.tsx | 36 ++ .../dashboard/segments/segments-dropdown.tsx | 298 +++++++-------- assets/js/dashboard/segments/segments.ts | 18 +- assets/js/dashboard/site-switcher.js | 2 +- 12 files changed, 607 insertions(+), 559 deletions(-) delete mode 100644 assets/js/dashboard/segments/segment-actions.tsx create mode 100644 assets/js/dashboard/segments/segment-expanded-context.tsx diff --git a/assets/js/dashboard/components/dropdown.tsx b/assets/js/dashboard/components/dropdown.tsx index 9941ee169fad..0350f09706bf 100644 --- a/assets/js/dashboard/components/dropdown.tsx +++ b/assets/js/dashboard/components/dropdown.tsx @@ -148,7 +148,9 @@ export const DropdownNavigationLink = ({ className={classNames( { 'font-bold': !!active }, 'flex items-center justify-between', - 'text-sm leading-tight hover:bg-gray-100 hover:text-gray-900 dark:hover:bg-gray-900 dark:hover:text-gray-100', + 'text-sm leading-tight', + {'hover:bg-gray-100 hover:text-gray-900 dark:hover:bg-gray-900 dark:hover:text-gray-100': !props['aria-disabled']}, + {'cursor-not-allowed text-gray-500': props['aria-disabled']}, !!actions && 'pr-4', className )} @@ -157,7 +159,8 @@ export const DropdownNavigationLink = ({ +
{ + const user = useUserContext() const dropdownRef = useRef(null) const [opened, setOpened] = useState(false) const site = useSiteContext() const filterListItems = useMemo(() => getFilterListItems(site), [site]) + const { query } = useQueryContext() + const { expandedSegment, modal } = useSegmentExpandedContext() + const queryClient = useQueryClient() + const navigate = useAppNavigate() + const patchSegment = useMutation({ + mutationFn: ({ + id, + name, + type, + segment_data + }: Pick & + Partial> & { + segment_data?: { + filters: DashboardQuery['filters'] + labels: DashboardQuery['labels'] + } + }) => { + return fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments/${id}`, + { + method: 'PATCH', + body: JSON.stringify({ + name, + type, + ...(segment_data && { + segment_data: { + filters: remapToApiFilters(segment_data.filters), + labels: cleanLabels(segment_data.filters, segment_data.labels) + } + }) + }), + headers: { + 'content-type': 'application/json', + accept: 'application/json' + } + } + ) + .then((res) => res.json()) + .then((d) => ({ + ...d, + segment_data: parseApiSegmentData(d.segment_data) + })) + }, + onSuccess: async (d) => { + navigate({ + search: (search) => { + const filters = [['is', 'segment', [d.id]]] + const labels = cleanLabels(filters, {}, 'segment', { + [formatSegmentIdAsLabelKey(d.id)]: d.name + }) + return { + ...search, + filters, + labels + } + }, + state: { + expandedSegment: null, + modal: null + } as SegmentExpandedLocationState + }) + queryClient.invalidateQueries({ queryKey: ['segments'] }) + } + }) + + const createSegment = useMutation({ + mutationFn: ({ + name, + type, + segment_data + }: { + name: string + type: 'personal' | 'site' + segment_data: { + filters: DashboardQuery['filters'] + labels: DashboardQuery['labels'] + } + }) => { + return fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments`, + { + method: 'POST', + body: JSON.stringify({ + name, + type, + segment_data: { + filters: remapToApiFilters(segment_data.filters), + labels: cleanLabels(segment_data.filters, segment_data.labels) + } + }), + headers: { 'content-type': 'application/json' } + } + ) + .then((res) => res.json()) + .then((d) => ({ + ...d, + segment_data: parseApiSegmentData(d.segment_data) + })) + }, + onSuccess: async (d) => { + navigate({ + search: (search) => { + const filters = [['is', 'segment', [d.id]]] + const labels = cleanLabels(filters, {}, 'segment', { + [formatSegmentIdAsLabelKey(d.id)]: d.name + }) + return { + ...search, + filters, + labels + } + }, + state: { + expandedSegment: null, + modal: null + } as SegmentExpandedLocationState + }) + queryClient.invalidateQueries({ queryKey: ['segments'] }) + } + }) + const deleteSegment = useMutation({ + mutationFn: (data: Pick) => { + return fetch( + `/internal-api/${encodeURIComponent(site.domain)}/segments/${data.id}`, + { + method: 'DELETE' + } + ) + .then((res) => res.json()) + .then((d) => ({ + ...d, + segment_data: parseApiSegmentData(d.segment_data) + })) + }, + onSuccess: (_d): void => { + navigate({ + search: (s) => { + return { + ...s, + filters: null, + labels: null + } + }, + state: { + expandedSegment: null, + modal: null + } as SegmentExpandedLocationState + }) + queryClient.invalidateQueries({ queryKey: ['segments'] }) + } + }) useOnClickOutside({ ref: dropdownRef, - active: opened, + active: opened && modal === null, handler: () => setOpened(false) }) return ( - setOpened((opened) => !opened)} - currentOption={ - - - Filter - - } - > - {opened && ( - - setOpened(false)} /> - - {filterListItems.map(({ modalKey, label }) => ( - setOpened(false)} - active={false} - key={modalKey} - path={filterRoute.path} - params={{ field: modalKey }} - search={(search) => search} - > - {label} - - ))} - - + <> + {user.loggedIn && modal === 'update' && expandedSegment && ( + + navigate({ + search: (s) => s, + state: { expandedSegment: expandedSegment, modal: null } + }) + } + onSave={({ id, name, type }) => + patchSegment.mutate({ + id, + name, + type, + segment_data: { + filters: query.filters, + labels: query.labels + } + }) + } + /> )} - + {user.loggedIn && modal === 'create' && ( + + navigate({ + search: (s) => s, + state: { expandedSegment: expandedSegment, modal: null } + }) + } + onSave={({ name, type }) => + createSegment.mutate({ + name, + type, + segment_data: { + filters: query.filters, + labels: query.labels + } + }) + } + /> + )} + {user.loggedIn && modal === 'delete' && expandedSegment && ( + + navigate({ + search: (s) => s, + state: { expandedSegment: expandedSegment, modal: null } + }) + } + onSave={({ id }) => deleteSegment.mutate({ id })} + /> + )} + + setOpened((opened) => !opened)} + currentOption={ + + + Filter + + } + > + {opened && ( + + setOpened(false)} /> + + {filterListItems.map(({ modalKey, label }) => ( + setOpened(false)} + active={false} + key={modalKey} + path={filterRoute.path} + params={{ field: modalKey }} + search={(search) => search} + > + {label} + + ))} + + {!!query.filters.length && ( + + ({ ...s, filters: null, labels: null })} + > + Clear all filters + + {expandedSegment === null && ( + s} + navigateOptions={{ + state: { + modal: 'create', + expandedSegment: null + } as SegmentExpandedLocationState + }} + {...query.filters.some(isSegmentFilter) && {"aria-disabled": true, navigateOptions: undefined }} + > + Save as segment + + )} + + )} + + )} + + ) } diff --git a/assets/js/dashboard/nav-menu/filters-bar.test.tsx b/assets/js/dashboard/nav-menu/filters-bar.test.tsx index 9cca865b04df..b722f3b75022 100644 --- a/assets/js/dashboard/nav-menu/filters-bar.test.tsx +++ b/assets/js/dashboard/nav-menu/filters-bar.test.tsx @@ -90,7 +90,7 @@ describe(`${handleVisibility.name}`, () => { const setVisibility = jest.fn() const input = { setVisibility, - topBarWidth: 1000, + leftoverWidth: 1000, actionsWidth: 100, seeMorePresent: false, seeMoreWidth: 50, @@ -115,7 +115,7 @@ describe(`${handleVisibility.name}`, () => { visibleCount: 4 }) - handleVisibility({ ...input, topBarWidth: 999 }) + handleVisibility({ ...input, leftoverWidth: 999 }) expect(setVisibility).toHaveBeenCalledTimes(3) expect(setVisibility).toHaveBeenLastCalledWith({ width: 675, @@ -127,7 +127,7 @@ describe(`${handleVisibility.name}`, () => { const setVisibility = jest.fn() const input = { setVisibility, - topBarWidth: 300, + leftoverWidth: 300, actionsWidth: 100, seeMorePresent: true, seeMoreWidth: 50, diff --git a/assets/js/dashboard/nav-menu/filters-bar.tsx b/assets/js/dashboard/nav-menu/filters-bar.tsx index cb2c0af8512b..5feba312659b 100644 --- a/assets/js/dashboard/nav-menu/filters-bar.tsx +++ b/assets/js/dashboard/nav-menu/filters-bar.tsx @@ -1,9 +1,8 @@ /** @format */ -import { EllipsisHorizontalIcon, XMarkIcon } from '@heroicons/react/20/solid' +import { EllipsisHorizontalIcon } from '@heroicons/react/20/solid' import classNames from 'classnames' import React, { useRef, useState, useLayoutEffect, useEffect } from 'react' -import { AppNavigationLink } from '../navigation/use-app-navigate' import { useOnClickOutside } from '../util/use-on-click-outside' import { DropdownMenuWrapper, @@ -11,17 +10,14 @@ import { } from '../components/dropdown' import { FilterPillsList, PILL_X_GAP } from './filter-pills-list' import { useQueryContext } from '../query-context' -import { SaveSegmentAction } from '../segments/segment-actions' -import { EditingSegmentState, isSegmentFilter } from '../segments/segments' -import { useLocation } from 'react-router-dom' -import { useUserContext } from '../user-context' +const LEFT_ACTIONS_GAP_PX = 16 const SEE_MORE_GAP_PX = 16 const SEE_MORE_WIDTH_PX = 36 export const handleVisibility = ({ setVisibility, - topBarWidth, + leftoverWidth: leftoverWidth, actionsWidth, seeMorePresent, seeMoreWidth, @@ -29,14 +25,14 @@ export const handleVisibility = ({ pillGap }: { setVisibility: (v: VisibilityState) => void - topBarWidth: number | null + leftoverWidth: number | null actionsWidth: number | null pillWidths: (number | null)[] | null seeMorePresent: boolean seeMoreWidth: number pillGap: number }): void => { - if (topBarWidth === null || actionsWidth === null || pillWidths === null) { + if (leftoverWidth === null || actionsWidth === null || pillWidths === null) { return } @@ -56,11 +52,13 @@ export const handleVisibility = ({ return { visibleCount, lastValidWidth } } - const fits = fitToWidth(topBarWidth - actionsWidth) + const fits = fitToWidth(leftoverWidth - actionsWidth) // Check if possible to fit one more if "See more" is removed if (seeMorePresent && fits.visibleCount === pillWidths.length - 1) { - const maybeFitsMore = fitToWidth(topBarWidth - actionsWidth + seeMoreWidth) + const maybeFitsMore = fitToWidth( + leftoverWidth - actionsWidth + seeMoreWidth + ) if (maybeFitsMore.visibleCount === pillWidths.length) { return setVisibility({ width: maybeFitsMore.lastValidWidth, @@ -71,7 +69,9 @@ export const handleVisibility = ({ // Check if the appearance of "See more" would cause overflow if (!seeMorePresent && fits.visibleCount < pillWidths.length) { - const maybeFitsLess = fitToWidth(topBarWidth - actionsWidth - seeMoreWidth) + const maybeFitsLess = fitToWidth( + leftoverWidth - actionsWidth - seeMoreWidth + ) if (maybeFitsLess.visibleCount < fits.visibleCount) { return setVisibility({ width: maybeFitsLess.lastValidWidth, @@ -95,34 +95,12 @@ type VisibilityState = { } export const FiltersBar = () => { - const user = useUserContext(); const containerRef = useRef(null) const pillsRef = useRef(null) const actionsRef = useRef(null) const seeMoreRef = useRef(null) const [visibility, setVisibility] = useState(null) const { query } = useQueryContext() - const { state: locationState } = useLocation() as { - state?: EditingSegmentState - } - const [editingSegment, setEditingSegment] = useState< - null | EditingSegmentState['editingSegment'] - >(null) - - useLayoutEffect(() => { - if (locationState?.editingSegment) { - setEditingSegment(locationState?.editingSegment) - } - if (locationState?.editingSegment === null) { - setEditingSegment(null) - } - }, [locationState?.editingSegment]) - - useLayoutEffect(() => { - if (!query.filters.length) { - setEditingSegment(null) - } - }, [query.filters.length]) const [opened, setOpened] = useState(false) @@ -139,6 +117,12 @@ export const FiltersBar = () => { }) useLayoutEffect(() => { + const topLeftActions = containerRef.current?.parentElement + const topBar = topLeftActions?.parentElement + const datepicker = topBar?.children[1] as HTMLElement | undefined + const sitepicker = topLeftActions?.children[0] as HTMLElement | undefined + const filterButton = topLeftActions?.children[2] as HTMLElement | undefined + const resizeObserver = new ResizeObserver((_entries) => { const pillWidths = pillsRef.current ? Array.from(pillsRef.current.children).map((el) => @@ -149,15 +133,22 @@ export const FiltersBar = () => { setVisibility, pillWidths, pillGap: PILL_X_GAP, - topBarWidth: getElementWidthOrNull(containerRef.current), + leftoverWidth: + topBar && datepicker && sitepicker && filterButton + ? getElementWidthOrNull(topBar)! - + getElementWidthOrNull(datepicker)! - + getElementWidthOrNull(sitepicker)! - + getElementWidthOrNull(filterButton)! - + 2 * LEFT_ACTIONS_GAP_PX + : null, actionsWidth: getElementWidthOrNull(actionsRef.current), seeMorePresent: !!seeMoreRef.current, seeMoreWidth: SEE_MORE_WIDTH_PX + SEE_MORE_GAP_PX }) }) - if (containerRef.current) { - resizeObserver.observe(containerRef.current) + if (containerRef.current && topBar) { + resizeObserver.observe(topBar) } return () => { @@ -172,7 +163,7 @@ export const FiltersBar = () => { return (
{ start: 0, end: visibility?.visibleCount }} - className="pb-1 overflow-hidden" + className="p-1 overflow-hidden" style={{ width: visibility?.width ?? '100%' }} /> -
+
{visibility !== null && visibility.visibleCount !== query.filters.length && ( { > {opened && typeof visibility.visibleCount === 'number' ? ( @@ -223,49 +214,7 @@ export const FiltersBar = () => { ) : null} )} - - {user.loggedIn && editingSegment === null && - !query.filters.some((f) => isSegmentFilter(f)) && ( - <> - - - - )} - {user.loggedIn && editingSegment !== null && ( - <> - - - - )}
) } - -export const ClearAction = () => ( - ({ - ...search, - filters: null, - labels: null - })} - > - - -) - -const VerticalSeparator = () => { - return ( -
- ) -} diff --git a/assets/js/dashboard/nav-menu/top-bar.tsx b/assets/js/dashboard/nav-menu/top-bar.tsx index d2134a4fea9c..5c1074153585 100644 --- a/assets/js/dashboard/nav-menu/top-bar.tsx +++ b/assets/js/dashboard/nav-menu/top-bar.tsx @@ -10,6 +10,7 @@ import Filters from '../filters' import classNames from 'classnames' import { useInView } from 'react-intersection-observer' import { FilterMenu } from './filter-menu' +import SegmentExpandedContextProvider from '../segments/segment-expanded-context' interface TopBarProps { showCurrentVisitors: boolean @@ -28,27 +29,36 @@ export function TopBar({ showCurrentVisitors, extraBar }: TopBarProps) {
-
+
- {showCurrentVisitors && ( + {saved_segments && !extraBar && showCurrentVisitors && ( )} - {saved_segments ? : } + {saved_segments && !!extraBar && extraBar} + {saved_segments ? ( + + + + ) : ( + + )}
- {!!saved_segments && !!extraBar && extraBar}
) diff --git a/assets/js/dashboard/segments/segment-actions.tsx b/assets/js/dashboard/segments/segment-actions.tsx deleted file mode 100644 index bb83d5b1546b..000000000000 --- a/assets/js/dashboard/segments/segment-actions.tsx +++ /dev/null @@ -1,253 +0,0 @@ -/** @format */ - -import React, { useState, useCallback } from 'react' -import { useAppNavigate } from '../navigation/use-app-navigate' -import { useQueryContext } from '../query-context' -import { useMutation, useQueryClient } from '@tanstack/react-query' -import { DashboardQuery } from '../query' -import { useSiteContext } from '../site-context' -import { - cleanLabels, - plainFilterText, - remapToApiFilters -} from '../util/filters' -import { - EditingSegmentState, - formatSegmentIdAsLabelKey, - parseApiSegmentData, - SavedSegment, - SegmentType -} from './segments' -import { CreateSegmentModal, UpdateSegmentModal } from './segment-modals' -import { useUserContext } from '../user-context' - -type M = 'create segment' | 'update segment' -type O = - | { type: 'create segment' } - | { type: 'update segment'; segment: SavedSegment } - -export const SaveSegmentAction = ({ options }: { options: O[] }) => { - const user = useUserContext() - const site = useSiteContext() - const { query } = useQueryContext() - const [modal, setModal] = useState(null) - const navigate = useAppNavigate() - const openCreateSegment = useCallback(() => { - return setModal('create segment') - }, []) - const openUpdateSegment = useCallback(() => { - return setModal('update segment') - }, []) - const close = useCallback(() => { - return setModal(null) - }, []) - const queryClient = useQueryClient() - const createSegment = useMutation({ - mutationFn: ({ - name, - type, - segment_data - }: { - name: string - type: 'personal' | 'site' - segment_data: { - filters: DashboardQuery['filters'] - labels: DashboardQuery['labels'] - } - }) => { - return fetch( - `/internal-api/${encodeURIComponent(site.domain)}/segments`, - { - method: 'POST', - body: JSON.stringify({ - name, - type, - segment_data: { - filters: remapToApiFilters(segment_data.filters), - labels: cleanLabels(segment_data.filters, segment_data.labels) - } - }), - headers: { 'content-type': 'application/json' } - } - ) - .then((res) => res.json()) - .then((d) => ({ - ...d, - segment_data: parseApiSegmentData(d.segment_data) - })) - }, - onSuccess: async (d) => { - navigate({ - search: (search) => { - const filters = [['is', 'segment', [d.id]]] - const labels = cleanLabels(filters, {}, 'segment', { - [formatSegmentIdAsLabelKey(d.id)]: d.name - }) - return { - ...search, - filters, - labels - } - }, - state: { editingSegment: null } as EditingSegmentState - }) - close() - queryClient.invalidateQueries({ queryKey: ['segments'] }) - } - }) - - const patchSegment = useMutation({ - mutationFn: ({ - id, - name, - type, - segment_data - }: { - id: number - name?: string - type?: SegmentType - segment_data?: { - filters: DashboardQuery['filters'] - labels: DashboardQuery['labels'] - } - }) => { - return fetch( - `/internal-api/${encodeURIComponent(site.domain)}/segments/${id}`, - { - method: 'PATCH', - body: JSON.stringify({ - name, - type, - ...(segment_data && { - segment_data: { - filters: remapToApiFilters(segment_data.filters), - labels: cleanLabels(segment_data.filters, segment_data.labels) - } - }) - }), - headers: { - 'content-type': 'application/json', - accept: 'application/json' - } - } - ) - .then((res) => res.json()) - .then((d) => ({ - ...d, - segment_data: parseApiSegmentData(d.segment_data) - })) - }, - onSuccess: async (d) => { - navigate({ - search: (search) => { - const filters = [['is', 'segment', [d.id]]] - const labels = cleanLabels(filters, {}, 'segment', { - [formatSegmentIdAsLabelKey(d.id)]: d.name - }) - return { - ...search, - filters, - labels - } - }, - state: { editingSegment: null } as EditingSegmentState - }) - close() - queryClient.invalidateQueries({ queryKey: ['segments'] }) - } - }) - - if (!user.loggedIn) { - return null - } - - const segmentNamePlaceholder = query.filters.reduce( - (combinedName, filter) => - combinedName.length > 100 - ? combinedName - : `${combinedName}${combinedName.length ? ' and ' : ''}${plainFilterText(query, filter)}`, - '' - ) - - const option = options.find((o) => o.type === modal) - const buttonClass = - 'whitespace-nowrap rounded font-semibold text-sm leading-tight p-2 h-9 text-gray-500 hover:text-indigo-700 dark:hover:text-indigo-500 disabled:cursor-not-allowed' - return ( -
- {options.map((o) => { - if (o.type === 'create segment') { - return ( - - ) - } - if (o.type === 'update segment') { - const canEdit = - (o.segment.type === SegmentType.personal && - o.segment.owner_id === user.id) || - (o.segment.type === SegmentType.site && - ['admin', 'owner', 'super_admin'].includes(user.role)) - - return ( - - ) - } - })} - {modal === 'create segment' && ( - o.type === 'update segment')?.segment} - namePlaceholder={segmentNamePlaceholder} - close={close} - onSave={({ name, type }) => - createSegment.mutate({ - name, - type, - segment_data: { - filters: query.filters, - labels: query.labels - } - }) - } - /> - )} - {option?.type === 'update segment' && ( - - patchSegment.mutate({ - id, - name, - type, - segment_data: { - filters: query.filters, - labels: query.labels - } - }) - } - /> - )} -
- ) -} diff --git a/assets/js/dashboard/segments/segment-expanded-context.tsx b/assets/js/dashboard/segments/segment-expanded-context.tsx new file mode 100644 index 000000000000..64e593f33268 --- /dev/null +++ b/assets/js/dashboard/segments/segment-expanded-context.tsx @@ -0,0 +1,68 @@ +/* @format */ +import React, { + createContext, + ReactNode, + useContext, + useLayoutEffect, + useState +} from 'react' +import { useLocation } from 'react-router-dom' +import { SavedSegment } from './segments' +import { useQueryContext } from '../query-context' + +export type SegmentExpandedLocationState = { + expandedSegment: SavedSegment | null + modal: 'create' | 'update' | 'delete' | null +} + +const segmentExpandedContextDefaultValue: SegmentExpandedLocationState = { + expandedSegment: null, + modal: null +} + +const SegmentExpandedContext = createContext< + typeof segmentExpandedContextDefaultValue +>(segmentExpandedContextDefaultValue) + +export const useSegmentExpandedContext = () => { + return useContext(SegmentExpandedContext) +} + +export default function SegmentExpandedContextProvider({ + children +}: { + children: ReactNode +}) { + const { query } = useQueryContext() + + const { state: locationState } = useLocation() as { + state?: SegmentExpandedLocationState + } + + const [expandedSegment, setExpandedSegment] = useState( + null + ) + + useLayoutEffect(() => { + if (locationState?.expandedSegment) { + setExpandedSegment(locationState?.expandedSegment) + } + if (locationState?.expandedSegment === null) { + setExpandedSegment(null) + } + }, [locationState?.expandedSegment]) + + useLayoutEffect(() => { + if (!query.filters.length) { + setExpandedSegment(null) + } + }, [query.filters.length]) + + return ( + + {children} + + ) +} diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx index 20060952c8e4..b054ba6880a3 100644 --- a/assets/js/dashboard/segments/segment-modals.tsx +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -102,6 +102,42 @@ export const CreateSegmentModal = ({ ) } +export const DeleteSegmentModal = ({ + close, + onSave, + segment +}: { + close: () => void + onSave: (input: Pick) => void + segment: SavedSegment +}) => { + return ( + +

+ { + { personal: 'Delete personal segment', site: 'Delete site segment' }[ + segment.type + ] + } + {` "${segment.name}"?`} +

+
+ + +
+ + ) +} + export const UpdateSegmentModal = ({ close, onSave, diff --git a/assets/js/dashboard/segments/segments-dropdown.tsx b/assets/js/dashboard/segments/segments-dropdown.tsx index 3d4f52d0e666..f3fdf5f1f338 100644 --- a/assets/js/dashboard/segments/segments-dropdown.tsx +++ b/assets/js/dashboard/segments/segments-dropdown.tsx @@ -8,7 +8,6 @@ import { import { useQueryContext } from '../query-context' import { useSiteContext } from '../site-context' import { - EditingSegmentState, formatSegmentIdAsLabelKey, isSegmentFilter, parseApiSegmentData, @@ -16,23 +15,27 @@ import { SegmentData, SegmentType } from './segments' -import { - QueryFunction, - useMutation, - useQuery, - useQueryClient -} from '@tanstack/react-query' +import { QueryFunction, useQuery, useQueryClient } from '@tanstack/react-query' import { cleanLabels } from '../util/filters' import { useAppNavigate } from '../navigation/use-app-navigate' import classNames from 'classnames' import { Tooltip } from '../util/tooltip' import { formatDayShort, parseUTCDate } from '../util/date' import { useUserContext } from '../user-context' +import { + ArrowsPointingInIcon, + ArrowsPointingOutIcon +} from '@heroicons/react/24/solid' +import { + SegmentExpandedLocationState, + useSegmentExpandedContext +} from './segment-expanded-context' export const SegmentsList = ({ closeList }: { closeList: () => void }) => { - // const user = useUserContext(); + const { expandedSegment } = useSegmentExpandedContext() const { query } = useQueryContext() const site = useSiteContext() + const { data } = useQuery({ queryKey: ['segments'], placeholderData: (previousData) => previousData, @@ -64,82 +67,139 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { const segmentFilter = query.filters.find(isSegmentFilter) const appliedSegmentIds = (segmentFilter ? segmentFilter[2] : []) as number[] + if (expandedSegment) { + return ( + <> + + ({ ...s, filters: null, labels: null })} + active={true} + navigateOptions={{ + state: { + expandedSegment: null, + modal: null + } as SegmentExpandedLocationState + }} + > + {expandedSegment.name} + + + s} + navigateOptions={{ + state: { + expandedSegment: expandedSegment, + modal: 'update' + } as SegmentExpandedLocationState + }} + > + Update segment + + s} + navigateOptions={{ + state: { + expandedSegment: expandedSegment, + modal: 'create' + } as SegmentExpandedLocationState + }} + > + Save as a new segment + + s} + navigateOptions={{ + state: { + expandedSegment: expandedSegment, + modal: 'delete' + } as SegmentExpandedLocationState + }} + > + Delete segment + + + + ) + } + return ( - !!data?.length && ( - - {data.map((s) => { - const authorLabel = (() => { - if (!site.members) { - return '' - } + <> + {!!data?.length && ( + + {data.map((s) => { + const authorLabel = (() => { + if (!site.members) { + return '' + } - if (!s.owner_id || !site.members[s.owner_id]) { - return '(Deleted User)' - } + if (!s.owner_id || !site.members[s.owner_id]) { + return '(Removed User)' + } - // if (s.owner_id === user.id) { - // return 'You' - // } + // if (s.owner_id === user.id) { + // return 'You' + // } - return site.members[s.owner_id] - })() + return site.members[s.owner_id] + })() - const showUpdatedAt = s.updated_at !== s.inserted_at + const showUpdatedAt = s.updated_at !== s.inserted_at - return ( - + return ( + - { +
{ - [SegmentType.personal]: 'Personal segment', - [SegmentType.site]: 'Segment' - }[s.type] - } -
-
- {`Created at ${formatDayShort(parseUTCDate(s.inserted_at))}`} - {!showUpdatedAt && !!authorLabel && ` by ${authorLabel}`} -
- {showUpdatedAt && ( + { + [SegmentType.personal]: 'Personal segment', + [SegmentType.site]: 'Segment' + }[s.type] + } +
- {`Last updated at ${formatDayShort(parseUTCDate(s.updated_at))}`} - {!!authorLabel && ` by ${authorLabel}`} + {`Created at ${formatDayShort(parseUTCDate(s.inserted_at))}`} + {!showUpdatedAt && !!authorLabel && ` by ${authorLabel}`}
- )} -
- } - > - - - ) - })} - - ) + {showUpdatedAt && ( +
+ {`Last updated at ${formatDayShort(parseUTCDate(s.updated_at))}`} + {!!authorLabel && ` by ${authorLabel}`} +
+ )} +
+ } + > + + + ) + })} + + )} + ) } const SegmentLink = ({ id, name, - type, - owner_id, - appliedSegmentIds, - closeList + // type, + // owner_id, + appliedSegmentIds + // closeList }: SavedSegment & { appliedSegmentIds: number[]; closeList: () => void }) => { const user = useUserContext() const canSeeActions = user.loggedIn - const canDeleteSegment = - user.loggedIn && - ((owner_id === user.id && type === SegmentType.personal) || - (type === SegmentType.site && - ['admin', 'owner', 'super_admin'].includes(user.role))) + // const canDeleteSegment = + // user.loggedIn && + // ((owner_id === user.id && type === SegmentType.personal) || + // (type === SegmentType.site && + // ['admin', 'owner', 'super_admin'].includes(user.role))) const site = useSiteContext() const { query } = useQueryContext() const queryClient = useQueryClient() @@ -198,15 +258,14 @@ const SegmentLink = ({ labels: data.segment_data.labels }), state: { - editingSegment: { + expandedSegment: { id: data.id, name: data.name, type: data.type, owner_id: data.owner_id } - } as EditingSegmentState + } as SegmentExpandedLocationState }) - closeList() } catch (_error) { return } @@ -218,7 +277,7 @@ const SegmentLink = ({ active={appliedSegmentIds.includes(id)} onMouseEnter={prefetchSegment} navigateOptions={{ - state: { editingSegment: null } as EditingSegmentState + state: { expandedSegment: null } as SegmentExpandedLocationState }} search={(search) => { const otherFilters = query.filters.filter((f) => !isSegmentFilter(f)) @@ -250,12 +309,7 @@ const SegmentLink = ({ actions={ !canSeeActions ? null : ( <> - - + ) } @@ -265,7 +319,7 @@ const SegmentLink = ({ ) } -const EditSegment = ({ +const ExpandSegment = ({ className, onClick }: { @@ -280,97 +334,7 @@ const EditSegment = ({ )} onClick={onClick} > - - - ) -} - -const DeleteSegment = ({ - disabled, - className, - segment -}: { - disabled?: boolean - className?: string - segment: SavedSegment -}) => { - const queryClient = useQueryClient() - const site = useSiteContext() - const navigate = useAppNavigate() - const { query } = useQueryContext() - const deleteSegment = useMutation({ - mutationFn: (data: SavedSegment) => { - return fetch( - `/internal-api/${encodeURIComponent(site.domain)}/segments/${data.id}`, - { - method: 'DELETE' - } - ) - .then((res) => res.json()) - .then((d) => ({ - ...d, - segment_data: parseApiSegmentData(d.segment_data) - })) - }, - onSuccess: (_d): void => { - queryClient.invalidateQueries({ queryKey: ['segments'] }) - - const segmentFilterIndex = query.filters.findIndex(isSegmentFilter) - if (segmentFilterIndex < 0) { - return - } - - const filter = query.filters[segmentFilterIndex] - const clauses = filter[2] - const updatedSegmentIds = clauses.filter((c) => c !== segment.id) - - if (updatedSegmentIds.length === clauses.length) { - return - } - - const newFilters = !updatedSegmentIds.length - ? query.filters.filter((_f, index) => index !== segmentFilterIndex) - : [ - ...query.filters.slice(0, segmentFilterIndex), - [filter[0], filter[1], updatedSegmentIds], - ...query.filters.slice(segmentFilterIndex + 1) - ] - - navigate({ - search: (s) => { - return { - ...s, - filters: newFilters, - labels: cleanLabels(newFilters, query.labels) - } - } - }) - } - }) - - return ( - ) } - -const DeleteSegmentIcon = () => ( - - - -) - -const EditSegmentIcon = () => ( - - - -) diff --git a/assets/js/dashboard/segments/segments.ts b/assets/js/dashboard/segments/segments.ts index c782e056d9ef..a194856b0c41 100644 --- a/assets/js/dashboard/segments/segments.ts +++ b/assets/js/dashboard/segments/segments.ts @@ -1,7 +1,7 @@ /** @format */ -import { Filter } from '../query' -import { remapFromApiFilters } from '../util/filters' +import { DashboardQuery, Filter } from '../query' +import { plainFilterText, remapFromApiFilters } from '../util/filters' export enum SegmentType { personal = 'personal', @@ -20,13 +20,17 @@ export type SegmentData = { labels: Record } -export type EditingSegmentState = { - /** null means to definitively close the edit mode */ - editingSegment: SavedSegment | null -} - const SEGMENT_LABEL_KEY_PREFIX = 'segment-' +export const getSegmentNamePlaceholder = (query: DashboardQuery) => + query.filters.reduce( + (combinedName, filter) => + combinedName.length > 100 + ? combinedName + : `${combinedName}${combinedName.length ? ' and ' : ''}${plainFilterText(query, filter)}`, + '' + ) + export function isSegmentIdLabelKey(labelKey: string): boolean { return labelKey.startsWith(SEGMENT_LABEL_KEY_PREFIX) } diff --git a/assets/js/dashboard/site-switcher.js b/assets/js/dashboard/site-switcher.js index 06c5258741d7..5b9b6f5f3567 100644 --- a/assets/js/dashboard/site-switcher.js +++ b/assets/js/dashboard/site-switcher.js @@ -234,7 +234,7 @@ export default class SiteSwitcher extends React.Component { : 'cursor-default' return ( -
+
+ ) + }, + { + key: 'type', + label: 'Type', + width: 'w-16', + align: 'right', + renderValue: ({ type }) => + ({ personal: 'Personal', site: 'Site' })[type] + } + ], + [] + ) + + return ( + +
+
+

Segments

+
+ setSearch(v)} /> +
+
+
+ ({ + ...i, + selected: selectedSegmentIds.includes(i.id) + })) + .filter((i) => + search?.trim().length + ? i.name.toLowerCase().includes(search.trim().toLowerCase()) + : true + ) ?? [] + } + /> +
+ { + const nonSegmentFilters = query.filters.filter( + (f) => !isSegmentFilter(f) + ) + if (!selectedSegmentIds.length) { + return { + ...s, + filters: nonSegmentFilters, + labels: cleanLabels( + nonSegmentFilters, + query.labels, + 'segment', + {} + ) + } + } + const filters = nonSegmentFilters.concat([ + ['is', 'segment', selectedSegmentIds] + ]) + const labels = cleanLabels( + filters, + query.labels, + 'segment', + Object.fromEntries( + selectedSegmentIds.map((id) => [ + formatSegmentIdAsLabelKey(id), + data?.find((i) => i.id === id)?.name ?? '' + ]) + ) + ) + return { + ...s, + filters, + labels + } + }} + > + Apply Segments + +
+ + + ) +} diff --git a/assets/js/dashboard/segments/segments-dropdown.tsx b/assets/js/dashboard/segments/segments-dropdown.tsx index f3fdf5f1f338..b3bdc9b41c12 100644 --- a/assets/js/dashboard/segments/segments-dropdown.tsx +++ b/assets/js/dashboard/segments/segments-dropdown.tsx @@ -24,19 +24,18 @@ import { formatDayShort, parseUTCDate } from '../util/date' import { useUserContext } from '../user-context' import { ArrowsPointingInIcon, - ArrowsPointingOutIcon + ArrowsPointingOutIcon, + ChevronRightIcon } from '@heroicons/react/24/solid' import { SegmentExpandedLocationState, useSegmentExpandedContext } from './segment-expanded-context' +import { filterRoute } from '../router' -export const SegmentsList = ({ closeList }: { closeList: () => void }) => { - const { expandedSegment } = useSegmentExpandedContext() - const { query } = useQueryContext() +export const useSegmentsListQuery = () => { const site = useSiteContext() - - const { data } = useQuery({ + return useQuery({ queryKey: ['segments'], placeholderData: (previousData) => previousData, queryFn: async () => { @@ -63,6 +62,14 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { return response } }) +} + +export const SegmentsList = ({ closeList }: { closeList: () => void }) => { + const { expandedSegment } = useSegmentExpandedContext() + const { query } = useQueryContext() + const site = useSiteContext() + + const { data } = useSegmentsListQuery() const segmentFilter = query.filters.find(isSegmentFilter) const appliedSegmentIds = (segmentFilter ? segmentFilter[2] : []) as number[] @@ -81,8 +88,8 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { } as SegmentExpandedLocationState }} > - {expandedSegment.name} - +
{expandedSegment.name}
+ s} @@ -126,7 +133,7 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { <> {!!data?.length && ( - {data.map((s) => { + {data.slice(0, 4).map((s) => { const authorLabel = (() => { if (!site.members) { return '' @@ -179,6 +186,14 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { ) })} + s} + onLinkClick={closeList} + > + View all + )} @@ -309,12 +324,12 @@ const SegmentLink = ({ actions={ !canSeeActions ? null : ( <> - + ) } > - {name} +
{name}
 ) } diff --git a/assets/js/dashboard/stats/modals/filter-modal.js b/assets/js/dashboard/stats/modals/filter-modal.js index 6762b3bee36d..c94c1060bb22 100644 --- a/assets/js/dashboard/stats/modals/filter-modal.js +++ b/assets/js/dashboard/stats/modals/filter-modal.js @@ -9,6 +9,7 @@ import { isModifierPressed, isTyping } from '../../keybinding'; import FilterModalGroup from "./filter-modal-group"; import { rootRoute } from '../../router'; import { useAppNavigate } from '../../navigation/use-app-navigate'; +import { AllSegmentsModal } from '../../segments/segment-modals'; function partitionFilters(modalType, filters) { const otherFilters = [] @@ -199,6 +200,9 @@ export default function FilterModalWithRouter(props) { const { field } = useParams() const { query } = useQueryContext() const site = useSiteContext() + if (field === 'segment') { + return + } return ( Date: Tue, 19 Nov 2024 11:18:43 +0200 Subject: [PATCH 06/18] Allow expanding segments from View all modal --- .../js/dashboard/segments/segment-modals.tsx | 27 +++- .../dashboard/segments/segments-dropdown.tsx | 116 ++++++++++-------- 2 files changed, 88 insertions(+), 55 deletions(-) diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx index e158da6d2945..5241500cde2e 100644 --- a/assets/js/dashboard/segments/segment-modals.tsx +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -10,16 +10,26 @@ import { SegmentType } from './segments' import { ColumnConfiguraton, Table } from '../components/table' -import { useSegmentsListQuery } from './segments-dropdown' +import { useSegmentPrefetch, useSegmentsListQuery } from './segments-dropdown' import { SearchInput } from '../components/search-input' import { useQueryContext } from '../query-context' import { AppNavigationLink } from '../navigation/use-app-navigate' import { cleanLabels } from '../util/filters' import { rootRoute } from '../router' +import { ArrowsPointingOutIcon } from '@heroicons/react/24/solid' const buttonClass = 'h-12 text-md font-medium py-2 px-3 rounded border dark:border-gray-100 dark:text-gray-100' +export const ExpandSegmentButton = ({ id }: Pick) => { + const { prefetchSegment, expandSegment } = useSegmentPrefetch({ id }) + return ( + + ) +} + export const CreateSegmentModal = ({ segment, close, @@ -256,11 +266,13 @@ export const AllSegmentsModal = () => { { key: 'name', label: 'Segment', - width: 'w-80', + width: 'w-full', align: 'left', renderItem: ({ id, name, selected }) => ( ) } + +// const EditSegmentIcon = () => ( +// +// +// +// ) From 3687270191f87d14d889c7df152c474b200ca28e Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Wed, 20 Nov 2024 19:27:05 +0200 Subject: [PATCH 08/18] Update dropdown behaviour --- assets/js/dashboard/components/dropdown.tsx | 7 +- .../js/dashboard/components/search-input.tsx | 5 +- assets/js/dashboard/nav-menu/filter-menu.tsx | 4 +- .../js/dashboard/segments/segment-modals.tsx | 283 ++++++++++-------- .../dashboard/segments/segments-dropdown.tsx | 201 ++++++++----- assets/js/dashboard/segments/segments.ts | 9 + assets/js/dashboard/stats/modals/modal.js | 68 +++-- 7 files changed, 330 insertions(+), 247 deletions(-) diff --git a/assets/js/dashboard/components/dropdown.tsx b/assets/js/dashboard/components/dropdown.tsx index 774bd0ca4612..14ba7577b2a8 100644 --- a/assets/js/dashboard/components/dropdown.tsx +++ b/assets/js/dashboard/components/dropdown.tsx @@ -23,7 +23,10 @@ export const DropdownSubtitle = ({ className?: string }) => (
{children}
@@ -167,7 +170,7 @@ export const DropdownNavigationLink = ({ 'hover:bg-gray-100 hover:text-gray-900 dark:hover:bg-gray-900 dark:hover:text-gray-100': !props['aria-disabled'] }, - { 'cursor-not-allowed text-gray-500': props['aria-disabled'] }, + 'aria-disabled:cursor-not-allowed aria-disabled:text-gray-500', !!actions && 'pr-4', className )} diff --git a/assets/js/dashboard/components/search-input.tsx b/assets/js/dashboard/components/search-input.tsx index 06872f97556d..7a0e475f46f9 100644 --- a/assets/js/dashboard/components/search-input.tsx +++ b/assets/js/dashboard/components/search-input.tsx @@ -6,11 +6,13 @@ import { useDebounce } from '../custom-hooks' import classNames from 'classnames' export const SearchInput = ({ + className, onSearch, - className + initialValue }: { className?: string onSearch: (value: string) => void + initialValue?: string }) => { const searchBoxRef = useRef(null) const [isFocused, setIsFocused] = useState(false) @@ -63,6 +65,7 @@ export const SearchInput = ({ ref={searchBoxRef} type="text" placeholder={isFocused ? 'Search' : 'Press / to search'} + value={initialValue} 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', className diff --git a/assets/js/dashboard/nav-menu/filter-menu.tsx b/assets/js/dashboard/nav-menu/filter-menu.tsx index 89282767bad5..2f157fc33252 100644 --- a/assets/js/dashboard/nav-menu/filter-menu.tsx +++ b/assets/js/dashboard/nav-menu/filter-menu.tsx @@ -274,7 +274,7 @@ export const FilterMenu = () => { )} segment={expandedSegment!} namePlaceholder={getSegmentNamePlaceholder(query)} - close={() => + onClose={() => navigate({ search: (s) => s, state: { expandedSegment: expandedSegment, modal: null } @@ -295,7 +295,7 @@ export const FilterMenu = () => { {user.loggedIn && modal === 'delete' && expandedSegment && ( + onClose={() => navigate({ search: (s) => s, state: { expandedSegment: expandedSegment, modal: null } diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx index 513b2afff9f1..0342badad14d 100644 --- a/assets/js/dashboard/segments/segment-modals.tsx +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -1,44 +1,63 @@ /** @format */ -import React, { useMemo, useState } from 'react' +import React, { ReactNode, useMemo, useState } from 'react' import ModalWithRouting from '../stats/modals/modal' import classNames from 'classnames' import { formatSegmentIdAsLabelKey, + getFilterSegmentsByNameInsensitive, isSegmentFilter, SavedSegment, SegmentType } from './segments' import { ColumnConfiguraton, Table } from '../components/table' -import { useSegmentPrefetch, useSegmentsListQuery } from './segments-dropdown' +import { + ExpandSegment, + useSegmentPrefetch, + useSegmentsListQuery +} from './segments-dropdown' import { SearchInput } from '../components/search-input' import { useQueryContext } from '../query-context' import { AppNavigationLink } from '../navigation/use-app-navigate' import { cleanLabels } from '../util/filters' import { rootRoute } from '../router' -import { ArrowsPointingOutIcon } from '@heroicons/react/24/solid' +// import { ArrowsPointingOutIcon } from '@heroicons/react/24/solid' const buttonClass = 'h-12 text-md font-medium py-2 px-3 rounded border dark:border-gray-100 dark:text-gray-100' -export const ExpandSegmentButton = ({ id }: Pick) => { +const ExpandSegmentButton = ({ id }: Pick) => { const { prefetchSegment, expandSegment } = useSegmentPrefetch({ id }) return ( - + ) } +const SegmentActionModal = ({ + children, + onClose +}: { + children: ReactNode + onClose: () => void +}) => ( + + {children} + +) + export const CreateSegmentModal = ({ segment, - close, + onClose, onSave, canTogglePersonal, namePlaceholder }: { segment?: SavedSegment - close: () => void + onClose: () => void onSave: (input: Pick) => void canTogglePersonal: boolean namePlaceholder: string @@ -46,65 +65,27 @@ export const CreateSegmentModal = ({ const [name, setName] = useState( segment?.name ? `Copy of ${segment.name}` : '' ) - const [type, setType] = useState(SegmentType.personal) + const [type, setType] = useState( + segment?.type === SegmentType.site && canTogglePersonal + ? SegmentType.site + : SegmentType.personal + ) return ( - -

- Create segment -

- - + Create segment + setName(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" + onChange={setName} + namePlaceholder={namePlaceholder} /> -
- Add a name to your segment to make it easier to find -
-
- - - Show this segment for all site users - -
-
- -
- + + ) } export const DeleteSegmentModal = ({ - close, + onClose, onSave, segment }: { - close: () => void + onClose: () => void onSave: (input: Pick) => void segment: SavedSegment }) => { return ( - +

{ { personal: 'Delete personal segment', site: 'Delete site segment' }[ @@ -143,8 +124,8 @@ export const DeleteSegmentModal = ({ } {` "${segment.name}"?`}

-
- -
- + + ) } -export const UpdateSegmentModal = ({ - close, - onSave, - segment, - canTogglePersonal, - namePlaceholder +const FormTitle = ({ children }: { children?: ReactNode }) => ( +

{children}

+) + +const ButtonsRow = ({ children }: { children?: ReactNode }) => ( +
{children}
+) + +const SegmentNameInput = ({ + namePlaceholder, + value, + onChange }: { - close: () => void - onSave: (input: Pick) => void - segment: SavedSegment - canTogglePersonal: boolean namePlaceholder: string + value: string + onChange: (value: string) => void }) => { - const [name, setName] = useState(segment.name) - const [type, setType] = useState(segment.type) - return ( - -

- Update segment -

+ <>
@@ -214,7 +214,6 @@ const SegmentTypeInput = ({ id="segment-type-site" type="radio" value="" - // name="default-radio" onClick={() => onChange(SegmentType.site)} className="w-4 h-4 text-blue-600 bg-gray-100 border-gray-300 focus:ring-blue-500 dark:focus:ring-blue-600 dark:ring-offset-gray-800 focus:ring-2 dark:bg-gray-700 dark:border-gray-600" disabled={disabled} @@ -224,7 +223,7 @@ const SegmentTypeInput = ({ className="ms-3 text-sm font-medium text-gray-900 dark:text-gray-300" >
Site segment
-
Visible to others on the site
+
Visible to others on the site
@@ -281,6 +280,102 @@ export const UpdateSegmentModal = ({ ) } +const ExpandSegmentButton = ({ + className, + onClick, + onMouseEnter, + expanded +}: { + className?: string + onClick: () => Promise + onMouseEnter?: () => Promise + expanded: boolean +}) => { + return ( + + ) +} +const SegmentRow = ({ + id, + name, + type, + toggleSelected, + selected +}: SavedSegment & { toggleSelected: () => void; selected: boolean }) => { + const { prefetchSegment, data, expandSegment, fetchSegment } = + useSegmentPrefetch({ + id + }) + const [segmentDataVisible, setSegmentDataVisible] = useState(false) + + return ( +
+ {/* */} +
+ {name} + {/* {' · '} */} + {/* + {{ personal: 'personal', site: 'site' }[type]} + */} +
+ setSegmentDataVisible(false) + : async () => { + setSegmentDataVisible(true) + fetchSegment() + } + } + > + { + expandSegment(data ?? (await fetchSegment())) + }} + /> + + {segmentDataVisible && ( +
+ {data?.segment_data ? ( + ({ + plainText: plainFilterText(data.segment_data.labels, filter), + children: styledFilterText(data.segment_data.labels, filter), + interactive: false + }))} + /> + ) : ( + 'loading' + )} + +
+ )} +
+ ) +} + export const AllSegmentsModal = () => { const { query } = useQueryContext() const querySegmentIds: number[] = @@ -289,51 +384,29 @@ export const AllSegmentsModal = () => { const [search, setSearch] = useState() const [selectedSegmentIds, setSelectedSegmentIds] = useState(querySegmentIds) + const getToggleSelected = useCallback( + (id: number) => () => + setSelectedSegmentIds((current) => + current.includes(id) + ? current.filter((i) => i !== id) + : current.concat([id]) + ), + [] + ) - const columns: ColumnConfiguraton[] = - useMemo( - () => [ - { - key: 'name', - label: 'Name', - width: 'w-60', - align: 'left', - renderItem: ({ id, name, selected }) => ( - - ) - }, - { - key: 'type', - label: 'Type', - width: 'w-20', - align: 'right', - renderValue: ({ type }) => - ({ personal: 'Personal', site: 'Site' })[type] - }, - { - key: 'owner_id', - width: 'w-8', - align: 'right', - label: '', - renderItem: ExpandSegmentButton - } - ], - [] - ) + const proposedSegmentFilter: Filter | null = selectedSegmentIds.length + ? ['is', 'segment', selectedSegmentIds] + : null + + const labelsForProposedSegmentFilter = !data + ? {} + : Object.fromEntries( + data?.flatMap((d) => + selectedSegmentIds.includes(d.id) + ? [[formatSegmentIdAsLabelKey(d.id), d.name]] + : [] + ) + ) return ( @@ -344,19 +417,61 @@ export const AllSegmentsModal = () => { setSearch(v)} />
+ +
+ {(data?.filter(getFilterSegmentsByNameInsensitive(search)) ?? []).map( + (item) => ( + + ) + )} +
+
+
-
({ - ...i, - selected: selectedSegmentIds.includes(i.id) - })) - .filter(getFilterSegmentsByNameInsensitive(search)) ?? [] - } - /> -
+ {!!data && !!proposedSegmentFilter && ( +
+ {/* ({ + children: styledFilterText(labelsForProposedSegmentFilter, [ + proposedSegmentFilter[0], + proposedSegmentFilter[1], + [c] + ]), + plainText: 'hi', + interactive: false + }))} + /> */} + setSelectedSegmentIds([])} + > + + + } + > + {styledFilterText( + labelsForProposedSegmentFilter, + proposedSegmentFilter + )} + +
+ )} +
{ const nonSegmentFilters = query.filters.filter( (f) => !isSegmentFilter(f) ) - if (!selectedSegmentIds.length) { + if (!proposedSegmentFilter) { return { ...s, filters: nonSegmentFilters, @@ -376,9 +491,7 @@ export const AllSegmentsModal = () => { ) } } - const filters = nonSegmentFilters.concat([ - ['is', 'segment', selectedSegmentIds] - ]) + const filters = nonSegmentFilters.concat([proposedSegmentFilter]) const labels = cleanLabels( filters, query.labels, @@ -397,7 +510,28 @@ export const AllSegmentsModal = () => { } }} > - Apply Segments + Apply selected + + { + const nonSegmentFilters = query.filters.filter( + (f) => !isSegmentFilter(f) + ) + return { + ...s, + filters: nonSegmentFilters, + labels: cleanLabels( + nonSegmentFilters, + query.labels, + 'segment', + {} + ) + } + }} + > + Clear selected
diff --git a/assets/js/dashboard/segments/segments-dropdown.tsx b/assets/js/dashboard/segments/segments-dropdown.tsx index d1d376df2aeb..98fade4a98f2 100644 --- a/assets/js/dashboard/segments/segments-dropdown.tsx +++ b/assets/js/dashboard/segments/segments-dropdown.tsx @@ -25,7 +25,6 @@ import { } from '../navigation/use-app-navigate' import classNames from 'classnames' import { Tooltip } from '../util/tooltip' -import { formatDayShort, parseUTCDate } from '../util/date' import { useUserContext } from '../user-context' import { ChevronLeftIcon, ChevronRightIcon } from '@heroicons/react/24/solid' import { @@ -34,6 +33,7 @@ import { } from './segment-expanded-context' import { filterRoute, rootRoute } from '../router' import { SearchInput } from '../components/search-input' +import { SegmentAuthorship } from './segment-authorship' export const useSegmentsListQuery = () => { const site = useSiteContext() @@ -71,7 +71,6 @@ const linkClass = 'text-xs' export const SegmentsList = ({ closeList }: { closeList: () => void }) => { const { expandedSegment } = useSegmentExpandedContext() const { query } = useQueryContext() - const site = useSiteContext() const { data } = useSegmentsListQuery() @@ -181,42 +180,16 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { {segments!.slice(0, 3).map((s) => { - const authorLabel = (() => { - if (!site.members) { - return '' - } - - if (!s.owner_id || !site.members[s.owner_id]) { - return '(Removed User)' - } - - // if (s.owner_id === user.id) { - // return 'You' - // } - - return site.members[s.owner_id] - })() - - const showUpdatedAt = s.updated_at !== s.inserted_at - return (
{s.name}
-
- {`Created at ${formatDayShort(parseUTCDate(s.inserted_at))}`} - {!showUpdatedAt && - !!authorLabel && - ` by ${authorLabel}`} -
- {showUpdatedAt && ( -
- {`Last updated at ${formatDayShort(parseUTCDate(s.updated_at))}`} - {!!authorLabel && ` by ${authorLabel}`} -
- )} + } > @@ -232,7 +205,10 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { ))} {!!data?.length && ( s} @@ -280,7 +256,10 @@ const SaveSelectionAsSegment = ({ closeList }: { closeList: () => void }) => { if (disabledReason === null) { return ( s} navigateOptions={{ state: { @@ -363,31 +342,33 @@ export const useSegmentPrefetch = ({ id }: Pick) => { [queryClient, getSegmentFn, queryKey] ) - const expandSegment = useCallback(async () => { - try { - const data = getSegment.data ?? (await fetchSegment()) - navigate({ - path: rootRoute.path, - search: (search) => ({ - ...search, - filters: data.segment_data.filters, - labels: data.segment_data.labels - }), - state: { - expandedSegment: { - id: data.id, - name: data.name, - type: data.type, - owner_id: data.owner_id - } - } as SegmentExpandedLocationState - }) - } catch (_error) { - return - } - }, [fetchSegment, getSegment.data, navigate]) + const expandSegment = useCallback( + (segment: SavedSegment & { segment_data: SegmentData }) => { + try { + navigate({ + path: rootRoute.path, + search: (search) => ({ + ...search, + filters: segment.segment_data.filters, + labels: segment.segment_data.labels + }), + state: { + expandedSegment: { + id: segment.id, + name: segment.name, + type: segment.type, + owner_id: segment.owner_id + } + } as SegmentExpandedLocationState + }) + } catch (_error) { + return + } + }, + [navigate] + ) - return { prefetchSegment, expandSegment } + return { prefetchSegment, data: getSegment.data, fetchSegment, expandSegment } } const SegmentLink = ({ @@ -406,7 +387,8 @@ const SegmentLink = ({ // (type === SegmentType.site && // ['admin', 'owner', 'super_admin'].includes(user.role))) const { query } = useQueryContext() - const { prefetchSegment, expandSegment } = useSegmentPrefetch({ id }) + const { prefetchSegment, expandSegment, data, fetchSegment } = + useSegmentPrefetch({ id }) return ( - + + expandSegment(data ?? (await fetchSegment())) + } + /> ) } @@ -457,7 +444,7 @@ const SegmentLink = ({ ) } -export const ExpandSegment = ({ +export const EditSegment = ({ className, onClick, onMouseEnter From 9ce987d8b1686dfeb2f4ea032af7f19253ab5f3f Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Wed, 27 Nov 2024 12:22:10 +0200 Subject: [PATCH 14/18] WIP3 --- .../dashboard/nav-menu/filter-pills-list.tsx | 2 +- assets/js/dashboard/nav-menu/filters-bar.tsx | 2 +- .../js/dashboard/segments/segment-modals.tsx | 116 ++++++++++++++---- .../dashboard/segments/segments-dropdown.tsx | 2 +- lib/plausible/stats/table_decider.ex | 10 +- 5 files changed, 102 insertions(+), 30 deletions(-) diff --git a/assets/js/dashboard/nav-menu/filter-pills-list.tsx b/assets/js/dashboard/nav-menu/filter-pills-list.tsx index a1e161cff7dc..3f7cd751faf1 100644 --- a/assets/js/dashboard/nav-menu/filter-pills-list.tsx +++ b/assets/js/dashboard/nav-menu/filter-pills-list.tsx @@ -34,7 +34,7 @@ type NoRenderOutsideSlice = { type AppliedFilterPillsListProps = Omit< FilterPillsListProps, - 'slice' | 'pillProps' + 'slice' | 'pillProps' | 'pills' > & { slice?: InvisibleOutsideSlice | NoRenderOutsideSlice } type FilterPillsListProps = { diff --git a/assets/js/dashboard/nav-menu/filters-bar.tsx b/assets/js/dashboard/nav-menu/filters-bar.tsx index f41185f46017..2a71cb020bbf 100644 --- a/assets/js/dashboard/nav-menu/filters-bar.tsx +++ b/assets/js/dashboard/nav-menu/filters-bar.tsx @@ -201,7 +201,7 @@ export const FiltersBar = () => { {opened && typeof visibility.visibleCount === 'number' ? ( {expanded ? ( - + ) : ( - + )} ) @@ -311,7 +313,7 @@ const ExpandSegmentButton = ({ const SegmentRow = ({ id, name, - type, + // type, toggleSelected, selected }: SavedSegment & { toggleSelected: () => void; selected: boolean }) => { @@ -320,16 +322,21 @@ const SegmentRow = ({ id }) const [segmentDataVisible, setSegmentDataVisible] = useState(false) - return (
{/* */} -
+
{name} {/* {' · '} */} {/* @@ -347,7 +354,7 @@ const SegmentRow = ({ fetchSegment() } } - > + /> { expandSegment(data ?? (await fetchSegment())) @@ -408,8 +415,26 @@ export const AllSegmentsModal = () => { ) ) + const searchResults = data?.filter(getFilterSegmentsByNameInsensitive(search)) + + const personalSegments = searchResults?.filter( + (i) => i.type === SegmentType.personal + ) + const siteSegments = searchResults?.filter((i) => i.type === SegmentType.site) + + const [upToPersonalSegment, setUpToPersonalSegment] = useState(4) + const [upToSiteSegment, setUpToSiteSegment] = useState(4) + + useEffect(() => { + setUpToPersonalSegment(4) + setUpToSiteSegment(4) + }, [data, search]) + return ( - +

Segments

@@ -419,22 +444,62 @@ export const AllSegmentsModal = () => {
- {(data?.filter(getFilterSegmentsByNameInsensitive(search)) ?? []).map( - (item) => ( - - ) + {[ + { + segments: personalSegments, + title: 'Personal', + sliceEnd: upToPersonalSegment, + showMore: () => setUpToPersonalSegment((curr) => curr + 10) + }, + { + segments: siteSegments, + title: 'Site', + sliceEnd: upToSiteSegment, + showMore: () => setUpToSiteSegment((curr) => curr + 10) + } + ] + .filter((i) => !!i.segments?.length) + .map(({ segments, title, sliceEnd, showMore }) => ( + <> +

+ {title} +

+ {segments!.slice(0, sliceEnd).map((item) => ( + + ))} + {segments?.length && sliceEnd < segments.length && ( + + )} + + ))} + {!personalSegments?.length && !siteSegments?.length && ( + + No segments found.{' '} + {/* {!!search?.trim().length && ( + + )} */} + )}
-
+ {/*
*/} + +
+

Selected filter

-
{!!data && !!proposedSegmentFilter && ( -
+
{/* {
)} + {proposedSegmentFilter === null && No segments selected.}
{ } }} > - Apply selected + Apply { } }} > - Clear selected + Clear
diff --git a/assets/js/dashboard/segments/segments-dropdown.tsx b/assets/js/dashboard/segments/segments-dropdown.tsx index 98fade4a98f2..f9b1a07f914d 100644 --- a/assets/js/dashboard/segments/segments-dropdown.tsx +++ b/assets/js/dashboard/segments/segments-dropdown.tsx @@ -456,7 +456,7 @@ export const EditSegment = ({ return ( - ) -} const SegmentRow = ({ id, name, - // type, toggleSelected, selected }: SavedSegment & { toggleSelected: () => void; selected: boolean }) => { @@ -322,44 +293,53 @@ const SegmentRow = ({ id }) const [segmentDataVisible, setSegmentDataVisible] = useState(false) + const toggleSegmentDataVisible = useCallback(async () => { + setSegmentDataVisible((currentVisible) => { + if (currentVisible) { + return false + } + fetchSegment() + return true + }) + }, [fetchSegment]) return (
- {/* */} -
+
+ {name} +
+
+ {segmentDataVisible ? ( + + ) : ( + + )} +
+ + +
- setSegmentDataVisible(false) - : async () => { - setSegmentDataVisible(true) - fetchSegment() - } - } - /> - { - expandSegment(data ?? (await fetchSegment())) - }} - /> + + {segmentDataVisible && (
@@ -376,7 +356,35 @@ const SegmentRow = ({ ) : ( 'loading' )} - + {!!data && } +
+ + s} + state={ + { + expandedSegment: data, + modal: 'delete' + } as SegmentExpandedLocationState + } + // onClick={async () => { + // expandSegment(data ?? (await fetchSegment())) + // }} + > + + Delete + +
)}
@@ -473,46 +481,22 @@ export const AllSegmentsModal = () => { /> ))} {segments?.length && sliceEnd < segments.length && ( - )} ))} {!personalSegments?.length && !siteSegments?.length && ( - - No segments found.{' '} - {/* {!!search?.trim().length && ( - - )} */} - + No segments found. )}
- {/*
*/}

Selected filter

{!!data && !!proposedSegmentFilter && (
- {/* ({ - children: styledFilterText(labelsForProposedSegmentFilter, [ - proposedSegmentFilter[0], - proposedSegmentFilter[1], - [c] - ]), - plainText: 'hi', - interactive: false - }))} - /> */} { const site = useSiteContext() + const { query } = useQueryContext() + const segmentsFilter = query.filters.find(isSegmentFilter) + const appliedSegmentIds = segmentsFilter + ? (segmentsFilter[2] as number[]) + : [] return useQuery({ queryKey: ['segments'], placeholderData: (previousData) => previousData, @@ -50,18 +55,13 @@ export const useSegmentsListQuery = () => { accept: 'application/json' } } - ).then( - ( - res - ): Promise< - (SavedSegment & { - owner_id: number - inserted_at: string - updated_at: string - })[] - > => res.json() + ).then((res): Promise => res.json()) + + return response.sort( + (a, b) => + appliedSegmentIds.findIndex((id) => id === b.id) - + appliedSegmentIds.findIndex((id) => id === a.id) ) - return response } }) } @@ -98,11 +98,9 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { modal: null } as SegmentExpandedLocationState } - // onClick={closeList} > - {/* */} -
Back to filters
+
Cancel editing
@@ -294,7 +292,7 @@ export const useSegmentPrefetch = ({ id }: Pick) => { const navigate = useAppNavigate() const getSegmentFn: QueryFunction< - { segment_data: SegmentData } & SavedSegment, + SavedSegment & { segment_data: SegmentData }, typeof queryKey > = useCallback( async ({ queryKey: [_, id] }) => { @@ -429,12 +427,15 @@ const SegmentLink = ({ actions={ !canSeeActions ? null : ( <> - expandSegment(data ?? (await fetchSegment())) } - /> + > + + ) } @@ -444,11 +445,16 @@ const SegmentLink = ({ ) } +export const iconButtonClass = + 'flex items-center justify-center w-5 h-5 fill-current hover:fill-indigo-600' + export const EditSegment = ({ + children, className, onClick, onMouseEnter }: { + children?: ReactNode onClick: () => Promise onMouseEnter?: () => Promise className?: string @@ -462,13 +468,13 @@ export const EditSegment = ({ onClick={onClick} onMouseEnter={onMouseEnter} > - {/* */} - + {children} + ) } -const EditSegmentIcon = ({ className }: { className?: string }) => ( +export const EditSegmentIcon = ({ className }: { className?: string }) => ( - query.filters.reduce( - (combinedName, filter) => - combinedName.length > 100 - ? combinedName - : `${combinedName}${combinedName.length ? ' and ' : ''}${plainFilterText(query.labels, filter)}`, - '' - ).slice(0,255) + query.filters + .reduce( + (combinedName, filter) => + combinedName.length > 100 + ? combinedName + : `${combinedName}${combinedName.length ? ' and ' : ''}${plainFilterText(query.labels, filter)}`, + '' + ) + .slice(0, 255) export function isSegmentIdLabelKey(labelKey: string): boolean { return labelKey.startsWith(SEGMENT_LABEL_KEY_PREFIX) From 99e1abbd9157ffe5a9a45d9a540f61f4f7510016 Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Thu, 28 Nov 2024 12:34:41 +0200 Subject: [PATCH 16/18] Disallow Delete from All Segments modal (buggy) --- .../js/dashboard/segments/segment-modals.tsx | 101 +++++++++++------- 1 file changed, 60 insertions(+), 41 deletions(-) diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx index 1ff5a9fa0d69..9d033e6bb526 100644 --- a/assets/js/dashboard/segments/segment-modals.tsx +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -25,16 +25,31 @@ import { XMarkIcon, ChevronUpIcon, ChevronDownIcon, - TrashIcon, + // TrashIcon, CheckIcon } from '@heroicons/react/24/outline' import { FilterPill } from '../nav-menu/filter-pill' import { Filter } from '../query' import { SegmentAuthorship } from './segment-authorship' -import { SegmentExpandedLocationState } from './segment-expanded-context' +// import { SegmentExpandedLocationState } from './segment-expanded-context' -const buttonClass = - 'h-12 text-md font-medium py-2 px-3 rounded border dark:border-gray-100 dark:text-gray-100' +const buttonClass = 'border text-md font-medium py-3 px-4 rounded-md' + +const primaryNeutralButtonClass = classNames( + buttonClass, + 'border-indigo-600 bg-indigo-600 text-gray-100' +) + +const primaryNegativeButtonClass = classNames( + buttonClass, + 'border-red-500 bg-red-500 text-gray-100' +) + +const secondaryButtonClass = classNames( + buttonClass, + // 'border-indigo-600 text-indigo-600', + 'border-indigo-500 text-indigo-500' +) const SegmentActionModal = ({ children, @@ -88,11 +103,11 @@ export const CreateSegmentModal = ({ disabled={!canTogglePersonal} /> - {` "${segment.name}"?`} -
-
+
{segmentDataVisible ? ( ) : ( @@ -348,6 +363,7 @@ const SegmentRow = ({ className="flex-wrap" direction="horizontal" pills={data.segment_data.filters.map((filter) => ({ + // className: 'dark:!bg-gray-700', plainText: plainFilterText(data.segment_data.labels, filter), children: styledFilterText(data.segment_data.labels, filter), interactive: false @@ -357,34 +373,36 @@ const SegmentRow = ({ 'loading' )} {!!data && } -
- - s} - state={ - { - expandedSegment: data, - modal: 'delete' - } as SegmentExpandedLocationState - } - // onClick={async () => { - // expandSegment(data ?? (await fetchSegment())) - // }} - > - - Delete - -
+ {!!data && ( +
+ + {/* s} + state={ + { + expandedSegment: data, + modal: 'delete' + } as SegmentExpandedLocationState + } + // onClick={async () => { + // expandSegment(data ?? (await fetchSegment())) + // }} + > + + Delete + */} +
+ )}
)}
@@ -498,6 +516,7 @@ export const AllSegmentsModal = () => { {!!data && !!proposedSegmentFilter && (
Date: Thu, 28 Nov 2024 13:12:53 +0200 Subject: [PATCH 17/18] Unify modal buttons --- .../js/dashboard/segments/segment-modals.tsx | 71 ++++++++++++------- .../dashboard/segments/segments-dropdown.tsx | 3 + 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/assets/js/dashboard/segments/segment-modals.tsx b/assets/js/dashboard/segments/segment-modals.tsx index 9d033e6bb526..3be2d6099407 100644 --- a/assets/js/dashboard/segments/segment-modals.tsx +++ b/assets/js/dashboard/segments/segment-modals.tsx @@ -7,6 +7,7 @@ import { getFilterSegmentsByNameInsensitive, isSegmentFilter, SavedSegment, + SegmentData, SegmentType } from './segments' import { @@ -33,22 +34,23 @@ import { Filter } from '../query' import { SegmentAuthorship } from './segment-authorship' // import { SegmentExpandedLocationState } from './segment-expanded-context' -const buttonClass = 'border text-md font-medium py-3 px-4 rounded-md' +const buttonClass = + 'transition border text-md font-medium py-3 px-4 rounded-md focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-indigo-500' const primaryNeutralButtonClass = classNames( buttonClass, - 'border-indigo-600 bg-indigo-600 text-gray-100' + 'bg-indigo-600 hover:bg-indigo-700 text-white border-transparent' ) const primaryNegativeButtonClass = classNames( buttonClass, - 'border-red-500 bg-red-500 text-gray-100' + 'border-transparent bg-red-500 hover:bg-red-600 text-white border-transparent' ) const secondaryButtonClass = classNames( buttonClass, - // 'border-indigo-600 text-indigo-600', - 'border-indigo-500 text-indigo-500' + 'border-indigo-500 text-indigo-500 hover:border-indigo-600 hover:text-indigo-600', + 'dark:hover:border-indigo-400 dark:hover:text-indigo-400' ) const SegmentActionModal = ({ @@ -130,18 +132,31 @@ export const DeleteSegmentModal = ({ }: { onClose: () => void onSave: (input: Pick) => void - segment: SavedSegment + segment: SavedSegment & { segment_data?: SegmentData } }) => { return ( -

+ { { personal: 'Delete personal segment', site: 'Delete site segment' }[ segment.type ] } {` "${segment.name}"?`} -

+ + {segment?.segment_data && ( + ({ + // className: 'dark:!bg-gray-700', + plainText: plainFilterText(segment.segment_data!.labels, filter), + children: styledFilterText(segment.segment_data!.labels, filter), + interactive: false + }))} + /> + )} + )} ))} {!personalSegments?.length && !siteSegments?.length && ( - No segments found. +

No segments found.

)}
@@ -539,10 +558,12 @@ export const AllSegmentsModal = () => {
)} - {proposedSegmentFilter === null && No segments selected.} -
+ {proposedSegmentFilter === null && ( +

No segments selected.

+ )} + { const nonSegmentFilters = query.filters.filter( @@ -582,7 +603,7 @@ export const AllSegmentsModal = () => { Apply { const nonSegmentFilters = query.filters.filter( @@ -602,7 +623,7 @@ export const AllSegmentsModal = () => { > Clear -
+
) diff --git a/assets/js/dashboard/segments/segments-dropdown.tsx b/assets/js/dashboard/segments/segments-dropdown.tsx index 27bedf33ea11..7872a91fdb14 100644 --- a/assets/js/dashboard/segments/segments-dropdown.tsx +++ b/assets/js/dashboard/segments/segments-dropdown.tsx @@ -115,6 +115,7 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { modal: 'update' } as SegmentExpandedLocationState }} + onClick={closeList} > Update segment @@ -127,6 +128,7 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { modal: 'create' } as SegmentExpandedLocationState }} + onClick={closeList} > Save as a new segment @@ -139,6 +141,7 @@ export const SegmentsList = ({ closeList }: { closeList: () => void }) => { modal: 'delete' } as SegmentExpandedLocationState }} + onClick={closeList} > Delete segment From e92c7b68fd148f44b5646025067dd40eafd66497 Mon Sep 17 00:00:00 2001 From: Artur Pata Date: Thu, 28 Nov 2024 16:22:26 +0200 Subject: [PATCH 18/18] Add missing Locations group --- assets/js/dashboard/nav-menu/filter-menu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/dashboard/nav-menu/filter-menu.tsx b/assets/js/dashboard/nav-menu/filter-menu.tsx index 150c6a0bf1c6..6b86c70ac8ff 100644 --- a/assets/js/dashboard/nav-menu/filter-menu.tsx +++ b/assets/js/dashboard/nav-menu/filter-menu.tsx @@ -63,7 +63,7 @@ export function getFilterListItems({ [ { title: 'Device', - modals: ['screen', 'browser', 'os'] + modals: ['location', 'screen', 'browser', 'os'] }, { title: 'Behaviour',