Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Team members query GraphQL resolvers - #47386

Merged
cbart merged 5 commits into
mainfrom
cbart/teams-graphql-members-query
Feb 7, 2023
Merged

Team members query GraphQL resolvers#47386
cbart merged 5 commits into
mainfrom
cbart/teams-graphql-members-query

Conversation

@cbart

@cbart cbart commented Feb 3, 2023

Copy link
Copy Markdown
Contributor

Part of #47043

This pull request adds resolvers for querying team members:

  • team/members(first,after,search)
    • totalCount
      • 🚨 countDeeplyNestedTeamMembers is not supported yet.
    • pageInfo
    • nodes
      • 🚨 user querying is not implemented efficiently to keep pull request size low
      • TODO: Follow up with another pull request that fetches users together with team members.
    • 🚨 search parameter is not tested for to keep the pull request size in check (it is already too long).

The implementation leverages ginger-cake pattern based off teams connection resolver.

  • There is a pending action item in the ticket to unify compute and applyArgs across connections.
  • Perhaps better: leverage graphqlutil.ConnectionResolver?

Test plan

Just one test case with graphQL query for team members' usernames with pagination.

This required however pretty substantial changes in test library:

  • Need to now fake database.UserStore with many users, so implemented a fake for that.

  • Going down that rabbit hole - needed to apply that to faking admin actor/user (I think it is not too bad).

  • Setting up a context for an admin actor looks like this now:

    ctx := userCtx(fakeUsers.newUser(types.User{SiteAdmin: true}))

    I think this reads better than somewhat opaque (see true), and heavily-destructuring:

    ctx, _, _ := fakeUser(t, context.Background(), db, true)
  • While setup now takes advantage of global variables (YOLO), to save from superfluous destructuring.

    Before:

    func TestFoo(t *testing.T) {
      // would require another return value now
      db, ts := setupDB()
    }

    After:

    var (
     db *database.MockDB
     fakeTeams *fakeTeamsDb
     fakeUsers *fakeUsersDB
    )
    //...
    func TestFoo(t *testing.T) {
      setupDB()
    }

@cla-bot cla-bot Bot added the cla-signed label Feb 3, 2023
@cbart
cbart requested a review from a team February 3, 2023 22:36
@cbart
cbart marked this pull request as ready for review February 3, 2023 22:36
@cbart
cbart marked this pull request as draft February 3, 2023 22:36
@cbart
cbart marked this pull request as ready for review February 3, 2023 22:37
@cbart
cbart merged commit ecdd486 into main Feb 7, 2023
@cbart
cbart deleted the cbart/teams-graphql-members-query branch February 7, 2023 14:56
cbart added a commit that referenced this pull request Feb 13, 2023
Part of #47043

## Test plan

This pull request just adds one test for team `members(...) { }` query
using `search` parameter.

This was separated from #47386 to limit the size of the pull request.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants