Open
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
…ss yarn check types
… handling on loading
Addressing the backend comments with fixes
Fixed. Added UserDataService to the ApplicationPageServiceImpl constructor and passed it to the super call. The compilation error should be resolved.
- UserData: atomic toggle favorites (addToSet/pull) to fix race condition - FavoritesSagas: remove unreachable/error action, use ApiResponse types - Remove toggleFavoriteApplicationError and TOGGLE_FAVORITE_APPLICATION_ERROR - Card: drop redundant overlay pointerEvents (handled by Wrapper) - WorkspaceSagas: refetch favorites when loading workspace (drop inaccessible) - ApplicationPageServiceCEImpl: use getBaseId() directly for favoriteId Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…ew on refresh - In ApplicationSagas and InitSagas: when a logged-in user hits 404 for an app (e.g. deleted favorite), redirect to applications?workspaceId=__favorites__, refetch favorites, and show error toast instead of crash page - Applications page: prepend Favorites workspace when URL has workspaceId=__favorites__ (not only when hasFavorites) so redirect lands on Favorites - Applications page: when active workspace is __favorites__ but not in list yet, use DEFAULT_FAVORITES_WORKSPACE and still dispatch SET_CURRENT_WORKSPACE and fetchEntitiesOfWorkspace so Favorites view loads correctly after refresh Co-authored-by: Cursor <cursoragent@cursor.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… action Co-authored-by: Cursor <cursoragent@cursor.com>
…e favorites loading state Move userPermissions enrichment from ApplicationCard into FavoritesSagas so the component no longer queries the global application list to resolve permissions for favorite apps. This removes a tight coupling between a presentational component and unrelated global state. Also introduce a dedicated isFetchingFavoriteApplications loading flag in selectedWorkspaceReducer so favorites fetches no longer reuse the workspace's isFetchingApplications flag, making Redux state unambiguous during debugging. Co-authored-by: Cursor <cursoragent@cursor.com>
salevine
added
Enhancement
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds an end-to-end "Favorites" feature: UI heart toggle and virtual Favorites workspace, Redux actions/reducers/selectors, sagas with optimistic toggle and 50-item limit, new backend endpoints/repository/service support, and persistence of users' favorite application IDs. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Client)
participant FrontendUI as Frontend UI
participant FrontendSaga as Favorites Saga
participant BackendAPI as Backend API
participant DB as Database
User->>FrontendUI: Click heart icon (toggle)
FrontendUI->>FrontendSaga: Dispatch TOGGLE_FAVORITE_APPLICATION_INIT (optimistic)
FrontendSaga->>FrontendUI: Dispatch TOGGLE_FAVORITE_APPLICATION_SUCCESS (optimistic UI update)
FrontendSaga->>BackendAPI: PUT /applications/{appId}/favorite
BackendAPI->>DB: Atomic addToSet or pull favoriteApplicationIds
DB-->>BackendAPI: Updated user favorite state
BackendAPI-->>FrontendSaga: Return success or error
alt success
FrontendSaga->>FrontendUI: Confirm success (no change)
else error
FrontendSaga->>FrontendUI: Dispatch TOGGLE_FAVORITE_APPLICATION_ERROR, revert optimistic update, show toast
end
sequenceDiagram
participant User as User (Client)
participant FrontendUI as Frontend UI
participant FrontendSaga as Favorites Saga
participant BackendAPI as Backend API
participant AppRepo as Application Repository
User->>FrontendUI: Navigate to Favorites workspace
FrontendUI->>FrontendSaga: Dispatch FETCH_FAVORITE_APPLICATIONS_INIT
FrontendSaga->>BackendAPI: GET /favoriteApplications
BackendAPI->>AppRepo: Load application details for favorite IDs
AppRepo-->>BackendAPI: Applications with metadata
BackendAPI->>BackendAPI: Enrich with permissions/defaultPageId
BackendAPI-->>FrontendSaga: Return List<Application>
FrontendSaga->>FrontendUI: Dispatch FETCH_FAVORITE_APPLICATIONS_SUCCESS and render list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java (1)
577-587:⚠️ Potential issue | 🟡 MinorAdd null guard for
favoriteIdto match defensive patterns used elsewhere in the codebase.The code calls
removeApplicationFromFavorites(favoriteId)without validating thatapplication.getBaseId()returns a non-null value. While the repository method handles null gracefully, the codebase uses defensive null checks forgetBaseId()in other places (e.g.,StaticUrlAnalyticsUtils.java). Add a null check to fall back toapplication.getId()if needed.Proposed fix
- String favoriteId = application.getBaseId(); + String favoriteId = application.getBaseId() != null ? application.getBaseId() : application.getId();
🤖 Fix all issues with AI agents
In `@app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx`:
- Around line 886-917: The toggle reducer
(ReduxActionTypes.TOGGLE_FAVORITE_APPLICATION_SUCCESS) mixes baseId and id into
favoriteApplicationIds causing mismatches with
FETCH_FAVORITE_APPLICATIONS_SUCCESS which uses app.id; fix by normalizing to the
canonical application.id before mutating favoriteApplicationIds: inside the
TOGGLE_FAVORITE_APPLICATION_SUCCESS handler, look up the app in
state.applicationList by matching (app.baseId || app.id) === applicationId to
obtain the canonical app.id (fall back to the incoming applicationId if no
match), then add/remove that canonical id in favoriteApplicationIds and still
update applicationList entries by matching (app.baseId || app.id) ===
applicationId to set isFavorited. This ensures favoriteApplicationIds and
FETCH_FAVORITE_APPLICATIONS_SUCCESS both use the same id space.
In `@app/client/src/ce/sagas/ApplicationSagas.tsx`:
- Around line 435-445: Replace the hardcoded toast string in the error branch
(the block that checks currentUser, ERROR_CODES.PAGE_NOT_FOUND and calls
toast.show) with a call to createMessage using a shared message constant (e.g.,
APPLICATION_NOT_FOUND) instead of the literal "Application not found or
deleted."; add that constant to the common messages module already used by
InitSagas.ts and import it into ApplicationSagas.tsx, then call
toast.show(createMessage(APPLICATION_NOT_FOUND), { kind: "error" }) so both
sagas use the same i18n-friendly message.
In `@app/client/src/sagas/FavoritesSagas.ts`:
- Around line 86-95: The permissions map is built from a search-filtered list
because the saga uses select(getApplicationList); change it to use
select(getApplications) so you get the unfiltered application list. In the saga
code that populates permissionsById (currently reading allApplications from
select(getApplicationList)), replace that select call with
select(getApplications) and keep the subsequent loop that checks
app.userPermissions and sets permissionsById for app.id unchanged so permissions
for non-search-matching apps are included.
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/UserControllerCE.java`:
- Around line 216-222: Controller mapping for toggleFavoriteApplication is using
"/applications/{applicationId}/favorite" but endpoints should live under
Url.USER_URL; change the mapping in UserControllerCE to include the user prefix
(use Url.USER_URL) and accept the userId path variable so the route matches
client expectations (e.g., replace
`@PutMapping`("/applications/{applicationId}/favorite") with a mapping that
composes Url.USER_URL + "/{userId}/applications/{applicationId}/favorite"), add
a `@PathVariable` String userId parameter to toggleFavoriteApplication, and
pass/forward that userId to userDataService as needed; also scan other
UserControllerCE methods for similar mappings and align them with Url.USER_URL
(or alternatively update ApplicationApi to call the users-prefixed path if you
prefer changing the client).
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java`:
- Around line 437-491: The in-memory size check in toggleFavoriteApplication can
race with the atomic addToSet and allow exceeding
MAX_FAVORITE_APPLICATIONS_LIMIT; remove that local check and perform a
conditional atomic update instead. Implement a repository method (e.g.,
addFavoriteApplicationForUserIfUnderLimit or addFavoriteApplicationWithMaxLimit)
that issues a single DB update which both $addToSet the applicationId and
enforces the max size (using a Mongo conditional update/filter or aggregation
expression to require array length < MAX_FAVORITE_APPLICATIONS_LIMIT), return a
signal indicating success vs failure (no-op vs error), and call that new repo
method from toggleFavoriteApplication (keeping the existing new-user save branch
for userData.getId() == null). Ensure toggleFavoriteApplication surfaces an
error when the conditional update indicates the limit was reached.
🧹 Nitpick comments (7)
app/client/src/ce/sagas/ApplicationSagas.tsx (1)
435-445: Duplicate redirect-to-favorites logic inInitSagas.ts.The same
history.replace+FETCH_FAVORITE_APPLICATIONS_INIT+toast.showpattern appears in bothhandleFetchApplicationError(here) andstartAppEngine(inInitSagas.ts, lines 425-436). Consider extracting a shared helper to avoid drift between the two.app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/UserControllerCE.java (1)
228-234: Consider whether returning fullApplicationobjects is necessary.
getFavoriteApplicationsreturnsList<Application>— the domain object may include fields beyond what the client needs. While@JsonView(Views.Public.class)filters the serialization, a lighter DTO projection could reduce payload size if the favorites list grows toward the 50-app cap.app/client/src/ce/pages/Applications/index.tsx (1)
31-42: Consolidate imports from the same module.
getHasFavorites(line 38) is imported separately from the sameee/selectors/applicationSelectorsmodule already imported on lines 31-37. Merge them into a single import statement.♻️ Proposed fix
import { getApplicationList, getApplicationSearchKeyword, getCreateApplicationError, getCurrentApplicationIdForCreateNewApp, getIsCreatingApplication, getIsDeletingApplication, + getHasFavorites, } from "ee/selectors/applicationSelectors"; -import { getHasFavorites } from "ee/selectors/applicationSelectors";app/client/src/components/common/Card.tsx (1)
421-430: Addaria-pressedto the toggle button for screen readers.The button has a static
aria-label="Toggle favorite"but doesn't communicate the current state. Toggle buttons should usearia-pressedso assistive technologies announce whether the item is currently favorited.♿ Proposed fix
<FavoriteIconWrapper aria-label="Toggle favorite" + aria-pressed={!!isFavorited} data-testid="t--favorite-icon" onClick={handleFavoriteClick} type="button" >app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx (1)
910-917:FETCH_FAVORITE_APPLICATIONS_SUCCESSdoes not updateisFavoritedonapplicationListentries.The toggle handler (line 897–901) correctly sets
isFavoritedon matchingapplicationListentries, but this fetch handler only populatesfavoriteApplicationIdswithout syncing the flag back toapplicationList. Any consumer readingapp.isFavoriteddirectly from the application list (rather than through the memoized selector inselectedWorkspaceSelectors.ts) will see stale data after a favorites fetch.Consider iterating over
applicationListhere to setisFavoritedbased on the fetched IDs, matching what the toggle handler already does.♻️ Proposed fix
[ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_SUCCESS]: ( state: ApplicationsReduxState, action: ReduxAction<ApplicationPayload[]>, - ) => ({ - ...state, - isFetchingFavorites: false, - favoriteApplicationIds: action.payload.map((app) => app.id), - }), + ) => { + const favoriteApplicationIds = action.payload.map((app) => app.id); + const favoriteIdSet = new Set(favoriteApplicationIds); + + return { + ...state, + isFetchingFavorites: false, + favoriteApplicationIds, + applicationList: state.applicationList.map((app) => ({ + ...app, + isFavorited: favoriteIdSet.has(app.id), + })), + }; + },app/client/src/ce/selectors/selectedWorkspaceSelectors.ts (1)
32-48: Consistent update — the OR logic here mirrorsgetIsFetchingApplicationsabove. Consider extracting the shared condition to a single selector to avoid drift, but low priority.app/client/src/ce/reducers/uiReducers/selectedWorkspaceReducer.ts (1)
272-290: Correct toggle behavior for favorites vs. regular workspaces.Minor Immer style nit: the
elsebranch on line 284 uses.map()which allocates a new array. Since this is an Immer reducer, you can mutate in place:♻️ Idiomatic Immer alternative
} else { - draftState.applications = draftState.applications.map((app) => - (app.baseId || app.id) === applicationId - ? { ...app, isFavorited } - : app, - ); + const app = draftState.applications.find( + (app) => (app.baseId || app.id) === applicationId, + ); + if (app) { + app.isFavorited = isFavorited; + } }
...erver/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/UserControllerCE.java
Show resolved
Hide resolved
...ver/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java
Show resolved
Hide resolved
…limit atomically Client API methods were hitting the wrong controller (ApplicationController instead of UserController). The toggle reducer was mixing baseId/id into favoriteApplicationIds causing mismatches with the fetch reducer. The in-memory size check in toggleFavoriteApplication could race with concurrent addToSet calls, allowing the limit to be exceeded. - Point client favorites API calls to v1/users/... to match UserControllerCE - Resolve canonical app.id in TOGGLE_FAVORITE_APPLICATION_SUCCESS before mutating favoriteApplicationIds so it stays consistent with FETCH - Add addFavoriteApplicationForUserIfUnderLimit repo method using a conditional MongoDB update (array-index existence trick + $addToSet) - Replace in-memory size check with atomic DB-level enforcement Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java`:
- Around line 466-479: The new-user branch in UserDataServiceCEImpl that checks
userData.getId() == null is vulnerable to a race where two concurrent requests
both save a transient UserData and create duplicates; instead implement an
atomic upsert or handle DuplicateKeyException and retry using the existing-user
conditional update path. Concretely, replace the repository.save(userData) call
in that branch with an upsert/atomic push (e.g., use a repository.upsert or
MongoTemplate.findAndModify with upsert=true and a $addToSet/$push plus size
check against MAX_FAVORITE_APPLICATIONS_LIMIT), or catch DuplicateKeyException
from repository.save and then invoke the same conditional update logic used for
the existing-user path to add the favorite id; ensure you reference
UserDataServiceCEImpl, repository.save, and the existing-user conditional update
code when making the change.
- Around line 437-453: The in-memory favorites.contains(applicationId) check in
toggleFavoriteApplication can be stale; instead, avoid making the add/remove
decision from the snapshot and perform atomic repository operations to determine
the outcome: call repository.removeFavoriteApplicationForUser(user.getId(),
applicationId) first and inspect its result (e.g., modified count / boolean) —
if it reports a modification return getForUser(user.getId()), otherwise call
repository.addFavoriteApplicationForUser(user.getId(), applicationId) and then
return getForUser(user.getId()); update toggleFavoriteApplication to rely on the
repository operation results rather than favorites.contains and use the existing
getForUser/sessionUserService calls to return the updated UserData.
🧹 Nitpick comments (3)
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomUserDataRepositoryCEImpl.java (2)
50-56: Bulk removal across all users — no criteria filter is intentional but worth noting.
queryBuilder().updateAll(update)with no.criteria(...)will scan and update everyUserDatadocument. This is fine for a cleanup-on-delete path (infrequent), but be aware it won't use an index efficiently. If thefavoriteApplicationIdsfield is sparsely populated, the write amplification should be minimal.
58-66: Remove unusedaddFavoriteApplicationForUsermethod.The service layer calls only
addFavoriteApplicationForUserIfUnderLimitfor existing users andrepository.save()for new users. This method serves no purpose and should be removed to reduce code clutter.app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java (1)
466-495: Duplicated error message string — extract a constant.The limit-reached message is duplicated on lines 472–474 and 489–491. Extract it to a
private static final Stringto keep them in sync.Suggested refactor
+ private static final String FAVORITE_LIMIT_REACHED_MESSAGE = + "Maximum favorite applications limit (%d) reached. Please remove some favorites before adding new ones."; + // Then in both locations: - String.format( - "Maximum favorite applications limit (%d) reached. Please remove some favorites before adding new ones.", - MAX_FAVORITE_APPLICATIONS_LIMIT) + String.format(FAVORITE_LIMIT_REACHED_MESSAGE, MAX_FAVORITE_APPLICATIONS_LIMIT)
...ver/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java
Outdated
Show resolved
Hide resolved
...ver/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java
Outdated
Show resolved
Hide resolved
salevine
changed the title
The blanket redirect of all logged-in-user 404s to the favorites page broke the standard error page (`.t--error-page-title`) for users who simply lack access to an app. This caused the ShareAppTests_Spec "validate public access disable" test to fail. - InitSagas: check getFavoriteApplicationIds before redirecting; only redirect if the app is actually in the user's favorites list, otherwise fall through to safeCrashAppRequest (error page) - ApplicationSagas: restore original handleFetchApplicationError behavior — anonymous users get the crash page, logged-in users get FETCH_APPLICATION_ERROR + FETCH_PAGE_LIST_ERROR (error page) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The in-memory favorites.contains() check was stale under concurrency, and the new-user repository.save() could race to create duplicate UserData documents. - removeFavoriteApplicationForUser now returns Mono<Integer> and adds an array-contains criteria so matchedCount signals whether a removal actually occurred - toggleFavoriteApplication for existing users: tries atomic remove first, inspects the DB result, then falls back to atomic add — no in-memory snapshot decision - toggleFavoriteApplication for new users: both save() calls catch DuplicateKeyException and retry through the atomic existing-user path (removeFavoriteApplicationForUser / addFavoriteApplicationFor- UserIfUnderLimit) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
getApplicationList is filtered by the active search keyword, so the permissions map built in fetchFavoriteApplicationsSaga could miss apps that don't match the current search query. Use getApplications instead to include all loaded apps. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…cator Favorites fetch was OR'd into getIsFetchingApplications, causing app cards to flash/disappear when the background favorites refresh set isFetchingFavoriteApplications=true after entity loading completed. Now only the Favorites virtual workspace shows a skeleton during favorites fetch; regular workspaces load favorites silently. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ashit-rath
reviewed
Feb 16, 2026
| applicationId: string, | ||
| ): Promise<AxiosPromise<ApiResponse>> { | ||
| return Api.put(`v1/users/applications/${applicationId}/favorite`); | ||
| } |
Contributor
There was a problem hiding this comment.
@salevine This endpoint seems wrong; shouldn't it be
v1/applications/${applicationId}/favorite and the below one as well?
I remember this to be correct earlier, you used ApplicationApi.baseURL; something changed?
Contributor
Author
There was a problem hiding this comment.
This happened when I was creating the new PR and then trying to fix the various code rabbit identified issues and getting the tests to pass. I correct this this.
Contributor
Author
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/22067006626. |
|
Deploy-Preview-URL: https://ce-41555.dp.appsmith.com |
2 tasks
ashit-rath
approved these changes
Feb 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/22001834951
Commit: 56d760c
Cypress dashboard.
Tags:
@tag.AllSpec:
Fri, 13 Feb 2026 21:58:17 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
UX
Chores