feat(backend): sort applications and workspaces alphabetically#41253
feat(backend): sort applications and workspaces alphabetically#41253
Conversation
…etic. Also added feature flag control of this functionality.
WalkthroughAdds a feature flag Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant AC as ApplicationControllerCE
participant AS as ApplicationServiceCEImpl
participant FF as FeatureFlagService
participant WS as WorkspaceService
participant AR as ApplicationRepository
U->>AC: GET /home?workspaceId=...
AC->>AS: findByWorkspaceIdAndBaseApplicationsForHome(workspaceId)
AS->>FF: check(release_alphabetical_ordering_enabled)
alt Flag ON
AS->>WS: load workspace (READ)
WS-->>AS: Workspace
AS->>AR: fetch apps for workspace
AR-->>AS: Flux<Application>
AS->>AS: sort by name (case-insensitive) & filter base apps
else Flag OFF
AS->>AS: findByWorkspaceIdAndBaseApplicationsInRecentlyUsedOrder(...)
end
AS-->>AC: Flux<Application>
AC-->>U: ResponseDTO<List<Application>>
sequenceDiagram
autonumber
actor U as User
participant WC as WorkspaceControllerCE
participant US as UserWorkspaceServiceCEImpl
participant FF as FeatureFlagService
participant WR as WorkspaceRepository
U->>WC: GET /workspaces/home
WC->>US: getUserWorkspacesForHome()
US->>FF: check(release_alphabetical_ordering_enabled)
alt Flag ON
US->>WR: fetch user's workspaces with READ
WR-->>US: Flux<Workspace>
US->>US: sort by name (case-insensitive)
else Flag OFF
US->>US: getUserWorkspacesByRecentlyUsedOrder()
end
US-->>WC: Mono<List<Workspace>>
WC-->>U: ResponseDTO<List<Workspace>>
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/17935697765. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java (2)
424-435: Null/locale-safe alphabetical sort + deterministic tie-breakerAvoid NPE on null names and default-locale pitfalls. Also add a stable secondary key.
Apply:
- return workspaceService - .getAll(workspacePermission.getReadPermission()) - .sort(Comparator.comparing(workspace -> workspace.getName().toLowerCase())) - .collectList(); + return workspaceService + .getAll(workspacePermission.getReadPermission()) + .sort( + Comparator.comparing( + ws -> StringUtils.hasText(ws.getName()) ? ws.getName().trim() : null, + Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER) + ).thenComparing(Workspace::getId) + ) + .collectList();
424-435: If you rename the interface method, mirror it hereChange method name to plural if adopting the naming refactor.
- public Mono<List<Workspace>> getUserWorkspaceInAlphabeticalOrder() { + public Mono<List<Workspace>> getUserWorkspacesInAlphabeticalOrder() {app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCE.java (1)
32-33: API surface addition looks fineSignature aligns with existing naming and return type. Consider adding Javadoc mirroring the “recently used” method.
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java (1)
112-125: Flag‑gated branch reads well; consider minor cleanupLooks correct. If you adopt the pluralized service name, update the call here.
- .getUserWorkspaceInAlphabeticalOrder() + .getUserWorkspacesInAlphabeticalOrder()app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java (1)
137-150: Flag‑gated ordering switch: LGTMBehavior toggles cleanly without API changes. Optional: extract a small helper to reduce ResponseDTO mapping duplication.
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (1)
218-251: Filter before sort + null/locale-safe comparator
- Filter first to reduce sort work.
- Avoid
toLowerCase()NPE/locale issues; add stable tie‑breaker.- return workspaceMono.thenMany( - this.findByWorkspaceId(workspaceId, applicationPermission.getReadPermission()) - .sort(Comparator.comparing(application -> application.getName().toLowerCase())) - .filter(application -> { - /* - * Filter applications based on the following criteria: - * - Applications that are not connected to Git. - * - Applications that, when connected, revert with default branch only. - */ - return !GitUtils.isArtifactConnectedToGit(application.getGitArtifactMetadata()) - || GitUtils.isDefaultBranchedArtifact(application.getGitArtifactMetadata()); - })); + return workspaceMono.thenMany( + this.findByWorkspaceId(workspaceId, applicationPermission.getReadPermission()) + .filter(app -> + !GitUtils.isArtifactConnectedToGit(app.getGitArtifactMetadata()) + || GitUtils.isDefaultBranchedArtifact(app.getGitArtifactMetadata()) + ) + .sort( + Comparator.comparing( + app -> StringUtils.hasText(app.getName()) ? app.getName().trim() : null, + Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER) + ).thenComparing(Application::getId) + ) + );
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java(2 hunks)app/server/appsmith-server/src/test/java/com/appsmith/server/services/UserWorkspaceServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java (1)
GitUtils(18-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-spotless / spotless-check
- GitHub Check: server-unit-tests / server-unit-tests
🔇 Additional comments (1)
app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java (1)
34-37: Flag addition looks goodName and placement are consistent; doc comment helps. No further action.
...er/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCE.java
Outdated
Show resolved
Hide resolved
...ver/appsmith-server/src/test/java/com/appsmith/server/services/UserWorkspaceServiceTest.java
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/17936490131. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/WorkspaceController.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ApplicationController.java (1)
14-14: Incorrect — FeatureFlagService is not forwarded to the CE superclassApplicationControllerCE is annotated with @requiredargsconstructor and does not declare FeatureFlagService; ApplicationController injects FeatureFlagService for local use but does not pass it to super(...). Confirm there is a single FeatureFlagService bean to avoid Spring injection ambiguity (see ApplicationControllerCE, ApplicationController, and FeatureFlagService in app/server/appsmith-server/src/main/java).
Likely an incorrect or invalid review comment.
|
|
||
| import com.appsmith.server.constants.Url; | ||
| import com.appsmith.server.controllers.ce.WorkspaceControllerCE; | ||
| import com.appsmith.server.services.FeatureFlagService; |
There was a problem hiding this comment.
🧩 Analysis chain
LGTM: FeatureFlagService added and propagated to CE.
Signature and super(...) call align with the feature‑flag pattern used elsewhere. Please verify the CE constructor includes the flag service and remains singular.
Quick verification script:
Also applies to: 15-15, 17-17
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ce_file="$(fd -a 'WorkspaceControllerCE.java' | head -n1)"
[[ -n "${ce_file:-}" ]] || { echo "WorkspaceControllerCE.java not found"; exit 1; }
echo "== CE constructor context =="
rg -nP -C2 'public\s+WorkspaceControllerCE\s*\(' "$ce_file"
echo "== Ensure FeatureFlagService is a constructor param =="
rg -nP 'WorkspaceControllerCE\s*\([^)]*FeatureFlagService' "$ce_file" >/dev/null || {
echo "FeatureFlagService missing in CE constructor"; exit 2; }
echo "== Ensure only one CE constructor (expected 1) =="
ctor_count="$(rg -nP 'WorkspaceControllerCE\s*\(' "$ce_file" | wc -l | tr -d ' ')"
echo "Constructor count: $ctor_count"
sub_file='app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/WorkspaceController.java'
echo "== Subclass super(...) call context =="
rg -n 'super\s*\(' "$sub_file" -C1
echo "== Ensure FeatureFlagService is passed to super(...) =="
rg -nP 'super\s*\([^)]*FeatureFlagService' "$sub_file" >/dev/null || {
echo "FeatureFlagService not passed to super"; exit 3; }Length of output: 28
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find CE file
ce_file="$(rg --hidden --files -g '**/WorkspaceControllerCE.java' | head -n1 || true)"
if [[ -z "${ce_file:-}" ]]; then
echo "WorkspaceControllerCE.java not found"
exit 1
fi
echo "CE file: $ce_file"
echo "== CE class declaration =="
rg -nP -C2 'class\s+WorkspaceControllerCE\b' "$ce_file" || true
echo "== CE constructor matches =="
rg -nP -n '(^|\s)WorkspaceControllerCE\s*\(' "$ce_file" -n -C2 || true
if rg -nP 'public\s+WorkspaceControllerCE\s*\([^)]*FeatureFlagService' "$ce_file" >/dev/null 2>&1; then
echo "FeatureFlagService present in CE constructor signature"
else
echo "FeatureFlagService missing in CE constructor signature"
fi
ctor_count="$(rg -nP '(^|\s)WorkspaceControllerCE\s*\(' "$ce_file" | wc -l | tr -d ' ' || true)"
echo "Constructor count: ${ctor_count:-0}"
# Find subclass file
sub_file="$(rg --hidden --files -g '**/WorkspaceController.java' | head -n1 || true)"
if [[ -z "${sub_file:-}" ]]; then
echo "WorkspaceController.java not found"
exit 1
fi
echo "Sub file: $sub_file"
echo "== subclass class declaration =="
rg -nP -C2 'class\s+WorkspaceController\b' "$sub_file" || true
echo "== super(...) call context =="
rg -n -C1 'super\s*\(' "$sub_file" || true
if rg -nP 'super\s*\([^)]*FeatureFlagService' "$sub_file" >/dev/null 2>&1; then
echo "FeatureFlagService passed to super(...)"
else
echo "FeatureFlagService NOT passed to super(...)"
fi
exit 0Length of output: 1048
CE constructor missing FeatureFlagService — fix required
WorkspaceControllerCE (app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java) is annotated with @requiredargsconstructor and only has final WorkspaceService and UserWorkspaceService; it does NOT accept FeatureFlagService. WorkspaceController (app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/WorkspaceController.java) calls super(..., featureFlagService) (line 17) — constructor mismatch / compile error.
- Add
private final FeatureFlagService featureFlagService;to WorkspaceControllerCE (or provide an explicit CE constructor) so the parent constructor matches the super(...) call; ensure only one CE constructor exists.
🤖 Prompt for AI Agents
In
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/WorkspaceController.java
around line 5, the subclass calls super(..., featureFlagService) but the parent
WorkspaceControllerCE constructor does not accept FeatureFlagService, causing a
constructor mismatch; to fix, add a private final FeatureFlagService
featureFlagService field to WorkspaceControllerCE (or create a single explicit
CE constructor that includes FeatureFlagService) so the generated/explicit
constructor signature matches the super(...) call, and ensure there is only one
CE constructor to avoid duplicates.
|
Deploy-Preview-URL: https://ce-41253.dp.appsmith.com |
Failed server tests
|
…tUserWorkspacesInAlphabeticalOrder
...ppsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java
Outdated
Show resolved
Hide resolved
.../appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java (3)
424-428: Clarify Javadoc to state case-insensitive ordering.
Doc says “alphabetical order” but implementation is case-insensitive. Make it explicit.
431-435: Use a locale-agnostic, null-safe, deterministic comparator; prefer collectSortedList().
CurrenttoLowerCase()uses default locale (e.g., Turkish-I issue), allocates strings, and ties are non-deterministic.Apply this diff:
- return workspaceService - .getAll(workspacePermission.getReadPermission()) - .sort(Comparator.comparing(workspace -> workspace.getName().toLowerCase())) - .collectList(); + return workspaceService + .getAll(workspacePermission.getReadPermission()) + .collectSortedList( + Comparator.comparing( + Workspace::getName, + Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER) + ).thenComparing(Workspace::getId) + );
429-435: Feature-flag wiring OK; add Unicode tests and use locale-aware sorting
- Controllers already gate the new behaviour behind FeatureFlagEnum.release_alphabetical_ordering_enabled (WorkspaceControllerCE.workspacesForHome, ApplicationControllerCE.findByWorkspaceIdAndRecentlyUsedOrder).
- Service currently sorts with Comparator.comparing(w -> w.getName().toLowerCase()) (UserWorkspaceServiceCEImpl) — this is locale-sensitive and can misorder non‑ASCII (e.g., Turkish İ/ı, accents). Use a locale-aware Collator (or at minimum toLowerCase(Locale.ROOT)) for deterministic Unicode ordering.
- Tests: UserWorkspaceServiceTest covers ASCII ordering but lacks non‑ASCII cases and appears to call getUserWorkspaceInAlphabeticalOrder (singular) vs. the interface/impl method getUserWorkspacesInAlphabeticalOrder — align names and add tests with accented and locale‑specific names (İ, ß, ñ, é) to validate behavior.
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java (1)
112-125: Add fail-safe default and remove duplication in reactive chain.Default to existing behavior (recently used) if flag resolution errors, and map to ResponseDTO once.
Apply:
- Mono<Boolean> isAlphabeticalOrderingEnabled = featureFlagService.check(FeatureFlagEnum.release_alphabetical_ordering_enabled); - return isAlphabeticalOrderingEnabled.flatMap(isEnabled -> { - if (isEnabled) { - // If alphabetical ordering is enabled, then we need to sort the workspaces in alphabetical order - return userWorkspaceService - .getUserWorkspacesInAlphabeticalOrder() - .map(workspaces -> new ResponseDTO<>(HttpStatus.OK, workspaces)); - } else { - // If alphabetical ordering is disabled, then we need to sort the workspaces in recently used order - return userWorkspaceService - .getUserWorkspacesByRecentlyUsedOrder() - .map(workspaces -> new ResponseDTO<>(HttpStatus.OK, workspaces)); - } - }); + return featureFlagService + .check(FeatureFlagEnum.release_alphabetical_ordering_enabled) + .onErrorReturn(false) // fail-safe: preserve existing behavior on flag errors + .flatMap(isEnabled -> isEnabled + ? userWorkspaceService.getUserWorkspacesInAlphabeticalOrder() + : userWorkspaceService.getUserWorkspacesByRecentlyUsedOrder()) + .map(workspaces -> new ResponseDTO<>(HttpStatus.OK, workspaces));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java(3 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCE.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (3)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java (1)
35-35: LGTM on the Comparator import.
Used by the new sort comparator. No issues.app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java (2)
10-10: LGTM on new imports.Imports for FeatureFlagService and FeatureFlagEnum look correct.
Also applies to: 13-13
38-38: DI wiring: ensure a bean implements FeatureFlagServiceWorkspaceControllerCE injects a final FeatureFlagService via @requiredargsconstructor — confirm a Spring bean implements FeatureFlagService (e.g., @service or an @bean) or add one to avoid startup failure; repo search returned no matches, so manual verification required.
…rom controllers for Workspace and Applications.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java (2)
435-440: Make alphabetical sort locale-safe, null-safe, and deterministic.toLowerCase() is locale-sensitive and NPE-prone. Prefer CASE_INSENSITIVE_ORDER with nullsLast and add a stable tie-breaker (id).
Apply this diff:
- return workspaceService - .getAll(workspacePermission.getReadPermission()) - .sort(Comparator.comparing(workspace -> workspace.getName().toLowerCase())) - .collectList(); + return workspaceService + .getAll(workspacePermission.getReadPermission()) + .sort( + Comparator.comparing( + Workspace::getName, + Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER) + ).thenComparing( + Workspace::getId, + Comparator.nullsLast(Comparator.naturalOrder()) + ) + ) + .collectList();
451-460: Harden feature‑flag path with graceful fallback and simplify.If the flag check errors, the home page shouldn’t fail. Fall back to “recently used” and inline the Mono.
Apply this diff:
- public Mono<List<Workspace>> getUserWorkspacesForHome() { - Mono<Boolean> isAlphabeticalOrderingEnabled = featureFlagService.check(FeatureFlagEnum.release_alphabetical_ordering_enabled); - return isAlphabeticalOrderingEnabled.flatMap(isEnabled -> { - if (isEnabled) { - // If alphabetical ordering is enabled, then we need to sort the workspaces in alphabetical order - return getUserWorkspacesInAlphabeticalOrder(); - } else { - // If alphabetical ordering is disabled, then we need to sort the workspaces in recently used order - return getUserWorkspacesByRecentlyUsedOrder(); - } - }); - } + public Mono<List<Workspace>> getUserWorkspacesForHome() { + return featureFlagService + .check(FeatureFlagEnum.release_alphabetical_ordering_enabled) + .onErrorResume(e -> { + log.warn("Feature flag check failed for release_alphabetical_ordering_enabled; falling back to recently used order", e); + return Mono.just(false); + }) + .flatMap(isEnabled -> + isEnabled ? getUserWorkspacesInAlphabeticalOrder() + : getUserWorkspacesByRecentlyUsedOrder() + ); + }app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (2)
244-256: Filter before sort; use a null‑safe, case‑insensitive comparator (avoids NPE/locale pitfalls).
- Sorting before filtering does extra work.
toLowerCase()can NPE on null names and is locale‑sensitive.Suggested change:
- return workspaceMono.thenMany( - this.findByWorkspaceId(workspaceId, applicationPermission.getReadPermission()) - .sort(Comparator.comparing(application -> application.getName().toLowerCase())) - .filter(application -> { + return workspaceMono.thenMany( + this.findByWorkspaceId(workspaceId, applicationPermission.getReadPermission()) + .filter(application -> { /* * Filter applications based on the following criteria: * - Applications that are not connected to Git. * - Applications that, when connected, revert with default branch only. */ return !GitUtils.isArtifactConnectedToGit(application.getGitArtifactMetadata()) || GitUtils.isDefaultBranchedArtifact(application.getGitArtifactMetadata()); - })); + }) + .sort(Comparator.comparing( + Application::getName, + Comparator.nullsLast(String.CASE_INSENSITIVE_ORDER) + )));
268-277: Add resilient fallback for feature-flag check (default to recently-used ordering on FF error)FeatureFlagEnum.release_alphabetical_ordering_enabled exists (app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java:37). Wrap the FF check with .onErrorReturn(false) to avoid regressions; apply the same change in UserWorkspaceServiceCEImpl as well.
- Mono<Boolean> isAlphabeticalOrderingEnabled = featureFlagService.check(FeatureFlagEnum.release_alphabetical_ordering_enabled); - return isAlphabeticalOrderingEnabled.flatMapMany(isEnabled -> { + return featureFlagService + .check(FeatureFlagEnum.release_alphabetical_ordering_enabled) + .onErrorReturn(false) + .flatMapMany(isEnabled -> { if (isEnabled) { // If alphabetical ordering is enabled, then we need to sort the applications in alphabetical order return findByWorkspaceIdAndBaseApplicationsInAlphabeticalOrder(workspaceId); } else { // If alphabetical ordering is disabled, then we need to sort the applications in recently used order return findByWorkspaceIdAndBaseApplicationsInRecentlyUsedOrder(workspaceId); } - }); + });
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java(6 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCE.java(1 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCE.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/ApplicationControllerCE.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/controllers/ce/WorkspaceControllerCE.java
- app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCE.java
🧰 Additional context used
🧬 Code graph analysis (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (1)
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitUtils.java (1)
GitUtils(18-202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (3)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/UserWorkspaceServiceCEImpl.java (2)
16-16: LGTM: imports match new functionality.New imports for FeatureFlagService, FeatureFlagEnum, and Comparator align with the added logic.
Also applies to: 22-22, 37-37
60-60: Constructor wiring looks good.FeatureFlagService is injected and assigned as final; consistent with existing patterns.
Also applies to: 71-73, 81-81
app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (1)
96-131: FeatureFlagService wiring looks correct.Constructor injection + final field are appropriate for gating behavior.
|
/build-deploy-preview skip-tests=false |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/17943020457. |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserWorkspaceServiceImpl.java(2 hunks)app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/ApplicationServiceCECompatibleImpl.java(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: server-unit-tests / server-unit-tests
- GitHub Check: server-spotless / spotless-check
🔇 Additional comments (4)
app/server/appsmith-server/src/main/java/com/appsmith/server/services/UserWorkspaceServiceImpl.java (2)
33-35: Super constructor call updated correctly
Verified: UserWorkspaceServiceCEImpl's constructor declares FeatureFlagService as the final parameter, so forwarding permissionGroupPermission, featureFlagService to super(...) is correct.
22-24: FeatureFlagService DI wiring verified — single bean in contextInterface: app/server/appsmith-server/src/main/java/com/appsmith/server/services/FeatureFlagService.java; single implementation: app/server/appsmith-server/src/main/java/com/appsmith/server/services/FeatureFlagServiceImpl.java (annotated @component); both in package com.appsmith.server.services so UserWorkspaceServiceImpl needs no import and injection is unambiguous.
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/ApplicationServiceCECompatibleImpl.java (2)
8-8: Import for FeatureFlagService is correct.Looks good and consistent with the new dependency.
54-55: Double-check super(...) argument order matches ApplicationServiceCEImpl constructorConfirm the arguments passed to super(...) in app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce_compatible/ApplicationServiceCECompatibleImpl.java (around lines 54–55) are in the same order as the ApplicationServiceCEImpl constructor parameters in app/server/appsmith-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java (constructor ~line 101), paying special attention to ObservationRegistry vs FeatureFlagService and any constructor overloads.
...main/java/com/appsmith/server/services/ce_compatible/ApplicationServiceCECompatibleImpl.java
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/17944689551. |
Failed server tests
|
1 similar comment
Failed server tests
|
…esting instead of save.
Failed server tests
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/17947471369. |
|
Deploy-Preview-URL: https://ce-41253.dp.appsmith.com |
|
Checking the failures on this PR. |
Description
Made changes in backend to sort applications and workspaces in alphabetic order
Also added feature flag control to this functionality.
Fixes #31108
Automation
/test Workspace
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/17998282833
Commit: ff76753
Cypress dashboard.
Tags:
@tag.WorkspaceSpec:
Thu, 25 Sep 2025 06:09:23 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests