fix: Address database options prepopulated with stale option, in dataset component #27797
Conversation
…se information from localstorage
jzhao62
changed the title
… so useLocation does not throw error
|
@justinpark add some lines to fix testcases, please take a look |
|
Adding @michael-s-molina as reviewer since he was trying to repro the issue (#27266) a while back. Also, closing/reopening this PR to kick-start the CI process. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #27797 +/- ##
==========================================
+ Coverage 69.69% 69.78% +0.09%
==========================================
Files 1908 1956 +48
Lines 74530 76797 +2267
Branches 8309 8975 +666
==========================================
+ Hits 51942 53594 +1652
- Misses 20535 21004 +469
- Partials 2053 2199 +146
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Woohoo, CI is green! |
| useRedux: true, | ||
| }); | ||
| render( | ||
| <MemoryRouter> |
There was a problem hiding this comment.
render(<LeftPanel setDataset={mockFun} />, {
useRedux: true,
useRouter: true,
});
| @@ -122,6 +123,9 @@ export default function LeftPanel({ | |||
| datasetNames, | |||
| }: LeftPanelProps) { | |||
| const { addDangerToast } = useToasts(); | |||
| const location = useLocation(); | |||
|
|
|||
| const isAddingDataSet = location.pathname.includes('/dataset/add') ?? false; | |||
There was a problem hiding this comment.
This is not the correct way of handling this case. This component receives the dataset as a parameter and it should be the responsibility of the caller to update the dataset accordingly, which will trigger a re-render of this component. If you check the call stack, you'll see the dataset is coming from a hook. We need to invalidate the cached results of that hook if the database changes. @justinpark
There was a problem hiding this comment.
i see, i will check if i can patch a fix on the backend
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
stale database data in localStorage
after fix demo
https://github.com/apache/superset/assets/25937657/8a20d1dc-cdb9-47a2-8cac-7d84b16eaa5a
TESTING INSTRUCTIONS
connect a database of your choice,
rename your database
create a new dataset, and you will see the option not prepopulated.
ADDITIONAL INFORMATION
[ x] Has associated issue: Fix #27266
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in #13351)
Migration is atomic, supports rollback & is backwards-compatible
Confirm DB migration upgrade and downgrade tested
Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API