-
Notifications
You must be signed in to change notification settings - Fork 109
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
Fix state display on preview #941
Conversation
Tested. It worked. |
I kicked off the tests by making an edit to the main.yml file that I needed to make any way! :-) |
My client is reporting that this doesn't fix the [webform_submission::values] token in emails. I'll try to look into this myself first though, since I think this PR is most of the fix. I think we may just need to check if the value is numeric rather than whether the type is "Text". |
$state_id = $value; | ||
} | ||
else { | ||
$state_id = $value['#plain_text'] ?? NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're sending an email with state_province_id
fields, and the email is in HTML format, $value['#plain_text']
doesn't exist. I this we want $state_id = $value['#plain_text'] ?? $value['#markup'] ?? NULL;
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MegaphoneJon If i use the default body having[webform_submission:values]
token to render all submission, it does work for me after the current PR. (Screenshot from email)
If i use a custom body with field tokens [webform_submission:values: civicrm_1_contact_1_address_state_province_id]
, it still works and renders the state correctly in email
The debug value of $text is Html & $value does have #plain_text
in this function -
I'm fine to include your change if $value['#markup']
works for you. But curious to know your usecase since we have a similar implementation for Existing Contact, so i think [webform_submission:values:civicrm_1_contact_1_contact_existing]
token might break for you too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Jitendra,
That's not what I'm seeing. Attached is a webform export that displays the state/province as an ID.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
92152b1
to
4b57ac4
Compare
4b57ac4
to
ed16a2a
Compare
I've been running the current version of this patch in production for a few months now with no issues, it looks good. I recommend removing the commented-out line in |
ah thanks for pointing that out. Have removed the line. |
Thank you all! |
Overview
Fix state display on preview
Before
After
Technical Details
The state has numerous options that we do not define as
#options
in the YAML configuration due to its daisy-chaining load based on country selection.This PR adds a format function for label displays. I believe it should also fix the tokens @MegaphoneJon.
Comments
Drupal Ticket: https://www.drupal.org/project/webform_civicrm/issues/3410313