Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Migrate DBform to UQI config #36168

Merged
merged 8 commits into from
Sep 17, 2024
Merged

Conversation

albinAppsmith
Copy link
Collaborator

@albinAppsmith albinAppsmith commented Sep 6, 2024

Description

This PR migrates the plugins which were old DBForm config to UQI config.
Plugins that got affected,

  1. Snowflake
  2. ArangoDB
  3. Oracle
  4. PostgreSQL
  5. MySQL
  6. Elasticsearch
  7. DynamoDB
  8. Redis
  9. MSSQL
  10. Redshift

Fixes #35496
Fixes #35497
Fixes #35500
Fixes #35487
Fixes #35490
Fixes #35491
Fixes #35499
Fixes #35501
Fixes #35502
Fixes #35506
Fixes #35890

Automation

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

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/10897087023
Commit: e8fe475
Cypress dashboard.
Tags: @tag.Datasource
Spec:


Tue, 17 Sep 2024 06:07:27 UTC

Communication

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

  • Yes
  • No

Summary by CodeRabbit

  • New Features

    • Enhanced rendering logic in the Query Editor to support additional UI component types.
    • Improved handling of form evaluations and query configurations across various plugins, including ArangoDB, DynamoDB, Elasticsearch, and others.
    • Introduced structured layouts for editor configurations, enhancing clarity and usability in multiple database plugins.
  • Bug Fixes

    • Adjusted control flow in sagas to ensure proper handling of UI component types, improving user interactions.
  • Chores

    • Refined JSON configurations across multiple plugins for better organization and maintainability.

@github-actions github-actions bot added IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product Task A simple Todo labels Sep 6, 2024
Copy link
Contributor

coderabbitai bot commented Sep 6, 2024

Walkthrough

The changes involve structural modifications to the JSON configurations across multiple database plugins, including MSSQL, MySQL, PostgreSQL, ArangoDB, DynamoDB, Elasticsearch, Oracle, Snowflake, Redis, and Redshift. These modifications introduce new control types and identifiers, enhancing the organization and clarity of the editor interfaces. The updates aim to improve the management of query configurations by establishing clear distinctions between prepared and non-prepared statements within a more modular framework.

Changes

File Path Change Summary
app/server/appsmith-plugins/mssqlPlugin/src/main/resources/editor.json Updated structure to include SECTION_V2 and SINGLE_COLUMN_ZONE with new property names for queries.
app/server/appsmith-plugins/mysqlPlugin/src/main/resources/editor.json Similar updates as MSSQL, introducing zones and sections for query handling.
app/server/appsmith-plugins/postgresPlugin/src/main/resources/editor.json Restructured to include SECTION_V2 and SINGLE_COLUMN_ZONE, clarifying query types.
app/server/appsmith-plugins/arangoDBPlugin/src/main/resources/editor.json Enhanced structure with new identifiers and control types for better organization.
app/server/appsmith-plugins/dynamoPlugin/src/main/resources/editor.json Introduced a nested structure with SINGLE_COLUMN_ZONE for improved clarity in query configurations.
app/server/appsmith-plugins/elasticSearchPlugin/src/main/resources/editor.json Updated to a more structured format with defined control types.
app/server/appsmith-plugins/oraclePlugin/src/main/resources/editor.json Similar restructuring to enhance clarity and organization in query handling.
app/server/appsmith-plugins/snowflakePlugin/src/main/resources/editor.json Updated to include structured zones and sections for better management of queries.
app/server/appsmith-plugins/redisPlugin/src/main/resources/editor.json Enhanced structure with clearer identifiers and control types.
app/server/appsmith-plugins/redshiftPlugin/src/main/resources/editor.json Restructured to improve clarity and organization in the editor configuration.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Editor
    participant Plugin

    User->>Editor: Open Query Editor
    Editor->>Plugin: Load Configuration
    Plugin->>Editor: Return Structured Config
    Editor->>User: Display Updated Interface
Loading

Assessment against linked issues

Objective Addressed Explanation
Update form config (35496, 35497, 35500, 35487, 35490, 35491, 35499, 35506)
Move prepared statement to settings (35496, 35497, 35500, 35499)

