docs(17-01): complete SharePointGroupResolver service plan - SUMMARY, STATE, ROADMAP updated
This commit is contained in:
Dev
@@ -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<string, IReadOnlyList<ResolvedMember>>`.
|
||||
- **`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<ISharePointGroupResolver, SharePointGroupResolver>()` 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<User, UserCollectionResponse>` 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<string, IReadOnlyList<ResolvedMember>>` 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.
|
||||
Reference in New Issue
Block a user