diff --git a/lib/govspeak.rb b/lib/govspeak.rb index b3a376db..54c859b2 100644 --- a/lib/govspeak.rb +++ b/lib/govspeak.rb @@ -1,5 +1,6 @@ require 'kramdown' require 'active_support/core_ext/hash' +require 'active_support/core_ext/array' require 'govspeak/header_extractor' require 'govspeak/structured_header_extractor' require 'govspeak/html_validator' @@ -10,6 +11,7 @@ require 'kramdown/parser/kramdown_with_automatic_external_links' require 'htmlentities' require 'presenters/attachment_presenter' +require 'presenters/contact_presenter' require 'presenters/h_card_presenter' require 'erb' @@ -33,9 +35,9 @@ def initialize(source, options = {}) options.deep_symbolize_keys! @source = source ? source.dup : "" @images = options.delete(:images) || [] - @attachments = Array(options.delete(:attachments)) - @links = Array(options.delete(:links)) - @contacts = Array(options.delete(:contacts)) + @attachments = Array.wrap(options.delete(:attachments)) + @links = Array.wrap(options.delete(:links)) + @contacts = Array.wrap(options.delete(:contacts)) @locale = options.fetch(:locale, "en") @options = {input: PARSER_CLASS_NAME}.merge(options) @options[:entity_output] = :symbolic @@ -188,19 +190,22 @@ def insert_strong_inside_p(body, parser=Govspeak::Document) end extension('attachment', /\[embed:attachments:([0-9a-f-]+)\]/) do |content_id, body| - attachment = attachments.detect { |a| a.content_id.match(content_id) } + attachment = attachments.detect { |a| a[:content_id].match(content_id) } next "" unless attachment attachment = AttachmentPresenter.new(attachment) - content = File.read('lib/govspeak/extension/attachment.html.erb') + 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, body| - attachment = attachments.detect { |a| a.content_id.match(content_id) } + attachment = attachments.detect { |a| a[:content_id].match(content_id) } next "" unless attachment attachment = AttachmentPresenter.new(attachment) - content = File.read('lib/govspeak/extension/inline_attachment.html.erb') - ERB.new(content).result(binding) + span_id = attachment.id ? %{ id="attachment_#{attachment.id}"} : "" + # new lines inside our title cause problems with govspeak rendering as this is expected to be on one line. + link = attachment.link(attachment.title.gsub("\n", " "), attachment.url) + attributes = attachment.attachment_attributes.empty? ? "" : " (#{attachment.attachment_attributes})" + %{#{encode(link.title)}} + if link[:url] + %Q{#{link[:title]}} else - encode(link.title) + link[:title] end end @@ -290,9 +295,10 @@ def render_hcard_address(contact) private :render_hcard_address extension('Contact', /\[Contact:([0-9a-f-]+)\]/) do |content_id| - contact = contacts.detect { |c| c.content_id.match(content_id) } + contact = contacts.detect { |c| c[:content_id].match(content_id) } next "" unless contact - @renderer ||= ERB.new(File.read('lib/templates/contact.html.erb')) + contact = ContactPresenter.new(contact) + @renderer ||= ERB.new(File.read(__dir__ + '/templates/contact.html.erb')) @renderer.result(binding) end end diff --git a/lib/govspeak/extension/inline_attachment.html.erb b/lib/govspeak/extension/inline_attachment.html.erb deleted file mode 100644 index e6f37096..00000000 --- a/lib/govspeak/extension/inline_attachment.html.erb +++ /dev/null @@ -1,4 +0,0 @@ - diff --git a/lib/presenters/attachment_presenter.rb b/lib/presenters/attachment_presenter.rb index a77b1f14..dadb9093 100644 --- a/lib/presenters/attachment_presenter.rb +++ b/lib/presenters/attachment_presenter.rb @@ -1,77 +1,80 @@ require "action_view" require "money" +require "htmlentities" class AttachmentPresenter attr_reader :attachment include ActionView::Helpers::TagHelper include ActionView::Helpers::NumberHelper include ActionView::Helpers::AssetTagHelper + include ActionView::Helpers::TextHelper def initialize(attachment) @attachment = attachment end def id - attachment.id + attachment[:id] end def order_url - attachment.order_url + attachment[:order_url] end def opendocument? - attachment.opendocument? + attachment[:opendocument?] end def url - attachment.url + attachment[:url] end def external? - attachment.external? + attachment[:external?] end def price - return unless attachment.price - Money.from_amount(attachment.price, 'GBP').format + return unless attachment[:price] + Money.from_amount(attachment[:price], 'GBP').format end def accessible? - attachment.accessible? + attachment[:accessible?] end def thumbnail_link return if hide_thumbnail? return if previewable? - link(attachment_thumbnail, url, "aria-hidden=true class=#{attachment_class}") + link(attachment_thumbnail, url, "aria-hidden" => "true", "class" => attachment_class) end def help_block_toggle_id - "attachment-#{attachment.id}-accessibility-request" + "attachment-#{id}-accessibility-request" end def section_class - attachment.external? ? "hosted-externally" : "embedded" + attachment[:external?] ? "hosted-externally" : "embedded" end def mail_to(email_address, name, options = {}) - "#{name}" + query_string = options.slice(:subject, :body).map { |k, v| "#{urlencode(k)}=#{urlencode(v)}" }.join("&") + "#{name}" end def alternative_format_order_link attachment_info = [] - attachment_info << " Title: #{attachment.title}" - attachment_info << " Original format: #{attachment.file_extension}" - attachment_info << " ISBN: #{attachment.isbn}" if attachment.isbn.present? - attachment_info << " Unique reference: #{attachment.unique_reference}" if attachment.unique_reference.present? - attachment_info << " Command paper number: #{attachment.command_paper_number}" if attachment.command_paper_number.present? - if attachment.hoc_paper_number.present? - attachment_info << " House of Commons paper number: #{attachment.hoc_paper_number}" - attachment_info << " Parliamentary session: #{attachment.parliamentary_session}" + attachment_info << " Title: #{title}" + attachment_info << " Original format: #{file_extension}" if file_extension.present? + attachment_info << " ISBN: #{attachment[:isbn]}" if attachment[:isbn].present? + attachment_info << " Unique reference: #{attachment[:unique_reference]}" if attachment[:unique_reference].present? + attachment_info << " Command paper number: #{attachment[:command_paper_number]}" if attachment[:command_paper_number].present? + if attachment[:hoc_paper_number].present? + attachment_info << " House of Commons paper number: #{attachment[:hoc_paper_number]}" + attachment_info << " Parliamentary session: #{attachment[:parliamentary_session]}" end options = { - subject: "Request for '#{attachment.title}' in an alternative format", + subject: "Request for '#{title}' in an alternative format", body: body_for_mail(attachment_info) } @@ -80,7 +83,7 @@ def alternative_format_order_link def body_for_mail(attachment_info) <<-END - Details of document required: +Details of document required: #{attachment_info.join("\n")} @@ -95,66 +98,93 @@ def alternative_format_contact_email "govuk-feedback@digital.cabinet-office.gov.uk" end + # FIXME: usage of image_tag will cause these to render at /images/ which seems + # very host dependent. I assume this will need links to static urls. def attachment_thumbnail - if attachment.pdf? - image_tag(attachment.file.thumbnail.url) - elsif attachment.html? + if file_extension == "pdf" && attachment[:thumbnail_url] + image_tag(attachment[:thumbnail_url]) + elsif file_extension == "html" image_tag('pub-cover-html.png') - elsif %w{doc docx odt}.include? attachment.file_extension + elsif %w{doc docx odt}.include?(file_extension) image_tag('pub-cover-doc.png') - elsif %w{xls xlsx ods csv}.include? attachment.file_extension + elsif %w{xls xlsx ods csv}.include?(file_extension) image_tag('pub-cover-spreadsheet.png') else image_tag('pub-cover.png') end end - def references + def reference + ref = [] + if attachment[:isbn].present? + ref << "ISBN " + content_tag(:span, attachment[:isbn], class: "isbn") + end + + if attachment[:unique_reference].present? + ref << content_tag(:span, attachment[:unique_reference], class: "unique_reference") + end + + if attachment[:command_paper_number].present? + ref << content_tag(:span, attachment[:command_paper_number], class: "command_paper_number") + end + + if attachment[:hoc_paper_number].present? + ref << content_tag(:span, "HC #{attachment[:hoc_paper_number]}", class: 'house_of_commons_paper_number') + ' ' + + content_tag(:span, attachment[:parliamentary_session], class: 'parliamentary_session') + end + + ref.join(', ').html_safe + end + + # FIXME this has english in it so will cause problems if the locale is not en + def references_for_title references = [] - references << "ISBN: #{attachment.isbn}" if attachment.isbn.present? - references << "Unique reference: #{attachment.unique_reference}" if attachment.unique_reference.present? - references << "Command paper number: #{attachment.command_paper_number}" if attachment.command_paper_number.present? - references << "HC: #{attachment.hoc_paper_number} #{attachment.parliamentary_session}" if attachment.hoc_paper_number.present? + references << "ISBN: #{attachment[:isbn]}" if attachment[:isbn].present? + references << "Unique reference: #{attachment[:unique_reference]}" if attachment[:unique_reference].present? + references << "Command paper number: #{attachment[:command_paper_number]}" if attachment[:command_paper_number].present? + references << "HC: #{attachment[:hoc_paper_number]} #{attachment[:parliamentary_session]}" if attachment[:hoc_paper_number].present? prefix = references.size == 1 ? "and its reference" : "and its references" references.any? ? ", #{prefix} (" + references.join(", ") + ")" : "" end def references? - !attachment.isbn.to_s.empty? || !attachment.unique_reference.to_s.empty? || !attachment.command_paper_number.to_s.empty? || !attachment.hoc_paper_number.to_s.empty? + !attachment[:isbn].to_s.empty? || !attachment[:unique_reference].to_s.empty? || !attachment[:command_paper_number].to_s.empty? || !attachment[:hoc_paper_number].to_s.empty? end def attachment_class - attachment.external? ? "hosted-externally" : "embedded" + attachment[:external?] ? "hosted-externally" : "embedded" end def unnumbered_paper? - attachment.unnumbered_command_paper? || attachment.unnumbered_hoc_paper? + attachment[:unnumbered_command_paper?] || attachment[:unnumbered_hoc_paper?] end def unnumbered_command_paper? - attachment.unnumbered_command_paper? + attachment[:unnumbered_command_paper?] end def download_link - link(attachment.preview_url, "Download #{attachment.file_extension.upcase}", number_to_human_size(attachment.file_size)) + options = {} + options[:title] = number_to_human_size(attachment[:file_size]) if attachment[:file_size].present? + link("Download #{file_extension.upcase}", attachment[:url], options) end def attachment_attributes attributes = [] - if attachment.html? + if file_extension == "html" attributes << content_tag(:span, 'HTML', class: 'type') - elsif attachment.external? + elsif attachment[:external?] attributes << content_tag(:span, url, class: 'url') else - attributes << content_tag(:span, humanized_content_type(attachment.file_extension), class: 'type') - attributes << content_tag(:span, number_to_human_size(attachment.file_size), class: 'file-size') - attributes << content_tag(:span, pluralize(attachment.number_of_pages, "page") , class: 'page-length') if attachment.number_of_pages.present? + attributes << content_tag(:span, humanized_content_type(file_extension), class: 'type') if file_extension + attributes << content_tag(:span, number_to_human_size(attachment[:file_size]), class: 'file-size') if attachment[:file_size] + attributes << content_tag(:span, pluralize(attachment[:number_of_pages], "page"), class: 'page-length') if attachment[:number_of_pages] end attributes.join(', ').html_safe end def preview_url - url << '/preview' + url + '/preview' end MS_WORD_DOCUMENT_HUMANIZED_CONTENT_TYPE = "MS Word Document" @@ -205,37 +235,52 @@ def humanized_content_type(file_extension) end def previewable? - attachment.csv? + file_extension == "csv" end def title - attachment.title + attachment[:title] + end + + def file_extension + # Note: this is a separate parameter rather than being calculated from the + # filename because at the time of writing not all apps were using the effects + # of this field. + attachment[:file_extension] end def hide_thumbnail? defined?(hide_thumbnail) && hide_thumbnail end - def attachement_details + def attachment_details return if previewable? - link(attachment.title, url, title_link_options) + link(title, url, title_link_options) end def title_link_options - title_link_options = '' - title_link_options << "rel=external" if attachment.external? - title_link_options << "aria-describedby=#{help_block_id}" unless attachment.accessible? + title_link_options = {} + title_link_options["rel"] = "external" if attachment[:external?] + title_link_options["aria-describedby"] = help_block_id unless attachment[:accessible?] + title_link_options end def help_block_id - "attachment-#{attachment.id}-accessibility-help" + "attachment-#{id}-accessibility-help" end - def link(body, url, options={}) - <<-END - - #{body} - - END + def link(body, url, options = {}) + options_str = options.map { |k, v| %{#{encode(k)}="#{encode(v)}"} }.join(" ") + %{#{body}} + end + +private + + def encode(text) + HTMLEntities.new.encode(text) + end + + def urlencode(text) + ERB::Util.url_encode(text) end end diff --git a/lib/presenters/contact_presenter.rb b/lib/presenters/contact_presenter.rb new file mode 100644 index 00000000..d81f1cec --- /dev/null +++ b/lib/presenters/contact_presenter.rb @@ -0,0 +1,22 @@ +require 'active_support/core_ext/array' +require 'ostruct' + +class ContactPresenter + attr_reader :contact + delegate :id, :content_id, :title, :recipient, :street_address, :postal_code, + :locality, :region, :country_code, :country_name, :email, :contact_form_url, + :comments, :worldwide_organisation_path, to: :contact + + def initialize(contact) + @contact = OpenStruct.new(contact) + end + + def contact_numbers + Array.wrap(contact[:contact_numbers]) + end + + def has_postal_address? + recipient.present? || street_address.present? || locality.present? || + region.present? || postal_code.present? + end +end diff --git a/lib/govspeak/extension/attachment.html.erb b/lib/templates/attachment.html.erb similarity index 73% rename from lib/govspeak/extension/attachment.html.erb rename to lib/templates/attachment.html.erb index f419a4bd..0c39351f 100644 --- a/lib/govspeak/extension/attachment.html.erb +++ b/lib/templates/attachment.html.erb @@ -3,12 +3,10 @@ <%= attachment.thumbnail_link %>
} end def render_image(url, alt_text, caption = nil) @@ -275,12 +280,12 @@ def self.devolved_options end extension('embed link', /\[embed:link:([0-9a-f-]+)\]/) do |content_id| - link = links.detect { |l| l.content_id.match(content_id) } + link = links.detect { |l| l[:content_id].match(content_id) } next "" unless link - if link.url - %Q{ <% end %> diff --git a/lib/templates/contact.html.erb b/lib/templates/contact.html.erb index bcd593d2..fee423fb 100644 --- a/lib/templates/contact.html.erb +++ b/lib/templates/contact.html.erb @@ -25,8 +25,8 @@ <% end %> <% contact.contact_numbers.each do |number| %>- <%= number.label %> - <%= number.number %> + <%= number[:label] %> + <%= number[:number] %>
<% end %> diff --git a/lib/templates/inline_attachment.html.erb b/lib/templates/inline_attachment.html.erb new file mode 100644 index 00000000..700fe651 --- /dev/null +++ b/lib/templates/inline_attachment.html.erb @@ -0,0 +1,6 @@ +id="attachment_<%= attachment.id %>" <% end %>class="attachment-inline"> + <%= attachment.link attachment.title, attachment.url %> + <% unless attachment.attachment_attributes.empty? %> + (<%= attachment.attachment_attributes %>) + <% end %> + diff --git a/test/govspeak_attachments_inline_test.rb b/test/govspeak_attachments_inline_test.rb new file mode 100644 index 00000000..538e15b5 --- /dev/null +++ b/test/govspeak_attachments_inline_test.rb @@ -0,0 +1,146 @@ +# encoding: UTF-8 + +require 'test_helper' + +class GovspeakAttachmentsInlineTest < Minitest::Test + def build_attachment(args = {}) + { + content_id: "2b4d92f3-f8cd-4284-aaaa-25b3a640d26c", + id: 456, + url: "http://example.com/attachment.pdf", + title: "Attachment Title", + }.merge(args) + end + + def render_govspeak(govspeak, attachments = []) + Govspeak::Document.new(govspeak, attachments: attachments).to_html + end + + test "renders an empty string for an inline attachment not found" do + assert_match("", 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 + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(id: 10, content_id: "1fe8")] + ) + assert_match(/span id="attachment_10" class="attachment-inline">/, rendered) + end + + test "wraps an attachment in a span without the id if the id is not present" 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]", + [build_attachment(content_id: "1fe8", url: "http://a.b/f.pdf", title: "My Pdf")] + ) + assert_match(%r{My Pdf}, rendered) + end + + test "renders on a single line" do + rendered = render_govspeak( + "[embed:attachments:inline:2bc1]", + [build_attachment(content_id: "2bc1", external?: true)] + ) + assert_match(%r{ }, rendered) + end + + test "will show HTML type (in brackets) if file_extension is specified as html" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(content_id: "1fe8", file_extension: "html")] + ) + assert_match(%r{(HTML)}, rendered) + end + + test "will show url (in brackets) if specified as external" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(content_id: "1fe8", url: "http://a.b/f.pdf", external?: true)] + ) + assert_match(%r{(http://a.b/f.pdf)}, rendered) + end + + test "will not show url if file_extension is specified as html" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(content_id: "1fe8", url: "http://a.b/f.pdf", file_extension: "html", external?: true)] + ) + refute_match(%r{(http://a.b/f.pdf)}, rendered) + end + + test "will show a file extension in a abbr element for non html" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(content_id: "1fe8", file_extension: "csv")] + ) + refute_match(%r{CSV}, rendered) + end + + test "will show file size in a span" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(content_id: "1fe8", file_size: 1024)] + ) + assert_match(%r{1 KB}, rendered) + end + + test "will show number of pages" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(content_id: "1fe8", number_of_pages: 1)] + ) + assert_match(%r{1 page}, rendered) + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment(content_id: "1fe8", number_of_pages: 2)] + ) + assert_match(%r{2 pages}, rendered) + end + + test "will show attributes after link" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8]", + [build_attachment( + content_id: "1fe8", + url: "http://a.b/test.txt", + title: "My Attached Text File", + file_extension: "txt", + file_size: 2048, + number_of_pages: 2, + )] + ) + link = %{My Attached Text File} + type = %{Plain text} + file_size = %{2 KB} + pages = %{2 pages} + assert_match(/#{link}\s+\(#{type}, #{file_size}, #{pages}\)/, rendered) + end + + test "can render two inline attachments on the same line" do + rendered = render_govspeak( + "[embed:attachments:inline:1fe8] and [embed:attachments:inline:2abc]", + [ + build_attachment(content_id: "1fe8", url: "http://a.b/test.txt", title: "Text File"), + build_attachment(content_id: "2abc", url: "http://a.b/test.pdf", title: "PDF File") + ] + ) + assert_match(%r{Text File}, rendered) + assert_match(%r{PDF File}, rendered) + end +end diff --git a/test/govspeak_attachments_test.rb b/test/govspeak_attachments_test.rb new file mode 100644 index 00000000..35d72cf7 --- /dev/null +++ b/test/govspeak_attachments_test.rb @@ -0,0 +1,397 @@ +# encoding: UTF-8 + +require 'test_helper' + +class GovspeakAttachmentTest < Minitest::Test + def build_attachment(args = {}) + { + content_id: "2b4d92f3-f8cd-4284-aaaa-25b3a640d26c", + id: 456, + url: "http://example.com/attachment.pdf", + title: "Attachment Title", + }.merge(args) + end + + def compress_html(html) + html.gsub(/[\n\r]+[\s]*/, '') + end + + def render_govspeak(govspeak, attachments = [], options = {}) + options = options.merge(attachments: attachments) + Govspeak::Document.new(govspeak, options).to_html + end + + test "Wraps an attachment in a section.attachment.embedded" do + rendered = render_govspeak( + "[embed:attachments:3ed2]", + [build_attachment(content_id: "3ed2")] + ) + assert_match(/