In the code's embrace, we find,
A structure clearer, well-defined.
With zones and sections, neat and bright,
Queries dance in organized light.
Plugins flourish, each in their place,
A harmonious update, a joyful space! 🎉


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    -- I pushed a fix in commit <commit_id>, please review it.
    -- Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    -- @coderabbitai generate unit testing code for this file.
    -- @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    -- @coderabbitai generate interesting stats about this repository and render them as a table.
    -- @coderabbitai read src/utils.ts and generate unit testing code.
    -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    -- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@albinAppsmith albinAppsmith added the ok-to-test Required label for CI label Sep 6, 2024
@github-actions github-actions bot added the Enhancement New feature or request label Sep 6, 2024
@albinAppsmith
Copy link
Collaborator Author

albinAppsmith commented Sep 6, 2024

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (1)
app/server/appsmith-plugins/arangoDBPlugin/src/main/resources/editor.json (1)

10-16: Review of nested child configuration.

The nested child retains its configProperty which is crucial for maintaining its functionality. The label is empty, which might be intentional but could benefit from a comment explaining why it's left blank, if that's the case.

Consider adding a comment explaining the purpose of an empty label, if it serves a specific function.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 58b07dd and c3f004b.

Files selected for processing (13)
  • app/client/src/pages/Editor/QueryEditor/FormRender.tsx (1 hunks)
  • app/client/src/sagas/QueryPaneSagas.ts (2 hunks)
  • app/client/src/workers/Evaluation/formEval.ts (1 hunks)
  • app/server/appsmith-plugins/arangoDBPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/dynamoPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/elasticSearchPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/mssqlPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/mysqlPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/oraclePlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/postgresPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/redisPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/redshiftPlugin/src/main/resources/editor.json (1 hunks)
  • app/server/appsmith-plugins/snowflakePlugin/src/main/resources/editor.json (1 hunks)
Additional comments not posted (28)
app/server/appsmith-plugins/arangoDBPlugin/src/main/resources/editor.json (2)

4-5: Updated section configuration to SECTION_V2 and SECTION-ONE.

The changes to the section's controlType and identifier enhance the clarity and organization of the configuration. This aligns well with the PR's objectives to improve maintainability and usability.


8-9: Introduced SINGLE_COLUMN_ZONE and SO-Z1 identifier for child element.

This modification suggests a shift towards a more structured and traceable configuration. The new controlType and identifier should help in better managing the content within the editor.

app/server/appsmith-plugins/redisPlugin/src/main/resources/editor.json (3)

4-5: Updated section configuration to SECTION_V2 and SECTION-ONE.

Just like in the ArangoDB plugin, these changes enhance the clarity and organization of the section. This is a positive step towards improving the configuration's usability and maintainability.


8-9: Introduced SINGLE_COLUMN_ZONE and SO-Z1 identifier for child element.

This change introduces a more structured layout for child components, which is beneficial for managing the content more effectively within the editor.


10-17: Review of nested child configuration.

The nested child configuration includes detailed properties such as label, internalLabel, and configProperty. This detailed structuring is good for clarity and traceability. The use of internalLabel is particularly noteworthy as it enhances the semantic clarity of the component.

app/server/appsmith-plugins/redshiftPlugin/src/main/resources/editor.json (3)

4-5: Updated section configuration to SECTION_V2 and SECTION-ONE.

This update, consistent with the other plugins, improves the clarity and organization of the section. It's a positive change towards enhancing the configuration's usability and maintainability.


8-9: Introduced SINGLE_COLUMN_ZONE and SO-Z1 identifier for child element.

The introduction of a more structured layout for child components, as seen in other plugins, is beneficial for managing the content more effectively within the editor.


10-17: Review of nested child configuration.

The nested child configuration includes detailed properties such as label, internalLabel, and configProperty. This detailed structuring is good for clarity and traceability. The use of internalLabel is particularly noteworthy as it enhances the semantic clarity of the component.

app/server/appsmith-plugins/snowflakePlugin/src/main/resources/editor.json (2)

4-5: Updated section control type and identifier.

The changes from a generic sectionName and id to controlType and identifier enhance the clarity and specificity of the section's purpose within the editor. This aligns well with the PR's objectives to improve usability and maintainability.


8-17: Introduction of SINGLE_COLUMN_ZONE and nested structure.

The introduction of SINGLE_COLUMN_ZONE with its own identifier and nested children structure is a significant improvement. It suggests a more organized layout for the query inputs, which should enhance the user experience by making the configuration more intuitive and easier to manage.

app/server/appsmith-plugins/mssqlPlugin/src/main/resources/editor.json (2)

