[HDS] Prep/cleanup steps for Highlighting the data source #39681
Conversation
This file contains 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
`isSavedQuestionSelected` is too narrow because we can also have other saved entities, such as models (and soon metrics) as the selected source.
|
ranquild
approved these changes
Mar 6, 2024
nemanjaglumac
added a commit
that referenced
this pull request
Mar 7, 2024
* Rename `DATA_BUCKET` key to `MODELS` * Rename misleading method `isSavedQuestionSelected` is too narrow because we can also have other saved entities, such as models (and soon metrics) as the selected source. * Fix type for `selectedTableId` * Extract `sourceId` * Rename saved question instances to saved entity * Rename related components
nemanjaglumac
added a commit
that referenced
this pull request
Mar 7, 2024
* [HDS] Prep/cleanup steps for Highlighting the data source (#39681) * Rename `DATA_BUCKET` key to `MODELS` * Rename misleading method `isSavedQuestionSelected` is too narrow because we can also have other saved entities, such as models (and soon metrics) as the selected source. * Fix type for `selectedTableId` * Extract `sourceId` * Rename saved question instances to saved entity * Rename related components * Show models correctly in the data source picker (#39686) * Show models correctly in the data source picker * Address review comments * Destructure props * Add E2E reproduction for #39699 * Prevent accessing potentially stale value Closes #39699
nemanjaglumac
added a commit
that referenced
this pull request
Mar 7, 2024
* [HDS] Prep/cleanup steps for Highlighting the data source (#39681) * Rename `DATA_BUCKET` key to `MODELS` * Rename misleading method `isSavedQuestionSelected` is too narrow because we can also have other saved entities, such as models (and soon metrics) as the selected source. * Fix type for `selectedTableId` * Extract `sourceId` * Rename saved question instances to saved entity * Rename related components * Show models correctly in the data source picker (#39686) * Show models correctly in the data source picker * Address review comments * Destructure props * Add E2E reproduction for #39699 * Prevent accessing potentially stale value Closes #39699
nemanjaglumac
added a commit
that referenced
this pull request
Mar 7, 2024
* [HDS] Prep/cleanup steps for Highlighting the data source (#39681) * Rename `DATA_BUCKET` key to `MODELS` * Rename misleading method `isSavedQuestionSelected` is too narrow because we can also have other saved entities, such as models (and soon metrics) as the selected source. * Fix type for `selectedTableId` * Extract `sourceId` * Rename saved question instances to saved entity * Rename related components * Show models correctly in the data source picker (#39686) * Show models correctly in the data source picker * Address review comments * Destructure props * Add E2E reproduction for #39699 * Prevent accessing potentially stale value Closes #39699 Co-authored-by: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR is only a cleanup phase that should support the work on #39542.
The problematic part in the
DataSelectorwas that it was never designed to truly work with models 100%. The naming of many related components suggests this. Everything was revolving around a "saved question". In this PR I renamed methods and the components to reflect the fact there can be multiple different entities that are saved.Currently, those are saved questions and (saved) models. But pretty soon we will have metrics as well (see #37358).
Since we're also moving from the use of the term "dataset" to "model" in more places in the code, I updated some of this references as well (see
DATA_BUCKETfor example).Other than that, no code logic was changed in this PR.
Everything should still work the same, and all tests should pass.
Note
Keep in mind this merges into a feature branch. Not
master.