feat(core): Scope getStatus for environments for project admin role #15404

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

Merged

afitzek merged 14 commits into master from pay-2822-adapt-get-status

May 22, 2025

Contributor

afitzek commented May 15, 2025 •

edited by guillaumejacquart

Loading

Summary

Changes the behavior of the getStatus endpoint to scope available changes in environments to only resources
project admins have access to.

Related Linear tickets, Github issues, and Community forum posts

closes https://linear.app/n8n/issue/PAY-2822/adapt-getstatus-endpoint-to-support-scoping-to-project-admins

Review / Merge checklist

PR title and summary are descriptive. (conventions)
Docs updated or follow-up ticket created.
Tests included.
PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

codecov bot commented May 15, 2025 •

edited

Loading

Codecov Report

Attention: Patch coverage is 95.00000% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ee/source-control/source-control-scoped.service.ts	91.17%	2 Missing and 1 partial ⚠️
...nts.ee/source-control/source-control.service.ee.ts	92.59%	1 Missing and 1 partial ⚠️
...source-control/source-control-import.service.ee.ts	98.07%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

n8n-assistant bot added core n8n team labels

afitzek force-pushed the pay-2822-adapt-get-status branch 4 times, most recently from 328652c to e45091b Compare

May 20, 2025 09:46

afitzek marked this pull request as ready for review

May 20, 2025 10:35

afitzek requested a review from guillaumejacquart

May 20, 2025 10:35

cubic-dev-ai bot reviewed

View reviewed changes

cubic-dev-ai bot left a comment

cubic reviewed 10 files and found no issues. Review PR in cubic.dev.

guillaumejacquart mentioned this pull request

feat(editor): Enable source environment push button for project admins #15527

Open

4 tasks

guillaumejacquart reviewed

View reviewed changes

Contributor

guillaumejacquart left a comment

Some comments

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated

+              		});
+              	}
+              	private getProjectFilter(context: SourceControlContext): FindOptionsWhere<Project> | undefined {

Contributor

guillaumejacquart May 20, 2025

I think it's a bit redundant to re-check context.accessToAllProjects() here, because getProjectFilter is called from functions which already do that check.
Since for now this method is not used elsewhere, what would you say if we called it something more explicit like getAdminProjectsByUserIdFilter(userId: string) ?
also I'm wondering if all those typeorm related method should maybe be put in a repository instead, what do you think ? (either entity repos or a dedicated source control repo 🤔 )

Contributor Author

afitzek May 20, 2025

Good point, I moved these functions into a SourceControlScopedService class.

Contributor

guillaumejacquart May 20, 2025

I think you will have a list issue that requires all files that import typeorm stuff to be inside the database folder or something

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/types/source-control-context.ts Outdated Show resolved Hide resolved

guillaumejacquart reviewed

View reviewed changes

Contributor

guillaumejacquart left a comment

Some additional comments

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated

+              		});
+              	}
+              	private getProjectFilter(context: SourceControlContext): FindOptionsWhere<Project> | undefined {

Contributor

guillaumejacquart May 20, 2025

I think you will have a list issue that requires all files that import typeorm stuff to be inside the database folder or something

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control-import.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control.service.ee.ts Outdated Show resolved Hide resolved

packages/cli/src/environments.ee/source-control/source-control.service.ee.ts Outdated Show resolved Hide resolved

afitzek added 13 commits

May 22, 2025 09:04


          Adapt source control service to scope select resources for project ad…

d9d8830

…mins


          added filters for more resources for source control import service

e6bff2a


          work in progress

c41e734


          Finish tests for scoped resources

c2c1c10


          fix compiling errors

6c7315a


          Extensive testing for pushing for project admin role

e77e6bc


          Include special case for workflow beeing moved out of scope.

1085c46


          Adapt initial check

0f7e8df


          Refactor based on review

89501d9


          Fix typecheck

af7df89


          Fix linting rule

e09d047


          Moved existing filter into filter operation

e113f9b


          address comments

e1d8690

afitzek force-pushed the pay-2822-adapt-get-status branch from d917126 to e1d8690 Compare

May 22, 2025 07:16


          fix lint

d3ea6d2

afitzek requested a review from guillaumejacquart

May 22, 2025 07:36

guillaumejacquart approved these changes

View reviewed changes

Contributor

guillaumejacquart left a comment

Looks good to me. Well done on the tests 👍
I think we should create a tech debt ticket to start splitting the souce-control service into multiple services though

Contributor

shortstacked commented May 22, 2025

Workflow Test Results 📊 🔴 2 Failed, ⚠️ 3 Warnings, 👍 78 Successful out of 83 total workflows.

Detail: Workflows failing: 243: Workflow contains 2 deleted data. View full workflow run

Tested Ref: d3ea6d2949f099d01adba901c1d035cbbfd22d60 by @guillaumejacquart

❌ Failed Tests (2)

Workflow ID	Workflow Name	Reason
243	Agent:ReAct	Workflow contains 2 deleted data.
258	Agent:auto-fix:openai	Workflow contains 2 deleted data.

⚠️ Warnings (3)

Workflow ID	Workflow Name	Reason
237	BasicLLMChain:AzureChat	Workflow contains new data that previously did not exist.
35	Slack:User:getPresence info:UserProfile:get update...	Workflow contains new data that previously did not exist.
257	Agent:auto-fix:anthropic	Workflow contains new data that previously did not exist.

Contributor

github-actions bot commented May 22, 2025

⚠️ Some Cypress E2E specs are failing, please fix them before merging

2 similar comments

Contributor

github-actions bot commented May 22, 2025

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Contributor

github-actions bot commented May 22, 2025

⚠️ Some Cypress E2E specs are failing, please fix them before merging

Contributor

github-actions bot commented May 22, 2025

✅ All Cypress E2E specs passed

afitzek merged commit f9f9597 into master

42 checks passed

afitzek deleted the pay-2822-adapt-get-status branch

May 22, 2025 11:49

ozcangungor pushed a commit to ozcangungor/n8n that referenced this pull request


          feat(core): Scope getStatus for environments for project admin role (n…

2a43bbe

…8n-io#15404)

etobella pushed a commit to etobella/n8n that referenced this pull request


          feat(core): Scope getStatus for environments for project admin role (n…

1d4a0ea

…8n-io#15404)

github-actions bot mentioned this pull request

🚀 Release 1.95.0 #15694

Merged

Member

janober commented May 26, 2025

Got released with [email protected]

janober added the Released label

Alexandero89 pushed a commit to Alexandero89/n8n that referenced this pull request


          feat(core): Scope getStatus for environments for project admin role (n…

b9a03ca

…8n-io#15404)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core n8n team Released