Skip to content

Conversation

@salevine
Copy link
Contributor

@salevine salevine commented Dec 14, 2025

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 #8479

Automation

/ok-to-test tags=""

🔍 Cypress test results

Warning

Tests have not run on the HEAD 6f9f36d yet


Mon, 26 Jan 2026 17:05:30 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Star/unstar apps directly on app cards with immediate (optimistic) UI updates and a red heart Favorites entry in the sidebar.
    • Dedicated "Favorites" workspace to browse favorited apps; favorites load automatically with workspaces.
  • Behavior

    • Favorites persist and sync with the server.
    • Max 50 favorites enforced with a user-facing error when exceeded.
    • Server automatically cleans up deleted or inaccessible apps from users' favorites.

✏️ Tip: You can customize this high-level summary in your review settings.

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
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 14, 2025

Walkthrough

Adds per-user application favorites: UI (card icon, virtual Favorites workspace), Redux actions/reducers/selectors, sagas with optimistic toggle and fetch, API helpers, and backend controller/service/repo support (toggling, retrieval, and cleanup), plus wiring to load favorites on workspace fetch.

Changes

Cohort / File(s) Summary
Frontend Actions
app/client/src/actions/applicationActions.ts
New action creators for toggle/fetch favorites (init/success/error).
Frontend API
app/client/src/ce/api/ApplicationApi.tsx
Added toggleFavoriteApplication (PUT) and getFavoriteApplications (GET) helpers.
Frontend Constants & Workspace
app/client/src/ce/constants/ReduxActionConstants.tsx, app/client/src/ce/constants/workspaceConstants.ts
New Redux action/error constants; added FAVORITES_KEY, DEFAULT_FAVORITES_WORKSPACE, and Workspace.isVirtual.
Frontend Reducers
app/client/src/ce/reducers/uiReducers/applicationsReducer.tsx, app/client/src/ce/reducers/uiReducers/selectedWorkspaceReducer.ts
Added favoriteApplicationIds, isFetchingFavorites; handlers for fetch/toggle actions; update application isFavorited and favorites workspace behavior.
Frontend Sagas
app/client/src/sagas/FavoritesSagas.ts, app/client/src/ce/sagas/WorkspaceSagas.ts, app/client/src/ce/sagas/index.tsx
New favorites sagas: optimistic toggle (limit 50, rollback on error), fetch favorites; dispatch fetch on workspace load; register saga.
Frontend Selectors
app/client/src/ce/selectors/applicationSelectors.tsx, app/client/src/ce/selectors/selectedWorkspaceSelectors.ts
New selectors: getFavoriteApplicationIds, getFavoriteApplications, getHasFavorites; memoized getApplicationsOfWorkspace enriches apps with isFavorited.
Frontend UI Components
app/client/src/components/common/Card.tsx, app/client/src/pages/Applications/ApplicationCard.tsx, app/client/src/ce/pages/Applications/index.tsx, app/client/src/pages/AppIDE/layouts/components/Header/index.tsx
Card props isFavorited/onToggleFavorite; ApplicationCard wires toggle and permission fallback; injects virtual Favorites workspace and displays red-heart row; header link adds workspaceId query.
Entities / Types
app/client/src/entities/Application/types.ts
Added optional isFavorited?: boolean to ApplicationPayload.
Minor Frontend Fixes
app/client/src/pages/AppIDE/layouts/hooks/useWidgetSelectionBlockListener.ts, app/client/src/components/editorComponents/Debugger/index.tsx
Adjusted effect dependencies to reduce re-renders and synchronize error count.
Backend Controller
app/server/.../UserControllerCE.java
New endpoints: PUT /applications/{applicationId}/favorite and GET /favoriteApplications.
Backend Domain & Repo
app/server/.../UserData.java, app/server/.../CustomUserDataRepositoryCE.java, .../CustomUserDataRepositoryCEImpl.java
Added favoriteApplicationIds to UserData; repo method to remove an application from all users' favorites implemented.
Backend Services
app/server/.../UserDataServiceCE.java, app/server/.../UserDataServiceCEImpl.java, app/server/.../UserDataServiceImpl.java, app/server/.../ApplicationPageServiceCEImpl.java, app/server/.../ApplicationPageServiceImpl.java
New service methods: toggleFavoriteApplication, getFavoriteApplications, removeApplicationFromAllFavorites; enforces limit (50), permission checks, and cleanup on delete; constructor wiring updated to inject ApplicationPermission and UserDataService.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Card as Card Component
    participant Redux as Redux Store
    participant Saga as Favorites Saga
    participant API as ApplicationApi
    participant Server as Backend Service

    User->>Card: Click favorite icon
    Card->>Redux: dispatch(TOGGLE_FAVORITE_APPLICATION_INIT)
    Redux->>Saga: action handled by saga
    Saga->>Redux: dispatch(TOGGLE_FAVORITE_APPLICATION_SUCCESS) [optimistic update]
    Redux->>Card: UI updates (isFavorited toggled)
    Saga->>API: PUT /users/applications/{appId}/favorite
    API->>Server: HTTP PUT
    Server->>Server: Toggle favorite in DB
    Server-->>API: Response
    alt Success
        Saga->>Redux: confirm/reconcile state
    else Failure
        Saga->>Redux: dispatch(TOGGLE_FAVORITE_APPLICATION_SUCCESS) [rollback]
        Saga->>Redux: dispatch(TOGGLE_FAVORITE_APPLICATION_ERROR)
        Saga->>User: show error toast
    end
