Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Whitespace disappears before vertical bar in email subject headings #1139

Closed
pcraig3 opened this issue Sep 17, 2020 · 2 comments · Fixed by cds-snc/notification-utils#61
Closed
Assignees

Comments

@pcraig3
Copy link
Contributor

pcraig3 commented Sep 17, 2020

Hello!

We are trying to send a subject line like Change your password | COVID Alert Portal, but the template strips the whitespace before the vertical bar.

While editing, it looks right:

image

But then on the "view template" screen, you can see the space is missing for the subject heading. (Not the sender though.)

image

Interestingly, it's also affecting your account modification email subject headings.

image

Again, pretty minor bug but I figured I should mention it.

@AntoineAugusti
Copy link
Contributor

Encountered this as well and it's annoying, even if it's a little thing

@AntoineAugusti AntoineAugusti transferred this issue from cds-snc/notification-admin Sep 21, 2020
@AntoineAugusti
Copy link
Contributor

Investigated a bit this issue and it's caused by the fact that there are a few rules to clean what's written in subject/body. Enforcing a space after a comma/dot, removing a space before a pipe (this bug) and many others.

The regex causing the pipe problem is here https://github.com/cds-snc/notification-utils/blob/39eb59a9b6befc5f6ad50d74f03ed277c858c9be/notifications_utils/formatters.py#L65. The subject field is cleaned this way https://github.com/cds-snc/notification-utils/blob/39eb59a9b6befc5f6ad50d74f03ed277c858c9be/notifications_utils/template.py#L275-L285. The rule we are interested in is remove_whitespace_before_punctuation.

I'd be in favour of removing the pipe from the list. It seems acceptable to have whitespace before a pipe.

@bryan-robitaille if you want to review other rules, they are located in this file https://github.com/cds-snc/notification-utils/blob/master/notifications_utils/template.py and method names read nicely

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants