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

[volodymyrZotov]

Resolves #396

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

[peter-evans]

Hi @volodymyrZotov

Thank you for this PR.

The reason I switched to GraphQL was to support the triage and maintain levels.

• Support triage and maintain permission levels #74

Assuming this is the correct REST API, it looks like triage and maintain 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]

Documentation for octokit.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 with triage role it will return read and for the user with maintain role it will return write.

If map it to what we can manually set via UI

That might be fine for it to return read for triage role and write for maintain role. Unless I'm missing something.

I was able to test it and see that it returns admin 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]

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

// pseudocode

func 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]

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 for triage and maintain. If those permission levels are not configured then there's no reason to call GraphQL and we can use REST. wdyt?

[volodymyrZotov]

@peter-evans I found that a response from octokit.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 that user.permissions object return proper values according to the permission level that I set. So I decided to refactor it use user.permissions object instead of top level permission property.

Seems we can also use role_name or user.role_name. But I'd rather go with user.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 for maintain and triage permissions.

What do you think about this approach?