Add Create Product Handler#43
Conversation
… in `CreateProduct` query and related implementation adjustments.
… and extend tests for missing fields
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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 (3)
📝 WalkthroughWalkthroughA new HTTP POST handler for creating products is implemented across the data, application, routing, and test layers. The product creation flow validates JSON input and required fields, sets a UTC timestamp, calls the database within a 5-second context timeout, and returns appropriate HTTP status codes. A single ChangesProduct Creation Handler
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 20 minutes and 50 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
api/product/product_test.go (2)
42-103: ⚡ Quick winTest bodies use
json.Marshal(CreateProductParams{…})which produces PascalCase keys, masking the snake_case decoding bug and leaving the real API contract untested.All test cases marshal the Go struct directly (e.g.,
{"PlatformID":1,"Name":"Test Product"}). Go'sUnmarshal()matches JSON keys in a case-insensitive manner as long as the characters and their order are the same, so this never exercises whether{"platform_id": 1}— the payload a real REST client would send — is handled correctly. Per coding guidelines, test coverage should be sufficient for the PR changes.Add at least one case that sends a raw snake_case JSON payload matching what a real client would produce:
{ name: "Snake-case JSON keys", requestBody: `{"platform_id":1,"name":"Test Product"}`, mockSetup: func(m *mockProductQuerier) { m.createProductFunc = func(_ context.Context, _ product.CreateProductParams) error { return nil } }, expectedStatus: http.StatusCreated, },As per coding guidelines, "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 the current code and only fix it if needed. In `@api/product/product_test.go` around lines 42 - 103, The TestCreateProduct table uses json.Marshal on product.CreateProductParams which emits PascalCase keys and misses exercising snake_case payload decoding; add a new test entry inside the tests slice in TestCreateProduct named e.g. "Snake-case JSON keys" where requestBody is a raw JSON string like `{"platform_id":1,"name":"Test Product"}`, mockSetup assigns mockProductQuerier.createProductFunc to accept product.CreateProductParams and return nil, and expectedStatus is http.StatusCreated so the handler will be exercised with real snake_case keys and validate decoding into product.CreateProductParams.
14-40: 💤 Low valueMock delegate methods panic on a nil function field.
Every mock method calls its function field unconditionally (e.g.,
m.deleteProductFunc(ctx, id)). If any future handler code path callsDeleteProduct,GetProductById, etc. when those fields are not initialised, the test panics instead of failing gracefully. A nil guard makes the mock safer to extend:♻️ Proposed refactor (example for one method; apply to all five)
func (m *mockProductQuerier) DeleteProduct(ctx context.Context, id int32) error { + if m.deleteProductFunc == nil { + return nil + } return m.deleteProductFunc(ctx, id) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/product/product_test.go` around lines 14 - 40, The mockProductQuerier methods (CreateProduct, DeleteProduct, GetProductById, GetProductsByPlatform, UpdateProduct) call their function fields unconditionally causing panics if nil; update each method to check the corresponding function field (e.g., m.deleteProductFunc, m.getProductByIdFunc, etc.) and if nil return a sensible zero value and a clear error (or testing.T-friendly error) instead of calling it, so tests fail gracefully when a mock handler was not initialized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/product/create.go`:
- Around line 13-22: The handler is using the auto-generated
product.CreateProductParams (which lacks json tags) so incoming snake_case keys
like "platform_id" are ignored; update ProductHandler.CreateProduct to decode
into a new local request DTO (e.g., CreateProductRequest) that defines
json:"name" and json:"platform_id" tags, validate that DTO (Name != "" and
PlatformID != 0), then map its fields into product.CreateProductParams before
proceeding with the existing creation logic; ensure you reference
CreateProductRequest in the CreateProduct method and perform the same validation
and error responses as before.
In `@api/product/product_test.go`:
- Around line 111-116: When marshaling tt.requestBody, don’t ignore the error
from json.Marshal; capture the error and fail the test with a clear message
(e.g., t.Fatalf or require.NoError) so marshal failures surface instead of
sending an empty body; keep the existing string branch (if s, ok :=
tt.requestBody.(string)) and only json.Marshal when needed, then on err != nil
call t.Fatalf("json.Marshal requestBody failed: %v", err) (or equivalent
assertion) to provide the root-cause in the test output.
---
Nitpick comments:
In `@api/product/product_test.go`:
- Around line 42-103: The TestCreateProduct table uses json.Marshal on
product.CreateProductParams which emits PascalCase keys and misses exercising
snake_case payload decoding; add a new test entry inside the tests slice in
TestCreateProduct named e.g. "Snake-case JSON keys" where requestBody is a raw
JSON string like `{"platform_id":1,"name":"Test Product"}`, mockSetup assigns
mockProductQuerier.createProductFunc to accept product.CreateProductParams and
return nil, and expectedStatus is http.StatusCreated so the handler will be
exercised with real snake_case keys and validate decoding into
product.CreateProductParams.
- Around line 14-40: The mockProductQuerier methods (CreateProduct,
DeleteProduct, GetProductById, GetProductsByPlatform, UpdateProduct) call their
function fields unconditionally causing panics if nil; update each method to
check the corresponding function field (e.g., m.deleteProductFunc,
m.getProductByIdFunc, etc.) and if nil return a sensible zero value and a clear
error (or testing.T-friendly error) instead of calling it, so tests fail
gracefully when a mock handler was not initialized.
🪄 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: 79738f1b-9bcb-4dae-ad93-89d9f83a5365
📒 Files selected for processing (6)
api/product/create.goapi/product/product.goapi/product/product_test.gointernal/db/product/products.sql.goqueries/products.sqlrouter/router.go
…, update parameter mapping, and adjust related tests.
… silent test failures.
Description
Code Rabbit Summary
Summary by CodeRabbit
Fixes
Closes #23
Post Deployment Tasks?