Build versus Create?

rubyonrails-talk
1

Hello, Rails newbie....

How can I go From:

@book = current_user.books.build(params[:book])

TO:

@book = current_user.books.create(params[:book], :instance => current_user.instance_id)

right now I get the following error "wrong number of arguments (2 for 1)"

Thanks

2

Hello, Rails newbie…

How can I go From:

@book = current_user.books.build(params[:book])

TO:

@book = current_user.books.create(params[:book], :instance =>

current_user.instance_id)

right now I get the following error "wrong number of arguments (2 for

1)"

Thanks

I’m assuming that ‘instance_id’ is an attribute of a Book. In that case, it needs to be part of the Hash you pass to #build or #create. You can do either:

params[:book][:instance_id] = current_user.instance_id

@book = current_user.books.create(params[:book])

or

@book = current_user.books.create(params[:book].merge(:instance_id => current_user.instance_id))

Both assume that params[:book] is non-nil and is a Hash. I’d prefer the first - it’s more readable.

Adam

3

Adam,

Thanks for the answer, let me add a little color as to what I'm trying to get working:

I have the following models: Users (id, name, email, instance_id, etc...) Instances (id, domain name) Books (id, name, user_id, instance_id) In Rails 3, When a new book is created, I need the user_id, and instance_id to be populated based on the current_user. Currently, user_id is being assigned when I create a new book but not instance_id?

class Instance < ActiveRecord::Base   has_many :users   has_many :books, :through => :users, :order => "created_at DESC" end

class User < ActiveRecord::Base   belongs_to :instance   has_many :books, :order => "created_at DESC" end

class Book < ActiveRecord::Base   belongs_to :user   has_one :instance, :through => :user end

Thoughts on this? I'm a newbie so I'd love to hear if I going down the right track or not... And I like your first suggestion assuming you like the models above:

4

Just tried: params[:book][:instance_id] = current_user.instance_id @book = current_user.books.create(params[:book])

for some reason, instance_id is still not being inserted into the DB. In the dev log it shows as NULL. Any ideas?

5

There’s a discrepancy there - Book has_one instance, but it has an instance_id. The foreign key goes with a belongs_to relationship.

The Instance is related to the User, so storing it in two places (once on the User, once on the Book) can only cause maintenance problems down the line. Better to store it one place (like on the User, which you do now) and let the Book retrieve its Instance through the User.

In other words, books should not have an instance_id column. It’s unused - the relationship is achieved through the has_one relationship set up in the Book model.

6

Tried this too:

@book = current_user.books.create(params[:book].merge(:instance_id => current_user.instance_id))

Also ended up inserting a NULL into the DB

7

Adam, interesting , what are you thoughts about when I want to return all books by instance_id.. won't that kill the db looking at the user's table?

8

I'm trying to work out your suggestion on how to not have a instance_id in books. can you take a look at let me know what's wrong and if this is what you suggest? thxs!

class Instance < ActiveRecord::Base   has_many :users   has_many :books end

class User < ActiveRecord::Base   belongs_to :instance   has_many :books, :order => "created_at DESC"   has_many :instance_books, :through => :instance, :source => :books,             :order => "created_at DESC" . . end

class Note < ActiveRecord::Base   belongs_to :user   belongs_to :instance end

.... Then to actually get all the notes for instance_id = 1

class NotesController < ApplicationController def index   @books = current_user.instance_books   . end . . end

current_user.instance_books doesn't return the right results. Ideas? thxs

9

You’re right that it would be extra logic in the query. If it will be a common lookup it may be better to store it on the books table as well.

10

I’m trying to work out your suggestion on how to not have a

instance_id in books. can you take a look at let me know what’s wrong

and if this is what you suggest? thxs!

class Instance < ActiveRecord::Base

    has_many        :users

    has_many        :books

end

class User < ActiveRecord::Base

    belongs_to :instance

    has_many   :books, :order => "created_at DESC"

has_many :instance_books, :through => :instance, :source => :books, :order => “created_at DESC”

.

.

end

class Note < ActiveRecord::Base

    belongs_to  :user

    belongs_to  :instance

end

… Then to actually get all the notes for instance_id = 1

class NotesController < ApplicationController

def index

@books = current_user.instance_books

.

end

.

.

end

current_user.instance_books doesn’t return the right results. Ideas?

thxs

I don’t think has_many :through will work that way - it needs a real join table as far as I know. One with two belongs_to relationships. If you’re wanting to keep instance_id on books, your original code can work, but Book belongs_to Instance, not has_one Instance :through User. If you want to ensure the instance is the same as the user’s, you can add a validation.

11

create takes a hash and params[:book] is a hash then you are adding a second hash with :intance => current_user.intance that is why is says you are passing 2 arguments

a good idea try doing this

@book = current_user.book.new(params[:book]) @book = current_user.instance if @book.save blah blah blah …

12

you are using has many throug the wrong way , that is used for many to many associations and i think you only have one to many in every model

and only the models that have belongs_to should have a foreing key

13

Hi radhames, thanks for the feedback. Given that I'm new and the above is not correct, would it be possible to see what you suggest for:

class Instance < ActiveRecord::Base         has_many :users         has_many :books end class User < ActiveRecord::Base         belongs_to :instance         has_many :books, :order => "created_at DESC"         has_many :instance_books, :through => :instance, :source => :books,                                                 :order => "created_at DESC" . . end class Book < ActiveRecord::Base         belongs_to :user         belongs_to :instance end

The relationship goes like this: Instance > User > Book Instance is a collection of user's from a company, based on their domain name example (abc.com) The idea is for only abc.com user's to see their books, which is why the books table has a instance_id value

Recommendations? on the above?

14

Looking forward to learning how to update the models.

Also, for some reason instance_id is not being set in the db with the following:

I checked the logs, it shows "WARNING: Can't mass-assign protected attributes: instance_id"

def create   @book = current_user.books.build(params[:book].merge(:instance_id => current_user.instance_id)) . . end

15

Adding attr_accessible :instance_id to the model gets the value in the db but the instance_id probably shouldn't be accessible as it would allow another instance to possibly steal another user/instance's info, right?

16

You’ll need to handle that through permission checking in the controller and validations in the model and database.

17

class Instance instance has_many :users ## there should not be a user_id field in the instances table

class User belongs_to :instance ## there should be a instance_id field in the table users table has_many :books ## there should not be a book_id field in the table books table

class Book belongs_to :users

to restrict everything just always scope , for example, then search for users dont do

@users = User.all

instead to

@user = current_instance.users

to get a book first get the current user

@book = current_user.book.find(params[:id])

by scoping like this you will always make only abc.com user’s to see their books