Is my routing wrong?

rubyonrails-talk
1

My routing:

resources :incidents do     resource :timesheet do   resource :command_officer     end end

My CommandOfficersController: def new     @command_officer = CommandOfficer.new ..

Now if I want to add a Command Officer to an Incident's Timesheet, I go to: /incidents/41/timesheet/command_officer/new

And I get this error:

NoMethodError in Command_officers#new Showing C:/Rails/costrecovery/app/views/command_officers/_form.html.erb where line #1 raised:

undefined method `command_officers_path' for #<#<Class:0x422f1c0>:0x422cb38>

Extracted source (around line #1): 1: <%= form_for(@command_officer) do |f| %> 2: <% if @command_officer.errors.any? %>

Can anyone explain why I'm getting this error. I've been trying to find out for hours.

2

Finne Jager wrote in post #971120:

My routing:

resources :incidents do     resource :timesheet do       resource :command_officer     end end

My CommandOfficersController: def new     @command_officer = CommandOfficer.new ..

Now if I want to add a Command Officer to an Incident's Timesheet, I go to: /incidents/41/timesheet/command_officer/new

And I get this error:

NoMethodError in Command_officers#new Showing C:/Rails/costrecovery/app/views/command_officers/_form.html.erb where line #1 raised:

undefined method `command_officers_path' for #<#<Class:0x422f1c0>:0x422cb38>

Extracted source (around line #1): 1: <%= form_for(@command_officer) do |f| %> 2: <% if @command_officer.errors.any? %>

Can anyone explain why I'm getting this error. I've been trying to find out for hours.

Since your resources are nested, you need to use the array syntax of form_for so that the proper route is generated.

Best,

3

Since your resources are nested, you need to use the array syntax of form_for so that the proper route is generated.

<%= form_for :command_officer, @command_officer do |f| %>

That one doesn't generate any errors, thanks. But when I fill in the form and press submit, I get:

Routing Error No route matches "/incidents/41/timesheet/command_officer/new"

4

Finne Jager wrote in post #971133:

Since your resources are nested, you need to use the array syntax of form_for so that the proper route is generated.

<%= form_for :command_officer, @command_officer do |f| %>

That one doesn't generate any errors, thanks. But when I fill in the form and press submit, I get:

Routing Error No route matches "/incidents/41/timesheet/command_officer/new"

Nevermind this! I forgot to add the options hash: :url => { :action => "create" }

5

Finne Jager wrote in post #971133:

Since your resources are nested, you need to use the array syntax of form_for so that the proper route is generated.

<%= form_for :command_officer, @command_officer do |f| %>

No! You need the *parent* resource names in there, I think. Rails already knows what kind of resource @command_officer itself is.

That one doesn't generate any errors, thanks. But when I fill in the form and press submit, I get:

Routing Error No route matches "/incidents/41/timesheet/command_officer/new"

Best,

6

Marnen Laibow-Koser wrote in post #971140:

Finne Jager wrote in post #971133:

Since your resources are nested, you need to use the array syntax of form_for so that the proper route is generated.

<%= form_for :command_officer, @command_officer do |f| %>

No! You need the *parent* resource names in there, I think. Rails already knows what kind of resource @command_officer itself is.

Hmm, it seems to work like that without the parent resource. Probably because I have CommandOfficers#create like this?

def create     @timesheet = Incident.find(params[:incident_id]).timesheet

    respond_to do |format|       if @timesheet.create_command_officer(params[:command_officer]) ..

7

I'm having another issue. This is my create method for a Command Officer:

def create   @timesheet = Incident.find(params[:incident_id]).timesheet

    respond_to do |format|       if @timesheet.create_command_officer(params[:command_officer])

This is working fine. But when I use the same code in FireFighter#create:       if @timesheet.create_fire_fighter(params[:fire_fighter])

I get this error: NoMethodError in Fire fightersController#create undefined method `create_fire_fighter' for "#<Timesheet:0x3f8b938>":Timesheet

As far as I know everything is set up identical to Command Officer and I don't know why the create_fire_fighter doesn't work, while it does work for Command Officer?

8

Finne Jager wrote in post #971310:

I'm having another issue. This is my create method for a Command Officer:

def create   @timesheet = Incident.find(params[:incident_id]).timesheet

    respond_to do |format|       if @timesheet.create_command_officer(params[:command_officer])

This is working fine. But when I use the same code in FireFighter#create:       if @timesheet.create_fire_fighter(params[:fire_fighter])

I get this error: NoMethodError in Fire fightersController#create undefined method `create_fire_fighter' for "#<Timesheet:0x3f8b938>":Timesheet

As far as I know everything is set up identical to Command Officer

Obviously it isn't.

and I don't know why the create_fire_fighter doesn't work, while it does work for Command Officer?

You forgot to set up the associations on Timesheet properly. What associations does Timesheet currently have?

Best,

9

You forgot to set up the associations on Timesheet properly. What associations does Timesheet currently have?

class Timesheet < ActiveRecord::Base   belongs_to :incident

  has_one :command_officer   has_one :fire_chief   has_many :fire_fighters

class FireFighter < ActiveRecord::Base   belongs_to :timesheet

10

Finne Jager wrote in post #971314:

You forgot to set up the associations on Timesheet properly. What associations does Timesheet currently have?

class Timesheet < ActiveRecord::Base   belongs_to :incident

  has_one :command_officer   has_one :fire_chief   has_many :fire_fighters

Still using the data model I advised you against, I see...

...but in any case, I think you want @timesheet.fire_fighters.create, not @timesheet.create_fire_fighter , which doesn't exist on has_many.

class FireFighter < ActiveRecord::Base   belongs_to :timesheet

I tested it with the console, Tiemsheet.last.fire_fighters returns an empty array

You do know that Timesheet.last will return an arbitrary timesheet, and that you can't predict which one it returns, right?

Best,

11

Marnen Laibow-Koser wrote in post #971320: ...

You do know that Timesheet.last will return an arbitrary timesheet, and that you can't predict which one it returns, right?

That is not entirely true. It is 'Timesheet.first' that would better be renamed to 'Timesheet.random' :wink: Same goes for Timesheet.find_by_name (that is really "pick an arbitrary entry with that name").

But 'Timesheet.last' actually has a default ordering on 'id DESC'.

I saw that behavior in Rails 2.3.5 and 3.0.3. Here in 3.0.3:

ruby-1.9.2-p136 :013 > Timesheet.first => #<Timesheet id: 1, name: "Jon", date: "2010-12-29", project: "test"> ruby-1.9.2-p136 :014 > Timesheet.last => #<Timesheet id: 4, name: "Jon", date: "2010-12-31", project: "test 4"> ruby-1.9.2-p136 :015 > Timesheet.find_by_name('Jon') => #<Timesheet id: 1, name: "Jon", date: "2010-12-29", project: "test">

yields:

  Timesheet Load (0.2ms) SELECT "timesheets".* FROM "timesheets" LIMIT 1   Timesheet Load (0.2ms) SELECT "timesheets".* FROM "timesheets" ORDER BY timesheets.id DESC LIMIT 1   Timesheet Load (0.2ms) SELECT "timesheets".* FROM "timesheets" WHERE ("timesheets"."name" = 'Jon') LIMIT 1

This caused us quite some fun at work (e.g. flickering tests at best, unpredictable results at worst), so I eventually implemented a ".single" method that barfs loudly if you _expect_ to find only zero or 1 entries for a certain query or in a certain array and then unexpectly, there are more entries there ...

For that reason, at work, I advise against the use of: * .first => replaced by .single * find_by => replaced by find_all_by().single

HTH,

Peter

12

Still using the data model I advised you against, I see...

Yes, unfortunately my boss has been pushing to get a first version online within two weeks. I've decided to stick with this structure for now and then restructure after launch, especially since only 3-4 clients will be using this initially.

...but in any case, I think you want @timesheet.fire_fighters.create, not @timesheet.create_fire_fighter , which doesn't exist on has_many.

Thanks, I did not know that 'create_modelname' was only available with has_one relationships.

I tested it with the console, Tiemsheet.last.fire_fighters returns an empty array

You do know that Timesheet.last will return an arbitrary timesheet, and that you can't predict which one it returns, right?

Yes, I was just using that to test if the association was working correctly.

13

Finne Jager wrote in post #971329:

Still using the data model I advised you against, I see...

Yes, unfortunately my boss has been pushing to get a first version online within two weeks. I've decided to use this current structure for now and then restructure after launch.

I advise against this course of action in the strongest possible terms. Normalizing the schema will be easy. Refactoring the application code will be easy if you have proper tests. But normalizing the data already entered into the denormalized schema will be nearly impossible.

This is one of those things that you want to get right the first time. Normalize your schema. It's easier to denormalize later than to normalize later.

...but in any case, I think you want @timesheet.fire_fighters.create, not @timesheet.create_fire_fighter , which doesn't exist on has_many.

Thanks, I did not know that 'create_modelname' was only available with has_one relationships.

Did you check the docs? :slight_smile:

I tested it with the console, Tiemsheet.last.fire_fighters returns an empty array

You do know that Timesheet.last will return an arbitrary timesheet, and that you can't predict which one it returns, right?

Yes, I was just using that to test if the association was working correctly.

OK. You'd probably be better served to get good automated tests working.

Best,

14

I advise against this course of action in the strongest possible terms. Normalizing the schema will be easy. Refactoring the application code will be easy if you have proper tests. But normalizing the data already entered into the denormalized schema will be nearly impossible.

This is one of those things that you want to get right the first time. Normalize your schema. It's easier to denormalize later than to normalize later.

Ok thanks, I am going to ask my boss to move the launch deadline in order to avoid those problems.