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

dev/core#2650 Add support for names & labels for token pseudoconstants #20961

Merged
merged 2 commits into from
Jul 30, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 27, 2021

Overview

This is per https://lab.civicrm.org/dev/core/-/issues/2650 - it adds variations on the token {contribution.contribution_status_id} which support names and labels, i.e.

{contribution.contribution_status_id}
{contribution.contribution_status_id:name}
{contribution.contribution_status_id:label}

Both CRM_Core_SelecetValues::contributionTokens and CRM_Contribute_Tokens->tokenNames
are tested to ensure they return the same keys and labels for each.

And both rendering CRM_Contribute_Tokens via scheduled reminders and
using CRM_Contribute_BAO_Contribution::replaceContributionTokens are tested to ensure
they render them the same.

Before

The contribution token processor and legacy contributions use different syntaxes around tokens with potential labels or names and there is no clear plan as to what to standardise on. Also, some of the message templates work pretty hard to get around NOT having {contribution.contribution_status_id:name} available

After

{contribution.contribution_status_id}
{contribution.contribution_status_id:name}
{contribution.contribution_status_id:label}

available & consistent in all 4 code places that refer to contribution tokens at the moment

image

Technical Details

In the context of this PR no existing tokens are altered or removed & there is
only addition. However, the next step would be to remove the following token
from {contribution.status}. Since there is no UI availability
of this token it is likely unused - but that step would entail an upgrade
script to remove it from the saved scheduled reminders.

With those parts in place it should be possible to reconcile the remaining tokens,
lock that parity in with tests and move on to exposing the contribution tokens
to message templates.

It would be nice to fully remove CRM_Contribute_BAO_Contribution::replaceContributionTokens
or make it a wrapper for - however, I fear that might be quite
challenging due to the way it's used with group bys & some pretty intense hackery.

Comments

@civibot
Copy link

civibot bot commented Jul 27, 2021

(Standard links)

@civibot civibot bot added the master label Jul 27, 2021
@eileenmcnaughton eileenmcnaughton force-pushed the status_name branch 3 times, most recently from 29ebaf4 to 046a056 Compare July 27, 2021 07:50
@eileenmcnaughton eileenmcnaughton force-pushed the status_name branch 2 times, most recently from 3706e67 to 313229e Compare July 27, 2021 10:07
@eileenmcnaughton
Copy link
Contributor Author

dang - that test passes locally & I think it might fail when not in isolation - maybe caching ...

@eileenmcnaughton
Copy link
Contributor Author

Dang that passes locally - I wish there were a way to run unit tests against just one test to see if it passes remotely in isolation

image

This is per https://lab.civicrm.org/dev/core/-/issues/2650 -
it
- ensures that the 4 existing functions that deal with tokens handle tokens for the
name and label for the (only) field that has had this treatment so far -contribution_status_id

Hence both CRM_Core_SelecetValues::contributionTokens and CRM_Contribute_Tokens->tokenNames
are tested to ensure they both return the same keys and labels for
{contribution.contribution_status_id}
{contribution.contribution_status_id:name}
{contribution.contribution_status_id:label}

And both rendering CRM_Contribute_Tokens via scheduled reminders and
using CRM_Contribute_BAO_Contribution::replaceContributionTokens are tested to ensure
they render them the same.

In the context of this PR no existing tokens are altered or removed & there is
only addition. However, the next step would be to remove the following token
from  {contribution.status}. Since there is no UI availability
of this token it is likely unused - but that step would entail an upgrade
script to remove it from the saved scheduled reminders.

With those parts in place it should be possible to reconcile the remaining tokens,
lock that parity in with tests and move on to exposing the contribution tokens
to message templates.

It would be nice to fully remove CRM_Contribute_BAO_Contribution::replaceContributionTokens
or make it a wrapper for  - however, I fear that might be quite
challenging due to the way it's used with group bys & some pretty intense hackery.
@totten
Copy link
Member

totten commented Jul 30, 2021

  • General standards
    • (r-explain) Pass
    • (r-user) Pass:
    • (r-doc) Pass
    • (r-run) Pass: Used the tokens with both a scheduled reminder and a thank-you PDF. Worked well.
      • (Tangentially, the {contribution.id} and {contribution.contribution_id} )
  • Developer standards

@totten totten merged commit 111222d into civicrm:master Jul 30, 2021
@eileenmcnaughton eileenmcnaughton deleted the status_name branch July 30, 2021 06:35
@eileenmcnaughton
Copy link
Contributor Author

I've put up this to remove the dead token from action_schedule while we still know it's only available from there #20978

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.

2 participants