Skip to content

feat(favorite/recent): implement new favorite/recent feature (with vue query) - #6136

Merged
piggggggggy merged 13 commits into
cloudforet-io:developfrom
piggggggggy:query/recent-favorite
Aug 22, 2025
Merged

piggggggggy merged 13 commits into
cloudforet-io:developfrom
piggggggggy:query/recent-favorite

Conversation

@piggggggggy

Copy link
Copy Markdown
Member

Skip Review (optional)

  • Minor changes that don't affect the functionality (e.g. style, chore, ci, test, docs)
  • Previously reviewed in feature branch, further review is not mandatory
  • Self-merge allowed for solo developers or urgent changes

Description (optional)

SSIA

Things to Talk About (optional)

Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
@vercel

vercel Bot commented Aug 19, 2025 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Preview Comments Updated (UTC)
console Ignored Ignored Preview Aug 22, 2025 7:38am
mfa-saas-qa Ignored Ignored Aug 22, 2025 7:38am
web-storybook Ignored Ignored Aug 22, 2025 7:38am

@github-actions

Copy link
Copy Markdown
Contributor

🎉 @skdud4659 has been randomly selected as the reviewer! Please review. 🙏

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request implements a new favorite/recent feature using Vue Query for better data management and performance. The implementation replaces the existing Vuex store-based favorite and recent functionality with a more modern approach using composables and Vue Query.

  • Refactors favorite and recent data management from Vuex stores to Vue Query-based composables
  • Introduces new config-data conversion utilities for better data transformation
  • Adds comprehensive recent list management with type-specific queries

Reviewed Changes

Copilot reviewed 37 out of 37 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/language-pack/*.json Adds new translation key "ALL_METRIC" and fixes typos in existing translations
apps/web/src/services/workspace-home/components/*.vue Updates to use new favorite/recent composables instead of store
apps/web/src/services/landing/components/*.vue Migrates landing page components to new workspace favorite system
apps/web/src/common/modules/favorites/core/*.ts New favorite management composables using Vue Query
apps/web/src/common/modules/recents/*.ts New recent list management with CRUD operations
apps/web/src/common/composables/config-data/*.ts New config data conversion utilities
apps/web/src/query/resource-query/shared/composable/*.ts Updates resource cache sync functionality
Comments suppressed due to low confidence (2)

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

projects: computed<ProjectReferenceMap>(() => allReferenceGetters.project),
projectGroups: computed<ProjectGroupReferenceMap>(() => allReferenceGetters.projectGroup),
recentList: computed<UserConfigModel[]>(() => workspaceHomePageState.recentList),
// recentList: computed<UserConfigModel[]>(() => workspaceHomePageState.recentList),

Copilot AI Aug 19, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the commented-out code as it's no longer needed and creates clutter in the codebase.

Suggested change
// recentList: computed<UserConfigModel[]>(() => workspaceHomePageState.recentList),

Copilot uses AI. Check for mistakes.
await _updateResourceCache(resourceType, res as Obj, queryClient);
return res;
};
// TODO: annotaion

Copilot AI Aug 19, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a typo in the TODO comment: 'annotaion' should be 'annotation'.

Suggested change
// TODO: annotaion
// TODO: annotation

Copilot uses AI. Check for mistakes.
type: RecentType;
workspaceId: string;
id: string;
options?: {[key:string]: any};

Copilot AI Aug 19, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Add a space after the colon in the object type definition: '{[key: string]: any}' for better readability and consistency with TypeScript style conventions.

Suggested change
options?: {[key:string]: any};
options?: { [key: string]: any };

Copilot uses AI. Check for mistakes.
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>

@skdud4659 skdud4659 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx!

@sulmoJ sulmoJ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.
I left some comments.

Comment on lines +23 to +32
return useScopedQuery({
queryKey: key,
queryFn: () => userConfigAPI.list(queryParams.value),
enabled: computed(() => {
if (enabled === undefined) {
return true;
}
return enabled.value;
}),
}, ['WORKSPACE', 'USER']);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Why doesn't UserConfig have a 'stale time'?

if (!dataSourceIds.value?.length) return;
loading.value = true;
const promises = dataSourceIds.value.map((dataSourceId) => costQuerySetFetcher(dataSourceId));
const results = await Promise.all(promises);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Shouldn't you use allSettled?


return {
map: computed(() => {
const workspaceMap = new Map<string, WorkspaceModel>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: Is it okay for map objects to be placed in a computed?

workspaceConfigList?: ComputedRef<ConfigData[]>;
}

export const useConvertReferencedConfigData = ({

@sulmoJ sulmoJ Aug 20, 2025 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: I couldn't understand that useConvertReferencedConfigData is a composable.
Could it be a composable that provides the recent and favorite config lists all at once for each resource?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: The return type is missing.

Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
Signed-off-by: samuel.park <samuel.park@megazone.com>
@piggggggggy
piggggggggy merged commit 300a631 into cloudforet-io:develop Aug 22, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants