Adding controls for verifying options #7468
Merged
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
Codecov Report
@@ Coverage Diff @@
## master #7468 +/- ##
==========================================
+ Coverage 65.17% 65.22% +0.04%
==========================================
Files 433 434 +1
Lines 21428 21459 +31
Branches 2360 2368 +8
==========================================
+ Hits 13966 13996 +30
- Misses 7342 7343 +1
Partials 120 120
Continue to review full report at Codecov.
|
kristw
reviewed
May 7, 2019
superset/assets/src/explore/components/controls/withVerification.jsx
Outdated
Show resolved
Hide resolved
superset/assets/src/explore/components/controls/withVerification.jsx
Outdated
Show resolved
Hide resolved
superset/assets/src/explore/components/controls/withVerification.jsx
Outdated
Show resolved
Hide resolved
superset/assets/src/explore/components/controls/withVerification.jsx
Outdated
Show resolved
Hide resolved
michellethomas
force-pushed
the
testing_filtering_for_controls
branch
from
47463a3 to
e82b52f
Compare
kristw
approved these changes
May 14, 2019
superset/assets/src/explore/components/controls/withVerification.jsx
Outdated
Show resolved
Hide resolved
michellethomas
force-pushed
the
testing_filtering_for_controls
branch
from
fa9d278 to
cb132f1
Compare
michellethomas
changed the title
michellethomas
commented
May 15, 2019
| if (Array.isArray(json)) { | ||
| this.setState({ validOptions: new Set(json) || new Set() }); | ||
| } | ||
| }).catch(error => console.log(error)); |
There was a problem hiding this comment.
This endpoint will be called frequently whenever control values change, so if there's a problem with the validation endpoint we probably don't want to notify users every time (because that could create a large number of error messages). Failing silently would just result in showing all options. But we may want to be able to debug if there are issues, so I thought about putting this console log in. It doesn't seem like the best solution, but it works for now unless someone has another thought.
michellethomas
force-pushed
the
testing_filtering_for_controls
branch
from
83482f0 to
ca6e868
Compare
|
Thanks for the feedback, I'll merge this tomorrow unless anyone has any final comments or concerns. |
michellethomas
added a commit
to michellethomas/panoramix
that referenced
this pull request
May 31, 2019
* Creating withVerification HOC * Updating to use componentDidMount and componentDidUpdate and adding propTypes * Adding tests to withVerification * Adding documentation to withVerification (cherry picked from commit 421183d)
michellethomas
added a commit
that referenced
this pull request
Jun 1, 2019
* Creating withVerification HOC * Updating to use componentDidMount and componentDidUpdate and adding propTypes * Adding tests to withVerification * Adding documentation to withVerification (cherry picked from commit 421183d)
6 tasks
mistercrunch
added
🏷️ bot
cccs-rc
pushed a commit
to CybercentreCanada/superset
that referenced
this pull request
Mar 6, 2024
* Creating withVerification HOC * Updating to use componentDidMount and componentDidUpdate and adding propTypes * Adding tests to withVerification * Adding documentation to withVerification (cherry picked from commit 41139ec)
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.
CATEGORY
Choose one
SUMMARY
We have a large table where not all combinations of values are applicable and we need to have a step that calls an endpoint to verify the dimensions and metrics included in the groupby, filter, and metrics controls. This creates a withVerification HOC that verifies options before passing them to the resulting control.
These controls aren't currently being used in the app, but we'll use them through control overrides in our deployment.
TEST PLAN
Use controlOverrides to set this control for certain chart types
Also in controlOverrides:
Check to make sure validation is working
Check to make sure normal options are present (no validation) when there's no getEndpoint
ADDITIONAL INFORMATION
REVIEWERS
@mistercrunch
cc: @graceguo-supercat @john-bodley