Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
49 changes: 31 additions & 18 deletions app/e2e/global-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,28 +249,41 @@ export default async function globalSetup() {
process.env.TEST_NEO4J_BOLT_URL = `bolt://localhost:${neo4jBoltPort}`;
process.env.TEST_PG_PORT = String(pgPort);

// ── Start the Next.js server on a dynamically allocated port ────────────
// Env vars are passed directly to the process — .env.local is never touched.
// ── Build & start the Next.js server on a dynamically allocated port ───
// Always use a production build + `next start` for consistent, fast E2E
// runs. `next dev` recompiles pages on demand which adds 10+ minutes of
// webpack overhead locally. A one-time `next build` (~2 min) then instant
// `next start` is what CI already does and is dramatically faster overall.
const appDir = path.resolve(__dirname, "..");
const serverCmd = process.env.CI ? "start" : "dev";
console.log(
`⏳ Starting Next.js ${serverCmd} server on port ${serverPort}...`,
);
const args = ["next", serverCmd, "--port", String(serverPort)];
// Use webpack explicitly — Turbopack (Next.js 16 default) doesn't correctly
// resolve CJS/ESM interop for the @neoboard/connection package at runtime.
if (serverCmd === "dev") args.push("--webpack");
const serverEnv = {
...process.env,
DATABASE_URL: databaseUrl,
ENCRYPTION_KEY: TEST_ENCRYPTION_KEY,
API_KEY_HMAC_SECRET: TEST_API_KEY_HMAC_SECRET,
NEXTAUTH_SECRET: TEST_NEXTAUTH_SECRET,
NEXTAUTH_URL: `http://localhost:${serverPort}`,
};

// Build once (skipped if .next/BUILD_ID already exists and E2E_SKIP_BUILD is set)
if (!process.env.E2E_SKIP_BUILD) {
console.log("⏳ Building Next.js (production)...");
const { execSync } = await import("node:child_process");
execSync("npx next build", {
cwd: appDir,
stdio: "inherit",
env: serverEnv,
});
console.log("✅ Next.js build complete");
} else {
console.log("⏡ Skipping build (E2E_SKIP_BUILD set)");
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment on lines +272 to +297

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 | 🟡 Minor

E2E_SKIP_BUILD=0 is truthy and will still skip the build.

process.env.E2E_SKIP_BUILD is a string, so E2E_SKIP_BUILD=0 / false / "" (unset via export then overridden) all evaluate truthy (except ""). A contributor who flips this to 0 expecting "off" will unintentionally skip the rebuild and quietly run stale bundles in E2E. Consider parsing it explicitly.

♻️ Proposed tweak
-  if (process.env.E2E_SKIP_BUILD && !hasCachedBuild) {
+  const skipBuild = /^(1|true|yes)$/i.test(process.env.E2E_SKIP_BUILD ?? "");
+
+  if (skipBuild && !hasCachedBuild) {
     throw new Error(
       "E2E_SKIP_BUILD is set but no prior build found at .next/BUILD_ID. " +
         "Run `npx next build` once or unset E2E_SKIP_BUILD.",
     );
   }
 
-  if (process.env.E2E_SKIP_BUILD) {
+  if (skipBuild) {
     console.log(
       "⏩ Skipping build (E2E_SKIP_BUILD set, reusing existing .next)",
     );
   } else {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (process.env.E2E_SKIP_BUILD && !hasCachedBuild) {
throw new Error(
"E2E_SKIP_BUILD is set but no prior build found at .next/BUILD_ID. " +
"Run `npx next build` once or unset E2E_SKIP_BUILD.",
);
}
if (process.env.E2E_SKIP_BUILD) {
console.log(
"⏩ Skipping build (E2E_SKIP_BUILD set, reusing existing .next)",
);
} else {
if (hasCachedBuild) {
console.log(
"⏳ Rebuilding Next.js (production)... (set E2E_SKIP_BUILD=1 to reuse previous build)",
);
} else {
console.log("⏳ Building Next.js (production)...");
}
execSync("npx next build", {
cwd: appDir,
stdio: "inherit",
env: serverEnv,
});
console.log("✅ Next.js build complete");
}
const skipBuild = /^(1|true|yes)$/i.test(process.env.E2E_SKIP_BUILD ?? "");
if (skipBuild && !hasCachedBuild) {
throw new Error(
"E2E_SKIP_BUILD is set but no prior build found at .next/BUILD_ID. " +
"Run `npx next build` once or unset E2E_SKIP_BUILD.",
);
}
if (skipBuild) {
console.log(
"⏩ Skipping build (E2E_SKIP_BUILD set, reusing existing .next)",
);
} else {
if (hasCachedBuild) {
console.log(
"⏳ Rebuilding Next.js (production)... (set E2E_SKIP_BUILD=1 to reuse previous build)",
);
} else {
console.log("⏳ Building Next.js (production)...");
}
execSync("npx next build", {
cwd: appDir,
stdio: "inherit",
env: serverEnv,
});
console.log("✅ Next.js build complete");
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/e2e/global-setup.ts` around lines 272 - 297, The code currently treats
process.env.E2E_SKIP_BUILD as a boolean which makes values like "0" or "false"
still count as truthy and skip the build; update the checks around
process.env.E2E_SKIP_BUILD (the block that decides whether to skip or run
execSync("npx next build", { cwd: appDir, stdio: "inherit", env: serverEnv }))
to explicitly parse/normalize the env var (e.g. treat "0", "false", "no" as
falsy and "1", "true", "yes" as truthy) before branching; use the normalized
boolean in both the initial hasCachedBuild guard and the later if/else that logs
skipping vs building so a user setting E2E_SKIP_BUILD=0 will not skip the
rebuild.


console.log(`⏳ Starting Next.js production server on port ${serverPort}...`);
const args = ["next", "start", "--port", String(serverPort)];
const server = spawn("npx", args, {
cwd: appDir,
stdio: "pipe",
env: {
...process.env,
DATABASE_URL: databaseUrl,
ENCRYPTION_KEY: TEST_ENCRYPTION_KEY,
API_KEY_HMAC_SECRET: TEST_API_KEY_HMAC_SECRET,
NEXTAUTH_SECRET: TEST_NEXTAUTH_SECRET,
NEXTAUTH_URL: `http://localhost:${serverPort}`,
},
env: serverEnv,
detached: true,
});
server.unref();
Expand Down
1 change: 0 additions & 1 deletion app/src/plugins/gauge/component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ function GaugePluginComponent({
min={settings.min}
max={settings.max}
showProgress={settings.showProgress}
showPointer={settings.showPointer}
showDetail={settings.showDetail}
startAngle={settings.startAngle}
endAngle={settings.endAngle}
Expand Down
1 change: 0 additions & 1 deletion app/src/plugins/gauge/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ export const gaugeSettingsSchema = z
min: z.coerce.number().default(0),
max: z.coerce.number().default(100),
showProgress: z.boolean().default(true),
showPointer: z.boolean().default(true),
showDetail: z.boolean().default(true),
startAngle: z.coerce.number().default(225),
endAngle: z.coerce.number().default(-45),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ describe("gaugeSettingsSchema", () => {
expect(result.min).toBe(0);
expect(result.max).toBe(100);
expect(result.showProgress).toBe(true);
expect(result.showPointer).toBe(true);
expect(result.showDetail).toBe(true);
expect(result.startAngle).toBe(225);
expect(result.endAngle).toBe(-45);
Expand Down
1 change: 1 addition & 0 deletions app/src/plugins/sunburst/component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ function SunburstPluginComponent({
<SunburstChart
data={(data as SunburstDataItem[]) ?? []}
showLabels={settings.showLabels}
maxLabelDepth={settings.maxLabelDepth}
sort={settings.sort}
highlightOnHover={settings.highlightOnHover}
colorPalette={settings.colorPalette}
Expand Down
1 change: 1 addition & 0 deletions app/src/plugins/sunburst/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { z } from "zod";
export const sunburstSettingsSchema = z
.object({
showLabels: z.boolean().default(true),
maxLabelDepth: z.coerce.number().default(2),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
sort: z.enum(["desc", "asc", "none"]).default("desc"),
highlightOnHover: z.boolean().default(true),
colorPalette: z.string().optional(),
Expand Down
48 changes: 26 additions & 22 deletions component/src/charts/__tests__/gauge-chart.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,42 +66,45 @@ describe("GaugeChart", () => {
expect(optionsCall.series[0].max).toBe(200);
});

// --- axisTick distance bug fix ---
it("sets axisTick.distance to -20 in non-compact mode", () => {
// --- minimal design: no ticks, no labels ---
it("hides axisTick in minimal design", () => {
render(<GaugeChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
const series = optionsCall.series[0];
expect(series.axisTick.show).toBe(true);
expect(series.axisTick.distance).toBe(-20);
expect(optionsCall.series[0].axisTick.show).toBe(false);
});

it("sets splitLine.distance to -20 in non-compact mode", () => {
it("hides splitLine in minimal design", () => {
render(<GaugeChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
const series = optionsCall.series[0];
expect(series.splitLine.show).toBe(true);
expect(series.splitLine.distance).toBe(-20);
expect(optionsCall.series[0].splitLine.show).toBe(false);
});

it("sets axisLabel.distance to 30 in non-compact mode", () => {
it("hides axisLabel in minimal design", () => {
render(<GaugeChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].axisLabel.show).toBe(false);
});

it("shows progress arc with roundCap by default", () => {
render(<GaugeChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
const series = optionsCall.series[0];
expect(series.axisLabel.show).toBe(true);
expect(series.axisLabel.distance).toBe(30);
expect(series.progress.show).toBe(true);
expect(series.progress.roundCap).toBe(true);
});

it("sets axisLabel.fontSize to 11", () => {
it("hides pointer and anchor", () => {
render(<GaugeChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].axisLabel.fontSize).toBe(11);
const series = optionsCall.series[0];
expect(series.pointer.show).toBe(false);
expect(series.anchor.show).toBe(false);
});

// --- axisTick splitNumber ---
it("sets axisTick.splitNumber to 2 to reduce number of minor ticks", () => {
it("uses roundCap on axisLine track", () => {
render(<GaugeChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
expect(optionsCall.series[0].axisTick.splitNumber).toBe(2);
expect(optionsCall.series[0].axisLine.roundCap).toBe(true);
});

it("shows loading state", () => {
Expand Down Expand Up @@ -177,17 +180,18 @@ describe("GaugeChart", () => {

// --- compact mode ---

it("hides axisTick, splitLine, and axisLabel in compact mode (container < 200px)", () => {
it("uses smaller arc width and font in compact mode (container < 200px)", () => {
mockSize.width = 150;
mockSize.height = 150;
render(<GaugeChart data={sampleData} />);
const optionsCall = mockSetOption.mock.calls[0][0];
const series = optionsCall.series[0];
expect(series.axisTick.show).toBe(false);
expect(series.splitLine.show).toBe(false);
expect(series.axisLabel.show).toBe(false);
// Detail stays visible in compact mode (smaller font), title hides
// Thinner arc in compact
expect(series.axisLine.lineStyle.width).toBe(10);
expect(series.progress.width).toBe(10);
// Smaller detail font, title hidden
expect(series.detail.show).toBe(true);
expect(series.detail.fontSize).toBe(18);
expect(series.title.show).toBe(false);
});
});
107 changes: 53 additions & 54 deletions component/src/charts/gauge-chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@
max?: number;
/** Show progress arc filling */
showProgress?: boolean;
/** Show the needle pointer */
showPointer?: boolean;
/** Show the numeric value and name detail */
showDetail?: boolean;
/** Start angle in degrees (0 = 3 o'clock) */
Expand All @@ -58,7 +56,6 @@
min = 0,
max = 100,
showProgress = true,
showPointer = true,
showDetail = true,
startAngle = 225,
endAngle = -45,
Expand All @@ -78,6 +75,31 @@
if (!data.length) return buildEmptyDataOption();

const point = data[0];
const arcWidth = compact ? 10 : 18;

const thresholdZones = parseGaugeThresholdZones(
thresholdZonesJson,
min,
max,
) as [number, string][];

Check warning on line 84 in component/src/charts/gauge-chart.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=alfredo1996_neoboard&issues=AZ27M0FThFkLk-eZDLQm&open=AZ27M0FThFkLk-eZDLQm&pullRequest=599

const hasCustomZones =
thresholdZones.length > 1 ||
(thresholdZones.length === 1 && thresholdZones[0][0] !== 1);

const resolvedColor = resolveItemColor(
point.value,
stylingRules,
paramValues,
);

// Determine the progress color: styling rule > custom zones > default accent
const progressColor = resolvedColor ?? "#5470c6";

// Track color — light gray that works in both themes
const trackColor = hasCustomZones
? (thresholdZones as never)
: ([[1, "rgba(140, 140, 140, 0.15)"]] as never);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

return {
tooltip: {
Expand All @@ -90,86 +112,64 @@
max,
startAngle,
endAngle,
radius: "90%",
progress: {
show: showProgress,
width: compact ? 10 : 16,
width: arcWidth,
roundCap: true,
itemStyle: {
color: progressColor,
},
},
pointer: {
show: showPointer,
length: "55%",
width: compact ? 4 : 6,
itemStyle: { color: "auto" },
show: false,
},
axisLine: {
roundCap: true,
lineStyle: {
width: compact ? 10 : 16,
color: parseGaugeThresholdZones(
thresholdZonesJson,
min,
max,
) as never,
width: arcWidth,
color: trackColor,
},
},
axisTick: {
show: !compact,
distance: compact ? 0 : -20,
splitNumber: 2,
length: 6,
lineStyle: { width: 1.5, color: "inherit" },
show: false,
},
splitLine: {
show: !compact,
distance: compact ? 0 : -20,
length: compact ? 8 : 12,
lineStyle: { width: 2, color: "inherit" },
show: false,
},
axisLabel: {
show: !compact,
distance: compact ? 0 : 30,
fontSize: 11,
color: "inherit",
show: false,
},
anchor: {
show: showPointer && !compact,
size: 10,
showAbove: true,
itemStyle: { borderWidth: 2, borderColor: "auto" },
show: false,
},
detail: {
show: showDetail,
valueAnimation: true,
fontSize: compact ? 14 : 24,
fontWeight: "bold",
fontSize: compact ? 18 : 36,
fontWeight: 600,
fontFamily:
'-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif',
formatter: "{value}",
offsetCenter: [0, showPointer ? "70%" : "0%"],
offsetCenter: [0, "0%"],
color: "inherit",
},
title: {
show: showDetail && !compact,
offsetCenter: [0, showPointer ? "90%" : "25%"],
fontSize: 12,
color: "inherit",
offsetCenter: [0, "22%"],
fontSize: compact ? 11 : 14,
color: "rgba(140, 140, 140, 0.8)",
fontWeight: 400,
},
animationDuration: 1000,
animationEasingUpdate: "cubicOut",
data: (() => {
const resolvedColor = resolveItemColor(
point.value,
stylingRules,
paramValues,
);
return [
{
value: point.value,
name: point.name ?? "",
...(resolvedColor
? { itemStyle: { color: resolvedColor } }
: {}),
},
];
})(),
data: [
{
value: point.value,
name: point.name ?? "",
...(resolvedColor ? { itemStyle: { color: resolvedColor } } : {}),
},
],
},
],
};
Expand All @@ -181,7 +181,6 @@
startAngle,
endAngle,
showProgress,
showPointer,
showDetail,
thresholdZonesJson,
compact,
Expand Down
Loading
Loading