Skip to content

Commit cae4686

Browse files
wesmclaude
andcommitted
fix: address review #6388 findings
- Refactor projectsParams into filterParams({ includeProject: false }) to eliminate duplicated date/timezone construction logic - Add 7 tests for setProject behavior: toggle on/off, project param included in filtered panels but excluded from fetchProjects, and correct behavior with selectedDate active Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 6e35874 commit cae4686

2 files changed

Lines changed: 98 additions & 24 deletions

File tree

frontend/src/lib/stores/analytics.svelte.ts

Lines changed: 18 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -107,28 +107,39 @@ class AnalyticsStore {
107107
topSessions: 0,
108108
};
109109

110-
private baseParams(): AnalyticsParams {
110+
private baseParams(
111+
opts: { includeProject?: boolean } = {},
112+
): AnalyticsParams {
113+
const includeProject = opts.includeProject ?? true;
111114
const p: AnalyticsParams = {
112115
from: this.from,
113116
to: this.to,
114117
timezone: Intl.DateTimeFormat().resolvedOptions().timeZone,
115118
};
116-
if (this.project) p.project = this.project;
119+
if (includeProject && this.project) {
120+
p.project = this.project;
121+
}
117122
return p;
118123
}
119124

120125
// Returns params narrowed to selectedDate when one is active.
121-
// Used by summary, activity, and projects — but not heatmap.
122-
private filterParams(): AnalyticsParams {
126+
private filterParams(
127+
opts: { includeProject?: boolean } = {},
128+
): AnalyticsParams {
129+
const includeProject = opts.includeProject ?? true;
123130
if (this.selectedDate) {
124-
return {
131+
const p: AnalyticsParams = {
125132
from: this.selectedDate,
126133
to: this.selectedDate,
127134
timezone:
128135
Intl.DateTimeFormat().resolvedOptions().timeZone,
129136
};
137+
if (includeProject && this.project) {
138+
p.project = this.project;
139+
}
140+
return p;
130141
}
131-
return this.baseParams();
142+
return this.baseParams({ includeProject });
132143
}
133144

134145
async fetchAll() {
@@ -217,30 +228,13 @@ class AnalyticsStore {
217228
// Projects chart always shows all projects (no project
218229
// filter) so the selected project can be highlighted in
219230
// context rather than shown in isolation.
220-
private projectsParams(): AnalyticsParams {
221-
if (this.selectedDate) {
222-
return {
223-
from: this.selectedDate,
224-
to: this.selectedDate,
225-
timezone:
226-
Intl.DateTimeFormat().resolvedOptions().timeZone,
227-
};
228-
}
229-
return {
230-
from: this.from,
231-
to: this.to,
232-
timezone:
233-
Intl.DateTimeFormat().resolvedOptions().timeZone,
234-
};
235-
}
236-
237231
async fetchProjects() {
238232
const v = ++this.versions.projects;
239233
this.loading.projects = true;
240234
this.errors.projects = null;
241235
try {
242236
const data = await getAnalyticsProjects(
243-
this.projectsParams(),
237+
this.filterParams({ includeProject: false }),
244238
);
245239
if (this.versions.projects === v) {
246240
this.projects = data;

frontend/src/lib/stores/analytics.test.ts

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type {
1616
SessionShapeResponse,
1717
VelocityResponse,
1818
ToolsAnalyticsResponse,
19+
TopSessionsResponse,
1920
} from "../api/types.js";
2021

2122
vi.mock("../api/client.js", () => ({
@@ -27,6 +28,7 @@ vi.mock("../api/client.js", () => ({
2728
getAnalyticsSessionShape: vi.fn(),
2829
getAnalyticsVelocity: vi.fn(),
2930
getAnalyticsTools: vi.fn(),
31+
getAnalyticsTopSessions: vi.fn(),
3032
}));
3133

3234

@@ -97,6 +99,10 @@ function makeTools(): ToolsAnalyticsResponse {
9799
};
98100
}
99101

102+
function makeTopSessions(): TopSessionsResponse {
103+
return { metric: "messages", sessions: [] };
104+
}
105+
100106
function mockAllAPIs() {
101107
vi.mocked(api.getAnalyticsSummary).mockResolvedValue(
102108
makeSummary(),
@@ -122,10 +128,14 @@ function mockAllAPIs() {
122128
vi.mocked(api.getAnalyticsTools).mockResolvedValue(
123129
makeTools(),
124130
);
131+
vi.mocked(api.getAnalyticsTopSessions).mockResolvedValue(
132+
makeTopSessions(),
133+
);
125134
}
126135

127136
function resetStore() {
128137
analytics.selectedDate = null;
138+
analytics.project = "";
129139
analytics.from = "2024-01-01";
130140
analytics.to = "2024-01-31";
131141
}
@@ -301,3 +311,73 @@ describe("AnalyticsStore heatmap uses full range", () => {
301311
);
302312
});
303313
});
314+
315+
describe("AnalyticsStore.setProject", () => {
316+
beforeEach(() => {
317+
resetStore();
318+
vi.clearAllMocks();
319+
mockAllAPIs();
320+
});
321+
322+
it("should toggle project on first click", () => {
323+
analytics.setProject("alpha");
324+
expect(analytics.project).toBe("alpha");
325+
});
326+
327+
it("should clear project when clicking same project", () => {
328+
analytics.setProject("alpha");
329+
analytics.setProject("alpha");
330+
expect(analytics.project).toBe("");
331+
});
332+
333+
it("should switch to different project", () => {
334+
analytics.setProject("alpha");
335+
analytics.setProject("beta");
336+
expect(analytics.project).toBe("beta");
337+
});
338+
339+
it("should include project in filtered panel params", () => {
340+
analytics.setProject("alpha");
341+
342+
const summaryParams =
343+
vi.mocked(api.getAnalyticsSummary).mock.lastCall?.[0];
344+
expect(summaryParams?.project).toBe("alpha");
345+
346+
const toolsParams =
347+
vi.mocked(api.getAnalyticsTools).mock.lastCall?.[0];
348+
expect(toolsParams?.project).toBe("alpha");
349+
});
350+
351+
it("should exclude project from fetchProjects params", () => {
352+
analytics.setProject("alpha");
353+
354+
const projectsParams =
355+
vi.mocked(api.getAnalyticsProjects).mock.lastCall?.[0];
356+
expect(projectsParams?.project).toBeUndefined();
357+
});
358+
359+
it("should exclude project from fetchProjects even with selectedDate", () => {
360+
analytics.selectDate("2024-01-15");
361+
vi.clearAllMocks();
362+
mockAllAPIs();
363+
364+
analytics.setProject("alpha");
365+
366+
const projectsParams =
367+
vi.mocked(api.getAnalyticsProjects).mock.lastCall?.[0];
368+
expect(projectsParams?.project).toBeUndefined();
369+
expect(projectsParams?.from).toBe("2024-01-15");
370+
});
371+
372+
it("should clear project param from panels after deselecting", () => {
373+
analytics.setProject("alpha");
374+
vi.clearAllMocks();
375+
mockAllAPIs();
376+
377+
analytics.setProject("alpha"); // deselect
378+
379+
const summaryParams =
380+
vi.mocked(api.getAnalyticsSummary).mock.lastCall?.[0];
381+
expect(summaryParams?.project).toBeUndefined();
382+
});
383+
});

0 commit comments

Comments
 (0)