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 c33eae9c862f..8ef93661873e 100644 --- a/assets/js/types/query-api.d.ts +++ b/assets/js/types/query-api.d.ts @@ -63,7 +63,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 @@ -91,6 +91,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 8b4a7445a458..2453ed5a25f6 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."} @@ -543,7 +452,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 -> @@ -588,22 +497,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 f5bd669a48a3..532354f7fa30 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 92345e536210..e1593d3f677e 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -294,7 +294,7 @@ defmodule PlausibleWeb.Api.StatsController do end defp fetch_top_stats(site, query) do - goal_filter? = Filters.filtering_on_dimension?(query, "event:goal") + goal_filter? = Filters.filtering_on_dimension?(query.filters, "event:goal") cond do query.period == "30m" && goal_filter? -> @@ -392,7 +392,7 @@ defmodule PlausibleWeb.Api.StatsController do end defp fetch_other_top_stats(site, query) do - page_filter? = Filters.filtering_on_dimension?(query, "event:page") + page_filter? = Filters.filtering_on_dimension?(query.filters, "event:page") metrics = [:visitors, :visits, :pageviews, :sample_percent] @@ -464,7 +464,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]) @@ -551,10 +551,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" -> @@ -578,7 +578,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]) @@ -606,7 +606,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]) @@ -634,7 +634,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]) @@ -662,7 +662,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]) @@ -690,7 +690,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]) @@ -718,7 +718,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]) @@ -835,7 +835,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]) @@ -863,7 +863,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, @@ -899,7 +899,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, @@ -972,7 +972,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]) @@ -1032,7 +1032,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]) @@ -1076,7 +1076,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]) @@ -1108,7 +1108,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]) @@ -1140,7 +1140,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]) @@ -1181,7 +1181,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]) @@ -1213,7 +1213,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]) @@ -1254,7 +1254,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]) @@ -1344,7 +1344,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 @@ -1361,7 +1361,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 @@ -1533,7 +1533,7 @@ defmodule PlausibleWeb.Api.StatsController do requires_goal_filter? = metric in [:conversion_rate, :events] - if requires_goal_filter? and !Filters.filtering_on_dimension?(query, "event:goal") do + if requires_goal_filter? and !Filters.filtering_on_dimension?(query.filters, "event:goal") do {:error, "Metric `#{metric}` can only be queried with a goal filter"} else {:ok, metric} @@ -1564,7 +1564,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 df89b56cc582..d10c2e961ec7 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 69bcbce97b1a..5fbfb687d605 100644 --- a/priv/json-schemas/query-api-schema.json +++ b/priv/json-schemas/query-api-schema.json @@ -347,6 +347,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, @@ -392,10 +397,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 a12395c3ffef..5a2e0a63d19d 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -49,7 +49,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 @@ -497,6 +497,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do "metrics" => ["visitors"], "date_range" => "all", "filters" => [ + ["is", "segment", [200]], [ "or", [ @@ -516,6 +517,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 4f400fe5e793..94526ab65d7c 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 3344046d8f0a..c5f6bb6dc9c8 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -390,6 +390,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),