4-5: Updated section control type and identifier.

The transformation from a generic sectionName and id to controlType and identifier is well-executed. This change enhances the clarity and specificity of the section's purpose within the editor, aligning with the PR's objectives.


8-37: Restructured query controls within SINGLE_COLUMN_ZONE.

The restructuring of query-related controls within the SINGLE_COLUMN_ZONE is commendable. The addition of propertyName attributes to distinguish between non-prepared and prepared query types is a thoughtful enhancement. This modular approach allows for better handling of different query execution methods, which should improve the user experience and reduce potential errors in query handling.

app/server/appsmith-plugins/mysqlPlugin/src/main/resources/editor.json (2)

4-5: Updated section control type and identifier.

The update from a generic section structure to a specific controlType labeled SECTION_V2 with a new identifier SECTION-ONE is well-executed. This change enhances the clarity and specificity of the section's purpose within the editor, aligning with the PR's objectives to improve usability and maintainability.


8-37: Restructured query controls within SINGLE_COLUMN_ZONE.

The restructuring of query controls within the SINGLE_COLUMN_ZONE is well-executed. The introduction of specific property names for different types of queries (mysql_query_non_prepared and mysql_query_prepared) enhances clarity and specificity in the configuration. This should improve the user experience by making the distinction between query types more explicit and easier to manage.

app/server/appsmith-plugins/postgresPlugin/src/main/resources/editor.json (3)

4-5: Introduction of SECTION_V2 and SECTION-ONE

The transition to SECTION_V2 and the introduction of the identifier SECTION-ONE are significant improvements. This change enhances the structure and clarity of the configuration, making it easier for developers to navigate and understand the settings. Well done on this update!


8-10: Use of SINGLE_COLUMN_ZONE

The restructuring of children elements under the SINGLE_COLUMN_ZONE is a thoughtful approach. It organizes the child components more effectively, aligning with the objectives to enhance clarity and maintainability of the configuration. This is a positive change that should help in managing the configurations more efficiently.


11-22: Detailed Configuration for Non-Prepared and Prepared Queries

The introduction of specific propertyName attributes for non-prepared (postgres_query_non_prepared) and prepared (postgres_query_prepared) queries is a commendable detail. This explicit distinction improves the usability and clarity of the configuration, making it easier for users to select the appropriate query type based on their needs.

The hidden conditions are well-preserved and correctly implemented, ensuring that the visibility logic based on actionConfiguration.pluginSpecifiedTemplates[0].value is maintained. This careful attention to maintaining existing functionalities while enhancing the configuration structure is excellent.

Also applies to: 24-35

app/server/appsmith-plugins/oraclePlugin/src/main/resources/editor.json (3)

4-5: Introduction of SECTION_V2 and SECTION-ONE

The transition to SECTION_V2 and the introduction of the identifier SECTION-ONE are significant improvements for the Oracle plugin as well. This change enhances the structure and clarity of the configuration, making it easier for developers to navigate and understand the settings. Excellent work on this update!


8-10: Use of SINGLE_COLUMN_ZONE

The restructuring of children elements under the SINGLE_COLUMN_ZONE is a thoughtful approach for the Oracle plugin too. It organizes the child components more effectively, aligning with the objectives to enhance clarity and maintainability of the configuration. This is a positive change that should help in managing the configurations more efficiently.


11-22: Detailed Configuration for Non-Prepared and Prepared Queries

The introduction of specific propertyName attributes for non-prepared (oracle_query_non_prepared) and prepared (oracle_query_prepared) queries is a commendable detail for the Oracle plugin. This explicit distinction improves the usability and clarity of the configuration, making it easier for users to select the appropriate query type based on their needs.

The hidden conditions are well-preserved and correctly implemented, ensuring that the visibility logic based on actionConfiguration.formData.preparedStatement.data is maintained. This careful attention to maintaining existing functionalities while enhancing the configuration structure is excellent.

Also applies to: 24-35

app/server/appsmith-plugins/elasticSearchPlugin/src/main/resources/editor.json (3)

4-5: Introduction of SECTION_V2 and SECTION-ONE

The transition to SECTION_V2 and the introduction of the identifier SECTION-ONE are significant improvements for the Elasticsearch plugin as well. This change enhances the structure and clarity of the configuration, making it easier for developers to navigate and understand the settings. Excellent work on this update!


