korbit-ai[bot] commented on code in PR #32608:
URL: https://github.com/apache/superset/pull/32608#discussion_r1990298646
##########
superset-frontend/packages/superset-ui-core/src/utils/lruCache.ts:
##########
@@ -67,6 +67,10 @@ class LRUCache<T> {
public get size() {
return this.cache.size;
}
+
+ public entries(): T[] {
+ return [...this.cache.values()];
+ }
Review Comment:
### Misleading Method Name <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The entries() method name is misleading as it returns only values, not
entries (key-value pairs) as is standard in JavaScript/TypeScript.
###### Why this matters
This can lead to confusion and bugs as developers expect entries() to return
key-value pairs based on standard JavaScript Map/Set behavior.
###### Suggested change ∙ *Feature Preview*
Either rename the method to values() to match its actual behavior or modify
it to return entries:
```typescript
public entries(): [string, T][] {
return [...this.cache.entries()];
}
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9f6aba30-dfbc-48be-a6bd-164209d2a014?suggestedFixEnabled=true)
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:c6554d10-a090-43ac-b197-01c033a34065 -->
##########
superset-frontend/src/views/CRUD/utils.tsx:
##########
@@ -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);
+ });
Review Comment:
### Incorrect Chronological Order in Recent Activities <sub></sub>
<details>
<summary>Tell me more</summary>
###### 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:
```typescript
recentsRes.json.result.forEach((record: RecentActivity) => {
distinctRes.set(record.item_url, record);
});
```
</details>
<sub>
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ddc1346b-0ff3-4f40-920d-a4e0c331b4a5?suggestedFixEnabled=true)
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:e78762da-335f-4900-8893-7f8cc66b6756 -->
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
