From ec99c6519c8e33ce2d29c3b94438dd091daa40ed Mon Sep 17 00:00:00 2001 From: Ahmed Darrazi Date: Tue, 20 Jan 2026 00:26:24 +0100 Subject: [PATCH] fix: green test suite after dev merge --- app/Filament/Pages/DriftLanding.php | 2 ++ .../Resources/OperationRunResource.php | 17 +++++---- app/Filament/Resources/PolicyResource.php | 4 +-- app/Policies/OperationRunPolicy.php | 9 +++-- app/Services/Graph/MicrosoftGraphClient.php | 23 ++++++++++++ .../RunBackupScheduleJobTest.php | 2 +- tests/Feature/BulkDeletePoliciesTest.php | 35 ++++++++++++------- ...BackupScheduleOperationRunsCommandTest.php | 12 ++++--- .../RestoreExecutionOperationRunSyncTest.php | 5 --- 9 files changed, 75 insertions(+), 34 deletions(-) diff --git a/app/Filament/Pages/DriftLanding.php b/app/Filament/Pages/DriftLanding.php index ef5acb5..7c9c3fa 100644 --- a/app/Filament/Pages/DriftLanding.php +++ b/app/Filament/Pages/DriftLanding.php @@ -17,6 +17,8 @@ use App\Support\OpsUx\OperationUxPresenter; use App\Support\OpsUx\OpsUxBrowserEvents; use BackedEnum; +use Filament\Actions\Action; +use Filament\Notifications\Notification; use Filament\Pages\Page; use UnitEnum; diff --git a/app/Filament/Resources/OperationRunResource.php b/app/Filament/Resources/OperationRunResource.php index f3f7eb0..91e8a08 100644 --- a/app/Filament/Resources/OperationRunResource.php +++ b/app/Filament/Resources/OperationRunResource.php @@ -37,6 +37,16 @@ class OperationRunResource extends Resource protected static ?string $navigationLabel = 'Operations'; + public static function getEloquentQuery(): Builder + { + $tenantId = Tenant::current()?->getKey(); + + return parent::getEloquentQuery() + ->with('user') + ->latest('id') + ->when($tenantId, fn (Builder $query) => $query->where('tenant_id', $tenantId)); + } + public static function form(Schema $schema): Schema { return $schema; @@ -243,13 +253,6 @@ public static function table(Table $table): Table ->bulkActions([]); } - public static function getEloquentQuery(): Builder - { - return parent::getEloquentQuery() - ->with('user') - ->latest('id'); - } - public static function getPages(): array { return [ diff --git a/app/Filament/Resources/PolicyResource.php b/app/Filament/Resources/PolicyResource.php index 6891653..4a2ef0a 100644 --- a/app/Filament/Resources/PolicyResource.php +++ b/app/Filament/Resources/PolicyResource.php @@ -391,7 +391,7 @@ public static function table(Table $table): Table return $user->canSyncTenant($tenant); }) - ->action(function (Policy $record) { + ->action(function (Policy $record, HasTable $livewire): void { $tenant = Tenant::current(); $user = auth()->user(); @@ -700,7 +700,7 @@ public static function table(Table $table): Table return $value === 'ignored'; }) - ->action(function (Collection $records) { + ->action(function (Collection $records, HasTable $livewire): void { $tenant = Tenant::current(); $user = auth()->user(); $count = $records->count(); diff --git a/app/Policies/OperationRunPolicy.php b/app/Policies/OperationRunPolicy.php index 0e1e400..aa2c3ae 100644 --- a/app/Policies/OperationRunPolicy.php +++ b/app/Policies/OperationRunPolicy.php @@ -6,6 +6,7 @@ use App\Models\Tenant; use App\Models\User; use Illuminate\Auth\Access\HandlesAuthorization; +use Illuminate\Auth\Access\Response; class OperationRunPolicy { @@ -22,7 +23,7 @@ public function viewAny(User $user): bool return $user->canAccessTenant($tenant); } - public function view(User $user, OperationRun $run): bool + public function view(User $user, OperationRun $run): Response|bool { $tenant = Tenant::current(); @@ -34,6 +35,10 @@ public function view(User $user, OperationRun $run): bool return false; } - return (int) $run->tenant_id === (int) $tenant->getKey(); + if ((int) $run->tenant_id !== (int) $tenant->getKey()) { + return Response::denyAsNotFound(); + } + + return true; } } diff --git a/app/Services/Graph/MicrosoftGraphClient.php b/app/Services/Graph/MicrosoftGraphClient.php index 78ffad5..66f9238 100644 --- a/app/Services/Graph/MicrosoftGraphClient.php +++ b/app/Services/Graph/MicrosoftGraphClient.php @@ -90,6 +90,29 @@ public function listPolicies(string $policyType, array $options = []): GraphResp $response = $this->send('GET', $endpoint, $sendOptions, $context); + // Some tenants intermittently return OData select parsing errors. + // Retry once with the same $select before applying the compatibility fallback. + if ($response->failed()) { + $graphResponse = $this->toGraphResponse( + action: 'list_policies', + response: $response, + transform: fn (array $json) => $json['value'] ?? (is_array($json) ? $json : []), + meta: [ + 'tenant' => $context['tenant'] ?? null, + 'path' => $endpoint, + 'full_path' => $fullPath, + 'method' => 'GET', + 'query' => $query ?: null, + 'client_request_id' => $clientRequestId, + ], + warnings: $warnings, + ); + + if ($this->shouldApplySelectFallback($graphResponse, $query)) { + $response = $this->send('GET', $endpoint, $sendOptions, $context); + } + } + if ($response->failed()) { $graphResponse = $this->toGraphResponse( action: 'list_policies', diff --git a/tests/Feature/BackupScheduling/RunBackupScheduleJobTest.php b/tests/Feature/BackupScheduling/RunBackupScheduleJobTest.php index 55d609b..f0b43de 100644 --- a/tests/Feature/BackupScheduling/RunBackupScheduleJobTest.php +++ b/tests/Feature/BackupScheduling/RunBackupScheduleJobTest.php @@ -156,7 +156,7 @@ public function createBackupSet($tenant, $policyIds, ?string $actorEmail = null, expect($operationRun->status)->toBe('completed'); expect($operationRun->outcome)->toBe('failed'); expect($operationRun->failure_summary)->toMatchArray([ - ['code' => 'unknown_policy_type', 'message' => $run->error_message], + ['code' => 'unknown_policy_type', 'message' => $run->error_message, 'reason_code' => 'unknown_error'], ]); }); diff --git a/tests/Feature/BulkDeletePoliciesTest.php b/tests/Feature/BulkDeletePoliciesTest.php index 9e49b4c..97e0758 100644 --- a/tests/Feature/BulkDeletePoliciesTest.php +++ b/tests/Feature/BulkDeletePoliciesTest.php @@ -6,6 +6,7 @@ use App\Models\Tenant; use App\Models\User; use App\Services\OperationRunService; +use App\Services\Operations\BulkSelectionIdentity; use Illuminate\Foundation\Testing\RefreshDatabase; uses(RefreshDatabase::class); @@ -19,22 +20,29 @@ /** @var OperationRunService $service */ $service = app(OperationRunService::class); - $opRun = $service->ensureRun( + /** @var BulkSelectionIdentity $selection */ + $selection = app(BulkSelectionIdentity::class); + + $selectionIdentity = $selection->fromIds($policyIds); + + $opRun = $service->enqueueBulkOperation( tenant: $tenant, type: 'policy.delete', - inputs: [ - 'scope' => 'subset', - 'policy_ids' => $policyIds, + targetScope: [ + 'entra_tenant_id' => (string) ($tenant->tenant_id ?? $tenant->external_id ?? $tenant->getKey()), ], + selectionIdentity: $selectionIdentity, + dispatcher: function ($operationRun) use ($tenant, $user, $policyIds): void { + // Simulate sync execution (workers will run immediately on sync queue) + BulkPolicyDeleteJob::dispatchSync( + tenantId: (int) $tenant->getKey(), + userId: (int) $user->getKey(), + policyIds: $policyIds, + operationRun: $operationRun, + ); + }, initiator: $user, - ); - - // Simulate Sync execution (workers will run immediately on sync queue) - BulkPolicyDeleteJob::dispatchSync( - tenantId: (int) $tenant->getKey(), - userId: (int) $user->getKey(), - policyIds: $policyIds, - operationRun: $opRun, + emitQueuedNotification: false, ); $opRun->refresh(); @@ -45,9 +53,10 @@ 'total' => 10, 'processed' => 10, 'succeeded' => 10, - 'failed' => 0, ]); + expect(($opRun->summary_counts['failed'] ?? 0))->toBe(0); + $policies->each(function ($policy) { expect($policy->refresh()->ignored_at)->not->toBeNull(); }); diff --git a/tests/Feature/Console/ReconcileBackupScheduleOperationRunsCommandTest.php b/tests/Feature/Console/ReconcileBackupScheduleOperationRunsCommandTest.php index 105ab0c..c9946fd 100644 --- a/tests/Feature/Console/ReconcileBackupScheduleOperationRunsCommandTest.php +++ b/tests/Feature/Console/ReconcileBackupScheduleOperationRunsCommandTest.php @@ -78,11 +78,15 @@ expect($operationRun->started_at?->format('Y-m-d H:i:s'))->toBe($startedAt->format('Y-m-d H:i:s')); expect($operationRun->completed_at?->format('Y-m-d H:i:s'))->toBe($finishedAt->format('Y-m-d H:i:s')); - expect($operationRun->summary_counts)->toMatchArray([ + expect($operationRun->context)->toMatchArray([ 'backup_schedule_id' => (int) $schedule->id, 'backup_schedule_run_id' => (int) $scheduleRun->id, - 'policies_total' => 5, - 'policies_backed_up' => 18, - 'sync_failures' => 0, + ]); + + expect($operationRun->summary_counts)->toMatchArray([ + 'total' => 5, + 'processed' => 5, + 'succeeded' => 18, + 'items' => 5, ]); }); diff --git a/tests/Feature/OpsUx/RestoreExecutionOperationRunSyncTest.php b/tests/Feature/OpsUx/RestoreExecutionOperationRunSyncTest.php index 622e503..06ea62e 100644 --- a/tests/Feature/OpsUx/RestoreExecutionOperationRunSyncTest.php +++ b/tests/Feature/OpsUx/RestoreExecutionOperationRunSyncTest.php @@ -5,7 +5,6 @@ use App\Jobs\ExecuteRestoreRunJob; use App\Models\OperationRun; use App\Models\RestoreRun; -use App\Services\BulkOperationService; use App\Services\Intune\AuditLogger; use App\Services\Intune\RestoreService; @@ -35,9 +34,6 @@ expect($operationRun)->not->toBeNull(); expect($operationRun?->status)->toBe('queued'); - $this->mock(BulkOperationService::class, function ($mock): void { - $mock->shouldReceive('sanitizeFailureReason')->andReturnUsing(fn (string $message): string => $message); - }); // Simulate downstream code updating RestoreRun status via query builder (no model events). $this->mock(RestoreService::class, function ($mock) use ($restoreRun): void { $mock->shouldReceive('executeForRun') @@ -56,7 +52,6 @@ $job->handle( app(RestoreService::class), app(AuditLogger::class), - app(BulkOperationService::class), ); $operationRun = $operationRun?->fresh();