Fix/entity name coincide issue 1396 (#1781)

* Fixing the telemetry on self-hosted instances (#1714)

* Fix issue where creating a new application twice does not work (#1713)

* add check if name is conflicting with internal fn

* use already existing selector

* add conflicting names checks on api/queries/widget names

* add test cases for invalid widget/api/query names

* refactor

* change array to map

* remove getDataTreeKeys selector

* refactor

Co-authored-by: Arpit Mohan <mohanarpit@users.noreply.github.com>
Co-authored-by: Abhinav Jha <abhinav@appsmith.com>
Co-authored-by: Pawan Kumar <pawankumar@Pawans-MacBook-Pro.local>

app/client/cypress/integration/Smoke_TestSuite/ApiPaneTests/API_Unique_name_spec.js

@@ -6,5 +6,6 @@ describe("Name uniqueness test", function() {
     cy.CreateAPI("UniqueName");
     cy.log("Creation of UniqueName Action successful");
     cy.CreationOfUniqueAPIcheck("UniqueName");
+    cy.CreationOfUniqueAPIcheck("download");
   });
 });

app/client/cypress/integration/Smoke_TestSuite/ExplorerTests/Entity_Explorer_Query_Datasource_spec.js

@@ -23,6 +23,15 @@ describe("Entity explorer tests related to query and datasource", function() {
 
     cy.testSaveDatasource();
 
+    // checking that conflicting names are not allowed
+    cy.get(".t--edit-datasource-name").click();
+    cy.get(".t--edit-datasource-name input")
+      .clear()
+      .type("download", { force: true })
+      .blur();
+    cy.get(".Toastify").should("contain", "Invalid name");
+
+    // checking a valid name
     cy.get(".t--edit-datasource-name").click();
     cy.get(".t--edit-datasource-name input")
       .clear()

app/client/cypress/integration/Smoke_TestSuite/FormWidgets/Button_spec.js

@@ -23,6 +23,9 @@ describe("Button Widget Functionality", function() {
       widgetsPage.buttonWidget + " " + commonlocators.widgetNameTag,
     );
 
+    // changing button to invalid name
+    cy.invalidWidgetText();
+
     //Changing the text on the Button
     cy.testCodeMirror(this.data.ButtonLabel);
     cy.EvaluateDataType("string");

app/client/cypress/support/commands.js

@@ -451,7 +451,6 @@ Cypress.Commands.add("CreateSubsequentAPI", apiname => {
 });
 
 Cypress.Commands.add("EditApiName", apiname => {
-  //cy.wait("@getUser");
   cy.get(apiwidget.ApiName).click({ force: true });
   cy.get(apiwidget.apiTxt)
     .clear()
@@ -912,6 +911,7 @@ Cypress.Commands.add(
 );
 
 Cypress.Commands.add("widgetText", (text, inputcss, innercss) => {
+  // checking valid widget name
   cy.get(commonlocators.editWidgetName)
     .click({ force: true })
     .type(text)
@@ -922,6 +922,15 @@ Cypress.Commands.add("widgetText", (text, inputcss, innercss) => {
   cy.get(innercss).should("have.text", text);
 });
 
+Cypress.Commands.add("invalidWidgetText", () => {
+  // checking invalid widget name
+  cy.get(commonlocators.editWidgetName)
+    .click({ force: true })
+    .type("download")
+    .type("{enter}");
+  cy.get(commonlocators.toastmsg).contains("download is already being used.");
+});
+
 Cypress.Commands.add("EvaluateDataType", dataType => {
   cy.get(commonlocators.evaluatedType)
     .should("be.visible")

app/client/src/components/editorComponents/ActionNameEditor.tsx

@@ -6,10 +6,12 @@ import styled from "styled-components";
 import EditableText, {
   EditInteractionKind,
 } from "components/editorComponents/EditableText";
-import { removeSpecialChars } from "utils/helpers";
+import { removeSpecialChars, isNameValid } from "utils/helpers";
 import { AppState } from "reducers";
 import { RestAction } from "entities/Action";
 import { Page } from "constants/ReduxActionConstants";
+import { getDataTree } from "selectors/dataTreeSelectors";
+import { getExistingPageNames } from "sagas/selectors";
 
 import { saveActionName } from "actions/actionActions";
 import { Spinner } from "@blueprintjs/core";
@@ -42,10 +44,6 @@ export const ActionNameEditor = () => {
     state.entities.actions.map(action => action.config),
   );
 
-  const existingPageNames: string[] = useSelector((state: AppState) =>
-    state.entities.pageList.pages.map((page: Page) => page.pageName),
-  );
-
   const currentActionConfig: RestAction | undefined = actions.find(
     action => action.id === params.apiId || action.id === params.queryId,
   );
@@ -56,6 +54,9 @@ export const ActionNameEditor = () => {
     ),
   );
 
+  const evalTree = useSelector(getDataTree);
+  const existingPageNames = useSelector(getExistingPageNames);
+
   const saveStatus: {
     isSaving: boolean;
     error: boolean;
@@ -68,12 +69,7 @@ export const ActionNameEditor = () => {
   });
 
   const hasActionNameConflict = useCallback(
-    (name: string) =>
-      !(
-        existingPageNames.indexOf(name) === -1 &&
-        actions.findIndex(action => action.name === name) === -1 &&
-        existingWidgetNames.indexOf(name) === -1
-      ),
+    (name: string) => !isNameValid(name, { ...existingPageNames, ...evalTree }),
     [existingPageNames, actions, existingWidgetNames],
   );
 

app/client/src/constants/WidgetValidation.ts

@@ -37,3 +37,52 @@ export type Validator = (
 ) => ValidationResponse;
 
 export const ISO_DATE_FORMAT = "YYYY-MM-DDTHH:mm:ss.SSSZ";
+
+export const JAVSCRIPT_KEYWORDS = {
+  true: "true",
+  await: "await",
+  break: "break",
+  case: "case",
+  catch: "catch",
+  class: "class",
+  const: "const",
+  continue: "continue",
+  debugger: "debugger",
+  default: "default",
+  delete: "delete",
+  do: "do",
+  else: "else",
+  enum: "enum",
+  export: "export",
+  extends: "extends",
+  false: "false",
+  finally: "finally",
+  for: "for",
+  function: "function",
+  if: "if",
+  implements: "implements",
+  import: "import",
+  in: "in",
+  instanceof: "instanceof",
+  interface: "interface",
+  let: "let",
+  new: "new",
+  null: "null",
+  package: "package",
+  private: "private",
+  protected: "protected",
+  public: "public",
+  return: "return",
+  static: "static",
+  super: "super",
+  switch: "switch",
+  this: "this",
+  throw: "throw",
+  try: "try",
+  typeof: "typeof",
+  var: "var",
+  void: "void",
+  while: "while",
+  with: "with",
+  yield: "yield",
+};

app/client/src/pages/Editor/DataSourceEditor/FormTitle.tsx

@@ -10,6 +10,8 @@ import { getDatasource } from "selectors/entitiesSelector";
 import { useSelector, useDispatch } from "react-redux";
 import { Datasource } from "api/DatasourcesApi";
 import { getDataSources } from "selectors/editorSelectors";
+import { getDataTree } from "selectors/dataTreeSelectors";
+import { isNameValid } from "utils/helpers";
 import { saveDatasourceName } from "actions/datasourceActions";
 import { Spinner } from "@blueprintjs/core";
 
@@ -35,6 +37,7 @@ const FormTitle = (props: FormTitleProps) => {
     getDatasource(state, params.datasourceId),
   );
   const datasources: Datasource[] = useSelector(getDataSources);
+  const evalTree = useSelector(getDataTree);
   const [forceUpdate, setForceUpdate] = useState(false);
   const dispatch = useDispatch();
   const saveStatus: {
@@ -50,11 +53,16 @@ const FormTitle = (props: FormTitleProps) => {
   });
 
   const hasNameConflict = React.useCallback(
-    (name: string) =>
-      datasources.some(
-        datasource =>
-          datasource.name === name && datasource.id !== currentDatasource?.id,
-      ),
+    (name: string) => {
+      const datasourcesNames: Record<string, any> = {};
+      datasources
+        .filter(datasource => datasource.id !== currentDatasource?.id)
+        .map(datasource => {
+          datasourcesNames[datasource.name] = datasource;
+        });
+
+      return !isNameValid(name, { ...datasourcesNames, ...evalTree });
+    },
     [datasources, currentDatasource],
   );
 

app/client/src/sagas/PageSagas.tsx

@@ -47,16 +47,15 @@ import {
 } from "redux-saga/effects";
 import history from "utils/history";
 import { BUILDER_PAGE_URL } from "constants/routes";
-
+import { isNameValid } from "utils/helpers";
 import { extractCurrentDSL } from "utils/WidgetPropsUtils";
 import {
   getAllPageIds,
   getEditorConfigs,
-  getExistingActionNames,
   getExistingPageNames,
-  getExistingWidgetNames,
   getWidgets,
 } from "./selectors";
+import { getDataTree } from "selectors/dataTreeSelectors";
 import { validateResponse } from "./ErrorSagas";
 import { executePageLoadActions } from "actions/widgetActions";
 import { ApiResponse } from "api/ApiResponses";
@@ -524,21 +523,32 @@ export function* clonePageSaga(clonePageAction: ReduxAction<ClonePageRequest>) {
   }
 }
 
+/**
+ * this saga do two things
+ *
+ * 1. Checks if the name of page is conflicting with any used name
+ * 2. dispatches a action which triggers a request to update the name
+ *
+ * @param action
+ */
 export function* updateWidgetNameSaga(
   action: ReduxAction<{ id: string; newName: string }>,
 ) {
   try {
     const { widgetName } = yield select(getWidgetName, action.payload.id);
     const layoutId = yield select(getCurrentLayoutId);
+    const evalTree = yield select(getDataTree);
     const pageId = yield select(getCurrentPageId);
-    const existingWidgetNames = yield select(getExistingWidgetNames);
-    const existingActionNames = yield select(getExistingActionNames);
     const existingPageNames = yield select(getExistingPageNames);
-    const hasWidgetNameConflict =
-      existingWidgetNames.indexOf(action.payload.newName) > -1 ||
-      existingActionNames.indexOf(action.payload.newName) > -1 ||
-      existingPageNames.indexOf(action.payload.newName) > -1;
-    if (!hasWidgetNameConflict) {
+
+    // check if name is not conflicting with any
+    // existing entity/api/queries/reserved words
+    if (
+      isNameValid(action.payload.newName, {
+        ...evalTree,
+        ...existingPageNames,
+      })
+    ) {
       const request: UpdateWidgetNameRequest = {
         newName: action.payload.newName,
         oldName: widgetName,

app/client/src/sagas/selectors.tsx

@@ -84,8 +84,20 @@ export const getExistingActionNames = createSelector(
   },
 );
 
-export const getExistingPageNames = (state: AppState) =>
-  state.entities.pageList.pages.map((page: Page) => page.pageName);
+/**
+ * returns a objects of existing page name in data tree
+ *
+ * @param state
+ */
+export const getExistingPageNames = (state: AppState) => {
+  const map: Record<string, any> = {};
+
+  state.entities.pageList.pages.map((page: Page) => {
+    map[page.pageName] = page.pageName;
+  });
+
+  return map;
+};
 
 export const getWidgetByName = (
   state: AppState,

app/client/src/selectors/dataTreeSelectors.ts

@@ -33,6 +33,11 @@ export const getUnevaluatedDataTree = createSelector(
   },
 );
 
+/**
+ * returns evaluation tree object
+ *
+ * @param state
+ */
 export const getDataTree = (state: AppState) => state.evaluations.tree;
 
 // For autocomplete. Use actions cached responses if

app/client/src/utils/helpers.tsx

@@ -1,4 +1,5 @@
 import { GridDefaults } from "constants/WidgetConstants";
+import { JAVSCRIPT_KEYWORDS } from "constants/WidgetValidation";
 export const snapToGrid = (
   columnWidth: number,
   rowHeight: number,
@@ -162,3 +163,26 @@ export const isEllipsisActive = (element: HTMLElement | null) => {
 export const convertArrayToSentence = (arr: string[]) => {
   return arr.join(", ").replace(/,\s([^,]+)$/, " and $1");
 };
+
+/**
+ * checks if the name is conflciting with
+ * 1. API names,
+ * 2. Queries name
+ * 3. Javascript reserved names
+ * 4. Few internal function names that are in the evaluation tree
+ *
+ * return if false name conflicts with anything from the above list
+ *
+ * @param name
+ * @param invalidNames
+ */
+export const isNameValid = (
+  name: string,
+  invalidNames: Record<string, any>,
+) => {
+  if (name in JAVSCRIPT_KEYWORDS || name in invalidNames) {
+    return false;
+  }
+
+  return true;
+};