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#2806 Fix accidental exposure of v4 tokens #21337

Merged
merged 1 commit into from
Sep 1, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

dev/core#2806 Fix accidental exposure of v4 tokens

Before

In master (only) tokens were being displayed like '{contribution.Partner.name'} referring to custom fields. Long term we want this more portable format but we are not ready to define it yet

After

Extraneous tokens removed

Technical Details

@demeritcowboy I haven't added a test here because I'm working on tests for this stuff in CRM_Utils_TokenConsistencyTest but I already have an open test against this class - https://github.com/civicrm/civicrm-core/pull/21327/files - unfortunately that one didn't pick it up because contribution recur doesn't 'normally' have custom data - but I do intend to add more tests for this stuff in that class . The class I did the original contribution tokens test in unfortunately uses rollback which doesn't play nice with custom fields so I 'moved'

Comments

@civibot
Copy link

civibot bot commented Sep 1, 2021

(Standard links)

@civibot civibot bot added the master label Sep 1, 2021
@demeritcowboy
Copy link
Contributor

Thanks this seems to run ok. I don't know enough to comment on the r-tech.

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Sep 1, 2021
@seamuslee001
Copy link
Contributor

Merging as per @demeritcowboy 's review

@seamuslee001 seamuslee001 merged commit 12a29dd into civicrm:master Sep 1, 2021
@seamuslee001 seamuslee001 deleted the tokens branch September 1, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants