feat: Informative error messages on cyclic dependency for queries on page load (#16634)

* This commit addresses the feature https://github.com/appsmithorg/appsmith/issues/16154

Co-authored-by: “sneha122” <“sneha@appsmith.com”>
This commit is contained in:
Manish Kumar 2022-09-24 15:31:52 +05:30 committed by GitHub
parent 4e13304232
commit bfc79bdfd3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 524 additions and 17 deletions

View File

@ -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,
);
});
});

View File

@ -1,7 +1,10 @@
import Api from "api/Api"; import Api from "api/Api";
import { ApiResponse } from "./ApiResponses"; import { ApiResponse } from "./ApiResponses";
import axios, { AxiosPromise, CancelTokenSource } from "axios"; 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 { DSLWidget } from "widgets/constants";
import { import {
ClonePageActionPayload, ClonePageActionPayload,
@ -31,6 +34,7 @@ export type PageLayout = {
dsl: Partial<DSLWidget>; dsl: Partial<DSLWidget>;
layoutOnLoadActions: PageAction[][]; layoutOnLoadActions: PageAction[][];
layoutActions: PageAction[]; layoutActions: PageAction[];
layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[];
}; };
export type FetchPageResponseData = { export type FetchPageResponseData = {
@ -41,6 +45,7 @@ export type FetchPageResponseData = {
layouts: Array<PageLayout>; layouts: Array<PageLayout>;
lastUpdatedTime: number; lastUpdatedTime: number;
customSlug?: string; customSlug?: string;
layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[];
}; };
export type FetchPublishedPageResponseData = FetchPageResponseData; export type FetchPublishedPageResponseData = FetchPageResponseData;
@ -56,6 +61,7 @@ export type SavePageResponseData = {
name: string; name: string;
collectionId?: string; collectionId?: string;
}>; }>;
layoutOnLoadActionErrors?: Array<LayoutOnLoadActionErrors>;
}; };
export type CreatePageRequest = Omit< export type CreatePageRequest = Omit<

View File

