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

mail#46 - show label, not value, on contribution custom field tokens #14658

Merged
merged 4 commits into from
Jul 22, 2019

Conversation

MegaphoneJon
Copy link
Contributor

Overview

When using contribution tokens that have option values ("multiple choice"), the token renders the value instead of the label.

Before

The token renders the value, e.g., "1", "2".

After

The token should render the label of the selected option, e.g. "Less than 1 year", "1-3 years".

Technical Details

Steps to replicate this are available on the lab.c.o ticket.

Comments

This also fixes a bug in CiviUnitTestCase::customFieldOptionValueCreate() where option values are created as inactive. I looked at the other places this method is called and it shouldn't have any effect.

@civibot
Copy link

civibot bot commented Jun 27, 2019

(Standard links)

@MegaphoneJon
Copy link
Contributor Author

Turns out that the thank-you letters use a different code path for tokens than contribution emails 👎 . I just updated this PR to extract the new code I wrote into a new function that can be called from both places, which seems like a valuable first step toward sharing the code there.

@eileenmcnaughton
Copy link
Contributor

@MegaphoneJon test fails looks like it is test clean up related

if (!empty($messageToken)) {
foreach ($result['values'][$id] as $fieldName => $fieldValue) {
if (strpos($fieldName, 'custom_') === 0 && array_search($fieldName, $messageToken['contribution']) !== FALSE) {
$result['values'][$id][$fieldName] = CRM_Core_BAO_CustomField::displayValue($result['values'][$id][$fieldName], $fieldName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be also written as

$result['values'][$id][$fieldName] = CRM_Core_BAO_CustomField::displayValue($fieldValue, $fieldName);

@eileenmcnaughton
Copy link
Contributor

Note the test error is

Stacktrace
CRM_Contribute_BAO_ContributionTest::testReplaceContributionTokens
The html does not match
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
 '<p>Contribution Source: ABC</p></br>\n
       <p>Contribution Invoice ID: 12345</p></br>\n
       <p>Contribution Receive Date: May 11th, 2015</p></br>\n
-      <p>Contribution Custom Field 1: Label2</p></br>'
+      <p>Contribution Custom Field 1: </p></br>'

/home/jenkins/bknix-dfl/build/core-14658-3vz3c/sites/all/modules/civicrm/tests/phpunit/CRM/Contribute/BAO/ContributionTest.php:1524
/home/jenkins/bknix-dfl/build/core-14658-3vz3c/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:214
/home/jenkins/bknix-dfl/civicrm-buildkit/extern/phpunit6/phpunit6.phar:570

@MegaphoneJon
Copy link
Contributor Author

Ah - figured it out. The test runs fine in isolation, but hard-codes a custom field ID that can't be hard-coded, or it fails when running the entire test class.

@eileenmcnaughton
Copy link
Contributor

I was able to confirm this doing a 'r-run' & creating a letter. I note it's still broken in other paths (ie. emailing a receipt) but this takes us forward.

I also note @pradpnayak made some stylistic suggestions but given this stalled on the test challenges I think they are non-blocking

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.

3 participants