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!
<%= render :partial=>‘service_desk_part/view_sd_details’ %>
<%= render :partial => 'service_desk_part/view_sd_attachment_details' %>
<%= render :partial => 'service_desk_part/view_sd_ci_details' %>
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
<%= sd_attachment.created_on %>
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
Description
I thought tr/th should be inside a thead, right?
<td width=""><%=h @sd_ticket.description %></td>
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…
<%= label %>...
<%= rows.call %>
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?
–
Phlip
O'Reilly Media - Technology and Business Training