Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix delayed dashboard url state load #4035

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

AdityaHegde
Copy link
Collaborator

Checklist

  • Manual verification
  • Unit test coverage
  • E2E test coverage
  • Needs manual QA?

Summary

Issue addressed:

Just after opening a dashboard with a url state that has non-default time range selected, changing the time grain for the 1st time will reset the time range to the default time range.

Details:

This is because there is a delay in url state applying to the dashboard store. This PR makes sure that the url state is loaded as soon as the store is created.

Steps to Verify

  1. Go to a dashboard.
  2. Select a different time range.
  3. Refresh the page.
  4. Change the time grain.
  5. Time range should not be reset.

Copy link
Contributor

@ericpgreen2 ericpgreen2 left a comment

Choose a reason for hiding this comment

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

I couldn't reproduce the issue, but happy to approve this if you think it solves the problem.

const metricsViewSchema = createQueryServiceMetricsViewSchema(
$runtime.instanceId,
metricViewName,
);

function syncDashboardState() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This function could be more clear if weren't a closure and instead accepted input parameters. With this PR, it's getting more complicated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree. A lot of this code might not be needed anymore. We should revisit this once when we refactor state managers.

@AdityaHegde AdityaHegde merged commit d3cc9fe into main Feb 15, 2024
6 checks passed
@AdityaHegde AdityaHegde deleted the adityahegde/fix-delayed-url-state-load branch February 15, 2024 04:53
mindspank pushed a commit that referenced this pull request Feb 23, 2024
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.

2 participants