Skip to content

Commit

Permalink
Render bad usage of markdown as empty strings
Browse files Browse the repository at this point in the history 
Prior to this change `[embed:attachments:filename.jpg]` would be
rendered on the frontend amongst the content. This change means that it
would be rendered as an empty string.

Although it is debatable which route is better, having this render empty
strings does allow us to resolve some issues with missing attachments on
Specialist Publisher.
  • Loading branch information
@kevindew
kevindew committed Sep 30, 2016
1 parent 06e208b commit 1edcb6b
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 27 deletions.
10 changes: 5 additions & 5 deletions lib/govspeak.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,15 +189,15 @@ def insert_strong_inside_p(body, parser=Govspeak::Document)
end
end

extension('attachment', /\[embed:attachments:([0-9a-f-]+)\]/) do |content_id, body|
extension('attachment', /\[embed:attachments:(?!inline:|image:)\s*(.*?)\s*\]/) do |content_id, body|
attachment = attachments.detect { |a| a[:content_id].match(content_id) }
next "" unless attachment
attachment = AttachmentPresenter.new(attachment)
content = File.read(__dir__ + '/templates/attachment.html.erb')
ERB.new(content).result(binding)
end

extension('attachment inline', /\[embed:attachments:inline:([0-9a-f-]+)\]/) do |content_id|
extension('attachment inline', /\[embed:attachments:inline:\s*(.*?)\s*\]/) do |content_id|
attachment = attachments.detect { |a| a[:content_id].match(content_id) }
next "" unless attachment
attachment = AttachmentPresenter.new(attachment)
Expand All @@ -209,7 +209,7 @@ def insert_strong_inside_p(body, parser=Govspeak::Document)
%{<span#{span_id} class="attachment-inline">#{link}#{attributes}</span>}
end

extension('attachment image', /\[embed:attachments:image:([0-9a-f-]+)\]/) do |content_id|
extension('attachment image', /\[embed:attachments:image:\s*(.*?)\s*\]/) do |content_id|
attachment = attachments.detect { |a| a[:content_id].match(content_id) }
next "" unless attachment
attachment = AttachmentPresenter.new(attachment)
Expand Down Expand Up @@ -299,7 +299,7 @@ def self.devolved_options
end
end

extension('embed link', /\[embed:link:([0-9a-f-]+)\]/) do |content_id|
extension('embed link', /\[embed:link:\s*(.*?)\s*\]/) do |content_id|
link = links.detect { |l| l[:content_id].match(content_id) }
next "" unless link
if link[:url]
Expand All @@ -314,7 +314,7 @@ def render_hcard_address(contact)
end
private :render_hcard_address

extension('Contact', /\[Contact:([0-9a-f-]+)\]/) do |content_id|
extension('Contact', /\[Contact:\s*(.*?)\s*\]/) do |content_id|
contact = contacts.detect { |c| c[:content_id].match(content_id) }
next "" unless contact
contact = ContactPresenter.new(contact)
Expand Down
20 changes: 19 additions & 1 deletion test/govspeak_attachments_image_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def compress_html(html)
end

test "renders an empty string for an image attachment not found" do
assert_match("", render_govspeak("[embed:attachments:image:1fe8]", [build_attachment]))
assert_equal("\n", render_govspeak("[embed:attachments:image:1fe8]", [build_attachment]))
end

test "wraps an attachment in a figure with the id if the id is present" do
Expand All @@ -40,6 +40,14 @@ def compress_html(html)
assert_match(/<figure class="image embedded">/, rendered)
end

test "renders an attachment if there are spaces around the content_id" do
rendered = render_govspeak(
"[embed:attachments:image: 1fe8 ]",
[build_attachment(id: 10, content_id: "1fe8")]
)
assert_match(/<figure id="attachment_10" class="image embedded">/, rendered)
end

test "has an image element to the file" do
rendered = render_govspeak(
"[embed:attachments:image:1fe8]",
Expand Down Expand Up @@ -87,4 +95,14 @@ def compress_html(html)
regex = %r{<td><figure class="image embedded"><div class="img">(.*?)</div></figure></td>}
assert_match(regex, rendered)
end

test "renders an empty string for no match" do
rendered = render_govspeak("[embed:attachments:image:1fe8]")
assert_equal("\n", rendered)
end

test "renders an empty string for a filename instead of a content id" do
rendered = render_govspeak("[embed:attachments:image: path/to/file.jpg ]")
assert_equal("\n", rendered)
end
end
20 changes: 19 additions & 1 deletion test/govspeak_attachments_inline_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def render_govspeak(govspeak, attachments = [])
end

test "renders an empty string for an inline attachment not found" do
assert_match("", render_govspeak("[embed:attachments:inline:1fe8]", [build_attachment]))
assert_equal("\n", render_govspeak("[embed:attachments:inline:1fe8]", [build_attachment]))
end

test "wraps an attachment in a span with the id if the id is present" do
Expand All @@ -36,6 +36,14 @@ def render_govspeak(govspeak, attachments = [])
assert_match(/<span class="attachment-inline">/, rendered)
end

test "renders an attachment where the content id has spaces" do
rendered = render_govspeak(
"[embed:attachments:inline: 1fe8 ]",
[build_attachment(id: nil, content_id: "1fe8")]
)
assert_match(/<span class="attachment-inline">/, rendered)
end

test "links to the attachment file" do
rendered = render_govspeak(
"[embed:attachments:inline:1fe8]",
Expand Down Expand Up @@ -151,4 +159,14 @@ def render_govspeak(govspeak, attachments = [])
assert_match(%r{<a href="http://a.b/test.txt">Text File</a>}, rendered)
assert_match(%r{<a href="http://a.b/test.pdf">PDF File</a>}, rendered)
end

test "will render a not found attachment as an empty string" do
rendered = render_govspeak("[embed:attachments:inline:1fe8]")
assert_equal("\n", rendered)
end

test "will render a file name used as a content id as an empty string" do
rendered = render_govspeak("[embed:attachments:inline: path/to/file name.jpg ]")
assert_equal("\n", rendered)
end
end
54 changes: 34 additions & 20 deletions test/govspeak_attachments_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,135 +21,143 @@ def render_govspeak(govspeak, attachments = [], options = {})
Govspeak::Document.new(govspeak, options).to_html
end

test "Wraps an attachment in a section.attachment.embedded" do
test "wraps an attachment in a section.attachment.embedded" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2")]
)
assert_match(/<section class="attachment embedded">/, rendered)
end

test "Wraps an external attachment in a section.attachment.hosted-externally" do
test "can convert an attachment with spaces" do
rendered = render_govspeak(
"[embed:attachments: 3ed2 ]",
[build_attachment(content_id: "3ed2")]
)
assert_match(/<section class="attachment embedded">/, rendered)
end

test "wraps an external attachment in a section.attachment.hosted-externally" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", external?: true)]
)
assert_match(/<section class="attachment hosted-externally">/, rendered)
end

test "Outputs a pub-cover.png thumbnail by default" do
test "outputs a pub-cover.png thumbnail by default" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2")]
)
assert_match(%r{<img src="/images/pub-cover.png"}, rendered)
end

test "Outputs a specified thumbnail for a pdf with a thumbnail_url" do
test "outputs a specified thumbnail for a pdf with a thumbnail_url" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", file_extension: "pdf", thumbnail_url: "http://a.b/custom.png")]
)
assert_match(%r{<img src="http://a.b/custom.png"}, rendered)
end

test "Outputs pub-cover.png for a pdf without thumbnail_url" do
test "outputs pub-cover.png for a pdf without thumbnail_url" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", file_extension: "pdf", thumbnail_url: nil)]
)
assert_match(%r{<img src="/images/pub-cover.png"}, rendered)
end

test "Outputs pub-cover-html.png for a file with html file_extension" do
test "outputs pub-cover-html.png for a file with html file_extension" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", file_extension: "html")]
)
assert_match(%r{<img src="/images/pub-cover-html.png"}, rendered)
end

test "Outputs pub-cover-doc.png for a file with docx file_extension" do
test "outputs pub-cover-doc.png for a file with docx file_extension" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", file_extension: "docx")]
)
assert_match(%r{<img src="/images/pub-cover-doc.png"}, rendered)
end

test "Outputs pub-cover-spreadsheet.png for a file with xls file_extension" do
test "outputs pub-cover-spreadsheet.png for a file with xls file_extension" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", file_extension: "xls")]
)
assert_match(%r{<img src="/images/pub-cover-spreadsheet.png"}, rendered)
end

