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

Fixed: the redefined methods problem #512

Closed
wants to merge 1 commit into from

Conversation

proAlexandr
Copy link

I have tried to fix problem with redefined attribute methods in a model.
Now, I get problem if I redefine an attribute getter and setter. For example:

def price=(value)
  write_attribute(:price, value / 60)
end

def price
  read_attribute(:price) * 60 
end

Version model use new getter (price) for store a value, but when it restore the value, version don't use my new setter.

@batter
Copy link
Collaborator

batter commented Apr 6, 2015

You shouldn't need the custom setter to be invoked because the getter will read the value out properly. We store the "unserialized" value, and thus that's what gets assigned to the model when reifying, which seems like it should be correct unless I'm mistaken.

If I'm wrong then please provide a test demonstrating what you're trying to address here. As you can see, we already have a test in the suite mimicking the type of thing you're referring to, and it passes with the current master but fails with your "patch" here..

@proAlexandr
Copy link
Author

I get a wrong behaviour when I use multilang-hstore
It uses overwriting default accessors and serialization simultaneously.
I am going to write a test soon.

@batter
Copy link
Collaborator

batter commented Apr 27, 2015

@proAlexandr, here is my proposed fix: f5f310b

It's on the vattr_support branch, so can you try bundling from that branch and seeing if it fixes your test case please?

@batter batter closed this in a8b5d7d Apr 29, 2015
@batter
Copy link
Collaborator

batter commented Apr 29, 2015

Thanks for the PR, and for bringing this to my attention. I made a slight modification so the reify method now handles both normal ActiveRecord attributes and custom defined virtual attributes and setters properly.

batter added a commit that referenced this pull request Apr 29, 2015
…with custom setters in VersionConcern#reify)
@batter batter added this to the 4.0.0 milestone Apr 29, 2015
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