Skip to content

Fix avoid removing tag style on custom sanitize#7018

Merged
ivan-mr merged 4 commits intodevelopfrom
fix/avoid_removing_style_on_sanitize
Dec 15, 2020
Merged

Fix avoid removing tag style on custom sanitize#7018
ivan-mr merged 4 commits intodevelopfrom
fix/avoid_removing_style_on_sanitize

Conversation

@ivan-mr
Copy link
Copy Markdown
Contributor

@ivan-mr ivan-mr commented Dec 14, 2020

🎩 What? Why?

The preview mailer continues showing some styles rules due to something is removing style tag.
In PR #6876 there was an adapter that avoid that in the premailer process. But also, there was a custom sanitize process on the image_text_cta cell that removes it. With this PR I widen the rules adding style tag to avoid the deletion.

📌 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.

  • Create a Decidim instance with the default seed data
  • Make sure you have defined the organization primary color
  • Go to the newsletter section and click to new one
  • See the beginning of the complete (not basic) template email.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Before
Captura de pantalla de 2020-12-14 20-06-18

After
Captura de pantalla de 2020-12-14 20-05-46

♥️ Thank you!

Comment thread decidim-core/app/scrubbers/decidim/user_input_scrubber.rb Outdated
Copy link
Copy Markdown
Contributor

@tramuntanal tramuntanal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tramuntanal tramuntanal requested a review from mrcasals December 15, 2020 11:33
expect(helper.decidim_sanitize(user_input)).to include("<p>")
expect(helper.decidim_sanitize(user_input)).to include("</p>")
expect(helper.decidim_sanitize(user_input)).to eq(user_input)
expect(helper.decidim_sanitize_newsletter(user_input)).to include("<p>")
Copy link
Copy Markdown
Contributor

@mrcasals mrcasals Dec 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are a little weird because they're testing two different methods, I think it's probably better to have different specs! 😄

Copy link
Copy Markdown
Contributor

@Leusev Leusev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Looks good @ivan-mr ! 👍
Thanks!

@ivan-mr ivan-mr removed the in-review label Dec 15, 2020
@ivan-mr ivan-mr merged commit a013a4c into develop Dec 15, 2020
@ivan-mr ivan-mr deleted the fix/avoid_removing_style_on_sanitize branch December 15, 2020 14:53
@mrcasals mrcasals added type: fix PRs that implement a fix for a bug and removed type: bug labels Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: core type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants