From 1e3a82522ee4e05bd51e9be3deb9938e7901edcf Mon Sep 17 00:00:00 2001 From: Ayangade Adeoluwa <37867493+Irongade@users.noreply.github.com> Date: Thu, 8 Jun 2023 10:09:19 +0100 Subject: [PATCH] fix: Disable run button when there are empty fields (#24031) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an API or SQL query is generated, we disable the run button if no input has been provided to the URL and BODY fields respectively. Fixes #23957 #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (fix or feature that would cause existing functionality to not work as expected) - Chore (housekeeping or task changes that don't impact user perception) - This change requires a documentation update > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [x] Manual - [ ] Jest - [ ] Cypress > > #### Test Plan > https://github.com/appsmithorg/TestSmith/issues/2412 > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Test-plan-implementation#speedbreaker-features-to-consider-for-every-change) have been covered - [x] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans/_edit#areas-of-interest) - [x] Test plan has been peer reviewed by project stakeholders and other QA members - [x] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed --- .../Binding/JSObject_Postgress_Table_spec.js | 2 +- .../SuggestedWidgets_spec.js | 2 +- .../ActionExecution/Block_Execution.ts | 33 +++++++++++++++++++ .../ServerSide/ApiTests/API_Search_spec.js | 2 -- .../QueryPane/AddWidgetTableAndBind_spec.js | 2 +- app/client/cypress/support/Pages/ApiPage.ts | 14 ++++++-- .../cypress/support/Pages/DataSources.ts | 10 ++++++ .../fields/EmbeddedDatasourcePathField.tsx | 3 +- .../ApiEditorConstants/ApiEditorConstants.ts | 4 ++- .../src/constants/QueryEditorConstants.ts | 9 ++++- app/client/src/entities/Action/index.ts | 17 +++++++--- .../Editor/APIEditor/CommonEditorForm.tsx | 18 +++++++++- .../Editor/QueryEditor/EditorJSONtoForm.tsx | 22 +++++++++++-- 13 files changed, 119 insertions(+), 19 deletions(-) create mode 100644 app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts diff --git a/app/client/cypress/e2e/Regression/ClientSide/Binding/JSObject_Postgress_Table_spec.js b/app/client/cypress/e2e/Regression/ClientSide/Binding/JSObject_Postgress_Table_spec.js index a40c9d6254..8691ffc1d8 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/Binding/JSObject_Postgress_Table_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/Binding/JSObject_Postgress_Table_spec.js @@ -28,7 +28,7 @@ describe("Addwidget from Query and bind with other widgets", function () { .focus() .type("SELECT * FROM configs LIMIT 10;"); // eslint-disable-next-line cypress/no-unnecessary-waiting - cy.wait(500); + cy.wait(1000); // Mock the response for this test cy.intercept("/api/v1/actions/execute", { fixture: "addWidgetTable-mock", diff --git a/app/client/cypress/e2e/Regression/ClientSide/MobileResponsiveTests/SuggestedWidgets_spec.js b/app/client/cypress/e2e/Regression/ClientSide/MobileResponsiveTests/SuggestedWidgets_spec.js index 9e31ca7e7f..d968a0a924 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/MobileResponsiveTests/SuggestedWidgets_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/MobileResponsiveTests/SuggestedWidgets_spec.js @@ -37,7 +37,7 @@ describe("Check Suggested Widgets Feature in Auto Layout", function () { .focus() .type("SELECT * FROM configs LIMIT 10;"); // eslint-disable-next-line cypress/no-unnecessary-waiting - cy.wait(500); + cy.wait(1000); // Mock the response for this test cy.intercept("/api/v1/actions/execute", { fixture: "addWidgetTable-mock", diff --git a/app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts b/app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts new file mode 100644 index 0000000000..7cee0f2f90 --- /dev/null +++ b/app/client/cypress/e2e/Regression/ServerSide/ActionExecution/Block_Execution.ts @@ -0,0 +1,33 @@ +import { ObjectsRegistry } from "../../../../support/Objects/Registry"; + +const agHelper = ObjectsRegistry.AggregateHelper, + ee = ObjectsRegistry.EntityExplorer, + apiPage = ObjectsRegistry.ApiPage, + dataSources = ObjectsRegistry.DataSources; + +const url = "https://www.google.com"; + +describe("Block Action Execution when no field is present", () => { + it("1. Ensure API Run button is disabled when no url is present", () => { + apiPage.CreateApi("FirstAPI", "GET"); + apiPage.AssertRunButtonDisability(true); + apiPage.EnterURL(url); + apiPage.AssertRunButtonDisability(false); + }); + + it("1. Ensure Run button is disabled when no SQL body field is present", () => { + let name: any; + dataSources.CreateDataSource("MySql", true, false); + cy.get("@dsName").then(($dsName) => { + name = $dsName; + + agHelper.Sleep(1000); + dataSources.NavigateFromActiveDS(name, true); + agHelper.GetNClick(dataSources._templateMenu); + dataSources.EnterQuery("SELECT * from users"); + dataSources.AssertRunButtonDisability(false); + dataSources.EnterQuery(""); + dataSources.AssertRunButtonDisability(true); + }); + }); +}); diff --git a/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Search_spec.js b/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Search_spec.js index dccd005fd7..eaeda9086d 100644 --- a/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Search_spec.js +++ b/app/client/cypress/e2e/Regression/ServerSide/ApiTests/API_Search_spec.js @@ -17,11 +17,9 @@ describe("API Panel Test Functionality ", function () { cy.log("Navigation to API Panel screen successful"); cy.generateUUID().then((uid) => { cy.CreateAPI(`FirstAPI_${uid}`); - cy.RunAPI(); cy.log("Creation of FirstAPI Action successful"); cy.NavigateToAPI_Panel(); cy.CreateAPI(`SecondAPI_${uid}`); - cy.RunAPI(); cy.CheckAndUnfoldEntityItem("Queries/JS"); cy.log("Creation of SecondAPI Action successful"); cy.get(".t--entity-name").contains("FirstAPI"); diff --git a/app/client/cypress/e2e/Regression/ServerSide/QueryPane/AddWidgetTableAndBind_spec.js b/app/client/cypress/e2e/Regression/ServerSide/QueryPane/AddWidgetTableAndBind_spec.js index 6d01aa8b5c..2661f4c838 100644 --- a/app/client/cypress/e2e/Regression/ServerSide/QueryPane/AddWidgetTableAndBind_spec.js +++ b/app/client/cypress/e2e/Regression/ServerSide/QueryPane/AddWidgetTableAndBind_spec.js @@ -32,7 +32,7 @@ describe("Addwidget from Query and bind with other widgets", function () { .focus() .type("SELECT * FROM configs LIMIT 10;"); // eslint-disable-next-line cypress/no-unnecessary-waiting - cy.wait(500); + cy.wait(1000); // Mock the response for this test cy.intercept("/api/v1/actions/execute", { fixture: "addWidgetTable-mock", diff --git a/app/client/cypress/support/Pages/ApiPage.ts b/app/client/cypress/support/Pages/ApiPage.ts index a5bee5319f..cb7f40f37b 100644 --- a/app/client/cypress/support/Pages/ApiPage.ts +++ b/app/client/cypress/support/Pages/ApiPage.ts @@ -113,10 +113,20 @@ export class ApiPage { this.CreateApi(apiName, apiVerb, aftDSSaved); this.EnterURL(url); //this.agHelper.Sleep(2000);// Added because api name edit takes some time to reflect in api sidebar after the call passes. - cy.get(this._apiRunBtn).should("not.be.disabled"); + this.AssertRunButtonDisability(); if (queryTimeout != 10000) this.SetAPITimeout(queryTimeout); } + AssertRunButtonDisability(disabled = false) { + let query = ""; + if (disabled) { + query = "be.disabled"; + } else { + query = "not.be.disabled"; + } + cy.get(this._apiRunBtn).should(query); + } + EnterURL(url: string) { this.agHelper.EnterValue(url, { propFieldName: this._resourceUrl, @@ -397,7 +407,7 @@ export class ApiPage { this.EnterURL(url); this.agHelper.AssertAutoSave(); //this.agHelper.Sleep(2000);// Added because api name edit takes some time to reflect in api sidebar after the call passes. - cy.get(this._apiRunBtn).should("not.be.disabled"); + this.AssertRunButtonDisability(); if (queryTimeout != 10000) this.SetAPITimeout(queryTimeout); } diff --git a/app/client/cypress/support/Pages/DataSources.ts b/app/client/cypress/support/Pages/DataSources.ts index 259d92e526..eb8246a900 100644 --- a/app/client/cypress/support/Pages/DataSources.ts +++ b/app/client/cypress/support/Pages/DataSources.ts @@ -834,6 +834,16 @@ export class DataSources { } } + AssertRunButtonDisability(disabled = false) { + let query = ""; + if (disabled) { + query = "be.disabled"; + } else { + query = "not.be.disabled"; + } + cy.get(this._runQueryBtn).should(query); + } + public ReadQueryTableResponse(index: number, timeout = 100) { //timeout can be sent higher values incase of larger tables this.agHelper.Sleep(timeout); //Settling time for table! diff --git a/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx b/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx index f97af329f9..8a3049473f 100644 --- a/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx +++ b/app/client/src/components/editorComponents/form/fields/EmbeddedDatasourcePathField.tsx @@ -59,6 +59,7 @@ import { TEMP_DATASOURCE_ID } from "constants/Datasource"; import LazyCodeEditor from "components/editorComponents/LazyCodeEditor"; import { getCodeMirrorNamespaceFromEditor } from "utils/getCodeMirrorNamespace"; import { isDynamicValue } from "utils/DynamicBindingUtils"; +import { DEFAULT_DATASOURCE_NAME } from "constants/ApiEditorConstants/ApiEditorConstants"; type ReduxStateProps = { workspaceId: string; @@ -527,7 +528,7 @@ class EmbeddedDatasourcePathComponent extends React.Component< evaluatedValue={this.handleEvaluatedValue()} focusElementName={`${this.props.actionName}.url`} /> - {datasource && datasource.name !== "DEFAULT_REST_DATASOURCE" && ( + {datasource && datasource.name !== DEFAULT_DATASOURCE_NAME && ( = [ + PluginName.POSTGRES, + PluginName.MS_SQL, + PluginName.MY_SQL, + PluginName.ORACLE, +]; + export const PLUGIN_PACKAGE_DBS = [ PluginPackageName.POSTGRES, PluginPackageName.MONGO, diff --git a/app/client/src/entities/Action/index.ts b/app/client/src/entities/Action/index.ts index a7a78c52c0..dd351a4e35 100644 --- a/app/client/src/entities/Action/index.ts +++ b/app/client/src/entities/Action/index.ts @@ -13,11 +13,6 @@ export enum PluginType { REMOTE = "REMOTE", } -// more can be added subsequently. -export enum PluginName { - MONGO = "MongoDB", -} - export enum PluginPackageName { POSTGRES = "postgres-plugin", MONGO = "mongo-plugin", @@ -30,6 +25,17 @@ export enum PluginPackageName { ORACLE = "oracle-plugin", } +// more can be added subsequently. +export enum PluginName { + MONGO = "MongoDB", + POSTGRES = "PostgreSQL", + MY_SQL = "MySQL", + MS_SQL = "Microsoft SQL Server", + GOOGLE_SHEETS = "Google Sheets", + FIRESTORE = "Firestore", + ORACLE = "Oracle", +} + export enum PaginationType { NONE = "NONE", PAGE_NO = "PAGE_NO", @@ -110,6 +116,7 @@ export const isStoredDatasource = (val: any): val is StoredDatasource => { return true; }; export interface StoredDatasource { + name?: string; id: string; pluginId?: string; datasourceConfiguration?: { url?: string }; diff --git a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx index 8c0efdf95c..9e748c0db3 100644 --- a/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx +++ b/app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx @@ -66,6 +66,7 @@ import SearchSnippets from "../../common/SearchSnippets"; import { DocsLink, openDoc } from "../../../constants/DocumentationLinks"; import { getApiPaneConfigSelectedTabIndex } from "selectors/apiPaneSelectors"; import { noop } from "lodash"; +import { DEFAULT_DATASOURCE_NAME } from "constants/ApiEditorConstants/ApiEditorConstants"; const Form = styled.form` position: relative; @@ -554,6 +555,7 @@ function CommonEditorForm(props: CommonFormPropsWithExtraParams) { (state: AppState) => state.entities.actions.map((action) => action.config), equal, ); + const currentActionConfig: Action | undefined = actions.find( (action) => action.id === params.apiId || action.id === params.queryId, ); @@ -572,6 +574,20 @@ function CommonEditorForm(props: CommonFormPropsWithExtraParams) { getPlugin(state, pluginId ?? ""), ); + // this gets the url of the current action's datasource + const actionDatasourceUrl = + currentActionConfig?.datasource?.datasourceConfiguration?.url || ""; + // this gets the name of the current action's datasource + const actionDatasourceName = currentActionConfig?.datasource.name || ""; + + // if the url is empty and the action's datasource name is the default datasource name (this means the api does not have a datasource attached) + // or the user does not have permission, + // we block action execution. + const blockExecution = + (!actionDatasourceUrl && + actionDatasourceName === DEFAULT_DATASOURCE_NAME) || + !isExecutePermitted; + // Debugger render flag const showDebugger = useSelector(showDebuggerFlag); @@ -621,7 +637,7 @@ function CommonEditorForm(props: CommonFormPropsWithExtraParams) { />