[FEATURE] 강사 수정 관련 기능 구현 #51
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthrough
Changes강사 시안 수정/확정 기능 구현
Sequence Diagram(s)sequenceDiagram
rect rgba(70, 130, 180, 0.5)
Note over Client,DraftController: 시안 최종 확정
Client->>DraftController: POST /{commissionId}/drafts/{draftId}/finalize
DraftController->>DraftFacade: finalizeDraft(instructorId, commissionId, draftId)
DraftFacade->>Commission: validateFinalizable()
DraftFacade->>DraftService: getLatestDraftOfSelectedApplication(commissionId)
DraftService-->>DraftFacade: latestDraft
DraftFacade->>Commission: complete() → status = COMPLETED
DraftFacade-->>DraftController: void
DraftController-->>Client: ApiResponse(success)
end
rect rgba(60, 179, 113, 0.5)
Note over Client,InstructorRevisionController: 수정 요청 생성
Client->>InstructorRevisionController: POST /{commissionId}/revisions
InstructorRevisionController->>InstructorRevisionFacade: createRevision(instructorId, commissionId, request)
InstructorRevisionFacade->>Commission: validateRevisable()
InstructorRevisionFacade->>Commission: isRevisionLimitExceeded(currentCount)
InstructorRevisionFacade->>DraftService: getLatestDraftOfSelectedApplication(commissionId)
DraftService-->>InstructorRevisionFacade: latestDraft
InstructorRevisionFacade->>RevisionService: hasRevisionRequestOnDraft(draftId)
RevisionService-->>InstructorRevisionFacade: false
InstructorRevisionFacade->>RevisionService: createRevisionRequest(commission, draft, request)
InstructorRevisionFacade-->>InstructorRevisionController: void
InstructorRevisionController-->>Client: ApiResponse(success)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/ditda/backend/domain/commission/draft/repository/CommissionDraftRepository.java`:
- Around line 46-55: The findLatestDraftInCommissionByStatus method uses
non-standard JPQL syntax with `limit 1` which can cause runtime parsing errors.
Remove the `limit 1` clause from the `@Query` annotation and instead add a Limit
parameter of type org.springframework.data.domain.Limit as the third parameter
to the method signature, keeping the return type as Optional<CommissionDraft>.
Import the Limit class from Spring Data JPA
(org.springframework.data.domain.Limit). Callers of this method will now pass
Limit.of(1) as the third argument to retrieve only the latest draft, using
standard JPA-compliant syntax.
In
`@src/main/java/ditda/backend/domain/commission/revision/controller/InstructorRevisionController.java`:
- Around line 29-40: The getRevisionDetail method in
InstructorRevisionController is a GET endpoint that currently performs
state-changing operations (acknowledging/checking comments), which violates REST
principles requiring GET to be idempotent. Refactor this by keeping the GET
endpoint to only retrieve and return the revision details without any state
changes, and create a separate POST or PATCH endpoint (e.g., acknowledgeRevision
or checkRevision) to handle the state-changing operation of marking comments as
checked. This ensures proper separation of concerns between data retrieval and
state modification operations.
In
`@src/main/java/ditda/backend/domain/commission/revision/entity/RevisionRequest.java`:
- Around line 22-23: The RevisionRequest entity has a `@OneToOne` relationship but
is missing unique constraint enforcement on the database level, allowing race
conditions where multiple RevisionRequest rows could share the same
target_draft_id and break the 1:1 invariant. Locate the field in RevisionRequest
that represents the foreign key to the target draft (likely annotated with
`@OneToOne` and `@JoinColumn`) and add unique = true to the `@JoinColumn` annotation
to ensure Hibernate generates a unique constraint in the database DDL.
In
`@src/main/java/ditda/backend/domain/commission/revision/facade/InstructorRevisionFacade.java`:
- Around line 72-84: The current implementation has a race condition where the
validation checks (calculateCurrentRevisionCount, validateCanCreateRevision,
hasRevisionRequestOnDraft) are performed separately from the actual revision
creation (createRevisionRequest), allowing concurrent requests to bypass
validation. Add synchronization protection around the entire
validation-to-creation block using pessimistic locking (such as a database lock
on the commission entity) or transactional synchronization to ensure the
check-then-act operations are atomic. Additionally, ensure appropriate database
unique constraints exist on the revision/draft relationship to prevent duplicate
entries at the database level as a secondary safety measure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro Plus
Run ID: 33df5266-7eba-466a-b4d4-37b6402f036a
📒 Files selected for processing (20)
src/main/java/ditda/backend/domain/commission/core/entity/Commission.javasrc/main/java/ditda/backend/domain/commission/core/exception/CommissionErrorCode.javasrc/main/java/ditda/backend/domain/commission/draft/controller/DraftController.javasrc/main/java/ditda/backend/domain/commission/draft/entity/CommissionDraft.javasrc/main/java/ditda/backend/domain/commission/draft/exception/DraftErrorCode.javasrc/main/java/ditda/backend/domain/commission/draft/facade/DraftFacade.javasrc/main/java/ditda/backend/domain/commission/draft/repository/CommissionDraftFileRepository.javasrc/main/java/ditda/backend/domain/commission/draft/repository/CommissionDraftRepository.javasrc/main/java/ditda/backend/domain/commission/draft/service/DraftService.javasrc/main/java/ditda/backend/domain/commission/revision/controller/InstructorRevisionController.javasrc/main/java/ditda/backend/domain/commission/revision/dto/request/RevisionCreateRequest.javasrc/main/java/ditda/backend/domain/commission/revision/dto/response/InstructorRevisionDetailResponse.javasrc/main/java/ditda/backend/domain/commission/revision/entity/RevisionRequest.javasrc/main/java/ditda/backend/domain/commission/revision/entity/RevisionResponse.javasrc/main/java/ditda/backend/domain/commission/revision/exception/RevisionErrorCode.javasrc/main/java/ditda/backend/domain/commission/revision/facade/InstructorRevisionFacade.javasrc/main/java/ditda/backend/domain/commission/revision/mapper/RevisionMapper.javasrc/main/java/ditda/backend/domain/commission/revision/repository/RevisionRequestRepository.javasrc/main/java/ditda/backend/domain/commission/revision/repository/RevisionResponseRepository.javasrc/main/java/ditda/backend/domain/commission/revision/service/RevisionService.java
💤 Files with no reviewable changes (1)
- src/main/java/ditda/backend/domain/commission/draft/entity/CommissionDraft.java
Jong0128
reviewed
Jun 23, 2026
Jong0128
left a comment
Jong0128
left a comment
Collaborator
There was a problem hiding this comment.
수고하셨습니다!
아래 커멘트 확인 부탁드립니다 👍
Comment on lines
+27
to
+41
| public static InstructorRevisionDetailResponse of( | ||
| Commission commission, | ||
| CommissionDraft draft, | ||
| String thumbnailUrl, | ||
| String designerComment, | ||
| int currentRevisionCount | ||
| ) { | ||
| return new InstructorRevisionDetailResponse( | ||
| commission.getId(), | ||
| commission.getTitle(), | ||
| new DraftInfo(draft.getId(), thumbnailUrl, designerComment), | ||
| currentRevisionCount, | ||
| commission.getMaxRevision() | ||
| ); | ||
| } |
Collaborator
There was a problem hiding this comment.
mapper에서 썸네일 url만 만들고, 실제 응답 객체 조립은 다시 InstructorRevisionDetailResponse.of()가 하고 있어서, 응답을 위한 조립이 mapper랑 DTO 두 군데로 나뉘어 있는 것 같습니다!
기존 DraftResponseMapper처럼 mapper가 InstructorRevisionDetailResponse까지 직접 책임지고, DTO의 of()는 빼는 방향이 더 일관적일 것 같은데 어떻게 생각하시나요??
| public int calculateCurrentRevisionCount(Commission commission) { | ||
| int used = revisionRequestRepository.countByCommissionId(commission.getId()); | ||
|
|
||
| return Math.min(used, commission.getMaxRevision()); |
Collaborator
There was a problem hiding this comment.
해당 부분에서 실제 수정 회차와 플랜에 따른 수정횟수 값들 중 작은 값을 반환해주는걸로 이해했습니다!
혹시 이렇게 작성한 이유가 있으실까요? 보통 실제 회차가 플랜 최대 수정횟수보다 더 크게 되면 문제가 생기는거지 않나요?
제가 느끼기엔 에러가 발생해야할 부분이 숨겨질꺼같습니다!
Comment on lines
+45
to
+46
| // 외주 수정 제한 | ||
| REVISION_LIMIT_EXCEEDED(HttpStatus.CONFLICT, "COMMISSION_409_05", "수정 횟수를 모두 사용했습니다."); |
Collaborator
There was a problem hiding this comment.
리뷰어 노트에 작성하신거 읽어봤습니다! 다만, 외주 수정 제한의 경우에는 그래도 RevisionErrorCode가 더 적절하지 않을까요?
Jong0128
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 Related issue
Closes #44
#️⃣ Summary
🔧 Changes
GET /commissions/{commissionId}/revisions/current)POST /commissions/{commissionId}/revisions)POST /commissions/{commissionId}/drafts/{draftId}/finalize)📸 Test Evidence
💬 Reviewer Notes
CommissionDraft에 있던is_final필드는 굳이 불필요하다고 판단해 제거했습니다.Commission) 자체의 룰 위반이므로 에러 코드 위치를CommissionErrorCode로 정리했습니다.호출자(
Facade)가 다른 애그리게이트의 에러 코드를 import하지 않도록 엔티티에validateXxx()패턴을 적용해 엔티티가 직접 throw하도록 했습니다.Summary by CodeRabbit
Release Notes
New Features