nytai commented on a change in pull request #11557:
URL:
https://github.com/apache/incubator-superset/pull/11557#discussion_r517611903
##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -32,17 +32,19 @@ interface ListViewResourceState<D extends object = any> {
permissions: string[];
lastFetchDataConfig: FetchDataConfig | null;
bulkSelectEnabled: boolean;
+ minesFetchDataConfig?: FetchDataConfig;
Review comment:
is this prop necessary? I'm not seeing it provided anywhere
##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -164,10 +166,14 @@ export function useListViewResource<D extends object =
any>(
hasPerm,
fetchData,
toggleBulkSelect,
- refreshData: () => {
+ refreshData: (mineConfig: FetchDataConfig | null = null) => {
Review comment:
nits:
- can we name this something more generic, `mineConfig` confused me when I
read it before seeing the lines below, how about `providedConfig` ?
- you can make the arg optional and you won't need the ` | null = null`
portion
##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -164,10 +166,14 @@ export function useListViewResource<D extends object =
any>(
hasPerm,
fetchData,
toggleBulkSelect,
- refreshData: () => {
+ refreshData: (mineConfig: FetchDataConfig | null = null) => {
if (state.lastFetchDataConfig) {
- fetchData(state.lastFetchDataConfig);
+ return fetchData(state.lastFetchDataConfig);
}
+ if (mineConfig) {
+ return fetchData(mineConfig);
+ }
Review comment:
shouldn't `providedConfig`/`mineConfig` take higher precedence than
`state.lastFetchDataConfig`?
##########
File path: superset-frontend/src/views/CRUD/types.ts
##########
@@ -45,6 +46,19 @@ export interface Dashboard {
loading?: boolean;
}
+export interface DashboardCardProps {
Review comment:
I moved this into the DashboardCard component file in
https://github.com/apache/incubator-superset/pull/11502 since I thought it made
sense to keep it next to the component. Can we keep it there? It doesn't appear
we're importing this anywhere else.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
