diff --git a/app/client/cypress/integration/Smoke_TestSuite/ServerSideTests/OnLoadTests/JSOnLoad_cyclic_dependency_errors_spec.js b/app/client/cypress/integration/Smoke_TestSuite/ServerSideTests/OnLoadTests/JSOnLoad_cyclic_dependency_errors_spec.js new file mode 100644 index 0000000000..878390c10e --- /dev/null +++ b/app/client/cypress/integration/Smoke_TestSuite/ServerSideTests/OnLoadTests/JSOnLoad_cyclic_dependency_errors_spec.js @@ -0,0 +1,184 @@ +import { ObjectsRegistry } from "../../../../support/Objects/Registry"; + +const dsl = require("../../../../fixtures/inputdsl.json"); +const buttonDsl = require("../../../../fixtures/buttondsl.json"); +const datasource = require("../../../../locators/DatasourcesEditor.json"); +const widgetsPage = require("../../../../locators/Widgets.json"); +const queryLocators = require("../../../../locators/QueryEditor.json"); +import jsActions from "../../../../locators/jsActionLocators.json"; +import { Entity } from "draft-js"; +const jsEditor = ObjectsRegistry.JSEditor; +const ee = ObjectsRegistry.EntityExplorer; +const explorer = require("../../../../locators/explorerlocators.json"); +const agHelper = ObjectsRegistry.AggregateHelper; +const pageid = "MyPage"; +let queryName; + +/* +Cyclic Depedency Error if occurs, Message would be shown in following 6 cases: +1. On page load actions +2. When updating DSL attribute +3. When updating JS Object name +4. When updating Js Object content +5. When updating DSL name +6. When updating Datasource query +*/ + +describe("Cyclic Dependency Informational Error Messagaes", function() { + before(() => { + cy.addDsl(dsl); + cy.wait(3000); //dsl to settle! + }); + + + it("1. Create Users Sample DB & Sample DB Query", () => { + //Step1 + cy.wait(2000); + cy.NavigateToDatasourceEditor(); + + //Step2 + cy.get(datasource.mockUserDatabase).click(); + + //Step3 & 4 + cy.get(`${datasource.datasourceCard}`) + .contains("Users") + .get(`${datasource.createQuery}`) + .last() + .click({ force: true }); + + //Step5.1: Click the editing field + cy.get(".t--action-name-edit-field").click({ force: true }); + + cy.generateUUID().then((uid) => { + queryName = "query" + uid; + //Step5.2: Click the editing field + cy.get(queryLocators.queryNameField).type(queryName); + + // switching off Use Prepared Statement toggle + cy.get(queryLocators.switch) + .last() + .click({ force: true }); + + //Step 6.1: Click on Write query area + cy.get(queryLocators.templateMenu).click(); + cy.get(queryLocators.query).click({ force: true }); + + // Step6.2: writing query to get the schema + cy.get(".CodeMirror textarea") + .first() + .focus() + .type("SELECT gender FROM users ORDER BY id LIMIT 10;", { + force: true, + parseSpecialCharSequences: false, + }); + cy.WaitAutoSave(); + }); + }); + + // Step 1: simulate cyclic depedency + it("2. Create Input Widget & Bind Input Widget Default text to Query Created", () => { + cy.get(widgetsPage.widgetSwitchId).click(); + cy.openPropertyPane("inputwidgetv2"); + cy.get(widgetsPage.defaultInput).type("{{" + queryName + ".data[0].gender"); + cy.widgetText("gender", widgetsPage.inputWidget, widgetsPage.inputval); + cy.assertPageSave(); + }); + + + + //Case 1: On page load actions + it ("3. Reload Page and it should provide errors in response", () => { + // cy.get(widgetsPage.NavHomePage).click({ force: true }); + cy.reload(); + cy.openPropertyPane("inputwidgetv2"); + cy.wait("@getPage").should( + "have.nested.property", + "response.body.data.layouts[0].layoutOnLoadActionErrors.length", + 1, + ); + }); + + it ("4. update input widget's placeholder property and check errors array", () => { + // Case 2: When updating DSL attribute + cy.get(widgetsPage.placeholder).type("cyclic placeholder"); + cy.wait("@updateLayout").should( + "have.nested.property", + "response.body.data.layoutOnLoadActionErrors.length", + 1, + ); + }); + + it("5. Add JSObject and update its name, content and check for errors", () => { + // Case 3: When updating JS Object name + jsEditor.CreateJSObject( + `export default { + fun: async () => { + showAlert("New Js Object"); + } + }`, + { + paste: true, + completeReplace: true, + toRun: true, + shouldCreateNewJSObj: true, + }, + ) + jsEditor.RenameJSObjFromPane("newName") + + cy.wait("@renameJsAction").should( + "have.nested.property", + "response.body.data.layoutOnLoadActionErrors.length", + 1, + ); + + // Case 4: When updating Js Object content + const syncJSCode = `export default { + asyncToSync : ()=>{ + return "yes"; + } + }`; + jsEditor.EditJSObj(syncJSCode, false); + + cy.wait("@jsCollections").should( + "have.nested.property", + "response.body.data.errorReports.length", + 1, + ); + }); + + // Case 5: When updating DSL name + it("6. Update Widget Name and check for errors", () => { + + let entityName = "gender"; + let newEntityName = "newInput"; + ee.SelectEntityByName(entityName, "Widgets"); + agHelper.RenameWidget(entityName, newEntityName); + cy.wait("@updateWidgetName").should( + "have.nested.property", + "response.body.data.layoutOnLoadActionErrors.length", + 1, + ); + + }); + + // Case 6: When updating Datasource query + it("7. Update Query and check for errors", () => { + cy.get(".t--entity-name") + .contains(queryName) + .click({ force: true }); + // update query and check for cyclic depedency issue + cy.get(queryLocators.query).click({ force: true }); + cy.get(".CodeMirror textarea") + .first() + .focus() + .type(" ", { + force: true, + parseSpecialCharSequences: false, + }); + cy.wait("@saveAction").should( + "have.nested.property", + "response.body.data.errorReports.length", + 1, + ); + }); +}); \ No newline at end of file diff --git a/app/client/src/api/PageApi.tsx b/app/client/src/api/PageApi.tsx index 741aecf85a..5e5b1c2ec1 100644 --- a/app/client/src/api/PageApi.tsx +++ b/app/client/src/api/PageApi.tsx @@ -1,7 +1,10 @@ import Api from "api/Api"; import { ApiResponse } from "./ApiResponses"; import axios, { AxiosPromise, CancelTokenSource } from "axios"; -import { PageAction } from "constants/AppsmithActionConstants/ActionConstants"; +import { + LayoutOnLoadActionErrors, + PageAction, +} from "constants/AppsmithActionConstants/ActionConstants"; import { DSLWidget } from "widgets/constants"; import { ClonePageActionPayload, @@ -31,6 +34,7 @@ export type PageLayout = { dsl: Partial; layoutOnLoadActions: PageAction[][]; layoutActions: PageAction[]; + layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[]; }; export type FetchPageResponseData = { @@ -41,6 +45,7 @@ export type FetchPageResponseData = { layouts: Array; lastUpdatedTime: number; customSlug?: string; + layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[]; }; export type FetchPublishedPageResponseData = FetchPageResponseData; @@ -56,6 +61,7 @@ export type SavePageResponseData = { name: string; collectionId?: string; }>; + layoutOnLoadActionErrors?: Array; }; export type CreatePageRequest = Omit< diff --git a/app/client/src/ce/constants/ReduxActionConstants.tsx b/app/client/src/ce/constants/ReduxActionConstants.tsx index b3d2bd3fb1..c3c890db73 100644 --- a/app/client/src/ce/constants/ReduxActionConstants.tsx +++ b/app/client/src/ce/constants/ReduxActionConstants.tsx @@ -1,5 +1,8 @@ import { WidgetCardProps, WidgetProps } from "widgets/BaseWidget"; -import { PageAction } from "constants/AppsmithActionConstants/ActionConstants"; +import { + LayoutOnLoadActionErrors, + PageAction, +} from "constants/AppsmithActionConstants/ActionConstants"; import { Workspace } from "constants/workspaceConstants"; import { ERROR_CODES } from "@appsmith/constants/ApiConstants"; import { AppLayoutConfig } from "reducers/entityReducers/pageListReducer"; @@ -916,6 +919,7 @@ export interface UpdateCanvasPayload { currentApplicationId: string; pageActions: PageAction[][]; updatedWidgetIds?: string[]; + layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[]; } export interface ShowPropertyPanePayload { diff --git a/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx b/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx index 4e3740e0c2..01be099555 100644 --- a/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx +++ b/app/client/src/constants/AppsmithActionConstants/ActionConstants.tsx @@ -129,6 +129,12 @@ export interface ExecuteErrorPayload extends ErrorActionPayload { data: ActionResponse; } +export interface LayoutOnLoadActionErrors { + errorType: string; + code: number; + message: string; +} + // Group 1 = datasource (https://www.domain.com) // Group 2 = path (/nested/path) // Group 3 = params (?param=123¶m2=12) diff --git a/app/client/src/entities/Action/index.ts b/app/client/src/entities/Action/index.ts index efce5a33c3..9de7b47ea0 100644 --- a/app/client/src/entities/Action/index.ts +++ b/app/client/src/entities/Action/index.ts @@ -1,6 +1,7 @@ import { EmbeddedRestDatasource } from "entities/Datasource"; import { DynamicPath } from "utils/DynamicBindingUtils"; import _ from "lodash"; +import { LayoutOnLoadActionErrors } from "constants/AppsmithActionConstants/ActionConstants"; import { Plugin } from "api/PluginApi"; export enum PluginType { @@ -114,6 +115,7 @@ export interface BaseAction { confirmBeforeExecute?: boolean; eventData?: any; messages: string[]; + errorReports?: Array; } interface BaseApiAction extends BaseAction { diff --git a/app/client/src/entities/AppsmithConsole/logtype.ts b/app/client/src/entities/AppsmithConsole/logtype.ts index 47ca6f557f..7b5c861e06 100644 --- a/app/client/src/entities/AppsmithConsole/logtype.ts +++ b/app/client/src/entities/AppsmithConsole/logtype.ts @@ -11,6 +11,7 @@ enum LOG_TYPE { JS_ACTION_UPDATE, JS_PARSE_ERROR, JS_PARSE_SUCCESS, + CYCLIC_DEPENDENCY_ERROR, } export default LOG_TYPE; diff --git a/app/client/src/entities/JSCollection/index.ts b/app/client/src/entities/JSCollection/index.ts index 04031eafec..eaca82280f 100644 --- a/app/client/src/entities/JSCollection/index.ts +++ b/app/client/src/entities/JSCollection/index.ts @@ -1,5 +1,6 @@ import { BaseAction } from "../Action"; import { PluginType } from "entities/Action"; +import { LayoutOnLoadActionErrors } from "constants/AppsmithActionConstants/ActionConstants"; export type Variable = { name: string; @@ -16,6 +17,7 @@ export interface JSCollection { actions: Array; body: string; variables: Array; + errorReports?: Array; } export interface JSActionConfig { diff --git a/app/client/src/reducers/uiReducers/editorReducer.tsx b/app/client/src/reducers/uiReducers/editorReducer.tsx index f9a156b879..46e1ee6a3c 100644 --- a/app/client/src/reducers/uiReducers/editorReducer.tsx +++ b/app/client/src/reducers/uiReducers/editorReducer.tsx @@ -6,7 +6,10 @@ import { ReduxActionErrorTypes, } from "@appsmith/constants/ReduxActionConstants"; import moment from "moment"; -import { PageAction } from "constants/AppsmithActionConstants/ActionConstants"; +import { + LayoutOnLoadActionErrors, + PageAction, +} from "constants/AppsmithActionConstants/ActionConstants"; const initialState: EditorReduxState = { initialized: false, @@ -116,6 +119,7 @@ const editorReducer = createReducer(initialState, { currentLayoutId, currentPageId, currentPageName, + layoutOnLoadActionErrors, pageActions, pageWidgetId, } = action.payload; @@ -129,6 +133,7 @@ const editorReducer = createReducer(initialState, { currentApplicationId, currentPageId, pageActions, + layoutOnLoadActionErrors, }; }, [ReduxActionTypes.CLONE_PAGE_INIT]: (state: EditorReduxState) => { @@ -222,6 +227,7 @@ export interface EditorReduxState { isSnipingMode: boolean; isPreviewMode: boolean; zoomLevel: number; + layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[]; loadingStates: { saving: boolean; savingError: boolean; diff --git a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts index 219421da1f..84144d0c75 100644 --- a/app/client/src/sagas/ActionExecution/PluginActionSaga.ts +++ b/app/client/src/sagas/ActionExecution/PluginActionSaga.ts @@ -31,7 +31,7 @@ import { getAppMode, getCurrentApplication, } from "selectors/applicationSelectors"; -import _, { get, isArray, isString, set } from "lodash"; +import { get, isArray, isString, set, find, isNil, flatten } from "lodash"; import AppsmithConsole from "utils/AppsmithConsole"; import { ENTITY_TYPE, PLATFORM_ERROR } from "entities/AppsmithConsole"; import { validateResponse } from "sagas/ErrorSagas"; @@ -50,6 +50,7 @@ import { import { Variant } from "components/ads/common"; import { EventType, + LayoutOnLoadActionErrors, PageAction, RESP_HEADER_DATATYPE, } from "constants/AppsmithActionConstants/ActionConstants"; @@ -57,6 +58,7 @@ import { getCurrentPageId, getIsSavingEntity, getLayoutOnLoadActions, + getLayoutOnLoadIssues, } from "selectors/editorSelectors"; import PerformanceTracker, { PerformanceTransactionName, @@ -110,6 +112,7 @@ import { isTrueObject, findDatatype } from "workers/evaluationUtils"; import { handleExecuteJSFunctionSaga } from "sagas/JSPaneSagas"; import { Plugin } from "api/PluginApi"; import { setDefaultActionDisplayFormat } from "./PluginActionSagaUtils"; +import { checkAndLogErrorsIfCyclicDependency } from "sagas/helper"; enum ActionResponseDataTypes { BINARY = "BINARY", @@ -119,12 +122,9 @@ export const getActionTimeout = ( state: AppState, actionId: string, ): number | undefined => { - const action = _.find( - state.entities.actions, - (a) => a.config.id === actionId, - ); + const action = find(state.entities.actions, (a) => a.config.id === actionId); if (action) { - const timeout = _.get( + const timeout = get( action, "config.actionConfiguration.timeoutInMillisecond", DEFAULT_EXECUTE_ACTION_TIMEOUT_MS, @@ -270,7 +270,7 @@ function* evaluateActionParams( executeActionRequest: ExecuteActionRequest, executionParams?: Record | string, ) { - if (_.isNil(bindings) || bindings.length === 0) { + if (isNil(bindings) || bindings.length === 0) { formData.append("executeActionDTO", JSON.stringify(executeActionRequest)); return []; } @@ -831,7 +831,13 @@ function* executePageLoadAction(pageAction: PageAction) { function* executePageLoadActionsSaga() { try { const pageActions: PageAction[][] = yield select(getLayoutOnLoadActions); - const actionCount = _.flatten(pageActions).length; + const layoutOnLoadActionErrors: LayoutOnLoadActionErrors[] = yield select( + getLayoutOnLoadIssues, + ); + const actionCount = flatten(pageActions).length; + + // when cyclical depedency issue is there, + // none of the page load actions would be executed PerformanceTracker.startAsyncTracking( PerformanceTransactionName.EXECUTE_PAGE_LOAD_ACTIONS, { numActions: actionCount }, @@ -846,10 +852,10 @@ function* executePageLoadActionsSaga() { PerformanceTracker.stopAsyncTracking( PerformanceTransactionName.EXECUTE_PAGE_LOAD_ACTIONS, ); - // We show errors in the debugger once onPageLoad actions // are executed yield put(hideDebuggerErrors(false)); + checkAndLogErrorsIfCyclicDependency(layoutOnLoadActionErrors); } catch (e) { log.error(e); diff --git a/app/client/src/sagas/ActionSagas.ts b/app/client/src/sagas/ActionSagas.ts index e7cf08b11a..e2e34df2f3 100644 --- a/app/client/src/sagas/ActionSagas.ts +++ b/app/client/src/sagas/ActionSagas.ts @@ -111,6 +111,7 @@ import { saasEditorApiIdURL, } from "RouteBuilder"; import { PLUGIN_PACKAGE_MONGO } from "constants/QueryEditorConstants"; +import { checkAndLogErrorsIfCyclicDependency } from "./helper"; export function* createActionSaga( actionPayload: ReduxAction< @@ -325,6 +326,7 @@ export function* updateActionSaga( // @ts-expect-error: Types are not available action, ); + const isValidResponse: boolean = yield validateResponse(response); if (isValidResponse) { const pageName: string = yield select( @@ -356,6 +358,9 @@ export function* updateActionSaga( ); yield put(updateActionSuccess({ data: response.data })); + checkAndLogErrorsIfCyclicDependency( + (response.data as Action).errorReports, + ); } } catch (error) { PerformanceTracker.stopAsyncTracking( diff --git a/app/client/src/sagas/JSActionSagas.ts b/app/client/src/sagas/JSActionSagas.ts index 2ab974d3ca..5d5c29947e 100644 --- a/app/client/src/sagas/JSActionSagas.ts +++ b/app/client/src/sagas/JSActionSagas.ts @@ -45,7 +45,7 @@ import { ERROR_JS_COLLECTION_RENAME_FAIL, } from "@appsmith/constants/messages"; import { validateResponse } from "./ErrorSagas"; -import PageApi, { FetchPageResponse } from "api/PageApi"; +import PageApi, { FetchPageResponse, PageLayout } from "api/PageApi"; import { updateCanvasWithDSL } from "sagas/PageSagas"; import { JSCollectionData } from "reducers/entityReducers/jsActionsReducer"; import { ApiResponse } from "api/ApiResponses"; @@ -56,6 +56,7 @@ import { CreateJSCollectionRequest } from "api/JSActionAPI"; import * as log from "loglevel"; import { builderURL, jsCollectionIdURL } from "RouteBuilder"; import AnalyticsUtil, { EventLocation } from "utils/AnalyticsUtil"; +import { checkAndLogErrorsIfCyclicDependency } from "./helper"; export function* fetchJSCollectionsSaga( action: EvaluationReduxAction, @@ -372,6 +373,9 @@ export function* refactorJSObjectName( } else { yield put(fetchJSCollectionsForPage(pageId)); } + checkAndLogErrorsIfCyclicDependency( + (refactorResponse.data as PageLayout).layoutOnLoadActionErrors, + ); } } } diff --git a/app/client/src/sagas/JSPaneSagas.ts b/app/client/src/sagas/JSPaneSagas.ts index 64258f3bf9..94ce608cea 100644 --- a/app/client/src/sagas/JSPaneSagas.ts +++ b/app/client/src/sagas/JSPaneSagas.ts @@ -83,6 +83,7 @@ import { UserCancelledActionExecutionError } from "sagas/ActionExecution/errorUt import { APP_MODE } from "entities/App"; import { getAppMode } from "selectors/applicationSelectors"; import AnalyticsUtil, { EventLocation } from "utils/AnalyticsUtil"; +import { checkAndLogErrorsIfCyclicDependency } from "./helper"; function* handleCreateNewJsActionSaga( action: ReduxAction<{ pageId: string; from: EventLocation }>, @@ -481,6 +482,9 @@ function* handleUpdateJSCollectionBody( if (isValidResponse) { // @ts-expect-error: response is of type unknown yield put(updateJSCollectionBodySuccess({ data: response?.data })); + checkAndLogErrorsIfCyclicDependency( + (response.data as JSCollection).errorReports, + ); } } } catch (error) { diff --git a/app/client/src/sagas/PageSagas.tsx b/app/client/src/sagas/PageSagas.tsx index 0f2b7599c4..3dff85531e 100644 --- a/app/client/src/sagas/PageSagas.tsx +++ b/app/client/src/sagas/PageSagas.tsx @@ -37,6 +37,7 @@ import PageApi, { FetchPublishedPageRequest, PageLayout, SavePageResponse, + SavePageResponseData, SetPageOrderRequest, UpdatePageRequest, UpdateWidgetNameRequest, @@ -115,6 +116,7 @@ import { DataTree } from "entities/DataTree/dataTreeFactory"; import { builderURL } from "RouteBuilder"; import { failFastApiCalls } from "./InitSagas"; import { takeEvery } from "redux-saga/effects"; +import { checkAndLogErrorsIfCyclicDependency } from "./helper"; const WidgetTypes = WidgetFactory.widgetTypes; @@ -199,6 +201,8 @@ export const getCanvasWidgetsPayload = ( currentLayoutId: pageResponse.data.layouts[0].id, // TODO(abhinav): Handle for multiple layouts currentApplicationId: pageResponse.data.applicationId, pageActions: pageResponse.data.layouts[0].layoutOnLoadActions || [], + layoutOnLoadActionErrors: + pageResponse.data.layouts[0].layoutOnLoadActionErrors || [], }; }; @@ -446,6 +450,10 @@ function* savePageSaga(action: ReduxAction<{ isRetry?: boolean }>) { PerformanceTracker.stopAsyncTracking( PerformanceTransactionName.SAVE_PAGE_API, ); + checkAndLogErrorsIfCyclicDependency( + (savePageResponse.data as SavePageResponseData) + .layoutOnLoadActionErrors, + ); } } catch (error) { PerformanceTracker.stopAsyncTracking( @@ -828,6 +836,9 @@ export function* updateWidgetNameSaga( dsl: response.data.dsl, }, }); + checkAndLogErrorsIfCyclicDependency( + (response.data as PageLayout).layoutOnLoadActionErrors, + ); } } else { yield put({ diff --git a/app/client/src/sagas/helper.ts b/app/client/src/sagas/helper.ts index b0384198ae..0fb9f37039 100644 --- a/app/client/src/sagas/helper.ts +++ b/app/client/src/sagas/helper.ts @@ -1,7 +1,20 @@ +import { Toaster } from "components/ads/Toast"; +import { createMessage } from "ce/constants/messages"; +import { Variant } from "components/ads"; +import { LayoutOnLoadActionErrors } from "constants/AppsmithActionConstants/ActionConstants"; import { FormEvalOutput, ConditionalOutput, } from "reducers/evaluationReducers/formEvaluationReducer"; +import AppsmithConsole from "utils/AppsmithConsole"; +import LOG_TYPE from "entities/AppsmithConsole/logtype"; +import { + ENTITY_TYPE, + Log, + LOG_CATEGORY, + PLATFORM_ERROR, + Severity, +} from "entities/AppsmithConsole"; // function to extract all objects that have dynamic values export const extractFetchDynamicValueFormConfigs = ( @@ -31,3 +44,72 @@ export const extractQueueOfValuesToBeFetched = (evalOutput: FormEvalOutput) => { }); return output; }; + +/** + * Function checks if in API response, cyclic dependency issues are there or not + * + * @param layoutErrors - array of cyclical dependency issues + * @returns boolean + */ +const checkIfNoCyclicDependencyErrors = ( + layoutErrors?: Array, +): boolean => { + return !layoutErrors || (!!layoutErrors && layoutErrors.length === 0); +}; + +/** + * // Function logs all cyclic dependency errors in debugger + * + * @param layoutErrors - array of cyclical dependency issues + */ +const logCyclicDependecyErrors = ( + layoutErrors?: Array, +) => { + if (!!layoutErrors) { + for (let index = 0; index < layoutErrors.length; index++) { + Toaster.show({ + text: createMessage(() => { + return layoutErrors[index]?.errorType; + }), + variant: Variant.danger, + }); + } + AppsmithConsole.addLogs( + layoutErrors.reduce((acc: Log[], error: LayoutOnLoadActionErrors) => { + acc.push({ + severity: Severity.ERROR, + category: LOG_CATEGORY.PLATFORM_GENERATED, + timestamp: Date.now().toString(), + id: error?.code?.toString(), + logType: LOG_TYPE.CYCLIC_DEPENDENCY_ERROR, + text: !!error.message ? error.message : error.errorType, + messages: [ + { + message: !!error.message ? error.message : error.errorType, + type: PLATFORM_ERROR.PLUGIN_EXECUTION, + }, + ], + source: { + type: ENTITY_TYPE.ACTION, + name: error?.code?.toString(), + id: error?.code?.toString(), + }, + }); + return acc; + }, []), + ); + } +}; + +/** + * // Function checks and logs cyclic depedency errors + * + * @param layoutErrors - array of cyclical dependency issues + */ +export const checkAndLogErrorsIfCyclicDependency = ( + layoutErrors?: Array, +) => { + if (!checkIfNoCyclicDependencyErrors(layoutErrors)) { + logCyclicDependecyErrors(layoutErrors); + } +}; diff --git a/app/client/src/selectors/editorSelectors.tsx b/app/client/src/selectors/editorSelectors.tsx index eb8b7c56d8..d273d6d0ec 100644 --- a/app/client/src/selectors/editorSelectors.tsx +++ b/app/client/src/selectors/editorSelectors.tsx @@ -97,6 +97,10 @@ export const getPageSavingError = (state: AppState) => { export const getLayoutOnLoadActions = (state: AppState) => state.ui.editor.pageActions || []; +export const getLayoutOnLoadIssues = (state: AppState) => { + return state.ui.editor.layoutOnLoadActionErrors || []; +}; + export const getIsPublishingApplication = (state: AppState) => state.ui.editor.loadingStates.publishing; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/ErrorDTO.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/ErrorDTO.java index bec7ec4bab..bfd0945db2 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/ErrorDTO.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/exceptions/ErrorDTO.java @@ -28,4 +28,11 @@ public class ErrorDTO implements Serializable { this.code = code; this.message = message; } + + public ErrorDTO(int code, String errorType, String message) { + this.code = code; + this.errorType = errorType; + this.message = message; + + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Layout.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Layout.java index a23b636c35..bd226315c8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Layout.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/Layout.java @@ -5,11 +5,13 @@ import com.appsmith.server.dtos.DslActionDTO; import com.appsmith.server.helpers.CollectionUtils; import com.appsmith.server.helpers.CompareDslActionDTO; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; import lombok.ToString; import net.minidev.json.JSONObject; +import com.appsmith.external.exceptions.ErrorDTO; import java.util.List; import java.util.Set; @@ -38,6 +40,10 @@ public class Layout extends BaseDomain { List> layoutOnLoadActions; + // this attribute will be used to display errors caused white calculating allOnLoadAction PageLoadActionsUtilCEImpl.java + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + List layoutOnLoadActionErrors; + @Deprecated @JsonIgnore Set publishedLayoutActions; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionCollectionDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionCollectionDTO.java index 77e3d8c6e8..bcedc07619 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionCollectionDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionCollectionDTO.java @@ -6,8 +6,10 @@ import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.ActionCollection; import com.appsmith.server.domains.PluginType; import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.external.exceptions.ErrorDTO; import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.annotation.JsonIgnore; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Getter; import lombok.NoArgsConstructor; import lombok.Setter; @@ -44,6 +46,11 @@ public class ActionCollectionDTO { // This field will only be populated if this collection is bound to one plugin (eg: JS) String pluginId; + //this attribute carries error messages while processing the actionCollection + @Transient + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + List errorReports; + PluginType pluginType; @JsonFormat(pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'", timezone = "UTC") diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionDTO.java index 12ab3a43d3..2a74cb8fe3 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionDTO.java @@ -8,6 +8,7 @@ import com.appsmith.external.models.Property; import com.appsmith.server.domains.ActionProvider; import com.appsmith.server.domains.Documentation; import com.appsmith.server.domains.PluginType; +import com.appsmith.external.exceptions.ErrorDTO; import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -61,6 +62,11 @@ public class ActionDTO { ActionConfiguration actionConfiguration; + //this attribute carries error messages while processing the actionCollection + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + @Transient + List errorReports; + Boolean executeOnLoad; Boolean clientSideExecution; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/LayoutDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/LayoutDTO.java index f6852327ec..b2741879a9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/LayoutDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/LayoutDTO.java @@ -1,12 +1,16 @@ package com.appsmith.server.dtos; import com.appsmith.server.domains.ScreenType; +import com.fasterxml.jackson.annotation.JsonProperty; import lombok.Getter; import lombok.Setter; import net.minidev.json.JSONObject; +import com.appsmith.external.exceptions.ErrorDTO; +import org.springframework.data.annotation.Transient; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; @Getter @@ -21,6 +25,11 @@ public class LayoutDTO { List> layoutOnLoadActions; + // this attribute will be used to display errors caused white calculating allOnLoadAction PageLoadActionsUtilCEImpl.java + @Transient + @JsonProperty(access = JsonProperty.Access.READ_ONLY) + List layoutOnLoadActionErrors; + // All the actions which have been updated as part of updateLayout function call List actionUpdates; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java index 88f9670a79..25b86f0708 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java @@ -23,6 +23,7 @@ import com.appsmith.server.dtos.LayoutDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.dtos.RefactorActionNameDTO; import com.appsmith.server.dtos.RefactorNameDTO; +import com.appsmith.external.exceptions.ErrorDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.helpers.DefaultResourcesUtils; @@ -98,6 +99,7 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE { */ private final String preWord = "\\b("; private final String postWord = ")\\b"; + private final String layoutOnLoadActionErrorToastMessage = "A cyclic dependency error has been encountered on current page, \nqueries on page load will not run. \n Please check debugger and Appsmith documentation for more information"; /** @@ -758,11 +760,20 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE { @Override public Mono updateSingleActionWithBranchName(String defaultActionId, ActionDTO action, String branchName) { + String pageId = action.getPageId(); action.setApplicationId(null); action.setPageId(null); return newActionService.findByBranchNameAndDefaultActionId(branchName, defaultActionId, MANAGE_ACTIONS) .flatMap(newAction -> updateSingleAction(newAction.getId(), action)) - .map(responseUtils::updateActionDTOWithDefaultResources); + .map(responseUtils::updateActionDTOWithDefaultResources) + .zipWith(newPageService.findPageById(pageId, MANAGE_PAGES, false), (actionDTO, pageDTO) -> { + // redundant check + if (pageDTO.getLayouts().size() > 0) { + actionDTO.setErrorReports(pageDTO.getLayouts().get(0).getLayoutOnLoadActionErrors()); + } + return actionDTO; + }); + } @Override @@ -891,11 +902,18 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE { AtomicReference validOnPageLoadActions = new AtomicReference<>(Boolean.TRUE); + // setting the layoutOnLoadActionActionErrors to null to remove the existing errors before new DAG calculation. + layout.setLayoutOnLoadActionErrors(new ArrayList<>()); + Mono>> allOnLoadActionsMono = pageLoadActionsUtil .findAllOnLoadActions(pageId, widgetNames, edges, widgetDynamicBindingsMap, flatmapPageLoadActions, actionsUsedInDSL) .onErrorResume(AppsmithException.class, error -> { log.info(error.getMessage()); validOnPageLoadActions.set(FALSE); + layout.setLayoutOnLoadActionErrors(List.of( + new ErrorDTO(error.getAppErrorCode(), + layoutOnLoadActionErrorToastMessage, + error.getMessage()))); return Mono.just(new ArrayList<>()); }); @@ -986,6 +1004,7 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE { layoutDTO.setDsl(layout.getDsl()); layoutDTO.setScreen(layout.getScreen()); layoutDTO.setLayoutOnLoadActions(layout.getLayoutOnLoadActions()); + layoutDTO.setLayoutOnLoadActionErrors(layout.getLayoutOnLoadActionErrors()); layoutDTO.setUserPermissions(layout.getUserPermissions()); return layoutDTO; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java index 0e871a6789..efef2468e2 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutCollectionServiceCEImpl.java @@ -34,9 +34,9 @@ import reactor.core.publisher.Mono; import java.time.Instant; import java.util.HashMap; import java.util.HashSet; -import java.util.List; -import java.util.Map; import java.util.Objects; +import java.util.Map; +import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -437,6 +437,8 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ID)); } + final String pageId = actionCollectionDTO.getPageId(); + Mono branchedActionCollectionMono = actionCollectionService .findByBranchNameAndDefaultCollectionId(branchName, id, MANAGE_ACTIONS) .cache(); @@ -532,6 +534,7 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE .collect(toMap(actionDTO -> actionDTO.getDefaultResources().getActionId(), ActionDTO::getId)) ); + // First collect all valid action ids from before, and diff against incoming action ids return branchedActionCollectionMono .map(branchedActionCollection -> { @@ -584,7 +587,17 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE .flatMap(actionCollectionDTO1 -> actionCollectionService.populateActionCollectionByViewMode( actionCollection.getUnpublishedCollection(), false))) - .map(responseUtils::updateCollectionDTOWithDefaultResources); + .map(responseUtils::updateCollectionDTOWithDefaultResources) + .zipWith(newPageService.findById(pageId, MANAGE_PAGES), + (branchedActionCollection, newPage) -> { + //redundant check + if (newPage.getUnpublishedPage().getLayouts().size() > 0 ) { + // redundant check as the collection lies inside a layout. Maybe required for testcases + branchedActionCollection.setErrorReports(newPage.getUnpublishedPage().getLayouts().get(0).getLayoutOnLoadActionErrors()); + } + + return branchedActionCollection; + }); } @Override diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java index baa526f205..09aee193be 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceImplTest.java @@ -143,6 +143,7 @@ public class ActionCollectionServiceImplTest { Mockito .when(analyticsService.sendArchiveEvent(Mockito.any(), Mockito.any())) .thenAnswer(invocationOnMock -> Mono.justOrEmpty(invocationOnMock.getArguments()[0])); + } DefaultResources setDefaultResources(T collection) { @@ -381,6 +382,12 @@ public class ActionCollectionServiceImplTest { .findByBranchNameAndDefaultPageId(Mockito.any(), Mockito.any(), Mockito.any())) .thenReturn(Mono.just(newPage)); + + Mockito + .when(newPageService + .findById(Mockito.any(), Mockito.any())) + .thenReturn(Mono.just(newPage)); + final Mono actionCollectionDTOMono = layoutCollectionService.updateUnpublishedActionCollection("testId", actionCollectionDTO, null); @@ -456,6 +463,12 @@ public class ActionCollectionServiceImplTest { .when(newPageService.findByBranchNameAndDefaultPageId(Mockito.any(), Mockito.any(), Mockito.any())) .thenReturn(Mono.just(newPage)); + Mockito + .when(newPageService + .findById(Mockito.any(), Mockito.any())) + .thenReturn(Mono.just(newPage)); + + final Mono actionCollectionDTOMono = layoutCollectionService.updateUnpublishedActionCollection("testCollectionId", modifiedActionCollectionDTO, null); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java index 547af5f765..15eb540a85 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java @@ -1551,4 +1551,104 @@ public class LayoutActionServiceTest { }) .verifyComplete(); } + + @Test + @WithUserDetails(value = "api_user") + public void introduceCyclicDependencyAndRemoveLater(){ + + Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor())); + + // creating new action based on which we will introduce cyclic dependency + ActionDTO actionDTO = new ActionDTO(); + actionDTO.setName("actionName"); + actionDTO.setPageId(testPage.getId()); + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionDTO.setActionConfiguration(actionConfiguration); + actionDTO.setDatasource(datasource); + actionDTO.setExecuteOnLoad(true); + + ActionDTO createdAction = layoutActionService.createSingleAction(actionDTO).block(); + + // retrieving layout from test page; + Layout layout = testPage.getLayouts().get(0); + + JSONObject mainDsl = layout.getDsl(); + JSONObject dsl = new JSONObject(); + dsl.put("widgetName", "inputWidget"); + JSONArray temp = new JSONArray(); + temp.addAll(List.of(new JSONObject(Map.of("key", "defaultText" )))); + dsl.put("dynamicBindingPathList", temp); + dsl.put("defaultText", "{{ \tactionName.data[0].inputWidget}}"); + + final JSONObject innerObjectReference = new JSONObject(); + innerObjectReference.put("k", "{{\tactionName.data[0].inputWidget}}"); + + final JSONArray innerArrayReference = new JSONArray(); + innerArrayReference.add(new JSONObject(Map.of("innerK", "{{\tactionName.data[0].inputWidget}}"))); + + dsl.put("innerArrayReference", innerArrayReference); + dsl.put("innerObjectReference", innerObjectReference); + + final ArrayList objects = new ArrayList<>(); + objects.add(dsl); + + mainDsl.put("children", objects); + layout.setDsl(mainDsl); + + LayoutDTO firstLayout = layoutActionService.updateLayout(testPage.getId(), layout.getId(), layout).block(); + + // by default there should be no error in the layout, hence no error should be sent to ActionDTO/ errorReports will be null + if (createdAction.getErrorReports() != null) { + assert(createdAction.getErrorReports() instanceof List || createdAction.getErrorReports() == null); + assert(createdAction.getErrorReports() == null); + } + + // since the dependency has been introduced calling updateLayout will return a LayoutDTO with a populated layoutOnLoadActionErrors + assert(firstLayout.getLayoutOnLoadActionErrors() instanceof List); + assert (firstLayout.getLayoutOnLoadActionErrors().size() ==1 ); + + // refactoring action to carry the existing error in DSL + RefactorActionNameDTO refactorActionNameDTO = new RefactorActionNameDTO(); + refactorActionNameDTO.setOldName("actionName"); + refactorActionNameDTO.setNewName("newActionName"); + refactorActionNameDTO.setLayoutId(layout.getId()); + refactorActionNameDTO.setPageId(testPage.getId()); + refactorActionNameDTO.setActionId(createdAction.getId()); + + Mono layoutDTOMono = layoutActionService.refactorActionName(refactorActionNameDTO); + StepVerifier.create(layoutDTOMono + .map(layoutDTO -> layoutDTO.getLayoutOnLoadActionErrors().size())) + .expectNext(1).verifyComplete(); + + + // updateAction to see if the error persists + actionDTO.setName("finalActionName"); + Mono actionDTOMono = layoutActionService.updateSingleActionWithBranchName(createdAction.getId(), actionDTO, null); + + StepVerifier.create(actionDTOMono.map( + + actionDTO1 -> actionDTO1.getErrorReports().size() + )) + .expectNext(1).verifyComplete(); + + + + JSONObject newDsl = new JSONObject(); + newDsl.put("widgetName", "newInputWidget"); + newDsl.put("innerArrayReference", innerArrayReference); + newDsl.put("innerObjectReference", innerObjectReference); + + objects.remove(0); + objects.add(newDsl); + mainDsl.put("children", objects); + + layout.setDsl(mainDsl); + + LayoutDTO changedLayoutDTO = layoutActionService.updateLayout(testPage.getId(), layout.getId(), layout).block(); + assert(changedLayoutDTO.getLayoutOnLoadActionErrors() instanceof List); + assert (changedLayoutDTO.getLayoutOnLoadActionErrors().size() == 0); + + } + }