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

Deleting using Has-Many-Through Associations problem #113

Closed
amree opened this issue Nov 24, 2011 · 9 comments
Closed

Deleting using Has-Many-Through Associations problem #113

amree opened this issue Nov 24, 2011 · 9 comments

Comments

@amree
Copy link

amree commented Nov 24, 2011

Hi there

From the README, I need to use the monkey patch to solve the Has-Many-Through Associations problems when deleting the data. However, I'm getting this error:

ArgumentError in ItemsController#update

wrong number of arguments (2 for 1)

config/initializers/core_extensions.rb:2:in `delete_records'
app/controllers/items_controller.rb:40:in `update'

Am I doing it wrong or is it because Rails have newer codes?

@airblade
Copy link
Collaborator

It's most probably because Rails has evolved since that patch. If you look at Rails' current implementation you can see it's changed quite a bit.

You could try removing the patch and setting up your has-many-through association with the :dependent => :destroy option. Hopefully that will solve the problem.

@amree
Copy link
Author

amree commented Nov 24, 2011

I did try dependent: :destroy and it's still not working. I've also tried replacing this line with this line and it worked. But I'm not sure on how to create a monkey patch for that (for now I just put the whole file (with those lines updated) in /lib directory. It'd be helpful if you can update the README with the correct solution. Thanks in advance.

@airblade
Copy link
Collaborator

I wonder whether that's sufficient: this line, which is called along the way, also uses delete rather than destroy...

@amree
Copy link
Author

amree commented Nov 24, 2011

I thought it would call destroy? However, it does work in my case since paper_trail picked up the delete process and saved the event in Version.

In the end, it'd be something like this:

        def delete_records(records, method)
          ensure_not_nested

          scope = through_association.scoped.where(construct_join_attributes(*records))

          case method
          when :destroy
            count = scope.destroy_all.length
          when :nullify
            count = scope.update_all(source_reflection.foreign_key => nil)
          else
            count = scope.destroy_all.length
            # count = scope.delete_all
          end

          delete_through_records(records)

          if through_reflection.macro == :has_many && update_through_counter?(method)
            update_counter(-count, through_reflection)
          end

          update_counter(-count)
        end

@airblade
Copy link
Collaborator

Yes, but the delete_through_records(records) line calls code which itself uses delete.

@amree
Copy link
Author

amree commented Nov 24, 2011

Not sure why, but it's working for me with that line replaced.

Item.rb

class Item < ActiveRecord::Base
  belongs_to :unit
  has_many :item_categories, dependent: :destroy
  has_many :categories, through: :item_categories

  has_paper_trail
end

Category.rb

class Category < ActiveRecord::Base
  has_many :item_categories, dependent: :destroy
  has_many :items, through: :item_categories

  has_paper_trail
end

ItemCategory.rb

class ItemCategory < ActiveRecord::Base
  belongs_to :item
  belongs_to :category

  has_paper_trail
end

_form.html.haml

= form_for @item do |f|
  - Category.all.each do |category|
    = check_box_tag :category_ids, category.id, @item.categories.include?(category), :name => 'item[category_ids][]'

items_controller.rb

def update
  # Set to an empty array if no value is set
  params[:item][:category_ids] ||= []

  @item = Item.find(params[:id])

  if @item.update_attributes(params[:item])
    redirect_to @item
  else
    render action: "edit"
  end
end

Without has_many_through_association.rb in my /config/initializers:

Started PUT "/items/22" for 127.0.0.1 at 2011-11-24 19:13:24 +0800
  Processing by ItemsController#update as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"1e8q3ePoYA9Cdcyf3wuKIoJeu+DxCtDCMqLMtnwwAoc=", "item"=>{"kod"=>"Kaycee Kuphal", "nama"=>"Duane Cruickshank", "unit_id"=>"17", "lokasi"=>"", "is_aktif"=>"1", "category_ids"=>["1"]}, "commit"=>"Save", "id"=>"22"}
  User Load (0.5ms)  SELECT `identiti`.`pengguna`.* FROM `identiti`.`pengguna` WHERE `identiti`.`pengguna`.`id` = 1 ORDER BY identiti.pengguna.nama LIMIT 1
  Item Load (0.6ms)  SELECT `items`.* FROM `items` WHERE `items`.`id` = 22 LIMIT 1
   (0.1ms)  BEGIN
  Category Load (0.7ms)  SELECT `categories`.* FROM `categories` WHERE `categories`.`id` = 1 LIMIT 1
  Category Load (0.7ms)  SELECT `categories`.* FROM `categories` INNER JOIN `item_categories` ON `categories`.`id` = `item_categories`.`category_id` WHERE `item_categories`.`item_id` = 22
  SQL (0.6ms)  DELETE FROM `item_categories` WHERE `item_categories`.`item_id` = 22 AND `item_categories`.`category_id` = 2
   (1.3ms)  COMMIT
Redirected to http://dev:3000/items/22
Completed 302 Found in 305ms

With the line replaced:

