Conversation
Cancel the embed continuation on teardown so a race cannot strand the 1 Hz tab poll on a destroyed iframe. Isolate the tab channel: a getActiveTabs failure now stops only the poll, leaving filter persistence through observeDataMask alive. Validate the cross-origin getActiveTabs result and refuse an empty first reading as the baseline, so a fresh load no longer writes a dashboard_state the user never asked for. Also guard against overlapping polls and seed lastEmittedKey from the url so reverting to the loaded state does not rewrite it.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes INT-64
Refreshing the analytics page dropped the user back on the Revenue Streams tab with all 16 filters reset, whatever they were looking at. It stings more than usual because refreshing is also the only workaround for the analytics timeouts in INT-63.
The dashboard now keeps its state in the URL as
?dashboard_state=<key>, so a refresh restores the tab and the filters, and the link is shareable.How it works
The dashboard runs in a cross-origin iframe, so state can only be injected as query params at mount and read back over the embedded SDK's RPC channel. Superset 6.1.0 restores a saved
activeTabsthroughpermalink_keyand through nothing else, so we ask the iframe for a permalink whenever the filters or the active tab change, keep the key in our URL, and hand it back aspermalink_keyon the next load.dashboardStateSync.tsowns the capture side and is deliberately framework-free: no React, no router, no knowledge of the URL. It takes anEmbeddedDashboardand a callback, and returns a teardown. Filters arrive by push throughobserveDataMask; tabs have no push event in the SDK, so they are polled once a second, paused while the document is hidden.Dashboard.tsxowns the URL side: read the param once at mount, feed it to the embed, write new keys back with areplacenavigation.Depends on a Superset-side permission
Creating a permalink needs
can_write on DashboardPermalinkRestApion theLagoViewerguest role, which is not deployed yet: getlago/lago-data#710.This branch does not wait for it. If the permalink call fails, the sync disables itself and the page behaves exactly as it does today, so the two changes can land in either order.
Also in here
The localStorage filter-persistence path is gone, along with its feature flag, its org-and-dashboard key scoping, and two utility modules that had no other consumer (
supersetFilters.ts,risonEncoder.ts). The URL is inherently org-scoped by the slug, so that scoping is no longer needed.Notes for review
Three things in here are load-bearing and easy to undo by accident:
useState, never subscribed to, andnavigateis held in a ref rather than listed as an effect dependency. The slug-awareuseNavigatewrapper returns a new identity every render, so either mistake remounts the iframe (a full dashboard reload) on every filter change.syncStateKeyre-checks itsdisabledflag after the awaited RPC resolves.debounce.cancel()stops a pending invocation but cannot abort one in flight, so without that check a permalink resolving after teardown would navigate onto whatever route the user has since moved to.embedDashboardresolves on the iframe'sloadevent, well before Superset hydrates, so baselining a pre-hydration reading would make the next poll read the real default tabs as a change and write adashboard_stateonto a page nobody touched.A permalink failure disables everything (without permalinks there is nothing to sync); a tab-poll failure stops only the poll, leaving the push channel alive.