-
-
Notifications
You must be signed in to change notification settings - Fork 812
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
[REF] Replace create with writeRecord in Group #31095
base: master
Are you sure you want to change the base?
[REF] Replace create with writeRecord in Group #31095
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
@monishdeb this looks good, but the So I think we remove the deprecated warning but keep the But one more thing. Since we allow the |
193d124
to
224c10e
Compare
// Fill title and frontend_title if not supplied | ||
if (empty($params['id']) && empty($params['title'])) { | ||
$params['title'] = $params['frontend_title'] ?? $params['name']; | ||
} | ||
if (empty($params['id']) && empty($params['frontend_title'])) { | ||
$params['frontend_title'] = $params['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 see you were able to drop this code due to #31096 😁
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.
However, I think some of the test failures are due to missing fallbacks, so I think we need to add more fallbacks to Group.entityType.php
for title
and frontend_title
2eb51fe
to
3f0fcd1
Compare
3f0fcd1
to
bc91019
Compare
Overview
In order to keep BAO CRUD fns cleaner and central to, this patch move all the Create logics (that includes updating related entities, defaulting columns like frontend_title, name etc, rebuilding cache if parent groups are added/updated etc. ) onto pre and post hook. And so that we can avoid using deprecated CRM_Contact_BAO_Group::create and use DAO::writeRecord instead.
Before
CRM_Contact_BAO_Group::create
with additional logics before and after writing record.After
Move all the additional logics on to pre/post hook and call
writeRecord
for ...... obviously writing record only.Comments
ping @colemanw @eileenmcnaughton @seamuslee001