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

Associations are persisted before validation of main record #5430

Closed
tkalliom opened this issue Jun 20, 2018 · 2 comments · May be fixed by #7437
Closed

Associations are persisted before validation of main record #5430

tkalliom opened this issue Jun 20, 2018 · 2 comments · May be fixed by #7437

Comments

@tkalliom
Copy link

Expected behavior

When updating a record with changes to its children that make the record fail validation, no changes are saved to the database.

Actual behavior

The invalid association change is persisted, and then the user gets redirected to a form displaying a validation error.

How to reproduce

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  gem 'activeadmin', ENV['ACTIVE_ADMIN_PATH'] ? {path: ENV['ACTIVE_ADMIN_PATH'], require: false} : {github: 'activeadmin/activeadmin', require: false}
  gem 'rails', '~> 5.1.0'
  gem 'sass-rails'
  gem 'sqlite3', platform: :mri
  gem 'activerecord-jdbcsqlite3-adapter', "~> 51.0", platform: :jruby
end

require 'active_record'

ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :agents, force: true do |t|
    t.string :full_name
  end

  create_table :agents_clients, force: true do |t|
    t.bigint :agent_id, null: false
    t.bigint :client_id, null: false
  end

  create_table :clients, force: true do |t|
    t.string :name
  end
end

require 'action_controller/railtie'
require 'action_view/railtie'
require 'active_admin'

class TestApp < Rails::Application
  config.root = __dir__
  secrets.secret_token = 'secret_token'
  secrets.secret_key_base = 'secret_key_base'

  config.eager_load = false
  Rails.logger = config.logger = Logger.new($stdout)
end

class ApplicationController < ActionController::Base
  include Rails.application.routes.url_helpers
end

class Client < ActiveRecord::Base
  has_and_belongs_to_many :agents
  validates :agents, presence: true
end

class Agent < ActiveRecord::Base
  has_and_belongs_to_many :clients
end

ActiveAdmin.setup do |config|
  config.authentication_method = false
  config.current_user_method = false
end

Rails.application.initialize!

ActiveAdmin.register Client do
  permit_params agent_ids: []
  
  form do |f|
    f.input :agents, as: :check_boxes
  end
end

Rails.application.routes.draw do
  ActiveAdmin.routes(self)
end

require 'minitest/autorun'
require 'rack/test'

class HabtmTest < ActionDispatch::IntegrationTest
  def test_clients
    Agent.create! full_name: 'John Doe'
    Client.create! name: 'ACME', agents: [Agent.first]

    patch admin_client_url(Client.first), params: {client: {agent_ids: []}}
    assert_response :success
    assert_select "p.inline-errors", "can't be blank"
    assert_equal 1, Client.first.agents.count
  end

  private

  def app
    Rails.application
  end
end

This seems to be due to the behavior of assign_attributes (as detailed in http://guides.rubyonrails.org/association_basics.html#has-and-belongs-to-many-association-reference). Maybe update_resource should check if an ids field is present, and if so use collection.build for that?

@ezeheinke
Copy link

ezeheinke commented Mar 16, 2020

So, for anyone having the same issue, I found a workaround, you can define a before_remove in the has_and_belongs_to_many association and raise an exception that then you can catch in ActiveAdmin.

For this it would be

class Client < ActiveRecord::Base
  has_and_belongs_to_many :agents, before_remove: :check_something
  validates :agents, presence: true
end

def check_something(agent)
    if self.agents.size == 1
      raise ActiveModel::MissingAttributeError.new 'something'
    end
end

@javierjulio
Copy link
Member

There is a PR ready for this. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment