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

Fix small bug with dirty attributes and list assignment as an array. #406

Merged
merged 2 commits into from
Jan 1, 2014

Conversation

rafael
Copy link
Contributor

@rafael rafael commented Sep 18, 2013

I notice that when you are not using the default delimiter and make the assignment with an array, the attributes are marked as dirty when they shouldn't. eg:

ActsAsTaggableOn.delimiter = ';'
a.topic_list 
=>
['topic1', 'topic2'] 
a.topic_list = ['topic1', 'topic2'] 
a.changes
=>
{"topic_list"=>["Politics; Test", ["Politics", "Test"]]} 

This commit attempts to fix this problem.

Cheers!

@bf4
Copy link
Collaborator

bf4 commented Dec 11, 2013

Please rebase off master and force push.

@rafael
Copy link
Contributor Author

rafael commented Dec 11, 2013

(y) I just did that.

@bf4
Copy link
Collaborator

bf4 commented Dec 11, 2013

Would you mind adding a test? Thanks

* When using a delimiter different from "," the dirty attributes
  should remain empty when assigning the topic list with an array:
@rafael
Copy link
Contributor Author

rafael commented Dec 11, 2013

Sure, I wasn't sure where to add this test. Do you think the one I just added is good?

@ghost ghost assigned bf4 Dec 11, 2013
bf4 added a commit that referenced this pull request Jan 1, 2014
[Fix]  dirty attributes incorrect when list assignment as an array and non-comma delimiter
@bf4 bf4 merged commit 41c4751 into mbleigh:master Jan 1, 2014
@bf4
Copy link
Collaborator

bf4 commented Jan 1, 2014

Thanks, merged! Would you be interested in writing a TagList stringify conversion method?

It would be something like this:

module ActsAsTaggableOn
  class TagList < Array
     def self.stringify(tag_list)
       if tag_list.class === Array
         new(tag_list)
       else
        tag_list
       end.to_s
    end
  end
end

In the referenced code it looks like all it really wanted was a string. And a TagList has a to_s that outputs a delimited string, but is_a?(Array) is extra work because TagList.is_a?(Array)

tekniklr pushed a commit to tekniklr/acts-as-taggable-on that referenced this pull request Mar 19, 2021
[Fix]  dirty attributes incorrect when list assignment as an array and non-comma delimiter
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

Successfully merging this pull request may close these issues.

2 participants