My objects are melting on the Rails

April 20, 2007 12:46 PM

Rails, Ruby, objects, models and the OO confusion of someone harmed by Goto as a child. Can ye help?

I'm playing about with a Rails app to get my head properly around OO and MVC, but have hit what I'm sure is a fairly simple snag. Could you clear it up? (I'll put this in newspaper terms so everyone knows what I mean)

In one of my controllers, I have an object which is a list of all the active stories in the system, @stories. Some of the stories will be unpublished as yet. I want to go through the list and only send to the view those stories which have been published, or unpublished but written by the current user.

That sort of processing sounds like it should go in the model, so in my controller I have something like (simplified to bare bones):

@authorised = []
for story in @stories
if story.is_authorised_for(story, current_user)
@authorised << story
end
end
@authorised

and in the story model, I have

def is_authorised_for(story, user)
if story.author == user
return true
else
return false
end
end

My question is, why do I have to pass "story" as a parameter over to the model? That seems wrong, but without it I get a "invalid method author for "{the contents of the story's article}:string" error.

What have I goofed on?

posted by bonaldi to computers & internet (9 answers total)

Consider adding a custom "find" method that embeds that logic, so that your controller only has to do something like

@stories = Stories.findAllPublushedPlusUnpublishedFor(current_user)
posted by dws at 12:59 PM on April 20, 2007

ooh, yeh, that would do it, thanks. However, I'm also interested in the general case of why I have to pass "story" to what in my befuddled brain is the story object.
posted by bonaldi at 1:04 PM on April 20, 2007

at first glance, author == user, or even this.author == user should work since that's in the story model/class file.
The prepended story shouldn't be necessary, but I'm honestly not knowledgeable enough about ruby to comment
posted by dblslash at 1:09 PM on April 20, 2007

Try not passing story and changing the line

if story.author == user

to

if self.author == user

But I would like to agree with dws- you might just put in the story model

def authorized_stories_for(user)
 find_by_user_id(user)
end

Although I believe through the magic of rails, if you have set up your models properly (if you have set up :has_many and whatnot) you can just do this in your controller (assuming 'user' is defined):

@stories = user.stories

And that one line will do the work of all the above code.
posted by JamesToast at 1:29 PM on April 20, 2007

This is untested, but I think the way you set up your models is like this:

Your Story model:

class Story < activerecord::basebr>
  belongs_to :user, :foreign_key=>:author_id
  # your model code here
end

Your User model:

class User < activerecord::basebr>
  has_many :stories
  # your model code here
end

That should let you do something like this (below) in your controllers:

@stories = user.stories

posted by JamesToast at 1:39 PM on April 20, 2007

you shouldn't have to pass story. try this:

def is_authorised_for(user)
if self.author == user
return true
else
return false
end
end

In Ruby, "self" refers to the object calling the method. Note, that this method can be simplified to:

def is_authorized_for(user)
return self.author == user
end

And in your controller code, Enumerables in ruby have a nice method called find_all, which takes a block of code and returns all the elements of the collection which are true for the block. Your code can be simplified to:

return @stories.find_all {|story| story.is_authorised_for(current_user)}

This would be read as, "return all stories where the story is authorized for the current user"

Definitely check out Programming Ruby. The first edition is available free on the web here. It'll be plenty good to get you started and you can pick up the second edition later. It is the bible.

My AIM screenname is AaRdVarK54. Feel free to bug me with more ruby/rails questions. I find it helps me understand things better when I explain them to others.
posted by AaRdVarK at 1:42 PM on April 20, 2007 [1 favorite]

And to echo others in this thread, this way of accessing the records is definitely not best practices, but since you are just learning ruby and rails, it's not too much of a problem. Ideally, you'd have your model relationships set up to do something similar to what JamesToast proposed, and just let ActiveRecord and the database sort it all out.
posted by AaRdVarK at 1:49 PM on April 20, 2007

Self! Of course. Thanks very much guys. AaRdVarK I may well take you up on the IM thing, thanks a lot.
posted by bonaldi at 2:41 PM on April 20, 2007

Check out A Little Ruby, a Lot of Objects for some basic OOP in Ruby (emphasis on the OOP) clues.
posted by Zed_Lopez at 8:44 AM on April 22, 2007