test "Outputs no thumbnail for a previewable file" do
test "outputs no thumbnail for a previewable file" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", file_extension: "csv")]
)
assert_match(%r{<div class="attachment-thumb"></div>}, compress_html(rendered))
end

test "Outputs a title link within a h2" do
test "outputs a title link within a h2" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", id: 1, url: "http://a.b/c.pdf", title: "Attachment Title")]
)
assert_match(%r{<h2 class="title">\s*<a href="http://a.b/c.pdf" aria-describedby="attachment-1-accessibility-help">Attachment Title</a></h2>}, compress_html(rendered))
end

test "Title link has rel='external' for an external link" do
test "title link has rel='external' for an external link" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", id: 1, url: "http://a.b/c.pdf", external?: true)]
)
assert_match(%r{<a href="http://a.b/c.pdf" rel="external" aria-describedby="attachment-1-accessibility-help">}, rendered)
end

test "Accessible attachment doesn't have the aria-describedby attribute" do
test "accessible attachment doesn't have the aria-describedby attribute" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", url: "http://a.b/c.pdf", accessible?: true)]
)
assert_match(%r{<a href="http://a.b/c.pdf">}, rendered)
end

test "Outputs reference if isbn is present" do
test "outputs reference if isbn is present" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", isbn: "123")]
)
assert_match(%r{<span class="references">Ref: ISBN <span class="isbn">123</span></span>}, rendered)
end

test "Outputs reference if uniuque_reference is present" do
test "outputs reference if uniuque_reference is present" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", unique_reference: "123")]
)
assert_match(%r{<span class="references">Ref: <span class="unique_reference">123</span></span>}, rendered)
end

test "Outputs reference if command_paper_number is present" do
test "outputs reference if command_paper_number is present" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", command_paper_number: "11")]
)
assert_match(%r{<span class="references">Ref: <span class="command_paper_number">11</span></span>}, rendered)
end

test "Outputs reference if hoc_paper_number is present" do
test "outputs reference if hoc_paper_number is present" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", hoc_paper_number: "15", parliamentary_session: "1")]
)
assert_match(%r{<span class="references">Ref: <span class="house_of_commons_paper_number">HC 15</span> <span class="parliamentary_session">1</span></span>}, rendered)
end

test "Can have multiple references" do
test "can have multiple references" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", isbn: "123", unique_reference: "55")]
Expand All @@ -173,15 +181,15 @@ def render_govspeak(govspeak, attachments = [], options = {})
assert_match(%r{<span class="unnumbered-paper">\s*Unnumbered act paper\s*</span>}, compress_html(rendered))
end

test "Unnumbered command paper takes precedence to unnumbered act paper" do
test "unnumbered command paper takes precedence to unnumbered act paper" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", unnumbered_command_paper?: true, unnumbered_hoc_paper?: true)]
)
assert_match(%r{<span class="unnumbered-paper">\s*Unnumbered command paper\s*</span>}, compress_html(rendered))
end

test "Shows a preview link for a previewable format" do
test "shows a preview link for a previewable format" do
rendered = render_govspeak(
"[embed:attachments:3ed2]",
[build_attachment(content_id: "3ed2", file_extension: "csv", url: "http://a.b/c.csv")]
Expand Down Expand Up @@ -392,6 +400,12 @@ def render_govspeak(govspeak, attachments = [], options = {})
test "attachment that isn't provided" do
govspeak = "[embed:attachments:906ac8b7-850d-45c6-98e0-9525c680f891]"
rendered = Govspeak::Document.new(govspeak).to_html
assert_match("", rendered)
assert_equal("\n", rendered)
end

test "attachment where filename is provided, rather than a content_id" do
govspeak = "[embed:attachments:/path/to/file%20name.pdf]"
rendered = Govspeak::Document.new(govspeak).to_html
assert_equal("\n", rendered)
end
end

0 comments on commit 1edcb6b

Please sign in to comment.