feat: Made changes to support datasources configuration page for workspace #41324
Conversation
WalkthroughThis pull request introduces a workspace datasources feature, enabling users to manage datasources from a dedicated workspace page. It adds Redux state tracking, initialization sagas, new route constants, UI components, header navigation, and conditional plugin filtering for workspace contexts, along with comprehensive E2E tests. Changes
Sequence DiagramsequenceDiagram
participant User
participant Browser as Browser/Router
participant WorkspaceDatasourcesPage as WorkspaceDatasourcesPage
participant Redux as Redux Store
participant Saga as Saga Middleware
participant Engine as WorkspaceDatasourceEngine
participant API as Backend API
User->>Browser: Navigate to /workspace/{id}/datasources
Browser->>WorkspaceDatasourcesPage: Mount component
WorkspaceDatasourcesPage->>Redux: Dispatch initWorkspaceDatasource({workspaceId})
Redux->>Redux: Set isWorkspaceDatasourceInitialized = false
Redux->>WorkspaceDatasourcesPage: Update state (initializing)
WorkspaceDatasourcesPage->>Browser: Render loading spinner
Redux->>Saga: Emit INITIALIZE_WORKSPACE_DATASOURCE action
Saga->>Engine: Create WorkspaceDatasourceEngine instance
Engine->>Engine: setupEngine() - start evaluation
Engine->>API: Fetch workspaces
API-->>Engine: Workspaces data
Engine->>Engine: loadWorkspace(workspaceId)
Engine->>API: Load plugins for workspace
API-->>Engine: Plugins data
Engine->>API: Load datasources for workspace
API-->>Engine: Datasources data
Engine->>API: Load mock datasources (if not airgapped)
API-->>Engine: Mock datasources data
Engine->>API: Fetch plugin form configs
API-->>Engine: Form configs
Engine->>Redux: Signal completion
Redux->>Redux: Set isWorkspaceDatasourceInitialized = true
Redux->>WorkspaceDatasourcesPage: Update state (ready)
WorkspaceDatasourcesPage->>Browser: Render two-pane layout with datasources list
WorkspaceDatasourcesPage->>Browser: Hide loading spinner
Note over Saga,Engine: On error: Dispatch INITIALIZE_WORKSPACE_DATASOURCE_ERROR
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (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 |
tomjose92
changed the title
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/18841755423. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (14)
app/client/src/ce/pages/Applications/WorkspaceMenu.tsx (1)
169-181: Use RouteBuilder + add test id/a11y role.
- Prefer central URL helper to avoid hard-coded paths.
- Add data-testid and role="menuitem" for QA/a11y parity with other items.
Apply within this block:
- <CustomMenuItem - className="workspace-menu-item" + <CustomMenuItem + className="workspace-menu-item" + data-testid="t--workspace-datasources" + role="menuitem" onClick={() => - getOnSelectAction(DropdownOnSelectActions.REDIRECT, { - path: `/workspace/${workspace.id}/datasources`, - }) + getOnSelectAction(DropdownOnSelectActions.REDIRECT, { + path: workspaceDatasourcesURL({ workspaceId: workspace.id }), + }) } >And add import at top:
+import { workspaceDatasourcesURL } from "ce/RouteBuilder";app/client/src/pages/workspace/index.tsx (1)
20-29: Prefer Route.render to avoid remounts and re-created component functions.
Keeps props typing simple and avoids new component identity per render.- <SentryRoute - component={({ - match, - }: { - match: { params: { workspaceId: string } }; - }) => ( - <WorkspaceDatasourcesPage workspaceId={match.params.workspaceId} /> - )} - path={`${path}/:workspaceId/datasources`} - /> + <SentryRoute + path={`${path}/:workspaceId/datasources`} + render={({ match }: { match: { params: { workspaceId: string } } }) => ( + <WorkspaceDatasourcesPage workspaceId={match.params.workspaceId} /> + )} + />app/client/src/ce/RouteBuilder.ts (1)
200-206: Consider aligning with the existing URL building pattern.These functions use template literals directly, while most other URL builders in this file use
urlBuilder.build(). For consistency and to leverage any URL handling logic (encoding, normalization) thaturlBuilderprovides, consider using the established pattern.Apply this diff to align with existing patterns:
-export const workspaceDatasourcesURL = (workspaceId: string): string => - `/workspace/${workspaceId}/datasources`; - -export const workspaceDatasourceEditorURL = ( - workspaceId: string, - datasourceId: string, -): string => `/workspace/${workspaceId}/datasources/${datasourceId}`; +export const workspaceDatasourcesURL = (workspaceId: string): string => + urlBuilder.build({ + suffix: `workspace/${workspaceId}/datasources`, + }); + +export const workspaceDatasourceEditorURL = ( + workspaceId: string, + datasourceId: string, +): string => + urlBuilder.build({ + suffix: `workspace/${workspaceId}/datasources/${datasourceId}`, + });Note: Review urlBuilder.build implementation to ensure it handles absolute paths starting with "workspace" correctly, or adjust the pattern accordingly.
app/client/src/ce/sagas/workspaceIDESagas.ts (2)
24-28: Await editor reset before initialization to avoid race conditions.RESET_EDITOR_REQUEST may have async side-effects. Wait for RESET_EDITOR_SUCCESS before proceeding.
-import { all, put, call, takeLatest } from "redux-saga/effects"; +import { all, put, call, take, takeLatest } from "redux-saga/effects"; ... yield put(resetEditorRequest()); + yield take(ReduxActionTypes.RESET_EDITOR_SUCCESS); yield call(workspaceEngine.setupEngine);
39-43: Simplify watcher.all([...]) with a single effect is unnecessary. Yield takeLatest directly.
-export default function* watchWorkspaceIDESagas() { - yield all([ - takeLatest(ReduxActionTypes.INITIALIZE_WORKSPACE_IDE, startWorkspaceIDE), - ]); -} +export default function* watchWorkspaceIDESagas() { + yield takeLatest(ReduxActionTypes.INITIALIZE_WORKSPACE_IDE, startWorkspaceIDE); +}app/client/src/ce/reducers/uiReducers/editorReducer.tsx (1)
45-62: Reset workspace IDE flag on RESET_EDITOR_REQUEST.Keeps flags consistent across editor switches.
[ReduxActionTypes.RESET_EDITOR_REQUEST]: (state: EditorReduxState) => { return { ...state, + isWorkspaceEditorInitialized: false, currentPageId: undefined,app/client/src/ce/actions/workspaceIDEActions.ts (1)
7-10: LGTM. Consider typing the action.Optionally return ReduxAction for stricter typing.
app/client/src/pages/workspace/WorkspaceDatasourceEditor.tsx (1)
16-24: Guard selector result and avoid magic string.
- Use optional chaining to avoid reading id from undefined.
- Replace "unused_env" with a shared constant.
useEffect(() => { - if ( - currentEnvironmentDetails.id && - currentEnvironmentDetails.id !== "unused_env" - ) { - dispatch(setCurrentEditingEnvironmentID(currentEnvironmentDetails.id)); - } -}, [dispatch, currentEnvironmentDetails.id]); + const envId = currentEnvironmentDetails?.id; + if (envId && envId !== UNUSED_ENV_ID) { + dispatch(setCurrentEditingEnvironmentID(envId)); + } +}, [dispatch, currentEnvironmentDetails?.id]);Add/import:
const UNUSED_ENV_ID = "unused_env";(or import an existing constant).app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
95-106: Add error fallback UI.Show an error state if initialization fails (pair with reducer error flag).
-if (!isWorkspaceEditorInitialized) { +const initError = useSelector( + (state: DefaultRootState) => state.ui.editor.workspaceEditorInitError, +); +if (initError) { + return ( + <PageContainer> + <CenteredWrapper style={{ height: `calc(100vh - ${IDE_HEADER_HEIGHT}px)` }}> + <div>Failed to initialize Workspace Datasources. Please retry.</div> + </CenteredWrapper> + </PageContainer> + ); +} +if (!isWorkspaceEditorInitialized) { return (app/client/src/ce/entities/Engine/WorkspaceEditorEngine.ts (1)
104-108: Optional: include workspaceId in SUCCESS payload.Helps consumers correlate which workspace initialized (useful for telemetry or race-proofing).
*completeChore() { yield put({ - type: ReduxActionTypes.INITIALIZE_WORKSPACE_IDE_SUCCESS, + type: ReduxActionTypes.INITIALIZE_WORKSPACE_IDE_SUCCESS, + // payload: { workspaceId }, // if you propagate it here }); }app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (4)
170-179: Add alt text to datasource icons (a11y).Provide meaningful alt; fallback to plugin or datasource name.
- startIcon: ( - <DatasourceIcon - src={getAssetUrl(pluginImages[data.pluginId])} - /> - ), + startIcon: ( + <DatasourceIcon + alt={`${data.name} icon`} + src={getAssetUrl(pluginImages[data.pluginId])} + /> + ),
36-40: Optional: avoid magic numbers in heights.Use a shared header height token to compute PaneBody height for consistency with IDE.
- height: calc(100vh - 120px); + /* import { IDE_HEADER_HEIGHT } from "@appsmith/ads"; */ + height: calc(100vh - var(--ide-header-height, 120px));And set the CSS variable at a higher container if available.
70-75: useCallback deps completeness.goToDatasource depends on history. It’s stable, but include it to silence exhaustive-deps lint (if enabled).
- const goToDatasource = useCallback( + const goToDatasource = useCallback( (id: string) => { history.push(workspaceDatasourceEditorURL(workspaceId, id)); }, - [workspaceId], + [workspaceId, history], );
124-137: Show spinner copy via i18n message key.Replace hardcoded “Loading workspace permissions...” with a messages constant for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
app/client/src/ce/IDE/Interfaces/IDETypes.ts(1 hunks)app/client/src/ce/IDE/constants/routes.ts(1 hunks)app/client/src/ce/RouteBuilder.ts(1 hunks)app/client/src/ce/actions/workspaceIDEActions.ts(1 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(2 hunks)app/client/src/ce/entities/Engine/WorkspaceEditorEngine.ts(1 hunks)app/client/src/ce/entities/URLRedirect/URLAssembly.ts(7 hunks)app/client/src/ce/pages/Applications/WorkspaceMenu.tsx(1 hunks)app/client/src/ce/reducers/uiReducers/editorReducer.tsx(3 hunks)app/client/src/ce/sagas/index.tsx(2 hunks)app/client/src/ce/sagas/workspaceIDESagas.ts(1 hunks)app/client/src/constants/routes/baseRoutes.ts(1 hunks)app/client/src/ee/actions/workspaceIDEActions.ts(1 hunks)app/client/src/ee/entities/Engine/WorkspaceEditorEngine.ts(1 hunks)app/client/src/ee/sagas/workspaceIDESagas.ts(1 hunks)app/client/src/pages/workspace/WorkspaceCreateNewDatasourceTab.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourceEditor.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)app/client/src/pages/workspace/index.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
app/client/src/ce/actions/workspaceIDEActions.ts (1)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1302-1346)
app/client/src/ce/IDE/constants/routes.ts (1)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)
app/client/src/ce/sagas/workspaceIDESagas.ts (4)
app/client/src/actions/ReduxActionTypes.ts (1)
ReduxAction(9-12)app/client/src/ce/actions/workspaceIDEActions.ts (1)
InitWorkspaceIDEPayload(3-5)app/client/src/ce/entities/Engine/WorkspaceEditorEngine.ts (1)
WorkspaceEditorEngine(22-109)app/client/src/actions/initActions.ts (1)
resetEditorRequest(51-53)
app/client/src/ce/reducers/uiReducers/editorReducer.tsx (1)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1302-1346)
app/client/src/pages/workspace/WorkspaceDatasourceEditor.tsx (1)
app/client/src/ce/actions/environmentAction.ts (1)
setCurrentEditingEnvironmentID(7-10)
app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (6)
app/client/src/ce/selectors/entitiesSelector.ts (4)
getDatasources(86-88)getDatasourcesGroupedByPluginCategory(114-144)getPluginImages(605-623)getPlugins(90-91)app/client/src/ce/entities/FeatureFlag.ts (1)
FEATURE_FLAG(2-67)app/client/src/ce/RouteBuilder.ts (1)
workspaceDatasourceEditorURL(203-206)app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
getHasCreateDatasourcePermission(56-62)app/client/src/ce/constants/messages.ts (3)
DATASOURCE_BLANK_STATE_CTA(2360-2360)DATA_PANE_TITLE(2356-2356)DATASOURCE_LIST_BLANK_DESCRIPTION(2357-2358)app/client/src/entities/Datasource/index.ts (1)
Datasource(154-162)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (5)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getDatasourceUsageCountForApp(1769-1787)app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/ce/actions/workspaceIDEActions.ts (1)
initWorkspaceIDE(7-10)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
WorkspaceDataSidePane(52-197)app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)
app/client/src/ce/RouteBuilder.ts (1)
app/client/cypress/support/commands.js (1)
workspaceId(396-396)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
app/client/cypress/support/commands.js (1)
workspaceId(396-396)
app/client/src/ce/entities/Engine/WorkspaceEditorEngine.ts (5)
app/client/src/ce/actions/environmentAction.ts (1)
fetchingEnvironmentConfigs(13-26)app/client/src/actions/ReduxActionTypes.ts (1)
ReduxAction(9-12)app/client/src/sagas/InitSagas.ts (1)
failFastApiCalls(147-167)app/client/src/entities/Engine/index.ts (2)
PluginsNotFoundError(61-61)PluginFormConfigsNotFoundError(62-62)app/client/src/actions/pluginActions.ts (1)
fetchPluginFormConfigs(22-29)
app/client/src/pages/workspace/index.tsx (2)
app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
WorkspaceDatasourcesPage(59-145)
⏰ 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). (5)
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
🔇 Additional comments (12)
app/client/src/ce/sagas/index.tsx (1)
60-60: Saga wiring looks good.
Placement alongside workspaceSagas is reasonable. No changes needed.Also applies to: 79-79
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
245-252: Workspace URL resolution needs refinement, but proposed fixes require design clarification.The concerns about empty workspaceId and context-coupled suffix rewriting are valid. However, the proposed diffs have structural issues that need addressing:
Confirmed issues:
resolveEntityIdForWorkspace()returns""instead of throwing—creates/workspace//datasourcesrisk- Suffix rewrite gates on
isWorkspaceContext()only, not explicitworkspaceIdintentIssues with proposed solution:
generateBasePath()receives onlybasePageIdandmode; diffs assume it receivesbuilderParams—misaligned with actual signature- No evidence of
datasource/suffix pattern in RouteBuilder (uses"datasource","datasources","datasource/{id}")- Diff E duplicates suffix logic; unclear whether
generateBasePath()should make the workspace vs. app decisionRecommended approach:
- Fix
resolveEntityIdForWorkspace()to throw on missing ID (align withresolveEntityIdForApp())- Clarify method responsibilities: Should
generateBasePath()be refactored to receivebuilderParamsand handle the workspace/app decision, or shouldbuild()handle routing logic?- Unify suffix handling: Gate on
!!builderParams.workspaceId || isWorkspaceContext()consistently—not just in one locationRedesign the routing layer to pass explicit intent (workspaceId) through method calls rather than relying on URL parsing alone.
app/client/src/constants/routes/baseRoutes.ts (1)
31-31: LGTM!The new workspace datasources route constant follows the established naming pattern and integrates cleanly with existing workspace routes.
app/client/src/ce/IDE/constants/routes.ts (1)
45-45: LGTM!Adding the Workspace IDE type to the IDEBasePaths mapping is consistent with the existing pattern for None and UIPackage types.
app/client/src/ee/actions/workspaceIDEActions.ts (1)
1-1: LGTM!Standard EE/CE facade pattern that allows for potential future extensions while maintaining a clean architecture boundary.
app/client/src/ee/entities/Engine/WorkspaceEditorEngine.ts (1)
1-1: LGTM!The EE facade for WorkspaceEditorEngine follows the established pattern and maintains clean architectural boundaries.
app/client/src/ee/sagas/workspaceIDESagas.ts (1)
1-2: LGTM!The EE saga facade follows the established pattern and properly re-exports both named and default exports from the CE layer.
app/client/src/ce/constants/ReduxActionConstants.tsx (2)
414-415: LGTM!The new workspace IDE action constants follow the established naming convention and are consistent with existing IDE initialization patterns (e.g., INITIALIZE_EDITOR).
483-483: LGTM!The error action constant is properly placed in IDEActionErrorTypes and follows the naming pattern.
app/client/src/pages/workspace/WorkspaceCreateNewDatasourceTab.tsx (1)
4-6: No issues found. CreateNewDatasourceTab properly accesses workspace context via Redux.The component is Redux-connected through
connect(mapStateToProps), which automatically injects workspace data (viagetCurrentAppWorkspace,getCurrentPageId, and other selectors) as props. The wrapper component correctly omits props since the child component obtains all required context from Redux state.app/client/src/ce/entities/Engine/WorkspaceEditorEngine.ts (1)
40-47: No action required—empty type is intentional.The
typeproperty in thefetchingEnvironmentConfigsaction creator is intentionally left as an empty string to imitate the functionality of the enterprise edition. The action properly propagates through sagas that handle environment configuration fetching. The code at lines 40–47 is correct as written.app/client/src/pages/workspace/WorkspaceDatasourceEditor.tsx (1)
5-5: Fix empty Redux action type in setCurrentEditingEnvironmentID.The action type is confirmed empty in
app/client/src/ce/actions/environmentAction.tsline 8. Redux actions require a non-emptytypeproperty for proper routing and reducer handling. Replace the empty string with a descriptive action type constant.⛔ Skipped due to learnings
Learnt from: AmanAgarwal041 PR: appsmithorg/appsmith#29266 File: app/client/src/ce/actions/environmentAction.ts:15-16 Timestamp: 2024-07-26T21:12:57.228Z Learning: The `type` property in the `fetchingEnvironmentConfigs` action creator is intentionally left as an empty string to imitate the functionality of the enterprise edition, as per the user's clarification.
| [ReduxActionTypes.INITIALIZE_WORKSPACE_IDE]: (state: EditorReduxState) => { | ||
| return { ...state, isWorkspaceEditorInitialized: false }; | ||
| }, | ||
| [ReduxActionTypes.INITIALIZE_WORKSPACE_IDE_SUCCESS]: ( | ||
| state: EditorReduxState, | ||
| ) => { | ||
| return { ...state, isWorkspaceEditorInitialized: true }; | ||
| }, |
There was a problem hiding this comment.
Handle initialization error to avoid infinite spinner.
Add INITIALIZE_WORKSPACE_IDE_ERROR handler and store the error so UI can show a fallback.
[ReduxActionTypes.INITIALIZE_WORKSPACE_IDE]: (state: EditorReduxState) => {
return { ...state, isWorkspaceEditorInitialized: false };
},
[ReduxActionTypes.INITIALIZE_WORKSPACE_IDE_SUCCESS]: (
state: EditorReduxState,
) => {
return { ...state, isWorkspaceEditorInitialized: true };
},
+ [ReduxActionErrorTypes.INITIALIZE_WORKSPACE_IDE_ERROR]: (
+ state: EditorReduxState,
+ action: ReduxAction<{ error: unknown }>,
+ ) => {
+ return {
+ ...state,
+ isWorkspaceEditorInitialized: false,
+ workspaceEditorInitError: action.payload?.error,
+ };
+ },Additionally add the field to state shape and initial state:
// initialState
onLoadActionExecution: false,
+ workspaceEditorInitError: undefined, export interface EditorReduxState {
...
onLoadActionExecution?: boolean;
+ workspaceEditorInitError?: unknown;📝 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.
| [ReduxActionTypes.INITIALIZE_WORKSPACE_IDE]: (state: EditorReduxState) => { | |
| return { ...state, isWorkspaceEditorInitialized: false }; | |
| }, | |
| [ReduxActionTypes.INITIALIZE_WORKSPACE_IDE_SUCCESS]: ( | |
| state: EditorReduxState, | |
| ) => { | |
| return { ...state, isWorkspaceEditorInitialized: true }; | |
| }, | |
| [ReduxActionTypes.INITIALIZE_WORKSPACE_IDE]: (state: EditorReduxState) => { | |
| return { ...state, isWorkspaceEditorInitialized: false }; | |
| }, | |
| [ReduxActionTypes.INITIALIZE_WORKSPACE_IDE_SUCCESS]: ( | |
| state: EditorReduxState, | |
| ) => { | |
| return { ...state, isWorkspaceEditorInitialized: true }; | |
| }, | |
| [ReduxActionErrorTypes.INITIALIZE_WORKSPACE_IDE_ERROR]: ( | |
| state: EditorReduxState, | |
| action: ReduxAction<{ error: unknown }>, | |
| ) => { | |
| return { | |
| ...state, | |
| isWorkspaceEditorInitialized: false, | |
| workspaceEditorInitError: action.payload?.error, | |
| }; | |
| }, |
🤖 Prompt for AI Agents
In app/client/src/ce/reducers/uiReducers/editorReducer.tsx around lines 69 to
76, add a new ReduxActionTypes.INITIALIZE_WORKSPACE_IDE_ERROR case that sets
isWorkspaceEditorInitialized to false and stores the error payload on state
(e.g., workspaceInitError or initializeWorkspaceError) so the UI can stop the
spinner and show a fallback; also add that error field to the EditorReduxState
type/shape and initialize it in the reducer's initial state (null or undefined
by default), and ensure the SUCCESS and INITIALIZE handlers clear/reset the
error.
| yield put({ | ||
| type: ReduxActionErrorTypes.INITIALIZE_WORKSPACE_IDE_ERROR, | ||
| payload: { | ||
| error, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Surface error state for UI.
INITIALIZE_WORKSPACE_IDE_ERROR is dispatched, but there’s no reducer handling to unblock or show an error. Pair this with a reducer flag and UI fallback. See reducer comment for concrete diff.
🤖 Prompt for AI Agents
In app/client/src/ce/sagas/workspaceIDESagas.ts around lines 30 to 36, the saga
dispatches INITIALIZE_WORKSPACE_IDE_ERROR but the app has no reducer or state
flag to surface this error to the UI; add a reducer case and state flag (e.g.,
workspaceIDEInitializationError or isWorkspaceIDEInitError) to store the error
payload and clear any "initializing" flag, update the reducer's initial state
accordingly, and ensure components read this flag to show a fallback/error UI
and stop loading spinners; also add an action handler to clear the error when
retrying or on successful init.
| return ( | ||
| <DataSourceEditor | ||
| applicationId={workspaceApplicationId} | ||
| datasourceId={datasourceId} | ||
| isWorkspaceContext | ||
| pageId={workspacePageId} | ||
| /> | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify DataSourceEditor workspace mode assumptions.
We pass synthetic applicationId/pageId with isWorkspaceContext. Confirm DataSourceEditor suppresses app/page fetches in this mode.
🏁 Script executed:
#!/bin/bash
# Ensure DataSourceEditor gates app/page dependent fetches behind `isWorkspaceContext`.
rg -nP --type=ts --type=tsx -C3 '\bfunction\s+DataSourceEditor\b|\bconst\s+DataSourceEditor\b' app/client/src \
| sed -n '1,120p'
rg -nP --type=ts --type=tsx -C3 'isWorkspaceContext' app/client/src/pages/Editor/DataSourceEditor*Length of output: 55
🏁 Script executed:
#!/bin/bash
# Find DataSourceEditor component files
fd -e ts -e tsx 'DataSourceEditor' app/client/src
# Search for DataSourceEditor definition without type flag
rg -n 'function DataSourceEditor|const DataSourceEditor' app/client/src -A 5
# Search for isWorkspaceContext usage in Editor directory
rg -n 'isWorkspaceContext' app/client/src/pages/Editor -C 3Length of output: 0
🏁 Script executed:
#!/bin/bash
# Broader search for DataSource-related files
find app/client/src -type f -name "*DataSource*" 2>/dev/null | head -20
# List structure of app/client/src if it exists
ls -la app/client/src/ 2>/dev/null | head -20
# Search broadly for isWorkspaceContext across entire codebase
rg 'isWorkspaceContext' -n 2>/dev/null | head -30
# Search for any Editor directory
find app/client -type d -name "Editor" 2>/dev/nullLength of output: 2312
🏁 Script executed:
#!/bin/bash
# Check what WorkspaceDatasourceEditor imports and uses
cat app/client/src/pages/workspace/WorkspaceDatasourceEditor.tsx
# Search for component definitions accepting isWorkspaceContext
rg 'isWorkspaceContext' app/client/src/pages/Editor -B 5 -A 10
# Look for Datasource related components in Editor directories
find app/client/src/pages/Editor -type f \( -name "*.tsx" -o -name "*.ts" \) | xargs grep -l "Datasource" 2>/dev/nullLength of output: 5898
🏁 Script executed:
#!/bin/bash
# View DataSourceEditor component definition
cat app/client/src/pages/Editor/DataSourceEditor/index.tsx
# Search for isWorkspaceContext usage in DataSourceEditor directory
rg 'isWorkspaceContext' app/client/src/pages/Editor/DataSourceEditor -B 3 -A 3
# Look for useEffect and fetch patterns in the directory
rg 'useEffect|fetch|applicationId|pageId' app/client/src/pages/Editor/DataSourceEditor/index.tsx -nLength of output: 40922
DataSourceEditor must handle isWorkspaceContext to suppress app/page dependency fetches.
The isWorkspaceContext prop passed from WorkspaceDatasourceEditor is never declared in the component's ReduxStateProps interface and never checked. The component unconditionally calls getApplicationByIdFromWorkspaces(state, applicationId) and passes both applicationId and pageId to subforms without guarding based on workspace context. This means synthetic IDs intended to avoid unnecessary API calls are still processed as legitimate application/page references, potentially triggering fetches.
Add isWorkspaceContext to the ReduxStateProps interface, extract it in mapStateToProps, and conditionally skip app/page lookups when true. Alternatively, prevent the synthetic IDs from reaching selectors that trigger API calls by gating the affected logic.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/18842120808. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (3)
71-73: Pass IDE_TYPE.Workspace for correct usage labels.The selector generates usage strings like "N queries in this app" when passed
IDE_TYPE.App. Since this is a workspace context, passIDE_TYPE.Workspaceso labels read "…in this workspace".-const dsUsageMap = useSelector((state) => - getDatasourceUsageCountForApp(state, IDE_TYPE.App), -); +const dsUsageMap = useSelector((state) => + getDatasourceUsageCountForApp(state, IDE_TYPE.Workspace), +);
81-86: Initialization won't re-run when switching workspaces.The
!isWorkspaceEditorInitializedguard prevents re-initialization when navigating from one workspace to another. The flag stays true from the first workspace, so workspace B never initializes.-useEffect(() => { - if (workspaceId && !isWorkspaceEditorInitialized) { - dispatch(initWorkspaceIDE({ workspaceId })); - } -}, [dispatch, workspaceId, isWorkspaceEditorInitialized]); +useEffect(() => { + if (workspaceId) { + dispatch(initWorkspaceIDE({ workspaceId })); + } +}, [dispatch, workspaceId]);
120-124: Route path case mismatch with navigation calls.This route uses "/new" (lowercase), but WorkspaceDataSidePane pushes "/NEW" (uppercase) on lines 43 and 102. Navigation will fail. Standardize on lowercase "/new" in WorkspaceDataSidePane.tsx.
🧹 Nitpick comments (2)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
89-106: Consider adding error handling for failed initialization.Currently there's no feedback if workspace fetch or IDE initialization fails. Users might see an indefinite spinner. Consider adding error boundaries or timeout handling.
123-135: Inconsistent workspaceId prop.Line 123 renders
WorkspaceCreateNewDatasourceTabwithout passingworkspaceId, but line 135 passes it explicitly. If the component extracts it from route params, the explicit prop is redundant. If not, line 123 is missing it.For consistency, either remove the explicit prop on line 135:
-<WorkspaceCreateNewDatasourceTab workspaceId={workspaceId} /> +<WorkspaceCreateNewDatasourceTab />Or add it to line 123:
-render={() => <WorkspaceCreateNewDatasourceTab />} +render={() => <WorkspaceCreateNewDatasourceTab workspaceId={workspaceId} />}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (5)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getDatasourceUsageCountForApp(1769-1787)app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/ce/actions/workspaceIDEActions.ts (1)
initWorkspaceIDE(7-10)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
WorkspaceDataSidePane(52-197)app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)
⏰ 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). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
70-73: Use Workspace context for usage strings.Pass IDE_TYPE.Workspace so labels read “…in this workspace”.
- const dsUsageMap = useSelector((state) => - getDatasourceUsageCountForApp(state, IDE_TYPE.App), - ); + const dsUsageMap = useSelector((state) => + getDatasourceUsageCountForApp(state, IDE_TYPE.Workspace), + );
81-86: Re-initialize IDE on workspace change.The guard can skip init when switching workspaces. Dispatch on every workspaceId change.
-useEffect(() => { - if (workspaceId && !isWorkspaceEditorInitialized) { - dispatch(initWorkspaceIDE({ workspaceId })); - } -}, [dispatch, workspaceId, isWorkspaceEditorInitialized]); +useEffect(() => { + if (workspaceId) { + dispatch(initWorkspaceIDE({ workspaceId })); + } +}, [dispatch, workspaceId]);
🧹 Nitpick comments (6)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
132-135: Make the default list route exact.Prevents accidental matches if route order shifts.
- <SentryRoute - path="/workspace/:workspaceId/datasources" - render={() => <WorkspaceCreateNewDatasourceTab />} - /> + <SentryRoute + exact + path="/workspace/:workspaceId/datasources" + render={() => <WorkspaceCreateNewDatasourceTab />} + />
121-124: Avoid magic “NEW” segment.Extract to a constant or reuse a RouteBuilder helper to keep callers and routes in sync.
- path="/workspace/:workspaceId/datasources/NEW" + path={`/workspace/:workspaceId/datasources/${NEW_SEGMENT}`}Consider adding workspaceDatasourceNewURL(workspaceId) in RouteBuilder and using it here.
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (3)
254-263: Don’t infer context from window.location. Prefer explicit params.Coupling to window pathname makes URL building dependent on the current page and breaks SSR/tests. Consider treating presence of builderParams.workspaceId as the workspace context switch, falling back to location only if absent.
Minimal approach: add an optional parameter or internal flag set in build():
- Determine
const inWorkspace = !!builderParams.workspaceId || this.isWorkspaceContext();- Pass workspaceId explicitly to generateBasePathForWorkspace rather than re-parsing location.
341-349: Suffix rewrite for “datasource/” is brittle.This special-case logic is easy to miss for other workspace routes and mixes concerns. Prefer dedicated helpers or route constants for workspace URLs and keep URLBuilder generic.
- Move workspace-datasource path assembly to RouteBuilder (already added) and remove this special-case.
- If retained, also handle exact "datasource" (no trailing slash) to avoid edge cases.
30-32: Document workspaceId usage.Clarify that providing workspaceId opts into workspace routing so callers aren’t surprised.
Add JSDoc on URLBuilderParams.workspaceId explaining precedence over window location and expected consumers.
app/client/src/ce/RouteBuilder.ts (1)
200-207: Unify with route constants and add a helper for "NEW".
- Constants
WORKSPACE_URLandWORKSPACE_DATASOURCES_PAGE_URLexist in baseRoutes and should be imported to avoid drift.- Add
workspaceDatasourceNewURLhelper to consolidate the three hardcoded/datasources/NEWstrings currently in WorkspaceDatasourcesPage.tsx and WorkspaceDataSidePane.tsx.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/src/ce/RouteBuilder.ts(1 hunks)app/client/src/ce/entities/URLRedirect/URLAssembly.ts(6 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)app/client/src/pages/workspace/index.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/workspace/index.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
app/client/src/ce/RouteBuilder.ts (1)
app/client/cypress/support/commands.js (1)
workspaceId(396-396)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
app/client/cypress/support/commands.js (1)
workspaceId(396-396)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (5)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getDatasourceUsageCountForApp(1769-1787)app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/ce/actions/workspaceIDEActions.ts (1)
initWorkspaceIDE(7-10)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
WorkspaceDataSidePane(52-197)app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)
⏰ 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). (5)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
| resolveEntityIdForWorkspace(builderParams: URLBuilderParams): string { | ||
| // Extract workspaceId from current URL if not provided | ||
| if (builderParams?.workspaceId) { | ||
| return builderParams.workspaceId; | ||
| } | ||
|
|
||
| const currentUrl = window.location.pathname; | ||
| const workspaceMatch = currentUrl.match(/^\/workspace\/([^\/]+)/); | ||
|
|
||
| return workspaceMatch ? workspaceMatch[1] : ""; | ||
| } | ||
|
|
There was a problem hiding this comment.
Validate workspaceId; don’t fall back to empty string.
Returning "" yields “/workspace//…” URLs. Align with app path behavior and throw.
- const workspaceMatch = currentUrl.match(/^\/workspace\/([^\/]+)/);
-
- return workspaceMatch ? workspaceMatch[1] : "";
+ const workspaceMatch = currentUrl.match(/^\/workspace\/([^\/]+)/);
+ const id = workspaceMatch ? workspaceMatch[1] : "";
+ if (!id) {
+ throw new URIError("Missing workspaceId. Provide builderParams.workspaceId or navigate under /workspace/:workspaceId.");
+ }
+ return id;🤖 Prompt for AI Agents
In app/client/src/ce/entities/URLRedirect/URLAssembly.ts around lines 312 to
323, the resolver returns an empty string when no workspaceId is found which
produces malformed paths like "/workspace//…"; change the logic to validate and
require a workspaceId: if builderParams.workspaceId is present return it;
otherwise attempt to extract from window.location.pathname and if extraction
fails throw a clear Error (or assert) indicating missing workspaceId instead of
returning ""; ensure the thrown error message includes context (e.g., "Missing
workspaceId in URL or builderParams") so callers can handle or surface it.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/18848305560. |
|
Deploy-Preview-URL: https://ce-41324.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
75-78: Use Workspace context for usage strings.As previously noted, passing
IDE_TYPE.Appcauses usage labels to read "in this app". Update toIDE_TYPE.Workspaceso the UI displays "in this workspace" for consistency with the workspace context.- const dsUsageMap = useSelector((state) => - getDatasourceUsageCountForApp(state, IDE_TYPE.App), - ); + const dsUsageMap = useSelector((state) => + getDatasourceUsageCountForApp(state, IDE_TYPE.Workspace), + );
86-91: Re-initialize when workspaceId changes.As previously noted, the current guard prevents re-initialization when navigating between different workspaces. The
isWorkspaceEditorInitializedflag remains true after the first workspace is loaded, so switching to a different workspace won't trigger initialization.- useEffect(() => { - if (workspaceId && !isWorkspaceEditorInitialized) { - dispatch(initWorkspaceIDE({ workspaceId })); - } - }, [dispatch, workspaceId, isWorkspaceEditorInitialized]); + useEffect(() => { + if (workspaceId) { + dispatch(initWorkspaceIDE({ workspaceId })); + } + }, [dispatch, workspaceId]);
🧹 Nitpick comments (3)
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (1)
330-335: Optional: Consider removing unnecessary optional chaining.The Plugin interface (from
entities/Plugin/index.ts) showspackageNameis a required field, so the optional chainingplugin?.packageNameis unnecessary. The variable namefinalFilteredMostPopularPluginscould be more descriptive (e.g.,workspaceFilteredPlugins), though the current name is acceptable.- // Filter out REST API from most popular plugins when on workspace datasources page - const finalFilteredMostPopularPlugins = isWorkspaceDatasourcesPage - ? filteredMostPopularPlugins.filter( - (plugin: Plugin) => plugin?.packageName !== PluginPackageName.REST_API, - ) - : filteredMostPopularPlugins; + // Filter out REST API from most popular plugins when on workspace datasources page + const workspaceFilteredPlugins = isWorkspaceDatasourcesPage + ? filteredMostPopularPlugins.filter( + (plugin: Plugin) => plugin.packageName !== PluginPackageName.REST_API, + ) + : filteredMostPopularPlugins;app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (1)
368-371: Consider using route matching utilities for robustness.The current implementation using
includes()will work for the expected paths but could theoretically match unintended URLs. For more robust path matching, consider using React Router'smatchPathor theuseLocationhook with explicit path comparison.Example refactor:
+import { useLocation, matchPath } from "react-router"; +import { WORKSPACE_DATASOURCES_PAGE_URL, WORKSPACE_DATASOURCE_EDITOR_PAGE_URL } from "constants/routes"; ... - // Check if we're on workspace datasources page - const isWorkspaceDatasourcesPage = - window.location.pathname.includes("/workspace/") && - window.location.pathname.includes("/datasources"); + const location = useLocation(); + const isWorkspaceDatasourcesPage = Boolean( + matchPath(location.pathname, { path: WORKSPACE_DATASOURCES_PAGE_URL }) || + matchPath(location.pathname, { path: WORKSPACE_DATASOURCE_EDITOR_PAGE_URL }) + );app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
123-141: Consider using lowercase "new" for route consistency.The route uses
/NEW(uppercase), which matches the navigation inWorkspaceDataSidePane. While this works, the convention in web routing typically favors lowercase path segments. Consider standardizing on/newacross both the route definition and all navigation calls for better consistency with common web practices.- path={`${WORKSPACE_DATASOURCES_PAGE_URL}/NEW`} + path={`${WORKSPACE_DATASOURCES_PAGE_URL}/new`}And update navigation calls in
WorkspaceDataSidePane.tsx:-history.push(`/workspace/${workspaceId}/datasources/NEW`); +history.push(`/workspace/${workspaceId}/datasources/new`);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/client/src/ce/pages/common/AppHeader.tsx(2 hunks)app/client/src/constants/routes/baseRoutes.ts(1 hunks)app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx(1 hunks)app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)app/client/src/pages/workspace/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/constants/routes/baseRoutes.ts
🧰 Additional context used
🧬 Code graph analysis (6)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (2)
app/client/src/pages/Editor/IntegrationEditor/util.ts (1)
filterSearch(11-20)app/client/src/ce/constants/messages.ts (1)
CREATE_NEW_DATASOURCE_REST_API(402-402)
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (2)
app/client/src/pages/Editor/AppsmithLink.tsx (1)
AppsmithLink(29-67)app/client/src/constants/routes/baseRoutes.ts (1)
APPLICATIONS_URL(8-8)
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (1)
app/client/src/entities/Plugin/index.ts (1)
Plugin(71-87)
app/client/src/pages/workspace/index.tsx (3)
app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/pages/common/PageWrapper.tsx (1)
PageWrapper(63-89)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
WorkspaceDatasourcesPage(64-146)
app/client/src/ce/pages/common/AppHeader.tsx (1)
app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_PAGE_URL(31-31)WORKSPACE_DATASOURCE_EDITOR_PAGE_URL(32-32)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (6)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getDatasourceUsageCountForApp(1769-1787)app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/ce/actions/workspaceIDEActions.ts (1)
initWorkspaceIDE(7-10)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
WorkspaceDataSidePane(52-197)app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_PAGE_URL(31-31)WORKSPACE_DATASOURCE_EDITOR_PAGE_URL(32-32)
⏰ 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). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
🔇 Additional comments (15)
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (1)
338-338: LGTM!The plugin selection correctly uses the workspace-filtered plugin list when displaying most popular plugins.
app/client/src/ce/pages/common/AppHeader.tsx (2)
19-20: LGTM!The new imports for workspace datasource routes and header component are correctly placed and follow the existing import patterns.
Also applies to: 25-25
49-56: LGTM!The new workspace datasource routes are properly integrated into the Switch statement and follow the existing routing patterns.
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (1)
373-386: LGTM!The visibility conditions correctly hide REST and GraphQL API creation options when on workspace datasources pages. The logic is consistently applied to both API types.
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (3)
1-7: LGTM!All imports are correctly placed and necessary for the component implementation.
9-16: LGTM!The fixed header styling is appropriate and uses the correct CSS variables for z-index and background color.
18-38: LGTM!The header component is well-structured and correctly implements the IDEHeader pattern with appropriate conditional rendering for the workspace name.
app/client/src/pages/workspace/index.tsx (4)
6-6: LGTM!The import for WorkspaceDatasourcesPage is correctly placed.
15-22: LGTM!The Settings route maintains the existing PageWrapper pattern and is correctly structured.
23-42: LGTM!The workspace datasources routes are correctly configured. Both routes properly extract
workspaceIdfrom route parameters and pass it toWorkspaceDatasourcesPage. The component handles its own layout, so not wrapping it inPageWrapperis appropriate.
43-49: LGTM!The default workspace page route maintains the existing PageWrapper pattern and is correctly positioned as the catch-all route.
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (4)
1-24: LGTM!All imports are appropriately organized and necessary for the component implementation.
27-58: LGTM!The styled components create an appropriate two-pane IDE layout that mirrors the AppIDE structure.
80-84: LGTM!The workspace selector correctly retrieves the current workspace by ID.
93-98: LGTM!The workspace fetching logic correctly ensures workspace data is loaded when needed.
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (2)
34-36: Remove or clarify the empty div.The empty div in IDEHeader.Right serves no clear purpose. Either remove it if IDEHeader.Right can be empty, or add a comment if it's a placeholder for future functionality.
<IDEHeader.Right> - <div /> </IDEHeader.Right>
9-16: WorkspaceDatasourceHeader height should be explicitly set to match padding-top.The page correctly applies
padding-top: ${IDE_HEADER_HEIGHT}pxonPageContainerto offset the fixed header. However,WorkspaceDatasourceHeaderdoesn't explicitly set its own height property. For alignment consistency, addheight: ${IDE_HEADER_HEIGHT}px;toStyledWorkspaceHeaderto ensure the header and content spacing remain in sync ifIDE_HEADER_HEIGHTchanges.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (2)
app/client/src/pages/Editor/AppsmithLink.tsx (1)
AppsmithLink(29-67)app/client/src/constants/routes/baseRoutes.ts (1)
APPLICATIONS_URL(8-8)
⏰ 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). (5)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
🔇 Additional comments (1)
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (1)
1-7: LGTM!Imports are clean and all used.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/pages/AppIDE/layouts/components/Header/index.tsx (2)
165-168: Consider adding a defensive check forcurrentWorkspace.id.The code checks
currentWorkspace.nameon line 163 but directly accessescurrentWorkspace.idwithout validation. If the id is undefined, the URL becomes malformed (/applications?workspaceId=undefined).Consider either:
- Adding
currentWorkspace.idto the condition on line 163- Using optional chaining with a fallback
- <> - <Link - className="mr-1.5" - to={`${APPLICATIONS_URL}?workspaceId=${currentWorkspace.id}`} - > + {currentWorkspace.id ? ( + <> + <Link + className="mr-1.5" + to={`${APPLICATIONS_URL}?workspaceId=${currentWorkspace.id}`} + > + {currentWorkspace.name} + </Link> + {"/"} + <EditorName + applicationId={applicationId} + className="t--application-name editable-application-name max-w-48" + defaultSavingState={ + isSavingName ? SavingState.STARTED : SavingState.NOT_STARTED + } + defaultValue={currentApplication?.name || ""} + editInteractionKind={EditInteractionKind.SINGLE} + editorName="Application" + fill + getNavigationMenu={useNavigationMenuData} + isError={isErroredSavingName} + isNewEditor={ + applicationList.filter((el) => el.id === applicationId) + .length > 0 + } + isPopoverOpen={isPopoverOpen} + onBlur={(value: string) => + updateApplicationDispatch(applicationId || "", { + name: value, + currentApp: true, + }) + } + setIsPopoverOpen={setIsPopoverOpen} + /> + </> + ) : null} - {currentWorkspace.name} - </Link> - {"/"} - <EditorName - applicationId={applicationId} - className="t--application-name editable-application-name max-w-48" - defaultSavingState={ - isSavingName ? SavingState.STARTED : SavingState.NOT_STARTED - } - defaultValue={currentApplication?.name || ""} - editInteractionKind={EditInteractionKind.SINGLE} - editorName="Application" - fill - getNavigationMenu={useNavigationMenuData} - isError={isErroredSavingName} - isNewEditor={ - applicationList.filter((el) => el.id === applicationId) - .length > 0 - } - isPopoverOpen={isPopoverOpen} - onBlur={(value: string) => - updateApplicationDispatch(applicationId || "", { - name: value, - currentApp: true, - }) - } - setIsPopoverOpen={setIsPopoverOpen} - /> - </>
167-167: Consider using a URL builder utility for query parameter construction.Manually constructing query parameters via template literals lacks type safety and proper URL encoding. Consider using
URLSearchParamsor a URL builder utility consistent with patterns elsewhere in the codebase (e.g.,viewerURLon line 144).Example with
URLSearchParams:const applicationsUrl = `${APPLICATIONS_URL}?${new URLSearchParams({ workspaceId: currentWorkspace.id }).toString()}`;Or define a URL builder function similar to other route builders in the codebase.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/pages/AppIDE/layouts/components/Header/index.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
app/client/src/pages/AppIDE/layouts/components/Header/index.tsx (1)
app/client/src/constants/routes/baseRoutes.ts (1)
APPLICATIONS_URL(8-8)
⏰ 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). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
|
This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
78-83: Re-initialize when workspaceId changes.The
!isWorkspaceEditorInitializedguard prevents re-initialization when navigating between different workspaces. Once initialized for workspace A, switching to workspace B will skip initialization because the flag is already true.Apply this diff to ensure initialization occurs for each workspace:
useEffect(() => { - if (workspaceId && !isWorkspaceEditorInitialized) { + if (workspaceId) { dispatch(initWorkspaceIDE({ workspaceId })); } -}, [dispatch, workspaceId, isWorkspaceEditorInitialized]); +}, [dispatch, workspaceId]);Note: If initialization should only occur once per workspace, track the initialized workspaceId separately rather than using a boolean flag.
🧹 Nitpick comments (1)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
114-118: Consider lowercase route convention.The route uses uppercase
/NEW, which is unconventional (most routes use lowercase/new). While this is consistent with the navigation inWorkspaceDataSidePane.tsx, lowercase is the standard convention for URLs.To align with web conventions, consider refactoring to lowercase:
- path={`${WORKSPACE_DATASOURCES_PAGE_URL}/NEW`} + path={`${WORKSPACE_DATASOURCES_PAGE_URL}/new`}And update the corresponding navigation in
WorkspaceDataSidePane.tsx:history.push(`/workspace/${workspaceId}/datasources/new`)Alternatively, extract to a shared constant to prevent future mismatches:
// In constants/routes.ts export const DATASOURCE_NEW_PATH = '/new'; // Usage path={`${WORKSPACE_DATASOURCES_PAGE_URL}${DATASOURCE_NEW_PATH}`}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/ce/IDE/constants/routes.ts(1 hunks)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/ce/IDE/constants/routes.ts
- app/client/src/pages/workspace/WorkspaceDataSidePane.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Aishwarya-U-R
Repo: appsmithorg/appsmith PR: 29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: Aishwarya-U-R
Repo: appsmithorg/appsmith PR: 29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
📚 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/pages/workspace/WorkspaceDatasourcesPage.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/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 29965
File: app/client/src/utils/storage.ts:129-129
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `migrateAppIdToEditorId` function in `app/client/src/utils/storage.ts` is intended to be called only when `appId` is present and `editorId` is not in the payload. This context should be considered in future reviews related to this function.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
🧬 Code graph analysis (1)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (4)
app/client/src/ce/actions/workspaceIDEActions.ts (1)
initWorkspaceIDE(7-10)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
WorkspaceDataSidePane(50-193)app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_PAGE_URL(31-31)WORKSPACE_DATASOURCE_EDITOR_PAGE_URL(32-32)
⏰ 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). (5)
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
🔇 Additional comments (1)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
85-103: No changes needed—code already handles undefined workspace gracefully.WorkspaceDataSidePane has its own
isFetchingCurrentWorkspaceguard (line 123) that displays a loading state while the workspace fetch completes. All workspace property accesses use safe optional chaining (currentWorkspace?.userPermissions), so rendering with undefined workspace is handled defensively. The parent'sisWorkspaceEditorInitializedgate and the child'sisFetchingCurrentWorkspacegate serve different purposes and work in tandem.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (1)
372-385: Add workspace context check to authApiPlugin visibility.The authenticated REST API (
authApiPlugin, lines 208-216) lacks the!isWorkspaceDatasourcesPagecheck that's applied to bothrestAPIVisibleandgraphQLAPIVisible. DBOrMostPopularPlugins.tsx also filters out REST API in workspace context (line 332), establishing a consistent pattern. Apply the same check to authApiPlugin at line 208 for consistency.
🧹 Nitpick comments (1)
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (1)
58-102: LGTM with minor suggestion.The plugin and datasource loading logic is correct, with proper airgap handling and error flows. The
failFastApiCallspattern is used appropriately.Consider including
workspaceIdin error messages for easier debugging:if (!initActionCalls) - throw new PluginsNotFoundError("Unable to fetch plugins"); + throw new PluginsNotFoundError(`Unable to fetch plugins for workspace ${workspaceId}`); // ... later ... if (!pluginFormCall) throw new PluginFormConfigsNotFoundError( - "Unable to fetch plugin form configs", + `Unable to fetch plugin form configs for workspace ${workspaceId}`, );Based on learnings, workspace IDs are obfuscated and safe to log.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/client/src/ce/sagas/workspaceIDESagas.ts(1 hunks)app/client/src/ee/sagas/workspaceIDESagas.ts(1 hunks)app/client/src/entities/Engine/WorkspaceEditorEngine.ts(1 hunks)app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx(2 hunks)app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/client/src/ce/sagas/workspaceIDESagas.ts
- app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Aishwarya-U-R
Repo: appsmithorg/appsmith PR: 29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Learnt from: Aishwarya-U-R
Repo: appsmithorg/appsmith PR: 29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
📚 Learning: 2024-10-11T09:28:32.636Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36622
File: app/client/src/entities/Engine/AppEditorEngine.ts:0-0
Timestamp: 2024-10-11T09:28:32.636Z
Learning: In the file `app/client/src/entities/Engine/AppEditorEngine.ts`, the IDs (`currentApplication.baseId`) and branches (`currentBranch`) are obfuscated, so it's acceptable to log them for debugging purposes.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
🧬 Code graph analysis (3)
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (5)
app/client/src/ce/actions/environmentAction.ts (1)
fetchingEnvironmentConfigs(13-26)app/client/src/actions/ReduxActionTypes.ts (1)
ReduxAction(9-12)app/client/src/sagas/InitSagas.ts (1)
failFastApiCalls(147-167)app/client/src/entities/Engine/index.ts (2)
PluginsNotFoundError(61-61)PluginFormConfigsNotFoundError(62-62)app/client/src/actions/pluginActions.ts (1)
fetchPluginFormConfigs(22-29)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (2)
app/client/src/pages/Editor/IntegrationEditor/util.ts (1)
filterSearch(11-20)app/client/src/ce/constants/messages.ts (1)
CREATE_NEW_DATASOURCE_REST_API(402-402)
app/client/src/ee/sagas/workspaceIDESagas.ts (2)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1302-1346)app/client/src/ce/sagas/workspaceIDESagas.ts (1)
startWorkspaceIDE(9-34)
🔇 Additional comments (7)
app/client/src/ee/sagas/workspaceIDESagas.ts (3)
1-1: LGTM! Standard EE/CE re-export pattern.The re-export statement correctly exposes CE layer functionality through the EE layer, following the established architecture pattern in the codebase.
3-5: LGTM! Import statements are correct.All imports follow the proper EE/CE conventions and reference the correct modules. The CE implementation of
startWorkspaceIDEincludes appropriate error handling with try-catch blocks.
7-11: LGTM! Saga wiring is correct.The saga properly uses
takeLatestto listen for workspace IDE initialization, which cancels stale initializations on rapid dispatches. The action typeINITIALIZE_WORKSPACE_IDEis defined in ReduxActionConstants, and the structure follows established saga patterns with proper delegation to the CE implementation.app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (1)
369-370: No issues found. The implementation is correct and consistent.The
urlBuilder.isWorkspaceContext()method correctly identifies workspace context by checking if the URL pathname starts with "/workspace". This detection pattern is consistently applied across bothAPIOrSaasPlugins.tsxandDBOrMostPopularPlugins.tsx, confirming the feature is intentionally designed to hide API creation options in workspace context.app/client/src/entities/Engine/WorkspaceEditorEngine.ts (3)
53-56: LGTM!Standard engine setup pattern: starts evaluation and resets autocomplete service.
104-108: LGTM!Clean completion signal dispatching success action.
23-28: Binding generator methods is valid and correctly preserves context.The search confirms these methods are called directly via
yield call()in workspaceIDESagas.ts and InitSagas.ts, and tests validate this pattern works correctly. Binding generator functions is a legitimate defensive pattern ensuringthiscontext is always preserved, even when method references are stored or passed around. No issues found.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/19262099578. |
|
Deploy-Preview-URL: https://ce-41324.dp.appsmith.com |
github-actions
bot
added
Datasources
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
62-70: Workspace-scoped datasource filtering still needed.This component uses
getDatasourceswhich returns all datasources from the store, not filtered by the current workspace. The default view may show incorrect data when a user has datasources in multiple workspaces.Consider using a workspace-filtered selector or passing
workspaceIdto filter the list.
96-101: Potential infinite fetch loop if workspace fetch fails.If
fetchAllWorkspacesfails,currentWorkspaceremains undefined, continuously triggering re-fetches. Add a loading/error state guard.Add a check like
isFetchingWorkspacesto prevent repeated dispatch attempts.app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (1)
96-107: Replacecy.wait(100)with deterministic waits on elements/aliases.The two hard waits conflict with the Cypress guidelines (“Avoid using cy.wait in code” / “Avoid using sleep functions”) and are unnecessary given the surrounding assertions and alias waits. Based on learnings, these static delays tend to create flakiness.
You can rely on Cypress’ built‑in retry behavior on the button instead:
- cy.wait(100); - cy.get(".t--add-datasource-button") - .first() - .should("be.visible") - .click({ force: true }); + cy.get(".t--add-datasource-button") + .first() + .should("be.visible") + .click({ force: true });And similarly in the later test:
- cy.wait(100); - cy.get(".t--add-datasource-button") - .first() - .should("be.visible") - .click({ force: true }); + cy.get(".t--add-datasource-button") + .first() + .should("be.visible") + .click({ force: true });Also applies to: 193-203
🧹 Nitpick comments (3)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
125-129: Consider lowercase route paths for consistency.Route uses
/NEW(uppercase). While internally consistent with the navigation inWorkspaceDataSidePane, lowercase URLs (/new) are the common convention. Consider standardizing if this is exposed to users.
83-87: Consider memoizing the workspace lookup.The inline selector recreates on each render. For minor optimization, consider
useMemoor extract to a reusable selector withworkspaceIdparameter.+import { useMemo } from "react"; + +const workspaces = useSelector(getFetchedWorkspaces); +const currentWorkspace = useMemo( + () => workspaces.find((ws) => ws.id === workspaceId), + [workspaces, workspaceId] +); -const currentWorkspace = useSelector((state: DefaultRootState) => { - const workspaces = getFetchedWorkspaces(state); - return workspaces.find((ws) => ws.id === workspaceId); -});app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (1)
31-42: Prefer locator variables anddata-*testids over raw class selectors.Throughout the spec, many selectors are raw class names (e.g.
.t--datasource,.t--data-blank-state,.t--add-datasource-button[-blank-screen],.t--save-datasource,.t--datasource-option-delete,.t--headers-array,.t--datasource-modal-save, etc.), passed directly as string literals tocy.get/cy.contains. As per coding guidelines, tests here should:
- Use locator variables instead of inline selector strings.
- Prefer
data-*attributes (e.g.data-testid) over CSS class selectors / paths.You’re already doing this in a few places (
[data-testid='t--workspace-datasources'],t--workspace-datasources-page,mockdatasource-name,datasource-name,t--context-menu-trigger), so for future iterations consider:
- Exposing these frequently reused selectors via page objects / ObjectsCore.
- Gradually migrating the
.t--*class selectors todata-testid‑based selectors.Not blocking, but aligning this spec with the locator guidelines will make it more resilient to UI refactors.
Also applies to: 58-60, 86-90, 102-107, 118-124, 143-145, 162-170, 189-191, 199-203, 220-227, 229-239
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts(1 hunks)app/client/src/ce/pages/Applications/WorkspaceMenu.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/ce/pages/Applications/WorkspaceMenu.tsx
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/**/**.*
⚙️ CodeRabbit configuration file
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
🧠 Learnings (20)
📚 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/pages/workspace/WorkspaceDatasourcesPage.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/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 29965
File: app/client/src/utils/storage.ts:129-129
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The `migrateAppIdToEditorId` function in `app/client/src/utils/storage.ts` is intended to be called only when `appId` is present and `editorId` is not in the payload. This context should be considered in future reviews related to this function.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 36665
File: app/client/src/IDE/Components/FileTab/styles.tsx:19-19
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Do not suggest replacing hardcoded `padding-top: 6px;` with design variables in `app/client/src/IDE/Components/FileTab/styles.tsx`.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 33863
File: app/client/src/pages/Editor/DataSourceEditor/DBForm.tsx:93-98
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The suggestion to optimize conditional rendering in `DatasourceDBEditor` is considered out of scope for the current issue related to making text selectable.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-06-26T06:22:15.976Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-06-26T06:22:15.976Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: sneha122
Repo: appsmithorg/appsmith PR: 30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2025-11-05T12:56:49.249Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts:0-0
Timestamp: 2025-11-05T12:56:49.249Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 4-6 depend on state (`page1Slug`, `page2Slug`) set in Tests 2-3. The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Learnt from: sagar-qa007
PR: appsmithorg/appsmith#34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-07-16T06:44:55.118Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep`, and other related sleep functions in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2025-11-05T12:58:32.486Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts:32-225
Timestamp: 2025-11-05T12:58:32.486Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 2-5 depend on state from previous tests (e.g., slug values modified in earlier tests). The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-07-24T08:29:41.208Z
Learnt from: abhishek-bandameedi
Repo: appsmithorg/appsmith PR: 35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-07-24T08:29:41.208Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-11-13T09:07:54.931Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js:44-47
Timestamp: 2024-11-13T09:07:54.931Z
Learning: When reviewing code in `app/client/cypress/**` paths, ensure that suggestions are appropriate for test code and not for `src` code. Avoid applying source code checks to test code unless relevant.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-11-13T09:11:36.959Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js:35-37
Timestamp: 2024-11-13T09:11:36.959Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js`, when writing Cypress end-to-end tests for the Image Widget in JavaScript, use `viewWidgetsPage.imageinner` as the selector when asserting 'src' and 'alt' attributes to maintain consistency.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: Aishwarya-U-R
Repo: appsmithorg/appsmith PR: 29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.114Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settingsTe_Spec.ts:33-33
Timestamp: 2024-10-08T15:32:34.114Z
Learning: Follow best practices for Cypress code and e2e automation:
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes, and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 35412
File: app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/mysql_spec.ts:99-99
Timestamp: 2024-10-08T15:32:39.374Z
Learning: In Cypress tests within the `app/client/cypress` directory, avoid using `agHelper.Sleep`, `this.Sleep`, and other related sleep functions to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable. Instead, use Cypress methods like `cy.get()`, `cy.contains()`, and `cy.intercept()` to wait for specific conditions.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-08-06T05:39:47.929Z
Learnt from: ApekshaBhosale
Repo: appsmithorg/appsmith PR: 35412
File: app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/mysql_spec.ts:99-99
Timestamp: 2024-08-06T05:39:47.929Z
Learning: In Cypress tests, avoid using `agHelper.Sleep` as it can lead to flaky tests and unnecessary delays. Instead, use Cypress methods like `cy.get()`, `cy.contains()`, and `cy.intercept()` to wait for specific conditions.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-07-16T08:25:23.799Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settingsTe_Spec.ts:33-33
Timestamp: 2024-07-16T08:25:23.799Z
Learning: Avoid using sleep functions like `agHelper.Sleep` and `this.Sleep` in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep` in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
🧬 Code graph analysis (2)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (5)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getDatasources(86-88)app/client/src/ce/actions/workspaceIDEActions.ts (1)
initWorkspaceIDE(7-10)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
WorkspaceDataSidePane(50-205)app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_PAGE_URL(31-31)WORKSPACE_DATASOURCE_EDITOR_PAGE_URL(32-32)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (1)
app/client/cypress/support/commands.js (1)
_(1071-1071)
⏰ 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). (5)
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
🔇 Additional comments (3)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
137-140: Route ordering looks correct.The default route properly comes last in the
Switch, ensuring more specific paths (/NEW,/:datasourceId) match first. Structure aligns with IDE patterns.
89-94: Initialization logic correctly re-triggers on workspace change.Dispatches
initWorkspaceIDEwheneverworkspaceIdchanges without checking the initialized flag, ensuring proper re-initialization when switching workspaces.app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (1)
4-241: Workspace datasource settings flows are well covered.The spec exercises blank state, connect CTA, create, delete, and discard/save flows with appropriate use of helpers and route aliases; overall structure and scenario coverage look solid.
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
Outdated
Show resolved
Hide resolved
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
Outdated
Show resolved
Hide resolved
| None: "None", | ||
| App: "App", | ||
| UIPackage: "UIPackage", | ||
| Workspace: "Workspace", |
There was a problem hiding this comment.
I don't think we should call this Workspace IDE. This is specific to datasources in a workspace and I believe we are not including anything else into this. Do you think we should do something like "WorkspaceDatasource"?
There was a problem hiding this comment.
@ashit-rath , if we call this WorkspaceDatasource, then the nomenclature of it would feel broken in a lot of these functions like URLAssembly, etc that checks by ideType.
We have to use those functions, but the name of the function might not suit because we are using WorkspaceDatasource.
I am thinking we can keep this as WorkspaceIDE since datasources can be managed. And later if there is scope for any other functionality for workspace specific, it can reuse the flow
There was a problem hiding this comment.
We are removing all traces of IDE
| const PageContainer = styled.div` | ||
| display: flex; | ||
| flex-direction: column; | ||
| height: 100vh; | ||
| overflow: hidden; | ||
| padding-top: ${IDE_HEADER_HEIGHT}px; | ||
| `; | ||
|
|
||
| // Use the SAME layout structure as AppIDE | ||
| const IDEContainer = styled.div` | ||
| display: grid; | ||
| grid-template-columns: 300px 1fr; | ||
| grid-template-rows: 100%; | ||
| overflow: hidden; | ||
| flex: 1; | ||
| `; | ||
|
|
||
| const LeftPane = styled.div` | ||
| background-color: var(--ads-v2-color-bg); | ||
| border-right: 1px solid var(--ads-v2-color-border); | ||
| overflow: hidden; | ||
| display: flex; | ||
| flex-direction: column; | ||
| `; | ||
|
|
||
| const MainPane = styled.div` | ||
| background-color: var(--ads-v2-color-bg); | ||
| overflow: hidden; | ||
| display: flex; | ||
| flex-direction: column; | ||
| `; |
There was a problem hiding this comment.
I believe all of these components should be already available. Check AppIDE StaticlLayout
There was a problem hiding this comment.
Yes, Left Pane and MainPane are available but it seems more complex than a simple styled component and specific to AppIDE
There was a problem hiding this comment.
Only using GridContainer from IDE/Components/LayoutComponents to replace IDEContainer
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (1)
317-320: Past feedback addressed - URL detection now uses IDE type.This approach correctly replaces the fragile string-based URL matching with proper IDE type detection.
app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
53-61: Scope datasources and groups byworkspaceId.This pane currently uses
getDatasources/getDatasourcesGroupedByPluginCategorywithout filtering byworkspaceId, so if the store ever contains datasources from multiple workspaces, this UI will show cross‑workspace entries. You already receiveworkspaceIdin props; use it to filter both the flat list and grouped map before rendering and before checking counts (e.g. for blank state and “+” icon visibility).One way to keep the change local:
-import React, { useCallback, useEffect, useState } from "react"; +import React, { useCallback, useEffect, useMemo, useState } from "react"; @@ - const datasources = useSelector(getDatasources); - const groupedDatasources = useSelector(getDatasourcesGroupedByPluginCategory); + const allDatasources = useSelector(getDatasources); + const groupedDatasources = useSelector(getDatasourcesGroupedByPluginCategory); + + const workspaceDatasources = useMemo( + () => allDatasources.filter((ds) => ds.workspaceId === workspaceId), + [allDatasources, workspaceId], + ); + + const workspaceGroupedDatasources = useMemo( + () => + Object.fromEntries( + Object.entries(groupedDatasources).map(([key, value]) => [ + key, + value.filter((ds) => ds.workspaceId === workspaceId), + ]), + ), + [groupedDatasources, workspaceId], + ); @@ - if (datasources.length === 0 || isConnectADatasourcePage) { + if (workspaceDatasources.length === 0 || isConnectADatasourcePage) { @@ - {datasources.length === 0 ? ( + {workspaceDatasources.length === 0 ? ( @@ - groups={Object.entries(groupedDatasources).map(([key, value]) => { + groups={Object.entries(workspaceGroupedDatasources).map( + ([key, value]) => {(Optionally also filter out groups with
value.length === 0.)This keeps the UI correctly workspace‑scoped and aligned with the page‑level behavior.
Also applies to: 149-217
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
52-60: Workspace default view should consider only datasources for this workspace.
WorkspaceDatasourceDefaultViewcurrently callsgetDatasourcesand decides betweenDatasourceBlankStateandCreateNewDatasourceTabbased on the global list, ignoringworkspaceId. This can show the wrong state when the Redux list contains datasources from other workspaces (same issue as the side pane).Since
WorkspaceDatasourcesPagealready hasworkspaceId, pass it down and filter:-const WorkspaceDatasourceDefaultView = () => { - const datasources = useSelector(getDatasources); - - if (!datasources || datasources.length === 0) { - return <DatasourceBlankState />; - } - - return <CreateNewDatasourceTab />; -}; +const WorkspaceDatasourceDefaultView = ({ + workspaceId, +}: { + workspaceId: string; +}) => { + const datasources = useSelector(getDatasources); + const hasWorkspaceDatasources = datasources.some( + (ds) => ds.workspaceId === workspaceId, + ); + + if (!hasWorkspaceDatasources) { + return <DatasourceBlankState />; + } + + return <CreateNewDatasourceTab />; +}; @@ - <SentryRoute - path={WORKSPACE_DATASOURCES_URL} - render={() => <WorkspaceDatasourceDefaultView />} - /> + <SentryRoute + path={WORKSPACE_DATASOURCES_URL} + render={() => ( + <WorkspaceDatasourceDefaultView workspaceId={workspaceId} /> + )} + />This keeps the main pane’s behavior aligned with the workspace context and with how the side pane is expected to behave.
Also applies to: 121-124
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
68-72: ValidateworkspaceIdinresolveEntityIdForWorkspaceinstead of returning"".
resolveEntityIdForWorkspacefalls back to""when it can’t find aworkspaceId, which leads to base paths like/workspace/and malformed URLs with a missing segment.resolveEntityIdForAppalready throws in this situation; the workspace variant should be consistent and fail fast.Consider:
resolveEntityIdForWorkspace(builderParams: URLBuilderParams): string { - if (builderParams?.workspaceId) { - return builderParams.workspaceId; - } - - const currentUrl = window.location.pathname; - const workspaceMatch = currentUrl.match(/^\/workspace\/([^\/]+)/); - - return workspaceMatch ? workspaceMatch[1] : ""; + if (builderParams?.workspaceId) { + return builderParams.workspaceId; + } + + const currentUrl = window.location.pathname; + const workspaceMatch = currentUrl.match(/^\/workspace\/([^/]+)/); + const id = workspaceMatch ? workspaceMatch[1] : ""; + + if (!id) { + throw new URIError( + "Missing workspaceId. Provide builderParams.workspaceId or navigate under /workspace/:workspaceId.", + ); + } + + return id; }This keeps URL generation predictable and makes mis‑use of the builder obvious during development.
Also applies to: 246-266, 307-326
🧹 Nitpick comments (1)
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (1)
322-338: Consider combining filter operations.The sequential filtering works correctly, but iterates the array twice. For a small list of popular plugins this is negligible, but you could combine:
- const filteredMostPopularPlugins: Plugin[] = !!isAirgappedInstance - ? mostPopularPlugins.filter( - (plugin: Plugin) => - plugin?.packageName !== PluginPackageName.GOOGLE_SHEETS, - ) - : mostPopularPlugins; - - // Filter out REST API from most popular plugins when on workspace datasources page - const finalFilteredMostPopularPlugins = isWorkspaceDatasourcesPage - ? filteredMostPopularPlugins.filter( - (plugin: Plugin) => plugin?.packageName !== PluginPackageName.REST_API, - ) - : filteredMostPopularPlugins; + const finalFilteredMostPopularPlugins = mostPopularPlugins.filter( + (plugin: Plugin) => { + if (isAirgappedInstance && plugin?.packageName === PluginPackageName.GOOGLE_SHEETS) { + return false; + } + if (isWorkspaceDatasourcesPage && plugin?.packageName === PluginPackageName.REST_API) { + return false; + } + return true; + }, + );This is a minor optimization - current code is clear and works fine.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts(1 hunks)app/client/cypress/locators/commonlocators.json(1 hunks)app/client/cypress/support/Objects/CommonLocators.ts(1 hunks)app/client/src/ce/IDE/constants/routes.ts(1 hunks)app/client/src/ce/RouteBuilder.ts(1 hunks)app/client/src/ce/entities/URLRedirect/URLAssembly.ts(5 hunks)app/client/src/ce/pages/common/AppHeader.tsx(2 hunks)app/client/src/constants/routes/baseRoutes.ts(1 hunks)app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx(2 hunks)app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx(2 hunks)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourceEditor.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- app/client/src/ce/pages/common/AppHeader.tsx
- app/client/src/ce/RouteBuilder.ts
- app/client/src/pages/workspace/WorkspaceDatasourceEditor.tsx
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/**/**.*
⚙️ CodeRabbit configuration file
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.tsapp/client/cypress/support/Objects/CommonLocators.tsapp/client/cypress/locators/commonlocators.json
🧠 Learnings (26)
📚 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/pages/workspace/WorkspaceDatasourcesPage.tsxapp/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 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/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 29965
File: app/client/src/utils/storage.ts:129-129
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `migrateAppIdToEditorId` function in `app/client/src/utils/storage.ts` is intended to be called only when `appId` is present and `editorId` is not in the payload. This context should be considered in future reviews related to this function.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsxapp/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 36665
File: app/client/src/IDE/Components/FileTab/styles.tsx:19-19
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Do not suggest replacing hardcoded `padding-top: 6px;` with design variables in `app/client/src/IDE/Components/FileTab/styles.tsx`.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 33863
File: app/client/src/pages/Editor/DataSourceEditor/DBForm.tsx:93-98
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The suggestion to optimize conditional rendering in `DatasourceDBEditor` is considered out of scope for the current issue related to making text selectable.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsxapp/client/src/pages/workspace/WorkspaceDataSidePane.tsx
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsxapp/client/src/pages/workspace/WorkspaceDataSidePane.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: sneha122
Repo: appsmithorg/appsmith PR: 30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsxapp/client/src/pages/workspace/WorkspaceDataSidePane.tsx
📚 Learning: 2025-11-05T12:56:49.249Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts:0-0
Timestamp: 2025-11-05T12:56:49.249Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 4-6 depend on state (`page1Slug`, `page2Slug`) set in Tests 2-3. The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Learnt from: sagar-qa007
PR: appsmithorg/appsmith#34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-07-16T06:44:55.118Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep`, and other related sleep functions in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2025-11-05T12:58:32.486Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts:32-225
Timestamp: 2025-11-05T12:58:32.486Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 2-5 depend on state from previous tests (e.g., slug values modified in earlier tests). The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: abhishek-bandameedi
Repo: appsmithorg/appsmith PR: 35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.tsapp/client/cypress/support/Objects/CommonLocators.tsapp/client/cypress/locators/commonlocators.json
📚 Learning: 2024-11-13T09:07:54.931Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js:44-47
Timestamp: 2024-11-13T09:07:54.931Z
Learning: When reviewing code in `app/client/cypress/**` paths, ensure that suggestions are appropriate for test code and not for `src` code. Avoid applying source code checks to test code unless relevant.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.tsapp/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
📚 Learning: 2024-11-13T09:11:36.959Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js:35-37
Timestamp: 2024-11-13T09:11:36.959Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js`, when writing Cypress end-to-end tests for the Image Widget in JavaScript, use `viewWidgetsPage.imageinner` as the selector when asserting 'src' and 'alt' attributes to maintain consistency.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.tsapp/client/cypress/support/Objects/CommonLocators.tsapp/client/cypress/locators/commonlocators.json
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: Aishwarya-U-R
Repo: appsmithorg/appsmith PR: 29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.114Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settingsTe_Spec.ts:33-33
Timestamp: 2024-10-08T15:32:34.114Z
Learning: Follow best practices for Cypress code and e2e automation:
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes, and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 35412
File: app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/mysql_spec.ts:99-99
Timestamp: 2024-10-08T15:32:39.374Z
Learning: In Cypress tests within the `app/client/cypress` directory, avoid using `agHelper.Sleep`, `this.Sleep`, and other related sleep functions to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable. Instead, use Cypress methods like `cy.get()`, `cy.contains()`, and `cy.intercept()` to wait for specific conditions.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ApekshaBhosale
Repo: appsmithorg/appsmith PR: 35412
File: app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/mysql_spec.ts:99-99
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests, avoid using `agHelper.Sleep` as it can lead to flaky tests and unnecessary delays. Instead, use Cypress methods like `cy.get()`, `cy.contains()`, and `cy.intercept()` to wait for specific conditions.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-07-16T08:25:23.799Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settingsTe_Spec.ts:33-33
Timestamp: 2024-07-16T08:25:23.799Z
Learning: Avoid using sleep functions like `agHelper.Sleep` and `this.Sleep` in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep` in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-11T09:34:24.680Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36622
File: app/client/cypress/e2e/Regression/ClientSide/Git/GitPersistBranch_spec.ts:41-45
Timestamp: 2024-10-11T09:34:24.680Z
Learning: In `app/client/cypress/**/**.*` Cypress test files, the cleanup guideline is to use `after` hooks for cleanup tasks.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-11-29T09:33:28.991Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 37829
File: app/client/packages/rts/src/ctl/backup/links/EnvFileLink.ts:50-65
Timestamp: 2024-11-29T09:33:28.991Z
Learning: In `app/client/packages/rts/src/ctl/backup/links/EnvFileLink.ts`, changes to the `removeSensitiveEnvData` function should be approached cautiously, as it is sensitive and potentially flaky.
Applied to files:
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
📚 Learning: 2024-11-13T09:41:48.119Z
Learnt from: abhvsn
Repo: appsmithorg/appsmith PR: 37326
File: deploy/docker/fs/opt/appsmith/utils/bin/export_db.js:11-17
Timestamp: 2024-11-13T09:41:48.119Z
Learning: In the `export_db.js` script located at `deploy/docker/fs/opt/appsmith/utils/bin/export_db.js`, it's acceptable to use simple string-based detection (e.g., `dbUrl.startsWith('mongodb')`) for determining the database type because the database URL is restrictive, and misconfiguration will cause the server to fail to start up. Therefore, using a URL parsing library for database URL detection is unnecessary in this context.
Applied to files:
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
📚 Learning: 2024-12-05T10:56:13.739Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchSSHKeyRequest.ts:5-9
Timestamp: 2024-12-05T10:56:13.739Z
Learning: In `app/client/src/git/requests/fetchSSHKeyRequest.ts`, the `fetchSSHKeyRequest` function does not require explicit error handling for `baseApplicationId`; the function should throw an error naturally if `baseApplicationId` is missing or invalid.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2025-01-15T20:46:05.401Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38681
File: app/client/src/git/requests/helpers/urlArtifactType.ts:3-12
Timestamp: 2025-01-15T20:46:05.401Z
Learning: In URL helper functions like `urlArtifactType`, error handling in switch statements is not required as TypeScript's compile-time checks are sufficient.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2025-10-28T03:30:58.299Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/sagas/InitSagas.ts:260-271
Timestamp: 2025-10-28T03:30:58.299Z
Learning: In app/client/src/sagas/InitSagas.ts, when constructing consolidatedApiParams for static page URLs, the code intentionally reuses applicationId and defaultPageId fields to pass staticApplicationSlug and staticPageSlug values respectively. This is by design, even though ConsolidatedApiParams type has dedicated staticApplicationSlug and staticPageSlug fields.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
🧬 Code graph analysis (6)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (3)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/pages/Editor/IntegrationEditor/util.ts (1)
filterSearch(11-20)app/client/src/ce/constants/messages.ts (1)
CREATE_NEW_DATASOURCE_REST_API(402-402)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (3)
app/client/src/IDE/Components/LayoutComponents.tsx (1)
GridContainer(3-7)app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (1)
WorkspaceDataSidePane(53-221)app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_URL(31-31)WORKSPACE_DATASOURCE_EDITOR_URL(32-32)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (2)
app/client/cypress/support/ApiCommands.js (1)
locator(17-17)app/client/cypress/support/commands.js (1)
_(1071-1071)
app/client/src/pages/workspace/WorkspaceDataSidePane.tsx (3)
app/client/src/ce/selectors/entitiesSelector.ts (4)
getDatasources(86-88)getDatasourcesGroupedByPluginCategory(114-144)getPluginImages(605-623)getPlugins(90-91)app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
getHasCreateDatasourcePermission(56-62)app/client/src/entities/Datasource/index.ts (1)
Datasource(154-162)
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (2)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/entities/Plugin/index.ts (1)
Plugin(71-87)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)
⏰ 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). (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (7)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (2)
51-51: LGTM - Import addition is appropriate.Adding
IDE_TYPEfor workspace detection aligns with the existing pattern in the codebase.
368-385: Clean implementation addressing prior feedback.Using
getIDETypeByUrlwithIDE_TYPE.Workspaceis the correct approach per prior review feedback. The conditional visibility logic forrestAPIVisibleandgraphQLAPIVisibleis clear and correctly excludes these options on the workspace datasources page.app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (1)
54-54: LGTM - Import addition aligns with APIOrSaasPlugins.tsx.app/client/src/constants/routes/baseRoutes.ts (1)
31-32: Workspace datasources route constants look consistent.The new
WORKSPACE_DATASOURCES_URLandWORKSPACE_DATASOURCE_EDITOR_URLalign with the/workspace/:workspaceId/...pattern and are used consistently elsewhere (IDE routes, page routing).app/client/src/ce/IDE/constants/routes.ts (1)
41-52: Correctly wires Workspace IDE base paths.Mapping
IDE_TYPE.Workspaceto[WORKSPACE_DATASOURCES_URL, WORKSPACE_DATASOURCE_EDITOR_URL]cleanly integrates workspace routing into the IDE routing matrix without impacting App/UIPackage types.app/client/cypress/support/Objects/CommonLocators.ts (1)
361-374: New workspace datasource locators are consistent and usable.The added locators (
workspaceDatasources,_addDatasourceButtonBlankScreen,_datasource, etc.) follow the existing naming conventions and match the data‑testids/classes used in the workspace datasources UI and tests.app/client/cypress/locators/commonlocators.json (1)
249-263: Workspace datasource locator entries match TS locators and test usage.The new JSON locators (
workspaceDatasources,addDatasourceButtonBlankScreen,datasource, etc.) mirror the TypeScriptCommonLocatorsfields and the Cypress spec, keeping selectors centralized and consistent.
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
322-331: Empty string fallback creates malformed URLs.This issue was previously flagged. Returning
""produces broken paths like/workspace//.... Consider throwing an error consistent withresolveEntityIdForApp.
🧹 Nitpick comments (1)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
247-253: Consider usingwindow.locationfor explicitness.While
locationworks, usingwindow.location.pathnameis more explicit and aligns with the pattern used elsewhere in this file (e.g., line 327).getDefaultEditorType() { - if (location.pathname.startsWith("/workspace")) { + if (window.location.pathname.startsWith("/workspace")) { return IDE_TYPE.Workspace; } return IDE_TYPE.App; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts(5 hunks)app/client/src/constants/routes/baseRoutes.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/constants/routes/baseRoutes.ts
🧰 Additional context used
🧠 Learnings (4)
📚 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/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2024-12-05T10:56:13.739Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchSSHKeyRequest.ts:5-9
Timestamp: 2024-12-05T10:56:13.739Z
Learning: In `app/client/src/git/requests/fetchSSHKeyRequest.ts`, the `fetchSSHKeyRequest` function does not require explicit error handling for `baseApplicationId`; the function should throw an error naturally if `baseApplicationId` is missing or invalid.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2025-01-15T20:46:05.401Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38681
File: app/client/src/git/requests/helpers/urlArtifactType.ts:3-12
Timestamp: 2025-01-15T20:46:05.401Z
Learning: In URL helper functions like `urlArtifactType`, error handling in switch statements is not required as TypeScript's compile-time checks are sufficient.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
🧬 Code graph analysis (1)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (3)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/cypress/support/commands.js (1)
workspaceId(396-396)app/client/src/constants/routes/baseRoutes.ts (1)
WORKSPACE_EDITOR_URL(31-31)
⏰ 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). (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (4)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (4)
11-18: LGTM!Imports are correctly added for workspace support:
WORKSPACE_EDITOR_URL, typedobjectKeys, andIDE_TYPE.
32-32: LGTM!Optional
workspaceIdparam added consistently with existing ID parameters.
72-72: LGTM!Using typed
objectKeysutility improves type inference.
255-271: LGTM!Workspace path generation correctly uses
generatePathwithWORKSPACE_EDITOR_URL. Themodeparameter being unused for workspace context is intentional since workspace editor doesn't have EDIT/PUBLISHED modes.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
50-58: Workspace-scoped datasource filtering needed.
getDatasourcesreturns all datasources, not just those for the current workspace. This component should filter byworkspaceIdor use a workspace-scoped selector.
105-111: Route path case should be consistent.This route uses
/NEW(uppercase). Verify all navigation calls use the same case to avoid route mismatches.
🧹 Nitpick comments (3)
app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx (1)
49-51: MakingdsUsageMapoptional looks correct; consider normalizing once for clarityChanging
dsUsageMapto optional aligns with the new usage patterns and is safe given you callget(dsUsageMap, data.id, ""), which already falls back to an empty string whendsUsageMapisundefined.If you want to make the intent a bit clearer and avoid ever passing
undefinedintoget, you could normalize once after destructuring:const { dsUsageMap } = props; const safeDsUsageMap = dsUsageMap ?? {}; // ... description: get(safeDsUsageMap, data.id, ""),Not required, but it tightens up the types and makes the fallback behavior explicit.
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (2)
71-76: Consider resetting initialization state on workspace change.When navigating from one workspace to another,
isWorkspaceEditorInitializedmight briefly show stale content from the previous workspace before the new initialization completes. Consider dispatching a reset action at the start of the effect or in the saga to clear the flag before re-initializing.useEffect(() => { if (workspaceId) { + // Reset state before initializing to avoid stale content dispatch(initWorkspaceIDE({ workspaceId })); } }, [dispatch, workspaceId]);Alternatively, ensure
initWorkspaceIDEsaga setsisWorkspaceEditorInitializedtofalseat the start.
93-100: Consider extracting inline styles.The inline
styleobject onGridContainercould be extracted to a styled component for consistency withLeftPaneandMainPane.+const StyledGridContainer = styled(GridContainer)` + grid-template-columns: 300px 1fr; + grid-template-rows: 100%; + overflow: hidden; + flex: 1; +`; + // In the render: -<GridContainer - style={{ - gridTemplateColumns: "300px 1fr", - gridTemplateRows: "100%", - overflow: "hidden", - flex: 1, - }} -> +<StyledGridContainer>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 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/pages/workspace/WorkspaceDatasourcesPage.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/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 29965
File: app/client/src/utils/storage.ts:129-129
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `migrateAppIdToEditorId` function in `app/client/src/utils/storage.ts` is intended to be called only when `appId` is present and `editorId` is not in the payload. This context should be considered in future reviews related to this function.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 36665
File: app/client/src/IDE/Components/FileTab/styles.tsx:19-19
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Do not suggest replacing hardcoded `padding-top: 6px;` with design variables in `app/client/src/IDE/Components/FileTab/styles.tsx`.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 33863
File: app/client/src/pages/Editor/DataSourceEditor/DBForm.tsx:93-98
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The suggestion to optimize conditional rendering in `DatasourceDBEditor` is considered out of scope for the current issue related to making text selectable.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: sneha122
Repo: appsmithorg/appsmith PR: 30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
⏰ 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). (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (5)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (5)
1-20: LGTM!Imports are well-organized and appropriate for the component's functionality.
22-44: LGTM!Styled components are clean and follow the design system with appropriate CSS variable usage.
46-48: LGTM!Interface is appropriately minimal.
78-89: LGTM!Loading state is properly implemented with correct height calculation.
101-103: No action needed - DataSidePane correctly handles workspace context.DataSidePane is workspace-aware through Redux initialization. When WorkspaceDatasourcesPage mounts, it calls
initWorkspaceIDE({ workspaceId }), which sets the workspace context in Redux viaSET_CURRENT_WORKSPACE_IDand callsfetchDatasources({ workspaceId })to load only the workspace's datasources. DataSidePane retrieves these pre-filtered datasources using thegetDatasourcesselector and also accesses workspace permissions viagetCurrentAppWorkspace. No props are needed because the Redux state is already scoped to the current workspace before the component renders.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
322-331: Returning empty string still produces malformed URLs.This was previously flagged. Returning
""will generate paths like/workspace//datasources. For consistency withresolveEntityIdForApp, throw aURIErrorwhen the workspaceId cannot be resolved.resolveEntityIdForWorkspace(builderParams: URLBuilderParams): string { if (builderParams?.workspaceId) { return builderParams.workspaceId; } const currentUrl = window.location.pathname; const workspaceMatch = currentUrl.match(/^\/workspace\/([^\/]+)/); + const workspaceId = workspaceMatch ? workspaceMatch[1] : ""; - return workspaceMatch ? workspaceMatch[1] : ""; + if (!workspaceId) { + throw new URIError( + "Missing workspaceId. Provide builderParams.workspaceId or navigate under /workspace/:workspaceId.", + ); + } + + return workspaceId; }app/client/src/entities/Engine/WorkspaceEditorEngine.ts (1)
43-66: Handle workspace fetch errors explicitly and guard the environment wait.Two related concerns in this block:
After
yield take([FETCH_ALL_WORKSPACES_SUCCESS, FETCH_WORKSPACES_ERROR])you ignore which action was received. IfFETCH_WORKSPACES_ERRORfires (or the workspace is still missing after a “success”),loadWorkspacestill proceeds to environment fetching andwaitForFetchEnvironments, which can lead to confusing follow‑up failures or a stuck init. Capture the taken action, branch onaction.type, and abort/throw on the error path (optionally re‑selecting the workspace to confirm it exists before continuing).
yield call(waitForFetchEnvironments)is still unguarded. If the underlying env fetch fails silently or the saga hangs, workspace IDE initialization will block indefinitely with no feedback. Prior feedback already raised this; the issue appears unresolved here. Either ensurewaitForFetchEnvironmentsitself implements robust error/timeout behavior, or wrap it in a guarded flow (e.g.,racewith a timeout and handling of an env‑fetch error action) and abort initialization on failure.
🧹 Nitpick comments (5)
app/client/src/ce/navigation/FocusStrategy/index.ts (1)
11-12: Workspace IDE currently has “no-op” focus behaviorRouting
IDE_TYPE.WorkspacetoNoIDEFocusStrategyis safe and avoids the default error, but it also means the workspace IDE won’t get any custom focus/keyboard navigation behavior (same as “None”). If the workspace views eventually need entity-focused navigation (like App IDE), consider introducing aWorkspaceIDEFocusStrategyinstead of reusingNoIDEFocusStrategy.app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
247-253: Consider extracting the path prefix to a constant.The hardcoded
"/workspace"string works but could drift if route constants change elsewhere. A minor maintainability improvement would be to derive this fromWORKSPACE_EDITOR_URLor a shared constant.+ private static readonly WORKSPACE_PATH_PREFIX = "/workspace"; + getDefaultEditorType() { - if (location.pathname.startsWith("/workspace")) { + if (location.pathname.startsWith(URLBuilder.WORKSPACE_PATH_PREFIX)) { return IDE_TYPE.Workspace; } return IDE_TYPE.App; }app/client/src/pages/workspace/index.tsx (1)
14-42: Routing structure looks correct; consider de-duplicating datasources routes and tightening typesThe new
Switchcorrectly adds:
/workspace/:workspaceId/settings→ wrappedSettings/workspace/:workspaceId/datasourcesand/workspace/:workspaceId/datasource/:datasourceId→WorkspaceDatasourcesPage- Fallback → wrapped
DefaultWorkspacePageTwo small follow-ups you might consider:
- The two datasources routes share the same
renderbody; you could collapse them into a singleSentryRoutewith apatharray to reduce duplication and future drift risk.- Instead of hand-writing the
matchtype, you could useRouteComponentProps<{ workspaceId: string }>(or rely on inference) for better alignment with React Router’s types.Functionally this LGTM; these are just maintainability tweaks.
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (2)
32-33: Avoidanyon saga return type and drop the ESLint suppression.Instead of
: anyplus// eslint-disable-next-line @typescript-eslint/no-explicit-any, prefer a proper saga return type (e.g.,SagaIteratorfromredux-saga) so you get type safety without suppressing lint rules.
73-105: Differentiate failure causes fromfailFastApiCallsfor clearer errors.
failFastApiCallsis watching plugins, datasources, and (optionally) mock datasources, but any failure leads toPluginsNotFoundError("Unable to fetch plugins"). That message is misleading when only datasources or mock datasources fail. Consider either:
- Throwing a more generic workspace‑init error, or
- Splitting the calls so plugin and datasource failures can surface more accurate exceptions/messages.
This is non‑blocking but will make debugging workspace IDE init failures easier.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts(5 hunks)app/client/src/ce/navigation/FocusStrategy/index.ts(1 hunks)app/client/src/entities/Engine/WorkspaceEditorEngine.ts(1 hunks)app/client/src/pages/workspace/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2024-10-11T09:28:32.636Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36622
File: app/client/src/entities/Engine/AppEditorEngine.ts:0-0
Timestamp: 2024-10-11T09:28:32.636Z
Learning: In the file `app/client/src/entities/Engine/AppEditorEngine.ts`, the IDs (`currentApplication.baseId`) and branches (`currentBranch`) are obfuscated, so it's acceptable to log them for debugging purposes.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T11:00:39.952Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchGitMetadataRequest.ts:6-10
Timestamp: 2024-12-05T11:00:39.952Z
Learning: The `fetchGitMetadataRequest` function in `app/client/src/git/requests/fetchGitMetadataRequest.ts` does not require additional error handling or timeout configuration.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.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/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T10:57:15.397Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/updateGlobalConfigRequest.ts:9-13
Timestamp: 2024-12-05T10:57:15.397Z
Learning: In the TypeScript files within `app/client/src/git/requests/`, functions should not include internal error handling or request timeouts; they should allow errors to be thrown directly.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T10:56:13.739Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchSSHKeyRequest.ts:5-9
Timestamp: 2024-12-05T10:56:13.739Z
Learning: In `app/client/src/git/requests/fetchSSHKeyRequest.ts`, the `fetchSSHKeyRequest` function does not require explicit error handling for `baseApplicationId`; the function should throw an error naturally if `baseApplicationId` is missing or invalid.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.tsapp/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 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/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2025-01-15T20:46:05.401Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38681
File: app/client/src/git/requests/helpers/urlArtifactType.ts:3-12
Timestamp: 2025-01-15T20:46:05.401Z
Learning: In URL helper functions like `urlArtifactType`, error handling in switch statements is not required as TypeScript's compile-time checks are sufficient.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
🧬 Code graph analysis (3)
app/client/src/pages/workspace/index.tsx (3)
app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/pages/common/PageWrapper.tsx (1)
PageWrapper(63-89)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
WorkspaceDatasourcesPage(60-128)
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (4)
app/client/src/ce/constants/ReduxActionConstants.tsx (2)
ReduxActionTypes(1302-1346)ReduxActionErrorTypes(1348-1375)app/client/src/sagas/InitSagas.ts (1)
failFastApiCalls(147-167)app/client/src/entities/Engine/index.ts (2)
PluginsNotFoundError(61-61)PluginFormConfigsNotFoundError(62-62)app/client/src/actions/pluginActions.ts (1)
fetchPluginFormConfigs(22-29)
app/client/src/ce/navigation/FocusStrategy/index.ts (2)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/ce/navigation/FocusStrategy/NoIDEFocusStrategy.ts (1)
NoIDEFocusStrategy(3-18)
⏰ 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). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (4)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (3)
17-18: LGTM on imports and interface extension.Using
objectKeysfor type safety is a good practice. TheworkspaceIdaddition toURLBuilderParamscleanly extends the interface.Also applies to: 32-32, 72-72
255-271: Clean dispatch pattern for multi-editor support.The
entityIdrename appropriately generalizes the parameter. Themodeparameter being unused for workspace paths is expected since workspaces don't have EDIT/PUBLISHED modes.
312-320: Dispatch logic mirrorsgenerateBasePathcorrectly.The editor-type-based branching is consistent with the path generation approach.
app/client/src/pages/workspace/index.tsx (1)
6-6: ImportingWorkspaceDatasourcesPagehere is appropriateThe import cleanly wires the new workspace-level datasources UI into the workspace routing; no issues from a dependency or layering standpoint.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
50-58: Verify datasource scoping inWorkspaceDatasourceDefaultView.
WorkspaceDatasourceDefaultViewusesgetDatasourceswithout a workspace filter, and treats any non-empty list as “this workspace has datasources”. IfgetDatasourcesever returns datasources from other workspaces, this will suppress the blank state incorrectly for workspaces that actually have none. Either ensuregetDatasourcesis already workspace-scoped in this context, or derive a workspace-filtered list (e.g., via a workspace-aware selector orworkspaceIdprop) before deciding which view to render.
🧹 Nitpick comments (2)
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
179-195: Code refactor correctly implements the permission logic; consider adding a clarifying comment for maintainability.The explicit control flow improves readability over the previous inline conditionals. The
hasDatasourcePermissionextraction reduces duplication effectively.Two minor notes:
IDE_TYPE.Workspacereturningfalseis intentional—workspace datasources are view-only—but a brief comment explaining this would aid future maintainers.IDE_TYPE.Nonefalling through to line 195 is correct; it's the default IDE type and should only require datasource permissions.The refactor is solid and addresses prior feedback about complexity.
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (1)
31-52: Handle failed workspace fetch explicitly inloadWorkspace.Right now, if
FETCH_WORKSPACES_ERRORis dispatched, the saga just continues without re-checking the workspace or aborting init. That can leave the IDE running without a valid workspace in state. Consider checking the result after thetakeand either throwing or dispatching a dedicated failure action so the caller can surface an error and/or stop initialization.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx(1 hunks)app/client/src/entities/Engine/WorkspaceEditorEngine.ts(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: saiprabhu-dandanayak
Repo: appsmithorg/appsmith PR: 33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-10-08T15:32:39.374Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsxapp/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-12-15T17:45:48.303Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38171
File: app/client/src/git/components/DefaultBranch/DefaultBranchCE.tsx:1-14
Timestamp: 2024-12-15T17:45:48.303Z
Learning: In `app/client/src/git/components/DefaultBranch/DefaultBranchCE.tsx`, the feature flag check is performed at a higher level, so it's acceptable to have `isGitProtectedFeatureLicensed={false}` in this component.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx
📚 Learning: 2024-10-11T09:28:32.636Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36622
File: app/client/src/entities/Engine/AppEditorEngine.ts:0-0
Timestamp: 2024-10-11T09:28:32.636Z
Learning: In the file `app/client/src/entities/Engine/AppEditorEngine.ts`, the IDs (`currentApplication.baseId`) and branches (`currentBranch`) are obfuscated, so it's acceptable to log them for debugging purposes.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T11:00:39.952Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchGitMetadataRequest.ts:6-10
Timestamp: 2024-12-05T11:00:39.952Z
Learning: The `fetchGitMetadataRequest` function in `app/client/src/git/requests/fetchGitMetadataRequest.ts` does not require additional error handling or timeout configuration.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.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/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T10:57:15.397Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/updateGlobalConfigRequest.ts:9-13
Timestamp: 2024-12-05T10:57:15.397Z
Learning: In the TypeScript files within `app/client/src/git/requests/`, functions should not include internal error handling or request timeouts; they should allow errors to be thrown directly.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T10:56:13.739Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchSSHKeyRequest.ts:5-9
Timestamp: 2024-12-05T10:56:13.739Z
Learning: In `app/client/src/git/requests/fetchSSHKeyRequest.ts`, the `fetchSSHKeyRequest` function does not require explicit error handling for `baseApplicationId`; the function should throw an error naturally if `baseApplicationId` is missing or invalid.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
📚 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/pages/workspace/WorkspaceDatasourcesPage.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/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 29965
File: app/client/src/utils/storage.ts:129-129
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `migrateAppIdToEditorId` function in `app/client/src/utils/storage.ts` is intended to be called only when `appId` is present and `editorId` is not in the payload. This context should be considered in future reviews related to this function.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 36665
File: app/client/src/IDE/Components/FileTab/styles.tsx:19-19
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Do not suggest replacing hardcoded `padding-top: 6px;` with design variables in `app/client/src/IDE/Components/FileTab/styles.tsx`.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 33863
File: app/client/src/pages/Editor/DataSourceEditor/DBForm.tsx:93-98
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The suggestion to optimize conditional rendering in `DatasourceDBEditor` is considered out of scope for the current issue related to making text selectable.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: sneha122
Repo: appsmithorg/appsmith PR: 30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
🧬 Code graph analysis (3)
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (3)
app/client/src/sagas/InitSagas.ts (1)
failFastApiCalls(147-167)app/client/src/entities/Engine/index.ts (2)
PluginsNotFoundError(61-61)PluginFormConfigsNotFoundError(62-62)app/client/src/actions/pluginActions.ts (1)
fetchPluginFormConfigs(22-29)
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (6)
app/client/src/ce/selectors/entitiesSelector.ts (1)
getDatasources(86-88)app/client/src/ce/actions/workspaceIDEActions.ts (1)
initWorkspaceIDE(7-10)app/client/packages/design-system/widgets/src/components/Spinner/src/index.tsx (1)
Spinner(16-16)app/client/src/IDE/Components/LayoutComponents.tsx (1)
GridContainer(3-7)app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_URL(32-32)WORKSPACE_DATASOURCE_EDITOR_URL(33-33)
⏰ 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). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (4)
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (3)
54-57: Engine setup flow looks consistent with existing editors.Dispatching
START_EVALUATIONand resettingCodemirrorTernServicematches the usual editor initialization pattern; no issues from my side.
59-103: Plugin/datasource initialization flow is sound and fail-fast.The
failFastApiCallsusage for plugins, datasources, mock datasources (airgap-aware), and plugin form configs, plus throwingPluginsNotFoundError/PluginFormConfigsNotFoundError, mirrors the existing App editor engine behavior and gives a clear failure path. This looks good.
105-109:completeChorecorrectly signals IDE initialization success.Emitting
INITIALIZE_WORKSPACE_IDE_SUCCESShere cleanly closes the initialization flow and matches how the UI flag is wired.app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
60-132: Workspace datasources page wiring and routing look good.
initWorkspaceIDE({ workspaceId })runs on everyworkspaceIdchange, and theisWorkspaceEditorInitializedgate cleanly drives the loading spinner vs. main layout.- The two‑pane layout using
PageContainer+GridContainer+LeftPane/MainPaneis straightforward and avoids the earlier height conflict.- Routing for
/datasources/NEW, the datasource editor, and the default datasources view is correctly ordered insideSwitchso more specific routes win.No functional issues spotted here.
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/19881429311. |
|
Deploy-Preview-URL: https://ce-41324.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (1)
204-204: Replace plain attribute selector with a locator variable.The selector
"input[name='url']"violates the guideline to use locator variables. This is the same issue as line 116. Use a locator variable defined in commonLocators.json.Based on coding guidelines: "Use locator variables for locators and do not use plain strings" and "Avoid Xpaths, Attributes and CSS path."
Apply this pattern:
- agHelper.TypeText("input[name='url']", "https://mock-api.appsmith.com"); + agHelper.TypeText(locator._datasourceUrl, "https://mock-api.appsmith.com");
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/**/**.*
⚙️ CodeRabbit configuration file
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
🧠 Learnings (14)
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Learnt from: sagar-qa007
PR: appsmithorg/appsmith#34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-07-16T06:44:55.118Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep`, and other related sleep functions in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2025-11-05T12:56:49.249Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts:0-0
Timestamp: 2025-11-05T12:56:49.249Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/PagePersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 4-6 depend on state (`page1Slug`, `page2Slug`) set in Tests 2-3. The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2025-11-05T12:58:32.486Z
Learnt from: vsvamsi1
Repo: appsmithorg/appsmith PR: 41363
File: app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts:32-225
Timestamp: 2025-11-05T12:58:32.486Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/StaticUrl/SlugPersistance_Spec.ts`, the test suite intentionally uses sequential tests where Tests 2-5 depend on state from previous tests (e.g., slug values modified in earlier tests). The author (vsvamsi1) prioritizes readability by breaking the flow into different segments over strict test isolation.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: abhishek-bandameedi
Repo: appsmithorg/appsmith PR: 35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-11-13T09:07:54.931Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js:44-47
Timestamp: 2024-11-13T09:07:54.931Z
Learning: When reviewing code in `app/client/cypress/**` paths, ensure that suggestions are appropriate for test code and not for `src` code. Avoid applying source code checks to test code unless relevant.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-11-13T09:11:36.959Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js:35-37
Timestamp: 2024-11-13T09:11:36.959Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js`, when writing Cypress end-to-end tests for the Image Widget in JavaScript, use `viewWidgetsPage.imageinner` as the selector when asserting 'src' and 'alt' attributes to maintain consistency.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: Aishwarya-U-R
Repo: appsmithorg/appsmith PR: 29405
File: app/client/cypress/e2e/Regression/ClientSide/Binding/TableV2_Widget_API_Pagination_spec.js:37-41
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The pull request titled "test: Cypress | Replace static with Dynamic waits - Part 1" is part of a phased approach where only certain test specifications are targeted for static wait removal in the initial phase. Future phases will address additional specs.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.114Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settingsTe_Spec.ts:33-33
Timestamp: 2024-10-08T15:32:34.114Z
Learning: Follow best practices for Cypress code and e2e automation:
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes, and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 35412
File: app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/mysql_spec.ts:99-99
Timestamp: 2024-10-08T15:32:39.374Z
Learning: In Cypress tests within the `app/client/cypress` directory, avoid using `agHelper.Sleep`, `this.Sleep`, and other related sleep functions to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable. Instead, use Cypress methods like `cy.get()`, `cy.contains()`, and `cy.intercept()` to wait for specific conditions.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ApekshaBhosale
Repo: appsmithorg/appsmith PR: 35412
File: app/client/cypress/e2e/Regression/ClientSide/OneClickBinding/TableWidget/mysql_spec.ts:99-99
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests, avoid using `agHelper.Sleep` as it can lead to flaky tests and unnecessary delays. Instead, use Cypress methods like `cy.get()`, `cy.contains()`, and `cy.intercept()` to wait for specific conditions.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-07-16T08:25:23.799Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settingsTe_Spec.ts:33-33
Timestamp: 2024-07-16T08:25:23.799Z
Learning: Avoid using sleep functions like `agHelper.Sleep` and `this.Sleep` in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sagar-qa007
Repo: appsmithorg/appsmith PR: 34955
File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Avoid using sleep functions like `agHelper.Sleep`, `this.Sleep` in Cypress tests within the `app/client/cypress` directory to prevent non-deterministic behaviors and ensure tests are more reliable and maintainable.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2024-10-11T09:34:24.680Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36622
File: app/client/cypress/e2e/Regression/ClientSide/Git/GitPersistBranch_spec.ts:41-45
Timestamp: 2024-10-11T09:34:24.680Z
Learning: In `app/client/cypress/**/**.*` Cypress test files, the cleanup guideline is to use `after` hooks for cleanup tasks.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
📚 Learning: 2025-06-05T11:32:48.592Z
Learnt from: sneha122
Repo: appsmithorg/appsmith PR: 40239
File: app/client/cypress/support/Pages/AggregateHelper.ts:0-0
Timestamp: 2025-06-05T11:32:48.592Z
Learning: In the AssertAutoSave method in app/client/cypress/support/Pages/AggregateHelper.ts, using this.Sleep(DEBOUNCE_TIMER + 100) is a necessary exception to the "avoid sleep" coding guideline due to technical constraints with the debounced save mechanism. waitUntil cannot be used because other wait timers cause the save status to disappear before assertions can run, leading to test failures.
Applied to files:
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
🧬 Code graph analysis (1)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (2)
app/client/cypress/support/ApiCommands.js (1)
locator(17-17)app/client/cypress/support/commands.js (2)
_(1071-1071)workspaceId(396-396)
⏰ 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). (5)
- GitHub Check: client-lint / client-lint
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (2)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts (2)
20-46: LGTM: Well-structured test using proper locator variables and helpers.This test correctly uses locator variables, helper methods, and network assertions throughout. Good adherence to Cypress best practices.
49-73: LGTM: Clean test implementation.Good use of locator variables and helper methods. The
cy.location()pattern for URL validation is appropriate here.
| .click({ force: true }); | ||
|
|
||
| agHelper.RenameDatasource("Mock API"); | ||
| agHelper.TypeText("input[name='url']", "https://mock-api.appsmith.com"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace plain attribute selector with a locator variable.
The selector "input[name='url']" violates the guideline to use locator variables and avoid attribute selectors. Define this selector in commonLocators.json and reference it via the locator variable.
Based on coding guidelines: "Use locator variables for locators and do not use plain strings" and "Avoid Xpaths, Attributes and CSS path."
Apply this pattern:
- agHelper.TypeText("input[name='url']", "https://mock-api.appsmith.com");
+ agHelper.TypeText(locator._datasourceUrl, "https://mock-api.appsmith.com");Add _datasourceUrl: "input[data-testid='t--datasource-url']" to commonLocators.json.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
around line 116, replace the plain attribute selector "input[name='url']" with a
locator variable: add an entry _datasourceUrl:
"input[data-testid='t--datasource-url']" to commonLocators.json and then update
the test to use that locator variable (e.g.,
agHelper.TypeText(commonLocators._datasourceUrl,
"https://mock-api.appsmith.com")). Ensure the new data-testid matches the input
element in the app and run the spec to verify the locator resolves.
| cy.contains("span", "Are you sure?") | ||
| .should("be.visible") | ||
| .click({ force: true }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace plain element selector with a locator variable.
The selector "span" in cy.contains("span", "Are you sure?") violates the guideline to use locator variables. Define this selector with a data-testid attribute in commonLocators.json.
Based on coding guidelines: "Use locator variables for locators and do not use plain strings."
Apply this pattern:
- cy.contains("span", "Are you sure?")
- .should("be.visible")
- .click({ force: true });
+ agHelper.GetNClick(locator._confirmDeleteButton);Add _confirmDeleteButton: "[data-testid='t--confirm-delete-button']" to commonLocators.json.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
around lines 163-165, the test uses a plain element selector cy.contains("span",
"Are you sure?"); add a locator variable to commonLocators.json named
_confirmDeleteButton with value "[data-testid='t--confirm-delete-button']" and
update the test to use that locator variable instead of the literal "span" (use
the locator to find the confirm-delete button and then assert visibility and
click).
| agHelper.GetNClick(dataSources._saveDs); | ||
| assertHelper.AssertNetworkStatus("@saveDatasource", 201); | ||
|
|
||
| cy.contains("button", "Edit").should("be.visible").click(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace plain element selector with a locator variable.
The selector "button" in cy.contains("button", "Edit") violates the guideline to use locator variables. Use a locator variable with a data-testid attribute.
Based on coding guidelines: "Use locator variables for locators and do not use plain strings."
Apply this pattern:
- cy.contains("button", "Edit").should("be.visible").click();
+ agHelper.GetNClick(locator._editDatasourceButton);Add _editDatasourceButton: "[data-testid='t--edit-datasource-button']" to commonLocators.json.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
around line 209, the test uses cy.contains("button", "Edit") which uses a plain
element selector; add a locator variable named _editDatasourceButton with value
"[data-testid='t--edit-datasource-button']" to commonLocators.json, then replace
the cy.contains call with a locator-based call referencing the new
commonLocators._editDatasourceButton (e.g., use
cy.get(commonLocators._editDatasourceButton).should("be.visible").click()) so
the test uses the data-testid locator variable instead of the plain "button"
string.
| agHelper.TypeText( | ||
| `${locator._headersArray} input[name$='.key']`, | ||
| "Content-Type", | ||
| ); | ||
| agHelper.TypeText( | ||
| `${locator._headersArray} input[name$='.value']`, | ||
| "application/json", | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace attribute selectors with data-testid based locator variables.
The composite selectors using input[name$='.key'] and input[name$='.value'] violate the guideline to avoid attribute selectors. Define these with data-testid attributes in commonLocators.json.
Based on coding guidelines: "Avoid Xpaths, Attributes and CSS path" and "Use data-* attributes for selectors."
Apply this pattern:
- agHelper.TypeText(
- `${locator._headersArray} input[name$='.key']`,
- "Content-Type",
- );
- agHelper.TypeText(
- `${locator._headersArray} input[name$='.value']`,
- "application/json",
- );
+ agHelper.TypeText(locator._headerKeyInput, "Content-Type");
+ agHelper.TypeText(locator._headerValueInput, "application/json");Add to commonLocators.json:
"_headerKeyInput": "[data-testid='t--header-key-input']",
"_headerValueInput": "[data-testid='t--header-value-input']"🤖 Prompt for AI Agents
In
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
around lines 211 to 218, replace the attribute-based composite selectors
`${locator._headersArray} input[name$='.key']` and `${locator._headersArray}
input[name$='.value']` with the new data-testid based locator variables (e.g.
use `${locator._headersArray} ${locator._headerKeyInput}` and
`${locator._headersArray} ${locator._headerValueInput}`), and add the
corresponding entries to commonLocators.json: "_headerKeyInput":
"[data-testid='t--header-key-input']" and "_headerValueInput":
"[data-testid='t--header-value-input']".
| cy.contains(".ads-v2-modal__content-header", "Are you sure?").should( | ||
| "be.visible", | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace CSS class selector with a data-testid based locator variable.
The selector .ads-v2-modal__content-header uses a CSS class, which violates the guideline to use data-testid attributes. Define this selector in commonLocators.json with a data-testid.
Based on coding guidelines: "Use data-* attributes for selectors" and "Avoid CSS path."
Apply this pattern:
- cy.contains(".ads-v2-modal__content-header", "Are you sure?").should(
- "be.visible",
- );
+ agHelper.AssertElementVisibility(locator._discardChangesModalHeader);
+ cy.contains(locator._discardChangesModalHeader, "Are you sure?").should("be.visible");Add _discardChangesModalHeader: "[data-testid='t--modal-header']" to commonLocators.json.
Committable suggestion skipped: line range outside the PR's diff.
| None: "None", | ||
| App: "App", | ||
| UIPackage: "UIPackage", | ||
| Workspace: "Workspace", |
There was a problem hiding this comment.
We are removing all traces of IDE
| dsPermissions, | ||
| ); | ||
|
|
||
| if (ideType === IDE_TYPE.Workspace) { |
There was a problem hiding this comment.
in src/ce/utils/workspaceHelpers.ts, create a function to lookup window.location.pathname.startsWith("/workspace")
|
|
||
| interface DataSidePaneProps { | ||
| dsUsageMap: Record<string, string>; | ||
| dsUsageMap?: Record<string, string>; |
There was a problem hiding this comment.
Add on click events to override default route builder function calls
| } from "constants/routes"; | ||
|
|
||
| // Page container for full viewport layout | ||
| const PageContainer = styled.div` |
There was a problem hiding this comment.
Check AppIDE -> StaticLayout and use the existing grid system. Remove these
tomjose92
force-pushed
the
feat/issue-35669/workspace-datasources
branch
from
3335671 to
ccc1395
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (1)
376-385: Validate workspaceId; returning empty string produces malformed URLs.Returning
""when noworkspaceIdis found will produce malformed paths like/workspace//datasources. This can cause routing failures and is difficult to debug.Apply this diff to throw an error instead of returning an empty string:
resolveEntityIdForWorkspace(builderParams: URLBuilderParams): string { if (builderParams?.workspaceId) { return builderParams.workspaceId; } const currentUrl = window.location.pathname; const workspaceMatch = currentUrl.match(/^\/workspace\/([^\/]+)/); + const workspaceId = workspaceMatch ? workspaceMatch[1] : ""; - return workspaceMatch ? workspaceMatch[1] : ""; + if (!workspaceId) { + throw new URIError( + "Missing workspaceId. Provide builderParams.workspaceId or navigate under /workspace/:workspaceId." + ); + } + + return workspaceId; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts(1 hunks)app/client/cypress/locators/commonlocators.json(1 hunks)app/client/cypress/support/Objects/CommonLocators.ts(1 hunks)app/client/src/ce/IDE/Interfaces/IDETypes.ts(1 hunks)app/client/src/ce/IDE/constants/routes.ts(2 hunks)app/client/src/ce/actions/workspaceIDEActions.ts(1 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(2 hunks)app/client/src/ce/entities/URLRedirect/URLAssembly.ts(5 hunks)app/client/src/ce/navigation/FocusStrategy/index.ts(1 hunks)app/client/src/ce/pages/Applications/WorkspaceMenu.tsx(1 hunks)app/client/src/ce/pages/common/AppHeader.tsx(2 hunks)app/client/src/ce/reducers/uiReducers/editorReducer.tsx(3 hunks)app/client/src/ce/sagas/index.tsx(2 hunks)app/client/src/ce/sagas/workspaceIDESagas.ts(1 hunks)app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx(1 hunks)app/client/src/constants/routes/baseRoutes.ts(1 hunks)app/client/src/ee/actions/workspaceIDEActions.ts(1 hunks)app/client/src/ee/sagas/workspaceIDESagas.ts(1 hunks)app/client/src/entities/Engine/WorkspaceEditorEngine.ts(1 hunks)app/client/src/pages/AppIDE/layouts/components/Header/index.tsx(1 hunks)app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx(1 hunks)app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx(2 hunks)app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx(2 hunks)app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)app/client/src/pages/workspace/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (11)
- app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx
- app/client/src/pages/AppIDE/layouts/components/Header/index.tsx
- app/client/src/ee/actions/workspaceIDEActions.ts
- app/client/src/ce/navigation/FocusStrategy/index.ts
- app/client/src/ce/constants/ReduxActionConstants.tsx
- app/client/src/ce/pages/Applications/WorkspaceMenu.tsx
- app/client/src/ce/sagas/index.tsx
- app/client/cypress/support/Objects/CommonLocators.ts
- app/client/src/ce/reducers/uiReducers/editorReducer.tsx
- app/client/src/ce/sagas/workspaceIDESagas.ts
- app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/**/**.*
⚙️ CodeRabbit configuration file
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Files:
app/client/cypress/locators/commonlocators.json
🧠 Learnings (22)
📚 Learning: 2024-10-11T09:28:32.636Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36622
File: app/client/src/entities/Engine/AppEditorEngine.ts:0-0
Timestamp: 2024-10-11T09:28:32.636Z
Learning: In the file `app/client/src/entities/Engine/AppEditorEngine.ts`, the IDs (`currentApplication.baseId`) and branches (`currentBranch`) are obfuscated, so it's acceptable to log them for debugging purposes.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T11:00:39.952Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchGitMetadataRequest.ts:6-10
Timestamp: 2024-12-05T11:00:39.952Z
Learning: The `fetchGitMetadataRequest` function in `app/client/src/git/requests/fetchGitMetadataRequest.ts` does not require additional error handling or timeout configuration.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.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/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T10:57:15.397Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/updateGlobalConfigRequest.ts:9-13
Timestamp: 2024-12-05T10:57:15.397Z
Learning: In the TypeScript files within `app/client/src/git/requests/`, functions should not include internal error handling or request timeouts; they should allow errors to be thrown directly.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.ts
📚 Learning: 2024-12-05T10:56:13.739Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchSSHKeyRequest.ts:5-9
Timestamp: 2024-12-05T10:56:13.739Z
Learning: In `app/client/src/git/requests/fetchSSHKeyRequest.ts`, the `fetchSSHKeyRequest` function does not require explicit error handling for `baseApplicationId`; the function should throw an error naturally if `baseApplicationId` is missing or invalid.
Applied to files:
app/client/src/entities/Engine/WorkspaceEditorEngine.tsapp/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsxapp/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: saiprabhu-dandanayak
Repo: appsmithorg/appsmith PR: 33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-10-08T15:32:39.374Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsxapp/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: sneha122
Repo: appsmithorg/appsmith PR: 30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsxapp/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 34471
File: app/client/src/ce/api/JSActionAPI.tsx:98-111
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `updateJSCollection` method in `JSActionAPI.tsx` uses optional chaining and logical AND to safely handle potential null or undefined values for `action.datasource`.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.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/entities/URLRedirect/URLAssembly.tsapp/client/src/pages/workspace/WorkspaceDatasourceHeader.tsxapp/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2025-01-15T20:46:05.401Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38681
File: app/client/src/git/requests/helpers/urlArtifactType.ts:3-12
Timestamp: 2025-01-15T20:46:05.401Z
Learning: In URL helper functions like `urlArtifactType`, error handling in switch statements is not required as TypeScript's compile-time checks are sufficient.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.ts
📚 Learning: 2025-10-30T07:15:20.287Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 41312
File: app/client/src/utils/helpers.tsx:1124-1136
Timestamp: 2025-10-30T07:15:20.287Z
Learning: In app/client/src/utils/helpers.tsx, within the getUpdateRouteForSlugPath function, the pageSlug parameter extracted from the route includes the trailing hyphen (e.g., "home-page-" not "home-page"). Additionally, when the static slug conversion branch is executed, params.basePageId is guaranteed to be defined because the code path requires successful page data fetch from the server as a precondition.
Applied to files:
app/client/src/ce/entities/URLRedirect/URLAssembly.tsapp/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-11-13T09:07:54.931Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js:44-47
Timestamp: 2024-11-13T09:07:54.931Z
Learning: When reviewing code in `app/client/cypress/**` paths, ensure that suggestions are appropriate for test code and not for `src` code. Avoid applying source code checks to test code unless relevant.
Applied to files:
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
📚 Learning: 2024-11-29T09:33:28.991Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 37829
File: app/client/packages/rts/src/ctl/backup/links/EnvFileLink.ts:50-65
Timestamp: 2024-11-29T09:33:28.991Z
Learning: In `app/client/packages/rts/src/ctl/backup/links/EnvFileLink.ts`, changes to the `removeSensitiveEnvData` function should be approached cautiously, as it is sensitive and potentially flaky.
Applied to files:
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
📚 Learning: 2024-11-13T09:41:48.119Z
Learnt from: abhvsn
Repo: appsmithorg/appsmith PR: 37326
File: deploy/docker/fs/opt/appsmith/utils/bin/export_db.js:11-17
Timestamp: 2024-11-13T09:41:48.119Z
Learning: In the `export_db.js` script located at `deploy/docker/fs/opt/appsmith/utils/bin/export_db.js`, it's acceptable to use simple string-based detection (e.g., `dbUrl.startsWith('mongodb')`) for determining the database type because the database URL is restrictive, and misconfiguration will cause the server to fail to start up. Therefore, using a URL parsing library for database URL detection is unnecessary in this context.
Applied to files:
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: abhishek-bandameedi
Repo: appsmithorg/appsmith PR: 35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Applied to files:
app/client/cypress/locators/commonlocators.json
📚 Learning: 2024-11-13T09:11:36.959Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js:35-37
Timestamp: 2024-11-13T09:11:36.959Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js`, when writing Cypress end-to-end tests for the Image Widget in JavaScript, use `viewWidgetsPage.imageinner` as the selector when asserting 'src' and 'alt' attributes to maintain consistency.
Applied to files:
app/client/cypress/locators/commonlocators.json
📚 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/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 29965
File: app/client/src/utils/storage.ts:129-129
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `migrateAppIdToEditorId` function in `app/client/src/utils/storage.ts` is intended to be called only when `appId` is present and `editorId` is not in the payload. This context should be considered in future reviews related to this function.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsxapp/client/src/ce/IDE/Interfaces/IDETypes.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/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 36665
File: app/client/src/IDE/Components/FileTab/styles.tsx:19-19
Timestamp: 2024-10-08T15:32:34.115Z
Learning: Do not suggest replacing hardcoded `padding-top: 6px;` with design variables in `app/client/src/IDE/Components/FileTab/styles.tsx`.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: alex-golovanov
Repo: appsmithorg/appsmith PR: 33863
File: app/client/src/pages/Editor/DataSourceEditor/DBForm.tsx:93-98
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The suggestion to optimize conditional rendering in `DatasourceDBEditor` is considered out of scope for the current issue related to making text selectable.
Applied to files:
app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
🧬 Code graph analysis (10)
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (5)
app/client/cypress/support/commands.js (1)
workspaceId(396-396)app/client/src/ce/constants/ReduxActionConstants.tsx (2)
ReduxActionTypes(1328-1373)ReduxActionErrorTypes(1375-1402)app/client/src/actions/ReduxActionTypes.ts (1)
ReduxAction(9-12)app/client/src/sagas/InitSagas.ts (1)
failFastApiCalls(148-168)app/client/src/entities/Engine/index.ts (2)
PluginsNotFoundError(63-63)PluginFormConfigsNotFoundError(64-64)
app/client/src/ce/pages/common/AppHeader.tsx (1)
app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_URL(32-32)WORKSPACE_DATASOURCE_EDITOR_URL(33-33)
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)
app/client/src/ce/entities/URLRedirect/URLAssembly.ts (2)
app/client/cypress/support/commands.js (1)
workspaceId(396-396)app/client/src/constants/routes/baseRoutes.ts (1)
WORKSPACE_EDITOR_URL(31-31)
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (2)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/entities/Plugin/index.ts (1)
Plugin(71-87)
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (2)
app/client/src/pages/Editor/AppsmithLink.tsx (1)
AppsmithLink(29-67)app/client/src/constants/routes/baseRoutes.ts (1)
APPLICATIONS_URL(8-8)
app/client/src/ee/sagas/workspaceIDESagas.ts (2)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1328-1373)app/client/src/ce/sagas/workspaceIDESagas.ts (1)
startWorkspaceIDE(9-34)
app/client/src/pages/workspace/index.tsx (3)
app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/pages/common/PageWrapper.tsx (1)
PageWrapper(63-89)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
WorkspaceDatasourcesPage(60-134)
app/client/src/ce/actions/workspaceIDEActions.ts (1)
app/client/src/ce/constants/ReduxActionConstants.tsx (1)
ReduxActionTypes(1328-1373)
app/client/src/ce/IDE/constants/routes.ts (2)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-6)app/client/src/constants/routes/baseRoutes.ts (2)
WORKSPACE_DATASOURCES_URL(32-32)WORKSPACE_DATASOURCE_EDITOR_URL(33-33)
⏰ 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). (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (19)
app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx (1)
50-50: LGTM — safe optional prop.The change to make
dsUsageMapoptional is safe. The usage at line 161 withget(dsUsageMap, data.id, "")correctly handlesundefinedby returning an empty string.app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
5-5: LGTM — IDE type addition is complete.The addition of
WorkspacetoIDE_TYPEis clean and well-integrated across routing, actions, sagas, and UI components.app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
179-196: LGTM — explicit control flow improves clarity.The refactor from ternary to explicit branching makes the permission logic much clearer. The Workspace guard returning
false(line 185) correctly reflects that datasource actions cannot be created from the workspace datasources context.app/client/src/ce/actions/workspaceIDEActions.ts (1)
1-10: LGTM — clean action creator implementation.The action creator follows standard Redux patterns with a well-defined payload interface.
app/client/src/entities/Engine/WorkspaceEditorEngine.ts (2)
54-57: LGTM — clean engine setup.
59-109: LGTM — proper error handling for plugin/datasource loading.The use of
failFastApiCallswith specific error types follows established patterns and provides clear error propagation.app/client/cypress/locators/commonlocators.json (1)
250-263: LGTM — proper locator patterns for workspace datasources.All new selectors follow Cypress best practices using
data-testidattributes and consistent.t--class naming. No XPath or complex CSS selectors present.app/client/src/ce/pages/common/AppHeader.tsx (1)
21-22: LGTM — workspace datasource routes properly integrated.The new routes for workspace datasources follow the existing pattern and are cleanly integrated into the routing structure.
Also applies to: 27-27, 51-58
app/client/src/ce/IDE/constants/routes.ts (1)
42-45: LGTM — workspace paths properly mapped.The addition of
IDE_TYPE.WorkspacetoIDEBasePathsfollows the established pattern and correctly includes both workspace datasource routes.Also applies to: 55-58
app/client/src/ee/sagas/workspaceIDESagas.ts (1)
1-11: LGTM!Clean CE/EE split pattern with proper saga wiring using
takeLatest.app/client/src/constants/routes/baseRoutes.ts (1)
31-33: LGTM!Workspace route constants follow the existing pattern and are properly composable.
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (2)
29-34: LGTM!The Link now correctly preserves workspace context by including
workspaceIdas a query parameter.
9-16: File not found in repository — unable to verify positioning necessity.The file
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsxcould not be located in the repository. Verification of whetherposition: fixedis redundant or necessary requires access to the actual file, its parent layout components (WorkspaceDatasourcesPage, PageContainer), and comparison with other header components in the codebase. Confirm the correct file path and ensure the code changes are included in the current branch before proceeding with this review.app/client/src/pages/workspace/index.tsx (1)
14-42: LGTM!Route structure correctly handles workspace settings and datasources contexts with appropriate component wrapping.
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (1)
318-334: LGTM!Workspace context detection using
getIDETypeByUrlis robust, and filtering REST_API for workspace datasources is appropriate.app/client/src/ce/entities/URLRedirect/URLAssembly.ts (3)
282-288: LGTM!
getDefaultEditorTypecorrectly identifies Workspace context based on pathname.
290-298: LGTM!
generateBasePathproperly dispatches to workspace or app path generation based on editor type.
300-306: LGTM!
generateBasePathForWorkspacecorrectly usesgeneratePathwithWORKSPACE_EDITOR_URL.app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
107-111: Route path and navigation already use consistent uppercase casing.The route at line 109 uses
/NEW(uppercase), and the navigation call inCreateDatasourceButton.tsx(line 37) also usesdatasources/NEW(uppercase). The cases are already aligned—no action needed.Likely an incorrect or invalid review comment.
| *loadWorkspace(workspaceId: string): any { | ||
| // Set the current workspace context | ||
| yield put({ | ||
| type: ReduxActionTypes.SET_CURRENT_WORKSPACE_ID, | ||
| payload: { | ||
| workspaceId, | ||
| editorId: workspaceId, // Use workspaceId as editorId for workspace context | ||
| }, | ||
| }); | ||
|
|
||
| // Check if workspace exists, if not fetch it | ||
| const workspace = yield select(getWorkspaceFromId, workspaceId); | ||
|
|
||
| if (!workspace) { | ||
| yield put(fetchAllWorkspaces({ workspaceId, fetchEntities: true })); | ||
| // Wait for workspaces to be fetched | ||
| yield take([ | ||
| ReduxActionTypes.FETCH_ALL_WORKSPACES_SUCCESS, | ||
| ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR, | ||
| ]); | ||
| } | ||
| } |
There was a problem hiding this comment.
Handle workspace fetch failure.
The method waits for either FETCH_ALL_WORKSPACES_SUCCESS or FETCH_WORKSPACES_ERROR (lines 47-50) but doesn't check which action was received. If the fetch fails, the saga continues without workspace data, potentially causing unclear downstream failures.
Check the result and throw on error:
// Wait for workspaces to be fetched
- yield take([
+ const result = yield take([
ReduxActionTypes.FETCH_ALL_WORKSPACES_SUCCESS,
ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR,
]);
+
+ if (result.type === ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR) {
+ throw new Error(`Failed to fetch workspace: ${result.payload?.error?.message || 'Unknown error'}`);
+ }
}📝 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.
| *loadWorkspace(workspaceId: string): any { | |
| // Set the current workspace context | |
| yield put({ | |
| type: ReduxActionTypes.SET_CURRENT_WORKSPACE_ID, | |
| payload: { | |
| workspaceId, | |
| editorId: workspaceId, // Use workspaceId as editorId for workspace context | |
| }, | |
| }); | |
| // Check if workspace exists, if not fetch it | |
| const workspace = yield select(getWorkspaceFromId, workspaceId); | |
| if (!workspace) { | |
| yield put(fetchAllWorkspaces({ workspaceId, fetchEntities: true })); | |
| // Wait for workspaces to be fetched | |
| yield take([ | |
| ReduxActionTypes.FETCH_ALL_WORKSPACES_SUCCESS, | |
| ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR, | |
| ]); | |
| } | |
| } | |
| *loadWorkspace(workspaceId: string): any { | |
| // Set the current workspace context | |
| yield put({ | |
| type: ReduxActionTypes.SET_CURRENT_WORKSPACE_ID, | |
| payload: { | |
| workspaceId, | |
| editorId: workspaceId, // Use workspaceId as editorId for workspace context | |
| }, | |
| }); | |
| // Check if workspace exists, if not fetch it | |
| const workspace = yield select(getWorkspaceFromId, workspaceId); | |
| if (!workspace) { | |
| yield put(fetchAllWorkspaces({ workspaceId, fetchEntities: true })); | |
| // Wait for workspaces to be fetched | |
| const result = yield take([ | |
| ReduxActionTypes.FETCH_ALL_WORKSPACES_SUCCESS, | |
| ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR, | |
| ]); | |
| if (result.type === ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR) { | |
| throw new Error(`Failed to fetch workspace: ${result.payload?.error?.message || 'Unknown error'}`); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/client/src/entities/Engine/WorkspaceEditorEngine.ts around lines 31 to
52, the saga waits for either FETCH_ALL_WORKSPACES_SUCCESS or
FETCH_WORKSPACES_ERROR but doesn’t inspect which action arrived; update the flow
to capture the result of the yield take, check its type, and handle errors by
throwing or returning early so downstream code doesn't run without workspace
data; on success re-select the workspace (or proceed) and on error throw an
informative error (or dispatch an error action) so callers know the fetch
failed.
| const WorkspaceDatasourceDefaultView = () => { | ||
| const datasources = useSelector(getDatasources); | ||
|
|
||
| if (!datasources || datasources.length === 0) { | ||
| return <DatasourceBlankState />; | ||
| } | ||
|
|
||
| return <CreateNewDatasourceTab />; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for getDatasources selector implementation
rg -n "getDatasources\s*=" --type ts --type tsx -A 5 | head -50Repository: appsmithorg/appsmith
Length of output: 91
🏁 Script executed:
# Also search for the selector file that might contain getDatasources
fd -e ts -e tsx | xargs grep -l "export.*getDatasources" | head -20Repository: appsmithorg/appsmith
Length of output: 2619
🏁 Script executed:
# Look for workspace-scoped datasource selectors
rg -n "workspace.*datasource|datasource.*workspace" --type ts --type tsx | grep -i selector | head -20Repository: appsmithorg/appsmith
Length of output: 91
🏁 Script executed:
cat app/client/src/selectors/datasourceSelectors.tsRepository: appsmithorg/appsmith
Length of output: 1426
🏁 Script executed:
# Also check the component file to see how getDatasources is used
cat app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsxRepository: appsmithorg/appsmith
Length of output: 4163
🏁 Script executed:
cat app/client/src/ce/selectors/entitiesSelector.tsRepository: appsmithorg/appsmith
Length of output: 50376
🏁 Script executed:
# Also check if there's an ee version that might override it
find app/client/src -name "entitiesSelector.ts" -type fRepository: appsmithorg/appsmith
Length of output: 159
🏁 Script executed:
head -100 app/client/src/ee/selectors/entitiesSelector.tsRepository: appsmithorg/appsmith
Length of output: 110
🏁 Script executed:
# Search for getDatasources in the ee version specifically
grep -n "getDatasources" app/client/src/ee/selectors/entitiesSelector.tsRepository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
grep -rn "workspace.*datasource\|datasource.*workspace" app/client/src/selectors --include="*.ts" | head -20Repository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
grep -rn "workspaceId" app/client/src/ee/selectors --include="*.ts" | head -20Repository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
# Check how other workspace pages handle datasources
grep -n "getDatasources\|workspaceId" app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsxRepository: appsmithorg/appsmith
Length of output: 422
🏁 Script executed:
grep -rn "initWorkspaceIDE" app/client/src --include="*.ts" -A 10 | head -50Repository: appsmithorg/appsmith
Length of output: 407
🏁 Script executed:
# Check datasource reducer to see if workspace filtering happens there
find app/client/src -name "*datasource*reducer*" -type fRepository: appsmithorg/appsmith
Length of output: 46
🏁 Script executed:
# Find datasource-related reducer files more broadly
find app/client/src -path "*/reducer*" -name "*.ts" | xargs grep -l "datasource" | head -10Repository: appsmithorg/appsmith
Length of output: 404
🏁 Script executed:
# Search for the state structure definition
rg -n "entities\.datasources" app/client/src --type ts -B 2 -A 2 | head -40Repository: appsmithorg/appsmith
Length of output: 3150
🏁 Script executed:
head -150 app/client/src/reducers/entityReducers/datasourceReducer.tsRepository: appsmithorg/appsmith
Length of output: 4804
Pass workspaceId to the selector or filter datasources by workspace.
getDatasources returns all datasources across workspaces without filtering. The component has workspaceId available from props but doesn't use it—the blank state will display incorrectly when datasources exist in other workspaces but not in the current one.
🤖 Prompt for AI Agents
In app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx around lines 50
to 58, the component uses getDatasources which returns all datasources across
workspaces so the blank state is wrong for the current workspace; update the
component to use the available workspaceId prop to scope datasource selection —
either call a workspace-aware selector (e.g. getDatasources(state, workspaceId))
or wrap/useSelector with a selector that filters by workspaceId
(useSelector(state => getDatasources(state).filter(d => d.workspaceId ===
workspaceId))) so the blank state and returned view reflect only datasources for
the current workspace.
ccc1395 to
7e44355
Compare
tomjose92
requested review from
a team,
nidhi-nair,
pratapaprasanna and
sharat87
as code owners
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 2, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. You can view the dependency diff in the run log. Look for the check-cyclic-dependencies job in the run. |
… at Workspace level. Added test cases.
tomjose92
force-pushed
the
feat/issue-35669/workspace-datasources
branch
from
7e44355 to
5e346ca
Compare
tomjose92
removed request for
a team,
nidhi-nair,
pratapaprasanna and
sharat87
|
🔴🔴🔴 Cyclic Dependency Check: This PR has increased the number of cyclic dependencies by 2, when compared with the release branch. Refer this document to identify the cyclic dependencies introduced by this PR. You can view the dependency diff in the run log. Look for the check-cyclic-dependencies job in the run. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
app/client/src/ce/reducers/uiReducers/editorReducer.tsx (1)
69-78: Missing error handler leaves UI in loading state indefinitely.There's no handler for
INITIALIZE_WORKSPACE_DATASOURCE_ERROR. If initialization fails,isWorkspaceDatasourceInitializedstaysfalseforever, causing an infinite spinner with no user feedback.Add the error handler:
[ReduxActionTypes.INITIALIZE_WORKSPACE_DATASOURCE_SUCCESS]: ( state: EditorReduxState, ) => { return { ...state, isWorkspaceDatasourceInitialized: true }; }, + [ReduxActionErrorTypes.INITIALIZE_WORKSPACE_DATASOURCE_ERROR]: ( + state: EditorReduxState, + ) => { + return { ...state, isWorkspaceDatasourceInitialized: true }; + },Alternatively, add a separate error flag to display appropriate UI feedback.
app/client/src/entities/Engine/WorkspaceDatasourceEngine.ts (1)
44-51: Handle workspace fetch failure.The saga waits for either
FETCH_ALL_WORKSPACES_SUCCESSorFETCH_WORKSPACES_ERRORbut doesn't check which action was received. If the fetch fails, the saga continues without workspace data, potentially causing unclear downstream failures.Apply this diff to check the result and handle errors:
if (!workspace) { yield put(fetchAllWorkspaces({ workspaceId, fetchEntities: true })); // Wait for workspaces to be fetched - yield take([ + const result = yield take([ ReduxActionTypes.FETCH_ALL_WORKSPACES_SUCCESS, ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR, ]); + + if (result.type === ReduxActionErrorTypes.FETCH_WORKSPACES_ERROR) { + throw new Error(`Failed to fetch workspace: ${result.payload?.error?.message || 'Unknown error'}`); + } }
🧹 Nitpick comments (4)
app/client/cypress/locators/commonlocators.json (1)
250-263: Standardize on data-testid attributes for new locators to align with guidelines.The new entries use inconsistent selector formats: some use
[data-testid='...'](lines 250-251, 260-262) while others use.t--...class selectors (lines 252-259), and one uses an ID selector (line 263). The coding guidelines explicitly state "Use data-* attributes for selectors."To maintain consistency and follow best practices, convert all
.t--class selectors and the ID selector todata-testidformat.Apply this diff to standardize on data-testid attributes:
- "addDatasourceButtonBlankScreen": ".t--add-datasource-button-blank-screen", - "addDatasourceButton": ".t--add-datasource-button", - "datasource": ".t--datasource", - "dataBlankState": ".t--data-blank-state", - "datasourceOptionDelete": ".t--datasource-option-delete", - "cancelEditDatasource": ".t--cancel-edit-datasource", - "datasourceModalSave": ".t--datasource-modal-save", - "headersArray": ".t--headers-array", + "addDatasourceButtonBlankScreen": "[data-testid='t--add-datasource-button-blank-screen']", + "addDatasourceButton": "[data-testid='t--add-datasource-button']", + "datasource": "[data-testid='t--datasource']", + "dataBlankState": "[data-testid='t--data-blank-state']", + "datasourceOptionDelete": "[data-testid='t--datasource-option-delete']", + "cancelEditDatasource": "[data-testid='t--cancel-edit-datasource']", + "datasourceModalSave": "[data-testid='t--datasource-modal-save']", + "headersArray": "[data-testid='t--headers-array']",- "newIntegrationsWrapper": "#new-integrations-wrapper" + "newIntegrationsWrapper": "[data-testid='new-integrations-wrapper']"Verify that the corresponding React components in the codebase have been updated with matching
data-testidattributes for these new selectors.app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (1)
37-39: Consider removing the empty placeholder.The empty
<div />inIDEHeader.Rightappears to be a placeholder. If the component requires children, consider usingnullor a fragment instead, or remove the Right section entirely if it's optional.<IDEHeader.Right> - <div /> </IDEHeader.Right>app/client/src/pages/workspace/index.tsx (1)
15-22: Inline functions incomponentprop cause remounts.Using inline arrow functions with
componentcreates a new component reference on every render, causing unnecessary unmount/remount cycles. Userenderprop instead (as done for the datasources routes).<SentryRoute - component={() => ( + path={`${path}/:workspaceId/settings`} + render={() => ( <PageWrapper displayName="Workspace Settings"> <Settings /> </PageWrapper> )} - path={`${path}/:workspaceId/settings`} /> ... <SentryRoute - component={() => ( + render={() => ( <PageWrapper displayName="Workspace Settings"> <DefaultWorkspacePage /> </PageWrapper> )} />Also applies to: 35-41
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (1)
369-370: Add a clarifying comment for the visibility logic.The check correctly identifies the workspace context, but a comment would help explain why REST/GraphQL APIs are hidden (they're API actions, not datasources) while regular plugins remain visible (they are datasources).
Apply this diff to add clarity:
+ // Hide REST API and GraphQL API creation on workspace datasources page + // (view-only context for datasources; API actions created in app context) const isWorkspaceDatasourcesPage = isWorkspaceContext();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts(1 hunks)app/client/cypress/locators/commonlocators.json(1 hunks)app/client/cypress/support/Objects/CommonLocators.ts(1 hunks)app/client/src/ce/actions/workspaceDatasourceActions.ts(1 hunks)app/client/src/ce/constants/ReduxActionConstants.tsx(2 hunks)app/client/src/ce/pages/Applications/WorkspaceMenu.tsx(1 hunks)app/client/src/ce/pages/common/AppHeader.tsx(2 hunks)app/client/src/ce/reducers/uiReducers/editorReducer.tsx(3 hunks)app/client/src/ce/sagas/index.tsx(2 hunks)app/client/src/ce/sagas/workspaceDatasourceSagas.ts(1 hunks)app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx(2 hunks)app/client/src/ce/utils/workspaceHelpers.ts(1 hunks)app/client/src/constants/routes/baseRoutes.ts(1 hunks)app/client/src/ee/actions/workspaceDatasourceActions.ts(1 hunks)app/client/src/ee/sagas/workspaceDatasourceSagas.ts(1 hunks)app/client/src/entities/Engine/WorkspaceDatasourceEngine.ts(1 hunks)app/client/src/pages/AppIDE/layouts/components/Header/index.tsx(1 hunks)app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx(1 hunks)app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx(2 hunks)app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx(2 hunks)app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx(1 hunks)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx(1 hunks)app/client/src/pages/workspace/index.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- app/client/src/ce/actions/workspaceDatasourceActions.ts
- app/client/src/ce/utils/workspaceHelpers.ts
- app/client/src/constants/routes/baseRoutes.ts
- app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
- app/client/src/ee/actions/workspaceDatasourceActions.ts
- app/client/src/pages/AppIDE/layouts/components/Header/index.tsx
- app/client/src/ce/constants/ReduxActionConstants.tsx
- app/client/cypress/e2e/Regression/ClientSide/Workspace/WorkspaceDatasourceSettings_spec.ts
- app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx
- app/client/cypress/support/Objects/CommonLocators.ts
- app/client/src/pages/Editor/DataSidePane/DataSidePane.tsx
- app/client/src/ce/pages/common/AppHeader.tsx
🧰 Additional context used
📓 Path-based instructions (1)
app/client/cypress/**/**.*
⚙️ CodeRabbit configuration file
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
Files:
app/client/cypress/locators/commonlocators.json
🧠 Learnings (14)
📚 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/index.tsxapp/client/src/ee/sagas/workspaceDatasourceSagas.tsapp/client/src/entities/Engine/WorkspaceDatasourceEngine.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/index.tsxapp/client/src/entities/Engine/WorkspaceDatasourceEngine.ts
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: ankitakinger
Repo: appsmithorg/appsmith PR: 29573
File: app/client/src/utils/autocomplete/__tests__/TernServer.test.ts:254-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: Integration tests cover the `hasCreateDSActionPermissionInApp` function, as clarified by the user.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx
📚 Learning: 2024-10-08T15:32:39.374Z
Learnt from: sharat87
Repo: appsmithorg/appsmith PR: 34471
File: app/client/src/ce/api/JSActionAPI.tsx:143-161
Timestamp: 2024-10-08T15:32:39.374Z
Learning: The `updateJSCollectionActionRefactor` method in `JSActionAPI.tsx` includes a check to ensure `action.datasource` is not null before manipulating its properties.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsxapp/client/src/ce/pages/Applications/WorkspaceMenu.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/utils/BusinessFeatures/permissionPageHelpers.tsxapp/client/src/ce/reducers/uiReducers/editorReducer.tsxapp/client/src/pages/workspace/WorkspaceDatasourceHeader.tsxapp/client/src/entities/Engine/WorkspaceDatasourceEngine.ts
📚 Learning: 2024-12-15T17:45:48.303Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38171
File: app/client/src/git/components/DefaultBranch/DefaultBranchCE.tsx:1-14
Timestamp: 2024-12-15T17:45:48.303Z
Learning: In `app/client/src/git/components/DefaultBranch/DefaultBranchCE.tsx`, the feature flag check is performed at a higher level, so it's acceptable to have `isGitProtectedFeatureLicensed={false}` in this component.
Applied to files:
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: ashit-rath
Repo: appsmithorg/appsmith PR: 35086
File: app/client/src/ce/sagas/helpers.ts:35-37
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The `transformTriggerEvalErrors` function in `app/client/src/ce/sagas/helpers.ts` is typed as a generator function to maintain type safety because it gets overridden in another repository where `yield` statements are used.
Applied to files:
app/client/src/ee/sagas/workspaceDatasourceSagas.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: abhishek-bandameedi
Repo: appsmithorg/appsmith PR: 35133
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0
Timestamp: 2024-10-08T15:32:34.115Z
Learning: In Cypress tests for the Appsmith project, ensure the use of locator variables for selectors and include multiple assertions for comprehensive testing.
Applied to files:
app/client/cypress/locators/commonlocators.json
📚 Learning: 2024-11-13T09:11:36.959Z
Learnt from: vhemery
Repo: appsmithorg/appsmith PR: 37371
File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js:35-37
Timestamp: 2024-11-13T09:11:36.959Z
Learning: In the file `app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js`, when writing Cypress end-to-end tests for the Image Widget in JavaScript, use `viewWidgetsPage.imageinner` as the selector when asserting 'src' and 'alt' attributes to maintain consistency.
Applied to files:
app/client/cypress/locators/commonlocators.json
📚 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/ce/reducers/uiReducers/editorReducer.tsx
📚 Learning: 2024-10-21T13:18:10.293Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 36989
File: app/client/packages/git/src/actions/helpers/createSingleArtifactAction.ts:12-41
Timestamp: 2024-10-21T13:18:10.293Z
Learning: In Redux Toolkit (TypeScript), since it uses Immer internally, assigning the same initial state object across artifacts does not lead to shared mutable state issues.
Applied to files:
app/client/src/ce/reducers/uiReducers/editorReducer.tsx
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: saiprabhu-dandanayak
Repo: appsmithorg/appsmith PR: 33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Applied to files:
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx
📚 Learning: 2024-12-10T10:52:57.789Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 38060
File: app/client/src/git/sagas/deleteBranchSaga.ts:38-45
Timestamp: 2024-12-10T10:52:57.789Z
Learning: In the TypeScript file `app/client/src/git/sagas/deleteBranchSaga.ts`, within the `deleteBranchSaga` function, error handling is managed outside the scope of the catch block. Therefore, casting `error` to `string` in this context is acceptable.
Applied to files:
app/client/src/entities/Engine/WorkspaceDatasourceEngine.ts
📚 Learning: 2024-12-05T10:56:13.739Z
Learnt from: brayn003
Repo: appsmithorg/appsmith PR: 37984
File: app/client/src/git/requests/fetchSSHKeyRequest.ts:5-9
Timestamp: 2024-12-05T10:56:13.739Z
Learning: In `app/client/src/git/requests/fetchSSHKeyRequest.ts`, the `fetchSSHKeyRequest` function does not require explicit error handling for `baseApplicationId`; the function should throw an error naturally if `baseApplicationId` is missing or invalid.
Applied to files:
app/client/src/entities/Engine/WorkspaceDatasourceEngine.ts
🧬 Code graph analysis (7)
app/client/src/ce/sagas/index.tsx (1)
app/client/src/ee/sagas/workspaceDatasourceSagas.ts (1)
workspaceDatasourceSagas(7-14)
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (2)
app/client/src/ce/utils/workspaceHelpers.ts (1)
isWorkspaceContext(37-39)app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
IDE_TYPE(1-5)
app/client/src/ee/sagas/workspaceDatasourceSagas.ts (1)
app/client/src/ce/sagas/workspaceDatasourceSagas.ts (1)
startWorkspaceDatasource(9-37)
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (3)
app/client/src/ce/utils/workspaceHelpers.ts (1)
isWorkspaceContext(37-39)app/client/src/pages/Editor/IntegrationEditor/util.ts (1)
filterSearch(11-20)app/client/src/ce/constants/messages.ts (1)
CREATE_NEW_DATASOURCE_REST_API(402-402)
app/client/src/pages/workspace/index.tsx (3)
app/client/src/components/SentryRoute.tsx (1)
SentryRoute(3-3)app/client/src/pages/common/PageWrapper.tsx (1)
PageWrapper(63-89)app/client/src/pages/workspace/WorkspaceDatasourcesPage.tsx (1)
WorkspaceDatasourcesPage(60-135)
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (2)
app/client/src/pages/Editor/AppsmithLink.tsx (1)
AppsmithLink(29-67)app/client/src/constants/routes/baseRoutes.ts (1)
APPLICATIONS_URL(8-8)
app/client/src/entities/Engine/WorkspaceDatasourceEngine.ts (5)
app/client/src/ce/constants/ReduxActionConstants.tsx (2)
ReduxActionTypes(1330-1375)ReduxActionErrorTypes(1377-1404)app/client/src/actions/ReduxActionTypes.ts (1)
ReduxAction(9-12)app/client/src/sagas/InitSagas.ts (1)
failFastApiCalls(148-168)app/client/src/entities/Engine/index.ts (2)
PluginsNotFoundError(63-63)PluginFormConfigsNotFoundError(64-64)app/client/src/actions/pluginActions.ts (1)
fetchPluginFormConfigs(22-29)
⏰ 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). (4)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (9)
app/client/src/ce/pages/Applications/WorkspaceMenu.tsx (1)
169-182: LGTM!The new Datasources menu item follows the existing pattern for Settings and Members. The hardcoded path approach is consistent with other menu items per the prior discussion.
app/client/src/ee/sagas/workspaceDatasourceSagas.ts (1)
1-14: LGTM!Clean saga wiring with
takeLatest- appropriate for initialization actions where only the latest request matters. The re-export pattern enables EE customization.app/client/src/ce/sagas/index.tsx (1)
60-60: LGTM!Standard saga registration following existing patterns.
Also applies to: 79-79
app/client/src/pages/workspace/WorkspaceDatasourceHeader.tsx (1)
18-43: LGTM overall.The header structure is clean and the workspaceId context is properly preserved in the navigation link.
app/client/src/pages/workspace/index.tsx (1)
23-34: LGTM!The datasource routes correctly use
renderprop and passworkspaceIdfrom match params.app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (2)
47-47: LGTM: Import follows codebase pattern.The import of
isWorkspaceContextis consistent with the CE/EE dual-import pattern used throughout this file.
180-196: Logic simplification is correct; workspace context behavior verified.The refactored code correctly handles three scenarios:
- Workspace context → returns
false(appropriate, as workspace is read-only)- App mode or undefined ideType → requires both datasource and page permissions
- Other IDE types → requires only datasource permission
All identified call sites are in Editor pages (
/editor/*routes), which never reach workspace context, so the earlyfalsereturn is safe and prevents unnecessary permission checks in workspace views.app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (2)
51-52: LGTM! Imports are correctly structured.The type-only import for
IDETypeand regular import forisWorkspaceContextare appropriate for their respective usages.
372-385: LGTM! Visibility logic correctly implemented.The conditions appropriately hide REST API and GraphQL API creation options on the workspace datasources page while preserving datasource plugin visibility. This aligns well with the PR's objective of providing a view-only datasources page at the workspace level.
Failed server tests
|
4 participants
Description
Made changes to show datasources settings page at a workspace level.
This allows an administrator to see all the datasources being used on the workspace from a page without the need to go to App Editor.
Datasource.config.page.mov
Fixes 35669
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Warning
Tests have not run on the HEAD 5e346ca yet
Thu, 04 Dec 2025 13:20:28 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.