Skip to content

Commit

Permalink
Add Xapian indexing attachment prominence checks
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
gbp committed Oct 26, 2022
1 parent 44ec0ca commit 6c302bf
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 9 deletions.
12 changes: 9 additions & 3 deletions app/models/incoming_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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", " ")
Expand Down Expand Up @@ -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)
Expand All @@ -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"?
Expand Down
2 changes: 1 addition & 1 deletion app/models/outgoing_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
""
Expand Down
4 changes: 2 additions & 2 deletions app/views/admin_general/_to_do_list.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
<tr>
<td class="link">
<% 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 %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/request/_incoming_correspondence.text.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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) -%>
Expand Down
28 changes: 26 additions & 2 deletions spec/models/incoming_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 6c302bf

Please sign in to comment.