Trouble on updating attributes when default accessors are overwritten #14307

Closed
Backoo opened this Issue Mar 7, 2014 · 8 comments

2 participants

@Backoo

I am using Ruby on Rails 4 and I have overwritten some default accessor method this way:

class Article < ActiveRecord::Base
  def title
    self.get_title
  end

  def content
    self.get_content
  end
end

self.get_title and self.get_content methods return some computed value and look like the following (note: has_one_association is a :has_one ActiveRecord::Association)

def get_title
  self.has_one_association.title.presence || read_attribute(:title)
end

def get_content
  self.has_one_association.content.presence || read_attribute(:content)
end

When I find and read @article instances from the database all works as expected: title and content values are respectively outputted with self.has_one_association.title and self.has_one_association.content.

However, I found that when attributes are assigned to an @article then the @article object is not updated as expected. That is, given in my controller I have:

def update
  # params # => {:article => {:title => "New title", :content => "New content"})}

  ...

  # BEFORE UPDATING
  # @article.title   # => "Old title"   # Note: "Old title" come from the 'get_title' method since the 'title' accessor implementation
  # @article.content # => "Old content" # Note: "Old content" come from the 'get_content' method since the 'content' accessor implementation

  if @article.update_attributes(article_params)

    # AFTER UPDATING
    # @article.title   # => "Old title"
    # @article.content # => "Old content"

    ...
  end
end

def article_params
  params.require(:article).permit(:title, :content)
end

Even if the @article is valid it has not been updated in the database (!), I think because the way I overwrite accessors and/or the way Rails would assign_attributes. Of course, if I remove the getter methods then all works as expected.

Is it a bug? How can I solve the problem? Or, should I adopt another approach in order to make what I would like to accomplish?


Source: Stackoverflow.

@carlosantoniodasilva
Ruby on Rails member

You're updating the title and content attributes of the article object, but returning the has_one_association value all the time. These don't get updated unless you override the writers to do so.

The behavior you are seeing seems correct to me.

Also, maybe you don't need the get methods + read_attribute, you can simply call super from the methods:

  def title
    has_one_association.title.presence || super
  end

I'm giving this a close as it doesn't look like an issue. Please let us know if you are still having problems, thanks!

@Backoo

@carlosantoniodasilva , I updated the question for clarity: "Even if the @article is valid it has not been updated in the database (!)".

Looking at the Rails source code I think the problem is somewhere in the storing process flow. In fact:

# params # => {:article => {:title => "New title", :content => "New content"})}

@article.update_attributes(article_params) # => true

does not store data to the database and even the following results the same

@article.assign_attributes(article_params) # => nil
@article.inspect # => #<Article :id => 1 :title => "New title" :content => "New content">
@article.save # => true

You said: "These don't get updated unless you override the writers to do so". So I also tried to state the setter methods but nothing changed.

def title
  has_one_association.title.presence || super
end

def title=(value)
  write_attribute(:title, value)
end

def content
  has_one_association.content.presence || super
end

def content=(value)
  write_attribute(:content, value)
end

The record still is not updated in the database.

By logging in the title (or, alternatively, in the content) setter method this way

def title=(value)
  logger.debug "Title is '#{value.inspect}'"

  write_attribute(:title, value)

  logger.debug "Changes are '#{self.changes.inspect}'"
end

Then in the logger file I get the following output (note: two calls to the setter method and the odd changes):

...
(0.2ms)  BEGIN
Title is 'New title''
Changes are '{"title"=>["Old title", "Old title"]}'
Title is 'Old title'
Changes are '{}'
(0.2ms)  COMMIT
...

As you can note in the above output no SQL UPDATE query runs, probabily because Rails do not recognizes changes.


Maybe a way to solve the problem is to state something in getter methods that return the super value only if the object is about to be updated, otherwise it should return has_one_association.title (or, alternatively, the has_one_association.content). If so, how check that the object is about to be updated?

@Backoo

Maybe a reasonable solution could be this one:

class Article < ActiveRecord::Base
  def title
    self.get_value_for(:title) || read_attribute(:title)
  end

  def content
    self.get_value_for(:content) || read_attribute(:content)
  end

  private

  def get_value_for(attribute_name)
    self.changed? ? nil : self.has_one_association.try(attribute_name).presence
  end
end

Explanation

During the Ruby on Rails storing process flow (that is, when a object is about to be updated or saved) attribute values are read internally by running getter methods. Without the self.changed? check, and since the implementation of overwritten getter methods title and content, reading values will result in returning self.has_one_association.try(attribute_name).presence values instead of the changed ones, so making the object to be set for storage (and so updated) with those values. In order to handle this case it is checked if the object changed and if so self.has_one_association.try(attribute_name).presence values are ignored and nil is returned immediately. It is left to overwritten getter methods to further look for reading attribute values in case the get_value_for method returns nil.


This works for me, but I am not sure if I will have problem in the future and if it is all linear. What do you think about? Any advice / alert?

@carlosantoniodasilva
Ruby on Rails member

It seems I'm still missing the point here: why updating an attribute, lets say title, with value foo, but always returning bar from the association in case it exists? Can you give more context on this please?

Related to your question, no I think relying on changed? is safe, however as I said, it's still unclear to me all this 😄.

@Backoo

@carlosantoniodasilva , for a @article I am trying to update the title attribute because I would like to retrieve and display a "custom" title (that stored in the has_one_association record, if this record exists) instead of that stored in the @article record. However, as I explained in previous comments, I end up with some unexpected behavior during the updating process so I considered to use the solution I provided in the latter comment. At this time my question is related to the "correctness" / "safety" of that solution according to the implementation of the aimed behavior for accessors.

@Backoo

@carlosantoniodasilva , "always returning bar from the association in case it exists" because my application would provide translation data. I tried the Globalize gem but, for more advanced use, I didn't like that so I implemented my custom handler.

@carlosantoniodasilva
Ruby on Rails member

We have a similar problem in the application I'm working on, and we decided not to override the reader methods but provided translated_* methods instead, made things a lot more easier.

@Backoo

@carlosantoniodasilva , by using translated_* methods you make things easier for "general understanding", but it is hard to refactor code mostly in big and already launched application (since refactoring and other "subtle" issues...).

Can you explain or point me to a web resource where I can read advantages and disadvantages of using translated_* methods or overwritring accessors? Thanks in advance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment