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

[FEAT] 한달 예산 수정 API 구현 #32

Merged
merged 10 commits into from
Aug 1, 2024
Merged

Conversation

ohu-star
Copy link
Contributor

@ohu-star ohu-star commented Jul 30, 2024

💡 관련 이슈

#26

📢 작업 내용

  • MonthBudgetService, MonthBudgetController에 기능 추가
  • Member
  • MonthBudget 관련 ErrorStatus 추가
  • Member 엔티티에서 monthBudget 컬럼 삭제
  • DayBudgetConverter 기능 추가
  • DayBudget 엔티티에서 prePersist 메소드 삭제
  • MemberApiService에 MonthBudget, DayBudget 생성 기능 추가

🗨️ 리뷰 요구사항(선택)

  • MonthBudgetRepository에서 findByMemberIdAndYearAndMonth의 반환형을 Optional으로 바꾸어서,
    BudgetRedistributionService 와 DayBudgetService에 관련 에러처리를 추가했는데 문제 없는지 확인 부탁드립니다!
  • DayBudgetConverter에 toDayBudget 추가했는데 이상 없는지 확인 부탁드립니다
  • DayBudget 엔티티에서 prePersist를 통해 현재 날짜로 설정하니 converter에서 day에 값을 주어도 database에는 수정되어 저장되지 않아서 삭제했습니다. 다른 방법이 있으면 조언 부탁드립니다!
  • DayBudget을 생성할 때, 이전 날짜의 status는 INACTIVE로 변경하고, 이후 날짜는 ACTIVE로 저장하였는데 맞는 로직인지 확인 부탁드립니다
  • DayBudgetRepository에서 findByMonthBudgetAndDay의 반환형을 Optional 으로 바꾸어서 해당 메소드 사용하는 BudgetRedistributionService와 DayBudgetService 수정했습니다

✅ 체크리스트

  • 코드가 정상적으로 컴파일되나요?
  • 이슈 내용을 전부 구현했나요?
  • 작업 기간 내에 개발을 완료했나요?
  • 리뷰어를 선택했나요?
  • 라벨을 지정했나요?

ohu-star added 2 commits July 31, 2024 01:48
B
[FEAT] 한달 예산 수정 API 구현
@ohu-star ohu-star added ✨ Feat 새 기능 추가 🔨 Fix 기능 수정 labels Jul 30, 2024
@ohu-star ohu-star self-assigned this Jul 30, 2024
Copy link
Member

@jimmy0524 jimmy0524 left a comment

Choose a reason for hiding this comment

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

optional 부분 문제없습니다!
근데 저희 회원가입되면 한달 예산 수정되고 거기에 해당하는 daybudget들 생성하는 로직 아닌가요? 이부분은 어떻게 구현하시는지 궁금합니다 !

Copy link
Member

@hcg0127 hcg0127 left a comment

Choose a reason for hiding this comment

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

  1. UpdateMonthDTO의 MonthBudget 필드에도 @positive@NotNull을 붙이면 좋을 것 같습니다.
  2. 한달 예산 수정 API에서 return 값에 null 대신 Converter에서 ResponseDTO로 변환한 값을 넣어줘야 합니다. (ApiResponse<> 안에 들어가는 것도 그에 맞춰 수정해야 합니다!)
  3. Service에서 MonthBudget이 null이거나 0 미만일 때 에러를 던지는 부분은 Controller의 @Valid에서 먼저 잡아주기 때문에 없애도 괜찮습니다.
  4. 한달예산 수정 시 해당 달에 마감되지 않은 하루예산에도 재분배가 일어나야 합니다! 제아님의 고르게 넘기기 부분을 참고하면 좋을 것 같습니다.
  5. Service에서 한달 예산 수정 후 monthBudgetRepository.save(monthBudget)을 return 해야 합니다.
  6. Controller에서 memberId를 @RequestParam으로 받아오는 대신, @AuthenticationPrincipal와 MemberDetail을 쓰면 입력한 토큰 값에서 회원의 정보를 뽑아올 수 있습니다! 지금 swagger와 회원가입, 로그인을 제외하곤 토큰을 입력하지 않으면 인증이 필요하다고 뜨도록 구현해놔서 이렇게 수정하는 게 좋을 것 같습니다.

@ohu-star
Copy link
Contributor Author

optional 부분 문제없습니다! 근데 저희 회원가입되면 한달 예산 수정되고 거기에 해당하는 daybudget들 생성하는 로직 아닌가요? 이부분은 어떻게 구현하시는지 궁금합니다 !

현재 회원가입 API에서 회원가입할 때 한달 예산이 생성되게까지는 해놨는데,
회원가입, 한달 예산 생성, 하루 예산 생성을 '회원가입 API' 하나에서 진행하면 관리하기에 복잡할 것 같아서 한달 예산 분배(하루치 분배) API를 하나 추가해서 진행하고자 했습니다.

한번에 진행하는게 나을까요? 의견 부탁드립니다!

@jimmy0524
Copy link
Member

jimmy0524 commented Jul 31, 2024

