-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat: add favorite applications #41462
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
base: release
Are you sure you want to change the base?
Conversation
Fixed FavitoristesSagas to put in the generic directory
…-push hook blocking The favorite actions are now in src/actions/applicationActions and are imported directly, so we don't need to re-export them from ee/actions/applicationActions.ts
WalkthroughThis pull request introduces a complete favorites feature for applications, enabling users to favorite/unfavorite applications and maintain a persistent list. Changes span Redux action creators, API integrations, state management reducers, sagas for side effects, UI components for favoriting interactions, and backend endpoints with database persistence. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Application Card
participant Redux as Redux<br/>(Actions/Dispatch)
participant Saga as Favorites Saga
participant API as ApplicationApi
participant Server as Backend<br/>(Controller/Service)
participant DB as Database
User->>UI: Click favorite icon
UI->>Redux: dispatch(toggleFavoriteApplication)
Redux->>Saga: TOGGLE_FAVORITE_APPLICATION_INIT
Note over Saga: Capture original state
Saga->>Redux: dispatch(toggleFavoriteApplicationSuccess)<br/>[Optimistic Update]
Redux->>UI: Update isFavorited flag
UI->>User: Star icon updates immediately
par Async API Call
Saga->>API: ApplicationApi.toggleFavoriteApplication(id)
API->>Server: PUT /applications/{id}/favorite
Server->>DB: Update UserData.favoriteApplicationIds
DB-->>Server: Success
Server-->>API: ResponseDTO<UserData>
end
alt API Success
Saga->>Redux: Action received (success confirmed)
else API Failure
Saga->>Redux: dispatch(toggleFavoriteApplicationError)
Saga->>Redux: dispatch(toggleFavoriteApplicationSuccess)<br/>[Rollback to original state]
Saga->>UI: Show toast error
end
sequenceDiagram
participant App as App Initialization
participant Saga as WorkspaceSagas
participant Redux as Redux<br/>(Favorites State)
participant FavSaga as FavoritesSagas
participant API as ApplicationApi
participant Server as Backend
App->>Saga: User loads applications page
Saga->>Saga: fetchAllWorkspacesSaga
Saga->>Redux: dispatch(FETCH_ALL_WORKSPACES_SUCCESS)
Saga->>Redux: dispatch(FETCH_FAVORITE_APPLICATIONS_INIT)
rect rgb(200, 220, 255)
Note over FavSaga: Fetch Favorites Flow
FavSaga->>API: ApplicationApi.getFavoriteApplications()
API->>Server: GET /favoriteApplications
Server->>Server: Filter by user permissions
Server->>Server: Cleanup inaccessible entries
Server-->>API: List<Application>
end
FavSaga->>Redux: dispatch(FETCH_FAVORITE_APPLICATIONS_SUCCESS)<br/>[with merged permissions]
Redux->>Redux: Update favoriteApplicationIds & applicationList
rect rgb(255, 240, 200)
Note over Redux: Virtual Favorites Workspace
Redux->>Redux: Applications now enriched with isFavorited
Redux->>Redux: Inject __favorites__ workspace if hasFavorites
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Failed server tests
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20201950013. |
|
Deploy-Preview-URL: https://ce-41462.dp.appsmith.com |
|
|
Regarding number 3 on the route, I disagree and am keeping it this way. I had purposefully made it not show edit but will make that change. I have corrected the 50 limit. |
|
@salevine, Still facing the Home page bug. On refreshing the page of a particular workspace, it shows Apps from the favorites instead of the Apps belonging to the particular workspace. Screen.Recording.2025-12-15.at.8.40.40.AM.mov |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20221639173. |
Failed server tests
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20239685411. |
Failed server tests
|
…ss yarn check types
… handling on loading
Failed server tests
|
1 similar comment
Failed server tests
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20247455203. |
|
Deploy-Preview-URL: https://ce-41462.dp.appsmith.com |
Failed server tests
|
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.
Actionable comments posted: 2
🧹 Nitpick comments (8)
app/client/src/pages/AppIDE/layouts/components/Header/index.tsx (1)
165-168: LGTM!The query parameter approach correctly ties the header navigation to the workspace context.
Consider URL-encoding the
workspaceIdif workspace IDs might contain special characters:<Link className="mr-1.5" - to={`${APPLICATIONS_URL}?workspaceId=${currentWorkspace.id}`} + to={`${APPLICATIONS_URL}?workspaceId=${encodeURIComponent(currentWorkspace.id)}`} >app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserDataRepositoryCEImpl.java (1)
49-55: LGTM!The bulk update operation correctly uses
BridgeUpdate.pullto remove the application from all users'favoriteApplicationIdsarrays.Consider adding logging for this bulk operation to aid operational observability:
@Override public Mono<Void> removeApplicationFromFavorites(String applicationId) { // MongoDB update query to pull applicationId from all users' favoriteApplicationIds arrays BridgeUpdate update = new BridgeUpdate(); update.pull(UserData.Fields.favoriteApplicationIds, applicationId); - return queryBuilder().updateAll(update).then(); + return queryBuilder().updateAll(update) + .doOnSuccess(count -> log.debug("Removed application {} from {} users' favorites", applicationId, count)) + .then(); }app/client/src/ce/sagas/WorkspaceSagas.ts (2)
65-67: Potential duplicate favorites fetch.
FETCH_FAVORITE_APPLICATIONS_INITis dispatched here afterFETCH_ALL_WORKSPACES_SUCCESS, and also at line 101 when handling the__favorites__virtual workspace. IffetchEntitiesOfWorkspaceSagais called via line 70 withworkspaceId === "__favorites__", favorites will be fetched twice in quick succession.Consider guarding the dispatch at line 101 or ensuring the saga handles deduplication.
89-104: Extract magic string to a shared constant.The
"__favorites__"identifier is used here and likely in other parts of the codebase (routes, selectors). Extract it to a shared constant to avoid typos and improve maintainability.+// In a constants file, e.g., constants/workspaceConstants.ts +export const FAVORITES_WORKSPACE_ID = "__favorites__"; // In this file +import { FAVORITES_WORKSPACE_ID } from "constants/workspaceConstants"; + // Handle virtual favorites workspace specially -if (workspaceId === "__favorites__") { +if (workspaceId === FAVORITES_WORKSPACE_ID) { yield put({ type: ReduxActionTypes.SET_CURRENT_WORKSPACE, payload: { - id: "__favorites__", + id: FAVORITES_WORKSPACE_ID, name: "Favorites", isVirtual: true, userPermissions: [], }, });app/client/src/ce/selectors/selectedWorkspaceSelectors.ts (1)
10-11: Potential memoization instability.The
|| []fallback creates a new array reference whenfavoriteApplicationIdsis undefined, which could break memoization and cause unnecessary re-renders.+const EMPTY_ARRAY: string[] = []; + const selectFavoriteApplicationIds = (state: DefaultRootState) => - state.ui.applications.favoriteApplicationIds || []; + state.ui.applications.favoriteApplicationIds ?? EMPTY_ARRAY;app/client/src/components/common/Card.tsx (1)
417-432: InlinepointerEventsstyle duplicates existing CSS logic.The
Wrapperstyled-component (line 241) already handlespointer-events: nonefor!hasReadPermission. These inline styles may conflict or create maintenance overhead.Consider consolidating this logic in the styled-components to avoid duplication:
- <div - className="overlay" - // Only enable pointer events when the user has read permission. - // For users without read access, keep the overlay non-interactive. - style={hasReadPermission ? { pointerEvents: "auto" } : undefined} - > + <div className="overlay">The existing CSS in
Wrapperat line 241 already handlespointer-events: nonewhen!hasReadPermission.app/client/src/pages/Applications/ApplicationCard.tsx (1)
177-212: Pre-existing bug: stale closure in useEffect with empty dependency array.This existing code mutates
moreActionItemsand referenceshasEditPermission,hasExportPermission,hasDeletePermissionwhich are derived fromeffectiveUserPermissions. The empty[]dependency means the effect only runs once, potentially with stale permission values.Not introduced by this PR, but the new permission fallback logic makes this more apparent.
Consider refactoring to use proper dependencies or restructuring to avoid the stale closure issue in a follow-up.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java (1)
444-462: List mutation may cause subtle bugs.At lines 444-446, if
favoritesis null, a newArrayListis created. However, iffavoritesis not null, the existing list reference fromuserData.getFavoriteApplicationIds()is used directly and mutated in place (lines 451, 461).Depending on the implementation of
UserData, this might mutate a shared or cached reference. To avoid side effects, consider creating a new list:- List<String> favorites = userData.getFavoriteApplicationIds(); - if (favorites == null) { - favorites = new ArrayList<>(); - } + List<String> favorites = userData.getFavoriteApplicationIds() != null + ? new ArrayList<>(userData.getFavoriteApplicationIds()) + : new ArrayList<>();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/client/src/assets/icons/ads/heart-fill-red.svgis excluded by!**/*.svg
📒 Files selected for processing (25)
app/client/src/actions/applicationActions.ts(1 hunks)app/client/src/ce/api/ApplicationApi.tsx(1 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(2 hunks)app/client/src/ce/constants/workspaceConstants.ts(1 hunks)app/client/src/ce/pages/Applications/index.tsx(4 hunks)app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx(3 hunks)app/client/src/ce/reducers/uiReducers/selectedWorkspaceReducer.ts(2 hunks)app/client/src/ce/sagas/WorkspaceSagas.ts(3 hunks)app/client/src/ce/sagas/index.tsx(2 hunks)app/client/src/ce/selectors/selectedWorkspaceSelectors.ts(1 hunks)app/client/src/components/common/Card.tsx(6 hunks)app/client/src/components/editorComponents/Debugger/index.tsx(1 hunks)app/client/src/entities/Application/types.ts(1 hunks)app/client/src/pages/AppIDE/layouts/components/Header/index.tsx(1 hunks)app/client/src/pages/AppIDE/layouts/hooks/useWidgetSelectionBlockListener.ts(1 hunks)app/client/src/pages/Applications/ApplicationCard.tsx(6 hunks)app/client/src/sagas/FavoritesSagas.ts(1 hunks)app/client/src/selectors/favoriteSelectors.ts(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/UserControllerCE.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserDataRepositoryCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserDataRepositoryCEImpl.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCE.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java(8 hunks)
🧰 Additional context used
🧠 Learnings (19)
📚 Learning: 2025-02-20T13:36:10.743Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 39379
File: app/client/src/pages/AppIDE/components/LibrariesList/JSLibrariesSection.tsx:0-0
Timestamp: 2025-02-20T13:36:10.743Z
Learning: When reviewing React components, ensure that useMemo and useEffect hooks include all variables used within their callbacks in their dependency arrays to prevent stale values and potential bugs.
Applied to files:
app/client/src/components/editorComponents/Debugger/index.tsx
📚 Learning: 2024-12-11T08:25:39.197Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38088
File: app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts:40-43
Timestamp: 2024-12-11T08:25:39.197Z
Learning: In `app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts`, the `useMemo` hook includes dependencies `artifactType` and `baseArtifactId` in its dependency array.
Applied to files:
app/client/src/components/editorComponents/Debugger/index.tsx
📚 Learning: 2024-12-11T08:33:24.352Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38088
File: app/client/src/git/components/GitQuickActions/BranchButton/index.tsx:72-74
Timestamp: 2024-12-11T08:33:24.352Z
Learning: In the 'BranchButton' component in 'app/client/src/git/components/GitQuickActions/BranchButton/index.tsx' (TypeScript, React), the `useEffect` hook that checks for label ellipsis does not need to include `currentBranch` in its dependency array.
Applied to files:
app/client/src/components/editorComponents/Debugger/index.tsxapp/client/src/pages/AppIDE/layouts/hooks/useWidgetSelectionBlockListener.ts
📚 Learning: 2025-10-30T07:17:49.646Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx:817-883
Timestamp: 2025-10-30T07:17:49.646Z
Learning: In app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx, the isErrorPersistingAppSlug flag is unused and should be removed. There is no corresponding selector to expose it, and no handlers set it to true.
Applied to files:
app/client/src/selectors/favoriteSelectors.tsapp/client/src/ce/reducers/uiReducers/applicationsReducer.tsxapp/client/src/pages/Applications/ApplicationCard.tsxapp/client/src/ce/selectors/selectedWorkspaceSelectors.tsapp/client/src/ce/reducers/uiReducers/selectedWorkspaceReducer.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: nidhi-nair
Repo: appsmithorg/appsmith PR: 29372
File: app/server/appsmith-server/src/main/java/com/appsmith/server/applications/jslibs/ApplicationJsLibServiceCEImpl.java:24-24
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The change from `private` to `protected` for the `applicationService` field in `ApplicationJsLibServiceCEImpl` class was confirmed to be intentional by the user nidhi-nair for subclass access.
Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.javaapp/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCE.java
📚 Learning: 2024-12-25T06:50:40.623Z
Learnt from: trishaanand
Repo: appsmithorg/appsmith PR: 38359
File: app/server/appsmith-server/src/main/java/com/appsmith/server/authentication/handlers/ce/CustomOAuth2UserServiceCEImpl.java:78-82
Timestamp: 2024-12-25T06:50:40.623Z
Learning: In `CustomOAuth2UserServiceCEImpl`, the second repository call for finding users by case-insensitive email is needed because older users might have uppercase or mixed-case emails, while new users have emails stored in lowercase.
Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.java
📚 Learning: 2024-12-10T10:52:38.873Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:8-13
Timestamp: 2024-12-10T10:52:38.873Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts` and similar Git sagas, error handling for `baseArtifactId` is managed outside the scope, so validation checks for `baseArtifactId` within the saga functions are unnecessary.
Applied to files:
app/client/src/ce/sagas/WorkspaceSagas.tsapp/client/src/ce/sagas/index.tsx
📚 Learning: 2024-12-16T19:43:14.764Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38171
File: app/client/src/git/components/GitContextProvider/index.tsx:60-70
Timestamp: 2024-12-16T19:43:14.764Z
Learning: Prefer not to throw errors when dispatching actions in the `setImportWorkspaceId` function in `app/client/src/git/components/GitContextProvider/index.tsx`.
Applied to files:
app/client/src/ce/sagas/WorkspaceSagas.ts
📚 Learning: 2024-12-10T10:52:38.244Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38060
File: app/client/src/git/sagas/fetchLocalProfileSaga.ts:28-34
Timestamp: 2024-12-10T10:52:38.244Z
Learning: In `app/client/src/git/sagas/fetchLocalProfileSaga.ts`, error handling is managed outside the scope, so casting errors directly to strings is acceptable.
Applied to files:
app/client/src/ce/sagas/WorkspaceSagas.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: riodeuno
Repo: appsmithorg/appsmith PR: 33062
File: app/client/src/widgets/anvil/ZoneWidget/widget/config/baseConfig.ts:10-13
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In the `app/client/src/widgets/anvil/ZoneWidget/widget/config/baseConfig.ts` file, the swapping of focus and selection colors for background and foreground is intentional to achieve a specific visual effect.
Applied to files:
app/client/src/pages/AppIDE/layouts/hooks/useWidgetSelectionBlockListener.ts
📚 Learning: 2024-10-24T08:38:20.429Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 37056
File: app/client/src/pages/Editor/JSEditor/JSObjectNameEditor/JSObjectNameEditor.tsx:22-25
Timestamp: 2024-10-24T08:38:20.429Z
Learning: "constants/AppConstants" does not export "SaveActionNameParams".
Applied to files:
app/client/src/pages/Applications/ApplicationCard.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 33602
File: app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/AppsmithRepository.java:29-29
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The codebase has mechanisms to gracefully handle `null` values, making the use of `Optional` for `AclPermission` unnecessary in certain contexts.
Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 33602
File: app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/BaseAppsmithRepositoryCEImpl.java:462-473
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The codebase has mechanisms to gracefully handle `null` values, making the use of `Optional` for `AclPermission` unnecessary in certain contexts. This is leveraged by replacing `Optional.empty()` with `null` in method signatures where `null` is already handled gracefully.
Applied to files:
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java
📚 Learning: 2024-12-14T22:24:11.946Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38155
File: app/client/src/git/store/selectors/gitConfigSelectors.ts:7-9
Timestamp: 2024-12-14T22:24:11.946Z
Learning: The selector `selectFetchGlobalProfileState` in `gitConfigSelectors.ts` is already memoized.
Applied to files:
app/client/src/ce/selectors/selectedWorkspaceSelectors.ts
📚 Learning: 2024-12-16T19:47:33.107Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38171
File: app/client/src/git/ce/sagas/index.ts:3-8
Timestamp: 2024-12-16T19:47:33.107Z
Learning: When adding actions to `blockingActionSagas` and `nonBlockingActionSagas`, using more specific generic types can lead to TypeScript errors. Therefore, it's acceptable to use `any` for the action payload types in these registries.
Applied to files:
app/client/src/ce/sagas/index.tsx
📚 Learning: 2024-11-12T07:37:42.598Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 37330
File: app/client/src/pages/Editor/gitSync/components/GitChangesList/StaticChange.tsx:52-52
Timestamp: 2024-11-12T07:37:42.598Z
Learning: The icon provider components in `app/client/packages/design-system/widgets-old/src/Icon/index.tsx` and `app/client/packages/design-system/ads/src/Icon/Icon.provider.tsx` should provide both `settings-2-line` and `settings-v3` icons and should not be updated to remove the old icon.
Applied to files:
app/client/src/components/common/Card.tsx
📚 Learning: 2024-11-12T11:42:28.998Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 37330
File: app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx:95-95
Timestamp: 2024-11-12T11:42:28.998Z
Learning: In icon provider components within the TypeScript/React codebase, old settings icons like `"settings-2-line"` and `"settings-control"` are intentionally provided alongside new icons. These references are acceptable and should not be flagged for updates.
Applied to files:
app/client/src/components/common/Card.tsx
📚 Learning: 2025-04-28T16:16:02.155Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 40462
File: app/client/src/git/components/ImportOverrideModal/ImportOverrideModalView.tsx:34-40
Timestamp: 2025-04-28T16:16:02.155Z
Learning: The Appsmith team prefers not to include HTML markup in string constants. Text styling or emphasis should be handled through proper React components rather than HTML tags in strings.
Applied to files:
app/client/src/components/common/Card.tsx
📚 Learning: 2024-10-21T13:17:59.251Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36989
File: app/client/packages/git/src/actions/helpers/createSingleArtifactAction.ts:52-68
Timestamp: 2024-10-21T13:17:59.251Z
Learning: In this codebase, when using Redux Toolkit's createSlice from Redux Toolkit, we can directly mutate the state within reducers since Immer handles immutability internally.
Applied to files:
app/client/src/ce/reducers/uiReducers/selectedWorkspaceReducer.ts
🧬 Code graph analysis (12)
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserDataRepositoryCEImpl.java (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java (1)
Fields(107-107)
app/client/src/components/editorComponents/Debugger/index.tsx (1)
app/client/src/actions/debuggerActions.ts (1)
setErrorCount(105-110)
app/client/src/sagas/FavoritesSagas.ts (5)
app/client/src/actions/applicationActions.ts (4)
toggleFavoriteApplicationSuccess(12-18)toggleFavoriteApplicationError(20-23)fetchFavoriteApplicationsSuccess(29-34)fetchFavoriteApplicationsError(36-38)app/client/src/ce/api/ApplicationApi.tsx (1)
ApplicationApi(291-577)app/client/src/sagas/ErrorSagas.tsx (1)
validateResponse(100-155)app/client/src/entities/Application/types.ts (1)
ApplicationPayload(12-54)app/client/src/pages/utils.ts (1)
findDefaultPage(33-37)
app/client/src/pages/AppIDE/layouts/components/Header/index.tsx (1)
app/client/src/constants/routes/baseRoutes.ts (1)
APPLICATIONS_URL(8-8)
app/client/src/actions/applicationActions.ts (2)
app/client/src/ce/constants/ReduxActionConstants.tsx (2)
ReduxActionTypes(1331-1376)ReduxActionErrorTypes(1378-1405)app/client/src/entities/Application/types.ts (1)
ApplicationPayload(12-54)
app/client/src/ce/pages/Applications/index.tsx (1)
app/client/src/selectors/favoriteSelectors.ts (1)
getHasFavorites(24-27)
app/client/src/ce/sagas/WorkspaceSagas.ts (1)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1331-1376)
app/client/src/pages/AppIDE/layouts/hooks/useWidgetSelectionBlockListener.ts (1)
app/client/src/actions/widgetActions.tsx (1)
setWidgetSelectionBlock(166-171)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCE.java (2)
app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx (1)
Application(977-988)app/client/src/ce/api/ApplicationApi.tsx (2)
toggleFavoriteApplication(568-572)getFavoriteApplications(574-576)
app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx (3)
app/client/src/ce/constants/ReduxActionConstants.tsx (2)
ReduxActionTypes(1331-1376)ReduxActionErrorTypes(1378-1405)app/client/src/actions/ReduxActionTypes.ts (1)
ReduxAction(9-12)app/client/src/entities/Application/types.ts (1)
ApplicationPayload(12-54)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java (2)
app/client/src/ce/api/ApplicationApi.tsx (2)
toggleFavoriteApplication(568-572)getFavoriteApplications(574-576)app/client/src/selectors/favoriteSelectors.ts (1)
getFavoriteApplicationIds(5-6)
app/client/src/ce/sagas/index.tsx (1)
app/client/src/sagas/FavoritesSagas.ts (1)
favoritesSagasListener(110-119)
🔇 Additional comments (25)
app/client/src/pages/AppIDE/layouts/hooks/useWidgetSelectionBlockListener.ts (1)
22-26: LGTM — Valid performance optimization.Narrowing the dependency to
currentFocus.entityis correct since the effect only uses that property. The comment clearly explains the rationale. This prevents unnecessary re-dispatches whencurrentFocusobject reference changes but theentityfield remains the same.Note: This change appears unrelated to the PR's favorites feature; if it's an incidental refactor, consider moving it to a separate PR for cleaner change tracking.
app/client/src/entities/Application/types.ts (1)
49-49: LGTM!The optional
isFavoritedfield is correctly typed and integrates well with the existing interface structure.app/client/src/ce/constants/workspaceConstants.ts (1)
16-16: LGTM!The
isVirtualfield appropriately extends the Workspace interface to support virtual workspace concepts like Favorites.app/client/src/components/editorComponents/Debugger/index.tsx (1)
20-24: LGTM!The
useEffecthook correctly includes all dependencies (dispatchandmessageCounters.errors) and appropriately synchronizes the global error count. The comment clearly explains the intent.app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserDataServiceImpl.java (1)
7-7: LGTM!The constructor correctly wires the
ApplicationPermissiondependency and passes it to the superclass, following standard Spring DI patterns.Also applies to: 25-25, 38-38
app/client/src/ce/sagas/index.tsx (1)
7-7: LGTM!The
favoritesSagasListeneris correctly imported and registered, following the established pattern for saga registration in this codebase.Also applies to: 119-119
app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java (1)
66-68: Field definition is correct and properly annotated.The
favoriteApplicationIdsfield is correctly set to@JsonView(Views.Public.class). The 50-application favorite limit is enforced inUserDataServiceCEImpl.toggleFavoriteApplication()via theMAX_FAVORITE_APPLICATIONS_LIMITconstant, which throws anAppsmithExceptionwhen the limit is exceeded.app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserDataRepositoryCE.java (1)
14-15: LGTM!Clean interface addition for removing an application from all users' favorites. The
Mono<Void>return type is appropriate for this bulk cleanup operation.app/client/src/ce/sagas/WorkspaceSagas.ts (1)
362-374: LGTM!Good fix for ensuring the UI reflects the newly created workspace instead of staying on the previous (e.g., Favorites) virtual workspace. The comment clearly explains the intent.
app/client/src/ce/api/ApplicationApi.tsx (1)
567-576: LGTM!New API methods for favorites functionality are clean and consistent with existing patterns. The endpoints correctly point to the user controller paths (
v1/users/...).app/client/src/ce/selectors/selectedWorkspaceSelectors.ts (1)
13-23: Good use of memoization.The selector correctly uses
createSelectorto avoid unnecessary re-renders. Given the 50-favorite limit, the.includes()lookup is acceptable. For future scalability, consider convertingfavoriteApplicationIdsto a Set within the selector.app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/UserControllerCE.java (2)
224-234: LGTM!Clean endpoint implementation. Given the 50-favorite limit, returning full
Applicationobjects without pagination is acceptable.
210-222: The endpoint implementation is solid. The service layer properly enforces the 50-favorite limit with appropriate error handling—returning a user-friendly exception message when the limit is reached. The logic correctly allows removal anytime but prevents additions once the limit is hit. No changes needed.app/client/src/components/common/Card.tsx (2)
110-131: LGTM! Clean implementation of the favorite icon wrapper.The styling is well-organized with appropriate transitions and hover effects. The absolute positioning at top-left with a semi-transparent background provides good visibility without obstructing the card content.
395-405: LGTM! Proper event handling for favorite toggle.Stopping propagation is correct to prevent the card's primary action from firing when clicking the favorite icon.
app/client/src/ce/pages/Applications/index.tsx (2)
488-504: LGTM! Clean rendering for favorites workspace item.The special handling for
__favorites__workspace with the heart icon is well-implemented and follows the existing patterns for workspace items with logos.
38-43: LGTM! Imports for favorites feature.app/client/src/pages/Applications/ApplicationCard.tsx (3)
216-238: LGTM! Smart fallback for permissions in favorites view.This ensures users see appropriate actions (Edit, Delete, etc.) even when viewing applications through the Favorites workspace where
userPermissionsmay not be directly populated.
536-542: LGTM! Clean implementation of favorite toggle handler.Proper memoization with correct dependencies and event propagation handling.
553-557: LGTM! Props correctly passed to Card component.app/client/src/sagas/FavoritesSagas.ts (3)
16-55: LGTM! Well-implemented optimistic update with proper rollback.The approach of capturing
isFavoritedbefore the optimistic update ensures accurate rollback on failure. Error handling is comprehensive with both state rollback and user notification.
57-108: LGTM! Good defensive merging of userPermissions.Merging permissions from the main application list ensures favorites behave consistently with workspace views.
One minor note: the fetch error path is silent (no toast). If this is intentional for UX reasons, consider adding a comment. Otherwise, users won't know if fetch failed.
110-118: LGTM! Appropriate use oftakeLatestfor both sagas.This ensures only the most recent request is processed, avoiding race conditions when users rapidly toggle favorites or trigger multiple fetches.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCE.java (1)
56-61: LGTM! Well-defined interface methods for favorites management.The three methods cover the necessary operations: toggle, fetch, and cleanup. Return types are appropriate for each operation. The 50-favorite limit is properly enforced in the implementation—
toggleFavoriteApplicationchecksMAX_FAVORITE_APPLICATIONS_LIMITbefore adding and throws an error if the limit is reached.app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java (1)
454-460: Verify consistent enforcement of the 50-favorite limit across all modification paths.The visible limit check is correctly structured, but the reported ability to exceed 50 favorites (per PR comments) suggests enforcement gaps elsewhere. Search for all code paths that modify
favoriteApplicationIdsto ensure the limit check is applied uniformly—including direct database operations, batch updates, and any alternative endpoints.
| // Inject virtual Favorites workspace at the top if user has favorites | ||
| if (hasFavorites && !isFetchingWorkspaces) { | ||
| const favoritesWorkspace = { | ||
| id: "__favorites__", | ||
| name: "Favorites", | ||
| isVirtual: true, | ||
| userPermissions: [], | ||
| }; | ||
|
|
||
| workspaces = [favoritesWorkspace, ...workspaces]; | ||
| } |
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.
Potential cause of intermittent navigation bug reported in PR comments.
When hasFavorites is true, the favorites workspace is prepended to the array. If workspaceIdFromQueryParams is null/undefined, line 1188 defaults to workspaces[0]?.id, which would be __favorites__.
This may explain the reviewer's report of sometimes landing on Favorites instead of the expected workspace when navigating Home.
Consider preserving the user's last-visited real workspace or excluding the virtual workspace from the default selection:
// Inject virtual Favorites workspace at the top if user has favorites
if (hasFavorites && !isFetchingWorkspaces) {
const favoritesWorkspace = {
id: "__favorites__",
name: "Favorites",
isVirtual: true,
userPermissions: [],
};
workspaces = [favoritesWorkspace, ...workspaces];
}
+ // Find first real workspace for default selection (skip virtual workspaces)
+ const defaultWorkspace = workspaces.find((ws: Workspace) => !ws.isVirtual);
+
const [activeWorkspaceId, setActiveWorkspaceId] = useState<
string | undefined
>(
- workspaceIdFromQueryParams ? workspaceIdFromQueryParams : workspaces[0]?.id,
+ workspaceIdFromQueryParams ? workspaceIdFromQueryParams : defaultWorkspace?.id,
);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In app/client/src/ce/pages/Applications/index.tsx around lines 1160 to 1170,
prepending a virtual "Favorites" workspace causes the later defaulting logic
(when workspaceIdFromQueryParams is falsy) to pick workspaces[0]?.id which
becomes "__favorites__"; change the default-selection logic so the virtual
workspace is excluded: when workspaceIdFromQueryParams is null/undefined, choose
lastVisitedWorkspaceId if available, otherwise pick the first workspace whose
isVirtual flag is false (or the first real workspace in the list), and only
prepend the virtual Favorites for display (not for determining the default
selection). Ensure the code checks for existence of a real workspace before
falling back to the virtual id so navigation no longer lands on Favorites
unexpectedly.
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.
@salevine This callout seems legit; can you check?
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.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
| public Mono<UserData> toggleFavoriteApplication(String applicationId) { | ||
| return sessionUserService.getCurrentUser().zipWhen(this::getForUser).flatMap(tuple -> { | ||
| User user = tuple.getT1(); | ||
| UserData userData = tuple.getT2(); | ||
|
|
||
| List<String> favorites = userData.getFavoriteApplicationIds(); | ||
| if (favorites == null) { | ||
| favorites = new ArrayList<>(); | ||
| } | ||
|
|
||
| // Toggle: remove if exists, add if doesn't | ||
| if (favorites.contains(applicationId)) { | ||
| favorites.remove(applicationId); | ||
| } else { | ||
| // Check if adding this favorite would exceed the limit | ||
| if (favorites.size() >= MAX_FAVORITE_APPLICATIONS_LIMIT) { | ||
| return Mono.error(new AppsmithException( | ||
| AppsmithError.INVALID_PARAMETER, | ||
| String.format( | ||
| "Maximum favorite applications limit (%d) reached. Please remove some favorites before adding new ones.", | ||
| MAX_FAVORITE_APPLICATIONS_LIMIT))); | ||
| } | ||
| favorites.add(applicationId); | ||
| } | ||
|
|
||
| userData.setFavoriteApplicationIds(favorites); | ||
| return repository.save(userData); | ||
| }); |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine UserData class for versioning support
# First, locate the UserData.java file
find app/server/appsmith-server -name "UserData.java" -type f
# Check the content of UserData class looking for version/optimistic locking
if [ -f "app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java" ]; then
echo "=== UserData.java content ==="
cat -n "app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.java"
fiRepository: appsmithorg/appsmith
Length of output: 4639
🏁 Script executed:
#!/bin/bash
# Check for @Version annotation and version field in UserData
rg -n "@Version|version|@DBRef|@Field" app/server/appsmith-server/src/main/java/com/appsmith/server/domains/UserData.javaRepository: appsmithorg/appsmith
Length of output: 159
🏁 Script executed:
#!/bin/bash
# Look for repository save operations and atomic operations in UserDataServiceCEImpl
rg -n "repository\.|save\|update\|@Query" app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java | head -30Repository: appsmithorg/appsmith
Length of output: 826
🏁 Script executed:
#!/bin/bash
# Check the repository interface implementation
find app/server/appsmith-server -name "*UserDataRepository*" -type fRepository: appsmithorg/appsmith
Length of output: 806
🏁 Script executed:
#!/bin/bash
# Check BaseDomain for @Version annotation
find app/server/appsmith-server -name "BaseDomain.java" -type f | xargs cat -n | head -100Repository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check CustomUserDataRepositoryImpl for atomic operations
cat -n app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/CustomUserDataRepositoryImpl.javaRepository: appsmithorg/appsmith
Length of output: 411
🏁 Script executed:
#!/bin/bash
# Check CustomUserDataRepositoryCEImpl for atomic operations
cat -n app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserDataRepositoryCEImpl.javaRepository: appsmithorg/appsmith
Length of output: 2973
🏁 Script executed:
#!/bin/bash
# Search for BaseDomain in external modules
find . -name "BaseDomain.java" -type f 2>/dev/null | head -5Repository: appsmithorg/appsmith
Length of output: 155
🏁 Script executed:
#!/bin/bash
# Search for @Version annotation across entire codebase
rg -n "@Version" app/server/appsmith-server/src/main/java --type java | head -20Repository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if there are any atomic toggle operations in UserData repository
rg -n "toggle|addToSet|pull.*favorite" app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ --type java -A 3 -B 3Repository: appsmithorg/appsmith
Length of output: 1489
🏁 Script executed:
#!/bin/bash
# Check BaseDomain for @Version annotation
cat -n app/server/appsmith-interfaces/src/main/java/com/appsmith/external/models/BaseDomain.java | head -80Repository: appsmithorg/appsmith
Length of output: 3382
🏁 Script executed:
#!/bin/bash
# Check if there's a dedicated atomic method for toggling favorites in repository
rg -n "toggleFavorite|addToSet.*favorite" app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ --type javaRepository: appsmithorg/appsmith
Length of output: 46
Race condition in concurrent favorite toggles.
The toggleFavoriteApplication method retrieves userData, modifies the favorites list in memory, and saves it back via repository.save(). Concurrent requests can cause lost updates when two users toggle favorites simultaneously—the second save overwrites the first without merging changes.
The codebase already uses atomic MongoDB operations elsewhere (e.g., removeApplicationFromFavorites uses $pull). Apply the same pattern here: add an atomic repository method using $addToSet and $pull instead of fetch-modify-save.
🤖 Prompt for AI Agents
In
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java
around lines 439-466, the current fetch-modify-save toggling of
favoriteApplicationIds causes lost updates under concurrent requests; replace it
with atomic MongoDB updates: implement repository methods that perform updateOne
with either $pull (to remove the id) or $addToSet (to add the id). For the add
case, make the update query include a guard that the current array length is
below MAX_FAVORITE_APPLICATIONS_LIMIT (use $expr with $lt and $size in the
filter) so the server-side limit is enforced atomically; execute the appropriate
repository.updateOne call based on whether the id already exists (or try $pull
first and if modifiedCount==0 attempt the guarded $addToSet), then inspect the
UpdateResult to return the updated UserData or raise the same AppsmithException
when the guarded add fails due to the limit. Ensure you remove the in-memory
list mutation and repository.save() call and instead return the result of the
atomic update/read operation.
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
subrata71
left a comment
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.
Reviewed the backend changes mostly. Need @ashit-rath to look at the frontend changes
| * @throws AppsmithException if the maximum favorite limit is reached when trying to add a favorite | ||
| */ | ||
| @Override | ||
| public Mono<UserData> toggleFavoriteApplication(String applicationId) { |
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.
toggleFavoriteApplication does not verify the user has the required access to the application before allowing them to favorite it. This allows favoriting applications the user cannot access.
| .flatMap(applications -> { | ||
| // Clean up favorites list if any apps were not found or are inaccessible | ||
| if (applications.size() < favoriteIds.size()) { | ||
| return cleanupMissingFavorites(userData, applications) |
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.
GET requests should be used to retrieve resources without modifying them. However, this code attempts to perform a cleanup operation conditionally, which goes against this convention. Let's move the cleanup logic elsewhere to maintain proper adherence to HTTP standards.
| "Maximum favorite applications limit (%d) reached. Please remove some favorites before adding new ones.", | ||
| MAX_FAVORITE_APPLICATIONS_LIMIT))); | ||
| } | ||
| favorites.add(applicationId); |
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.
The method doesn't verify the application exists before adding it to favorites. This can create invalid references thus creating a possibility to compromise data integrity.
| * @return Updated user data with modified favorites list | ||
| */ | ||
| @JsonView(Views.Public.class) | ||
| @PutMapping("/applications/{applicationId}/favorite") |
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.
QQ: When an application is connected to Git, which applicationId does the client side pass? Just to clarify, a Git-connected app with three branches will have a separate applicationId for each branch, but all of them recognize the base applicationId they were created from. My question is: Is there a possibility that the same app could be marked as a favorite by the user, given that the app is Git-connected and has multiple branches?
cc: @sondermanish
| } | ||
|
|
||
| // Toggle: remove if exists, add if doesn't | ||
| if (favorites.contains(applicationId)) { |
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.
Nit (Race-condition): If two concurrent request comes with the same applicationId there is a chance that the same app can be added to the favorite list twice
| * @param applicationId ID of deleted application | ||
| */ | ||
| @Override | ||
| public Mono<Void> removeApplicationFromAllFavorites(String applicationId) { |
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.
I’m not sure if this method is being used. Could you please check on that? Also, I’m unclear about what happens when an application is deleted. Do we remove it from all users' favorites? Additionally, if a user is on a branch other than the main/default one and marks the app as a favorite, do we store the applicationId of the base branch? And what happens if a branch is deleted?
I have more questions regarding Git-connected apps, but these are some key areas where clarification would be really helpful.
| yield put(toggleFavoriteApplicationSuccess(applicationId, isFavorited)); | ||
| yield put(toggleFavoriteApplicationError(applicationId)); | ||
|
|
||
| toast.show("Failed to update favorite status", { kind: "error" }); |
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.
Users don't know why favoriting failed. If this is due to 50 favorite limit reached then it's really poor UX as the user is completely unaware of such constraints.
| yield put(toggleFavoriteApplicationSuccess(applicationId, newIsFavorited)); | ||
|
|
||
| // API call | ||
| const response: unknown = yield call( |
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.
Nit: The frontend doesn't check the limit before making the API call, causing unnecessary requests.
| import { findDefaultPage } from "pages/utils"; | ||
| import type { ApplicationPayload } from "entities/Application"; | ||
|
|
||
| function* toggleFavoriteApplicationSaga( |
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.
Please consider the scenario what happens when user does multiple rapid toggles.
| * @return List of favorited applications | ||
| */ | ||
| @JsonView(Views.Public.class) | ||
| @GetMapping("/favoriteApplications") |
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.
GAC: What happens when a user’s access to an application is revoked, especially if that app was previously marked as a favorite?
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20598271075. |
|
Deploy-Preview-URL: https://ce-41462.dp.appsmith.com |
| // Inject virtual Favorites workspace at the top if user has favorites | ||
| if (hasFavorites && !isFetchingWorkspaces) { | ||
| const favoritesWorkspace = { | ||
| id: "__favorites__", | ||
| name: "Favorites", | ||
| isVirtual: true, | ||
| userPermissions: [], | ||
| }; | ||
|
|
||
| workspaces = [favoritesWorkspace, ...workspaces]; | ||
| } |
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.
@salevine This callout seems legit; can you check?
|
|
||
| if (!workspace.id) return null; | ||
|
|
||
| const isFavoritesWorkspace = workspace.id === "__favorites__"; |
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.
Let's create a new constant like FAVORITES_KEY = "__favorites__" in src/ce/constants/workspaceConstants.ts and use FAVORITES_KEY here. Let's avoid magic strings
| const favoritesWorkspace = { | ||
| id: "__favorites__", | ||
| name: "Favorites", | ||
| isVirtual: true, | ||
| userPermissions: [], | ||
| }; |
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.
Since this is also a constant; put the whole object in the workspaceConstants file as
DEFAULT_FAVORITES_WORKSPACE = {
id: "__favorites__",
name: "Favorites",
isVirtual: true,
userPermissions: [],
}
| [ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_INIT]: ( | ||
| draftState: SelectedWorkspaceReduxState, | ||
| ) => { | ||
| draftState.loadingStates.isFetchingApplications = true; |
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.
Why are we updating isFetchingApplications while fetching favorite applications?
| if (workspaceId === "__favorites__") { | ||
| yield put({ | ||
| type: ReduxActionTypes.SET_CURRENT_WORKSPACE, | ||
| payload: { |
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.
Use DEFAULT_FAVORITES_WORKSPACE here
|
|
||
| if (isValidResponse) { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| const rawApplications = (response as any).data; |
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.
Avoid using any and remove all the eslint-disables. We should use any in the most extreme cases where we just cannot escape
| } else { | ||
| // Non-successful API response – notify reducers so loading state is cleared. | ||
| yield put(fetchFavoriteApplicationsError()); | ||
| } |
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 else block is unreachable and can be removed
| const allApplications: ApplicationPayload[] = yield select( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (state: any) => state.ui.applications.applicationList, | ||
| ); | ||
|
|
||
| // Transform applications to include defaultBasePageId (needed for Launch button) | ||
| // This matches the transformation done in ApplicationSagas.tsx | ||
| const applications = rawApplications.map( | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| (application: any) => { | ||
| const defaultPage = findDefaultPage(application.pages); | ||
|
|
||
| // Find the corresponding application from the main list (if loaded) | ||
| const existing = allApplications?.find( | ||
| (app) => app.id === application.id, | ||
| ); | ||
|
|
||
| return { | ||
| ...application, | ||
| // Prefer userPermissions from the main application list so edit | ||
| // permissions match the regular workspace cards. | ||
| userPermissions: | ||
| existing?.userPermissions ?? application.userPermissions, | ||
| defaultPageId: defaultPage?.id, | ||
| defaultBasePageId: defaultPage?.baseId, | ||
| }; | ||
| }, | ||
| ); |
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.
Why are we doing this? why can't we get the userPermissions from the server?
| @@ -0,0 +1,30 @@ | |||
| import { createSelector } from "reselect"; | |||
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.
Let's not create a new selector file and have these in the application selector
| (favoriteIds: string[]) => favoriteIds.length > 0, | ||
| ); | ||
|
|
||
| export const getIsFetchingFavorites = (state: DefaultRootState) => |
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.
Where are we using this?




Description
Feature Request 8479: Allow a user to favorite applications.
In bigger factories there can be many workspaces and applications. Users might have access to a lot of applications, but generally only use a handful. This feature allows a user to favorite one or more applications (up to 50). Any favorited applications will show up in a virtual Favorites workspace in alphabetical order
Fixes #
8479Automation
/ok-to-test tags=""
🔍 Cypress test results
Warning
Tests have not run on the HEAD c90366b yet
Mon, 15 Dec 2025 21:05:11 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.