Adds create capability step call - #104
Conversation
- Add queries to create, delete, and retrieve capability steps in `capabilities.sql`. - Implement corresponding methods in generated database code (`capabilities.sql.gen.go`). - Extend `Querier` interface and `models.gen.go` for capability steps management.
…rsion methods - Add `TestCreateCapabilityStepRequest_Validate` to test request validation logic. - Add `TestCreateCapabilityStepRequest_ToParams` to test parameter conversion logic. - Implement `ToParams` method in `createCapabilityStepRequest` to map to database parameters.
… clarity and organization in capability service tests
…, and database error scenarios
…alidation and error handling
…lityStepRequest` struct and tests
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds capability-step persistence queries and models, validates and creates capability steps through the service layer, exposes ChangesCapability step creation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Handler
participant postgresService
participant Queries
Client->>Handler: POST /capability-steps
Handler->>postgresService: Validate and create request
postgresService->>Queries: Check capability and flow step
postgresService->>Queries: Insert capability step
Queries-->>postgresService: Created CapabilityStep
postgresService-->>Handler: Created CapabilityStep
Handler-->>Client: 201 Created
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
🧹 Nitpick comments (5)
internal/capability/capabilities.sql (1)
52-53: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winNaming implies single-row delete, but this deletes all steps for a capability.
DeleteCapabilityStep(singular) deletes every row matchingcapability_id, not a single step by its own primary key — unlikeDeleteCapabilityabove which deletes byid. If a future caller expects to delete one specific step, this will silently wipe out all steps for that capability. Consider renaming toDeleteCapabilitySteps(plural) to match its actual bulk-delete semantics, or scope by the step's ownidif single-row deletion was intended.🤖 Prompt for 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. In `@internal/capability/capabilities.sql` around lines 52 - 53, Rename the SQL query symbol DeleteCapabilityStep to DeleteCapabilitySteps to reflect that it deletes all capability_steps rows matching capability_id, and update every generated or handwritten caller referencing the old name. Preserve the existing bulk-delete behavior.internal/capability/request_test.go (1)
178-239: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing whitespace-only test case for Target/Protocol.
The table covers missing (empty-string)
Target/Protocol, but not whitespace-only values (e.g." "), which is the case that specifically exercisesstrings.TrimSpacerather than a plain emptiness check.✅ Suggested additional case
{ name: "Missing protocol", req: createCapabilityStepRequest{ CapabilityId: 1, FlowStepId: 1, Target: "target", }, wantErr: true, }, + { + name: "Whitespace-only target", + req: createCapabilityStepRequest{ + CapabilityId: 1, + FlowStepId: 1, + Target: " ", + Protocol: "protocol", + }, + wantErr: true, + }, }As per path instructions, "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request."
🤖 Prompt for 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. In `@internal/capability/request_test.go` around lines 178 - 239, Extend TestCreateCapabilityStepRequest_Validate with cases where Target and Protocol contain only whitespace, such as " ", and expect validation errors. Keep the other required fields populated so each case specifically exercises whitespace handling via strings.TrimSpace.Source: Path instructions
internal/capability/db/querier.gen.go (1)
13-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the redundant
n == 0check inCreateCapabilityStep.GetFlowStepalready selects the step id directly, and the missing-row case comes back as an error; the extra integer check only obscures that contract.🤖 Prompt for 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. In `@internal/capability/db/querier.gen.go` around lines 13 - 21, Remove the redundant n == 0 validation from CreateCapabilityStep and rely on GetFlowStep’s direct step-ID result and error for missing rows. Preserve the existing error propagation and creation flow otherwise.internal/capability/handler_command_cap_step_test.go (1)
23-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the response body and invalid-request short-circuiting.
For success, decode and verify the returned capability-step JSON. For malformed and invalid requests, assert the mocked service was not invoked. Status-only assertions can miss response serialization and accidental service execution regressions.
As per path instructions,
**/*_test.go: “Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request”.🤖 Prompt for 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. In `@internal/capability/handler_command_cap_step_test.go` around lines 23 - 105, The CreateCapabilityStep table tests should validate response content and prevent service calls for invalid input. Extend mockCapabilityService to track invocations, assert the success response decodes to the expected db.CapabilityStep, and verify the service was not called for the “Invalid JSON” and “Validation Error” cases while preserving the existing status assertions.Source: Path instructions
internal/capability/service_capability_steps_test.go (1)
41-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the flow-step branch and persistence contract.
Add a
GetFlowStepnot-found/error case, assert lookup errors are propagated correctly, and verifyCreateCapabilityStepreceives the expected capability ID, flow-step ID, protocol, and target. The current success test only checks one returned field, so mapper or validation regressions can pass unnoticed.As per path instructions,
**/*_test.go: “Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request”.🤖 Prompt for 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. In `@internal/capability/service_capability_steps_test.go` around lines 41 - 102, Expand the CreateCapabilityStep tests around postgresService.CreateCapabilityStep: add a GetFlowStep error/not-found case and assert the lookup error is propagated. In the success case, capture and validate CreateCapabilityStep’s arguments for capability ID, flow-step ID, protocol, and target, while retaining the existing result assertion; keep the existing persistence-error coverage.Source: Path instructions
🤖 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 `@_bruno/Capabilities/Create` Capability Step.yml:
- Line 74: Update the endpoint description in “Create Capability Step” so it
describes creating a new capability step, not creating a capability for a flow.
In `@internal/capability/service_capability_steps.go`:
- Around line 13-25: Update the validation goroutines around GetCapability and
GetFlowStep so only confirmed missing records are sent to errChan as
"capability" or "flow_step". Preserve and propagate database, connection, and
context deadline errors unchanged so the handler retains 500/timeout semantics,
including adapting the errChan result and caller logic as needed.
---
Nitpick comments:
In `@internal/capability/capabilities.sql`:
- Around line 52-53: Rename the SQL query symbol DeleteCapabilityStep to
DeleteCapabilitySteps to reflect that it deletes all capability_steps rows
matching capability_id, and update every generated or handwritten caller
referencing the old name. Preserve the existing bulk-delete behavior.
In `@internal/capability/db/querier.gen.go`:
- Around line 13-21: Remove the redundant n == 0 validation from
CreateCapabilityStep and rely on GetFlowStep’s direct step-ID result and error
for missing rows. Preserve the existing error propagation and creation flow
otherwise.
In `@internal/capability/handler_command_cap_step_test.go`:
- Around line 23-105: The CreateCapabilityStep table tests should validate
response content and prevent service calls for invalid input. Extend
mockCapabilityService to track invocations, assert the success response decodes
to the expected db.CapabilityStep, and verify the service was not called for the
“Invalid JSON” and “Validation Error” cases while preserving the existing status
assertions.
In `@internal/capability/request_test.go`:
- Around line 178-239: Extend TestCreateCapabilityStepRequest_Validate with
cases where Target and Protocol contain only whitespace, such as " ", and
expect validation errors. Keep the other required fields populated so each case
specifically exercises whitespace handling via strings.TrimSpace.
In `@internal/capability/service_capability_steps_test.go`:
- Around line 41-102: Expand the CreateCapabilityStep tests around
postgresService.CreateCapabilityStep: add a GetFlowStep error/not-found case and
assert the lookup error is propagated. In the success case, capture and validate
CreateCapabilityStep’s arguments for capability ID, flow-step ID, protocol, and
target, while retaining the existing result assertion; keep the existing
persistence-error coverage.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 616af5b8-bc8a-4abf-ba0d-01287effebd1
📒 Files selected for processing (18)
_bruno/Capabilities/Create Capability Step.ymlinternal/capability/capabilities.sqlinternal/capability/db/capabilities.sql.gen.gointernal/capability/db/models.gen.gointernal/capability/db/querier.gen.gointernal/capability/handler.gointernal/capability/handler_command_cap_step_test.gointernal/capability/handler_command_capability_test.gointernal/capability/handler_query_capability_test.gointernal/capability/handler_test.gointernal/capability/request.gointernal/capability/request_test.gointernal/capability/router.gointernal/capability/router_test.gointernal/capability/service_capability_steps.gointernal/capability/service_capability_steps_test.gointernal/capability/service_capability_test.gorouter/router_test.go
…en "not found" and general errors, and add new test cases for detailed validation
1 participant
Description
Code Rabbit Summary
Summary by CodeRabbit
New Features
Tests
Fixes
Closes #101