optional 부분 문제없습니다! 근데 저희 회원가입되면 한달 예산 수정되고 거기에 해당하는 daybudget들 생성하는 로직 아닌가요? 이부분은 어떻게 구현하시는지 궁금합니다 !

현재 회원가입 API에서 회원가입할 때 한달 예산이 생성되게까지는 해놨는데, 회원가입, 한달 예산 생성, 하루 예산 생성을 '회원가입 API' 하나에서 진행하면 관리하기에 복잡할 것 같아서 한달 예산 분배(하루치 분배) API를 하나 추가해서 진행하고자 했습니다.

한번에 진행하는게 나을까요? 의견 부탁드립니다!

그러면 회원가입 완료되자마자 프론트에서 해당 API를 요청하게 되는걸까요? 아니면 저번에 앤드님이 하루 에산 조회하는데 null값이 오면 루베님 메서드 호출한다고했는데 API까진 아니더라도 해당 메서드 구현해놓으시면 최초 설정도 이 방법으로 해도 괜찮을 것 같습니다!

@hcg0127
Copy link
Member

hcg0127 commented Jul 31, 2024

한달예산 수정만 API로 빼놓고, 하루예산 재분배는 메서드로 만드는 게 나을 것 같습니다!

  • 회원가입 API -> 한달예산 생성 & 하루예산 재분배 메서드 호출
  • 한달예산 수정 API -> 한달예산 수정 & 하루예산 재분배 메서드 호출

@ohu-star ohu-star force-pushed the feat/5-monthbudget#1.2 branch from 36e62a7 to 80eabb7 Compare July 31, 2024 02:13
@ohu-star
Copy link
Contributor Author

ohu-star commented Jul 31, 2024

  1. UpdateMonthDTO의 MonthBudget 필드에도 @positive@NotNull을 붙이면 좋을 것 같습니다.
  2. 한달 예산 수정 API에서 return 값에 null 대신 Converter에서 ResponseDTO로 변환한 값을 넣어줘야 합니다. (ApiResponse<> 안에 들어가는 것도 그에 맞춰 수정해야 합니다!)
  3. Service에서 MonthBudget이 null이거나 0 미만일 때 에러를 던지는 부분은 Controller의 @Valid에서 먼저 잡아주기 때문에 없애도 괜찮습니다.
  4. 한달예산 수정 시 해당 달에 마감되지 않은 하루예산에도 재분배가 일어나야 합니다! 제아님의 고르게 넘기기 부분을 참고하면 좋을 것 같습니다.
  5. Service에서 한달 예산 수정 후 monthBudgetRepository.save(monthBudget)을 return 해야 합니다.
  6. Controller에서 memberId를 @RequestParam으로 받아오는 대신, @AuthenticationPrincipal와 MemberDetail을 쓰면 입력한 토큰 값에서 회원의 정보를 뽑아올 수 있습니다! 지금 swagger와 회원가입, 로그인을 제외하곤 토큰을 입력하지 않으면 인증이 필요하다고 뜨도록 구현해놔서 이렇게 수정하는 게 좋을 것 같습니다.

모두 수정해서 커밋했습니다! 4번은 API로 빼려고 했는데, 메서드로 추가하겠습니다.
꼼꼼한 수정요청 감사합니다😊

@ohu-star
Copy link
Contributor Author

한달예산 수정만 API로 빼놓고, 하루예산 재분배는 메서드로 만드는 게 나을 것 같습니다!

  • 회원가입 API -> 한달예산 생성 & 하루예산 재분배 메서드 호출
  • 한달예산 수정 API -> 한달예산 수정 & 하루예산 재분배 메서드 호출

이렇게하면 깔끔할 것 같네요! 감사합니다😊

@ohu-star ohu-star requested a review from hcg0127 July 31, 2024 02:48
ssongmina

This comment was marked as duplicate.

Copy link
Contributor

@ssongmina ssongmina left a 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으로 받아온 것 같은데 저희 시큐리티로 사용하기로 해서 바꾸셔야 할 것 같습니다.

수고하셨습니다😊😊

Copy link
Member

@hcg0127 hcg0127 left a 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)하는 걸 추가해주시면 괜찮을 것 같습니다!

@jimmy0524
Copy link
Member

재분배 코드 잘 작성하셨습니다 ! 근데 지금 코드에 테스트 코드가 들어가 있는 것 같아요

int nowDay = 5;//now.getDayOfMonth();

    //남은 일자
       int remainingDays = 26;//dayInMonth - nowDay + 1;

이 부분 수정 부탁드립니다 !

@ohu-star
Copy link
Contributor Author

ohu-star commented Aug 1, 2024

@hcg0127 @jimmy0524 @ssongmina 완료했습니다 확인 부탁드려요!

Copy link
Member

@jimmy0524 jimmy0524 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 🙌

Copy link
Member

@hcg0127 hcg0127 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 😊

@ssongmina
Copy link
Contributor

수고하셨습니다😊😊😊

@ohu-star ohu-star merged commit 87bb743 into develop Aug 1, 2024
@ohu-star ohu-star mentioned this pull request Aug 1, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 Fix 기능 수정 ✨ Feat 새 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants