Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Regression in EmailReplyPipeline: unfiltered content is being ommitted #253

Merged
merged 2 commits into from
May 3, 2016
Merged

Regression in EmailReplyPipeline: unfiltered content is being ommitted #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
596d5f2
fix regression where gsub! would return nil and omit content
oreoshake Apr 26, 2016
514a221
add green test from before 2.4 release
oreoshake Apr 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/html/pipeline/email_reply_filter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def call
if fragment.quoted?
if context[:hide_quoted_email_addresses]
pieces.map! do |piece|
piece.gsub!(EMAIL_REGEX, HIDDEN_EMAIL_PATTERN)
piece.gsub(EMAIL_REGEX, HIDDEN_EMAIL_PATTERN)
end
end
pieces.unshift EMAIL_QUOTED_HEADER
Expand Down
34 changes: 34 additions & 0 deletions test/html/pipeline/email_reply_filter_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,38 @@ def test_hides_email_addresses_when_configured
refute_match %r([email protected]), doc
refute_match %r([email protected]), doc
end

def test_preserves_non_email_content_while_filtering
str = <<-EMAIL
> Thank you! I have some thoughts on this pull request.
>
> * acme provides cmake and a wrapper for it. Please use '$(TARGET)-cmake' instead of cmake -DCMAKE_TOOLCHAIN_FILE='$(CMAKE_TOOLCHAIN_FILE)' -DCMAKE_BUILD_TYPE=Release.

Okay -- I'm afraid I just blindly copied the eigen3.mk file, since that's a library I'm familiar with :-)

> * Do you need -DCMAKE_SYSTEM_PROCESSOR=x86?

Yes, this is a bit dumb, but vc checks for that (or amd) to determine that it's not being built on ARM.

--
Boaty McBoatface | http://example.org
EMAIL

filter = EmailReplyFilter.new(str, :hide_quoted_email_addresses => true)
doc = filter.call.to_s

expected = <<-EXPECTED
<div class="email-quoted-reply"> Thank you! I have some thoughts on this pull request.

* acme provides cmake and a wrapper for it. Please use &#39;$(TARGET)-cmake&#39; instead of cmake -DCMAKE_TOOLCHAIN_FILE=&#39;$(CMAKE_TOOLCHAIN_FILE)&#39; -DCMAKE_BUILD_TYPE=Release.</div>
<div class="email-fragment">Okay -- I&#39;m afraid I just blindly copied the eigen3.mk file, since that&#39;s a library I&#39;m familiar with :-)</div>
<div class="email-quoted-reply"> * Do you need -DCMAKE_SYSTEM_PROCESSOR=x86?</div>
<div class="email-fragment">Yes, this is a bit dumb, but vc checks for that (or amd) to determine that it&#39;s not being built on ARM.</div>
<span class="email-hidden-toggle"><a href="#">&hellip;</a></span><div class="email-hidden-reply" style="display:none"><div class="email-signature-reply">--
Boaty McBoatface | http:&#47;&#47;example.org</div>
</div>
EXPECTED

assert_equal(expected.chomp, doc)
end
end