fix: capture assignments when adding policies to backup sets

- Add includeAssignments parameter to addPoliciesToSet method - Add 'Include Assignments' checkbox to UI (default: true) - Fix AssignmentFetcher to use request() instead of non-existent get() - Fix GroupResolver to use request() instead of non-existent post() - Replace GraphLogger calls with Laravel Log facade - Add tests for addPoliciesToSet with/without assignments
2025-12-22 17:17:52 +01:00 · 2025-12-22 17:17:52 +01:00 · f314703ac6
commit f314703ac6
parent 72d71765db
5 changed files with 206 additions and 21 deletions
--- a/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php
+++ b/app/Filament/Resources/BackupSetResource/RelationManagers/BackupItemsRelationManager.php
@ -71,7 +71,7 @@ public function table(Table $table): Table
            ->filters([])
            ->headerActions([
                Actions\Action::make('addPolicies')
-                    ->label('Policies hinzufügen')
+                    ->label('Add Policies')
                    ->icon('heroicon-o-plus')
                    ->form([
                        Forms\Components\Select::make('policy_ids')
@ -94,6 +94,10 @@ public function table(Table $table): Table
                                    ->orderBy('display_name')
                                    ->pluck('display_name', 'id');
                            }),
+                        Forms\Components\Checkbox::make('include_assignments')
+                            ->label('Include Assignments')
+                            ->default(true)
+                            ->helperText('Capture policy assignments and scope tags'),
                    ])
                    ->action(function (array $data, BackupService $service) {
                        if (empty($data['policy_ids'])) {
@ -114,6 +118,7 @@ public function table(Table $table): Table
                            policyIds: $data['policy_ids'],
                            actorEmail: auth()->user()?->email,
                            actorName: auth()->user()?->name,
+                            includeAssignments: $data['include_assignments'] ?? false,
                        );

                        Notification::make()
--- a/app/Services/Graph/AssignmentFetcher.php
+++ b/app/Services/Graph/AssignmentFetcher.php
@ -2,11 +2,12 @@

 namespace App\Services\Graph;

+use Illuminate\Support\Facades\Log;
+
 class AssignmentFetcher
 {
    public function __construct(
        private readonly MicrosoftGraphClient $graphClient,
-        private readonly GraphLogger $logger,
    ) {}

    /**
@ -24,7 +25,7 @@ public function fetch(string $tenantId, string $policyId): array
            $assignments = $this->fetchPrimary($tenantId, $policyId);

            if (! empty($assignments)) {
-                $this->logger->logDebug('Fetched assignments via primary endpoint', [
+                Log::debug('Fetched assignments via primary endpoint', [
                    'tenant_id' => $tenantId,
                    'policy_id' => $policyId,
                    'count' => count($assignments),
@ -34,7 +35,7 @@ public function fetch(string $tenantId, string $policyId): array
            }

            // Try fallback with $expand
-            $this->logger->logDebug('Primary endpoint returned empty, trying fallback', [
+            Log::debug('Primary endpoint returned empty, trying fallback', [
                'tenant_id' => $tenantId,
                'policy_id' => $policyId,
            ]);
@ -42,7 +43,7 @@ public function fetch(string $tenantId, string $policyId): array
            $assignments = $this->fetchWithExpand($tenantId, $policyId);

            if (! empty($assignments)) {
-                $this->logger->logDebug('Fetched assignments via fallback endpoint', [
+                Log::debug('Fetched assignments via fallback endpoint', [
                    'tenant_id' => $tenantId,
                    'policy_id' => $policyId,
                    'count' => count($assignments),
@ -52,14 +53,14 @@ public function fetch(string $tenantId, string $policyId): array
            }

            // Both methods returned empty
-            $this->logger->logDebug('No assignments found for policy', [
+            Log::debug('No assignments found for policy', [
                'tenant_id' => $tenantId,
                'policy_id' => $policyId,
            ]);

            return [];
        } catch (GraphException $e) {
-            $this->logger->logWarning('Failed to fetch assignments', [
+            Log::warning('Failed to fetch assignments', [
                'tenant_id' => $tenantId,
                'policy_id' => $policyId,
                'error' => $e->getMessage(),
@ -77,9 +78,11 @@ private function fetchPrimary(string $tenantId, string $policyId): array
    {
        $path = "/deviceManagement/configurationPolicies/{$policyId}/assignments";

-        $response = $this->graphClient->get($path, $tenantId);
+        $response = $this->graphClient->request('GET', $path, [
+            'tenant' => $tenantId,
+        ]);

-        return $response['value'] ?? [];
+        return $response->data['value'] ?? [];
    }

    /**
@ -93,9 +96,12 @@ private function fetchWithExpand(string $tenantId, string $policyId): array
            '$filter' => "id eq '{$policyId}'",
        ];

-        $response = $this->graphClient->get($path, $tenantId, $params);
+        $response = $this->graphClient->request('GET', $path, [
+            'tenant' => $tenantId,
+            'query' => $params,
+        ]);

-        $policies = $response['value'] ?? [];
+        $policies = $response->data['value'] ?? [];

        if (empty($policies)) {
            return [];
--- a/app/Services/Graph/GroupResolver.php
+++ b/app/Services/Graph/GroupResolver.php
@ -3,12 +3,12 @@
 namespace App\Services\Graph;

 use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Log;

 class GroupResolver
 {
    public function __construct(
        private readonly MicrosoftGraphClient $graphClient,
-        private readonly GraphLogger $logger,
    ) {}

    /**
@ -41,16 +41,19 @@ public function resolveGroupIds(array $groupIds, string $tenantId): array
    private function fetchAndResolveGroups(array $groupIds, string $tenantId): array
    {
        try {
-            $response = $this->graphClient->post(
+            $response = $this->graphClient->request(
+                'POST',
                '/directoryObjects/getByIds',
                [
-                    'ids' => array_values($groupIds),
-                    'types' => ['group'],
-                ],
-                $tenantId
+                    'tenant' => $tenantId,
+                    'json' => [
+                        'ids' => array_values($groupIds),
+                        'types' => ['group'],
+                    ],
+                ]
            );

-            $resolvedGroups = $response['value'] ?? [];
+            $resolvedGroups = $response->data['value'] ?? [];

            // Create result map
            $result = [];
@ -78,7 +81,7 @@ private function fetchAndResolveGroups(array $groupIds, string $tenantId): array
                }
            }

-            $this->logger->logDebug('Resolved group IDs', [
+            Log::debug('Resolved group IDs', [
                'tenant_id' => $tenantId,
                'requested' => count($groupIds),
                'resolved' => count($resolvedIds),
@ -87,7 +90,7 @@ private function fetchAndResolveGroups(array $groupIds, string $tenantId): array

            return $result;
        } catch (GraphException $e) {
-            $this->logger->logWarning('Failed to resolve group IDs', [
+            Log::warning('Failed to resolve group IDs', [
                'tenant_id' => $tenantId,
                'group_ids' => $groupIds,
                'error' => $e->getMessage(),
--- a/app/Services/Intune/BackupService.php
+++ b/app/Services/Intune/BackupService.php
@ -134,6 +134,7 @@ public function addPoliciesToSet(
        array $policyIds,
        ?string $actorEmail = null,
        ?string $actorName = null,
+        bool $includeAssignments = false,
    ): BackupSet {
        $this->assertActiveTenant($tenant);

@ -169,7 +170,7 @@ public function addPoliciesToSet(
        $itemsCreated = 0;

        foreach ($policies as $policy) {
-            [$item, $failure] = $this->snapshotPolicy($tenant, $backupSet, $policy, $actorEmail);
+            [$item, $failure] = $this->snapshotPolicy($tenant, $backupSet, $policy, $actorEmail, $includeAssignments);

            if ($failure !== null) {
                $failures[] = $failure;
--- a/tests/Feature/BackupWithAssignmentsTest.php
+++ b/tests/Feature/BackupWithAssignmentsTest.php
@ -361,3 +361,173 @@
    expect($backupItem->metadata['has_orphaned_assignments'])->toBeTrue()
        ->and($backupItem->metadata['assignment_count'])->toBe(2);
 });
+
+test('adds policies to existing backup set with assignments', function () {
+    // Create an existing backup set without assignments
+    $backupSet = BackupSet::factory()->create([
+        'tenant_id' => $this->tenant->id,
+        'name' => 'Existing Backup',
+        'status' => 'completed',
+        'item_count' => 0,
+    ]);
+
+    // Create a second policy to add
+    $secondPolicy = Policy::factory()->create([
+        'tenant_id' => $this->tenant->id,
+        'external_id' => 'policy-789',
+        'policy_type' => 'settingsCatalogPolicy',
+        'platform' => 'windows10',
+    ]);
+
+    // Mock PolicySnapshotService
+    $this->mock(PolicySnapshotService::class, function (MockInterface $mock) {
+        $mock->shouldReceive('fetch')
+            ->once()
+            ->andReturn([
+                'payload' => [
+                    'id' => 'policy-789',
+                    'name' => 'Second Policy',
+                    'roleScopeTagIds' => ['0'],
+                    'settings' => [],
+                ],
+                'metadata' => [],
+                'warnings' => [],
+            ]);
+    });
+
+    // Mock AssignmentFetcher
+    $this->mock(AssignmentFetcher::class, function (MockInterface $mock) {
+        $mock->shouldReceive('fetch')
+            ->once()
+            ->with('tenant-123', 'policy-789')
+            ->andReturn([
+                [
+                    'id' => 'assignment-3',
+                    'target' => [
+                        '@odata.type' => '#microsoft.graph.groupAssignmentTarget',
+                        'groupId' => 'group-xyz',
+                    ],
+                    'intent' => 'apply',
+                ],
+            ]);
+    });
+
+    // Mock GroupResolver
+    $this->mock(GroupResolver::class, function (MockInterface $mock) {
+        $mock->shouldReceive('resolveGroupIds')
+            ->once()
+            ->with(['group-xyz'], 'tenant-123')
+            ->andReturn([
+                'group-xyz' => [
+                    'id' => 'group-xyz',
+                    'displayName' => 'Test Group',
+                    'orphaned' => false,
+                ],
+            ]);
+    });
+
+    // Mock ScopeTagResolver
+    $this->mock(ScopeTagResolver::class, function (MockInterface $mock) {
+        $mock->shouldReceive('resolve')
+            ->once()
+            ->with(['0'], Mockery::type(Tenant::class))
+            ->andReturn([
+                ['id' => '0', 'displayName' => 'Default'],
+            ]);
+    });
+
+    /** @var BackupService $backupService */
+    $backupService = app(BackupService::class);
+
+    $updatedBackupSet = $backupService->addPoliciesToSet(
+        tenant: $this->tenant,
+        backupSet: $backupSet,
+        policyIds: [$secondPolicy->id],
+        actorEmail: $this->user->email,
+        actorName: $this->user->name,
+        includeAssignments: true
+    );
+
+    expect($updatedBackupSet->item_count)->toBe(1);
+
+    $backupItem = $updatedBackupSet->items()->first();
+
+    expect($backupItem)->toBeInstanceOf(BackupItem::class)
+        ->and($backupItem->assignments)->toBeArray()
+        ->and($backupItem->assignments)->toHaveCount(1)
+        ->and($backupItem->metadata['assignment_count'])->toBe(1)
+        ->and($backupItem->metadata['scope_tag_ids'])->toBe(['0'])
+        ->and($backupItem->metadata['scope_tag_names'])->toBe(['Default'])
+        ->and($backupItem->metadata['has_orphaned_assignments'])->toBeFalse();
+});
+
+test('adds policies to existing backup set without assignments when flag is false', function () {
+    // Create an existing backup set
+    $backupSet = BackupSet::factory()->create([
+        'tenant_id' => $this->tenant->id,
+        'name' => 'Existing Backup',
+        'status' => 'completed',
+        'item_count' => 0,
+    ]);
+
+    // Create a second policy
+    $secondPolicy = Policy::factory()->create([
+        'tenant_id' => $this->tenant->id,
+        'external_id' => 'policy-999',
+        'policy_type' => 'settingsCatalogPolicy',
+        'platform' => 'windows10',
+    ]);
+
+    // Mock PolicySnapshotService
+    $this->mock(PolicySnapshotService::class, function (MockInterface $mock) {
+        $mock->shouldReceive('fetch')
+            ->once()
+            ->andReturn([
+                'payload' => [
+                    'id' => 'policy-999',
+                    'name' => 'Third Policy',
+                    'roleScopeTagIds' => ['0'],
+                    'settings' => [],
+                ],
+                'metadata' => [],
+                'warnings' => [],
+            ]);
+    });
+
+    // AssignmentFetcher should NOT be called when includeAssignments is false
+    $this->mock(AssignmentFetcher::class, function (MockInterface $mock) {
+        $mock->shouldNotReceive('fetch');
+    });
+
+    // Mock ScopeTagResolver (still called for scope tags in policy payload)
+    $this->mock(ScopeTagResolver::class, function (MockInterface $mock) {
+        $mock->shouldReceive('resolve')
+            ->once()
+            ->with(['0'], Mockery::type(Tenant::class))
+            ->andReturn([
+                ['id' => '0', 'displayName' => 'Default'],
+            ]);
+    });
+
+    /** @var BackupService $backupService */
+    $backupService = app(BackupService::class);
+
+    $updatedBackupSet = $backupService->addPoliciesToSet(
+        tenant: $this->tenant,
+        backupSet: $backupSet,
+        policyIds: [$secondPolicy->id],
+        actorEmail: $this->user->email,
+        actorName: $this->user->name,
+        includeAssignments: false
+    );
+
+    expect($updatedBackupSet->item_count)->toBe(1);
+
+    $backupItem = $updatedBackupSet->items()->first();
+
+    expect($backupItem)->toBeInstanceOf(BackupItem::class)
+        ->and($backupItem->assignments)->toBeNull()
+        ->and($backupItem->metadata['assignment_count'])->toBe(0)
+        ->and($backupItem->metadata['scope_tag_ids'])->toBe(['0'])
+        ->and($backupItem->metadata['scope_tag_names'])->toBe(['Default']);
+});