Skip to content

fix(mfe): refresh drilldown pagination from history - #258

Merged
harjotgill merged 1 commit into
coderabbit_micro_frontendfrom
codex/grafana-mfe-pagination-history
Sep 18, 2026
Merged

harjotgill merged 1 commit into
coderabbit_micro_frontendfrom
codex/grafana-mfe-pagination-history

Conversation

@harjotgill

@harjotgill harjotgill commented Sep 17, 2026 •

Copy link
Copy Markdown

Summary

Fix query pagination in the embedded comment drilldown without refreshing the parent dashboard or adding a global refresh mechanism.

The live Major drilldown returned A=10 rows and B=11,523 with HTTP200. Next/Previous changed the URL offset and enabled pagination controls, but emitted no new /grafana/api/ds/query request and left the same rows visible. Parent IDE pagination does issue a new query; this is not a claim that all pagination was broken.

Cause and fix

RenderFNDashboard sampled Grafana history only during host-prop renders and passed only host query parameters to DashboardPage. BusinessTable changes Grafana history directly. The child portal therefore missed the variable-change refresh boundary. Additionally, fnPathnameChange mutates the shared history location in place.

Subscribe to history for the currently owning portal, apply only changed var-* keys to its local query snapshot, and pass an immutable search snapshot to the existing real DashboardPage variable-change path. Host updates supersede the local snapshot; unchanged host echoes do not trigger another refresh. Preserve host uid and non-variable routing props; do not merge the entire shared URL. Unsubscribe on cleanup. No backend, authorization, SQL, result-limit, or deployment changes.

Validation

  • Fail-before: real FNDashboard/store/history → RenderFNDashboard → connected DashboardPage; offset10 expected one templateVarsChangedInUrl call, received zero.
  • Pass-after: 24 tests across DashboardPageLoading, fn-dashboard, and create-mfe. Covers two consecutive child page changes with unchanged host props, exact changed variables only, unchanged parent variables, immutable host input, host echo without duplicate refresh, host reset, and unsubscribe.
  • Tests stub dashboard initialization/results and the existing variable thunk at the DashboardPage boundary; they do not claim a production SQL round trip. Only the decorative ellipsis SVG asset fetch is stubbed, with all unexpected fetches rejected.
  • Scoped ESLint, Prettier and diff whitespace checks pass.
  • Local production frontend build could not complete: the reused dependency tree lacks @prometheus-io/lezer-promql. No dependency changes are included; production container CI remains the artifact build gate.
  • npm run typecheck reports122 errors in95 unchanged files, none in this patch. Normalized error lines exactly match the existing Fix cross-dashboard cancellation when closing drill-downs #257 final typecheck log (122/122, no added/removed errors); this comparison reuses that recorded baseline, not a fresh baseline run.
  • Normal PR CI must build the production container and verify startup/database health before merge.

Rollout and rollback

Based on merged #257 (c09763ea2d). Neither #257 nor this fix is implied deployed by the mono dashboard-template releases. After review/CI and explicit deployment authorization, publish the Grafana base and verify exact regional derived-image provenance before testing child Next/Previous, page-size changes, close/reopen, and parent refresh overlap. Reverting this commit retains #257; no production dispatch is part of this PR.

Agent handoff

  • Intent: repair the observed child-pagination history refresh gap using the existing MFE query path, not a new ownership/refresh framework.
  • Changed files: public/app/fn-app/fn-dashboard-page/render-fn-dashboard.tsx and public/app/features/dashboard/containers/DashboardPageLoading.test.tsx.
  • Worktree/branch: /tmp/grafana-mfe-pagination-history, codex/grafana-mfe-pagination-history.
  • No repository AGENTS.md or PR template was present in the tracked Grafana checkout; existing lifecycle regression and build/startup CI were reused.
  • Next: review exact diff, require normal CI, then obtain deployment authorization. Live recovery remains unverified until the new base is published and exercised.
  • First command: git status --short && git diff origin/coderabbit_micro_frontend...HEAD --stat.
  • Focused test command: node node_modules/jest/bin/jest.js public/app/features/dashboard/containers/DashboardPageLoading.test.tsx public/app/fn-app/fn-dashboard-page/fn-dashboard.test.tsx public/app/fn-app/create-mfe.test.tsx --runInBand --watch=false.

Summary by CodeRabbit

  • Bug Fixes
    • Improved dashboard navigation when multiple dashboards are embedded on the same page.
    • Changes to pagination history now refresh only the relevant dashboard.
    • Preserved unrelated URL query parameters during dashboard navigation.
    • Removed dashboard variables are now correctly reflected in the URL.
    • Prevented redundant URL updates and stopped navigation updates after a dashboard is removed.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: 348aedef-65b5-4460-b032-b09af834e1cf

📥 Commits

Reviewing files that changed from the base of the PR and between c09763e and 5cefd25.

📒 Files selected for processing (2)
  • public/app/features/dashboard/containers/DashboardPageLoading.test.tsx
  • public/app/fn-app/fn-dashboard-page/render-fn-dashboard.tsx
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • coderabbitai/bitbucket (manual)

Included review availability: Your plan provides up to 100 included reviews per hour; 84 remain after this review.

📜 Recent review details
🔇 Additional comments (2)
public/app/fn-app/fn-dashboard-page/render-fn-dashboard.tsx (1)

2-8: LGTM!

Also applies to: 34-67, 95-108

public/app/features/dashboard/containers/DashboardPageLoading.test.tsx (1)

12-13: LGTM!

Also applies to: 53-53, 73-81, 87-218


📝 Walkthrough

Walkthrough

RenderFNDashboard now observes history changes for the owning microfrontend dashboard. It applies only changed var-* parameters and preserves host and unrelated routing state. Effective query parameters flow through the existing DashboardPage variable-change path. Tests cover pagination changes, host resets and echoes, duplicate suppression, immutable inputs, and listener cleanup.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: 🔵 Low · up to 5cefd

With multiple embedded portals, a variable change in one portal can refresh another dashboard with the wrong values. The impact is bounded to that multi-portal interaction, but should be addressed or explicitly accepted before merge.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is concise and accurately describes the main change: refreshing embedded drilldown pagination from browser history.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR

A rabbit hops where page numbers turn
Child charts refresh as query changes burn
Host paths stay steady, untouched and clear
Old listeners vanish when unmounts appear
Variables dance, while portals cheer

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

@harjotgill

Copy link
Copy Markdown
Author

Reviewed the multi-portal risk noted in the summary. The regression mounts parent and child together and asserts only the owning child receives the exact variable delta; parent query parameters and host input remain unchanged. It also covers consecutive paging, host echo without duplicate refresh, host reset, and unsubscribe. Host routing/UID remains authoritative, and this change does not alter server-side query scope or credentials. Keeping the narrow fix; production acceptance will repeat child paging and parent close/reopen behavior after deployment.

@harjotgill
harjotgill merged commit 37934b8 into coderabbit_micro_frontend Sep 18, 2026
4 checks passed
@harjotgill
harjotgill deleted the codex/grafana-mfe-pagination-history branch September 18, 2026 00:11
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.

1 participant