fix event_name parameter collision. Closes #4461
This commit is contained in:
1 parent
a2374e1515
commit
1a91b5f1f3
9 files changed
+281
-21
No files matched your search
@@ -0,0 +1,58 @@
|
||||
import { beforeEach, expect, test, vi } from 'vitest';
|
||||
import { getQueryFilters, parseRequest } from '@/lib/request';
|
||||
import { canViewWebsiteSection } from '@/permissions';
|
||||
import { getEventDataFields } from '@/queries/sql';
|
||||
import { GET } from './route';
|
||||
|
||||
vi.mock('@/lib/request', () => ({
|
||||
getQueryFilters: vi.fn(),
|
||||
parseRequest: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/permissions', () => ({
|
||||
canViewWebsiteSection: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/queries/sql', () => ({
|
||||
getEventDataFields: vi.fn(),
|
||||
}));
|
||||
|
||||
const parseRequestMock = vi.mocked(parseRequest);
|
||||
const getQueryFiltersMock = vi.mocked(getQueryFilters);
|
||||
const canViewWebsiteSectionMock = vi.mocked(canViewWebsiteSection);
|
||||
const getEventDataFieldsMock = vi.mocked(getEventDataFields);
|
||||
|
||||
beforeEach(() => {
|
||||
parseRequestMock.mockReset();
|
||||
getQueryFiltersMock.mockReset();
|
||||
canViewWebsiteSectionMock.mockReset();
|
||||
getEventDataFieldsMock.mockReset();
|
||||
});
|
||||
|
||||
test('uses eventName as the explicit event selector for property fields', async () => {
|
||||
const query = {
|
||||
startAt: 1786986000000,
|
||||
endAt: 1787075999999,
|
||||
event: 'eq.revenue-demo',
|
||||
eventName: 'revenue-demo',
|
||||
};
|
||||
const filters = { startDate: new Date('2026-08-17T07:00:00.000Z') };
|
||||
|
||||
parseRequestMock.mockResolvedValue({ auth: {}, query, error: undefined });
|
||||
canViewWebsiteSectionMock.mockResolvedValue(true);
|
||||
getQueryFiltersMock.mockResolvedValue(filters as any);
|
||||
getEventDataFieldsMock.mockResolvedValue([{ propertyName: 'currency', dataType: 1, total: 1 }] as any);
|
||||
|
||||
const response = await GET(
|
||||
new Request(
|
||||
'http://localhost/api/websites/website-1/event-data/fields?event=eq.revenue-demo&eventName=revenue-demo',
|
||||
),
|
||||
{
|
||||
params: Promise.resolve({ websiteId: 'website-1' }),
|
||||
},
|
||||
);
|
||||
|
||||
expect(getQueryFiltersMock).toHaveBeenCalledWith(query, 'website-1');
|
||||
expect(getEventDataFieldsMock).toHaveBeenCalledWith('website-1', 'revenue-demo', filters);
|
||||
expect(response.status).toBe(200);
|
||||
});
|
||||
@@ -12,6 +12,7 @@ export async function GET(
|
||||
const schema = z.object({
|
||||
startAt: z.coerce.number().int(),
|
||||
endAt: z.coerce.number().int(),
|
||||
eventName: z.string().optional(),
|
||||
...filterParams,
|
||||
});
|
||||
|
||||
@@ -27,9 +28,10 @@ export async function GET(
|
||||
return unauthorized();
|
||||
}
|
||||
|
||||
const { eventName } = query;
|
||||
const filters = await getQueryFilters(query, websiteId);
|
||||
|
||||
const data = await getEventDataFields(websiteId, filters);
|
||||
const data = await getEventDataFields(websiteId, eventName, filters);
|
||||
|
||||
return json(data);
|
||||
}
|
||||
@@ -0,0 +1,95 @@
|
||||
import { beforeEach, expect, test, vi } from 'vitest';
|
||||
import { getQueryFilters, parseRequest } from '@/lib/request';
|
||||
import { canViewWebsiteSection } from '@/permissions';
|
||||
import { getEventDataValues } from '@/queries/sql';
|
||||
import { GET } from './route';
|
||||
|
||||
vi.mock('@/lib/request', () => ({
|
||||
getQueryFilters: vi.fn(),
|
||||
parseRequest: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/permissions', () => ({
|
||||
canViewWebsiteSection: vi.fn(),
|
||||
}));
|
||||
|
||||
vi.mock('@/queries/sql', () => ({
|
||||
getEventDataValues: vi.fn(),
|
||||
}));
|
||||
|
||||
const parseRequestMock = vi.mocked(parseRequest);
|
||||
const getQueryFiltersMock = vi.mocked(getQueryFilters);
|
||||
const canViewWebsiteSectionMock = vi.mocked(canViewWebsiteSection);
|
||||
const getEventDataValuesMock = vi.mocked(getEventDataValues);
|
||||
|
||||
beforeEach(() => {
|
||||
parseRequestMock.mockReset();
|
||||
getQueryFiltersMock.mockReset();
|
||||
canViewWebsiteSectionMock.mockReset();
|
||||
getEventDataValuesMock.mockReset();
|
||||
});
|
||||
|
||||
test('uses eventName as the explicit event selector and leaves event filter expressions intact', async () => {
|
||||
const query = {
|
||||
startAt: 1786986000000,
|
||||
endAt: 1787075999999,
|
||||
event: 'eq.revenue-demo',
|
||||
eventName: 'revenue-demo',
|
||||
propertyName: 'currency',
|
||||
dataType: 1,
|
||||
};
|
||||
const filters = { startDate: new Date('2026-08-17T07:00:00.000Z') };
|
||||
|
||||
parseRequestMock.mockResolvedValue({ auth: {}, query, error: undefined });
|
||||
canViewWebsiteSectionMock.mockResolvedValue(true);
|
||||
getQueryFiltersMock.mockResolvedValue(filters as any);
|
||||
getEventDataValuesMock.mockResolvedValue([{ value: 'USD', total: 1 }] as any);
|
||||
|
||||
const response = await GET(
|
||||
new Request(
|
||||
'http://localhost/api/websites/website-1/event-data/values?event=eq.revenue-demo&eventName=revenue-demo&propertyName=currency&dataType=1',
|
||||
),
|
||||
{
|
||||
params: Promise.resolve({ websiteId: 'website-1' }),
|
||||
},
|
||||
);
|
||||
|
||||
expect(getQueryFiltersMock).toHaveBeenCalledWith(query, 'website-1');
|
||||
expect(getEventDataValuesMock).toHaveBeenCalledWith('website-1', 'revenue-demo', {
|
||||
...filters,
|
||||
propertyName: 'currency',
|
||||
dataType: 1,
|
||||
});
|
||||
expect(response.status).toBe(200);
|
||||
});
|
||||
|
||||
test('does not treat event filter expressions as exact event names when eventName is omitted', async () => {
|
||||
const query = {
|
||||
startAt: 1786986000000,
|
||||
endAt: 1787075999999,
|
||||
event: 'eq.revenue-demo',
|
||||
propertyName: 'currency',
|
||||
dataType: 1,
|
||||
};
|
||||
const filters = { startDate: new Date('2026-08-17T07:00:00.000Z') };
|
||||
|
||||
parseRequestMock.mockResolvedValue({ auth: {}, query, error: undefined });
|
||||
canViewWebsiteSectionMock.mockResolvedValue(true);
|
||||
getQueryFiltersMock.mockResolvedValue(filters as any);
|
||||
getEventDataValuesMock.mockResolvedValue([{ value: 'USD', total: 1 }] as any);
|
||||
|
||||
await GET(
|
||||
new Request(
|
||||
'http://localhost/api/websites/website-1/event-data/values?event=eq.revenue-demo&propertyName=currency&dataType=1',
|
||||
),
|
||||
{
|
||||
params: Promise.resolve({ websiteId: 'website-1' }),
|
||||
},
|
||||
);
|
||||
|
||||
expect(getEventDataValuesMock).toHaveBeenCalledWith('website-1', undefined, {
|
||||
...filters,
|
||||
propertyName: 'currency',
|
||||
dataType: 1,
|
||||
});
|
||||
});
|
||||
@@ -12,7 +12,7 @@ export async function GET(
|
||||
const schema = z.object({
|
||||
startAt: z.coerce.number().int(),
|
||||
endAt: z.coerce.number().int(),
|
||||
event: z.string().optional(),
|
||||
eventName: z.string().optional(),
|
||||
propertyName: z.string(),
|
||||
dataType: z.coerce.number().int().optional(),
|
||||
...filterParams,
|
||||
@@ -30,7 +30,7 @@ export async function GET(
|
||||
return unauthorized();
|
||||
}
|
||||
|
||||
const { event: eventName, propertyName, dataType } = query;
|
||||
const { eventName, propertyName, dataType } = query;
|
||||
const filters = await getQueryFilters(query, websiteId);
|
||||
|
||||
const data = await getEventDataValues(websiteId, eventName, {
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
import { renderHook } from '@testing-library/react';
|
||||
import { beforeEach, describe, expect, test, vi } from 'vitest';
|
||||
|
||||
const getMock = vi.fn();
|
||||
const useQueryMock = vi.fn(options => options);
|
||||
|
||||
vi.mock('../useApi', () => ({
|
||||
useApi: () => ({
|
||||
get: getMock,
|
||||
useQuery: useQueryMock,
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../useDateParameters', () => ({
|
||||
useDateParameters: () => ({
|
||||
startAt: 100,
|
||||
endAt: 200,
|
||||
unit: 'day',
|
||||
timezone: 'UTC',
|
||||
}),
|
||||
}));
|
||||
|
||||
vi.mock('../useFilterParameters', () => ({
|
||||
useFilterParameters: () => ({
|
||||
event: 'eq.signup',
|
||||
country: 'US',
|
||||
}),
|
||||
}));
|
||||
|
||||
import { DATA_TYPE } from '@/lib/constants';
|
||||
import { OPERATORS } from '@/lib/constants';
|
||||
import { usePropertyFieldsQuery } from './usePropertyFieldsQuery';
|
||||
import { usePropertyValuesQuery } from './usePropertyValuesQuery';
|
||||
|
||||
describe('event property query params', () => {
|
||||
beforeEach(() => {
|
||||
getMock.mockReset();
|
||||
useQueryMock.mockClear();
|
||||
});
|
||||
|
||||
test('keeps the selected event name when loading property values', async () => {
|
||||
renderHook(() =>
|
||||
usePropertyValuesQuery(
|
||||
'event',
|
||||
'website-1',
|
||||
'plan',
|
||||
DATA_TYPE.string,
|
||||
[{ propertyName: 'tier', dataType: DATA_TYPE.string, operator: OPERATORS.equals, value: 'pro' }],
|
||||
'signup',
|
||||
),
|
||||
);
|
||||
|
||||
const [{ queryFn }] = useQueryMock.mock.calls.at(-1) as [{ queryFn: () => Promise<unknown> }];
|
||||
|
||||
await queryFn();
|
||||
|
||||
expect(getMock).toHaveBeenCalledWith('/websites/website-1/event-data/values', {
|
||||
event: 'eq.signup',
|
||||
eventName: 'signup',
|
||||
country: 'US',
|
||||
startAt: 100,
|
||||
endAt: 200,
|
||||
unit: 'day',
|
||||
timezone: 'UTC',
|
||||
propertyName: 'plan',
|
||||
dataType: DATA_TYPE.string,
|
||||
pf_tier: `${DATA_TYPE.string}.${OPERATORS.equals}.pro`,
|
||||
});
|
||||
});
|
||||
|
||||
test('keeps the selected event name when loading property fields', async () => {
|
||||
renderHook(() => usePropertyFieldsQuery('event', 'website-1', 'signup'));
|
||||
|
||||
const [{ queryFn }] = useQueryMock.mock.calls.at(-1) as [{ queryFn: () => Promise<unknown> }];
|
||||
|
||||
await queryFn();
|
||||
|
||||
expect(getMock).toHaveBeenCalledWith('/websites/website-1/event-data/fields', {
|
||||
event: 'eq.signup',
|
||||
eventName: 'signup',
|
||||
country: 'US',
|
||||
startAt: 100,
|
||||
endAt: 200,
|
||||
unit: 'day',
|
||||
timezone: 'UTC',
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -26,12 +26,12 @@ export function usePropertyFieldsQuery(
|
||||
? `/websites/${websiteId}/event-data/fields`
|
||||
: `/websites/${websiteId}/session-data/properties`,
|
||||
{
|
||||
...(source === 'event' && eventName ? { event: eventName } : {}),
|
||||
...params,
|
||||
...(source === 'event' && eventName ? { eventName } : {}),
|
||||
startAt,
|
||||
endAt,
|
||||
unit,
|
||||
timezone,
|
||||
...params,
|
||||
},
|
||||
),
|
||||
enabled: !!websiteId,
|
||||
|
||||
@@ -40,7 +40,8 @@ export function usePropertyValuesQuery(
|
||||
? `/websites/${websiteId}/event-data/values`
|
||||
: `/websites/${websiteId}/session-data/values`,
|
||||
{
|
||||
...(source === 'event' && eventName ? { event: eventName } : {}),
|
||||
...params,
|
||||
...(source === 'event' && eventName ? { eventName } : {}),
|
||||
startAt,
|
||||
endAt,
|
||||
unit,
|
||||
@@ -48,7 +49,6 @@ export function usePropertyValuesQuery(
|
||||
propertyName,
|
||||
dataType,
|
||||
...serializePropertyFilters(propertyFilters),
|
||||
...params,
|
||||
},
|
||||
),
|
||||
enabled: !!(websiteId && propertyName),
|
||||
|
||||
@@ -31,16 +31,20 @@ export function MenuButton({
|
||||
</Button>
|
||||
<Popover side="bottom" align="start">
|
||||
<Menu aria-label="menu" style={{ minWidth: '140px' }}>
|
||||
{Children.map(children, child =>
|
||||
isValidElement(child)
|
||||
? cloneElement(child as ReactElement<{ onAction?: (key: Key) => void }>, {
|
||||
onAction: (key: Key) => {
|
||||
child.props.onAction?.(key);
|
||||
handleAction(key);
|
||||
},
|
||||
})
|
||||
: child,
|
||||
)}
|
||||
{Children.map(children, child => {
|
||||
if (!isValidElement(child)) {
|
||||
return child;
|
||||
}
|
||||
|
||||
const menuChild = child as ReactElement<{ onAction?: (key: Key) => void }>;
|
||||
|
||||
return cloneElement(menuChild, {
|
||||
onAction: (key: Key) => {
|
||||
menuChild.props.onAction?.(key);
|
||||
handleAction(key);
|
||||
},
|
||||
});
|
||||
})}
|
||||
</Menu>
|
||||
</Popover>
|
||||
</DialogTrigger>
|
||||
|
||||
@@ -5,19 +5,26 @@ import type { QueryFilters } from '@/lib/types';
|
||||
|
||||
const FUNCTION_NAME = 'getEventDataFields';
|
||||
|
||||
export async function getEventDataFields(...args: [websiteId: string, filters: QueryFilters]) {
|
||||
export async function getEventDataFields(
|
||||
...args: [websiteId: string, eventName: string | undefined, filters: QueryFilters]
|
||||
) {
|
||||
return runQuery({
|
||||
[PRISMA]: () => relationalQuery(...args),
|
||||
[CLICKHOUSE]: () => clickhouseQuery(...args),
|
||||
});
|
||||
}
|
||||
|
||||
async function relationalQuery(websiteId: string, filters: QueryFilters) {
|
||||
async function relationalQuery(
|
||||
websiteId: string,
|
||||
eventName: string | undefined,
|
||||
filters: QueryFilters,
|
||||
) {
|
||||
const { rawQuery, parseFilters } = prisma;
|
||||
const { filterQuery, cohortQuery, joinSessionQuery, queryParams } = parseFilters({
|
||||
...filters,
|
||||
websiteId,
|
||||
});
|
||||
const eventNameFilter = eventName ? 'and website_event.event_name = {{eventName}}' : '';
|
||||
|
||||
return rawQuery(
|
||||
`
|
||||
@@ -29,6 +36,7 @@ async function relationalQuery(websiteId: string, filters: QueryFilters) {
|
||||
join website_event on website_event.event_id = event_data.website_event_id
|
||||
and website_event.website_id = {{websiteId::uuid}}
|
||||
and website_event.created_at between {{startDate}} and {{endDate}}
|
||||
${eventNameFilter}
|
||||
${cohortQuery}
|
||||
${joinSessionQuery}
|
||||
where event_data.website_id = {{websiteId::uuid}}
|
||||
@@ -37,17 +45,20 @@ async function relationalQuery(websiteId: string, filters: QueryFilters) {
|
||||
group by data_key, data_type
|
||||
order by "total" desc, "propertyName" asc
|
||||
`,
|
||||
queryParams,
|
||||
{ ...queryParams, eventName },
|
||||
FUNCTION_NAME,
|
||||
);
|
||||
}
|
||||
|
||||
async function clickhouseQuery(
|
||||
websiteId: string,
|
||||
eventName: string | undefined,
|
||||
filters: QueryFilters,
|
||||
): Promise<{ propertyName: string; dataType: number; total: number }[]> {
|
||||
const { rawQuery, parseFilters } = clickhouse;
|
||||
const { filterQuery, cohortQuery, queryParams } = parseFilters({ ...filters, websiteId });
|
||||
const eventNameFilter = eventName ? 'and event_name = {eventName:String}' : '';
|
||||
const eventDataNameFilter = eventName ? 'and event_data.event_name = {eventName:String}' : '';
|
||||
|
||||
return rawQuery(
|
||||
`
|
||||
@@ -61,6 +72,7 @@ async function clickhouseQuery(
|
||||
from website_event
|
||||
where website_id = {websiteId:UUID}
|
||||
and created_at between {startDate:DateTime64} and {endDate:DateTime64}
|
||||
${eventNameFilter}
|
||||
and event_type = 2) website_event
|
||||
on website_event.event_id = event_data.event_id
|
||||
and website_event.session_id = event_data.session_id
|
||||
@@ -68,11 +80,12 @@ async function clickhouseQuery(
|
||||
${cohortQuery}
|
||||
where event_data.website_id = {websiteId:UUID}
|
||||
and event_data.created_at between {startDate:DateTime64} and {endDate:DateTime64}
|
||||
${eventDataNameFilter}
|
||||
${filterQuery}
|
||||
group by data_key, data_type
|
||||
order by total desc, propertyName asc
|
||||
`,
|
||||
queryParams,
|
||||
{ ...queryParams, eventName },
|
||||
FUNCTION_NAME,
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user