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

Participant tokens - remove / replace unsupportable from badges (conversion preparation) #21520

Merged
merged 3 commits into from
Sep 20, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 17, 2021

Overview

Participant tokens - remove / replace unsupportable from badges (conversion preparation)

Before

The drop down for event tokens offers 2 tokens for event title, start date, end date. One is 'standard' - ie {event.title} the other isn't ie {participant.event_title}

Also 2 tokens are offered that are hard to support and don't make sense in this context

{participant.currency} (does not return anything - the field is 'fee_currency' and doesn't make much sense in this context)
{participant.note} (this does work but I think we can drop support on this one as it is ambiguous if there are multiple notes, it's really hard to support, was exposed by accident and I think it's unlikely it is used).

After

Ambiguous tokens consolidated. Support dropped for the 2 problem tokens.

Technical Details

@seamuslee001 If we merge this then next steps are

  1. add a participant token processor & switch this code to use it (date fields are the only issue I can see)
  2. update pdf & email classes to use these (once cleanup is finished on those classes)

Comments

I added to the upgrade notes
https://lab.civicrm.org/-/ide/project/documentation/docs/sysadmin/tree/case/-/docs/upgrade/version-specific.md/

@civibot
Copy link

civibot bot commented Sep 17, 2021

(Standard links)

@civibot civibot bot added the master label Sep 17, 2021
@eileenmcnaughton eileenmcnaughton force-pushed the badger branch 2 times, most recently from a5289dc to 057e8e6 Compare September 17, 2021 22:45
There are literally 2 tokens for the same option in the drop down.. remove the non-standard one
These tokens are
a) non-standard, not easily supported
b) illogical in the only current core usage context
that currently uses these tokens (event badges)
c) partially broken - currency returns nothing, note doesn't make sense
@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Sep 18, 2021

It turns out 2 more tokens will need to go to be supportable. They are in #21530 - I could add them to this but I'm kinda hoping this will be merged & I can simplify that

    '{participant.default_role_id}' => 'Default Role',
      '{participant.template_title}' => 'Event Template Title',

https://github.com/civicrm/civicrm-core/pull/21530/files#diff-d8f3ea846fa5593a3eec2f1e0c755a5bdade3580b5db78ff81a779340326a6e9L602-L603

I've added them also to https://lab.civicrm.org/-/ide/project/documentation/docs/sysadmin/tree/case/-/docs/upgrade/version-specific.md/

'{participant.fee_label}' => 'Fee Label',
'{participant.default_role_id}' => 'Default Role',
'{participant.template_title}' => 'Event Template Title',
'{participant.currency}' => 'Currency',
'{participant.participant_note}' => 'Participant Note',
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton is there a replacement suggested for these 2?

Copy link
Contributor Author

@eileenmcnaughton eileenmcnaughton Sep 20, 2021

Choose a reason for hiding this comment

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

@seamuslee001 so per the pr block & docs update - these are only currently available in badges & currency doesn't work so that's easy.

For note - I'm proposing we remove & rely on the docs update (linked above) - since I think it's
a) unlikely to be in use for badges & that is the only usage of this function in core now (fortunately)
b) not on any other token class
c) not a one to one relationship - at the db level anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

ok I guess that is enough then

@seamuslee001
Copy link
Contributor

This seems fine to me merging

@seamuslee001 seamuslee001 merged commit 62d3fbb into civicrm:master Sep 20, 2021
@seamuslee001 seamuslee001 deleted the badger branch September 20, 2021 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants