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

Fix errors in pluralisation and remove dots #616

Merged
merged 3 commits into from
Dec 19, 2019
Merged

Fix errors in pluralisation and remove dots #616

merged 3 commits into from
Dec 19, 2019

Conversation

cintamani
Copy link
Contributor

From: https://eaflood.atlassian.net/browse/RUBY-814

QA have found out some inconsistencies with the use of dots and pluralization of order descriptions. This PR includes fixes for both. Instructions will be give to deal with legacy data during testing.
There is also a fix about the email address used in the wireframe, which didn't match what is currently sent out to the user.

From: https://eaflood.atlassian.net/browse/RUBY-814

QA have found out some inconsistencies with the use of dots and pluralization of order descriptions. This PR includes fixes for both. Instructions will be give to deal with legacy data during testing.
There is also a fix about the email address used in the wireframe, which didn't match what is currently sent out to the user.
@cintamani cintamani added the enhancement New feature or request label Dec 19, 2019
@cintamani cintamani self-assigned this Dec 19, 2019
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

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

Comments to come

@@ -4,26 +4,26 @@ en:
send_order_completed_email:
subject: "We’re printing your waste carriers registration cards"
dear: "Dear %{full_name}"
heading: "We’re printing your waste carriers registration cards. They should arrive within 10 working days."
heading: "We’re printing your waste carriers registration cards. They should arrive within 10 working days"
Copy link
Member

Choose a reason for hiding this comment

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

I have spoken with @andrewhick and confirmed this. If we are trying to ensure the pluralisation is correct for copy cards, then this should include the subject and heading in the email as well, not just the back office UI.

And as this PR is focused on fixing these issues, I thought it would make sense to make those changes here rather than as subsequent one. However, if you don't agree I'm happy to approve. We'll just need another PR for handling pluralisation in the email content.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since doing that requires to add a quantity field to the OrderItem object, I think I will open a different PR :)

Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

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

Agree with @cintamani . We need to make structural changes to the registration to handle this, so is best done in a separate PR.

@cintamani cintamani merged commit ca89e49 into master Dec 19, 2019
@cintamani cintamani deleted the 814-qa branch December 19, 2019 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants