Merge pull request #1592 from jdi-testing/issue_1577_locator-type-dro…

…pdown-fix-state

Issue 1577: locator type dropdown fix state

manifest.json

@@ -3,7 +3,7 @@
   "name": "JDN",
   "description": "JDN – helps Test Automation Engineer to create Page Objects in the test automation framework and speed up test development",
   "devtools_page": "index.html",
-  "version": "3.14.30",
+  "version": "3.14.31",
   "icons": {
     "128": "icon128.png"
   },

package.json

@@ -1,6 +1,6 @@
 {
   "name": "jdn-ai-chrome-extension",
-  "version": "3.14.30",
+  "version": "3.14.31",
   "description": "jdn-ai chrome extension",
   "scripts": {
     "start": "webpack --watch --env devenv",
@@ -130,4 +130,4 @@
     "webpack": "^5.83.1",
     "webpack-cli": "5.1.4"
   }
-}
+}

src/__tests__/locators/locatorsSlice.test.js

@@ -23,26 +23,38 @@ describe('changeLocatorAttributes reducer', () => {
     removeElementSpy = jest.spyOn(sendMessage, 'removeElement');
   });
 
-  test('edit type and name', () => {
-    store.dispatch(
-      changeLocatorAttributes({
-        element_id: '8736312404689610766421832473',
-        is_shown: true,
-        locator: "//*[@class='sidebar-menu left']",
-        name: 'myAwesomeLocator',
-        type: 'ProgressBar',
-        message: '',
-        library: ElementLibrary.MUI,
-      }),
-    );
+  test('should update type and name when editing a locator', () => {
+    // Arrange
+    const locatorMock = {
+      ...locator1,
+      type: 'ProgressBar',
+      name: 'myAwesomeLocator',
+      locator: "//*[@class='sidebar-menu left']",
+      message: '',
+      library: ElementLibrary.MUI,
+      locatorType: LocatorType.xPath,
+    };
+
+    // Act
+    store.dispatch(changeLocatorAttributes(locatorMock));
+
+    // Assert
     const locator = selectLocatorById(store.getState(), '8736312404689610766421832473');
     expect(locator.type).toBe('ProgressBar');
     expect(locator.name).toBe('myAwesomeLocator');
-    expect(locator.locator).toStrictEqual(locator1.locator);
+    expect(locator.locator).toStrictEqual({
+      attributes: {},
+      cssSelector: '.sidebar-menu.left',
+      output: "//*[@class='sidebar-menu left']",
+      taskStatus: LocatorTaskStatus.SUCCESS,
+      xPath: "//*[@class='sidebar-menu left']",
+      xPathStatus: LocatorTaskStatus.SUCCESS,
+    });
     expect(changeElementNameSpy).toHaveBeenCalledWith(locator);
   });
 
-  test("didn't edit type and name", () => {
+  test('should not update type and name when they remain the same', () => {
+    // Act
     store.dispatch(
       changeLocatorAttributes({
         element_id: '8736312404689610766421832473',
@@ -52,16 +64,27 @@ describe('changeLocatorAttributes reducer', () => {
         type: 'ProgressBar',
         message: '',
         library: ElementLibrary.MUI,
+        locatorType: LocatorType.xPath,
       }),
     );
+
+    // Assert
     const locator = selectLocatorById(store.getState(), '8736312404689610766421832473');
     expect(locator.type).toBe('ProgressBar');
     expect(locator.name).toBe('myAwesomeLocator');
-    expect(locator.locator).toStrictEqual(locator1.locator);
+    expect(locator.locator).toStrictEqual({
+      attributes: {},
+      cssSelector: '.sidebar-menu.left',
+      output: "//*[@class='sidebar-menu left']",
+      taskStatus: LocatorTaskStatus.SUCCESS,
+      xPath: "//*[@class='sidebar-menu left']",
+      xPathStatus: LocatorTaskStatus.SUCCESS,
+    });
     expect(changeElementNameSpy).toHaveBeenCalledWith(locator);
   });
 
-  test('edit type, custom name remains the same', () => {
+  test('should update type and keep custom name when editing a locator', () => {
+    // Act
     store.dispatch(
       changeLocatorAttributes({
         element_id: '8736312404689610766421832473',
@@ -71,16 +94,27 @@ describe('changeLocatorAttributes reducer', () => {
         message: '',
         type: 'Dialog',
         library: ElementLibrary.MUI,
+        locatorType: LocatorType.xPath,
       }),
     );
+
+    // Assert
     const locator = selectLocatorById(store.getState(), '8736312404689610766421832473');
     expect(locator.type).toBe('Dialog');
     expect(locator.name).toBe('myAwesomeLocator');
-    expect(locator.locator).toStrictEqual(locator1.locator);
+    expect(locator.locator).toStrictEqual({
+      attributes: {},
+      cssSelector: '.sidebar-menu.left',
+      output: "//*[@class='sidebar-menu left']",
+      taskStatus: LocatorTaskStatus.SUCCESS,
+      xPath: "//*[@class='sidebar-menu left']",
+      xPathStatus: LocatorTaskStatus.SUCCESS,
+    });
     expect(changeElementNameSpy).toHaveBeenCalledWith(locator);
   });
 
-  test('edit locator', () => {
+  test('should update locator when editing a locator', () => {
+    // Act
     store.dispatch(
       changeLocatorAttributes({
         element_id: '8736312404689610766421832473',
@@ -94,6 +128,8 @@ describe('changeLocatorAttributes reducer', () => {
         library: ElementLibrary.MUI,
       }),
     );
+
+    // Assert
     const locator = selectLocatorById(store.getState(), '8736312404689610766421832473');
     expect(locator.type).toBe('Dialog');
     expect(locator.name).toBe('myAwesomeLocator');
@@ -103,8 +139,11 @@ describe('changeLocatorAttributes reducer', () => {
     expect(locator.locator.xPathStatus).toBe(LocatorTaskStatus.SUCCESS);
   });
 
-  test('warned validation', () => {
+  test('should handle warned validation and remove element', () => {
+    // Arrange
     const oldLocator = selectLocatorById(store.getState(), '8736312404689610766421832473');
+
+    // Act
     store.dispatch(
       changeLocatorAttributes({
         element_id: '8736312404689610766421832473',
@@ -114,8 +153,11 @@ describe('changeLocatorAttributes reducer', () => {
         type: 'Dialog',
         message: 'NOT_FOUND',
         library: ElementLibrary.MUI,
+        locatorType: LocatorType.cssSelector,
       }),
     );
+
+    // Assert
     const locator = selectLocatorById(store.getState(), '8736312404689610766421832473');
     expect(locator).toBeDefined();
     expect(locator.message).toBe('NOT_FOUND');

src/features/locators/components/LocatorEditDialog.tsx

@@ -20,7 +20,7 @@ import {
 } from '../types/locator.types';
 
 import { changeLocatorAttributes } from '../locators.slice';
-import { createNewName, isValidLocator, getLocatorValidationStatus, getLocatorValueOnTypeSwitch } from '../utils/utils';
+import { createNewName, getLocatorValidationStatus, getLocatorValueOnTypeSwitch } from '../utils/utils';
 import { createLocatorValidationRules } from '../utils/locatorValidationRules';
 import { createNameValidationRules } from '../utils/nameValidationRules';
 import FormItem from 'antd/es/form/FormItem';
@@ -32,6 +32,7 @@ import { addCustomLocator } from '../reducers/addCustomLocator.thunk';
 import { selectPresentLocatorsByPO } from '../selectors/locatorsByPO.selectors';
 import { LocatorMessageForDuplicate } from './LocatorMessageForDuplicate';
 import { createLocatorTypeOptions } from '../utils/createLocatorTypeOptions';
+import { validateLocator } from '../utils/locatorValidation';
 
 interface Props extends ILocator {
   isModalOpen: boolean;
@@ -112,9 +113,7 @@ export const LocatorEditDialog: React.FC<Props> = ({
     form.resetFields();
     setIsModalOpen(false);
   };
-  // ToDo: fix legacy:
-  // eslint-disable-next-line @typescript-eslint/naming-convention
-  const _locatorValidationRules: () => Rule[] = () =>
+  const getLocatorValidationRules: () => Rule[] = () =>
     createLocatorValidationRules(
       isCreatingForm,
       form.getFieldValue('locatorType') || defaultLocatorType,
@@ -124,7 +123,7 @@ export const LocatorEditDialog: React.FC<Props> = ({
       jdnHash,
       element_id,
     );
-  const [locatorValidationRules, setLocatorValidationRules] = useState<Rule[]>(_locatorValidationRules());
+  const [locatorValidationRules, setLocatorValidationRules] = useState<Rule[]>(getLocatorValidationRules());
 
   const handleTypeChange = (value: string) => {
     if (isEditedName) return;
@@ -175,11 +174,11 @@ export const LocatorEditDialog: React.FC<Props> = ({
   const handleEditLocator = async () => {
     // ToDo: fix legacy
     // eslint-disable-next-line @typescript-eslint/no-shadow
-    const { name, type, locator, locatorType, annotationType } = await form.validateFields();
+    const { name, type, locator: locatorValue, locatorType, annotationType } = await form.validateFields();
     const updatedLocator = {
       name,
       type,
-      locator,
+      locator: locatorValue,
       annotationType,
       locatorType,
       element_id,
@@ -226,8 +225,7 @@ export const LocatorEditDialog: React.FC<Props> = ({
     ) : null;
 
   const onLocatorTypeChange = async () => {
-    setLocatorValidationRules(_locatorValidationRules());
-
+    setLocatorValidationRules(getLocatorValidationRules());
     const newLocatorType = form.getFieldValue('locatorType');
     const newLocator = form.getFieldValue('locator');
 
@@ -246,7 +244,7 @@ export const LocatorEditDialog: React.FC<Props> = ({
 
   const onFieldsChange = async (changedValues: FieldData[]) => {
     const isLocatorTypeChanged = changedValues.some((value) => value.name.toString().includes('locatorType'));
-    isLocatorTypeChanged && onLocatorTypeChange();
+    if (isLocatorTypeChanged) await onLocatorTypeChange();
     setIsOkButtonDisabled(computeIsOkButtonDisabled());
   };
 
@@ -262,6 +260,29 @@ export const LocatorEditDialog: React.FC<Props> = ({
 
   const locatorTypeOptions = createLocatorTypeOptions(locator, locators, element_id);
 
+  const handleLocatorDropdownOnChange = async () => {
+    setValidationMessage('');
+
+    try {
+      const locatorTypeFromForm = await form.getFieldValue('locatorType');
+      const locatorValue = await form.getFieldValue('locator');
+
+      const updatedValidationMessage: LocatorValidationErrorType = await validateLocator(
+        locatorValue,
+        locatorTypeFromForm,
+        jdnHash,
+        locators,
+        element_id,
+        isCreatingForm,
+      );
+
+      setValidationMessage(updatedValidationMessage);
+    } catch (error) {
+      const newValidationMessage = error.message;
+      setValidationMessage(newValidationMessage);
+    }
+  };
+
   return (
     <DialogWithForm
       onboardingRefProps={{
@@ -308,7 +329,7 @@ export const LocatorEditDialog: React.FC<Props> = ({
       </Form.Item>
       <FormItem name="locatorType" label="Locator" style={{ marginBottom: '8px' }}>
         <Select
-          disabled={!isValidLocator(validationMessage)}
+          onChange={handleLocatorDropdownOnChange}
           options={locatorTypeOptions}
           popupClassName="custom-divider-for-dropdown"
           virtual={false}

src/features/locators/locators.slice.ts

@@ -60,21 +60,48 @@ const locatorsSlice = createSlice({
       state.status = IdentificationStatus.success;
     },
     changeLocatorAttributes(state, { payload }: PayloadAction<ChangeLocatorAttributesPayload>) {
+      // ToDo: fix legacy naming
+      // eslint-disable-next-line @typescript-eslint/naming-convention
       const { locator, element_id, locatorType, ...rest } = payload;
 
-      const _locator = simpleSelectLocatorById(state, element_id);
+      const currentLocator = simpleSelectLocatorById(state, element_id);
 
-      if (!_locator) return;
+      if (!currentLocator) return;
 
-      const newValue = { ..._locator, locator: { ..._locator.locator }, locatorType, ...rest };
+      const newValue: ILocator = { ...currentLocator, locator: { ...currentLocator.locator }, locatorType, ...rest };
+      const isStandardLocator: boolean =
+        locatorType === LocatorType.cssSelector ||
+        locatorType === LocatorType.className ||
+        locatorType === LocatorType.id ||
+        locatorType === LocatorType.linkText ||
+        locatorType === LocatorType.name ||
+        locatorType === LocatorType.tagName ||
+        locatorType.startsWith('data-');
 
       if (locatorType === LocatorType.cssSelector) {
         newValue.locator.cssSelector = locator;
       } else if (locatorType === LocatorType.xPath) {
         newValue.locator.xPath = locator;
+      } else if (isStandardLocator) {
+        if (locatorType === LocatorType.dataAttributes || locatorType.startsWith('data-')) {
+          newValue.locator.attributes = {
+            ...newValue.locator.attributes,
+            dataAttributes: { ...(newValue.locator.attributes?.dataAttributes || {}) },
+          }; // unfreeze object
+
+          if (newValue.locator.attributes.dataAttributes) {
+            newValue.locator.attributes.dataAttributes[locatorType] = locator;
+          }
+        } else {
+          const attributes = { ...newValue.locator.attributes }; // unfreeze object
+          attributes[locatorType] = locator;
+          newValue.locator.attributes = attributes;
+        }
       }
 
-      if (rest.message === LocatorValidationWarnings.NotFound) newValue.jdnHash = '';
+      if (rest.message === LocatorValidationWarnings.NotFound) {
+        newValue.jdnHash = '';
+      }
 
       locatorsAdapter.upsertOne(state, newValue);
     },
@@ -140,6 +167,8 @@ const locatorsSlice = createSlice({
       state,
       { payload }: PayloadAction<{ element_id?: ElementId; priority: LocatorCalculationPriority; ids?: ElementId[] }>,
     ) {
+      // ToDo: fix legacy naming
+      // eslint-disable-next-line @typescript-eslint/naming-convention
       const { element_id, ids, priority } = payload;
       if (element_id) locatorsAdapter.upsertOne(state, { element_id, priority } as ILocator);
       if (ids) {
@@ -181,6 +210,8 @@ const locatorsSlice = createSlice({
       locatorsAdapter.upsertMany(state, locators.map(({ element_id }) => ({ element_id, isGenerated })) as ILocator[]);
     },
     setJdnHash(state, { payload }: PayloadAction<{ element_id: ElementId; jdnHash: string }>) {
+      // ToDo: fix legacy naming
+      // eslint-disable-next-line @typescript-eslint/naming-convention
       const { element_id, jdnHash } = payload;
       locatorsAdapter.upsertOne(state, { element_id, jdnHash } as ILocator);
     },
@@ -212,12 +243,16 @@ const locatorsSlice = createSlice({
       // ToDo isGenerated refactoring
       const locator = typeof payload === 'string' ? simpleSelectLocatorById(state, payload) : payload;
       if (!locator) return;
+      // ToDo: fix legacy naming
+      // eslint-disable-next-line @typescript-eslint/naming-convention
       const { isGenerated, element_id } = locator;
       locatorsAdapter.upsertOne(state, { element_id, isGenerated: !isGenerated } as ILocator);
     },
     toggleLocatorIsChecked(state, { payload }: PayloadAction<string>) {
       const locator = simpleSelectLocatorById(state, payload);
       if (!locator) return;
+      // ToDo: fix legacy naming
+      // eslint-disable-next-line @typescript-eslint/naming-convention
       const { isChecked, element_id } = locator;
       locatorsAdapter.upsertOne(state, { element_id, isChecked: !isChecked } as ILocator);
     },

src/features/locators/utils/locatorOutput.tsx

@@ -37,7 +37,7 @@ export const getLocatorValueByType = (locatorValue: LocatorValue, type: LocatorT
   };
 
   if (locatorValue.attributes.dataAttributes && type.startsWith('data-')) {
-    dataAttribute = `[${type}='${locatorValue.attributes.dataAttributes[type]}']`;
+    dataAttribute = locatorValue.attributes.dataAttributes[type] ?? '';
 
     value[type] = dataAttribute;
   }