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#2834 Use event tokens for participant badges #21587

Merged
merged 1 commit into from
Sep 28, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

Convert the rest of event badge token replacement to use
the token processor

Before

Testing and conversion had been done for contact
and participant tokens within the event badges - but
for the event tokens tests had been added but
the conversion was pending solving date formatting

After

Event badges use the token processor for event tokens

At this point all token processing in core for
participant or event tokens is done in the token
processor & attention can switch to cleaning that up

Technical Details

The challenge switching over event tokens was that badges used
a custom date format and we needed to figure out how to support
that first.

This also standardises the event.event_id token to participant.event_id
and updates it in the db for scheduled reminders (badges were using
'event.id' format & reminders the event.event_id format).

There are still some tokens on the scheduled reminders to
standarsise and a loading challenge to fix - but this
concludes the badge portion of fixing up event tokens

Note that the tokenConsistencyTest is the main test cover
for event tokens and it turns out we are not quite there
on the dates based on #21584

@civibot
Copy link

civibot bot commented Sep 23, 2021

(Standard links)

@civibot civibot bot added the master label Sep 23, 2021
@eileenmcnaughton
Copy link
Contributor Author

@agileware-justin ping - this is part of getting participant & event tokens working properly

@agileware-justin
Copy link
Contributor

@eileenmcnaughton thanks for the ping, I tried to test this using CiviCRM 5.43.alpha1 from nightly, https://download.civicrm.org/latest/civicrm-NIGHTLY-wordpress.zip

But that build is failing to save any changes, Contacts and Scheduled Reminders fail to save. The patch applies, but I'm not sure if that's the right version.

It looks like this PR is just for Event badges? Does it apply to Scheduled Reminders as well, as that's the functionality I am trying to test.

@eileenmcnaughton
Copy link
Contributor Author

@agileware-justin no - it doesn't touch scheduled reminders at this stage - it is preliminary work - the job of fixing the tokens involves ensuring tha they are consistently rendered and tested to be consistently rendered for everywhere in the code. So this is part of that foundation-laying.

@agileware-justin
Copy link
Contributor

Ahh right, false ping? Maybe let me and other interested ppl know on this Gitlab, https://lab.civicrm.org/dev/core/-/issues/348

@eileenmcnaughton
Copy link
Contributor Author

well the standardisation & tests are 'the work' - but yeah - you might be best to test the rc once it's merged / stable rather than engage in all the work that gets it to that point

@magnolia61
Copy link
Contributor

magnolia61 commented Sep 25, 2021

Thanks @eileenmcnaughton for all the hard work in the token area in the last weeks. I have been following the process and your commits and am glad this will all work towards a more robust and reliable token processing.

About this PR: I have tested name badge creation with and without this patch.
I experienced no difference in behavior, so that means all is working well with the token processor.
One observation though: I was not able to select custom event fields (with and without this patch)
On the participant side both core participant fields as custom fields are selectable.

Update on date fields:
register date showed up as: 999901202101132034 (also after applying #21584)
A custom participant date field rendered as expected: January 13th

@eileenmcnaughton
Copy link
Contributor Author

@magnolia61 thanks for testing!

Regarding the event custom fields - with this patch I think they would work - but not be selectable since the event tokens on this page are hard coded still... - it wouldn't be too big a lift if there is demand for it but it add a lot more fields (until we resolve a more filterable list)

Note I've added quite a long write up to the dev-digest about tokens & also created a meta-ticket https://lab.civicrm.org/dev/core/-/issues/2864

@eileenmcnaughton eileenmcnaughton force-pushed the badge_ev branch 4 times, most recently from d6b5eaf to 47cbd81 Compare September 28, 2021 05:41
@eileenmcnaughton
Copy link
Contributor Author

@magnolia61 can you test again now - I just tried & got this

image

{"entryURL":"https:\/\/dmaster.localhost:32353\/civicrm\/admin\/badgelayout?action=update&id=1&reset=1","title":"Annual Conference Hanging Badge (Avery 5395)","label_format_name":"Avery 5395","description":"For our annual conference","token":{"1":"{event.title}","2":"{contact.display_name}","3":"{contact.current_employer}","4":"{event.start_date}","5":"{participant.register_date}","6":""},"font_name":{"1":"dejavusans","2":"dejavusans","3":"dejavusans","4":"dejavusans","5":"dejavusans","6":"dejavusans"},"font_size":{"1":"9","2":"20","3":"15","4":"9","5":"6","6":"6"},"font_style":{"1":"","2":"","3":"","4":"","5":"","6":""},"text_alignment":{"1":"L","2":"C","3":"C","4":"R","5":"R","6":"R"},"barcode_type":"barcode","barcode_alignment":"R","image_1":"","width_image_1":"","height_image_1":"","image_2":"","width_image_2":"","height_image_2":"","width_participant_image":"","height_participant_image":"","alignment_participant_image":"R","is_default":"1","is_active":"1","is_reserved":"1","_qf_default":"Layout:next","_qf_Layout_next":"1"}

@eileenmcnaughton eileenmcnaughton force-pushed the badge_ev branch 4 times, most recently from ff07249 to 87906a8 Compare September 28, 2021 06:09
@eileenmcnaughton
Copy link
Contributor Author

Here is with date formatting in the install

image

@magnolia61
Copy link
Contributor

Just tested the updated pr again and date handling is perfect now and event core tokens, participant core and custom tokens work as expected in the badge creation.

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 ^^ ok to merge?

Thanks @magnolia61 !

@eileenmcnaughton
Copy link
Contributor Author

@magnolia61 I made a couple of edits to get this to pass. Can you do one final test - post merge probably since I'll still merge this if tests pass.

Overview
----------------------------------------
Convert the rest of event badge token replacement to use
the token processor

Before
----------------------------------------
Testing and conversion had been done for contact
and participant tokens within the event badges - but
for the event tokens tests had been added but
the conversion was pending solving date formatting

After
----------------------------------------
Event badges use the token processor for event tokens

At this point all token processing in core for
participant or event tokens is done in the token
processor & attention can switch to cleaning that up

Technical Details
----------------------------------------
The challenge switching over event tokens was that badges used
a custom date format and we needed to figure out how to support
that first.

This also standardises the event.event_id token to participant.event_id
and updates it in the db for scheduled reminders (badges were using
'event.id' format & reminders the event.event_id format).

There are still some tokens on the scheduled reminders to
standarsise and a loading challenge to fix - but this
concludes the badge portion of fixing up event tokens

Note that the tokenConsistencyTest is the main test cover
for event tokens and it turns out we are not quite there
on the dates based on civicrm#21584
@magnolia61
Copy link
Contributor

magnolia61 commented Sep 28, 2021

Tested the updated PR: same result. All works fine and as expected!

I hope this groundwork will help opening up the same for the scheduled reminders in some next step.

Thanks again for your passion for robust and reliable quality and looking at what is good for civicrm longterm.

@eileenmcnaughton
Copy link
Contributor Author

@magnolia61 yes - so I am close to having a working patch locally for you to test on that!

@eileenmcnaughton eileenmcnaughton merged commit 1c34065 into civicrm:master Sep 28, 2021
@eileenmcnaughton eileenmcnaughton deleted the badge_ev branch September 28, 2021 22:45
'updateMessageToken', '', 'contribution.campaign', 'contribution.campaign_id:label', $rev
'updateMessageToken', '', 'contribution.campaign', 'contribution.campaign_id:label', $rev);
$this->addTask('Update start date token in event badges',
'updatePrintLabelToken', 'event.start_date', 'event.start_date|crmDate:"%B %E%f', $rev
Copy link
Contributor

Choose a reason for hiding this comment

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

Here's the typo (note the missing quotation mark after %f) that I'm referencing in my note on core#3952.

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.

5 participants