Adds create flow call#63
Conversation
- Modify SQLC query to use `:one` annotation and include `RETURNING *`. - Adjust `CreateFlow` function to return detailed `Flow` object with proper scanning. - Update `Querier` interface signature to reflect the change.
- Implement `createFlowRequest` struct with request fields and `ToParams` method. - Add `CreateFlow` method to `Handler` interface and `flowHandler` struct. - Validate request body and required fields in `CreateFlow` handler. - Write response using `application/json` with appropriate status codes. - Add unit tests for `CreateFlow`, covering success, bad request, and internal server error cases.
- Combine platform, product, and flow route registration for improved readability.
- Introduce `CreateFlow` route under `/api/products/{id}/flows`.
- Modify `createFlowRequest.ToParams` to accept `productID` as a parameter. - Update `CreateFlow` handler to parse and validate `productID` from the request path. - Add test cases to validate behavior for valid, missing, and invalid `productID`.
- Introduce `Flows` folder and define its sequence metadata. - Add `Create Flow` POST request to create a product flow. - Include example request and response for testing and documentation.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements the ChangesCreate Flow Endpoint
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
internal/flow/request_test.go (1)
7-27: ⚡ Quick winAdd a case for empty
descriptionmapping.
ToParamshas a branch where empty description should produceDescription.Valid == false, but this test only covers the non-empty branch.Suggested test extension
func TestCreateFlowRequest_ToParams(t *testing.T) { + t.Run("non-empty description", func(t *testing.T) { req := createFlowRequest{ Name: "Test Flow", Description: "Test Description", } productID := 123 params := req.ToParams(productID) @@ if !params.TimeStamp.Valid { t.Error("expected TimeStamp to be valid") } + }) + + t.Run("empty description becomes null", func(t *testing.T) { + req := createFlowRequest{Name: "Test Flow", Description: ""} + params := req.ToParams(123) + if params.Description.Valid { + t.Error("expected Description.Valid to be false for empty description") + } + }) }As per coding guidelines,
**/*_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/flow/request_test.go` around lines 7 - 27, Extend the TestCreateFlowRequest_ToParams test to include a subcase where createFlowRequest.Description is an empty string and verify ToParams(productID) yields params.Description.Valid == false (and params.Description.String is empty), while still asserting params.Name == req.Name, params.ProductID == productID, and params.TimeStamp.Valid is true; update or add a new subtest within TestCreateFlowRequest_ToParams to cover this branch of the createFlowRequest.ToParams logic.internal/flow/handler_test.go (1)
57-186: ⚡ Quick winAdd a test case for
product not found -> 404.This table set does not verify the endpoint’s required 404 behavior for unknown product IDs, so that contract can regress unnoticed.
As per coding guidelines,
**/*_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/flow/handler_test.go` around lines 57 - 186, Add a test case to TestCreateFlow that verifies a missing product returns 404: add a new table entry named e.g. "Product Not Found" with pathID "1", requestBody createFlowRequest{Name: "Missing Product"}, mockSetup that sets mock.createFlowFunc = func(ctx context.Context, arg CreateFlowParams) (Flow, error) { return Flow{}, sql.ErrNoRows } (or the repository’s sentinel NotFound error), and expectedStatus http.StatusNotFound; ensure the test uses the existing flowHandler.CreateFlow call and checks rr.Code equals http.StatusNotFound (no response body assertions needed).router/router.go (1)
69-71: ⚡ Quick winRemove unused empty function.
registerPlatformCallHandleris flagged as unused by golangci-lint. The function body is empty and route registration has been moved inline intoSetupRouter. Remove this dead code.♻️ Proposed fix
-func registerPlatformCallHandler(platformHandler platform.Handler, r *chi.Mux) { - -}🤖 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 `@router/router.go` around lines 69 - 71, Remove the dead unused function registerPlatformCallHandler by deleting its declaration and empty body from the file since route registration is already handled inline in SetupRouter; ensure there are no remaining references to registerPlatformCallHandler elsewhere (if found, update those call sites to use the inline registration in SetupRouter or remove the calls).
🤖 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/Flows/Create` Flow.yml:
- Around line 22-26: The assertion in the runtime assertions block currently
checks res.status == "200" but the endpoint should return 201; update the
assertion under runtime.assertions (the entry with expression: res.status,
operator: eq) to expect "201" (or numeric 201) so the test matches the example
response and PR objectives.
In `@internal/flow/handler.go`:
- Around line 44-46: The current error branch always returns 500 on create
failures; update the error handling around the err from the flow creation call
so that if err represents a missing product (e.g., errors.Is(err, sql.ErrNoRows)
or matches your product-service sentinel like product.ErrNotFound) you call
internal.HandleHttpError(w, internal.ErrorEnvelope{Detail: "Product not found"},
http.StatusNotFound), otherwise preserve the existing internal server error
behavior (internal.HandleHttpError(..., "Failed to create flow",
http.StatusInternalServerError)). Use the existing err variable and
internal.HandleHttpError/internal.ErrorEnvelope symbols to implement this
branching.
- Around line 37-39: The handler currently treats whitespace-only names as valid
because it checks req.Name == ""; change the validation to trim whitespace first
(use strings.TrimSpace) and treat the trimmed result as empty to reject names
like " "; update the check around the block that calls
internal.HandleHttpError (and either assign the trimmed value back to req.Name
or use a local trimmedName variable) and ensure the strings package is imported
so whitespace-only inputs are rejected.
In `@router/router.go`:
- Line 42: The route registration has a typo: u.Get("/}",
platformHandler.GetPlatform) should use a correct path string; update the route
call on the router (the u.Get invocation that registers
platformHandler.GetPlatform) to use the correct pattern (replace "/}" with "/")
so the GetPlatform handler is registered correctly and GET /api/platforms/{id}
requests will match.
---
Nitpick comments:
In `@internal/flow/handler_test.go`:
- Around line 57-186: Add a test case to TestCreateFlow that verifies a missing
product returns 404: add a new table entry named e.g. "Product Not Found" with
pathID "1", requestBody createFlowRequest{Name: "Missing Product"}, mockSetup
that sets mock.createFlowFunc = func(ctx context.Context, arg CreateFlowParams)
(Flow, error) { return Flow{}, sql.ErrNoRows } (or the repository’s sentinel
NotFound error), and expectedStatus http.StatusNotFound; ensure the test uses
the existing flowHandler.CreateFlow call and checks rr.Code equals
http.StatusNotFound (no response body assertions needed).
In `@internal/flow/request_test.go`:
- Around line 7-27: Extend the TestCreateFlowRequest_ToParams test to include a
subcase where createFlowRequest.Description is an empty string and verify
ToParams(productID) yields params.Description.Valid == false (and
params.Description.String is empty), while still asserting params.Name ==
req.Name, params.ProductID == productID, and params.TimeStamp.Valid is true;
update or add a new subtest within TestCreateFlowRequest_ToParams to cover this
branch of the createFlowRequest.ToParams logic.
In `@router/router.go`:
- Around line 69-71: Remove the dead unused function registerPlatformCallHandler
by deleting its declaration and empty body from the file since route
registration is already handled inline in SetupRouter; ensure there are no
remaining references to registerPlatformCallHandler elsewhere (if found, update
those call sites to use the inline registration in SetupRouter or remove the
calls).
🪄 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: bf42d04e-843e-41fb-b645-d15107a9373b
📒 Files selected for processing (11)
_bruno/Flows/Create Flow.yml_bruno/Flows/folder.ymlinternal/flow/flows.sqlinternal/flow/flows.sql.gen.gointernal/flow/handler.gointernal/flow/handler_test.gointernal/flow/interface.gointernal/flow/querier.gen.gointernal/flow/request.gointernal/flow/request_test.gorouter/router.go
…lidation - Return `404 Not Found` if `pgx.ErrNoRows` occurs in `CreateFlow`. - Trim whitespace when validating `Name` field. - Add test to cover missing product scenario.
Description
Code Rabbit Summary
Summary by CodeRabbit
Release Notes
New Features
Tests
Fixes
Closes #34
Post Deployment Tasks?