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] 게시글 수정 및 상태 변경 #38

Merged
merged 9 commits into from
Jan 12, 2024
Merged

Conversation

OJOJIN
Copy link
Contributor

@OJOJIN OJOJIN commented Jan 12, 2024

Related Issue 🪢

Summary 🌿

  • 게시물 수정 API
  • 게시물 모집 상태 변경 API

Before i request PR review 🧤

  • 서비스 코드에 두 API가 모두 동일하게
User user = getUserOrThrow(userId);
        Post post = getPostOrThrow(postId);
        validatePostWriter(user, post);

의 과정을 거쳐 3단계의 검증을 거치는데 이렇게 두는게 좋을지 아니면

// PostRepository.java
Optional<Post> findByIdAndUserId(Long id, Long userId);

// PostService.java
postRepository.findByIdAndUserId(postId, userId)
   .orElseThrow(() -> 어쩌고);

이렇게 하나의 과정으로 줄여서 검증을 진행하는게 좋을지 고민이됩니다.

첫번째 방법은 검증이 자세한 이점이 있지만 쿼리가 유저, 포스트 부를때 각각 1번씩 나가 총 2번 호출되고
두번째 방법은 검증은 자세하게 하지 못하지만 한번의 쿼리로 해결한다는 점에서 이점이 있어보입니다.

많은 의견 부탁드립니다. 감사합니다.

  • 또한 게시글 수정, 게시글 모집 상태 수정 때 현재는 response로 바뀐 값을 다시 넘겨주었는데 이렇게 값을 다시 넘겨주는게 좋을지 아니면 SuccessResponse.ok(null)정도로 빈 응답코드를 넘겨주면 좋을지 고민입니다.

주저리주저리 길어졌는데 모쪼록 천천히 잘부탁드립니다 🙇‍♂️🙇‍♂️

@OJOJIN OJOJIN added the FEAT 새로운 기능을 추가 label Jan 12, 2024
@OJOJIN OJOJIN self-assigned this Jan 12, 2024
Copy link
Member

@ziiyouth ziiyouth left a comment

Choose a reason for hiding this comment

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

개인적으로 저는 어떠한 부분에서 어떠한 검증이 이루어져 명시된 에러가 반환되었는지 아는 게 좋다고 생각해 전자가 좋다고 생각했습니다! 하지만 과도한 중복된 쿼리 호출로 성능상 문제가 발생할 여지가 있다면 후자를 고려해봐야 할 거 같습니다.

두 번째 내용은 프론트 개발자분들에게도 여쭈어 보는 게 좋을 거 같습니다. 새로 호출이 이루어진다고 한다면 null로만 반환해도 충분할 것 같아서요!

고생하셨구 감사합니다! 🤗

@ziiyouth ziiyouth merged commit 434c2aa into develop Jan 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FEAT 새로운 기능을 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feat] 게시글 수정 및 상태 변경
2 participants