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.

Sign up for GitHub

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

feat: Migrate DBform to UQI config #36168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7ae7ae2
fix: Migrated DBForm to UQI
albinAppsmith Sep 6, 2024
45eedc5
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Sep 6, 2024
c3f004b
fix: eval fix for action form
albinAppsmith Sep 6, 2024
2d32589
fix: toggle prepared statement
albinAppsmith Sep 9, 2024
edb664b
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Sep 9, 2024
efd630c
fix: cypress failure due to prepared statment location change
albinAppsmith Sep 9, 2024
7c88887
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Sep 9, 2024
e8fe475
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
albinAppsmith Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/client/cypress/e2e/Regression/ClientSide/Widgets/ListV2/ListV2_PageNo_PageSize_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ describe(
cy.get(queryLocators.queryNameField).type("Query1");

// switching off Use Prepared Statement toggle
cy.get(queryLocators.switch).last().click({ force: true });
_.dataSources.ToggleUsePreparedStatement(false);

_.dataSources.EnterQuery(
"SELECT * FROM users OFFSET {{List1.pageNo * 1}} LIMIT {{List1.pageSize}};",
Expand Down
4 changes: 1 addition & 3 deletions ...lient/cypress/e2e/Regression/ClientSide/Widgets/ListV2/Listv2_BasicServerSideData_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,7 @@ describe(
cy.get(queryLocators.queryNameField).type("Query2");

// switching off Use Prepared Statement toggle
cy.get(queryLocators.switch).last().click({
force: true,
});
_.dataSources.ToggleUsePreparedStatement(false);

//.1: Click on Write query area
_.dataSources.EnterQuery("SELECT * FROM users LIMIT 20;");
Expand Down
2 changes: 1 addition & 1 deletion app/client/cypress/e2e/Regression/ServerSide/Params/ExecutionParams_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ describe(
"select * from {{ this.params.tableName || 'users' }} limit 10",
);
cy.get(queryLocators.settings).click({ force: true });
cy.get(queryLocators.switch).last().click({ force: true });
dataSources.ToggleUsePreparedStatement(false);
cy.xpath(queryLocators.query).click({ force: true });
cy.runQuery();
});
Expand Down
3 changes: 2 additions & 1 deletion app/client/cypress/support/Pages/ApiPage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ export class ApiPage {
| "Response"
| "Errors"
| "Logs"
| "Inspect entity",
| "Inspect entity"
| "Query",
) {
this.agHelper.PressEscape();
this.agHelper.GetNClick(this._visibleTextSpan(tabName), 0, true);
Expand Down
8 changes: 3 additions & 5 deletions app/client/cypress/support/Pages/DataSources.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1116,13 +1116,11 @@ export class DataSources {
});
}

ToggleUsePreparedStatement(
enable = true || false,
toNavigateToSettings = false,
) {
toNavigateToSettings && this.apiPage.SelectPaneTab("Settings");
ToggleUsePreparedStatement(enable = true || false) {
this.apiPage.SelectPaneTab("Settings");
Comment on lines +1119 to +1120
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

if (enable) this.agHelper.CheckUncheck(this._usePreparedStatement, true);
else this.agHelper.CheckUncheck(this._usePreparedStatement, false);
this.apiPage.SelectPaneTab("Query");
}

public EnterQuery(query: string, sleep = 500, toVerifySave = true) {
Expand Down
5 changes: 4 additions & 1 deletion app/client/src/pages/Editor/QueryEditor/FormRender.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ const FormRender = (props: Props) => {
const renderConfig = () => {
try {
// Selectively rendering form based on uiComponent prop
if (uiComponent === UIComponentTypes.UQIDbEditorForm) {
if (
uiComponent === UIComponentTypes.UQIDbEditorForm ||
uiComponent === UIComponentTypes.DbEditorForm
) {
// If the formEvaluation is not ready yet, just show loading state.
if (
props.hasOwnProperty("formEvaluationState") &&
Expand Down
8 changes: 6 additions & 2 deletions app/client/src/sagas/QueryPaneSagas.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,10 @@ function* changeQuerySaga(actionPayload: ReduxAction<ChangeQueryPayload>) {
// Set the initialValues in the state for redux-form lib
yield put(initialize(QUERY_EDITOR_FORM_NAME, formInitialValues));

if (uiComponent === UIComponentTypes.UQIDbEditorForm) {
if (
uiComponent === UIComponentTypes.UQIDbEditorForm ||
uiComponent === UIComponentTypes.DbEditorForm
) {
// Once the initial values are set, we can run the evaluations based on them.
yield put(
startFormEvaluations(
Expand Down Expand Up @@ -312,7 +315,8 @@ function* formValueChangeSaga(
datasourceStorages[currentEnvironment]?.datasourceConfiguration;
}
const postEvalActions =
uiComponent === UIComponentTypes.UQIDbEditorForm
uiComponent === UIComponentTypes.UQIDbEditorForm ||
uiComponent === UIComponentTypes.DbEditorForm
? [
startFormEvaluations(
values.id,
Expand Down
2 changes: 1 addition & 1 deletion app/client/src/workers/Evaluation/formEval.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ export function setFormEvaluationSaga(
hasRouteChanged,
} = payload;
// In case the formData is not ready or the form is not of type UQI, return empty state
if (!actionConfiguration || !actionConfiguration.formData) {
if (!actionConfiguration) {
return currentEvalState;
} else {
return getFormEvaluation(
Expand Down
16 changes: 11 additions & 5 deletions app/server/appsmith-plugins/arangoDBPlugin/src/main/resources/editor.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
{
"editor": [
{
"sectionName": "",
"id": 1,
"controlType": "SECTION_V2",
"identifier": "SECTION-ONE",
"children": [
{
"label": "",
"configProperty": "actionConfiguration.body",
"controlType": "QUERY_DYNAMIC_TEXT"
"controlType": "SINGLE_COLUMN_ZONE",
"identifier": "SO-Z1",
"children": [
{
"label": "",
"configProperty": "actionConfiguration.body",
"controlType": "QUERY_DYNAMIC_TEXT"
}
]
}
]
}
Expand Down
Loading
Loading