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

Authorising Scope for non Active Record collections via Elastic Search #3173

Closed
ashikajith opened this issue Sep 30, 2020 · 2 comments
Closed

Comments

@ashikajith
Copy link

Unable to scope records returning from Elastic Search since they are Non Active Record collections

I have a custom connection to fetch a collection and I'm using Elastic Search to retrieve that records. Since it's returning the collection in plain Array, it is not calling the designated Policy class and because of this, I'm unable to scope the collection.

Is there any workaround in which I can apply the scope for the collection which is Non AR? I'm using pundit for authorization link

eg:

# Query Type
module Types
  class QueryType < ::GraphQL::Schema::Object
    field :records_by_category_search, Types::Connections::RecordByCategorySearchConnection, null: false do
      argument :filter, String, "The filter string", required: false
    end

    def records_by_category_search(**args)
       Searcher::Elasticsearch.new(args).fetch_records
    end
  end
end

# Connection
class Types::Connections::RecordSearchConnection < Types::BaseConnection
  edge_type(Types::Edges::RecordSearchEdge) 
  field :filter_options, Types::Objects::FilterType, null: true

  def filter_options
    some_method
  end
end

# Node
class Types::Edges::RecordByCategoryEdge < Types::BaseEdge
  node_type(Types::Objects::RecordByCategoryType)
end

# Object Type
module Types
  module Objects
    class RecordByCategoryType < BaseObject
      field :category, Types::Objects::CategoryType, null: true
      field :records, [Types::Objects::RecordType], null: true # -> Unable to add scope inside RecordPolicy class
    end
  end
end

Versions

graphql version: 1.10.7
rails: 5.2.4
graphql-pro: 1.11.0

Hope the code example is helpful
Thanks in Advance

@rmosolgo
Copy link
Owner

rmosolgo commented Oct 2, 2020

Hey, thanks for the detailed writeup. I can think of two basic approaches:

  1. After returning an array, use Pundit to scope the array. You'll have to do two things to do this:

    • Add a pundit_policy_class: ... to the field, because otherwise, Pundit will misinterpret the array of items as a "namespaced object" ([pro] Pundit integration fails with plain Array field #2008).
    • Inside RecordType.scope, call scope_by_pundit_policy(context, items) (which the pundit integration usually skips with arrays, because of the misinterpretation described above):
    class Types::Objects::RecordType < ... 
      def self.scope(context, items)
        if items.is_a?(Array)
          # Usually, the integration skips this, but we explicitly need it here for elastic search results:
          scope_by_pundit_policy(context, items)
        else 
          super 
        end 
      end 
    end 
  2. Instead of returning the array of results, you could return the Searcher::Elasticsearch instance. In that case, the returned object would be passed as items in def self.scope, and so the pundit integration would look up a policy and call its scope. After that, you'd have to update the scope to perform filtering and call .fetch_records. One advantage here is that, if your search engine supports it, you could add filters to the query before calling Elastic Search. That way, you wouldn't have to load the full result set into memory before filtering it; Elastic Search would probably be a more efficient filterer than Ruby! But it would depend on the specifics of the search setup.

I hope one of those helps! If you run into trouble, let me know which approach you tried and what code you used for it, and what issue or error you encountered when you tried it.

@ashikajith
Copy link
Author

Sorry for the delay in response and thanks for the detailed suggestions. I think we can go for Solution # 2 since scoping from the array side (Solution # 1) might leads to a mismatch in the count of records and the content we display on the page.
I'll post my findings here after making it work and let you know If I had any issue in implementation.

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

No branches or pull requests

2 participants