momchenr
momchenr

Reputation: 275

Rails Scope, Helper Method?

I have three models. One is an Employee, one is an Item, and one is a Transaction that belongs to both Employee and Items. It's a simple app that allows Employees to check in and check out items - 'Transaction' has a boolean column for checked-in/checked-out.

What I'm trying to do is show within the employee/show view the current list of Items that an Employee has checked out. This is some rough code that I sketched out, but I'm not sure that it's going to work, and I was told not to use a lot of nested conditionals in my views anyway.

    <% if @employee.transactions.exists? %>
        <h3>Currently Checked-OUT Items</h3>
        <table>
            <tr>
                <th>Item Asset Tag</th>
                <th>Item Description</th>
            </tr>
        <% @employee.transactions.each do |transaction| %>
            <% if item.transaction.last? && transaction.status == false %>
                <tr>
                    <td><% transaction.assettag %></td>
                    <td><% transaction.description %></td>
                </tr>
            <% else %>
            NO CHECKED OUT ITEMS
            <% end %>
        </table>
        <% end %>
    <% end %>   

Basically, I'm trying to:

Is this a better job for a scope within the Transaction model, or a helper method? I've never used either, I'm really new at rails.

Upvotes: 0

Views: 1620

Answers (2)

RadBrad
RadBrad

Reputation: 7304

First, you are on the right track. When views get ugly and hard to read because of extensive embedded ruby conditionals and such, think about moving the logic into a helper.

If you have a typical rails app, you'll already have app/helpers/application_helper.rb

So you could just create a helper in that file

def make_employee_list(employee)

   if employee.transactions.exists?
     content_tag(:div) do
       content_tag(:h3, "Currently Checked-OUT Items")
       content_tag(:table) do
         employee.transactions.each do |transaction|
           #  you get the idea
         end
       end
     end
   end
end

Then in your view you could do this:

<%= make_employee_list(@employee) %>

Upvotes: 0

Mike Szyndel
Mike Szyndel

Reputation: 10592

You should do a couple of things in here.

First - create a scope that will fetch last item transaction for you. There's no point in going through al item transactions if you're interested in the last one only, right?

Second, use partials. In this example it's hard to show how I would refactor code to use them (some things doesn't make sense here, ex. where does item variable come from?)

Scope example (take last transaction)

@item.transactions.order('created_at DESC').first

You can as well add scopes for checkin / checkout

class Transaction
    scope :checkin, -> { where(status: true) }
    scope :checkout, -> { where(status: false) }
end

Upvotes: 1

Related Questions