Conversation
WalkthroughThe PR adds an ImageTagService and repository batch-update method, and integrates ImageTagService into UserTagController so user tag updates and deletes also propagate to image-tag records. It also raises the user tag limit from 30 to 40 and updates tests accordingly. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UC as UserTagController
participant UTS as UserTagService
participant ITS as ImageTagService
participant ITR as ImageTagRepository
participant UR as UserRepository
participant TR as TagRepository
Note over UC,ITS: Tag update flow
User->>UC: PUT /user-tags/{id}
UC->>UTS: update(userTagDto)
UTS-->>UC: updatedUserTagResponse
UC->>ITS: update(loginUser, oldTagId, newTagId)
ITS->>UR: findByUsername(loginUser.username)
UR-->>ITS: user
ITS->>ITR: updateImageTagsForUser(user.id, oldTagId, newTagId)
ITR-->>ITS: update result
ITS-->>UC: done
UC-->>User: 200 OK
Note over UC,ITS: Tag delete flow
User->>UC: DELETE /user-tags/{id}
UC->>UTS: delete(tagId)
UTS-->>UC: deletionConfirmed
UC->>ITS: delete(loginUser, tagId)
ITS->>UR: findByUsername(loginUser.username)
UR-->>ITS: user
ITS->>TR: findById(tagId)
TR-->>ITS: tag
ITS->>ITR: deleteByTagAndUser(tag, user)
ITR-->>ITS: deletionResult
ITS-->>UC: done
UC-->>User: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes
Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (1)
129-147: Fix incorrect mock in delete test.Line 132 mocks
userTagService.createinstead ofuserTagService.deletein a delete test. This appears to be copy-pasted from the create test and should be corrected.Apply this diff to fix the mock:
@Test void 유저_태그_삭제() { // given - BDDMockito.given(userTagService.create(any(), anyString())).willReturn(new TagResponse(1L, "java")); + BDDMockito.willDoNothing().given(userTagService).delete(any(), anyLong()); BDDMockito.willDoNothing().given(imageTagService).delete(any(), anyLong());
🧹 Nitpick comments (1)
capturecat-core/src/main/java/com/capturecat/core/domain/tag/ImageTagRepository.java (1)
35-37: Document the assumption about newTagId validity.The bulk update directly modifies the
tag.idforeign key, bypassing JPA entity management for performance. WhileclearAutomatically=truemitigates persistence context issues, this assumesnewTagIdis valid. Consider adding a JavaDoc comment documenting this precondition:+/** + * Bulk updates image tags for a specific user by replacing oldTagId with newTagId. + * @param userId the user whose image tags should be updated + * @param oldTagId the current tag ID to replace + * @param newTagId the new tag ID (must exist in Tag table) + */ @Modifying(clearAutomatically = true, flushAutomatically = true) @Query("UPDATE ImageTag it SET it.tag.id = :newTagId WHERE it.tag.id = :oldTagId AND it.image.user.id = :userId") void updateImageTagsForUser(Long userId, Long oldTagId, Long newTagId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java(3 hunks)capturecat-core/src/main/java/com/capturecat/core/domain/tag/ImageTagRepository.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/service/tag/ImageTagService.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java(4 hunks)capturecat-core/src/test/java/com/capturecat/core/domain/tag/ImageTagRepositoryTest.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/service/tag/ImageTagServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
capturecat-core/src/test/java/com/capturecat/core/domain/tag/ImageTagRepositoryTest.java (2)
capturecat-core/src/test/java/com/capturecat/core/DummyObject.java (1)
DummyObject(18-100)capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagFixture.java (1)
TagFixture(5-16)
capturecat-core/src/test/java/com/capturecat/core/service/tag/ImageTagServiceTest.java (2)
capturecat-core/src/test/java/com/capturecat/core/DummyObject.java (1)
DummyObject(18-100)capturecat-core/src/test/java/com/capturecat/core/domain/tag/TagFixture.java (1)
TagFixture(5-16)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
capturecat-core/src/test/java/com/capturecat/core/domain/tag/ImageTagRepositoryTest.java (1)
118-175: Excellent test coverage for user-scoped tag updates!Both new tests effectively validate the batch update operation:
- User isolation is properly tested (only the specified user's tags are updated)
- Selective updates are verified (only images with the old tag are affected)
- Proper use of
entityManager.flush()andclear()ensures database state is verifiedcapturecat-core/src/main/java/com/capturecat/core/service/tag/ImageTagService.java (1)
33-41: Delete method logic looks correct.Proper validation of both user and tag entities before deletion.
capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java (1)
48-57: Update operation order is correct.Creating/updating the user tag first, then propagating to image tags ensures the new tag ID is available for the batch update.
capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (2)
43-49: Test setup correctly integrates ImageTagService.Proper mock initialization and controller construction with the new dependency.
104-127: Update test properly mocks ImageTagService.The test correctly stubs
imageTagService.updateto verify the integration.capturecat-core/src/test/java/com/capturecat/core/service/tag/ImageTagServiceTest.java (1)
44-114: Comprehensive unit test coverage with excellent error handling verification.The test suite thoroughly validates:
- Happy paths for both update and delete operations
- Proper exception handling when user or tag is not found
- Repository interaction verification
- No unintended side effects on failure cases
| @DeleteMapping | ||
| public ApiResponse<?> delete(@AuthenticationPrincipal LoginUser loginUser, @RequestParam Long tagId) { | ||
| userTagService.delete(loginUser, tagId); | ||
| imageTagService.delete(loginUser, tagId); | ||
|
|
||
| return ApiResponse.success(); | ||
| } |
There was a problem hiding this comment.
Reverse the delete operation order to avoid potential errors.
Deleting the user tag before the image tags can cause TAG_NOT_FOUND exception in ImageTagService.delete at line 37-38, which attempts to load the tag after it may have been deleted. This creates fragile transaction-dependent coupling.
Apply this diff to delete image tags first:
@DeleteMapping
public ApiResponse<?> delete(@AuthenticationPrincipal LoginUser loginUser, @RequestParam Long tagId) {
- userTagService.delete(loginUser, tagId);
imageTagService.delete(loginUser, tagId);
+ userTagService.delete(loginUser, tagId);
return ApiResponse.success();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @DeleteMapping | |
| public ApiResponse<?> delete(@AuthenticationPrincipal LoginUser loginUser, @RequestParam Long tagId) { | |
| userTagService.delete(loginUser, tagId); | |
| imageTagService.delete(loginUser, tagId); | |
| return ApiResponse.success(); | |
| } | |
| @DeleteMapping | |
| public ApiResponse<?> delete(@AuthenticationPrincipal LoginUser loginUser, @RequestParam Long tagId) { | |
| imageTagService.delete(loginUser, tagId); | |
| userTagService.delete(loginUser, tagId); | |
| return ApiResponse.success(); | |
| } |
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java
around lines 59 to 65, the controller currently deletes the user tag before
deleting related image tags which can cause ImageTagService.delete to throw
TAG_NOT_FOUND when it tries to load a tag that has already been removed; modify
the method to call imageTagService.delete(loginUser, tagId) before
userTagService.delete(loginUser, tagId) so image tags are removed while the tag
still exists, and ensure the operation remains within the same transaction (or
annotate with @Transactional at the controller/service level) to keep atomicity.
| @Transactional | ||
| public void update(LoginUser loginUser, Long oldTagId, Long newTagId) { | ||
| User user = userRepository.findByUsername(loginUser.getUsername()) | ||
| .orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND)); | ||
|
|
||
| imageTagRepository.updateImageTagsForUser(user.getId(), oldTagId, newTagId); | ||
| } |
There was a problem hiding this comment.
Validate that newTagId exists before updating.
The update method doesn't verify that newTagId corresponds to an existing tag. If an invalid ID is passed, the repository update will create orphaned foreign key references, potentially corrupting data.
Apply this diff to add validation:
@Transactional
public void update(LoginUser loginUser, Long oldTagId, Long newTagId) {
User user = userRepository.findByUsername(loginUser.getUsername())
.orElseThrow(() -> new CoreException(ErrorType.USER_NOT_FOUND));
+ tagRepository.findById(newTagId)
+ .orElseThrow(() -> new CoreException(ErrorType.TAG_NOT_FOUND));
imageTagRepository.updateImageTagsForUser(user.getId(), oldTagId, newTagId);
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
capturecat-core/src/main/java/com/capturecat/core/service/tag/ImageTagService.java
around lines 25 to 31, the update method currently looks up the User but does
not validate that newTagId exists; add a check that the Tag with newTagId exists
(e.g., tagRepository.findById(newTagId).orElseThrow(() -> new
CoreException(ErrorType.TAG_NOT_FOUND))) before calling
imageTagRepository.updateImageTagsForUser, and throw an appropriate
CoreException if not found so the update cannot create invalid foreign-key
references.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java (1)
33-33: Limit increase looks good, though scope seems unrelated.The increase from 30 to 40 is straightforward and properly tested. Note that this change appears unrelated to the main PR objective of syncing user tag changes to image tags—consider grouping unrelated changes in separate PRs for clearer review context in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: deploy-dev
🔇 Additional comments (1)
capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (1)
118-118: Test correctly updated for new limit.The mock value correctly reflects the updated
MAX_USER_TAG_COUNTof 40, properly testing the boundary condition where the user already has 40 tags and attempts to create another.
2 participants
📌 관련 이슈 (필수)
📝 작업 내용 (필수)
유저 태그 수정 및 삭제할 때 이미지 태그에도 반영이 되도록 했습니다.
💬 리뷰 참고 사항 (선택)
Summary by CodeRabbit
New Features
Tests