Code Quality: inconsistent module-scope var usage, unprotected Graph calls, duplicated AppProtection URI logic, oversized functions

[ugurkocde]

Review of the v4.0 module in Module/IntuneAssignmentChecker/ (introduced in b2378c5). Findings are ordered by priority.

1. Error handling — unprotected Invoke-MgGraphRequest calls in main category loops

Several mandatory category fetches run bare Invoke-MgGraphRequest without a try/catch. A single transient network blip or throttling response aborts the entire user/device/group run partway through, leaving no partial results.

Representative locations:

• Module/IntuneAssignmentChecker/Public/Get-IntuneUserAssignment.ps1:227-233 — Applications fetch + @odata.nextLink pagination.
• Module/IntuneAssignmentChecker/Public/Get-IntuneUserAssignment.ps1:246-247 — per-app assignments fetch.
• Same pattern repeats in Get-IntuneAllPolicies.ps1, Get-IntuneAllUsersAssignment.ps1, Get-IntuneAllDevicesAssignment.ps1, Get-IntuneDeviceAssignment.ps1, Get-IntuneGroupAssignment.ps1, Test-IntuneGroupMembership.ps1, Test-IntuneGroupRemoval.ps1, Search-IntunePolicy.ps1.

Compare with the app-protection block at Get-IntuneUserAssignment.ps1:162-207, which already wraps its request in try/catch and reports a per-policy error without aborting the run. The applications block should follow the same pattern (continue to the next category on failure, log the error).

2. Inconsistent $script:GraphEndpoint scoping — fragile

IntuneAssignmentChecker.psm1:5 declares $script:GraphEndpoint. Some files correctly reference $script:GraphEndpoint (Connect-IntuneAssignmentChecker.ps1, Set-Environment.ps1, Update-IntuneSettingDefinition.ps1), but many files read the bare $GraphEndpoint and rely on the scope-chain fallback to find the module-scoped value:

• Module/IntuneAssignmentChecker/Private/Get-IntuneEntities.ps1:18,22
• Module/IntuneAssignmentChecker/Public/Compare-IntuneGroupAssignment.ps1 — 22 occurrences
• Module/IntuneAssignmentChecker/Public/Test-IntuneGroupMembership.ps1 — 12 occurrences
• Module/IntuneAssignmentChecker/Public/Test-IntuneGroupRemoval.ps1 — 12 occurrences
• Module/IntuneAssignmentChecker/Public/Get-IntuneAllDevicesAssignment.ps1 — 11
• Module/IntuneAssignmentChecker/Public/Get-IntuneAllUsersAssignment.ps1 — 11
• Plus 10 other Public/Private files.

This works today, but a local $GraphEndpoint = … introduced anywhere up the call stack would silently shadow the module value and quietly route traffic to the wrong cloud. Make all reads explicit: $script:GraphEndpoint.

3. Quality — $totalCategories magic number drifts from actual step count

Module/IntuneAssignmentChecker/Public/Get-IntuneUserAssignment.ps1:82 sets $totalCategories = 16. The $relevantPolicies hashtable immediately below (lines 86–107) declares 20 named categories, and there are 16 $currentCategory++ calls, so the progress bar is internally consistent but the relationship is invisible — exactly how commit b2378c5 recently had to re-adjust from 17 → 16.

The numbers also disagree between cmdlets that do essentially the same work:

• Get-IntuneUserAssignment.ps1:82 → 16
• Test-IntuneGroupMembership.ps1:139 → 18
• Test-IntuneGroupRemoval.ps1:140 → 18
• Search-IntunePolicy.ps1:33 → 18

Either derive the total from the list of steps ($steps.Count) or assemble the category list once and iterate, so the counter can't drift.

4. Quality — App Protection assignment URI switch duplicated across 11 files

The 3-case switch that maps @odata.type → android/ios/windowsManagedAppProtections/.../assignments appears in every cmdlet that walks App Protection policies:

• Public/Get-IntuneUserAssignment.ps1:154-159
• Public/Get-IntuneDeviceAssignment.ps1
• Public/Get-IntuneGroupAssignment.ps1
• Public/Get-IntuneAllPolicies.ps1
• Public/Get-IntuneAllUsersAssignment.ps1
• Public/Get-IntuneAllDevicesAssignment.ps1
• Public/Get-IntuneEmptyGroup.ps1:106-108
• Public/Get-IntuneUnassignedPolicy.ps1
• Public/Search-IntunePolicy.ps1
• Public/Test-IntuneGroupMembership.ps1
• Public/Test-IntuneGroupRemoval.ps1
• html-export.ps1:684-690

Extract a single Get-AppProtectionAssignmentUri -PolicyType -PolicyId private helper. Any future AppProtection subtype (e.g. mdmWindowsInformationProtection) then needs a one-place change.

5. Quality — oversized functions

PowerShell style guides commonly flag functions >200 lines as candidates for decomposition. Current Public/ sizes:

File	Lines
Get-IntuneUserAssignment.ps1	1422
Get-IntuneDeviceAssignment.ps1	990
Test-IntuneGroupMembership.ps1	955
Test-IntuneGroupRemoval.ps1	947
Get-IntuneGroupAssignment.ps1	685
Get-IntuneAllUsersAssignment.ps1	685
Get-IntuneAllDevicesAssignment.ps1	668
Get-IntuneAllPolicies.ps1	667
Compare-IntuneGroupAssignment.ps1	665
Search-IntunePolicy.ps1	515

These files implement near-identical "walk all 16–18 category endpoints and collect matches" pipelines with small differences in the match predicate. Consider extracting a shared pipeline (list-of-categories → per-category Invoke-CategoryScan -Predicate {…}) into Private/ so each public cmdlet shrinks to the bit that differs: its predicate, its printing, and its export columns. This will also remove most of the duplicated $totalCategories/App-Protection URI/bare $GraphEndpoint instances in one pass.

---

Nothing here is a user-visible bug today, but the combination (fragile scoping, duplicated switches, unprotected network calls) is exactly what made the hardcoded graph.microsoft.com / totalCategories drift easy to miss until the USGov fix in #115.

[ugurkocde]

Good analysis. I dug into the code to validate each point — here's a prioritized action plan with specific file/line references and suggested fixes.

---

1. Unprotected Invoke-MgGraphRequest calls

Confirmed. The App Protection block (Get-IntuneUserAssignment.ps1:162-207) correctly wraps its call in try/catch and continues on failure. But the Applications block right below (Get-IntuneUserAssignment.ps1:227-233) runs bare:

# Line 227-233 — no try/catch
$appUri = "$script:GraphEndpoint/beta/deviceAppManagement/mobileApps?`$filter=isAssigned eq true"
$appResponse = Invoke-MgGraphRequest -Uri $appUri -Method Get
$allApps = $appResponse.value
while ($appResponse.'@odata.nextLink') {
    $appResponse = Invoke-MgGraphRequest -Uri $appResponse.'@odata.nextLink' -Method Get
    $allApps += $appResponse.value
}

Fix: Wrap each category fetch in try/catch, log the error, and continue to the next category. Follow the existing pattern from lines 162-207. This applies to all the files listed in the issue (Get-IntuneAllPolicies.ps1, Get-IntuneAllUsersAssignment.ps1, Get-IntuneAllDevicesAssignment.ps1, Get-IntuneDeviceAssignment.ps1, Get-IntuneGroupAssignment.ps1, Test-IntuneGroupMembership.ps1, Test-IntuneGroupRemoval.ps1, Search-IntunePolicy.ps1).

---

2. Inconsistent $script:GraphEndpoint scoping

Confirmed. IntuneAssignmentChecker.psm1:5 declares $script:GraphEndpoint = $null, and files like Connect-IntuneAssignmentChecker.ps1 and Set-Environment.ps1 correctly use $script:GraphEndpoint. But many files use the bare $GraphEndpoint:

