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] BillActionDetail 초기화시 totalPrice와 정합성 안맞는 버그 수정 #460

Merged
merged 6 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from all 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 @@ -9,7 +9,6 @@
import server.haengdong.application.response.BillActionDetailsAppResponse;
import server.haengdong.domain.action.BillAction;
import server.haengdong.domain.action.BillActionDetail;
import server.haengdong.domain.action.BillActionDetailRepository;
import server.haengdong.domain.action.BillActionRepository;
import server.haengdong.domain.event.Event;
import server.haengdong.exception.HaengdongErrorCode;
Expand All @@ -20,16 +19,14 @@
@Service
public class BillActionDetailService {

private final BillActionDetailRepository billActionDetailRepository;
private final BillActionRepository billActionRepository;

public BillActionDetailsAppResponse findBillActionDetails(String token, Long actionId) {
BillAction billAction = billActionRepository.findByAction_Id(actionId)
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.BILL_ACTION_NOT_FOUND));
validateToken(token, billAction);

List<BillActionDetail> billActionDetails = billActionDetailRepository.findAllByBillAction(billAction);

List<BillActionDetail> billActionDetails = billAction.getBillActionDetails();
return BillActionDetailsAppResponse.of(billActionDetails);
}

Expand All @@ -43,7 +40,7 @@ public void updateBillActionDetails(String token, Long actionId, BillActionDetai
validateToken(token, billAction);
validateTotalPrice(billActionDetailUpdateAppRequests, billAction);

List<BillActionDetail> billActionDetails = billActionDetailRepository.findAllByBillAction(billAction);
List<BillActionDetail> billActionDetails = billAction.getBillActionDetails();

for (BillActionDetailUpdateAppRequest updateRequest : billActionDetailUpdateAppRequests) {
BillActionDetail detailToUpdate = billActionDetails.stream()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
import server.haengdong.domain.action.Action;
import server.haengdong.domain.action.ActionRepository;
import server.haengdong.domain.action.BillAction;
import server.haengdong.domain.action.BillActionDetail;
import server.haengdong.domain.action.BillActionDetailRepository;
import server.haengdong.domain.action.BillActionRepository;
import server.haengdong.domain.action.CurrentMembers;
import server.haengdong.domain.action.MemberAction;
Expand All @@ -26,7 +24,6 @@
public class BillActionService {

private final BillActionRepository billActionRepository;
private final BillActionDetailRepository billActionDetailRepository;
private final MemberActionRepository memberActionRepository;
private final ActionRepository actionRepository;
private final EventRepository eventRepository;
Expand All @@ -39,12 +36,9 @@ public void saveAllBillAction(String eventToken, List<BillActionAppRequest> requ
CurrentMembers currentMembers = CurrentMembers.of(findMemberActions);

for (BillActionAppRequest request : requests) {
BillAction billAction = request.toBillAction(action);
BillAction billAction = request.toBillAction(action, currentMembers);
billActionRepository.save(billAction);
action = action.next();
if (currentMembers.isNotEmpty()) {
saveBillActionDetails(billAction, currentMembers);
}
}
}

Expand All @@ -54,58 +48,33 @@ private Action createStartAction(Event event) {
.orElse(Action.createFirst(event));
}

private void saveBillActionDetails(BillAction billAction, CurrentMembers currentMembers) {
long pricePerMember = billAction.getPrice() / currentMembers.size();
currentMembers.getMembers().stream()
.map(memberName -> new BillActionDetail(billAction, memberName, pricePerMember, false))
.forEach(billActionDetailRepository::save);
}

@Transactional
public void updateBillAction(String token, Long actionId, BillActionUpdateAppRequest request) {
BillAction billAction = billActionRepository.findByAction_Id(actionId)
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.BILL_ACTION_NOT_FOUND));

validateToken(token, billAction);

resetBillActionDetail(billAction, request.price());

billAction.update(request.title(), request.price());
}

private void resetBillActionDetail(BillAction billAction, Long updatePrice) {
if (!billAction.getPrice().equals(updatePrice)) {
List<BillActionDetail> billActionDetails = billActionDetailRepository.findAllByBillAction(billAction);
int memberCount = billActionDetails.size();
if (memberCount != 0) {
Long eachPrice = updatePrice / memberCount;
billActionDetails.forEach(billActionDetail -> {
billActionDetail.updatePrice(eachPrice);
billActionDetail.updateIsFixed(false);
}
);
}
}
}

private void validateToken(String token, BillAction billAction) {
Event event = billAction.getEvent();
if (event.isTokenMismatch(token)) {
throw new HaengdongException(HaengdongErrorCode.BILL_ACTION_NOT_FOUND);
}
}

private Event getEvent(String eventToken) {
return eventRepository.findByToken(eventToken)
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.EVENT_NOT_FOUND));
}

