fix(sla): validate SLA target hours (non-negative, <=2dp, first_response<=resolution) (PMS-338) #255

Merged
vas2000-work merged 1 commit from fix/PMS-338-sla-target-validation into main 2026-06-16 01:43:56 +02:00
Owner

Background

UpsertSlaTargetRequest in src/modules/sla/models.rs accepted any first_response_hours / resolution_hours (both Option<Decimal>, DECIMAL(10,2) in the DB) with no validation at any layer: negative or zero hours, more than two decimal places, and an inverted pair (first-response deadline later than resolution) all passed through.

Goal

Reject incoherent SLA targets with a 422 at the request layer.

Proposed approach

Add validate_sla_target_hours in src/utils/validation.rs, mirroring the budget rule (validate_budget_hours) but requiring a strictly-positive value. A blank/None field already means "no target", so a stored 0 (an instantaneous deadline) is never the intent and is rejected; the validator also enforces at most two decimal places and the DECIMAL(10,2) magnitude bound. It is applied to both hours fields via #[validate(custom(...))]; on Option<Decimal> the validator runs on the inner value and skips None.

Add a schema-level #[validate(schema(function = validate_sla_target_range))] on the struct (mirroring validate_invoice_date_range's shape) that, when both hours are Some, rejects first_response_hours > resolution_hours.

Assumption (documented inline): strictly-positive hours required; blank/None means "no target".

Alternatives considered

Allowing zero hours was rejected as it encodes an instantaneous deadline that no SLA intends; None already expresses "no target". A DB-level CHECK alone was rejected because it would surface as a 500 rather than a 422.

Acceptance criteria

  • validate_sla_target_hours added to src/utils/validation.rs (strictly-positive, <=2dp, DECIMAL(10,2) bound).
  • Applied to first_response_hours and resolution_hours on UpsertSlaTargetRequest.
  • Schema-level validate_sla_target_range rejects first_response_hours > resolution_hours when both are present.
  • Unit tests: negative rejected, zero rejected, >2dp rejected, first_response>resolution rejected, valid pair passes.
  • cargo fmt --all --check, cargo test --lib, and cargo clippy --all-targets -- -D warnings pass.
## Background `UpsertSlaTargetRequest` in `src/modules/sla/models.rs` accepted any `first_response_hours` / `resolution_hours` (both `Option<Decimal>`, DECIMAL(10,2) in the DB) with no validation at any layer: negative or zero hours, more than two decimal places, and an inverted pair (first-response deadline later than resolution) all passed through. ## Goal Reject incoherent SLA targets with a 422 at the request layer. ## Proposed approach Add `validate_sla_target_hours` in `src/utils/validation.rs`, mirroring the budget rule (`validate_budget_hours`) but requiring a strictly-positive value. A blank/`None` field already means "no target", so a stored `0` (an instantaneous deadline) is never the intent and is rejected; the validator also enforces at most two decimal places and the DECIMAL(10,2) magnitude bound. It is applied to both hours fields via `#[validate(custom(...))]`; on `Option<Decimal>` the validator runs on the inner value and skips `None`. Add a schema-level `#[validate(schema(function = validate_sla_target_range))]` on the struct (mirroring `validate_invoice_date_range`'s shape) that, when both hours are `Some`, rejects `first_response_hours > resolution_hours`. Assumption (documented inline): strictly-positive hours required; blank/None means "no target". ## Alternatives considered Allowing zero hours was rejected as it encodes an instantaneous deadline that no SLA intends; `None` already expresses "no target". A DB-level CHECK alone was rejected because it would surface as a 500 rather than a 422. ## Acceptance criteria - [x] `validate_sla_target_hours` added to `src/utils/validation.rs` (strictly-positive, <=2dp, DECIMAL(10,2) bound). - [x] Applied to `first_response_hours` and `resolution_hours` on `UpsertSlaTargetRequest`. - [x] Schema-level `validate_sla_target_range` rejects `first_response_hours > resolution_hours` when both are present. - [x] Unit tests: negative rejected, zero rejected, >2dp rejected, first_response>resolution rejected, valid pair passes. - [x] `cargo fmt --all --check`, `cargo test --lib`, and `cargo clippy --all-targets -- -D warnings` pass.
fix(sla): validate SLA target hours (non-negative, <=2dp, first_response<=resolution)
All checks were successful
E2E / Playwright against staging (pull_request) Successful in 38s
Check / fmt + clippy + compile + unit/doc tests (pull_request) Successful in 1m5s
Integration / integration tests (pull_request) Successful in 3m1s
1cf55058d8
SLA targets accepted negative, zero, and over-precise hours and an inverted first_response/resolution pair at every layer; a new validate_sla_target_hours custom validator (mirroring the budget rule, but requiring strictly-positive hours since blank already means "no target") is applied to both hours fields and a schema-level validate_sla_target_range rejects first_response_hours > resolution_hours when both are present.

#PMS-338

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!255
No description provided.