diff --git a/Agents.md b/Agents.md index 5b0f824..16f2830 100644 --- a/Agents.md +++ b/Agents.md @@ -365,6 +365,7 @@ ## Conventions - You must follow all existing code conventions used in this application. When creating or editing a file, check sibling files for the correct structure, approach, naming. - Use descriptive names for variables and methods. For example, `isRegisteredForDiscounts`, not `discount()`. - Check for existing components to reuse before writing a new one. +- UI consistency: Prefer Filament components (``, infolist/table entries, etc.) over custom HTML/Tailwind for admin UI; only roll custom markup when Filament cannot express the UI. ## Verification Scripts - Do not create verification scripts or tinker when tests cover that functionality and prove it works. Unit and feature tests are more important. diff --git a/app/Filament/Resources/PolicyVersionResource.php b/app/Filament/Resources/PolicyVersionResource.php index 89cce02..bb3e19d 100644 --- a/app/Filament/Resources/PolicyVersionResource.php +++ b/app/Filament/Resources/PolicyVersionResource.php @@ -55,8 +55,9 @@ public static function infolist(Schema $schema): Schema ->columnSpanFull() ->tabs([ Tab::make('Normalized settings') + ->id('normalized-settings') ->schema([ - Infolists\Components\ViewEntry::make('normalized_settings') + Infolists\Components\ViewEntry::make('normalized_settings_catalog') ->view('filament.infolists.entries.normalized-settings') ->state(function (PolicyVersion $record) { $normalized = app(PolicyNormalizer::class)->normalize( @@ -69,15 +70,34 @@ public static function infolist(Schema $schema): Schema $normalized['record_id'] = (string) $record->getKey(); return $normalized; - }), + }) + ->visible(fn (PolicyVersion $record) => ($record->policy_type ?? '') === 'settingsCatalogPolicy'), + + Infolists\Components\ViewEntry::make('normalized_settings_standard') + ->view('filament.infolists.entries.policy-settings-standard') + ->state(function (PolicyVersion $record) { + $normalized = app(PolicyNormalizer::class)->normalize( + is_array($record->snapshot) ? $record->snapshot : [], + $record->policy_type ?? '', + $record->platform + ); + + $normalized['context'] = 'version'; + $normalized['record_id'] = (string) $record->getKey(); + + return $normalized; + }) + ->visible(fn (PolicyVersion $record) => ($record->policy_type ?? '') !== 'settingsCatalogPolicy'), ]), Tab::make('Raw JSON') + ->id('raw-json') ->schema([ Infolists\Components\ViewEntry::make('snapshot_pretty') ->view('filament.infolists.entries.snapshot-json') ->state(fn (PolicyVersion $record) => $record->snapshot ?? []), ]), Tab::make('Diff') + ->id('diff') ->schema([ Infolists\Components\ViewEntry::make('normalized_diff') ->view('filament.infolists.entries.normalized-diff') @@ -93,8 +113,9 @@ public static function infolist(Schema $schema): Schema return $diff->compare($from, $to); }), - Infolists\Components\TextEntry::make('diff') - ->label('Diff JSON vs previous') + Infolists\Components\ViewEntry::make('diff_json') + ->label('Raw diff (advanced)') + ->view('filament.infolists.entries.snapshot-json') ->state(function (PolicyVersion $record) { $previous = $record->previous(); @@ -102,11 +123,38 @@ public static function infolist(Schema $schema): Schema return ['summary' => 'No previous version']; } - return app(VersionDiff::class) - ->compare($previous->snapshot ?? [], $record->snapshot ?? []); - }) - ->formatStateUsing(fn ($state) => json_encode($state ?? [], JSON_PRETTY_PRINT)) - ->copyable(), + $diff = app(VersionDiff::class)->compare( + $previous->snapshot ?? [], + $record->snapshot ?? [] + ); + + $filter = static fn (array $items): array => array_filter( + $items, + static fn (mixed $value, string $key): bool => ! str_contains($key, '@odata.context'), + ARRAY_FILTER_USE_BOTH + ); + + $added = $filter($diff['added'] ?? []); + $removed = $filter($diff['removed'] ?? []); + $changed = $filter($diff['changed'] ?? []); + + return [ + 'summary' => [ + 'added' => count($added), + 'removed' => count($removed), + 'changed' => count($changed), + 'message' => sprintf( + '%d added, %d removed, %d changed', + count($added), + count($removed), + count($changed) + ), + ], + 'added' => $added, + 'removed' => $removed, + 'changed' => $changed, + ]; + }), ]), ]), ]); diff --git a/app/Services/Intune/CompliancePolicyNormalizer.php b/app/Services/Intune/CompliancePolicyNormalizer.php index a87ea65..2bcd299 100644 --- a/app/Services/Intune/CompliancePolicyNormalizer.php +++ b/app/Services/Intune/CompliancePolicyNormalizer.php @@ -44,7 +44,12 @@ public function normalize(?array $snapshot, string $policyType, ?string $platfor */ public function flattenForDiff(?array $snapshot, string $policyType, ?string $platform = null): array { - return $this->defaultNormalizer->flattenForDiff($snapshot, $policyType, $platform); + $snapshot = $snapshot ?? []; + + $normalized = $this->normalize($snapshot, $policyType, $platform); + $flat = $this->defaultNormalizer->flattenNormalizedForDiff($normalized); + + return array_merge($flat, $this->flattenComplianceNotificationsForDiff($snapshot)); } /** @@ -85,6 +90,85 @@ private function buildComplianceBlocks(array $snapshot): array return $blocks; } + /** + * @return array + */ + private function flattenComplianceNotificationsForDiff(array $snapshot): array + { + $scheduled = $snapshot['scheduledActionsForRule'] ?? null; + + if (! is_array($scheduled)) { + return []; + } + + $templateIds = []; + + foreach ($scheduled as $rule) { + if (! is_array($rule)) { + continue; + } + + $configs = $rule['scheduledActionConfigurations'] ?? null; + + if (! is_array($configs)) { + continue; + } + + foreach ($configs as $config) { + if (! is_array($config)) { + continue; + } + + if (($config['actionType'] ?? null) !== 'notification') { + continue; + } + + $templateKey = $this->resolveNotificationTemplateKey($config); + + if ($templateKey === null) { + continue; + } + + $templateId = $config[$templateKey] ?? null; + + if (! is_string($templateId) || $templateId === '' || $this->isEmptyGuid($templateId)) { + continue; + } + + $templateIds[] = $templateId; + } + } + + $templateIds = array_values(array_unique($templateIds)); + sort($templateIds); + + if ($templateIds === []) { + return []; + } + + return [ + 'Compliance notifications > Template IDs' => $templateIds, + ]; + } + + private function resolveNotificationTemplateKey(array $config): ?string + { + if (array_key_exists('notificationTemplateId', $config)) { + return 'notificationTemplateId'; + } + + if (array_key_exists('notificationMessageTemplateId', $config)) { + return 'notificationMessageTemplateId'; + } + + return null; + } + + private function isEmptyGuid(string $value): bool + { + return strtolower($value) === '00000000-0000-0000-0000-000000000000'; + } + /** * @return array{keys: array, labels?: array} */ @@ -282,6 +366,7 @@ private function ignoredKeys(): array 'settingCount', 'settingsCount', 'templateReference', + 'scheduledActionsForRule@odata.context', 'scheduledActionsForRule', ]; } diff --git a/app/Services/Intune/DefaultPolicyNormalizer.php b/app/Services/Intune/DefaultPolicyNormalizer.php index 6cf7145..f890a10 100644 --- a/app/Services/Intune/DefaultPolicyNormalizer.php +++ b/app/Services/Intune/DefaultPolicyNormalizer.php @@ -106,23 +106,49 @@ public function normalize(?array $snapshot, string $policyType, ?string $platfor public function flattenForDiff(?array $snapshot, string $policyType, ?string $platform = null): array { $normalized = $this->normalize($snapshot ?? [], $policyType, $platform); + + return $this->flattenNormalizedForDiff($normalized); + } + + /** + * Flatten an already normalized payload into key/value pairs for diffing. + * + * @param array{settings: array>, settings_table?: array} $normalized + * @return array + */ + public function flattenNormalizedForDiff(array $normalized): array + { $map = []; if (isset($normalized['settings_table']['rows']) && is_array($normalized['settings_table']['rows'])) { + $title = $normalized['settings_table']['title'] ?? 'Settings'; + $prefix = is_string($title) && $title !== '' ? $title.' > ' : ''; + foreach ($normalized['settings_table']['rows'] as $row) { if (! is_array($row)) { continue; } - $key = $row['path'] ?? $row['definition'] ?? 'entry'; + $key = $prefix.($row['path'] ?? $row['definition'] ?? 'entry'); $map[$key] = $row['value'] ?? null; } } - foreach ($normalized['settings'] as $block) { + foreach ($normalized['settings'] ?? [] as $block) { + if (! is_array($block)) { + continue; + } + + $title = $block['title'] ?? null; + $prefix = is_string($title) && $title !== '' ? $title.' > ' : ''; + if (($block['type'] ?? null) === 'table') { foreach ($block['rows'] ?? [] as $row) { - $key = $row['path'] ?? $row['label'] ?? 'entry'; + if (! is_array($row)) { + continue; + } + + $key = $prefix.($row['path'] ?? $row['label'] ?? 'entry'); $map[$key] = $row['value'] ?? null; } @@ -130,7 +156,11 @@ public function flattenForDiff(?array $snapshot, string $policyType, ?string $pl } foreach ($block['entries'] ?? [] as $entry) { - $key = $entry['key'] ?? 'entry'; + if (! is_array($entry)) { + continue; + } + + $key = $prefix.($entry['key'] ?? 'entry'); $map[$key] = $entry['value'] ?? null; } } @@ -554,6 +584,8 @@ private function normalizeStandard(array $snapshot): array 'omaSettings', 'settings', 'settingsDelta', + 'scheduledActionsForRule', + 'scheduledActionsForRule@odata.context', ]; $filtered = Arr::except($snapshot, $metadataKeys); diff --git a/app/Services/Intune/DeviceConfigurationPolicyNormalizer.php b/app/Services/Intune/DeviceConfigurationPolicyNormalizer.php index 5c88051..12574da 100644 --- a/app/Services/Intune/DeviceConfigurationPolicyNormalizer.php +++ b/app/Services/Intune/DeviceConfigurationPolicyNormalizer.php @@ -46,7 +46,10 @@ public function normalize(?array $snapshot, string $policyType, ?string $platfor */ public function flattenForDiff(?array $snapshot, string $policyType, ?string $platform = null): array { - return $this->defaultNormalizer->flattenForDiff($snapshot, $policyType, $platform); + $snapshot = $snapshot ?? []; + $normalized = $this->normalize($snapshot, $policyType, $platform); + + return $this->defaultNormalizer->flattenNormalizedForDiff($normalized); } /** diff --git a/resources/views/filament/infolists/entries/normalized-diff.blade.php b/resources/views/filament/infolists/entries/normalized-diff.blade.php index 42effc5..c57ebd1 100644 --- a/resources/views/filament/infolists/entries/normalized-diff.blade.php +++ b/resources/views/filament/infolists/entries/normalized-diff.blade.php @@ -1,35 +1,169 @@ @php $diff = $getState() ?? ['summary' => [], 'added' => [], 'removed' => [], 'changed' => []]; $summary = $diff['summary'] ?? []; + + $groupByBlock = static function (array $items): array { + $groups = []; + + foreach ($items as $path => $value) { + if (! is_string($path) || $path === '') { + continue; + } + + $parts = explode(' > ', $path, 2); + + if (count($parts) === 2) { + [$group, $label] = $parts; + } else { + $group = 'Other'; + $label = $path; + } + + $groups[$group][$label] = $value; + } + + ksort($groups); + + return $groups; + }; + + $stringify = static function (mixed $value): string { + if ($value === null) { + return '—'; + } + + if (is_bool($value)) { + return $value ? 'Enabled' : 'Disabled'; + } + + if (is_scalar($value)) { + return (string) $value; + } + + return json_encode($value, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE) ?: ''; + }; + + $isExpandable = static function (mixed $value): bool { + if (is_array($value)) { + return true; + } + + return is_string($value) && strlen($value) > 160; + }; @endphp -
-
Normalized diff
-
- {{ $summary['message'] ?? sprintf('%d added, %d removed, %d changed', $summary['added'] ?? 0, $summary['removed'] ?? 0, $summary['changed'] ?? 0) }} -
+
+ +
+ + {{ (int) ($summary['added'] ?? 0) }} added + + + {{ (int) ($summary['removed'] ?? 0) }} removed + + + {{ (int) ($summary['changed'] ?? 0) }} changed + +
+ - @foreach (['added' => 'Added', 'removed' => 'Removed', 'changed' => 'Changed'] as $key => $label) + @foreach (['changed' => ['label' => 'Changed', 'collapsed' => false], 'added' => ['label' => 'Added', 'collapsed' => true], 'removed' => ['label' => 'Removed', 'collapsed' => true]] as $key => $meta) @php $items = $diff[$key] ?? []; + $groups = $groupByBlock(is_array($items) ? $items : []); @endphp - @if (! empty($items)) -
-
{{ $label }}
-
    - @foreach ($items as $name => $value) -
  • - {{ $name }}: - @if (is_array($value)) - 
    {{ json_encode($value, JSON_PRETTY_PRINT) }}
    - @else - {{ is_bool($value) ? ($value ? 'true' : 'false') : (string) $value }} - @endif -
    • + @if ($groups !== []) + +
    + @foreach ($groups as $group => $groupItems) +
    +
    +
    + {{ $group }} +
    + + {{ count($groupItems) }} + +
    + +
    + @foreach ($groupItems as $name => $value) +
    + @if ($key === 'changed' && is_array($value) && array_key_exists('from', $value) && array_key_exists('to', $value)) + @php + $from = $value['from']; + $to = $value['to']; + $fromText = $stringify($from); + $toText = $stringify($to); + @endphp +
    +
    + {{ (string) $name }} +
    +
    + From + @if ($isExpandable($from)) +
    + + View + + 
    {{ $fromText }}
    +
    + @else +
    {{ $fromText }}
    + @endif +
    +
    + To + @if ($isExpandable($to)) +
    + + View + + 
    {{ $toText }}
    +
    + @else +
    {{ $toText }}
    + @endif +
    +
    + @else + @php + $text = $stringify($value); + @endphp +
    +
    + {{ (string) $name }} +
    +
    + @if ($isExpandable($value)) +
    + + View + + 
    {{ $text }}
    +
    + @else +
    {{ $text }}
    + @endif +
    +
    + @endif +
    + @endforeach +
    +
    @endforeach -
-
+
+ @endif @endforeach
diff --git a/resources/views/livewire/policy-version-assignments-widget.blade.php b/resources/views/livewire/policy-version-assignments-widget.blade.php index 780eb83..fd8466a 100644 --- a/resources/views/livewire/policy-version-assignments-widget.blade.php +++ b/resources/views/livewire/policy-version-assignments-widget.blade.php @@ -1,22 +1,11 @@ -
+
@if($version->assignments && count($version->assignments) > 0) -
-
-
-
-

- Assignments -

-

- Captured with this version on {{ $version->captured_at->format('M d, Y H:i') }} -

-
-
-
- -
- -
+ +
+

Summary

{{ count($version->assignments) }} assignment(s) @@ -29,12 +18,11 @@

- @php $scopeTags = $version->scope_tags['names'] ?? []; @endphp @if(!empty($scopeTags)) -
+

Scope Tags

@foreach($scopeTags as $tag) @@ -46,7 +34,6 @@
@endif -

Assignment Details

@@ -56,7 +43,7 @@ $type = $target['@odata.type'] ?? ''; $typeKey = strtolower((string) $type); $intent = $assignment['intent'] ?? 'apply'; - + $typeName = match (true) { str_contains($typeKey, 'exclusiongroupassignmenttarget') => 'Exclude group', str_contains($typeKey, 'groupassignmenttarget') => 'Include group', @@ -64,7 +51,7 @@ str_contains($typeKey, 'alldevicesassignmenttarget') => 'All Devices', default => 'Unknown', }; - + $groupId = $target['groupId'] ?? null; $groupName = $target['group_display_name'] ?? null; $groupOrphaned = $target['group_orphaned'] ?? ($version->metadata['has_orphaned_assignments'] ?? false); @@ -78,7 +65,7 @@
• {{ $typeName }} - + @if($groupId) : @if($groupOrphaned) @@ -104,56 +91,52 @@ Filter{{ $filterType !== 'none' ? " ({$filterType})" : '' }}: {{ $filterLabel }} @endif - + ({{ $intent }})
@endforeach
-
+ @else -
-
-

- Assignments -

- @php - $assignmentsFetched = $version->metadata['assignments_fetched'] ?? false; - $assignmentsFetchFailed = $version->metadata['assignments_fetch_failed'] ?? false; - $assignmentsFetchError = $version->metadata['assignments_fetch_error'] ?? null; - @endphp - @if($assignmentsFetchFailed) -

- Assignments could not be fetched from Microsoft Graph. -

- @if($assignmentsFetchError) -

- {{ $assignmentsFetchError }} -

- @endif - @elseif($assignmentsFetched) -

- No assignments found for this version. -

- @else -

- Assignments were not captured for this version. + + @php + $assignmentsFetched = $version->metadata['assignments_fetched'] ?? false; + $assignmentsFetchFailed = $version->metadata['assignments_fetch_failed'] ?? false; + $assignmentsFetchError = $version->metadata['assignments_fetch_error'] ?? null; + @endphp + @if($assignmentsFetchFailed) +

+ Assignments could not be fetched from Microsoft Graph. +

+ @if($assignmentsFetchError) +

+ {{ $assignmentsFetchError }}

@endif - @php - $hasBackupItem = $version->policy->backupItems() - ->whereNotNull('assignments') - ->where('created_at', '<=', $version->captured_at) - ->exists(); - @endphp - @if($hasBackupItem) -

