Conversation
[#169][FEATURE] 그룹장 권한 양도
Summary of ChangesHello @unikal1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 그룹 내에서 그룹장 권한을 다른 멤버에게 양도할 수 있는 핵심 기능을 도입합니다. 이 변경은 그룹 관리의 유연성을 높이고, 권한 변경 시 발생할 수 있는 동시성 문제를 비관적 락을 통해 안전하게 처리합니다. 새로운 API와 서비스 계층이 추가되어 기능의 구조적 분리를 명확히 하며, 기존 빌드 문제를 해결하고 코드 스타일을 통일하여 전반적인 코드 품질을 향상시켰습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
안녕하세요. 그룹장 권한 양도 기능 추가에 대한 PR 리뷰입니다. 전반적으로 기능 구현이 잘 이루어졌습니다. 특히 동시성 문제를 해결하기 위해 비관적 락을 사용하신 점이 인상 깊습니다. 몇 가지 개선점을 제안 드립니다.
GroupMemberRepository에서 native query 대신 JPA의@Lock어노테이션을 사용하여 비관적 락을 구현하는 것을 제안합니다. 이를 통해 코드의 가독성과 데이터베이스 독립성을 높일 수 있습니다.GroupMemberWriter의 예외 메시지에서 작은 오타를 발견하여 수정을 제안했습니다.
자세한 내용은 각 파일의 주석을 확인해주세요. 수고하셨습니다.
| @Query( | ||
| value = | ||
| """ | ||
| SELECT * | ||
| FROM group_member | ||
| WHERE member_id = :userId AND group_id = :groupId | ||
| FOR UPDATE | ||
| """, | ||
| nativeQuery = true) | ||
| Optional<GroupMember> findByMemberIdAndGroupIdForUpdate(Long userId, Long groupId); |
There was a problem hiding this comment.
JPA의 비관적 락(Pessimistic Lock) 기능을 활용하면 더 깔끔하고 데이터베이스에 독립적인 코드를 작성할 수 있습니다. nativeQuery 대신 @Lock 어노테이션과 JPQL을 사용하는 것을 제안합니다. 이렇게 하면 코드가 더 선언적이 되고 JPA가 잠금 처리를 관리하게 되어 유지보수성이 향상됩니다. 이 방법을 적용하려면 org.springframework.data.jpa.repository.Lock와 jakarta.persistence.LockModeType를 import해야 합니다.
@Lock(LockModeType.PESSIMISTIC_WRITE)
@Query("SELECT gm FROM GroupMember gm WHERE gm.member.id = :userId AND gm.group.id = :groupId")
Optional<GroupMember> findByMemberIdAndGroupIdForUpdate(@Param("userId") Long userId, @Param("groupId") Long groupId);| .findByMemberIdAndGroupId(nextLeaderId, groupId) | ||
| .orElseThrow(() -> { | ||
| String message = String.format( | ||
| "[GroupMemberWriter#promoteReader] member %d not found in group %d", nextLeaderId, groupId); |
There was a problem hiding this comment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21d461aff3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| include::{snippets}/group-member-controller-rest-docs-test/promote-leader_success/http-request.adoc[] | ||
| include::{snippets}/group-member-controller-rest-docs-test/promote-leader_success/path-parameters.adoc[] | ||
|
|
||
| *Response* + | ||
| include::{snippets}/group-member-controller-rest-docs-test/promote-leader_success/http-response.adoc[] |
There was a problem hiding this comment.
Fix RestDocs snippet path mismatch
The new API doc includes snippet paths like promote-leader_success, but RestDocs is configured to use {class-name}/{method-name} (see RestDocsConfig), and the test method is named promoteLeader_success. This means the generated snippet directory will be .../promoteLeader_success, so these includes will fail when Asciidoctor builds the docs (missing snippet files). This breaks doc generation for the group API unless the include paths are updated to match the method name (or the method is renamed).
Useful? React with 👍 / 👎.
✨ 구현 기능 명세
기존 fork 된 브랜치를 현재 브랜치로 이전 및 이전 빌드 실패와 일부 PR 리뷰를 적용하였습니다.
✅ PR Point