fix(assets): distinguish loading / empty / failed in Change History panel (MAPPS-304) #297
No reviewers
Labels
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
psa-systems/mokosh-apps!297
Loading…
Reference in a new issue
No description provided.
Delete branch "fix/MAPPS-304-asset-change-history-states"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Before, the asset Change History panel collapsed every fetch outcome through
unwrap_or_default(), so a 403 from theRequireAdmingate on/assets/{id}/audit-log(which a non-admin role hits) rendered the same "No history yet" string as a genuinely empty audit log. That misled the QA report into thinking change-tracking was broken when the panel was actually permission-denied. The auditResourcenow carriesResult<Vec<AuditEntry>, String>and the render picks one of three states (Loading / Ready / Failed), with the failure branch showing the underlying error so a real fetch failure is visible and actionable instead of disguised as empty. The "Edited by X" header label reads through the same projection and only renders when the latest entry actually loaded. PMS-447 (single-tenancy admin floor) stops most users from hitting the 403 in the first place; this change handles the remaining failure cases (network drop, future role downgrade) honestly.The companion "spurious error toast on every successful update" half of the ticket still needs a fresh reproduction with the MAPPS-299 panic-hook PR deployed: the asset PUT path is
put_authed::<serde_json::Value, _>againstPUT /assets/{id}which returnsJson<AssetResponse>on 200 (not 204), andValuedeserialises any JSON shape, so the failure case is not visible from static analysis. Tracking that follow-up on the ticket.#MAPPS-304
Before, the asset Change History panel collapsed every fetch outcome through `unwrap_or_default()`, so a 403 from the `RequireAdmin` gate on `/assets/{id}/audit-log` (which a non-admin role hits) rendered the same "No history yet" string as a genuinely empty audit log. That misled the QA report into thinking change-tracking was broken when the panel was actually permission-denied. The audit `Resource` now carries `Result<Vec<AuditEntry>, String>` and the render picks one of three states (Loading / Ready / Failed), with the failure branch showing the underlying error so a real fetch failure is visible and actionable instead of disguised as empty. The "Edited by X" header label reads through the same projection and only renders when the latest entry actually loaded. PMS-447 (single-tenancy admin floor) stops most users from hitting the 403 in the first place; this change handles the remaining failure cases (network drop, future role downgrade) honestly. The companion "spurious error toast on every successful update" half of the ticket still needs a fresh reproduction with the MAPPS-299 panic-hook PR deployed: the asset PUT path is `put_authed::<serde_json::Value, _>` against `PUT /assets/{id}` which returns `Json<AssetResponse>` on 200 (not 204), and `Value` deserialises any JSON shape, so the failure case is not visible from static analysis. Tracking that follow-up on the ticket. #MAPPS-304