Help setting up relationships needed

Rails
1

This is a bit of a long question, but to those of you with some
experience, it should be fairly simple I think.

I have a notes page that should list all notes the user has entered for
all books chronologically like this.
Book One: This is the note.
Book Four: This is the note.
Book One: A different note, entered later.

Right now my models are:
User
has_many :books
Book
has_many :notes
belongs_to :user
Note (has column book_id)
belongs_to :book
has_one :book

The NotesController has:
@books = current_user.books.find(:all)
@notes = @books.notes

In my views/notes/list.rhtml I can get all the notes fine, but what I
can’t get is what book they belong to.
<% for note in @notes %>
Doesn’t work: <%= note.book.title %>
Works: <%= note.description %>

I can’t figure out a clean way to display which book a note belongs to.

Any help on correctly setting up my models or how the controller should
work is much appreciated.

2

I see a couple newbie errors…

  1. I don’t think you should have both a belongs_to and has_one from Note
    to Book - if Book has_many :notes, then Note belongs_to :book and that’s
    all you need. Having both is probably what’s messing up the list view
    (among other things).

  2. @books.notes isn’t going to work. You need to either enumerate the
    @books collection and get the notes from each book (an inject block is a
    good option for that), or craft a special query to gather all the notes
    for a user’s books. If you want it to look pretty and are using Edge
    Rails (or soon, 1.1), you can put it in an association extention so you
    can use the books.notes syntax.

–josh
http://blog.hasmanythrough.com

Marcus V. wrote:

This is a bit of a long question, but to those of you with some
experience, it should be fairly simple I think.

I have a notes page that should list all notes the user has entered for
all books chronologically like this.
Book One: This is the note.
Book Four: This is the note.
Book One: A different note, entered later.

Right now my models are:
User
has_many :books
Book
has_many :notes
belongs_to :user
Note (has column book_id)
belongs_to :book
has_one :book

The NotesController has:
@books = current_user.books.find(:all)
@notes = @books.notes

In my views/notes/list.rhtml I can get all the notes fine, but what I
can’t get is what book they belong to.
<% for note in @notes %>
Doesn’t work: <%= note.book.title %>
Works: <%= note.description %>

I can’t figure out a clean way to display which book a note belongs to.

Any help on correctly setting up my models or how the controller should
work is much appreciated.

3

Marcus - I didn’t know you could do such an assignment:

@notes = @books.notes

Since “@books” is a collection of book objects, and each “book” in
“@books” should have a collection of “notes”, this seems invalid.

Can you please clarify?

4

Brian V. Hughes wrote:

book_ids = @books.collect { |b| b.id} # this gets you a list of book ids
@notes = Note.find( :all,
:include => :book,
:order => “notes.created_on”,
:conditions => [“notes.book_id in ?”, book_ids])

Nice response, Brian. Better than I was able to do with my groggy brain
this morning :slight_smile: But I’d suggest a different approach to yours. What you
are doing is essentially a manual join, and I’d prefer to let the
database do that for me. It’s safer, as with your approach you could
have a stale list of book ids that is no longer valid by the time you
run the notes query. Try this instead:

In User class…
def notes
Notes.find(:all,
:select => ‘notes.*’,
:joins => ‘INNER JOIN books ON books.id = notes.book_id’,
:conditions => “books.user_id = #{self.id}”,
:readonly => false)
end

Of course, the simplest way to do this is to use Edge Rails has_many
:through associations (coming soon in 1.1):

class User < ActiveRecord::Base
has_many :books
has_many :notes, :through => :books, :order => “created_at”

Then you can just say @user.notes and be done with it.

–josh
http://blog.hasmanythrough.com

5

Josh S. wrote:

Of course, the simplest way to do this is to use Edge Rails has_many
:through associations (coming soon in 1.1):

class User < ActiveRecord::Base
has_many :books
has_many :notes, :through => :books, :order => “created_at”

Then you can just say @user.notes and be done with it.

Oops, forget about that. You can’t use :through in this case, as the
association from books to notes is backwards from what you need for
:through. My bad.

–josh

6

Marcus V. wrote:

This is a bit of a long question, but to those of you with some
experience, it should be fairly simple I think.

This is a bit of a long reply, but I tried to keep my solution as close
to the
“Rails-way” as I could. :slight_smile:

I have a notes page that should list all notes the user has entered for
all books chronologically like this.
Book One: This is the note.
Book Four: This is the note.
Book One: A different note, entered later.

Hmm… that’s kind of a strange way to display the notes that user has
entered,
but it’s not too difficult to get what you want.

Right now my models are:
User
has_many :books
Book
has_many :notes
belongs_to :user
Note (has column book_id)
belongs_to :book
has_one :book

