-
Notifications
You must be signed in to change notification settings - Fork 1
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 [#10] 이력서 등록 POST API 구현 #12
Changes from 11 commits
0c19e7b
bbc07a3
676d5b5
24251b4
47f40d3
9b08c0a
2953edb
655be24
bc4f323
9ca4040
a83f13e
8fe9af2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package org.sopt.jumpit.resume.controller; | ||
|
||
|
||
import lombok.RequiredArgsConstructor; | ||
import org.sopt.jumpit.global.common.dto.SuccessResponse; | ||
import org.sopt.jumpit.resume.dto.ResumeCreateRequest; | ||
import org.sopt.jumpit.resume.service.ResumeService; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.PostMapping; | ||
import org.springframework.web.bind.annotation.RequestBody; | ||
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
import java.net.URI; | ||
|
||
@RestController | ||
@RequestMapping("/api/v1/resumes") | ||
@RequiredArgsConstructor | ||
public class ResumeController { | ||
private final ResumeService resumeService; | ||
|
||
@PostMapping | ||
public ResponseEntity<SuccessResponse> createResume ( | ||
@RequestBody ResumeCreateRequest resumeCreateRequest | ||
) { | ||
resumeService.createResume(resumeCreateRequest); | ||
return ResponseEntity.ok() | ||
.build(); | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Controller를 잘 구성해주신 것 같은데요! 조금만 개선하면 더 좋은 코드가 될 것 같아요 :)
이력서를 생성하는 API이기 때문에 201 Created 를 반환해야 합니다 ! 이 두가지를 한번 리마인드 해보시면서 리팩토링 해주면 더 멋진 코드가 될 것 같아요 🙂 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 궁금한 게 있는데 그렇게 되면 클라 단에서는 어차피 GET API를 사용해서 이력서 리스트를 받게 되는데 id 값을 반환해줘야 하는 이유를 알 수 있을까요!? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DB에 엔티티를 등록하는 API에서는 💡 RESTful API 설계 원칙에 따라, 새로운 리소스를 생성할 때는 201 Created 상태 코드와 함께 Location 헤더를 포함하는 것이 관례입니다! 이로써 API 사용자가 생성된 리소스의 위치를 쉽게 알 수 있도록 합니다 🙂 또한, 채은님의 질문처럼 명세서의 Response 부분이 수정되어야 하겠네요! |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package org.sopt.jumpit.resume.dto; | ||
|
||
public record ResumeCreateRequest( | ||
String title, | ||
Long userId | ||
) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
package org.sopt.jumpit.resume.service; | ||
|
||
import jakarta.transaction.Transactional; | ||
import lombok.RequiredArgsConstructor; | ||
import org.sopt.jumpit.global.common.dto.message.ErrorMessage; | ||
import org.sopt.jumpit.global.exception.BusinessException; | ||
import org.sopt.jumpit.resume.domain.Resume; | ||
import org.sopt.jumpit.resume.dto.ResumeCreateRequest; | ||
import org.sopt.jumpit.resume.repository.ResumeRepository; | ||
import org.sopt.jumpit.user.domain.User; | ||
import org.sopt.jumpit.user.repository.UserRepository; | ||
import org.springframework.stereotype.Service; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
public class ResumeService { | ||
private final ResumeRepository resumeRepository; | ||
private final UserRepository userRepository; | ||
|
||
@Transactional | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Transactional 키워드 좋네요 :)!! |
||
public void createResume( | ||
ResumeCreateRequest resumeCreateRequest | ||
) { | ||
User findUser = userRepository.findById(resumeCreateRequest.userId()).orElseThrow( | ||
() -> new BusinessException(ErrorMessage.USER_NOT_FOUND_BY_ID_EXCEPTION) | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exception 처리를 적절하게 해주신 건 아주 좋은 포인트입니다! 하지만, BusinessException은 상위 Exception 클래스이기 때문에 NotFoundException 커스텀 클래스를 이용해 핸들링해주면 더 좋을 것 같아요 :)! |
||
Resume resume = Resume.create(findUser, "내 이력서"); | ||
resumeRepository.save(resume); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Controller 에서 말씀드렸듯이, id 값을 같이 반환해주는 로직을 마지막 여기에 추가해주면 되겠죠 ? 😆🔥 |
||
} | ||
} |
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.
/api/v1
은 Controller 레벨에다 두고/resumes
는 메소드 레벨로 낮추는 것이 좋을 것 같습니다 :))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.
구체적으로 말씀드려보면,
요렇게 수정해보면 어떨까요 :)!