From 6c302bfe097998c9453bbb8fe6f19a6daf5fad9b Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Mon, 24 Oct 2022 13:51:49 +0100 Subject: [PATCH] Add Xapian indexing attachment prominence checks We need to check attachment prominence while updating the Xapian search index for events. To achieve the `IncomingMessae#get_body_for_quoting` method needed to be split, as it was being used for both indexing and displaying the body as text. --- app/models/incoming_message.rb | 12 ++++++-- app/models/outgoing_message.rb | 2 +- app/views/admin_general/_to_do_list.html.erb | 4 +-- .../request/_incoming_correspondence.text.erb | 2 +- spec/models/incoming_message_spec.rb | 28 +++++++++++++++++-- 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/app/models/incoming_message.rb b/app/models/incoming_message.rb index d88012e79da..1a5f61178c4 100644 --- a/app/models/incoming_message.rb +++ b/app/models/incoming_message.rb @@ -579,9 +579,13 @@ def get_body_for_html_display(collapse_quoted_sections = true) text.html_safe end + def get_body_for_indexing # rubocop:disable Naming/AccessorMethodName + return '' if Ability.guest.cannot?(:read, get_main_body_text_part) + get_body_for_text_display + end # Returns text of email for using in quoted section when replying - def get_body_for_quoting + def get_body_for_text_display # rubocop:disable Naming/AccessorMethodName # Get the body text with emails and quoted sections removed text = get_main_body_text_folded.dup text.gsub!("FOLDED_QUOTED_SECTION", " ") @@ -622,6 +626,8 @@ def get_attachment_text_clipped def _extract_text # Extract text from each attachment self.get_attachments_for_display.reduce('') { |memo, attachment| + return memo if Ability.guest.cannot?(:read, attachment) + memo += MailHandler.get_attachment_text_one_file(attachment.content_type, attachment.default_body, attachment.charset) @@ -634,11 +640,11 @@ def _get_attachment_text_internal # Returns text for indexing def get_text_for_indexing_full - return get_body_for_quoting + "\n\n" + get_attachment_text_full + return get_body_for_indexing + "\n\n" + get_attachment_text_full end # Used for excerpts in search results, when loading full text would be too slow def get_text_for_indexing_clipped - return get_body_for_quoting + "\n\n" + get_attachment_text_clipped + return get_body_for_indexing + "\n\n" + get_attachment_text_clipped end # Has message arrived "recently"? diff --git a/app/models/outgoing_message.rb b/app/models/outgoing_message.rb index 73948137c80..b16e9a5a52c 100644 --- a/app/models/outgoing_message.rb +++ b/app/models/outgoing_message.rb @@ -330,7 +330,7 @@ def prepare_message_for_resend def quoted_part_to_append_to_email if message_type == 'followup' && !incoming_message_followup.nil? quoted = "\n\n-----Original Message-----\n\n" - quoted += incoming_message_followup.get_body_for_quoting + quoted += incoming_message_followup.get_body_for_text_display quoted += "\n" else "" diff --git a/app/views/admin_general/_to_do_list.html.erb b/app/views/admin_general/_to_do_list.html.erb index b011c49705e..a24d7af98ae 100644 --- a/app/views/admin_general/_to_do_list.html.erb +++ b/app/views/admin_general/_to_do_list.html.erb @@ -17,11 +17,11 @@ <% if item.is_a? IncomingMessage %> - <% if item.get_body_for_quoting.gsub(/[[:space:]]/, "").size == 0 %> + <% if item.get_body_for_text_display.gsub(/[[:space:]]/, "").size == 0 %> <%= link_to "(no body)", admin_raw_email_path(item.raw_email_id) %> <% else %> <%= link_to admin_raw_email_path(item.raw_email_id) do %> - <%= excerpt(item.get_body_for_quoting, "", :radius => 60) %> + <%= excerpt(item.get_body_for_text_display, "", :radius => 60) %> <% end %> <% end %> <% elsif item.is_a? InfoRequest %> diff --git a/app/views/request/_incoming_correspondence.text.erb b/app/views/request/_incoming_correspondence.text.erb index 112bd92c4f4..9d3ac5c9e6e 100644 --- a/app/views/request/_incoming_correspondence.text.erb +++ b/app/views/request/_incoming_correspondence.text.erb @@ -8,7 +8,7 @@ <%= render_prominence(incoming_message.get_main_body_text_part, format: :text) -%> <% if can?(:read, incoming_message.get_main_body_text_part) %> -<%= incoming_message.get_body_for_quoting %> +<%= incoming_message.get_body_for_text_display %> <% end %> <% incoming_message.get_attachments_for_display.each do |a| %> <%= _('Attachment:') %> <% if can?(:read, a) %><%= a.display_filename %> (<%= a.display_size %>)<% end %> <%= render_prominence(a, format: :text) -%> diff --git a/spec/models/incoming_message_spec.rb b/spec/models/incoming_message_spec.rb index dfda35c1e2b..a0e0c9b4f92 100644 --- a/spec/models/incoming_message_spec.rb +++ b/spec/models/incoming_message_spec.rb @@ -403,11 +403,35 @@ end - describe '#get_body_for_quoting' do + describe '#get_body_for_indexing' do + subject { incoming_message.get_body_for_indexing } + + let(:incoming_message) { FactoryBot.build(:incoming_message) } + + context 'guest can read main body part' do + it 'returns body for text display' do + is_expected.to eq('hereisthetext') + end + end + + context 'guest cannot read main body part' do + before do + ability = Object.new.extend(CanCan::Ability) + ability.cannot :read, incoming_message.get_main_body_text_part + allow(Ability).to receive(:guest).and_return(ability) + end + + it 'returns blank string' do + is_expected.to eq '' + end + end + end + + describe '#get_body_for_text_display' do it 'does not incorrectly cache without the FOLDED_QUOTED_SECTION marker' do message = FactoryBot.create(:plain_incoming_message) - message.get_body_for_quoting + message.get_body_for_text_display expect(message.get_main_body_text_folded). to include('FOLDED_QUOTED_SECTION') end