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

Reduce use of undeclared properties (php8.x errors) , use trait to track entities #27146

Closed
Closed
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
125 changes: 125 additions & 0 deletions CRM/Core/Form/EntityTrackingTrait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
<?php

/*
+--------------------------------------------------------------------+
| Copyright CiviCRM LLC. All rights reserved. |
| |
| This work is published under the GNU AGPLv3 license with some |
| permitted exceptions and without any warranty. For full license |
| and copyright information, see https://civicrm.org/licensing |
+--------------------------------------------------------------------+
*/

/**
* This trait provides a form of lazy loading for forms.
*
* It is intended to reduce the need for forms to pass values around while avoiding
* constantly reloading them from the database.
*
* @package CRM
* @copyright CiviCRM LLC https://civicrm.org/licensing
*/
trait CRM_Core_Form_EntityTrackingTrait {

/**
* Array of loaded entities.
*
* These are stored with keys that are consistent with apiv4 style parameters.
*
* @var array
*/
private $entities = [];

/**
* Set the entity to the specified values.
*
* @param string $entity
* @param int $identifier
* @param array $values
*/
public function setEntity(string $entity, int $identifier, array $values): void {
$this->entities[$entity][$identifier] = $values;
}

/**
* Get the value for a property of an entity, loading from the database if needed.
*
* Permissions are not applied to the api call.
*
* @param string $entity
* @param int $id
* @param string $key
*
* @api supported for use outside of core. Will not change in a point release.
*
* @return mixed
*/
public function getEntityValue(string $entity, int $id, string $key) {
if (!isset($this->entities[$entity][$id]) || !array_key_exists($key, $this->entities[$entity][$id])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

so I suppose $identifier in L34 may not be an actual int but here we are assuming it will be an int right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm yeah - I think I go back & forth on whether we should only support ids or whether we should support string indentifiers too...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok - all ints now

Copy link
Member

Choose a reason for hiding this comment

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

Actually I think it's quite useful to be able to look up stuff by name as well as id.

$this->loadEntity($entity, $id);
}
return $this->entities[$entity][$id][$key];
}

/**
* Get a value from a participant record.
*
* This function requires that the form implements `getParticipantID()`.
*
* @param string $fieldName
* @param int|null $id If not provided getParticipantID() is called.
*
* @api supported for use outside of core. Will not change in a point release.
*
* @return mixed
*/
public function getParticipantValue(string $fieldName, ?int $id = NULL) {
$id = $id ?? $this->getParticipantID();
return $this->getEntityValue('Participant', $id, $fieldName);
Copy link
Contributor

Choose a reason for hiding this comment

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

@eileenmcnaughton this doesn't seem to be exactly the same as the original as the original could find the value in participant_$fieldname if it wasn't present in $fieldName` right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 true - but the actual usage of this function means that isn't an issue -

image

}

/**
* Get a value from a contact record.
*
* This function requires that the form implements `getContactID()`.
*
* @param string $fieldName
* @param int|null $id If not provided getContactID() is called.
*
* @api supported for use outside of core. Will not change in a point release.
*
* @return mixed
*/
public function getContactValue(string $fieldName, ?int $id = NULL) {
$id = $id ?? $this->getContactID();
return $this->getEntityValue('Contact', $id, $fieldName);
}

/**
* Load the requested entity.
*
* If we are going to load an entity we generally load all the values for it.
*
* @param string $entity
* @param int $id
*
* @noinspection PhpUnhandledExceptionInspection
* @noinspection PhpDocMissingThrowsInspection
*/
protected function loadEntity(string $entity, int $id): void {
if ($entity === 'Contact') {
// If we are loading a contact we generally also want their email.
$select = ['email_primary.email', 'email_primary.on_hold', '*', 'custom.*'];
}
else {
$select = ['*', 'custom.*'];
Copy link
Member

Choose a reason for hiding this comment

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

I'm 👎🏻 on loading custom data by default. This class is a lazy loader so could fetch it if you request the value of a custom field but I think loading the "kitchen sink" with potentially dozens of joins on custom tables is a waste of cpu & memory(and invites getting bitten by that max join limit in SQL).

}
$this->entities[$entity][$id] = civicrm_api4($entity, 'get', [
'where' => [['id', '=', $id]],
'checkPermissions' => FALSE,
// @todo - load pseudoconstants too...
'select' => $select,
])->first();
}

}
24 changes: 11 additions & 13 deletions CRM/Event/Form/Participant.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
*/
class CRM_Event_Form_Participant extends CRM_Contribute_Form_AbstractEditPayment {

use CRM_Core_Form_EntityTrackingTrait;

/**
* Participant ID - use getParticipantID.
*
Expand Down Expand Up @@ -1382,19 +1384,19 @@ public function setCustomDataTypes(): void {
protected function getStatusMsg(array $params, int $numberSent, int $numberNotSent, string $updateStatusMsg): string {
$statusMsg = '';
if (($this->_action & CRM_Core_Action::UPDATE)) {
$statusMsg = ts('Event registration information for %1 has been updated.', [1 => $this->_contributorDisplayName]);
$statusMsg = ts('Event registration information for %1 has been updated.', [1 => $this->getContactValue('display_name')]);
if (!empty($params['send_receipt']) && $numberSent) {
$statusMsg .= ' ' . ts('A confirmation email has been sent to %1', [1 => $this->_contributorEmail]);
$statusMsg .= ' ' . ts('A confirmation email has been sent to %1', [1 => $this->getContactValue('email_primary.email')]);
}

if ($updateStatusMsg) {
$statusMsg = "{$statusMsg} {$updateStatusMsg}";
}
}
elseif ($this->_action & CRM_Core_Action::ADD) {
$statusMsg = ts('Event registration for %1 has been added.', [1 => $this->_contributorDisplayName]);
$statusMsg = ts('Event registration for %1 has been added.', [1 => $this->getContactValue('display_name')]);
if (!empty($params['send_receipt']) && $numberSent) {
$statusMsg .= ' ' . ts('A confirmation email has been sent to %1.', [1 => $this->_contributorEmail]);
$statusMsg .= ' ' . ts('A confirmation email has been sent to %1.', [1 => $this->getContactValue('email_primary.email')]);
}
}
return $statusMsg;
Expand Down Expand Up @@ -1933,18 +1935,14 @@ protected function getEventValue(string $fieldName) {
}

/**
* Get a value from the existing participant record (applies to edits).
* Get the contact id that the form is being submitted for.
*
* @param string $fieldName
* @todo consolidate Contact ID properties....
*
* @return array
* @throws \CRM_Core_Exception
* @return int
*/
protected function getParticipantValue($fieldName) {
if (!$this->participantRecord) {
$this->participantRecord = civicrm_api3('Participant', 'getsingle', ['id' => $this->_id]);
}
return $this->participantRecord[$fieldName] ?? $this->participantRecord['participant_' . $fieldName];
public function getContactID() {
return $this->_contactId;
}

/**
Expand Down