ahmido/TenantAtlas
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects 1 Releases Wiki Activity

fix: harden SyncPoliciesJob input handling; pass supported types from TenantResource; add tests

Browse Source
This commit is contained in:
Ahmed Darrazi 2026-01-26 19:35:10 +01:00
parent eef85af990
commit dbc9f2fc0b
16 changed files with 479 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
app/Filament/Resources/PolicyResource.php
View File

@ -448,11 +448,7 @@ public static function table(Table $table): Table
return;
}
SyncPoliciesJob::dispatch(
tenantId: (int) $tenant->getKey(),
policyIds: [(int) $record->getKey()],
operationRun: $opRun
);
SyncPoliciesJob::dispatch((int) $tenant->getKey(), null, [(int) $record->getKey()], $opRun);
OpsUxBrowserEvents::dispatchRunEnqueued($livewire);
OperationUxPresenter::queuedToast((string) $opRun->type)
->actions([
@ -766,11 +762,7 @@ public static function table(Table $table): Table
return;
}
SyncPoliciesJob::dispatch(
tenantId: (int) $tenant->getKey(),
policyIds: $ids,
operationRun: $opRun
);
SyncPoliciesJob::dispatch((int) $tenant->getKey(), null, $ids, $opRun);
OpsUxBrowserEvents::dispatchRunEnqueued($livewire);
OperationUxPresenter::queuedToast((string) $opRun->type)
->actions([

6
app/Filament/Resources/PolicyResource/Pages/ListPolicies.php
View File

@ -84,11 +84,7 @@ protected function getHeaderActions(): array
}
$opService->dispatchOrFail($opRun, function () use ($tenant, $requestedTypes, $opRun): void {
SyncPoliciesJob::dispatch(
tenantId: (int) $tenant->getKey(),
types: $requestedTypes,
operationRun: $opRun
);
SyncPoliciesJob::dispatch((int) $tenant->getKey(), $requestedTypes, null, $opRun);
});
OpsUxBrowserEvents::dispatchRunEnqueued($livewire);
OperationUxPresenter::queuedToast((string) $opRun->type)

32
app/Filament/Resources/TenantResource.php
View File

@ -212,13 +212,39 @@ public static function table(Table $table): Table
// Phase 3: Canonical Operation Run Start
/** @var OperationRunService $opService */
$opService = app(OperationRunService::class);
$supportedTypes = config('tenantpilot.supported_policy_types', []);
$typeNames = array_map(
static fn (array $typeConfig): string => (string) $typeConfig['type'],
$supportedTypes,
);
sort($typeNames);
$inputs = [
'scope' => 'full',
'types' => $typeNames,
];
$opRun = $opService->ensureRun(
tenant: $record,
type: 'policy.sync',
inputs: ['scope' => 'full'],
inputs: $inputs,
initiator: auth()->user()
);
if (! $opRun->wasRecentlyCreated && $opService->isStaleQueuedRun($opRun)) {
$opService->failStaleQueuedRun(
$opRun,
message: 'Run was queued but never started (likely a previous dispatch error). Re-queuing.'
);
$opRun = $opService->ensureRun(
tenant: $record,
type: 'policy.sync',
inputs: $inputs,
initiator: auth()->user()
);
}
if (! $opRun->wasRecentlyCreated && in_array($opRun->status, ['queued', 'running'], true)) {
Notification::make()
->title('Policy sync already active')
@ -234,7 +260,9 @@ public static function table(Table $table): Table
return;
}
SyncPoliciesJob::dispatch($record->getKey(), null, $opRun);
$opService->dispatchOrFail($opRun, function () use ($record, $supportedTypes, $opRun): void {
SyncPoliciesJob::dispatch((int) $record->getKey(), $supportedTypes, null, $opRun);
});
$auditLogger->log(
tenant: $record,

71
app/Jobs/SyncPoliciesJob.php
View File

@ -6,6 +6,8 @@
use App\Models\OperationRun;
use App\Models\Policy;
use App\Models\Tenant;
use App\Services\Graph\GraphClientInterface;
use App\Services\Graph\NullGraphClient;
use App\Services\Intune\PolicySyncService;
use App\Services\OperationRunService;
use App\Support\OperationRunOutcome;
@ -23,7 +25,7 @@ class SyncPoliciesJob implements ShouldQueue
public ?OperationRun $operationRun = null;
/**
* @param array<int, string>|null $types
* @param array<int, string>|array<int, array{type: string, platform?: string|null, filter?: string|null}>|null $types
* @param array<int, int>|null $policyIds
*/
public function __construct(
@ -42,6 +44,28 @@ public function middleware(): array
public function handle(PolicySyncService $service, OperationRunService $operationRunService): void
{
$graph = app(GraphClientInterface::class);
if (! config('graph.enabled') || $graph instanceof NullGraphClient) {
if ($this->operationRun) {
$operationRunService->updateRun(
$this->operationRun,
status: OperationRunStatus::Completed->value,
outcome: OperationRunOutcome::Failed->value,
failures: [
[
'code' => 'graph.disabled',
'message' => 'Microsoft Graph is not enabled. Set GRAPH_ENABLED=true (and/or GRAPH_TENANT_ID) in .env to use the real Graph client.',
],
],
);
return;
}
throw new \RuntimeException('Microsoft Graph is not enabled (GRAPH_ENABLED/GRAPH_TENANT_ID missing).');
}
$tenant = Tenant::findOrFail($this->tenantId);
if ($this->policyIds !== null) {
@ -117,7 +141,50 @@ public function handle(PolicySyncService $service, OperationRunService $operatio
$supported = config('tenantpilot.supported_policy_types', []);
if ($this->types !== null) {
$supported = array_values(array_filter($supported, fn ($type) => in_array($type['type'], $this->types, true)));
$first = $this->types[0] ?? null;
$typesLookLikeSupportedConfig = is_array($first) && array_key_exists('type', $first);
if ($typesLookLikeSupportedConfig) {
$supported = array_values(array_filter(
$this->types,
static fn ($type): bool => is_array($type) && isset($type['type']) && is_string($type['type']) && $type['type'] !== ''
));
} else {
$requestedTypes = array_values(array_unique(array_filter(array_map(
static fn ($type): ?string => is_string($type) ? $type : (is_array($type) ? (string) ($type['type'] ?? '') : null),
$this->types,
), static fn ($type): bool => is_string($type) && $type !== '')));
$supported = array_values(array_filter(
$supported,
static fn ($type): bool => is_array($type)
&& isset($type['type'])
&& is_string($type['type'])
&& in_array($type['type'], $requestedTypes, true)
));
}
}
if ($supported === []) {
if ($this->operationRun) {
$operationRunService->updateRun(
$this->operationRun,
status: OperationRunStatus::Completed->value,
outcome: OperationRunOutcome::Failed->value,
failures: [
[
'code' => $this->types === null
? 'tenantpilot.supported_policy_types.empty'
: 'tenantpilot.supported_policy_types.no_match',
'message' => $this->types === null
? 'No supported policy types configured (tenantpilot.supported_policy_types is empty).'
: 'No requested policy types matched the supported policy type configuration.',
],
],
);
}
return;
}
$result = $service->syncPoliciesWithReport($tenant, $supported);

32
app/Services/OperationRunService.php
View File

@ -22,6 +22,38 @@
class OperationRunService
{
public function isStaleQueuedRun(OperationRun $run, int $thresholdMinutes = 5): bool
{
if ($run->status !== OperationRunStatus::Queued->value) {
return false;
}
if ($run->started_at !== null) {
return false;
}
if ($run->created_at === null) {
return false;
}
return $run->created_at->lte(now()->subMinutes($thresholdMinutes));
}
public function failStaleQueuedRun(OperationRun $run, string $message = 'Run was queued but never started.'): OperationRun
{
return $this->updateRun(
$run,
status: OperationRunStatus::Completed->value,
outcome: OperationRunOutcome::Failed->value,
failures: [
[
'code' => 'run.stale_queued',
'message' => $message,
],
],
);
}
public function ensureRun(
Tenant $tenant,
string $type,

13
database/seeders/DatabaseSeeder.php
View File

@ -5,6 +5,7 @@
use App\Models\User;
use Illuminate\Database\Console\Seeds\WithoutModelEvents;
use Illuminate\Database\Seeder;
use Illuminate\Support\Facades\Hash;
class DatabaseSeeder extends Seeder
{
@ -19,9 +20,13 @@ public function run(): void
PoliciesSeeder::class,
]);
User::factory()->create([
'name' => 'Test User',
'email' => 'test@example.com',
]);
User::query()->updateOrCreate(
['email' => 'test@example.com'],
[
'name' => 'Test User',
'email_verified_at' => now(),
'password' => Hash::make('password'),
],
);
}
}

5
database/seeders/PoliciesSeeder.php
View File

@ -10,10 +10,13 @@ class PoliciesSeeder extends Seeder
{
public function run(): void
{
$seedTenantId = env('INTUNE_TENANT_ID', 'local-tenant');
$tenant = Tenant::firstOrCreate([
'tenant_id' => env('INTUNE_TENANT_ID', 'local-tenant'),
'tenant_id' => $seedTenantId,
], [
'name' => 'Default Tenant',
'external_id' => $seedTenantId,
'domain' => null,
'metadata' => [],
]);

17
specs/999-seeder-external-id/plan.md Normal file
View File

@ -0,0 +1,17 @@
# Plan — 999 Fix seeding: tenants.external_id not-null
## Tech
- Laravel 12
- PostgreSQL via Sail
## Approach
1. Confirm `tenants.external_id` schema requirements in migrations.
2. Update the relevant seeder(s) so the default tenant includes `external_id`.
- Prefer generating a UUID (`Str::uuid()`) if `external_id` is meant to be globally unique.
3. Run `./vendor/bin/sail artisan migrate:fresh --seed`.
4. Add/adjust a small Pest test to prevent regression.
## Files
- `database/seeders/PoliciesSeeder.php` (likely)
- `database/seeders/DatabaseSeeder.php` (if orchestration changes needed)
- `tests/Feature/...` (new/updated regression test)

19
specs/999-seeder-external-id/spec.md Normal file
View File

@ -0,0 +1,19 @@
# 999 - Fix seeding: tenants.external_id not-null
## Problem
Running `php artisan migrate:fresh --seed` fails with:
- `SQLSTATE[23502]: Not null violation: null value in column "external_id" of relation "tenants"`
The failing insert is from `Database\Seeders\PoliciesSeeder` creating a default tenant without populating `external_id`.
## Goal
- `migrate:fresh --seed` succeeds on a clean database.
## Non-goals
- No changes to production data or existing tenant records.
## Acceptance Criteria
- Seeding succeeds with PostgreSQL.
- A default tenant is created with a valid `external_id`.
- Tests covering the seeding path (minimal) pass.

15
specs/999-seeder-external-id/tasks.md Normal file
View File

@ -0,0 +1,15 @@
# Tasks — 999 Fix seeding: tenants.external_id not-null
## Setup
- [x] T001 Verify schema requirement for `tenants.external_id` in migrations.
## Tests
- [x] T010 Add regression test ensuring `migrate:fresh --seed` (or the tenant seeder) creates a tenant with non-null `external_id`.
## Core
- [x] T020 Fix seeder to set `external_id` for the default tenant.
## Validation
- [x] T900 Run `./vendor/bin/sail artisan migrate:fresh --seed`.
- [x] T910 Run targeted Pest test(s) for the change.
- [x] T920 Run `./vendor/bin/sail bin pint --dirty`.

23
tests/Feature/Database/PoliciesSeederTest.php Normal file
View File

@ -0,0 +1,23 @@
<?php
use App\Models\Tenant;
use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
test('policies seeder creates default tenant with external_id', function () {
config()->set('tenantpilot.supported_policy_types', []);
$_ENV['INTUNE_TENANT_ID'] = 'test-tenant-id';
$_SERVER['INTUNE_TENANT_ID'] = 'test-tenant-id';
putenv('INTUNE_TENANT_ID=test-tenant-id');
$this->artisan('db:seed', ['--class' => Database\Seeders\PoliciesSeeder::class])
->assertExitCode(0);
$tenant = Tenant::query()->first();
expect($tenant)->not->toBeNull();
expect($tenant->tenant_id)->toBe('test-tenant-id');
expect($tenant->external_id)->toBe('test-tenant-id');
});

16
tests/Feature/DatabaseSeederTest.php Normal file
View File

@ -0,0 +1,16 @@
<?php
use Database\Seeders\DatabaseSeeder;
use Illuminate\Support\Facades\Auth;
it('can run the database seeder twice without duplicate user errors', function () {
$this->seed(DatabaseSeeder::class);
$this->seed(DatabaseSeeder::class);
$user = \App\Models\User::query()->where('email', 'test@example.com')->first();
expect($user)->not->toBeNull();
expect($user->email)->toBe('test@example.com');
expect(Auth::attempt(['email' => 'test@example.com', 'password' => 'password']))->toBeTrue();
});

53
tests/Feature/OperationRunServiceStaleQueuedRunTest.php Normal file
View File

@ -0,0 +1,53 @@
<?php
use App\Models\OperationRun;
use App\Services\OperationRunService;
use App\Support\OperationRunOutcome;
use App\Support\OperationRunStatus;
it('detects a stale queued run that never started', function () {
$run = OperationRun::factory()->create([
'status' => OperationRunStatus::Queued->value,
'started_at' => null,
'created_at' => now()->subMinutes(6),
]);
$service = app(OperationRunService::class);
expect($service->isStaleQueuedRun($run))->toBeTrue();
});
it('does not treat recent queued runs as stale', function () {
$run = OperationRun::factory()->create([
'status' => OperationRunStatus::Queued->value,
'started_at' => null,
'created_at' => now()->subMinute(),
]);
$service = app(OperationRunService::class);
expect($service->isStaleQueuedRun($run))->toBeFalse();
});
it('fails a stale queued run and marks it completed', function () {
$run = OperationRun::factory()->create([
'status' => OperationRunStatus::Queued->value,
'outcome' => OperationRunOutcome::Pending->value,
'started_at' => null,
'completed_at' => null,
'failure_summary' => [],
]);
$service = app(OperationRunService::class);
$updated = $service->failStaleQueuedRun($run, message: 'Stale test run');
expect($updated->status)->toBe(OperationRunStatus::Completed->value);
expect($updated->outcome)->toBe(OperationRunOutcome::Failed->value);
expect($updated->completed_at)->not->toBeNull();
$failures = is_array($updated->failure_summary ?? null) ? $updated->failure_summary : [];
expect($failures)->not->toBeEmpty();
expect($failures[0]['code'] ?? null)->toBe('run.stale_queued');
});

20
tests/Feature/SyncPoliciesJobDispatchTest.php Normal file
View File

@ -0,0 +1,20 @@
<?php
use App\Jobs\SyncPoliciesJob;
use App\Models\OperationRun;
use Illuminate\Support\Facades\Bus;
it('dispatches SyncPoliciesJob with an OperationRun as the 4th argument', function () {
Bus::fake();
$operationRun = OperationRun::factory()->create();
SyncPoliciesJob::dispatch((int) $operationRun->tenant_id, null, null, $operationRun);
Bus::assertDispatched(SyncPoliciesJob::class, function (SyncPoliciesJob $job) use ($operationRun): bool {
return $job->tenantId === (int) $operationRun->tenant_id
&& $job->types === null
&& $job->policyIds === null
&& (int) $job->operationRun?->getKey() === (int) $operationRun->getKey();
});
});

43
tests/Feature/SyncPoliciesJobGraphDisabledTest.php Normal file
View File

@ -0,0 +1,43 @@
<?php
use App\Jobs\SyncPoliciesJob;
use App\Models\OperationRun;
use App\Models\Tenant;
use App\Services\Intune\PolicySyncService;
use App\Services\OperationRunService;
use App\Support\OperationRunOutcome;
use App\Support\OperationRunStatus;
it('marks policy sync run failed when Graph is disabled', function () {
config(['graph.enabled' => false]);
$tenant = Tenant::factory()->create([
'status' => 'active',
]);
$run = OperationRun::factory()->create([
'tenant_id' => $tenant->getKey(),
'type' => 'policy.sync',
'status' => OperationRunStatus::Queued->value,
'outcome' => OperationRunOutcome::Pending->value,
]);
$job = new SyncPoliciesJob(
tenantId: (int) $tenant->getKey(),
types: null,
policyIds: null,
operationRun: $run,
);
$job->handle(app(PolicySyncService::class), app(OperationRunService::class));
$run->refresh();
expect($run->status)->toBe(OperationRunStatus::Completed->value);
expect($run->outcome)->toBe(OperationRunOutcome::Failed->value);
$failures = is_array($run->failure_summary ?? null) ? $run->failure_summary : [];
expect($failures)->not->toBeEmpty();
expect($failures[0]['code'] ?? null)->toBe('graph.disabled');
});

126
tests/Feature/SyncPoliciesJobSupportedTypesTest.php Normal file
View File

@ -0,0 +1,126 @@
<?php
use App\Jobs\SyncPoliciesJob;
use App\Models\OperationRun;
use App\Models\Tenant;
use App\Services\Graph\GraphClientInterface;
use App\Services\Graph\GraphResponse;
use App\Services\Intune\PolicySyncService;
use App\Services\OperationRunService;
use App\Support\OperationRunOutcome;
use App\Support\OperationRunStatus;
use function Pest\Laravel\mock;
class FakeGraphClient implements GraphClientInterface
{
public function listPolicies(string $policyType, array $options = []): GraphResponse
{
return new GraphResponse(true);
}
public function getPolicy(string $policyType, string $policyId, array $options = []): GraphResponse
{
return new GraphResponse(true);
}
public function getOrganization(array $options = []): GraphResponse
{
return new GraphResponse(true);
}
public function applyPolicy(string $policyType, string $policyId, array $payload, array $options = []): GraphResponse
{
return new GraphResponse(true);
}
public function getServicePrincipalPermissions(array $options = []): GraphResponse
{
return new GraphResponse(true);
}
public function request(string $method, string $path, array $options = []): GraphResponse
{
return new GraphResponse(true);
}
}
it('fails the run when supported policy types are empty', function () {
config(['graph.enabled' => true]);
config(['tenantpilot.supported_policy_types' => []]);
app()->instance(GraphClientInterface::class, new FakeGraphClient);
$tenant = Tenant::factory()->create();
$run = OperationRun::factory()->for($tenant)->create([
'type' => 'policy.sync',
'status' => OperationRunStatus::Queued->value,
'outcome' => OperationRunOutcome::Pending->value,
]);
$job = new SyncPoliciesJob(
tenantId: (int) $tenant->getKey(),
types: null,
policyIds: null,
operationRun: $run,
);
$service = mock(PolicySyncService::class);
$service->shouldNotReceive('syncPoliciesWithReport');
$job->handle($service, app(OperationRunService::class));
$run->refresh();
expect($run->status)->toBe(OperationRunStatus::Completed->value)
->and($run->outcome)->toBe(OperationRunOutcome::Failed->value)
->and($run->failure_summary)->toBeArray()
->and($run->failure_summary[0]['code'] ?? null)->toBe('tenantpilot.supported_policy_types.empty');
});
it('accepts supported policy config passed via the types argument', function () {
config(['graph.enabled' => true]);
config(['tenantpilot.supported_policy_types' => []]);
app()->instance(GraphClientInterface::class, new FakeGraphClient);
$tenant = Tenant::factory()->create();
$run = OperationRun::factory()->for($tenant)->create([
'type' => 'policy.sync',
'status' => OperationRunStatus::Queued->value,
'outcome' => OperationRunOutcome::Pending->value,
]);
$supported = [
['type' => 'deviceConfiguration', 'platform' => 'all'],
];
$job = new SyncPoliciesJob(
tenantId: (int) $tenant->getKey(),
types: $supported,
policyIds: null,
operationRun: $run,
);
$service = mock(PolicySyncService::class);
$service->shouldReceive('syncPoliciesWithReport')
->once()
->withArgs(function (Tenant $tenantArg, array $supportedArg) use ($tenant, $supported) {
expect($tenantArg->getKey())->toBe($tenant->getKey());
expect($supportedArg)->toBe($supported);
return true;
})
->andReturn([
'synced' => [101, 102, 103],
'failures' => [],
]);
$job->handle($service, app(OperationRunService::class));
$run->refresh();
expect($run->status)->toBe(OperationRunStatus::Completed->value)
->and($run->outcome)->toBe(OperationRunOutcome::Succeeded->value)
->and($run->summary_counts['succeeded'] ?? null)->toBe(3);
});