[WIP] EPIC FOUR-29110 - Cases Retention Policy#8721
[WIP] EPIC FOUR-29110 - Cases Retention Policy#8721sanjacornelius wants to merge 9 commits intodevelopfrom
Conversation
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| // Get deleted timestamp | ||
| // $deletedAt = Carbon::now(); | ||
| // RetentionPolicyLog::record($process->id, $caseIds, $deletedAt); | ||
| }); |
There was a problem hiding this comment.
Retention policy deletes CaseNumber instead of ProcessRequest
High Severity
The retention policy deletes CaseNumber records, which are just sequence generators for creating unique IDs, not the actual case data. The actual cases are stored in ProcessRequest. Deleting CaseNumber records doesn't remove any case data - it only affects the ID generation mechanism. The retention policy should target ProcessRequest records and their associated data to actually delete cases.
|
QA server K8S was successfully deployed https://ci-4c7e0ec47b.engk8s.processmaker.net |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| } | ||
|
|
||
| // Default to 6 months if retention_period is not set | ||
| $retentionPeriod = $process->properties['retention_period'] ?? '6_months'; |
There was a problem hiding this comment.
Null properties causes TypeError on array access
High Severity
The code accesses $process->properties['retention_period'] using the null coalescing operator, but if $process->properties is null (which is possible since the database column is nullable according to migrations), PHP 7.4+ throws a "Trying to access array offset on value of type null" error. The null coalescing operator only handles the case when the array key doesn't exist, not when the array itself is null. This would cause the job to fail for any process with null properties.
|
|
||
| namespace ProcessMaker\Models; | ||
|
|
||
| use Database\Factories\ProcessMaker\Models\CaseNumberFactory; |
There was a problem hiding this comment.
Unused import added to CaseNumber model
Low Severity
The import Database\Factories\ProcessMaker\Models\CaseNumberFactory is added but never referenced in the class body. Laravel's HasFactory trait auto-resolves factories based on naming conventions, making this explicit import unnecessary. Other models in the codebase (like Bookmark, Bundle, etc.) use HasFactory without importing their factory class explicitly.
2 participants
[WIP]
ci:deploy
Issue & Reproduction Steps
Describe the issue this ticket solves and describe how to reproduce the issue (please attach any fixtures used to reproduce the issue).
Solution
How to Test
Describe how to test that this solution works.
Related Tickets & Packages
Code Review Checklist
Note
High Risk
Automatically deletes case records in production on a schedule; incorrect cutoff logic or configuration could cause unintended data loss despite test coverage.
Overview
Adds a new daily
cases:retention:evaluatescheduled command that fans out per-process queue jobs to enforce a case retention policy.Introduces
EvaluateProcessRetentionJob, gated byCASE_RETENTION_POLICY_ENABLED, which deletesCaseNumberrecords in chunks based on each process’properties.retention_period(default 6 months) andproperties.retention_updated_atrules (different cutoffs for cases created before vs after the update). Adds aCaseNumberFactoryand a comprehensive job test suite covering deletes, non-deletes, batching, policy updates, disabled policy, and default behavior.Written by Cursor Bugbot for commit f3d2573. This will update automatically on new commits. Configure here.