rill icon indicating copy to clipboard operation
rill copied to clipboard

refactor (state management): Remove extra layer when mapping URL parameters to Explore state

Open ericpgreen2 opened this issue 8 months ago • 2 comments

In our current state mapping flow, we convert URLSearchParamsV1ExplorePresetMetricsExplorerEntity. The intermediate V1ExplorePreset layer is unnecessary and adds complexity.

This PR removes the extra layer, directly mapping URLSearchParams to MetricsExplorerEntity.

Note: This PR doesn’t much cleanup the state mapping logic itself, but it simplifies the flow and sets us up for future cleanup.

Builds on https://github.com/rilldata/rill/pull/7084, https://github.com/rilldata/rill/pull/7087

Checklist:

  • [x] Covered by tests
  • [x] Ran it and it works as intended
  • [ ] Reviewed the diff before requesting a review
  • [ ] Checked for unhandled edge cases
  • [ ] Linked the issues it closes
  • [x] Checked if the docs need to be updated
  • [ ] Intend to cherry-pick into the release branch
  • [x] I'm proud of this work!

ericpgreen2 avatar Apr 07 '25 14:04 ericpgreen2

This sadly conflicts heavily with https://github.com/rilldata/rill/pull/7062. I am getting rid of the intermediate step in there already. This would be redundant.

AdityaHegde avatar Apr 08 '25 04:04 AdityaHegde

This sadly conflicts heavily with #7062. I am getting rid of the intermediate step in there already. This would be redundant.

Nice to see that! Just this is the kind of thing that should be a standalone PR. Do you want to split your work out into a dedicated PR or should I get this one over-the-line?

ericpgreen2 avatar Apr 08 '25 07:04 ericpgreen2