Autofills Source for monetary contribution#480
Conversation
WalkthroughThe changes introduce a new method, Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (2)
114-123: Improve error logging and use CiviCRM's logging system.Replace direct
error_logcalls with CiviCRM's logging system for better integration and consistency.- error_log("autoFillData: " . print_r($autoFillData, TRUE)); + \Civi::log()->debug('Autofilling monetary form fields', [ + 'form' => $formName, + 'data' => $autoFillData + ]); foreach ($autoFillData as $fieldName => $value) { if (isset($form->_elements) && is_array($form->_elements)) { foreach ($form->_elements as $element) { if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { $form->setDefaults([$fieldName => $value]); - error_log("Auto-filled $fieldName with value $value."); + \Civi::log()->debug('Field auto-filled', [ + 'field' => $fieldName, + 'value' => $value + ]);
117-126: Simplify nested conditions for better readability.The deeply nested conditions make the code harder to follow. Consider extracting the element search logic into a separate method.
+ private function findFormElement($form, $fieldName) { + if (!isset($form->_elements) || !is_array($form->_elements)) { + return null; + } + foreach ($form->_elements as $element) { + if (isset($element->_attributes['name']) && + $element->_attributes['name'] === $fieldName) { + return $element; + } + } + return null; + } foreach ($autoFillData as $fieldName => $value) { - if (isset($form->_elements) && is_array($form->_elements)) { - foreach ($form->_elements as $element) { - if (isset($element->_attributes['name']) && - $element->_attributes['name'] === $fieldName) { - $form->setDefaults([$fieldName => $value]); - } - } + if ($element = $this->findFormElement($form, $fieldName)) { + $form->setDefaults([$fieldName => $value]); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(1 hunks)
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
72-75: LGTM: Event subscription added correctly.
The new hook subscription follows the existing pattern and is properly integrated.
| $custom554 = NULL; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a more robust session handling mechanism.
The direct usage of $_SESSION and $_GET could be improved:
- Add session validation before access
- Consider using CiviCRM's built-in session handling
- Add type checking for the retrieved values
- $custom554 = NULL;
-
- // Check if this is the initial form that provides the value.
- if ($formName === 'CRM_Contribute_Form_Contribution' && isset($_GET['custom_554_-1'])) {
- $custom554 = $_GET['custom_554_-1'];
- // Store the value in the session for subsequent use.
- $_SESSION['custom_554'] = $custom554;
+ $sourceFieldValue = NULL;
+ $session = CRM_Core_Session::singleton();
+
+ if ($formName === 'CRM_Contribute_Form_Contribution') {
+ $sourceFieldValue = CRM_Utils_Request::retrieve('custom_554_-1', 'String');
+ if ($sourceFieldValue) {
+ $session->set('monetary_source', $sourceFieldValue);
+ }Also applies to: 94-96
| $autoFillData = [ | ||
| // Autofills the Collection camp and Droppung center. | ||
| 'custom_554_-1' => $custom554, | ||
| // Autofills the Office. | ||
| 'custom_555_-1' => '19', | ||
| ]; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract hardcoded field IDs and values into constants or configuration.
The code contains magic numbers and hardcoded field IDs which make maintenance difficult.
+ private const COLLECTION_CAMP_FIELD = 'custom_554_-1';
+ private const OFFICE_FIELD = 'custom_555_-1';
+ private const DEFAULT_OFFICE_VALUE = '19';
+
$autoFillData = [
- 'custom_554_-1' => $custom554,
- 'custom_555_-1' => '19',
+ self::COLLECTION_CAMP_FIELD => $sourceFieldValue,
+ self::OFFICE_FIELD => self::DEFAULT_OFFICE_VALUE,
];Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (2)
114-114: Remove debug logging statements.Production code should not contain debug logging statements. Consider using proper logging levels or removing these statements.
- error_log("_GET: " . print_r($_GET, TRUE)); - error_log("autoFillData: " . print_r($autoFillData, TRUE));Also applies to: 126-126
129-138: Simplify form element iteration logic.The nested loops and multiple conditions make the code hard to follow. Consider extracting this into a separate method with clearer logic.
+ private function setFormDefaults(&$form, array $defaults) { + if (!isset($form->_elements) || !is_array($form->_elements)) { + return; + } + + foreach ($form->_elements as $element) { + $name = $element->_attributes['name'] ?? null; + if ($name && isset($defaults[$name])) { + $form->setDefaults([$name => $defaults[$name]]); + } + } + } public function autofillMonetaryFormSource($formName, &$form) { // ... previous code ... - foreach ($autoFillData as $fieldName => $value) { - if (isset($form->_elements) && is_array($form->_elements)) { - foreach ($form->_elements as $element) { - if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { - $form->setDefaults([$fieldName => $value]); - error_log("Auto-filled $fieldName with value $value."); - } - } - } - } + $this->setFormDefaults($form, $autoFillData); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(1 hunks)
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
72-75: LGTM: Hook registration is properly structured.
The new hook registration follows the existing pattern and maintains consistent formatting.
| if (!empty($custom554)) { | ||
| $autoFillData['custom_554_-1'] = $custom554; | ||
| } | ||
| elseif (!empty($custom555)) { | ||
| $autoFillData['custom_555_-1'] = $custom555; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract magic strings into constants.
Form field identifiers are hardcoded throughout the code. This makes maintenance difficult and error-prone.
Add these constants at the class level:
+ private const CUSTOM_FIELD_554 = 'custom_554_-1';
+ private const CUSTOM_FIELD_555 = 'custom_555_-1';
public function autofillMonetaryFormSource($formName, &$form) {
// ...
if (!empty($custom554)) {
- $autoFillData['custom_554_-1'] = $custom554;
+ $autoFillData[self::CUSTOM_FIELD_554] = $custom554;
}
elseif (!empty($custom555)) {
- $autoFillData['custom_555_-1'] = $custom555;
+ $autoFillData[self::CUSTOM_FIELD_555] = $custom555;
}Committable suggestion skipped: line range outside the PR's diff.
| $custom554 = NULL; | ||
| $custom555 = NULL; | ||
|
|
||
| // Determine the parameter to use based on the form and query parameters. | ||
| if ($formName === 'CRM_Contribute_Form_Contribution') { | ||
| if (isset($_GET['custom_554_-1'])) { | ||
| $custom554 = $_GET['custom_554_-1']; | ||
| $_SESSION['custom_554'] = $custom554; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_555']); | ||
| } | ||
| elseif (isset($_GET['custom_555_-1'])) { | ||
| $custom555 = $_GET['custom_555_-1']; | ||
| $_SESSION['custom_555'] = $custom555; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_554']); | ||
| } | ||
| } | ||
| else { | ||
| // Retrieve from session if not provided in query parameters. | ||
| $custom554 = $_SESSION['custom_554'] ?? NULL; | ||
| $custom555 = $_SESSION['custom_555'] ?? NULL; | ||
| } |
There was a problem hiding this comment.
Security: Avoid direct superglobal access.
The direct usage of $_SESSION and $_GET is unsafe and violates encapsulation principles. Consider using CiviCRM's built-in session and request handling mechanisms.
Apply this refactor:
- $custom554 = NULL;
- $custom555 = NULL;
-
- // Determine the parameter to use based on the form and query parameters.
- if ($formName === 'CRM_Contribute_Form_Contribution') {
- if (isset($_GET['custom_554_-1'])) {
- $custom554 = $_GET['custom_554_-1'];
- $_SESSION['custom_554'] = $custom554;
- // Ensure only one session value is active.
- unset($_SESSION['custom_555']);
- }
- elseif (isset($_GET['custom_555_-1'])) {
- $custom555 = $_GET['custom_555_-1'];
- $_SESSION['custom_555'] = $custom555;
- // Ensure only one session value is active.
- unset($_SESSION['custom_554']);
- }
- }
- else {
- // Retrieve from session if not provided in query parameters.
- $custom554 = $_SESSION['custom_554'] ?? NULL;
- $custom555 = $_SESSION['custom_555'] ?? NULL;
- }
+ $session = CRM_Core_Session::singleton();
+ $custom554 = NULL;
+ $custom555 = NULL;
+
+ if ($formName === 'CRM_Contribute_Form_Contribution') {
+ $custom554 = CRM_Utils_Request::retrieve('custom_554_-1', 'String');
+ $custom555 = CRM_Utils_Request::retrieve('custom_555_-1', 'String');
+
+ if ($custom554) {
+ $session->set('custom_554', $custom554);
+ $session->set('custom_555', NULL);
+ } elseif ($custom555) {
+ $session->set('custom_555', $custom555);
+ $session->set('custom_554', NULL);
+ }
+ } else {
+ $custom554 = $session->get('custom_554');
+ $custom555 = $session->get('custom_555');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $custom554 = NULL; | |
| $custom555 = NULL; | |
| // Determine the parameter to use based on the form and query parameters. | |
| if ($formName === 'CRM_Contribute_Form_Contribution') { | |
| if (isset($_GET['custom_554_-1'])) { | |
| $custom554 = $_GET['custom_554_-1']; | |
| $_SESSION['custom_554'] = $custom554; | |
| // Ensure only one session value is active. | |
| unset($_SESSION['custom_555']); | |
| } | |
| elseif (isset($_GET['custom_555_-1'])) { | |
| $custom555 = $_GET['custom_555_-1']; | |
| $_SESSION['custom_555'] = $custom555; | |
| // Ensure only one session value is active. | |
| unset($_SESSION['custom_554']); | |
| } | |
| } | |
| else { | |
| // Retrieve from session if not provided in query parameters. | |
| $custom554 = $_SESSION['custom_554'] ?? NULL; | |
| $custom555 = $_SESSION['custom_555'] ?? NULL; | |
| } | |
| $session = CRM_Core_Session::singleton(); | |
| $custom554 = NULL; | |
| $custom555 = NULL; | |
| if ($formName === 'CRM_Contribute_Form_Contribution') { | |
| $custom554 = CRM_Utils_Request::retrieve('custom_554_-1', 'String'); | |
| $custom555 = CRM_Utils_Request::retrieve('custom_555_-1', 'String'); | |
| if ($custom554) { | |
| $session->set('custom_554', $custom554); | |
| $session->set('custom_555', NULL); | |
| } elseif ($custom555) { | |
| $session->set('custom_555', $custom555); | |
| $session->set('custom_554', NULL); | |
| } | |
| } else { | |
| $custom554 = $session->get('custom_554'); | |
| $custom555 = $session->get('custom_555'); | |
| } |
| /** | ||
| * Implements hook_civicrm_buildForm. | ||
| * | ||
| * Auto-fills custom fields in the form based on the provided parameters. | ||
| * | ||
| * @param string $formName | ||
| * The name of the form being built. | ||
| * @param object $form | ||
| * The form object. | ||
| */ | ||
| public function autofillMonetaryFormSource($formName, &$form) { | ||
| $custom554 = NULL; | ||
| $custom555 = NULL; | ||
|
|
||
| // Determine the parameter to use based on the form and query parameters. | ||
| if ($formName === 'CRM_Contribute_Form_Contribution') { | ||
| if (isset($_GET['custom_554_-1'])) { | ||
| $custom554 = $_GET['custom_554_-1']; | ||
| $_SESSION['custom_554'] = $custom554; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_555']); | ||
| } | ||
| elseif (isset($_GET['custom_555_-1'])) { | ||
| $custom555 = $_GET['custom_555_-1']; | ||
| $_SESSION['custom_555'] = $custom555; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_554']); | ||
| } | ||
| } | ||
| else { | ||
| // Retrieve from session if not provided in query parameters. | ||
| $custom554 = $_SESSION['custom_554'] ?? NULL; | ||
| $custom555 = $_SESSION['custom_555'] ?? NULL; | ||
| } | ||
|
|
||
| error_log("_GET: " . print_r($_GET, TRUE)); | ||
|
|
||
| // Autofill logic for the custom fields. | ||
| if ($formName === 'CRM_Custom_Form_CustomDataByType') { | ||
| $autoFillData = []; | ||
| if (!empty($custom554)) { | ||
| $autoFillData['custom_554_-1'] = $custom554; | ||
| } | ||
| elseif (!empty($custom555)) { | ||
| $autoFillData['custom_555_-1'] = $custom555; | ||
| } | ||
|
|
||
| error_log("autoFillData: " . print_r($autoFillData, TRUE)); | ||
|
|
||
| // Set default values for the specified fields. | ||
| foreach ($autoFillData as $fieldName => $value) { | ||
| if (isset($form->_elements) && is_array($form->_elements)) { | ||
| foreach ($form->_elements as $element) { | ||
| if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { | ||
| $form->setDefaults([$fieldName => $value]); | ||
| error_log("Auto-filled $fieldName with value $value."); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Input validation and error handling need improvement
The code has several security and robustness issues:
- Direct use of
$_GETand$_SESSIONwithout proper validation or sanitization - No type checking or validation of form values before using them
- Basic error logging but no proper error handling for invalid inputs
- No validation of custom field values before setting them as defaults
Here are the specific locations that need attention:
- Lines 79-83: Direct access to
$_GET['custom_554_-1']without validation - Lines 84-88: Direct access to
$_GET['custom_555_-1']without validation - Lines 95-96: Session values used without validation
- Lines 108-109: Form values set without sanitization
Required changes:
- Add input validation for all GET parameters:
$custom554 = filter_input(INPUT_GET, 'custom_554_-1', FILTER_SANITIZE_STRING);- Add type checking before using values:
if (!is_string($value) || strlen($value) > MAX_LENGTH) {
error_log("Invalid input value");
return;
}- Add proper error handling:
try {
$this->validateInput($value);
$form->setDefaults([$fieldName => $value]);
} catch (InvalidArgumentException $e) {
error_log("Validation failed: " . $e->getMessage());
return;
}- Add session data validation:
if (!isset($_SESSION['custom_554']) || !$this->isValidCustomValue($_SESSION['custom_554'])) {
return;
}🔗 Analysis chain
Add input validation and error handling.
The method lacks proper input validation and error handling for form parameters and session data.
Consider adding:
- Type checking for form values
- Sanitization of input data
- Error handling for invalid form states
- Logging of validation failures
Let me gather more information about the input validation and error handling in this file.
Let me gather more information about the input validation and sanitization patterns in this file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential XSS vulnerabilities in form handling
# Look for direct output of user input to forms
rg -A 2 'setDefaults|setValue|assign'
Length of output: 137077
Script:
#!/bin/bash
# Check for input validation and error handling patterns
ast-grep --pattern $'if (isset($_GET[$_]) || isset($_SESSION[$_])) {
$$$
}'
# Look for error handling patterns
rg -A 2 'throw|catch|try|error_log'
# Look for sanitization patterns
rg -A 2 'sanitize|filter|htmlspecialchars|strip_tags|addslashes|mysql_real_escape_string|pg_escape_string|escapeshellarg'
Length of output: 109932
Script:
#!/bin/bash
# Look for input validation patterns in the file
rg -A 2 "isset|empty|is_null|is_string|is_numeric|is_array|is_object|validate|filter_var" wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php
# Look for sanitization patterns in the file
rg -A 2 "sanitize|filter|htmlspecialchars|strip_tags|addslashes|mysql_real_escape_string|pg_escape_string|escapeshellarg|intval|floatval" wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php
# Look for error handling patterns in the file
rg -A 2 "throw|catch|try|CRM_Core_Error" wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php
Length of output: 4243
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (3)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (3)
72-74: LGTM: Event subscription follows existing patterns.
The addition of the form build hook subscription is clean and consistent with the existing code structure.
1351-1412: Extract magic strings into class constants.
The code uses hardcoded field names and form identifiers throughout.
Consider adding these constants at the class level:
+ private const FORM_CONTRIBUTION = 'CRM_Contribute_Form_Contribution';
+ private const FORM_CUSTOM_DATA = 'CRM_Custom_Form_CustomDataByType';
+ private const FIELD_CUSTOM_554 = 'custom_554_-1';
+ private const FIELD_CUSTOM_555 = 'custom_555_-1';Then use them in the code:
- if ($formName === 'CRM_Contribute_Form_Contribution') {
+ if ($formName === self::FORM_CONTRIBUTION) {1361-1384:
Use CiviCRM's form and session handling APIs.
The current implementation directly accesses superglobals and lacks proper validation.
As noted in previous reviews, consider using CiviCRM's APIs:
- $custom554 = NULL;
- $custom555 = NULL;
-
- if ($formName === 'CRM_Contribute_Form_Contribution') {
- if (isset($_GET['custom_554_-1'])) {
- $custom554 = $_GET['custom_554_-1'];
- $_SESSION['custom_554'] = $custom554;
- unset($_SESSION['custom_555']);
- }
- elseif (isset($_GET['custom_555_-1'])) {
- $custom555 = $_GET['custom_555_-1'];
- $_SESSION['custom_555'] = $custom555;
- unset($_SESSION['custom_554']);
- }
- }
- else {
- $custom554 = $_SESSION['custom_554'] ?? NULL;
- $custom555 = $_SESSION['custom_555'] ?? NULL;
- }
+ $session = CRM_Core_Session::singleton();
+ $custom554 = NULL;
+ $custom555 = NULL;
+
+ if ($formName === 'CRM_Contribute_Form_Contribution') {
+ $custom554 = CRM_Utils_Request::retrieve('custom_554_-1', 'String');
+ $custom555 = CRM_Utils_Request::retrieve('custom_555_-1', 'String');
+
+ if ($custom554) {
+ $session->set('custom_554', $custom554);
+ $session->set('custom_555', NULL);
+ } elseif ($custom555) {
+ $session->set('custom_555', $custom555);
+ $session->set('custom_554', NULL);
+ }
+ } else {
+ $custom554 = $session->get('custom_554');
+ $custom555 = $session->get('custom_555');
+ }| $custom555 = $_SESSION['custom_555'] ?? NULL; | ||
| } | ||
|
|
||
| error_log("_GET: " . print_r($_GET, TRUE)); |
There was a problem hiding this comment.
Remove or improve error logging statements.
The current error logging implementation has several issues:
- Using print_r with $_GET can expose sensitive data in logs
- Debug statements appear to be temporary
- Logging is not using CiviCRM's logging framework
Consider using CiviCRM's logging framework and removing sensitive data:
- error_log("_GET: " . print_r($_GET, TRUE));
+ \Civi::log()->debug('Autofilling monetary form', [
+ 'form_name' => $formName,
+ 'has_custom_554' => isset($_GET['custom_554_-1']),
+ 'has_custom_555' => isset($_GET['custom_555_-1'])
+ ]);Also applies to: 1398-1398, 1406-1406
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
1355-1359: Add parameter type hints in PHPDoc.The method parameters lack type information in the PHPDoc block.
Update the PHPDoc to include parameter types:
/** * Implements hook_civicrm_buildForm(). * * Auto-fills custom fields in the form based on the provided parameters. * - * @param string $formName - * The name of the form being built. - * @param object $form - * The form object. + * @param string $formName + * The name of the form being built. + * @param \CRM_Core_Form $form + * The form object. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
72-74: LGTM: Hook registration follows existing pattern.
The new hook registration for autofillMonetaryFormSource is properly integrated into the getSubscribedEvents method.
| public function autofillMonetaryFormSource($formName, &$form) { | ||
| $custom554 = NULL; | ||
| $custom555 = NULL; | ||
|
|
||
| // Determine the parameter to use based on the form and query parameters. | ||
| if ($formName === 'CRM_Contribute_Form_Contribution') { | ||
| if (isset($_GET['custom_554_-1'])) { | ||
| $custom554 = $_GET['custom_554_-1']; | ||
| $_SESSION['custom_554'] = $custom554; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_555']); | ||
| } | ||
| elseif (isset($_GET['custom_555_-1'])) { | ||
| $custom555 = $_GET['custom_555_-1']; | ||
| $_SESSION['custom_555'] = $custom555; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_554']); | ||
| } | ||
| } | ||
| else { | ||
| // Retrieve from session if not provided in query parameters. | ||
| $custom554 = $_SESSION['custom_554'] ?? NULL; | ||
| $custom555 = $_SESSION['custom_555'] ?? NULL; | ||
| } | ||
|
|
||
| // Autofill logic for the custom fields. | ||
| if ($formName === 'CRM_Custom_Form_CustomDataByType') { | ||
| $autoFillData = []; | ||
| if (!empty($custom554)) { | ||
| $autoFillData['custom_554_-1'] = $custom554; | ||
| } | ||
| elseif (!empty($custom555)) { | ||
| $autoFillData['custom_555_-1'] = $custom555; | ||
| } | ||
|
|
||
| // Set default values for the specified fields. | ||
| foreach ($autoFillData as $fieldName => $value) { | ||
| if (isset($form->_elements) && is_array($form->_elements)) { | ||
| foreach ($form->_elements as $element) { | ||
| if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { | ||
| $form->setDefaults([$fieldName => $value]); | ||
| error_log("Auto-filled $fieldName with value $value."); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider splitting the method into smaller, focused functions.
The method currently handles multiple responsibilities:
- Session state management
- Form auto-filling logic
This violates the Single Responsibility Principle and makes the code harder to maintain.
Consider refactoring into separate methods:
- public function autofillMonetaryFormSource($formName, &$form) {
+ private function getSourceValue($formName) {
$custom554 = NULL;
$custom555 = NULL;
if ($formName === 'CRM_Contribute_Form_Contribution') {
- if (isset($_GET['custom_554_-1'])) {
- $custom554 = $_GET['custom_554_-1'];
- $_SESSION['custom_554'] = $custom554;
- unset($_SESSION['custom_555']);
- }
- elseif (isset($_GET['custom_555_-1'])) {
- $custom555 = $_GET['custom_555_-1'];
- $_SESSION['custom_555'] = $custom555;
- unset($_SESSION['custom_554']);
- }
+ return $this->handleContributionForm();
} else {
- $custom554 = $_SESSION['custom_554'] ?? NULL;
- $custom555 = $_SESSION['custom_555'] ?? NULL;
+ return $this->getSessionValues();
}
+ }
+ public function autofillMonetaryFormSource($formName, &$form) {
+ $sourceValue = $this->getSourceValue($formName);
if ($formName === 'CRM_Custom_Form_CustomDataByType') {
- $autoFillData = [];
- if (!empty($custom554)) {
- $autoFillData['custom_554_-1'] = $custom554;
- }
- elseif (!empty($custom555)) {
- $autoFillData['custom_555_-1'] = $custom555;
- }
+ $this->fillCustomForm($form, $sourceValue);
+ }
+ }Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
72-74: LGTM! Event subscription follows existing pattern.
The addition of the form build hook subscription is clean and consistent with the existing code structure.
| foreach ($form->_elements as $element) { | ||
| if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { | ||
| $form->setDefaults([$fieldName => $value]); | ||
| error_log("Auto-filled $fieldName with value $value."); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace error_log with CiviCRM's logging framework.
Using error_log directly is not recommended. Use CiviCRM's logging framework instead.
Replace the error logging with:
- error_log("Auto-filled $fieldName with value $value.");
+ \Civi::log()->debug('Auto-filled field', [
+ 'field' => $fieldName,
+ 'value' => $value
+ ]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| error_log("Auto-filled $fieldName with value $value."); | |
| \Civi::log()->debug('Auto-filled field', [ | |
| 'field' => $fieldName, | |
| 'value' => $value | |
| ]); |
| foreach ($autoFillData as $fieldName => $value) { | ||
| if (isset($form->_elements) && is_array($form->_elements)) { | ||
| foreach ($form->_elements as $element) { | ||
| if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { | ||
| $form->setDefaults([$fieldName => $value]); | ||
| error_log("Auto-filled $fieldName with value $value."); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize form field processing.
The nested loops make the code harder to read and less efficient.
Apply this refactor to simplify the logic:
- foreach ($autoFillData as $fieldName => $value) {
- if (isset($form->_elements) && is_array($form->_elements)) {
- foreach ($form->_elements as $element) {
- if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) {
- $form->setDefaults([$fieldName => $value]);
- error_log("Auto-filled $fieldName with value $value.");
- }
- }
- }
- }
+ if (!isset($form->_elements) || !is_array($form->_elements)) {
+ return;
+ }
+
+ $elementsByName = array_column($form->_elements, NULL, function($element) {
+ return $element->_attributes['name'] ?? '';
+ });
+
+ foreach ($autoFillData as $fieldName => $value) {
+ if (isset($elementsByName[$fieldName])) {
+ $form->setDefaults([$fieldName => $value]);
+ \Civi::log()->debug('Auto-filled field', [
+ 'field' => $fieldName,
+ 'value' => $value
+ ]);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($autoFillData as $fieldName => $value) { | |
| if (isset($form->_elements) && is_array($form->_elements)) { | |
| foreach ($form->_elements as $element) { | |
| if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { | |
| $form->setDefaults([$fieldName => $value]); | |
| error_log("Auto-filled $fieldName with value $value."); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| if (!isset($form->_elements) || !is_array($form->_elements)) { | |
| return; | |
| } | |
| $elementsByName = array_column($form->_elements, NULL, function($element) { | |
| return $element->_attributes['name'] ?? ''; | |
| }); | |
| foreach ($autoFillData as $fieldName => $value) { | |
| if (isset($elementsByName[$fieldName])) { | |
| $form->setDefaults([$fieldName => $value]); | |
| \Civi::log()->debug('Auto-filled field', [ | |
| 'field' => $fieldName, | |
| 'value' => $value | |
| ]); | |
| } | |
| } |
| // Fetching custom field for collection source. | ||
| $sourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'Source') | ||
| ->execute()->single(); | ||
|
|
||
| $sourceFieldId = 'custom_' . $sourceField['id'] . '_-1'; | ||
|
|
||
| // Fetching custom field for goonj offfice. | ||
| $puSourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'PU_Source') | ||
| ->execute()->single(); | ||
|
|
||
| $puSourceFieldId = 'custom_' . $puSourceField['id'] . '_-1'; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract field IDs into class constants.
The field IDs are hardcoded and scattered throughout the code. This makes maintenance difficult.
Add these constants at the class level:
+ private const SOURCE_FIELD_NAME = 'Source';
+ private const PU_SOURCE_FIELD_NAME = 'PU_Source';
+ private const CONTRIBUTION_DETAILS_GROUP = 'Contribution_Details';
public function autofillMonetaryFormSource($formName, &$form) {
// Fetching custom field for collection source.
$sourceField = CustomField::get(FALSE)
->addSelect('id')
- ->addWhere('custom_group_id:name', '=', 'Contribution_Details')
- ->addWhere('name', '=', 'Source')
+ ->addWhere('custom_group_id:name', '=', self::CONTRIBUTION_DETAILS_GROUP)
+ ->addWhere('name', '=', self::SOURCE_FIELD_NAME)
->execute()->single();
// Fetching custom field for goonj office.
$puSourceField = CustomField::get(FALSE)
->addSelect('id')
- ->addWhere('custom_group_id:name', '=', 'Contribution_Details')
- ->addWhere('name', '=', 'PU_Source')
+ ->addWhere('custom_group_id:name', '=', self::CONTRIBUTION_DETAILS_GROUP)
+ ->addWhere('name', '=', self::PU_SOURCE_FIELD_NAME)
->execute()->single();Committable suggestion skipped: line range outside the PR's diff.
| public function autofillMonetaryFormSource($formName, &$form) { | ||
| $custom554 = NULL; | ||
| $custom555 = NULL; | ||
|
|
||
| // Fetching custom field for collection source. | ||
| $sourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'Source') | ||
| ->execute()->single(); | ||
|
|
||
| $sourceFieldId = 'custom_' . $sourceField['id'] . '_-1'; | ||
|
|
||
| // Fetching custom field for goonj offfice. | ||
| $puSourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'PU_Source') | ||
| ->execute()->single(); | ||
|
|
||
| $puSourceFieldId = 'custom_' . $puSourceField['id'] . '_-1'; | ||
|
|
||
| // Determine the parameter to use based on the form and query parameters. | ||
| if ($formName === 'CRM_Contribute_Form_Contribution') { | ||
| if (isset($_GET[$sourceFieldId])) { | ||
| $custom554 = $_GET[$sourceFieldId]; | ||
| $_SESSION['custom_554'] = $custom554; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_555']); | ||
| } | ||
| elseif (isset($_GET[$puSourceFieldId])) { | ||
| $custom555 = $_GET[$puSourceFieldId]; | ||
| $_SESSION['custom_555'] = $custom555; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['custom_554']); | ||
| } | ||
| } | ||
| else { | ||
| // Retrieve from session if not provided in query parameters. | ||
| $custom554 = $_SESSION['custom_554'] ?? NULL; | ||
| $custom555 = $_SESSION['custom_555'] ?? NULL; | ||
| } |
There was a problem hiding this comment.
Security: Replace direct superglobal access with CiviCRM's API.
The direct usage of $_SESSION and $_GET is unsafe. Use CiviCRM's session and request handling mechanisms instead.
Apply this refactor:
- $custom554 = NULL;
- $custom555 = NULL;
-
- if ($formName === 'CRM_Contribute_Form_Contribution') {
- if (isset($_GET[$sourceFieldId])) {
- $custom554 = $_GET[$sourceFieldId];
- $_SESSION['custom_554'] = $custom554;
- unset($_SESSION['custom_555']);
- }
- elseif (isset($_GET[$puSourceFieldId])) {
- $custom555 = $_GET[$puSourceFieldId];
- $_SESSION['custom_555'] = $custom555;
- unset($_SESSION['custom_554']);
- }
- }
- else {
- $custom554 = $_SESSION['custom_554'] ?? NULL;
- $custom555 = $_SESSION['custom_555'] ?? NULL;
- }
+ $session = CRM_Core_Session::singleton();
+ $custom554 = NULL;
+ $custom555 = NULL;
+
+ if ($formName === 'CRM_Contribute_Form_Contribution') {
+ $custom554 = CRM_Utils_Request::retrieve($sourceFieldId, 'String');
+ $custom555 = CRM_Utils_Request::retrieve($puSourceFieldId, 'String');
+
+ if ($custom554) {
+ $session->set('custom_554', $custom554);
+ $session->set('custom_555', NULL);
+ } elseif ($custom555) {
+ $session->set('custom_555', $custom555);
+ $session->set('custom_554', NULL);
+ }
+ } else {
+ $custom554 = $session->get('custom_554');
+ $custom555 = $session->get('custom_555');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function autofillMonetaryFormSource($formName, &$form) { | |
| $custom554 = NULL; | |
| $custom555 = NULL; | |
| // Fetching custom field for collection source. | |
| $sourceField = CustomField::get(FALSE) | |
| ->addSelect('id') | |
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | |
| ->addWhere('name', '=', 'Source') | |
| ->execute()->single(); | |
| $sourceFieldId = 'custom_' . $sourceField['id'] . '_-1'; | |
| // Fetching custom field for goonj offfice. | |
| $puSourceField = CustomField::get(FALSE) | |
| ->addSelect('id') | |
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | |
| ->addWhere('name', '=', 'PU_Source') | |
| ->execute()->single(); | |
| $puSourceFieldId = 'custom_' . $puSourceField['id'] . '_-1'; | |
| // Determine the parameter to use based on the form and query parameters. | |
| if ($formName === 'CRM_Contribute_Form_Contribution') { | |
| if (isset($_GET[$sourceFieldId])) { | |
| $custom554 = $_GET[$sourceFieldId]; | |
| $_SESSION['custom_554'] = $custom554; | |
| // Ensure only one session value is active. | |
| unset($_SESSION['custom_555']); | |
| } | |
| elseif (isset($_GET[$puSourceFieldId])) { | |
| $custom555 = $_GET[$puSourceFieldId]; | |
| $_SESSION['custom_555'] = $custom555; | |
| // Ensure only one session value is active. | |
| unset($_SESSION['custom_554']); | |
| } | |
| } | |
| else { | |
| // Retrieve from session if not provided in query parameters. | |
| $custom554 = $_SESSION['custom_554'] ?? NULL; | |
| $custom555 = $_SESSION['custom_555'] ?? NULL; | |
| } | |
| public function autofillMonetaryFormSource($formName, &$form) { | |
| // Fetching custom field for collection source. | |
| $sourceField = CustomField::get(FALSE) | |
| ->addSelect('id') | |
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | |
| ->addWhere('name', '=', 'Source') | |
| ->execute()->single(); | |
| $sourceFieldId = 'custom_' . $sourceField['id'] . '_-1'; | |
| // Fetching custom field for goonj offfice. | |
| $puSourceField = CustomField::get(FALSE) | |
| ->addSelect('id') | |
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | |
| ->addWhere('name', '=', 'PU_Source') | |
| ->execute()->single(); | |
| $puSourceFieldId = 'custom_' . $puSourceField['id'] . '_-1'; | |
| $session = CRM_Core_Session::singleton(); | |
| $custom554 = NULL; | |
| $custom555 = NULL; | |
| if ($formName === 'CRM_Contribute_Form_Contribution') { | |
| $custom554 = CRM_Utils_Request::retrieve($sourceFieldId, 'String'); | |
| $custom555 = CRM_Utils_Request::retrieve($puSourceFieldId, 'String'); | |
| if ($custom554) { | |
| $session->set('custom_554', $custom554); | |
| $session->set('custom_555', NULL); | |
| } elseif ($custom555) { | |
| $session->set('custom_555', $custom555); | |
| $session->set('custom_554', NULL); | |
| } | |
| } else { | |
| $custom554 = $session->get('custom_554'); | |
| $custom555 = $session->get('custom_555'); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
1350-1424: Add error handling and logging.The method lacks proper error handling and logging mechanisms.
Add error handling:
public function autofillMonetaryFormSource($formName, &$form) { + try { + \Civi::log()->debug('Auto-filling monetary form', [ + 'form_name' => $formName, + 'has_source_field' => isset($_GET[$sourceFieldId]), + 'has_pu_source_field' => isset($_GET[$puSourceFieldId]) + ]); + // ... existing code ... + + } catch (\Exception $e) { + \Civi::log()->error('Error auto-filling monetary form: ' . $e->getMessage(), [ + 'form_name' => $formName, + 'exception' => $e + ]); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (2)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (2)
72-74: LGTM! Hook registration is properly implemented.
The new hook registration follows the existing pattern and correctly maps to the new method.
1364-1381: 🛠️ Refactor suggestion
Extract field identifiers into constants.
The hardcoded field names and custom group names make the code harder to maintain.
Define these as class constants:
+ private const CONTRIBUTION_DETAILS_GROUP = 'Contribution_Details';
+ private const SOURCE_FIELD_NAME = 'Source';
+ private const PU_SOURCE_FIELD_NAME = 'PU_Source';
public function autofillMonetaryFormSource($formName, &$form) {
$sourceField = CustomField::get(FALSE)
->addSelect('id')
- ->addWhere('custom_group_id:name', '=', 'Contribution_Details')
- ->addWhere('name', '=', 'Source')
+ ->addWhere('custom_group_id:name', '=', self::CONTRIBUTION_DETAILS_GROUP)
+ ->addWhere('name', '=', self::SOURCE_FIELD_NAME)
->execute()->single();
$puSourceField = CustomField::get(FALSE)
->addSelect('id')
- ->addWhere('custom_group_id:name', '=', 'Contribution_Details')
- ->addWhere('name', '=', 'PU_Source')
+ ->addWhere('custom_group_id:name', '=', self::CONTRIBUTION_DETAILS_GROUP)
+ ->addWhere('name', '=', self::PU_SOURCE_FIELD_NAME)
->execute()->single();Likely invalid or redundant comment.
| // Set default values for the specified fields. | ||
| foreach ($autoFillData as $fieldName => $value) { | ||
| if (isset($form->_elements) && is_array($form->_elements)) { | ||
| foreach ($form->_elements as $element) { | ||
| if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { | ||
| $form->setDefaults([$fieldName => $value]); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize form field processing.
The nested loops make the code harder to read and less efficient.
Simplify the logic:
- foreach ($autoFillData as $fieldName => $value) {
- if (isset($form->_elements) && is_array($form->_elements)) {
- foreach ($form->_elements as $element) {
- if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) {
- $form->setDefaults([$fieldName => $value]);
- }
- }
- }
- }
+ if (!isset($form->_elements) || !is_array($form->_elements)) {
+ return;
+ }
+
+ $elementsByName = array_column($form->_elements, NULL, function($element) {
+ return $element->_attributes['name'] ?? '';
+ });
+
+ foreach ($autoFillData as $fieldName => $value) {
+ if (isset($elementsByName[$fieldName])) {
+ $form->setDefaults([$fieldName => $value]);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Set default values for the specified fields. | |
| foreach ($autoFillData as $fieldName => $value) { | |
| if (isset($form->_elements) && is_array($form->_elements)) { | |
| foreach ($form->_elements as $element) { | |
| if (isset($element->_attributes['name']) && $element->_attributes['name'] === $fieldName) { | |
| $form->setDefaults([$fieldName => $value]); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| // Set default values for the specified fields. | |
| if (!isset($form->_elements) || !is_array($form->_elements)) { | |
| return; | |
| } | |
| $elementsByName = array_column($form->_elements, NULL, function($element) { | |
| return $element->_attributes['name'] ?? ''; | |
| }); | |
| foreach ($autoFillData as $fieldName => $value) { | |
| if (isset($elementsByName[$fieldName])) { | |
| $form->setDefaults([$fieldName => $value]); | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
1350-1359: Enhance method documentation.Consider improving the PHPDoc block:
- Add
@return voidtag- Add type hints for parameters
/** * Implements hook_civicrm_buildForm(). * * Auto-fills custom fields in the form based on the provided parameters. * * @param string $formName * The name of the form being built. * @param object $form * The form object. + * @return void */ - public function autofillMonetaryFormSource($formName, &$form) { + public function autofillMonetaryFormSource(string $formName, object &$form): void {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (2)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (2)
72-74: LGTM: Hook registration follows existing pattern.
The addition of the form building hook is consistent with the class's event subscription pattern.
1382-1401:
Security: Replace direct superglobal access with CiviCRM's API.
The direct usage of $_SESSION and $_GET is unsafe. Use CiviCRM's session and request handling mechanisms instead.
This issue was previously identified in past review comments. Please implement the suggested changes using CRM_Core_Session::singleton() and CRM_Utils_Request::retrieve().
| foreach ($autoFillData as $fieldName => $value) { | ||
| if (isset($form->_elements) && is_array($form->_elements)) { | ||
| foreach ($form->_elements as $element) { | ||
| if (!isset($element->_attributes['data-api-params'])) { | ||
| continue; | ||
| } | ||
| $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); | ||
| if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') { | ||
| $form->setDefaults([$fieldName => $value]); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize form field processing logic.
The nested loops and JSON parsing make the code complex and potentially slow.
- foreach ($autoFillData as $fieldName => $value) {
- if (isset($form->_elements) && is_array($form->_elements)) {
- foreach ($form->_elements as $element) {
- if (!isset($element->_attributes['data-api-params'])) {
- continue;
- }
- $apiParams = json_decode($element->_attributes['data-api-params'], TRUE);
- if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') {
- $form->setDefaults([$fieldName => $value]);
- }
- }
- }
- }
+ if (!isset($form->_elements) || !is_array($form->_elements)) {
+ return;
+ }
+
+ $sourceElements = array_filter($form->_elements, function($element) {
+ if (!isset($element->_attributes['data-api-params'])) {
+ return false;
+ }
+ $apiParams = json_decode($element->_attributes['data-api-params'], TRUE);
+ return $apiParams['fieldName'] === 'Contribution.Contribution_Details.Source';
+ });
+
+ foreach ($autoFillData as $fieldName => $value) {
+ if (!empty($sourceElements)) {
+ $form->setDefaults([$fieldName => $value]);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| foreach ($autoFillData as $fieldName => $value) { | |
| if (isset($form->_elements) && is_array($form->_elements)) { | |
| foreach ($form->_elements as $element) { | |
| if (!isset($element->_attributes['data-api-params'])) { | |
| continue; | |
| } | |
| $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); | |
| if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') { | |
| $form->setDefaults([$fieldName => $value]); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| if (!isset($form->_elements) || !is_array($form->_elements)) { | |
| return; | |
| } | |
| $sourceElements = array_filter($form->_elements, function($element) { | |
| if (!isset($element->_attributes['data-api-params'])) { | |
| return false; | |
| } | |
| $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); | |
| return $apiParams['fieldName'] === 'Contribution.Contribution_Details.Source'; | |
| }); | |
| foreach ($autoFillData as $fieldName => $value) { | |
| if (!empty($sourceElements)) { | |
| $form->setDefaults([$fieldName => $value]); | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (2)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (2)
72-74: LGTM: Event subscription properly configured.
The new hook registration follows the established pattern in the class.
1382-1401:
Security: Replace direct superglobal access with CiviCRM's API.
The direct usage of $_SESSION and $_GET is unsafe. Use CiviCRM's session and request handling mechanisms instead.
- if ($formName === 'CRM_Contribute_Form_Contribution') {
- if (isset($_GET[$sourceFieldId])) {
- $campSource = $_GET[$sourceFieldId];
- $_SESSION['camp_source'] = $campSource;
- unset($_SESSION['pu_source']);
- }
- elseif (isset($_GET[$puSourceFieldId])) {
- $puSource = $_GET[$puSourceFieldId];
- $_SESSION['pu_source'] = $puSource;
- unset($_SESSION['camp_source']);
- }
- }
- else {
- $campSource = $_SESSION['camp_source'] ?? NULL;
- $puSource = $_SESSION['pu_source'] ?? NULL;
- }
+ $session = CRM_Core_Session::singleton();
+
+ if ($formName === 'CRM_Contribute_Form_Contribution') {
+ $campSource = CRM_Utils_Request::retrieve($sourceFieldId, 'String');
+ $puSource = CRM_Utils_Request::retrieve($puSourceFieldId, 'String');
+
+ if ($campSource) {
+ $session->set('camp_source', $campSource);
+ $session->set('pu_source', NULL);
+ } elseif ($puSource) {
+ $session->set('pu_source', $puSource);
+ $session->set('camp_source', NULL);
+ }
+ } else {
+ $campSource = $session->get('camp_source');
+ $puSource = $session->get('pu_source');
+ }Likely invalid or redundant comment.
| // Set default values for the specified fields. | ||
| foreach ($autoFillData as $fieldName => $value) { | ||
| if (isset($form->_elements) && is_array($form->_elements)) { | ||
| foreach ($form->_elements as $element) { | ||
| if (!isset($element->_attributes['data-api-params'])) { | ||
| continue; | ||
| } | ||
| $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); | ||
| if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') { | ||
| $formFieldName = $fieldName . '_+1'; | ||
| $form->setDefaults([$formFieldName => $value]); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize form field processing logic.
The nested loops and JSON parsing make the code complex and potentially slow. Consider optimizing the logic.
- foreach ($autoFillData as $fieldName => $value) {
- if (isset($form->_elements) && is_array($form->_elements)) {
- foreach ($form->_elements as $element) {
- if (!isset($element->_attributes['data-api-params'])) {
- continue;
- }
- $apiParams = json_decode($element->_attributes['data-api-params'], TRUE);
- if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') {
- $formFieldName = $fieldName . '_+1';
- $form->setDefaults([$formFieldName => $value]);
- }
- }
- }
- }
+ if (!isset($form->_elements) || !is_array($form->_elements)) {
+ return;
+ }
+
+ $sourceElements = array_filter($form->_elements, function($element) {
+ if (!isset($element->_attributes['data-api-params'])) {
+ return false;
+ }
+ try {
+ $apiParams = json_decode($element->_attributes['data-api-params'], TRUE);
+ return $apiParams['fieldName'] === 'Contribution.Contribution_Details.Source';
+ } catch (\Exception $e) {
+ \Civi::log()->error('Failed to parse API params: ' . $e->getMessage());
+ return false;
+ }
+ });
+
+ foreach ($autoFillData as $fieldName => $value) {
+ if (!empty($sourceElements)) {
+ $formFieldName = $fieldName . '_+1';
+ $form->setDefaults([$formFieldName => $value]);
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Set default values for the specified fields. | |
| foreach ($autoFillData as $fieldName => $value) { | |
| if (isset($form->_elements) && is_array($form->_elements)) { | |
| foreach ($form->_elements as $element) { | |
| if (!isset($element->_attributes['data-api-params'])) { | |
| continue; | |
| } | |
| $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); | |
| if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') { | |
| $formFieldName = $fieldName . '_+1'; | |
| $form->setDefaults([$formFieldName => $value]); | |
| } | |
| } | |
| } | |
| } | |
| // Set default values for the specified fields. | |
| if (!isset($form->_elements) || !is_array($form->_elements)) { | |
| return; | |
| } | |
| $sourceElements = array_filter($form->_elements, function($element) { | |
| if (!isset($element->_attributes['data-api-params'])) { | |
| return false; | |
| } | |
| try { | |
| $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); | |
| return $apiParams['fieldName'] === 'Contribution.Contribution_Details.Source'; | |
| } catch (\Exception $e) { | |
| \Civi::log()->error('Failed to parse API params: ' . $e->getMessage()); | |
| return false; | |
| } | |
| }); | |
| foreach ($autoFillData as $fieldName => $value) { | |
| if (!empty($sourceElements)) { | |
| $formFieldName = $fieldName . '_+1'; | |
| $form->setDefaults([$formFieldName => $value]); | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (2)
1417-1431: Optimize form field processing logic.The nested loops and JSON parsing make the code complex and potentially slow. Consider optimizing the logic.
- foreach ($autoFillData as $fieldName => $value) { - if (isset($form->_elements) && is_array($form->_elements)) { - foreach ($form->_elements as $element) { - if (!isset($element->_attributes['data-api-params'])) { - continue; - } - $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); - if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') { - $formFieldName = $fieldName . '_+1'; - $form->setDefaults([$formFieldName => $value]); - } - } - } - } + if (!isset($form->_elements) || !is_array($form->_elements)) { + return; + } + + $sourceElements = array_filter($form->_elements, function($element) { + if (!isset($element->_attributes['data-api-params'])) { + return false; + } + try { + $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); + return $apiParams['fieldName'] === 'Contribution.Contribution_Details.Source'; + } catch (\Exception $e) { + \Civi::log()->error('Failed to parse API params: ' . $e->getMessage()); + return false; + } + }); + + foreach ($autoFillData as $fieldName => $value) { + if (!empty($sourceElements)) { + $formFieldName = $fieldName . '_+1'; + $form->setDefaults([$formFieldName => $value]); + } + }
1350-1433: Add PHPDoc blocks for parameters and return type.The method lacks proper documentation for parameters and return type.
/** * Implements hook_civicrm_buildForm(). * * Auto-fills custom fields in the form based on the provided parameters. * + * @param string $formName + * The name of the form being built. + * @param \CRM_Core_Form $form + * The form object being built. + * + * @return void */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (1)
72-74: LGTM: Hook registration looks good.
The hook registration in getSubscribedEvents is properly implemented.
| * The form object. | ||
| */ | ||
| public function autofillMonetaryFormSource($formName, &$form) { | ||
| if (!in_array($formName, ['CRM_Contribute_Form_Contribution', 'CRM_Custom_Form_CustomDataByType']) { |
There was a problem hiding this comment.
Fix syntax error in if condition.
There's a syntax error in the if condition - missing closing parenthesis.
- if (!in_array($formName, ['CRM_Contribute_Form_Contribution', 'CRM_Custom_Form_CustomDataByType']) {
+ if (!in_array($formName, ['CRM_Contribute_Form_Contribution', 'CRM_Custom_Form_CustomDataByType'])) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!in_array($formName, ['CRM_Contribute_Form_Contribution', 'CRM_Custom_Form_CustomDataByType']) { | |
| if (!in_array($formName, ['CRM_Contribute_Form_Contribution', 'CRM_Custom_Form_CustomDataByType'])) { |
| // Fetching custom field for collection source. | ||
| $sourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'Source') | ||
| ->execute()->single(); | ||
|
|
||
| $sourceFieldId = 'custom_' . $sourceField['id']; | ||
|
|
||
| // Fetching custom field for goonj offfice. | ||
| $puSourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'PU_Source') | ||
| ->execute()->single(); | ||
|
|
||
| $puSourceFieldId = 'custom_' . $puSourceField['id']; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract duplicate field fetching logic into a reusable method.
The field fetching logic is duplicated for both source and PU source fields. Consider extracting it into a reusable method.
+ private function getCustomField(string $name): array {
+ return CustomField::get(FALSE)
+ ->addSelect('id')
+ ->addWhere('custom_group_id:name', '=', 'Contribution_Details')
+ ->addWhere('name', '=', $name)
+ ->execute()->single();
+ }
public function autofillMonetaryFormSource($formName, &$form) {
- $sourceField = CustomField::get(FALSE)
- ->addSelect('id')
- ->addWhere('custom_group_id:name', '=', 'Contribution_Details')
- ->addWhere('name', '=', 'Source')
- ->execute()->single();
+ $sourceField = $this->getCustomField('Source');
- $puSourceField = CustomField::get(FALSE)
- ->addSelect('id')
- ->addWhere('custom_group_id:name', '=', 'Contribution_Details')
- ->addWhere('name', '=', 'PU_Source')
- ->execute()->single();
+ $puSourceField = $this->getCustomField('PU_Source');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Fetching custom field for collection source. | |
| $sourceField = CustomField::get(FALSE) | |
| ->addSelect('id') | |
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | |
| ->addWhere('name', '=', 'Source') | |
| ->execute()->single(); | |
| $sourceFieldId = 'custom_' . $sourceField['id']; | |
| // Fetching custom field for goonj offfice. | |
| $puSourceField = CustomField::get(FALSE) | |
| ->addSelect('id') | |
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | |
| ->addWhere('name', '=', 'PU_Source') | |
| ->execute()->single(); | |
| $puSourceFieldId = 'custom_' . $puSourceField['id']; | |
| private function getCustomField(string $name): array { | |
| return CustomField::get(FALSE) | |
| ->addSelect('id') | |
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | |
| ->addWhere('name', '=', $name) | |
| ->execute()->single(); | |
| } | |
| // Fetching custom field for collection source. | |
| $sourceField = $this->getCustomField('Source'); | |
| $sourceFieldId = 'custom_' . $sourceField['id']; | |
| // Fetching custom field for goonj offfice. | |
| $puSourceField = $this->getCustomField('PU_Source'); | |
| $puSourceFieldId = 'custom_' . $puSourceField['id']; |
| // Determine the parameter to use based on the form and query parameters. | ||
| if ($formName === 'CRM_Contribute_Form_Contribution') { | ||
| if (isset($_GET[$sourceFieldId])) { | ||
| $campSource = $_GET[$sourceFieldId]; | ||
| $_SESSION['camp_source'] = $campSource; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['pu_source']); | ||
| } | ||
| elseif (isset($_GET[$puSourceFieldId])) { | ||
| $puSource = $_GET[$puSourceFieldId]; | ||
| $_SESSION['pu_source'] = $puSource; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['camp_source']); | ||
| } | ||
| } | ||
| else { | ||
| // Retrieve from session if not provided in query parameters. | ||
| $campSource = $_SESSION['camp_source'] ?? NULL; | ||
| $puSource = $_SESSION['pu_source'] ?? NULL; | ||
| } |
There was a problem hiding this comment.
Replace direct superglobal access with CiviCRM's API.
Direct usage of $_SESSION and $_GET is unsafe. Use CiviCRM's session and request handling mechanisms instead.
+ $session = CRM_Core_Session::singleton();
if ($formName === 'CRM_Contribute_Form_Contribution') {
- if (isset($_GET[$sourceFieldId])) {
- $campSource = $_GET[$sourceFieldId];
- $_SESSION['camp_source'] = $campSource;
- unset($_SESSION['pu_source']);
+ $campSource = CRM_Utils_Request::retrieve($sourceFieldId, 'String');
+ if ($campSource) {
+ $session->set('camp_source', $campSource);
+ $session->set('pu_source', NULL);
}
- elseif (isset($_GET[$puSourceFieldId])) {
- $puSource = $_GET[$puSourceFieldId];
- $_SESSION['pu_source'] = $puSource;
- unset($_SESSION['camp_source']);
+ else {
+ $puSource = CRM_Utils_Request::retrieve($puSourceFieldId, 'String');
+ if ($puSource) {
+ $session->set('pu_source', $puSource);
+ $session->set('camp_source', NULL);
+ }
}
}
else {
- $campSource = $_SESSION['camp_source'] ?? NULL;
- $puSource = $_SESSION['pu_source'] ?? NULL;
+ $campSource = $session->get('camp_source');
+ $puSource = $session->get('pu_source');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Determine the parameter to use based on the form and query parameters. | |
| if ($formName === 'CRM_Contribute_Form_Contribution') { | |
| if (isset($_GET[$sourceFieldId])) { | |
| $campSource = $_GET[$sourceFieldId]; | |
| $_SESSION['camp_source'] = $campSource; | |
| // Ensure only one session value is active. | |
| unset($_SESSION['pu_source']); | |
| } | |
| elseif (isset($_GET[$puSourceFieldId])) { | |
| $puSource = $_GET[$puSourceFieldId]; | |
| $_SESSION['pu_source'] = $puSource; | |
| // Ensure only one session value is active. | |
| unset($_SESSION['camp_source']); | |
| } | |
| } | |
| else { | |
| // Retrieve from session if not provided in query parameters. | |
| $campSource = $_SESSION['camp_source'] ?? NULL; | |
| $puSource = $_SESSION['pu_source'] ?? NULL; | |
| } | |
| // Determine the parameter to use based on the form and query parameters. | |
| $session = CRM_Core_Session::singleton(); | |
| if ($formName === 'CRM_Contribute_Form_Contribution') { | |
| $campSource = CRM_Utils_Request::retrieve($sourceFieldId, 'String'); | |
| if ($campSource) { | |
| $session->set('camp_source', $campSource); | |
| $session->set('pu_source', NULL); | |
| } | |
| else { | |
| $puSource = CRM_Utils_Request::retrieve($puSourceFieldId, 'String'); | |
| if ($puSource) { | |
| $session->set('pu_source', $puSource); | |
| $session->set('camp_source', NULL); | |
| } | |
| } | |
| } | |
| else { | |
| $campSource = $session->get('camp_source'); | |
| $puSource = $session->get('pu_source'); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php(2 hunks)
🔇 Additional comments (4)
wp-content/civi-extensions/goonjcustom/Civi/CollectionCampService.php (4)
72-74: LGTM! Hook registration looks good.
The new hook registration for form building follows the existing pattern and is properly integrated.
1368-1384: 🛠️ Refactor suggestion
Extract duplicate field fetching logic into a reusable method.
The field fetching logic for source and PU source fields is duplicated. This violates the DRY principle.
1386-1405:
Replace direct superglobal access with CiviCRM's API.
Direct usage of $_SESSION and $_GET is unsafe. Use CiviCRM's session and request handling mechanisms instead.
1417-1432: 🛠️ Refactor suggestion
Optimize form field processing logic.
The nested loops and JSON parsing make the code complex and potentially slow. Consider using array functions to simplify the logic.
| public function autofillMonetaryFormSource($formName, &$form) { | ||
| if (!in_array($formName, ['CRM_Contribute_Form_Contribution', 'CRM_Custom_Form_CustomDataByType'])) { | ||
| return; | ||
| } | ||
|
|
||
| $campSource = NULL; | ||
| $puSource = NULL; | ||
|
|
||
| // Fetching custom field for collection source. | ||
| $sourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'Source') | ||
| ->execute()->single(); | ||
|
|
||
| $sourceFieldId = 'custom_' . $sourceField['id']; | ||
|
|
||
| // Fetching custom field for goonj offfice. | ||
| $puSourceField = CustomField::get(FALSE) | ||
| ->addSelect('id') | ||
| ->addWhere('custom_group_id:name', '=', 'Contribution_Details') | ||
| ->addWhere('name', '=', 'PU_Source') | ||
| ->execute()->single(); | ||
|
|
||
| $puSourceFieldId = 'custom_' . $puSourceField['id']; | ||
|
|
||
| // Determine the parameter to use based on the form and query parameters. | ||
| if ($formName === 'CRM_Contribute_Form_Contribution') { | ||
| if (isset($_GET[$sourceFieldId])) { | ||
| $campSource = $_GET[$sourceFieldId]; | ||
| $_SESSION['camp_source'] = $campSource; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['pu_source']); | ||
| } | ||
| elseif (isset($_GET[$puSourceFieldId])) { | ||
| $puSource = $_GET[$puSourceFieldId]; | ||
| $_SESSION['pu_source'] = $puSource; | ||
| // Ensure only one session value is active. | ||
| unset($_SESSION['camp_source']); | ||
| } | ||
| } | ||
| else { | ||
| // Retrieve from session if not provided in query parameters. | ||
| $campSource = $_SESSION['camp_source'] ?? NULL; | ||
| $puSource = $_SESSION['pu_source'] ?? NULL; | ||
| } | ||
|
|
||
| // Autofill logic for the custom fields. | ||
| if ($formName === 'CRM_Custom_Form_CustomDataByType') { | ||
| $autoFillData = []; | ||
| if (!empty($campSource)) { | ||
| $autoFillData[$sourceFieldId] = $campSource; | ||
| } | ||
| elseif (!empty($puSource)) { | ||
| $autoFillData[$puSourceFieldId] = $puSource; | ||
| } | ||
|
|
||
| // Set default values for the specified fields. | ||
| foreach ($autoFillData as $fieldName => $value) { | ||
| if (isset($form->_elements) && is_array($form->_elements)) { | ||
| foreach ($form->_elements as $element) { | ||
| if (!isset($element->_attributes['data-api-params'])) { | ||
| continue; | ||
| } | ||
| $apiParams = json_decode($element->_attributes['data-api-params'], TRUE); | ||
| if ($apiParams['fieldName'] === 'Contribution.Contribution_Details.Source') { | ||
| $formFieldName = $fieldName . '_-1'; | ||
| $form->setDefaults([$formFieldName => $value]); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Add error handling and validation to prevent data corruption and improve reliability
The autofillMonetaryFormSource method needs several critical safeguards:
- Add try-catch blocks around API calls to
CustomField::get()to handle potential exceptions - Validate form parameters before processing:
if (!$form instanceof CRM_Core_Form) { Civi::log()->warning('Invalid form object passed to autofillMonetaryFormSource'); return; }
- Add null checks before accessing array indices:
$sourceField = CustomField::get(FALSE)->execute()->first(); if (empty($sourceField['id'])) { Civi::log()->error('Required custom field "Source" not found'); return; }
- Validate JSON parsing:
$apiParams = json_decode($element->_attributes['data-api-params'] ?? '', TRUE); if (json_last_error() !== JSON_ERROR_NONE) { Civi::log()->warning('Invalid JSON in form field attributes'); continue; }
The codebase shows consistent error handling patterns using try-catch and logging in other services, but this method lacks these essential safeguards.
🔗 Analysis chain
Add input validation and error handling.
The method lacks proper validation for:
- Form parameters
- Session data
- JSON parsing
- Field existence
Add appropriate error handling and logging using CiviCRM's logging framework.
Let me analyze the error handling patterns in the codebase to provide a comprehensive response.
Let me analyze the results to understand the validation patterns and generate a final response.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for error handling patterns in similar form handling code
rg -A 2 "try|catch|CRM_Core_Error|Civi::log" wp-content/civi-extensions/goonjcustom/Civi/
Length of output: 22542
Script:
#!/bin/bash
# Check for API calls and field validations in the autofillMonetaryFormSource method
ast-grep --pattern 'CustomField::get($$$)'
# Check for form validation patterns
rg -A 2 "validate|isValid|checkPermission" wp-content/civi-extensions/goonjcustom/Civi/
Length of output: 9666
2 participants
When admin adds the monetary contribution then we are auto-filling the source for that camp or office.
Camp URL:
civicrm/wp-admin/admin.php?page=CiviCRM&q=civicrm%2Fcontribute%2Fadd&reset=1&action=add&context=standalone&custom_554_-1=[Contribution_Details.Source]Office URL:
civicrm/wp-admin/admin.php?page=CiviCRM&q=civicrm%2Fcontribute%2Fadd&reset=1&action=add&context=standalone&custom_555_-1=[Contribution_Details.PU_Source]hook_civicrm_buildForm()hookSummary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes