fix: Update CRUD implementation for categories #10
Conversation
kofimokome
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe pull request introduces a comprehensive refactoring of the category management system in the application. The changes consolidate separate income and expense category models into a single Changes
Sequence DiagramsequenceDiagram
participant User
participant CategoryController
participant Category
participant Database
User->>CategoryController: Create Category
CategoryController->>User: Validate Request
CategoryController->>Category: firstOrCreate
Category->>Database: Save Category
Database-->>Category: Return Saved Category
Category-->>CategoryController: Return Category
CategoryController-->>User: Return JSON Response
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🔭 Outside diff range comments (1)
public/docs/api.json (1)
Line range hint
659-869: Add authentication responses to all protected endpoints.The Party endpoints include 401 (Unauthorized) responses, but similar responses are missing from Categories and Groups endpoints. This inconsistency might indicate missing authentication checks or incomplete documentation.
Consider adding 401 responses to all endpoints that require authentication:
- /categories/*
- /groups/*
🧹 Nitpick comments (8)
app/Http/Controllers/API/v1/CategoryController.php (3)
11-12: Ensure tag description consistency.You're using:
#[OA\Tag(name: 'Categories', description: 'Endpoints for managing transaction categories')]while the class name isCategoryController. To maintain consistency, consider removing "transaction" from the description if these endpoints manage categories in general, not just transaction categories.
41-48: Handle unexpected or missing 'type' parameter.The code handles
'income'and'expense'types but does not return an error or fallback if an unrecognized type is provided. Consider:
- Returning a 400 for invalid type values.
- Defaulting to all categories when
typeis missing or invalid.
193-193: Partial updates for 'type'?The validation rule forces
'type' => 'required|string|in:income,expense', while'name'is sometimes required. If partial updates are intended, consider adjusting'type'to'sometimes|required'.database/migrations/2024_07_20_125455_create_categories_table.php (1)
17-19: Ensure unique indexing and constraints if necessary.Adding
slug, makingdescriptionnullable, and introducing anenum('type', ['income','expense'])is effective. Consider adding:
- A unique index on
slugif it must be unique.- Collation or length constraints on
slugif needed.app/Models/Category.php (1)
14-17: Add enum validation for the type field in OpenAPI schemaThe 'type' property should specify allowed values to match the business logic.
new OA\Property(property: 'id', type: 'integer', description: 'ID of the category'), new OA\Property(property: 'name', type: 'string', description: 'Name of the category'), new OA\Property(property: 'description', type: 'string', description: 'Description of the category'), - new OA\Property(property: 'type', type: 'string', description: 'Type of the category (income or expense)'), + new OA\Property( + property: 'type', + type: 'string', + description: 'Type of the category (income or expense)', + enum: ['income', 'expense'] + ),public/docs/api.json (3)
Line range hint
238-465: Consider allowing category type updates in the PUT endpoint.The PUT endpoint for updating categories doesn't include the
typefield in its request body schema. This might limit the ability to change a category's type after creation.Apply this diff to add the type field:
"requestBody": { "content": { "application/json": { "schema": { "properties": { + "type": { + "description": "Type of the category", + "type": "string", + "enum": [ + "income", + "expense" + ] + }, "name": { "description": "Name of the category", "type": "string" },
Line range hint
466-658: Standardize response codes across similar endpoints.The Groups endpoints use different HTTP status codes compared to Categories endpoints for similar operations:
- DELETE /categories returns 204 (No Content)
- DELETE /groups returns 200 (OK)
Consider standardizing the response codes for consistency.
Apply this diff to standardize the DELETE response:
"responses": { - "200": { - "description": "Group deleted successfully" + "204": { + "description": "Group deleted successfully" },
1389-1409: Enhance the Category schema definition.The Category schema could be improved in several ways:
- Mark the
typefield as required since it's mandatory in POST requests- Add example values to improve documentation clarity
Apply this diff to enhance the schema:
"Category": { + "required": [ + "type", + "name" + ], "properties": { "id": { "description": "ID of the category", - "type": "integer" + "type": "integer", + "example": 1 }, "name": { "description": "Name of the category", - "type": "string" + "type": "string", + "example": "Salary" }, "description": { "description": "Description of the category", - "type": "string" + "type": "string", + "example": "Monthly salary income" }, "type": { "description": "Type of the category (income or expense)", - "type": "string" + "type": "string", + "enum": [ + "income", + "expense" + ], + "example": "income" } }, "type": "object" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
app/Http/Controllers/API/v1/CategoryController.php(10 hunks)app/Models/Category.php(1 hunks)app/Models/ExpenseCategory.php(0 hunks)app/Models/IncomeCategory.php(0 hunks)app/Models/TransactionCategory.php(0 hunks)app/Models/User.php(1 hunks)composer.json(1 hunks)database/migrations/2024_07_20_125455_create_categories_table.php(2 hunks)database/migrations/2024_07_20_125501_create_expense_categories_table.php(0 hunks)public/docs/api.json(21 hunks)routes/api.php(2 hunks)tests/Feature/CategoriesTest.php(1 hunks)
💤 Files with no reviewable changes (4)
- app/Models/ExpenseCategory.php
- app/Models/IncomeCategory.php
- database/migrations/2024_07_20_125501_create_expense_categories_table.php
- app/Models/TransactionCategory.php
🔇 Additional comments (21)
app/Http/Controllers/API/v1/CategoryController.php (16)
17-17: OpenAPI tags alignment.The tag
"Categories"here aligns well with the updated class name. No further changes are needed.
31-31: Confirm schema reference correctness.Referencing the
#/components/schemas/Categoryschema for an array of categories is correct for listing. Great job.
56-56: OpenAPI tags reaffirmation.Tags set to
['Categories']are consistent with the updated naming convention. Good job.
64-64: Clarity on optional field.Defining
descriptionas a string is clear; confirming in the docs that it is optional is helpful. Looks good.
72-72: Accurate schema reference.The reference to
#/components/schemas/Categoryfor creation responses is correct. Good to see consistent usage.
89-89: Validation for description.Marking
descriptionas nullable is a good approach for optional data. Nice validation practice.
108-108: Doc block alignment.Reiterating the
'Categories'tag is consistent with the new controller name. All good here.
122-122: Accurate schema binding.Referencing the
#/components/schemas/Categoryschema for the show endpoint is correct. Great job.
136-137: User scoping on retrieval.Fetching categories by
$user->categories()ensures correct scoping and prevents unauthorized access to other users’ categories. Good approach.
149-149: Consistent method documentation.Continuing the
'Categories'tag for the update method is consistent.
164-164: Documentation coverage for 'description'.Demonstrating the
descriptionfield in the request body is helpful for clarity. Good detail.
172-172: Schema reference in update response.The
#/components/schemas/Categoryreference is consistent. No concerns here.
201-203: User scoping on update.Using
$user->categories()->find($id)ensures the category belongs to the authenticated user. Great security measure.
209-209: Category update propagation.Calling
$category->update($data);directly is straightforward and leverages mass assignment rules. Looks good.
217-217: Delete endpoint clarity.Tags for
Categoriesremain consistent. This finalizes the CRUD coverage.
248-249: Secure category deletion.Using
$user->categories()->find($id)to restrict deletion to owned categories is the right approach. Thumbs up.database/migrations/2024_07_20_125455_create_categories_table.php (2)
14-14: Renamed table creation.
Schema::create('categories')replaces the oldincome_categories. Ensure that related references and any raw queries have been updated across the codebase.
30-30: Accurate rollback.Using
Schema::dropIfExists('categories')ensures a clean rollback if the migration fails or is reversed.app/Models/User.php (1)
48-52: Establishing the one-to-many relationship.The new
categories()method succinctly sets up a one-to-many relationship betweenUserandCategory. This is a proper Eloquent approach for resource ownership.routes/api.php (1)
37-37: LGTM!The route is properly secured with authentication middleware and follows the API versioning convention.
composer.json (1)
9-9: LGTM!The eloquent-sluggable package is a good choice for slug generation, and the version constraint is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
app/Http/Controllers/API/v1/CategoryController.php (1)
Line range hint
251-260: Add dependency check before deletion.Check for any transactions using this category before deletion to maintain data integrity.
Apply this diff to add the check:
$user = $request->user(); $category = $user->categories()->find($id); if (! $category) { return $this->failure('Category not found', 404); } +if ($category->transactions()->exists()) { + return $this->failure('Cannot delete category with associated transactions', 400); +} + $category->delete(); return $this->success(null, 'Category deleted successfully', 204);🧰 Tools
🪛 GitHub Actions: Checks
[error] Code style issue: not_operator_with_successor_space rule violation
🧹 Nitpick comments (2)
app/Http/Controllers/API/v1/CategoryController.php (1)
41-49: Consider enhancing the listing functionality.The implementation could be improved by:
- Making the page size configurable via request parameter
- Adding sorting capabilities (e.g., by name, created date)
Apply this diff to enhance the functionality:
$user = $request->user(); $categories = $user->categories(); if ($request->has('type') && $request->type == 'income') { $categories = $categories->where('type', 'income'); } elseif ($request->has('type') && $request->type == 'expense') { $categories = $categories->where('type', 'expense'); } -$categories = $categories->paginate(20); +$perPage = $request->input('per_page', 20); +$sortBy = $request->input('sort_by', 'created_at'); +$sortOrder = $request->input('sort_order', 'desc'); +$categories = $categories->orderBy($sortBy, $sortOrder)->paginate($perPage);🧰 Tools
🪛 GitHub Actions: Checks
[error] Code style issue: not_operator_with_successor_space rule violation
public/docs/api.json (1)
1389-1409: Add response examples to the Category schema.Consider adding example values to the Category schema properties to improve API documentation.
Apply this diff to add examples:
"Category": { "properties": { "id": { "description": "ID of the category", - "type": "integer" + "type": "integer", + "example": 1 }, "name": { "description": "Name of the category", - "type": "string" + "type": "string", + "example": "Groceries" }, "description": { "description": "Description of the category", - "type": "string" + "type": "string", + "example": "Daily grocery expenses" }, "type": { "description": "Type of the category (income or expense)", - "type": "string" + "type": "string", + "example": "expense", + "enum": ["income", "expense"] } }, "type": "object" },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
app/Http/Controllers/API/v1/CategoryController.php(10 hunks)app/Models/Category.php(1 hunks)app/Models/ExpenseCategory.php(0 hunks)app/Models/IncomeCategory.php(0 hunks)app/Models/TransactionCategory.php(0 hunks)app/Models/User.php(1 hunks)composer.json(1 hunks)config/sluggable.php(1 hunks)database/migrations/2024_07_20_125455_create_categories_table.php(2 hunks)database/migrations/2024_07_20_125501_create_expense_categories_table.php(0 hunks)public/docs/api.json(21 hunks)routes/api.php(2 hunks)tests/Feature/CategoriesTest.php(1 hunks)
💤 Files with no reviewable changes (4)
- app/Models/ExpenseCategory.php
- app/Models/IncomeCategory.php
- app/Models/TransactionCategory.php
- database/migrations/2024_07_20_125501_create_expense_categories_table.php
🚧 Files skipped from review as they are similar to previous changes (6)
- app/Models/User.php
- composer.json
- routes/api.php
- app/Models/Category.php
- database/migrations/2024_07_20_125455_create_categories_table.php
- tests/Feature/CategoriesTest.php
🧰 Additional context used
🪛 GitHub Actions: Checks
app/Http/Controllers/API/v1/CategoryController.php
[error] Code style issue: not_operator_with_successor_space rule violation
🔇 Additional comments (1)
config/sluggable.php (1)
1-148: LGTM! Well-documented configuration.The sluggable configuration is comprehensive and well-documented, providing flexibility for slug generation with sensible defaults.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/Http/Controllers/API/v1/CategoryController.php (1)
41-48: Add request validation and extract pagination constant.While the implementation is good, consider these improvements:
- Add validation for the type parameter
- Extract the pagination limit to a constant
+ private const PAGINATION_LIMIT = 20; + public function index(Request $request): JsonResponse { + $validator = Validator::make($request->all(), [ + 'type' => 'nullable|string|in:income,expense', + ]); + + if ($validator->fails()) { + return $this->failure('Validation error', 400, $validator->errors()->all()); + } + $user = $request->user(); $categories = $user->categories(); if ($request->has('type') && $request->type == 'income') { $categories = $categories->where('type', 'income'); } elseif ($request->has('type') && $request->type == 'expense') { $categories = $categories->where('type', 'expense'); } - $categories = $categories->paginate(20); + $categories = $categories->paginate(self::PAGINATION_LIMIT);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
app/Http/Controllers/API/v1/CategoryController.php(10 hunks)app/Models/Category.php(1 hunks)composer.json(1 hunks)config/sluggable.php(1 hunks)database/migrations/2024_07_20_125455_create_categories_table.php(2 hunks)tests/Feature/CategoriesTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- composer.json
- app/Models/Category.php
- config/sluggable.php
- database/migrations/2024_07_20_125455_create_categories_table.php
- tests/Feature/CategoriesTest.php
🔇 Additional comments (4)
app/Http/Controllers/API/v1/CategoryController.php (4)
11-12: LGTM! Clean class renaming and OpenAPI tag update.The class name change and OpenAPI tag update align well with the consolidated category management approach.
Line range hint
139-144: LGTM! Clean implementation of category retrieval.The implementation correctly uses the user relationship and handles the not-found case appropriately.
193-196: Update validation rules for consistency.The 'type' field should follow the same validation pattern as 'name' to allow partial updates.
Line range hint
251-261: LGTM! Clean implementation of category deletion.The implementation correctly uses the user relationship, handles the not-found case, and returns the appropriate HTTP status code.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Breaking Changes
Dependencies
eloquent-sluggablepackage for slug generation.