Skip to content

Commit c6861a7

Browse files
nicobytesnicobytes
authored
feat(analytics): enhance DotAnalyticsService (#35549)
This pull request refactors the DotAnalyticsService API and its consumers to use unified parameter objects for analytics queries, improving flexibility and maintainability. The service methods now accept a single options object (including optional fields like `granularity`, `eventType`, and `siteId`), and the store feature is updated accordingly. Additional tests are added for new query combinations, and utility functions are introduced to build query parameters consistently. **DotAnalyticsService API refactor and enhancements:** * Refactored all analytics service methods (`getTotalEvents`, `getUniqueVisitors`, `getTopContent`, `getPageviewsByDeviceBrowser`) to accept a single parameter object with optional fields (`granularity`, `eventType`, `siteId`), replacing multiple parameters and overloads. Added private utility methods to build HTTP query parameters for each API call. [[1]](diffhunk://#diff-c0a594cb6bf35693b60035c742263700865b55e180715509ac6b9377ddaeb95bL13-R19) [[2]](diffhunk://#diff-c0a594cb6bf35693b60035c742263700865b55e180715509ac6b9377ddaeb95bL92-R160) [[3]](diffhunk://#diff-c0a594cb6bf35693b60035c742263700865b55e180715509ac6b9377ddaeb95bR176-R220) * Updated the store feature (`withPageview`) to use the new parameter object format for all analytics queries, ensuring correct passing of `eventType` and `siteId`, and added type guards for array vs. object responses. [[1]](diffhunk://#diff-989f33d2983ed4588112a6b12fa172e230e6e75dcc308948fcc4bc7e73161d59L10-R10) [[2]](diffhunk://#diff-989f33d2983ed4588112a6b12fa172e230e6e75dcc308948fcc4bc7e73161d59L95-R105) [[3]](diffhunk://#diff-989f33d2983ed4588112a6b12fa172e230e6e75dcc308948fcc4bc7e73161d59L146-R158) [[4]](diffhunk://#diff-989f33d2983ed4588112a6b12fa172e230e6e75dcc308948fcc4bc7e73161d59L200-R218) [[5]](diffhunk://#diff-989f33d2983ed4588112a6b12fa172e230e6e75dcc308948fcc4bc7e73161d59L216-R231) [[6]](diffhunk://#diff-989f33d2983ed4588112a6b12fa172e230e6e75dcc308948fcc4bc7e73161d59L262-R290) [[7]](diffhunk://#diff-989f33d2983ed4588112a6b12fa172e230e6e75dcc308948fcc4bc7e73161d59L316-R354) **Testing improvements:** * Added comprehensive tests for `getTotalEvents` in `dot-analytics.service.spec.ts`, covering various combinations of query parameters and validating request construction. Introduced a helper function to match requests by URL and method. [[1]](diffhunk://#diff-99331c74e1a5318c227da2286205a09c838813f9e2e732e45c6734cdde4b1793R3-R21) [[2]](diffhunk://#diff-99331c74e1a5318c227da2286205a09c838813f9e2e732e45c6734cdde4b1793R136-R232) **Minor improvements:** * Fixed strict typing for test variables in the service spec file. [[1]](diffhunk://#diff-99331c74e1a5318c227da2286205a09c838813f9e2e732e45c6734cdde4b1793L53-R67) [[2]](diffhunk://#diff-99331c74e1a5318c227da2286205a09c838813f9e2e732e45c6734cdde4b1793L77-R91) This PR fixes: #34849
1 parent f93ac7e commit c6861a7

5 files changed

Lines changed: 736 additions & 254 deletions

File tree

‎core-web/libs/portlets/dot-analytics/data-access/src/lib/services/dot-analytics.service.spec.ts‎

Lines changed: 316 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,66 @@
11
import { createHttpFactory, HttpMethod, SpectatorHttp } from '@ngneat/spectator/jest';
22

3+
import { HttpTestingController } from '@angular/common/http/testing';
4+
import { TestBed } from '@angular/core/testing';
5+
36
import { DotAnalyticsService } from './dot-analytics.service';
47

58
import { CubeJSQuery, Granularity } from '../types';
69

710
const ANALYTICS_API_ENDPOINT = '/api/v1/analytics/content/_query/cube';
11+
const ANALYTICS_EVENT_TOTAL_EVENTS = '/api/v1/analytics/event/total-events';
12+
const ANALYTICS_EVENT_UNIQUE_VISITORS = '/api/v1/analytics/event/unique-visitors';
13+
const ANALYTICS_EVENT_TOP_CONTENT = '/api/v1/analytics/event/top-content';
14+
const ANALYTICS_EVENT_PAGE_VIEWS_BY_DEVICE_BROWSER =
15+
'/api/v1/analytics/event/pageviews-by-device-browser';
16+
17+
/** SpectatorHttp.expectOne always wraps URL in an object, so function matchers break; use the real backend matcher. */
18+
function expectTotalEventsReq(httpMock: HttpTestingController) {
19+
return httpMock.expectOne(
20+
(req) =>
21+
req.method === 'GET' &&
22+
(req.urlWithParams === ANALYTICS_EVENT_TOTAL_EVENTS ||
23+
req.urlWithParams.startsWith(`${ANALYTICS_EVENT_TOTAL_EVENTS}?`))
24+
);
25+
}
26+
27+
function expectUniqueVisitorsReq(httpMock: HttpTestingController) {
28+
return httpMock.expectOne(
29+
(req) =>
30+
req.method === 'GET' &&
31+
(req.urlWithParams === ANALYTICS_EVENT_UNIQUE_VISITORS ||
32+
req.urlWithParams.startsWith(`${ANALYTICS_EVENT_UNIQUE_VISITORS}?`))
33+
);
34+
}
35+
36+
function expectTopContentReq(httpMock: HttpTestingController) {
37+
return httpMock.expectOne(
38+
(req) =>
39+
req.method === 'GET' &&
40+
(req.urlWithParams === ANALYTICS_EVENT_TOP_CONTENT ||
41+
req.urlWithParams.startsWith(`${ANALYTICS_EVENT_TOP_CONTENT}?`))
42+
);
43+
}
44+
45+
function expectPageviewsByDeviceBrowserReq(httpMock: HttpTestingController) {
46+
return httpMock.expectOne(
47+
(req) =>
48+
req.method === 'GET' &&
49+
(req.urlWithParams === ANALYTICS_EVENT_PAGE_VIEWS_BY_DEVICE_BROWSER ||
50+
req.urlWithParams.startsWith(`${ANALYTICS_EVENT_PAGE_VIEWS_BY_DEVICE_BROWSER}?`))
51+
);
52+
}
53+
54+
function dotCMSWrap<T>(data: T) {
55+
return {
56+
entity: { data },
57+
errors: [],
58+
i18nMessagesMap: {},
59+
messages: [],
60+
pagination: null,
61+
permissions: []
62+
};
63+
}
864

965
describe('DotAnalyticsService', () => {
1066
let spectator: SpectatorHttp<DotAnalyticsService>;
@@ -50,7 +106,7 @@ describe('DotAnalyticsService', () => {
50106
permissions: []
51107
};
52108

53-
let result: unknown[];
109+
let result!: unknown[];
54110
spectator.service.cubeQuery(testQuery).subscribe((data) => {
55111
result = data;
56112
});
@@ -74,7 +130,7 @@ describe('DotAnalyticsService', () => {
74130
permissions: []
75131
};
76132

77-
let result: unknown[];
133+
let result!: unknown[];
78134
spectator.service.cubeQuery(testQuery).subscribe((data) => {
79135
result = data;
80136
});
@@ -119,6 +175,264 @@ describe('DotAnalyticsService', () => {
119175
});
120176
});
121177

178+
describe('getTotalEvents', () => {
179+
it('should GET total-events with range only and omit optional query keys', () => {
180+
spectator.service.getTotalEvents({ range: 'last_7_days' }).subscribe();
181+
182+
const req = expectTotalEventsReq(TestBed.inject(HttpTestingController));
183+
expect(req.request.params.get('range')).toBe('last_7_days');
184+
expect(req.request.params.get('granularity')).toBeNull();
185+
expect(req.request.params.get('eventType')).toBeNull();
186+
expect(req.request.params.get('siteId')).toBeNull();
187+
188+
req.flush({
189+
entity: { data: { totalEvents: 42 } },
190+
errors: [],
191+
i18nMessagesMap: {},
192+
messages: [],
193+
pagination: null,
194+
permissions: []
195+
});
196+
});
197+
198+
it('should GET total-events with from and to', () => {
199+
spectator.service.getTotalEvents({ from: '2026-01-01', to: '2026-01-31' }).subscribe();
200+
201+
const req = expectTotalEventsReq(TestBed.inject(HttpTestingController));
202+
expect(req.request.params.get('from')).toBe('2026-01-01');
203+
expect(req.request.params.get('to')).toBe('2026-01-31');
204+
205+
req.flush({
206+
entity: { data: { totalEvents: 10 } },
207+
errors: [],
208+
i18nMessagesMap: {},
209+
messages: [],
210+
pagination: null,
211+
permissions: []
212+
});
213+
});
214+
215+
it('should append eventType, siteId, and granularity when provided in options', () => {
216+
let result!: unknown;
217+
spectator.service
218+
.getTotalEvents({
219+
range: 'last_7_days',
220+
granularity: 'day',
221+
eventType: 'pageview',
222+
siteId: 'site-abc'
223+
})
224+
.subscribe((data) => {
225+
result = data;
226+
});
227+
228+
const req = expectTotalEventsReq(TestBed.inject(HttpTestingController));
229+
expect(req.request.params.get('range')).toBe('last_7_days');
230+
expect(req.request.params.get('granularity')).toBe('day');
231+
expect(req.request.params.get('eventType')).toBe('pageview');
232+
expect(req.request.params.get('siteId')).toBe('site-abc');
233+
234+
req.flush({
235+
entity: {
236+
data: [
237+
{ day: '2026-05-01', totalEvents: 3 },
238+
{ day: '2026-05-02', totalEvents: 5 }
239+
]
240+
},
241+
errors: [],
242+
i18nMessagesMap: {},
243+
messages: [],
244+
pagination: null,
245+
permissions: []
246+
});
247+
248+
expect(result).toEqual([
249+
{ day: '2026-05-01', totalEvents: 3 },
250+
{ day: '2026-05-02', totalEvents: 5 }
251+
]);
252+
});
253+
254+
it('should append impression eventType without granularity', () => {
255+
spectator.service
256+
.getTotalEvents({ range: 'last_30_days', eventType: 'impressions' })
257+
.subscribe();
258+
259+
const req = expectTotalEventsReq(TestBed.inject(HttpTestingController));
260+
expect(req.request.params.get('range')).toBe('last_30_days');
261+
expect(req.request.params.get('eventType')).toBe('impressions');
262+
expect(req.request.params.get('granularity')).toBeNull();
263+
264+
req.flush(dotCMSWrap({ totalEvents: 99 }));
265+
});
266+
267+
it('should propagate HTTP errors for total-events', (done) => {
268+
spectator.service.getTotalEvents({ range: 'last_7_days' }).subscribe({
269+
error: (e) => {
270+
expect(e.status).toBe(500);
271+
done();
272+
}
273+
});
274+
275+
const req = expectTotalEventsReq(TestBed.inject(HttpTestingController));
276+
req.flush('Server error', { status: 500, statusText: 'Internal Server Error' });
277+
});
278+
});
279+
280+
describe('getUniqueVisitors', () => {
281+
it('should GET unique-visitors with range only and omit optional query keys', () => {
282+
spectator.service.getUniqueVisitors({ range: 'last_7_days' }).subscribe();
283+
284+
const req = expectUniqueVisitorsReq(TestBed.inject(HttpTestingController));
285+
expect(req.request.params.get('range')).toBe('last_7_days');
286+
expect(req.request.params.get('granularity')).toBeNull();
287+
expect(req.request.params.get('siteId')).toBeNull();
288+
289+
req.flush(dotCMSWrap({ uniqueVisitors: 100 }));
290+
});
291+
292+
it('should GET unique-visitors with from, to, granularity, and siteId', () => {
293+
let result!: unknown;
294+
spectator.service
295+
.getUniqueVisitors({
296+
from: '2026-01-01',
297+
to: '2026-01-31',
298+
granularity: 'day',
299+
siteId: 'site-x'
300+
})
301+
.subscribe((data) => {
302+
result = data;
303+
});
304+
305+
const req = expectUniqueVisitorsReq(TestBed.inject(HttpTestingController));
306+
expect(req.request.params.get('from')).toBe('2026-01-01');
307+
expect(req.request.params.get('to')).toBe('2026-01-31');
308+
expect(req.request.params.get('granularity')).toBe('day');
309+
expect(req.request.params.get('siteId')).toBe('site-x');
310+
311+
req.flush(
312+
dotCMSWrap([
313+
{ day: '2026-01-01', uniqueVisitors: 1 },
314+
{ day: '2026-01-02', uniqueVisitors: 2 }
315+
])
316+
);
317+
318+
expect(result).toEqual([
319+
{ day: '2026-01-01', uniqueVisitors: 1 },
320+
{ day: '2026-01-02', uniqueVisitors: 2 }
321+
]);
322+
});
323+
324+
it('should propagate HTTP errors for unique-visitors', (done) => {
325+
spectator.service.getUniqueVisitors({ range: 'last_7_days' }).subscribe({
326+
error: (e) => {
327+
expect(e.status).toBe(500);
328+
done();
329+
}
330+
});
331+
332+
const req = expectUniqueVisitorsReq(TestBed.inject(HttpTestingController));
333+
req.flush('Server error', { status: 500, statusText: 'Internal Server Error' });
334+
});
335+
});
336+
337+
describe('getTopContent', () => {
338+
it('should GET top-content with range and optional filters', () => {
339+
let result!: unknown;
340+
spectator.service
341+
.getTopContent({
342+
range: 'last_7_days',
343+
eventType: 'pageview',
344+
siteId: 's1'
345+
})
346+
.subscribe((data) => {
347+
result = data;
348+
});
349+
350+
const req = expectTopContentReq(TestBed.inject(HttpTestingController));
351+
expect(req.request.params.get('range')).toBe('last_7_days');
352+
expect(req.request.params.get('eventType')).toBe('pageview');
353+
expect(req.request.params.get('siteId')).toBe('s1');
354+
355+
req.flush(
356+
dotCMSWrap([
357+
{ identifier: '1', title: 'A', totalEvents: 5 },
358+
{ identifier: '2', title: 'B', totalEvents: 3 }
359+
])
360+
);
361+
362+
expect(result).toEqual([
363+
{ identifier: '1', title: 'A', totalEvents: 5 },
364+
{ identifier: '2', title: 'B', totalEvents: 3 }
365+
]);
366+
});
367+
368+
it('should GET top-content with from and to omitting optional keys', () => {
369+
spectator.service.getTopContent({ from: '2026-05-01', to: '2026-05-07' }).subscribe();
370+
371+
const req = expectTopContentReq(TestBed.inject(HttpTestingController));
372+
expect(req.request.params.get('from')).toBe('2026-05-01');
373+
expect(req.request.params.get('to')).toBe('2026-05-07');
374+
expect(req.request.params.get('eventType')).toBeNull();
375+
376+
req.flush(dotCMSWrap([]));
377+
});
378+
379+
it('should propagate HTTP errors for top-content', (done) => {
380+
spectator.service.getTopContent({ range: 'last_7_days' }).subscribe({
381+
error: (e) => {
382+
expect(e.status).toBe(500);
383+
done();
384+
}
385+
});
386+
387+
const req = expectTopContentReq(TestBed.inject(HttpTestingController));
388+
req.flush('Server error', { status: 500, statusText: 'Internal Server Error' });
389+
});
390+
});
391+
392+
describe('getPageviewsByDeviceBrowser', () => {
393+
it('should GET pageviews-by-device-browser with params', () => {
394+
let result!: unknown;
395+
spectator.service
396+
.getPageviewsByDeviceBrowser({
397+
range: 'last_30_days',
398+
eventType: 'pageview',
399+
siteId: 'host1'
400+
})
401+
.subscribe((data) => {
402+
result = data;
403+
});
404+
405+
const req = expectPageviewsByDeviceBrowserReq(TestBed.inject(HttpTestingController));
406+
expect(req.request.params.get('range')).toBe('last_30_days');
407+
expect(req.request.params.get('eventType')).toBe('pageview');
408+
expect(req.request.params.get('siteId')).toBe('host1');
409+
410+
req.flush(
411+
dotCMSWrap([
412+
{ browser: 'Chrome', device: 'desktop', total: 10 },
413+
{ browser: 'Safari', device: 'mobile', total: 4 }
414+
])
415+
);
416+
417+
expect(result).toEqual([
418+
{ browser: 'Chrome', device: 'desktop', total: 10 },
419+
{ browser: 'Safari', device: 'mobile', total: 4 }
420+
]);
421+
});
422+
423+
it('should propagate HTTP errors for pageviews-by-device-browser', (done) => {
424+
spectator.service.getPageviewsByDeviceBrowser({ range: 'last_30_days' }).subscribe({
425+
error: (e) => {
426+
expect(e.status).toBe(500);
427+
done();
428+
}
429+
});
430+
431+
const req = expectPageviewsByDeviceBrowserReq(TestBed.inject(HttpTestingController));
432+
req.flush('Server error', { status: 500, statusText: 'Internal Server Error' });
433+
});
434+
});
435+
122436
describe('Service Integration', () => {
123437
it('should be injectable and create instance', () => {
124438
expect(spectator.service).toBeTruthy();

0 commit comments

Comments
 (0)