Skip to content

fix(app): refresh button silently no-op'd due to queryKey mismatch (#779) - #788

Closed
alfredo1996 wants to merge 814 commits into
devfrom
fix/issue-779-refresh-button-key
Closed

fix(app): refresh button silently no-op'd due to queryKey mismatch (#779)#788
alfredo1996 wants to merge 814 commits into
devfrom
fix/issue-779-refresh-button-key

Conversation

@alfredo1996

@alfredo1996 alfredo1996 commented May 16, 2026

Copy link
Copy Markdown
Owner

Summary

Per-widget manual refresh button silently no-op'd. The dashboard-container.tsx onRefresh handler invalidated TanStack Query with a 4-element key (["widget-query", connectionId, query, params]), but useWidgetQuery caches under a 5-element key (["widget-query", connectionId, query, mergedParams, staleTime]). The 3rd position differs (widget.params raw template ≠ mergedParams resolved at hook call time) so the prefix match failed and no refetch fired.

Fix: truncate the invalidation prefix to 3 elements (stop at query). TanStack Query's prefix matching catches all variants regardless of params/staleTime — and params/staleTime are computed at hook call time and not reproducible from widget alone.

Original implementation reference

The same fix shipped on release/2.1 as commit 25617a55 (bundled with #778). This PR ports the #779 portion only, against current dev. See that commit for the original diagnostic notes.

Re-implementation context

Part of the v2.1 — Re-implementation on dev milestone (cascade strategy abandoned 2026-05-16; see #787 for full context).

Test plan

  • Unit: 2 new tests in dashboard-container-refresh.test.tsx
    • asserts invalidate-prefix matches the cached key shape
    • asserts invalidation scopes to the clicked widget's connection + query (other widgets untouched)
  • npm -w app run test -- dashboard-container — 7/7 pass
  • CI verifies full suite (local has a stale-deps zod-4 mismatch unrelated to this fix)
  • E2E auto-refresh.spec.ts + widget-states.spec.ts refresh-button tests pass in CI

Closes #779

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved widget refresh button functionality with optimized cache key handling, ensuring the refresh button correctly targets only the selected widget without inadvertently affecting data from other widgets
  • Tests

    • Added comprehensive regression test suite for widget refresh functionality, verifying proper cache invalidation behavior and correct scoping per widget

Review Change Stack

alfredorubin96 and others added 30 commits April 4, 2026 04:20
…w area

- widget-states E2E: look for error button aria-label instead of banner text
- widgets E2E: type a query before expecting connector warning (matches #355)
- widget-editor-modal: show error state in preview area when query fails,
  guard waiting-spinner with !previewQuery.isError

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- reset.ts: validate postgres user/database as safe SQL identifiers
- seed.ts: validate neo4j credentials for shell-safe chars, validate
  seed script path stays within project root

Addresses CodeRabbit security review comments on PR #358.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…le closure

handlePreview captured connectionId/query/selectedConnection in its
closure, but in lab-edit mode these values were set in the same
render cycle. By the time the auto-preview effect fired, the closure
still had empty/stale values. Move all dependencies to refs so
handlePreview always reads the latest committed state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix: v1.0 bug fixes — CLI integration + 5 P0 issues (#350, #354, #355, #356, #357)
release/1.0: all P0/P1 fixes + quick wins (#323-#331, #336, #339, #341, #342)
…tter help

Local mode (--mode local):
- Health checks use pg_isready/curl instead of docker exec
- start.ts skips Docker compose entirely in local mode
- dev.ts auto-starts DB containers if not running, validates env
- doctor.ts treats Docker checks as warnings (not failures) in local mode

New commands:
- `neoboard config list/get/set` — manage neoboard.config.json
- `neoboard logs [service]` — tail Docker container logs

UX improvements:
- Rich help text with quick-start examples on every command
- writeProjectConfig extracted to lib/config.ts (shared by init + config)
- Actionable error messages when DBs not reachable in local mode

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
feat(cli): frictionless install — local mode, config cmd, logs, better help
New sections (8 pages):
- CLI: overview, Docker setup, local setup, command reference
- Guides: first dashboard, connecting databases, managing users, index

Updated pages (9 pages):
- Welcome page with hero screenshot
- Installation with 3 install paths
- Quick start with step-by-step screenshots
- Configuration with env var table and examples
- Architecture with Mermaid diagram
- Connectors, Dashboards, Widgets, Parameters — all with screenshots

Assets:
- 6 screenshots: login, dashboard-list, connections, widget-lab, users, settings

Infra:
- Fixed fumadocs-mdx API migration (resolveFiles → toFumadocsSource)
- Updated tsconfig paths for new .source structure

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace the Fumadocs/Next.js docs site with Astro Starlight for a
completely standalone, zero-JS static docs site.

- Starlight 0.31 + Astro 5.3 (compatible with Node 20)
- Built-in search (Pagefind), dark/light theme, GitHub link
- All 30 MDX content pages migrated (Cards → CardGrid/LinkCard)
- Screenshots render from public/screenshots/
- Sidebar auto-generated from directory structure
- Builds to static HTML — deployable anywhere

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New chart documentation (13 pages):
- Individual pages for bar, line, pie, single-value, table, graph,
  map, json, form, markdown, iframe, param-select
- Each page has: overview, Cypher + SQL query examples, full options
  table with types/defaults, practical tips
- Chart Types index with comparison table

Hero page improvements:
- Clear tagline and feature bullet list
- Three installation paths table with time estimates
- CardGrid links to key sections

Fixes:
- Removed broken /guides/building-widgets links (3 files)
- Fixed demo credentials (admin@neoboard.local / admin123)
- Fixed Creator role description in multi-tenancy page
- Added Charts section to sidebar config

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Critical:
- All SQL/Cypher examples now use actual demo schema (Person, Movie,
  user table) instead of non-existent tables (categories, users, Order)
- .env.local path fixed to app/.env.local
- Removed broken developer links to non-existent architecture/internals pages
- UNION ALL query in single-value trend mode now has ORDER BY

High:
- Terminology consistency: "From source" → "Fully local"
- Form widget docs explain how to enable write permission
- Map examples marked as "requires your own geo data"

CodeRabbit:
- Added engines.node >= 20.3.0 to docs/package.json
- Added "check": "astro check" script

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
docs: v1.0 documentation overhaul — screenshots, CLI docs, user guides
Convert the user footer section from a static div to a clickable
button that navigates to /settings/profile. Adds cursor-pointer,
hover styling, and aria-label for keyboard accessibility.

Closes #351

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a widget is first placed on a dashboard grid, the container
often starts with 0 dimensions during ECharts init. The first
setOption() draws to a 0x0 canvas, leaving the chart blank until
the user resizes or reloads.

Force instance.resize() after setOption so the chart picks up the
real container size on first render.

Closes #332

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
buildCategoryAxisLabel now considers container width when computing
rotation — narrower containers rotate more aggressively so labels
don't overlap.

Thresholds (pixels per label):
- < 40px: 60°
- < 70px: 45°
- < 100px: 30°
- >= 100px: 0°

Also tightens label truncation to 10 chars when container is < 400px.

Closes #337

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add two new boolean options to LineChart:
- connectNulls: draw lines through missing (null) data points
- endLabel: show series name label at the end of each line

Exposed via the chart options schema so users can toggle in the UI.

Closes #146

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add edge ID to metadata items in relationship inspection panel
- Add Badge indicator showing 'Node' or 'Relationship' in panel header
- Thread optional id through GraphEdge type and transform pipeline so
  the inspection panel can surface the internal element id for edges

Closes #352

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add rightAxisSeries prop to LineChart so series with different scales
can share the same chart. When rightAxisSeries is non-empty, yAxis
becomes an array [leftAxis, rightAxis] and each series gets
yAxisIndex 0 or 1 based on whether its name appears in the set.

Also adds a rightYAxisLabel prop for the secondary axis label, exposes
both options in the chart options panel as text inputs (comma-separated
for series names), and wires them through chart-renderer.

Closes #159
Adds static options support for the form widget's select field type.
Users can now provide a comma-separated list of options (e.g.
'low,medium,high') instead of writing a seed query. When staticOptions
is set, it takes precedence over the seed query. Multi-select and
cascading-select still use seed queries only.

Closes #151
Replace the minimal EmptyState on the dashboards page with a welcoming
Getting Started guide for first-time users. Shows a welcome message, a
prominent "Create your first dashboard" CTA, a link to the docs, and a
three-step guide (connect, create, add widgets) with deep links to the
relevant pages.

Closes #333

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
alfredorubin96 and others added 21 commits May 13, 2026 15:30
Add optional multi-step wizard mode to the form widget. Fields can be
assigned to numbered steps, enabling guided data entry workflows.

Data model:
- FormFieldDef gains optional `step?: number` field (no DB migration)
- chartOptions.stepLabels: string[] for custom step names
- chartOptions.enableSummary: boolean (default true when steps exist)

New components:
- FormStepIndicator (component/) — progress bar with completed/current states
- useFormWizard hook — manages step navigation, per-step validation, reset

Renderer changes:
- Wizard mode: shows step indicator, renders only current step's fields
- Next button validates current step before advancing
- Back button navigates freely (no validation)
- Summary step shows all values for review before submit
- Non-wizard forms render unchanged (backward compatible)

Editor changes:
- Step number input per field in form-fields-editor
- Step label inputs in advanced tab (shown when steps are assigned)

Closes #165

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Skip per-keystroke delays in login page tests that timed out in CI.
Add unit tests for useFormWizard hook, FormStepIndicator component,
and FormWidgetRenderer wizard mode to satisfy SonarCloud 80% new
code coverage gate.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Fix unreachable number-range branch in formatSummaryValue by moving
  the check before the generic array early-return
- Add NaN guard to step number input in form-fields-editor
- Add bounds validation (step >= 0) to goToStep in useFormWizard
- Remove unused import in form-step-indicator test
- Rename misleading test description

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Skip per-keystroke delays in signup page tests that timed out in CI.
Fix TS spread error in wizard test mock. Export and add direct unit
tests for formatSummaryValue covering all branches (number-range,
multi-select, date-range, scalars, empty values).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Revert the export and drop the 15-test file that only existed to
compensate for SonarCloud recounting lines after the source edit.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove 4 TypeScript type-checking tests from advanced-connection-options
that only verified compilation, not runtime behavior. Consolidate 5
repetitive date-range-picker preset tests into a single test.each,
adding the missing 'Last 30 days' case in the process.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- connection: exclude dist/ from Jest testPathIgnorePatterns (fixes 2
  spurious suite failures from compiled output)
- app: rename chart-error-boundary.test.tsx → chart-renderer.test.tsx
  to match the component it actually tests
- e2e/transforms: remove 1s sleep after connection select (Run button
  enabled check already gates); replace 1s post-save sleep with
  Save button re-enabled assertion
- e2e/form-widget: remove duplicate 400ms debounce wait
- e2e/widget-lab: replace 1s post-save sleep with Save button
  re-enabled assertion

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use `vi.useFakeTimers({ shouldAdvanceTime: true })` in all five
DateRangeParameter preset tests to prevent Radix Popover animation
timers from hanging under fake timers in CI. Matches the established
pattern in date-range-picker.test.tsx and copy-button.test.tsx.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Shiki v4 exports individual themes and languages from @shikijs/themes/*
and @shikijs/langs/* instead of shiki/themes/* and shiki/langs/*.
The old deep import paths lack TypeScript type declarations, causing
TS2307 errors.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract inline percent tooltip formatter from bar-chart.tsx into
buildPercentTooltipFormatter in chart-utils.ts, reusing the existing
TooltipParam type. Add renderBarOptions test helper to eliminate
repeated render + extract-options boilerplate across 20+ test cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…provements

fix(form): form widget UX, performance, and a11y improvements
…ed-bar

feat(charts): percentage stacked bar chart mode
…ax-highlighting

feat(markdown): syntax highlighting for fenced code blocks
…zard

# Conflicts:
#	app/src/components/form-widget-renderer.tsx
#	app/src/components/widget-editor-modal.tsx
- Add escapeHtml() to chart-utils.ts and apply it to all database-
  originated values in buildTooltipFormatter and
  buildPercentTooltipFormatter (header, seriesName, absValue).
  ECharts renders tooltip formatters via innerHTML, so unescaped
  database content (e.g. <img onerror=...>) would execute.
- Sanitize Shiki syntax-highlighting output in markdown-widget before
  dangerouslySetInnerHTML: strip <script>, <style>, and on* attributes
  as defense-in-depth against unexpected highlighter output.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- When submit finds validation errors in wizard mode, navigate to the
  first step containing an error so the user sees what needs fixing
  (previously the Submit button was just disabled with no explanation).
- Always reset wizard to step 0 on successful submission, even when
  resetOnSuccess is false — leaving the user on the summary step
  after submit is a dead end with no way forward.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove duplicate escapeHtml definitions from map-chart.tsx and
markdown-widget.tsx. Both now import from chart-utils.ts which
was already added as the canonical location in the tooltip XSS fix.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
)

dashboard-container.tsx invalidated TanStack Query with a 4-element key
(["widget-query", connectionId, query, params]) but useWidgetQuery's
actual cache key is 5 elements (["widget-query", connectionId, query,
mergedParams, staleTime]). Position 3 mismatched because widget.params
(raw template) ≠ mergedParams (resolved with dashboard parameter values
at hook call time), so the prefix match failed and no refetch ever fired.

Fix: truncate the invalidation prefix to 3 elements (stop at query).
TanStack Query's prefix matching catches all variants regardless of
params or staleTime. params/staleTime are computed at hook call time
and not reproducible from `widget` alone.

Tests:
- dashboard-container-refresh.test.tsx (new): 2 cases asserting
  invalidate-prefix matches the cached key shape and scopes to the
  clicked widget's connection + query
- 27/27 dashboard-container tests pass

Original implementation reference: commit 25617a5 on release/2.1
(bundled #778 + #779 together; here we ship #779 alone).

Refs: #779
@coderabbitai

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown

Walkthrough

This PR fixes a TanStack Query cache invalidation bug in the dashboard widget refresh feature. The widget refresh button now invalidates cached entries using a shorter query key prefix ["widget-query", connectionId, query] instead of including variable fields like mergedParams and staleTime, enabling reliable prefix-based matching. Regression tests verify the fix prevents stale cache entries from blocking refreshes and enforces per-widget scoping.

Changes

Widget Refresh Query Key Invalidation Fix

Layer / File(s) Summary
Invalidation key prefix fix and documentation
app/src/components/dashboard-container.tsx
queryClient.invalidateQueries now uses a 3-element prefix key ["widget-query", connectionId, query] instead of including widget.params and staleTime. Inline comments explain the full cache key structure and why shorter prefixes prevent refresh no-ops when params/staleTime vary across cache slots.
Test infrastructure, mocks, and fixtures
app/src/components/__tests__/dashboard-container-refresh.test.tsx
Regression test suite with detailed comments describing the original bug. Comprehensive mocks for dashboard components, widgets, parameters, and query utilities. Helper functions construct DashboardWidget and DashboardPage fixtures with grid layout.
Cache invalidation and scoping test cases
app/src/components/__tests__/dashboard-container-refresh.test.tsx
Two regression tests: (1) verify that refresh button invalidation uses a prefix key matching cached widget-query entries, confirming they are marked invalidated; (2) verify that refreshing one widget does not invalidate other widgets' cached entries with different connectionId or query values.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

  • alfredo1996/neoboard#779: This PR addresses the widget refresh cache key prefix issue; the related issue argues whether refresh should use refetchQueries vs. invalidateQueries—both concern the same refresh call behavior in DashboardContainer.

Possibly related PRs

  • alfredo1996/neoboard#519: Both PRs target the per-widget manual refresh behavior; this PR fixes the TanStack Query invalidation key prefix, while the related PR adds E2E validation that refresh triggers the expected /api/query refetch.

Suggested labels

bug, pkg:app, area:dashboard, testing

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the core fix: the refresh button was silently failing due to a queryKey prefix mismatch, which directly aligns with the primary change of truncating the invalidation key from 4 to 3 elements.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/issue-779-refresh-button-key

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
app/src/components/__tests__/dashboard-container-refresh.test.tsx (1)

96-137: ⚡ Quick win

Skip the Next.js module mocks in this test — the component doesn't use next/navigation or next/dynamic.

The DashboardContainer component (under test) doesn't import or use Next.js navigation or dynamic imports. Adding these mocks would be unnecessary boilerplate without functional benefit. Standard practice is to mock dependencies that the component or test actually needs; transitive mocking of unused modules adds clutter.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/__tests__/dashboard-container-refresh.test.tsx` around
lines 96 - 137, Remove the unnecessary Next.js mocks from the test (specifically
any vi.mock calls for "next/navigation" and "next/dynamic") since
DashboardContainer doesn't import or use those modules; keep only the mocks
actually used by this test (e.g., CardContainer, interpolateTitle,
buildExportData, widget-utils, widget-actions, parameter-store,
format-parameter-value, resolve-cache-options and the shouldShowRefreshButton
override) so the test file only stubs the real dependencies referenced by the
DashboardContainer component.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/src/components/__tests__/dashboard-container-refresh.test.tsx`:
- Around line 96-137: Remove the unnecessary Next.js mocks from the test
(specifically any vi.mock calls for "next/navigation" and "next/dynamic") since
DashboardContainer doesn't import or use those modules; keep only the mocks
actually used by this test (e.g., CardContainer, interpolateTitle,
buildExportData, widget-utils, widget-actions, parameter-store,
format-parameter-value, resolve-cache-options and the shouldShowRefreshButton
override) so the test file only stubs the real dependencies referenced by the
DashboardContainer component.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 59e9c42b-e94b-4319-afec-da4f878fccc6

📥 Commits

Reviewing files that changed from the base of the PR and between eaca140 and a0b6185.

📒 Files selected for processing (2)
  • app/src/components/__tests__/dashboard-container-refresh.test.tsx
  • app/src/components/dashboard-container.tsx

@sonarqubecloud

Copy link
Copy Markdown

@alfredo1996

Copy link
Copy Markdown
Owner Author

Obsolete: the refresh-button queryKey fix is already on the new dev tip via the cascade work that landed on release/2.0. The force-push of chore/release-2.0-preflight to dev (#789) included it. Closing without merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(e2e): stabilize refresh-button flakes (auto-refresh + widget-states + widget-lab)

2 participants