Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(editor): Disable data pinning on multiple output node types #5111

Merged
merged 3 commits into from
Jan 9, 2023

Conversation

alexgrozav
Copy link
Member

Github issue / Community forum post (link here to close automatically):

ivov
ivov previously approved these changes Jan 9, 2023
Copy link
Contributor

@ivov ivov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In future we should think about turning all these lists at the client into settings at node definition level.

@@ -141,7 +142,11 @@ export const NON_ACTIVATABLE_TRIGGER_NODE_TYPES = [
EXECUTE_WORKFLOW_TRIGGER_NODE_TYPE,
];

export const MULTIPLE_OUTPUT_NODE_TYPES = [IF_NODE_TYPE, SWITCH_NODE_TYPE];
export const MULTIPLE_OUTPUT_NODE_TYPES = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we instead check if that node has multiple outputs? the outputs are specified as an array in the node properties.. can we check for that instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea! Updated.

@n8n-assistant n8n-assistant bot added n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Jan 9, 2023
@alexgrozav alexgrozav changed the title fix(editor): Disable data pinning on Compare Datasets node fix(editor): Disable data pinning on multiple output node types Jan 9, 2023
mutdmour
mutdmour previously approved these changes Jan 9, 2023
Copy link
Contributor

@mutdmour mutdmour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for addressing feedback 😊

...MULTIPLE_OUTPUT_NODE_TYPES,
SPLIT_IN_BATCHES_NODE_TYPE,
];
export const PIN_DATA_NODE_TYPES_DENYLIST = [SPLIT_IN_BATCHES_NODE_TYPE];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree with @ivov that this can be a node property

@@ -82,6 +82,7 @@ export const BAMBOO_HR_NODE_TYPE = 'n8n-nodes-base.bambooHr';
export const CALENDLY_TRIGGER_NODE_TYPE = 'n8n-nodes-base.calendlyTrigger';
export const CODE_NODE_TYPE = 'n8n-nodes-base.code';
export const CRON_NODE_TYPE = 'n8n-nodes-base.cron';
export const COMPARE_DATASETS_NODE_TYPE = 'n8n-nodes-base.compareDatasets';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be deleted since it's not used

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@alexgrozav alexgrozav merged commit 56951e8 into master Jan 9, 2023
@alexgrozav alexgrozav deleted the n8n-5710-disallow-pinning-data-on-any-node-with branch January 9, 2023 15:15
@alexgrozav alexgrozav added the Upcoming Release Will be part of the upcoming release label Jan 9, 2023
MiloradFilipovic added a commit that referenced this pull request Jan 10, 2023
* master: (21 commits)
  test: Fix default owner test to be able to reach setup page (#5113)
  refactor: Reduce custom docker image size by about 30% (#5104)
  fix(editor): Disable data pinning on multiple output node types (#5111)
  📚 Update CHANGELOG.md and main package.json to 0.210.2
  🔖 Release n8n@0.210.2
  ⬆️ Set n8n-core@0.150.1, n8n-editor-ui@0.176.2, n8n-nodes-base@0.208.2 and n8n-workflow@0.132.1 on n8n
  🔖 Release n8n-editor-ui@0.176.2
  ⬆️ Set n8n-workflow@0.132.1 on n8n-editor-ui
  🔖 Release n8n-nodes-base@0.208.2
  ⬆️ Set n8n-core@0.150.1 and n8n-workflow@0.132.1 on n8n-nodes-base
  🔖 Release n8n-node-dev@0.89.1
  ⬆️ Set n8n-core@0.150.1 and n8n-workflow@0.132.1 on n8n-node-dev
  🔖 Release n8n-core@0.150.1
  ⬆️ Set n8n-workflow@0.132.1 on n8n-core
  🔖 Release n8n-workflow@0.132.1
  feat: Add source to all View Plans links (no-changelog) (#5097)
  fix: Do not attempt to save statistics data for unsaved workflows (#5106)
  fix: Update links for user management and SMTP help (#5109)
  fix(editor): Omit `pairedItem` from proxy completions (#5098)
  test: Fix e2e tests for inline expression editor (#5108)
  ...
@janober
Copy link
Member

janober commented Jan 13, 2023

Got released with n8n@0.211.0

@janober janober removed the Upcoming Release Will be part of the upcoming release label Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants