Feat [#14] 이력서 비공개 토글 PATCH API 구현 #16

chaentopia · 2024-05-18T21:04:47Z

🍀 작업한 내용에 대해 설명해주세요

도메인에서 isPrivate update를 하는 로직을 구현했습니다.

Jumpit-SERVER/jumpit/src/main/java/org/sopt/jumpit/resume/domain/Resume.java

Lines 47 to 51 in d51bf50

    
           public void updateIsPrivate( 
        
                   Boolean isPrivate 
        
           ) { 
        
               this.isPrivate = isPrivate; 
        
           }

Service는 아래와 같이 구현했습니다.

Jumpit-SERVER/jumpit/src/main/java/org/sopt/jumpit/resume/service/ResumeService.java

Lines 39 to 48 in d51bf50

    
           @Transactional 
        
           public void updateResumePrivate( 
        
                   Long resumeId, 
        
                   ResumePrivateRequest resumePrivateRequest 
        
           ) { 
        
               Resume resume = resumeRepository.findById(resumeId).orElseThrow( 
        
                       () -> new BusinessException(ErrorMessage.RESUME_NOT_FOUND_BY_ID_EXCEPTION) 
        
               ); 
        
               resume.updateIsPrivate(resumePrivateRequest.isPrivate()); 
        
           }

Controller는 아래와 같이 구현했습니다.

Jumpit-SERVER/jumpit/src/main/java/org/sopt/jumpit/resume/controller/ResumeController.java

Lines 34 to 42 in d51bf50

    
           @PatchMapping("/{resumeId}") 
        
           public ResponseEntity<SuccessResponse> updateIsPrivate( 
        
                   @PathVariable Long resumeId, 
        
                   @RequestBody ResumePrivateRequest resumePrivateRequest 
        
           ) { 
        
               resumeService.updateResumePrivate(resumeId, resumePrivateRequest); 
        
               return ResponseEntity.ok() 
        
                       .build(); 
        
           }

🍀 어떤 것을 중점으로 리뷰 해주길 바라시나요?

Patch에서는 특별히 더 처리해줄 것이 없는지 궁금합니다!

🍀 공통 작업 부분에 대한 수정 사항이 있다면 적어주세요

에러 메세지 추가

🍀 PR 유형

어떤 변경 사항인가요?

새로운 기능 추가
버그 수정
코드에 영향을 주지 않는 변경사항 (오타 수정, 탭 사이즈 변경, 변수명 변경)
코드 리팩토링
주석 추가 및 수정
문서 수정
파일 혹은 패키지명 수정
파일 혹은 패키지 삭제

🍀 Checklist

코드 컨벤션을 지켰나요?
git 컨벤션을 지켰나요?
PR 날리기 전에 검토하셨나요?

코드리뷰를 반영했나요?

🍀 Issue

Resolved #14

geniusYoo

솝커톤 하면서 API 3개 다 개발하느라 너무너무 너무너무너무너무 고생했어요 :))

코멘트가 너무 많지만, 하나하나 읽어보면 좋을 것 같아요! 너무 열심히 해주셔서 감사함니다 😆🙆🏻‍♂️👍🏻

geniusYoo · 2024-05-19T12:21:56Z

jumpit/src/main/java/org/sopt/jumpit/resume/domain/Resume.java

+            Boolean isPrivate
+    ) {
+        this.isPrivate = isPrivate;
+    }


updateIsPrivate 메소드는 엔티티의 상태를 변경하므로, 도메인 클래스에 존재하는 것은 적절하지 않습니다!
이는 Service 레이어로 옮겨주는 것이 적절할 것 같습니다 :)
Service에서 resume 객체를 찾은 후, 해당 객체의 isPrivate 상태를 변경해주면 되겠죠 ? :))

geniusYoo · 2024-05-19T12:23:51Z

jumpit/src/main/java/org/sopt/jumpit/resume/controller/ResumeController.java

+            @RequestBody ResumePrivateRequest resumePrivateRequest
+    ) {
+        resumeService.updateResumePrivate(resumeId, resumePrivateRequest);
+        return ResponseEntity.ok()


응답으로 아무 내용도 담지 않을 것이라면, 204 No Content를 반환하는 건 어떨까요 ??

또한, SucessResponse 형태의 반환을 정의해주셨는데, 아래 return 문에서는 반영되지 않은 것으로 보이네요 👀

jsoonworld · 2024-05-20T02:20:19Z

jumpit/src/main/java/org/sopt/jumpit/resume/controller/ResumeController.java

+    @PatchMapping("/{resumeId}")
+    public ResponseEntity<SuccessResponse> updateIsPrivate(
+            @PathVariable Long resumeId,
+            @RequestBody ResumePrivateRequest resumePrivateRequest
+    ) {
+        resumeService.updateResumePrivate(resumeId, resumePrivateRequest);
+        return ResponseEntity.ok()


굳이굳이 찾아보자면 메서드 안에서 사용되는 변수명인 걸 고려해 볼 때 resumePrivateRequest -> request 이렇게 해보는 것에 대해서 고민해 보는 것도 좋을 것 같아요!

geniusYoo

너무너무 고생하셨습니다 :)

코멘트가 많아서 ,,,, 힘드시겠지만 한번 천천히 읽어보시고 얘기 다시 나눠보면 너무 좋을 것 같아요 화이팅 🔥👍🏻👍🏻

geniusYoo · 2024-05-21T07:17:34Z

jumpit/src/main/java/org/sopt/jumpit/resume/controller/ResumeController.java

+        return ResponseEntity.status(HttpStatus.CREATED)
+                .location(location)
+                .body(SuccessResponse.of(SuccessMessage.RESUME_CREATED_COMPLETED_SUCCESS));
+    }


아핫 제가 설명을 더 자세하게 해줬어야 했는데!

생성된 엔티티의 pk 값을 Request Header의 Location 필드에 넣어주면 되는데요!

제가 생각한 방향성을 제시해볼게요! 참고해보세요 !ㅎㅎ

public ResponseEntity<SuccessResponse<Void>> createResume (..) { // 요기 <Void>라고 반환값이 없다는 것을 명시해주면 더 좋겠죠? return ResponseEntity.status(HttpStatus.CREATED) .header( "Location", resumeService.createResume(resumeCreateRequest)) .body(SuccessStatusResponse.of(SuccessMessage.RESUME_CREATED_COMPLETED_SUCCESS));

앗 제가 하나 빠트린 게 있네요 💡

Location 필드에 들어가는 value는 String이어야 하는 걸로 알고있습니다!
resumeService.createResume의 반환형을 String으로 바꾸고 resume.getId().toString() 요렇게 반환해주면 될것같아요!

geniusYoo · 2024-05-21T07:19:50Z

jumpit/src/main/java/org/sopt/jumpit/resume/controller/ResumeController.java

+        ResumeSearchResponse resumeSearchResponse = resumeService.findResumeById(userId);
+        return ResponseEntity.status(HttpStatus.OK)
+                .body(SuccessResponse.of(SuccessMessage.RESUME_SEARCH_COMPLETED_SUCCESS, resumeSearchResponse));
+    }


요런 리턴으로 받아주는 구문이 있다면 보기는 편하겠지만, 요정도 간단한 코드는 변수로 선언해서 넣어주지 않고 바로 메소드 호출을 return 구문 안에 넣어서 더욱 간단하게 만들수도 있답니다 :) !!

... SuccessResponse.of( SuccessMessage.RESUME_SEARCH_COMPLETED_SUCCESS, resumeService.findResumeById(userId))

요런식으로요 ! 😎 (필수는 아니고 참고용입니다 ㅎ!ㅎ)

geniusYoo · 2024-05-21T07:21:37Z

jumpit/src/main/java/org/sopt/jumpit/resume/dto/ResumeSearchResponse.java

+
+public record ResumeSearchResponse(
+        Long userId,
+        List<ResumeResponse> resume


요기 변수명은 resume 보다는 단수가 아닌 복수형이 더 좋을 것 같네요 :)

geniusYoo · 2024-05-21T07:23:09Z

jumpit/src/main/java/org/sopt/jumpit/resume/service/ResumeService.java

+        );
+        Resume resume = Resume.create(findUser, "내 이력서");
+        resumeRepository.save(resume);
+        return resume.getId();


요 세줄도 한줄로 간편하게 줄일 수 있겠죠?? 기능엔 전혀 상관은 없지만 한번 해보시길 추천드립니다아 👍

Resume resume = Resume.create(findUser, "내 이력서"); resumeRepository.save(resume); return resume.getId(); // 위 3줄짜리 코드를 1줄로 줄이면 ? return ~

geniusYoo · 2024-05-21T07:34:42Z

jumpit/src/main/java/org/sopt/jumpit/resume/service/ResumeService.java

+        userRepository.findById(userId).orElseThrow(
+                () -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_ID_EXCEPTION)
+        );
+        return new ResumeSearchResponse(userId, resumes);


지금의 플로우를 보면,

resumeRepository에서 해당 서비스에 등록된 모든 Resume들을 검색

userRepository에서 파라미터로 넘어온 userId로 유저를 검색

dto에 userId와 resume들을 넣어서 리턴

요 흐름으로 정리해볼 수 있을 것 같은데요!
지금은 유저가 1명이라고 생각했기 때문에 이 기능이 정상 작동하는 것으로 보이지만, 유저가 2명 이상만 되어도 문제가 발생합니다!

예시를 들어보면 💡
서비스의 유저가 2명이고 각 유저가 생성한 이력서가 3개씩 있다고 가정할 때,
지금의 API에서는 어떤 userId가 파라미터로 들어와도 이력서가 6개가 리턴될겁니다!
왜냐하면 현재는 userId에 해당하는 이력서를 찾는 것이 아니라, userId에 관계없이 모든 이력서를 각 유저에게 매핑하여 전달하고 있기 때문입니다!

이를 해결하려면 Resume 엔티티에는 User라는 필드를 가지고 있기 때문에,
ResumeRepository에서 findAll()이 아닌, findByOwnerId(Long ownerId) 이 메소드를 정의하여 사용해야겠죠? :)

한번 수정해보시고, 어려운 게 있다면 언제든지 다시 물어봐주세요 화이팅 👏

geniusYoo

고생하셨습니다 :) 😊

하나만 살짝 수정하면 더 좋은 코드가 될 것 같아요!

geniusYoo · 2024-05-21T12:21:10Z

jumpit/src/main/java/org/sopt/jumpit/resume/repository/ResumeRepository.java

 @Repository
 public interface ResumeRepository extends JpaRepository<Resume, Long> {
+    List<Resume> findByOwnerId(Long userId);


userId에 대한 Resume이 존재하지 않거나, user 자체가 존재하지 않을 수도 있으니 Optional 키워드를 붙이는 게 좋을 것 같아요!

geniusYoo · 2024-05-21T12:22:26Z

jumpit/src/main/java/org/sopt/jumpit/resume/service/ResumeService.java

+                () -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_ID_EXCEPTION)
+        );
+        return ResumeSearchResponse.of(userId, resumes);
+    }


또한 여기서, userRepository에서 user의 존재 여부를 검사하지 않고

resumeRepository.findByOwnerId()의 Exception 처리로 한번에 진행할 수 있을 것 같아요! (유저가 존재하지 않거나, 유저의 이력서가 존재하지 않거나)

이 부분에서 한가지 더 말씀드리면,
User의 존재 여부만을 판단해서 Exception을 날리기 위해 DB를 검색하는 건 성능 상 오버헤드라고 생각합니다!

모든 코드에는 적절한 이유가 있어야 한다는 걸 다시금 리마인드 해보면서 생각해보면 좋을 것 같네요 😆👍🏻 고생하셨어요!

…ePATCH

@NoArgsConstructor

@NoArgsConstructor 어노테이션 추가

chaentopia added 21 commits May 16, 2024 15:50

Merge branch 'feat/#5-searchAllPositions()' into feat/#10-resumesPOST

0c19e7b

Merge remote-tracking branch 'origin/main' into feat/#10-resumesPOST

bbc07a3

Chore [#10] gitignore 수정, properies 삭제

676d5b5

Feat [#10] 생성 Request Body DTO

24251b4

Feat [#10] 도메인 설정

47f40d3

Feat [#10] 에러메세지 수정

9b08c0a

Feat [#10] 서비스 구현

2953edb

Feat [#10] 컨트롤러 구현

655be24

Fix [#10] 에러 메세지 수정

bc4f323

Fix [#10] 이력서 제목 디폴트값으로 수정

9ca4040

Merge branch 'feat/#10-resumesPOST' into feat/#13-resumeGET

4177969

Feat [#13] 도메인 구현

7814f3e

Feat [#13] 서비스 구현

07fd2cb

Feat [#13] DTO 구현

535fee8

Feat [#13] 컨트롤러 구현

4c53d0e

Merge branch 'feat/#13-resumeGET' into feat/#14-resumePrivatePATCH

3256339

Feat [#14] 도메인 update 로직 구현

dd44e02

Feat [#14] 에러메세지 추가

a560573

Feat [#13] DTO 추가

110e94a

Feat [#14] 서비스 구현

f7785c1

Feat [#14] Controller 구현

d51bf50

chaentopia added the feat label May 18, 2024

chaentopia requested review from jsoonworld and geniusYoo May 18, 2024 21:04

chaentopia self-assigned this May 18, 2024

geniusYoo reviewed May 19, 2024

View reviewed changes

jsoonworld reviewed May 20, 2024

View reviewed changes

chaentopia added 3 commits May 20, 2024 15:51

Fix [#14] error exception 코드리뷰 반영

a73a0d2

Fix [#14] 불필요한 import 삭제

caeb119

Fix [#14] path 코드리뷰 반영

4e1f82c

chaentopia and others added 9 commits May 20, 2024 16:20

Fix [#14] 204 반환 코드리뷰 반영

a816867

Fix [#14] 코드리뷰 반영

367be2e

Fix [#14] id값 반환 서비스 로직 구현

f0eaad4

Fix [#14] setter로 isPrivate 수정

1390215

Fix [#14] 이름 변경

56c022a

Add [#14] SuccessMessage 추가

0ea71d2

Feat [#14] 이력서 리스트 형태로 반환하도록 수정

53b5f52

Fix [#14] 전체 반환 값 수정

07059a3

Merge branch 'main' into feat/#14-resumePrivatePATCH

4adc3f1

geniusYoo reviewed May 21, 2024

View reviewed changes

Fix [#14] 코드리뷰 반영

87934a1

geniusYoo reviewed May 21, 2024

View reviewed changes

chaentopia and others added 2 commits May 21, 2024 22:38

Merge remote-tracking branch 'origin/main' into feat/#14-resumePrivat…

b331b39

…ePATCH

Fix [#14] Default Constructor 이슈 해결

7e547b5

@NoArgsConstructor 어노테이션 추가

geniusYoo merged commit 7e547b5 into main May 21, 2024
0 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat [#14] 이력서 비공개 토글 PATCH API 구현 #16

Feat [#14] 이력서 비공개 토글 PATCH API 구현 #16

chaentopia commented May 18, 2024

geniusYoo left a comment

geniusYoo May 19, 2024

geniusYoo May 19, 2024

jsoonworld May 20, 2024

geniusYoo left a comment

geniusYoo May 21, 2024

geniusYoo May 21, 2024

geniusYoo May 21, 2024

geniusYoo May 21, 2024

geniusYoo May 21, 2024

geniusYoo May 21, 2024

geniusYoo left a comment

geniusYoo May 21, 2024

geniusYoo May 21, 2024

geniusYoo May 21, 2024

	public void updateIsPrivate(
	Boolean isPrivate
	) {
	this.isPrivate = isPrivate;
	}

	@Transactional
	public void updateResumePrivate(
	Long resumeId,
	ResumePrivateRequest resumePrivateRequest
	) {
	Resume resume = resumeRepository.findById(resumeId).orElseThrow(
	() -> new BusinessException(ErrorMessage.RESUME_NOT_FOUND_BY_ID_EXCEPTION)
	);
	resume.updateIsPrivate(resumePrivateRequest.isPrivate());
	}

	@PatchMapping("/{resumeId}")
	public ResponseEntity<SuccessResponse> updateIsPrivate(
	@PathVariable Long resumeId,
	@RequestBody ResumePrivateRequest resumePrivateRequest
	) {
	resumeService.updateResumePrivate(resumeId, resumePrivateRequest);
	return ResponseEntity.ok()
	.build();
	}

Feat [#14] 이력서 비공개 토글 PATCH API 구현 #16

Feat [#14] 이력서 비공개 토글 PATCH API 구현 #16

Conversation

chaentopia commented May 18, 2024

🍀 작업한 내용에 대해 설명해주세요

🍀 어떤 것을 중점으로 리뷰 해주길 바라시나요?

🍀 공통 작업 부분에 대한 수정 사항이 있다면 적어주세요

🍀 PR 유형

🍀 Checklist

🍀 Issue

geniusYoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geniusYoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

geniusYoo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment