Skip to content
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

ProjectMembers#add should accept username if user_id is not provided #3665

Closed
2 tasks done
talyssonoc opened this issue Dec 14, 2024 · 1 comment · Fixed by #3666
Closed
2 tasks done

ProjectMembers#add should accept username if user_id is not provided #3665

talyssonoc opened this issue Dec 14, 2024 · 1 comment · Fixed by #3666
Labels
released This issue/pull request has been released.

Comments

@talyssonoc
Copy link
Contributor

Description

  • Node.js version: 22.11.0
  • Gitbeaker version: 41.3.0
  • Gitbeaker release (cli, rest, core, requester-utils): REST
  • OS & version: Manjaro with Linux kernel 6.12

The method ProjectMembers#add should be able to receive the username as an alternative to the user_id since the API allows it.

Steps to reproduce

  • Instantiate Gitlab client from @gitbeaker/rest
  • Try to call client.ProjectMembers.add('repo_id', 'username', AccessLevel.MAINTAINER)

Expected behaviour

It should not cause a type error and should call this API endpoint passing the username as the username field of the API endpoint payload

Actual behaviour

It causes a type error saying 'username' can't be a string, since the method assumes the second parameter is the user_id (source: https://github.com/jdalrymple/gitbeaker/blob/main/packages/core/src/resources/ProjectMembers.ts#L21), which is a number. Even if the type was right, the implementation still passes this attribute as the userId of the API (source: https://github.com/jdalrymple/gitbeaker/blob/main/packages/core/src/templates/ResourceMembers.ts#L67).

Possible fixes

I see three possibilities here:

  • Make the second argument be of the type string | number. If it's a number assume it's the id, and if it's a string assume it's the username. This might be the safest backward-compatible option.
  • Make the second argument an object of the type { username: string } | { userId: number } and send some of them conditionally when calling the API.
  • Make the user_id and username fields part of the options argument instead of being an argument on itself.

Checklist

  • I have checked that this is not a duplicate issue.
  • I have read the documentation.
@jdalrymple
Copy link
Owner

🚀 Issue was released in 42.0.0 🚀

@jdalrymple jdalrymple added the released This issue/pull request has been released. label Dec 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants