kawa/Sharepoint-Toolbox
1
0
Fork 0
Code Issues Pull Requests Actions Releases 9 Wiki Activity
Files
0336f4341fd31594799f08c84e3a99a8c6bf7bfa
Sharepoint-Toolbox/.planning/phases/16-report-consolidation-toggle/16-RESEARCH.md
Dev 0336f4341f docs(phase-16): research report consolidation toggle 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-09 12:12:13 +02:00

24 KiB
Raw Blame History

Phase 16: Report Consolidation Toggle - Research

Researched: 2026-04-09 Domain: WPF MVVM toggle / HTML export rendering / CSV export extension Confidence: HIGH

Summary

Phase 16 wires the PermissionConsolidator.Consolidate() helper from Phase 15 into the actual export flow via a user-visible checkbox. All foundational models (ConsolidatedPermissionEntry, LocationInfo, UserAccessEntry) and the consolidation algorithm are complete and tested. This phase is purely integration work: add a MergePermissions bool property to the ViewModel, bind a new "Export Options" GroupBox in both XAML views, and branch inside BuildHtml / BuildCsv / WriteSingleFileAsync based on that flag.

The consolidation path for HTML requires rendering a different by-user table structure — one row per ConsolidatedPermissionEntry with a "Sites" column that shows either an inline site title (1 location) or an [N sites] badge that expands an inline sub-list via the existing toggleGroup() JS. The by-site view must be hidden when consolidation is ON. The CSV path replaces flat per-user rows with a "Locations" column that lists all site titles separated by semicolons (or similar). The by-site view is disabled by hiding/graying the view-toggle buttons in the HTML; the ViewModel IsGroupByUser WPF DataGrid grouping is unrelated (it controls the WPF DataGrid, not the HTML export).

The session-global toggle lives as a single [ObservableProperty] bool _mergePermissions on UserAccessAuditViewModel. The PermissionsViewModel receives the same bool property as a no-op placeholder. Because both ViewModels are independent instances (no shared service is required), the CONTEXT.md decision of "session-scoped global setting" is implemented by putting the property in the ViewModel and never persisting it — it resets to false on app restart by default initialization.

Primary recommendation: Add MergePermissions to UserAccessAuditViewModel, bind it in both XAML tabs, pass it to BuildHtml/BuildCsv, and add a consolidated rendering branch inside those services. Keep the non-consolidated path byte-identical to the current output (no structural changes to the existing code path).

<user_constraints>

User Constraints (from CONTEXT.md)

Locked Decisions

Decision Value
Consolidation scope User access audit report only — site-centric permission report is unchanged
Consolidation key UserLogin + PermissionLevel + AccessType + GrantedThrough (4-field match)
Source model UserAccessEntry (already normalized, one user per row)
Consolidation defaults OFF Toggle must default to unchecked
No API calls Pure data transformation via PermissionConsolidator.Consolidate()
Existing exports unchanged when OFF Output must be identical to pre-v2.3 when toggle is OFF

Discussed (Locked)

  • Toggle Placement: New "Export Options" GroupBox in the left panel of both audit tabs, always visible; single CheckBox labeled "Merge duplicate permissions"; follows the same pattern as the existing "Scan Options" GroupBox (lines 199-210 of UserAccessAuditView.xaml)
  • Toggle applies to both HTML and CSV exports (user override of REQUIREMENTS.md CSV exclusion)
  • Consolidated HTML rendering: by-user view consolidated rows with "Sites" column; 1 location = inline title; 2+ locations = [N sites] badge expanding inline sub-list via existing toggleGroup() JS pattern; by-site view is disabled/hidden when consolidation is ON
  • Session persistence: session-scoped property on ViewModel — resets to OFF on app restart; global state shared across all tabs (both ViewModels read/write same logical property)
  • PermissionsViewModel: toggle present in UI but no-op — stores value, does not apply consolidation

Claude's Discretion

  • Exact CSS for the [N sites] badge (follow existing .badge class style already in UserAccessHtmlExportService.cs)
  • CSV consolidated column format (semicolon-separated site titles is the most natural approach)
  • Whether BuildHtml takes a bool mergePermissions parameter or a richer options object (bool parameter is simpler given only one flag at this phase)

Deferred Ideas (OUT OF SCOPE)

  • Site-centric consolidation logic (toggle present in UI but no-op for site-centric exports)
  • Group expansion within consolidated rows (Phase 17)
  • Persistent consolidation preference across app restarts (session-only for now)
  • "Consolidated view" report header indicator (decided: not needed) </user_constraints>

<phase_requirements>

Phase Requirements

ID Description Research Support
RPT-03 User can enable/disable entry consolidation per export (toggle in export settings) Toggle as [ObservableProperty] bool _mergePermissions in UserAccessAuditViewModel; PermissionsViewModel has same property as placeholder; both XAML views get "Export Options" GroupBox; BuildHtml and BuildCsv/WriteSingleFileAsync accept bool mergePermissions and branch accordingly
</phase_requirements>

Standard Stack

Core

Library Version Purpose Why Standard
CommunityToolkit.Mvvm already referenced [ObservableProperty] source gen for MergePermissions bool Same pattern used for IncludeInherited, ScanFolders, IncludeSubsites throughout both ViewModels
WPF / XAML .NET 10 Windows GroupBox + CheckBox binding Established UI pattern in this codebase
PermissionConsolidator.Consolidate() Phase 15 (complete) Transforms IReadOnlyList<UserAccessEntry> → IReadOnlyList<ConsolidatedPermissionEntry> Purpose-built for this exact use case

No New Dependencies

Phase 16 adds zero new NuGet packages. All required types and services are already present.

Architecture Patterns

Recommended Project Structure Changes

SharepointToolbox/
├── ViewModels/Tabs/
│   ├── UserAccessAuditViewModel.cs   ← add MergePermissions [ObservableProperty]
│   └── PermissionsViewModel.cs       ← add MergePermissions [ObservableProperty] (no-op)
├── Views/Tabs/
│   ├── UserAccessAuditView.xaml      ← add Export Options GroupBox after Scan Options GroupBox
│   └── PermissionsView.xaml          ← add Export Options GroupBox after Display Options GroupBox
└── Services/Export/
    ├── UserAccessHtmlExportService.cs ← BuildHtml(entries, mergePermissions, branding)
    └── UserAccessCsvExportService.cs  ← BuildCsv(…, mergePermissions) + WriteSingleFileAsync(…, mergePermissions)

SharepointToolbox.Tests/
├── Services/Export/
│   └── UserAccessHtmlExportServiceTests.cs  ← add consolidated path tests
│   └── UserAccessCsvExportServiceTests.cs   ← add consolidated path tests
└── ViewModels/
    └── UserAccessAuditViewModelTests.cs     ← add MergePermissions property test

Pattern 1: ObservableProperty Bool on ViewModel (CommunityToolkit.Mvvm)

What: Declare a source-generated bool property that defaults to false. When to use: Every scan/export option toggle in this codebase uses this pattern.

// In UserAccessAuditViewModel.cs — matches existing IncludeInherited / ScanFolders pattern
[ObservableProperty]
private bool _mergePermissions;

The source generator emits MergePermissions property with OnPropertyChanged. No partial handler needed unless additional side effects are required (none needed here).

Pattern 2: GroupBox + CheckBox in XAML (following Scan Options pattern)

What: A new GroupBox labeled "Export Options" with a single CheckBox, placed below Scan Options in the left panel DockPanel. Reference location: UserAccessAuditView.xaml lines 199-210 — the existing "Scan Options" GroupBox.

<!-- Export Options (always visible) — add after Scan Options GroupBox -->
<GroupBox Header="Export Options"
          DockPanel.Dock="Top" Margin="0,0,0,8" Padding="8">
  <StackPanel>
    <CheckBox Content="Merge duplicate permissions"
              IsChecked="{Binding MergePermissions}" />
  </StackPanel>
</GroupBox>

Localization note: The project uses TranslationSource.Instance for all user-visible strings. New keys are required: audit.grp.export and chk.merge.permissions in both Strings.resx and Strings.fr.resx.

Pattern 3: Export Service Signature Extension

What: Add bool mergePermissions = false parameter to BuildHtml, BuildCsv, and WriteSingleFileAsync. Callers in the ViewModel pass MergePermissions. Why a plain bool: Only one flag at this phase; using a richer options object adds indirection with no benefit yet.

// UserAccessHtmlExportService
public string BuildHtml(
    IReadOnlyList<UserAccessEntry> entries,
    bool mergePermissions = false,
    ReportBranding? branding = null)
{
    if (mergePermissions)
    {
        var consolidated = PermissionConsolidator.Consolidate(entries);
        return BuildConsolidatedHtml(consolidated, entries, branding);
    }
    // existing path unchanged
    ...
}

// UserAccessCsvExportService  
public async Task WriteSingleFileAsync(
    IReadOnlyList<UserAccessEntry> entries,
    string filePath,
    CancellationToken ct,
    bool mergePermissions = false)
{
    if (mergePermissions)
    {
        var consolidated = PermissionConsolidator.Consolidate(entries);
        // write consolidated CSV rows
    }
    // existing path unchanged
    ...
}

ViewModel call sites (in ExportHtmlAsync and ExportCsvAsync):

await _htmlExportService.WriteAsync(Results, dialog.FileName, CancellationToken.None, branding, MergePermissions);
await _csvExportService.WriteSingleFileAsync(Results, dialog.FileName, CancellationToken.None, MergePermissions);

Pattern 4: Consolidated HTML By-User View Structure

What: When mergePermissions = true, the by-user table renders one row per ConsolidatedPermissionEntry with a new "Sites" column replacing the "Site" column. The by-site view toggle is hidden.

Sites column rendering logic:

  • LocationCount == 1: render entry.Locations[0].SiteTitle as plain text (no badge/expand)
  • LocationCount >= 2: render <span class="badge sites-badge" onclick="toggleGroup('loc{idx}')">N sites</span> followed by sub-rows data-group="loc{idx}" each containing a link to the location's SiteUrl with SiteTitle label

Reuse of toggleGroup() JS: The existing toggleGroup(id) function (line 276 in UserAccessHtmlExportService.cs) hides/shows rows by data-group attribute. Expandable location sub-lists use the same mechanism — each sub-row gets data-group="loc{idx}" and starts hidden.

By-site view suppression: When consolidation is ON, omit the "By Site" button and view-site div from the HTML entirely (simplest approach — no dead HTML). Alternatively, render the button as disabled. Omitting is cleaner.

Column layout for consolidated by-user view:

Column Source Field
User UserDisplayName (+ Guest badge if IsExternalUser)
Permission Level PermissionLevel (+ high-priv icon if IsHighPrivilege)
Access Type badge from AccessType
Granted Through GrantedThrough
Sites inline title OR [N sites] badge + expandable sub-list

Pattern 5: Consolidated CSV Format

What: When consolidation is ON, each ConsolidatedPermissionEntry becomes one row with a "Locations" column containing all site titles joined by "; ".

Column layout for consolidated CSV:

"User","User Login","Permission Level","Access Type","Granted Through","Locations","Location Count"

The "Locations" value: string.Join("; ", entry.Locations.Select(l => l.SiteTitle)).

This is a distinct schema from the flat CSV, so the consolidated path writes its own header line.

Anti-Patterns to Avoid

  • Modifying the existing non-consolidated code path in any way — the success criterion is byte-for-byte identical output when toggle is OFF. Touch nothing in the existing rendering branches.
  • Using a shared service for MergePermissions state — CONTEXT.md says session-scoped property on the ViewModel, not a singleton service. Both ViewModels have independent instances; no cross-ViewModel coordination is needed because site-centric is a no-op.
  • Re-using IsGroupByUser to control by-site view in HTML — IsGroupByUser controls the WPF DataGrid grouping only; the HTML export is stateless and renders both views based solely on the mergePermissions flag.
  • Adding a partial handler for OnMergePermissionsChanged — no side effects are needed on toggle change (no view refresh required, HTML export is computed at export time).

Don't Hand-Roll

Problem Don't Build Use Instead Why
Permission grouping / merging Custom LINQ grouping PermissionConsolidator.Consolidate() Already unit-tested with 9 cases; handles case-insensitivity, ordering, LocationInfo construction
Toggle state persistence Session service, file-backed setting [ObservableProperty] bool _mergePermissions defaults to false Session-scoped = default bool initialization; no persistence infrastructure needed
Expandable sub-list JS New JS function Existing toggleGroup() in UserAccessHtmlExportService.cs Already handles show/hide of data-group rows; locations sub-list uses same mechanism with loc{idx} IDs

Common Pitfalls

Pitfall 1: Breaking the Byte-Identical Guarantee

What goes wrong: Any change to the non-consolidated HTML rendering path (even whitespace changes to sb.AppendLine calls) breaks the "byte-for-byte identical" success criterion. Why it happens: Developers add a parameter and reorganize the method, inadvertently altering the existing branches. How to avoid: Add the consolidated branch as a separate early-return path (if (mergePermissions) { ... return; }). Leave the existing StringBuilder building code completely untouched below that branch. Warning signs: A test that captures the current output (BuildHtml(entries, mergePermissions: false)) and compares it character-by-character to the pre-toggle output fails.

Pitfall 2: ID Collisions Between User Groups and Location Sub-lists

What goes wrong: The consolidated by-user view uses ugrp{n} IDs for user group headers and loc{n} IDs for expandable location sub-lists. If the counter resets between the two, toggleGroup clicks the wrong rows. Why it happens: Two independent counters that both start at 0 use different prefixes — this is fine — but if a single counter is used for both, collisions occur. How to avoid: Use a separate counter for location groups (int locIdx = 0) distinct from the user group counter.

Pitfall 3: WriteAsync Signature Mismatch

What goes wrong: UserAccessHtmlExportService.WriteAsync calls BuildHtml internally. If BuildHtml gets the new mergePermissions parameter but WriteAsync doesn't pass it through, the HTML file ignores the toggle. Why it happens: WriteAsync is the method called by the ViewModel; BuildHtml is the method being extended. How to avoid: Extend WriteAsync with the same bool mergePermissions = false parameter and pass it to BuildHtml.

Pitfall 4: CSV BuildCsv vs WriteSingleFileAsync

What goes wrong: The ViewModel calls WriteSingleFileAsync (single-file export). BuildCsv is per-user. Consolidation applies at the whole-collection level, so only WriteSingleFileAsync needs a consolidated path. Adding a consolidated path to BuildCsv (per-user) is incorrect — a consolidated row already spans users. Why it happens: Developer sees BuildCsv and WriteSingleFileAsync and tries to add consolidation to both. How to avoid: Add the consolidated branch only to WriteSingleFileAsync. BuildCsv is not called by the consolidation path. The per-user export (WriteAsync) is also not called by the ViewModel's export command (ExportCsvAsync calls WriteSingleFileAsync only).

Pitfall 5: Localization Keys Missing from French .resx

What goes wrong: New string keys added to Strings.resx but not to Strings.fr.resx cause TranslationSource.Instance[key] to return an empty string in French locale. Why it happens: Developers add only the default .resx. How to avoid: Add both audit.grp.export and chk.merge.permissions to both .resx files in the same task.

Code Examples

Current Export Call Sites (ViewModel — to be extended)

// ExportHtmlAsync — current (line 526 of UserAccessAuditViewModel.cs)
await _htmlExportService.WriteAsync(Results, dialog.FileName, CancellationToken.None, branding);

// ExportCsvAsync — current (line 494)
await _csvExportService.WriteSingleFileAsync(Results, dialog.FileName, CancellationToken.None);

After Phase 16, becomes:

await _htmlExportService.WriteAsync(Results, dialog.FileName, CancellationToken.None, branding, MergePermissions);
await _csvExportService.WriteSingleFileAsync(Results, dialog.FileName, CancellationToken.None, MergePermissions);

Existing toggleGroup JS (reuse as-is)

// Source: UserAccessHtmlExportService.cs inline JS, line 276-279
function toggleGroup(id) {
  var rows = document.querySelectorAll('tr[data-group="' + id + '"]');
  var isHidden = rows.length > 0 && rows[0].style.display === 'none';
  rows.forEach(function(r) { r.style.display = isHidden ? '' : 'none'; });
}

Note: the current source has "" doubled quotes in the selector ('tr[data-group=""' + id + '""]') — this is a C# string literal escaping artifact. When rendered to HTML the output is correct single-quoted attribute selectors. Location sub-list rows must use the same data-group attribute format.

Consolidated HTML Sites Column — location sub-rows pattern

<!-- 1 location: inline -->
<td>HR Site</td>

<!-- 2+ locations: badge + hidden sub-rows -->
<td><span class="badge" onclick="toggleGroup('loc3')" style="cursor:pointer">3 sites</span></td>
<!-- followed immediately after the main row: -->
<tr data-group="loc3" style="display:none">
  <td colspan="5" style="padding-left:2em">
    <a href="https://contoso.sharepoint.com/sites/hr">HR Site</a>
  </td>
</tr>
<tr data-group="loc3" style="display:none">
  <td colspan="5" style="padding-left:2em">
    <a href="https://contoso.sharepoint.com/sites/fin">Finance Site</a>
  </td>
</tr>

State of the Art

Old Approach Current Approach When Changed Impact
BuildHtml(entries, branding) BuildHtml(entries, mergePermissions, branding) Phase 16 Callers must pass the flag
WriteSingleFileAsync(entries, path, ct) WriteSingleFileAsync(entries, path, ct, mergePermissions) Phase 16 All callers (just the ViewModel) must pass the flag
No export option panel in XAML Export Options GroupBox visible in both tabs Phase 16 Both XAML views change; both ViewModels gain the property

Open Questions

  1. WriteAsync vs BuildHtml parameter ordering

    • What we know: WriteAsync current signature is (entries, filePath, ct, branding?) — branding is already optional
    • What's unclear: Should mergePermissions go before or after branding in the parameter list?
    • Recommendation: Place mergePermissions = false after ct and before branding? so it's grouped with behavioral flags, not rendering decorations: WriteAsync(entries, filePath, ct, mergePermissions = false, branding = null)

  2. French translation for "Merge duplicate permissions"

    • What we know: French .resx exists and all existing keys have French equivalents
    • What's unclear: Exact French translation (developer decision)
    • Recommendation: Use "Fusionner les permissions en double" — consistent with existing terminology in the codebase

Validation Architecture

Test Framework

Property Value
Framework xUnit (project: SharepointToolbox.Tests)
Config file SharepointToolbox.Tests/SharepointToolbox.Tests.csproj
Quick run command dotnet test --filter "FullyQualifiedName~UserAccessHtmlExportServiceTests" --no-build
Full suite command dotnet test

Phase Requirements → Test Map

Req ID Behavior Test Type Automated Command File Exists?
RPT-03-a MergePermissions defaults to false on ViewModel unit dotnet test --filter "FullyQualifiedName~UserAccessAuditViewModelTests" ✅ (extend existing)
RPT-03-b BuildHtml(entries, false) output is byte-identical to current output unit dotnet test --filter "FullyQualifiedName~UserAccessHtmlExportServiceTests" ✅ (extend existing)
RPT-03-c BuildHtml(entries, true) includes consolidated rows and "Sites" column unit dotnet test --filter "FullyQualifiedName~UserAccessHtmlExportServiceTests" ✅ (extend existing)
RPT-03-d BuildHtml(entries, true) with 2+ locations renders [N sites] badge unit dotnet test --filter "FullyQualifiedName~UserAccessHtmlExportServiceTests" ✅ (extend existing)
RPT-03-e BuildHtml(entries, true) omits by-site view toggle unit dotnet test --filter "FullyQualifiedName~UserAccessHtmlExportServiceTests" ✅ (extend existing)
RPT-03-f WriteSingleFileAsync(entries, path, ct, false) output unchanged unit dotnet test --filter "FullyQualifiedName~UserAccessCsvExportServiceTests" ✅ (extend existing)
RPT-03-g WriteSingleFileAsync(entries, path, ct, true) writes consolidated CSV rows unit dotnet test --filter "FullyQualifiedName~UserAccessCsvExportServiceTests" ✅ (extend existing)

Sampling Rate

  • Per task commit: dotnet test --filter "FullyQualifiedName~UserAccessHtmlExportServiceTests|FullyQualifiedName~UserAccessCsvExportServiceTests|FullyQualifiedName~UserAccessAuditViewModelTests" --no-build
  • Per wave merge: dotnet test
  • Phase gate: Full suite green before /gsd:verify-work

Wave 0 Gaps

None — existing test infrastructure covers all phase requirements. No new test files needed; existing test files are extended with new test methods.

Sources

Primary (HIGH confidence)

  • Direct inspection of UserAccessHtmlExportService.cs — full rendering pipeline, toggleGroup() JS, BuildHtml/WriteAsync signatures
  • Direct inspection of UserAccessCsvExportService.cs — BuildCsv, WriteSingleFileAsync, WriteAsync signatures
  • Direct inspection of UserAccessAuditViewModel.cs — [ObservableProperty] pattern, ExportCsvAsync, ExportHtmlAsync call sites
  • Direct inspection of PermissionsViewModel.cs — confirms independent ViewModel, no shared state service
  • Direct inspection of UserAccessAuditView.xaml — Scan Options GroupBox pattern at lines 199-210
  • Direct inspection of PermissionsView.xaml — Display Options GroupBox pattern and left panel structure
  • Direct inspection of PermissionConsolidator.cs + ConsolidatedPermissionEntry.cs + LocationInfo.cs — complete Phase 15 API
  • Direct inspection of PermissionConsolidatorTests.cs — 9 test cases confirming consolidator behavior
  • Direct inspection of Strings.resx — existing localization key naming convention

Secondary (MEDIUM confidence)

  • CommunityToolkit.Mvvm [ObservableProperty] source generation behavior — confirmed by existing usage of _includeInherited, _scanFolders, _mergePermissions pattern throughout codebase

Metadata

Confidence breakdown:

  • Standard stack: HIGH — all dependencies already in the project; no new libraries
  • Architecture: HIGH — all patterns directly observed in existing code
  • Pitfalls: HIGH — derived from direct reading of the code paths being modified
  • Test map: HIGH — existing test file locations and xUnit patterns confirmed

Research date: 2026-04-09 Valid until: 2026-05-09 (stable codebase, no fast-moving dependencies)

Reference in New Issue View Git Blame Copy Permalink