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

Use field value function to get html signature #22445

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Use field value function to get html signature

Before

Enotice when accessing signature_text if not set

After

no notice

Technical Details

The getFieldValue function is used for all other tokens on the contact object and will better access
the right value and return an empty string rather NULL if not set.

I hit this as an enotice writing a test - but it is possibly the cause of this issue
eileenmcnaughton/nz.co.fuzion.civitoken#58 (comment)

hence targetting the rc

The tokens() function does weird things if you pass it NULL

Comments

The  function is used for all other tokens on the contact object and will better access
the right value and return an empty string rather NULL if not set.

I hit this as an enotice writing a test - but it is possibly the cause of this issue
eileenmcnaughton/nz.co.fuzion.civitoken#58 (comment)

hence targetting the rc

The tokens() function does weird things if you pass it NULL
@civibot
Copy link

civibot bot commented Jan 10, 2022

(Standard links)

@civibot civibot bot added the 5.46 label Jan 10, 2022
@seamuslee001
Copy link
Contributor

This seems fine to me

@totten
Copy link
Member

totten commented Jan 11, 2022

The description here seems to focus on describing the code mechanics (eg "Changes code to use a widget to get a gizmo").

This Github page would be the main link for the issue in the 5.45.1 release notes, so it'd be useful to give some general sense of the subsystem/use-case impacted. Like... do I interpret correctly that....

  • The general heading/subject could fairly say: "Mail Merge - Fix warnings when processing certain empty tokens" (ie it's a problem that affects any mail-merge use-cases - like CiviMail/PDFs/Receipts)
  • You've seen that this patch fixes a warning - and strongly suspect that it also fixes a complex regression (involving some combination of civicrm-core + civitokens + gdpr). It's worth publishing because it's safe and maybe/possibly/probably fixes a regression (but the circumstance of the regression is still a bit too complex/unclear to say positively)...?

@eileenmcnaughton
Copy link
Contributor Author

@totten yeah - I think it might only fix one token not working properly actually (email_html) or just prevent a notice on it. I couldn't replicate

I think like most bugs in the token code the complex one is that NULL is being passed to $row->format('text/html')->tokens() - but I think it might not be in this place

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

Successfully merging this pull request may close these issues.

3 participants