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

REF Convert deprecated functions to buildOptions for case #13364

Merged

Conversation

mattwire
Copy link
Contributor

Overview

Convert CRM_Core_PseudoConstant::activityType() and CRM_Core_PseudoConstant::activityStatus() to non deprecated buildOptions() function.

Fix a couple of PHP notices.

Before

Deprecated code, php notices.

After

Less deprecated code, less PHP notices

Technical Details

In most cases these methods are functionally the same. In some cases the buildOptions() replacement for activityType() function also returns campaign activities but this does not matter because the activity type arrays are being used to lookup (via array_search) specific activity types.

Comments

@mattwire mattwire force-pushed the activitytype_pseudoconstant branch 2 times, most recently from 9da60b3 to 889e61f Compare December 28, 2018 13:43
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@mattwire mattwire force-pushed the activitytype_pseudoconstant branch from 889e61f to 0f103b7 Compare December 29, 2018 12:27
@mattwire
Copy link
Contributor Author

mattwire commented Feb 7, 2019

@colemanw @seamuslee001 Either of you able to give this a quick review?

@@ -112,8 +112,8 @@ public static function actionLinks(
$activityId = NULL,
$key = NULL,
$compContext = NULL) {
static $activityActTypes = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes to this file checked & oK

@@ -1058,7 +1058,6 @@ public static function getCaseActivity($caseID, &$params, $contactID, $context =
$caseCount = CRM_Core_DAO::singleValueQuery('SELECT FOUND_ROWS()');
Copy link
Contributor

Choose a reason for hiding this comment

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

this file is ok

@eileenmcnaughton eileenmcnaughton added the merge ready PR will be merged after a few days if there are no objections label Feb 17, 2019
@eileenmcnaughton
Copy link
Contributor

@mattwire I think there are a few places where this could be a little improved but none are blocking so if you don't wish to revisit them I'm OK to merge - although I don't think they will get later if you don't

@mattwire mattwire force-pushed the activitytype_pseudoconstant branch from 0f103b7 to 003de00 Compare February 22, 2019 16:32
@mattwire mattwire force-pushed the activitytype_pseudoconstant branch from 003de00 to b864360 Compare February 22, 2019 16:41
@mattwire
Copy link
Contributor Author

@eileenmcnaughton I've addressed your comments - can we merge?

@eileenmcnaughton eileenmcnaughton merged commit b6b7691 into civicrm:master Feb 22, 2019
@mattwire mattwire deleted the activitytype_pseudoconstant branch March 1, 2019 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants