-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
9dfef70
to
be3c0be
Compare
be3c0be
to
0640e9e
Compare
build() { | ||
// TODO: disable next build attempts | ||
|
||
this.classDirectives.reverse(); | ||
this.fieldDirectives.reverse(); | ||
this.classExtensions.reverse(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is necessary or else the first decorator declared will have priority, e.g.:
@ObjectType
@Extensions({ duplicate: "one" })
@Extensions({ duplicate: "two" })
would end up with the object having duplicate: "one"
, it seems counter intuitive to me and I think this is why it was done in the "directives" feature in the same way. I would have added a code comment but since there was none in the directives PR, I'm not sure it would be appreciated. Will ask the question to the maintainer
@@ -57,3 +59,10 @@ export function ensureReflectMetadataExists() { | |||
throw new ReflectMetadataMissingError(); | |||
} | |||
} | |||
|
|||
export function flattenExtensions( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not such a fan of having a "util" just for this but I didn't see a proper alternative.
I could make it more generic by accepting a key by which we would flatten instead of hardcoding extensions
, but since there is no other use through the codebase now it looked like premature abstraction to me
TODO tomorrow: Add code examples and documentation and then propose the PR to |
5c48a1f
to
9409d3d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1 +/- ##
=========================================
Coverage ? 95.21%
=========================================
Files ? 77
Lines ? 1359
Branches ? 261
=========================================
Hits ? 1294
Misses ? 62
Partials ? 3
Continue to review full report at Codecov.
|
My PR was merged over at type-graphql: MichalLytek#521, so we don't really need this one anymore. I will probably remove the fork as well, we can fork again later if we need |
Should address issue MichalLytek#124 of https://github.com/MichalLytek/type-graphql
Implementation and tests look for the most part correct to me. The idea is to allow adding an arbitrary "extensions" field to any kind of entity in our gql schema.
With this, we would be able to write custom logic to handle authorization / permissions. In a middleware, we could read these specific extensions meta data and perform some checks there.
I need to double/triple check that we have access to this data as I expect it manually against our backend, and I also need to double check if it allows us to implement logic that would e.g. allow us to tag a
Resolver
or anObjectType
with some authorization logic that would apply to all of its child fields / methods