Fix dev/core issue-5155: Contribution page help text does not match f…

[artfulrobot]

…orm text

See
https://lab.civicrm.org/dev/core/-/issues/5155

[civibot]

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...

• If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
• A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
• A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
• If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
• Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
• A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
• To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
• For more information about contributing, see CONTRIBUTING.md.

Quick links for reviewers...

• ➡️ PR Demo site (can be rebuilt with the command test this please).
• 📖 Review standards
• 🗒️ Review template (brief or verbose)

➡️ Online demo of this PR 🔗

[demeritcowboy]

I'm not sure what the best solution is but e.g. for paypal it does say "Continue", e.g.: https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/Payment.php#L604

[alifrumin]

I tested this fix and it works for me!

Thank you!!!!

[alifrumin]

Code change seems reasonable too

[artfulrobot]

@demeritcowboy "click Continue/Make Payment" ?

[demeritcowboy]

This could become a long conversation. What's funny is right before it assigns the text it knows the button text already:

civicrm-core/CRM/Contribute/Form/Contribution/Confirm.php

Lines 567 to 574 in 799d421

	$this->assign('button', $contributionButtonText);
	$this->assign('continueText',
	$this->getPaymentProcessorObject()->getText('contributionPageContinueText', [
	'is_payment_to_existing' => !empty($this->_ccid),
	'amount' => $this->_amount,
	])
	);

, but changing it to insert it as a %1 isn't as good for translation.

Hmm. I probably shouldn't have said anything... but if it's going to break already translated strings then we should think a little bit.

[aydun]

Irrelevant to the issue, but just noting the PR number - that's a lot of PR's!

• CRM-11926 Improve error message on APi failure to include DB error message #1 2013
• CRM-19710 - Preserve is_email_receipt parameter through to email sent #10000 2017
• 5.37 #20000 2021
• Fix dev/core issue-5155: Contribution page help text does not match f… #30000 2024

[artfulrobot]

but if it's going to break already translated strings then we should think a little bit.

yes, I think so too.

Thing is, I can't close this PR; 2013 looks so shameful in that list with its red icon... 😆 But then again I don't have the energy for one of those long conversations ↑ ...

Marked this draft for now. Will close in a bit if we don't come up with a sensible solution.

[demeritcowboy]

How about doing the same if block as in the button text? It duplicates some logic, but they are called right after each other, from the same place, and the params are the same.

        if ($params['amount'] <= 0.0 || (int) $this->_paymentProcessor['billing_mode'] === 4) {
          return ts('Click the <strong>Continue</strong> button to proceed with the payment.');
        }
        if ($params['is_payment_to_existing']) {
          return ts('Click the <strong>Make Payment</strong> button to proceed with the payment.');
        }
        return ts('Click the <strong>Make Contribution</strong> button to proceed with the payment.');

[artfulrobot]

Preach! sounds like idea. I've done that. @alifrumin would you mind testing this again with the latest changes?

[artfulrobot]

(aside: oh look, we also use the %1 solution in a really weird way at https://github.com/civicrm/civicrm-core/pull/30000/files/2d23553c35e3e5e4237d91f4f80f254e43cba565#diff-3d2a990f0b1960f0b3e6671c645e18033883e6d1eaf535b8d19d085b67dde00bR598-R600)

[demeritcowboy]

echo ts('%1', [1 => ts('Yep')]);

[alifrumin]

Looks good to me!

[demeritcowboy]

Thanks all.