2

My question is: why doesn't .becomes pass errors over to the new object? Isn't this the expected behaviour?


I have the following single table inheritance classes in a rails app:

class Document < ActiveRecord::Base
  validates :title, :presence => true
end

class LegalDocument < Document
end

class MarketingDocument < Document
end

I want to use the same controller and set of views to edit both LegalDocuments and MarketingDocuments, so I am using DocumentsController < ApplicationController with the following edit and update actions:

def edit
  @document = Document.find(params[:id])
end

def update
  @document = Document.find(params[:id])
  if @document.update_attributes(params[:document])
    redirect_to documents_path, :notice => "#{t(:Document)} was successfully updated."
  else
    render :action => "edit"
  end
end

and the following in my edit view:

<%= form_for @document.becomes(Document) do |f| %>
  <% if f.object.errors.present? %>
    <div class="error_message">
      <h4><%= pluralize(f.object.errors.count, 'error') %> occurred</h4>
    </div>
  <% end %>
  <div>
    <%= f.label :title %>
    <%= f.text_field :title, :class => "inputText" %>
  </div>
  <%= f.submit %>
<% end %>
  • If title is filled in, the documents update correctly.
  • If title is left blank, I am returned to the edit view BUT the error is not shown.

From debugging, I know it's not showing because f.object.errors is nil. However, from debugging, I also know @document.errors is NOT nil, as expected.


My question is: why doesn't .becomes pass errors over to the new object? Isn't this the expected behaviour?

Mike
  • 9,692
  • 6
  • 44
  • 61
  • I should add that I got round the problem by changing the view to use `if @document.errors.present?` but would still like to know why I couldn't use `if f.object.errors.present?` – Mike Dec 08 '11 at 16:57
  • I didn't see your comment. Just updated my answer. – Damien Dec 08 '11 at 17:20

2 Answers2

2

Yes, I noticed that too.

Just change f.object.errors.present? by @document.errors.any? ( or @document.errors.present?).

If you really want to use f.object.errors.present?, write becomes in the controller (both edit and update actions) instead of in the view:

def edit
  @document = Document.find(params[:id]).becomes(Document)
end

def update
  @document = Document.find(params[:id]).becomes(Document)
  # ....
end

And then in the view:

<%= form_for @document do |f| %>
  <% if f.object.errors.present? %>
    <p>Errrorsss....</p>
  <% end %>
  #.....

It happens because the url of the form is build according to @document.becomes(Document) (=> PUT document/:id) but @document is created according to its "true" class (a subclass of Document).

If you have pry (highly recommended), write:

def update
  @document = Document.find(params[:id])
  binding.pry
  # ...
end

And then inspect @document. You will see that @document is an instance of LegalDocument or the other subclass even though you called @document.becomes(Document) in your form.

So in final f.object and @document are not the same.

This explains why you can't see f.object.errors when validation fails.

Edit

The 'best way' to deal with STI and form is NOT to use becomes:

<= form_for @document, url: { controller: 'documents', action: 'update' }, as: :document do |f| %>

  <% if @document.errors.any? %>
  # or if f.object.errors.any?
  # handle validation errors
  <% end %>

  # your form... 

<% end %>

This enables you:

  • to have only one controller (documents_controller)

  • to have only one resource (resources :documents)

  • it keeps trace of your subclasses: a LegalDocument will be store as a LegalDocument. No conversion: You don't have to store its class before the conversion to Document and then reassign it later. Plus, your subclass is available in your form, so you can (let's imagine) build a select for the type.

  • your controller looks cleaner: @document = Document.find params[:id] nothing more. Just like a classic resource.

If you want to share this form across different actions(typically edit and new):

<%= form_for @document, url: { controller: 'media_files', action: action }, as: :media_file do |f| %>%>

# edit.html.erb
<%= render 'form', action: 'update' %>

# new.html.erb
<%= render 'form', action: 'create' %>
Damien
  • 26,933
  • 7
  • 39
  • 40
  • Thanks @Delba. I ended up doing the following in my controller to keep track of the Document type. It works exactly as I hoped: I can show/hide certain fields based on the value of `@document.type`. def edit document = Document.find(params[:id]) type = document.class.name @document = document.becomes(Document) @document.type = type end – Mike Dec 08 '11 at 18:10
  • Good to know! I am going to update my answer because there is a better way to trace the initial subclass value. Stay tuned ;) – Damien Dec 08 '11 at 18:24
1

Pretty much it is a bug and it should work as you initially expected. The following patch to address the issue looks like it was pulled back in October

https://github.com/lazyatom/rails/commit/73cb0f98289923c8fa0287bf1cc8857664078d43

dmcnally
  • 771
  • 5
  • 8
  • That's good to know. I see it's in the master branch of Rails now so I guess it'll be fixed in a future version of Rails. – Mike Dec 08 '11 at 19:42