jdalrymple / gitbeaker

🦊🧪 A comprehensive and typed Gitlab SDK for Node.js, Browsers, Deno and CLI
Other
1.5k stars 283 forks source link

Add support for Admin endpoint typing #3575

Closed unilynx closed 2 months ago

unilynx commented 2 months ago

Description

I get a TypeScript error when trying to access 'identities'

  const allusers = await gitlab.Users.all({});
  //Property 'find' does not exist on type '{}'
  console.dir(allusers.filter(user => user.identities?.find(_ => _.provider === 'google_oauth2')), { depth: 3 });

but it exists in the output

    identities: [
      {
        provider: 'google_oauth2',
        extern_uid: '1062……'
      },
      {
        provider: 'openid_connect',
        extern_uid: '4598a……'
      }
    ],

Expected behaviour

No errors

Actual behaviour

Property 'find' does not exist on type '{}'

Possible fixes

Add identities to the types as shown here in the examples: https://docs.gitlab.com/ee/api/users.html#for-administrators-1

The actual fields id & provider seem to be documented here: https://docs.gitlab.com/ee/api/users.html#delete-authentication-identity-from-user

Checklist

jdalrymple commented 2 months ago

I believe it is exported, but the wrapper has no idea if youre an admin user or a non admin user when you make the request so the return types include both the extended and non extended versions of a user object.

For now ill return the extended version only, which should solve the typing.

unilynx commented 2 months ago

@jdalrymple thanks for the explanation. are there more of these cases? and is this admin vs non-admin or might even group/project access level affect it? Maybe the GitLab type could take 'isAdmin' as a type parameter to know it's allowed to assume admin interfaces ?

I see GitLab already taking a 'C' type parameter which seems to stand for Camelized

jdalrymple commented 2 months ago

It would probably be much simpler to just do:

import type { ExpandedUserSchema } from '@gitbeaker/rest'

const users: ExpandedUserSchema = await Users.all() as ExpandedUserSchema[]

But yea, in the example you mentioned, passing a generic would also work:

type AsAdmin = boolean;

async function Users.all<AsAdmin extends boolean = false>(): Promise<AsAdmin extends false ? ExpandedUserSchema : UserSchema> {
...
}

const users = await Users.all<AsAdmin>()

BUT that would be poor because its not alone, there are other generics on these functions, which would require you to explicitly set both since they dont have selective generic inferences yet in TS.

Lastly I could add it as an extra option, like the "showExpanded" option (which is probably what you were suggesting haha):

const users = await Users.all({asAdmin: true})

This would work. Ill write something up and put together a PR. I know there are more than just this endpoint that differ based on admin or that are available without authentication, but it would be a bit of work finding each instance. For now ill focus on this endpoint, and i can always add from there.

Good call :rocket: