Use records for joined resources for 0.10 (#1381)

Add tracking of join options for jsonapi-authorization gem
Merge related `records` in `apply_join`
Add `use_related_resource_records_for_joins` to configuration and relationships

Author: lgebhardt

Diff for: lib/jsonapi/active_relation/join_manager.rb

@@ -147,9 +147,15 @@ def perform_joins(records, options)
             related_resource_klass = join_details[:related_resource_klass]
             join_type = relationship_details[:join_type]
 
+            join_options = {
+              relationship: relationship,
+              relationship_details: relationship_details,
+              related_resource_klass: related_resource_klass,
+            }
+
             if relationship == :root
               unless source_relationship
-                add_join_details('', {alias: resource_klass._table_name, join_type: :root})
+                add_join_details('', {alias: resource_klass._table_name, join_type: :root, join_options: join_options})
               end
               next
             end
@@ -163,7 +169,7 @@ def perform_joins(records, options)
                 options: options)
             }
 
-            details = {alias: self.class.alias_from_arel_node(join_node), join_type: join_type}
+            details = {alias: self.class.alias_from_arel_node(join_node), join_type: join_type, join_options: join_options}
 
             if relationship == source_relationship
               if relationship.polymorphic? && relationship.belongs_to?

Diff for: lib/jsonapi/active_relation_resource.rb

@@ -324,6 +324,11 @@ def apply_join(records:, relationship:, resource_type:, join_type:, options:)
             records = records.joins_left(relation_name)
           end
         end
+
+        if relationship.use_related_resource_records_for_joins
+          records = records.merge(self.records(options))
+        end
+
         records
       end
 

Diff for: lib/jsonapi/configuration.rb

@@ -39,7 +39,8 @@ class Configuration
                 :default_resource_cache_field,
                 :resource_cache_digest_function,
                 :resource_cache_usage_report_function,
-                :default_exclude_links
+                :default_exclude_links,
+                :use_related_resource_records_for_joins
 
     def initialize
       #:underscored_key, :camelized_key, :dasherized_key, or custom
@@ -158,6 +159,11 @@ def initialize
       # and relationships. Accepts either `:default`, `:none`, or array containing the
       # specific default links to exclude, which may be `:self` and `:related`.
       self.default_exclude_links = :none
+
+      # Use a related resource's `records` when performing joins. This setting allows included resources to account for
+      # permission scopes. It can be overridden explicitly per relationship. Furthermore, specifying a `relation_name`
+      # on a relationship will cause this setting to be ignored.
+      self.use_related_resource_records_for_joins = true
     end
 
     def cache_formatters=(bool)
@@ -299,6 +305,8 @@ def allow_include=(allow_include)
     attr_writer :resource_cache_usage_report_function
 
     attr_writer :default_exclude_links
+
+    attr_writer :use_related_resource_records_for_joins
   end
 
   class << self

Diff for: lib/jsonapi/relationship.rb

@@ -3,7 +3,7 @@ class Relationship
     attr_reader :acts_as_set, :foreign_key, :options, :name,
                 :class_name, :polymorphic, :always_include_optional_linkage_data,
                 :parent_resource, :eager_load_on_include, :custom_methods,
-                :inverse_relationship, :allow_include
+                :inverse_relationship, :allow_include, :use_related_resource_records_for_joins
 
     attr_writer :allow_include
 
@@ -23,6 +23,15 @@ def initialize(name, options = {})
         @polymorphic_types ||= options[:polymorphic_relations]
       end
 
+      use_related_resource_records_for_joins_default = if options[:relation_name]
+                                                         false
+                                                       else
+                                                         JSONAPI.configuration.use_related_resource_records_for_joins
+                                                       end
+      
+      @use_related_resource_records_for_joins = options.fetch(:use_related_resource_records_for_joins,
+                                                              use_related_resource_records_for_joins_default) == true
+
       @always_include_optional_linkage_data = options.fetch(:always_include_optional_linkage_data, false) == true
       @eager_load_on_include = options.fetch(:eager_load_on_include, false) == true
       @allow_include = options[:allow_include]

Diff for: test/fixtures/active_record.rb

