Reorganize code into packages#50
Merged
Merged
Conversation
- Consolidate and migrate API handlers for `platform` and `product` into their respective internal packages for improved modularity. - Replace `db` references with query-specific instances and interfaces. - Update imports, tests, and routes to reflect refactored handler structure. - Regenerate SQLC files and adjust configurations for modular placement.
…ndlers to use interfaces, and update SQLC configuration for interface output.
…d streamline connection closure
This reverts commit 38e475d.
…atformHandler` and `productHandler` structs
…nition and request validation
…tructs, and improve test consistency
… file for improved modularity in `product` package.
…st structs into dedicated files, implement `ToParams` methods, and add unit tests to improve modularity and test coverage.
… file for improved modularity in `platform` package.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits 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 (5)
📝 WalkthroughWalkthroughThis PR restructures the project from a horizontal layer-separated architecture (with HTTP handlers in ChangesVertical Domain Package Reorganization
Sequence Diagram(s)No sequence diagrams generated. While this PR involves multiple components (router, handlers, database), the changes are primarily architectural reorganization and refactoring rather than functional flow modifications. The control flow for request handling remains unchanged; only the package structure and handler instantiation pattern have been altered. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 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. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/platform/handler_test.go (1)
371-375:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
TestUpdatePlatformuses the wrong request body type for handler decoding.
UpdatePlatformdecodes intoupdatePlatformRequestwhereDescriptionis a string, but tests sendPlatformwhereDescriptionispgtype.Text. Whenpgtype.Textis JSON-marshaled, it becomes an object shape, causing the JSON decoder to fail. This prevents the success path from being properly tested.Suggested fix
- requestBody: Platform{ + requestBody: updatePlatformRequest{ ID: 1, Name: "Updated Platform", - Description: pgtype.Text{String: "Updated Description", Valid: true}, + Description: "Updated Description", },Also update the mock assertion:
- expectedBody := tt.requestBody.(Platform) + expectedBody := tt.requestBody.(updatePlatformRequest) if arg.ID != expectedBody.ID { t.Errorf("expected ID %d, got %d", expectedBody.ID, arg.ID) } if arg.Name != expectedBody.Name { t.Errorf("expected Name %s, got %s", expectedBody.Name, arg.Name) } - if arg.Description.String != expectedBody.Description.String { - t.Errorf("expected Description %s, got %s", expectedBody.Description.String, arg.Description.String) + if arg.Description.String != expectedBody.Description { + t.Errorf("expected Description %s, got %s", expectedBody.Description, arg.Description.String) }Applies to lines: 371-375, 382-385, 392-395, 402-405, 412-415, 422-425, 432-435, 449-452, 459-462, 474-483
🤖 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/platform/handler_test.go` around lines 371 - 375, TestUpdatePlatform is sending a Platform value (with Description as pgtype.Text) but the handler decodes into updatePlatformRequest where Description is a string, causing JSON decoding failures; change the test requestBody to use updatePlatformRequest (or a struct literal matching updatePlatformRequest) with Description as a plain string ("Updated Description") and update all related test cases (the requestBody entries at the referenced locations) so the mock expectation/assertions match the string description type (adjust the mock call args to expect the string description instead of pgtype.Text) to exercise the success path correctly.internal/product/handler_test.go (1)
89-105:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a whitespace-only
Namecase forCreateProduct.Current create tests don’t cover
" "input, so the invalid-name path at Line 34 in handler logic is not protected by tests.Suggested test case
@@ { name: "Missing Name", requestBody: createProductRequest{ PlatformID: 1, }, mockSetup: func(m *mockProductQuerier) {}, expectedStatus: http.StatusBadRequest, }, + { + name: "Whitespace Name", + requestBody: createProductRequest{ + PlatformID: 1, + Name: " ", + }, + mockSetup: func(m *mockProductQuerier) {}, + expectedStatus: http.StatusBadRequest, + },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 current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/product/handler_test.go` around lines 89 - 105, Add a new table-driven test case for CreateProduct that uses createProductRequest{Name: " ", PlatformID: 1} (a whitespace-only Name) to exercise the invalid-name branch in the CreateProduct handler; call the same mockSetup (no DB interactions) as the other bad-request cases, expect http.StatusBadRequest, and ensure the test sends this request through the same test harness used by the existing cases (the test table and test runner around mockProductQuerier and the CreateProduct handler).
🤖 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 `@internal/platform/handler.go`:
- Around line 93-99: The delete handler currently writes raw internal errors to
clients (the branch after errors.Is(err, pgx.ErrNoRows)), leaking DB details via
err.Error(); change that response to a generic error message (e.g., "Internal
server error") and an appropriate 500 status instead of returning err.Error();
keep the special-case NotFound branch (errors.Is(err, pgx.ErrNoRows)) as-is and
only replace the generic http.Error(w, err.Error(),
http.StatusInternalServerError) call in this handler to a non-revealing message.
In `@internal/platform/request_test.go`:
- Around line 50-62: Add a test that verifies updatePlatformRequest.ToParams
prefers the method argument ID over the struct field: create an
updatePlatformRequest with ID set to a different value than the int32 id passed
into ToParams, call req.ToParams(id), and assert params.ID equals the method arg
id (and optionally assert Name/Description are preserved). This targets
updatePlatformRequest and its ToParams method to ensure path ID precedence is
enforced.
In `@internal/product/handler.go`:
- Around line 34-36: The request name blank check currently only tests for empty
string so names like " " pass; in the create handler (where req.Name and
req.PlatformID are validated) trim whitespace from req.Name (e.g.,
strings.TrimSpace(req.Name)) before performing the empty check and
required-field validation, mirroring the update-path behavior; ensure you update
the same validation block that returns http.StatusBadRequest on missing fields
so whitespace-only names are rejected.
---
Outside diff comments:
In `@internal/platform/handler_test.go`:
- Around line 371-375: TestUpdatePlatform is sending a Platform value (with
Description as pgtype.Text) but the handler decodes into updatePlatformRequest
where Description is a string, causing JSON decoding failures; change the test
requestBody to use updatePlatformRequest (or a struct literal matching
updatePlatformRequest) with Description as a plain string ("Updated
Description") and update all related test cases (the requestBody entries at the
referenced locations) so the mock expectation/assertions match the string
description type (adjust the mock call args to expect the string description
instead of pgtype.Text) to exercise the success path correctly.
In `@internal/product/handler_test.go`:
- Around line 89-105: Add a new table-driven test case for CreateProduct that
uses createProductRequest{Name: " ", PlatformID: 1} (a whitespace-only Name)
to exercise the invalid-name branch in the CreateProduct handler; call the same
mockSetup (no DB interactions) as the other bad-request cases, expect
http.StatusBadRequest, and ensure the test sends this request through the same
test harness used by the existing cases (the test table and test runner around
mockProductQuerier and the CreateProduct handler).
🪄 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: 80bd7e23-588e-4510-b494-537840070299
📒 Files selected for processing (34)
api/platform/create.goapi/platform/delete.goapi/platform/get.goapi/platform/platform.goapi/platform/update.goapi/product/create.goapi/product/delete.goapi/product/get.goapi/product/product.goapi/product/update.gointernal/db/store.gointernal/platform/db.gen.gointernal/platform/handler.gointernal/platform/handler_test.gointernal/platform/interface.gointernal/platform/models.gen.gointernal/platform/platforms.sqlinternal/platform/platforms.sql.gen.gointernal/platform/querier.gen.gointernal/platform/request.gointernal/platform/request_test.gointernal/product/db.gen.gointernal/product/handler.gointernal/product/handler_test.gointernal/product/interface.gointernal/product/models.gen.gointernal/product/products.sqlinternal/product/products.sql.gen.gointernal/product/querier.gen.gointernal/product/request.gointernal/product/request_test.gomain.gorouter/router.gosqlc.yml
💤 Files with no reviewable changes (11)
- api/product/product.go
- api/platform/delete.go
- api/product/delete.go
- api/platform/update.go
- api/product/create.go
- api/product/update.go
- internal/db/store.go
- api/product/get.go
- api/platform/get.go
- api/platform/create.go
- api/platform/platform.go
…NoRows` handling, enhance test coverage, and log internal server errors
This was referenced May 7, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Code Rabbit Summary
Summary by CodeRabbit
Note: This release contains internal infrastructure improvements with no changes to user-facing functionality or API behavior.
Fixes
Closes #49
Post Deployment Tasks?