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.

Sign up for GitHub

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

Jump to bottom

[Feature] 프로필 수정 API #134

Merged
merged 11 commits into from
Nov 29, 2022
Merged

[Feature] 프로필 수정 API #134

merged 11 commits into from
Nov 29, 2022

Conversation

pythonstrup
Copy link
Member

체크 리스트

  • 적절한 제목으로 수정했나요?
  • 관련된 이슈와 연결 시켰나요?
  • Target Branch를 올바르게 설정했나요?
  • Label을 알맞게 설정했나요?

작업 내역

  • 프로필 수정 API

고민

꼭 Request DTO를 풀어헤쳐서 Service에 넘겨야하는가?

  • 어차피 바뀌는 내용이 없는데 DTO를 그냥 넘기면 안 될까..?
  • 팀원과의 합의
    • 물론 DTO를 풀어헤쳐서 Service에 넘겨주면 Request에 대한 의존성이 줄어들기 때문에 얻을 수 있는 이점들이 있다.
      • side effect를 줄일 수 있다.
      • 같은 경로에서 요청이 바뀌었을 때의 유연성, 확장성을 챙길 수 있다.
    • 하지만 위의 이점을 얻을 정도로 중요하지 않거나, 복잡하지 않은 로직에는 DTO를 그냥 전달하는 것이 생산성 측면에서 더 좋지 않을까?
    • 엄청 크리티컬한 로직이 아니라면 굳이 풀어헤쳐서 Service에 전달하는 것이 아닌, Request를 그냥 전달하자고 합의했다.

image

위를 이어서.. 공부

비고

  • 참고했던 링크 등 참고 사항을 적어주세요. 코드 리뷰하는 사람이 참고해야 하는 내용을 자유로운 형식으로 적을 수 있습니다.

@pythonstrup pythonstrup added this to the 프로필 수정 milestone Nov 29, 2022
@pythonstrup pythonstrup self-assigned this Nov 29, 2022
@pythonstrup pythonstrup linked an issue Nov 29, 2022 that may be closed by this pull request
[Feature] 프로필 수정 API #128
Closed
3 tasks
username1103
username1103 reviewed Nov 29, 2022
View reviewed changes
backend/src/app/myinfo/exception/username-duplicate.exception.ts Outdated
constructor(
message = '중복된 유저이름으로 요청했습니다! 해당 유저이름으로 바꿀 수 없습니다.',
) {
super({ status: 'USER_NAME_BAD_REQUEST', message });
Copy link
Member

Choose a reason for hiding this comment

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

USER_NAME_BAD_REQUEST가 중복된 유저에 대한 내용인지 알기 힘든 것 같아요.

Copy link
Member Author

Choose a reason for hiding this comment

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

반영했습니다.

kong430
kong430 approved these changes Nov 29, 2022
View reviewed changes
Copy link
Collaborator

@kong430 kong430 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!

@pythonstrup
♻️ refactor: 코드리뷰 반영
4dc4944 
- Exception 이름 변경
@pythonstrup
♻️ refactor: 삭제된 유저를 find 논리에서 제외
a705e82 
- 회원탈퇴했을 경우 탈퇴한 유저의 userName을 다른 유저가 사용할 수 있도록 로직을 변경
@pythonstrup
♻️ refactor: myinfo Controller & Service 리팩토링
23f60ec 
- 인자 값을 객체로 넘기자!
username1103
username1103 approved these changes Nov 29, 2022
View reviewed changes
Copy link
Member

@username1103 username1103 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다.

@pythonstrup pythonstrup merged commit 91e828e into develop Nov 29, 2022
@username1103 username1103 deleted the feature/128-modify-profile-api branch November 30, 2022 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kong430 kong430 kong430 approved these changes

@username1103 username1103 username1103 approved these changes

@sxungchxn sxungchxn Awaiting requested review from sxungchxn

Assignees

@pythonstrup pythonstrup

Labels
✍ 리뷰요청 🌿 백엔드 🚨 우선순위-긴급 ✨ feature
Projects
None yet
Milestone
프로필 수정
Development

Successfully merging this pull request may close these issues.

[Feature] 프로필 수정 API
3 participants
@pythonstrup @kong430 @username1103