fix: include has_one through associations in compute_select_fields in ResourcesGetter (#732)

Author: Pacyfik

README.md

@@ -164,7 +164,7 @@ Here is the contribution workflow:
 Please ensure that the **tests** are passing before submitting any pull request:
 
 ```
-$ RAILS_ENV=test bundle exec rake --trace db:migrate test
+$ RAILS_ENV=test bundle exec rake --trace db:migrate test; bundle exec rspec
 ```
 
 ## Community

app/services/forest_liana/resources_getter.rb

@@ -227,10 +227,20 @@ def pagination?
     def compute_select_fields
       select = ['_forest_admin_eager_load']
       @params[:fields][@collection_name].split(',').each do |path|
+        association = get_one_association(path)
+        if association
+          while association.options[:through]
+            association = get_one_association(association.options[:through])
+          end
+
+          if SchemaUtils.polymorphic?(association)
+            select << "#{@resource.table_name}.#{association.foreign_type}"
+          end
+          select << "#{@resource.table_name}.#{association.foreign_key}"
+        end
+
         if @params[:fields].key?(path)
-          association = ForestLiana::QueryHelper.get_one_associations(@resource)
-                                                .select { |association| association.name == path.to_sym }
-                                                .first
+          association = get_one_association(path)
           table_name = association.table_name
 
           @params[:fields][path].split(',').each do |association_path|
@@ -245,7 +255,13 @@ def compute_select_fields
         end
       end
 
-      select
+      select.uniq
+    end
+
+    def get_one_association(name)
+      ForestLiana::QueryHelper.get_one_associations(@resource)
+        .select { |association| association.name == name.to_sym }
+        .first
     end
   end
 end

spec/dummy/app/models/tree.rb

@@ -7,4 +7,6 @@ class Tree < ActiveRecord::Base
     class_name: 'Island',
     inverse_of: :eponymous_tree,
     optional: true
+
+  has_one :location, through: :island
 end

spec/helpers/forest_liana/query_helper_spec.rb

@@ -37,6 +37,7 @@ module ForestLiana
             { name: :cutter, klass: User },
             { name: :island, klass: Island },
             { name: :eponymous_island, klass: Island },
+            { name: :location, klass: Location },
           ]
         end
 
@@ -52,13 +53,17 @@ module ForestLiana
 
     describe 'get_one_association_names_symbol' do
       it 'should return the one-one associations names as symbols' do
-        expect(QueryHelper.get_one_association_names_symbol(Tree)).to eq([:owner, :cutter, :island, :eponymous_island])
+        expect(QueryHelper.get_one_association_names_symbol(Tree)).to eq(
+          [:owner, :cutter, :island, :eponymous_island, :location]
+        )
       end
     end
 
     describe 'get_one_association_names_string' do
       it 'should return the one-one associations names as strings' do
-        expect(QueryHelper.get_one_association_names_string(Tree)).to eq(['owner', 'cutter', 'island', 'eponymous_island'])
+        expect(QueryHelper.get_one_association_names_string(Tree)).to eq(
+          ['owner', 'cutter', 'island', 'eponymous_island', 'location']
+        )
       end
     end
 
@@ -71,12 +76,12 @@ module ForestLiana
         end
       end
 
-      context 'on a model having 2 belongsTo associations' do
+      context 'on a model having 3 belongsTo/hasOne associations' do
         tables_associated_to_relations_name =
           QueryHelper.get_tables_associated_to_relations_name(Tree)
 
         it 'should return the one-one associations' do
-          expect(tables_associated_to_relations_name.keys.length).to eq(2)
+          expect(tables_associated_to_relations_name.keys.length).to eq(3)
         end
 
         it 'should return relationships having a name different than the targeted model' do

spec/requests/resources_spec.rb

@@ -4,7 +4,9 @@
   let(:scope_filters) { {'scopes' => {}, 'team' => {'id' => '1', 'name' => 'Operations'}} }
   before do
     user = User.create(name: 'Michel')
-    Tree.create(name: 'Lemon Tree', owner: user, cutter: user)
+    tree = Tree.create(name: 'Lemon Tree', owner: user, cutter: user)
+    island = Island.create(name: 'Lemon Island', trees: [tree])
+    Location.create(coordinates: '1,2', island: island)
 
     Rails.cache.write('forest.users', {'1' => { 'id' => 1, 'roleId' => 1, 'rendering_id' => '1' }})
     Rails.cache.write('forest.has_permission', true)
@@ -32,6 +34,8 @@
   after do
     User.destroy_all
     Tree.destroy_all
+    Island.destroy_all
+    Location.destroy_all
   end
 
   token = JWT.encode({
@@ -54,7 +58,7 @@
   describe 'index' do
     describe 'without any filter' do
       params = {
-        fields: { 'Tree' => 'id,name' },
+        fields: { 'Tree' => 'id,name,location' },
         page: { 'number' => '1', 'size' => '10' },
         searchExtended: '0',
         sort: '-id',
@@ -98,7 +102,8 @@
 
       it 'should respond the tree data' do
         get '/forest/Tree', params: params, headers: headers
-        expect(JSON.parse(response.body)).to eq({
+        
+        expect(JSON.parse(response.body)).to include({
           "data" => [{
             "type" => "Tree",
             "id" => "1",
@@ -108,9 +113,30 @@
             },
             "links" => {
               "self" => "/forest/tree/1"
+            },
+            "relationships" => {
+              "location" => {
+                "data" => { "id" => "1", "type" => "Location" },
+                "links" => { "related" => {} }
+              }
             }
           }],
-          "included" => []
+          "included" => [{
+            "type" => "Location",
+            "id" => "1",
+            "attributes" => include(
+              "id" => 1,
+              "created_at" => nil,
+              "updated_at" => nil,
+              "coordinates" => nil
+            ),
+            "links" => { "self" => "/forest/location/1" },
+            "relationships" => {
+              "island" => {
+                "links" => { "related" => {} }
+              }
+            }
+          }]
         })
       end
     end

spec/services/forest_liana/resources_getter_spec.rb

@@ -290,6 +290,22 @@ def association_connection.current_database
       end
     end
 
+    describe 'when getting a has_one through association' do
+      let(:resource) { Tree }
+      let(:fields) { { 'Tree' => 'id,location' } }
+
+      it 'should get the expected records, including the foreign_key for the direct association' do
+        getter.perform
+        records = getter.records
+        count = getter.count
+
+        expect(records.count).to eq Tree.count
+        expect(count).to eq Tree.count
+        expect(records.map(&:id)).to match_array(Tree.pluck(:id))
+        expect(records.map(&:island_id)).to match_array(Tree.pluck(:island_id))
+      end
+    end
+
     describe 'when filtering on an ambiguous field' do
       let(:resource) { Tree }
       let(:pageSize) { 5 }

spec/services/forest_liana/schema_adapter_spec.rb

@@ -61,7 +61,18 @@ module ForestLiana
           end
 
           expect(collection.fields.map { |field| field[:field].to_s}).to eq(
-            ["age", "created_at", "cutter", "eponymous_island", "id", "island", "name", "owner", "updated_at"]
+            [
+              "age",
+              "created_at",
+              "cutter",
+              "eponymous_island",
+              "id",
+              "island",
+              "location",
+              "name",
+              "owner",
+              "updated_at"
+            ]
           )
         end
       end