Fix ContributionRecur.payment_processor_id pseudoconstant

[colemanw]

Overview

Fixes several bugs introduced by #13698 which are causing a test failure in #22740.

Before

• Pseudoconstant mistakenly uses 'name' instead of 'title' for the labelColumn
• Bad caching in ContributionRecur::buildOptions causes incorrect output
• Unit test was locking in the incorrect behavior

After

Name and title are treated correctly, test updated.

Comments

This solves the blatant bugs in #13698 however I think we should discuss with @mattwire and @eileenmcnaughton the bigger picture here. I'm concerned that the design of #13698 seems like an incomplete workaround to a deeper problem of how to tell the difference between live & test processors. Appending _test when fetching options seems incomplete because:

1. There is more than one entity in the schema with a payment_processor_id field and Add pseudoconstant support for payment_processor_id on ContributionRecur #13698 only applies to one of them.
2. In general we are trying to get away from duplicating FKs and Pseudoconstants on the same field, preferring just one or the other. If the PR to add the pseudoconstant was put up today I wouldn't have merged it on those grounds.
3. It seems to me that a more stable solution would involve adding a unique index on the name column of the PaymentProcessor entity, and forcing unique names by appending _test to test processors when they are created.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

Test failure looks unrelated. Retest this please.

[totten]

Chatted a bit with @eileenmcnaughton and we both agreed the change to TokenConsistencyTest.php was appropriate (ie the old test value was a reflection of buggy behavior). All the other changes also look individually reasonable.

Given that Coleman+Eileen are both +1 for this on the RC, and that this is wrapped-up with the regression-fix for https://lab.civicrm.org/dev/core/-/issues/3063 -- and that we want to maximize exposure during RC period -- I'm inclined to say merge on pass.

... I think we should discuss with @mattwire and @eileenmcnaughton the bigger picture here. I'm concerned that the design of #13698 seems like an incomplete workaround to a deeper problem...

Is there any reason that discussion should be regarded as a blocker for this PR? Would it work just as well as a separate Gitlab issue?

[colemanw]

@totten no, further discussion is not a blocker to merging this.