Questioning My Design

Rails
1

Hi All,

I’m writing a web app for a non-profit NGO that helps farmers in a co-
op plan their crops better to reduce shortages, surpluses and
unhealthy competition between themselves. Right now, I’m doing
something that doesn’t feel Rails-ee and want some feedback. Here’s
the problem.

Here’s the set up: the farmers are Users with a ‘farmer’ role. I want
to all the farmer to view their account to view their User profile
(name, phone, email, etc.) as well as their Sales, Purchases, and the
Crops they are currently managing. (Sales and Purchases are actually
both Orders handled polymorphically, which might not be important for
this discussion).

In the view for User#show I have partial that displays some links that
go back to the Show action but have set a param that gives context to
which chunk of information the person wants to see, such as:

  1. <%= link_to 'Profile', user_path(current_user, :ui_selector => 'profile') %>
  2. <%= link_to 'Sales', user_path(current_user, :ui_selector => 'seller') %>
  3. <%= link_to 'Purchases', user_path(current_user, :ui_selector => 'buyer') %>
  4. <%= link_to 'Current Crops', user_path(current_user, :ui_selector => 'crops') %>

You can probably tell where this question is going… Inside the
controller, User#show I set instance variables based on the value of
params[:ui_selector], like thus:

@crop = Crop.new(:user_id => @user.id) if params[:ui_selector] ==
‘crops’
@order = Order.new(:buyer_id => @user.id) if params[:ui_selector] ==
‘buyer’
@order = Order.new(:seller_id => @user.id) if params[:ui_selector] ==
‘seller’

Then, also in the view User#show, I show the right stuff based on
params[:ui_selector], like this:

<% if params[:ui_selector] == ‘profile’ %>

Name: <%= @user.name %>
Email: <%= @user.email %>
Phone: <%= number_to_phone @user.cell %>

<% elsif params[:ui_selector] == ‘seller’ %>
<%= render :partial => “sales”, :object => @user %>

<% elsif params[:ui_selector] == ‘buyer’ %>
<%= render :partial => “purchases”, :object => @user %>

<% else params[:ui_selector] == ‘crops’ %>
<%= render :partial => “crops”, :object => @user %>

<% end %>

The sales, purchases and crops partials set up a table and then
basically call partials for those classes indexes using @user.sales,
@user.purchases and @user.crops respectively.

It is all working okay, but alarms go off every time I use logic
blocks in Rails, like there has got to be a better way, I’m missing
something obvious. It doesn’t feel very OOP nor DRY but I can’t see
what I’m missing.

Also, is it bad practice to have the views use the collections of an
instance variable as I have done here with the partials accessing
@user.sales, for instance? It seems like I should be setting maybe a
@sales in the controller, or is the tax on the database the same?
(Come to think of it, I’ll probably need to move that into the
controller if I want to use will_paginate with these.)

Thanks,
Dee
P.S. After we get this rolled out in about two-months time, I’ll throw
it onto github and make it open-source.

2

On May 6, 2010, at 11:04 AM, Dee wrote:

Dee
Still perfectly RESTful. You’d end up with a url path like:

/user/:id/crops

Which would route to your new ‘crops’ action in the UserController

There’s nothing magical about the typical seven actions: index, new,
create, show, edit, update, destroy

You’re just adding a few that return a different representation of the
state of the resources related to a user.

-Rob

3

On May 6, 2010, at 9:34 AM, Dee wrote:

(name, phone, email, etc.) as well as their Sales, Purchases, and the
Crops they are currently managing. (Sales and Purchases are actually
both Orders handled polymorphically, which might not be important for
this discussion).

In the view for User#show I have partial that displays some links that
go back to the Show action but have set a param that gives context to
which chunk of information the person wants to see, such as:

Perhaps it would be easier if you had different actions/views for each:

map.resources :user, :member => { :profile => :get, :seller => :get,
:buyer => :get, :crops => :get }

  1. <%= link_to 'Profile', user_path(current_user, :ui_selector => 'profile') %>

profile_user_path(current_user)

params[:ui_selector], like thus:

@crop = Crop.new(:user_id => @user.id) if params[:ui_selector] ==
‘crops’

Then have a separate action for each:

def crops
@crops = @user.crops.current
end

Assuming that you have an association and named_scope such as:
class User
has_many :crops
end
class Crop
belongs_to :user
named_scope :current, { :conditions => [‘season = ?’,
season_for(Date.today)] }
end

(Of course, I’m assuming that you have a definition of what “Current
Crops” are somewhere.)

Your links look like you are creating a crop, sale, purchase, or
profile, but the text seems like you mean to see the current status of
these things.

Email: <%= @user.email %><br />

<% end %>

You would then have separate views for each action. You can call out
to a partial if you do have common stuff (such as an order).

Also, is it bad practice to have the views use the collections of an
instance variable as I have done here with the partials accessing
@user.sales, for instance? It seems like I should be setting maybe a
@sales in the controller, or is the tax on the database the same?
(Come to think of it, I’ll probably need to move that into the
controller if I want to use will_paginate with these.)

Thanks,
Dee
P.S. After we get this rolled out in about two-months time, I’ll throw
it onto github and make it open-source.

Good Luck!

-Rob

Rob B.
http://agileconsultingllc.com
[email protected]

[email protected]

4

Ah ha! That does make sense, Rob! I’ve had similar problems before but
haven’t considered additional member functions. You are correct in
assuming that I have those associations and named_scopes. The “current
crops” is actually a named scope as is past crops, and more
collections associated with the User, which I left out for brevity.

By adding these additional members what does that do to its
RESTfulness?

Thanks,
Dee

On May 6, 10:32 am, Rob B. [email protected]