Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 6 additions & 42 deletions app/src/components/card-container.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,14 @@
"use client";

import { useWidgetQuery } from "@/hooks/use-widget-query";
import { useClickAction } from "@/hooks/use-click-action";
import { resolveCacheOptions } from "@/lib/resolve-cache-options";
import { getChartConfig } from "@/lib/chart-registry";
import type { ColumnMapping } from "@/lib/chart-registry";
import type {
DashboardWidget,
ClickAction,
StylingConfig,
} from "@/lib/db/schema";
import type { DashboardWidget, StylingConfig } from "@/lib/db/schema";
import type { ParameterSourceMap } from "@/lib/collect-parameter-names";
import type { ColorScaleConfig } from "@neoboard/components";
import {
useParameterStore,
useParameterValues,
} from "@/stores/parameter-store";
import {
resolveClickActions,
deriveClickableColumns,
} from "@/lib/resolve-click-action";
import { useParameterValues } from "@/stores/parameter-store";
import { scrollAndHighlight } from "@/lib/scroll-to-widget";
import { applyTransforms } from "@/lib/data-transforms";
import type { Transform } from "@/lib/data-transforms";
Expand Down Expand Up @@ -165,37 +155,11 @@ export function CardContainer({
parameterSourceMap,
}: CardContainerProps) {
const chartConfig = getChartConfig(widget.chartType);

const setParameter = useParameterStore((s) => s.setParameter);
const handleChartClick = useCallback(
(point: Record<string, unknown>) => {
const result = resolveClickActions(widget, point);
if (!result) return;

if (result.setParameter) {
const { parameterName, value, label, sourceField } =
result.setParameter;
setParameter(
parameterName,
value,
label,
sourceField,
"text",
"click-action",
widget.id,
);
}

if (result.navigateToPageId) {
onNavigateToPage?.(result.navigateToPageId);
}
},
[widget, setParameter, onNavigateToPage],
const { handleChartClick, hasClickAction, clickableColumns } = useClickAction(
widget,
onNavigateToPage,
);
const ws = widget.settings ?? {};
const clickAction = ws.clickAction as ClickAction | undefined;
const hasClickAction = !!clickAction;
const clickableColumns = deriveClickableColumns(clickAction);

// Cache settings from widget config. Default: cache enabled, 5-min TTL.
const enableCache = ws.enableCache !== false;
Expand Down
282 changes: 282 additions & 0 deletions app/src/hooks/__tests__/use-click-action.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,282 @@
// @vitest-environment jsdom
/**
* Tests for useClickAction hook logic.
*
* Tests the underlying functions (resolveClickActions, deriveClickableColumns)
* directly, and also exercises the hook itself via renderHook to cover the
* useCallback / useParameterStore wiring inside use-click-action.ts.
*/
import { describe, it, expect, beforeEach, vi } from "vitest";
import { renderHook, act } from "@testing-library/react";
import { useParameterStore } from "@/stores/parameter-store";
import type { ClickAction, DashboardWidget } from "@/lib/db/schema";
import {
resolveClickActions,
deriveClickableColumns,
} from "@/lib/resolve-click-action";

import { useClickAction } from "@/hooks/use-click-action";

function resetStore() {
useParameterStore.getState().clearAll();
}

// ---------------------------------------------------------------------------
// Module exports
// ---------------------------------------------------------------------------
describe("useClickAction — module exports", () => {
it("exports useClickAction as a function", () => {
expect(typeof useClickAction).toBe("function");
});
});

// ---------------------------------------------------------------------------
// hasClickAction derivation
// ---------------------------------------------------------------------------
describe("useClickAction — hasClickAction derivation", () => {
it("is true when clickAction exists in settings", () => {
const ws = {
clickAction: {
type: "set-parameter",
parameterMapping: { parameterName: "x", sourceField: "y" },
},
};
const clickAction = ws.clickAction as ClickAction | undefined;
expect(!!clickAction).toBe(true);
});

it("is false when clickAction is undefined", () => {
const ws: Record<string, unknown> = {};
const clickAction = ws.clickAction as ClickAction | undefined;
expect(!!clickAction).toBe(false);
});
});

// ---------------------------------------------------------------------------
// resolveClickActions integration (pure function)
// ---------------------------------------------------------------------------
describe("useClickAction — resolveClickActions integration", () => {
beforeEach(resetStore);

it("resolveClickActions returns setParameter with parameterName and value", () => {
const widget = {
id: "w1",
settings: {
clickAction: {
type: "set-parameter",
parameterMapping: { parameterName: "region", sourceField: "name" },
},
},
} as unknown as DashboardWidget;

const result = resolveClickActions(widget, { name: "US", value: 100 });
if (result?.setParameter) {
expect(result.setParameter.parameterName).toBe("region");
expect(result.setParameter.value).toBe("US");
}
});
Comment on lines +61 to +77

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid conditional assertions that can silently pass.

At Line 73, the if (result?.setParameter) guard can make the test pass even when resolveClickActions returns null or misses setParameter. Assert the shape directly so failures are surfaced.

Suggested fix
   const result = resolveClickActions(widget, { name: "US", value: 100 });
-  if (result?.setParameter) {
-    expect(result.setParameter.parameterName).toBe("region");
-    expect(result.setParameter.value).toBe("US");
-  }
+  expect(result).toEqual(
+    expect.objectContaining({
+      setParameter: expect.objectContaining({
+        parameterName: "region",
+        value: "US",
+      }),
+    }),
+  );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/src/hooks/__tests__/use-click-action.test.ts` around lines 61 - 77, The
test for resolveClickActions uses a conditional guard (if
(result?.setParameter)) which can hide failures; update the test to assert the
result shape directly by removing the if and adding explicit expectations such
as expecting result and result.setParameter to be defined (or use
toHaveProperty), then assert result.setParameter.parameterName and
result.setParameter.value equal the expected values so missing or null returns
fail the test; target the resolveClickActions call and the result/setParameter
properties in your assertions.


it("resolveClickActions returns null when no click action configured", () => {
const widget = { id: "w2", settings: {} } as unknown as DashboardWidget;
const result = resolveClickActions(widget, { name: "US" });
expect(result).toBeNull();
});
});

// ---------------------------------------------------------------------------
// deriveClickableColumns (pure function)
// ---------------------------------------------------------------------------
describe("useClickAction — deriveClickableColumns", () => {
it("returns column list from click action config", () => {
const clickAction: ClickAction = {
type: "set-parameter",
parameterMapping: { parameterName: "id", sourceField: "id" },
clickableColumns: ["id", "name"],
};
const cols = deriveClickableColumns(clickAction);
expect(cols).toEqual(["id", "name"]);
});

it("returns undefined when no click action", () => {
expect(deriveClickableColumns(undefined)).toBeUndefined();
});

it("returns undefined when clickableColumns not specified", () => {
const clickAction: ClickAction = {
type: "set-parameter",
parameterMapping: { parameterName: "id", sourceField: "name" },
};
const cols = deriveClickableColumns(clickAction);
expect(cols).toBeUndefined();
});
});

// ---------------------------------------------------------------------------
// parameter store wiring (pure function)
// ---------------------------------------------------------------------------
describe("useClickAction — parameter store wiring", () => {
beforeEach(resetStore);

it("setParameter stores click-action source correctly", () => {
const { setParameter } = useParameterStore.getState();
setParameter("region", "US", "US", "name", "text", "click-action", "w1");

const entry = useParameterStore.getState().parameters["region"];
expect(entry.value).toBe("US");
expect(entry.sourceType).toBe("click-action");
expect(entry.sourceWidgetId).toBe("w1");
});
});

// ---------------------------------------------------------------------------
// renderHook tests — exercises the actual hook body
// ---------------------------------------------------------------------------
describe("useClickAction — renderHook", () => {
beforeEach(resetStore);

it("returns handleChartClick, hasClickAction, and clickableColumns", () => {
const widget = {
id: "w1",
chartType: "bar",
settings: {},
} as unknown as DashboardWidget;

const { result } = renderHook(() => useClickAction(widget));

expect(typeof result.current.handleChartClick).toBe("function");
expect(result.current.hasClickAction).toBe(false);
expect(result.current.clickableColumns).toBeUndefined();
});

it("hasClickAction is true when widget has clickAction configured", () => {
const widget = {
id: "w2",
chartType: "bar",
settings: {
clickAction: {
type: "set-parameter",
parameterMapping: { parameterName: "region", sourceField: "name" },
},
},
} as unknown as DashboardWidget;

const { result } = renderHook(() => useClickAction(widget));

expect(result.current.hasClickAction).toBe(true);
});

it("clickableColumns derives from clickAction config", () => {
const widget = {
id: "w3",
chartType: "table",
settings: {
clickAction: {
type: "set-parameter",
parameterMapping: { parameterName: "id", sourceField: "id" },
clickableColumns: ["id", "name"],
},
},
} as unknown as DashboardWidget;

const { result } = renderHook(() => useClickAction(widget));

expect(result.current.clickableColumns).toEqual(["id", "name"]);
});

it("handleChartClick sets parameter in store via click action", () => {
const widget = {
id: "w4",
chartType: "bar",
settings: {
title: "Revenue Chart",
clickAction: {
type: "set-parameter",
parameterMapping: { parameterName: "region", sourceField: "name" },
},
},
} as unknown as DashboardWidget;

const { result } = renderHook(() => useClickAction(widget));

act(() => {
result.current.handleChartClick({ name: "US", value: 100 });
});

const entry = useParameterStore.getState().parameters["region"];
expect(entry).toBeDefined();
expect(entry.value).toBe("US");
expect(entry.sourceType).toBe("click-action");
expect(entry.sourceWidgetId).toBe("w4");
});

it("handleChartClick does nothing when no click action configured", () => {
const widget = {
id: "w5",
chartType: "bar",
settings: {},
} as unknown as DashboardWidget;

const { result } = renderHook(() => useClickAction(widget));

act(() => {
result.current.handleChartClick({ name: "US" });
});

// Store should still be empty
const params = useParameterStore.getState().parameters;
expect(Object.keys(params)).toHaveLength(0);
});

it("handleChartClick calls onNavigateToPage when navigate action configured", () => {
const widget = {
id: "w6",
chartType: "bar",
settings: {
clickAction: {
type: "navigate-to-page",
targetPageId: "page-42",
},
},
} as unknown as DashboardWidget;

const onNavigateToPage = vi.fn();
const { result } = renderHook(() =>
useClickAction(widget, onNavigateToPage),
);

act(() => {
result.current.handleChartClick({ name: "US" });
});

expect(onNavigateToPage).toHaveBeenCalledWith("page-42");
});

it("handleChartClick sets parameter and navigates for combined action", () => {
const widget = {
id: "w7",
chartType: "bar",
settings: {
title: "Sales",
clickAction: {
type: "set-parameter-and-navigate",
parameterMapping: { parameterName: "city", sourceField: "name" },
targetPageId: "detail-page",
},
},
} as unknown as DashboardWidget;

const onNavigateToPage = vi.fn();
const { result } = renderHook(() =>
useClickAction(widget, onNavigateToPage),
);

act(() => {
result.current.handleChartClick({ name: "Berlin", value: 42 });
});

const entry = useParameterStore.getState().parameters["city"];
expect(entry).toBeDefined();
expect(entry.value).toBe("Berlin");
expect(onNavigateToPage).toHaveBeenCalledWith("detail-page");
});
});
Loading
Loading