Skip to content

Restore consultation description HTML format tags#6893

Merged
Leusev merged 1 commit intodecidim:release/0.22-stablefrom
coopdevs:fix/consultation-description-rich-text
Nov 30, 2020
Merged

Restore consultation description HTML format tags#6893
Leusev merged 1 commit intodecidim:release/0.22-stablefrom
coopdevs:fix/consultation-description-rich-text

Conversation

@sauloperez
Copy link
Copy Markdown
Contributor

@sauloperez sauloperez commented Nov 19, 2020

🎩 What? Why?

This fixes a regression introduced in #5684. The formatting of a consultation's description was totally gone.

📌 Related Issues

Link your PR to an issue

Testing

Checking the consultation's public page is enough to see whether the formatting is applied or not.

📋 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

Before:
Screenshot from 2020-11-19 09-24-36

After: Screenshot from 2020-11-19 09-20-26

@sauloperez
Copy link
Copy Markdown
Contributor Author

sauloperez commented Nov 19, 2020

Could someone rerun the lint code CI job for me 🙏 it's passing locally and by looking at the error I bet it's temporal.

@mrcasals
Copy link
Copy Markdown
Contributor

mrcasals commented Nov 19, 2020

@sauloperez done!

@sauloperez
Copy link
Copy Markdown
Contributor Author

sauloperez commented Nov 19, 2020

very weird 🤔 I'm getting the following on the Lint code job

Run ./.github/run_erblint.sh
bundler: failed to load command: erblint (/home/runner/work/decidim/decidim/vendor/bundle/ruby/2.5.0/bin/erblint)
NameError: uninitialized constant BetterHtml::Parser::HtmlError
Did you mean?  HtmlTokenizer

does it ring a bell to anyone?

@sauloperez sauloperez force-pushed the fix/consultation-description-rich-text branch 2 times, most recently from 3d68e0f to fc95f75 Compare November 20, 2020 14:30
@microstudi
Copy link
Copy Markdown
Contributor

microstudi commented Nov 20, 2020

very weird thinking I'm getting the following on the Lint code job

Run ./.github/run_erblint.sh
bundler: failed to load command: erblint (/home/runner/work/decidim/decidim/vendor/bundle/ruby/2.5.0/bin/erblint)
NameError: uninitialized constant BetterHtml::Parser::HtmlError
Did you mean?  HtmlTokenizer

does it ring a bell to anyone?

happening to mee since yesterday, I'm disabling erblint for the moment until someone figures out what's happening

@sauloperez
Copy link
Copy Markdown
Contributor Author

sauloperez commented Nov 20, 2020 •
edited
Loading

Blindly upgrading the workflow to Ruby 2.6 didn't fix it 😅 . I don't know what could be different between GH Actions and my machine. It's a load error in better_html but the necessary require is already there. We might need to open an issue.

@sauloperez
Restore consultation description HTML format tags
308ecdc 
Without it all formatting in the description is lost.
@sauloperez sauloperez force-pushed the fix/consultation-description-rich-text branch from b8e1133 to 9608fef Compare November 23, 2020 10:15
@sauloperez sauloperez mentioned this pull request Nov 23, 2020
Closed
12 tasks
@sauloperez
Copy link
Copy Markdown
Contributor Author

sauloperez commented Nov 23, 2020

I'm trying to fix the erblint issue in #6913.

@microstudi
Copy link
Copy Markdown
Contributor

microstudi commented Nov 23, 2020

Strangely, my PR had no problems... #6899

@sauloperez
Copy link
Copy Markdown
Contributor Author

sauloperez commented Nov 26, 2020 •
edited
Loading

The only difference I see is that it's only affecting PRs against the 0.22 branch and not develop.

@sauloperez sauloperez force-pushed the fix/consultation-description-rich-text branch from 74e5f33 to c2920e0 Compare November 26, 2020 11:40
This was referenced Nov 26, 2020
Merged
Merged
@Leusev Leusev self-requested a review November 27, 2020 07:42
Leusev
Leusev suggested changes Nov 27, 2020
View reviewed changes
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.

Hi @sauloperez
we'll check your #6913 PR in order to fix ErbLint error, but by the moment, for this PR I thinks it would be better that you revert "Backport Lint Code CI Job from develop" commit to not mix things please.
Thanks in advance! 😄

@sauloperez sauloperez force-pushed the fix/consultation-description-rich-text branch from c2920e0 to 308ecdc Compare November 27, 2020 08:07
@sauloperez
Copy link
Copy Markdown
Contributor Author

sauloperez commented Nov 27, 2020

Thanks for the heads-up @Leusev . I removed all commits except for waht brought me here, the fix to consultation_details 😂

@Leusev Leusev self-requested a review November 27, 2020 08:22
Leusev
Leusev reviewed Nov 27, 2020
View reviewed changes
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.

Hi @sauloperez
perfect! 😄 When all tests have passed, I'll merge this
Thanks a lot for your big effort trying to solve ErbLint error also! 👍

@Leusev Leusev self-requested a review November 30, 2020 08:24
@Leusev Leusev merged commit ad71041 into decidim:release/0.22-stable Nov 30, 2020
sauloperez added a commit to CoopCat-Confederacio-de-Cooperatives/decidim-coopcat that referenced this pull request Nov 30, 2020
@sauloperez
Replace Coopdevs fork with Decidim's 0.22 branch
a0c1f19 
Now we can point back to upstream because
decidim/decidim#6938,
decidim/decidim#6905 and
decidim/decidim#6893 have been merged. There
were also other bug fixes in between.
@sauloperez sauloperez mentioned this pull request Nov 30, 2020
Merged
microstudi pushed a commit to Platoniq/decidim that referenced this pull request Dec 16, 2020
@sauloperez @microstudi
Restore consultation description HTML format tags (decidim#6893)
aba9298 
Without it all formatting in the description is lost.
@Leusev Leusev mentioned this pull request Jan 25, 2021
Merged
12 tasks
davidbeig pushed a commit to CoopCat-Confederacio-de-Cooperatives/decidim-coopcat that referenced this pull request Dec 2, 2025
@sauloperez
Replace Coopdevs fork with Decidim's 0.22 branch
d03ff80 
Now we can point back to upstream because
decidim/decidim#6938,
decidim/decidim#6905 and
decidim/decidim#6893 have been merged. There
were also other bug fixes in between.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Leusev Leusev Leusev approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

in-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sauloperez @mrcasals @microstudi @Leusev