feat: 유저 태그 삭제 API #128
feat: 유저 태그 삭제 API #128
Conversation
WalkthroughAdds a DELETE /v1/user-tags endpoint to remove a user tag by id, implements corresponding service logic with validations and deletion, and updates API/docs to include success and error documentation. Introduces controller, service, and test coverage for success and error scenarios; no changes to other features. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Controller as UserTagController
participant Service as UserTagService
participant UserRepo as UserRepository
participant TagRepo as TagRepository
participant UTRepo as UserTagRepository
Client->>Controller: DELETE /v1/user-tags?tagId={id}\nAuthorization
Controller->>Service: delete(loginUser, tagId)
Service->>UserRepo: findByUsername(loginUser.username)
alt User not found
Service-->>Controller: throw USER_NOT_FOUND
Controller-->>Client: 4xx with error
else User found
Service->>TagRepo: findById(tagId)
alt Tag not found
Service-->>Controller: throw TAG_NOT_FOUND
Controller-->>Client: 4xx with error
else Tag found
Service->>UTRepo: findByUserAndTag(user, tag)
alt UserTag not found
Service-->>Controller: throw USER_TAG_NOT_FOUND
Controller-->>Client: 4xx with error
else UserTag found
Service->>UTRepo: delete(userTag)
Service-->>Controller: void
Controller-->>Client: 200 ApiResponse.success()
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🧪 Generate unit tests
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: 0
🧹 Nitpick comments (7)
capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java (1)
66-83: Stub the correct service method in delete test.You're stubbing create() in a delete test; use delete() to reflect the behavior under test.
Apply this diff:
@@ @Test void 유저_태그_삭제() { // given - BDDMockito.given(userTagService.create(any(), anyString())).willReturn(new TagResponse(1L, "java")); + BDDMockito.willDoNothing().given(userTagService).delete(any(), anyLong()); // when & then given() .header(HttpHeaders.AUTHORIZATION, JwtUtil.BEARER_PREFIX + ACCESS_TOKEN) .contentType(ContentType.JSON) .queryParam("tagId", 1L) .when().delete("/v1/user-tags") .then().status(HttpStatus.OK) .apply(document("deleteUserTag", requestPreprocessor(), responsePreprocessor(), requestHeaders(headerWithName(HttpHeaders.AUTHORIZATION).description("유효한 Access 토큰")), queryParameters(parameterWithName("tagId").description("태그 ID")), responseFields( fieldWithPath("result").type(JsonFieldType.STRING).description("요청 결과")))); }If you want to assert interaction as well, consider:
BDDMockito.then(userTagService).should().delete(any(), anyLong());capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTagRepository.java (1)
11-12: Consider direct delete to avoid extra SELECT and add a uniqueness guarantee.
- Optional: add
deleteByUserAndTag(...)and rely on affected rows to detect not-found, reducing one query.- Ensure a unique constraint/index on (user_id, tag_id) to prevent duplicates and guarantee deterministic deletes.
Proposed repository addition:
public interface UserTagRepository extends JpaRepository<UserTag, Long> { Optional<UserTag> findByUserAndTag(User user, Tag tag); + + int deleteByUserAndTag(User user, Tag tag); boolean existsByUserAndTag(User user, Tag tag); long countByUser(User user); }capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java (1)
45-50: Return ApiResponse to signal no body on delete.Minor clarity improvement; semantics unchanged.
Apply this diff:
- @DeleteMapping - public ApiResponse<?> delete(@AuthenticationPrincipal LoginUser loginUser, @RequestParam Long tagId) { + @DeleteMapping + public ApiResponse<Void> delete(@AuthenticationPrincipal LoginUser loginUser, @RequestParam Long tagId) { userTagService.delete(loginUser, tagId); return ApiResponse.success(); }capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java (1)
51-61: Reduce queries and make delete atomic with a single deleteBy... repository methodCurrent flow performs 3 selects then delete. Consider a single-statement delete to cut round-trips and avoid loading the entity.
- Assumes a unique constraint on (user_id, tag_id). Please confirm it exists.
Apply within this method:
- UserTag userTag = userTagRepository.findByUserAndTag(user, tag) - .orElseThrow(() -> new CoreException(ErrorType.USER_TAG_NOT_FOUND)); - - userTagRepository.delete(userTag); + long deleted = userTagRepository.deleteByUserAndTag(user, tag); + if (deleted == 0) { + throw new CoreException(ErrorType.USER_TAG_NOT_FOUND); + }Add to repository (outside this hunk) if not present:
long deleteByUserAndTag(User user, Tag tag);capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (3)
141-152: Optionally assert no downstream calls after USER_NOT_FOUNDHelps ensure early exit behavior.
Apply:
verify(userTagRepository, never()).delete(any()); + verify(tagRepository, never()).findById(anyLong()); + verify(userTagRepository, never()).findByUserAndTag(any(), any());
154-169: Optionally verify no lookup of UserTag when TAG_NOT_FOUNDTighter interaction contract for negative path.
Apply:
verify(userTagRepository, never()).delete(any()); + verify(userTagRepository, never()).findByUserAndTag(any(), any());
170-185: Strengthen assertion: check the specific error message for USER_TAG_NOT_FOUNDAligns with other negative-path tests and guards error-type regressions.
Apply:
- assertThatThrownBy(() -> userTagService.delete(new LoginUser(DummyObject.newUser("test")), 1L)) - .isInstanceOf(CoreException.class); + assertThatThrownBy(() -> userTagService.delete(new LoginUser(DummyObject.newUser("test")), 1L)) + .isInstanceOf(CoreException.class) + .hasMessage(ErrorType.USER_TAG_NOT_FOUND.getCode().getMessage());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
capturecat-core/src/docs/asciidoc/error-codes.adoc(1 hunks)capturecat-core/src/docs/asciidoc/user.adoc(1 hunks)capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java(2 hunks)capturecat-core/src/main/java/com/capturecat/core/domain/user/UserTagRepository.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java(3 hunks)capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java(1 hunks)capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java(2 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java (1)
capturecat-core/src/test/java/com/capturecat/core/api/tag/TagErrorCodeControllerTest.java (1)
TagErrorCodeControllerTest(13-20)
🔇 Additional comments (11)
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorCode.java (1)
43-44: LGTM: New user-tag error codes registered correctly.Delimiter fix and additions look consistent with existing enums.
capturecat-core/src/main/java/com/capturecat/core/support/error/ErrorType.java (1)
48-49: LGTM: Mapped user-tag errors to appropriate HTTP statuses.BAD_REQUEST for count overflow and NOT_FOUND for missing user-tag are appropriate.
capturecat-core/src/docs/asciidoc/error-codes.adoc (1)
97-99: Docs entry for deleteUserTag added in the right section.Anchor, heading, and snippet path align with tests.
capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java (1)
33-37: LGTM: Error-code docs for delete path are covered.The descriptors list matches expected failures (USER_NOT_FOUND, TAG_NOT_FOUND, USER_TAG_NOT_FOUND).
capturecat-core/src/docs/asciidoc/user.adoc (1)
52-60: LGTM: Added delete section mirrors create section and links error codes correctly.Consistent snippet usage and cross-links.
capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java (2)
12-12: LGTM: TagRepository import is appropriate for delete flowThe dependency aligns with the new deletion path.
32-32: Constructor-injected TagRepository looks goodFinal field + @requiredargsconstructor wires it correctly.
capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (4)
6-6: LGTM: anyLong import added for new stubsMatches usage with tagRepository.findById(anyLong()).
25-25: LGTM: TagRepository import for delete testsConsistent with service dependency.
42-44: LGTM: Added @mock TagRepositoryMock wiring is correct.
123-139: LGTM: Successful delete path is well assertedVerifies delete was invoked with the expected entity.
leeedohyun
force-pushed
the
feature/#126-delete-user-tag
branch
from
6501d51 to
1986468
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (5)
241-250: Tighten interaction verification in success caseAdd explicit verifications that the correct tag id is looked up and the user-tag association is queried once.
Apply:
// then verify(userTagRepository, times(1)).delete(eq(userTag)); + verify(tagRepository, times(1)).findById(eq(1L)); + verify(userTagRepository, times(1)).findByUserAndTag(eq(user), eq(tag));
253-263: Assert short-circuiting when user is missingAlso verify no downstream repository calls happen.
Apply:
// when & then assertThatThrownBy(() -> userTagService.delete(new LoginUser(DummyObject.newUser("test")), 1L)) .isInstanceOf(CoreException.class) .hasMessage(ErrorType.USER_NOT_FOUND.getCode().getMessage()); verify(userTagRepository, never()).delete(any()); + verify(tagRepository, never()).findById(anyLong()); + verify(userTagRepository, never()).findByUserAndTag(any(), any());
265-279: Verify tag lookup and absence of further queries when tag is missingConfirm we queried the right id and didn’t proceed to userTag lookup.
Apply:
// when & then assertThatThrownBy(() -> userTagService.delete(new LoginUser(DummyObject.newUser("test")), 1L)) .isInstanceOf(CoreException.class) .hasMessage(ErrorType.TAG_NOT_FOUND.getCode().getMessage()); verify(userTagRepository, never()).delete(any()); + verify(tagRepository, times(1)).findById(eq(1L)); + verify(userTagRepository, never()).findByUserAndTag(any(), any());
281-296: Assert the specific error code for missing UserTagStrengthen the assertion to ensure USER_TAG_NOT_FOUND is emitted; optionally verify the lookup occurred exactly once.
Apply:
// when & then assertThatThrownBy(() -> userTagService.delete(new LoginUser(DummyObject.newUser("test")), 1L)) - .isInstanceOf(CoreException.class); + .isInstanceOf(CoreException.class) + .hasMessage(ErrorType.USER_TAG_NOT_FOUND.getCode().getMessage()); verify(userTagRepository, never()).delete(any()); + verify(userTagRepository, times(1)).findByUserAndTag(eq(user), eq(tag));
258-258: Minor: reuse the local ‘user’ for LoginUser constructionUse new LoginUser(user) for consistency/readability instead of instantiating a second dummy user inline.
Also applies to: 274-274, 292-292
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
capturecat-core/src/docs/asciidoc/error-codes.adoc(1 hunks)capturecat-core/src/docs/asciidoc/user.adoc(1 hunks)capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java(2 hunks)capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java(1 hunks)capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- capturecat-core/src/test/java/com/capturecat/core/api/user/UserTagControllerTest.java
- capturecat-core/src/main/java/com/capturecat/core/api/user/UserTagController.java
- capturecat-core/src/docs/asciidoc/error-codes.adoc
- capturecat-core/src/test/java/com/capturecat/core/api/user/UserErrorCodeControllerTest.java
- capturecat-core/src/main/java/com/capturecat/core/service/user/UserTagService.java
- capturecat-core/src/docs/asciidoc/user.adoc
🧰 Additional context used
🧬 Code graph analysis (1)
capturecat-core/src/test/java/com/capturecat/core/service/user/UserTagServiceTest.java (3)
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/domain/user/UserTagFixture.java (1)
UserTagFixture(7-14)
⏰ 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)
233-297: Good coverage for delete pathHappy-path and three failure cases are covered and read cleanly.
📌 관련 이슈 (필수)
📝 작업 내용 (필수)
태그 설정 부분에 사용할 유저 태그 삭제 API를 구현했습니다.
💬 리뷰 참고 사항 (선택)
Summary by CodeRabbit
New Features
Documentation