refactor(core): Convert dynamic node-parameter routes to a decorated controller (no-changelog) #7284
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
n8n-assistant
bot
added
core
This comment was marked as outdated.
This comment was marked as outdated.
netroy
force-pushed
the
refactor-node-details-endpoints
branch
from
c6c0af4
to
eb9d0af
Compare
netroy
force-pushed
the
refactor-node-details-endpoints
branch
2 times, most recently
from
93db1ad
to
380b6f8
Compare
netroy
force-pushed
the
refactor-node-details-endpoints
branch
from
380b6f8
to
39b865e
Compare
netroy
force-pushed
the
refactor-node-details-endpoints
branch
from
39b865e
to
90f13f5
Compare
netroy
changed the title
netroy
force-pushed
the
refactor-node-details-endpoints
branch
from
90f13f5
to
81ac215
Compare
ivov
reviewed
Oct 27, 2023
There was a problem hiding this comment.
Getting this using a valid Postgres credential. Same for Google Drive.
| export class DynamicNodeParametersController { | ||
| constructor(private readonly service: DynamicNodeParametersService) {} | ||
|
|
||
| @Middleware() |
There was a problem hiding this comment.
Not to change now, just to consider - it might be more consistent to have @RestController('/dynamic-node-parameters', { middlewares: [fn] }) for controller-level middlewares, similar to endpoint-level middlewares.
There was a problem hiding this comment.
I think that's how we started with, but then the middleware functions could not use the injected dependencies, as the method being passed needed to exist before the class was instantiated.
packages/cli/src/controllers/dynamicNodeParameters.controller.ts
Outdated
Show resolved
Hide resolved
netroy
force-pushed
the
refactor-node-details-endpoints
branch
from
81ac215
to
029ea8d
Compare
2 flaky tests on run #2938 ↗︎
Details:
|
||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| Resource Mapper > should correctly delete single field |
Screenshots
Video
|
|
| Resource Mapper > should correctly delete all fields |
Screenshots
Video
|
|
Review all test suite changes for PR #7284 ↗︎
netroy
changed the title
netroy
force-pushed
the
refactor-node-details-endpoints
branch
2 times, most recently
from
de0f016
to
43632db
Compare
netroy
force-pushed
the
refactor-node-details-endpoints
branch
2 times, most recently
from
6bf0c40
to
6971d9e
Compare
|
Retested on GDrive and PG and confirmed working + code looks good. I'll approve after the conflict is resolved. |
…controller (no-changelog)
netroy
force-pushed
the
refactor-node-details-endpoints
branch
from
6971d9e
to
6070044
Compare
|
rebased |
ivov
previously approved these changes
Nov 17, 2023
|
|
|
This test was added after the PR was started, and I didn't catch it. |
ivov
approved these changes
Nov 17, 2023
|
✅ All Cypress E2E specs passed |
MiloradFilipovic
added a commit
that referenced
this pull request
Nov 20, 2023
* master: (27 commits) fix: Include cypress TypeScript types in /cypress folder (no-changelog) (#7746) refactor(core): Stop reporting to Sentry `NodeApiError` outside 500 range (no-changelog) (#7753) fix(core): Guard against node not found on cancelling test webhook (#7750) fix(JotForm Trigger Node): Fix iteration on form loader (#7751) refactor(core): Stop reporting to Sentry unknown cred on mapping (no-changelog) (#7752) feat(core): Coordinate manual workflow activation and deactivation in multi-main scenario (#7643) ci: Fix "Release: Create Pull Request" workflow fix(editor): Fix Admin panel icon in the sidebar for cloud deployments (no-changelog) (#7738) fix(editor): Remove `n8nHooksNext` flag (no-changelog) (#7733) fix(editor): Show v1 banner dismiss button if owner (#7722) fix(GitHub Node): Fix issue preventing file edits on branches (#7734) fix(core): Fix all dependency versions for backend packages (no-changelog) (#7745) refactor(core): Convert dynamic node-parameter routes to a decorated controller (no-changelog) (#7284) refactor: Stop reporting to Sentry Facebook multi-webhook error (no-changelog) (#7743) refactor(core): Stop reporting to Sentry unrecognized node errors (no-changelog) (#7728) fix(core): Account for non-ASCII chars in filename on binary data download (#7742) ci: Fix DB tests and Workflow tests (no-changelog) (#7741) refactor: Extract Invitation routes to InvitationController (no-changelog) (#7726) fix(editor): Handle permission edge cases (empty scopes) (#7723) ci: Skip the regularly failing tests in 2-credentials.cy.ts (no-changelog) (#7736) ...
|
Got released with |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Server.ts