Skip to content

Commit

Permalink
fix(core): Fix issue with fixedCollection having all default values
Browse files Browse the repository at this point in the history
  • Loading branch information
@janober
janober committed May 15, 2022
1 parent 03cdb1f commit 7ced654
Show file tree
Hide file tree
Showing 2 changed files with 331 additions and 0 deletions.
31 changes: 31 additions & 0 deletions packages/workflow/src/NodeHelpers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -720,6 +720,18 @@ export function getNodeParameters(
}
}

if (
!returnDefaults &&
nodeProperties.typeOptions?.multipleValues === false &&
propertyValues &&
Object.keys(propertyValues).length === 0
) {
// For fixedCollections, which only allow one value, it is important to still return
// the empty object which indicates that a value got added, even if it does not have
// anything set. If that is not done, the value would get lost.
return nodeValues;
}

// Iterate over all collections
for (const itemName of Object.keys(propertyValues || {})) {
if (
Expand Down Expand Up @@ -795,6 +807,25 @@ export function getNodeParameters(
}
}

if (
!returnDefaults &&
nodeProperties.typeOptions?.multipleValues === false &&
collectionValues &&
Object.keys(collectionValues).length === 0 &&
propertyValues &&
propertyValues?.constructor.name === 'Object' &&
Object.keys(propertyValues).length !== 0
) {
// For fixedCollections, which only allow one value, it is important to still return
// the object with an empty collection property which indicates that a value got added
// which contains all default values. If that is not done, the value would get lost.
const returnValue = {} as INodeParameters;
Object.keys(propertyValues || {}).forEach((value) => {
returnValue[value] = {};
});
return { [nodeProperties.name]: returnValue };
}

if (Object.keys(collectionValues).length !== 0 || returnDefaults) {
// Set only if value got found
if (returnDefaults) {
Expand Down
300 changes: 300 additions & 0 deletions packages/workflow/test/NodeHelpers.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3008,6 +3008,306 @@ describe('Workflow', () => {
},
},
},
{
description:
'complex type "collection" which contains a "fixedCollection" with "multipleValues: false" that has a collection value added but nothing to the fixedCollection',
input: {
nodePropertiesArray: [
{
displayName: 'Options',
name: 'options',
placeholder: 'Add Option',
type: 'collection',
default: {},
options: [
{
displayName: 'Sort',
name: 'sort',
type: 'fixedCollection',
typeOptions: {
multipleValues: false,
},
default: {},
placeholder: 'Add Sort',
options: [
{
displayName: 'Sort',
name: 'value',
values: [
{
displayName: 'Descending',
name: 'descending',
type: 'boolean',
default: true,
description: 'Sort by descending order',
},
{
displayName: 'Order By',
name: 'ordering',
type: 'options',
default: 'date',
options: [
{
name: 'Date',
value: 'date',
},
{
name: 'Name',
value: 'name',
},
],
},
],
},
],
},
],
},
],
nodeValues: {
options: {
sort: {},
},
},
},
output: {
noneDisplayedFalse: {
defaultsFalse: {
options: {
sort: {},
},
},
defaultsTrue: {
options: {
sort: {},
},
},
},
noneDisplayedTrue: {
defaultsFalse: {
options: {
sort: {},
},
},
defaultsTrue: {
options: {
sort: {},
},
},
},
},
},
{
description:
'complex type "collection" which contains a "fixedCollection" with "multipleValues: false" that has all values set to the default values (by having it as an empty object)',
input: {
nodePropertiesArray: [
{
displayName: 'Options',
name: 'options',
placeholder: 'Add Option',
type: 'collection',
default: {},
options: [
{
displayName: 'Sort',
name: 'sort',
type: 'fixedCollection',
typeOptions: {
multipleValues: false,
},
default: {},
placeholder: 'Add Sort',
options: [
{
displayName: 'Sort',
name: 'value',
values: [
{
displayName: 'Descending',
name: 'descending',
type: 'boolean',
default: true,
description: 'Sort by descending order',
},
{
displayName: 'Order By',
name: 'ordering',
type: 'options',
default: 'date',
options: [
{
name: 'Date',
value: 'date',
},
{
name: 'Name',
value: 'name',
},
],
},
],
},
],
},
],
},
],
nodeValues: {
options: {
sort: {
value: {},
},
},
},
},
output: {
noneDisplayedFalse: {
defaultsFalse: {
options: {
sort: {
value: {},
},
},
},
defaultsTrue: {
options: {
sort: {
value: {
descending: true,
ordering: 'date',
},
},
},
},
},
noneDisplayedTrue: {
defaultsFalse: {
options: {
sort: {
value: {},
},
},
},
defaultsTrue: {
options: {
sort: {
value: {
descending: true,
ordering: 'date',
},
},
},
},
},
},
},
{
description:
'complex type "collection" which contains a "fixedCollection" with "multipleValues: false" that has all values set to the default values (by having each value set)',
input: {
nodePropertiesArray: [
{
displayName: 'Options',
name: 'options',
placeholder: 'Add Option',
type: 'collection',
default: {},
options: [
{
displayName: 'Sort',
name: 'sort',
type: 'fixedCollection',
typeOptions: {
multipleValues: false,
},
default: {},
options: [
{
displayName: 'Sort',
name: 'value',
values: [
{
displayName: 'Descending',
name: 'descending',
type: 'boolean',
default: true,
},
{
displayName: 'Order By',
name: 'ordering',
type: 'options',
default: 'date',
options: [
{
name: 'Date',
value: 'date',
},
{
name: 'Name',
value: 'name',
},
],
},
],
},
],
},
],
},
],
nodeValues: {
options: {
sort: {
value: {
descending: true,
ordering: 'date',
},
},
},
},
},
output: {
noneDisplayedFalse: {
defaultsFalse: {
options: {
sort: {
value: {},
},
},
},
defaultsTrue: {
options: {
sort: {
value: {
descending: true,
ordering: 'date',
},
},
},
},
},
noneDisplayedTrue: {
defaultsFalse: {
options: {
sort: {
value: {},
},
},
},
defaultsTrue: {
options: {
sort: {
value: {
descending: true,
ordering: 'date',
},
},
},
},
},
},
},
];

for (const testData of tests) {
Expand Down

0 comments on commit 7ced654

Please sign in to comment.