Handle has_one assignment on the field level, making patched has_one getter/setter unnecessary

Author: mshibuya

lib/rails_admin/adapters/active_record/object_extension.rb

@@ -4,24 +4,6 @@ module RailsAdmin
   module Adapters
     module ActiveRecord
       module ObjectExtension
-        def self.extended(object)
-          object.class.reflect_on_all_associations.each do |association|
-            association = Association.new(association, object.class)
-            case association.type
-            when :has_one
-              object.instance_eval <<-RUBY, __FILE__, __LINE__ + 1
-                def #{association.name}_id
-                  self.#{association.name}&.id
-                end
-
-                def #{association.name}_id=(item_id)
-                  self.#{association.name} = (#{association.klass}.find(item_id) rescue nil)
-                end
-              RUBY
-            end
-          end
-        end
-
         def assign_attributes(attributes)
           super if attributes
         end

lib/rails_admin/adapters/mongoid/object_extension.rb

@@ -20,11 +20,6 @@ def self.extended(object)
                   send(name)&.save
                 end
               end
-              object.instance_eval <<-RUBY, __FILE__, __LINE__ + 1
-                def #{name}_id=(item_id)
-                  self.#{name} = (#{association.klass}.find(item_id) rescue nil)
-                end
-              RUBY
             end
           end
         end

lib/rails_admin/config/fields/types/has_one_association.rb

@@ -23,6 +23,10 @@ class HasOneAssociation < RailsAdmin::Config::Fields::Association
             (o = value) && o.send(associated_model_config.object_label_method)
           end
 
+          register_instance_option :allowed_methods do
+            nested_form ? [method_name] : [name]
+          end
+
           def selected_id
             value.try(:id).try(:to_s)
           end
@@ -38,6 +42,13 @@ def multiple?
           def associated_prepopulate_params
             {associated_model_config.abstract_model.param_key => {association.foreign_key => bindings[:object].try(:id)}}
           end
+
+          def parse_input(params)
+            return if nested_form
+
+            id = params.delete(method_name)
+            params[name] = associated_model_config.abstract_model.get(id) if id
+          end
         end
       end
     end

spec/dummy_app/app/active_record/managing_user.rb

@@ -3,8 +3,4 @@
 class ManagingUser < User
   has_one :team, class_name: 'ManagedTeam', foreign_key: :manager, primary_key: :email, inverse_of: :user
   has_many :teams, class_name: 'ManagedTeam', foreign_key: :manager, primary_key: :email, inverse_of: :user
-
-  def team_id=(id)
-    self.team = ManagedTeam.find_by_id(id)
-  end
 end

spec/dummy_app/app/mongoid/managing_user.rb

@@ -5,8 +5,4 @@ class ManagingUser < User
   has_many :teams, class_name: 'ManagedTeam', foreign_key: :manager, primary_key: :email, inverse_of: :user
   has_and_belongs_to_many :players, foreign_key: :player_names, primary_key: :name, inverse_of: :nil
   has_and_belongs_to_many :balls, primary_key: :color, inverse_of: :nil
-
-  def team_id=(id)
-    self.team = ManagedTeam.where(_id: id).first
-  end
 end

spec/integration/fields/has_one_association_spec.rb

@@ -14,15 +14,19 @@
   context 'on create' do
     before do
       @draft = FactoryBot.create :draft
+      visit new_path(model_name: 'player')
     end
 
     it 'shows selects' do
-      visit new_path(model_name: 'player')
       is_expected.to have_selector('select#player_draft_id')
     end
 
     it 'creates an object with correct associations' do
-      post new_path(model_name: 'player', player: FactoryBot.attributes_for(:player).merge(name: 'Jackie Robinson', draft_id: @draft.id))
+      fill_in 'Name', with: 'Jackie Robinson'
+      fill_in 'Number', with: @draft.player.number + 1
+      select("Draft ##{@draft.id}", from: 'Draft')
+      click_button 'Save'
+      is_expected.to have_content 'Player successfully created'
       @player = Player.where(name: 'Jackie Robinson').first
       @draft.reload
       expect(@player.draft).to eq(@draft)
@@ -31,22 +35,23 @@
 
   context 'on update' do
     before do
-      @player = FactoryBot.create :player
-      @draft = FactoryBot.create :draft
-      @number = @draft.player.number + 1 # to avoid collision
-      put edit_path(model_name: 'player', id: @player.id, player: {name: 'Jackie Robinson', draft_id: @draft.id, number: @number, position: 'Second baseman'})
-      @player.reload
+      @drafts = FactoryBot.create_list :draft, 2
+      @player = FactoryBot.create :player, draft: @drafts[0]
+      visit edit_path(model_name: 'player', id: @player.id)
     end
 
-    it 'updates an object with correct attributes' do
-      expect(@player.name).to eq('Jackie Robinson')
-      expect(@player.number).to eq(@number)
-      expect(@player.position).to eq('Second baseman')
+    it 'updates an object with correct associations' do
+      select("Draft ##{@drafts[1].id}", from: 'Draft')
+      click_button 'Save'
+      @player.reload
+      expect(@player.draft).to eq(@drafts[1])
     end
 
-    it 'updates an object with correct associations' do
-      @draft.reload
-      expect(@player.draft).to eq(@draft)
+    it 'clears the current selection' do
+      select('', from: 'Draft')
+      click_button 'Save'
+      @player.reload
+      expect(@player.draft).to be nil
     end
   end
 

spec/rails_admin/adapters/active_record/object_extension_spec.rb

@@ -11,63 +11,4 @@
       expect(object.assign_attributes(nil)).to be nil
     end
   end
-
-  describe 'has_one association' do
-    let(:draft) { FactoryBot.create(:draft) }
-    let(:player) { FactoryBot.build(:player).extend(RailsAdmin::Adapters::ActiveRecord::ObjectExtension) }
-    before do
-      class PlayerWithAutoSave < Player
-        has_one :draft, inverse_of: :player, foreign_key: :player_id, autosave: true
-      end
-    end
-
-    it 'provides id getter' do
-      player.draft = draft
-      expect(player.draft_id).to eq draft.id
-    end
-
-    context 'on create' do
-      before do
-        player.draft_id = draft.id
-        expect(player.draft).to receive(:save).once.and_call_original
-        player.save
-      end
-
-      it 'persists associated documents changes on save' do
-        expect(player.reload.draft).to eq draft
-      end
-    end
-
-    context 'on update' do
-      let(:player) { FactoryBot.create(:player).extend(RailsAdmin::Adapters::ActiveRecord::ObjectExtension) }
-      before do
-        player.draft_id = draft.id
-      end
-
-      it 'persists associated documents changes on assignment' do
-        expect(player.reload.draft).to eq draft
-      end
-    end
-
-    context 'with explicit id setter' do
-      let(:user) { ManagingUser.create(FactoryBot.attributes_for(:user)) }
-      let(:team) { ManagedTeam.create(FactoryBot.attributes_for(:team)) }
-
-      it 'works without issues' do
-        user.team_id = team.id
-        expect(user.reload.team).to eq team
-      end
-    end
-
-    context 'when associated class has custom primary key' do
-      let(:league) { FactoryBot.build(:league).extend(RailsAdmin::Adapters::ActiveRecord::ObjectExtension) }
-      let(:division) { FactoryBot.create :division }
-
-      it 'does not break' do
-        league.division_id = division.id
-        league.save!
-        expect(league.reload.division).to eq division
-      end
-    end
-  end
 end

spec/rails_admin/adapters/mongoid/object_extension_spec.rb

@@ -69,7 +69,7 @@ class PlayerWithAutoSave < Player
 
     context 'on create' do
       before do
-        player.draft_id = draft.id
+        player.draft = draft
         expect(player.draft._target).to receive(:save).once.and_call_original
         player.save
       end
@@ -93,7 +93,7 @@ class PlayerWithAutoSave < Player
 
     context 'on update' do
       before do
-        player.draft_id = draft.id
+        player.draft = draft
       end
 
       context 'with autosave: false' do
@@ -112,15 +112,5 @@ class PlayerWithAutoSave < Player
         end
       end
     end
-
-    context 'with explicit id setter' do
-      let(:user) { ManagingUser.create(FactoryBot.attributes_for(:user)) }
-      let(:team) { ManagedTeam.create(FactoryBot.attributes_for(:team)) }
-
-      it 'works without issues' do
-        user.team_id = team.id
-        expect(user.reload.team).to eq team
-      end
-    end
   end
 end