-
Notifications
You must be signed in to change notification settings - Fork 0
Draft > Application: Draft 제목, 시리즈, URL을 임시저장 합니다. #440
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
base: develop
Are you sure you want to change the base?
Conversation
…e, path, and series article
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.
빠른 작업 감사합니다...! 💪
조금 더 고칠 부분도 발견되어서 아직은 approve 하지 않아야 할 것 같지만, 수정하시면 다시 리뷰하겠습니다!
public class DraftCommandService implements | ||
DraftCreateUseCase, | ||
DraftUpdateUseCase, | ||
DraftDeleteUseCase, | ||
DraftImageCreateUseCase, | ||
DraftPatchUseCase |
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.
🎨 라인이 끝나지 않을 때 줄바꿈 시, 들여쓰기는 2탭입니다!
(중괄호, 세미콜론이 등장하기 전 줄바꿈)
...application/src/main/java/nettee/blolet/article/application/service/DraftCommandService.java
Show resolved
Hide resolved
public SeriesArticle patchSeriesArticle(String userId, String draftId, String seriesId, String articleId) { | ||
var seriesArticle = draftCommandPort.findSeriesArticleById(draftId) | ||
.orElseThrow(SERIES_ARTICLE_NOT_FOUND::exception); | ||
|
||
validateOwnership(userId, draftId); | ||
|
||
return draftCommandPort.updateSeriesArticle(seriesArticle.getDraftId(), seriesId, articleId); | ||
} |
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.
❗️ 시리즈 아티클을 등록하는 것이 본작업입니다.
지금 코드는 약간 오해를 담고 있는 것 같아요. 👀
구현할 기능
- 시리즈 아티클 등록
: 지금 구조와 반대로 '이미 등록되어 있습니다.' 등으로 필터
(같은 드래프트, 같은 시리즈에 대한 연결일 때) - 드래프트에 연결된 시리즈 정보 수정 (비정규화 시)
사전 조건(Preconditions)
- 요청한 사용자가 시리즈 및 드래프트에 대한 수정 권한을 갖고 있어야 합니다.
- 시리즈와 드래프트가 같은 블로그에 속해야 합니다.
var seriesArticle = draftCommandPort.findSeriesArticleById(draftId) | ||
.orElseThrow(SERIES_ARTICLE_NOT_FOUND::exception); | ||
|
||
validateOwnership(userId, draftId); |
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.
❗️ 블로그 소유권을 확인하는 함수입니다.
지금처럼 실수가 생기지 않도록 함수 이름을 수정해 둬야 할 것 같기도 하네요. 저도 헷갈렸던 경험이 있어서 이참에 수정하는 게 좋을 것 같습니다!
이 요청 처리에는 사용자로부터 드래프트 소유권, 시리즈 소유권을 모두 확인해야 하고, 두 자원(드래프트, 시리즈)이 같은 블로그에 속해야 하므로 이하 확인이 필요해 보여요.
사전 조건
- 드래프트의 블로그 아이디와 시리즈의 블로그 아이디가 일치.
- 사용자가 해당 블로그의 소유권을 갖고 있음.
바쁜 중에 작업하다 보면 이렇게 리뷰로 확인할 수 있는 부분이 있어 리뷰 문화의 중요함을 알게 되네요.
앞으로 제 실수도 자주 발견되면 좋겠습니다!
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.
Pull Request Overview
This PR implements draft temporary saving functionality for title, series, and URL in the application layer. The changes add new use cases and service methods to handle partial updates to draft entities, allowing users to save drafts incrementally.
- Adds
DraftPatchUseCase
interface with methods for patching title, path, and registering series articles - Implements patch operations in
DraftCommandService
with proper validation and error handling - Extends
DraftCommandPort
with new methods for granular draft updates and series management
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
DraftCommandAdapter.java | Renames method from findById to findDraftById for clarity |
DraftPatchUseCase.java | New interface defining patch operations for drafts |
DraftCommandService.java | Implements patch use cases with validation logic |
DraftCommandPort.java | Extends port with new methods for draft and series operations |
DraftErrorCode.java | Adds new error codes for series-related validation failures |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
Draft updateTitle(String draftId, String title); | ||
Draft updatePath(String draftId, String path); | ||
SeriesArticle updateSeriesArticle(String draftId, String seriesId, String articleId); |
Copilot
AI
Sep 18, 2025
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.
The updateSeriesArticle
method is declared in the port interface but is not implemented or used in the service. This unused method should be removed to avoid confusion.
SeriesArticle updateSeriesArticle(String draftId, String seriesId, String articleId); |
Copilot uses AI. Check for mistakes.
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.
코 덜 파셨나
import java.util.function.Supplier; | ||
|
||
public enum DraftErrorCode implements ErrorCode { | ||
|
Copilot
AI
Sep 18, 2025
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.
[nitpick] The empty line and comment placement creates inconsistent formatting. Remove the blank line after the enum declaration or move the comment to align with other comment groupings.
Copilot uses AI. Check for mistakes.
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.
일부 수정이 필요한 부분을 발견하여 남겨 두었습니다!
생각보다 검토할 게 많이 얽혀서 수정할 게 있는 기능이네요.
초기 개발에서는 어느 정도 작업 편의에 맞추되 올바른 데이터로 관리하고,
추후에는 개선 여지도 많은 기능 같습니다!
수정 후 다시 리뷰해야 할 것 같습니다.
DEFAULT("임시글 조작 오류", HttpStatus.INTERNAL_SERVER_ERROR), | ||
BLOG_MISMATCH("드래프트와 시리즈가 같은 블로그에 속하지 않습니다.", HttpStatus.BAD_REQUEST), | ||
|
||
// series status | ||
SERIES_ARTICLE_NOT_FOUND("시리즈 아티클을 찾을 수 없습니다.", HttpStatus.NOT_FOUND), | ||
SERIES_NOT_FOUND("시리즈를 찾을 수 없습니다.", HttpStatus.NOT_FOUND), | ||
SERIES_ALREADY_REGISTERED("이미 등록된 시리즈 아티클입니다.", HttpStatus.CONFLICT); |
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.
- 💊
DEFAULT
를 가장 아래에 배치하는 게 좋아 보입니다. - 💊 접두사를 최대한 붙이는 게 좋아 보입니다.
BLOG_MISMATCH
→DRAFT_BLOG_MISMATCH
SERIES_ARTICLE_NOT_FOUND
→DRAFT_SERIES_ARTICLE_NOT_FOUND
등- 정적 임포트 허용·권장 대상이므로 접두사를 통해 이름 충돌을 최소화하면 사용하기 편리할 것으로 예상합니다.
|
||
Draft updateTitle(String draftId, String title); | ||
Draft updatePath(String draftId, String path); | ||
SeriesArticle updateSeriesArticle(String draftId, String seriesId, String articleId); |
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.
코 덜 파셨나
validateOwnership(userId, draft.blogId()); | ||
validateOwnership(userId, series.blogId()); |
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.
다음처럼 개선해 볼 수 있을까요?
// [1] 서로 같은 블로그의 리소스인지 확인
var blogId = draft.blogId();
var seriesBlogId = series.blogId();
if (!Objects.equals(blogId, seriesBlogId)) {
// NOTE 정상적인 시나리오에서 발생하지 않으므로 너무 구체적인 메시지를 주지 않아도 충분합니다:
throw DRAFT_FORBIDDEN.exception();
}
// [2] 권한 검증
validateOwnership(userId, blogId);
- 통신을 줄일 수 있습니다.
// 권한 검증 | ||
validateOwnership(userId, draft.blogId()); | ||
validateOwnership(userId, series.blogId()); | ||
|
||
// 블로그 동일 여부 확인 | ||
if (!draft.blogId().equals(series.blogId())) { | ||
throw BLOG_MISMATCH.exception(); | ||
} |
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.
다음처럼 개선해 볼 수 있을까요?
// [1] 서로 같은 블로그의 리소스인지 확인
var blogId = draft.blogId();
var seriesBlogId = series.blogId();
if (!Objects.equals(blogId, seriesBlogId)) {
// NOTE 정상적인 시나리오에서 발생하지 않으므로 너무 구체적인 메시지를 주지 않아도 충분합니다:
throw DRAFT_FORBIDDEN.exception();
}
// [2] 권한 검증
validateOwnership(userId, blogId);
- 권한 검증 통신을 1회만 하므로, 통신을 줄일 수 있습니다.
// 시리즈 등록 | ||
var seriesArticle = draftCommandPort.createSeriesArticle(draftId, seriesId, articleId); |
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.
❓ 아티클 아이디, 드래프트 아이디를 모두 입력받는지, 그중 일부만 입력받을 수도 있는 건지 궁금합니다.
- 만약 모두 입력받는다면, 입력받은 값 사이에 관계를 검토해야 합니다.
ex:draftId
로 조회해 온 draft의articleId
가, 사용자가 입력한articleId
와 일치하는지 확인 - 만약 일부만 입력받는다면, 입력받은 값을 단서로(ex:
draftId
), 다른 값(ex:articleId
)을 데이터베이스에서 조회해 올 수 있습니다. (이미 조회하는 코드는 위에 존재하는 것 같습니다.)
따라서, 모두 입력받는다는 전제에서는 다음과 같은 흐름을 표현하는 코드가 선행되어야 할 수 있습니다.
var draftArticleId = draft.articleId();
if (!Objects.equals(draftArticleId, articleId)) {
throw ...
}
또는 일부만 입력받는다는 전제에서 다음과 같은 코드가 추가될 수 있습니다.
// [1] 최소 하나 이상 입력
// articleId 또는 draftId 중 하나는 필수로 입력되어야 함.
if (articleId == null && draftId == null) {
throw ...
}
// draftId 조회
if (articleId != null) {
// [2] article 조회
// [3] 파라미터로 입력받은 draftId는 null이거나 article.draftId()와 일치해야 함.
// [4] 파라미터로 입력받은 draftId가 null이었다면 article.draftId()를 draftId로 사용.
}
// articleId 조회
// [5] draft 조회
// [6] 파라미터로 입력받은 articleId는 null이거나 draft.articleId()와 일치해야 함.
// [7] 파라미터로 입력받은 articleId가 null이었다면 draft.articleId()를 articleId로 사용.
Issues
Description
Review Points
How Has This Been Tested?