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] Preliminary cleanup in update greeting #21909

Merged
merged 1 commit into from
Nov 12, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 23, 2021

Overview

[REF] Preliminary cleanup in update greeting

Before

Code is sooooo confusing

After

Code to get the greeting strings extracted

Technical Details

We start with

if ($contact->email_greeting_custom != 'null' && $contact->email_greeting_custom) {

}
elseif ($contact->email_greeting_id != 'null' && $contact->email_greeting_id) {
}
else {
   if ($contact->email_greeting_custom) {
}

Which leaves it very unclear what the last else is for.

My analysis is that this is

  • if email_greeting_custom has a real value then use that as the email greeting template,
  • if not but a real value (which means a number) has been passed for email_greeting_id then use the template corresponding to the id supplied

So the if clause winds up like

 if (!CRM_Utils_System::isNull($contact->email_greeting_custom)) {
}
 elseif (is_numeric($contact->email_greeting_id)) {
elseif ($contact->email_greeting_custom === 'null') {
}

Comments

@civibot
Copy link

civibot bot commented Oct 23, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

failures are now fixed in master - test this please

@eileenmcnaughton eileenmcnaughton force-pushed the greeting_sanity branch 2 times, most recently from bf1a694 to b87e93c Compare November 11, 2021 21:50
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 as dicussed - I took another look at where this got to & rebased & refreshed this PR to reflect interim merges

@seamuslee001
Copy link
Contributor

This seems fine to me

@eileenmcnaughton eileenmcnaughton force-pushed the greeting_sanity branch 2 times, most recently from d26779e to ec8ca30 Compare November 12, 2021 00:38
@eileenmcnaughton
Copy link
Contributor Author

unrelated fail

@eileenmcnaughton eileenmcnaughton merged commit 96b7d62 into civicrm:master Nov 12, 2021
@eileenmcnaughton eileenmcnaughton deleted the greeting_sanity branch November 12, 2021 20:10
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