Loading
sequenceDiagram
    participant App as App Init
    participant WorkspaceSaga as Workspace Sagas
    participant Redux as Redux Store
    participant FavoritesSaga as Favorites Saga
    participant API as ApplicationApi
    participant Server as Backend Service
    participant DB as Database

    App->>WorkspaceSaga: fetchAllWorkspacesSaga
    WorkspaceSaga->>Redux: dispatch(FETCH_ALL_WORKSPACES_SUCCESS)
    WorkspaceSaga->>Redux: dispatch(FETCH_FAVORITE_APPLICATIONS_INIT)
    Redux->>FavoritesSaga: action received
    FavoritesSaga->>API: GET /users/favoriteApplications
    API->>Server: HTTP GET
    Server->>DB: Read favoriteApplicationIds, fetch Applications
    Server->>Server: Filter by permissions, cleanup missing entries
    Server-->>API: List<Application>
    FavoritesSaga->>Redux: dispatch(FETCH_FAVORITE_APPLICATIONS_SUCCESS) [apps]
    Redux->>App: Favorites workspace populated with applications
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

"A tiny heart on each card beats bright,
Click once — optimistic day or night,
Backend keeps fifty treasures tidy,
Sagas hum and rollback if things go shoddy,
Favorites now gleam in workspace light." ❤️

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The linked issue #8479 describes form field components, which is unrelated to the favorite applications feature implemented in this PR. The changeset does not fulfill the objectives from the linked issue. The linked issue #8479 appears to be incorrectly associated with this PR. Verify the correct issue number for the favorite applications feature and link the appropriate issue.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat: add favorite applications' is clear, concise, and directly describes the main feature implemented in this changeset.
Description check ✅ Passed The PR description provides context (Feature Request 8479), explains the rationale, mentions the 50-app limit, references the related issue, and indicates communication team notification is required.
Out of Scope Changes check ✅ Passed All changes are consistently scoped to implementing favorite applications functionality across client and server tiers, including UI components, Redux actions/reducers, sagas, API methods, and database operations.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

Failed server tests

  • com.appsmith.server.helpers.GitUtilsTest#isRepoPrivate
  • com.appsmith.server.helpers.UserUtilsTest#makeInstanceAdministrator_WhenUserAlreadyAdmin_MaintainsPermissionsSuccessfully

@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20201950013.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

@github-actions
Copy link

Deploy-Preview-URL: https://ce-41462.dp.appsmith.com

@tomjose92
Copy link
Contributor

tomjose92 commented Dec 14, 2025

  1. As a developer/administrator of the App, I am not able to click edit on it. We should expect both App Viewers/Developers to use this
  2. Along with it, the same options to fork, export or delete. We already have these cards, it would make sense to reuse them. App Viewer would see only launch, and developers would see exactly how it was on their respective workspace
  3. When I am on the favorites page, i see the link as /applications?workspaceId=__favorites__. Can we have a separate route for it instead of reusing workspaceId, like /applications/favorites or better?
  4. I was able to favorite more than 50 applications, so would need to disallow and notify the user if they try to favorite more than 50
  5. There is a weird bug where when i click on Home from the App editor in my workspace, its going to the favorites page and not to the workspace of the App. It happens intermittently, so something on the client side. Screenshot belows. Image one shows it should actually look like when tom's apps workspace is selected. Image two shows that the workspace is selected, but showing the favorites page

Screenshot 2025-12-14 at 12 18 21 PM       Screenshot 2025-12-14 at 12 18 12 PM

@salevine
Copy link
Contributor Author

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.

@tomjose92
Copy link
Contributor

@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.
This occurs randomly, but easily reproducible. I feel this issue is happening because something got introduced around the URL and workspaceId route for favorites and breaking the existing flow. Please check.
Attaching a video below:

Screen.Recording.2025-12-15.at.8.40.40.AM.mov

@pratapaprasanna
Copy link
Contributor

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20221639173.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

@tomjose92
Copy link
Contributor

tomjose92 commented Dec 15, 2025

  1. Still not able to see Edit or other actions on Application card in the Favorites.
  2. I am still able to favorite more than 50 Apps after the last set of changes.
  3. Also can we reduce the size of the favorite icon slightly. Reason is we support the name of the App upto 3 lines, after which its shown as truncated (...). On reducing the icon size slightly, it wouldnt cover the name in such scenarios and would look better. We can even get the help of a designer/PM to comment on how it should look. Screenshot below
Screenshot 2025-12-15 at 3 39 29 PM

@github-actions
Copy link

Failed server tests

  • com.appsmith.server.helpers.UserUtilsTest#makeInstanceAdministrator_WhenUserAlreadyAdmin_MaintainsPermissionsSuccessfully

@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20239685411.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

@github-actions
Copy link

Failed server tests

  • com.appsmith.server.helpers.UserUtilsTest#makeInstanceAdministrator_WhenUserAlreadyAdmin_MaintainsPermissionsSuccessfully

@salevine
Copy link
Contributor Author

  1. Still not able to see Edit or other actions on Application card in the Favorites.
  2. I am still able to favorite more than 50 Apps after the last set of changes.
  3. Also can we reduce the size of the favorite icon slightly. Reason is we support the name of the App upto 3 lines, after which its shown as truncated (...). On reducing the icon size slightly, it wouldnt cover the name in such scenarios and would look better. We can even get the help of a designer/PM to comment on how it should look. Screenshot below
Screenshot 2025-12-15 at 3 39 29 PM

This hadn't been committed yet. It's building the DP now with that fix. The favorite icon is 16x16 which is the same size as the other icons.

@github-actions
Copy link

Failed server tests

  • com.appsmith.server.helpers.UserUtilsTest#makeInstanceAdministrator_WhenUserAlreadyAdmin_MaintainsPermissionsSuccessfully

1 similar comment
@github-actions
Copy link

Failed server tests

  • com.appsmith.server.helpers.UserUtilsTest#makeInstanceAdministrator_WhenUserAlreadyAdmin_MaintainsPermissionsSuccessfully

@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20247455203.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/20598271075.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

@github-actions
Copy link

Deploy-Preview-URL: https://ce-41462.dp.appsmith.com

@github-actions github-actions bot removed the Stale label Dec 30, 2025
Comment on lines 1160 to 1170
// 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];
}
Copy link
Contributor

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__";
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok . Did this. The magic strings are gone and new constant is used FAVORITES_KEY

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ashit-rath - I am not sure what callout you are talking about. That was just a comment .

Comment on lines 1162 to 1167
const favoritesWorkspace = {
id: "__favorites__",
name: "Favorites",
isVirtual: true,
userPermissions: [],
};
Copy link
Contributor

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: [],
    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added export const DEFAULT_FAVORITES_WORKSPACE = {
id: FAVORITES_KEY,
name: "Favorites",
isVirtual: true,
userPermissions: [] as string[],
};

and replaced inline with the constants

