Rails controller problems

  1. Users_Controller CRUD expects param[:id] to create User instance.
    With
    Orders_Controller, I’d like to retrieve a list of users who have
    ordered. I
    know Orders_Controller expects param[:id] to be for creating Order
    instance. So does this mean if I want to retrieve a list of users who
    have
    ordered, I should create a method called ‘get_orders’ in
    Users_Controller?

  2. Can user’s id be passed as the param[:id] for controllers other than
    Users_Controller? I find that it seems to make CanCan hard to maintain.

  3. There seem to be too many methods in Users_Controller (e.g.
    deactivate,
    change_role, etc.). How do you organise them/reduce them?

On 1 March 2014 19:20, Brandon [email protected] wrote:

change_role, etc.). How do you organise them/reduce them?
It seems there are a lot of basic things that you have not yet got the
hang of. I suggest that you start by working right through a good
tutorial such as railstutorial.org (which is free to use online) which
will show you the basics of rails, so that you will be able to answer
most questions yourself.

Colin

Thanks for that Colin, after revisiting a few chapters there I
understand
it better now.

But it seems that User controller will be pretty FAT over time.

What are the strategies to DRY it up?

On 3 March 2014 09:18, Brandon [email protected] wrote:

Thanks for that Colin, after revisiting a few chapters there I understand it
better now.

But it seems that User controller will be pretty FAT over time.

Why?

On Mon, Mar 3, 2014 at 5:36 AM, Colin L. [email protected] wrote:

On 3 March 2014 09:18, Brandon [email protected] wrote:

But it seems that User controller will be pretty FAT over time.

Why?

Because User has a classic tendency to become a God Object. :slight_smile:

Brandon, sorry if I’m repeating stuff that came earlier, I didn’t pay
much attention to this thread until the above exchange caught my eye.

The User class tends to accumulate a lot of crud (not to be confused
with CRUD), becoming what we call a God Object, one that sees, knows,
and tells all.

To combat that tendency, remember the Single Responsibility Principle,
and look for clear dividing lines to separate responsibilities. What
I usually do is keep User responsible only for logging in and out, so
it usually contains only username, encrypted password, and (only for
purposes of sending password reset tokens and suchlike) an email
address. Any other personal info, like a “real” name, address, phone
number, biography, picture, etc. goes in a Profile object that
belongs_to that User. Other info for specific aspects of using the
system, like availability and desired salary if it’s a job board, go
in more specific profiles, like JobSeekerProfile or JobPosterProfile.

-Dave


Dave A., the T. Rex of Codosaurus LLC (www.codosaur.us);
FREELANCE SOFTWARE DEVELOPER, AVAILABLE AS OF MARCH 1st 2014;
creator of Pull Request Roulette, at PullRequestRoulette.com.

On Mon, Mar 3, 2014 at 11:14 AM, Brandon [email protected] wrote:

So I assumed that getting an Order list from User
should be a method placed inside Order.

Not necessarily, though it will have to interact with Order at some
point. Getting a User’s Orders should be as simple as user.orders,
assuming orders belong_to users.

Then I noticed it expects to get param[:id] as User.id

Things like this are usually changeable.

so I was tempted to put it back to User (the God
object). How do reckon I solve this Order list problem?

IIRC you were after a list of users who have ordered, right? The
immediately obvious solution would be something like “User.all.select
{ |u| u.orders.any? }”, but that would have horrible performance as it
would fetch the orders for each user in a separate query (N+1
problem). Better would be “User.where(id:
Order.pluck(:user_id).uniq)”, which would do one query to get the IDs
and one to turn them into users. There’s probably also some AR syntax
to have the DB do the uniq’ing for you, without stooping to
hand-rolled SQL, but I forget it offhand.

I’ve been reading a lot about Slim Controller, Fat Model and now looking at
my codebase with new lenses

Here, let me grind down those lenses a bit more. :slight_smile: The models
shouldn’t be horribly fat either, it’s just that it’s a better place
for business logic than the controller, never mind the view. If a
model is getting unwieldy, you can possibly extract a service object,
or a data object, or some other thing, from it. Let the Single
Responsibility Principle be your guide.

-Dave


Dave A., the T. Rex of Codosaurus LLC (www.codosaur.us);
FREELANCE SOFTWARE DEVELOPER, AVAILABLE AS OF MARCH 1st 2014;
creator of Pull Request Roulette, at PullRequestRoulette.com.

