Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Fix getting permissions for the contributors who are a part of the team#424

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
volodymyrZotov wants to merge4 commits intopeter-evans:main
base:main
Choose a base branch
Loading
fromvolodymyrZotov:main

Conversation

@volodymyrZotov
Copy link

Resolves#396

This PR fixes thegetActorPermission function, so now it returns permissions for the contributors who are a part of the team.

@peter-evans
Copy link
Owner

Hi@volodymyrZotov

Thank you for this PR.

The reason I switched to GraphQL was to support thetriage andmaintain levels.

Assuming this is the correct REST API, it looks liketriage andmaintain permission levels might be supported now. Are you able to confirm that's the case from your testing?
https://docs.github.com/en/rest/collaborators/collaborators?apiVersion=2022-11-28

@volodymyrZotov
Copy link
Author

Documentation foroctokit.rest.repos.getCollaboratorPermissionLevel function says:

The permission attribute provides the legacy base roles of admin, write, read, and none, where the maintain role is mapped to write and the triage role is mapped to read. The role_name attribute provides the name of the assigned role, including custom roles. The permission can also be used to determine which base level of access the collaborator has to the repository.

As far as I understand it for user withtriage role it will returnread and for the user withmaintain role it will returnwrite.

If map it to what we can manually set via UI
Screenshot 2025-09-09 at 5 12 55 PM

That might be fine for it to returnread fortriage role andwrite for maintain role. Unless I'm missing something.

I was able to test it and see that it returnsadmin permission from the team user (myself). I saw that you have tests for this specific function and thought, that you might be able to run them against this branch to double check 😄 It would be difficult for me to test for other permission levels.

@volodymyrZotov
Copy link
Author

As an alternative, maybe we can leave the graphql endpoint and use it as the main method to find permissions, and if it returnsnone it tries to use this rest api one. So it has a fallback

// pseudocodefunc getActorPermission() {    let collaboratorPermission =      await this.octokit.graphql<CollaboratorPermission>(query, {        ...repo,        collaborator: actor      })      if (collaboratorPermission === 'none') {             // user might be a part of the team, try to find his permissions             collaboratorPermission =        await this.octokit.rest.repos.getCollaboratorPermissionLevel({          ...repo,          username: actor        })                  }       return collaboratorPermission}

@peter-evans
Copy link
Owner

I suggested having a fallback here:#120 (comment)

Another possible option, but one that I haven't looked into yet to see if it's feasible, is only using GraphQL if the slash-command-dispatch configuration contains permission levels configured fortriage andmaintain. If those permission levels are not configured then there's no reason to call GraphQL and we can use REST. wdyt?

@volodymyrZotov
Copy link
Author

volodymyrZotov commentedSep 11, 2025
edited
Loading

@peter-evans I found that a response fromoctokit.rest.repos.getCollaboratorPermissionLevel func contains current user and his permissions

{  permission: 'write',  user: {    login: 'TestUser',    id: 1,    .... other props    permissions: {      admin: false,      maintain: true,      push: true,      triage: true,      pull: true    },    role_name: 'maintain'  },  role_name: 'maintain'}

I tested it against individual users with different permission level and also against team with different permission level. And see thatuser.permissions object return proper values according to the permission level that I set. So I decided to refactor it useuser.permissions object instead of top levelpermission property.

Seems we can also userole_name oruser.role_name. But I'd rather go withuser.permissions as the purpose of this object to hold the user permissions.

This work around seems more reliable to me and works well with teams and individual contributors. As well as when user is within the team and also an individual contributor, it returns the highest permission for him correctly. Also covers support formaintain andtriage permissions.

What do you think about this approach?

Screenshot 2025-09-11 at 10 40 41 AM
peter-evans reacted with heart emoji

@peter-evans
Copy link
Owner

Nice find! Looks good to me.

I'm going to be releasing a new major version soon for node 24, so I'll aim to release this at the same time.

volodymyrZotov reacted with thumbs up emoji

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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Command 'ok-to-test' is not configured for the user's permission level 'none' for GitHub Teams

2 participants

@volodymyrZotov@peter-evans

[8]ページ先頭

©2009-2025 Movatter.jp