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#2745 - Contribution Tokens - Support 'contributionId' #21134

Merged
merged 2 commits into from
Aug 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 2 additions & 60 deletions CRM/Contribute/Tokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,6 @@
*/
class CRM_Contribute_Tokens extends CRM_Core_EntityTokens {

/**
* @return string
*/
protected function getEntityName(): string {
return 'contribution';
}

/**
* @return string
*/
Expand All @@ -47,61 +40,10 @@ protected function getApiEntityName(): string {
}

/**
* Get a list of tokens for the entity for which access is permitted to.
*
* This list is historical and we need to question whether we
* should filter out any fields (other than those fields, like api_key
* on the contact entity) with permissions defined.
*
* @return array
*/
protected function getExposedFields(): array {
$fields = [
'contribution_page_id',
'source',
'id',
'receive_date',
'total_amount',
'fee_amount',
'net_amount',
'non_deductible_amount',
'trxn_id',
'invoice_id',
'currency',
'cancel_date',
'receipt_date',
'thankyou_date',
'tax_amount',
'contribution_status_id',
'financial_type_id',
'payment_instrument_id',
'cancel_reason',
'amount_level',
'check_number',
];
if (CRM_Campaign_BAO_Campaign::isCampaignEnable()) {
$fields[] = 'campaign_id';
}
return $fields;
}

/**
* Get tokens supporting the syntax we are migrating to.
*
* In general these are tokens that were not previously supported
* so we can add them in the preferred way or that we have
* undertaken some, as yet to be written, db update.
*
* See https://lab.civicrm.org/dev/core/-/issues/2650
*
* @return string[]
*/
public function getBasicTokens(): array {
$return = [];
foreach ($this->getExposedFields() as $fieldName) {
$return[$fieldName] = $this->getFieldMetadata()[$fieldName]['title'];
}
return $return;
public function getCurrencyFieldName() {
return ['currency'];
}

}
126 changes: 122 additions & 4 deletions CRM/Core/EntityTokens.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,17 @@
*/
class CRM_Core_EntityTokens extends AbstractTokenSubscriber {

/**
* @var array
*/
protected $prefetch = [];

/**
* @inheritDoc
* @throws \CRM_Core_Exception
*/
public function evaluateToken(TokenRow $row, $entity, $field, $prefetch = NULL) {
$this->prefetch = (array) $prefetch;
$fieldValue = $this->getFieldValue($row, $field);

if ($this->isPseudoField($field)) {
Expand All @@ -40,7 +46,7 @@ public function evaluateToken(TokenRow $row, $entity, $field, $prefetch = NULL)
}
if ($this->isMoneyField($field)) {
return $row->format('text/plain')->tokens($entity, $field,
\CRM_Utils_Money::format($fieldValue, $this->getFieldValue($row, 'currency')));
\CRM_Utils_Money::format($fieldValue, $this->getCurrency($row)));
}
if ($this->isDateField($field)) {
return $row->format('text/plain')->tokens($entity, $field, \CRM_Utils_Date::customFormat($fieldValue));
Expand Down Expand Up @@ -214,6 +220,15 @@ public function isAddPseudoTokens($fieldName): bool {
// from the metadata as yet.
return FALSE;
}
if ($this->getFieldMetadata()[$fieldName]['type'] === 'Custom') {
// If we remove this early return then we get that extra nuanced goodness
// and support for the more portable v4 style field names
// on custom fields - where labels or names can be returned.
// At present the gap is that the metadata for the label is not accessed
// and tests failed on the enotice and we don't have a clear plan about
// v4 style custom tokens - but medium term this IF will probably go.
return FALSE;
}
return (bool) $this->getFieldMetadata()[$fieldName]['options'];
}

Expand Down Expand Up @@ -247,7 +262,11 @@ public function getPseudoValue(string $realField, string $pseudoKey, $fieldValue
protected function getFieldValue(TokenRow $row, string $field) {
$actionSearchResult = $row->context['actionSearchResult'];
$aliasedField = $this->getEntityAlias() . $field;
return $actionSearchResult->{$aliasedField} ?? NULL;
if (isset($actionSearchResult->{$aliasedField})) {
return $actionSearchResult->{$aliasedField};
}
$entityID = $row->context[$this->getEntityIDField()];
return $this->prefetch[$entityID][$field] ?? '';
}

/**
Expand All @@ -266,8 +285,10 @@ public function __construct() {
* @return bool
*/
public function checkActive(TokenProcessor $processor) {
return !empty($processor->context['actionMapping'])
&& $processor->context['actionMapping']->getEntity() === $this->getExtendableTableName();
return (!empty($processor->context['actionMapping'])
// This makes the 'schema context compulsory - which feels accidental
// since recent discu
&& $processor->context['actionMapping']->getEntity()) || in_array($this->getEntityIDField(), $processor->context['schema']);
}

/**
Expand All @@ -284,4 +305,101 @@ public function alterActionScheduleQuery(MailingQueryEvent $e): void {
}
}

/**
* Get tokens supporting the syntax we are migrating to.
*
* In general these are tokens that were not previously supported
* so we can add them in the preferred way or that we have
* undertaken some, as yet to be written, db update.
*
* See https://lab.civicrm.org/dev/core/-/issues/2650
*
* @return string[]
* @throws \API_Exception
*/
public function getBasicTokens(): array {
$return = [];
foreach ($this->getExposedFields() as $fieldName) {
$return[$fieldName] = $this->getFieldMetadata()[$fieldName]['title'];
}
return $return;
}

/**
* Get entity fields that should be exposed as tokens.
*
* @return string[]
*
*/
public function getExposedFields(): array {
$return = [];
foreach ($this->getFieldMetadata() as $field) {
if (!in_array($field['name'], $this->getSkippedFields(), TRUE)) {
$return[] = $field['name'];
}
}
return $return;
}

/**
* Get entity fields that should not be exposed as tokens.
*
* @return string[]
*/
public function getSkippedFields(): array {
$fields = ['contact_id'];
if (!CRM_Campaign_BAO_Campaign::isCampaignEnable()) {
$fields[] = 'campaign_id';
}
return $fields;
}

/**
* @return string
*/
protected function getEntityName(): string {
return CRM_Core_DAO_AllCoreTables::convertEntityNameToLower($this->getApiEntityName());
}

public function getEntityIDField() {
return $this->getEntityName() . 'Id';
}

public function prefetch(\Civi\Token\Event\TokenValueEvent $e): ?array {
$entityIDs = $e->getTokenProcessor()->getContextValues($this->getEntityIDField());
if (empty($entityIDs)) {
return [];
}
$select = $this->getPrefetchFields($e);
$result = (array) civicrm_api4($this->getApiEntityName(), 'get', [
'checkPermissions' => FALSE,
// Note custom fields are not yet added - I need to
// re-do the unit tests to support custom fields first.
'select' => $select,
'where' => [['id', 'IN', $entityIDs]],
], 'id');
return $result;
}

public function getCurrencyFieldName() {
Copy link
Member

Choose a reason for hiding this comment

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

This asserts a strong opinion that currency can be fetched as a single DB field. While the civicrm_contribution record meets that expectation, I don't see why it's a good thing to bake that opinion into the base-class of token source.

return [];
}

/**
* Get the currency to use for formatting money.
* @param $row
*
* @return string
*/
public function getCurrency($row): string {
Copy link
Member

Choose a reason for hiding this comment

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

This asserts a strong opinion that currency is one-per-row. While the civicrm_contribution record meets that expectation, I don't see why it's a good thing to bake that opinion into the base-class of token source.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah - it's also overrideable by class. I can't offhand think of an exception to there being zero or one per row as this expects - but if there were more than one it could be added

if (!empty($this->getCurrencyFieldName())) {
return $this->getFieldValue($row, $this->getCurrencyFieldName()[0]);
}
return CRM_Core_Config::singleton()->defaultCurrency;
}

public function getPrefetchFields(\Civi\Token\Event\TokenValueEvent $e): array {
return array_intersect($this->getActiveTokens($e), $this->getCurrencyFieldName(), array_keys($this->getAllTokens()));
}

}
22 changes: 21 additions & 1 deletion tests/phpunit/CRM/Contribute/ActionMapping/ByTypeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/

use Civi\Api4\Contribution;
use Civi\Token\TokenProcessor;

/**
* Class CRM_Contribute_ActionMapping_ByTypeTest
Expand Down Expand Up @@ -255,6 +256,7 @@ public function useHelloFirstNameStatus(): void {
* legacy processor function. Once this is true we can expose the listener on the
* token processor for contribution and call it internally from the legacy code.
*
* @throws \API_Exception
* @throws \CiviCRM_API3_Exception
*/
public function testTokenRendering(): void {
Expand Down Expand Up @@ -319,6 +321,22 @@ public function testTokenRendering(): void {
];
$this->mut->checkMailLog($expected);

$tokenProcessor = new TokenProcessor(\Civi::dispatcher(), [
'controller' => get_class(),
'smarty' => FALSE,
'schema' => ['contributionId'],
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 'schema' is required if I copy the mechanism from #21144 - but that feels like it should be unnecessary

Copy link
Member

Choose a reason for hiding this comment

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

Off the cuff, I'm OK with that because (one way or another) the signal has to be provided in the listTokens use-case, and it's good discipline to have an upfront idea of what data is going in.

OTOH, yes, it's sort of redundant for this use-case. There's enough information in the row-data to infer that contributionId is present. I suppose an opportune spot to leverage that might be at the start of TokenProcessor->evaluate() -- recompute schema to include array_unique(array_merge(array_keys(...each row...)). And the recently added bit in ActionSchedule::sendMailings() (vis-a-vis schema) would be -3 SLOC if the autodetection were better.

'contributionId' => $this->ids['Contribution']['alice'],
'contactId' => $this->contacts['alice']['id'],
]);
$tokenProcessor->addRow([]);
$tokenProcessor->addMessage('html', $this->schedule->body_text, 'text/plain');
$tokenProcessor->evaluate();
foreach ($tokenProcessor->getRows() as $row) {
foreach ($expected as $value) {
$this->assertStringContainsString($value, $row->render('html'));
}
}

$messageToken = CRM_Utils_Token::getTokens($this->schedule->body_text);

$contributionDetails = CRM_Contribute_BAO_Contribution::replaceContributionTokens(
Expand Down Expand Up @@ -381,7 +399,9 @@ public function testTokenRendering(): void {
foreach ($fields as $field) {
$allFields[$field['name']] = $field['title'];
}
// $this->assertEquals($realLegacyTokens, $allFields);
// contact ID is skipped.
unset($allFields['contact_id']);
$this->assertEquals($allFields, $realLegacyTokens);
$this->assertEquals($legacyTokens, $processor->tokenNames);
foreach ($tokens as $token) {
$this->assertEquals(CRM_Core_SelectValues::contributionTokens()['{contribution.' . $token . '}'], $processor->tokenNames[$token]);
Expand Down
3 changes: 2 additions & 1 deletion tests/phpunit/CRM/Contribute/BAO/ContributionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,8 @@ public function testProcessOnBehalfOrganization() {

/**
* Test for replaceContributionTokens.
* This function tests whether the contribution tokens are replaced with
*
* This function tests whether the contribution tokens are replaced with
* values from contribution.
*
* @throws \CiviCRM_API3_Exception
Expand Down