Skip to content

Commit

Permalink
fix: Fix mapping paths when appending to empty expression (#5591)
Browse files Browse the repository at this point in the history
* fix: Fix mapping when appending to empty expression

* fix: refactor logic out

* test: add tests

* test: add tests

* fix: fix bug where value does not get updated when mapping

* test: add test for bug

* test: add test for bug
  • Loading branch information
mutdmour committed Mar 2, 2023
1 parent 31cc8de commit 1f7b478
Show file tree
Hide file tree
Showing 9 changed files with 294 additions and 33 deletions.
30 changes: 30 additions & 0 deletions cypress/e2e/14-mapping.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,4 +262,34 @@ describe('Data mapping', () => {
.find('input')
.should('have.value', 'input[0]["hello.world"]["my count"]');
});

it('maps expressions to updated fields correctly', () => {
cy.fixture('Test_workflow_3.json').then((data) => {
cy.get('body').paste(JSON.stringify(data));
});

workflowPage.actions.openNode('Set');

ndv.actions.typeIntoParameterInput('value', 'delete me');
ndv.actions.dismissMappingTooltip();

ndv.actions.typeIntoParameterInput('name', 'test');

ndv.actions.typeIntoParameterInput('value', 'fun');
ndv.actions.clearParameterInput('value'); // keep focus on param

ndv.getters.inputDataContainer().should('exist').find('span').contains('count').realMouseDown();

ndv.actions.mapToParameter('value');
ndv.getters.inlineExpressionEditorInput().should('have.text', '{{ $json.input[0].count }}');
ndv.getters.parameterExpressionPreview('value').should('include.text', '0');

ndv.getters.inputDataContainer().find('span').contains('input').realMouseDown();

ndv.actions.mapToParameter('value');
ndv.getters
.inlineExpressionEditorInput()
.should('have.text', '{{ $json.input[0].count }} {{ $json.input }}');
ndv.getters.parameterExpressionPreview('value').should('include.text', '0 [object Object]');
});
});
3 changes: 3 additions & 0 deletions cypress/pages/ndv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ export class NDV extends BasePage {
selectOptionInParameterDropdown: (parameterName: string, content: string) => {
this.getters.parameterInput(parameterName).find('.option-headline').contains(content).click();
},
dismissMappingTooltip: () => {
cy.getByTestId('dismiss-mapping-tooltip').click();
},
rename: (newName: string) => {
this.getters.nodeNameContainer().click();
this.getters.nodeRenameInput().should('be.visible').type('{selectall}').type(newName);
Expand Down
2 changes: 2 additions & 0 deletions packages/design-system/src/types/button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export type IN8nButton = {
active?: boolean;
float?: 'left' | 'right';
square?: boolean;
// eslint-disable-next-line @typescript-eslint/naming-convention
'data-test-id'?: string;
};
listeners?: Record<string, (event: Event) => void>;
};
6 changes: 6 additions & 0 deletions packages/editor-ui/src/components/Draggable.vue
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ export default Vue.extend({
window.addEventListener('mousemove', this.onDrag);
window.addEventListener('mouseup', this.onDragEnd);
// blur so that any focused inputs update value
const activeElement = document.activeElement as HTMLElement;
if (activeElement) {
activeElement.blur();
}
},
onDrag(e: MouseEvent) {
e.preventDefault();
Expand Down
38 changes: 8 additions & 30 deletions packages/editor-ui/src/components/ParameterInputFull.vue
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ import { mapStores } from 'pinia';
import { useNDVStore } from '@/stores/ndv';
import { useSegment } from '@/stores/segment';
import { externalHooks } from '@/mixins/externalHooks';
import { getMappedResult } from '../utils/mappingUtils';
export default mixins(showMessage, externalHooks).extend({
name: 'parameter-input-full',
Expand Down Expand Up @@ -146,6 +147,7 @@ export default mixins(showMessage, externalHooks).extend({
{
attrs: {
label: this.$locale.baseText('_reusableBaseText.dismiss' as BaseTextKey),
'data-test-id': 'dismiss-mapping-tooltip',
},
listeners: {
click: mappingTooltipDismissHandler,
Expand Down Expand Up @@ -228,39 +230,15 @@ export default mixins(showMessage, externalHooks).extend({
param?.$emit('optionSelected', 'addExpression');
}
},
onDrop(data: string) {
const useDataPath = !!this.parameter.requiresDataPath && data.startsWith('{{ $json');
if (!useDataPath) {
onDrop(newParamValue: string) {
const updatedValue = getMappedResult(this.parameter, newParamValue, this.value);
const prevValue = this.isResourceLocator ? this.value.value : this.value;
if (updatedValue.startsWith('=')) {
this.forceShowExpression = true;
}
setTimeout(() => {
if (this.node) {
const prevValue = this.isResourceLocator ? this.value.value : this.value;
let updatedValue: string;
if (useDataPath) {
const newValue = data
.replace('{{ $json', '')
.replace(new RegExp('^\\.'), '')
.replace(new RegExp('}}$'), '')
.trim();
if (prevValue && this.parameter.requiresDataPath === 'multiple') {
updatedValue = `${prevValue}, ${newValue}`;
} else {
updatedValue = newValue;
}
} else if (
typeof prevValue === 'string' &&
prevValue.startsWith('=') &&
prevValue.length > 1
) {
updatedValue = `${prevValue} ${data}`;
} else if (prevValue && ['string', 'json'].includes(this.parameter.type)) {
updatedValue = prevValue === '=' ? `=${data}` : `=${prevValue} ${data}`;
} else {
updatedValue = `=${data}`;
}
let parameterData;
if (this.isResourceLocator) {
if (!isResourceLocatorValue(this.value)) {
Expand Down Expand Up @@ -334,7 +312,7 @@ export default mixins(showMessage, externalHooks).extend({
}, 200);
},
onMappingTooltipDismissed() {
this.localStorageMappingFlag = true;
this.ndvStore.disableMappingHint(false);
},
},
watch: {
Expand Down
6 changes: 4 additions & 2 deletions packages/editor-ui/src/stores/ndv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,11 @@ export const useNDVStore = defineStore(STORES.NDV, {
setNDVPanelDataIsEmpty(payload: { panel: 'input' | 'output'; isEmpty: boolean }): void {
Vue.set(this[payload.panel].data, 'isEmpty', payload.isEmpty);
},
disableMappingHint() {
disableMappingHint(store = true) {
this.isMappingOnboarded = true;
window.localStorage.setItem(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, 'true');
if (store) {
window.localStorage.setItem(LOCAL_STORAGE_MAPPING_IS_ONBOARDED, 'true');
}
},
},
});
202 changes: 202 additions & 0 deletions packages/editor-ui/src/utils/__tests__/mappingUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
import { INodeProperties } from 'n8n-workflow';
import { getMappedResult } from '../mappingUtils';

const RLC_PARAM: INodeProperties = {
displayName: 'Base',
name: 'application',
type: 'resourceLocator',
default: {
mode: 'url',
value: '',
},
required: true,
description: 'The Airtable Base in which to operate on',
modes: [
{
displayName: 'ID',
name: 'id',
type: 'string',
validation: [
{
type: 'regex',
properties: {
regex: '[a-zA-Z0-9]{2,}',
errorMessage: 'Not a valid Airtable Base ID',
},
},
],
placeholder: 'appD3dfaeidke',
url: '=https://airtable.com/{{$value}}',
},
],
};

const STRING_PARAM: INodeProperties = {
displayName: 'Value',
name: 'value',
type: 'string',
default: '',
description: 'The string value to write in the property',
};

const SINGLE_DATA_PATH_PARAM: INodeProperties = {
displayName: 'Name',
name: 'name',
type: 'string',
default: 'propertyName',
requiresDataPath: 'single',
description:
'Name of the property to write data to. Supports dot-notation. Example: "data.person[0].name"',
};

const MULTIPLE_DATA_PATH_PARAM: INodeProperties = {
displayName: 'For Everything Except',
name: 'exceptWhenMix',
type: 'string',
default: '',
placeholder: 'e.g. id, country',
hint: 'Enter the names of the input fields as text, separated by commas',
displayOptions: {
show: {
resolve: ['mix'],
},
},
requiresDataPath: 'multiple',
};

const JSON_PARAM: INodeProperties = {
displayName: 'JSON Payload',
name: 'payloadJson',
type: 'json',
typeOptions: {
alwaysOpenEditWindow: true,
editor: 'code',
},
default: '',
};

const BOOLEAN_PARAM: INodeProperties = {
displayName: 'Value',
name: 'value',
type: 'boolean',
default: false,
description: 'The boolean value to write in the property',
};

const NUMBER_PARAM: INodeProperties = {
displayName: 'Value',
name: 'value',
type: 'number',
default: 0,
description: 'The number value to write in the property',
};

describe('Mapping Utils', () => {
describe('getMappedResult', () => {
it('turns empty string into expression', () => {
expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '')).toEqual(
'={{ $json["Readable date"] }}',
);
});

it('keeps spaces when mapping data to fixed value', () => {
expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', ' ')).toEqual(
'= {{ $json["Readable date"] }}',
);
});

it('sets expression when mapping to an empty expression', () => {
expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '=')).toEqual(
'={{ $json["Readable date"] }}',
);
});

it('keeps spaces when mapping data to expression value', () => {
expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '= ')).toEqual(
'= {{ $json["Readable date"] }}',
);
});

it('appends to string fixed value and turns into expression', () => {
expect(getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', 'test')).toEqual(
'=test {{ $json["Readable date"] }}',
);
});

it('appends to json fixed value', () => {
expect(getMappedResult(JSON_PARAM, '{{ $json["Readable date"] }}', 'test')).toEqual(
'=test {{ $json["Readable date"] }}',
);
});

it('replaces number value with expression', () => {
expect(getMappedResult(NUMBER_PARAM, '{{ $json["Readable date"] }}', 0)).toEqual(
'={{ $json["Readable date"] }}',
);
});

it('replaces boolean value with expression', () => {
expect(getMappedResult(BOOLEAN_PARAM, '{{ $json["Readable date"] }}', false)).toEqual(
'={{ $json["Readable date"] }}',
);
});

it('appends existing expression value', () => {
expect(
getMappedResult(STRING_PARAM, '{{ $json["Readable date"] }}', '={{$json.test}}'),
).toEqual('={{$json.test}} {{ $json["Readable date"] }}');
});

it('sets data path, replacing if expecting single path', () => {
expect(
getMappedResult(SINGLE_DATA_PATH_PARAM, '{{ $json["Readable date"] }}', '={{$json.test}}'),
).toEqual('["Readable date"]');

expect(
getMappedResult(SINGLE_DATA_PATH_PARAM, '{{ $json.path }}', '={{$json.test}}'),
).toEqual('path');
});

it('appends to existing data path, if multiple', () => {
expect(
getMappedResult(MULTIPLE_DATA_PATH_PARAM, '{{ $json["Readable date"] }}', 'path'),
).toEqual('path, ["Readable date"]');
});

it('replaces existing dadata path if multiple and is empty expression', () => {
expect(getMappedResult(MULTIPLE_DATA_PATH_PARAM, '{{ $json.test }}', '=')).toEqual('test');
});

it('handles data when dragging from grand-parent nodes', () => {
expect(
getMappedResult(
MULTIPLE_DATA_PATH_PARAM,
'{{ $node["Schedule Trigger"].json["Day of week"] }}',
'',
),
).toEqual('={{ $node["Schedule Trigger"].json["Day of week"] }}');

expect(
getMappedResult(
SINGLE_DATA_PATH_PARAM,
'{{ $node["Schedule Trigger"].json["Day of week"] }}',
'=data',
),
).toEqual('=data {{ $node["Schedule Trigger"].json["Day of week"] }}');

expect(
getMappedResult(
SINGLE_DATA_PATH_PARAM,
'{{ $node["Schedule Trigger"].json["Day of week"] }}',
'= ',
),
).toEqual('= {{ $node["Schedule Trigger"].json["Day of week"] }}');
});

it('handles RLC values', () => {
expect(getMappedResult(RLC_PARAM, '{{ test }}', '')).toEqual('={{ test }}');
expect(getMappedResult(RLC_PARAM, '{{ test }}', '=')).toEqual('={{ test }}');
expect(getMappedResult(RLC_PARAM, '{{ test }}', '=test')).toEqual('=test {{ test }}');
});
});
});
2 changes: 1 addition & 1 deletion packages/editor-ui/src/utils/__tests__/typesUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import jp from 'jsonpath';
import { isEmpty, intersection, mergeDeep, getSchema, isValidDate } from '@/utils';
import { Schema } from '@/Interface';

describe('Utils', () => {
describe('Types Utils', () => {
describe('isEmpty', () => {
test.each([
[undefined, true],
Expand Down

0 comments on commit 1f7b478

Please sign in to comment.