views:

69

answers:

2

I need help refactoring this multi-loop thing. Here is what I have:

Campaign has_many Contacts
Campaign also has many Models which are templates: (Email, Call, and Letter).

Because I am looking for overdue on each, I created an array called Event which I'd like to loop through that contains ['email', 'call', 'letter'].

I need a list of all the Emails, Calls and Letters that are "overdue" for every Contact that belongs to a Campaign. Overdue is determined by a from_today method which looks at the date the Contact was entered in the system and the number of days that needs to pass for any given Event. from_today() outputs the number of days from today that the Event should be done for a given Contact.

Here is what I've done, it works for all Emails in a Campaign across all contacts. I was going to try to create another each do loop to change the class names.

Wasn't sure where to begin: named_scope, push some things into a method, etcetera, or -- minimum to be able to dynamically change the class names so at least it loops three times across the different events rather than repeating the code three times:

<% @campaigns.each do |campaign| %>
   <h2><%= link_to campaign.name, campaign %></h2>

   <% @events.each do |event| %>
       <%= event %>
       <% for email in campaign.emails %>
          <h4><%= link_to email.title, email  %> <%= email.days %> days</h4>

          <% for contact in campaign.contacts.find(:all, :order => "date_entered ASC" ) %>
             <% if (from_today(contact, email.days) < 0) %>
                <% if show_status(contact, email) == 'no status'%>
                    <p> <%= full_name(contact) %> 
                        is <%= from_today(contact,email.days).abs%> days overdue:
                        <%= do_event(contact, email) %>
                    </p>
                <% end %>
             <% end %>
          <% end %>
       <% end %>
     <% end %>
<% end %>
+1  A: 

I'd put the output for each resource into a partial, like so:

<% @campaigns.each do |campaign| %>
  <h2><%= link_to campaign.name, campaign %></h2>
  <%= render 'events', :events => campaign.events %>
<% end %>

then in app/views/campaigns/_events.html.erb

<% events.each do |event| %>
  <%= event %>
  <%= render 'emails', :emails => event.emails %>
<% end %>

then in app/views/campaigns/_emails.html.erb

<% emails.each do |email| %>
  <h4><%= link_to email.title, email  %> <%= email.days %> days</h4>
  <%= render 'contacts', :contacts => email.contacts.all(:order => "date_entered ASC", :email => email) %>
<% end %>

then in app/views/campaigns/_contacts.html.erb

<% contacts.each do |contact| %>
  <% if (from_today(contact, email.days) < 0) %>
    <% if show_status(contact, email) == 'no status'%>
      <p> <%= full_name(contact) %> 
          is <%= from_today(contact,email.days).abs%> days overdue:
          <%= do_event(contact, email) %>
      </p>
    <% end %>
  <% end %>
<% end %>
Patrick Klingemann
thanks....event is an array of [email, call, letter] so i could cycle through each of the different models but I wasn't able to set it up..how can I do each of these for each model? thanks!
Angela
do you mean an event has an email, a call and a letter? If so, you could make a resource that is called Communications or something similar. Then you would have Event has many Communications and you could do event.communications.each, etc. to loop through the communications.
Patrick Klingemann
Hi, sorry for the confusion. Actually an event isn't a created Resource yet. Event is a concept that Call, Email, and Letter fit into, and I was thinking of needing to cycle through each of these. It is looking like I need to create EVent as a class and then do a Single Table Inheritance?
Angela
+1  A: 

Just to add to Patrick's answer, I would also use the :collection option of render to simplify this a bit further, e.g. have a partial _contact.html.erb to render each contact:

<% if (from_today(contact, email.days) < 0) %>
    <% if show_status(contact, email) == 'no status'%>
      <p> <%= full_name(contact) %> 
          is <%= from_today(contact,email.days).abs%> days overdue:
          <%= do_event(contact, email) %>
      </p>
    <% end %>
  <% end %>
<% end %>

and then render the contacts collection with

= render :partial => "contact", :collection => @contacts

I also wouldn't do a find in the view, instead I would setup all the variables in the controller, and probably move all the conditional code into a helper. It's preferable to keep as much logic as possible out of the views.

Alex - Aotea Studios
How do I call all the different Models classes (email, call, letter)?
Angela
btw, yes, I'd like to move stuff out of the view, but wasn't sure because the variables were dependent on logic that seemed would come from outside of the controller...let me play around with this. thanks.
Angela
@Angela: you can also use the `:locals` option with `render`, e.g. `render :partial => "contact", :collection => @contacts, :locals => { :email => email}`
Alex - Aotea Studios
I see, so that is a way of passing variables into the partial...is that right @alex?
Angela
@Angela: actually, I've just checked, and you don't even need to use `:locals`. Variables defined in the main view are available in the partial as well (same goes for member variables from the controller). So you only need to use `:locals` when you want to make variables available under different names.
Alex - Aotea Studios
interesting...that's good to know, though< i've had some issues with that....so any local varialbes in the view or available in the partial, as well as those from the controller for the main view...thank you @Alex!
Angela
So rather than the "for" statement, I would just say @contacts = campaign.contacts.find(:all, :order => "date_entered ASC" )
Angela
@Alex, btw, is there something I can do to collapse needing *two* if statements? I tried to have an AND conditional but it bombed....I really shouldn't need two IF statements...maybe AND is the wrong conditional?
Angela
hmmm...I moved the same code as described into the partial _contact and it is undefined...may neeed to use :locals after all, checking now...
Angela
Alex - Aotea Studios