rails 5.2 compability

[doits]

Since rails 5.2, @changed_attributes is not available anymore (besides being a frozen hash when calling changed_attributes). This lead to:

     NoMethodError:
       undefined method `[]=' for nil:NilClass
     # /cache/gems/ruby_2.4/gems/acts-as-taggable-on-5.0.0/lib/acts_as_taggable_on/taggable/core.rb:206:in `process_dirty_object'
     # /cache/gems/ruby_2.4/gems/acts-as-taggable-on-5.0.0/lib/acts_as_taggable_on/taggable/core.rb:184:in `set_tag_list_on'
     # /cache/gems/ruby_2.4/gems/acts-as-taggable-on-5.0.0/lib/acts_as_taggable_on/taggable/core.rb:45:in `tag_list='

I quickly worked around the issue by using attributes_changed_by_setter which fixes the error and makes things looks like they work (like setting and getting tags etc). Though two specs regarding changed attributes still fail:

  1) ActsAsTaggableOn::Taggable::Dirty with un-contexted tags when tag_list changed should show changes of dirty object
     Failure/Error: expect(@taggable.changes).to eq({'tag_list' => ['awesome, epic', ['one']]})

       expected: {"tag_list"=>["awesome, epic", ["one"]]}
            got: {"tag_list"=>["awesome, epic", nil]}

       (compared using ==)

       Diff:
       @@ -1,2 +1,2 @@
       -"tag_list" => ["awesome, epic", ["one"]],
       +"tag_list" => ["awesome, epic", nil]

 2) ActsAsTaggableOn::Taggable::Dirty with context tags when language_list changed should show changes of dirty object
     Failure/Error: expect(@taggable.changes).to eq({'language_list' => ['awesome, epic', ['one']]})

       expected: {"language_list"=>["awesome, epic", ["one"]]}
            got: {"language_list"=>["awesome, epic", nil]}

       (compared using ==)

       Diff:
       @@ -1,2 +1,2 @@
       -"language_list" => ["awesome, epic", ["one"]],
       +"language_list" => ["awesome, epic", nil]

This seems to be the same spec for contexed and uncontexec tags, so the same failure. I don't exactly understand why the expected output is ["awesome, epic", ["one"]] and not ["awesome, epic", "one"] (no array), but nevertheless ["awesome, epic", nil] is wrong.

So my fix is not 100 % correct yet, but I did not have time to look into it at more detail.

[jkburges]

I don't exactly understand why the expected output is ["awesome, epic", ["one"]] and not ["awesome, epic", "one"] (no array)

The line https://github.com/doits/acts-as-taggable-on/blob/master/lib/acts_as_taggable_on/taggable/core.rb#L186 calls ActsAsTaggableOn.default_parser.new(new_list).parse which converts "one" to ["one"]

[jkburges]

Just writing down what I've found, maybe it'll help someone who knows more than me...

Compare https://github.com/rails/rails/blob/57e90ef5b1478dd190f9ab7b60493b37e302f02f/activemodel/lib/active_model/dirty.rb#L229 to https://github.com/rails/rails/blob/3ae1e1f3c055fe5b89287fb8e0e5b31416383fb8/activemodel/lib/active_model/dirty.rb#L300:

def attribute_change(attr)
  [changed_attributes[attr], __send__(attr)] if attribute_changed?(attr)
end

vs

def attribute_change(attr)
  [changed_attributes[attr], _read_attribute(attr)] if attribute_changed?(attr)
end

If I put a break point just before expect(@taggable.changes).to eq({'tag_list' => ['awesome, epic', ['one']]}), then @taggable.__send__("tag_list") gives ["one"], whereas @taggable._read_attribute("tag_list") gives nil.

I have no idea why this is :-/

[doits]

I don't exactly understand why the expected output is ["awesome, epic", ["one"]] and not ["awesome, epic", "one"] (no array)

The line https://github.com/doits/acts-as-taggable-on/blob/master/lib/acts_as_taggable_on/taggable/core.rb#L186 calls ActsAsTaggableOn.default_parser.new(new_list).parse which converts "one" to ["one"]

You misunderstood me, I was not thinking about the code but why this is the expected result. I'd think that it should be either [["awesome", "epic"], ["one"]] (both arrays) or ["awesome, epic", "one"](both strings). Not sure why it is expected that it is mixed.

[jkburges]

@doits not sure if you have authorisation to re-run the travis job? It failed due to a timeout (possibly due to rubygems being unavailable?) - I think unrelated to any of the code changes here, but not certain about that.

If and when the build passes, you might want to update the title and/or OP of this PR to indicate that it's now working so as to not discourage any potential upstream reviewers! :-)

[doits]

I just amended the last commit and force pushed it again to start travis.

I'm not 100 % sure about overwriting attribute_change (because this changes it back to the pre 5.2 version for all attributes, and I'd think it was changed with some purpose in rails 5.2), but I did not look further at this.

[rgbskills]

Will this be merged soon ?

[jkburges]

I now get:

NameError: undefined local variable or method `attributes_changed_by_setter'

with rails 5.2.0rc1.

[rgbskills]

Is there any quick fix I can apply to make this work in rails 5.2 until this fix is merged ?

[doits]

I don't use this gem anymore (it often broke on rails major upgrades) and replaced it by my own (simpler) tagging solution (with only features I need). So I cannot update/fix this MR anymore, that's why I closed it.

[jacobwalkr]

My only problem here is that I was relying on users "owning" tags and I don't see that capability in another gem.

Do you still have a copy of that branch, @doits? I'd like to give this a go, rather than trying to implement my own.

[jacobwalkr]

Ah, turns out I can get a reference to your commits! Sorry!

[Fodoj]

@jacobwalkr I've ressurected work on this PR here https://github.com/mbleigh/acts-as-taggable-on/pull/887.

[pioz]

Any update for rails 5.2 compatibility?

[Fodoj]

@pioz see my pr #887. I am pushing some changes, but problem is that rspec still didn't release rails 5.2 version - meaning I had to use github refs instead of rubygems.

[Twistedben]

Thanks @Fodoj . Your branch saved me hours of trouble!

[glendel]

Please go to here : Pull Request #887 to see all the ready to merge and to use solutions to work with Rails 5.2! It is fully tested and ready to go! It is just missing to merge the changes into the main project!