-
-
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
Assign profileID to template in UFNotify, add example #29441
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
57e0a04
to
6d09d5e
Compare
protected $groupTitle; | ||
|
||
public function getGroupTitle(): string { | ||
return CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFGroup', $this->getProfileID(), 'title'); |
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.
@eileenmcnaughton should this be the public title?
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.
@seamuslee001 yeah that makes sense - it wasn't before but I think it should be
6d09d5e
to
3b3b51c
Compare
@pradpnayak does this work for you? |
protected $groupTitle; | ||
|
||
public function getGroupTitle(): string { | ||
return CRM_Core_DAO::getFieldValue('CRM_Core_DAO_UFGroup', $this->getProfileID(), 'frontend_title'); |
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.
I got error because my profile had missing frontend_title
TypeError: CRM_Core_WorkflowMessage_UFNotify::getGroupTitle(): Return value must be of type string, null returned in CRM_Core_WorkflowMessage_UFNotify->getGroupTitle() (line 65 of /Users/pradeep/Sites/drupal7/sites/all/modules/civicrm/CRM/Core/WorkflowMessage/UFNotify.php).
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.
How about using
CRM_Core_BAO_UFGroup::getFrontEndTitle($this->getProfileID());
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.
@pradpnayak did you test this by backporting it - in 5.72 frontend_title should be a required field
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.
My bad, tested by upgrading to 5.71 beta. The frontend title is not updated with the title field and the field is required through UI.
This look good to merge besides the return type fatal error mentioned above. Thank you so much @eileenmcnaughton for doing a alternate PR. |
I have tested the above failure and it seems to work fine. This PR is good to merge. |
@seamuslee001 any last concerns? |
Overview
Assign profileID to template in UFNotify, add example
Before
Not possible to render example,
profileID
not available, userDisplayName mixed with the person the profile relates toAfter
Technical Details
This started out as just a basic alternative to #29439 but I got carried away
Comments