• Private/Get-IntuneEntities.ps1:18,22
• Public/Compare-IntuneGroupAssignment.ps1 — lines 67, 114, 169, 182, and ~18 more
• Public/Test-IntuneGroupMembership.ps1 — lines 90, 222-224, 280, 298, and ~6 more
• Public/Test-IntuneGroupRemoval.ps1 — similar pattern

Fix: Global find-and-replace "$GraphEndpoint/ → "$script:GraphEndpoint/ across all .ps1 files under Module/IntuneAssignmentChecker/. This is safe because there are no local $GraphEndpoint assignments anywhere — every usage is reading the module-scoped value:

# In each affected file, change:
$baseUri = "$GraphEndpoint/beta"
# To:
$baseUri = "$script:GraphEndpoint/beta"

---

3. $totalCategories magic number drift

Confirmed. Current values:

File	Value	Actual $currentCategory++ calls
Get-IntuneUserAssignment.ps1:82	16	16
Test-IntuneGroupMembership.ps1:139	18	18
Test-IntuneGroupRemoval.ps1:140	18	18
Search-IntunePolicy.ps1:33	18	18

The numbers happen to match their respective increment counts today, but the $relevantPolicies hashtable at Get-IntuneUserAssignment.ps1:86-107 has 20 keys while the progress total is 16 — confusing because some categories (like Applications) are counted as a single step despite spanning three hashtable keys (Required/Available/Uninstall).

Suggested fix: Define a $steps array and derive the count:

$steps = @(
    "Device Configurations",
    "Settings Catalog Policies",
    "Compliance Policies",
    "App Protection Policies",
    # ... etc
)
$totalCategories = $steps.Count

Then use $steps[$currentCategory] for the progress label. This eliminates the magic number entirely.

---

4. Duplicated App Protection URI switch

Confirmed across 11+ files. The 3-case switch mapping @odata.type → assignment URI appears in every cmdlet. Example from Get-IntuneUserAssignment.ps1:154-159:

$assignmentsUri = switch ($policyType) {
    "#microsoft.graph.androidManagedAppProtection" { "…/androidManagedAppProtections('$($policy.id)')/assignments" }
    "#microsoft.graph.iosManagedAppProtection" { "…/iosManagedAppProtections('$($policy.id)')/assignments" }
    "#microsoft.graph.windowsManagedAppProtection" { "…/windowsManagedAppProtections('$($policy.id)')/assignments" }
    default { $null }
}

Fix: Extract to a private helper:

# Private/Get-AppProtectionAssignmentUri.ps1
function Get-AppProtectionAssignmentUri {
    param(
        [string]$PolicyType,
        [string]$PolicyId
    )
    switch ($PolicyType) {
        "#microsoft.graph.androidManagedAppProtection"  { "$script:GraphEndpoint/beta/deviceAppManagement/androidManagedAppProtections('$PolicyId')/assignments" }
        "#microsoft.graph.iosManagedAppProtection"      { "$script:GraphEndpoint/beta/deviceAppManagement/iosManagedAppProtections('$PolicyId')/assignments" }
        "#microsoft.graph.windowsManagedAppProtection"  { "$script:GraphEndpoint/beta/deviceAppManagement/windowsManagedAppProtections('$PolicyId')/assignments" }
        default { $null }
    }
}

Then every call site simplifies to:

$assignmentsUri = Get-AppProtectionAssignmentUri -PolicyType $policyType -PolicyId $policy.id

---

5. Oversized functions — suggested decomposition

This is the largest effort but would resolve points 2, 3, and 4 simultaneously. The core pattern shared by all 10 oversized files is: iterate category list → fetch → filter by predicate → collect results.

Recommended approach: Extract a shared Invoke-CategoryScan private helper that takes a list of category definitions (endpoint, label) and a predicate scriptblock. Each public cmdlet would then only define its unique predicate + output formatting, shrinking from 600-1400 lines to ~100-200.

I'd suggest tackling these in order: fix #2 first (scope prefix — straightforward find/replace), then #4 (helper extraction), then #1 (try/catch wrapping), and finally #3 and #5 together as a refactor pass.

---

Generated by Claude Code