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

Dirty checking not working for context tags #321

Closed
saurabhnanda opened this issue Feb 14, 2013 · 7 comments
Closed

Dirty checking not working for context tags #321

saurabhnanda opened this issue Feb 14, 2013 · 7 comments
Labels
Milestone

Comments

@saurabhnanda
Copy link

Can someone please confirm this bug:

class User < ActiveRecord::Base
  acts_as_taggable
  acts_as_taggable_on :admin_tags
end

u = User.last
u.tag_list_changed?
 => false 
u.admin_tag_list_changed?
NoMethodError: undefined method `admin_tag_list_changed?
@shekibobo
Copy link
Contributor

👍 Confirmed, causing issue when using tag list fields in nested forms.

@shekibobo
Copy link
Contributor

class User < ActiveRecord::Base
  acts_as_taggable_on :admin_tags
end
> u = User.last
> u.admin_tag_list << "NewTag"
> u.changed?
=> false
> u.admin_tag_list_changed?
 => false 
> u.changed_attributes
=> {}

So it looks like the fundamental assumption of dirty.rb is flawed:

def #{tag_type}_list_changed?
  changed_attributes.include?("#{tag_type}_list")
end

Using Rails 3.2.13 on Ruby 1.9.3-p392

@tilsammans
Copy link
Contributor

Thanks for the failing specs. Is a fix underway or should I take a look?

@shekibobo
Copy link
Contributor

I'm taking a look, but I don't know the library to well. If you've got a lead, let me know and I'll try it out.

@tilsammans
Copy link
Contributor

I'm not available right now, but might be in a few hours.

I've marked this one to be included in 2.5.0, since I am even having trouble releasing 2.4.0 at this time. So try to be patient, there is a backlog with issues.

shekibobo added a commit to shekibobo/acts-as-taggable-on that referenced this issue Apr 13, 2013
Related mbleigh#321.

Split tests for single-assertions
@tilsammans
Copy link
Contributor

2.4.0 will be out on Tuesday and I think 2.5.0 (which will include your fix) should come fairly close behind.

Thanks for the contribution!

@tilsammans
Copy link
Contributor

Just to be clear, your fix will be in 2.4.1 already.

ches added a commit to ches/acts-as-taggable-on that referenced this issue Sep 26, 2013
This reverts commit e44b69d.

This undid the work of 16c4567 such that we were now
re-including most the library on subsequent calls of the macro method
like:

    acts_as_taggable_on :pros
    acts_as_taggable_on :cons

The change was made in order to fix dirty tracking on mbleigh#321, but the
proper fix is that the dirty tracking module is missing a hook
`acts_as_taggable_on` macro that calls super.
ches added a commit to ches/acts-as-taggable-on that referenced this issue Dec 10, 2013
This reverts commit e44b69d.

This undid the work of 16c4567 such that we were now
re-including most the library on subsequent calls of the macro method
like:

    acts_as_taggable_on :pros
    acts_as_taggable_on :cons

The change was made in order to fix dirty tracking on mbleigh#321, but the
proper fix is that the dirty tracking module is missing a hook
`acts_as_taggable_on` macro that calls super.
tekniklr pushed a commit to tekniklr/acts-as-taggable-on that referenced this issue Mar 19, 2021
Related mbleigh#321.

Split tests for single-assertions
tekniklr pushed a commit to tekniklr/acts-as-taggable-on that referenced this issue Mar 19, 2021
Fix dirty checking for subsequent calls to taggable_on. Fixes mbleigh#321.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants