feat: ownership and system standard properties to categories and units#131
Open
matheusandre1 wants to merge 3 commits into
Open
feat: ownership and system standard properties to categories and units#131matheusandre1 wants to merge 3 commits into
matheusandre1 wants to merge 3 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements ownership-based access control for categories and units. Categories and Units now track an owner and system-standard flag, repositories filter by deletion and user access, services enforce ownership guards, DTOs expose ownership information, and controllers require authenticated users with proper HTTP response codes. Comprehensive tests validate authorization across service, controller, and integration test layers. ChangesOwnership-Based Access Control for Categories and Units
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java (2)
180-196: ⚡ Quick winExtract the hardcoded test-user email to a shared constant and eliminate the duplicate DB lookup.
"user@gmail.com"is duplicated across both helpers and silently couples these tests to whatever emailTestUserFactoryprovisions. If that email changes, both calls toorElseThrow()throw a genericNoSuchElementExceptionwith no indication of the expected address. Additionally,createItemToTest()calls both helpers, resulting in two separatefindByEmailround-trips for the same user. Resolving the user once and passing it (or caching it at field level) removes both problems.♻️ Suggested refactor
+ private static final String TEST_USER_EMAIL = "user@gmail.com"; Category createCategoryToTest() { - User testUser = userRepository.findByEmail("user@gmail.com").orElseThrow(); + return createCategoryToTest(resolveTestUser()); + } + + Category createCategoryToTest(User testUser) { Category categoryEntity = CategoryTestFactory.createValidCategory(); categoryEntity.setId(null); categoryEntity.setOwner(testUser); categoryEntity.setIsSystemStandard(false); return categoryRepository.save(categoryEntity); } Unit createUnitToTest() { - User testUser = userRepository.findByEmail("user@gmail.com").orElseThrow(); + return createUnitToTest(resolveTestUser()); + } + + Unit createUnitToTest(User testUser) { Unit unitEntity = UnitTestFactory.createValidUnit(); unitEntity.setId(null); unitEntity.setOwner(testUser); unitEntity.setIsSystemStandard(false); return unitRepository.save(unitEntity); } Item createItemToTest() { + User testUser = resolveTestUser(); Item itemEntity = ItemTestFactory.createValidItem(); itemEntity.setId(null); - itemEntity.setCategory(createCategoryToTest()); - itemEntity.setUnit(createUnitToTest()); + itemEntity.setCategory(createCategoryToTest(testUser)); + itemEntity.setUnit(createUnitToTest(testUser)); return itemRepository.save(itemEntity); } + private User resolveTestUser() { + return userRepository.findByEmail(TEST_USER_EMAIL) + .orElseThrow(() -> new IllegalStateException( + "Test user '%s' not found – ensure TestUserFactory has provisioned it".formatted(TEST_USER_EMAIL))); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java` around lines 180 - 196, Introduce a single shared constant for the test user email (e.g., TEST_USER_EMAIL) and stop doing duplicate repository lookups by resolving the User once and reusing it; specifically, change the helpers Category createCategoryToTest() and Unit createUnitToTest() to either accept a User parameter or use a cached field resolved once (e.g., in a `@BeforeEach` setup) so that createItemToTest() reuses the same User instance instead of calling userRepository.findByEmail("user@gmail.com") twice, and when resolving the user include the constant in the lookup so any failure message/reference points to the named constant rather than a hardcoded string.
55-56: ⚖️ Poor tradeoffPrefer constructor injection over
@Autowiredfield injection.The new
userRepositoryfield uses field injection, which violates the constructor injection guideline. The four existing@Autowiredfields in this class share the same pattern, so the full fix spans the whole class, but the new addition compounds the violation.As per coding guidelines: "Dependency Injection: Always use constructor injection with
finalfields."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java` around lines 55 - 56, The test class ItemControllerWithIntegrationTest uses field injection for userRepository (and the other four `@Autowired` fields); change these to constructor injection: make each dependency a private final field (including UserRepository), remove the `@Autowired` annotations from the fields, and add a single constructor for ItemControllerWithIntegrationTest that accepts UserRepository (and the other repository/service fields) and assigns them to the final fields; you can omit `@Autowired` on the constructor if using a single constructor or annotate it if preferred.backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java (1)
3-9: 💤 Low valueImport order and
varusage violate coding guidelines.Two issues:
- Import order (line 9):
org.springframework.stereotype.Componentis placed after all project imports. Required order isjava.*,jakarta.*,org.springframework.*, third-party, then project packages.varnot used (lines 21, 30): Both local variables have types obvious from context.♻️ Proposed fix
-import com.omatheusmesmo.shoppmate.unit.dto.UnitRequestDTO; -import com.omatheusmesmo.shoppmate.unit.dto.UnitResponseDTO; -import com.omatheusmesmo.shoppmate.unit.entity.Unit; -import com.omatheusmesmo.shoppmate.user.dtos.UserResponseDTO; -import com.omatheusmesmo.shoppmate.user.entity.User; -import com.omatheusmesmo.shoppmate.user.mapper.UserMapper; -import org.springframework.stereotype.Component; +import org.springframework.stereotype.Component; + +import com.omatheusmesmo.shoppmate.unit.dto.UnitRequestDTO; +import com.omatheusmesmo.shoppmate.unit.dto.UnitResponseDTO; +import com.omatheusmesmo.shoppmate.unit.entity.Unit; +import com.omatheusmesmo.shoppmate.user.dtos.UserResponseDTO; +import com.omatheusmesmo.shoppmate.user.entity.User; +import com.omatheusmesmo.shoppmate.user.mapper.UserMapper;public Unit toEntity(UnitRequestDTO requestDTO, User owner) { - Unit unit = new Unit(); + var unit = new Unit(); unit.setName(requestDTO.name()); unit.setSymbol(requestDTO.symbol()); unit.setIsSystemStandard(false); unit.setOwner(owner); return unit; } public UnitResponseDTO toResponseDTO(Unit unit) { - UserResponseDTO ownerDTO = unit.getOwner() != null ? userMapper.toResponseDTO(unit.getOwner()) : null; + var ownerDTO = unit.getOwner() != null ? userMapper.toResponseDTO(unit.getOwner()) : null; return new UnitResponseDTO(unit.getId(), unit.getName(), unit.getSymbol(), unit.getIsSystemStandard(), ownerDTO); }As per coding guidelines, imports must follow the order
java.*,jakarta.*,org.springframework.*, third-party, project; andvarmust be used for local variables when the type is obvious from context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java` around lines 3 - 9, Reorder the imports so org.springframework.stereotype.Component appears with other framework imports (after java.*/jakarta.* and before project packages) instead of after the project imports, and within the UnitMapper class replace the explicit local-type declarations on the obvious-context locals (the two variables currently declared at lines shown in the review) with var (e.g., in the mapToEntity/mapToResponse helper bodies or the methods that construct Unit/UnitResponseDTO) to comply with the coding guidelines.backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java (2)
75-154: ⚡ Quick winAccess-control negative tests are missing — the core security guarantee of this PR is not validated by the integration test.
The PR's acceptance criteria require that users cannot delete or modify system-standard records, or records owned by other users. The existing tests only cover the owner/happy-path. The following scenarios are untested:
- Attempting to
DELETEa system-standard category → should fail (e.g., 403/405)- Attempting to
PUTa system-standard category → should fail- Attempting to
DELETEorPUTa category whoseowner_iddiffers from the authenticated user → should failWithout these, the permission enforcement added in the service layer is not validated end-to-end.
Do you want me to generate the missing negative test cases, or open a new issue to track this gap?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java` around lines 75 - 154, The integration test suite in CategoryControllerWithIntegrationTest is missing access-control negative tests that verify service-layer permission checks; add tests that exercise DELETE and PUT against system-standard categories and against categories owned by another user and assert forbidden responses (e.g., 403 or 405) instead of success. Implement new test methods (e.g., testDeleteSystemCategory_Forbid, testPutSystemCategory_Forbid, testDeleteOtherUserCategory_Forbid, testPutOtherUserCategory_Forbid) that create a system-standard Category (setIsSystemStandard(true)) or a Category with a different owner (create another User via userRepository), then call the controller endpoints using given(specification).pathParam(...).when().delete/put(...) and assert the expected error status; reuse createCategoryToTest as a template but adapt it to setIsSystemStandard(true) or assign a different owner to cover both denial scenarios.
147-154: Hardcoded email creates fragile coupling toTestUserFactoryinternals and is duplicated across multiple test files.
"user@gmail.com"is hardcoded as a local variable inTestUserFactory.createTokenForTestUser()(line 21) with no public constant or getter method exposing it. The same email string is repeated in at least two test files (ItemControllerWithIntegrationTestandCategoryControllerWithIntegrationTest). If that implementation changes,orElseThrow()will fail silently withNoSuchElementException. Extract the email to a public constant inTestUserFactory(e.g.,static final String TEST_USER_EMAIL = "user@gmail.com") and reference it in all test files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java` around lines 147 - 154, Replace the hardcoded email literal used in createCategoryToTest() (the userRepository.findByEmail("user@gmail.com").orElseThrow() call) with a public constant exposed by TestUserFactory; add a static final String TEST_USER_EMAIL = "user@gmail.com" to TestUserFactory (where createTokenForTestUser() currently hardcodes the email) and update all tests (e.g., CategoryControllerWithIntegrationTest.createCategoryToTest() and ItemControllerWithIntegrationTest) to call userRepository.findByEmail(TestUserFactory.TEST_USER_EMAIL).orElseThrow() so tests no longer depend on duplicate string literals.backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java (1)
22-32: ⚡ Quick winAll four dependencies should use constructor injection with
finalfields.The new
categoryMapperandunitMapperfields (lines 28–32) are added using field injection, extending the existingcategoryRepository/unitRepositorypattern. Constructor injection is required by the project guidelines.♻️ Proposed refactor
`@Component` public class ItemMapper { - `@Autowired` - private CategoryRepository categoryRepository; - - `@Autowired` - private UnitRepository unitRepository; - - `@Autowired` - private CategoryMapper categoryMapper; - - `@Autowired` - private UnitMapper unitMapper; + private final CategoryRepository categoryRepository; + private final UnitRepository unitRepository; + private final CategoryMapper categoryMapper; + private final UnitMapper unitMapper; + + public ItemMapper(CategoryRepository categoryRepository, UnitRepository unitRepository, + CategoryMapper categoryMapper, UnitMapper unitMapper) { + this.categoryRepository = categoryRepository; + this.unitRepository = unitRepository; + this.categoryMapper = categoryMapper; + this.unitMapper = unitMapper; + }As per coding guidelines: "Always use constructor injection with
finalfields."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java` around lines 22 - 32, The four injected dependencies in ItemMapper (categoryRepository, unitRepository, categoryMapper, unitMapper) are using field injection; change them to final fields and use constructor injection instead: make the fields private final, remove `@Autowired` on fields, add a constructor (or annotate a single constructor with `@Autowired`) that takes CategoryRepository, UnitRepository, CategoryMapper and UnitMapper and assigns them to the final fields so the ItemMapper uses constructor injection per project guidelines.backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java (1)
105-139: ⚡ Quick winMissing
removeCategory_NotFound_ThrowsNoSuchElementExceptiontest.When
findByIdAndDeletedFalsereturnsOptional.empty(),removeCategoryshould throwNoSuchElementException. This path is already covered forfindCategoryById(line 80) but is absent forremoveCategory.✅ Proposed addition
+ `@Test` + void removeCategory_NotFound_ThrowsNoSuchElementException() { + // Arrange + Long missingId = category.getId() + 1000; + when(categoryRepository.findByIdAndDeletedFalse(missingId)).thenReturn(Optional.empty()); + + // Act & Assert + assertThrows(NoSuchElementException.class, () -> categoryService.removeCategory(missingId, user)); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java` around lines 105 - 139, Add a test in CategoryServiceTest named removeCategory_NotFound_ThrowsNoSuchElementException that mocks categoryRepository.findByIdAndDeletedFalse(...) to return Optional.empty() for the target id and then asserts that calling categoryService.removeCategory(id, user) throws NoSuchElementException; reference the existing removeCategory test patterns, use the same id (e.g., category.getId() or a fresh id) and ensure you verify no other interactions are required.backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java (1)
98-146: ⚡ Quick winMissing
removeUnitById_NotFoundandverifyOwnershipOrSystem_OtherOwnertest cases.Two scenarios present in
CategoryServiceTestare absent here:
removeUnitById_NotFound_ThrowsNoSuchElementException— whenfindByIdAndDeletedFalsereturnsOptional.empty(), the service should throwNoSuchElementException. Without this test, the not-found path inremoveUnitByIdis uncovered.verifyOwnershipOrSystem_OtherOwner_ThrowsResourceOwnershipException—CategoryServiceTestcovers this at line 153, but the symmetric unit test is missing, leaving a coverage gap in the unit ownership-check path.✅ Proposed additions
+ `@Test` + void removeUnitById_NotFound_ThrowsNoSuchElementException() { + // Arrange + when(unitRepository.findByIdAndDeletedFalse(unit.getId())).thenReturn(Optional.empty()); + + // Act & Assert + assertThrows(NoSuchElementException.class, () -> unitService.removeUnitById(unit.getId(), user)); + } + + `@Test` + void verifyOwnershipOrSystem_OtherOwner_ThrowsResourceOwnershipException() { + // Arrange + User otherOwner = UserTestFactory.createValidUser(); + unit.setOwner(otherOwner); + + // Act & Assert + assertThrows(ResourceOwnershipException.class, () -> unitService.verifyOwnershipOrSystem(unit, user)); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java` around lines 98 - 146, Add two unit tests to cover the missing not-found and other-owner paths: create removeUnitById_NotFound_ThrowsNoSuchElementException which stubs unitRepository.findByIdAndDeletedFalse(unit.getId()) to return Optional.empty() and asserts that unitService.removeUnitById(...) throws NoSuchElementException; and create verifyOwnershipOrSystem_OtherOwner_ThrowsResourceOwnershipException which sets unit.setOwner(otherOwner) (use UserTestFactory.createValidUser()), then calls unitService.verifyOwnershipOrSystem(unit, user) and asserts it throws ResourceOwnershipException. Ensure you reference the existing unitRepository.findByIdAndDeletedFalse, unitService.removeUnitById, and unitService.verifyOwnershipOrSystem paths and use the same test fixtures (unit, user) as other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.java`:
- Around line 19-20: The JPQL in CategoryRepository.findAllAccessibleByUserId
uses the path expression c.owner.id which causes an implicit inner join and thus
excludes system-standard categories with null owner; update the `@Query` to LEFT
JOIN c.owner (alias it, e.g., o) and replace c.owner.id = :userId with o.id =
:userId so the WHERE reads something like c.deleted = false AND
(c.isSystemStandard = true OR o.id = :userId), keeping the same method signature
and `@Param`.
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.java`:
- Around line 41-55: removeCategory currently passes a soft-deleted Category
through saveCategory which hardcodes auditService.setAuditData(category, true),
so deletes/updates are being saved as new rows; change the persistence path so
updates and deletes call setAuditData(..., false) before saving (either by
adding an isNew parameter to saveCategory or by saving directly in
removeCategory and the update handler), and update
CategoryController.updateCategory/editCategory to call
auditService.setAuditData(category, false) prior to repository.save; ensure
removeCategory calls auditService.softDelete(category) then
auditService.setAuditData(category, false) and persists via the repository (or
saveCategory(false)) rather than using the "new" path.
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.java`:
- Around line 47-55: The addUnit POST currently returns 201 without a Location
header; update UnitController.addUnit to build a Location URI for the newly
created Unit using ServletUriComponentsBuilder with the new entity id from
addedUnit (e.g., addedUnit.getId()) and return
ResponseEntity.created(location).body(responseDTO); keep the existing
unitMapper.toEntity, unitService.saveUnit, and unitMapper.toResponseDTO calls
but replace ResponseEntity.status(HttpStatus.CREATED).body(...) with the
created(...) variant that includes the Location header.
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.java`:
- Around line 21-22: The JPQL path expression u.owner.id in
findAllAccessibleByUserId causes an implicit inner join that excludes
system-standard units (owner = null); change the query to explicitly LEFT JOIN
the owner (e.g., "SELECT u FROM Unit u LEFT JOIN u.owner o WHERE u.deleted =
false AND (u.isSystemStandard = true OR o.id = :userId)") so null-owner rows are
preserved; apply the same LEFT JOIN fix to
CategoryRepository.findAllAccessibleByUserId to ensure system-standard
categories are returned.
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.java`:
- Around line 58-63: removeUnitById currently soft-deletes the Unit via
auditService.softDelete(unit) and then calls saveUnit(unit), but saveUnit always
calls auditService.setAuditData(unit, true) which incorrectly marks the deleted
entity as new; instead, after auditService.softDelete(unit) do not route through
saveUnit; call the persistence save directly (e.g., unitRepository.save(unit))
and ensure you call auditService.setAuditData(unit, false) (or omit setting
isNew=true) before persisting so the deletion is recorded correctly; update
UnitService.removeUnitById to use auditService.softDelete(unit) +
auditService.setAuditData(unit, false) + direct repository save rather than
saveUnit.
In
`@backend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sql`:
- Around line 1-12: Add proper foreign key constraints for the new owner_id
columns so DB enforces referential integrity: alter the categories and units
tables to add FK constraints referencing users(id) with ON DELETE SET NULL (use
descriptive names like fk_categories_owner_id_users_id and
fk_units_owner_id_users_id) so categories.owner_id and units.owner_id cannot
reference non-existent users and are set to NULL when a user is deleted; keep
the existing indexes as-is.
---
Nitpick comments:
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java`:
- Around line 22-32: The four injected dependencies in ItemMapper
(categoryRepository, unitRepository, categoryMapper, unitMapper) are using field
injection; change them to final fields and use constructor injection instead:
make the fields private final, remove `@Autowired` on fields, add a constructor
(or annotate a single constructor with `@Autowired`) that takes
CategoryRepository, UnitRepository, CategoryMapper and UnitMapper and assigns
them to the final fields so the ItemMapper uses constructor injection per
project guidelines.
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java`:
- Around line 3-9: Reorder the imports so
org.springframework.stereotype.Component appears with other framework imports
(after java.*/jakarta.* and before project packages) instead of after the
project imports, and within the UnitMapper class replace the explicit local-type
declarations on the obvious-context locals (the two variables currently declared
at lines shown in the review) with var (e.g., in the mapToEntity/mapToResponse
helper bodies or the methods that construct Unit/UnitResponseDTO) to comply with
the coding guidelines.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java`:
- Around line 75-154: The integration test suite in
CategoryControllerWithIntegrationTest is missing access-control negative tests
that verify service-layer permission checks; add tests that exercise DELETE and
PUT against system-standard categories and against categories owned by another
user and assert forbidden responses (e.g., 403 or 405) instead of success.
Implement new test methods (e.g., testDeleteSystemCategory_Forbid,
testPutSystemCategory_Forbid, testDeleteOtherUserCategory_Forbid,
testPutOtherUserCategory_Forbid) that create a system-standard Category
(setIsSystemStandard(true)) or a Category with a different owner (create another
User via userRepository), then call the controller endpoints using
given(specification).pathParam(...).when().delete/put(...) and assert the
expected error status; reuse createCategoryToTest as a template but adapt it to
setIsSystemStandard(true) or assign a different owner to cover both denial
scenarios.
- Around line 147-154: Replace the hardcoded email literal used in
createCategoryToTest() (the
userRepository.findByEmail("user@gmail.com").orElseThrow() call) with a public
constant exposed by TestUserFactory; add a static final String TEST_USER_EMAIL =
"user@gmail.com" to TestUserFactory (where createTokenForTestUser() currently
hardcodes the email) and update all tests (e.g.,
CategoryControllerWithIntegrationTest.createCategoryToTest() and
ItemControllerWithIntegrationTest) to call
userRepository.findByEmail(TestUserFactory.TEST_USER_EMAIL).orElseThrow() so
tests no longer depend on duplicate string literals.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java`:
- Around line 105-139: Add a test in CategoryServiceTest named
removeCategory_NotFound_ThrowsNoSuchElementException that mocks
categoryRepository.findByIdAndDeletedFalse(...) to return Optional.empty() for
the target id and then asserts that calling categoryService.removeCategory(id,
user) throws NoSuchElementException; reference the existing removeCategory test
patterns, use the same id (e.g., category.getId() or a fresh id) and ensure you
verify no other interactions are required.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java`:
- Around line 180-196: Introduce a single shared constant for the test user
email (e.g., TEST_USER_EMAIL) and stop doing duplicate repository lookups by
resolving the User once and reusing it; specifically, change the helpers
Category createCategoryToTest() and Unit createUnitToTest() to either accept a
User parameter or use a cached field resolved once (e.g., in a `@BeforeEach`
setup) so that createItemToTest() reuses the same User instance instead of
calling userRepository.findByEmail("user@gmail.com") twice, and when resolving
the user include the constant in the lookup so any failure message/reference
points to the named constant rather than a hardcoded string.
- Around line 55-56: The test class ItemControllerWithIntegrationTest uses field
injection for userRepository (and the other four `@Autowired` fields); change
these to constructor injection: make each dependency a private final field
(including UserRepository), remove the `@Autowired` annotations from the fields,
and add a single constructor for ItemControllerWithIntegrationTest that accepts
UserRepository (and the other repository/service fields) and assigns them to the
final fields; you can omit `@Autowired` on the constructor if using a single
constructor or annotate it if preferred.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java`:
- Around line 98-146: Add two unit tests to cover the missing not-found and
other-owner paths: create removeUnitById_NotFound_ThrowsNoSuchElementException
which stubs unitRepository.findByIdAndDeletedFalse(unit.getId()) to return
Optional.empty() and asserts that unitService.removeUnitById(...) throws
NoSuchElementException; and create
verifyOwnershipOrSystem_OtherOwner_ThrowsResourceOwnershipException which sets
unit.setOwner(otherOwner) (use UserTestFactory.createValidUser()), then calls
unitService.verifyOwnershipOrSystem(unit, user) and asserts it throws
ResourceOwnershipException. Ensure you reference the existing
unitRepository.findByIdAndDeletedFalse, unitService.removeUnitById, and
unitService.verifyOwnershipOrSystem paths and use the same test fixtures (unit,
user) as other tests.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: e84842f6-7f95-445f-a969-6584fc8820ac
📒 Files selected for processing (24)
backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/mapper/CategoryMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.javabackend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.javabackend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sqlbackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
matheusandre1
force-pushed
the
issue20
branch
2 times, most recently
from
3f10841 to
774bf56
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java (1)
13-37: ⚡ Quick winExtract shared initialization into a private helper to eliminate duplication.
Lines 14–20 in
createValidUnit()are repeated verbatim increateValidSystemUnit()(lines 27–33). Introduce a private builder to satisfy DRY.♻️ Proposed refactor
public class UnitTestFactory { private static final AtomicLong ID_GENERATOR = new AtomicLong(1); + private static Unit buildBaseUnit() { + var unit = new Unit(); + unit.setId(ID_GENERATOR.getAndIncrement()); + unit.setName(FakerUtil.getFaker().science().unit()); + unit.setSymbol(FakerUtil.getFaker().science().unit()); + unit.setCreatedAt(LocalDateTime.now()); + unit.setUpdatedAt(LocalDateTime.now()); + unit.setDeleted(false); + return unit; + } + public static Unit createValidUnit() { - Unit unit = new Unit(); - unit.setId(ID_GENERATOR.getAndIncrement()); - unit.setName(FakerUtil.getFaker().science().unit()); - unit.setSymbol(FakerUtil.getFaker().science().unit()); - unit.setCreatedAt(LocalDateTime.now()); - unit.setUpdatedAt(LocalDateTime.now()); - unit.setDeleted(false); + var unit = buildBaseUnit(); unit.setIsSystemStandard(false); unit.setOwner(UserTestFactory.createValidUser()); return unit; } public static Unit createValidSystemUnit() { - Unit unit = new Unit(); - unit.setId(ID_GENERATOR.getAndIncrement()); - unit.setName(FakerUtil.getFaker().science().unit()); - unit.setSymbol(FakerUtil.getFaker().science().unit()); - unit.setCreatedAt(LocalDateTime.now()); - unit.setUpdatedAt(LocalDateTime.now()); - unit.setDeleted(false); + var unit = buildBaseUnit(); unit.setIsSystemStandard(true); unit.setOwner(null); return unit; } }As per coding guidelines, "Use
varfor local variables when the type is obvious" (backend/**/*).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java` around lines 13 - 37, Extract the repeated initialization in createValidUnit and createValidSystemUnit into a private helper (e.g., a private static method like buildBaseUnit or createBaseUnit) that sets id (ID_GENERATOR.getAndIncrement()), name, symbol (FakerUtil.getFaker().science().unit()), createdAt, updatedAt, deleted=false and returns the Unit; then have createValidUnit call that helper, set isSystemStandard=false and owner=UserTestFactory.createValidUser(), and have createValidSystemUnit call the helper, set isSystemStandard=true and owner=null; also use var for local Unit variables where the type is obvious (replace explicit Unit unit = ... with var unit = ...).backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java (1)
182-196: 💤 Low value
createCategoryToTest()andcreateUnitToTest()correctly reflect the new ownership model.Both helpers mirror the pattern established in
CategoryControllerWithIntegrationTest(lines 147–157). One minor inconsistency: both methods are package-private, while the analogous helper inCategoryControllerWithIntegrationTestis declaredprivate. SincetestUseris an instance field initialised in@BeforeEach, any invocation outside the JUnit lifecycle would produce a silent NPE. Marking themprivatecloses that door.♻️ Suggested visibility fix
- Category createCategoryToTest() { + private Category createCategoryToTest() { - Unit createUnitToTest() { + private Unit createUnitToTest() { - Item createItemToTest() { + private Item createItemToTest() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java` around lines 182 - 196, The helper methods createCategoryToTest() and createUnitToTest() are package-private but should be declared private to match CategoryControllerWithIntegrationTest and avoid accidental use outside the JUnit lifecycle (which could cause a silent NPE on the testUser field); update the method declarations for createCategoryToTest() and createUnitToTest() to be private, leaving their bodies and usage unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.java`:
- Around line 58-65: The verifyOwnershipOrSystem method currently allows
modification when category.getOwner() == null and category.getIsSystemStandard()
== false, so update verifyOwnershipOrSystem(Category category, User currentUser)
to treat a null owner on non-system categories as unauthorized: after the
isSystemStandard check, if category.getOwner() is null then throw
ResourceOwnershipException with a clear message (e.g., "Non-system categories
must have an owner; operation not permitted"), and keep the existing check that
owner.getId().equals(currentUser.getId()) for owned categories; alternatively,
if your intended model is to guarantee owners, enforce the invariant earlier
(e.g., in Category creation/validation) so verifyOwnershipOrSystem can safely
assume owner != null.
---
Nitpick comments:
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java`:
- Around line 182-196: The helper methods createCategoryToTest() and
createUnitToTest() are package-private but should be declared private to match
CategoryControllerWithIntegrationTest and avoid accidental use outside the JUnit
lifecycle (which could cause a silent NPE on the testUser field); update the
method declarations for createCategoryToTest() and createUnitToTest() to be
private, leaving their bodies and usage unchanged.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java`:
- Around line 13-37: Extract the repeated initialization in createValidUnit and
createValidSystemUnit into a private helper (e.g., a private static method like
buildBaseUnit or createBaseUnit) that sets id (ID_GENERATOR.getAndIncrement()),
name, symbol (FakerUtil.getFaker().science().unit()), createdAt, updatedAt,
deleted=false and returns the Unit; then have createValidUnit call that helper,
set isSystemStandard=false and owner=UserTestFactory.createValidUser(), and have
createValidSystemUnit call the helper, set isSystemStandard=true and owner=null;
also use var for local Unit variables where the type is obvious (replace
explicit Unit unit = ... with var unit = ...).
🪄 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: 6f07c582-1a83-4795-b1de-a8abc6f75951
📒 Files selected for processing (25)
backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/mapper/CategoryMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.javabackend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.javabackend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sqlbackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testcontainers/utils/TestUserFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
✅ Files skipped from review due to trivial changes (9)
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testcontainers/utils/TestUserFactory.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (7)
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java
- backend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sql
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.java
Contributor
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 (1)
backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java (1)
34-45:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftResolve Category and Unit through user-scoped lookups, not unrestricted
findById().This path still accepts any existing
idCategory/idUnit, so a user can attach an item to another user's private Category/Unit or to a soft-deleted record if they know the id. That breaks the ownership/access rules this PR is introducing. Move these lookups into a service method that has the authenticated user context, or switch to repository/service lookups that enforce “accessible and not deleted” before mapping.🤖 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 `@backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java` around lines 34 - 45, ItemMapper.toEntity currently resolves Category and Unit via categoryRepository.findById(...) and unitRepository.findById(...), which allows attaching items to any existing or soft-deleted records; change the mapping so the mapper does not perform unrestricted lookups: either accept already-resolved Category and Unit (resolved by a service that enforces ownership/soft-delete rules) or call repository/service methods that enforce access (e.g. findByIdAndOwnerAndNotDeleted or a service like CategoryService.resolveForUser(dto.idCategory(), currentUser)). Update ItemMapper.toEntity to use those guarded lookups or injected resolved entities, and rely on the service layer to throw when a Category/Unit is not accessible.
🧹 Nitpick comments (4)
backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java (1)
13-34: ⚡ Quick winRefactor duplicated category setup into a base builder.
Both factory methods repeat the same initialization block, which can drift over time.
Possible refactor
+ private static Category buildBaseCategory() { + var category = new Category(); + category.setId(ID_GENERATOR.getAndIncrement()); + category.setName(FakerUtil.getFaker().commerce().department()); + category.setCreatedAt(LocalDateTime.now()); + category.setUpdatedAt(LocalDateTime.now()); + category.setDeleted(false); + return category; + } + public static Category createValidCategory() { - Category category = new Category(); - category.setId(ID_GENERATOR.getAndIncrement()); - category.setName(FakerUtil.getFaker().commerce().department()); - category.setCreatedAt(LocalDateTime.now()); - category.setUpdatedAt(LocalDateTime.now()); - category.setDeleted(false); + var category = buildBaseCategory(); category.setIsSystemStandard(false); category.setOwner(UserTestFactory.createValidUser()); return category; } public static Category createValidSystemCategory() { - Category category = new Category(); - category.setId(ID_GENERATOR.getAndIncrement()); - category.setName(FakerUtil.getFaker().commerce().department()); - category.setCreatedAt(LocalDateTime.now()); - category.setUpdatedAt(LocalDateTime.now()); - category.setDeleted(false); + var category = buildBaseCategory(); category.setIsSystemStandard(true); category.setOwner(null); return category; }🤖 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 `@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java` around lines 13 - 34, The two factory methods createValidCategory and createValidSystemCategory in CategoryTestFactory duplicate the same Category initialization; extract the common setup into a single private helper (e.g., buildBaseCategory or initializeCategory) that sets id via ID_GENERATOR.getAndIncrement(), name via FakerUtil.getFaker().commerce().department(), createdAt/updatedAt to LocalDateTime.now(), deleted to false, and then have createValidCategory and createValidSystemCategory call that helper and only apply their specific tweaks (setIsSystemStandard and owner) to avoid duplication and drift.backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java (1)
144-167: ⚡ Quick winAdd a regression test for non-system units with null owner.
This edge case should assert
ResourceOwnershipExceptionto prevent future authorization regressions.🧪 Suggested test
+ `@Test` + void verifyOwnershipOrSystem_NonSystemWithoutOwner_ThrowsResourceOwnershipException() { + // Arrange + unit.setIsSystemStandard(false); + unit.setOwner(null); + + // Act & Assert + assertThrows(ResourceOwnershipException.class, () -> unitService.verifyOwnershipOrSystem(unit, user)); + }🤖 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 `@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java` around lines 144 - 167, Add a regression test in UnitServiceTest that covers a non-system unit with a null owner: create a non-system Unit (e.g., via UnitTestFactory.createValidUnit() or the existing unit test fixture), set its owner to null (unit.setOwner(null)), then assert that unitService.verifyOwnershipOrSystem(unit, user) throws ResourceOwnershipException; reference the verifyOwnershipOrSystem method, the unit test class UnitServiceTest, and the unit variable when locating where to add this test.backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.java (1)
60-65: ⚡ Quick winReturn
ResponseEntity<Void>from the delete endpoint instead ofvoid.That keeps the controller contract consistent with the rest of this layer and with the controller guideline.
Suggested fix
- `@DeleteMapping`("/{id}") - `@ResponseStatus`(HttpStatus.NO_CONTENT) - public void deleteCategory(`@PathVariable` Long id, `@AuthenticationPrincipal` User user) { + `@DeleteMapping`("/{id}") + public ResponseEntity<Void> deleteCategory(`@PathVariable` Long id, `@AuthenticationPrincipal` User user) { categoryService.removeCategory(id, user); + return ResponseEntity.noContent().build(); }As per coding guidelines,
backend/src/**/controller/*.java: "Controllers must use@RestController,@Operation(summary),ResponseEntity<T>, andServletUriComponentsBuilderfor Location headers".🤖 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 `@backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.java` around lines 60 - 65, The deleteCategory endpoint currently returns void; change its signature in CategoryController to return ResponseEntity<Void> and remove the `@ResponseStatus` annotation; call categoryService.removeCategory(id, user) as before and then return ResponseEntity.noContent().build() to keep the controller contract consistent with other endpoints and guidelines (referencing deleteCategory and categoryService.removeCategory).backend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.java (1)
32-35: ⚡ Quick winInclude the category id in the not-found exception message.
Dropping the identifier makes API errors and logs harder to diagnose than the service convention used elsewhere.
Suggested fix
- return categoryRepository.findByIdAndDeletedFalse(id) - .orElseThrow(() -> new NoSuchElementException("Category not found")); + return categoryRepository.findByIdAndDeletedFalse(id) + .orElseThrow(() -> new NoSuchElementException("Category not found with id: " + id));As per coding guidelines,
backend/src/main/java/**/service/**/*.java: "Throw NoSuchElementException for not found errors with message format: "Item not found with id: " + id".🤖 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 `@backend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.java` around lines 32 - 35, Update CategoryService.findCategoryById to include the id in the NoSuchElementException message: when categoryRepository.findByIdAndDeletedFalse(id) is empty, throw new NoSuchElementException("Item not found with id: " + id) (follow the project's required message format). Locate the throw in findCategoryById and replace the current "Category not found" literal with the specified formatted message including the id.
🤖 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
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.java`:
- Around line 22-23: The field declaration for isSystemStandard uses boxed
Boolean while the JPA column is non-nullable; change the Category class's
private Boolean isSystemStandard = false to the primitive type private boolean
isSystemStandard = false, update any corresponding getter/setter signatures
(getIsSystemStandard()/setIsSystemStandard(...) or isSystemStandard() as used)
to use primitive boolean, and audit usages of Category.isSystemStandard or
setIsSystemStandard to handle primitive boolean (remove null checks) so
in-memory state cannot be null and persistence matches the non-nullable column.
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.java`:
- Around line 66-72: The verifyOwnershipOrSystem method currently allows units
with owner == null to be modified; change its authorization to fail-closed by
checking owner presence and identity: keep the existing check for
unit.getIsSystemStandard(), then replace the second condition with a single
guard that throws ResourceOwnershipException when unit.getOwner() is null OR
unit.getOwner().getId() does not equal currentUser.getId(); reference method
verifyOwnershipOrSystem, class Unit (getIsSystemStandard, getOwner) and User
(getId) to locate and update the logic so null-owner units are denied.
In
`@backend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sql`:
- Around line 2-7: The migration adds is_system_standard and owner_id to
categories and units but doesn't backfill existing rows, which will be
inaccessible under the new "system-standard OR owned-by-user" filter; update
V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sql to, immediately
after the ALTER TABLE statements, backfill existing rows so they match one
branch of the access filter (e.g., set is_system_standard = TRUE for all
existing rows in categories and units, or assign a valid system owner_id to them
if your design prefers owned-by-user), and ensure you update both tables
(categories and units) for both new columns so legacy rows are discoverable
after migration.
---
Outside diff comments:
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java`:
- Around line 34-45: ItemMapper.toEntity currently resolves Category and Unit
via categoryRepository.findById(...) and unitRepository.findById(...), which
allows attaching items to any existing or soft-deleted records; change the
mapping so the mapper does not perform unrestricted lookups: either accept
already-resolved Category and Unit (resolved by a service that enforces
ownership/soft-delete rules) or call repository/service methods that enforce
access (e.g. findByIdAndOwnerAndNotDeleted or a service like
CategoryService.resolveForUser(dto.idCategory(), currentUser)). Update
ItemMapper.toEntity to use those guarded lookups or injected resolved entities,
and rely on the service layer to throw when a Category/Unit is not accessible.
---
Nitpick comments:
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.java`:
- Around line 60-65: The deleteCategory endpoint currently returns void; change
its signature in CategoryController to return ResponseEntity<Void> and remove
the `@ResponseStatus` annotation; call categoryService.removeCategory(id, user) as
before and then return ResponseEntity.noContent().build() to keep the controller
contract consistent with other endpoints and guidelines (referencing
deleteCategory and categoryService.removeCategory).
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.java`:
- Around line 32-35: Update CategoryService.findCategoryById to include the id
in the NoSuchElementException message: when
categoryRepository.findByIdAndDeletedFalse(id) is empty, throw new
NoSuchElementException("Item not found with id: " + id) (follow the project's
required message format). Locate the throw in findCategoryById and replace the
current "Category not found" literal with the specified formatted message
including the id.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java`:
- Around line 13-34: The two factory methods createValidCategory and
createValidSystemCategory in CategoryTestFactory duplicate the same Category
initialization; extract the common setup into a single private helper (e.g.,
buildBaseCategory or initializeCategory) that sets id via
ID_GENERATOR.getAndIncrement(), name via
FakerUtil.getFaker().commerce().department(), createdAt/updatedAt to
LocalDateTime.now(), deleted to false, and then have createValidCategory and
createValidSystemCategory call that helper and only apply their specific tweaks
(setIsSystemStandard and owner) to avoid duplication and drift.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java`:
- Around line 144-167: Add a regression test in UnitServiceTest that covers a
non-system unit with a null owner: create a non-system Unit (e.g., via
UnitTestFactory.createValidUnit() or the existing unit test fixture), set its
owner to null (unit.setOwner(null)), then assert that
unitService.verifyOwnershipOrSystem(unit, user) throws
ResourceOwnershipException; reference the verifyOwnershipOrSystem method, the
unit test class UnitServiceTest, and the unit variable when locating where to
add this test.
🪄 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: ee54240f-fada-4a97-bdbe-dac70ec48f8d
📒 Files selected for processing (25)
backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/mapper/CategoryMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.javabackend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.javabackend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sqlbackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testcontainers/utils/TestUserFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java (1)
3-9: ⚡ Quick winFix import ordering to match project Java standards.
org.springframework.*imports must come before projectcom.omatheusmesmo.shoppmate.*imports in this codebase’s configured order.Suggested change
-import com.omatheusmesmo.shoppmate.unit.dto.UnitRequestDTO; -import com.omatheusmesmo.shoppmate.unit.dto.UnitResponseDTO; -import com.omatheusmesmo.shoppmate.unit.entity.Unit; -import com.omatheusmesmo.shoppmate.user.dtos.UserResponseDTO; -import com.omatheusmesmo.shoppmate.user.entity.User; -import com.omatheusmesmo.shoppmate.user.mapper.UserMapper; import org.springframework.stereotype.Component; + +import com.omatheusmesmo.shoppmate.unit.dto.UnitRequestDTO; +import com.omatheusmesmo.shoppmate.unit.dto.UnitResponseDTO; +import com.omatheusmesmo.shoppmate.unit.entity.Unit; +import com.omatheusmesmo.shoppmate.user.dtos.UserResponseDTO; +import com.omatheusmesmo.shoppmate.user.entity.User; +import com.omatheusmesmo.shoppmate.user.mapper.UserMapper;As per coding guidelines, “Order imports as: java., jakarta., org.springframework., third-party, project packages in com.omatheusmesmo.shoppmate. domain”.
🤖 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 `@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java` around lines 3 - 9, Import ordering in UnitMapper.java does not follow project standards; move the org.springframework imports above the project com.omatheusmesmo.shoppmate.* imports so the file follows "java.*, jakarta.*, org.springframework.*, third-party, com.omatheusmesmo.shoppmate.*" ordering. Specifically, in the UnitMapper class file adjust the import block so imports like org.springframework.stereotype.Component appear before imports such as com.omatheusmesmo.shoppmate.unit.dto.UnitRequestDTO, com.omatheusmesmo.shoppmate.unit.dto.UnitResponseDTO, com.omatheusmesmo.shoppmate.unit.entity.Unit, com.omatheusmesmo.shoppmate.user.dtos.UserResponseDTO, com.omatheusmesmo.shoppmate.user.entity.User, and com.omatheusmesmo.shoppmate.user.mapper.UserMapper to comply with the configured import order.backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java (1)
124-142: ⚡ Quick winAssert no mutation on authorization failures.
For forbidden paths, also verify
auditService.softDelete(...),auditService.setAuditData(...), andunitRepository.save(...)are never called. This protects against accidental side effects before throwing.As per coding guidelines, "Structure unit tests with Arrange, Act, Assert comments and phases" and this strengthens Assert-phase guarantees on protected flows.Proposed assertion additions
`@Test` void removeUnitById_SystemStandard_ThrowsResourceOwnershipException() { // Arrange Unit systemUnit = UnitTestFactory.createValidSystemUnit(); when(unitRepository.findByIdAndDeletedFalse(systemUnit.getId())).thenReturn(Optional.of(systemUnit)); // Act & Assert assertThrows(ResourceOwnershipException.class, () -> unitService.removeUnitById(systemUnit.getId(), user)); + verify(auditService, times(0)).softDelete(any(Unit.class)); + verify(auditService, times(0)).setAuditData(any(Unit.class), any(Boolean.class)); + verify(unitRepository, times(0)).save(any(Unit.class)); }Also applies to: 151-176
🤖 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 `@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java` around lines 124 - 142, The tests removeUnitById_SystemStandard_ThrowsResourceOwnershipException and removeUnitById_OtherUserOwner_ThrowsResourceOwnershipException should assert that no side-effects occur before the exception: after calling unitService.removeUnitById(...), add Mockito verifications that auditService.softDelete(...), auditService.setAuditData(...), and unitRepository.save(...) are never invoked (use verify(..., never()) or equivalent); apply the same never-call assertions to the other related tests referenced in the comment so forbidden flows guarantee no mutation.
🤖 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.
Nitpick comments:
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java`:
- Around line 3-9: Import ordering in UnitMapper.java does not follow project
standards; move the org.springframework imports above the project
com.omatheusmesmo.shoppmate.* imports so the file follows "java.*, jakarta.*,
org.springframework.*, third-party, com.omatheusmesmo.shoppmate.*" ordering.
Specifically, in the UnitMapper class file adjust the import block so imports
like org.springframework.stereotype.Component appear before imports such as
com.omatheusmesmo.shoppmate.unit.dto.UnitRequestDTO,
com.omatheusmesmo.shoppmate.unit.dto.UnitResponseDTO,
com.omatheusmesmo.shoppmate.unit.entity.Unit,
com.omatheusmesmo.shoppmate.user.dtos.UserResponseDTO,
com.omatheusmesmo.shoppmate.user.entity.User, and
com.omatheusmesmo.shoppmate.user.mapper.UserMapper to comply with the configured
import order.
In
`@backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java`:
- Around line 124-142: The tests
removeUnitById_SystemStandard_ThrowsResourceOwnershipException and
removeUnitById_OtherUserOwner_ThrowsResourceOwnershipException should assert
that no side-effects occur before the exception: after calling
unitService.removeUnitById(...), add Mockito verifications that
auditService.softDelete(...), auditService.setAuditData(...), and
unitRepository.save(...) are never invoked (use verify(..., never()) or
equivalent); apply the same never-call assertions to the other related tests
referenced in the comment so forbidden flows guarantee no mutation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 292e4c2a-80ec-4819-92b4-60491e9aa2aa
📒 Files selected for processing (25)
backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/mapper/CategoryMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.javabackend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.javabackend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sqlbackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testcontainers/utils/TestUserFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
✅ Files skipped from review due to trivial changes (2)
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.java
🚧 Files skipped from review as they are similar to previous changes (17)
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitResponseDTO.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testcontainers/utils/TestUserFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java
- backend/src/main/resources/db/migration/V9__ADD_SYSTEM_STANDARD_AND_OWNER_TO_CATEGORIES_AND_UNITS.sql
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.java
Signed-off-by: Matheus André <matheusandr2@gmail.com>
omatheusmesmo
requested changes
May 7, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
backend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.java (1)
4-13: ⚡ Quick winReorder imports to match project Java conventions.
Line 4 places project imports before
jakarta.*; this should follow the configured order (java.*,jakarta.*,org.springframework.*, third-party, then project packages).As per coding guidelines, "Order imports as: java., jakarta., org.springframework., third-party, project packages in com.omatheusmesmo.shoppmate. domain".
🤖 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 `@backend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.java` around lines 4 - 13, The import statements in Category.java are ordered incorrectly (project imports appear before jakarta imports); reorder the imports in the file so they follow the project convention: java.*, jakarta.* (e.g., jakarta.persistence.* imports), org.springframework.* (if any), third-party (lombok.*), then project packages (com.omatheusmesmo.shoppmate.* such as com.omatheusmesmo.shoppmate.user.entity.User), ensuring the existing symbols (Class: Category, imports: jakarta.persistence.Column, Entity, FetchType, JoinColumn, ManyToOne; lombok.Getter/Setter/NoArgsConstructor; project User) are preserved and no unused imports are introduced or removed.
🤖 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
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/mapper/CategoryMapper.java`:
- Around line 28-33: The toEntity(Long, CategoryRequestDTO) in CategoryMapper
creates a partial Category with only id and name which risks clearing
owner/isSystemStandard when persisted; update CategoryService.editCategory to
load the existing managed Category (e.g., findById or getReference), then copy
only mutable fields (name) from the DTO into that managed entity instead of
saving the mapper-produced object, or change the mapper to provide an
updateInto(existingCategory, requestDTO) method used by editCategory to preserve
owner and isSystemStandard.
In `@backend/src/main/resources/db/migration/V2__CREATE_TABLE_UNITS.sql`:
- Around line 5-6: The current global UNIQUE index on the name column conflicts
with owner-scoped private units after adding owner_id; drop the global
UNIQUE(name) index and replace it with two owner-aware constraints: a
UNIQUE(name) partial index for system-standard units (WHERE is_system_standard =
TRUE) to preserve global uniqueness for system units, and a UNIQUE(owner_id,
name) partial index (WHERE owner_id IS NOT NULL) to enforce per-owner uniqueness
for private units; apply the same pattern for any other global unique
constraints referenced (lines 19-20) so they become owner-aware (use partial
unique indexes on owner_id + column and separate partial globals for
system-standard rows).
In `@backend/src/main/resources/db/migration/V3__CREATE_TABLE_CATEGORIES.sql`:
- Around line 4-5: The migration currently enforces a global unique constraint
on categories.name which prevents different users from having the same category
name; update the uniqueness to be ownership-aware: remove or replace the global
unique index on name and create a composite unique index for user-owned
categories (e.g., UNIQUE INDEX on (owner_id, name) with owner_id IS NOT NULL)
and a separate unique index for system-standard categories (e.g., UNIQUE INDEX
on (name) WHERE is_system_standard = TRUE) so system rows remain globally unique
while private rows are scoped by owner_id; target the CATEGORIES table and the
columns is_system_standard and owner_id when making this change.
---
Nitpick comments:
In
`@backend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.java`:
- Around line 4-13: The import statements in Category.java are ordered
incorrectly (project imports appear before jakarta imports); reorder the imports
in the file so they follow the project convention: java.*, jakarta.* (e.g.,
jakarta.persistence.* imports), org.springframework.* (if any), third-party
(lombok.*), then project packages (com.omatheusmesmo.shoppmate.* such as
com.omatheusmesmo.shoppmate.user.entity.User), ensuring the existing symbols
(Class: Category, imports: jakarta.persistence.Column, Entity, FetchType,
JoinColumn, ManyToOne; lombok.Getter/Setter/NoArgsConstructor; project User) are
preserved and no unused imports are introduced or removed.
🪄 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: 00b2e45f-c73f-443e-ac40-a293258d9042
📒 Files selected for processing (28)
backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/entity/Category.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/mapper/CategoryMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.javabackend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitResponseDTO.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.javabackend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.javabackend/src/main/resources/db/migration/V2__CREATE_TABLE_UNITS.sqlbackend/src/main/resources/db/migration/V3__CREATE_TABLE_CATEGORIES.sqlbackend/src/main/resources/db/migration/V6__CREATE_TABLE_LIST_ITEMS.sqlbackend/src/main/resources/db/migration/V8__ADD_PRICE_TO_LIST_ITEMS.sqlbackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testcontainers/utils/TestUserFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.javabackend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
💤 Files with no reviewable changes (1)
- backend/src/main/resources/db/migration/V8__ADD_PRICE_TO_LIST_ITEMS.sql
🚧 Files skipped from review as they are similar to previous changes (21)
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testcontainers/utils/TestUserFactory.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/mapper/UnitMapper.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitRequestDTO.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/dto/CategoryResponseDTO.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitDTO.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/item/mapper/ItemMapper.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/repository/CategoryRepository.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/CategoryTestFactory.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/shared/testutils/UnitTestFactory.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/dto/UnitResponseDTO.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/item/controller/ItemControllerWithIntegrationTest.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/entity/Unit.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/repository/UnitRepository.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/service/CategoryService.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/service/CategoryServiceTest.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/category/controller/CategoryController.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/unit/service/UnitServiceTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerWithIntegrationTest.java
- backend/src/test/java/com/omatheusmesmo/shoppmate/category/controller/CategoryControllerTest.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/controller/UnitController.java
- backend/src/main/java/com/omatheusmesmo/shoppmate/unit/service/UnitService.java
Signed-off-by: Matheus André <matheusandr2@gmail.com>
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
2 participants
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.
Closes: #20
Summary by CodeRabbit
Release Notes
New Features
Improvements