CIVIMM-396: Ensure Awards custom group is stored with correct values #299
Conversation
olayiwola-compucorp
force-pushed
the
CIVIMM-396-customfield
branch
2 times, most recently
from
db5cf47 to
9bce376
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where Awards custom groups were being incorrectly matched to case type categories due to reliance on the extends property instead of the proper case type category name. The fix ensures custom groups are correctly associated with their intended case categories and prevents undefined index errors.
- Updated custom group post-processor to use proper case type category resolution
- Modified form handling to correctly set default values for case categories
- Improved error handling and validation in the form building process
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| CRM/CiviAwards/Service/ApplicantManagementCustomGroupPostProcessor.php | Updated to use getCategoryNameForCaseType() method instead of relying on extends property for proper case type category matching |
| CRM/CiviAwards/Hook/BuildForm/SetCustomGroupSubTypeValues.php | Enhanced form value handling with better validation, error handling, and correct element manipulation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| $caseCategoryInstance = CaseCategoryHelper::getInstanceObject($extendsId); | ||
| if (empty($caseCategoryInstance || !$caseCategoryInstance instanceof ApplicantManagementUtils)) { |
There was a problem hiding this comment.
The logical OR operator should be AND (&&) here. The current condition empty($caseCategoryInstance || !$caseCategoryInstance instanceof ApplicantManagementUtils) will always evaluate to true when $caseCategoryInstance is not empty, making the instanceof check meaningless.
| if (empty($caseCategoryInstance || !$caseCategoryInstance instanceof ApplicantManagementUtils)) { | |
| if (empty($caseCategoryInstance) || !$caseCategoryInstance instanceof ApplicantManagementUtils) { |
| $extendSelect->setValue($categoryName); | ||
| } | ||
| catch (Exception $e) { | ||
| Civi::log()->warning('CiviCase: Could not set hierarchical select value: ' . $e->getMessage()); |
There was a problem hiding this comment.
The error message prefix 'CiviCase:' is misleading since this is in the CiviAwards module. Consider changing to 'CiviAwards:' for consistency.
| Civi::log()->warning('CiviCase: Could not set hierarchical select value: ' . $e->getMessage()); | |
| Civi::log()->warning('CiviAwards: Could not set hierarchical select value: ' . $e->getMessage()); |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
| private function setDefaultFormValueForCaseCategory(CRM_Core_Form &$form) { | ||
| $defaults = $form->getVar('_defaults'); | ||
| $extendsId = $defaults['extends_entity_column_id']; | ||
| $defaults = $form->getVar('_values'); | ||
| $extendsId = $defaults['extends_entity_column_id'] ?? NULL; | ||
|
|
||
| if (empty($extendsId)) { | ||
| return; | ||
| } | ||
|
|
||
| $caseCategoryInstance = CaseCategoryHelper::getInstanceObject($extendsId); | ||
| if (empty($caseCategoryInstance || !$caseCategoryInstance instanceof ApplicantManagementUtils)) { | ||
| return; | ||
| } | ||
|
|
||
| $customGroupId = $form->getVar('_id'); | ||
| $postProcessHelper = new CustomGroupPostProcessHelper(); | ||
| $customGroupSubTypes = $postProcessHelper->getCustomGroupSubTypesList($customGroupId); | ||
| $caseTypeCategories = (CRM_Case_BAO_CaseType::buildOptions('case_type_category', 'validate')); | ||
| $defaults['extends'][0] = $caseTypeCategories[$extendsId]; | ||
| $hierSelect = $form->getElement('extends'); | ||
| $hierSelectElements = $hierSelect->getElements(); | ||
| $hierSelectElements[1]->setValue(!empty($customGroupSubTypes) ? array_filter($customGroupSubTypes) : ''); | ||
| if (!isset($caseTypeCategories[$extendsId])) { | ||
| return; | ||
| } | ||
|
|
||
| $categoryName = $caseTypeCategories[$extendsId]; | ||
| $defaults['extends'][0] = $categoryName; | ||
|
|
||
| try { | ||
| $hierSelect = $form->getElement('extends_entity_column_value'); | ||
| $extendSelect = $form->getElement('extends'); | ||
|
|
||
| if (isset($hierSelect->_attributes['data-select-params'])) { | ||
| $hierSelect->_attributes['data-select-params'] = []; | ||
| } | ||
|
|
||
| $hierSelect->setValue(''); | ||
| $extendSelect->setValue($categoryName); | ||
| } |
There was a problem hiding this comment.
The previous implementation pre‑populated the subtype selector from CustomGroupPostProcessHelper::getCustomGroupSubTypesList() so that an edited custom group retained its stored award subtypes. The new code no longer loads those values and explicitly calls $hierSelect->setValue(''), which wipes out any existing selections whenever the form is rendered. If an admin opens an award custom group and saves it without manually reselecting all subtypes, the record will be saved with an empty subtype list and the mapping to case types will be lost. This is a regression that causes unintentional data loss for every edit of an award custom group.
Useful? React with 👍 / 👎.
olayiwola-compucorp
force-pushed
the
CIVIMM-396-customfield
branch
from
9bce376 to
eafa4a9
Compare
Overview
This PR fixes an issue in the
ApplicantManagementCustomGroupPostProcessorservice where case type categories were being matched incorrectly when processing custom groups linked to awards.Previously, the logic relied on the
extendsproperty of the custom group, which does not correspond accurately to the case category name. This caused incorrect mapping of custom groups to case types or failures when saving category-specific custom data.Before
After creating an Award custom data, and a user tried to edit it, it was wrongly linked to event data instead of the awards
After
The user can edit the Awards custom data after being created
Technical Details
$customGroup->extendsto reference the case type category; this resulted in mismatches when$extendsheld a generic value (e.g.,"Case") or when the actual category was determined by theextends_entity_column_id, causing the post-processing step could attempt to access undefined category indexes in$caseTypeCategories, or merge data into incorrect categories.The code now determines the correct case type category using
$caseTypeinstead of$customGroup->extendswhen retrieving the category from$this->caseTypeCategories; this ensures custom groups are properly associated with their intended case category and avoids undefined index errors.