docs(phase-17): complete phase execution and verification
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -4,7 +4,7 @@ milestone: v2.3
|
|||||||
milestone_name: Tenant Management & Report Enhancements
|
milestone_name: Tenant Management & Report Enhancements
|
||||||
status: planning
|
status: planning
|
||||||
stopped_at: Completed 17-02-PLAN.md
|
stopped_at: Completed 17-02-PLAN.md
|
||||||
last_updated: "2026-04-09T11:11:11.458Z"
|
last_updated: "2026-04-09T11:13:35.417Z"
|
||||||
last_activity: 2026-04-09 — Roadmap created for v2.3 (phases 15-19)
|
last_activity: 2026-04-09 — Roadmap created for v2.3 (phases 15-19)
|
||||||
progress:
|
progress:
|
||||||
total_phases: 5
|
total_phases: 5
|
||||||
|
|||||||
@@ -0,0 +1,124 @@
|
|||||||
|
---
|
||||||
|
phase: 17-group-expansion-html-reports
|
||||||
|
verified: 2026-04-09T00:00:00Z
|
||||||
|
status: passed
|
||||||
|
score: 10/10 must-haves verified
|
||||||
|
re_verification: false
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 17: Group Expansion in HTML Reports — Verification Report
|
||||||
|
|
||||||
|
**Phase Goal:** Add group member expansion to HTML permission reports — clicking a SharePoint group expands to show individual members.
|
||||||
|
**Verified:** 2026-04-09
|
||||||
|
**Status:** PASSED
|
||||||
|
**Re-verification:** No — initial verification
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Goal Achievement
|
||||||
|
|
||||||
|
### Observable Truths
|
||||||
|
|
||||||
|
| # | Truth | Status | Evidence |
|
||||||
|
|---|-------|--------|----------|
|
||||||
|
| 1 | SharePointGroupResolver returns a dict keyed by group name with OrdinalIgnoreCase | VERIFIED | `new Dictionary<string, ...>(StringComparer.OrdinalIgnoreCase)` at line 40–41 of `SharePointGroupResolver.cs`; test `ResolveGroupsAsync_EmptyGroupNames_DictUsesOrdinalIgnoreCase` confirms cast + lookup with different casing |
|
||||||
|
| 2 | Resolver returns empty list (never throws) when group cannot be resolved | VERIFIED | Per-group `try/catch` at lines 86–90: `result[groupName] = Array.Empty<ResolvedMember>()` on any exception |
|
||||||
|
| 3 | IsAadGroup correctly identifies AAD group login patterns | VERIFIED | `internal static bool IsAadGroup(string login) => login.StartsWith("c:0t.c|", ...)` at line 102; 5 unit tests cover true/false/casing/empty |
|
||||||
|
| 4 | AAD groups are resolved transitively via Graph | VERIFIED | `ResolveAadGroupAsync` calls `graphClient.Groups[aadGroupId].TransitiveMembers.GraphUser.GetAsync()` with `PageIterator` pagination |
|
||||||
|
| 5 | ResolvedMember record exists in Core/Models | VERIFIED | `public record ResolvedMember(string DisplayName, string Login)` at line 10 of `ResolvedMember.cs` |
|
||||||
|
| 6 | Group pills in HTML report are clickable and expand to show members | VERIFIED | `<span class="user-pill group-expandable" onclick="toggleGroup('grpmem{idx}')">` rendered in both `BuildHtml` overloads |
|
||||||
|
| 7 | Empty member list (resolution failed) renders "members unavailable" label | VERIFIED | `memberContent = "<em style=\"color:#888\">members unavailable</em>"` in both overloads; test `BuildHtml_WithEmptyMemberList_RendersMembersUnavailable` confirms |
|
||||||
|
| 8 | Null groupMembers preserves identical pre-Phase 17 output | VERIFIED | `bool isExpandableGroup = ... && groupMembers != null && ...` guard; test `BuildHtml_NoGroupMembers_IdenticalToDefault` asserts output equality |
|
||||||
|
| 9 | toggleGroup() JS function in HtmlExportService inline script | VERIFIED | `function toggleGroup(id)` present in both `BuildHtml` overloads (lines 178 and 390 respectively) |
|
||||||
|
| 10 | PermissionsViewModel calls ISharePointGroupResolver before export and passes results to BuildHtml | VERIFIED | `_groupResolver.ResolveGroupsAsync(...)` at line 353 of `PermissionsViewModel.cs`; both `WriteAsync` call sites pass `groupMembers` |
|
||||||
|
|
||||||
|
**Score:** 10/10 truths verified
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Required Artifacts
|
||||||
|
|
||||||
|
### Plan 01 Artifacts
|
||||||
|
|
||||||
|
| Artifact | Expected | Status | Details |
|
||||||
|
|----------|----------|--------|---------|
|
||||||
|
| `SharepointToolbox/Core/Models/ResolvedMember.cs` | Value record for resolved group member | VERIFIED | Contains `record ResolvedMember(string DisplayName, string Login)` |
|
||||||
|
| `SharepointToolbox/Services/ISharePointGroupResolver.cs` | Interface contract | VERIFIED | Exports `ISharePointGroupResolver` with `ResolveGroupsAsync` signature |
|
||||||
|
| `SharepointToolbox/Services/SharePointGroupResolver.cs` | CSOM + Graph implementation | VERIFIED | Contains `ResolveGroupsAsync`, `IsAadGroup`, `ExtractAadGroupId`, `StripClaims`, transitive Graph resolution |
|
||||||
|
| `SharepointToolbox/App.xaml.cs` | DI registration | VERIFIED | Line 162: `services.AddTransient<ISharePointGroupResolver, SharePointGroupResolver>()` |
|
||||||
|
| `SharepointToolbox.Tests/Services/SharePointGroupResolverTests.cs` | Unit tests | VERIFIED | 14 tests (12 unit + 2 skip-marked live), covers `IsAadGroup`, `ExtractAadGroupId`, `StripClaims`, empty-list, OrdinalIgnoreCase |
|
||||||
|
|
||||||
|
### Plan 02 Artifacts
|
||||||
|
|
||||||
|
| Artifact | Expected | Status | Details |
|
||||||
|
|----------|----------|--------|---------|
|
||||||
|
| `SharepointToolbox/Services/Export/HtmlExportService.cs` | Expandable pills + toggleGroup JS | VERIFIED | Both `BuildHtml` overloads contain `toggleGroup`, `group-expandable`, `data-group` sub-rows, `members unavailable` fallback; both `WriteAsync` overloads pass `groupMembers` through |
|
||||||
|
| `SharepointToolbox/ViewModels/Tabs/PermissionsViewModel.cs` | Group resolution orchestration | VERIFIED | `_groupResolver` field injected, `ResolveGroupsAsync` called in `ExportHtmlAsync`, both write paths pass `groupMembers` |
|
||||||
|
| `SharepointToolbox.Tests/Services/Export/HtmlExportServiceTests.cs` | Expansion + backward compat tests | VERIFIED | Contains `grpmem` patterns; 6 new group expansion tests: `NoGroupMembers_IdenticalToDefault`, `WithGroupMembers_RendersExpandablePill`, `RendersHiddenMemberSubRow`, `WithEmptyMemberList`, `ContainsToggleGroupJs`, `Simplified_WithGroupMembers` |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Key Link Verification
|
||||||
|
|
||||||
|
| From | To | Via | Status | Details |
|
||||||
|
|------|----|-----|--------|---------|
|
||||||
|
| `SharePointGroupResolver.cs` | `GraphClientFactory` | Constructor injection | VERIFIED | Field `_graphClientFactory`; `graphClient ??= await _graphClientFactory!.CreateClientAsync(...)` |
|
||||||
|
| `SharePointGroupResolver.cs` | `ExecuteQueryRetryHelper` | CSOM retry | VERIFIED | `await ExecuteQueryRetryHelper.ExecuteQueryRetryAsync(ctx, null, ct)` at line 59 |
|
||||||
|
| `PermissionsViewModel.cs` | `ISharePointGroupResolver` | Constructor injection + `ExportHtmlAsync` | VERIFIED | `_groupResolver.ResolveGroupsAsync(ctx, _currentProfile.ClientId, groupNames, ct)` |
|
||||||
|
| `PermissionsViewModel.cs` | `HtmlExportService.BuildHtml` | Passing groupMembers dict | VERIFIED | Both `WriteAsync` calls include `groupMembers` as last argument |
|
||||||
|
| `HtmlExportService.cs` | `toggleGroup` JS | Inline script block | VERIFIED | `function toggleGroup(id)` present in both `BuildHtml` overloads; `filterTable()` guards `data-group` rows |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Requirements Coverage
|
||||||
|
|
||||||
|
| Requirement | Source Plan | Description | Status | Evidence |
|
||||||
|
|-------------|------------|-------------|--------|----------|
|
||||||
|
| RPT-01 | 17-02 | User can expand SharePoint groups in HTML reports to see group members | SATISFIED | Expandable group pills with `onclick="toggleGroup('grpmem{idx}')"` and hidden member sub-rows implemented in both `BuildHtml` overloads; `PermissionsViewModel` wires resolver before export |
|
||||||
|
| RPT-02 | 17-01, 17-02 | Group member resolution uses transitive membership for nested group members | SATISFIED | `ResolveAadGroupAsync` uses Graph `transitiveMembers/microsoft.graph.user` endpoint with `PageIterator` pagination; AAD group detection via `IsAadGroup` |
|
||||||
|
|
||||||
|
No orphaned requirements — both RPT-01 and RPT-02 are claimed by plans and implemented.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Anti-Patterns Found
|
||||||
|
|
||||||
|
| File | Line | Pattern | Severity | Impact |
|
||||||
|
|------|------|---------|----------|--------|
|
||||||
|
| `HtmlExportService.cs` | 92, 303 | `placeholder="Filter permissions..."` | Info | HTML input placeholder attribute — not a stub marker, expected UI text |
|
||||||
|
|
||||||
|
No blocker or warning anti-patterns found. The two "placeholder" matches are valid HTML `<input placeholder="...">` attributes, not code stubs.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Human Verification Required
|
||||||
|
|
||||||
|
### 1. Click-to-expand group pills in browser
|
||||||
|
|
||||||
|
**Test:** Open a generated HTML report that includes SharePoint groups. Click a group pill with the down-arrow indicator.
|
||||||
|
**Expected:** A hidden row appears below the group row listing member display names and logins. Clicking again collapses it. The filter input does not interfere with expanded rows.
|
||||||
|
**Why human:** JavaScript runtime behavior (DOM toggle) cannot be verified by static code inspection.
|
||||||
|
|
||||||
|
### 2. Members unavailable fallback display
|
||||||
|
|
||||||
|
**Test:** Trigger a report where group resolution fails (e.g., network error or group not found). Open the HTML.
|
||||||
|
**Expected:** Group pill is still expandable; clicking it reveals italic "members unavailable" text in grey.
|
||||||
|
**Why human:** Requires a runtime scenario with a failing resolver.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Commits
|
||||||
|
|
||||||
|
All commits from git log are present and correspond to the documented plan phases:
|
||||||
|
|
||||||
|
- `0f8b195` — test(17-01): failing tests (RED phase)
|
||||||
|
- `543b863` — feat(17-01): ResolvedMember, ISharePointGroupResolver, SharePointGroupResolver
|
||||||
|
- `1aa0d15` — feat(17-01): DI registration in App.xaml.cs
|
||||||
|
- `c35ee76` — test(17-02): failing tests for group pill expansion (RED)
|
||||||
|
- `07ed6e2` — feat(17-02): HtmlExportService implementation
|
||||||
|
- `aab3aee` — feat(17-02): PermissionsViewModel wiring
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
_Verified: 2026-04-09_
|
||||||
|
_Verifier: Claude (gsd-verifier)_
|
||||||
Reference in New Issue
Block a user