Model.has_many_through_association.find(id) returns a read-only record

This is not a bug. It is an undocumented feature. :-)

Look at following test cases which is already there in ActiveRecord.

  def test_habtm_find_readonly
    dev = Developer.find(1)
    assert !dev.projects.empty?
    assert dev.projects.all?(&:readonly?)
    assert dev.projects.find(:all).all?(&:readonly?)
    assert dev.projects.readonly(true).all?(&:readonly?)
  end

  def test_has_many_find_readonly
    post = Post.find(1)
    assert !post.comments.empty?
    assert !post.comments.any?(&:readonly?)
    assert !post.comments.find(:all).any?(&:readonly?)
    assert post.comments.readonly(true).all?(&:readonly?)
  end

As you can see by default with habtm all the retrieved records are marked as readonly.

Although it is not there in the test but you can pass readonly(false) and you will get non readonly records.

Check this out

class Post < ActiveRecord::Base
  has_many :taggings
  has_many :tags, :through => :taggings

  def self.lab
    tmp = Post.first.tags.readonly(true).find(1).readonly?
    puts tmp #=> true
  end
end


class Post < ActiveRecord::Base
  has_many :taggings
  has_many :tags, :through => :taggings

  def self.lab
    tmp = Post.first.tags.readonly(false).find(1).readonly?
    puts tmp #=> false
  end
end

This is a rather obscure bug. I'm glad to say I finally traced it down. ActiveRecord::QueryMethods#build_select wasn't resetting @implicit_readonly since it was passed empty selects argument. The RC version was using selects.present? check which returned true even for empty array. And the selects were empty because of the finder options slicing.

The fix is attached.

[bulk edit]

Automatic cleanup of spam.

Using the "patch" tag instead of prefixing the ticket title with "[PATCH]" to make sure patched tickets end up in the open patches bin. :)

[bulk edit]

[bulk edit]

Hi,

This patch didn't apply cleanly for me initially. I merged in the tests and ran them, and they passed on master and on 3-0-stable. I've attached an updated patch containing these tests which I think should be applied to guard against regressions.

I'm not sure whether the change that was proposed in AssociationCollection#find should still be applied, but if so presumably that's a different issue as these tests not pass. I've left it out for now.

Thanks

Contrary to my earlier assertion, this is not fixed in 3-0-stable. It is, however, fixed in master.

I've looked into fixing it in 3-0-stable but my personal opinion is that it's too complicated and therefore not worth it. The root of the problem is that 3-0-stable generates strings for the INNER JOINs it performs, which then causes the @implicit_readonly flag to get set when building the Arel query. In master, the joins are built as Arel objects instead, so this flag is not set.

I've also attached a new patch against master which cleans up the tests a little, and also fixes the third one (which was not actually testing what it said it was testing).