-
-
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
Rename 'Provider' entity to 'SmsProvider' #29555
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
OK - test cover should be good on these changes |
@eileenmcnaughton I added this FIXME 10 years ago! |
@colemanw well done! But, I bet I can find an older fixme.... |
dang test fail is real - ironically the one I wanted to tag ornery api_v3_SyntaxConformanceTest::testCustomDataGet with data set #75 ('SmsProvider') /home/homer/buildkit/build/build-4/web/sites/all/modules/civicrm/CRM/Core/DAO/AllCoreTables.php:365 |
Renames BAO, DAO & Api4 'Provider' to 'SmsProvider', as the previous name was too ambiguous. This fixes a 10-year-old FIXME!
@eileenmcnaughton I had just missed a couple spots. Good thing the test caught it! |
At the sprint we wanted to finish some AdminUI work, and we looked at #28512 but got confused because we couldn't find the SmsProvider api entity... then finally realized that it does exist just has a terrible name. |
Overview
Fixes a longstanding
fixme
to rename a badly-named entity.Before
This FIXME was added in 2013!
civicrm-core/api/v3/utils.php
Lines 317 to 319 in 25bc61b
After
Renamed. Won't impact extensions because class_aliases have been added.
Technical Details
This is the same solution we've used for other entities that needed to be renamed.
class_alias
works well to preserve backward-compatibility.Comments
Targeting the RC because this API was added very recently in #28510