@ -1,5 +1,8 @@
import { WidgetCardProps, WidgetProps } from "widgets/BaseWidget"; 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 { Workspace } from "constants/workspaceConstants";
import { ERROR_CODES } from "@appsmith/constants/ApiConstants"; import { ERROR_CODES } from "@appsmith/constants/ApiConstants";
import { AppLayoutConfig } from "reducers/entityReducers/pageListReducer"; import { AppLayoutConfig } from "reducers/entityReducers/pageListReducer";
@ -916,6 +919,7 @@ export interface UpdateCanvasPayload {
currentApplicationId: string; currentApplicationId: string;
pageActions: PageAction[][]; pageActions: PageAction[][];
updatedWidgetIds?: string[]; updatedWidgetIds?: string[];
layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[];
} }
export interface ShowPropertyPanePayload { export interface ShowPropertyPanePayload {

View File

@ -129,6 +129,12 @@ export interface ExecuteErrorPayload extends ErrorActionPayload {
data: ActionResponse; data: ActionResponse;
} }
export interface LayoutOnLoadActionErrors {
errorType: string;
code: number;
message: string;
}
// Group 1 = datasource (https://www.domain.com) // Group 1 = datasource (https://www.domain.com)
// Group 2 = path (/nested/path) // Group 2 = path (/nested/path)
// Group 3 = params (?param=123&param2=12) // Group 3 = params (?param=123&param2=12)

View File

@ -1,6 +1,7 @@
import { EmbeddedRestDatasource } from "entities/Datasource"; import { EmbeddedRestDatasource } from "entities/Datasource";
import { DynamicPath } from "utils/DynamicBindingUtils"; import { DynamicPath } from "utils/DynamicBindingUtils";
import _ from "lodash"; import _ from "lodash";
import { LayoutOnLoadActionErrors } from "constants/AppsmithActionConstants/ActionConstants";
import { Plugin } from "api/PluginApi"; import { Plugin } from "api/PluginApi";
export enum PluginType { export enum PluginType {
@ -114,6 +115,7 @@ export interface BaseAction {
confirmBeforeExecute?: boolean; confirmBeforeExecute?: boolean;
eventData?: any; eventData?: any;
messages: string[]; messages: string[];
errorReports?: Array<LayoutOnLoadActionErrors>;
} }
interface BaseApiAction extends BaseAction { interface BaseApiAction extends BaseAction {

View File

@ -11,6 +11,7 @@ enum LOG_TYPE {
JS_ACTION_UPDATE, JS_ACTION_UPDATE,
JS_PARSE_ERROR, JS_PARSE_ERROR,
JS_PARSE_SUCCESS, JS_PARSE_SUCCESS,
CYCLIC_DEPENDENCY_ERROR,
} }
export default LOG_TYPE; export default LOG_TYPE;

View File

@ -1,5 +1,6 @@
import { BaseAction } from "../Action"; import { BaseAction } from "../Action";
import { PluginType } from "entities/Action"; import { PluginType } from "entities/Action";
import { LayoutOnLoadActionErrors } from "constants/AppsmithActionConstants/ActionConstants";
export type Variable = { export type Variable = {
name: string; name: string;
@ -16,6 +17,7 @@ export interface JSCollection {
actions: Array<JSAction>; actions: Array<JSAction>;
body: string; body: string;
variables: Array<Variable>; variables: Array<Variable>;
errorReports?: Array<LayoutOnLoadActionErrors>;
} }
export interface JSActionConfig { export interface JSActionConfig {

View File

@ -6,7 +6,10 @@ import {
ReduxActionErrorTypes, ReduxActionErrorTypes,
} from "@appsmith/constants/ReduxActionConstants"; } from "@appsmith/constants/ReduxActionConstants";
import moment from "moment"; import moment from "moment";
import { PageAction } from "constants/AppsmithActionConstants/ActionConstants"; import {
LayoutOnLoadActionErrors,
PageAction,
} from "constants/AppsmithActionConstants/ActionConstants";
const initialState: EditorReduxState = { const initialState: EditorReduxState = {
initialized: false, initialized: false,
@ -116,6 +119,7 @@ const editorReducer = createReducer(initialState, {
currentLayoutId, currentLayoutId,
currentPageId, currentPageId,
currentPageName, currentPageName,
layoutOnLoadActionErrors,
pageActions, pageActions,
pageWidgetId, pageWidgetId,
} = action.payload; } = action.payload;
@ -129,6 +133,7 @@ const editorReducer = createReducer(initialState, {
currentApplicationId, currentApplicationId,
currentPageId, currentPageId,
pageActions, pageActions,
layoutOnLoadActionErrors,
}; };
}, },
[ReduxActionTypes.CLONE_PAGE_INIT]: (state: EditorReduxState) => { [ReduxActionTypes.CLONE_PAGE_INIT]: (state: EditorReduxState) => {
@ -222,6 +227,7 @@ export interface EditorReduxState {
isSnipingMode: boolean; isSnipingMode: boolean;
isPreviewMode: boolean; isPreviewMode: boolean;
zoomLevel: number; zoomLevel: number;
layoutOnLoadActionErrors?: LayoutOnLoadActionErrors[];
loadingStates: { loadingStates: {
saving: boolean; saving: boolean;
savingError: boolean; savingError: boolean;

View File

@ -31,7 +31,7 @@ import {
getAppMode, getAppMode,
getCurrentApplication, getCurrentApplication,
} from "selectors/applicationSelectors"; } 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 AppsmithConsole from "utils/AppsmithConsole";
import { ENTITY_TYPE, PLATFORM_ERROR } from "entities/AppsmithConsole"; import { ENTITY_TYPE, PLATFORM_ERROR } from "entities/AppsmithConsole";
import { validateResponse } from "sagas/ErrorSagas"; import { validateResponse } from "sagas/ErrorSagas";
@ -50,6 +50,7 @@ import {
import { Variant } from "components/ads/common"; import { Variant } from "components/ads/common";
import { import {
EventType, EventType,
LayoutOnLoadActionErrors,
PageAction, PageAction,
RESP_HEADER_DATATYPE, RESP_HEADER_DATATYPE,
} from "constants/AppsmithActionConstants/ActionConstants"; } from "constants/AppsmithActionConstants/ActionConstants";
@ -57,6 +58,7 @@ import {
getCurrentPageId, getCurrentPageId,
getIsSavingEntity, getIsSavingEntity,
getLayoutOnLoadActions, getLayoutOnLoadActions,
getLayoutOnLoadIssues,
} from "selectors/editorSelectors"; } from "selectors/editorSelectors";
import PerformanceTracker, { import PerformanceTracker, {
PerformanceTransactionName, PerformanceTransactionName,
@ -110,6 +112,7 @@ import { isTrueObject, findDatatype } from "workers/evaluationUtils";
import { handleExecuteJSFunctionSaga } from "sagas/JSPaneSagas"; import { handleExecuteJSFunctionSaga } from "sagas/JSPaneSagas";
import { Plugin } from "api/PluginApi"; import { Plugin } from "api/PluginApi";
import { setDefaultActionDisplayFormat } from "./PluginActionSagaUtils"; import { setDefaultActionDisplayFormat } from "./PluginActionSagaUtils";
import { checkAndLogErrorsIfCyclicDependency } from "sagas/helper";
enum ActionResponseDataTypes { enum ActionResponseDataTypes {
BINARY = "BINARY", BINARY = "BINARY",
@ -119,12 +122,9 @@ export const getActionTimeout = (
state: AppState, state: AppState,
actionId: string, actionId: string,
): number | undefined => { ): number | undefined => {
const action = _.find( const action = find(state.entities.actions, (a) => a.config.id === actionId);
state.entities.actions,
(a) => a.config.id === actionId,
);
if (action) { if (action) {
const timeout = _.get( const timeout = get(
action, action,
"config.actionConfiguration.timeoutInMillisecond", "config.actionConfiguration.timeoutInMillisecond",
DEFAULT_EXECUTE_ACTION_TIMEOUT_MS, DEFAULT_EXECUTE_ACTION_TIMEOUT_MS,
@ -270,7 +270,7 @@ function* evaluateActionParams(
executeActionRequest: ExecuteActionRequest, executeActionRequest: ExecuteActionRequest,
executionParams?: Record<string, any> | string, executionParams?: Record<string, any> | string,
) { ) {
if (_.isNil(bindings) || bindings.length === 0) { if (isNil(bindings) || bindings.length === 0) {
formData.append("executeActionDTO", JSON.stringify(executeActionRequest)); formData.append("executeActionDTO", JSON.stringify(executeActionRequest));
return []; return [];
} }
@ -831,7 +831,13 @@ function* executePageLoadAction(pageAction: PageAction) {
function* executePageLoadActionsSaga() { function* executePageLoadActionsSaga() {
try { try {
const pageActions: PageAction[][] = yield select(getLayoutOnLoadActions); 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( PerformanceTracker.startAsyncTracking(
PerformanceTransactionName.EXECUTE_PAGE_LOAD_ACTIONS, PerformanceTransactionName.EXECUTE_PAGE_LOAD_ACTIONS,
{ numActions: actionCount }, { numActions: actionCount },
@ -846,10 +852,10 @@ function* executePageLoadActionsSaga() {
PerformanceTracker.stopAsyncTracking( PerformanceTracker.stopAsyncTracking(
PerformanceTransactionName.EXECUTE_PAGE_LOAD_ACTIONS, PerformanceTransactionName.EXECUTE_PAGE_LOAD_ACTIONS,
); );
// We show errors in the debugger once onPageLoad actions // We show errors in the debugger once onPageLoad actions
// are executed // are executed
yield put(hideDebuggerErrors(false)); yield put(hideDebuggerErrors(false));
checkAndLogErrorsIfCyclicDependency(layoutOnLoadActionErrors);
} catch (e) { } catch (e) {
log.error(e); log.error(e);

View File

@ -111,6 +111,7 @@ import {
saasEditorApiIdURL, saasEditorApiIdURL,
} from "RouteBuilder"; } from "RouteBuilder";
import { PLUGIN_PACKAGE_MONGO } from "constants/QueryEditorConstants"; import { PLUGIN_PACKAGE_MONGO } from "constants/QueryEditorConstants";
import { checkAndLogErrorsIfCyclicDependency } from "./helper";
export function* createActionSaga( export function* createActionSaga(
actionPayload: ReduxAction< actionPayload: ReduxAction<
@ -325,6 +326,7 @@ export function* updateActionSaga(
// @ts-expect-error: Types are not available // @ts-expect-error: Types are not available
action, action,
); );
const isValidResponse: boolean = yield validateResponse(response); const isValidResponse: boolean = yield validateResponse(response);
if (isValidResponse) { if (isValidResponse) {
const pageName: string = yield select( const pageName: string = yield select(
@ -356,6 +358,9 @@ export function* updateActionSaga(
); );
yield put(updateActionSuccess({ data: response.data })); yield put(updateActionSuccess({ data: response.data }));
checkAndLogErrorsIfCyclicDependency(
(response.data as Action).errorReports,
);
} }
} catch (error) { } catch (error) {
PerformanceTracker.stopAsyncTracking( PerformanceTracker.stopAsyncTracking(

View File

@ -45,7 +45,7 @@ import {
ERROR_JS_COLLECTION_RENAME_FAIL, ERROR_JS_COLLECTION_RENAME_FAIL,
} from "@appsmith/constants/messages"; } from "@appsmith/constants/messages";
import { validateResponse } from "./ErrorSagas"; import { validateResponse } from "./ErrorSagas";
import PageApi, { FetchPageResponse } from "api/PageApi"; import PageApi, { FetchPageResponse, PageLayout } from "api/PageApi";
import { updateCanvasWithDSL } from "sagas/PageSagas"; import { updateCanvasWithDSL } from "sagas/PageSagas";
import { JSCollectionData } from "reducers/entityReducers/jsActionsReducer"; import { JSCollectionData } from "reducers/entityReducers/jsActionsReducer";
import { ApiResponse } from "api/ApiResponses"; import { ApiResponse } from "api/ApiResponses";
@ -56,6 +56,7 @@ import { CreateJSCollectionRequest } from "api/JSActionAPI";
import * as log from "loglevel"; import * as log from "loglevel";
import { builderURL, jsCollectionIdURL } from "RouteBuilder"; import { builderURL, jsCollectionIdURL } from "RouteBuilder";
import AnalyticsUtil, { EventLocation } from "utils/AnalyticsUtil"; import AnalyticsUtil, { EventLocation } from "utils/AnalyticsUtil";
import { checkAndLogErrorsIfCyclicDependency } from "./helper";
export function* fetchJSCollectionsSaga( export function* fetchJSCollectionsSaga(
action: EvaluationReduxAction<FetchActionsPayload>, action: EvaluationReduxAction<FetchActionsPayload>,
@ -372,6 +373,9 @@ export function* refactorJSObjectName(
} else { } else {
yield put(fetchJSCollectionsForPage(pageId)); yield put(fetchJSCollectionsForPage(pageId));
} }
checkAndLogErrorsIfCyclicDependency(
(refactorResponse.data as PageLayout).layoutOnLoadActionErrors,
);
} }
} }
} }

View File

@ -83,6 +83,7 @@ import { UserCancelledActionExecutionError } from "sagas/ActionExecution/errorUt
import { APP_MODE } from "entities/App"; import { APP_MODE } from "entities/App";
import { getAppMode } from "selectors/applicationSelectors"; import { getAppMode } from "selectors/applicationSelectors";
import AnalyticsUtil, { EventLocation } from "utils/AnalyticsUtil"; import AnalyticsUtil, { EventLocation } from "utils/AnalyticsUtil";
import { checkAndLogErrorsIfCyclicDependency } from "./helper";
function* handleCreateNewJsActionSaga( function* handleCreateNewJsActionSaga(
action: ReduxAction<{ pageId: string; from: EventLocation }>, action: ReduxAction<{ pageId: string; from: EventLocation }>,
@ -481,6 +482,9 @@ function* handleUpdateJSCollectionBody(
if (isValidResponse) { if (isValidResponse) {
// @ts-expect-error: response is of type unknown // @ts-expect-error: response is of type unknown
yield put(updateJSCollectionBodySuccess({ data: response?.data })); yield put(updateJSCollectionBodySuccess({ data: response?.data }));
checkAndLogErrorsIfCyclicDependency(
(response.data as JSCollection).errorReports,
);
} }
} }
} catch (error) { } catch (error) {

View File

@ -37,6 +37,7 @@ import PageApi, {
FetchPublishedPageRequest, FetchPublishedPageRequest,
PageLayout, PageLayout,
SavePageResponse, SavePageResponse,
SavePageResponseData,
SetPageOrderRequest, SetPageOrderRequest,
UpdatePageRequest, UpdatePageRequest,
UpdateWidgetNameRequest, UpdateWidgetNameRequest,
@ -115,6 +116,7 @@ import { DataTree } from "entities/DataTree/dataTreeFactory";
import { builderURL } from "RouteBuilder"; import { builderURL } from "RouteBuilder";
import { failFastApiCalls } from "./InitSagas"; import { failFastApiCalls } from "./InitSagas";
import { takeEvery } from "redux-saga/effects"; import { takeEvery } from "redux-saga/effects";
import { checkAndLogErrorsIfCyclicDependency } from "./helper";
const WidgetTypes = WidgetFactory.widgetTypes; const WidgetTypes = WidgetFactory.widgetTypes;
@ -199,6 +201,8 @@ export const getCanvasWidgetsPayload = (
currentLayoutId: pageResponse.data.layouts[0].id, // TODO(abhinav): Handle for multiple layouts currentLayoutId: pageResponse.data.layouts[0].id, // TODO(abhinav): Handle for multiple layouts
currentApplicationId: pageResponse.data.applicationId, currentApplicationId: pageResponse.data.applicationId,
pageActions: pageResponse.data.layouts[0].layoutOnLoadActions || [], pageActions: pageResponse.data.layouts[0].layoutOnLoadActions || [],
layoutOnLoadActionErrors:
pageResponse.data.layouts[0].layoutOnLoadActionErrors || [],
}; };
}; };
@ -446,6 +450,10 @@ function* savePageSaga(action: ReduxAction<{ isRetry?: boolean }>) {
PerformanceTracker.stopAsyncTracking( PerformanceTracker.stopAsyncTracking(
PerformanceTransactionName.SAVE_PAGE_API, PerformanceTransactionName.SAVE_PAGE_API,
); );
checkAndLogErrorsIfCyclicDependency(
(savePageResponse.data as SavePageResponseData)
.layoutOnLoadActionErrors,
);
} }
} catch (error) { } catch (error) {
PerformanceTracker.stopAsyncTracking( PerformanceTracker.stopAsyncTracking(
@ -828,6 +836,9 @@ export function* updateWidgetNameSaga(
dsl: response.data.dsl, dsl: response.data.dsl,
}, },
}); });
checkAndLogErrorsIfCyclicDependency(
(response.data as PageLayout).layoutOnLoadActionErrors,
);
} }
} else { } else {
yield put({ yield put({

View File

@ -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 { import {
FormEvalOutput, FormEvalOutput,
ConditionalOutput, ConditionalOutput,
} from "reducers/evaluationReducers/formEvaluationReducer"; } 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 // function to extract all objects that have dynamic values
export const extractFetchDynamicValueFormConfigs = ( export const extractFetchDynamicValueFormConfigs = (
@ -31,3 +44,72 @@ export const extractQueueOfValuesToBeFetched = (evalOutput: FormEvalOutput) => {
}); });
return output; 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<LayoutOnLoadActionErrors>,
): 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<LayoutOnLoadActionErrors>,
) => {
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<LayoutOnLoadActionErrors>,
) => {
if (!checkIfNoCyclicDependencyErrors(layoutErrors)) {
logCyclicDependecyErrors(layoutErrors);
}
};

View File

@ -97,6 +97,10 @@ export const getPageSavingError = (state: AppState) => {
export const getLayoutOnLoadActions = (state: AppState) => export const getLayoutOnLoadActions = (state: AppState) =>
state.ui.editor.pageActions || []; state.ui.editor.pageActions || [];
export const getLayoutOnLoadIssues = (state: AppState) => {
return state.ui.editor.layoutOnLoadActionErrors || [];
};
export const getIsPublishingApplication = (state: AppState) => export const getIsPublishingApplication = (state: AppState) =>
state.ui.editor.loadingStates.publishing; state.ui.editor.loadingStates.publishing;

View File

@ -28,4 +28,11 @@ public class ErrorDTO implements Serializable {
this.code = code; this.code = code;
this.message = message; this.message = message;
} }
public ErrorDTO(int code, String errorType, String message) {
this.code = code;
this.errorType = errorType;
this.message = message;
}
} }

View File

@ -5,11 +5,13 @@ import com.appsmith.server.dtos.DslActionDTO;
import com.appsmith.server.helpers.CollectionUtils; import com.appsmith.server.helpers.CollectionUtils;
import com.appsmith.server.helpers.CompareDslActionDTO; import com.appsmith.server.helpers.CompareDslActionDTO;
import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Getter; import lombok.Getter;
import lombok.NoArgsConstructor; import lombok.NoArgsConstructor;
import lombok.Setter; import lombok.Setter;
import lombok.ToString; import lombok.ToString;
import net.minidev.json.JSONObject; import net.minidev.json.JSONObject;
import com.appsmith.external.exceptions.ErrorDTO;
import java.util.List; import java.util.List;
import java.util.Set; import java.util.Set;
@ -38,6 +40,10 @@ public class Layout extends BaseDomain {
List<Set<DslActionDTO>> layoutOnLoadActions; List<Set<DslActionDTO>> layoutOnLoadActions;
// this attribute will be used to display errors caused white calculating allOnLoadAction PageLoadActionsUtilCEImpl.java
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
List<ErrorDTO> layoutOnLoadActionErrors;
@Deprecated @Deprecated
@JsonIgnore @JsonIgnore
Set<DslActionDTO> publishedLayoutActions; Set<DslActionDTO> publishedLayoutActions;

View File

@ -6,8 +6,10 @@ import com.appsmith.server.constants.FieldName;
import com.appsmith.server.domains.ActionCollection; import com.appsmith.server.domains.ActionCollection;
import com.appsmith.server.domains.PluginType; import com.appsmith.server.domains.PluginType;
import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.external.exceptions.ErrorDTO;
import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Getter; import lombok.Getter;
import lombok.NoArgsConstructor; import lombok.NoArgsConstructor;
import lombok.Setter; 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) // This field will only be populated if this collection is bound to one plugin (eg: JS)
String pluginId; String pluginId;
//this attribute carries error messages while processing the actionCollection
@Transient
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
List<ErrorDTO> errorReports;
PluginType pluginType; PluginType pluginType;
@JsonFormat(pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'", timezone = "UTC") @JsonFormat(pattern = "yyyy-MM-dd'T'HH:mm:ss'Z'", timezone = "UTC")

View File

@ -8,6 +8,7 @@ import com.appsmith.external.models.Property;
import com.appsmith.server.domains.ActionProvider; import com.appsmith.server.domains.ActionProvider;
import com.appsmith.server.domains.Documentation; import com.appsmith.server.domains.Documentation;
import com.appsmith.server.domains.PluginType; import com.appsmith.server.domains.PluginType;
import com.appsmith.external.exceptions.ErrorDTO;
import com.fasterxml.jackson.annotation.JsonFormat; import com.fasterxml.jackson.annotation.JsonFormat;
import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonProperty;
@ -61,6 +62,11 @@ public class ActionDTO {
ActionConfiguration actionConfiguration; ActionConfiguration actionConfiguration;
//this attribute carries error messages while processing the actionCollection
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
@Transient
List<ErrorDTO> errorReports;
Boolean executeOnLoad; Boolean executeOnLoad;
Boolean clientSideExecution; Boolean clientSideExecution;

View File

@ -1,12 +1,16 @@
package com.appsmith.server.dtos; package com.appsmith.server.dtos;
import com.appsmith.server.domains.ScreenType; import com.appsmith.server.domains.ScreenType;
import com.fasterxml.jackson.annotation.JsonProperty;
import lombok.Getter; import lombok.Getter;
import lombok.Setter; import lombok.Setter;
import net.minidev.json.JSONObject; import net.minidev.json.JSONObject;
import com.appsmith.external.exceptions.ErrorDTO;
import org.springframework.data.annotation.Transient;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;
@Getter @Getter
@ -21,6 +25,11 @@ public class LayoutDTO {
List<Set<DslActionDTO>> layoutOnLoadActions; List<Set<DslActionDTO>> layoutOnLoadActions;
// this attribute will be used to display errors caused white calculating allOnLoadAction PageLoadActionsUtilCEImpl.java
@Transient
@JsonProperty(access = JsonProperty.Access.READ_ONLY)
List<ErrorDTO> layoutOnLoadActionErrors;
// All the actions which have been updated as part of updateLayout function call // All the actions which have been updated as part of updateLayout function call
List<LayoutActionUpdateDTO> actionUpdates; List<LayoutActionUpdateDTO> actionUpdates;

View File

@ -23,6 +23,7 @@ import com.appsmith.server.dtos.LayoutDTO;
import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.dtos.RefactorActionNameDTO; import com.appsmith.server.dtos.RefactorActionNameDTO;
import com.appsmith.server.dtos.RefactorNameDTO; import com.appsmith.server.dtos.RefactorNameDTO;
import com.appsmith.external.exceptions.ErrorDTO;
import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.DefaultResourcesUtils; import com.appsmith.server.helpers.DefaultResourcesUtils;
@ -98,6 +99,7 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE {
*/ */
private final String preWord = "\\b("; private final String preWord = "\\b(";
private final String postWord = ")\\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 @Override
public Mono<ActionDTO> updateSingleActionWithBranchName(String defaultActionId, ActionDTO action, String branchName) { public Mono<ActionDTO> updateSingleActionWithBranchName(String defaultActionId, ActionDTO action, String branchName) {
String pageId = action.getPageId();
action.setApplicationId(null); action.setApplicationId(null);
action.setPageId(null); action.setPageId(null);
return newActionService.findByBranchNameAndDefaultActionId(branchName, defaultActionId, MANAGE_ACTIONS) return newActionService.findByBranchNameAndDefaultActionId(branchName, defaultActionId, MANAGE_ACTIONS)
.flatMap(newAction -> updateSingleAction(newAction.getId(), action)) .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 @Override
@ -891,11 +902,18 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE {
AtomicReference<Boolean> validOnPageLoadActions = new AtomicReference<>(Boolean.TRUE); AtomicReference<Boolean> validOnPageLoadActions = new AtomicReference<>(Boolean.TRUE);
// setting the layoutOnLoadActionActionErrors to null to remove the existing errors before new DAG calculation.
layout.setLayoutOnLoadActionErrors(new ArrayList<>());
Mono<List<Set<DslActionDTO>>> allOnLoadActionsMono = pageLoadActionsUtil Mono<List<Set<DslActionDTO>>> allOnLoadActionsMono = pageLoadActionsUtil
.findAllOnLoadActions(pageId, widgetNames, edges, widgetDynamicBindingsMap, flatmapPageLoadActions, actionsUsedInDSL) .findAllOnLoadActions(pageId, widgetNames, edges, widgetDynamicBindingsMap, flatmapPageLoadActions, actionsUsedInDSL)
.onErrorResume(AppsmithException.class, error -> { .onErrorResume(AppsmithException.class, error -> {
log.info(error.getMessage()); log.info(error.getMessage());
validOnPageLoadActions.set(FALSE); validOnPageLoadActions.set(FALSE);
layout.setLayoutOnLoadActionErrors(List.of(
new ErrorDTO(error.getAppErrorCode(),
layoutOnLoadActionErrorToastMessage,
error.getMessage())));
return Mono.just(new ArrayList<>()); return Mono.just(new ArrayList<>());
}); });
@ -986,6 +1004,7 @@ public class LayoutActionServiceCEImpl implements LayoutActionServiceCE {
layoutDTO.setDsl(layout.getDsl()); layoutDTO.setDsl(layout.getDsl());
layoutDTO.setScreen(layout.getScreen()); layoutDTO.setScreen(layout.getScreen());
layoutDTO.setLayoutOnLoadActions(layout.getLayoutOnLoadActions()); layoutDTO.setLayoutOnLoadActions(layout.getLayoutOnLoadActions());
layoutDTO.setLayoutOnLoadActionErrors(layout.getLayoutOnLoadActionErrors());
layoutDTO.setUserPermissions(layout.getUserPermissions()); layoutDTO.setUserPermissions(layout.getUserPermissions());
return layoutDTO; return layoutDTO;

View File

@ -34,9 +34,9 @@ import reactor.core.publisher.Mono;
import java.time.Instant; import java.time.Instant;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet; import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Map;
import java.util.List;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@ -437,6 +437,8 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ID)); return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.ID));
} }
final String pageId = actionCollectionDTO.getPageId();
Mono<ActionCollection> branchedActionCollectionMono = actionCollectionService Mono<ActionCollection> branchedActionCollectionMono = actionCollectionService
.findByBranchNameAndDefaultCollectionId(branchName, id, MANAGE_ACTIONS) .findByBranchNameAndDefaultCollectionId(branchName, id, MANAGE_ACTIONS)
.cache(); .cache();
@ -532,6 +534,7 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE
.collect(toMap(actionDTO -> actionDTO.getDefaultResources().getActionId(), ActionDTO::getId)) .collect(toMap(actionDTO -> actionDTO.getDefaultResources().getActionId(), ActionDTO::getId))
); );
// First collect all valid action ids from before, and diff against incoming action ids // First collect all valid action ids from before, and diff against incoming action ids
return branchedActionCollectionMono return branchedActionCollectionMono
.map(branchedActionCollection -> { .map(branchedActionCollection -> {
@ -584,7 +587,17 @@ public class LayoutCollectionServiceCEImpl implements LayoutCollectionServiceCE
.flatMap(actionCollectionDTO1 -> actionCollectionService.populateActionCollectionByViewMode( .flatMap(actionCollectionDTO1 -> actionCollectionService.populateActionCollectionByViewMode(
actionCollection.getUnpublishedCollection(), actionCollection.getUnpublishedCollection(),
false))) 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 @Override

View File

@ -143,6 +143,7 @@ public class ActionCollectionServiceImplTest {
Mockito Mockito
.when(analyticsService.sendArchiveEvent(Mockito.any(), Mockito.any())) .when(analyticsService.sendArchiveEvent(Mockito.any(), Mockito.any()))
.thenAnswer(invocationOnMock -> Mono.justOrEmpty(invocationOnMock.getArguments()[0])); .thenAnswer(invocationOnMock -> Mono.justOrEmpty(invocationOnMock.getArguments()[0]));
} }
<T> DefaultResources setDefaultResources(T collection) { <T> DefaultResources setDefaultResources(T collection) {
@ -381,6 +382,12 @@ public class ActionCollectionServiceImplTest {
.findByBranchNameAndDefaultPageId(Mockito.any(), Mockito.any(), Mockito.any())) .findByBranchNameAndDefaultPageId(Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(Mono.just(newPage)); .thenReturn(Mono.just(newPage));
Mockito
.when(newPageService
.findById(Mockito.any(), Mockito.any()))
.thenReturn(Mono.just(newPage));
final Mono<ActionCollectionDTO> actionCollectionDTOMono = final Mono<ActionCollectionDTO> actionCollectionDTOMono =
layoutCollectionService.updateUnpublishedActionCollection("testId", actionCollectionDTO, null); layoutCollectionService.updateUnpublishedActionCollection("testId", actionCollectionDTO, null);
@ -456,6 +463,12 @@ public class ActionCollectionServiceImplTest {
.when(newPageService.findByBranchNameAndDefaultPageId(Mockito.any(), Mockito.any(), Mockito.any())) .when(newPageService.findByBranchNameAndDefaultPageId(Mockito.any(), Mockito.any(), Mockito.any()))
.thenReturn(Mono.just(newPage)); .thenReturn(Mono.just(newPage));
Mockito
.when(newPageService
.findById(Mockito.any(), Mockito.any()))
.thenReturn(Mono.just(newPage));
final Mono<ActionCollectionDTO> actionCollectionDTOMono = final Mono<ActionCollectionDTO> actionCollectionDTOMono =
layoutCollectionService.updateUnpublishedActionCollection("testCollectionId", modifiedActionCollectionDTO, null); layoutCollectionService.updateUnpublishedActionCollection("testCollectionId", modifiedActionCollectionDTO, null);

View File

@ -1551,4 +1551,104 @@ public class LayoutActionServiceTest {
}) })
.verifyComplete(); .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<Object> 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<LayoutDTO> 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<ActionDTO> 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);
}
} }