Skip to content

Commit

Permalink
Fix Video embeds are not shown in short_description field
Browse files Browse the repository at this point in the history
* Fix: Video embeds are not shown in short_description field

* Add test for ParticipatoryProcess

* Add admin_support for Blog

* Add decidim-budget admin sanitization

* Add decidim-consultations

* Enable iframes for decidim-pages

* Enable iframes for decidim-proposals

* Enable iframes for decidim-debates

* Enable iframes for decidim-sortitions

* Enable full editor in decidim-elections

* Enable full content in decidim-surveys

* revert sortitions

* Running Linters

* Revert proposals fields

* Fix Election spec

* Fix decidim assemblies

* Implement new restrictions

* Fix the scrubbers

* Add more restrictions to the scrubber scrubber

* Sort the restricted tags

* Revert strip tags for questions in consultations

* Fix consultations change requests
  • Loading branch information
alecslupu committed Apr 19, 2023
1 parent b69c8f5 commit 478bdb9
Show file tree
Hide file tree
Showing 37 changed files with 370 additions and 47 deletions.
Expand Up @@ -37,7 +37,7 @@ edit_link(
<div class="section">
<h2 class="show-for-sr"><%= t(".title") %></h2>
<div class="lead">
<%= decidim_sanitize_editor translated_attribute(current_participatory_space.short_description) %>
<%= decidim_sanitize_editor_admin translated_attribute(current_participatory_space.short_description) %>
</div>
<%= decidim_sanitize_editor_admin translated_attribute(current_participatory_space.description) %>
Expand All @@ -57,21 +57,21 @@ edit_link(
<% if translated_attribute(current_participatory_space.purpose_of_action).present? %>
<div class="section">
<h3 class="section-heading"><%= t("purpose_of_action", scope: "decidim.assemblies.show") %></h3>
<%= decidim_sanitize_editor translated_attribute(current_participatory_space.purpose_of_action) %>
<%= decidim_sanitize_editor_admin translated_attribute(current_participatory_space.purpose_of_action) %>
</div>
<% end %>
<% if translated_attribute(current_participatory_space.internal_organisation).present? %>
<div class="section">
<h3 class="section-heading"><%= t("internal_organisation", scope: "decidim.assemblies.show") %></h3>
<%= decidim_sanitize_editor translated_attribute(current_participatory_space.internal_organisation) %>
<%= decidim_sanitize_editor_admin translated_attribute(current_participatory_space.internal_organisation) %>
</div>
<% end %>
<% if translated_attribute(current_participatory_space.composition).present? %>
<div class="section">
<h3 class="section-heading"><%= t("composition", scope: "decidim.assemblies.show") %></h3>
<%= decidim_sanitize_editor translated_attribute(current_participatory_space.composition) %>
<%= decidim_sanitize_editor_admin translated_attribute(current_participatory_space.composition) %>
</div>
<% end %>
<div class="section text-center">
Expand Down
46 changes: 42 additions & 4 deletions decidim-assemblies/spec/system/assemblies_spec.rb
Expand Up @@ -6,14 +6,24 @@
describe "Assemblies", type: :system do
let(:organization) { create(:organization) }
let(:show_statistics) { true }
let(:base_description) { { en: "Description", ca: "Descripció", es: "Descripción" } }

let(:description) { { en: "Description", ca: "Descripció", es: "Descripción" } }
let(:short_description) { { en: "Short description", ca: "Descripció curta", es: "Descripción corta" } }
let(:purpose_of_action) { { en: "Purpose of action", ca: "Propòsit de l'acció", es: "Propósito de la acción" } }
let(:internal_organisation) { { en: "Internal organisation", ca: "Organització interna", es: "Organización interna" } }
let(:composition) { { en: "Composition", ca: "Composició", es: "Composición" } }
let(:closing_date_reason) { { en: "Closing date reason", ca: "Motiu de la data de tancament", es: "Razón de la fecha de cierre" } }
let(:base_assembly) do
create(
:assembly,
:with_type,
organization: organization,
description: base_description,
short_description: { en: "Short description", ca: "Descripció curta", es: "Descripción corta" },
description: description,
short_description: short_description,
purpose_of_action: purpose_of_action,
internal_organisation: internal_organisation,
composition: composition,
closing_date_reason: closing_date_reason,
show_statistics: show_statistics
)
end
Expand Down Expand Up @@ -182,7 +192,35 @@
let(:attached_to) { assembly }
end

it_behaves_like "has embedded video in description", :base_description
context "when having rich content" do
context "when short_description" do
it_behaves_like "has embedded video in description", :short_description
end

context "when description" do
before { click_button("Read more") }

it_behaves_like "has embedded video in description", :description
end

context "when purpose_of_action" do
before { click_button("Read more") }

it_behaves_like "has embedded video in description", :purpose_of_action
end

context "when internal_organisation" do
before { click_button("Read more") }

it_behaves_like "has embedded video in description", :internal_organisation
end

context "when composition" do
before { click_button("Read more") }

it_behaves_like "has embedded video in description", :composition
end
end

context "when the assembly has some components" do
it "shows the components" do
Expand Down
5 changes: 4 additions & 1 deletion decidim-blogs/spec/system/explore_posts_spec.rb
Expand Up @@ -55,7 +55,8 @@
describe "show" do
let(:posts_count) { 1 }
let(:author) { organization }
let!(:post) { create(:post, component: component, author: author) }
let(:body) { { en: "Short description", ca: "Descripció curta", es: "Descripción corta" } }
let!(:post) { create(:post, component: component, author: author, body: body) }

before do
visit resource_locator(post).path
Expand Down Expand Up @@ -96,6 +97,8 @@
expect(page).to have_content(post.created_at.strftime("%d/%m/%Y %H:%M "))
end

it_behaves_like "has embedded video in description", :body

it "shows the back button" do
expect(page).to have_link(href: "#{main_component_path(component)}posts")
end
Expand Down
@@ -1,7 +1,7 @@
<div class="row">
<div class="columns medium-7 mediumlarge-8">
<div class="section">
<%= decidim_sanitize_editor(landing_page_content) %>
<%= decidim_sanitize_editor_admin(landing_page_content) %>
</div>
</div>
</div>
Expand Up @@ -59,7 +59,7 @@ edit_link(
<%= render partial: "decidim/shared/static_map", locals: { icon_name: "projects", geolocalizable: project } %>
<% end %>
<%= cell("decidim/budgets/project_selected_status", project, as_label: true) %>
<%= decidim_sanitize_editor translated_attribute project.description %>
<%= decidim_sanitize_editor_admin translated_attribute project.description %>
<%= cell "decidim/budgets/project_tags", project, context: { extra_classes: ["tags--project"] } %>
</div>
<%= attachments_for project %>
Expand Down
10 changes: 10 additions & 0 deletions decidim-budgets/spec/system/explore_budgets_spec.rb
Expand Up @@ -44,6 +44,14 @@
end

describe "budget list item" do
let!(:component) do
create(:budgets_component,
:with_vote_threshold_percent,
manifest: manifest,
participatory_space: participatory_process,
settings: { landing_page_content: description })
end
let(:description) { { en: "Short description", ca: "Descripció curta", es: "Descripción corta" } }
let(:budget) { budgets.first }
let(:item) { page.find(".budget-list .card--list__item:first-child", match: :first) }
let!(:projects) { create_list(:project, 3, budget: budget, budget_amount: 10_000_000) }
Expand All @@ -52,6 +60,8 @@
login_as user, scope: :user
end

it_behaves_like "has embedded video in description", :description

it "has a clickable title" do
expect(item).to have_link(translated(budget.title), href: budget_path(budget))
end
Expand Down
12 changes: 12 additions & 0 deletions decidim-budgets/spec/system/explore_projects_spec.rb
Expand Up @@ -13,6 +13,18 @@
let!(:project) { projects.first }
let(:categories) { create_list(:category, 3, participatory_space: component.participatory_space) }

describe "show" do
let(:description) { { en: "Short description", ca: "Descripció curta", es: "Descripción corta" } }
let(:project) { create(:project, budget: budget, description: description) }

before do
visit_budget
click_link translated(project.title)
end

it_behaves_like "has embedded video in description", :description
end

describe "index" do
it "shows all resources for the given component" do
visit_budget
Expand Down
Expand Up @@ -34,9 +34,7 @@ def badge_name
# find the opening `<p>` tag and include the badge right after it. This
# makes the layout look good.
def description
text = super
text.sub!(/<p>/, "<p>#{render :badge}")
html_truncate(text, length: 100)
render(:badge) + truncate(strip_tags(super), length: 100)
end

def resource_path
Expand Down
@@ -0,0 +1,17 @@
# frozen_string_literal: true

module Decidim
module Consultations
class QuestionTitleScrubber < Decidim::UserInputScrubber
private

def custom_allowed_tags
%w(strong em u b i br ul ol li p a code)
end

def custom_allowed_attributes
%w(class href target rel)
end
end
end
end
@@ -1,6 +1,6 @@
<div class="row section" id="consultation-details">
<div class="columns medium-6 large-5 large-push-1">
<p class="lead"><%= decidim_sanitize_editor translated_attribute(consultation.description) %></p>
<p class="lead"><%= decidim_sanitize_editor_admin translated_attribute(consultation.description) %></p>
</div>
<div class="columns medium-6 large-5 large-pull-1">
<% if consultation.introductory_video_url.blank? %>
Expand Down
Expand Up @@ -3,7 +3,7 @@
<div class="columns mediumlarge-8 large-9 card--process__column">
<div class="card__content">
<%= link_to decidim_consultations.question_path(question), class: "card__link" do %>
<h2 class="heading5"><%= decidim_sanitize translated_attribute question.title %></h2>
<h2 class="heading5"><%= decidim_sanitize translated_attribute(question.title), strip_tags: true %></h2>
<% end %>
<p><%= translated_attribute question.subtitle %></p>
</div>
Expand Down
Expand Up @@ -10,7 +10,7 @@
<p><%= t "questions.vote_modal.contextual_help", scope: "decidim" %></p>
<div class="card card--secondary">
<div class="card__content">
<h4 class="heading5 text-center"><%= translated_attribute(question.title).html_safe %></h4>
<h4 class="heading5 text-center"><%= decidim_sanitize translated_attribute(question.title), scrubber: Decidim::Consultations::QuestionTitleScrubber.new %></h4>
</div>
</div>

Expand Down
Expand Up @@ -2,7 +2,7 @@
<div class="row">
<div class="columns medium-7 mediumlarge-8">
<div class="section">
<%= decidim_sanitize_editor translated_attribute current_question.question_context %>
<%= decidim_sanitize_editor_admin translated_attribute current_question.question_context %>

<div class="show-more">
<button class="button hollow small"><%= t "read_more", scope: "decidim.questions.show" %></button>
Expand All @@ -13,12 +13,12 @@

<div class="section">
<h2 class=section-heading><%= t "question.what_is_decided", scope: "activemodel.attributes" %></h2>
<p><%= decidim_sanitize_editor translated_attribute(current_question.what_is_decided), strip_tags: true %></p>
<p><%= decidim_sanitize_editor_admin translated_attribute(current_question.what_is_decided) %></p>
</div>

<div class="section">
<h2 class=section-heading><%= t "question.question_context", scope: "activemodel.attributes" %></h2>
<p><%= decidim_sanitize_editor translated_attribute(current_question.question_context), strip_tags: true %></p>
<p><%= decidim_sanitize_editor_admin translated_attribute(current_question.question_context) %></p>
</div>
</div>
</div>
Expand Down
Expand Up @@ -23,7 +23,7 @@
<%= yield :question_header_instructions if content_for? :question_header_instructions %>

<div class="row column consultations-title">
<h2 class="heading2"><%= decidim_sanitize translated_attribute question.title %></h2>
<h2 class="heading2"><%= decidim_sanitize translated_attribute(question.title), scrubber: Decidim::Consultations::QuestionTitleScrubber.new %></h2>
<% unless question.hashtag.blank? %>
<div class="text-center">
<%= link_to "##{question.hashtag}", twitter_hashtag_url(question.hashtag), target: "_blank" %>
Expand Down
@@ -0,0 +1,61 @@
# frozen_string_literal: true

require "spec_helper"

describe Decidim::Consultations::QuestionTitleScrubber do
subject { described_class.new }

def scrub(html)
Loofah.scrub_fragment(html, subject).to_s
end

RSpec::Matchers.define :be_scrubbed do
match do |actual|
expect(scrub(actual)).to eq actual
end

failure_message do |actual|
"expected \"#{actual}\" to eq \"#{scrub(actual)}\" after scrubbing"
end
end

RSpec::Matchers.define :be_scrubbed_as do |expected|
match do |actual|
expect(scrub(actual)).to eq expected
end

failure_message do |actual|
"expected \"#{actual}\" to eq \"#{expected}\" after scrubbing, scrubbed as \"#{scrub(actual)}\" instead"
end
end

it "does not allow iframes" do
html = "<iframe frameborder=\"0\" allowfullscreen=\"true\" src=\"url\"></iframe>"
expect(html).to be_scrubbed_as("")
end

it "does not allow comments" do
html = "<p>Hello, <!-- world! --></p>"
expect(html).to be_scrubbed_as("<p>Hello, </p>")
end

it "does not allow disabled iframes" do
html = %(<div class="disabled-iframe"><!-- <iframe src="url"></iframe> --></div>)
expect(html).to be_scrubbed_as("")
end

it "allows most basic tags" do
html = "<a></a><b></b><strong></strong><em></em><i></i><p></p><br>"
expect(html).to be_scrubbed
end

it "does not allow scripts" do
html = "<script></script>"
expect(html).to be_scrubbed_as("")
end

it "does not allow onerror attributes" do
html = "<img src=x onerror=alert(1)>"
expect(html).to be_scrubbed_as("")
end
end
8 changes: 7 additions & 1 deletion decidim-consultations/spec/system/consultation_spec.rb
Expand Up @@ -4,7 +4,9 @@

describe "Consultation", type: :system do
let!(:organization) { create(:organization) }
let!(:consultation) { create(:consultation, :published, organization: organization) }
let(:description) { { en: "Short description", ca: "Descripció curta", es: "Descripción corta" } }
let(:introductory_video_url) { "https://www.youtube.com/watch?v=1234567890" }
let!(:consultation) { create(:consultation, :published, organization: organization, description: description, introductory_video_url: introductory_video_url) }
let!(:user) { create :user, :confirmed, organization: organization }

before do
Expand All @@ -20,6 +22,10 @@
visit decidim_consultations.consultation_path(consultation)
end

it_behaves_like "has embedded video in description", :description do
let(:introductory_video_url) { nil }
end

it "Shows the basic consultation data" do
expect(page).to have_i18n_content(consultation.title)
expect(page).to have_i18n_content(consultation.subtitle)
Expand Down
18 changes: 16 additions & 2 deletions decidim-consultations/spec/system/question_spec.rb
Expand Up @@ -4,9 +4,11 @@

describe "Question", type: :system do
let(:organization) { create(:organization) }
let(:consultation) { create(:consultation, :published, organization: organization) }
let!(:consultation) { create(:consultation, :published, organization: organization) }
let(:question_context) { Decidim::Faker::Localized.wrapped("<p>", "</p>") { generate_localized_title } }
let(:what_is_decided) { Decidim::Faker::Localized.wrapped("<p>", "</p>") { generate_localized_title } }
let(:previous_question) { create :question, consultation: consultation }
let(:question) { create :question, consultation: consultation }
let(:question) { create :question, consultation: consultation, question_context: question_context, what_is_decided: what_is_decided }
let(:next_question) { create :question, consultation: consultation }

context "when shows question information" do
Expand All @@ -15,6 +17,18 @@
visit decidim_consultations.question_path(question)
end

context "when displaying question context" do
it_behaves_like "has embedded video in description", :question_context, count: 2 do
before { click_button("Read more") }
end
end

context "when displaying what is decided" do
it_behaves_like "has embedded video in description", :what_is_decided do
before { click_button("Read more") }
end
end

it "Shows the basic question data" do
expect(page).to have_i18n_content(question.promoter_group)
expect(page).to have_i18n_content(question.scope.name)
Expand Down
4 changes: 3 additions & 1 deletion decidim-core/app/scrubbers/decidim/admin_input_scrubber.rb
Expand Up @@ -14,12 +14,14 @@ module Decidim
class AdminInputScrubber < UserInputScrubber
private

DECIDIM_ALLOWED_TAGS = %w(img video audio source comment iframe).freeze

def custom_allowed_attributes
super + %w(frameborder allowfullscreen) - %w(onerror)
end

def custom_allowed_tags
super + %w(comment iframe)
super + DECIDIM_ALLOWED_TAGS
end
end
end

0 comments on commit 478bdb9

Please sign in to comment.