Build versus Create?

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

On Tue, Sep 7, 2010 at 11:02 PM, nobosh [email protected] wrote:

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

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:

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

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?

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

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?

On Tue, Sep 7, 2010 at 11:41 PM, nobosh [email protected] wrote:

instance_id to be populated based on the current_user.
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…

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.

On Wed, Sep 8, 2010 at 12:07 AM, nobosh [email protected] wrote:

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?

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.

On Wed, Sep 8, 2010 at 12:55 AM, nobosh [email protected] wrote:

   belongs_to :instance
   belongs_to  :instance

.
.
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.

validate :instance_is_same_as_user_instance
def instance_is_same_as_user_instance
unless self.user and self.instance == self.user.instance
errors.add(:instance, “must be the same as the instance on this Book’s
User”)

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

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 …

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

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?

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?

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

On Wed, Sep 8, 2010 at 10:43 AM, nobosh [email protected] wrote:

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?

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

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