Skip to content
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

Merged
merged 5 commits into from
Aug 4, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Comment on lines +66 to +67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 이런 방식으로 event와 actionId를 검증하는 것도 깔끔하겠네요! 👍

MemberAction memberAction = memberActionRepository.findByAction(action)
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.NOT_FOUND_MEMBER_ACTION));
Comment on lines +64 to +69
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Up @@ -18,4 +18,6 @@ public interface ActionRepository extends JpaRepository<Action, Long> {
LIMIT 1
""")
Optional<Action> findLastByEvent(@Param("event") Event event);

Optional<Action> findByIdAndEvent(Long id, Event event);
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package server.haengdong.domain.action;

import java.util.List;
import java.util.Optional;

import org.springframework.data.jpa.repository.EntityGraph;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.stereotype.Repository;
Expand All @@ -11,4 +13,6 @@ public interface BillActionRepository extends JpaRepository<BillAction, Long> {

@EntityGraph(attributePaths = {"action"})
List<BillAction> findByAction_Event(Event event);

Optional<BillAction> findByAction(Action action);
}
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;
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r: 메소드 이름만 보고 뒷 순서의 memeberAction이 전부 지워짐을 예측하기 어려운 것 같아요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deleteAllByMemberNameAndMinSequence 이런식으로 하면 좀 나을까요?
해당 로직이 들어나는 메서드명 짓는게 상당히 어렵네요 🤯

}
Original file line number Diff line number Diff line change
Expand Up @@ -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", "서버 내부에서 에러가 발생했습니다.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
INTERNAL_SERVER_ERROR("S_001", "서버 내부에서 에러가 발생했습니다.");
INTERNAL_SERVER_ERROR("S_001", "서버 내부에서 에러가 발생했습니다."),
;

c: 머지 시에 충돌날 부분이지만 남겨 놓아요!


private final String code;
private final String message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -36,4 +37,14 @@ public ResponseEntity<CurrentMembersResponse> getCurrentMembers(@PathVariable("e
return ResponseEntity.ok()
.body(CurrentMembersResponse.of(currentMembers));
}

@DeleteMapping("/api/events/{eventId}/actions/{actionId}/members")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@DeleteMapping("/api/events/{eventId}/actions/{actionId}/members")
@DeleteMapping("/api/events/{eventId}/actions/members/{actionId}")

위처럼 작성하는 걸 컨벤션으로 기억하는데, 다시 논의해 봐야겠어요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Up @@ -7,12 +7,14 @@
import static server.haengdong.domain.action.MemberActionStatus.OUT;

import java.util.List;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import server.haengdong.application.response.MemberBillReportAppResponse;
import server.haengdong.domain.action.Action;
import server.haengdong.domain.action.ActionRepository;
import server.haengdong.domain.action.BillAction;
import server.haengdong.domain.action.BillActionRepository;
import server.haengdong.domain.action.MemberAction;
Expand All @@ -31,12 +33,23 @@ class ActionServiceTest {
@Autowired
private EventRepository eventRepository;

@Autowired
private ActionRepository actionRepository;

@Autowired
private BillActionRepository billActionRepository;

@Autowired
private MemberActionRepository memberActionRepository;

@AfterEach
void tearDown() {
billActionRepository.deleteAllInBatch();
memberActionRepository.deleteAllInBatch();
actionRepository.deleteAllInBatch();
eventRepository.deleteAllInBatch();
}

@DisplayName("참여자별 정산 현황을 조회한다.")
@Test
void getMemberBillReports() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
import static org.assertj.core.api.Assertions.tuple;

import java.util.List;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import server.haengdong.application.request.BillActionAppRequest;
import server.haengdong.domain.action.ActionRepository;
import server.haengdong.domain.action.BillAction;
import server.haengdong.domain.event.Event;
import server.haengdong.domain.action.BillActionRepository;
import server.haengdong.domain.event.Event;
import server.haengdong.domain.event.EventRepository;
import server.haengdong.exception.HaengdongException;

Expand All @@ -22,12 +24,22 @@ class BillActionServiceTest {
@Autowired
private BillActionService billActionService;

@Autowired
private ActionRepository actionRepository;

@Autowired
private EventRepository eventRepository;

@Autowired
private BillActionRepository billActionRepository;

@AfterEach
void tearDown() {
billActionRepository.deleteAllInBatch();
actionRepository.deleteAllInBatch();
eventRepository.deleteAllInBatch();
}

@DisplayName("지출 내역을 생성한다.")
@Test
void saveAllBillAction() {
Expand Down
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;

Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r: action의 sequence에 3,4,5,6,7 이 들어가는 것을 의도한 것 같은데 action.next()는 전부 sequence가 2로 고정되어있습니다.
sequence 3을 원하시면 action.next().next()를 해야합니다.

테스트 코드에서는 new Action(event, 3L)를 사용하는 것이 더 명시적일 것 같아요.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞네요 제가 놓쳤습니다!
action.next()로 전부 받으려면 action = action.next(); 이런 식으로 사용했어야 했는데 잘못 생각했네요.
짚어주셔서 감사해요 백호! 👍

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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r: static 은 없어도 될 것 같고 파라미터 개행 컨벤션을 지켜주세요.

return new MemberAction(action, memberName, memberActionStatus, memberGroupId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.BDDMockito.given;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import com.fasterxml.jackson.databind.ObjectMapper;
Expand All @@ -17,7 +19,6 @@
import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
import server.haengdong.application.MemberActionService;
import server.haengdong.application.response.CurrentMemberAppResponse;
import server.haengdong.presentation.request.MemberActionsSaveRequest;
Expand Down Expand Up @@ -61,7 +62,18 @@ void getCurrentMembers() throws Exception {
.accept(MediaType.APPLICATION_JSON))
.andDo(print())
.andExpect(status().isOk())
.andExpect(MockMvcResultMatchers.jsonPath("$.members[0].name").value(equalTo("소하")))
.andExpect(MockMvcResultMatchers.jsonPath("$.members[1].name").value(equalTo("토다리")));
.andExpect(jsonPath("$.members[0].name").value(equalTo("소하")))
.andExpect(jsonPath("$.members[1].name").value(equalTo("토다리")));
}

@DisplayName("이벤트에 속한 멤버 액션을 삭제하면 이후에 기록된 해당 참여자의 모든 멤버 액션을 삭제한다.")
@Test
void deleteMemberAction() throws Exception {
String token = "TOKEN";
Long actionId = 2L;

mockMvc.perform(delete("/api/events/{token}/actions/{actionId}/members", token, actionId))
.andDo(print())
.andExpect(status().isOk());
}
}
Loading