Skip to content

Commit

Permalink
fix: prevent lint warnings from blocking execution in js objects (app…
Browse files Browse the repository at this point in the history
  • Loading branch information
ohansFavour committed May 11, 2022
1 parent 97c5aee commit 876d1a1
Show file tree
Hide file tree
Showing 10 changed files with 182 additions and 40 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { ObjectsRegistry } from "../../../../support/Objects/Registry";

const jsEditor = ObjectsRegistry.JSEditor;

describe("JS Function Execution", function() {
it("Allows execution of js function when lint warnings(not errors) are present in code", function() {
jsEditor.CreateJSObject(
`export default {
myFun1: ()=>{
debugger;
return "yes"
}
}`,
true,
true,
false,
);

jsEditor.AssertParseError(false);
});

it("Prevents execution of js function when parse errors are present in code", function() {
jsEditor.CreateJSObject(
`export default {
myFun1: ()=>{
return "yes"
},
myFun2: ()==>{}
}`,
true,
true,
false,
);

jsEditor.AssertParseError(true);
});

it("Allows execution of other JS functions when lint error is present in code one JS function", function() {
jsEditor.CreateJSObject(
`export default {
myFun1: (a ,b)=>{
return f
},
myFun2 :()=>{
return "yes"
}
}`,
true,
true,
false,
);

jsEditor.AssertParseError(false);
});
});
93 changes: 67 additions & 26 deletions app/client/cypress/support/Pages/JSEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,58 @@ export class JSEditor {
private _runButton = "button.run-js-action";
private _settingsTab = ".tab-title:contains('Settings')";
private _codeTab = ".tab-title:contains('Code')";
private _jsObjectParseErrorCallout =
"//div[contains(@class,'t--js-response-parse-error-call-out')]";
private _onPageLoadRadioButton = (functionName: string, onLoad: boolean) =>
`.${functionName}-on-page-load-setting label:contains(${onLoad ? "Yes" : "No"
`.${functionName}-on-page-load-setting label:contains(${
onLoad ? "Yes" : "No"
}) span.checkbox`;
private _onPageLoadRadioButtonStatus = (functionName: string, onLoad: boolean) =>
`.${functionName}-on-page-load-setting label:contains(${onLoad ? "Yes" : "No"
private _onPageLoadRadioButtonStatus = (
functionName: string,
onLoad: boolean,
) =>
`.${functionName}-on-page-load-setting label:contains(${
onLoad ? "Yes" : "No"
})>input`;
private _confirmBeforeExecuteRadioButton = (
functionName: string,
shouldConfirm: boolean,
) =>
`.${functionName}-confirm-before-execute label:contains(${shouldConfirm ? "Yes" : "No"
`.${functionName}-confirm-before-execute label:contains(${
shouldConfirm ? "Yes" : "No"
}) span.checkbox`;
private _confirmBeforeExecuteRadioButtonStatus = (
functionName: string,
shouldConfirm: boolean,
) =>
`.${functionName}-confirm-before-execute label:contains(${shouldConfirm ? "Yes" : "No"
`.${functionName}-confirm-before-execute label:contains(${
shouldConfirm ? "Yes" : "No"
})>input`;
private _outputConsole = ".CodeEditorTarget";
private _jsObjName = ".t--js-action-name-edit-field span";
private _jsObjTxt = ".t--js-action-name-edit-field input";
private _newJSobj = "span:contains('New JS Object')"
private _bindingsClose = ".t--entity-property-close"
private _propertyList = ".t--entity-property"
private _responseTabAction = (funName: string) => "//div[@class='function-name'][text()='" + funName + "']/following-sibling::div//*[local-name()='svg']"
private _functionSetting = (settingTxt: string) => "//span[text()='" + settingTxt + "']/parent::div/following-sibling::input[@type='checkbox']"
_dialog = (dialogHeader: string) => "//div[contains(@class, 'bp3-dialog')]//h4[contains(text(), '" + dialogHeader + "')]"
private _closeSettings = "span[icon='small-cross']"
_dialogBody = (jsFuncName: string) => "//div[@class='bp3-dialog-body']//*[contains(text(), '" + Cypress.env('MESSAGES').QUERY_CONFIRMATION_MODAL_MESSAGE() + "')]//*[contains(text(),'" + jsFuncName + "')]"
private _newJSobj = "span:contains('New JS Object')";
private _bindingsClose = ".t--entity-property-close";
private _propertyList = ".t--entity-property";
private _responseTabAction = (funName: string) =>
"//div[@class='function-name'][text()='" +
funName +
"']/following-sibling::div//*[local-name()='svg']";
private _functionSetting = (settingTxt: string) =>
"//span[text()='" +
settingTxt +
"']/parent::div/following-sibling::input[@type='checkbox']";
_dialog = (dialogHeader: string) =>
"//div[contains(@class, 'bp3-dialog')]//h4[contains(text(), '" +
dialogHeader +
"')]";
private _closeSettings = "span[icon='small-cross']";
_dialogBody = (jsFuncName: string) =>
"//div[@class='bp3-dialog-body']//*[contains(text(), '" +
Cypress.env("MESSAGES").QUERY_CONFIRMATION_MODAL_MESSAGE() +
"')]//*[contains(text(),'" +
jsFuncName +
"')]";

//#endregion

Expand Down Expand Up @@ -111,7 +134,7 @@ export class JSEditor {
input.type(JSCode, {
parseSpecialCharSequences: false,
delay: 150,
force: true
force: true,
});
}
});
Expand Down Expand Up @@ -178,9 +201,9 @@ export class JSEditor {
} else {
cy.get(
this.locator._propertyControl +
endp.replace(/ +/g, "").toLowerCase() +
" " +
this.locator._codeMirrorTextArea,
endp.replace(/ +/g, "").toLowerCase() +
" " +
this.locator._codeMirrorTextArea,
)
.first()
.then((el: any) => {
Expand Down Expand Up @@ -227,9 +250,9 @@ export class JSEditor {
public RemoveText(endp: string) {
cy.get(
this.locator._propertyControl +
endp +
" " +
this.locator._codeMirrorTextArea,
endp +
" " +
this.locator._codeMirrorTextArea,
)
.first()
.focus()
Expand Down Expand Up @@ -266,7 +289,7 @@ export class JSEditor {

public validateDefaultJSObjProperties(jsObjName: string) {
this.ee.ActionContextMenuByEntityName(jsObjName, "Show Bindings");
cy.get(this._propertyList).then(function ($lis) {
cy.get(this._propertyList).then(function($lis) {
const bindingsLength = $lis.length;
expect(bindingsLength).to.be.at.least(4);
expect($lis.eq(0).text()).to.be.oneOf([
Expand All @@ -289,7 +312,6 @@ export class JSEditor {
cy.get(this._bindingsClose).click({ force: true });
}


// public EnableDisableOnPageLoad(funName: string, onLoad: 'enable' | 'disable' | '', bfrCalling: 'enable' | 'disable' | '') {
// this.agHelper.GetNClick(this._responseTabAction(funName))
// this.agHelper.AssertElementPresence(this._dialog('Function settings'))
Expand All @@ -301,16 +323,26 @@ export class JSEditor {
// this.agHelper.GetNClick(this._closeSettings)
// }

public VerifyOnPageLoadSetting(funName: string, onLoad = true, bfrCalling = true) {
public VerifyOnPageLoadSetting(
funName: string,
onLoad = true,
bfrCalling = true,
) {
// this.agHelper.GetNClick(this._responseTabAction(funName))
// this.agHelper.AssertElementPresence(this._dialog('Function settings'))
// this.agHelper.AssertExistingToggleState(this._functionSetting(Cypress.env("MESSAGES").JS_SETTINGS_ONPAGELOAD()), onLoad)
// this.agHelper.AssertExistingToggleState(this._functionSetting(Cypress.env("MESSAGES").JS_SETTINGS_CONFIRM_EXECUTION()), bfrCalling)
// this.agHelper.GetNClick(this._closeSettings)

this.agHelper.GetNClick(this._settingsTab);
this.agHelper.AssertExistingToggleState(this._onPageLoadRadioButtonStatus(funName, onLoad), onLoad == true ? 'checked' : 'unchecked')
this.agHelper.AssertExistingToggleState(this._confirmBeforeExecuteRadioButtonStatus(funName, bfrCalling), bfrCalling == true ? 'checked' : 'unchecked')
this.agHelper.AssertExistingToggleState(
this._onPageLoadRadioButtonStatus(funName, onLoad),
onLoad == true ? "checked" : "unchecked",
);
this.agHelper.AssertExistingToggleState(
this._confirmBeforeExecuteRadioButtonStatus(funName, bfrCalling),
bfrCalling == true ? "checked" : "unchecked",
);
}

public EnableDisableOnPageLoad(
Expand All @@ -323,10 +355,19 @@ export class JSEditor {
// Set onPageLoad
this.agHelper.GetNClick(this._onPageLoadRadioButton(funName, onLoad));
// Set confirmBeforeExecute
this.agHelper.GetNClick(this._confirmBeforeExecuteRadioButton(funName, bfrCalling));
this.agHelper.GetNClick(
this._confirmBeforeExecuteRadioButton(funName, bfrCalling),
);
// Return to code tab
this.agHelper.GetNClick(this._codeTab);
}

public AssertParseError(exists: boolean) {
// Assert presence/absence of parse error
cy.xpath(this._jsObjectParseErrorCallout).should(
exists ? "exist" : "not.exist",
);
}

//#endregion
}
3 changes: 2 additions & 1 deletion app/client/src/ce/constants/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,8 @@ export const JS_FUNCTION_UPDATE_SUCCESS = () =>
export const JS_FUNCTION_DELETE_SUCCESS = () =>
"JS function deleted successfully";
export const JS_OBJECT_BODY_INVALID = () => "JS object could not be parsed";

export const JS_ACTION_EXECUTION_ERROR = (jsFunctionName: string) =>
`An error occured while trying to execute ${jsFunctionName}, please check error logs to debug`;
//Editor Page
export const EDITOR_HEADER_SAVE_INDICATOR = () => "Saved";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

export const WARNING_LINT_ERRORS = {
W098: "'{a}' is defined but never used.",
W014:
"Misleading line break before '{a}'; readers may interpret this as an expression boundary.",
};

export const LINT_TOOLTIP_CLASS = "CodeMirror-lint-tooltip";
Expand Down
29 changes: 23 additions & 6 deletions app/client/src/components/editorComponents/JSResponseView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
EMPTY_RESPONSE_FIRST_HALF,
EMPTY_JS_RESPONSE_LAST_HALF,
NO_JS_FUNCTION_RETURN_VALUE,
JS_ACTION_EXECUTION_ERROR,
} from "@appsmith/constants/messages";
import { EditorTheme } from "./CodeEditor/EditorConfig";
import DebuggerLogs from "./Debugger/DebuggerLogs";
Expand Down Expand Up @@ -147,6 +148,7 @@ enum JSResponseState {
interface ReduxStateProps {
responses: Record<string, any>;
isExecuting: Record<string, boolean>;
isDirtyList: Record<string, boolean>;
}

type Props = ReduxStateProps &
Expand All @@ -165,6 +167,7 @@ function JSResponseView(props: Props) {
currentFunction,
disabled,
errors,
isDirtyList,
isExecuting,
isLoading,
jsObject,
Expand Down Expand Up @@ -197,6 +200,11 @@ function JSResponseView(props: Props) {
!isExecuting.hasOwnProperty(currentFunction.id)
) {
setResponseStatus(JSResponseState.NoResponse);
} else if (
responses.hasOwnProperty(currentFunction.id) &&
isDirtyList[currentFunction.id]
) {
setResponseStatus(JSResponseState.IsDirty);
} else if (
responses.hasOwnProperty(currentFunction.id) &&
responses[currentFunction.id] === undefined
Expand All @@ -206,23 +214,30 @@ function JSResponseView(props: Props) {
setResponseStatus(JSResponseState.ShowResponse);
}
}, [responses, isExecuting, currentFunction]);

const tabs = [
{
key: "body",
title: "Response",
panelComponent: (
<>
{errors.length > 0 && (
<HelpSection>
{(errors.length > 0 ||
responseStatus === JSResponseState.IsDirty) && (
<HelpSection className=".t--js-response-parse-error-call-out">
<StyledCallout
fill
label={
<FailedMessage>
<DebugButton onClick={onDebugClick} />
</FailedMessage>
}
text={createMessage(PARSING_ERROR)}
text={
responseStatus === JSResponseState.IsDirty
? createMessage(
JS_ACTION_EXECUTION_ERROR,
`${jsObject.name}.${currentFunction?.name}`,
)
: createMessage(PARSING_ERROR)
}
variant={Variant.danger}
/>
</HelpSection>
Expand Down Expand Up @@ -317,10 +332,12 @@ const mapStateToProps = (
(action: JSCollectionData) => action.config.id === jsObject.id,
);
const responses = (seletedJsObject && seletedJsObject.data) || {};
const isDirtyList = (seletedJsObject && seletedJsObject.isDirty) || {};
const isExecuting = (seletedJsObject && seletedJsObject.isExecuting) || {};
return {
responses: responses,
isExecuting: isExecuting,
responses,
isExecuting,
isDirtyList,
};
};

Expand Down
1 change: 0 additions & 1 deletion app/client/src/pages/Editor/JSEditor/Form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ function JSEditorForm({ jsCollection: currentJSCollection }: Props) {
}
setSelectedJSActionOption(getJSActionOption(activeJSAction, jsActions));
}, [parseErrors, jsActions, activeJSActionId]);

return (
<FormWrapper>
<JSObjectHotKeys runActiveJSFunction={handleRunAction}>
Expand Down
12 changes: 12 additions & 0 deletions app/client/src/reducers/entityReducers/jsActionsReducer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export interface JSCollectionData {
data?: Record<string, unknown>;
isExecuting?: Record<string, boolean>;
activeJSActionId?: string;
// Existence of parse errors for each action (updates after execution)
isDirty?: Record<string, boolean>;
}
export type JSCollectionDataState = JSCollectionData[];
export interface PartialActionData {
Expand Down Expand Up @@ -268,7 +270,9 @@ const jsActionsReducer = createReducer(initialState, {
state.map((a) => {
if (a.config.id === action.payload.collectionId) {
const newData = { ...a.data };
const newIsDirty = { ...a.isDirty };
unset(newData, action.payload.action.id);
unset(newIsDirty, action.payload.action.id);
return {
...a,
isExecuting: {
Expand All @@ -278,6 +282,9 @@ const jsActionsReducer = createReducer(initialState, {
data: {
...newData,
},
isDirty: {
...newIsDirty,
},
};
}
return a;
Expand All @@ -288,6 +295,7 @@ const jsActionsReducer = createReducer(initialState, {
results: any;
collectionId: string;
actionId: string;
isDirty: boolean;
}>,
): JSCollectionDataState =>
state.map((a) => {
Expand All @@ -302,6 +310,10 @@ const jsActionsReducer = createReducer(initialState, {
...a.isExecuting,
[action.payload.actionId]: false,
},
isDirty: {
...a.isDirty,
[action.payload.actionId]: action.payload.isDirty,
},
};
}
return a;
Expand Down
Loading

0 comments on commit 876d1a1

Please sign in to comment.