-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecation warnings with graphql-1.13.1 #53
Comments
👍 up |
Hmm, I'm getting this error now on the latest
Not quite sure what's happening as I didn't think they removed the Downgrading to graphql |
Is there any news on this? I get the same warnings when updating to graphql-ruby 1.13.8 |
I just tried to find a fix for this, but I'm not sure how to get the graphql-guard/lib/graphql/guard.rb Lines 16 to 19 in fb42e72
In sum there are 3 places where As mentioned here https://github.com/rmosolgo/graphql-ruby/blob/master/CHANGELOG.md#deprecations-3
But of course, we can't just remove the call itself, somehow it has to load the masks. Maybe @rmosolgo can help? |
I just discovered, that it's not only a deprecation warning, but an error:
|
module MaskFilter
attr_accessor :mask
end
class Types::BaseObject < GraphQL::Schema::Object
extend MaskFilter
end
# ^^ do the same for other base classes
MASKING_FILTER = ->(schema_member, ctx) do
mask = schema_member.respond_to?(:mask) && schema_member.mask
mask ? mask.call(ctx) : true
end Hope that helps! |
@rmosolgo thanks for the quick answer! It seems, that the changes require more work than I first anticipated to get masking to work. So I wondered, if you can give some advice how to implement it? The current implementation is this: graphql-guard/lib/graphql/guard.rb Lines 57 to 63 in fb42e72
I see some room for improvement here:
What would be a more future proof way of providing a masking feature? I found the visiblity stuff in your docs (see https://graphql-ruby.org/schema/dynamic_types.html#using-visiblecontext-1), but it seems that it's intended to dynamically switch the implementation of a type, right? Regarding "metadata is part of the legacy type definition system": Does this imply, that using graphql-guard/lib/graphql/guard.rb Lines 95 to 100 in fb42e72
Before you could write something like this
I'm not sure how I would achieve something like this using your approach with |
Yes, Besides swapping implementations at runtime, you can also hide implementations at runtime -- and if a type, field, argument, etc is hidden, then it doesn't exist for the current query.
Yes, it was a way to bridge the gap from legacy definition to class-based definition, but it's removed in graphql-ruby 2.0. Honestly, for class-based definitions, I would recommend a method-based approach instead of a proc-based approach. Something like this: module GraphQL::Guard::Integration
def masked?(context)
# library users should implement this method to hide things
false
end
def visible?(context)
(!masked?(context)) && super
end
def guarded?(*args) # for objects, `[obj, ctx]`, but for fields and arguments, `[obj, args, ctx]`
# library users should implement this method to flag things as not authorized
false
end
def authorized?(*args)
(!guarded?(*args)) && super
end
end
class Types::BaseObject < GraphQL::Schema::Object
extend GraphQL::Guard::Integration
def self.guarded?(obj, ctx)
ctx[:current_user].admin?
end
end
# or:
class Types::BaseField < GraphQL::Schema::Field
include GraphQL::Guard::Integration
def guarded?(obj, args, ctx)
ctx[:current_user].admin?
end
end More details about graphql-ruby's Hope that helps! |
Hello @exAspArk, are there any plans for this library to support GraphQL Ruby 1.13 or 2.0? Are you open to PRs to address this issue? |
@rmosolgo thank you, this helps a lot. We are going to need something similar in https://github.com/graphql-devise/graphql_devise and I think it might help for this gem too. It looks like our only way to implement this, will be to provide a custom field class in the gem, and that one should be able to handle what you described in #53 (comment). Is there a way to assign a default |
Yeah, there's not a way to install a field class for every base type in one line of code -- the base Object, Interface, and Mutation classes all need that configuration. I definitely agree that would be nice, but I haven't been able to figure out how it'd work... |
That's fine, thank you! |
Any updates? I started having this problem after updating some old libs 😞 |
Is still repo still active? |
Sorry, I don't have the bandwidth to address it. But if anyone is willing to submit a PR, I'd be happy to review it! |
FWIW to others. My company moved away from this repo recently. GraphQL implemented some hooks for authorization: So we switched over to the base GraphQL gem for authorization. Also note: Rails 6.1.7.8 does not agree with the highest GraphQL version this gem allows. We were getting this error:
|
Hello,
Deprecation warnings starting graphql-1.13.1:
From changelog
Also I noticed, that inline guards return Array wrapped guard Proc and not just proc right away. Is this intended outcome in graphql or some undesired behaviour - I don't know, should investigate more.
The text was updated successfully, but these errors were encountered: