diff options
| -rw-r--r-- | app/controllers/request_controller.rb | 19 | ||||
| -rw-r--r-- | app/models/incoming_message.rb | 109 | ||||
| -rw-r--r-- | app/views/request/_bubble.rhtml | 12 | ||||
| -rw-r--r-- | app/views/request/_correspondence.rhtml | 4 | ||||
| -rw-r--r-- | config/routes.rb | 3 | ||||
| -rw-r--r-- | todo.txt | 2 |
6 files changed, 128 insertions, 21 deletions
diff --git a/app/controllers/request_controller.rb b/app/controllers/request_controller.rb index 46a71d78f..40eb48ccb 100644 --- a/app/controllers/request_controller.rb +++ b/app/controllers/request_controller.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: request_controller.rb,v 1.38 2008-01-14 11:02:49 francis Exp $ +# $Id: request_controller.rb,v 1.39 2008-01-18 03:30:21 francis Exp $ class RequestController < ApplicationController @@ -145,5 +145,22 @@ class RequestController < ApplicationController end end + # Download an attachment + def get_attachment + @incoming_message = IncomingMessage.find(params[:incoming_message_id]) + @info_request = @incoming_message.info_request + if @incoming_message.info_request_id != params[:id].to_i + raise sprintf("Incoming message %d does not belong to request %d", @incoming_message.info_request_id, params[:id]) + end + @part_number = params[:part].to_i + + @attachment = IncomingMessage.get_attachment_by_url_part_number(@incoming_message.get_attachments_for_display, @part_number) + response.content_type = 'application/octet-stream' + if !@attachment.content_type.nil? + response.content_type = @attachment.content_type + end + render :text => @attachment.body + end + private end diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index 32e5a6073..4bdb7323b 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -20,7 +20,20 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: incoming_message.rb,v 1.27 2008-01-10 19:59:33 francis Exp $ +# $Id: incoming_message.rb,v 1.28 2008-01-18 03:30:21 francis Exp $ + +module TMail + class Mail + attr_accessor :url_part_number + + def self.get_part_file_name(part) + file_name = (part['content-location'] && + part['content-location'].body) || + part.sub_header("content-type", "name") || + part.sub_header("content-disposition", "filename") + end + end +end class IncomingMessage < ActiveRecord::Base belongs_to :info_request @@ -32,6 +45,34 @@ class IncomingMessage < ActiveRecord::Base has_many :outgoing_message_followups, :class_name => OutgoingMessage + # Number the attachments in depth first tree order, for use in URLs. + def after_initialize + if !@mail.nil? + @count_parts_count = 0 + count_parts_recursive(self.mail) + end + end + def count_parts_recursive(part) + if part.multipart? + part.parts.each do |p| + count_parts_recursive(p) + end + else + @count_parts_count += 1 + part.url_part_number = @count_parts_count + end + end + # And look up by URL part number + def self.get_attachment_by_url_part_number(attachments, found_url_part_number) + @count_parts_count = 0 + attachments.each do |a| + if a.url_part_number == found_url_part_number + return a + end + end + return nil + end + # Return the structured TMail::Mail object # Documentation at http://i.loveruby.net/en/projects/tmail/doc/ def mail @@ -99,34 +140,44 @@ class IncomingMessage < ActiveRecord::Base return text end - # Returns body text from main text part of email, converted to UTF-8 - def get_main_body_text - # XXX make this part scanning for mime parts properly recursive, - # allow download of specific parts, and always show them all (in - # case say the HTML differs from the text part) - if self.mail.multipart? - if self.mail.sub_type == 'alternative' + # Flattens all the attachments, picking only one part where there are alternatives. + # (This risks losing info if the unchosen alternative is the only one to contain + # useful info, but let's worry about that another time) + def get_attachment_leaves + return get_attachment_leaves_recursive(self.mail, []) + end + def get_attachment_leaves_recursive(curr_mail, leaves_so_far) + if curr_mail.multipart? + if curr_mail.sub_type == 'alternative' # Choose best part from alternatives best_part = nil - mail.parts.each do |m| + self.mail.parts.each do |m| # Take the first one, or the last text/plain one + # XXX - could do better! if not best_part best_part = m elsif m.content_type == 'text/plain' best_part = m end end - text = best_part.body - text_charset = best_part.charset + leaves_so_far += get_attachment_leaves_recursive(best_part, []) else - # Just turn them all into text using built in - text = self.mail.body - text_charset = self.mail.charset + # Add all parts + curr_mail.parts.each do |m| + leaves_so_far += get_attachment_leaves_recursive(m, []) + end end else - text = self.mail.body - text_charset = self.mail.charset.to_s + leaves_so_far += [curr_mail] end + return leaves_so_far + end + + # Returns body text from main text part of email, converted to UTF-8 + def get_main_body_text + main_part = get_main_body_text_part + text = main_part.body + text_charset = main_part.charset # Charset conversion, turn everything into UTF-8 if not text_charset.nil? @@ -139,6 +190,32 @@ class IncomingMessage < ActiveRecord::Base return text end + # Returns part which contains main body text + def get_main_body_text_part + leaves = get_attachment_leaves + + # Find first part which is text + leaves.each do |p| + # XXX do we need to look at content-disposition? I'm guessing not *really*. + #(part['content-disposition'] && part['content-disposition'].disposition == "attachment") || + if p.main_type == 'text' + return p + end + end + # ... or if none, just first part (covers cases of one part, not + # labelled as text - not sure # what the better way to handle this is) + return leaves[0] + end + + # Returns all attachments for use in display code + def get_attachments_for_display + main_part = get_main_body_text_part + leaves = get_attachment_leaves + leaves = leaves.select do |p| + p != main_part + end + return leaves + end # Returns body text as HTML with quotes flattened, and emails removed. def get_body_for_html_display(collapse_quoted_sections = true) diff --git a/app/views/request/_bubble.rhtml b/app/views/request/_bubble.rhtml index 8480693e3..be5251e50 100644 --- a/app/views/request/_bubble.rhtml +++ b/app/views/request/_bubble.rhtml @@ -1,6 +1,18 @@ <blockquote class="xsnazzy"> <b class="xb1"></b><b class="xb2"></b><b class="xb3"></b><b class="xb4"></b><b class="xb5"></b><b class="xb6"></b><b class="xb7"></b> <div class="xboxcontent"> + <% if not attachments.nil? and attachments.size > 0 %> + <p> + <% attachments.each do |a| %> + Attachment: + <%= link_to (TMail::Mail.get_part_file_name(a) || "download.bin"), get_attachment_url(:id => incoming_message.info_request_id, + :incoming_message_id => incoming_message.id, :part => a.url_part_number, + :file_name => (TMail::Mail.get_part_file_name(a) || "download.bin")) %> + (<%= a.content_type %>) + <br> + <% end %> + </p> + <% end %> <p><%= body %></p> </div> <b class="xb7"></b><b class="xb6"></b><b class="xb5"></b><b class="xb4"></b><b class="xb3"></b><b class="xb2"></b><b class="xb1"></b> diff --git a/app/views/request/_correspondence.rhtml b/app/views/request/_correspondence.rhtml index d22aa5d67..2f64d1246 100644 --- a/app/views/request/_correspondence.rhtml +++ b/app/views/request/_correspondence.rhtml @@ -4,7 +4,7 @@ if (correspondence.class.to_s == 'IncomingMessage') incoming_message = correspondence%> - <%= render :partial => 'bubble', :locals => { :body => incoming_message.get_body_for_html_display(@collapse_quotes) } %> + <%= render :partial => 'bubble', :locals => { :incoming_message => incoming_message, :body => incoming_message.get_body_for_html_display(@collapse_quotes), :attachments => incoming_message.get_attachments_for_display } %> <p class="event_bubble"> <% if !incoming_message.safe_mail_from.nil? %> @@ -28,7 +28,7 @@ if info_request_event.event_type == 'sent' || info_request_event.event_type == 'followup_sent' outgoing_message = OutgoingMessage.find(info_request_event.params[:outgoing_message_id]) %> - <%= render :partial => 'bubble', :locals => { :body => outgoing_message.get_body_for_html_display() } %> + <%= render :partial => 'bubble', :locals => { :body => outgoing_message.get_body_for_html_display(), :attachments => nil } %> <p class="event_bubble"> <%= user_link(@info_request.user) %> diff --git a/config/routes.rb b/config/routes.rb index d52b4ef76..9687fad70 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,7 +4,7 @@ # Copyright (c) 2007 UK Citizens Online Democracy. All rights reserved. # Email: francis@mysociety.org; WWW: http://www.mysociety.org/ # -# $Id: routes.rb,v 1.28 2008-01-09 19:34:07 francis Exp $ +# $Id: routes.rb,v 1.29 2008-01-18 03:30:22 francis Exp $ ActionController::Routing::Routes.draw do |map| # The priority is based upon order of creation: first created -> highest priority. @@ -19,6 +19,7 @@ ActionController::Routing::Routes.draw do |map| request.new_request '/new', :action => 'new' request.show_request '/request/:id', :action => 'show' request.show_response '/request/:id/response/:incoming_message_id', :action => 'show_response' + request.get_attachment '/request/:id/response/:incoming_message_id/attach/:part/*file_name', :action => 'get_attachment' end map.with_options :controller => 'user' do |user| @@ -23,7 +23,7 @@ Followups: - link to the follow up form, or embed in bottom of main request page - don't show classify link on /response/ page - rename get_quoted_part_of_followup - - Copy To: for followups properly (including name not just email) + - Copy the To: for followups properly (including name not just email) Alert somewhere if working days table not up to date Test it works if exim is down - e.g. not accepting connections |