-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Participant - Translate untranslated string #27837
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing translation is good, but we lose some information with the change from CRM_Event_BAO_Event::getEvents()
to CRM_Core_DAO::getFieldValue()
. For example:
Before this PR: "Registration selections changed for Fall Fundraiser Dinner - March 19th, 2024 5:00 PM"
After this PR: "Registration selections changed for Fall Fundraiser Dinner"
(note: to |
Thanks @highfalutin - is the old version actually helpful? All of that code looks half-assed to me so I figured it was there by mistake. IMO adding the date to the subject is potentially confusing because it's unclear what date it's referring to: you'd think it's the date that the selections were changed, but it's not. |
It is possible that an org would have multiple events with the same title on different dates (recurring events?), making it helpful to see the event date at a glance. However, my guess is it's rare to have same-name events and have fee-selection changes that someone is looking for in the list of activities. And anyway, the activity is linked to the event via the participant record ( So, my |
If we get any, I guarantee they won't be in any language other than English! |
Good point 😆 |
Overview
Simplifies event title lookup and uses
ts()
instead ofsprintf()
so the string will be translated.