-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
feat(core): Add workflowId filter to the test-definitions endpoint (no-changelog) #11831
feat(core): Add workflowId filter to the test-definitions endpoint (no-changelog) #11831
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one minor comment but feel free to merge without it
if (!accessibleWorkflowIds.includes(options.filter.workflowId as string)) { | ||
throw new ForbiddenError('User does not have access to the workflow'); | ||
} | ||
|
||
where.workflow = { | ||
id: options.filter.workflowId as string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we improve the type of options here instead of coercing it to string 2x?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be great to do for all places using filter middleware. I will check it later
|
n8n Run #8082
Run Properties:
|
Project |
n8n
|
Branch Review |
ai-456-api-add-option-to-filter-test-definitions-by-a-specific
|
Run status |
Failed #8082
|
Run duration | 38m 38s |
Commit |
4057ed4c14: 🌳 🖥️ browsers:node18.12.0-chrome107 🤖 burivuhster 🗃️ e2e/*
|
Committer | Eugene Molodkin |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
1
|
Flaky |
1
|
Pending |
0
|
Skipped |
0
|
Passing |
280
|
View all changes introduced in this branch ↗︎ |
Tests for review
45-workflow-selector-parameter.cy.ts • 1 failed test
Test | Artifacts | |
---|---|---|
Workflow Selector Parameter > should render add resource option and redirect to the correct route when clicked |
Test Replay
Screenshots
Video
|
The first 5 failed specs are shown, see all 49 specs in Cypress Cloud.
e2e/30-langchain.cy.ts • 1 flaky test
Test | Artifacts | |
---|---|---|
Langchain Integration > should be able to open and execute Agent node |
Test Replay
Screenshots
Video
|
|
4057ed4
to
0b94403
Compare
0b94403
to
73df6de
Compare
Got released with |
Summary
This PR adds filter by
workflowId
to the/test-definitions
list endpoint.Related Linear tickets, Github issues, and Community forum posts
Review / Merge checklist
release/backport
(if the PR is an urgent fix that needs to be backported)