[ReduxActionTypes.FETCH_FAVORITE_APPLICATIONS_INIT]: (
draftState: SelectedWorkspaceReduxState,
) => {
draftState.loadingStates.isFetchingApplications = true;
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

favorites is being treated like a regular workspace - so we reused this without creating a new loading state. If you really wanted, we could create a different state. Seemed simpler to keep one

if (workspaceId === "__favorites__") {
yield put({
type: ReduxActionTypes.SET_CURRENT_WORKSPACE,
payload: {
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with changes above


if (isValidResponse) {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const rawApplications = (response as any).data;
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok. done

Comment on lines 100 to 103
} else {
// Non-successful API response – notify reducers so loading state is cleared.
yield put(fetchFavoriteApplicationsError());
}
Copy link
Contributor

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

Copy link
Contributor Author

@salevine salevine Jan 23, 2026

Choose a reason for hiding this comment

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

The else block handles the case when validateResponse returns false, which happens when the response has AXIOS_CONNECTION_ABORTED_CODE (connection aborted). It ensures loading state is cleared in that scenario.

If you really don't like this block, we could let the catch block handle errors - but this is cleaner.

Comment on lines 70 to 97
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,
};
},
);
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.. switched to trust the server logic.

@@ -0,0 +1,30 @@
import { createSelector } from "reselect";
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. done

@github-actions
Copy link

This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

@github-actions github-actions bot added the Stale label Jan 12, 2026
@github-actions
Copy link

This PR has been closed because of inactivity.

@github-actions github-actions bot closed this Jan 19, 2026
@salevine salevine reopened this Jan 23, 2026
@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@app/client/src/ce/selectors/applicationSelectors.tsx`:
- Around line 250-258: The selector getFavoriteApplications should guard against
undefined inputs and use a Set for faster lookups: ensure the inputs from
getApplications and getFavoriteApplicationIds are defaulted to empty
arrays/values (e.g., treat allApps as [] and favoriteIds as [] if undefined),
create a Set from favoriteIds (e.g., favoriteIdSet) and use
favoriteIdSet.has(app.id) in the filter, then sort the filtered results by name
as before; update references to allApps and favoriteIds in
getFavoriteApplications to use these defaults and the Set-based membership
check.

@github-actions
Copy link

Failed server tests

  • com.external.plugins.SnowflakePluginTest#
  • com.external.plugins.SmtpPluginTest#
  • com.external.plugins.ChatCommandTest#
  • com.external.plugins.OpenAIPluginTest#
  • com.external.plugins.VisionCommandTest#
  • com.external.plugins.EmbeddingCommandTest#
  • com.external.plugins.AnthropicPluginTest#
  • com.external.plugins.GoogleAiPluginTest#
  • com.external.plugins.GenerateContentCommandTest#
  • com.external.plugins.services.AiFeatureServiceFactoryTest#
  • com.external.plugins.services.FileUtilTest#
  • com.external.plugins.services.HeadersUtilTest#
  • com.external.plugins.services.FieldValidationHelperTest#
  • com.external.plugins.services.features.TextGenerationServiceImplTest#
  • com.external.plugins.services.features.TextClassificationServiceImplTest#
  • com.external.plugins.services.features.TextSummarizationServiceImplTest#
  • com.external.plugins.services.features.TextEntityExtractionServiceImplTest#
  • com.external.plugins.services.features.ImageClassificationServiceImplTest#
  • com.external.plugins.services.features.ImageCaptioningServiceImplTest#
  • com.external.plugins.services.features.ImageEntityExtractionServiceImplTest#
  • com.external.plugins.AwsLambdaPluginTest#
  • com.external.plugins.DatabricksPluginTest#
  • GsonUnorderedToOrderedSerializationTest#
  • com.appsmith.git.converters.GsonDoubleToLongConverterTest#
  • com.appsmith.git.helpers.DSLTransformerHelperTest#
  • com.appsmith.git.helpers.FileUtilsImplTest#
  • com.external.plugins.GraphQLPluginTest#testValidGraphQLApiExecutionWithQueryVariablesWithHttpGet

@github-actions github-actions bot removed the Stale label Jan 23, 2026
@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

1 similar comment
@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/21300017121.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

@github-actions
Copy link

Deploy-Preview-URL: https://ce-41462.dp.appsmith.com

@salevine salevine requested a review from subrata71 January 26, 2026 03:00
Addressing the backend comments with fixes
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🤖 Fix all issues with AI agents
In `@app/client/src/pages/Applications/ApplicationCard.tsx`:
- Line 110: In ApplicationCard, the component uses
useSelector(getApplicationList) which returns a search-filtered list and can
omit the current app, breaking the permission fallback; change the selector to
use useSelector(getApplications) (the unfiltered selector) so the
allApplications lookup used by the permission fallback logic in ApplicationCard
(the code that falls back to finding permissions when direct props are missing)
will always find the app. Ensure you update the const allApplications
declaration to call getApplications instead of getApplicationList.

In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java`:
- Line 577: Replace the null-check/ternary for favoriteId with the same
empty-string-safe check used elsewhere: use
StringUtils.isEmpty(application.getBaseId()) to decide whether to fall back to
application.getId(). Update the assignment for favoriteId in
ApplicationPageServiceCEImpl (the line that declares String favoriteId) to
mirror the pattern used at other sites (lines that use
StringUtils.isEmpty(baseId)) so empty strings are treated like nulls.
♻️ Duplicate comments (2)
app/client/src/pages/Applications/ApplicationCard.tsx (1)

217-223: Workaround for missing userPermissions — root cause still unaddressed.

This fallback pattern addresses a symptom but not the underlying issue. A past reviewer also noted this concern. Consider investigating why Favorites views receive applications without userPermissions populated in the first place.

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserDataServiceCEImpl.java (1)

430-479: Non-atomic toggle can lose updates under concurrency.
The fetch‑modify‑save flow can drop favorites when two toggles race; prefer atomic $addToSet/$pull updates with a guarded limit check.

const theme = useContext(ThemeContext);
const isSavingName = useSelector(getIsSavingAppName);
const isErroredSavingName = useSelector(getIsErroredSavingAppName);
const allApplications = useSelector(getApplicationList);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Bug: getApplicationList filters by search keyword, breaking fallback lookup.

getApplicationList applies fuzzy search filtering. When a search is active, the filtered list may not contain the application being rendered, causing the permission fallback at lines 219-223 to fail silently.

Use getApplications (the unfiltered selector) instead.

🐛 Proposed fix
-import {
-  getApplicationList,
-  getIsSavingAppName,
+import {
+  getIsSavingAppName,
   getIsErroredSavingAppName,
 } from "ee/selectors/applicationSelectors";
+import { getApplications } from "ee/selectors/applicationSelectors";
-  const allApplications = useSelector(getApplicationList);
+  const allApplications = useSelector(getApplications);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const allApplications = useSelector(getApplicationList);
const allApplications = useSelector(getApplications);
🤖 Prompt for AI Agents
In `@app/client/src/pages/Applications/ApplicationCard.tsx` at line 110, In
ApplicationCard, the component uses useSelector(getApplicationList) which
returns a search-filtered list and can omit the current app, breaking the
permission fallback; change the selector to use useSelector(getApplications)
(the unfiltered selector) so the allApplications lookup used by the permission
fallback logic in ApplicationCard (the code that falls back to finding
permissions when direct props are missing) will always find the app. Ensure you
update the const allApplications declaration to call getApplications instead of
getApplicationList.

Mono<AclPermission> actionPermissionMono =
actionPermission.getDeletePermission().cache();
Mono<AclPermission> pagePermissionMono = pagePermission.getDeletePermission();
String favoriteId = application.getBaseId() != null ? application.getBaseId() : application.getId();
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for getBaseId() usage patterns
echo "=== getBaseId() usage patterns ==="
rg -n 'getBaseId\(\)' --type=java -B2 -A2 | head -80

Repository: appsmithorg/appsmith

Length of output: 11577


🏁 Script executed:

#!/bin/bash
# Check for StringUtils usage in this file and similar patterns
echo "=== StringUtils.hasLength usage in project ==="
rg -n 'StringUtils\.hasLength' --type=java | head -30

Repository: appsmithorg/appsmith

Length of output: 4939


🏁 Script executed:

#!/bin/bash
# Check for similar null-check patterns with IDs in ApplicationPageServiceCEImpl.java
echo "=== ApplicationPageServiceCEImpl patterns ==="
rg -n '!= null.*:.*getId\(\)' app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java

Repository: appsmithorg/appsmith

Length of output: 222


🏁 Script executed:

#!/bin/bash
# Check BaseId class/interface definition to understand its nature
fd -e java | xargs rg -l 'getBaseId\(\)' | head -5
echo "=== Checking Application class for baseId field ==="
rg -n 'class Application|baseId' app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Application.java | head -20

Repository: appsmithorg/appsmith

Length of output: 685


Use StringUtils.isEmpty() for consistency with similar checks in this file.

Line 211 and lines 738-740 already use StringUtils.isEmpty(baseId) for the same getBaseId() fallback pattern. Line 577 should align with that approach.

Suggested change
-        String favoriteId = application.getBaseId() != null ? application.getBaseId() : application.getId();
+        String favoriteId = StringUtils.isEmpty(application.getBaseId()) ? application.getId() : application.getBaseId();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
String favoriteId = application.getBaseId() != null ? application.getBaseId() : application.getId();
String favoriteId = StringUtils.isEmpty(application.getBaseId()) ? application.getId() : application.getBaseId();
🤖 Prompt for AI Agents
In
`@app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java`
at line 577, Replace the null-check/ternary for favoriteId with the same
empty-string-safe check used elsewhere: use
StringUtils.isEmpty(application.getBaseId()) to decide whether to fall back to
application.getId(). Update the assignment for favoriteId in
ApplicationPageServiceCEImpl (the line that declares String favoriteId) to
mirror the pattern used at other sites (lines that use
StringUtils.isEmpty(baseId)) so empty strings are treated like nulls.

@salevine
Copy link
Contributor Author

/build-deploy-preview skip-tests=true

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/21364208936.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

Fixed. Added UserDataService to the ApplicationPageServiceImpl constructor and passed it to the super call. The compilation error should be resolved.
@github-actions
Copy link

Deploy-Preview-URL: https://ce-41462.dp.appsmith.com

@github-actions
Copy link

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/21364208936.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 41462.
recreate: .

@github-actions
Copy link

Deploy-Preview-URL: https://ce-41462.dp.appsmith.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants