feat(API): Add user management endpoints to the Projects Public API #12329
Conversation
packages/cli/src/public-api/v1/handlers/projects/spec/paths/projects.projectId.yml
Show resolved
Hide resolved
n8n-assistant
bot
added
core
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
MarcL
changed the title
…-delete-users-from-project
netroy
force-pushed
the
pay-1852-public-api-delete-users-from-project
branch
from
3c650ef to
e9d96f5
Compare
…-delete-users-from-project
…-delete-users-from-project
netroy
changed the title
… personal projects with the public API
| @@ -76,8 +81,8 @@ export class ProjectService { | |||
| } | |||
|
|
|||
| const project = await this.getProjectWithScope(user, projectId, ['project:delete']); | |||
| if (!project) { | |||
| throw new NotFoundError(`Could not find project with ID: ${projectId}`); | |||
There was a problem hiding this comment.
I re-added the project ID. That should make it easier to debug this when a user reports this.
| // 0. check if this is a team project | ||
| if (project.type !== 'team') { | ||
| throw new ForbiddenError( | ||
| `Can't delete project. Project with ID "${projectId}" is not a team project.`, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
I'd prefer to send back a ForbiddenError instead of a NotFound. The user has access to the project, they are just not allowed to delete it.
There was a problem hiding this comment.
I'm wondering if the public API DTOs should be in it's own folder?
There was a problem hiding this comment.
And if the public API should define their own DTOs. We don't want to give the public API more power by accident when we add more power to the internal API.
| @@ -20,7 +20,7 @@ export const projectRoleSchema = z.enum([ | |||
| export type ProjectRole = z.infer<typeof projectRoleSchema>; | |||
|
|
|||
| export const projectRelationSchema = z.object({ | |||
| userId: z.string(), | |||
| userId: z.string().min(1), | |||
There was a problem hiding this comment.
question (non-blocking): Why the min(1)?
| userId: z.string(), | ||
| role: projectRoleSchema, | ||
| userId: z.string().min(1), | ||
| role: projectRoleSchema.exclude(['project:personalOwner']), |
There was a problem hiding this comment.
I've excluded the project:personalOwner here too. Personal projects are special, they can only have one owner with that role. There should not be any API endpoint that allows changing the relation. They are created when the user is created and deleted when the user is deleted.
| @@ -211,28 +224,80 @@ export class ProjectService { | |||
|
|
|||
| async syncProjectRelations( | |||
There was a problem hiding this comment.
TODO: add a test to make sure this throws when used with personal projects or the personalOwner role.
| throw new ProjectNotFoundError(projectId); | ||
| } | ||
|
|
||
| // TODO: do we need to prevent project owner from being removed? |
There was a problem hiding this comment.
We don't, as long as this function only works with team projects.
TODO: add test to make sure this function throws when used on a personal project.
| throw new ProjectNotFoundError(projectId); | ||
| } | ||
|
|
||
| // TODO: do we need to block any specific roles here? |
There was a problem hiding this comment.
Yeah, this function also should only work with team projects and should disallow the personalOwner role.
TODO: add test for this
Summary
Adds the ability to add and delete users to and from projects using the public API endpoint:
Also allow the ability to alter user roles in projects.
Note: There were problems getting the integration test server to work when I referenced the
projectId.ymlschema within the OpenAPI spec rather than inlined it. As we'll be looking to improve the public API at some point in the future, I've moved all the references to inline instead.Related Linear tickets, Github issues, and Community forum posts
Partially fixes: PAY-1852
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)