Thanks Dave, does that mean I’m no longer a novice since I found God? :slight_smile:

Yes I always though I was doing it correctly by putting everything
related
to say Order into Order. So I assumed that getting an Order list from
User
should be a method placed inside Order. Then I noticed it expects to get
param[:id] as User.id so I was tempted to put it back to User (the God
object). How do reckon I solve this Order list problem?

I’ve been reading a lot about Slim Controller, Fat Model and now looking
at
my codebase with new lenses but still pretty hard to move stuff into
Model.
I saw in one Railscast example, Send_password_reset is inside User.rb.
Then
I went WTF and then soon discovered the Model layer wasn’t meant to map
database and can hold logic. Right now I still have a lot of UserMailer
stuff in my Controller layer. But I see the tunnel light could be from
here:

I’m also exploring model scope and Has_Scope gem to reduce the amount of
methods in Controller. I’m trying to avoid STI and polymorphism stuff
which
seems to have me change current_user paths to some weird stuff.

And then there is ActiveSupport::Concern to extend the Controller

On Mon, Mar 3, 2014 at 4:01 PM, Brandon [email protected] wrote:

This is what my User/Create looks like after rethinking my controller. Does
it need more work to make it slimmer?

I’ve seen (and even made) much worse, but this can be slimmed down
fairly easily. The sign_in and that big if-statement, have nothing to
do with what screen to show next, data to show there other than what’s
already in some already-used model, or other such things that properly
belong in the controller. So, they can be extracted and put into the
User model, though you may need to pass in the current_order_id and
current_follow_id. You’d wind up with something like:

def create
user.updating_password = true
if user.save
user.process_initial_session(current_order_id, current_follow_id)
redirect_back_or root_url, flash => { :success => ‘Welcome!’ }
else
render ‘new’
end
end

where user.process_initial_session (or whatever you choose to call it;
could be welcome, set_up_stuff, link_to_order_or_followers, whatever,
depending what else you may want to put in it) encapsulates all that
extracted stuff.

-Dave


Dave A., the T. Rex of Codosaurus LLC (www.codosaur.us);
FREELANCE SOFTWARE DEVELOPER, AVAILABLE AS OF MARCH 1st 2014;
creator of Pull Request Roulette, at PullRequestRoulette.com.

Thanks Dave, been spending days cleaning up my code based on your
suggestions and pretty proud of it now.

I’ve a dilemma now with CanCan vs Nested Resources in Routes.rb:

In Routes.rb:

resources :users do
resources :orders do
collection do
get :payment_received
end
end
end

In orders_controller.rb:

def payment_received
@user = User.find(params[:user_id])
@orders = Order.where(seller_id: @user.id).order(“id ASC”)
render ‘payment_received’
end

In ability.rb:
can :payment_made, Order, :user_id => user.id

The problem

With the following route:

payment_received_user_orders GET
/users/:user_id/orders/payment_received(.:format)
orders#payment_received

Through CanCan, I can’t seem to enforce the “:user_id => user.id”
whereby
the current_user can only see his own payment_received (based on his
own
user_id) and not someone else’s payment_received.

This is what my User/Create looks like after rethinking my controller.
Does
it need more work to make it slimmer?

Basically current_order_id and current_follow_id returns Session values
from Application Controller. Basically site visitors can click place an
order or follow someone and then after that they create their account as
follows. I used CanCan so the loading and authorization is taken care
of.

def create
user.updating_password = true
if user.save
sign_in user
if current_order_id.present?
order = Order.find(current_order_id)
order.link_user(user)
elsif current_follow_id.present?
other_user = User.find(current_follow_id)
user.follow_mutually(other_user)
elsif user.membership_plan.present?
UserMailer.plan_registered_notify_admin(user).deliver
else
UserMailer.client_registered_notify_admin(user).deliver
end
redirect_back_or root_url, flash => { :success => ‘Welcome!’ }
else
render ‘new’
end
end

Worked it out now. :slight_smile:

  can :payment_received, Order, :*seller_id* => user.id

Found the answer:

  can :payment_made, Order, : *seller_id* => user.id

Then leave payment_received blank or make changes to @orders as follows.

def payment_received
@orders = @orders.order(“id DESC”)
end

This way the user can see his own and not other’s payment_received.