8-10: Introduction of DOUBLE_COLUMN_ZONE and SINGLE_COLUMN_ZONE

The restructuring of children elements under the DOUBLE_COLUMN_ZONE and SINGLE_COLUMN_ZONE is a thoughtful approach for the Elasticsearch plugin. It organizes the child components more effectively, aligning with the objectives to enhance clarity and maintainability of the configuration. This is a positive change that should help in managing the configurations more efficiently.

Also applies to: 44-46


12-33: Detailed Configuration for Method and Path Selections, and Body Input

The detailed configurations for method and path selections, and body input are well-implemented. The use of DROP_DOWN for method selection and QUERY_DYNAMIC_INPUT_TEXT for path input, along with QUERY_DYNAMIC_TEXT for body input, ensures that the user interface is both functional and user-friendly. The clear delineation of different areas of functionality through the use of distinct zones improves the overall user experience and interface design.

Also applies to: 37-39, 48-51

app/server/appsmith-plugins/dynamoPlugin/src/main/resources/editor.json (2)

4-5: Enhanced Section and Zone Definitions

The introduction of SECTION_V2 and SINGLE_COLUMN_ZONE control types, along with their respective identifiers SECTION-ONE and SO-Z1, significantly enhances the modularity and clarity of the UI configuration. This change aligns with the PR's objectives to improve the organization and usability of the plugin configurations.

Also applies to: 8-9


11-187: Refined Dropdown and Query Configuration

The restructuring of the dropdown and query configuration elements within the new SINGLE_COLUMN_ZONE is well-executed. The detailed breakdown of dropdown options for DynamoDB actions and the clear separation of the query configuration area enhance user interaction by making the UI more intuitive and easier to navigate.

  • The dropdown retains its essential properties such as isRequired, initialValue, and a comprehensive list of options, ensuring that the functionality remains consistent while improving the UI layout.
  • The addition of an empty label for the query dynamic text area (label: "") might seem minimal but is a thoughtful touch to maintain a clean and uncluttered UI.
app/client/src/pages/Editor/QueryEditor/FormRender.tsx (1)

82-85: Expanded Conditional Rendering Logic

The update to include UIComponentTypes.DbEditorForm alongside UIComponentTypes.UQIDbEditorForm in the conditional rendering logic is a positive change. This enhancement broadens the component's flexibility and capability to handle different types of UI components, which is crucial for maintaining a robust and adaptable UI rendering system.

  • Ensure that the rest of the application correctly recognizes and handles the new UIComponentTypes.DbEditorForm to prevent any potential integration issues.
  • Consider adding unit tests to cover these changes to ensure that both conditions are handled correctly without side effects.
app/client/src/sagas/QueryPaneSagas.ts (1)

169-172: Enhanced Conditional Logic in Sagas

The expansion of the conditional checks to include UIComponentTypes.DbEditorForm alongside UIComponentTypes.UQIDbEditorForm in both changeQuerySaga and formValueChangeSaga is a strategic enhancement. This change ensures that the sagas can handle additional UI component types, thereby improving the robustness and flexibility of the form handling logic.

  • It is crucial to ensure that these changes are well-integrated with the rest of the application to maintain consistency and prevent any functional discrepancies.
  • Adding unit tests to cover these new conditions would be beneficial to ensure their correct execution under various scenarios.

Also applies to: 318-319

app/client/src/workers/Evaluation/formEval.ts (1)

561-561: Simplify conditional logic in setFormEvaluationSaga.

The change in the conditional logic simplifies the early exit conditions of the function. Previously, the function checked both the existence of actionConfiguration and the presence of formData within it. Now, it only checks for the existence of actionConfiguration. This change broadens the scenarios where the function can exit early, potentially affecting how different states of input data are handled.

While this simplification can make the code cleaner and possibly more efficient by reducing unnecessary checks, it's crucial to ensure that this change aligns with the intended logic and does not introduce any regressions. Particularly, we need to verify that the absence of formData checks does not skip necessary evaluations that might depend on formData being present.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between c3f004b and 2d32589.

Files selected for processing (1)
  • app/client/cypress/support/Pages/DataSources.ts (1 hunks)
Additional context used
Path-based instructions (1)
app/client/cypress/support/Pages/DataSources.ts (1)

Pattern 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.

