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.

Sign up for GitHub

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

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

New Filter component + implementation in If node (v2)

image

@github-actions
Copy link
Contributor

github-actions bot commented Oct 23, 2023 •
edited

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

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 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
19 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

None yet
5 participants
@elsmr @MiloradFilipovic @janober @michael-radency @gandreini