search

serlo / api.serlo.org

Public GraphQL API of https://serlo.org/
https://api.serlo.org/___graphql
Apache License 2.0
15 stars 4 forks source link

Rename context variable user to userId #182

Closed kulla closed 3 years ago

kulla commented 3 years ago

At https://github.com/serlo/api.serlo.org/blob/master/src/internals/graphql/context.ts#L25-L31 we defined our context. However since user is number | null it would be better to rename it to userId. With yarn lint you see which variables also need to be renamed.

inyono commented 3 years ago

The refactoring tools of your editor / IDE of choice should already automate most of this (e.g. WebStorm, VS Code, not sure about others). So this issue is also a good way to get to know the refactoring tools or your editor / IDE.

elbotho commented 3 years ago

Very interesting! Rename Symbol in VSCode was quite nice, only stumbled on these constructions:

context({ req }): Promise<Pick<Context, 'service' | 'userId'>>

could you please check if that is the intended outcome or if it was to eager? 😄 https://github.com/serlo/api.serlo.org/pull/187

kulla commented 3 years ago

could you please check if that is the intended outcome or if it was to eager?

:+1: (It picks the service and userId property of the interface Context

elbotho commented 3 years ago

👍 (It picks the service and userId property of the interface Context

sorry I don't understand what you mean here

kulla commented 3 years ago

sorry I don't understand what you mean here

I wanted to explain the Pick<A, B> utility type: https://www.typescriptlang.org/docs/handbook/utility-types.html#picktype-keys

elbotho commented 3 years ago

Ah, now I get it, thanks!