Started PUT "/items/22" for 127.0.0.1 at 2011-11-24 19:20:13 +0800
  Processing by ItemsController#update as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"1e8q3ePoYA9Cdcyf3wuKIoJeu+DxCtDCMqLMtnwwAoc=", "item"=>{"kod"=>"Kaycee Kuphal", "nama"=>"Duane Cruickshank", "unit_id"=>"17", "lokasi"=>"", "is_aktif"=>"1", "category_ids"=>["1"]}, "commit"=>"Save", "id"=>"22"}
  User Load (0.4ms)  SELECT `identiti`.`pengguna`.* FROM `identiti`.`pengguna` WHERE `identiti`.`pengguna`.`id` = 1 ORDER BY identiti.pengguna.nama LIMIT 1
  Item Load (0.6ms)  SELECT `items`.* FROM `items` WHERE `items`.`id` = 22 LIMIT 1
   (0.1ms)  BEGIN
  Category Load (0.6ms)  SELECT `categories`.* FROM `categories` WHERE `categories`.`id` = 1 LIMIT 1
  Category Load (1.0ms)  SELECT `categories`.* FROM `categories` INNER JOIN `item_categories` ON `categories`.`id` = `item_categories`.`category_id` WHERE `item_categories`.`item_id` = 22
  ItemCategory Load (0.8ms)  SELECT `item_categories`.* FROM `item_categories` WHERE `item_categories`.`item_id` = 22 AND `item_categories`.`category_id` = 2
  SQL (0.3ms)  DELETE FROM `item_categories` WHERE `item_categories`.`id` = 24
  SQL (0.3ms)  INSERT INTO `versions` (`created_at`, `event`, `ip`, `item_id`, `item_type`, `object`, `object_changes`, `whodunnit`) VALUES ('2011-11-24 11:20:13', 'destroy', '127.0.0.1', 24, 'ItemCategory', '---\nitem_id: 22\ncategory_id: 2\ncreated_at: 2011-11-24 11:19:53.000000000 Z\nupdated_at: 2011-11-24 11:19:53.000000000 Z\nid: 24\n', NULL, 1)
  Version Load (0.7ms)  SELECT `versions`.* FROM `versions` WHERE `versions`.`item_id` = 24 AND `versions`.`item_type` = 'ItemCategory' ORDER BY created_at ASC, id ASC
   (0.7ms)  COMMIT
Redirected to http://dev:3000/items/22
Completed 302 Found in 258ms

@amree
Copy link
Author

amree commented Nov 24, 2011

It seems like this one is working:

module ActiveRecord
  # = Active Record Has Many Through Association
  module Associations
    class HasManyThroughAssociation < HasManyAssociation #:nodoc:
      alias_method :ori_delete_records, :delete_records

      def delete_records(records, method)
        method ||= :destroy
        ori_delete_records(records, method)
      end
    end
  end
end

From the log:

Started PUT "/items/22" for 127.0.0.1 at 2011-11-25 07:24:56 +0800
  Processing by ItemsController#update as HTML
  Parameters: {"utf8"=>"✓", "authenticity_token"=>"1e8q3ePoYA9Cdcyf3wuKIoJeu+DxCtDCMqLMtnwwAoc=", "item"=>{"kod"=>"Kaycee Kuphal", "nama"=>"Duane Cruickshank", "unit_id"=>"17", "lokasi"=>"", "is_aktif"=>"1", "category_ids"=>["1"]}, "commit"=>"Save", "id"=>"22"}
  User Load (0.5ms)  SELECT `identiti`.`pengguna`.* FROM `identiti`.`pengguna` WHERE `identiti`.`pengguna`.`id` = 1 ORDER BY identiti.pengguna.nama LIMIT 1
  Item Load (0.7ms)  SELECT `items`.* FROM `items` WHERE `items`.`id` = 22 LIMIT 1
   (0.1ms)  BEGIN
  Category Load (0.6ms)  SELECT `categories`.* FROM `categories` WHERE `categories`.`id` = 1 LIMIT 1
  Category Load (0.7ms)  SELECT `categories`.* FROM `categories` INNER JOIN `item_categories` ON `categories`.`id` = `item_categories`.`category_id` WHERE `item_categories`.`item_id` = 22
  ItemCategory Load (0.6ms)  SELECT `item_categories`.* FROM `item_categories` WHERE `item_categories`.`item_id` = 22 AND `item_categories`.`category_id` = 2
  SQL (0.4ms)  DELETE FROM `item_categories` WHERE `item_categories`.`id` = 34
  SQL (0.3ms)  INSERT INTO `versions` (`created_at`, `event`, `ip`, `item_id`, `item_type`, `object`, `object_changes`, `whodunnit`) VALUES ('2011-11-24 23:24:57', 'destroy', '127.0.0.1', 34, 'ItemCategory', '---\nitem_id: 22\ncategory_id: 2\ncreated_at: 2011-11-24 23:24:50.000000000 Z\nupdated_at: 2011-11-24 23:24:50.000000000 Z\nid: 34\n', NULL, 1)
  Version Load (0.8ms)  SELECT `versions`.* FROM `versions` WHERE `versions`.`item_id` = 34 AND `versions`.`item_type` = 'ItemCategory' ORDER BY created_at ASC, id ASC
   (0.7ms)  COMMIT
Redirected to http://dev:3000/items/22
Completed 302 Found in 296ms

@airblade
Copy link
Collaborator

That looks good. I'll update the README. Thanks!

@adsummos
Copy link

That patch looks extremely dangerous because it will change the behavior of you application. Records that you expected to get nullified or deleted will instead by destroyed. If you want the same behavior, just attach dependent => :destroy to your association

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants