fix: [alt-276] 사장님(MANAGER) 구인구직 API 결함 수정 - #98
Conversation
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough공고 생성·수정 요청을 도메인 커맨드로 변환합니다. 관리자 소유 워크스페이스를 검증합니다. 삭제된 공고와 일정은 조회에서 제외합니다. 응답에는 활성 일정과 공고 상태를 반영합니다. 채팅 스키마와 기존 데이터 백필 마이그레이션을 추가합니다. Changes공고 생성·수정 및 상태 처리
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ManagerPostingController
participant CreatePosting
participant WorkspaceQueryRepository
participant Posting
participant PostingRepository
ManagerPostingController->>CreatePosting: execute(command, actor)
CreatePosting->>WorkspaceQueryRepository: findByIdAndManagerUser(workspaceId, managerUser)
WorkspaceQueryRepository-->>CreatePosting: activated workspace
CreatePosting->>Posting: create(command, workspace)
CreatePosting->>PostingRepository: save(posting)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: 2
🤖 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/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.java`:
- Around line 46-51: Update UpdatePostingScheduleDto to validate each
workingDays value against the accepted DayOfWeek enum names and validate
startTime/endTime using the required ISO time format before toCommand() performs
DayOfWeek.valueOf and LocalTime.parse. Ensure invalid, null, or empty values are
reported as field validation errors through the existing `@Valid` path rather than
runtime exceptions.
In
`@src/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.java`:
- Around line 29-31: Group the 조회 실패, 삭제 공고, and 정상 수정 scenarios in
ManagerUpdatePostingTests into separate `@Nested` classes, each with a descriptive
`@DisplayName`. Also group the 활성 일정 조회 scenarios in PostingTests into an
appropriately named `@Nested` class with `@DisplayName`; update both test files
while preserving their existing test behavior.
🪄 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.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 482a201f-fc82-4663-b1b7-2374be08918e
📒 Files selected for processing (27)
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingScheduleRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/controller/ManagerPostingController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/controller/ManagerPostingControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/ManagerPostingListResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/PostingQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/PostingScheduleQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/ManagerPostingDetailResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/ManagerPostingListResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/PostingDetailResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/PostingListResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/workspace/persistence/WorkspaceQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/CreatePosting.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePosting.javasrc/main/java/com/dreamteam/alter/domain/posting/command/CreatePostingCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/PostingScheduleCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/UpdatePostingCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/UpdatePostingScheduleCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/entity/Posting.javasrc/main/java/com/dreamteam/alter/domain/posting/port/inbound/CreatePostingUseCase.javasrc/main/java/com/dreamteam/alter/domain/posting/port/inbound/ManagerUpdatePostingUseCase.javasrc/main/java/com/dreamteam/alter/domain/workspace/port/outbound/WorkspaceQueryRepository.javasrc/test/java/com/dreamteam/alter/application/posting/usecase/CreatePostingTests.javasrc/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.javasrc/test/java/com/dreamteam/alter/domain/posting/entity/PostingTests.java
| @ExtendWith(MockitoExtension.class) | ||
| @DisplayName("ManagerUpdatePosting 테스트") | ||
| class ManagerUpdatePostingTests { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
관련 테스트를 @Nested 클래스로 그룹화하세요.
src/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.java#L29-L31: 조회 실패·삭제 공고·정상 수정 시나리오를@DisplayName이 있는@Nested그룹으로 구성하세요.src/test/java/com/dreamteam/alter/domain/posting/entity/PostingTests.java#L14-L15: 활성 일정 조회 시나리오를@DisplayName이 있는@Nested그룹으로 구성하세요.
📍 Affects 2 files
src/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.java#L29-L31(this comment)src/test/java/com/dreamteam/alter/domain/posting/entity/PostingTests.java#L14-L15
🤖 Prompt for 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.
In
`@src/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.java`
around lines 29 - 31, Group the 조회 실패, 삭제 공고, and 정상 수정 scenarios in
ManagerUpdatePostingTests into separate `@Nested` classes, each with a descriptive
`@DisplayName`. Also group the 활성 일정 조회 scenarios in PostingTests into an
appropriately named `@Nested` class with `@DisplayName`; update both test files
while preserving their existing test behavior.
Source: Path instructions
hodoon
left a comment
hodoon
left a comment
There was a problem hiding this comment.
전체적으로 방향 좋습니다. 특히 도메인 엔티티에서 adapter.inbound DTO 의존을 걷어낸 건 CLAUDE.md 헥사고날 규칙에 정확히 맞고, javax.validation → jakarta 교체는 그동안 @NotNull이 아예 동작하지 않던 실제 결함을 잡은 수정입니다. Posting.create에서 .toList() → new ArrayList<>()로 바꾼 것도, 기존에는 스케줄 없이 생성하면 schedules가 null이고 있으면 불변 리스트라 이후 addSchedules 호출 시 NPE/UnsupportedOperationException이 나는 잠재 결함이었는데 같이 해소됐습니다.
./gradlew compileJava compileTestJava test --tests "*Posting*" 통과 확인했습니다.
확인했고 문제없는 부분:
.on(status.ne(DELETED))가 count·list 쿼리 양쪽에 동일하게 들어가 커서 페이지네이션totalCount가 어긋나지 않습니다. 두 번째 엔티티 fetch 쿼리에는 조건을 걸지 않은 것도 맞는 판단입니다(fetchJoin에 필터를 걸면 영속 컬렉션이 깨짐 — 대신getActiveSchedules()로 메모리에서 거르는 구조).- 지도 목록(
PostingMapListResponseDto)도PostingListResponse를 재사용하므로 활성 일정 필터가 자동으로 적용됩니다. ManagerUpdatePosting의 DELETED 가드는 기존ManagerUpdatePostingStatus와 동일한 패턴(CONFLICT)이고 새 ErrorCode를 만들지 않은 것도 맞습니다. 지금은 2곳이라 공통 추출까진 불필요해 보입니다.
아래 라인 코멘트 중 1·2번(중) 만 정리되면 머지해도 될 것 같고, 3~5번은 후속으로 빼도 무방합니다.
| for (UpdatePostingScheduleCommand updateCommand : updateSchedules) { | ||
| PostingSchedule existingSchedule = this.schedules.stream() | ||
| .filter(schedule -> schedule.getId().equals(updateDto.getId())) | ||
| .filter(schedule -> schedule.getId().equals(updateCommand.id())) |
There was a problem hiding this comment.
[중] updateSchedules/deleteSchedules가 DELETED 스케줄까지 대상으로 잡습니다.
이 PR의 취지가 "삭제된 근무일정은 노출되지 않고 지원도 불가"인데, 정작 수정/삭제 경로는 this.schedules 전체를 스캔합니다.
- 이미
DELETED인 스케줄 ID로updateSchedules를 보내면 → 내용은 바뀌고 상태는DELETED로 남는 유령 수정이 됩니다. 게다가PostingSchedule.update()는positionsAvailable까지 재계산해서 삭제된 일정의 잔여 인원이 되살아납니다. deleteScheduleIds에 이미 삭제된 ID를 넣어도 조용히 200이 나갑니다(같은 문제, 아래deleteSchedules에도 동일).
두 메서드 모두 getActiveSchedules() 기준으로 찾도록 바꾸면 한 곳에서 정리됩니다.
PostingSchedule existingSchedule = getActiveSchedules().stream()
.filter(schedule -> schedule.getId().equals(updateCommand.id()))
.findFirst()
.orElseThrow(() -> new CustomException(ErrorCode.NOT_FOUND, "수정할 스케줄을 찾을 수 없습니다."));| return new UpdatePostingScheduleCommand( | ||
| id, | ||
| workingDays.stream().map(DayOfWeek::valueOf).toList(), | ||
| LocalTime.parse(startTime), |
There was a problem hiding this comment.
[중] 시각 파싱 실패가 400이 아니라 500으로 나갑니다.
LocalTime.parse()가 던지는 DateTimeParseException은 IllegalArgumentException이 아니라 DateTimeException 계열이라 GlobalExceptionHandler의 어떤 핸들러에도 걸리지 않습니다 → 500.
반면 바로 위 DayOfWeek.valueOf()는 IllegalArgumentException이라 400(ILLEGAL_ARGUMENT)으로 나갑니다. 같은 DTO 안에서 동작이 갈립니다.
@Valid는 바인딩 시점에 끝나고 toCommand()는 컨트롤러 본문에서 호출되므로 검증으로도 못 막습니다.
한 쌍인 CreatePostingScheduleRequestDto는 이미 List<DayOfWeek>/LocalTime 타입이라 Jackson이 역직렬화 단계에서 400을 내줍니다. 이쪽 DTO도 같은 타입으로 맞추면 동작이 통일되고 toCommand()의 파싱 코드도 통째로 사라집니다.
private List<DayOfWeek> workingDays;
private LocalTime startTime;
private LocalTime endTime;| .selectFrom(qWorkspace) | ||
| .where( | ||
| qWorkspace.id.eq(id), | ||
| qWorkspace.managerUser.eq(managerUser) |
There was a problem hiding this comment.
[하] 업장 상태 조건이 없습니다.
소유권만 검증하고 status는 보지 않아서, 승인 대기(PENDING)·승인 취소(REVOKED)·폐업(CLOSED)·삭제(DELETED) 업장에도 공고를 등록할 수 있습니다.
소유권 검증을 넣는 김에 qWorkspace.status.eq(WorkspaceStatus.ACTIVATED)도 같이 걸지 판단이 필요해 보입니다. 정책상 승인 전 업장의 공고 등록을 허용하는 게 의도라면 그대로 두면 됩니다.
| assertThatThrownBy(() -> managerUpdatePosting.execute(1L, command(), actor)) | ||
| .isInstanceOf(CustomException.class) | ||
| .satisfies(ex -> assertThat(((CustomException) ex).getErrorCode()).isEqualTo(ErrorCode.CONFLICT)); | ||
| then(posting).should(never()).updateContent(command()); |
There was a problem hiding this comment.
[하] 이 never() 검증은 사실상 무의미합니다.
인자 매처가 command()가 만든 특정 값이라, updateContent가 다른 인자로 호출되더라도 "그 인자로는 호출된 적 없음"이 되어 통과합니다. 실제로 호출 자체가 없었는지를 잡으려면 매처를 넓혀야 합니다.
then(posting).should(never()).updateContent(any());| @@ -167,14 +171,6 @@ ResponseEntity<CommonApiResponse<Void>> updatePostingStatus( | |||
| name = "존재하지 않는 공고", | |||
| value = "{\"code\" : \"B007\"}" | |||
There was a problem hiding this comment.
[하] 이번에 추가된 409 케이스가 스펙에 빠졌습니다.
공고 등록 쪽에는 B008 예시를 추가하셨는데, 이 PR에서 새로 생긴 "삭제된 공고 수정 차단"(ErrorCode.CONFLICT / B020 / 409)은 수정 엔드포인트 문서에 반영되지 않았습니다. 409 @ApiResponse 블록을 하나 추가해주시면 좋겠습니다.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/dreamteam/alter/domain/posting/entity/Posting.java (1)
105-109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win삭제 공고 검증을
Posting.updateContent()에도 추가하세요.
ManagerUpdatePosting가PostingStatus.DELETED를 검사하더라도, Line [105]-[109]의 엔티티 메서드는 상태를 검사하지 않고 공고 내용을 변경합니다. 다른 유스케이스가 이 public 메서드를 호출하면 삭제 공고 수정 규칙을 우회할 수 있습니다.메서드 시작 부분에서 삭제 상태를 거부하는 도메인 검증을 추가하세요.
ManagerUpdatePosting의 검사는 권한과 입력 경계 검증으로 유지하세요.As per path instructions, 엔티티는 비즈니스 규칙과 불변식(상태 전이)을 캡슐화해야 합니다.
권장 수정
public void updateContent(UpdatePostingCommand command) { + if (PostingStatus.DELETED.equals(status)) { + throw new CustomException(ErrorCode.CONFLICT, "삭제 상태의 공고는 내용 수정이 불가합니다."); + } + this.title = command.title();🤖 Prompt for 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. In `@src/main/java/com/dreamteam/alter/domain/posting/entity/Posting.java` around lines 105 - 109, Update Posting.updateContent to reject updates when the posting status is PostingStatus.DELETED before applying any command fields. Keep ManagerUpdatePosting’s existing check for authorization and input-boundary validation, while enforcing the deleted-posting state invariant within the public entity method.Source: Path instructions
🤖 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.
Outside diff comments:
In `@src/main/java/com/dreamteam/alter/domain/posting/entity/Posting.java`:
- Around line 105-109: Update Posting.updateContent to reject updates when the
posting status is PostingStatus.DELETED before applying any command fields. Keep
ManagerUpdatePosting’s existing check for authorization and input-boundary
validation, while enforcing the deleted-posting state invariant within the
public entity method.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e1e0675d-e902-4b73-948e-e4aaaa593d28
📒 Files selected for processing (6)
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/controller/ManagerPostingControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/workspace/persistence/WorkspaceQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/domain/posting/entity/Posting.javasrc/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.javasrc/test/java/com/dreamteam/alter/domain/posting/entity/PostingTests.java
hodoon
left a comment
hodoon
left a comment
There was a problem hiding this comment.
Code review
라인별로 코멘트 남겼습니다. 전반적으로 방향이 좋은 PR입니다. 특히 아래 두 가지가 실질적인 수확이라고 봅니다.
CreatePostingRequestDto의javax.validation→jakarta.validationimport 교체. 그동안workspaceId·paymentType의@NotNull이 아예 동작하지 않고 있었습니다. 이 PR 에서 가장 값진 수정입니다.Posting엔티티에서 어댑터 DTO 의존을 걷어내고command패키지를 도입한 것. 헥사고날 의존성 방향 위반을 실제로 해소했습니다.
블로커는 없습니다. 확인 부탁드리는 항목은 다음과 같습니다.
- 활성 근무일정이 0개인 공고가 여전히 목록에 노출됩니다 (
.on()+leftJoin조합). - 검증이 실질적으로 강화되면서 기존 클라이언트가 400 을 받게 되는 케이스가 생깁니다 — 릴리스 노트 필요.
- CLAUDE.md 의 "DTO 그대로 전달" 규칙과 새 Command 패턴이 어긋납니다 — 문서 갱신 필요.
머지 순서: #99 가 PostingQueryRepositoryImpl 의 eqPostingStatus 를 inPostingStatusOrDefault 로 바꾸고 ManagerPostingListResponse 의 status 필드를 전제로 하고 있습니다. 이 PR 을 먼저 머지하는 편이 충돌이 적습니다.
| .select(qPosting.countDistinct()) | ||
| .from(qPosting) | ||
| .leftJoin(qPosting.schedules, qPostingSchedule) | ||
| .on(qPostingSchedule.status.ne(PostingStatus.DELETED)) |
There was a problem hiding this comment.
[중간] 활성 근무일정이 0개인 공고가 목록에 남습니다
.on() 으로 DELETED 일정을 조인에서 걸러낸 방향은 정확합니다. 이전에는 삭제된 일정의 시간대로도 공고가 검색됐으니 실제 결함 수정입니다.
다만 leftJoin 이라 매칭되는 활성 일정이 하나도 없으면 NULL 행이 남습니다. 그리고 아래 WHERE 의 시간 필터는
gteStartTime(qPostingSchedule, filter.getStartTime()),
lteEndTime(qPostingSchedule, filter.getEndTime())필터가 null 이면 조건 자체를 만들지 않으므로 NULL 행이 걸러지지 않습니다. 결과적으로 시간 필터 없이 목록을 조회하면, 근무일정이 전부 삭제된 OPEN 공고가 schedules: [] 로 응답에 포함됩니다.
"삭제된 근무일정을 공고 응답에서 제외" 라는 이 PR 의 목적을 생각하면 이런 공고는 목록에서 빠지는 게 자연스러워 보입니다. leftJoin → join (inner) 으로 바꾸거나 WHERE 에 활성 일정 exists 조건을 추가하는 방향을 검토해 주세요.
의도적으로 남기신 거라면 그대로 두셔도 됩니다. 다만 98번 라인의 getPostingsWithCursor 도 동일 패턴이라 count 와 list 는 서로 일관되게 동작합니다.
| "}" + | ||
| "]") | ||
| @Valid | ||
| @NotEmpty |
There was a problem hiding this comment.
[중간] 실질적으로 파괴적인 검증 변경 — 릴리스 노트에 남겨주세요
이 PR 에서 javax.validation.constraints.NotNull import 를 jakarta.validation.constraints.NotNull 로 교체하셨는데, 이게 생각보다 영향이 큽니다.
Jakarta EE 9+ 검증기는 javax.validation 어노테이션을 인식하지 않습니다. 즉 지금까지 workspaceId 와 paymentType 의 @NotNull 은 아무 일도 하지 않았고, workspaceId: null 로 요청하면 그대로 통과해 NPE 나 엉뚱한 조회로 이어졌을 겁니다. 좋은 수정입니다.
다만 결과적으로 아래 세 가지가 동시에 클라이언트에 400 을 새로 발생시킵니다.
workspaceId누락 (기존엔 통과)paymentType누락 (기존엔 통과)schedules빈 배열 · 누락 (@NotEmpty신규)
PR 설명의 "필수값 검증 보강" 만으로는 앱 팀이 영향 범위를 알기 어렵습니다. 어떤 필드가 새로 400 을 내는지 목록으로 적어두시면 좋겠습니다.
| @NotEmpty | ||
| @Schema(description = "근무일", example = "[\"MONDAY\", \"WEDNESDAY\"]") | ||
| private List<String> workingDays; | ||
| private List<DayOfWeek> workingDays; | ||
|
|
||
| @NotNull | ||
| @Schema(description = "시작 시간", example = "09:00") | ||
| private String startTime; | ||
| private LocalTime startTime; |
There was a problem hiding this comment.
타입 정리 좋습니다 — 응답 코드 변화만 확인 부탁드립니다
List<String> → List<DayOfWeek>, String → LocalTime 전환으로 Posting.updateSchedules 안에 있던 DayOfWeek.valueOf · LocalTime.parse 파싱이 사라졌습니다. 도메인이 파싱 책임을 지지 않게 된 점, CreatePostingScheduleRequestDto 와 타입이 통일된 점 모두 좋습니다.
와이어 포맷("09:00", "MONDAY")은 Jackson JavaTimeModule 이 동일하게 처리하므로 정상 요청은 호환됩니다.
다만 잘못된 값이 올 때 응답이 달라집니다. 기존에는 도메인까지 들어와 LocalTime.parse 가 DateTimeParseException 을 던졌고, 이제는 역직렬화 단계에서 HttpMessageNotReadableException 이 납니다. 프로젝트의 GlobalExceptionHandler 가 이걸 어떤 ErrorCode 로 내보내는지 한 번 확인해 주세요.
참고로 @NotNull → @Positive (positionsNeeded) 도 함께 고치셨는데, primitive int 에 @NotNull 은 항상 통과하므로 이것도 그동안 무의미했던 검증입니다. 좋은 수정입니다.
| workspaceQueryRepository.findByIdAndManagerUser(command.workspaceId(), actor.getManagerUser()) | ||
| .orElseThrow(() -> new CustomException(ErrorCode.WORKSPACE_NOT_FOUND)); |
There was a problem hiding this comment.
권한 검증 추가 — 핵심 수정입니다
기존 findById 는 업장 소유권을 전혀 확인하지 않아서, 매니저가 남의 업장 ID 로 공고를 등록할 수 있었습니다. findByIdAndManagerUser 로 바꾸면서 소유권과 ACTIVATED 상태까지 함께 거른 건 정확한 처리입니다.
WORKSPACE_NOT_FOUND(400 / B008) 를 재사용해 "없음" 과 "권한 없음" 을 구분하지 않은 것도 정보 노출 관점에서 맞는 선택입니다. 스웨거 예시 문구도 그에 맞게 적어두셨네요.
| if (PostingStatus.DELETED.equals(posting.getStatus())) { | ||
| throw new CustomException(ErrorCode.CONFLICT); |
There was a problem hiding this comment.
[낮음] 동일한 가드가 두 UseCase 에 중복됩니다
ManagerUpdatePostingStatus:29-32 에 글자 그대로 같은 블록이 이미 있습니다. 기존 패턴을 따르신 거라 이 PR 에서 이렇게 쓴 건 맞습니다.
다만 findByManagerAndId 를 쓰는 모든 호출부가 같은 가드를 각자 들고 있어야 하는 구조라, 앞으로 이 리포지터리 메서드를 쓰는 UseCase 가 하나 더 생기면 빠뜨리기 쉽습니다. Posting 에 validateModifiable() 같은 도메인 메서드로 접거나, 리포지터리에서 DELETED 를 아예 제외하는 방향을 다음 기회에 검토해 보시면 좋겠습니다.
ErrorCode.CONFLICT 가 409 / B020 이고 스웨거에도 409 블록으로 정확히 추가하셨습니다.
| toCommands(createSchedules, CreatePostingScheduleRequestDto::toCommand), | ||
| toCommands(updateSchedules, UpdatePostingScheduleDto::toCommand), | ||
| deleteScheduleIds | ||
| ); | ||
| } | ||
|
|
||
| private static <T, R> List<R> toCommands(List<T> source, Function<T, R> mapper) { |
There was a problem hiding this comment.
[낮음] deleteScheduleIds 만 정규화에서 빠졌습니다
createSchedules · updateSchedules 는 toCommands 로 null → List.of() 정규화가 되는데, deleteScheduleIds 는 raw 로 넘어가 Command 안에서 null 일 수 있습니다.
updateContent 가 ObjectUtils.isNotEmpty 로 방어하고 있어 지금 당장 깨지지는 않습니다. 다만 Command 를 받는 쪽에서 "세 리스트 중 두 개만 non-null 보장" 이라는 규칙을 알아야 하는 건 불필요한 부담입니다.
ObjectUtils.isEmpty(deleteScheduleIds) ? List.of() : deleteScheduleIds한 줄이면 셋 다 동일해집니다.
| posting | ||
| )) | ||
| .toList(); | ||
| posting.schedules = new ArrayList<>(); |
There was a problem hiding this comment.
잠재 버그 하나가 같이 사라졌습니다
기존 코드는 posting.schedules 를 stream().toList() 결과로 채웠는데, 이건 불변 리스트라서 그 인스턴스에 addSchedules 를 호출하면 UnsupportedOperationException 이 났을 겁니다. new ArrayList<>() 로 바꾸면서 해소됐네요.
덧붙여 ObjectUtils.isNotEmpty(command.schedules()) 조건은 이제 없어도 됩니다. CreatePostingRequestDto.schedules 에 @NotEmpty 가 붙어 컨트롤러 단에서 걸러지고, 비어 있어도 addSchedules 의 for 문이 그냥 0회 도니까요. 취향 문제라 그대로 두셔도 무방합니다.
| /** | ||
| * 삭제되지 않은 근무일정만 반환한다. (응답 노출 기준) | ||
| */ | ||
| public List<PostingSchedule> getActiveSchedules() { | ||
| if (ObjectUtils.isEmpty(this.schedules)) { | ||
| return List.of(); | ||
| } | ||
|
|
||
| return this.schedules.stream() | ||
| .filter(schedule -> !PostingStatus.DELETED.equals(schedule.getStatus())) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
[낮음] CLAUDE.md 의 인자 전달 규칙과 어긋납니다 — 문서 갱신 필요
CLAUDE.md 의 "Controller → UseCase 인자 전달 규칙" 은 @RequestBody DTO 를 값 추출 없이 그대로 넘기라고 규정합니다. 이 PR 은 request.toCommand() 로 바꿨습니다.
방향 자체는 이 PR 이 맞다고 봅니다. 기존 규칙대로면 domain 패키지가 adapter.inbound DTO 를 import 해야 하고, 그건 CLAUDE.md 의 헥사고날 원칙(의존성은 안쪽으로만)을 정면으로 위반합니다. 실제로 이 PR 이 Posting 엔티티에서 그 import 세 줄을 걷어냈습니다.
다만 두 규칙이 충돌한 채로 두면 다음 작업자가 어느 쪽을 따를지 알 수 없습니다. 마무리하면서 아래를 함께 갱신해 주세요.
CLAUDE.md의 인자 전달 규칙에 "도메인으로 넘길 때는domain/*/command의 Command record 로 변환한다" 를 명시- Obsidian
01-아키텍처/에 Command 패턴 문서 추가,02-도메인/공고-도메인.md의 UseCase 시그니처 갱신
getActiveSchedules() 자체는 깔끔합니다. updateSchedules · deleteSchedules 가 루프 안에서 매번 재평가하는 것도, 같은 ID 를 중복으로 보내면 두 번째가 NOT_FOUND 로 걸리는 바람직한 동작이 되네요.
| examples = { | ||
| @ExampleObject( | ||
| name = "존재하지 않거나, 자신이 관리하지 않거나, 활성화되지 않은 업장", | ||
| value = "{\"code\" : \"B008\"}" |
There was a problem hiding this comment.
[사소] 이 400 블록에 C001 이 섞여 있습니다 (기존 문제)
B008 을 400 블록에 넣으신 건 정확합니다. WORKSPACE_NOT_FOUND 가 400 이니까요.
다만 바로 아래 "서버 내부 오류 C001" 예시가 같은 400 블록에 들어 있는데, C001 은 500 입니다. 이 PR 이 만든 문제는 아니지만 바로 옆 줄을 건드리신 김에 500 블록으로 옮기시면 어떨까요.
| qPosting.id.eq(postingId), | ||
| qWorkspace.managerUser.eq(managerUser) | ||
| qWorkspace.managerUser.eq(managerUser), | ||
| qPosting.status.ne(PostingStatus.DELETED) |
There was a problem hiding this comment.
[사소] 매니저 목록 2차 쿼리와 필터링 방식이 다릅니다
상세 조회에 DELETED 제외를 넣으신 건 맞습니다.
다만 전체적으로 보면 삭제 필터링이 세 가지 방식으로 섞여 있습니다.
- 일반 목록: 조인
.on()으로 SQL 에서 제외 (50 · 98 라인) - 매니저 목록: 387 라인 에서 DELETED 일정까지 fetch join 한 뒤
getActiveSchedules()로 애플리케이션에서 제외 - 상세: 여기처럼 WHERE 조건
최종 응답은 어느 경로든 getActiveSchedules() 를 거치므로 결과는 동일합니다. 급한 문제는 아니지만, 나중에 일정 필터 조건이 하나 더 생기면 세 곳을 각각 고쳐야 합니다.
- 포트 시그니처에서 어댑터 DTO를 제거하고 도메인 Command를 받도록 변경 - Posting 엔티티의 adapter.inbound 의존 제거, 요일·시각 파싱을 어댑터 경계로 이동 - 공고 등록 시 매니저 스코프 업장 조회로 소유권 검증 추가 - 삭제된 공고의 내용 수정 차단 - 필수값 검증 보강: workspaceId·paymentType의 jakarta 어노테이션 교정, schedules NotEmpty, 수정 DTO의 description NotBlank, 수정 스케줄의 workingDays NotEmpty·positionsNeeded Positive 소유권 검증과 Command 전환이 동일한 UseCase 시그니처를 건드려 한 커밋으로 묶음.
- 알바생 목록의 근무시간 필터가 삭제된 일정으로 매칭되던 문제 수정 (건수·목록 쿼리 모두 스케줄 조인 시 DELETED 제외) - 삭제된 공고의 매니저 상세 조회 차단
HttpMessageNotReadableException 핸들러가 없어 잘못된 요일·시각 값이 오면 상태는 400이지만 body가 Spring 기본 형식으로 나갔다. 앱의 오류 파싱이 깨진다.
같은 가드가 ManagerUpdatePosting과 ManagerUpdatePostingStatus에 중복돼 있었고, Posting.updateContent 자체는 상태를 검사하지 않아 다른 호출부가 규칙을 우회할 수 있었다. validateModifiable()로 엔티티가 불변식을 지키게 한다. UseCase 테스트의 삭제 공고 케이스는 mock으로는 검증할 게 없어져 PostingTests의 실제 엔티티 검증으로 대체한다.
일정을 전부 삭제해도 공고가 OPEN으로 남아 목록에 schedules 빈 배열로 노출됐다. 모집할 일정이 없으면 마감된 공고이므로 CLOSED로 넘긴다. 목록 쿼리가 모두 status=OPEN을 걸고 있어 자연히 빠지고, 지원도 함께 닫힌다. 판정은 삭제·수정·추가를 모두 마친 뒤에 한다. 전부 지우고 새로 추가하는 요청을 마감으로 잘못 처리하지 않기 위해서다.
createSchedules·updateSchedules만 null을 빈 목록으로 바꿔 넘기고 deleteScheduleIds는 그대로 넘어갔다. Command를 받는 쪽이 세 목록 중 둘만 non-null이라는 규칙을 알아야 했다.
존재하지 않는 공고(B007), 모집 종료·중복 지원(B001), 잠금 실패(429/E001)가 스펙에 빠져 있었다. B001 두 건은 코드가 같아 예시에 message를 함께 적는다.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java`:
- Around line 50-63: Apply element-level validation to the three DTOs: in
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java
lines 50-63, declare schedules as List elements with `@NotNull` and `@Valid`; in
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.java
lines 46-70, apply `@Valid` to elements of createSchedules and updateSchedules and
`@NotNull` to elements of deleteScheduleIds; in
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.java
lines 26-48, declare workingDays as elements with `@NotNull`.
In `@src/main/resources/db/migration/V11__unify_chat_schema.sql`:
- Around line 6-7: Ensure the chat_rooms schema enforces that GROUP rooms always
have a non-null workspace_id, preventing type = 'GROUP' rows from bypassing
workspace-based queries and uniqueness protection. If V11__unify_chat_schema.sql
has already been deployed, add this constraint in a new migration rather than
modifying V11; otherwise update the V11 migration accordingly.
In `@src/main/resources/db/migration/V14__unique_workspace_group_chat_room.sql`:
- Around line 4-6: Flyway 설정에서 V14__unique_workspace_group_chat_room 마이그레이션을
트랜잭션 없이 실행하도록 구성하세요. 기존 uq_chat_group_room_per_workspace 인덱스 정의는 유지하고, 해당
마이그레이션에 대한 executeInTransaction 비활성화 설정이 적용되는지 확인하세요.
In
`@src/test/java/com/dreamteam/alter/application/posting/usecase/CreatePostingTests.java`:
- Around line 49-92: Group the two CreatePosting.execute tests inside an `@Nested`
class annotated with `@DisplayName`("execute"), preserving their existing
assertions and setup. In the exception test execute_타매니저업장_예외발생, separate the
invocation under a // when section from the assertions under a // then section.
In
`@src/test/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandlerTests.java`:
- Around line 37-39: Update both tests in GlobalExceptionHandlerTests to assert
the response message in addition to the existing code assertion, verifying
$.message equals "요청 본문을 해석할 수 없습니다." for each GlobalExceptionHandler response.
- Around line 18-52: GlobalExceptionHandlerTests의 관련 테스트를 `@Nested` 클래스(적절한
`@DisplayName` 포함)로 그룹화하고, 테스트 메서드명을 action_condition_expectedResult 형식으로 변경하십시오.
각 테스트에 Given-When-Then 주석을 명시적으로 추가하되, 기존 요청 본문과 검증 결과는 그대로 유지하십시오.
🪄 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.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f6d4d3d7-39bf-4f2e-989e-d75149c1ca00
📒 Files selected for processing (35)
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/controller/PostingControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingScheduleRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/controller/ManagerPostingController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/controller/ManagerPostingControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/ManagerPostingListResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/PostingQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/PostingScheduleQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/ManagerPostingDetailResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/ManagerPostingListResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/PostingDetailResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/PostingListResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/workspace/persistence/WorkspaceQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/CreatePosting.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePosting.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingStatus.javasrc/main/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandler.javasrc/main/java/com/dreamteam/alter/domain/posting/command/CreatePostingCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/PostingScheduleCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/UpdatePostingCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/UpdatePostingScheduleCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/entity/Posting.javasrc/main/java/com/dreamteam/alter/domain/posting/port/inbound/CreatePostingUseCase.javasrc/main/java/com/dreamteam/alter/domain/posting/port/inbound/ManagerUpdatePostingUseCase.javasrc/main/java/com/dreamteam/alter/domain/workspace/port/outbound/WorkspaceQueryRepository.javasrc/main/resources/db/migration/V11__unify_chat_schema.sqlsrc/main/resources/db/migration/V12__chat_message_content_nullable.sqlsrc/main/resources/db/migration/V13__backfill_workspace_group_chat.sqlsrc/main/resources/db/migration/V14__unique_workspace_group_chat_room.sqlsrc/test/java/com/dreamteam/alter/application/posting/usecase/CreatePostingTests.javasrc/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.javasrc/test/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandlerTests.javasrc/test/java/com/dreamteam/alter/domain/posting/entity/PostingTests.java
💤 Files with no reviewable changes (1)
- src/main/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingStatus.java
| @Valid | ||
| @NotEmpty | ||
| private List<CreatePostingScheduleRequestDto> schedules; | ||
|
|
||
| public CreatePostingCommand toCommand() { | ||
| return new CreatePostingCommand( | ||
| workspaceId, | ||
| title, | ||
| description, | ||
| payAmount, | ||
| paymentType, | ||
| schedules.stream().map(CreatePostingScheduleRequestDto::toCommand).toList() | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg 'CreatePostingRequestDto|UpdatePostingRequestDto|UpdatePostingScheduleDto' || true
echo
echo "== annotations/imports and relevant snippets =="
for f in \
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java \
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.java \
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.java
do
echo "--- $f"
wc -l "$f"
sed -n '1,120p' "$f" | cat -n
done
echo
echo "== Search for constraint imports in relevant files =="
rg -n "import .*constraint|`@NotNull`|`@NotEmpty`|`@Valid`|List<@|List<[^>]*DayOfWeek|List<[^>]*Long" src/main/java/com/dreamteam/alter/adapter/inbound -g '*.java' || true
echo
echo "== Build files and Jakarta/Hibernate validator versions =="
git ls-files | rg '^(build.gradle|build.gradle.kts|settings.gradle|pom.xml|gradle.properties)$' || true
rg -n "hibernate-validator|jakarta.validation|validation-api|spring-boot-starter-validation" -S . --glob '!build/*' --glob '!**/target/**' --glob '!**/.gradle/**' || trueRepository: alter-app/alter-backend
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in [
Path("src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java"),
Path("src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.java"),
Path("src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.java"),
]:
text = p.read_text()
print(f"== {p} ==")
print({
"contains_List_at_field_level": any("List@" in line or "List<" in line for line in text.splitlines()),
"annotations_lines_near_collections": [line.strip() for line in text.splitlines() if any(a in line for a in ["`@List`", "List<@", "`@NonNull`", "NotNull", "NotEmpty", "Valid"])]
})
PYRepository: alter-app/alter-backend
Length of output: 1166
🌐 Web query:
Jakarta Bean Validation element cascading @Valid applied to collection element List annotation type parameter
💡 Result:
In Jakarta Bean Validation (formerly Bean Validation), you can achieve cascading validation for elements within a collection (such as a List) using the @Valid annotation on the generic type argument [1][2]. To validate the elements of a List, place the @Valid annotation directly on the type argument of the List [1][3]. This instructs the validation engine to cascade validation to each element within the collection when the parent object is validated [1][2]. Example usage: private List<@Valid Employee> employees; When the object containing this list is validated, the validator uses a ValueExtractor (in-built for standard types like List, Iterable, and Map) to extract each element and apply cascading validation to the Employee objects [1]. Key concepts: - Cascading via Type Argument: By placing @Valid inside the angle brackets (e.g., List<@Valid T>), you specifically target the elements for validation rather than the collection itself [4]. - Recursion: Cascaded validation is applied recursively; if the elements (e.g., Employee) contain fields marked with @Valid, those will also be validated [5][2]. - Performance: It is more efficient to specify the validation intent explicitly on the type argument than to place @Valid on the collection container itself, which forces the validator to perform unnecessary metadata lookups to determine if the container type is a collection [4]. - Compatibility: This feature requires Jakarta Bean Validation 2.0 or later (which introduced support for type-use annotations on generic type arguments) [6][3][7]. Standard container types like List and Map have built-in ValueExtractor support in compliant implementations like Hibernate Validator [1][3].
Citations:
- 1: https://jakarta.ee/learn/jakartaee-tutorial/current/beanvalidation/bean-validation-advanced/bean-validation-advanced.html
- 2: https://docs.hibernate.org/stable/validator/reference/en-US/html_single/
- 3: https://www.baeldung.com/bean-validation-container-elements
- 4: Clarify that placing
@Validon the containers to validate container elements is discouraged jakartaee/validation#266 - 5: https://jakarta.ee/specifications/bean-validation/3.0/apidocs/jakarta/validation/valid
- 6: https://jakarta.ee/learn/jakartaee-tutorial/current/beanvalidation/bean-validation/bean-validation.html
- 7: https://stackoverflow.com/questions/20384337/how-to-use-java-bean-validators-jsr-303-jsr-349-on-elements-of-an-array-list-c
컬렉션 원소 제약을 세 DTO에 추가해 주세요.
@NotEmpty와 @Valid는 리스트 자체만 확인하므로 null 원소는 Command로 전달될 수 있습니다.
CreatePostingRequestDto.schedules를List<@NotNullCreatePostingScheduleRequestDto>로 선언해 주세요.UpdatePostingRequestDto.createSchedules,updateSchedules에도@Valid를 원소에 적용해 주세요.deleteScheduleIds에는 원소별@NotNull를 추가해 주세요.UpdatePostingScheduleDto.workingDays를List<@NotNullDayOfWeek>로 선언해 주세요.
📍 Affects 3 files
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java#L50-L63(this comment)src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.java#L46-L70src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.java#L26-L48
🤖 Prompt for 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.
In
`@src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java`
around lines 50 - 63, Apply element-level validation to the three DTOs: in
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java
lines 50-63, declare schedules as List elements with `@NotNull` and `@Valid`; in
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.java
lines 46-70, apply `@Valid` to elements of createSchedules and updateSchedules and
`@NotNull` to elements of deleteScheduleIds; in
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.java
lines 26-48, declare workingDays as elements with `@NotNull`.
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 6
🤖 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/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java`:
- Around line 50-63: Apply element-level validation to the three DTOs: in
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.java
lines 50-63, declare schedules as List elements with `@NotNull` and `@Valid`; in
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.java
lines 46-70, apply `@Valid` to elements of createSchedules and updateSchedules and
`@NotNull` to elements of deleteScheduleIds; in
src/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.java
lines 26-48, declare workingDays as elements with `@NotNull`.
In `@src/main/resources/db/migration/V11__unify_chat_schema.sql`:
- Around line 6-7: Ensure the chat_rooms schema enforces that GROUP rooms always
have a non-null workspace_id, preventing type = 'GROUP' rows from bypassing
workspace-based queries and uniqueness protection. If V11__unify_chat_schema.sql
has already been deployed, add this constraint in a new migration rather than
modifying V11; otherwise update the V11 migration accordingly.
In `@src/main/resources/db/migration/V14__unique_workspace_group_chat_room.sql`:
- Around line 4-6: Flyway 설정에서 V14__unique_workspace_group_chat_room 마이그레이션을
트랜잭션 없이 실행하도록 구성하세요. 기존 uq_chat_group_room_per_workspace 인덱스 정의는 유지하고, 해당
마이그레이션에 대한 executeInTransaction 비활성화 설정이 적용되는지 확인하세요.
In
`@src/test/java/com/dreamteam/alter/application/posting/usecase/CreatePostingTests.java`:
- Around line 49-92: Group the two CreatePosting.execute tests inside an `@Nested`
class annotated with `@DisplayName`("execute"), preserving their existing
assertions and setup. In the exception test execute_타매니저업장_예외발생, separate the
invocation under a // when section from the assertions under a // then section.
In
`@src/test/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandlerTests.java`:
- Around line 37-39: Update both tests in GlobalExceptionHandlerTests to assert
the response message in addition to the existing code assertion, verifying
$.message equals "요청 본문을 해석할 수 없습니다." for each GlobalExceptionHandler response.
- Around line 18-52: GlobalExceptionHandlerTests의 관련 테스트를 `@Nested` 클래스(적절한
`@DisplayName` 포함)로 그룹화하고, 테스트 메서드명을 action_condition_expectedResult 형식으로 변경하십시오.
각 테스트에 Given-When-Then 주석을 명시적으로 추가하되, 기존 요청 본문과 검증 결과는 그대로 유지하십시오.
🪄 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.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f6d4d3d7-39bf-4f2e-989e-d75149c1ca00
📒 Files selected for processing (35)
src/main/java/com/dreamteam/alter/adapter/inbound/general/posting/controller/PostingControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/general/posting/dto/CreatePostingScheduleRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/controller/ManagerPostingController.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/controller/ManagerPostingControllerSpec.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/ManagerPostingListResponseDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingRequestDto.javasrc/main/java/com/dreamteam/alter/adapter/inbound/manager/posting/dto/UpdatePostingScheduleDto.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/PostingQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/PostingScheduleQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/ManagerPostingDetailResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/ManagerPostingListResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/PostingDetailResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/posting/persistence/readonly/PostingListResponse.javasrc/main/java/com/dreamteam/alter/adapter/outbound/workspace/persistence/WorkspaceQueryRepositoryImpl.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/CreatePosting.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePosting.javasrc/main/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingStatus.javasrc/main/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandler.javasrc/main/java/com/dreamteam/alter/domain/posting/command/CreatePostingCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/PostingScheduleCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/UpdatePostingCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/command/UpdatePostingScheduleCommand.javasrc/main/java/com/dreamteam/alter/domain/posting/entity/Posting.javasrc/main/java/com/dreamteam/alter/domain/posting/port/inbound/CreatePostingUseCase.javasrc/main/java/com/dreamteam/alter/domain/posting/port/inbound/ManagerUpdatePostingUseCase.javasrc/main/java/com/dreamteam/alter/domain/workspace/port/outbound/WorkspaceQueryRepository.javasrc/main/resources/db/migration/V11__unify_chat_schema.sqlsrc/main/resources/db/migration/V12__chat_message_content_nullable.sqlsrc/main/resources/db/migration/V13__backfill_workspace_group_chat.sqlsrc/main/resources/db/migration/V14__unique_workspace_group_chat_room.sqlsrc/test/java/com/dreamteam/alter/application/posting/usecase/CreatePostingTests.javasrc/test/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingTests.javasrc/test/java/com/dreamteam/alter/common/exception/handler/GlobalExceptionHandlerTests.javasrc/test/java/com/dreamteam/alter/domain/posting/entity/PostingTests.java
💤 Files with no reviewable changes (1)
- src/main/java/com/dreamteam/alter/application/posting/usecase/ManagerUpdatePostingStatus.java
🛑 Comments failed to post (2)
src/main/resources/db/migration/V11__unify_chat_schema.sql (1)
6-7: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
GROUP 방의
workspace_id불변식을 DB에서도 강제하세요.도메인
ChatRoom.createGroup는workspaceId를 필수로 요구합니다. 그러나 현재 스키마는type = 'GROUP'및workspace_id IS NULL행을 허용합니다. 이 행은 업장 기반 방 조회와 유니크 인덱스의 보호 범위에서 벗어납니다.수정 예시
ALTER TABLE chat_rooms ADD COLUMN type varchar(20) NOT NULL DEFAULT 'DIRECT'; ALTER TABLE chat_rooms ADD COLUMN workspace_id bigint NULL; +ALTER TABLE chat_rooms + ADD CONSTRAINT ck_chat_rooms_group_workspace + CHECK (type <> 'GROUP' OR workspace_id IS NOT NULL);V11이 이미 배포된 환경이면 이 제약은 새 마이그레이션으로 추가하세요.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.ALTER TABLE chat_rooms ADD COLUMN type varchar(20) NOT NULL DEFAULT 'DIRECT'; ALTER TABLE chat_rooms ADD COLUMN workspace_id bigint NULL; ALTER TABLE chat_rooms ADD CONSTRAINT ck_chat_rooms_group_workspace CHECK (type <> 'GROUP' OR workspace_id IS NOT NULL);🧰 Tools
🪛 Squawk (2.61.0)
[warning] 6-6: Changing the size of a
varcharfield requires anACCESS EXCLUSIVElock, that will prevent all reads and writes to the table. Use aTEXTfield with aCHECKconstraint.(prefer-text-field)
🤖 Prompt for 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. In `@src/main/resources/db/migration/V11__unify_chat_schema.sql` around lines 6 - 7, Ensure the chat_rooms schema enforces that GROUP rooms always have a non-null workspace_id, preventing type = 'GROUP' rows from bypassing workspace-based queries and uniqueness protection. If V11__unify_chat_schema.sql has already been deployed, add this constraint in a new migration rather than modifying V11; otherwise update the V11 migration accordingly.src/main/resources/db/migration/V14__unique_workspace_group_chat_room.sql (1)
4-6: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Flyway의 마이그레이션 트랜잭션 설정을 확인한다. fd -HI -t f 'application.*' . -x rg -n -C 2 'flyway|executeInTransaction|spring\.flyway' {} \; || true # 기존 마이그레이션의 인덱스 생성 방식과 대상 파일을 확인한다. rg -n -C 2 'CREATE( UNIQUE)? INDEX|CONCURRENTLY' src/main/resources/db/migrationRepository: alter-app/alter-backend
Length of output: 2783
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "## application files containing Flyway" fd -HI -t f 'application.*' . -print echo "## Flyway and db settings" rg -n -C 3 'flyway|spring\.database|spring\.jdbc|executeInTransaction|ddl-auto|hibernate' -g 'application*.y*ml' -g 'application*.properties' . || true echo "## V14 target file" cat -n src/main/resources/db/migration/V14__unique_workspace_group_chat_room.sqlRepository: alter-app/alter-backend
Length of output: 361
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "## application files containing Flyway" fd -HI -t f 'application.*' . || true echo "## Flyway and db settings" rg -n -C 3 'flyway|spring\.database|spring\.jdbc|executeInTransaction|ddl-auto|hibernate' $(fd -HI -t f 'application.*' .) || true echo "## V14 target file" cat -n src/main/resources/db/migration/V14__unique_workspace_group_chat_room.sqlRepository: alter-app/alter-backend
Length of output: 8450
🌐 Web query:
PostgreSQL CREATE INDEX CONCURRENTLY requirements CREATE INDEX CONCURRENTLY cannot run inside a transaction block💡 Result:
In PostgreSQL, the CREATE INDEX CONCURRENTLY command cannot be executed within a transaction block (i.e., inside a BEGIN... COMMIT block) because it is not a single atomic transaction [1][2]. The requirement that it must run outside of a transaction block exists because the operation itself involves multiple internal steps that must be committed independently to make the index visible to other concurrent transactions before the index build is complete [3][1]. If it were wrapped in a user-defined transaction block, the system would be unable to properly manage these intermediate states and visibility requirements [1]. Key requirements and characteristics of CREATE INDEX CONCURRENTLY include: 1. Outside a Transaction: You must execute the command in its own session or outside of any explicit BEGIN/COMMIT block [1][2]. 2. Longer Duration: The process performs two scans of the table and must wait for all existing transactions that could potentially modify or use the index to terminate [4][5]. Consequently, it takes significantly longer and requires more total work than a standard index build [4][6]. 3. Non-Blocking: It allows normal DML operations (INSERT, UPDATE, DELETE) to continue on the table because it does not take locks that prevent these operations [4][3]. It primarily holds a ShareUpdateExclusiveLock for most of the operation, which only blocks concurrent schema changes and VACUUM FULL [7]. 4. Failure Handling: Because it is not a simple transactional DDL, if the command fails, the index may be left in an INVALID state [4][7]. You must manually drop such invalid indexes using DROP INDEX before attempting to recreate them [7]. 5. Limited Concurrency: While multiple standard index builds can occur simultaneously, only one concurrent index build is allowed on a specific table at a time [5][2]. For detailed information on the internal phases of the build and locking requirements, refer to the official PostgreSQL documentation for the CREATE INDEX command [4][5].
Citations:
- 1: https://www.cybertec-postgresql.com/en/transactional-ddls/
- 2: https://www.postgresql.org/docs/17/sql-createindex.html
- 3: https://www.enterprisedb.com/blog/explaining-create-index-concurrently
- 4: https://www.postgresql.org/docs/current/sql-createindex.html
- 5: https://www.postgresql.org/docs/19/sql-createindex.html
- 6: https://www.postgresql.org/docs/18/sql-createindex.html
- 7: https://alexstoica.com/blog/create-index-concurrently-locks
운영 배포에서 이 마이그레이션을 트랜잭션 밖으로 실행하세요.
PostgreSQL의
CREATE UNIQUE INDEX CONCURRENTLY는 CREATE INDEX CONCURRENTLY와 다르게 DML을 대기시킬 수 있습니다. 이 마이그레이션을 트랜잭션 밖으로 실행하도록 Flyway 설정을 확인하세요.🧰 Tools
🪛 Squawk (2.61.0)
[warning] 4-6: During normal index creation, table updates are blocked, but reads are still allowed. Use
concurrentlyto avoid blocking writes.(require-concurrent-index-creation)
🤖 Prompt for 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. In `@src/main/resources/db/migration/V14__unique_workspace_group_chat_room.sql` around lines 4 - 6, Flyway 설정에서 V14__unique_workspace_group_chat_room 마이그레이션을 트랜잭션 없이 실행하도록 구성하세요. 기존 uq_chat_group_room_per_workspace 인덱스 정의는 유지하고, 해당 마이그레이션에 대한 executeInTransaction 비활성화 설정이 적용되는지 확인하세요.Source: Linters/SAST tools
2 participants
변경사항
https://app.notion.com/p/3ac86553162881b59052c82350965088
리뷰 반영 (2차)
Posting엔티티로 이동하고 UseCase 중복 가드 제거deleteScheduleIds정규화V4·V5·V6가 각각 두 개씩 존재했습니다. #97(업종)이 먼저 병합돼 배포된 뒤 #95(채팅)가 같은 번호를 다시 쓰면서, Flyway가 중복 버전을 만나 기동 단계에서 실패했습니다. #99 배포 실패도 이 충돌을 물려받은 것입니다.채팅 계열 네 개를 뒤 번호로 옮겼습니다 (내용 변경 없이 파일명만).
V4__unify_chat_schema.sqlV11__unify_chat_schema.sqlV5__chat_message_content_nullable.sqlV12__chat_message_content_nullable.sqlV6__backfill_workspace_group_chat.sqlV13__backfill_workspace_group_chat.sqlV7__unique_workspace_group_chat_room.sqlV14__unique_workspace_group_chat_room.sql네 파일은 스키마 생성 → 백필 → 인덱스 순서로 서로 의존하므로 상대 순서를 유지한 채 함께 옮겼습니다. 이미 적용된 업종 계열(V4·V5·V6)은 그대로 뒀습니다.
배포 전 확인 — 채팅 계열을
V4~V7로 이미 적용한 환경이 있으면 "적용된 마이그레이션이 로컬에 없음"으로 검증에 실패합니다. 배포가 계속 실패해 온 정황상 적용된 환경은 없을 것으로 보이나, 병합 전 확인 부탁드립니다.📢 앱 팀 확인 필요 — 클라이언트 영향
1. 새로 400을 반환하는 필드 (공고 등록
POST /manager/postings)javax.validation→jakarta.validation교체로 그동안 동작하지 않던 검증이 살아납니다.workspaceId@NotNull정상 동작paymentType@NotNull정상 동작schedules@NotEmpty신규payAmount@NotNull→@Positiveint에@NotNull은 항상 통과)응답 예시:
{"code": "B001", "message": "잘못된 요청입니다.", "data": [{"field": "workspaceId", "message": "널이어서는 안됩니다"}]}2. 잘못된 요일·시각 값의 응답 본문 형식 (공고 수정
PUT /manager/postings/{id})workingDays·startTime·endTime을 타입 바인딩으로 바꾸면서 역직렬화 단계에서 걸립니다. 기존에는 상태만 400이고 본문이 Spring 기본 형식이라 오류 파싱이 깨졌습니다.{"code": "B001", "message": "요청 본문을 해석할 수 없습니다."}"monday"(소문자),"25:00"등이 해당합니다. 모든 API의 잘못된 JSON 응답이 함께 이 형식으로 통일됩니다.3. 근무일정을 전부 삭제하면 공고가 자동으로 마감됩니다
수정을 마친 뒤 남은 활성 근무일정이 0개면 공고 상태가
CLOSED로 바뀝니다.status = OPEN기준){"code": "B001", "message": "모집이 종료된 공고입니다."}OPEN을 유지합니다PATCH /manager/postings/{id}/status)로 명시적으로OPEN처리해야 합니다자동 마감으로 "모집이 종료된 공고입니다" 오류의 발생 빈도가 늘어납니다. 오류 코드는 기존과 같은
B001입니다.4. 삭제된 공고 변경 시 409
내용 수정·상태 변경 모두
409 B020입니다. 기존 동작과 동일하며 검증 위치만 엔티티로 옮겼습니다.테스트
./gradlew clean build전체 통과.단위 테스트 —
PostingTests(자동 마감 3케이스, 삭제 공고 가드 2케이스),GlobalExceptionHandlerTests(신규, 역직렬화 실패 2케이스),ManagerUpdatePostingTests·CreatePostingTests.e2e — 로컬 서버 기동 후 #98·#99 변경사항 28건을 curl로 검증, 전부 통과.
B008/ 필수값 4종 각각 400"monday"·"25:00"→ 400B001B019CLOSED+ 목록에서 제외, 전부 삭제+추가는OPEN유지B020?status=OPEN하위 호환, 다중 상태 필터 동작, DELETED 항상 제외applicantCountCANCELLED이력만 있으면 허용리뷰 코멘트 중 코드 변경 없이 답변드린 항목
ErrorCode.INTERNAL_SERVER_ERROR가 400으로 선언돼 있어(ErrorCode.java) 현재 스펙이 enum과 일치합니다. enum 상태값 자체는 별도 이슈로 봅니다.CLAUDE.md는.gitignore대상이라 레포에 없고, 규칙은 이미 Command/Result 기준으로 갱신돼 있습니다.@Nested그룹화 — 후속으로 분리합니다.Summary by CodeRabbit
새로운 기능
버그 수정
테스트