Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

chore: show alert modal while turning off prepared statement#41399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
subrata71 wants to merge4 commits intorelease
base:release
Choose a base branch
Loading
fromchore/show-alert-for-turning-off-prepared-statement

Conversation

@subrata71
Copy link
Collaborator

@subrata71subrata71 commentedNov 19, 2025•
edited by github-actionsbot
Loading

Description

Slack thread

Fixes #Issue Number
or
FixesIssue URL

Warning

If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.

Automation

/ok-to-test tags="@tag.All"

🔍 Cypress test results

Caution

🔴 🔴 🔴 Some tests have failed.
Workflow run:https://github.com/appsmithorg/appsmith/actions/runs/19566645196
Commit:bf47c77
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:

  1. cypress/e2e/Regression/Apps/PgAdmin_spec.js
List of identified flaky tests.
Fri, 21 Nov 2025 11:24:05 UTC

Communication

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

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Added security confirmation dialogs for disabling prepared statements and smart BSON substitution, including distinct headings, descriptions and an optional "Learn more" link.

  • Behavior Changes

    • Relevant toggles now open a confirmation modal and defer the change until the user confirms; cancel leaves the setting unchanged.
    • Confirmation and cancel actions can invoke configured callbacks before finalizing.

  • Tests

    • Automated test flows updated to handle and confirm the new security dialogs.

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

@subrata71subrata71 requested a review froma team as acode ownerNovember 19, 2025 11:47
@subrata71subrata71 requested review fromrahulbarwal and removed request fora teamNovember 19, 2025 11:47
@subrata71subrata71 self-assigned thisNov 19, 2025
@subrata71
Copy link
CollaboratorAuthor

/build-deploy-preview skip-tests=true

github-actions[bot] reacted with rocket emojigithub-actions[bot] reacted with eyes emoji

@github-actionsgithub-actionsbot added the skip-changelogAdding this label to a PR prevents it from being listed in the changelog labelNov 19, 2025
@coderabbitai
Copy link
Contributor

coderabbitaibot commentedNov 19, 2025•
edited
Loading

Walkthrough

Adds security confirmation flows and messages for disabling prepared statements and smart BSON substitution: new message constants, a prepared-statement confirmation modal, dynamic security modal content in the generic request confirmation modal, Redux modal type/extensions with optional callbacks, Redux-connected switch handlers that defer disabling until user confirmation, and E2E test updates to handle the modal.

Changes

Cohort / File(s)Summary
Security Message Constants
app/client/src/ce/constants/messages.ts		Adds four exported helper strings: headings and descriptions for disabling prepared statements and disabling smart BSON substitution.
Form control Switch (Redux-integrated)
app/client/src/components/formControls/SwitchControl.tsx		WrapsSwitchField with Reduxconnect, addsSecurityToggleMeta andSECURITY_TOGGLE_CONFIG, introduceshandleSwitchChange to dispatch security modals and defer value changes until confirmation; props updated to includedispatch.
Property control Switch (prepared statement flow)
app/client/src/components/propertyControls/SwitchControl.tsx		AddsSwitchControlState to manage modal visibility, detects prepared-statement switches, opensDisablePreparedStatementConfirmationModal when disabling, and implements confirm/cancel handlers to apply or revert the change.
Prepared-statement confirmation modal
app/client/src/pages/Editor/DisablePreparedStatementConfirmationModal.tsx		New default-exported React modal component withisOpen,onCancel,onConfirm props; uses message constants and renders Cancel / "Disable anyway" actions (confirm button includes testid).
Request confirmation modal (dynamic security content)
app/client/src/pages/Editor/RequestConfirmationModal.tsx		AddsSECURITY_MODAL_CONTENT mapping for security modal types (headings, descriptions, optional docLink); renders dynamic header/body and Learn more link; invokes optionalmodalInfo.onConfirm/onCancel; adjusts button labels and styles for security modals.
Redux modal state & types
app/client/src/reducers/uiReducers/modalActionReducer.ts		AddsDISABLE_PREPARED_STATEMENT andDISABLE_SMART_SUBSTITUTION toModalType; extendsModalInfo with optionalonConfirm?: () => void andonCancel?: () => void.
E2E test support update
app/client/cypress/support/Pages/DataSources.ts		UpdatesToggleUsePreparedStatement helper to check current state, handle the confirmation modal when disabling, confirm it programmatically, and wait for state updates.

Sequence Diagram(s)

sequenceDiagram    autonumber    participant U as User    participant UI as SwitchControl (component)    participant Redux as Store / Modal Reducer    participant Modal as RequestConfirmationModal / DisablePreparedStatementConfirmationModal    participant Form as Form / Field    U->>UI: Click toggle OFF (prepared-statement or smart-substitution)    UI->>UI: Check SECURITY_TOGGLE_CONFIG    alt security toggle        UI->>Redux: dispatch(openModal(type, { onConfirm, onCancel }))        Redux->>Modal: show modal (dynamic heading/description/docLink)        Modal->>U: render Cancel / "Disable anyway"        alt user confirms            U->>Modal: Click "Disable anyway"            Modal->>Redux: invoke modalInfo.onConfirm (if present)            Redux->>Form: apply new false value            Form->>UI: update UI state        else user cancels            U->>Modal: Click "Cancel"            Modal->>Redux: invoke modalInfo.onCancel (if present)            Redux->>Form: keep previous value            Form->>UI: revert UI state        end    else not security toggle        UI->>Form: apply new boolean value directly        Form->>UI: update UI state    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Pay attention to i18n/message usage inRequestConfirmationModal.tsx andmessages.ts.
  • VerifyModalInfo callbacks typing and invocations inmodalActionReducer.ts and dispatch sites (formControls/SwitchControl.tsx).
  • Review confirm/cancel state handling inpropertyControls/SwitchControl.tsx to ensure consistent revert/apply behavior.
  • Confirm E2E helper uses stable selectors/testids (confirm button testid) and appropriate waits.

Poem

🔐 A toggle halted to ask for thought,
A modal warns of risks we ought not.
"Disable anyway" or simply cancel true—
Small confirmations keep the query crew. ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.You can run@coderabbitai generate docstrings to improve docstring coverage.
Description check⚠️ WarningThe PR description references a Slack thread and Cypress test failures but lacks key details like issue link, motivation, and context.Provide the associated issue number/URL, explain the motivation and security impact, and confirm that the failing PgAdmin tests are unrelated or fixed.
✅ Passed checks (1 passed)
Check nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: adding an alert modal when disabling prepared statements, which aligns with the changeset across multiple files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branchchore/show-alert-for-turning-off-prepared-statement

Thanks for usingCodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link

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

Copy link
Contributor

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (5)
app/client/src/components/propertyControls/SwitchControl.tsx (1)

88-100:ResetisUpdatedViaKeyboard when the disable flow is cancelled

If a user initiates the toggle via keyboard and then cancels in the confirmation modal,isUpdatedViaKeyboard staystrue and will incorrectly tag the next toggle as keyboard‑driven. You can clear it on cancel without affecting behavior:

  handleCancelDisable = () => {-    this.setState({ showPreparedStatementConfirmation: false });+    this.isUpdatedViaKeyboard = false;+    this.setState({ showPreparedStatementConfirmation: false });  };
app/client/src/pages/Editor/RequestConfirmationModal.tsx (1)

116-120:Consider firingonCancel when the modal is closed via ESC/backdrop

onOpenChange={() => this.handleClose(modalInfo)} closes the modal without invokingmodalInfo.onCancel, so click‑away/ESC closes won’t trigger the custom cancel hook. For symmetry with the Cancel button (and withDisablePreparedStatementConfirmationModal), consider:

-          <Modal-            key={modalInfo.name}-            onOpenChange={() => this.handleClose(modalInfo)}-            open={modalInfo?.modalOpen}-          >+          <Modal+            key={modalInfo.name}+            onOpenChange={(isOpen) => {+              if (!isOpen && modalInfo.onCancel) {+                modalInfo.onCancel();+              }+              this.handleClose(modalInfo);+            }}+            open={modalInfo?.modalOpen}+          >
app/client/src/ce/constants/messages.ts (1)

690-698:Copy reads well; minor grammar tweak is optional

The new security messages are clear and actionable. Tiny optional tweak for the prepared‑statement description:

