fix(core): Return homeProject when filtering workflows by project id #12077
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
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
n8n-assistant
bot
added
core
r00gm
reviewed
Dec 6, 2024
ivov
approved these changes
Dec 6, 2024
Comment on lines
+69
to
+81
| // Since we're filtering using project ID as part of the relation, | ||
| // we end up filtering out all the other relations, meaning that if | ||
| // it's shared to a project, it won't be able to find the home project. | ||
| // To solve this, we have to get all the relation now, even though | ||
| // we're deleting them later. | ||
| if (typeof options?.filter?.projectId === 'string' && options.filter.projectId !== '') { | ||
| const relations = await this.sharedWorkflowRepository.getAllRelationsForWorkflows( | ||
| workflows.map((c) => c.id), | ||
| ); | ||
| workflows.forEach((c) => { | ||
| c.shared = relations.filter((r) => r.workflowId === c.id); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The facts that we're having to do two queries for this, in-memory processing and fetching things that we later discard makes me wish we could use the query builder, but that'd be a larger refactor. Maybe to improve in future.
n8n
|
||||||||||||||||||||||||||||
| Project |
n8n
|
| Branch Review |
master
|
| Run status |
|
| Run duration | 04m 36s |
| Commit |
|
| Committer | Danny Martini |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
1
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
480
|
| View all changes introduced in this branch ↗︎ | |
|
✅ All Cypress E2E specs passed |
despairblue
deleted the
pay-2271-community-issue-credentials-in-project-personal-are-not-just
branch
Merged
|
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.
Summary
When filtering workflows by project Id we'd not return the home project if the user was not the owner of the workflow. Without that the FE shows incorrect ownerships of WFs:
The reason for that is that that we're filtering the workflows based on
workflow.shared.projectIdwhich will also filter down theworkflow.sharedlist to only contain the sharings matching that projectId:n8n/packages/cli/src/databases/repositories/workflow.repository.ts
Lines 102 to 105 in 146f489
Later we iterate of
workflow.sharedassuming it contains all sharings:n8n/packages/cli/src/services/ownership.service.ts
Lines 82 to 98 in 146f489
In raw SQL and with a query builder this can be fixed using sub=queries, but it's not possible to have this behavior with TypeORM's repository interface.
For credentials this was fixed 3 months ago: #10144
This PR implements the same fix for workflows.
Related Linear tickets, Github issues, and Community forum posts
This does not address what's in the description, but what this comment mentions:
https://linear.app/n8n/issue/PAY-2271/community-issue-credentials-in-project-personal-are-not-just-my#comment-9fe3bc10
Review / Merge checklist
Docs updated or follow-up ticket created.PR Labeled withrelease/backport(if the PR is an urgent fix that needs to be backported)