fix: clear modal state after adding dataset #17044
Conversation
| @@ -176,13 +169,22 @@ const TableSelector: FunctionComponent<TableSelectorProps> = ({ | |||
| const [tableOptions, setTableOptions] = useState<TableOption[]>([]); | |||
|
|
|||
| useEffect(() => { | |||
| if (currentDbId && currentSchema) { | |||
| // reset selections | |||
| if (database === undefined) { | |||
There was a problem hiding this comment.
could this ever be null? Do you want to check a more generic falsy value iow?
There was a problem hiding this comment.
Good point, I'll check for any falsy values.
| @@ -61,15 +64,11 @@ const DatasetModal: FunctionComponent<DatasetModalProps> = ({ | |||
| ); | |||
|
|
|||
| useEffect(() => { | |||
| setDisableSave(isNil(datasourceId) || isEmpty(currentTableName)); | |||
| }, [currentTableName, datasourceId]); | |||
| setDisableSave(isNil(currentDatabase) || isEmpty(currentTableName)); | |||
There was a problem hiding this comment.
should this be isEmpty(currentDatabase)?
There was a problem hiding this comment.
We can get rid of lodash here, let me do it.
| const onSave = () => { | ||
| const data = { | ||
| database: datasourceId, | ||
| database: currentDatabase?.id || 0, |
There was a problem hiding this comment.
what does setting the default to 0 do in this case, rather than undefined or null?
There was a problem hiding this comment.
Let me clean this up... here currentDatabase will always be defined, otherwise you can't save.
Codecov Report
@@ Coverage Diff @@
## master #17044 +/- ##
==========================================
- Coverage 76.92% 76.91% -0.02%
==========================================
Files 1031 1031
Lines 55157 55176 +19
Branches 7501 7503 +2
==========================================
+ Hits 42430 42437 +7
- Misses 12475 12487 +12
Partials 252 252
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
🏷️ 2021.40 |
* fix: clear modal state after adding dataset * Fix test * Small fixes (cherry picked from commit 16a1df7)
* fix: clear modal state after adding dataset * Fix test * Small fixes (cherry picked from commit 16a1df7)
* fix: clear modal state after adding dataset * Fix test * Small fixes
* fix: clear modal state after adding dataset * Fix test * Small fixes
mistercrunch
added
🍒 1.4.0
🍒 1.4.1
🍒 1.4.2
🏷️ bot
SUMMARY
This PR fixes a bug where after adding a dataset the modal would not reset properly. For example, adding if a user added,
database.schema.table, when they tried to add another dataset the model would be already populated withdatabase,schemaandtableon the dropdowns.While troubleshooting the problem I also cleaned up the code.
<TableSelector>would take as props both adatabaseobject as well as a database ID. The logic was not correct so they were falling out of sync. I modified the component to take only a database object.I also consolidated a database type that was used in slightly different ways — the only difference was that one had
allow_multi_schema_metadata_fetchand the other didn't.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
modal.add.dataset.mov
TESTING INSTRUCTIONS
Add a dataset; add another. Modal should be empty when it loads.
ADDITIONAL INFORMATION