As Josh mentioned in his reply, you have to remove the “has_one :book”
from the
Note model class. Since there’s no note_id column in the books table,
the
association you’re specifying is invalid.

The NotesController has:
@books = current_user.books.find(:all)
@notes = @books.notes

Also, as Josh mentioned, @books is an array, it won’t have a notes
method for
you to access. Second, you don’t need to do a find on the
current_user.books,
since you want all of their books. So that line can just be:

@books = current_user.books

I would also recommend that when you do the find to set current_user,
that you
add an “:include => :books” option to the find, so that you get the
books in the
first SQL call. But you don’t have to do that for “current_user.books”
to get
you the @books collection.

Finally, each item in the @books array, which is a book model item, will
have a
notes method, but you don’t actually want to loop over them, because
those notes
won’t be in the order that you specified above. They will be tied to the
order
of the books that were found.

To get the notes in a special order you need to perform a find on the
Note
class, so you can specify a sort order for the notes. Here’s how I would
probably do it, assume these lines of code come after my revised @books
statement:

book_ids = @books.collect { |b| b.id} # this gets you a list of book
ids
@notes = Note.find( :all,
:include => :book,
:order => “notes.created_on”,
:conditions => [“notes.book_id in ?”, book_ids])

What this Find does, it bring back all the notes for the current user,
since we
only find the notes for the current user’s books, and sorts them by the
date/time each note was created. In addition, it tells ActiveRecord to
also grab
the book information for each note found.

In my views/notes/list.rhtml I can get all the notes fine, but what I
can’t get is what book they belong to.
<% for note in @notes %>
Doesn’t work: <%= note.book.title %>
Works: <%= note.description %>

I can’t figure out a clean way to display which book a note belongs to.

Now, using the @notes collection returned by the custom Find, you should
be able
to use your views/notes/list.rhtml template as you have it defined
above.

There are probably other ways to arrive at the same thing, but what I
put above
is definitely how I would go about getting the notes for a given user,
in their
creation order, with each note’s associated book info. It’s not the most
efficient way of working with your model, since it means duplicating the
book
info for every book that has more than one note, but it will give you
the result
you said you were looking for…

-Brian

7

Wow! Thanks to both of you for your help. It’s great to come back to
such helpful responses.

I tried both methods–both worked. In the end, I changed my User model
as Josh suggested.

It’s funny you should mention the :through method in edge. I upgraded a
couple days ago hoping that would solve the problem but obviously it
didn’t work in this instance :slight_smile:

Again, thank you both for your responses–I learned a lot from both
methods.

-Marcus

Josh S. wrote:

Josh S. wrote:

Of course, the simplest way to do this is to use Edge Rails has_many
:through associations (coming soon in 1.1):

class User < ActiveRecord::Base
has_many :books
has_many :notes, :through => :books, :order => “created_at”

Then you can just say @user.notes and be done with it.

Oops, forget about that. You can’t use :through in this case, as the
association from books to notes is backwards from what you need for
:through. My bad.

–josh

8

Josh S. wrote:

In User class…
def notes
Notes.find(:all,
:select => ‘notes.*’,
:joins => ‘INNER JOIN books ON books.id = notes.book_id’,
:conditions => “books.user_id = #{self.id}”,
:readonly => false)
end

I like the idea of making this a method of the User model class, but
what you’ve
done here is chop up what would normally be a find_by_sql, so that it
sort of
reads like a normal ActiveRecord find. Especially with using both
:select (not
sure why, actually) and :join. But I actually think what you’ve got
(while it
might be technically “more correct” than my solution) is a good deal
more
cumbersome than the style that I used.

It also forces the programmer to have to know that an inner join, not an
outer
join, is the “right” way to do the find. That kind of database detail is
what
you should use :include to handle for you. In fact, I think your method
and my
method can be combined to produce a better looking, and reading,
solution to the
problem:

def notes
Notes.find(:all,
:order => “notes.created_on”,
:include => :book,
:conditions => [“books.user_id = ?”, self.id],
:readonly => false)
end

Won’t this find do the same as yours, and be easier to maintain from a
Rails
standpoint? Also, I’m not sure that you actually need to specify the
:readonly
option, especially since you have it set to false, which is its default
value.

-Brian

9

Josh S. wrote:

I think it’s interesting how many ways there are to come at this. I did
actually work out the find_by_sql approach first, but then opted for the
“choppy” approach because it seemed more idiomatic for Rails. To address
your questions (and I’m more than happy to hear if my reasoning is
poor)…

Wow… I was right about your find really being a find_by_sql call.
Cool. :slight_smile:

