From 607da7ae476deab17c84ffa9fd67acc56654b8a3 Mon Sep 17 00:00:00 2001 From: NguyenND Date: Mon, 17 Aug 2026 16:46:26 -0400 Subject: [PATCH] Aug 17 - Update customer inspector can update issue status --- JanitorialQC/CLAUDE.md | 19 +++++++++ JanitorialQC/Utils/Constants.swift | 40 +++++++++++++++++++ JanitorialQC/Views/Dashboard/IssuesView.swift | 13 +++--- .../Inspection/InspectionHistoryView.swift | 7 +++- 4 files changed, 72 insertions(+), 7 deletions(-) diff --git a/JanitorialQC/CLAUDE.md b/JanitorialQC/CLAUDE.md index a366c69..323d3b6 100644 --- a/JanitorialQC/CLAUDE.md +++ b/JanitorialQC/CLAUDE.md @@ -295,6 +295,25 @@ let x = all.filter { ... } --- +### Role gates — `Constants.Roles` (Aug 2026) + +**Never write `role == "inspector"` in a view.** `external_inspector` ("Customer Inspector" — an inspector employed by the customer) has the same powers as our own `inspector` and the API scopes it identically, so a literal equality check locks that account out of actions the server would happily accept. It fails **silently**: no error, no 403 to debug — the control simply is not drawn. + +That is exactly what happened to Update Status, Handled By and Start Follow-up, which were three separate hand-written lists in two files: + +| Site | Was | Now | +|---|---|---| +| `IssuesView.canUpdateStatus` | `admin \| director \| inspector` | `Constants.Roles.issueActors` | +| `IssuesView.canEditHandler` | `admin \| director \| inspector \| project_manager` | same | +| `InspectionHistoryView.canStartFollowUp` | `admin \| director \| inspector \| project_manager` | same | + +`Constants.Roles` in `Utils/Constants.swift` is the single definition, mirroring `User.INSPECTOR_ROLES` / `User.is_inspector` on the server (server rule 87): + +- `inspectorRoles` = `{inspector, external_inspector}` — test membership, never `==`. +- `issueActors` = `{admin, director, project_manager} ∪ inspectorRoles` — a **subset** of the API's `_ALLOWED_ROLES` for these endpoints, so every role it admits is one the server accepts. `auditor` is deliberately excluded (read-only in the app). + +The server stays the authority and additionally enforces facility scope; these gates only decide whether to draw the control. + ## 9. API Client (APIClient) `actor APIClient` — singleton via `APIClient.shared`. All methods are `async throws`. diff --git a/JanitorialQC/Utils/Constants.swift b/JanitorialQC/Utils/Constants.swift index 371d0fc..3fecf65 100644 --- a/JanitorialQC/Utils/Constants.swift +++ b/JanitorialQC/Utils/Constants.swift @@ -76,4 +76,44 @@ nonisolated enum Constants { } static let tokenRefreshBufferMinutes: Double = 5 + + // MARK: - Roles + // + // The server's role strings, and the ONE definition of which of them the + // app treats as an inspector. This mirrors `User.INSPECTOR_ROLES` / + // `User.is_inspector` in the Flask app (see server rule 87). + // + // Why this exists: `external_inspector` (displayed as "Customer Inspector" + // — an inspector employed by the customer) has exactly the same powers as + // our own `inspector`, and the API scopes it identically. The views here + // were hand-written as `role == "admin" || role == "director" || role == + // "inspector"`, so every one of them silently locked customer inspectors + // out of actions the SERVER was perfectly willing to accept — Update + // Status, Handled By, Start Follow-up. The failure is invisible: no error, + // the control simply isn't drawn. + // + // Add a role in ONE place here; never re-write the literals in a view. + nonisolated enum Roles { + static let admin = "admin" + static let director = "director" + static let projectManager = "project_manager" + static let auditor = "auditor" + static let inspector = "inspector" + /// "Customer Inspector" — employed by the customer, same powers as + /// `inspector`, scoped to their assigned contracts. + static let externalInspector = "external_inspector" + + /// Both inspector roles. Test membership of this, never `== inspector`. + static let inspectorRoles: Set = [inspector, externalInspector] + + /// May change an issue's status / handler, and start a follow-up. + /// Matches what the API actually accepts for these actions; the server + /// remains the authority and additionally enforces facility scope. + static let issueActors: Set = + [admin, director, projectManager].union(inspectorRoles) + + static func isInspector(_ role: String) -> Bool { + inspectorRoles.contains(role) + } + } } diff --git a/JanitorialQC/Views/Dashboard/IssuesView.swift b/JanitorialQC/Views/Dashboard/IssuesView.swift index f6b7bc0..36b577a 100644 --- a/JanitorialQC/Views/Dashboard/IssuesView.swift +++ b/JanitorialQC/Views/Dashboard/IssuesView.swift @@ -290,10 +290,15 @@ struct IssueDetailView: View { /// Inspector can update status only if the issue has synced (has a serverId) /// and we are online. Admins/directors can always update when online. + /// + /// Uses `Constants.Roles.issueActors` rather than a hand-written list: the + /// old `role == "inspector"` check silently excluded Customer Inspectors + /// (`external_inspector`), who the API has always accepted here — the + /// Update Status control simply never appeared for them, with no error to + /// explain why. private var canUpdateStatus: Bool { guard sync.isOnline, issue.serverId != nil else { return false } - let role = AuthManager.shared.currentUserRole - return role == "admin" || role == "director" || role == "inspector" + return Constants.Roles.issueActors.contains(AuthManager.shared.currentUserRole) } private let allStatuses: [(value: String, label: String, color: Color)] = [ @@ -308,9 +313,7 @@ struct IssueDetailView: View { /// admin/director/PM); the server enforces facility scope for inspectors. private var canEditHandler: Bool { guard sync.isOnline, issue.serverId != nil else { return false } - let role = AuthManager.shared.currentUserRole - return role == "admin" || role == "director" - || role == "inspector" || role == "project_manager" + return Constants.Roles.issueActors.contains(AuthManager.shared.currentUserRole) } private func handlerTypeLabel(_ type: String) -> String { diff --git a/JanitorialQC/Views/Inspection/InspectionHistoryView.swift b/JanitorialQC/Views/Inspection/InspectionHistoryView.swift index c470b6b..6b3adb9 100644 --- a/JanitorialQC/Views/Inspection/InspectionHistoryView.swift +++ b/JanitorialQC/Views/Inspection/InspectionHistoryView.swift @@ -371,9 +371,12 @@ struct HistoryDetailView: View { /// Auditors are read-only everywhere else and the API rejects them (403), /// so the two action buttons are hidden rather than shown failing. + /// + /// `issueActors` is the same set minus auditor, and — unlike the literal + /// list this replaced — it includes Customer Inspectors, who perform + /// inspections exactly as our own do. private var canStartFollowUp: Bool { - ["admin", "director", "inspector", "project_manager"] - .contains(auth.currentUserRole) + Constants.Roles.issueActors.contains(auth.currentUserRole) } // Local SwiftData copy — used only for follow-up sync-back.