Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

feat(editor): Filter component + implement in If node #7490

Merged
merged 49 commits into from
Dec 13, 2023
Merged

feat(editor): Filter component + implement in If node #7490

merged 49 commits into from
Dec 13, 2023

Conversation

elsmr
Copy link
Member

@elsmr elsmr commented Oct 23, 2023 •
edited
Loading

New Filter component + implementation in If node (v2)

image

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2023 •
edited
Loading

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

  • If fixing bug, added test to cover scenario.
  • If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

  • Added unit tests to cover new or updated functionality.

Files matching **/*.vue:

  • Used composition API for all new components.
  • Added component or unit tests to cover functionality.

Files matching packages/editor-ui/**/*.vue:

  • Added E2E if adding new features.
  • Used design system tokens (colors, spacings...) where possible.

Files matching packages/nodes-base/nodes/**:

  • Added workflow tests for nodes if possible.

Files matching packages/design-system/**/*.vue:

  • Used design system tokens (colors, spacings...) where possible.
  • Updated Storybook with new component or updated functionality.

Files matching cypress/e2e/**:

  • Avoided chaining commands more than two or three times (to avoid flakiness because only last one will be retried).
  • Spoofed endpoints that are not critical for the test (to avoid flakiness).
  • Picked most efficient path to start the test (for example skipped account setup and starting at /workflow/new for a canvas test).
  • Avoided adding waits on time (use request intercepts instead).
  • Ensured each spec does not depend on any another spec to pass.

Make sure to check off this list before asking for review.

@elsmr elsmr marked this pull request as draft October 23, 2023 08:12
@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team node/improvement New feature or request node/new Creation of an entirely new node ui Enhancement in /editor-ui or /design-system labels Oct 23, 2023
@cypress
Copy link

cypress bot commented Oct 27, 2023 •
edited
Loading

1 flaky test on run #3326 ↗︎

0 296 5 0 Flakiness 1

Details:

🌳 🖥️ browsers:node18.12.0-chrome107 🤖 elsmr 🗃️ e2e/*
Project: n8n Commit: 5875ecdd8e
Status: Passed Duration: 06:41 💡
Started: Dec 13, 2023 12:27 PM Ended: Dec 13, 2023 12:33 PM
Flakiness  cypress/e2e/24-ndv-paired-item.cy.ts • 1 flaky test

View Output Video

Test Artifacts
NDV > resolves expression with default item when input node is not parent, while still pairing items Screenshots Video

Review all test suite changes for PR #7490 ↗︎

@elsmr elsmr marked this pull request as ready for review October 30, 2023 08:52
@MiloradFilipovic MiloradFilipovic self-requested a review December 8, 2023 15:23
MiloradFilipovic
MiloradFilipovic previously approved these changes Dec 8, 2023
View reviewed changes
Copy link
Contributor

@MiloradFilipovic MiloradFilipovic left a comment

Choose a reason for hiding this comment

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

Nice one 🥇 It was a pleasure going through this codebase, good job. I left a few comments regarding some stuff I found while working with the component but, from the code review standpoint, all is good on my side.

packages/editor-ui/src/components/ParameterInputFull.vue Show resolved Hide resolved
packages/workflow/src/Interfaces.ts Show resolved Hide resolved
@MiloradFilipovic
Copy link
Contributor

I am getting this error when opening NDV for the new If node:
image

@MiloradFilipovic
Copy link
Contributor

Also opening NDV for the new If node seems to be much slower (it hangs for a couple of seconds sometimes) compared to other nodes in a relatively simple setup.

@MiloradFilipovic
Copy link
Contributor

I was expecting this to resolve to true:
image

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 8, 2023

✅ All Cypress E2E specs passed

@elsmr
Copy link
Member Author

elsmr commented Dec 11, 2023

I am getting this error when opening NDV for the new If node: image

Fixed 😄

Also opening NDV for the new If node seems to be much slower (it hangs for a couple of seconds sometimes) compared to other nodes in a relatively simple setup.

Also noticed this, I improved it in my latest commit by not rendering all the nested selects eagerly (that was taking the most time)

I was expecting this to resolve to true:

Interesting, the way the operator is meant to work now is the right side value should exactly be part of the left side array. (Right side should be 3 for example here).

@github-actions GitHub Actions
Copy link
Contributor

✅ All Cypress E2E specs passed

@elsmr elsmr merged commit 8a53434 into master Dec 13, 2023
16 checks passed
@elsmr elsmr deleted the node-785-filter-component branch December 13, 2023 13:45
This was referenced Dec 13, 2023
Closed
Merged
ivov added a commit that referenced this pull request Dec 13, 2023
@github-actions @ivov
🚀 Release 1.21.0 (#8019)
1516c84 
#
[1.21.0](https://github.com/n8n-io/n8n/compare/n8n@1.20.0...n8n@1.21.0)
(2023-12-13)


### Bug Fixes

* **core:** Ensure inviter and invitee are set correctly in invite link
([#7943](#7943))
([386bd61](386bd61))
* **core:** Fix user comparison in same-user subworkflow caller policy
([#7913](#7913))
([92bab72](92bab72))
* **core:** Perform multi-main leader check against key ID
([#7964](#7964))
([1a87f70](1a87f70))
* **core:** Ensure external hooks post workflow execute run in queue
mode ([#7947](#7947))
([3ba7deb](3ba7deb))
* **core:** Fix issue preventing secrets from loading if the path
contains - or / ([#7988](#7988))
([0ac9594](0ac9594))
* **core:** Restrict updating/deleting of shared but not owned
credentials ([#7950](#7950))
([42e828d](42e828d))
* **core:** Prevent workflow history saving error from happening
([#7812](#7812))
([e5581ce](e5581ce))
* **editor:** Add missing string for worker in log streaming
([#7971](#7971))
([148bc1d](148bc1d))
* **editor:** Allow SSH protocol in git repository URL for environments
([#7944](#7944))
([bc1c72f](bc1c72f))
* **editor:** Fix bug with node names with certain characters
([#8013](#8013))
([26f0d57](26f0d57))
* **editor:** Fix Webhook URL expansion icon
([#8011](#8011))
([b00b905](b00b905))
* **editor:** Prevent opening NDV search if `/` is typed in a
contenteditable element
([#7968](#7968))
([e8a493f](e8a493f))
* **editor:** Return early in ws message handler if no 'command' keyword
is found ([#7946](#7946))
([5b2defc](5b2defc))
* **FileMaker Node:** Prevent erroring on zero fields loaded
([#7955](#7955))
([10ad386](10ad386))
* **Google Sheets Node:** Prevent erroring on zero sheet search results
([#7957](#7957))
([9b877a9](9b877a9))
* **Google Sheets Node:** Prevent erroring when fetching mapping columns
([#7972](#7972))
([29a1066](29a1066))
* **Postgres Node:** Do not include id column in upsert fields selection
if it's not unique ([#7975](#7975))
([435392c](435392c))
* **Postgres Trigger Node:** Increase manual trigger timeout from 30 to
60 seconds ([#8015](#8015))
([09a5729](09a5729))
* **Webhook Node:** Binary data handling
([#7804](#7804))
([565b409](565b409))
* **Webhook Node:** Do not create binary data when there is no data in
the request ([#8000](#8000))
([70f0755](70f0755))


### Features

* **core:** Add config option for external secret update interval
([#7995](#7995))
([b6c1c04](b6c1c04))
* AI nodes usability fixes + Summarization Chain V2
([#7949](#7949))
([dcf1286](dcf1286))
* **editor:** Data transformation nodes and actions in Nodes Panel
([#7760](#7760))
([675ec21](675ec21))
* **editor:** Add AppCues tracking for onboarding event
([#7945](#7945))
([04cabaf](04cabaf))
* **editor:** Add option to disable NDV in workflow previews
([#7990](#7990))
([393afef](393afef))
* **editor:** Filter component + implement in If node
([#7490](#7490))
([8a53434](8a53434))
* **editor:** Show template credential setup based on feature flag
([#7989](#7989))
([08ee307](08ee307))
* **editor:** Introduce advanced permissions
([#7844](#7844))
([dbd62a4](dbd62a4))
* **Google Ads Node:** Update to support v15
([#7962](#7962))
([7f01269](7f01269))
* **Local File Trigger Node:** Add polling option typically good to
watch network files/folders
([#7942](#7942))
([2fbdfec](2fbdfec))
* **n8n Form Trigger Node:** Improvements
([#7571](#7571))
([953a58f](953a58f))

Co-authored-by: ivov <ivov@users.noreply.github.com>
@janober
Copy link
Member

janober commented Dec 13, 2023

Got released with n8n@1.21.0

MiloradFilipovic added a commit that referenced this pull request Dec 14, 2023
@MiloradFilipovic
Merge branch 'master' into ADO-1546-lead-enrichment-components
02ea2cd 
* master:
  fix(editor): Turn off executions list auto-refresh after leaving the page (#8005)
  fix(editor): Show credential share info only to appropriate users (#8020)
  🚀 Release 1.21.0 (#8019)
  fix(ActiveCampaign Node): Fix pagination issue when loading tags (#8017)
  feat(n8n Form Trigger Node): Improvements (#7571)
  fix(editor): Fix bug with node names with certain characters (#8013)
  feat(editor): Filter component + implement in If node (#7490)
  fix(Postgres Trigger Node): Increase manual trigger timeout from 30 to 60 seconds (#8015)
  fix(core): Prevent workflow history saving error from happening (#7812)
  fix(editor): Fix Webhook URL expansion icon (#8011)
  docs: Add aliases to compare datasets node (#8010)
  refactor(core): Add telemetry for RBAC roles (#7969)
  docs(editor): Change to Summarize icon and Advanced section description (no-changelog) (#8008)
  refactor(core): Introduce import service (no-changelog) (#8001)
  fix(Webhook Node): Do not create binary data when there is no data in the request (#8000)
  🚀 Release 1.20.0 (#7940)
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@michael-radency michael-radency michael-radency approved these changes

@MiloradFilipovic MiloradFilipovic MiloradFilipovic left review comments

Assignees

@elsmr elsmr

Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team node/improvement New feature or request node/new Creation of an entirely new node Released ui Enhancement in /editor-ui or /design-system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@elsmr @MiloradFilipovic @janober @michael-radency @gandreini