feat: typed enum for OSCAL implementation status state#383
Conversation
Replace free-form string State on ImplementationStatus with a well-defined ImplementationStatusState type constrained to the five OSCAL SSP states: implemented, partial, planned, alternative, not-applicable. - Add ImplementationStatusState type, constants, and validation - Add Validate() method on ImplementationStatus - Add validation in Create/Update ByComponent handler endpoints - Ensure serialization/deserialization remains OSCAL JSON compatible - Add comprehensive tests for valid/invalid states, omitted status, marshal/unmarshal round-trips, and JSON compatibility - Fix existing test using non-lowercase 'Implemented' state Co-Authored-By: Gustavo Fernandes <gusfcarvalho@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
There was a problem hiding this comment.
Pull request overview
This PR tightens OSCAL SSP implementation-status.state handling by replacing a free-form string with a typed enum, and enforcing valid values on the ByComponent create/update API paths while preserving OSCAL JSON round-trip compatibility.
Changes:
- Introduces
ImplementationStatusState+ constants and refactorsImplementationStatusto use the typed state with aValidate()method. - Adds request-time validation in the three ByComponent create/update handlers to reject invalid states with HTTP 400.
- Updates/extends unit and integration tests to cover enum validity and JSON marshal/unmarshal round-trips.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/service/relational/system_security_plan.go | Adds the typed enum + validation and updates OSCAL marshal/unmarshal conversions for ImplementationStatus. |
| internal/service/relational/system_security_plan_test.go | Adds unit tests for valid/invalid states and JSON compatibility/round-trips. |
| internal/service/relational/system_component_suggestions.go | Switches hard-coded "implemented" to the typed constant. |
| internal/api/handler/oscal/system_security_plans.go | Adds validation hook for ByComponent create/update flows. |
| internal/api/handler/oscal/system_security_plans_test.go | Fixes integration test input to use lowercase "implemented". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review feedback: when the implementation-status field is present in the request but has an empty state, return 400 instead of silently allowing it. Only skip validation when the entire field is absent (zero JSONType). Add integration test covering invalid state, empty state with remarks, and omitted implementation-status scenarios. Co-Authored-By: Gustavo Fernandes <gusfcarvalho@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix compilation error in TestCreateByComponentInvalidImplementationStatus by using the correct test helper and adding missing handler registration. Co-Authored-By: Gustavo Fernandes <gusfcarvalho@gmail.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Replaces the free-form
stringtype onImplementationStatus.Statewith a well-definedImplementationStatusStateenum constrained to the five OSCAL SSP states:implemented— the control is fully implementedpartial— the control is partially implementedplanned— there is a plan for implementing the controlalternative— there is an alternative implementationnot-applicable— the control does not apply to this systemChanges:
ImplementationStatusStatewith five valid constants and aValidate()method onImplementationStatusUpdateImplementedRequirementByComponent,UpdateImplementedRequirementStatementByComponent,CreateImplementedRequirementStatementByComponent) now validate the implementation status state, returning 400 for invalid valuessystem_component_suggestions.goto use the typed constant, fixed existing test using non-lowercase"Implemented"stateReview & Testing Checklist for Human
make oscal import) still loads documents withimplementation-statusfields correctly — the deserialization accepts any string from existing data, but new writes are validatedimplemented,partial,planned,alternative,not-applicable) and confirm 200/201 responses"Implemented","invalid") and confirm a 400 response with a clear error messageimplementation-statusentirely in a ByComponent request still works (no validation error)Notes
ImplementationStatusstruct is now a standalone type (no longertype ImplementationStatus oscalTypes_1_1_3.ImplementationStatus) to allow using the typedImplementationStatusStatefor theStatefield while keeping JSON tags identical for OSCAL compatibilityLink to Devin session: https://app.devin.ai/sessions/e6bb63391665462eb8f427a9cbd8bcf5
Requested by: @gusfcarvalho