Comment on lines +1119 to +1120
ToggleUsePreparedStatement(enable = true || false) {
this.apiPage.SelectPaneTab("Settings");
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Update method calls to match refactored ToggleUsePreparedStatement.

The ToggleUsePreparedStatement method is still being called with a parameter in several test files, which does not align with the recent refactoring. This discrepancy needs to be addressed to ensure consistency and prevent potential issues.

  • Files with outdated method calls:

    • app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js
    • app/client/cypress/e2e/Regression/Apps/PgAdmin_spec.js
    • app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad_cyclic_dependency_errors_spec.js
  • Missing file: MaintainContext&Focus_spec.js was not found. Verify the file path or existence.

Please update the method calls in these files to remove the parameter and ensure the tests reflect the new behavior of the method.

Analysis chain

Refactor: Simplify ToggleUsePreparedStatement method.

The method ToggleUsePreparedStatement has been refactored to remove the toNavigateToSettings parameter, which simplifies its usage. This change aligns with the PR's goal to streamline configurations. The method now unconditionally navigates to the "Settings" tab, which could be seen as a limitation if conditional navigation was a required feature. Ensure that this behavior change is documented and that all tests and usages of this method have been updated accordingly.


Let's verify if the usages of ToggleUsePreparedStatement in the identified files have been updated to reflect the new method signature. We'll check each file to ensure the method is called correctly without the toNavigateToSettings parameter.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify that all usages of `ToggleUsePreparedStatement` have been updated to reflect the new method signature.
rg --type js "ToggleUsePreparedStatement" --files-with-matches

Length of output: 390


Script:

#!/bin/bash
# Check the usage of `ToggleUsePreparedStatement` in the identified files.
rg --type js "ToggleUsePreparedStatement" -A 3 app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js
rg --type js "ToggleUsePreparedStatement" -A 3 app/client/cypress/e2e/Regression/ClientSide/IDE/MaintainContext&Focus_spec.js
rg --type js "ToggleUsePreparedStatement" -A 3 app/client/cypress/e2e/Regression/Apps/PgAdmin_spec.js
rg --type js "ToggleUsePreparedStatement" -A 3 app/client/cypress/e2e/Regression/ServerSide/OnLoadTests/JSOnLoad_cyclic_dependency_errors_spec.js

Length of output: 1528

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (2)
app/client/cypress/support/Pages/ApiPage.ts (2)

Line range hint 183-183: Replace agHelper.Sleep() with a more deterministic wait mechanism.

The use of agHelper.Sleep() in the EnterURL method is not recommended as it can lead to flaky tests. Instead, consider using Cypress's built-in waiting mechanisms that wait for specific conditions to be met, which can provide more reliable and efficient test outcomes.

Consider replacing the Sleep() call with cy.wait() for a specific alias or using cy.get() with a timeout to wait for elements to be visible or actionable.


Line range hint 1-500: Consider refactoring the ApiPage class for better maintainability and performance.

The ApiPage class could benefit from several improvements:

  • Reduce the use of XPath selectors: These are generally slower and less reliable in Cypress tests. Where possible, replace them with data-* attributes or specific CSS selectors.
  • Avoid direct string usage for selectors: Instead, use locator variables to make the code more maintainable and less prone to errors.
  • Decompose the class: The class is quite large and handles multiple functionalities. Consider breaking it down into smaller, more focused classes or modules to improve readability and maintainability.

These changes would align with best practices for Cypress tests and enhance the overall quality and performance of the test suite.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2d32589 and efd630c.

Files selected for processing (5)
  • app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js (1 hunks)
  • app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js (1 hunks)
  • app/client/cypress/e2e/Regression/ServerSide/Params/ExecutionParams_spec.js (1 hunks)
  • app/client/cypress/support/Pages/ApiPage.ts (1 hunks)
  • app/client/cypress/support/Pages/DataSources.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • app/client/cypress/support/Pages/DataSources.ts
Additional context used
Path-based instructions (4)
app/client/cypress/e2e/Regression/ServerSide/Params/ExecutionParams_spec.js (1)

Pattern 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.
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js (1)

Pattern 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.
app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js (1)

Pattern 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.
app/client/cypress/support/Pages/ApiPage.ts (1)

Pattern 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.
Additional comments not posted (4)
app/client/cypress/e2e/Regression/ServerSide/Params/ExecutionParams_spec.js (1)

40-40: Good shift towards more reliable testing methods.

Replacing UI interactions with direct method calls, as seen with dataSources.ToggleUsePreparedStatement(false), enhances test reliability and reduces potential flakiness. This is a commendable improvement in test automation practices.

app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js (1)

172-172: Excellent move towards more stable test automation.

The use of _.dataSources.ToggleUsePreparedStatement(false) to programmatically control the prepared statement toggle is a robust approach. It avoids the pitfalls of relying on UI element interactions, which can lead to flaky tests. This change should contribute positively to the stability and reliability of your test suite.

app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js (1)

201-201: Well-executed improvement in test automation.

The transition from UI-based interactions to direct method calls, specifically _.dataSources.ToggleUsePreparedStatement(false), is a significant enhancement. This approach not only makes the tests more reliable but also simplifies the automation code. Well done on this improvement.

app/client/cypress/support/Pages/ApiPage.ts (1)

290-291: Approved addition of "Query" to tabName, but verify integration.

The addition of "Query" to the tabName parameter is a welcome enhancement, aligning with the PR's objectives to improve the user interface. However, it's crucial to ensure that this new tab is integrated properly within the UI and that all related functionalities are tested thoroughly.

Please verify that the "Query" tab functions as expected and integrates seamlessly with the existing tabs. This includes checking for any UI issues or side effects that might arise from this addition.

Copy link

github-actions bot commented Sep 9, 2024

Failed server tests

  • com.appsmith.server.git.ServerSchemaMigrationEnforcerTest#saveGitRepo_ImportAndThenExport_diffOccurs
  • com.appsmith.server.imports.internal.ImportServiceTests#extractFileAndUpdateApplication_addNewPageAfterImport_addedPageRemoved
  • com.appsmith.server.imports.internal.ImportServiceTests#extractFileAndUpdateExistingApplication_existingApplication_applicationNameAndSlugRemainsUnchanged
  • com.appsmith.server.imports.internal.ImportServiceTests#extractFileAndUpdateExistingApplication_gitConnectedApplication_throwUnsupportedOperationException
  • com.appsmith.server.imports.internal.ImportServiceTests#importApplicationInWorkspaceFromGit_WithAppLayoutInEditMode_ImportedAppHasAppLayoutInEditAndViewMode
  • com.appsmith.server.imports.internal.ImportServiceTests#importApplicationInWorkspaceFromGit_WithNavSettingsInEditMode_ImportedAppHasNavSettingsInEditAndViewMode

@albinAppsmith
Copy link
Collaborator Author

EE DP: https://ee-5059.dp.appsmith.com/

@albinAppsmith albinAppsmith merged commit 66c815f into release Sep 17, 2024
42 checks passed
@albinAppsmith albinAppsmith deleted the action-redesign/dbform-update branch September 17, 2024 08:52
Shivam-z pushed a commit to Shivam-z/appsmith that referenced this pull request Sep 26, 2024
## Description

This PR migrates the plugins which were old DBForm config to UQI config.
Plugins that got affected,
1. Snowflake
2. ArangoDB
3. Oracle
4. PostgreSQL
5. MySQL
6. Elasticsearch
7. DynamoDB
8. Redis
9. MSSQL
10. Redshift


Fixes appsmithorg#35496
Fixes appsmithorg#35497
Fixes appsmithorg#35500
Fixes appsmithorg#35487 
Fixes appsmithorg#35490 
Fixes appsmithorg#35491
Fixes appsmithorg#35499
Fixes appsmithorg#35501 
Fixes appsmithorg#35502
Fixes appsmithorg#35506 
Fixes appsmithorg#35890

## Automation

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

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10897087023>
> Commit: e8fe475
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10897087023&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Datasource`
> Spec:
> <hr>Tue, 17 Sep 2024 06:07:27 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **New Features**
- Enhanced rendering logic in the Query Editor to support additional UI
component types.
- Improved handling of form evaluations and query configurations across
various plugins, including ArangoDB, DynamoDB, Elasticsearch, and
others.
- Introduced structured layouts for editor configurations, enhancing
clarity and usability in multiple database plugins.

- **Bug Fixes**
- Adjusted control flow in sagas to ensure proper handling of UI
component types, improving user interactions.

- **Chores**
- Refined JSON configurations across multiple plugins for better
organization and maintainability.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@coderabbitai coderabbitai bot mentioned this pull request Nov 20, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product ok-to-test Required label for CI Task A simple Todo
Projects
None yet
5 participants