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

Add unit test on updateGreeting & remove deprecated fn call #22482

Merged
merged 1 commit into from
Jan 16, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Add unit test on updateGreeting & remove deprecated fn call

Before

The call to tokenDetails loads the contact details and they are passed along - however
if not loaded here they will be loaded later so there is no gain & it
calls a deprecated function

After

poof

Technical Details

Comments

The call to tokenDetails loads the contact details and they are passed along - however
if not loaded here they will be loaded later so there is no gain & it
calls a deprecated function
@civibot
Copy link

civibot bot commented Jan 12, 2022

(Standard links)

@civibot civibot bot added the master label Jan 12, 2022
@@ -1034,17 +1034,12 @@ public static function updateGreeting($params) {
}

if (empty($filterContactFldIds)) {
$greetingDetails = [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if greetingDetails & filterContactFldIds are empty it is the same as returning as the following code is only invoked if they are not empty

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 can I get a merge on this - we didn't remove it last round because of the lack of test but ..... tada

@seamuslee001
Copy link
Contributor

seem fines to me

@seamuslee001 seamuslee001 merged commit 3e60b6f into civicrm:master Jan 16, 2022
@seamuslee001 seamuslee001 deleted the token branch January 16, 2022 22:24
@eileenmcnaughton
Copy link
Contributor Author

thanks @seamuslee001

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants