From 21b6479ace1900d2c5b31be51ee0b4f8bfe96af6 Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Mon, 6 Nov 2023 19:01:25 +0100 Subject: [PATCH] feat(messages): re-enable simple autolink for instructeurs messages Closes #9541 --- .../message_component.html.haml | 2 +- app/components/simple_format_component.rb | 20 +++++++- .../simple_format_component.html.haml | 2 +- .../dossiers/message_component_spec.rb | 5 ++ .../simple_format_component_spec.rb | 46 +++++++++++++++++-- 5 files changed, 68 insertions(+), 7 deletions(-) diff --git a/app/components/dossiers/message_component/message_component.html.haml b/app/components/dossiers/message_component/message_component.html.haml index 1839981a0..8148a9709 100644 --- a/app/components/dossiers/message_component/message_component.html.haml +++ b/app/components/dossiers/message_component/message_component.html.haml @@ -17,7 +17,7 @@ - elsif commentaire.sent_by_system? = sanitize(commentaire.body, scrubber: Sanitizers::MailScrubber.new) - else - = render SimpleFormatComponent.new(commentaire.body, allow_a: false) + = render SimpleFormatComponent.new(commentaire.body, allow_a: false, allow_autolink: commentaire.sent_by_instructeur?) .message-extras.flex.justify-start diff --git a/app/components/simple_format_component.rb b/app/components/simple_format_component.rb index 84db5d841..d62b2bf48 100644 --- a/app/components/simple_format_component.rb +++ b/app/components/simple_format_component.rb @@ -21,11 +21,18 @@ class SimpleFormatComponent < ApplicationComponent no_images: true } - SIMPLE_URL_REGEX = %r{https?://\S+} + SIMPLE_URL_REGEX = %r{https?://[^\s<>]+} EMAIL_IN_TEXT_REGEX = Regexp.new(Devise.email_regexp.source.gsub(/\\A|\\z/, '\b')) - def initialize(text, allow_a: true, class_names_map: {}) + def initialize(text, allow_a: true, allow_autolink: true, class_names_map: {}) @allow_a = allow_a + @allow_autolink = allow_a || allow_autolink + + # Logic for html links/autolinks: + # Sometimes we want to allow autolinking of urls, without allowing html/markdown links from users. + # Because we sanitize the rendered markdown, when html links are not allowed, we can't enable redcarpet autolink + # (it would be sanitized), so we manually autolink after sanitization. + # At the contrary, when links are allowed, autolinking is always made with redcarpet. list_item = false lines = (text || "") @@ -56,6 +63,15 @@ class SimpleFormatComponent < ApplicationComponent ) end + def autolink(text) + return text if !@allow_autolink + return text if @allow_a # already autolinked + + text.gsub(SIMPLE_URL_REGEX) do |url| + helpers.link_to(ERB::Util.html_escape(url), ERB::Util.html_escape(url), title: helpers.new_tab_suffix(nil), **helpers.external_link_attributes) + end + end + def tags if @allow_a Rails.configuration.action_view.sanitized_allowed_tags + ['a'] diff --git a/app/components/simple_format_component/simple_format_component.html.haml b/app/components/simple_format_component/simple_format_component.html.haml index 61913e949..398bc733a 100644 --- a/app/components/simple_format_component/simple_format_component.html.haml +++ b/app/components/simple_format_component/simple_format_component.html.haml @@ -1 +1 @@ -= sanitize(@renderer.render(@text), tags:, attributes:) += autolink(sanitize(@renderer.render(@text), tags:, attributes:)).html_safe diff --git a/spec/components/dossiers/message_component_spec.rb b/spec/components/dossiers/message_component_spec.rb index 413e7b9f4..50a57d737 100644 --- a/spec/components/dossiers/message_component_spec.rb +++ b/spec/components/dossiers/message_component_spec.rb @@ -97,6 +97,11 @@ RSpec.describe Dossiers::MessageComponent, type: :component do it { is_expected.not_to have_selector("form[action=\"#{form_url}\"]") } end end + + describe 'autolink simple urls' do + let(:commentaire) { create(:commentaire, instructeur: instructeur, body: "rdv sur https://demarches.gouv.fr") } + it { is_expected.to have_link("https://demarches.gouv.fr", href: "https://demarches.gouv.fr") } + end end describe '#commentaire_from_guest?' do diff --git a/spec/components/simple_format_component_spec.rb b/spec/components/simple_format_component_spec.rb index c0c9be838..9a3c4a5af 100644 --- a/spec/components/simple_format_component_spec.rb +++ b/spec/components/simple_format_component_spec.rb @@ -1,6 +1,7 @@ describe SimpleFormatComponent, type: :component do let(:allow_a) { false } - before { render_inline(described_class.new(text, allow_a: allow_a)) } + let(:allow_autolink) { false } + before { render_inline(described_class.new(text, allow_a: allow_a, allow_autolink: allow_autolink)) } context 'one line' do let(:text) do @@ -108,10 +109,12 @@ TEXT bonjour https://www.demarches-simplifiees.fr nohttp www.ds.io ecrivez à ds@rspec.io + lien html + [lien markdown](https://github.com) TEXT end - context 'enabled' do + context 'enabled with html links' do let(:allow_a) { true } it { expect(page).to have_selector("a") } it "inject expected attributes" do @@ -132,9 +135,46 @@ TEXT expect(link[:rel]).to eq("noopener noreferrer") expect(link[:title]).to eq("Nouvel onglet") end + + it "render html link" do + link = page.find_link("lien html").native + expect(link[:href]).to eq("https://demarches.gouv.fr") + end + + it "convert markdown link" do + link = page.find_link("lien markdown").native + expect(link[:href]).to eq("https://github.com") + expect(link[:rel]).to eq("noopener noreferrer") + expect(link[:title]).to eq("Nouvel onglet") + end end - context 'disabled' do + context 'enabled only without html links' do + let(:allow_autolink) { true } + + it "convert only visible http link, not html links" do + expect(page).to have_link("https://www.demarches-simplifiees.fr") + expect(page).to have_selector("a", count: 1) + end + + it "inject expected attributes" do + link = page.find_link("https://www.demarches-simplifiees.fr").native + expect(link[:rel]).to eq("noopener noreferrer") + expect(link[:title]).to include("Nouvel onglet") + end + + context 'url ending the paragraph' do + let(:text) { "bonjour https://www.demarches-simplifiees.fr" } + + it "does not include the closing p" do + link = page.find_link("https://www.demarches-simplifiees.fr").native + expect(link[:href]).to eq("https://www.demarches-simplifiees.fr") + expect(link.text).to eq("https://www.demarches-simplifiees.fr") + end + end + end + + context 'completely disabled' do it { expect(page).not_to have_selector("a") } end end