diff --git a/lib/jsonapi/active_relation/join_manager.rb b/lib/jsonapi/active_relation/join_manager.rb index 3d1ec34b..c41d4a7f 100644 --- a/lib/jsonapi/active_relation/join_manager.rb +++ b/lib/jsonapi/active_relation/join_manager.rb @@ -70,7 +70,7 @@ def join_details_by_relationship(relationship) @join_details[segment] end - def self.get_join_arel_node(records, options = {}) + def self.get_join_arel_node(records, relationship, join_type, options = {}) init_join_sources = records.arel.join_sources init_join_sources_length = init_join_sources.length @@ -80,9 +80,27 @@ def self.get_join_arel_node(records, options = {}) if join_sources.length > init_join_sources_length last_join = (join_sources - init_join_sources).last else + # Try to find a pre-existing join for this table. + # We can get here if include_optional_linkage_data is true + # (or always_include_to_xxx_linkage_data), + # and the user's custom `records` method has already added that join. + # + # If we want a left join and there is already an inner/left join, + # then we can use that. + # If we want an inner join and there is alrady an inner join, + # then we can use that (but not a left join, since that doesn't filter things out). + valid_join_types = [Arel::Nodes::InnerJoin] + valid_join_types << Arel::Nodes::OuterJoin if join_type == :left + table_name = relationship.resource_klass._table_name + + last_join = join_sources.find { |j| + valid_join_types.any? { |t| j.is_a?(t) } && j.left.name == table_name + } + end + + if last_join.nil? # :nocov: warn "get_join_arel_node: No join added" - last_join = nil # :nocov: end @@ -154,7 +172,7 @@ def perform_joins(records, options) next end - records, join_node = self.class.get_join_arel_node(records, options) {|records, options| + records, join_node = self.class.get_join_arel_node(records, relationship, join_type, options) {|records, options| related_resource_klass.join_relationship( records: records, resource_type: related_resource_klass._type, @@ -294,4 +312,4 @@ def add_relationships(relationships) end end end -end \ No newline at end of file +end diff --git a/test/fixtures/active_record.rb b/test/fixtures/active_record.rb index 1209302f..f8959317 100644 --- a/test/fixtures/active_record.rb +++ b/test/fixtures/active_record.rb @@ -1059,6 +1059,8 @@ def context module V3 class PostsController < JSONAPI::ResourceController end + class MoonsController < JSONAPI::ResourceController + end end module V4 @@ -2042,6 +2044,15 @@ module Api module V3 class PostResource < PostResource; end class PreferencesResource < PreferencesResource; end + class PlanetResource < JSONAPI::Resource + end + class MoonResource < JSONAPI::Resource + has_one :planet, always_include_optional_linkage_data: true + + def self.records(options = {}) + Moon.joins(:planet).merge(Planet.where(name: 'Satern')) # sic + end + end end end diff --git a/test/integration/requests/request_test.rb b/test/integration/requests/request_test.rb index 1863b5c7..b7895608 100644 --- a/test/integration/requests/request_test.rb +++ b/test/integration/requests/request_test.rb @@ -1855,4 +1855,10 @@ def test_destroy_singleton_not_found JSONAPI.configuration = original_config $test_user = $original_test_user end + + def test_include_optional_linkage_data_with_join + get "/api/v3/moons", headers: { 'Accept' => JSONAPI::MEDIA_TYPE } + assert_jsonapi_response 200 + refute_nil json_response['data'][0]['relationships']['planet'] + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 9850a49c..c1faea37 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -312,6 +312,9 @@ class CatResource < JSONAPI::Resource jsonapi_link :author, except: [:destroy] jsonapi_links :tags, only: [:show, :create] end + + jsonapi_resources :planets + jsonapi_resources :moons end JSONAPI.configuration.route_format = :camelized_route