Skip to content

Collection camp code bug #1594

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.

Sign up for GitHub

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

Open
tarunnjoshi wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Collection camp code bug #1594

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 14 additions & 19 deletions wp-content/civi-extensions/goonjcustom/Civi/Traits/CollectionSource.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,21 @@ public static function generateCollectionSourceCode(string $op, string $objectNa
if (!$statusDetails) {
return;
}
error_log('Institution CC code generation working');
\Civi::log()->debug('generateCollectionSourceCode code is running');

$newStatus = $statusDetails['newStatus'];
$currentStatus = $statusDetails['currentStatus'];

if ($currentStatus !== $newStatus) {
if ($newStatus === 'authorized') {
$subtypeId = $objectRef['subtype'] ?? NULL;
error_log('subtype:' . print_r($subtypeId, TRUE));
\Civi::log()->debug('[CodeGen] subtypeId', ['subtypeId' => $subtypeId]);
if (!$subtypeId) {
return;
}

$sourceId = $objectRef['id'] ?? NULL;
error_log('sourceId:' . print_r($sourceId, TRUE));
\Civi::log()->debug('[CodeGen] sourceId', ['sourceId' => $sourceId]);
if (!$sourceId) {
return;
}
Expand All @@ -121,52 +121,47 @@ public static function generateCollectionSourceCode(string $op, string $objectNa
->execute()->single();

$collectionSourceCreatedDate = $collectionSource['created_date'] ?? NULL;
error_log('collectionSourceCreatedDate:' . print_r($collectionSourceCreatedDate, TRUE));
\Civi::log()->debug('[CodeGen] createdDate', ['createdDate' => $collectionSourceCreatedDate]);

$sourceTitle = $collectionSource['title'] ?? NULL;

$year = date('Y', strtotime($collectionSourceCreatedDate));

$stateId = self::getStateIdForSourceType($objectRef, $subtypeId, $sourceTitle);
error_log('stateId:' . print_r($stateId, TRUE));
\Civi::log()->debug('[CodeGen] stateId', ['stateId' => $stateId]);

if (!$stateId) {
\Civi::log()->debug('[CodeGen] Aborting: stateId missing', ['sourceId' => $sourceId]);
return;
}

$stateProvince = StateProvince::get(FALSE)
->addWhere('id', '=', $stateId)
->execute()->single();
error_log('stateProvince:' . print_r($stateProvince, TRUE));

if (empty($stateProvince)) {
\Civi::log()->debug('[CodeGen] Aborting: stateProvince not found', ['stateId' => $stateId]);
return;
}

$stateAbbreviation = $stateProvince['abbreviation'] ?? NULL;
if (!$stateAbbreviation) {
\Civi::log()->debug('[CodeGen] Aborting: stateAbbreviation missing', ['stateId' => $stateId]);
return;
}

// Fetch the Goonj-specific state code.
$config = self::getConfig();
$stateCode = $config['state_codes'][$stateAbbreviation] ?? 'UNKNOWN';
// Get the current event title.
$currentTitle = $sourceTitle ?? 'Collection Camp';
$sourceTypeName = self::getSourceTypeName($subtypeId);
$sourceTypeTitle = str_replace('_', ' ', $sourceTypeName);
$eventCode = $config['event_codes'][$sourceTypeTitle] ?? 'UNKNOWN';
\Civi::log()->debug('[CodeGen] eventCode', ['sourceTypeName' => $sourceTypeName, 'sourceTypeTitle' => $sourceTypeTitle, 'eventCode' => $eventCode]);
Comment on lines +155 to +158
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid silently generating event codes with UNKNOWN.

If subtype-to-event mapping fails, the code still proceeds and produces a title with UNKNOWN, which can corrupt identifier semantics downstream. Abort generation when mapping is missing.

Suggested hardening 
-        $sourceTypeName = self::getSourceTypeName($subtypeId);
-        $sourceTypeTitle = str_replace('_', ' ', $sourceTypeName);
-        $eventCode = $config['event_codes'][$sourceTypeTitle] ?? 'UNKNOWN';
-        \Civi::log()->debug('[CodeGen] eventCode', ['sourceTypeName' => $sourceTypeName, 'sourceTypeTitle' => $sourceTypeTitle, 'eventCode' => $eventCode]);
+        $sourceTypeName = self::getSourceTypeName((string) $subtypeId);
+        if (!$sourceTypeName) {
+          \Civi::log()->debug('[CodeGen] Aborting: sourceTypeName missing', ['subtypeId' => $subtypeId]);
+          return;
+        }
+        $sourceTypeTitle = str_replace('_', ' ', $sourceTypeName);
+        $eventCode = $config['event_codes'][$sourceTypeTitle] ?? NULL;
+        if (!$eventCode) {
+          \Civi::log()->debug('[CodeGen] Aborting: eventCode mapping missing', ['sourceTypeTitle' => $sourceTypeTitle]);
+          return;
+        }
+        \Civi::log()->debug('[CodeGen] eventCode', ['sourceTypeName' => $sourceTypeName, 'sourceTypeTitle' => $sourceTypeTitle, 'eventCode' => $eventCode]);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@wp-content/civi-extensions/goonjcustom/Civi/Traits/CollectionSource.php`
around lines 155 - 158, The mapping lookup for event codes can return 'UNKNOWN'
and the code continues, which can corrupt downstream identifiers; in the block
using getSourceTypeName($subtypeId) and $config['event_codes'], validate that
$config['event_codes'][$sourceTypeTitle] exists and is not empty before
proceeding—if missing, abort generation by throwing an exception or returning an
error (do not silently set $eventCode = 'UNKNOWN'), and log a clear error
including $sourceTypeName/$sourceTypeTitle to help debugging.


// Fetch the event code.
$eventCode = $config['event_codes'][$currentTitle] ?? 'UNKNOWN';

// Modify the title to include the year, state code, event code, and camp Id.
$newTitle = $year . '/' . $stateCode . '/' . $eventCode . '/' . $sourceId;
error_log('newTitle:' . print_r($newTitle, TRUE));
\Civi::log()->info('[CodeGen] newTitle', ['newTitle' => $newTitle]);
$objectRef['title'] = $newTitle;

// Save the updated title back to the Collection Camp entity.
EckEntity::update('Collection_Camp')
->addWhere('id', '=', $sourceId)
->addValue('title', $newTitle)
->execute();
\Civi::log()->info('[CodeGen] Camp code generated', ['sourceId' => $sourceId, 'newTitle' => $newTitle]);
Comment on lines 160 to +164
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Generated title is not persisted in an after-write hook.

Line 162 only mutates $objectRef['title']. Since this hook runs after DB write, the generated code is unlikely to be saved to Collection_Camp.title unless explicitly updated.

Suggested persistence fix 
         $newTitle = $year . '/' . $stateCode . '/' . $eventCode . '/' . $sourceId;
         \Civi::log()->info('[CodeGen] newTitle', ['newTitle' => $newTitle]);
         $objectRef['title'] = $newTitle;
+        if (($sourceTitle ?? NULL) !== $newTitle) {
+          EckEntity::update('Collection_Camp', FALSE)
+            ->addValue('title', $newTitle)
+            ->addWhere('id', '=', $sourceId)
+            ->execute();
+        }
 
         \Civi::log()->info('[CodeGen] Camp code generated', ['sourceId' => $sourceId, 'newTitle' => $newTitle]);
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@wp-content/civi-extensions/goonjcustom/Civi/Traits/CollectionSource.php`
around lines 160 - 164, The generated title stored into $objectRef['title'] in
the CollectionSource trait is only an in-memory change for the after-write hook
and is not persisted; update the DB explicitly after generating $newTitle (use
the record id from $objectRef['id']) so Collection_Camp.title is saved (e.g.,
call the appropriate API/DAO update for the Collection_Camp entity rather than
just mutating $objectRef), ensure you check the id exists and log/report any
update errors alongside the existing \Civi::log() calls.

}
}
}
Expand Down
Loading