lisa/feat/ui-create-ssp#254
Conversation
|
Caution Review failedPull request was closed or merged during review Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a create route and SystemSecurityPlanCreateView with UUID generation/regeneration, title/version inputs, POST submission to /api/oscal/system-security-plans (decamelized), toast error handling, isSubmitting guard, navigation to the editor on success, and tests for routing, creation, list navigation, and test harness updates. ChangesSystem Security Plan Creation Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant CreateView as SystemSecurityPlanCreateView
participant API as /api/oscal/system-security-plans
participant Router
participant Toast
User->>CreateView: Mount (UUID generated)
User->>CreateView: Regenerate UUID
User->>CreateView: Enter title + submit
CreateView->>API: POST { uuid, metadata.title } (decamelizeKeys)
alt Success (response.uuid)
API-->>CreateView: created plan with uuid
CreateView->>Router: push editor route with uuid
else Missing uuid
API-->>CreateView: created plan without uuid
CreateView->>Toast: show missing id error
else Validation error
API-->>CreateView: AxiosError with validation details
CreateView->>Toast: show validation error
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/views/system-security-plans/SystemSecurityPlanCreateView.vue`:
- Around line 6-7: The submit handler in SystemSecurityPlanCreateView.vue lacks
an in-flight guard causing duplicate POSTs; add a reactive flag (e.g., data
property or ref named isSubmitting) and update the submit method to return early
if isSubmitting is true, set isSubmitting = true before the async create call
and reset it in a finally block (try { await ... } finally { isSubmitting =
false }); also bind this flag to the form controls (disable the submit button
and any inputs, e.g., :disabled="isSubmitting") so rapid repeats are
prevented—apply the same pattern to the other submit paths referenced (the other
submit handlers around the component).
- Around line 73-76: The redirect after creation uses data.value?.uuid directly
when calling router.push (name: 'system-security-plan-editor') which can push an
invalid route if the UUID is missing; update the create/submit handler in
SystemSecurityPlanCreateView.vue to check that data.value and data.value.uuid
are present before calling router.push, and if missing avoid the navigation and
instead handle the failure (e.g., show a user-facing error/toast or navigate to
a safe fallback like the list view); ensure you reference the same router.push
call and data.value?.uuid symbols so the guard is applied at the exact
redirection site.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ea9b7c60-17df-4d07-8217-b4d63a83d065
📒 Files selected for processing (6)
src/router/__tests__/index.spec.tssrc/router/index.tssrc/views/__tests__/SystemSecurityPlanCreateView.spec.tssrc/views/__tests__/SystemSecurityPlanListView.spec.tssrc/views/system-security-plans/SystemSecurityPlanCreateView.vuesrc/views/system-security-plans/SystemSecurityPlanListView.vue
gusfcarvalho
left a comment
gusfcarvalho
left a comment
There was a problem hiding this comment.
Findings inline. The UUID field is missing required (Medium) and there is a silent undefined navigation risk on the redirect (Low). Two low-severity test concerns are also noted.
gusfcarvalho
left a comment
gusfcarvalho
left a comment
There was a problem hiding this comment.
All findings resolved. UUID required added; isSubmitting guard locks the form during submission and resets in finally; createdUuid guard prevents silent navigation to /undefined; __name assertion removed. CI green on 53edd51. Good to merge.
|
PR approved. Marking as ready for e2e. |
gusfcarvalho
left a comment
gusfcarvalho
left a comment
There was a problem hiding this comment.
The SSP creation form will always fail with from the API because is never initialised in the reactive ref.
Signed-off-by: Gustavo Carvalho <gustavo.carvalho@container-solutions.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/views/__tests__/PreferencesView.spec.ts`:
- Around line 88-95: Add a brief inline comment next to the tooltip directive
stub in the mountPreferencesView test helper explaining why it's stubbed (e.g.,
to avoid mounting the real tooltip directive and its DOM side-effects in unit
tests), so future maintainers understand the intent; update the block where
mountPreferencesView is defined (the tooltip: () => undefined entry for
PreferencesView) with that short explanatory comment consistent with other mocks
in the file.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 67561b84-1ff6-45d0-8e90-4009510e291a
📒 Files selected for processing (2)
src/views/__tests__/PreferencesView.spec.tssrc/views/__tests__/SystemSecurityPlanCreateView.spec.ts
|
PR approved. Marking as ready for e2e. |
1 participant
automated implementation by lisa.
Summary by CodeRabbit
New Features
Bug Fixes
Tests