Skip to content
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

[pro] Pundit integration fails with plain Array field #2008

Closed
cainlevy opened this issue Dec 13, 2018 · 4 comments
Closed

[pro] Pundit integration fails with plain Array field #2008

cainlevy opened this issue Dec 13, 2018 · 4 comments

Comments

@cainlevy
Copy link

@cainlevy cainlevy commented Dec 13, 2018

Context

Every mutation in my GraphQL API may optionally return a list of validation errors (as data). This is typed as:

field :errors, [ServiceError], null: true

Now I'm adding Pundit integration, and my tests fail with:

GraphQL::Pro::PunditIntegration::PolicyNotFoundError: No policy found for:

- Type: ServiceError
- Runtime value: `[#<struct ServiceError field="description", code=:blank>, #<struct ServiceError field="name", code=:too_long>]`

Pundit error: Pundit::NotDefinedError, unable to find scope `ServiceError::ServiceErrorPolicy::Scope` for `[#<struct ServiceError field="description", code=:blank>, #<struct ServiceError field="name", code=:too_long>]`

Bug

The problem is that Pundit::PolicyFinder expects an Array to be a list of constant names, not a list of values. Thus, any attempt to return more than one ServiceError will fail the policy lookup.

This logic is not problematic for Relations, because a Relation is not an Array.

Example

irb(main):001:0> Pundit::PolicyFinder.new(nil).send(:find, [ServiceError.new])
=> "::ServiceErrorPolicy"
irb(main):002:0> Pundit::PolicyFinder.new(nil).send(:find, [ServiceError.new, ServiceError.new])
=> "ServiceError::ServiceErrorPolicy"
irb(main):003:0> Pundit::PolicyFinder.new(nil).send(:find, [ServiceError.new, ServiceError.new, ServiceError.new])
=> "ServiceError::ServiceError::ServiceErrorPolicy"

Suggestion

Disable scoping logic for plain arrays. It's a conceptual mismatch.

Workarounds

  • scope: false on everything
  • monkey patch or override ScopeByPolicy logic
@rmosolgo
Copy link
Owner

@rmosolgo rmosolgo commented Dec 14, 2018

Disable scoping logic for plain arrays.

👌 Thanks for the bug report, I'll take a look and follow up here.

@aarons22
Copy link

@aarons22 aarons22 commented Jan 12, 2019

Any progress/update on this? We're currently blocked on a few fronts because of this bug. Thanks 😄

@cainlevy
Copy link
Author

@cainlevy cainlevy commented Jan 12, 2019

My workaround:

module Types
  class BaseObject < GraphQL::Schema::Object
    # override
    # see: https://github.com/rmosolgo/graphql-ruby/issues/2008
    def self.scope_by_policy(items, context)
      items.is_a?(Array) ? items : super
    end
  end
end
@rmosolgo
Copy link
Owner

@rmosolgo rmosolgo commented Jan 14, 2019

Thanks for the bump here, this slipped off my radar over the holidays 😊 I just pushed graphql-pro 1.9.5 which includes the same kind of solution that @cainlevy posted. Thanks for sharing your workaround and let me know if you run into any trouble with that new version!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants