-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[discover] Less field list loading #147825
Changes from all commits
231ba16
0dc8292
a4ee983
0cd7c17
2be0396
73636cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,46 +34,6 @@ interface DataViewData { | |
stateValFound: boolean; | ||
} | ||
|
||
export function findDataViewById( | ||
dataViews: DataViewListItem[], | ||
id: string | ||
): DataViewListItem | undefined { | ||
if (!Array.isArray(dataViews) || !id) { | ||
return; | ||
} | ||
return dataViews.find((o) => o.id === id); | ||
} | ||
|
||
/** | ||
* Checks if the given defaultIndex exists and returns | ||
* the first available data view id if not | ||
*/ | ||
export function getFallbackDataViewId( | ||
dataViews: DataViewListItem[], | ||
defaultIndex: string = '' | ||
): string { | ||
if (defaultIndex && findDataViewById(dataViews, defaultIndex)) { | ||
return defaultIndex; | ||
} | ||
return dataViews && dataViews[0]?.id ? dataViews[0].id : ''; | ||
} | ||
|
||
/** | ||
* A given data view id is checked for existence and a fallback is provided if it doesn't exist | ||
* The provided defaultIndex is usually configured in Advanced Settings, if it's also invalid | ||
* the first entry of the given list of dataViews is used | ||
*/ | ||
export function getDataViewId( | ||
id: string = '', | ||
dataViews: DataViewListItem[] = [], | ||
defaultIndex: string = '' | ||
): string { | ||
if (!id || !findDataViewById(dataViews, id)) { | ||
return getFallbackDataViewId(dataViews, defaultIndex); | ||
} | ||
return id; | ||
} | ||
|
||
/** | ||
* Function to load the given data view by id, providing a fallback if it doesn't exist | ||
*/ | ||
|
@@ -104,9 +64,10 @@ export async function loadDataView( | |
fetchId = dataViewSpec.id!; | ||
} | ||
|
||
let fetchedDataView: DataView | undefined; | ||
// try to fetch adhoc data view first | ||
try { | ||
const fetchedDataView = fetchId ? await dataViews.get(fetchId) : undefined; | ||
fetchedDataView = fetchId ? await dataViews.get(fetchId) : undefined; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously this code was loading a data view, checking if it was adhoc, then tossing the result. Now we hold on to it in case its the data view we wanted to load. |
||
if (fetchedDataView && !fetchedDataView.isPersisted()) { | ||
return { | ||
list: dataViewList || [], | ||
|
@@ -122,12 +83,14 @@ export async function loadDataView( | |
} catch (e) {} | ||
|
||
// fetch persisted data view | ||
const actualId = getDataViewId(fetchId, dataViewList, config.get('defaultIndex')); | ||
return { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need |
||
list: dataViewList || [], | ||
loaded: await dataViews.get(actualId), | ||
loaded: fetchedDataView | ||
? fetchedDataView | ||
: // we can be certain that the data view exists due to an earlier hasData check | ||
((await dataViews.getDefaultDataView()) as DataView), | ||
stateVal: fetchId, | ||
stateValFound: !!fetchId && actualId === fetchId, | ||
stateValFound: !!fetchId && !!fetchedDataView, | ||
}; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This functionality duplicates what
dataViews.hasData
is doing. If it doesn't work, it should be fixed.