Conversation
There was a problem hiding this comment.
Pull request overview
Introduces typed (static) response payload structs for product endpoints and updates the generated OpenAPI artifacts to use a shared handlers.Error schema for error responses.
Changes:
- Added dedicated response wrapper structs for product-by-id and product-search handlers, and switched handlers to return these types instead of
gin.H. - Updated Swagger (yaml/json/docs.go) to define and reference
handlers.Errorfor 4xx/5xx responses on the product endpoints.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/handlers/product/getId.go | Adds a typed response wrapper for the 200 payload and uses it in c.JSON. |
| internal/handlers/product/get.go | Adds a typed response wrapper for the 200 payload and uses it in c.JSON. |
| docs/swagger.yaml | Adds handlers.Error definition and refs it for error responses. |
| docs/swagger.json | Adds handlers.Error definition and refs it for error responses. |
| docs/docs.go | Updates generated Swagger template to include handlers.Error definition and refs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type getProductByIdResponse struct { | ||
| Product product.Product `json:"product"` | ||
| } |
There was a problem hiding this comment.
getProductByIdResponse uses Id in the type name; in Go initialisms are typically capitalized (e.g., ID). Renaming to getProductByIDResponse would also align with GetProductByID used below.
| } | ||
|
|
||
| c.JSON(http.StatusOK, gin.H{"product": product}) | ||
| c.JSON(http.StatusOK, getProductByIdResponse{Product: *product}) |
There was a problem hiding this comment.
This copies the entire product.Product struct (*product) into the response. Since product.Product is large, prefer returning a pointer in the response type (e.g., make the field *product.Product and set it to product) to avoid an extra full-struct copy per request.
| } | ||
|
|
||
| c.JSON(http.StatusOK, gin.H{"product": product}) | ||
| c.JSON(http.StatusOK, getProductByIdResponse{Product: *product}) |
There was a problem hiding this comment.
The handler now returns getProductByIdResponse, but the Swagger annotation still documents the 200 response as map[string]product.Product (@Success 200 ...). Update the @Success type to the new response struct so generated OpenAPI matches the actual response body.
| } | ||
|
|
||
| c.JSON(http.StatusOK, gin.H{"products": products}) | ||
| c.JSON(http.StatusOK, productGetResponse{Products: *products}) |
There was a problem hiding this comment.
The handler now returns productGetResponse, but the Swagger annotation still documents the 200 response as map[string][]product.Product (@Success 200 ...). Update the @Success type to the new response struct so generated OpenAPI matches the actual response body.
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.
| type getProductByIdResponse struct { | ||
| Product product.Product `json:"product"` | ||
| } |
There was a problem hiding this comment.
getProductByIdResponse stores product.Product by value, which forces a full struct copy when building the response (*product). product.Product is very large (see internal/database/product/product.go), so this adds avoidable per-request copying. Prefer making the field a pointer (e.g., *product.Product) and returning the pointer directly to avoid the copy and keep behavior consistent with the previous gin.H{"product": product}.
1 participant
No description provided.