@Transactional
public void deleteBillAction(String token, Long actionId) {
Event event = eventRepository.findByToken(token)
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.EVENT_NOT_FOUND));

billActionDetailRepository.deleteByBillAction_Action_EventAndBillAction_ActionId(event, actionId);
billActionRepository.deleteByAction_EventAndActionId(event, actionId);
}

private Event getEvent(String eventToken) {
return eventRepository.findByToken(eventToken)
.orElseThrow(() -> new HaengdongException(HaengdongErrorCode.EVENT_NOT_FOUND));
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package server.haengdong.application;

import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
Expand All @@ -11,8 +9,6 @@
import server.haengdong.domain.action.Action;
import server.haengdong.domain.action.ActionRepository;
import server.haengdong.domain.action.BillAction;
import server.haengdong.domain.action.BillActionDetail;
import server.haengdong.domain.action.BillActionDetailRepository;
import server.haengdong.domain.action.BillActionRepository;
import server.haengdong.domain.action.CurrentMembers;
import server.haengdong.domain.action.MemberAction;
Expand All @@ -31,7 +27,6 @@ public class MemberActionService {
private final MemberActionRepository memberActionRepository;
private final EventRepository eventRepository;
private final ActionRepository actionRepository;
private final BillActionDetailRepository billActionDetailRepository;
private final BillActionRepository billActionRepository;

@Transactional
Expand Down Expand Up @@ -100,37 +95,6 @@ private void resetBillAction(Event event, BillAction billAction) {
billAction.getSequence());
CurrentMembers currentMembers = CurrentMembers.of(memberActions);

billActionDetailRepository.deleteAllByBillAction(billAction);

if (currentMembers.isNotEmpty()) {
Long price = billAction.getPrice();
int currentMemberCount = currentMembers.size();
long eachPrice = price / currentMemberCount;
long remainder = price % currentMemberCount;
List<BillActionDetail> billActionDetails = getBillActionDetails(
billAction,
currentMembers,
eachPrice,
remainder
);
billActionDetailRepository.saveAll(billActionDetails);
}
}

private List<BillActionDetail> getBillActionDetails(
BillAction billAction,
CurrentMembers currentMembers,
long eachPrice,
long remainder
) {
List<String> members = currentMembers.getMembers().stream().toList();
List<BillActionDetail> billActionDetails = IntStream.range(0, members.size() - 1)
.mapToObj(index -> new BillActionDetail(billAction, members.get(index), eachPrice, false))
.collect(Collectors.toList());
BillActionDetail lastBillActionDetail = new BillActionDetail(billAction, members.get(members.size() - 1),
eachPrice + remainder, false);
billActionDetails.add(lastBillActionDetail);

return billActionDetails;
billAction.resetBillActionDetails(currentMembers);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@

import server.haengdong.domain.action.Action;
import server.haengdong.domain.action.BillAction;
import server.haengdong.domain.action.CurrentMembers;

public record BillActionAppRequest(
String title,
Long price
) {

public BillAction toBillAction(Action action) {
return new BillAction(action, title, price);
public BillAction toBillAction(Action action, CurrentMembers currentMembers) {
return BillAction.create(action, title, price, currentMembers);
}
}
84 changes: 60 additions & 24 deletions server/src/main/java/server/haengdong/domain/action/BillAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
import jakarta.persistence.OneToMany;
import jakarta.persistence.OneToOne;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand Down Expand Up @@ -45,13 +48,8 @@ public class BillAction implements Comparable<BillAction> {
private List<BillActionDetail> billActionDetails = new ArrayList<>();

public BillAction(Action action, String title, Long price) {
Copy link
Contributor

Choose a reason for hiding this comment

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

c: private으로 닫고 싶다

this(null, action, title, price);
}

private BillAction(Long id, Action action, String title, Long price) {
validateTitle(title);
validatePrice(price);
this.id = id;
this.action = action;
this.title = title.trim();
this.price = price;
Expand All @@ -70,23 +68,69 @@ private void validatePrice(Long price) {
}
}

public static BillAction create(Action action, String title, Long price, CurrentMembers currentMembers) {
BillAction billAction = new BillAction(action, title, price);
billAction.resetBillActionDetails(currentMembers);
return billAction;
}

public void resetBillActionDetails(CurrentMembers currentMembers) {
this.billActionDetails.clear();
Iterator<Long> priceIterator = distributePrice(currentMembers.size()).iterator();

for (String member : currentMembers.getMembers()) {
BillActionDetail billActionDetail = new BillActionDetail(this, member, priceIterator.next(), false);
this.billActionDetails.add(billActionDetail);
}
}

private void resetBillActionDetails() {
Iterator<Long> priceIterator = distributePrice(billActionDetails.size()).iterator();

billActionDetails.forEach(billActionDetail -> {
billActionDetail.updatePrice(priceIterator.next());
billActionDetail.updateIsFixed(false);
});
}

private List<Long> distributePrice(int memberCount) {
Copy link
Contributor

Choose a reason for hiding this comment

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

A: 지금은 나머지 값을 한 명에게 몰아주는데, 적절하게 분배하는 방향도 좋을 것 같아요. 이 작업은 프론트랑도 함께 논의해야 겠어요.

if (memberCount == 0) {
return new ArrayList<>();
}
long eachPrice = price / memberCount;
long remainder = price % memberCount;

List<Long> results = Stream.generate(() -> eachPrice)
.limit(memberCount - 1)
.collect(Collectors.toList());
results.add(eachPrice + remainder);
Comment on lines +103 to +106
Copy link
Contributor

@3Juhwan 3Juhwan Aug 21, 2024

Choose a reason for hiding this comment

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

A: 가만보니, add 순서도 중요하겠군요. 현재는 프론트랑 백 모두 마지막 사람에게 나머지를 몰아주는 방식이에요. 양쪽 모두 정책이 동일하게 유지되어야 하는 비용이 있네요.

return results;
}

public void update(String title, Long price) {
validateTitle(title);
validatePrice(price);
this.title = title;
this.title = title.trim();
this.price = price;
resetBillActionDetails();
}

public boolean isSamePrice(Long price) {
return this.price.equals(price);
public void addDetails(List<BillActionDetail> billActionDetails) {
billActionDetails.forEach(this::addDetail);
}

public Long getSequence() {
return action.getSequence();
private void addDetail(BillActionDetail billActionDetail) {
this.billActionDetails.add(billActionDetail);
billActionDetail.setBillAction(this);
}

public Event getEvent() {
return action.getEvent();
public boolean isFixed() {
return billActionDetails.stream()
.anyMatch(BillActionDetail::isFixed);
}

public boolean isSamePrice(Long price) {
return this.price.equals(price);
}

public Long findPriceByMemberName(String memberName) {
Expand All @@ -97,20 +141,12 @@ public Long findPriceByMemberName(String memberName) {
.orElse(DEFAULT_PRICE);
}

public boolean isFixed() {
return billActionDetails.stream()
.map(BillActionDetail::getPrice)
.distinct()
.count() != 1L;
}

public void addDetails(List<BillActionDetail> billActionDetails) {
billActionDetails.forEach(this::addDetail);
public Long getSequence() {
return action.getSequence();
}

private void addDetail(BillActionDetail billActionDetail) {
this.billActionDetails.add(billActionDetail);
billActionDetail.setBillAction(this);
Comment on lines -111 to -113
Copy link
Contributor

Choose a reason for hiding this comment

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

c: 테스트에서만 사용하는 메소드 제거 대상

public Event getEvent() {
return action.getEvent();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query;
import org.springframework.stereotype.Repository;
import server.haengdong.domain.event.Event;

@Repository
public interface BillActionDetailRepository extends JpaRepository<BillActionDetail, Long> {
Copy link
Contributor

Choose a reason for hiding this comment

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

c: 테스트에서만 사용하는 클래스 제거 대상

Expand All @@ -15,8 +14,4 @@ public interface BillActionDetailRepository extends JpaRepository<BillActionDeta
where bd.billAction = :billAction
""")
List<BillActionDetail> findAllByBillAction(BillAction billAction);
Copy link
Contributor

Choose a reason for hiding this comment

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

c: 테스트에서만 사용하는 메소드 제거 대상

Copy link
Contributor

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋ 제거 대상 왤케 많아


void deleteAllByBillAction(BillAction billAction);

void deleteByBillAction_Action_EventAndBillAction_ActionId(Event event, Long actionId);
Copy link
Contributor

Choose a reason for hiding this comment

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

A: 악마 같은 메서드가 지워져서 다행입니다 😀

}
Loading