-
Notifications
You must be signed in to change notification settings - Fork 16.5k
fix(welcome): perf on distinct recent activities #32608
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -26,6 +26,7 @@ import { | |
| SupersetTheme, | ||
| getClientErrorObject, | ||
| t, | ||
| lruCache, | ||
| } from '@superset-ui/core'; | ||
| import Chart from 'src/types/Chart'; | ||
| import { intersection } from 'lodash'; | ||
|
|
@@ -34,7 +35,7 @@ import { FetchDataConfig, FilterValue } from 'src/components/ListView'; | |
| import SupersetText from 'src/utils/textUtils'; | ||
| import { findPermission } from 'src/utils/findPermission'; | ||
| import { User } from 'src/types/bootstrapTypes'; | ||
| import { WelcomeTable } from 'src/features/home/types'; | ||
| import { RecentActivity, WelcomeTable } from 'src/features/home/types'; | ||
| import { Dashboard, Filter, TableTab } from './types'; | ||
|
|
||
| // Modifies the rison encoding slightly to match the backend's rison encoding/decoding. Applies globally. | ||
|
|
@@ -223,10 +224,14 @@ export const getRecentActivityObjs = ( | |
| ) => | ||
| SupersetClient.get({ endpoint: recent }).then(recentsRes => { | ||
| const res: any = {}; | ||
| const distinctRes = lruCache<RecentActivity>(6); | ||
| recentsRes.json.result.reverse().forEach((record: RecentActivity) => { | ||
| distinctRes.set(record.item_url, record); | ||
| }); | ||
| return getFilteredChartsandDashboards(addDangerToast, filters).then( | ||
| ({ other }) => { | ||
| res.other = other; | ||
| res.viewed = recentsRes.json.result; | ||
| res.viewed = distinctRes.values().reverse(); | ||
|
Member
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. Is it necessary to reverse twice or can this be optimized?
Member
Author
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. The API returns the latest accesses in reverse chronological order. (i.e. [2025-03-10, 2025-03-09, 2025-03-07, ...]) For an LRU cache, access records need to be stored in chronological order. Therefore, the API list must be reversed to maintain a record of accesses in chronological order. After that, since the final LRU cache list has the most recent entries placed at the end, it needs to be reversed again to position the latest entries at the front. |
||
| return res; | ||
| }, | ||
| ); | ||
|
|
||
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.
Incorrect Chronological Order in Recent Activities
Tell me more
What is the issue?
The original array is being reversed before being processed, which modifies the chronological order of activities before applying the LRU cache.
Why this matters
This can lead to incorrect chronological ordering of recent activities in the UI, as older activities might appear as more recent than they actually are.
Suggested change ∙ Feature Preview
Process the records in their original order to maintain correct chronological sequence:
💬 Looking for more details? Reply to this comment to chat with Korbit.