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

Feat: 신규 그룹 생성 기능 #24

Merged
merged 7 commits into from
Jul 13, 2024
Merged

Feat: 신규 그룹 생성 기능 #24

merged 7 commits into from
Jul 13, 2024

Conversation

loveysuby
Copy link
Contributor

🚀 Related Issue

close #23

📌 Tasks

  • 새로운 그룹을 생성하는 기능을 추가하였습니다.

📝 Details

  • 아래 endpoint로 접근 가능합니다.
// request
// `POST` | /groups/create (name 필드를 requestBody에 동봉합니다.)
{
    "name" : "hart-group"
}

//response
{
      "timestamp": "2024.07.12 16:02:07",
      "code": "TYU-200",
      "message": "응답 성공",
      "data": {
          "id": 1,
          "name": "hart-group"
      }
}

📚 Remarks

Points or opinions to share teams

  • users, groups 등등을 단수형으로 표시하도록 수정하는 것이 명확해 보이는데 어떻게 생각하실까요 ?
    • 단일 엔티티를 복수 개 원하는 api에서의 표현이 불명확해질 것 같다는 생각이 들어서요 🤔

@loveysuby loveysuby added 🧾 API API feature ✨ feat Improve performance or feature 🥵 hart labels Jul 12, 2024
@loveysuby loveysuby requested a review from woosung1223 July 12, 2024 07:09
@loveysuby loveysuby self-assigned this Jul 12, 2024
@loveysuby loveysuby linked an issue Jul 12, 2024 that may be closed by this pull request
[Feat] 신규 그룹 생성 기능 #23
Closed
woosung1223
woosung1223 requested changes Jul 12, 2024
View reviewed changes
src/main/java/slvtwn/khu/toyouserver/application/GroupService.java Outdated
return Optional.of(new Group(name))
.map(groupRepository::save)
.map(group -> new GroupResponse(group.getId(), group.getName()))
.orElseThrow(() -> new ToyouException(ErrorType.INVALID_GROUP_DATA));
Copy link
Contributor

Choose a reason for hiding this comment

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

혹시 map 사용하시려고 Optional.of로 감싸셨습니까!

Optional은 null-safe한 코드를 작성하기 위해 사용하는 것입니다.

A container object which may or may not contain a non-null value.

map을 Optional에서 정적 함수로 제공하는 이유도 비슷한데, null의 가능성이 있는 객체를 직접적으로 다루지 않고도 타입을 변환시킬 수 있기 때문입니다.

라이브러리를 사용할 땐 (자바 표준 라이브러리도 마찬가지) 설계자의 의도를 지키는게 중요합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

하마터면 엔티티에 명시된 생성자로 가져온 객체인데 불필요한 로직이 들어갈 뻔 했네요 ! 역시 .. 예리하게 짚어주셔서 바로 수정했습니다.

src/main/java/slvtwn/khu/toyouserver/common/ErrorType.java Outdated
@@ -8,13 +8,15 @@ public enum ErrorType {
// 400 ~ 499 (요청 오류)
RESPONSE_FORMAT_ERROR(HttpStatus.BAD_REQUEST, "TYU-400", "응답 형식 오류"),
BAD_REQUEST(HttpStatus.BAD_REQUEST, "TYU-400", "잘못된 요청입니다."),
INVALID_GROUP_DATA(HttpStatus.BAD_REQUEST, "TYU-4001", "Invalid group data provided"),
Copy link
Contributor

Choose a reason for hiding this comment

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

왜 요기만 메세지가 영어인가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for mistake. 🧑‍🔬

src/main/java/slvtwn/khu/toyouserver/presentation/GroupController.java
public GroupResponse registerMember(@PathVariable long groupId) {
return groupService.registerUser(groupId, 1L); // TODO: user -> argumentResolver 등록 필요
}
@PostMapping("/groups/create")
Copy link
Contributor

Choose a reason for hiding this comment

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

엔드포인트가 저희 API 명세와 다른 것 같은데요!?!?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

애초에 그룹 생성에 대한 명세가 없어서 추가하고 그룹 가입과 별개로 두었는데 빠뜨린 부분 있으면 수정하겠습니다 🙀

Copy link
Contributor

Choose a reason for hiding this comment

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

오 이야기했던 기억은 있는 것 같은데 따로 문서화하진 않았었군요.. 알겠읍니다

다만 API 엔드포인트에 동사는 지양하도록 설계해주세요!
그룹 생성이라면 /groups에 POST 요청을 날리는게 RESTful한 접근 방식입니다.

@loveysuby loveysuby mentioned this pull request Jul 12, 2024
Merged
loveysuby
loveysuby commented Jul 12, 2024
View reviewed changes
Copy link
Contributor Author

@loveysuby loveysuby left a comment

Choose a reason for hiding this comment

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

하하 create는 애초에 errorCode를 따로 만들지 않아도 표준 에러로 명시해줄 수 있을 것 같습니다.

반영해서 수정하겠습니다 !

src/main/java/slvtwn/khu/toyouserver/application/GroupService.java Outdated
return Optional.of(new Group(name))
.map(groupRepository::save)
.map(group -> new GroupResponse(group.getId(), group.getName()))
.orElseThrow(() -> new ToyouException(ErrorType.INVALID_GROUP_DATA));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

하마터면 엔티티에 명시된 생성자로 가져온 객체인데 불필요한 로직이 들어갈 뻔 했네요 ! 역시 .. 예리하게 짚어주셔서 바로 수정했습니다.

src/main/java/slvtwn/khu/toyouserver/presentation/GroupController.java
public GroupResponse registerMember(@PathVariable long groupId) {
return groupService.registerUser(groupId, 1L); // TODO: user -> argumentResolver 등록 필요
}
@PostMapping("/groups/create")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

애초에 그룹 생성에 대한 명세가 없어서 추가하고 그룹 가입과 별개로 두었는데 빠뜨린 부분 있으면 수정하겠습니다 🙀

src/main/java/slvtwn/khu/toyouserver/common/ErrorType.java Outdated
@@ -8,13 +8,15 @@ public enum ErrorType {
// 400 ~ 499 (요청 오류)
RESPONSE_FORMAT_ERROR(HttpStatus.BAD_REQUEST, "TYU-400", "응답 형식 오류"),
BAD_REQUEST(HttpStatus.BAD_REQUEST, "TYU-400", "잘못된 요청입니다."),
INVALID_GROUP_DATA(HttpStatus.BAD_REQUEST, "TYU-4001", "Invalid group data provided"),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for mistake. 🧑‍🔬

@loveysuby loveysuby requested a review from woosung1223 July 12, 2024 14:15
@woosung1223 woosung1223 merged commit 9498902 into develop Jul 13, 2024
1 check passed
@woosung1223 woosung1223 deleted the feat/23 branch August 4, 2024 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@woosung1223 woosung1223 Awaiting requested review from woosung1223

Assignees

@loveysuby loveysuby

Labels
🧾 API API feature ✨ feat Improve performance or feature 🥵 hart
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feat] 신규 그룹 생성 기능
2 participants
@loveysuby @woosung1223