From aaf87c3edd434ab464f3ec4a4001c07895370cb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milorad=20FIlipovi=C4=87?= Date: Thu, 14 Sep 2023 10:54:25 +0200 Subject: [PATCH] fix(editor): Testing flaky resource mapper feature in e2e tests (#7165) --- cypress/e2e/28-resource-mapper.cy.ts | 53 ++++++++++++++++- cypress/pages/ndv.ts | 4 ++ .../src/components/ParameterInputWrapper.vue | 7 ++- .../ResourceMapper/MappingFields.vue | 6 +- .../__tests__/ResourceMapper.test.ts | 57 ++----------------- .../nodes-base/nodes/E2eTest/E2eTest.node.ts | 8 +++ packages/nodes-base/nodes/E2eTest/mock.ts | 9 +++ 7 files changed, 86 insertions(+), 58 deletions(-) diff --git a/cypress/e2e/28-resource-mapper.cy.ts b/cypress/e2e/28-resource-mapper.cy.ts index 8c47dda832..ec4341d2ae 100644 --- a/cypress/e2e/28-resource-mapper.cy.ts +++ b/cypress/e2e/28-resource-mapper.cy.ts @@ -17,7 +17,7 @@ describe('Resource Mapper', () => { .resourceMapperFieldsContainer() .should('be.visible') .findChildByTestId('parameter-input') - .should('have.length', 2); + .should('have.length', 3); ndv.actions.setInvalidExpression('fieldId'); @@ -34,7 +34,7 @@ describe('Resource Mapper', () => { .resourceMapperFieldsContainer() .should('be.visible') .findChildByTestId('parameter-input') - .should('have.length', 2); + .should('have.length', 3); ndv.actions.setInvalidExpression('otherField'); @@ -43,6 +43,53 @@ describe('Resource Mapper', () => { .resourceMapperFieldsContainer() .should('be.visible') .findChildByTestId('parameter-input') - .should('have.length', 2); + .should('have.length', 3); + }); + + it('should correctly delete single field', () => { + workflowPage.actions.addInitialNodeToCanvas('E2e Test', { + action: 'Resource Mapping Component', + }); + ndv.getters.parameterInput('id').type('001'); + ndv.getters.parameterInput('name').type('John'); + ndv.getters.parameterInput('age').type('30'); + ndv.getters.nodeExecuteButton().click(); + ndv.getters.outputTableHeaderByText('id').should('exist'); + ndv.getters.outputTableHeaderByText('name').should('exist'); + ndv.getters.outputTableHeaderByText('age').should('exist'); + // Remove the 'name' field + ndv.getters.resourceMapperRemoveFieldButton('name').should('exist').click({ force: true }); + ndv.getters.nodeExecuteButton().click(); + ndv.getters.parameterInput('id').should('exist'); + ndv.getters.outputTableHeaderByText('id').should('exist'); + // After removing the field, text field and the output table column for the 'name' should not be there anymore + ndv.getters.parameterInput('age').should('exist'); + ndv.getters.outputTableHeaderByText('age').should('exist'); + ndv.getters.parameterInput('name').should('not.exist'); + ndv.getters.outputTableHeaderByText('name').should('not.exist'); + }); + + it('should correctly delete all fields', () => { + workflowPage.actions.addInitialNodeToCanvas('E2e Test', { + action: 'Resource Mapping Component', + }); + ndv.getters.parameterInput('id').type('001'); + ndv.getters.parameterInput('name').type('John'); + ndv.getters.parameterInput('age').type('30'); + ndv.getters.nodeExecuteButton().click(); + ndv.getters.outputTableHeaderByText('id').should('exist'); + ndv.getters.outputTableHeaderByText('name').should('exist'); + ndv.getters.outputTableHeaderByText('age').should('exist'); + ndv.getters.resourceMapperColumnsOptionsButton().click(); + // Click on the 'Remove All Fields' option + ndv.getters.resourceMapperRemoveAllFieldsOption().should('be.visible').click(); + ndv.getters.nodeExecuteButton().click(); + ndv.getters.parameterInput('id').should('exist'); + ndv.getters.outputTableHeaderByText('id').should('exist'); + // After removing the all fields, only required one should be in UI and output table + ndv.getters.parameterInput('name').should('not.exist'); + ndv.getters.outputTableHeaderByText('name').should('not.exist'); + ndv.getters.parameterInput('age').should('not.exist'); + ndv.getters.outputTableHeaderByText('age').should('not.exist'); }); }); diff --git a/cypress/pages/ndv.ts b/cypress/pages/ndv.ts index dfaf0d0292..b8e28069ac 100644 --- a/cypress/pages/ndv.ts +++ b/cypress/pages/ndv.ts @@ -28,6 +28,7 @@ export class NDV extends BasePage { this.getters.runDataPaneHeader().find('button').filter(':visible').contains('Save'), outputTableRows: () => this.getters.outputDataContainer().find('table tr'), outputTableHeaders: () => this.getters.outputDataContainer().find('table thead th'), + outputTableHeaderByText: (text: string) => this.getters.outputTableHeaders().contains(text), outputTableRow: (row: number) => this.getters.outputTableRows().eq(row), outputTbodyCell: (row: number, col: number) => this.getters.outputTableRow(row).find('td').eq(col), @@ -71,6 +72,9 @@ export class NDV extends BasePage { this.getters.resourceLocator(paramName).find('[data-test-id="rlc-mode-selector"]'), resourceMapperFieldsContainer: () => cy.getByTestId('mapping-fields-container'), resourceMapperSelectColumn: () => cy.getByTestId('matching-column-select'), + resourceMapperRemoveFieldButton: (fieldName: string) => cy.getByTestId(`remove-field-button-${fieldName}`), + resourceMapperColumnsOptionsButton: () => cy.getByTestId('columns-parameter-input-options-container'), + resourceMapperRemoveAllFieldsOption: () => cy.getByTestId('action-removeAllFields'), }; actions = { diff --git a/packages/editor-ui/src/components/ParameterInputWrapper.vue b/packages/editor-ui/src/components/ParameterInputWrapper.vue index 3d9a4901c6..4318932e8f 100644 --- a/packages/editor-ui/src/components/ParameterInputWrapper.vue +++ b/packages/editor-ui/src/components/ParameterInputWrapper.vue @@ -18,7 +18,7 @@ :expressionEvaluated="expressionValueComputed" :additionalExpressionData="resolvedAdditionalExpressionData" :label="label" - :data-test-id="`parameter-input-${parameter.name}`" + :data-test-id="`parameter-input-${parsedParameterName}`" :event-bus="eventBus" @focus="onFocus" @blur="onBlur" @@ -61,7 +61,7 @@ import type { import { isResourceLocatorValue } from 'n8n-workflow'; import type { INodeUi, IUpdateInformation, TargetItem } from '@/Interface'; import { workflowHelpers } from '@/mixins/workflowHelpers'; -import { isValueExpression } from '@/utils'; +import { isValueExpression, parseResourceMapperFieldName } from '@/utils'; import { useNDVStore } from '@/stores/ndv.store'; import { useEnvironmentsStore, useExternalSecretsStore } from '@/stores'; @@ -226,6 +226,9 @@ export default defineComponent({ ...this.additionalExpressionData, }; }, + parsedParameterName() { + return parseResourceMapperFieldName(this.parameter?.name ?? ''); + }, }, methods: { onFocus() { diff --git a/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue b/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue index dd6e295cbb..1ba56e0bcd 100644 --- a/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue +++ b/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue @@ -242,6 +242,10 @@ function getParamType(field: ResourceMapperField): NodePropertyTypes { return 'string'; } +function getParsedFieldName(fullName: string): string { + return parseResourceMapperFieldName(fullName) ?? fullName; +} + function onValueChanged(value: IUpdateInformation): void { emit('fieldValueChanged', value); } @@ -344,7 +348,7 @@ defineExpose({ }, }) " - data-test-id="remove-field-button" + :data-test-id="`remove-field-button-${getParsedFieldName(field.name)}`" @click="removeField(field.name)" /> diff --git a/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts b/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts index 417985e382..4d270d5ef3 100644 --- a/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts +++ b/packages/editor-ui/src/components/__tests__/ResourceMapper.test.ts @@ -223,7 +223,11 @@ describe('ResourceMapper.vue', () => { expect( getByTestId('mapping-fields-container').querySelectorAll('.parameter-input').length, ).toBe(4); - expect(queryAllByTestId('remove-field-button').length).toBe(1); + expect( + getByTestId('mapping-fields-container').querySelectorAll( + '[data-test-id^="remove-field-button"]', + ).length, + ).toBe(1); }); it('should render correct options based on saved schema', async () => { @@ -291,55 +295,4 @@ describe('ResourceMapper.vue', () => { await waitAllPromises(); expect(fetchFieldsSpy).not.toHaveBeenCalled(); }); - - it.skip('should delete fields from UI and parameter value when they are deleted', async () => { - const { getByTestId, emitted } = renderComponent({ - props: { - node: { - parameters: { - columns: { - schema: null, - }, - }, - }, - }, - }); - await waitAllPromises(); - // Add some values so we can test if they are gone after deletion - const idInput = getByTestId('parameter-input-value["id"]').querySelector('input'); - const firstNameInput = getByTestId('parameter-input-value["First name"]').querySelector( - 'input', - ); - const lastNameInput = getByTestId('parameter-input-value["Last name"]').querySelector('input'); - const usernameInput = getByTestId('parameter-input-value["Username"]').querySelector('input'); - const addressInput = getByTestId('parameter-input-value["Address"]').querySelector('input'); - if (idInput && firstNameInput && lastNameInput && usernameInput && addressInput) { - await userEvent.type(idInput, '123'); - await userEvent.type(firstNameInput, 'John'); - await userEvent.type(lastNameInput, 'Doe'); - await userEvent.type(usernameInput, 'johndoe'); - await userEvent.type(addressInput, '123 Main St'); - // All field values should be in parameter value - const valueBeforeRemove = getLatestValueChangeEvent(emitted()); - expect(valueBeforeRemove[0].value.value).toHaveProperty('id'); - expect(valueBeforeRemove[0].value.value).toHaveProperty('First name'); - expect(valueBeforeRemove[0].value.value).toHaveProperty('Last name'); - expect(valueBeforeRemove[0].value.value).toHaveProperty('Username'); - expect(valueBeforeRemove[0].value.value).toHaveProperty('Address'); - // Click on 'Remove all fields' option - await userEvent.click(getByTestId('columns-parameter-input-options-container')); - await userEvent.click(getByTestId('action-removeAllFields')); - // Should delete all non-mandatory fields: - // 1. From UI - expect( - getByTestId('resource-mapper-container').querySelectorAll('.parameter-item').length, - ).toBe(3); - // 2. And their values from parameter value - const valueAfterRemove = getLatestValueChangeEvent(emitted()); - expect(valueAfterRemove[0].value.value).not.toHaveProperty('Username'); - expect(valueAfterRemove[0].value.value).not.toHaveProperty('Address'); - } else { - throw new Error('Could not find input fields'); - } - }); }); diff --git a/packages/nodes-base/nodes/E2eTest/E2eTest.node.ts b/packages/nodes-base/nodes/E2eTest/E2eTest.node.ts index e5d20ecbd5..c5de614ec4 100644 --- a/packages/nodes-base/nodes/E2eTest/E2eTest.node.ts +++ b/packages/nodes-base/nodes/E2eTest/E2eTest.node.ts @@ -202,6 +202,14 @@ export class E2eTest implements INodeType { }; async execute(this: IExecuteFunctions): Promise { + const operation = this.getNodeParameter('operation', 0); + // For resource mapper testing, return actual node values + if (operation === 'resourceMapper') { + const rmValue = this.getNodeParameter('resourceMapper.value', 0); + if (rmValue) { + return [[{ json: rmValue as INodeExecutionData }]]; + } + } return [returnData]; } } diff --git a/packages/nodes-base/nodes/E2eTest/mock.ts b/packages/nodes-base/nodes/E2eTest/mock.ts index e86f90592e..15422441dc 100644 --- a/packages/nodes-base/nodes/E2eTest/mock.ts +++ b/packages/nodes-base/nodes/E2eTest/mock.ts @@ -51,6 +51,15 @@ export const resourceMapperFields: ResourceMapperFields = { display: true, type: 'string', }, + { + id: 'age', + displayName: 'Age', + defaultMatch: false, + canBeUsedToMatch: false, + required: false, + display: true, + type: 'number', + }, ], };