From 7bebbbcc027f3e63406cdaf9b46eed1241cf5cfd Mon Sep 17 00:00:00 2001 From: Dev Date: Thu, 9 Apr 2026 13:06:16 +0200 Subject: [PATCH] docs(17-01): complete SharePointGroupResolver service plan - SUMMARY, STATE, ROADMAP updated --- .planning/REQUIREMENTS.md | 4 +- .planning/ROADMAP.md | 4 +- .planning/STATE.md | 14 +-- .../17-01-SUMMARY.md | 88 +++++++++++++++++++ 4 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 .planning/phases/17-group-expansion-html-reports/17-01-SUMMARY.md diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 784dfec..9249628 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -24,7 +24,7 @@ Requirements for v2.3 Tenant Management & Report Enhancements. Each maps to road ### Report Enhancements - [ ] **RPT-01**: User can expand SharePoint groups in HTML reports to see group members -- [ ] **RPT-02**: Group member resolution uses transitive membership to include nested group members +- [x] **RPT-02**: Group member resolution uses transitive membership to include nested group members - [x] **RPT-03**: User can enable/disable entry consolidation per export (toggle in export settings) - [x] **RPT-04**: Consolidated reports merge rows for the same user with identical access levels across multiple locations into a single row @@ -57,7 +57,7 @@ Requirements for v2.3 Tenant Management & Report Enhancements. Each maps to road | OWN-01 | Phase 18 | Pending | | OWN-02 | Phase 18 | Pending | | RPT-01 | Phase 17 | Pending | -| RPT-02 | Phase 17 | Pending | +| RPT-02 | Phase 17 | Complete | | RPT-03 | Phase 16 | Complete | | RPT-04 | Phase 15 | Complete | diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 6acd4ba..eeca2ae 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -88,7 +88,7 @@ Plans: 2. Member resolution includes transitive membership: nested groups are recursively resolved so every leaf user is shown 3. Group expansion is triggered at export time via Graph API — the permission scan itself is unchanged 4. When Graph cannot resolve a group's members (throttled or insufficient scope), the report shows the group row with a "members unavailable" label rather than failing the export -**Plans:** 2 plans +**Plans:** 1/2 plans executed Plans: - [ ] 17-01-PLAN.md — ResolvedMember model + ISharePointGroupResolver service (CSOM + Graph transitive resolution) + DI registration - [ ] 17-02-PLAN.md — HtmlExportService expandable group pills + toggleGroup JS + PermissionsViewModel wiring @@ -125,6 +125,6 @@ Plans: | 10-14 | v2.2 | 14/14 | Shipped | 2026-04-09 | | 15. Consolidation Data Model | v2.3 | 2/2 | Complete | 2026-04-09 | | 16. Report Consolidation Toggle | v2.3 | 2/2 | Complete | 2026-04-09 | -| 17. Group Expansion in HTML Reports | v2.3 | 0/2 | Not started | — | +| 17. Group Expansion in HTML Reports | 1/2 | In Progress| | — | | 18. Auto-Take Ownership | v2.3 | 0/? | Not started | — | | 19. App Registration & Removal | v2.3 | 0/? | Not started | — | diff --git a/.planning/STATE.md b/.planning/STATE.md index 57e5976..1435285 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,14 +3,14 @@ gsd_state_version: 1.0 milestone: v2.3 milestone_name: Tenant Management & Report Enhancements status: planning -stopped_at: Completed 16-02-PLAN.md -last_updated: "2026-04-09T10:43:01.998Z" +stopped_at: Completed 17-01-PLAN.md +last_updated: "2026-04-09T11:06:09.877Z" last_activity: 2026-04-09 — Roadmap created for v2.3 (phases 15-19) progress: total_phases: 5 completed_phases: 2 - total_plans: 4 - completed_plans: 4 + total_plans: 6 + completed_plans: 5 --- # Project State @@ -68,6 +68,8 @@ Decisions are logged in PROJECT.md Key Decisions table. - [Phase 16-01]: PermissionsViewModel gets MergePermissions as no-op placeholder reserved for future use - [Phase 16-report-consolidation-toggle]: BuildConsolidatedHtml is a private method via early-return in BuildHtml — existing code path completely untouched - [Phase 16-report-consolidation-toggle]: Separate locIdx counter for location groups (loc0, loc1...) distinct from grpIdx for user groups (ugrp0...) prevents ID collision +- [Phase 17]: Static helpers IsAadGroup/ExtractAadGroupId/StripClaims declared internal to enable unit testing via InternalsVisibleTo without polluting public API +- [Phase 17]: Graph client created lazily on first AAD group encountered to avoid unnecessary auth overhead for groups with no nested AAD members ### Pending Todos @@ -79,7 +81,7 @@ None. ## Session Continuity -Last session: 2026-04-09T10:39:46.923Z -Stopped at: Completed 16-02-PLAN.md +Last session: 2026-04-09T11:06:05.361Z +Stopped at: Completed 17-01-PLAN.md Resume file: None Next step: `/gsd:plan-phase 15` diff --git a/.planning/phases/17-group-expansion-html-reports/17-01-SUMMARY.md b/.planning/phases/17-group-expansion-html-reports/17-01-SUMMARY.md new file mode 100644 index 0000000..09aacf8 --- /dev/null +++ b/.planning/phases/17-group-expansion-html-reports/17-01-SUMMARY.md @@ -0,0 +1,88 @@ +--- +phase: 17-group-expansion-html-reports +plan: "01" +subsystem: services +tags: [group-resolution, csom, graph-api, tdd, di] +dependency_graph: + requires: [] + provides: [ISharePointGroupResolver, SharePointGroupResolver, ResolvedMember] + affects: [App.xaml.cs DI container] +tech_stack: + added: [] + patterns: [CSOM SiteGroups.GetByName, Graph transitiveMembers PageIterator, OrdinalIgnoreCase dict, lazy Graph client init] +key_files: + created: + - SharepointToolbox/Core/Models/ResolvedMember.cs + - SharepointToolbox/Services/ISharePointGroupResolver.cs + - SharepointToolbox/Services/SharePointGroupResolver.cs + - SharepointToolbox.Tests/Services/SharePointGroupResolverTests.cs + modified: + - SharepointToolbox/App.xaml.cs +decisions: + - "Static helpers IsAadGroup/ExtractAadGroupId/StripClaims declared internal to enable unit testing via InternalsVisibleTo without polluting public API" + - "Graph client created lazily on first AAD group encountered to avoid unnecessary auth overhead for groups with no nested AAD members" + - "GraphUser/GraphUserCollectionResponse aliased to resolve ambiguity between Microsoft.SharePoint.Client.User and Microsoft.Graph.Models.User" +metrics: + duration_minutes: 3 + tasks_completed: 2 + files_created: 4 + files_modified: 1 + completed_date: "2026-04-09" +--- + +# Phase 17 Plan 01: SharePoint Group Resolver Service Summary + +**One-liner:** CSOM + Graph transitive member resolver with OrdinalIgnoreCase dictionary, graceful error fallback, and internal static helpers for unit testing. + +## What Was Built + +Created the `ISharePointGroupResolver` service that pre-resolves SharePoint group members before HTML export. The service uses CSOM to enumerate direct group users, then calls Microsoft Graph `groups/{id}/transitiveMembers/microsoft.graph.user` for any nested AAD groups — satisfying the RPT-02 transitive membership requirement. + +### Files Created + +- **`SharepointToolbox/Core/Models/ResolvedMember.cs`** — Simple `record ResolvedMember(string DisplayName, string Login)` value type. +- **`SharepointToolbox/Services/ISharePointGroupResolver.cs`** — Interface with single `ResolveGroupsAsync(ctx, clientId, groupNames, ct)` method returning `IReadOnlyDictionary>`. +- **`SharepointToolbox/Services/SharePointGroupResolver.cs`** — Implementation: CSOM group user loading, AAD group detection via `IsAadGroup()`, Graph `TransitiveMembers.GraphUser.GetAsync()` with `PageIterator` pagination, per-group try/catch for graceful fallback. +- **`SharepointToolbox.Tests/Services/SharePointGroupResolverTests.cs`** — 14 tests (12 passing unit tests + 2 live-tenant skip-marked): covers `IsAadGroup`, `ExtractAadGroupId`, `StripClaims`, empty-list behavior, and OrdinalIgnoreCase comparer verification. + +### Files Modified + +- **`SharepointToolbox/App.xaml.cs`** — Added `services.AddTransient()` under Phase 17 comment. + +## Decisions Made + +1. **Internal static helpers for testability:** `IsAadGroup`, `ExtractAadGroupId`, `StripClaims` are `internal static` — accessible to the test project via the existing `InternalsVisibleTo("SharepointToolbox.Tests")` assembly attribute without exposing them as public API. + +2. **Lazy Graph client creation:** `graphClient` is created on-demand only when the first AAD group is encountered in the loop. This avoids a Graph auth round-trip for sites with no nested AAD groups (common case). + +3. **Type alias for ambiguous User:** `GraphUser = Microsoft.Graph.Models.User` and `GraphUserCollectionResponse` aliased to resolve CS0104 ambiguity — both CSOM (`Microsoft.SharePoint.Client.User`) and Graph SDK (`Microsoft.Graph.Models.User`) are referenced in the same file. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] CS0104 ambiguous User reference** +- **Found during:** Task 1 GREEN phase (first build) +- **Issue:** `PageIterator` was ambiguous between `Microsoft.SharePoint.Client.User` and `Microsoft.Graph.Models.User` — both namespaces imported. +- **Fix:** Added `using GraphUser = Microsoft.Graph.Models.User` and `using GraphUserCollectionResponse = Microsoft.Graph.Models.UserCollectionResponse` aliases. +- **Files modified:** `SharepointToolbox/Services/SharePointGroupResolver.cs` + +**2. [Rule 1 - Bug] IReadOnlyDictionary.Comparer not accessible** +- **Found during:** Task 1 GREEN phase (test compile) +- **Issue:** Test cast `result.Comparer` fails because `IReadOnlyDictionary<,>` does not expose `.Comparer`. The concrete `Dictionary<,>` does. +- **Fix:** Updated test to cast the result to `Dictionary>` before accessing comparer behavior — works because `SharePointGroupResolver.ResolveGroupsAsync` returns a `Dictionary<>` via `IReadOnlyDictionary<>`. +- **Files modified:** `SharepointToolbox.Tests/Services/SharePointGroupResolverTests.cs` + +## Test Results + +``` +dotnet test --filter "FullyQualifiedName~SharePointGroupResolverTests" +Passed: 12, Skipped: 2 (live tenant), Failed: 0 + +dotnet test (full suite) +Passed: 314, Skipped: 28, Failed: 0 +``` + +## Self-Check + +Files created: verified. Commits verified below.