Skip to content

feat: validate embed set state params upfront with setValidState#9670

Open
AdityaHegde wants to merge 5 commits into
mainfrom
feat/embed-setstate-upfront-validation
Open

feat: validate embed set state params upfront with setValidState#9670
AdityaHegde wants to merge 5 commits into
mainfrom
feat/embed-setstate-upfront-validation

Conversation

@AdityaHegde

@AdityaHegde AdityaHegde commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Adding setValidState({ state, failOnError }) to our embed state API.

  1. Validates the state against the dashboard's metrics view and explore specs before applying it.
  2. Invalid parameters are reported back, and the applied URL is canonicalized the same way it would be if the user navigated there directly.
  3. The applied state is returned in appliedState

Closes APP-918

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@nishantmonu51 nishantmonu51 added Type:Feature New feature request Size:M Medium change: 100-499 lines labels Jul 10, 2026
@AdityaHegde AdityaHegde marked this pull request as ready for review July 10, 2026 12:58
@nishantmonu51

Copy link
Copy Markdown
Collaborator

@codex: review

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: df211696db

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

"Expected params to be an object with a string `state` property",
);
}
const { state, failOnError = false } = params;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Default to failing when validation errors are present

When callers omit failOnError, this defaults to applying the cleaned URL even if validation reported errors, but both the inline API contract above and the new public docs say the default is to fail and not apply invalid state. In that common omitted-flag case, an embedder can receive success: true and have invalid parameters silently dropped instead of the state remaining unchanged as documented.

Useful? React with 👍 / 👎.

}

const currentUrl = new URL(pageState.url);
currentUrl.search = url.toString();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid applying default-stripped URLs through session restore

Applying the canonical URL here can render a different state than the appliedState returned when the validated URL is empty or view-only and the viewer has prior embed session state. The explore embed uses DashboardStateManager with session storage enabled, and DashboardStateDataLoader.getMergedExploreState calls getPartialExploreStateFromSessionStorage for empty URLs / view-only URLs, so after a parent calls setValidState({ state: "" }) to reset the dashboard, the subsequent navigation can restore the previous session filter instead of the rill defaults that buildValidatedExploreUrl computed.

Useful? React with 👍 / 👎.

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

Labels

Size:M Medium change: 100-499 lines Type:Feature New feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants