Skip to content

fix(grafana): restore parent time controls after drawer close - #262

Merged
harjotgill merged 1 commit into
coderabbit_micro_frontendfrom
codex/grafana-parent-time-owner
Sep 18, 2026
Merged

harjotgill merged 1 commit into
coderabbit_micro_frontendfrom
codex/grafana-parent-time-owner

Conversation

@harjotgill

Copy link
Copy Markdown

Summary

Restore the preserved parent dashboard as Grafana's time/dashboard service owner after closing a comment-details drawer.

Cause and correction

The parent dashboard stays mounted while a drawer initializes Grafana's singleton TimeSrv with the drawer model. Closing the drawer destroys that model, but previously only the MFE rendering UID returned to the parent. DashNavTimeControls uses the singleton for date selection and Refresh, so those controls kept targeting the destroyed drawer. The URL changed while the visible panels retained the old results.

The existing post-commit portal owner selection now rebinds TimeSrv and DashboardSrv to the surviving model only when ownership differs. It does not remount the dashboard, issue a query, or refresh just because a drawer closed. The shared URL remains the authoritative selected date range.

Evidence and testing

  • Reproduced on the authenticated internal Quality dashboard: after opening and closing Critical Comment Details, selecting Last 7 days changed the URL but retained 30-day values; header Refresh was also ineffective. Reloading that 7-day URL changed the values. The same date change before opening a drawer worked normally.
  • Added a regression using real TimeSrv, MFE stores/portals, and the actual date-picker/Refresh handlers. It fails before the correction because the time owner remains the destroyed drawer.
  • Covers surviving parent selection, current URL range, subsequent date change and manual refresh, no query on return, no parent destruction, and no redundant initialization.
  • Focused tests: 25 passed across fn-dashboard.test.tsx, DashboardPageLoading.test.tsx, and create-mfe.test.tsx. ESLint, Prettier, and git diff --check passed.
  • yarn typecheck: both unchanged baseline and fix report the same 122 errors across 95 files; none in changed files. The existing standalone TimeSrv suite fails during setup on both baseline and fix (standardEditorsRegistry.setInit). These are not claimed green.
  • Production-image build and startup health are CI merge gates.

Deployment / rollback

Deploy through the existing Grafana fork image and mono Grafana path. No database, IAM, query, ingestion, feature-flag, or permission-cache changes. Keep wider isolated-reader activation off. Revert this commit for rollback.

After deployment, repeat parent → drawer → close → change date → Refresh without a page reload, verify new requests/results and preserved controls. This local regression does not claim production validation or complete analytics parity.

Agent handoff

  • User intent: finish internal analytics validation, fix observed failures, preserve isolation and existing customer behavior, and ship coherent fixes promptly.
  • Repository/base: coderabbitai/grafana, coderabbit_micro_frontend (not upstream main).
  • Source: public/app/fn-app/fn-dashboard-page/fn-dashboard.tsx; lifecycle regression in adjacent fn-dashboard.test.tsx; existing DashboardPageLoading test service mock updated for the ownership path.
  • No external I/O, credentials, persistent ownership, new service, or architecture change.
  • Companion mono readiness correction: https://github.com/coderabbitai/mono/pull/41216. That fixes fresh parent readiness after drawer close; this fixes the independent Grafana singleton ownership failure subsequently observed.
  • Next step: inspect CI with gh pr checks, merge normally after green checks, then verify the deployed flow. No admin-merge authorization.

@coderabbitai

coderabbitai Bot commented Sep 18, 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: 9290c301-a4c5-4219-a432-ce8dc8405788

📥 Commits

Reviewing files that changed from the base of the PR and between aedd25b and e8e85b8.

📒 Files selected for processing (3)
  • public/app/features/dashboard/containers/DashboardPageLoading.test.tsx
  • public/app/fn-app/fn-dashboard-page/fn-dashboard.test.tsx
  • public/app/fn-app/fn-dashboard-page/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; 83 remain after this review.

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

6-7: LGTM!

Also applies to: 135-153

public/app/fn-app/fn-dashboard-page/fn-dashboard.test.tsx (1)

4-4: LGTM!

Also applies to: 6-10, 22-34, 37-48, 59-88, 98-98, 140-198

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

70-73: LGTM!


📝 Walkthrough

Walkthrough

DashboardPortal now restores the surviving parent dashboard as the owner of Grafana’s TimeSrv and DashboardSrv after a drawer closes. The change avoids remounting or refreshing the parent dashboard. Tests cover model lifecycle behavior, preserved URL ranges, date changes, manual refresh, and service mocks.

Suggested reviewers: gurinderrawala

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to e8e85

The drawer-close flow restores the parent dashboard without an identified merge-blocking regression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Comment Severity Gate ❓ Inconclusive No CodeRabbit findings or posted review comments were supplied in the review context. The repository contains no review-comment metadata for the reviewed refs, and the commit patch does not provide fi… Provide the complete CodeRabbit finding list, including severity, file/line location, and the supplied resolution status for each posted comment. Then re-run the severity gate.
✅ 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 clearly and concisely describes the main change: restoring the parent dashboard time controls after the drawer closes.
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.
Full details: Comment Severity Gate

Explanation

No CodeRabbit findings or posted review comments were supplied in the review context. The repository contains no review-comment metadata for the reviewed refs, and the commit patch does not provide finding resolution statuses. Therefore, the presence and resolution of Critical or Major findings cannot be determined.

✨ 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 watched the drawer glide away
The parent kept its time controls in play
The dashboard services found their home
No stale drawer owned the controls to roam
Fresh ranges and refresh now work just right
Hoppy tests confirm the fix tonight

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

@harjotgill
harjotgill merged commit ed62fdb into coderabbit_micro_frontend Sep 18, 2026
4 checks passed
@harjotgill
harjotgill deleted the codex/grafana-parent-time-owner branch September 18, 2026 18:28
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