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

Towards full metadata - fully declare 'bespoke' tokens #21713

Merged
merged 1 commit into from
Oct 5, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Oct 3, 2021

Overview

Towards full metadata - fully declare 'bespoke' tokens

The goal is to declare all tokens with full metadata and cache that - rather than the convoluted ways we have of looking up metadata - I'm working towards #21706 but working through roadblocks

Before

'bespokeTokens' (only in master) declared as a flat array

After

declared as a nuanced array

Technical Details

This functionality all has really good test cover. One thing it does change is that membership & event tokens start to advertise the
machine name tokens - the intent is to get to the point where ALL fields are declared as token metadata (which will be cached) but only those with 'audience' = 'user' will be in the widget - see #21706

Comments

@demeritcowboy @colemanw if you have a better function name that 'getBespokeFunctions` please speak now

variants on custom are too close to custom fields & calculated is not necessarily accurate.

@civibot
Copy link

civibot bot commented Oct 3, 2021

(Standard links)

@civibot civibot bot added the master label Oct 3, 2021
@eileenmcnaughton eileenmcnaughton force-pushed the meta_lite branch 3 times, most recently from 0342988 to b4c3292 Compare October 3, 2021 21:31
@colemanw
Copy link
Member

colemanw commented Oct 4, 2021

I wonder about calculating these fields at the APIv4 level rather than in the token layer. For example, #21718 demonstrates how easy it is to add a calculated field to APIv4 which acts very much like a "real" field (you can use it in any clause - select, where, having, order by).
We could do something similar for things like Participant.balance (or amount_owed or whatever)

@eileenmcnaughton
Copy link
Contributor Author

@colemanw yeah - but not this round

@eileenmcnaughton
Copy link
Contributor Author

eileenmcnaughton commented Oct 4, 2021

@colemanw longer answer - I have some real doubts about the validity of that participant balance token - but what would be REALLY useful is a similar token on the contribution side for balance - I had thought to add that if I can get far enough before the rc is cut. But for now I'm treating those tokens as ones we are stuck with

protected function getBespokeTokens(): array {
return [
'fee' => [
'title' => ts('Membership Fee'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we get to the point where the number formatting is settled I would deprecated this in favour of {membership.membership_type_id.minimum_fee}

if (array_key_exists('CiviCase', CRM_Core_Component::getEnabledComponents())) {
$tokens['case_id'] = ts('Activity Case ID');
return [
'case_id' => [
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This token exists because it was added in the past but there are 1-1 entity validilty issues

Copy link
Member

Choose a reason for hiding this comment

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

Right, at best it could get you a case id (but no guarantee which one)

@colemanw
Copy link
Member

colemanw commented Oct 5, 2021

Ok let's merge this since it's

  1. Tested
  2. Not hindering a full APIv4 approach
  3. Sounds like the right direction

@colemanw colemanw merged commit 19db696 into civicrm:master Oct 5, 2021
@colemanw colemanw deleted the meta_lite branch October 5, 2021 01:25
@eileenmcnaughton
Copy link
Contributor Author

thanks @colemanw

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