Sijo Kg wrote:
I have in the controller
I'm going to give your code a slightly aggressive review.
Firstly, I don't know if our e-mail software took out your indentation, but you need to indent your blocks, with spaces (and never tabs). def and end should be over 2, and the lines inside them should be over 4.
def show_details
@sd_ticket=ServiceDeskTicket.find(params[:id])
In a controller, only use @ if you actually need a variable to live longer than the current method.
@sd_attachments=ServiceDeskAttachment.find_all_by_service_desk_ticket_id(params[:id])
Always put the correct amount of spaces around operators like =, for consistent readability.
Next, don't use shortcuts or acronyms for variable names. You don't need 'sd_' as a "registered package prefix" for "show_details", because your variables should either be for everyone, or they should not use @.
Next, ServiceDeskTicket should use a 'has_many :service_desk_attachments' directive. Then instead of '@sd_attachments = ...', you can just say ticket.service_desk_attachments.
@servicedesk_cis_join = @sd_ticket.service_desk_cis
@sd_cis = @servicedesk_cis_join.map(&:ci)
@sd_cis_association_type_ids =
@servicedesk_cis_join.map(&:service_desk_ci_association_type_id)
The same critique. All these associations should be declared inside the model, using either directives like 'has_many' or 'belongs_to', or even simple methods that return fully cooked data. Then the controller only needs to call the few methods it needs.
And I suspect your type_id could go into a polymorphic Model, for even more transparency. That's the root principle of Object Oriented Programming, so it's also frequently a good idea!
And write unit tests for everything, too!
end
And the view show_details contains many partial files in tabs for
displayig each of the above like
<li class="selected"><a href="#" rel="view_sd_details">Details</a></li>
<li><a href="#" rel="view_sd_attachment_details">Attachments</a></li>
<li><a href="#" rel="view_sd_ci_details">CIs</a></li>
<div id="view_sd_details" class="tabcontent">
<%= render :partial=>'service_desk_part/view_sd_details' %>
</div>
<div id="view_sd_attachment_details" class="tabcontent">
<%= render :partial => 'service_desk_part/view_sd_attachment_details' %>
</div>
<div id="view_sd_ci_details" class="tabcontent">
<%= render :partial => 'service_desk_part/view_sd_ci_details' %>
</div>
Good! Partials tame RHTML!
In _view_sd_attachment_details.rhtml a for loop there like following(I
have included only a small portion of the code)
<% for sd_attachment in @sd_attachments %> //sd_attachment contains say
100items
<td><%= sd_attachment.created_on %></td>
You might also try render :partial ... :collection.
What i need is pagination for the above
_view_sd_attachment_details.rhtml.And here view_sd_details is
same in all pages like a header information..For example
<tr>
<th width=""><label for="" >Description</label></th>
I thought tr/th should be inside a thead, right?
<td width=""><%=h @sd_ticket.description %></td>
</tr>
How can i do this with custom pagination..And if I use Ajax can I avoid
show_details method executing information for view_sd_details again and
again..And how can I do that....(The same thing to view_sd_ci_detils tab
also)
Try this (roughly):
render :partial => 'header',
:locals => {
:label => 'Attachments',
:ticket => @sd_ticket,
:rows => proc{
render :partial => 'row',
:collection => @sd_ticket.attachments }
}
Then inside _header.rhtml use something like...
<thead>
<tr><th><%= label %>...</th></tr>
</thead>
<tbody>
<%= rows.call %>
</tbody>
Now _header.rhtml is completely reusable, and you reuse it by writing that big render... line again, and changing the values of :label and :rows.
BTW I don't see no Ajax around here. Good thing, too, because simple reports and partials should all be raw HTML, right?