-export const DISABLE_PREPARED_STATEMENT_CONFIRMATION_DESCRIPTION = () =>-  "Disabling prepared statements exposes your application to SQL injection attacks. This setting will be saved and apply to production queries. Only disable if you absolutely need dynamic SQL patterns that cannot be parameterized. Learn more about the risks in our documentation.";+export const DISABLE_PREPARED_STATEMENT_CONFIRMATION_DESCRIPTION = () =>+  "Disabling prepared statements exposes your application to SQL injection attacks. This setting will be saved and applied to production queries. Only disable if you absolutely need dynamic SQL patterns that cannot be parameterized. Learn more about the risks in our documentation.";

Purely a wording improvement; behavior is fine as‑is.

app/client/src/components/formControls/SwitchControl.tsx (2)

70-97:Logic is correct; minor consistency improvement suggested.

The handler correctly identifies when security toggles are disabled and triggers the confirmation flow. The modal callbacks properly update or preserve the form state.

For consistency, consider usingisSelected instead of hardcodingfalse on line 85:

         onConfirm: () => {           // Update the form value when confirmed-          input.onChange(false);+          input.onChange(isSelected);         },

While functionally equivalent (sinceisTurningOff guaranteesisSelected === false), this makes the intent clearer and reduces cognitive load.

115-116:Consider explicit Redux connection for clarity.

Whileconnect() without arguments correctly injectsdispatch, being explicit improves readability and testability.

-// Connect SwitchField to Redux to access modals and dispatch-const ConnectedSwitchField = connect()(SwitchField);+// Connect SwitchField to Redux to access dispatch+const ConnectedSwitchField = connect(+  null,+  (dispatch) => ({ dispatch })+)(SwitchField);
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between3788dee andd0533b1.

📒 Files selected for processing (6)
  • app/client/src/ce/constants/messages.ts (1 hunks)
  • app/client/src/components/formControls/SwitchControl.tsx (4 hunks)
  • app/client/src/components/propertyControls/SwitchControl.tsx (4 hunks)
  • app/client/src/pages/Editor/DisablePreparedStatementConfirmationModal.tsx (1 hunks)
  • app/client/src/pages/Editor/RequestConfirmationModal.tsx (4 hunks)
  • app/client/src/reducers/uiReducers/modalActionReducer.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-16T19:45:00.807Z
Learnt from: brayn003Repo: appsmithorg/appsmith PR: 38171File: app/client/src/git/ce/components/GitModals/index.tsx:1-7Timestamp: 2024-12-16T19:45:00.807ZLearning: The imports in `GitModals` for `ConnectModal`, `DisableAutocommitModal`, `DisconnectModal`, and `SettingsModal` are correct and should not be flagged.

Applied to files:

  • app/client/src/pages/Editor/DisablePreparedStatementConfirmationModal.tsx
📚 Learning: 2024-10-22T04:46:13.268Z
Learnt from: ashit-rathRepo: appsmithorg/appsmith PR: 36926File: app/client/src/ce/pages/Editor/IDE/Header/useLibraryHeaderTitle.ts:1-1Timestamp: 2024-10-22T04:46:13.268ZLearning: In this project, importing constants from 'ee/constants/messages' in CE files is a known and acceptable pattern.

Applied to files:

  • app/client/src/ce/constants/messages.ts
🧬 Code graph analysis (4)
app/client/src/components/propertyControls/SwitchControl.tsx (1)
app/client/src/components/formControls/BaseControl.tsx (1)
  • ControlProps (52-57)
app/client/src/pages/Editor/DisablePreparedStatementConfirmationModal.tsx (1)
app/client/src/ce/constants/messages.ts (2)
  • DISABLE_PREPARED_STATEMENT_CONFIRMATION_HEADING (690-691)
  • DISABLE_PREPARED_STATEMENT_CONFIRMATION_DESCRIPTION (692-693)
app/client/src/components/formControls/SwitchControl.tsx (3)
app/client/src/reducers/uiReducers/modalActionReducer.ts (1)
  • ModalInfo (30-36)
app/client/src/actions/pluginActionActions.ts (1)
  • showActionConfirmationModal (145-150)
app/client/src/components/propertyControls/SwitchControl.tsx (1)
  • SwitchControlProps (132-132)
app/client/src/pages/Editor/RequestConfirmationModal.tsx (2)
app/client/src/ce/constants/messages.ts (5)
  • DISABLE_PREPARED_STATEMENT_CONFIRMATION_HEADING (690-691)
  • DISABLE_PREPARED_STATEMENT_CONFIRMATION_DESCRIPTION (692-693)
  • DISABLE_SMART_SUBSTITUTION_CONFIRMATION_HEADING (695-696)
  • DISABLE_SMART_SUBSTITUTION_CONFIRMATION_DESCRIPTION (697-698)
  • QUERY_CONFIRMATION_MODAL_MESSAGE (1635-1636)
app/client/src/actions/pluginActionActions.ts (1)
  • cancelActionConfirmationModal (152-156)
⏰ 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 (7)
app/client/src/reducers/uiReducers/modalActionReducer.ts (1)

23-36:Enum and ModalInfo extensions look safe and backwards compatible

NewModalType members and optionalonConfirm /onCancel hooks integrate cleanly with the existing reducer shape and shouldn’t disturb current modal flows.

app/client/src/pages/Editor/DisablePreparedStatementConfirmationModal.tsx (1)

1-66:Confirmation modal implementation is straightforward and consistent

The controlledModal wiring, shared message usage, and routing of all close paths throughonCancel/onConfirm look good and match the intended security‑warning UX.

app/client/src/components/propertyControls/SwitchControl.tsx (1)

102-118:Modal integration is nicely scoped to prepared‑statement switches

RenderingDisablePreparedStatementConfirmationModal alongside the existingSwitch and gating its visibility viashowPreparedStatementConfirmation keeps the new UX isolated to the prepared‑statement properties without impacting other switch usages.

app/client/src/pages/Editor/RequestConfirmationModal.tsx (1)

31-49:Security modal content mapping is clean and extensible

TheSECURITY_MODAL_CONTENT map plus the conditional header/body rendering gives a clear separation between security‑sensitive confirmations and generic ones, and should make it easy to add new security modal types later without touching the core rendering logic.

Also applies to: 125-157

app/client/src/components/formControls/SwitchControl.tsx (3)

9-14:LGTM: Imports support the security confirmation workflow.

The imports correctly bring in Redux integration and modal-related types needed for the confirmation flow.

58-58:LGTM: Component signature correctly updated.

The type parameter properly reflects the new dispatch dependency.

124-124:LGTM: Correctly uses the Redux-connected component.

The Field now renders the connected version with dispatch access.

@subrata71subrata71 added the ok-to-testRequired label for CI labelNov 19, 2025
@github-actions
Copy link

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

Copy link
Contributor

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweend0533b1 and760116e.

📒 Files selected for processing (2)
  • app/client/cypress/support/Pages/DataSources.ts (1 hunks)
  • app/client/src/pages/Editor/RequestConfirmationModal.tsx (4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/client/src/pages/Editor/RequestConfirmationModal.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/support/Pages/DataSources.ts
⏰ 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). (8)
  • GitHub Check: perform-test / rts-build / build
  • GitHub Check: perform-test / server-build / server-unit-tests
  • GitHub Check: perform-test / client-build / client-build
  • 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

as it has been deprecated. Are you sure you want to proceed?`;

exportconstDISABLE_PREPARED_STATEMENT_CONFIRMATION_HEADING=()=>
"⚠️ Security Warning: Disable Prepared Statements?";
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not use emojis and have only texts here

this.agHelper.CheckUncheck(this._usePreparedStatement,false);
// Wait for and confirm the security warning modal
this.agHelper.AssertElementVisibility(
"[data-testid='t--disable-prepared-statement-confirm-button']",
Copy link
Contributor

Choose a reason for hiding this comment

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

Put the selector in a locator file as constant, import and use

};

onToggle=()=>{
constisPreparedStatement=this.isPreparedStatementSwitch();
Copy link
Contributor

Choose a reason for hiding this comment

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

The implementation is incorrect. The SwitchControl is a generic form field that can be used anywhere in any form. Since this is generic, we cannot inject specific logic of one form into it.

One way I can think of is to update thepluginFormConfigs that we receive in consolidated api to have properties to enable modals in this SwitchControl and keep it generic
or create a a new control calledSwitchWithConfirmationControl which has the modal and then update the plugin config to use this switch type

<ModalBody>
{createMessage(QUERY_CONFIRMATION_MODAL_MESSAGE)}{" "}
<b>{modalInfo.name}</b> ?
{SECURITY_MODAL_CONTENT[modalInfo.modalType] ?(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this modal serves to different confirmation types, and all of their content seems to be different. I'd suggest to do the following

  1. MoveSECURITY_MODAL_CONTENT to a constants file
  2. Render the query confirmation modal info similar to the prepared statement and smart substitution

Basically drive all confirmation content in a similar way (old and new). Have a constant file to keep all the data and then use the modalType to select the right content.

Copy link
Contributor

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
app/client/cypress/support/Pages/DataSources.ts (1)

1210-1213:Extract hard-coded selector to a locator variable.

The inline selector"[data-testid='t--disable-prepared-statement-confirm-button']" should be defined as a class property for consistency with the existing pattern (see lines 69-323). This has already been raised in previous review comments.

Based on coding guidelines

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between2de9e1f andbf47c77.

📒 Files selected for processing (1)
  • app/client/cypress/support/Pages/DataSources.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/support/Pages/DataSources.ts
🧠 Learnings (7)
📚 Learning: 2024-10-08T15:32:34.114Z
Learnt from: sagar-qa007Repo: appsmithorg/appsmith PR: 34955File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settingsTe_Spec.ts:33-33Timestamp: 2024-10-08T15:32:34.114ZLearning: 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/support/Pages/DataSources.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: abhishek-bandameediRepo: appsmithorg/appsmith PR: 35133File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Checkbox/CheckBox2_spec.js:0-0Timestamp: 2024-10-08T15:32:34.115ZLearning: 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/support/Pages/DataSources.ts
📚 Learning: 2024-10-08T15:32:34.115Z
Learnt from: sagar-qa007Repo: appsmithorg/appsmith PR: 34955File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56Timestamp: 2024-10-08T15:32:34.115ZLearning: Learnt from: sagar-qa007PR: appsmithorg/appsmith#34955File: app/client/cypress/e2e/Regression/ClientSide/ActionExecution/General_settings_Spec.ts:14-56Timestamp: 2024-07-16T06:44:55.118ZLearning: 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/support/Pages/DataSources.ts
📚 Learning: 2024-11-13T09:07:54.931Z
Learnt from: vhemeryRepo: appsmithorg/appsmith PR: 37371File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_spec.js:44-47Timestamp: 2024-11-13T09:07:54.931ZLearning: 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/support/Pages/DataSources.ts
📚 Learning: 2024-11-13T09:11:36.959Z
Learnt from: vhemeryRepo: appsmithorg/appsmith PR: 37371File: app/client/cypress/e2e/Regression/ClientSide/Widgets/Image/Image_base64_spec.js:35-37Timestamp: 2024-11-13T09:11:36.959ZLearning: 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/support/Pages/DataSources.ts
📚 Learning: 2024-07-26T21:12:57.228Z
Learnt from: alex-golovanovRepo: appsmithorg/appsmith PR: 33863File: app/client/src/pages/Editor/DataSourceEditor/DBForm.tsx:93-98Timestamp: 2024-07-26T21:12:57.228ZLearning: 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/cypress/support/Pages/DataSources.ts
📚 Learning: 2025-06-05T11:32:48.592Z
Learnt from: sneha122Repo: appsmithorg/appsmith PR: 40239File: app/client/cypress/support/Pages/AggregateHelper.ts:0-0Timestamp: 2025-06-05T11:32:48.592ZLearning: 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/support/Pages/DataSources.ts
⏰ 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). (8)
  • GitHub Check: perform-test / client-build / client-build
  • GitHub Check: perform-test / server-build / server-unit-tests
  • GitHub Check: perform-test / rts-build / build
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-prettier / prettier-check
  • GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
  • GitHub Check: client-build / client-build
  • GitHub Check: client-unit-tests / client-unit-tests

@github-actions
Copy link

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

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

Reviewers

@ashit-rathashit-rathashit-rath left review comments

@coderabbitaicoderabbitai[bot]coderabbitai[bot] left review comments

@rahulbarwalrahulbarwalAwaiting requested review from rahulbarwalrahulbarwal is a code owner automatically assigned from appsmithorg/widgets-blocks

At least 1 approving review is required to merge this pull request.

Assignees

@subrata71subrata71

Labels

ok-to-testRequired label for CIskip-changelogAdding this label to a PR prevents it from being listed in the changelogStale

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@subrata71@ashit-rath
[8]ページ先頭
©2009-2025 Movatter.jp