-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BE] 멤버 액션 삭제 기능 구현 #181
[BE] 멤버 액션 삭제 기능 구현 #181
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,4 +58,16 @@ private Event findEvent(String token) { | |
return eventRepository.findByToken(token) | ||
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.NOT_FOUND_EVENT)); | ||
} | ||
|
||
@Transactional | ||
public void deleteMemberAction(String token, Long actionId) { | ||
Event event = eventRepository.findByToken(token) | ||
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.NOT_FOUND_EVENT)); | ||
Action action = actionRepository.findByIdAndEvent(actionId, event) | ||
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.NOT_FOUND_ACTION)); | ||
MemberAction memberAction = memberActionRepository.findByAction(action) | ||
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.NOT_FOUND_MEMBER_ACTION)); | ||
Comment on lines
+64
to
+69
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a: repository default 메서드하면 깔끔해 질 것 같아요. 다만 검증을 위한 쿼리가 많이 발생할 것이 우려되네요. 추후 개선해 보아요. |
||
|
||
memberActionRepository.deleteAllByMemberAction(memberAction.getMemberName(), memberAction.getSequence()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
package server.haengdong.domain.action; | ||
|
||
import java.util.List; | ||
import java.util.Optional; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
import org.springframework.data.jpa.repository.Modifying; | ||
import org.springframework.data.jpa.repository.Query; | ||
import org.springframework.data.repository.query.Param; | ||
import org.springframework.stereotype.Repository; | ||
|
@@ -12,4 +14,14 @@ public interface MemberActionRepository extends JpaRepository<MemberAction, Long | |
|
||
@Query("select m from MemberAction m join fetch m.action where m.action.event = :event") | ||
List<MemberAction> findAllByEvent(@Param("event") Event event); | ||
|
||
Optional<MemberAction> findByAction(Action action); | ||
|
||
@Modifying | ||
@Query(""" | ||
delete | ||
from MemberAction m | ||
where m.memberName = :memberName and m.action.sequence >= :sequence | ||
""") | ||
void deleteAllByMemberAction(String memberName, Long sequence); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. r: 메소드 이름만 보고 뒷 순서의 memeberAction이 전부 지워짐을 예측하기 어려운 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. deleteAllByMemberNameAndMinSequence 이런식으로 하면 좀 나을까요? |
||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -10,9 +10,10 @@ public enum HaengdongErrorCode { | |||||||
DUPLICATED_MEMBER_ACTION("MA_001", "중복된 인원이 존재합니다."), | ||||||||
INVALID_MEMBER_IN_ACTION("MA_002", "현재 참여하고 있는 인원이 존재합니다."), | ||||||||
INVALID_MEMBER_OUT_ACTION("MA_003", "현재 참여하고 있지 않는 인원이 존재합니다."), | ||||||||
NOT_FOUND_MEMBER_ACTION("MA_400", "존재하지 않는 멤버 액션입니다."), | ||||||||
NOT_FOUND_EVENT("EV_400", "존재하지 않는 행사입니다."), | ||||||||
INTERNAL_SERVER_ERROR("S_001", "서버 내부에서 에러가 발생했습니다."), | ||||||||
; | ||||||||
NOT_FOUND_ACTION("AC_400", "존재하지 않는 액션입니다."), | ||||||||
INTERNAL_SERVER_ERROR("S_001", "서버 내부에서 에러가 발생했습니다."); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
c: 머지 시에 충돌날 부분이지만 남겨 놓아요! |
||||||||
|
||||||||
private final String code; | ||||||||
private final String message; | ||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -3,6 +3,7 @@ | |||||
import java.util.List; | ||||||
import lombok.RequiredArgsConstructor; | ||||||
import org.springframework.http.ResponseEntity; | ||||||
import org.springframework.web.bind.annotation.DeleteMapping; | ||||||
import org.springframework.web.bind.annotation.GetMapping; | ||||||
import org.springframework.web.bind.annotation.PathVariable; | ||||||
import org.springframework.web.bind.annotation.PostMapping; | ||||||
|
@@ -36,4 +37,14 @@ public ResponseEntity<CurrentMembersResponse> getCurrentMembers(@PathVariable("e | |||||
return ResponseEntity.ok() | ||||||
.body(CurrentMembersResponse.of(currentMembers)); | ||||||
} | ||||||
|
||||||
@DeleteMapping("/api/events/{eventId}/actions/{actionId}/members") | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
위처럼 작성하는 걸 컨벤션으로 기억하는데, 다시 논의해 봐야겠어요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추후 api 컨벤션 통일하는 작업에서 반영하겠습니다! |
||||||
public ResponseEntity<Void> deleteMemberAction( | ||||||
@PathVariable("eventId") String token, | ||||||
@PathVariable("actionId") Long actionId | ||||||
) { | ||||||
memberActionService.deleteMemberAction(token, actionId); | ||||||
|
||||||
return ResponseEntity.ok().build(); | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
package server.haengdong.application; | ||
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatCode; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
import static org.assertj.core.api.Assertions.tuple; | ||
import static server.haengdong.domain.action.MemberActionStatus.IN; | ||
import static server.haengdong.domain.action.MemberActionStatus.OUT; | ||
|
||
|
@@ -17,6 +19,7 @@ | |
import server.haengdong.domain.action.ActionRepository; | ||
import server.haengdong.domain.action.MemberAction; | ||
import server.haengdong.domain.action.MemberActionRepository; | ||
import server.haengdong.domain.action.MemberActionStatus; | ||
import server.haengdong.domain.event.Event; | ||
import server.haengdong.domain.event.EventRepository; | ||
import server.haengdong.exception.HaengdongException; | ||
|
@@ -48,7 +51,7 @@ void tearDown() { | |
void saveMemberActionTest() { | ||
Event event = eventRepository.save(new Event("test", "TOKEN")); | ||
Action action = new Action(event, 1L); | ||
MemberAction memberAction = new MemberAction(action, "망쵸", IN, 1L); | ||
MemberAction memberAction = createMemberAction(action, "망쵸", IN, 1L); | ||
memberActionRepository.save(memberAction); | ||
|
||
assertThatCode(() -> memberActionService.saveMemberAction("TOKEN", new MemberActionsSaveAppRequest( | ||
|
@@ -61,11 +64,11 @@ void saveMemberActionTest() { | |
void saveMemberActionTest1() { | ||
Event event = eventRepository.save(new Event("test", "TOKEN")); | ||
Action actionOne = new Action(event, 1L); | ||
MemberAction memberActionOne = new MemberAction(actionOne, "망쵸", IN, 1L); | ||
MemberAction memberActionOne = createMemberAction(actionOne, "망쵸", IN, 1L); | ||
memberActionRepository.save(memberActionOne); | ||
|
||
Action actionTwo = new Action(event, 2L); | ||
MemberAction memberActionTwo = new MemberAction(actionTwo, "망쵸", OUT, 1L); | ||
MemberAction memberActionTwo = createMemberAction(actionTwo, "망쵸", OUT, 1L); | ||
memberActionRepository.save(memberActionTwo); | ||
|
||
assertThatCode(() -> memberActionService.saveMemberAction("TOKEN", new MemberActionsSaveAppRequest( | ||
|
@@ -89,4 +92,49 @@ void getCurrentMembers() { | |
assertThatThrownBy(() -> memberActionService.getCurrentMembers("token")) | ||
.isInstanceOf(HaengdongException.class); | ||
} | ||
|
||
@DisplayName("이벤트에 속한 멤버 액션을 삭제하면 이후에 기록된 해당 참여자의 모든 멤버 액션을 삭제한다.") | ||
@Test | ||
void deleteMemberAction() { | ||
String token = "TOKEN"; | ||
Event event = new Event("행동대장 회식", token); | ||
eventRepository.save(event); | ||
Action action = Action.createFirst(event); | ||
MemberAction memberAction1 = createMemberAction(action, "토다리", IN, 1L); | ||
Action targetAction = action.next(); | ||
MemberAction memberAction2 = createMemberAction(targetAction, "토다리", OUT, 2L); | ||
MemberAction memberAction3 = createMemberAction(action.next(), "쿠키", IN, 3L); | ||
MemberAction memberAction4 = createMemberAction(action.next(), "웨디", IN, 4L); | ||
MemberAction memberAction5 = createMemberAction(action.next(), "토다리", IN, 5L); | ||
MemberAction memberAction6 = createMemberAction(action.next(), "토다리", OUT, 6L); | ||
MemberAction memberAction7 = createMemberAction(action.next(), "쿠키", OUT, 7L); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. r: action의 sequence에 3,4,5,6,7 이 들어가는 것을 의도한 것 같은데 action.next()는 전부 sequence가 2로 고정되어있습니다. 테스트 코드에서는 new Action(event, 3L)를 사용하는 것이 더 명시적일 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 맞네요 제가 놓쳤습니다! |
||
memberActionRepository.saveAll( | ||
List.of(memberAction1, | ||
memberAction2, | ||
memberAction3, | ||
memberAction4, | ||
memberAction5, | ||
memberAction6, | ||
memberAction7) | ||
); | ||
|
||
memberActionService.deleteMemberAction(token, targetAction.getId()); | ||
List<MemberAction> memberActions = memberActionRepository.findAll(); | ||
|
||
assertThat(memberActions).hasSize(4) | ||
.extracting("id", "memberName", "status") | ||
.containsExactly( | ||
tuple(memberAction1.getId(), "토다리", IN), | ||
tuple(memberAction3.getId(), "쿠키", IN), | ||
tuple(memberAction4.getId(), "웨디", IN), | ||
tuple(memberAction7.getId(), "쿠키", OUT) | ||
); | ||
} | ||
|
||
private static MemberAction createMemberAction(Action action, | ||
String memberName, | ||
MemberActionStatus memberActionStatus, | ||
long memberGroupId) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. r: static 은 없어도 될 것 같고 파라미터 개행 컨벤션을 지켜주세요. |
||
return new MemberAction(action, memberName, memberActionStatus, memberGroupId); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a: 이런 방식으로 event와 actionId를 검증하는 것도 깔끔하겠네요! 👍