From 058724c3593495c2609f375849c9a88aad8ed372 Mon Sep 17 00:00:00 2001
From: Ahmed Darrazi <ahmeddarrazi@adsmac.local>
Date: Thu, 1 Jan 2026 13:07:06 +0100
Subject: [PATCH] fix: avoid assignment expand fallback

---
 app/Services/Graph/AssignmentFetcher.php | 28 ++++++++-----
 tests/Unit/AssignmentFetcherTest.php     | 50 ++++++++++++++----------
 2 files changed, 49 insertions(+), 29 deletions(-)

diff --git a/app/Services/Graph/AssignmentFetcher.php b/app/Services/Graph/AssignmentFetcher.php
index 6bd2139..29aa0b5 100644
--- a/app/Services/Graph/AssignmentFetcher.php
+++ b/app/Services/Graph/AssignmentFetcher.php
@@ -39,7 +39,7 @@ public function fetch(
 
         $primaryException = null;
         $assignments = [];
-        $primarySucceeded = false;
+        $lastSuccessfulAssignments = null;
 
         // Try primary endpoint(s)
         $listPathTemplates = [];
@@ -65,7 +65,12 @@ public function fetch(
                     $context,
                     $throwOnFailure
                 );
-                $primarySucceeded = true;
+
+                if ($assignments === null) {
+                    continue;
+                }
+
+                $lastSuccessfulAssignments = $assignments;
 
                 if (! empty($assignments)) {
                     Log::debug('Fetched assignments via primary endpoint', [
@@ -77,20 +82,25 @@ public function fetch(
 
                     return $assignments;
                 }
+
+                if ($policyType !== 'appProtectionPolicy') {
+                    // Empty is a valid outcome (policy not assigned). Do not attempt fallback.
+                    return [];
+                }
             } catch (GraphException $e) {
                 $primaryException = $primaryException ?? $e;
             }
         }
 
-        if ($primarySucceeded && $policyType === 'appProtectionPolicy') {
+        if ($lastSuccessfulAssignments !== null && $policyType === 'appProtectionPolicy') {
             Log::debug('Assignments fetched via primary endpoint(s)', [
                 'tenant_id' => $tenantId,
                 'policy_type' => $policyType,
                 'policy_id' => $policyId,
-                'count' => count($assignments),
+                'count' => count($lastSuccessfulAssignments),
             ]);
 
-            return $assignments;
+            return $lastSuccessfulAssignments;
         }
 
         // Try fallback with $expand
@@ -215,15 +225,15 @@ private function fetchPrimary(
         array $options,
         array $context,
         bool $throwOnFailure
-    ): array {
+    ): ?array {
         if (! is_string($listPathTemplate) || $listPathTemplate === '') {
-            return [];
+            return null;
         }
 
         $path = $this->resolvePath($listPathTemplate, $policyId);
 
         if ($path === null) {
-            return [];
+            return null;
         }
 
         $response = $this->graphClient->request('GET', $path, $options);
@@ -239,7 +249,7 @@ private function fetchPrimary(
                 );
             }
 
-            return [];
+            return null;
         }
 
         return $response->data['value'] ?? [];
diff --git a/tests/Unit/AssignmentFetcherTest.php b/tests/Unit/AssignmentFetcherTest.php
index ce174ee..b634752 100644
--- a/tests/Unit/AssignmentFetcherTest.php
+++ b/tests/Unit/AssignmentFetcherTest.php
@@ -75,15 +75,11 @@
     expect($result)->toBe($assignments);
 });
 
-test('fallback on empty response', function () {
+test('does not use fallback when primary succeeds with empty assignments', function () {
     $tenantId = 'tenant-123';
     $policyId = 'policy-456';
     $policyType = 'settingsCatalogPolicy';
-    $assignments = [
-        ['id' => 'assign-1', 'target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-1']],
-    ];
 
-    // Primary returns empty
     $primaryResponse = new GraphResponse(
         success: true,
         data: ['value' => []]
@@ -97,7 +93,34 @@
         ])
         ->andReturn($primaryResponse);
 
-    // Fallback returns assignments
+    $result = $this->fetcher->fetch($policyType, $tenantId, $policyId);
+
+    expect($result)->toBe([]);
+});
+
+test('uses fallback when primary endpoint fails', function () {
+    $tenantId = 'tenant-123';
+    $policyId = 'policy-456';
+    $policyType = 'settingsCatalogPolicy';
+    $assignments = [
+        ['id' => 'assign-1', 'target' => ['@odata.type' => '#microsoft.graph.groupAssignmentTarget', 'groupId' => 'group-1']],
+    ];
+
+    $primaryFailure = new GraphResponse(
+        success: false,
+        data: [],
+        status: 400,
+        errors: [['message' => 'Bad Request']]
+    );
+
+    $this->graphClient
+        ->shouldReceive('request')
+        ->once()
+        ->with('GET', "/deviceManagement/configurationPolicies/{$policyId}/assignments", [
+            'tenant' => $tenantId,
+        ])
+        ->andReturn($primaryFailure);
+
     $fallbackResponse = new GraphResponse(
         success: true,
         data: ['value' => [['id' => $policyId, 'assignments' => $assignments]]]
@@ -152,18 +175,6 @@
         ->with('GET', "/deviceManagement/configurationPolicies/{$policyId}/assignments", Mockery::any())
         ->andReturn($primaryResponse);
 
-    // Fallback returns empty
-    $fallbackResponse = new GraphResponse(
-        success: true,
-        data: ['value' => []]
-    );
-
-    $this->graphClient
-        ->shouldReceive('request')
-        ->once()
-        ->with('GET', 'deviceManagement/configurationPolicies', Mockery::any())
-        ->andReturn($fallbackResponse);
-
     $result = $this->fetcher->fetch($policyType, $tenantId, $policyId);
 
     expect($result)->toBe([]);
@@ -174,9 +185,8 @@
     $policyId = 'policy-456';
     $policyType = 'settingsCatalogPolicy';
 
-    // Primary returns empty
     $primaryResponse = new GraphResponse(
-        success: true,
+        success: false,
         data: ['value' => []]
     );