I opted for :joins instead of :includes because I don’t want to prefetch
the book data, but only to get the notes. The :select option is to limit
the returned fields to just those in the notes table. If you don’t do
that, you’ll get all the joined fields too.

But the key thing the OP was looking for was a way to display notes for
a users,
in chronological order, with their associated book information. Which
makes
:include the perfect choice, IMO, since pre-fetching the book data for
each note
(while somewhat inefficient based on the model) is still likely to be
more
efficient than making the n+1 database calls.

The default value for :readonly is true when you set :joins because you
normally get the joined fields back too and AR can’t write mixed records. You
don’t need to muck with :readonly in your version as you’re using :includes
instead of :joins.

Ah… OK. Now that makes sense.

Note
belongs_to :user
belongs_to :book

Book
has_many :notes
has_many :users, :through :books

This way you get all the magic for free.

Once again, my obsession with has_many :through pays off!

This is an interesting idea, but if a Book never has more than one User,
then
you’re doing a whole lot of additional database work for very little
benefit.
Even though has_many :through makes working with HABTM models a lot
easier,
shouldn’t you still be starting from a HABTM model? If not, this might
be the
first case I’ve seen of refactoring into a more complex database
model… ;->

-Brian

10

Brian V. Hughes wrote:

But the key thing the OP was looking for was a way to display notes for a users,
in chronological order, with their associated book information. Which makes
:include the perfect choice, IMO, since pre-fetching the book data for each note
(while somewhat inefficient based on the model) is still likely to be more
efficient than making the n+1 database calls.

True, that. For that purpose you’re absolutely right. We don’t know if
there are other use cases where the access from users to notes doesn’t
need the book information. If that never happens your :includes approach
is a clear winner, otherwise it could end up being a bit of a dog.

Note
belongs_to :user
belongs_to :book

Book
has_many :notes
has_many :users, :through :notes # [corrected]

This is an interesting idea, but if a Book never has more than one User, then
you’re doing a whole lot of additional database work for very little benefit.
Even though has_many :through makes working with HABTM models a lot easier,
shouldn’t you still be starting from a HABTM model? If not, this might be the
first case I’ve seen of refactoring into a more complex database model… ;->

LOL. OK, we don’t know if Marcus’ system would want multiple users to be
able to annotate the same book, but I see your point. By the way, nice
discussion.

–josh
http://blog.hasmanythrough.com

11

Brian V. Hughes wrote:

Josh S. wrote:

In User class…
def notes
Notes.find(:all,
:select => ‘notes.*’,
:joins => ‘INNER JOIN books ON books.id = notes.book_id’,
:conditions => “books.user_id = #{self.id}”,
:readonly => false)
end

I like the idea of making this a method of the User model class, but
what you’ve
done here is chop up what would normally be a find_by_sql, so that it
sort of
reads like a normal ActiveRecord find. Especially with using both
:select (not
sure why, actually) and :join. But I actually think what you’ve got
(while it
might be technically “more correct” than my solution) is a good deal
more
cumbersome than the style that I used.

It also forces the programmer to have to know that an inner join, not an
outer
join, is the “right” way to do the find. That kind of database detail is
what
you should use :include to handle for you. In fact, I think your method
and my
method can be combined to produce a better looking, and reading,
solution to the
problem:

def notes
Notes.find(:all,
:order => “notes.created_on”,
:include => :book,
:conditions => [“books.user_id = ?”, self.id],
:readonly => false)
end

Won’t this find do the same as yours, and be easier to maintain from a
Rails
standpoint? Also, I’m not sure that you actually need to specify the
:readonly
option, especially since you have it set to false, which is its default
value.

I think it’s interesting how many ways there are to come at this. I did
actually work out the find_by_sql approach first, but then opted for the
“choppy” approach because it seemed more idiomatic for Rails. To address
your questions (and I’m more than happy to hear if my reasoning is
poor)…

I opted for :joins instead of :includes because I don’t want to prefetch
the book data, but only to get the notes. The :select option is to limit
the returned fields to just those in the notes table. If you don’t do
that, you’ll get all the joined fields too. The default value for
:readonly is true when you set :joins because you normally get the
joined fields back too and AR can’t write mixed records. You don’t need
to muck with :readonly in your version as you’re using :includes instead
of :joins.

Hmm. I just had a bit of a brainstorm. I think this would all work out
better with a slight refactoring of the data model. If you really care
so much about finding a user’s notes, that relationship should be
expressed as an association. So try turning a Note into a join model for
User and Book using has_many :through.

User
has_many :notes
has_many :books, :through :notes

Note
belongs_to :user
belongs_to :book

Book
has_many :notes
has_many :users, :through :books

This way you get all the magic for free.

Once again, my obsession with has_many :through pays off!

–josh
http://blog.hasmanythrough.com