mirror of
https://github.com/n8n-io/n8n.git
synced 2024-12-25 12:44:07 -08:00
fix(editor): Fix operation change failing in certain conditions (#8114)
## Summary
This PR handles the case when there are multiple parameters with the
same name but different `options` and `displayOptions`. In this case, if
one of such fields is set, changing the dependent parameter value so the
other should be shown causes an error in case their options are not
compatible (this
[check](7806a65229/packages/workflow/src/NodeHelpers.ts (L786)
)).
#### Example:
LDAP node has two `options` properties with the same name:
1. `attributes` with predefined options (`add`, `replace`, `delete`).
Shown when **Update** operation is selected
2. `attributes` with a collection of `attribute` objects. Shows for the
**Create** operation
Setting one of these parameter values and switching operation so the
other is shown breaks the app.
This PR checks if there is a value saved for such parameter and removes
it before calling `getNodeParameters` in `valueChanged` handler.
## Related tickets and issues
Fixes ADO-1589
## Review / Merge checklist
- [x] PR title and summary are descriptive. **Remember, the title
automatically goes into the changelog. Use `(no-changelog)` otherwise.**
([conventions](https://github.com/n8n-io/n8n/blob/master/.github/pull_request_title_conventions.md))
- [ ] [Docs updated](https://github.com/n8n-io/n8n-docs) or follow-up
ticket created.
- [x] Tests included.
> A bug is not considered fixed, unless a test is added to prevent it
from happening again.
> A feature is not complete without tests.
This commit is contained in:
parent
15ffd4fb9f
commit
711fa2b925
|
@ -490,4 +490,19 @@ describe('NDV', () => {
|
||||||
ndv.getters.nodeVersion().should('have.text', 'Function node version 1 (Deprecated)');
|
ndv.getters.nodeVersion().should('have.text', 'Function node version 1 (Deprecated)');
|
||||||
ndv.actions.close();
|
ndv.actions.close();
|
||||||
});
|
});
|
||||||
|
it('Should handle mismatched option attributes', () => {
|
||||||
|
workflowPage.actions.addInitialNodeToCanvas('LDAP', { keepNdvOpen: true, action: 'Create a new entry' });
|
||||||
|
// Add some attributes in Create operation
|
||||||
|
cy.getByTestId('parameter-item').contains('Add Attributes').click();
|
||||||
|
ndv.actions.changeNodeOperation('Update');
|
||||||
|
// Attributes should be empty after operation change
|
||||||
|
cy.getByTestId('parameter-item').contains('Currently no items exist').should('exist');
|
||||||
|
});
|
||||||
|
it('Should keep RLC values after operation change', () => {
|
||||||
|
const TEST_DOC_ID = '1111';
|
||||||
|
workflowPage.actions.addInitialNodeToCanvas('Google Sheets', { keepNdvOpen: true, action: 'Append row in sheet' });
|
||||||
|
ndv.actions.setRLCValue('documentId', TEST_DOC_ID);
|
||||||
|
ndv.actions.changeNodeOperation('Update Row');
|
||||||
|
ndv.getters.resourceLocatorInput('documentId').find('input').should('have.value', TEST_DOC_ID);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
@ -246,10 +246,14 @@ export class NDV extends BasePage {
|
||||||
});
|
});
|
||||||
this.actions.validateExpressionPreview(fieldName, `node doesn't exist`);
|
this.actions.validateExpressionPreview(fieldName, `node doesn't exist`);
|
||||||
},
|
},
|
||||||
|
|
||||||
openSettings: () => {
|
openSettings: () => {
|
||||||
this.getters.nodeSettingsTab().click();
|
this.getters.nodeSettingsTab().click();
|
||||||
},
|
},
|
||||||
|
changeNodeOperation: (operation: string) => {
|
||||||
|
this.getters.parameterInput('operation').click();
|
||||||
|
cy.get('.el-select-dropdown__item').contains(new RegExp(`^${operation}$`)).click({ force: true });
|
||||||
|
this.getters.parameterInput('operation').find('input').should('have.value', operation);
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -864,6 +864,12 @@ export default defineComponent({
|
||||||
} else {
|
} else {
|
||||||
set(nodeParameters as object, parameterPath, newValue);
|
set(nodeParameters as object, parameterPath, newValue);
|
||||||
}
|
}
|
||||||
|
// If value is updated, remove parameter values that have invalid options
|
||||||
|
// so getNodeParameters checks don't fail
|
||||||
|
this.removeMismatchedOptionValues(nodeType, nodeParameters, {
|
||||||
|
name: parameterPath,
|
||||||
|
value: newValue,
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Get the parameters with the now new defaults according to the
|
// Get the parameters with the now new defaults according to the
|
||||||
|
@ -919,6 +925,44 @@ export default defineComponent({
|
||||||
this.workflowsStore.setNodeValue(updateInformation);
|
this.workflowsStore.setNodeValue(updateInformation);
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
/**
|
||||||
|
* Removes node values that are not valid options for the given parameter.
|
||||||
|
* This can happen when there are multiple node parameters with the same name
|
||||||
|
* but different options and display conditions
|
||||||
|
* @param nodeType The node type description
|
||||||
|
* @param nodeParameterValues Current node parameter values
|
||||||
|
* @param updatedParameter The parameter that was updated. Will be used to determine which parameters to remove based on their display conditions and option values
|
||||||
|
*/
|
||||||
|
removeMismatchedOptionValues(
|
||||||
|
nodeType: INodeTypeDescription,
|
||||||
|
nodeParameterValues: INodeParameters | null,
|
||||||
|
updatedParameter: { name: string; value: NodeParameterValue },
|
||||||
|
) {
|
||||||
|
nodeType.properties.forEach((prop) => {
|
||||||
|
const displayOptions = prop.displayOptions;
|
||||||
|
// Not processing parameters that are not set or don't have options
|
||||||
|
if (!nodeParameterValues?.hasOwnProperty(prop.name) || !displayOptions || !prop.options) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Only process the parameters that should be hidden
|
||||||
|
const showCondition = displayOptions.show?.[updatedParameter.name];
|
||||||
|
const hideCondition = displayOptions.hide?.[updatedParameter.name];
|
||||||
|
if (showCondition === undefined && hideCondition === undefined) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Every value should be a possible option
|
||||||
|
const hasValidOptions = Object.keys(nodeParameterValues).every(
|
||||||
|
(key) => (prop.options ?? []).find((option) => option.name === key) !== undefined,
|
||||||
|
);
|
||||||
|
if (
|
||||||
|
!hasValidOptions ||
|
||||||
|
showCondition !== updatedParameter.value ||
|
||||||
|
hideCondition === updatedParameter.value
|
||||||
|
) {
|
||||||
|
unset(nodeParameterValues as object, prop.name);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
},
|
||||||
/**
|
/**
|
||||||
* Sets the values of the active node in the internal settings variables
|
* Sets the values of the active node in the internal settings variables
|
||||||
*/
|
*/
|
||||||
|
|
Loading…
Reference in a new issue