diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index c206b72..46f954d 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -44,7 +44,7 @@ ### v2.3 Tenant Management & Report Enhancements (Phases 15-19) - [x] **Phase 15: Consolidation Data Model** (2 plans) — PermissionConsolidator service and merged-row model; zero API calls, pure data shapes (completed 2026-04-09) -- [ ] **Phase 16: Report Consolidation Toggle** (2 plans) — Export settings toggle wired to PermissionConsolidator; first user-visible consolidation behavior +- [x] **Phase 16: Report Consolidation Toggle** (2 plans) — Export settings toggle wired to PermissionConsolidator; first user-visible consolidation behavior (completed 2026-04-09) - [ ] **Phase 17: Group Expansion in HTML Reports** — Clickable group expansion in HTML exports with transitive membership resolution - [ ] **Phase 18: Auto-Take Ownership** — Global toggle and automatic site collection admin elevation on access denied - [ ] **Phase 19: App Registration & Removal** — Automated Entra app registration with guided fallback and clean removal @@ -74,7 +74,7 @@ Plans: 2. When the toggle is OFF, the exported HTML report is byte-for-byte identical to the pre-v2.3 output 3. When the toggle is ON, the exported HTML report merges rows for the same user with identical access levels into a single row showing all affected locations 4. The toggle state is remembered for the session (does not reset between exports within the same session) -**Plans:** 1/2 plans executed +**Plans:** 2/2 plans complete Plans: - [ ] 16-01-PLAN.md — ViewModel properties + XAML Export Options GroupBox + localization + CSV consolidation - [ ] 16-02-PLAN.md — HTML consolidated rendering with expandable location sub-lists + full test verification @@ -121,7 +121,7 @@ Plans: | 6-9 | v1.1 | 25/25 | Shipped | 2026-04-08 | | 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 | 1/2 | In Progress| | — | +| 16. Report Consolidation Toggle | 2/2 | Complete | 2026-04-09 | — | | 17. Group Expansion in HTML Reports | v2.3 | 0/? | Not started | — | | 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 20f88d2..74b5dba 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-01-PLAN.md -last_updated: "2026-04-09T10:34:56.209Z" +stopped_at: Completed 16-02-PLAN.md +last_updated: "2026-04-09T10:39:46.925Z" last_activity: 2026-04-09 — Roadmap created for v2.3 (phases 15-19) progress: total_phases: 5 - completed_phases: 1 + completed_phases: 2 total_plans: 4 - completed_plans: 3 + completed_plans: 4 --- # Project State @@ -66,6 +66,8 @@ Decisions are logged in PROJECT.md Key Decisions table. - [Phase 15-consolidation-data-model]: RPT-04-g test data uses 11 rows (not 10) to produce 7 consolidated rows — plan description had a counting error; 4 unique rows + 3 merged groups = 7 - [Phase 16-01]: Consolidated branch uses early-return pattern inside WriteSingleFileAsync to leave existing code path untouched - [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 ### Pending Todos @@ -77,7 +79,7 @@ None. ## Session Continuity -Last session: 2026-04-09T10:34:56.207Z -Stopped at: Completed 16-01-PLAN.md +Last session: 2026-04-09T10:39:46.923Z +Stopped at: Completed 16-02-PLAN.md Resume file: None Next step: `/gsd:plan-phase 15` diff --git a/.planning/phases/16-report-consolidation-toggle/16-02-SUMMARY.md b/.planning/phases/16-report-consolidation-toggle/16-02-SUMMARY.md new file mode 100644 index 0000000..03b832a --- /dev/null +++ b/.planning/phases/16-report-consolidation-toggle/16-02-SUMMARY.md @@ -0,0 +1,125 @@ +--- +phase: 16-report-consolidation-toggle +plan: "02" +subsystem: export +tags: [html-export, consolidation, expandable-rows, toggleGroup, tdd] +dependency-graph: + requires: [16-01, 15-01, 15-02] + provides: [Consolidated HTML export path, expandable location sub-lists, by-site view suppression] + affects: [UserAccessHtmlExportService, UserAccessAuditViewModel] +tech-stack: + added: [] + patterns: [early-return branch, optional parameter default, TDD red-green, private method extraction] +key-files: + created: [] + modified: + - SharepointToolbox/Services/Export/UserAccessHtmlExportService.cs + - SharepointToolbox/ViewModels/Tabs/UserAccessAuditViewModel.cs + - SharepointToolbox.Tests/Services/Export/UserAccessHtmlExportServiceTests.cs +decisions: + - "BuildConsolidatedHtml is a private method called via early-return in BuildHtml — existing non-consolidated code path is completely untouched" + - "Separate locIdx counter for location group IDs (loc0, loc1...) is distinct from grpIdx for user group IDs (ugrp0, ugrp1...) — avoids ID collision Pitfall 2" + - "Existing branding test call site updated to use named parameter branding: because mergePermissions was inserted before branding in the signature" +metrics: + duration: ~10min + completed: 2026-04-09 + tasks: 2 + files-modified: 3 +--- + +# Phase 16 Plan 02: Consolidated HTML Rendering Path Summary + +Consolidated HTML export with expandable [N sites] badges using toggleGroup() JS, by-site view suppression when mergePermissions=true, and ViewModel wiring for the HTML export call site. + +## Tasks Completed + +| Task | Name | Commit | Files | +|------|------|--------|-------| +| 1 (RED) | Add failing tests for RPT-03-b through RPT-03-e | 3d95d2a | UserAccessHtmlExportServiceTests.cs | +| 1 (GREEN) | Implement consolidated HTML rendering path and wire ViewModel | 0ebe707 | UserAccessHtmlExportService.cs, UserAccessAuditViewModel.cs, UserAccessHtmlExportServiceTests.cs | +| 2 | Full solution build and test suite verification | — | (no source changes — verification only) | + +## What Was Built + +### BuildHtml Signature Change +`BuildHtml` now accepts `bool mergePermissions = false` as the second parameter (before `branding`): +```csharp +public string BuildHtml(IReadOnlyList entries, bool mergePermissions = false, ReportBranding? branding = null) +``` +The early-return branch: +```csharp +if (mergePermissions) +{ + var consolidated = PermissionConsolidator.Consolidate(entries); + return BuildConsolidatedHtml(consolidated, entries, branding); +} +``` +The entire existing code path below this branch is completely untouched. + +### WriteAsync Signature Change +`WriteAsync` now accepts `bool mergePermissions = false` after `ct` and passes it through to `BuildHtml`: +```csharp +public async Task WriteAsync(IReadOnlyList entries, string filePath, CancellationToken ct, bool mergePermissions = false, ReportBranding? branding = null) +``` + +### BuildConsolidatedHtml Private Method +Produces a consolidated HTML report: +- Same HTML shell (DOCTYPE, head, CSS, stats cards, user summary cards) +- Single by-user table with columns: User, Permission Level, Access Type, Granted Through, Sites +- Group headers per UserLogin with `ugrp{n}` IDs +- Sites column behavior: + - 1 location: plain text site title (no badge, no sub-rows) + - 2+ locations: `N sites` with hidden sub-rows (`data-group="loc{n}" style="display:none"`) containing linked site titles +- By-site view (view-site div) is completely omitted +- btn-site is completely omitted — only By User button rendered +- Separate `locIdx` counter for location groups, distinct from `grpIdx` for user groups + +### ViewModel Wiring +`UserAccessAuditViewModel.ExportHtmlAsync` now passes `MergePermissions` to `WriteAsync`: +```csharp +await _htmlExportService.WriteAsync(Results, dialog.FileName, CancellationToken.None, MergePermissions, branding); +``` + +### Tests (TDD) +4 new test methods added: +- `BuildHtml_mergePermissionsFalse_identical_to_default` — RPT-03-b: byte-identical output to default call +- `BuildHtml_mergePermissionsTrue_contains_sites_column` — RPT-03-c: "Sites" column header present +- `BuildHtml_mergePermissionsTrue_multiLocation_has_badge_and_subrows` — RPT-03-d: onclick toggleGroup + data-group=loc pattern +- `BuildHtml_mergePermissionsTrue_omits_bysite_view` — RPT-03-e: no btn-site, no view-site + +All 12 UserAccessHtmlExportServiceTests pass. Full suite: 302 passed, 26 skipped. + +## Decisions Made + +1. **Early-return + private method** — Same pattern as Plan 01's consolidated CSV path. `BuildConsolidatedHtml` is extracted as a private method to keep `BuildHtml` clean. The branch guarantees the existing code path is never reached when `mergePermissions=true`. + +2. **Separate locIdx counter** — RESEARCH.md Pitfall 2 explicitly warned about ID collision between user group headers and location sub-rows. Used distinct `int grpIdx` (for `ugrp{n}`) and `int locIdx` (for `loc{n}`) to prevent any overlap. + +3. **Named parameter fix for branding test** — The existing `BuildHtml_WithBranding_ContainsLogoImg` test called `BuildHtml(entries, MakeBranding(...))` positionally. Inserting `mergePermissions` before `branding` broke that call — fixed by adding `branding:` named argument. This is a Rule 1 auto-fix (broken test). + +## Verification Results + +- `dotnet build` — 0 errors, 0 warnings +- `dotnet test --filter "FullyQualifiedName~UserAccessHtmlExportServiceTests"` — 12/12 passed +- `dotnet test` full suite — 302 passed, 26 skipped, 0 failed (no regressions) +- Test count increased by 4 from Plan 02 (RPT-03-b through RPT-03-e) + 2 previously flaky tests now stable + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Fixed branding test call site broken by parameter insertion** +- **Found during:** Task 1 (GREEN phase — first test run) +- **Issue:** Existing `BuildHtml_WithBranding_ContainsLogoImg` test passed `MakeBranding(...)` as positional argument 2, which collided with the newly inserted `bool mergePermissions` parameter at position 2 +- **Fix:** Added named `branding:` argument: `svc.BuildHtml(new[] { DefaultEntry }, branding: MakeBranding(msp: true))` +- **Files modified:** SharepointToolbox.Tests/Services/Export/UserAccessHtmlExportServiceTests.cs +- **Commit:** 0ebe707 + +## Self-Check: PASSED + +Files exist: +- SharepointToolbox/Services/Export/UserAccessHtmlExportService.cs — contains `mergePermissions`, `BuildConsolidatedHtml`, `PermissionConsolidator.Consolidate` +- SharepointToolbox/ViewModels/Tabs/UserAccessAuditViewModel.cs — contains `MergePermissions` in WriteAsync call +- SharepointToolbox.Tests/Services/Export/UserAccessHtmlExportServiceTests.cs — contains 4 new consolidation test methods + +Commits exist: 3d95d2a (RED), 0ebe707 (GREEN)