Skip to content

Add HTML escaping to the spec expectations as the strings are escaped#7760

Merged
mrcasals merged 1 commit intodecidim:developfrom
mainio:fix/user-group-mentioned-event-html-escaping
Mar 27, 2021
Merged

Add HTML escaping to the spec expectations as the strings are escaped#7760
mrcasals merged 1 commit intodecidim:developfrom
mainio:fix/user-group-mentioned-event-html-escaping

Conversation

@ahukkanen
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen commented Mar 26, 2021 •
edited
Loading

🎩 What? Why?

This spec failed for me locally because some of the names contained single quote characters.

This adds HTML escaping to the expectations to match what is generated by the event class.

The failures that I saw:

  1) Decidim::Comments::UserGroupMentionedEvent email_outro is generated correctly
     Failure/Error:
       expect(subject.email_outro)
         .to eq("You have received this notification because you are a member of the group #{group.name} that has been mentioned in #{translated resource.title}.")
     
       expected: "You have received this notification because you are a member of the group Mayer, O'Reilly and Bauch 22 that has been mentioned in Lanny Flatley 146."
            got: "You have received this notification because you are a member of the group Mayer, O'Reilly and Bauch 22 that has been mentioned in Lanny Flatley 146."
     
       (compared using ==)
     # ./spec/events/decidim/comments/user_group_mentioned_event_spec.rb:51:in `block (3 levels) in <top (required)>'

  2) Decidim::Comments::UserGroupMentionedEvent email_subject when the group name contains HTML-escapable characters is generated correctly
     Failure/Error: expect(subject.email_subject).to eq("You have been mentioned in #{translated resource.title} as a member of #{group.name}")
     
       expected: "You have been mentioned in Chara Tillman 174 as a member of O'Group"
            got: "You have been mentioned in Chara Tillman 174 as a member of O&#39;Group"

Testing

Force e.g. the group name to be generated using a string that contains characters that need HTML escaping. For example:

  let(:group) { create :user_group, name: "O'Group", organization: comment.organization, users: [comment.author, member] }

📋 Checklist

  • ❓ 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.

@mrcasals mrcasals merged commit 1f9a850 into decidim:develop Mar 27, 2021
@ahukkanen ahukkanen deleted the fix/user-group-mentioned-event-html-escaping branch March 27, 2021 10:03
entantoencuanto added a commit that referenced this pull request Mar 31, 2021
@entantoencuanto
Merge branch 'develop' into chore/rails-6-upgrade
70c3d12 
* develop: (26 commits)
  Fix trustees admin menu (#7772)
  Do not modify the controller class in the controller tests that render views (#7755)
  Add HTML escaping to the expectations as the strings are escaped (#7760)
  Add automated accessibility audit + HTML validation to CI pipeline (#7751)
  fix(elections): js assets manifest (#7759)
  Add admin missing translations (#7702)
  Add Conferences and Admin missing translations (#7653)
  New Crowdin updates (#7735)
  Improve vote flow (#7682)
  Strip the <p> tags from inside the heading elements (#7732)
  Fix the date cell spec failing randomly close to day changes (#7703)
  Change the timeline date color for accessible color contrast against its background (#7750)
  Remove the opacity from process upcoming/past/all filters for accessible contrast (#7749)
  Fix color contrast against the sidebar navigation background (#7748)
  Validate the HTML for the account page (#7747)
  Fix report modal form accessibility (#7746)
  Accessibility fixes for conversations (#7745)
  Add a landmark ARIA role to the cookie banner (#7738)
  Fix HTML validation on standalone content page (#7744)
  Add aria-label to the area filter on participatory space pages (#7743)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mrcasals mrcasals mrcasals approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

target: developer-experience

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ahukkanen @mrcasals