@@ -1413,7 +1413,7 @@ class PostResource < JSONAPI::Resource
 
   has_one :author, class_name: 'Person'
   has_one :section
-  has_many :tags, acts_as_set: true, inverse_relationship: :posts, eager_load_on_include: false
+  has_many :tags, acts_as_set: true, inverse_relationship: :posts
   has_many :comments, acts_as_set: false, inverse_relationship: :post
 
   # Not needed - just for testing
@@ -1932,16 +1932,7 @@ class AuthorResource < JSONAPI::Resource
       model_name 'Person'
       attributes :name
 
-      has_many :books, inverse_relationship: :authors, relation_name: -> (options) {
-        book_admin = options[:context][:book_admin] || options[:context][:current_user].try(:book_admin)
-
-        if book_admin
-          :books
-        else
-          :not_banned_books
-        end
-      }
-
+      has_many :books
       has_many :book_comments
     end
 
@@ -1981,6 +1972,9 @@ class BookResource < JSONAPI::Resource
               }
 
       filter :banned, apply: :apply_filter_banned
+      filter :title, apply: ->(records, value, options) {
+        records.where('books.title LIKE ?', "#{value[0]}%")
+      }
 
       class << self
         def books
@@ -1992,10 +1986,9 @@ def not_banned_books
         end
 
         def records(options = {})
-          context = options[:context]
-          current_user = context ? context[:current_user] : nil
+          current_user = options.dig(:context, :current_user)
 
-          records = _model_class.all
+          records = super
           # Hide the banned books from people who are not book admins
           unless current_user && current_user.book_admin
             records = records.where(not_banned_books)
@@ -2004,8 +1997,7 @@ def records(options = {})
         end
 
         def apply_filter_banned(records, value, options)
-          context = options[:context]
-          current_user = context ? context[:current_user] : nil
+          current_user = options.dig(:context, :current_user)
 
           # Only book admins might filter for banned books
           if current_user && current_user.book_admin
@@ -2045,7 +2037,7 @@ def approved_comments(approved = true)
         end
 
         def records(options = {})
-          current_user = options[:context][:current_user]
+          current_user = options.dig(:context, :current_user)
           _model_class.for_user(current_user)
         end
       end
@@ -2100,7 +2092,7 @@ class PostResource < JSONAPI::Resource
 
       has_one :author, class_name: 'Person', exclude_links: [:self, "related"]
       has_one :section, exclude_links: [:self, :related]
-      has_many :tags, acts_as_set: true, inverse_relationship: :posts, eager_load_on_include: false, exclude_links: :default
+      has_many :tags, acts_as_set: true, inverse_relationship: :posts, exclude_links: :default
       has_many :comments, acts_as_set: false, inverse_relationship: :post, exclude_links: ["self", :related]
     end
 

Diff for: test/integration/book_authorization_test.rb

@@ -0,0 +1,38 @@
+require File.expand_path('../../test_helper', __FILE__)
+
+class BookAuthorizationTest < ActionDispatch::IntegrationTest
+  def setup
+    DatabaseCleaner.start
+    JSONAPI.configuration.json_key_format = :underscored_key
+    JSONAPI.configuration.route_format = :underscored_route
+    Api::V2::BookResource.paginator :offset
+  end
+
+  def test_restricted_records_primary
+    Api::V2::BookResource.paginator :none
+
+    # Not a book admin
+    $test_user = Person.find(1001)
+    assert_cacheable_jsonapi_get '/api/v2/books?filter[title]=Book%206'
+    assert_equal 12, json_response['data'].size
+
+    # book_admin
+    $test_user = Person.find(1005)
+    assert_cacheable_jsonapi_get '/api/v2/books?filter[title]=Book%206'
+    assert_equal 111, json_response['data'].size
+  end
+
+  def test_restricted_records_related
+    Api::V2::BookResource.paginator :none
+
+    # Not a book admin
+    $test_user = Person.find(1001)
+    assert_cacheable_jsonapi_get '/api/v2/authors/1002/books'
+    assert_equal 1, json_response['data'].size
+
+    # book_admin
+    $test_user = Person.find(1005)
+    assert_cacheable_jsonapi_get '/api/v2/authors/1002/books'
+    assert_equal 2, json_response['data'].size
+  end
+end