-
Notifications
You must be signed in to change notification settings - Fork 2
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
[FEAT] 한달 예산 수정 API 구현 #32
Conversation
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.
optional 부분 문제없습니다!
근데 저희 회원가입되면 한달 예산 수정되고 거기에 해당하는 daybudget들 생성하는 로직 아닌가요? 이부분은 어떻게 구현하시는지 궁금합니다 !
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.
- UpdateMonthDTO의 MonthBudget 필드에도 @positive와 @NotNull을 붙이면 좋을 것 같습니다.
- 한달 예산 수정 API에서 return 값에 null 대신 Converter에서 ResponseDTO로 변환한 값을 넣어줘야 합니다. (ApiResponse<> 안에 들어가는 것도 그에 맞춰 수정해야 합니다!)
- Service에서 MonthBudget이 null이거나 0 미만일 때 에러를 던지는 부분은 Controller의 @Valid에서 먼저 잡아주기 때문에 없애도 괜찮습니다.
- 한달예산 수정 시 해당 달에 마감되지 않은 하루예산에도 재분배가 일어나야 합니다! 제아님의 고르게 넘기기 부분을 참고하면 좋을 것 같습니다.
- Service에서 한달 예산 수정 후 monthBudgetRepository.save(monthBudget)을 return 해야 합니다.
- Controller에서 memberId를 @RequestParam으로 받아오는 대신, @AuthenticationPrincipal와 MemberDetail을 쓰면 입력한 토큰 값에서 회원의 정보를 뽑아올 수 있습니다! 지금 swagger와 회원가입, 로그인을 제외하곤 토큰을 입력하지 않으면 인증이 필요하다고 뜨도록 구현해놔서 이렇게 수정하는 게 좋을 것 같습니다.
현재 회원가입 API에서 회원가입할 때 한달 예산이 생성되게까지는 해놨는데, 한번에 진행하는게 나을까요? 의견 부탁드립니다! |
그러면 회원가입 완료되자마자 프론트에서 해당 API를 요청하게 되는걸까요? 아니면 저번에 앤드님이 하루 에산 조회하는데 null값이 오면 루베님 메서드 호출한다고했는데 API까진 아니더라도 해당 메서드 구현해놓으시면 최초 설정도 이 방법으로 해도 괜찮을 것 같습니다! |
한달예산 수정만 API로 빼놓고, 하루예산 재분배는 메서드로 만드는 게 나을 것 같습니다!
|
36e62a7
to
80eabb7
Compare
모두 수정해서 커밋했습니다! 4번은 API로 빼려고 했는데, 메서드로 추가하겠습니다. |
이렇게하면 깔끔할 것 같네요! 감사합니다😊 |
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.
-
DayBudgetConverter에 toDayBudget 메서드는 별 문제 없습니다.
-
하루 예산을 생성할 때 날짜도 같이 제공되어 저장되는거라면 prePersist() 메서드는 삭제해도 될 것 같습니다.
-
이전 날짜는 INACTIVE, 이후 날짜는 ACTIVE로 하는 로직이 맞습니다.
-
monthBudgetController에서 memberId를 request param으로 받아온 것 같은데 저희 시큐리티로 사용하기로 해서 바꾸셔야 할 것 같습니다.
수고하셨습니다😊😊
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.
MemberService의 joinMember 메서드에 하루예산 분배가 되도록 메서드(distributeDayBudgets)와 한달예산 저장(monthBudgetRepository), 하루예산 저장(dayBudgetRepository)하는 걸 추가해주시면 괜찮을 것 같습니다!
재분배 코드 잘 작성하셨습니다 ! 근데 지금 코드에 테스트 코드가 들어가 있는 것 같아요 int nowDay = 5;//now.getDayOfMonth();
//남은 일자
int remainingDays = 26;//dayInMonth - nowDay + 1; 이 부분 수정 부탁드립니다 ! |
@hcg0127 @jimmy0524 @ssongmina 완료했습니다 확인 부탁드려요! |
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.
수고하셨습니다 🙌
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.
수고하셨습니다! 😊
수고하셨습니다😊😊😊 |
💡 관련 이슈
#26
📢 작업 내용
🗨️ 리뷰 요구사항(선택)
MonthBudgetRepository에서 findByMemberIdAndYearAndMonth의 반환형을 Optional으로 바꾸어서,BudgetRedistributionService 와 DayBudgetService에 관련 에러처리를 추가했는데 문제 없는지 확인 부탁드립니다!
DayBudgetConverter에 toDayBudget 추가했는데 이상 없는지 확인 부탁드립니다DayBudget 엔티티에서 prePersist를 통해 현재 날짜로 설정하니 converter에서 day에 값을 주어도 database에는 수정되어 저장되지 않아서 삭제했습니다. 다른 방법이 있으면 조언 부탁드립니다!DayBudget을 생성할 때, 이전 날짜의 status는 INACTIVE로 변경하고, 이후 날짜는 ACTIVE로 저장하였는데 맞는 로직인지 확인 부탁드립니다✅ 체크리스트