-
Notifications
You must be signed in to change notification settings - Fork 243
[WIP] EPIC FOUR-29110 - Cases Retention Policy #8721
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
base: develop
Are you sure you want to change the base?
Changes from all commits
816505b
4a87002
5875f22
98ab8d8
9db1e9a
b1e3c39
082163c
eca7b57
f3d2573
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| <?php | ||
|
|
||
| namespace ProcessMaker\Console\Commands; | ||
|
|
||
| use Illuminate\Console\Command; | ||
| use ProcessMaker\Jobs\EvaluateProcessRetentionJob; | ||
| use ProcessMaker\Models\Process; | ||
|
|
||
| class EvaluateCaseRetention extends Command | ||
| { | ||
| /** | ||
| * The name and signature of the console command. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $signature = 'cases:retention:evaluate'; | ||
|
|
||
| /** | ||
| * The console command description. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $description = 'Evaluate and delete cases past their retention period'; | ||
|
|
||
| /** | ||
| * Execute the console command. | ||
| */ | ||
| public function handle() | ||
| { | ||
| $this->info('Evaluating and deleting cases past their retention period'); | ||
|
|
||
| // Process all processes when retention policy is enabled | ||
| // Processes without retention_period will default to 6 months | ||
| Process::chunkById(100, function ($processes) { | ||
| foreach ($processes as $process) { | ||
| dispatch(new EvaluateProcessRetentionJob($process->id)); | ||
| } | ||
| }); | ||
|
|
||
| $this->info('Cases retention evaluation complete'); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| <?php | ||
|
|
||
| namespace ProcessMaker\Jobs; | ||
|
|
||
| use Carbon\Carbon; | ||
| use Illuminate\Contracts\Queue\ShouldQueue; | ||
| use Illuminate\Foundation\Queue\Queueable; | ||
| use Illuminate\Support\Facades\Log; | ||
| use ProcessMaker\Models\CaseNumber; | ||
| use ProcessMaker\Models\Process; | ||
| use ProcessMaker\Models\ProcessRequest; | ||
|
|
||
| class EvaluateProcessRetentionJob implements ShouldQueue | ||
| { | ||
| use Queueable; | ||
|
|
||
| /** | ||
| * Create a new job instance. | ||
| */ | ||
| public function __construct(public int $processId) | ||
| { | ||
| } | ||
|
|
||
| /** | ||
| * Execute the job. | ||
| */ | ||
| public function handle(): void | ||
| { | ||
| // Only run if case retention policy is enabled | ||
| // Use getenv() to read directly from environment (works better in tests) | ||
| $enabled = getenv('CASE_RETENTION_POLICY_ENABLED'); | ||
| if ($enabled === false || $enabled === 'false' || $enabled === '0' || $enabled === '') { | ||
| return; | ||
| } | ||
|
|
||
| $process = Process::find($this->processId); | ||
| if (!$process) { | ||
| Log::error('CaseRetentionJob: Process not found', ['process_id' => $this->processId]); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| // Default to 6 months if retention_period is not set | ||
| $retentionPeriod = $process->properties['retention_period'] ?? '6_months'; | ||
| $retentionMonths = match ($retentionPeriod) { | ||
| '6_months' => 6, | ||
| '1_year' => 12, | ||
| '3_years' => 36, | ||
| '5_years' => 60, | ||
| default => 6, // Default to 6 months | ||
| }; | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Default retention_updated_at to now if not set | ||
| // This means the retention policy applies from now for processes without explicit retention settings | ||
| $retentionUpdatedAt = isset($process->properties['retention_updated_at']) | ||
| ? Carbon::parse($process->properties['retention_updated_at']) | ||
| : Carbon::now(); | ||
|
|
||
| // Get all process request IDs for this process | ||
| $processRequestIds = ProcessRequest::where('process_id', $this->processId)->pluck('id'); | ||
|
|
||
| // If there are no process requests, nothing to delete | ||
| if ($processRequestIds->isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| // Handle two scenarios: | ||
| // 1. Cases created BEFORE retention_updated_at: Delete if older than retention period from retention_updated_at | ||
| // (These cases were subject to the old retention policy, but we apply current retention from update date) | ||
| // 2. Cases created AFTER retention_updated_at: Delete if older than retention period from their creation date | ||
| // (These cases are subject to the new retention policy) | ||
|
|
||
| $now = Carbon::now(); | ||
|
|
||
| // For cases created before retention_updated_at: cutoff is retention_updated_at - retention_period | ||
| $oldCasesCutoff = $retentionUpdatedAt->copy()->subMonths($retentionMonths); | ||
|
|
||
| // For cases created after retention_updated_at: cutoff is now - retention_period | ||
| $newCasesCutoff = $now->copy()->subMonths($retentionMonths); | ||
|
|
||
| CaseNumber::whereIn('process_request_id', $processRequestIds) | ||
| ->where(function ($query) use ($retentionUpdatedAt, $oldCasesCutoff, $newCasesCutoff) { | ||
| // Cases created before retention_updated_at: delete if created before (retention_updated_at - retention_period) | ||
| $query->where(function ($q) use ($retentionUpdatedAt, $oldCasesCutoff) { | ||
| $q->where('created_at', '<', $retentionUpdatedAt) | ||
| ->where('created_at', '<', $oldCasesCutoff); | ||
| }) | ||
| // Cases created after retention_updated_at: delete if created before (now - retention_period) | ||
| ->orWhere(function ($q) use ($retentionUpdatedAt, $newCasesCutoff) { | ||
| $q->where('created_at', '>=', $retentionUpdatedAt) | ||
| ->where('created_at', '<', $newCasesCutoff); | ||
| }); | ||
| }) | ||
| ->chunkById(100, function ($cases) { | ||
| $caseIds = $cases->pluck('id'); | ||
| // Delete the cases | ||
| CaseNumber::whereIn('id', $caseIds)->delete(); | ||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // TODO: Add logs to track the number of cases deleted | ||
| // Get deleted timestamp | ||
| // $deletedAt = Carbon::now(); | ||
| // RetentionPolicyLog::record($process->id, $caseIds, $deletedAt); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Retention policy deletes CaseNumber instead of ProcessRequestHigh Severity The retention policy deletes |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| namespace ProcessMaker\Models; | ||
|
|
||
| use Database\Factories\ProcessMaker\Models\CaseNumberFactory; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unused import added to CaseNumber modelLow Severity The import |
||
| use Illuminate\Database\Eloquent\Factories\HasFactory; | ||
| use Illuminate\Database\Eloquent\Model; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| <?php | ||
|
|
||
| namespace Database\Factories\ProcessMaker\Models; | ||
|
|
||
| use Illuminate\Database\Eloquent\Factories\Factory; | ||
| use ProcessMaker\Models\CaseNumber; | ||
| use ProcessMaker\Models\ProcessRequest; | ||
|
|
||
| class CaseNumberFactory extends Factory | ||
| { | ||
| protected $model = CaseNumber::class; | ||
|
|
||
| public function definition(): array | ||
| { | ||
| return [ | ||
| 'process_request_id' => function () { | ||
| return ProcessRequest::factory()->create()->getKey(); | ||
| }, | ||
| ]; | ||
| } | ||
| } |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Null properties causes TypeError on array access
High Severity
The code accesses
$process->properties['retention_period']using the null coalescing operator, but if$process->propertiesisnull(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 nullproperties.