- 💡 Assignment data may be available in related backup items. -

- @endif -
-
+ @elseif($assignmentsFetched) +

+ No assignments found for this version. +

+ @else +

+ Assignments were not captured for this version. +

+ @endif + + @php + $hasBackupItem = $version->policy->backupItems() + ->whereNotNull('assignments') + ->where('created_at', '<=', $version->captured_at) + ->exists(); + @endphp + @if($hasBackupItem) +

+ 💡 Assignment data may be available in related backup items. +

+ @endif + @endif @php @@ -161,40 +144,29 @@ $complianceTemplates = $compliance['templates'] ?? []; @endphp @if($complianceTotal > 0) -
-
-
-
-

- Compliance notifications -

-

- {{ $complianceTotal }} action(s) • {{ count($complianceTemplates) }} template(s) -

-
-
-
-
-
- @foreach($compliance['items'] ?? [] as $item) - @php - $ruleName = $item['rule_name'] ?? null; - $templateId = $item['template_id'] ?? null; - @endphp -
- • - - {{ $ruleName ?: 'Unnamed rule' }} + +
+ @foreach($compliance['items'] ?? [] as $item) + @php + $ruleName = $item['rule_name'] ?? null; + $templateId = $item['template_id'] ?? null; + @endphp +
+ • + + {{ $ruleName ?: 'Default rule' }} + + @if($templateId) + + Template: {{ $templateId }} - @if($templateId) - - Template: {{ $templateId }} - - @endif -
- @endforeach -
+ @endif +
+ @endforeach
-
+ @endif
diff --git a/tests/Feature/PolicyVersionViewAssignmentsTest.php b/tests/Feature/PolicyVersionViewAssignmentsTest.php index e8297a1..ff174ad 100644 --- a/tests/Feature/PolicyVersionViewAssignmentsTest.php +++ b/tests/Feature/PolicyVersionViewAssignmentsTest.php @@ -144,3 +144,58 @@ $response->assertSee('Test rule'); $response->assertSee('template-123'); }); + +it('uses a default label when compliance rule name is missing', function () { + $version = PolicyVersion::factory()->create([ + 'tenant_id' => $this->tenant->id, + 'policy_id' => $this->policy->id, + 'version_number' => 1, + 'policy_type' => 'deviceCompliancePolicy', + 'assignments' => null, + 'snapshot' => [ + 'scheduledActionsForRule' => [ + [ + 'ruleName' => null, + 'scheduledActionConfigurations' => [ + [ + 'actionType' => 'notification', + 'notificationTemplateId' => 'template-456', + ], + ], + ], + ], + ], + ]); + + $this->actingAs($this->user); + + $response = $this->get("/admin/policy-versions/{$version->id}"); + + $response->assertOk(); + $response->assertSee('Compliance notifications'); + $response->assertSee('Default rule'); + $response->assertSee('template-456'); +}); + +it('renders structured normalized settings for compliance policy versions', function () { + $version = PolicyVersion::factory()->create([ + 'tenant_id' => $this->tenant->id, + 'policy_id' => $this->policy->id, + 'version_number' => 1, + 'policy_type' => 'deviceCompliancePolicy', + 'platform' => 'all', + 'snapshot' => [ + '@odata.type' => '#microsoft.graph.windows10CompliancePolicy', + 'passwordRequired' => true, + ], + ]); + + $this->actingAs($this->user); + + $response = $this->get("/admin/policy-versions/{$version->id}?tab=normalized-settings"); + + $response->assertOk(); + $response->assertSee('Password & Access'); + $response->assertSee('Password required'); + $response->assertSee('Enabled'); +}); diff --git a/tests/Unit/CompliancePolicyNormalizerTest.php b/tests/Unit/CompliancePolicyNormalizerTest.php index d788e4f..2ef9b53 100644 --- a/tests/Unit/CompliancePolicyNormalizerTest.php +++ b/tests/Unit/CompliancePolicyNormalizerTest.php @@ -23,6 +23,7 @@ ], ], ], + 'scheduledActionsForRule@odata.context' => 'https://graph.microsoft.com/beta/$metadata#deviceManagement/deviceCompliancePolicies', 'customSetting' => 'Custom value', ]; @@ -41,6 +42,48 @@ ->toContain('Custom Setting'); expect(collect($additionalBlock['rows'])->pluck('label')->all()) ->not->toContain('Scheduled Actions For Rule'); + expect(collect($additionalBlock['rows'])->pluck('label')->all()) + ->not->toContain('Scheduled Actions For Rule@Odata.context'); expect($settings->pluck('title')->all())->not->toContain('General'); }); + +it('flattens compliance notifications into a compact diff key', function () { + $normalizer = app(CompliancePolicyNormalizer::class); + + $snapshot = [ + '@odata.type' => '#microsoft.graph.windows10CompliancePolicy', + 'passwordRequired' => true, + 'scheduledActionsForRule' => [ + [ + 'ruleName' => null, + 'scheduledActionConfigurations' => [ + [ + 'actionType' => 'notification', + 'notificationTemplateId' => 'template-123', + ], + [ + 'actionType' => 'notification', + 'notificationTemplateId' => '00000000-0000-0000-0000-000000000000', + ], + [ + 'actionType' => 'block', + 'notificationTemplateId' => 'template-ignored', + ], + ], + ], + ], + 'scheduledActionsForRule@odata.context' => 'https://graph.microsoft.com/beta/$metadata#deviceManagement/deviceCompliancePolicies', + ]; + + $flat = $normalizer->flattenForDiff($snapshot, 'deviceCompliancePolicy', 'windows'); + + expect($flat)->toHaveKey('Password & Access > Password required'); + expect($flat['Password & Access > Password required'])->toBeTrue(); + + expect($flat)->toHaveKey('Compliance notifications > Template IDs'); + expect($flat['Compliance notifications > Template IDs'])->toBe(['template-123']); + + expect(array_keys($flat))->not->toContain('scheduledActionsForRule'); + expect(array_keys($flat))->not->toContain('scheduledActionsForRule@odata.context'); +}); diff --git a/tests/Unit/DefaultPolicyNormalizerDiffTest.php b/tests/Unit/DefaultPolicyNormalizerDiffTest.php new file mode 100644 index 0000000..8774a84 --- /dev/null +++ b/tests/Unit/DefaultPolicyNormalizerDiffTest.php @@ -0,0 +1,21 @@ + '#microsoft.graph.somePolicy', + 'displayName' => 'Example Policy', + 'customSetting' => true, + ]; + + $flat = $normalizer->flattenForDiff($snapshot, 'somePolicyType', 'all'); + + expect($flat)->toHaveKey('General > Display Name', 'Example Policy'); + expect($flat)->toHaveKey('General > Custom Setting'); + expect($flat['General > Custom Setting'])->toBeTrue(); +});