Skip to content

fix: [ALT-285] 근무 시간 겹침 검사를 사용자 단위로 확장하고 빠진 경로 보강 - #103

Open
hodoon wants to merge 1 commit into
devfrom
fix/ALT-285
Open

hodoon wants to merge 1 commit into
devfrom
fix/ALT-285

Conversation

@hodoon

@hodoon hodoon commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

요약

QA 2026-08-26 통합 테스트 D-03(시간 겹침 배정 차단 실패)·D-07·E-05 관련 결함을 수정했습니다. 겹침 검사가 업장별 WorkspaceWorker 행 기준이라 두 업장에 소속된 알바생의 타 업장 근무와 겹치는 배정을 막지 못했고, 스케줄 시간 수정·대타 승인 경로에는 검사 자체가 없었습니다. DB 스키마 변경 없음, 신규 ErrorCode 없음. 응답 스키마 변경 없음(기존 400/409가 새로 나는 케이스만 추가).

변경 내용

P2 — 타 업장 근무와의 겹침 미감지

  • WorkspaceShiftQueryRepositoryImpl.hasConflictingSchedule: assignedWorkspaceWorker = 해당 행 → assignedWorkspaceWorker.user = 해당 사용자. 같은 사람이 다른 업장(퇴사한 업장의 잔여 CONFIRMED 근무 포함)에 배정된 근무도 겹침으로 봅니다. 호출부 4곳(ManagerAssignWorkerToSchedule, ManagerUpdateWorkerInSchedule, CreateSubstituteRequest, AcceptSubstituteRequest)이 한 번에 적용됩니다.
  • excludeShiftId를 받는 오버로드를 추가했습니다(자기 자신 제외용).
  • 교환 후보 조회 3곳(getExchangeableWorkerIds/Count/ListWithCursor)의 중복 EXISTS 서브쿼리를 hasConflictingShiftForUser 하나로 합치고 같은 기준(사용자 단위)으로 바꿨습니다. 별도 alias(conflictingShift)를 써서 목록 쿼리의 바깥 leftJoin과 alias가 겹치지 않습니다.

P1 — 스케줄 시간 수정 시 재검증 없음

  • ManagerUpdateWorkSchedule: 배정된 근무자가 있으면 바뀐 시간대를 그 근무자의 다른 근무와 비교합니다(자기 자신 제외). 겹치면 기존 배정 경로와 같은 B001 + "해당 근무자가 이미 같은 시간대에 배정된 스케줄이 있습니다.". 미배정 스케줄은 검사하지 않습니다.

P1 — 고정 근무 vs 일반 근무 (정책: 생성 시 건너뛰기)

  • 등록 시점 차단이 아니라 다음 달 자동 생성 시 겹치는 회차를 건너뛰는 기존 동작(GenerateNextMonthWorkspaceShiftTx.hasConflict)을 유지하고, 기존 근무 조회를 findConfirmedByUserIdsAndDateRange로 바꿔 스킵 판정도 사용자 단위로 했습니다.
  • 한 배치 안에서 앞 업장에 생성한 근무가 뒤 업장 판정에 반영되지 않던 구멍을 막았습니다 — Tx가 생성분을 existingShiftsByUserId에 즉시 추가합니다(맵 가변 필수, Map.of() → HashMap).

리뷰에서 추가로 잡은 것

  • ManagerApproveSubstituteRequest: 수락(AcceptSubstituteRequest)만 검사하고 실제 assignWorker가 일어나는 승인엔 검사가 없었습니다. 승인 직전 재검증을 추가해 겹치면 CONFLICT. 수락~승인 사이 다른 배정, 겹치는 ALL 요청 2건 동시 수락 케이스를 막습니다.
  • ManagerUpdateWorkerInSchedule: 자기 스케줄을 제외해, 퇴사 행에 남은 근무를 같은 사용자의 재입사 행으로 교체하는 경우가 막히지 않게 했습니다.

프론트 영향

  • PUT /manager/schedules/{shiftId} (시간 수정): 배정된 근무자의 다른 근무와 겹치면 400 B001 이 새로 발생합니다.
  • 대타 승인 API: 수락자가 승인 시점에 다른 근무와 겹치면 409 CONFLICT 가 새로 발생합니다.
  • 근무자 배정·교체·대타 대상 지정·수락·교환 후보 목록: 타 업장 근무와 겹치는 경우도 차단/제외됩니다. 응답 형태 변경 없음.

테스트

  • WorkspaceShiftQueryRepositoryImplTests(신규, @DataJpaTest) — 타 업장·RESIGNED 잔여 근무 겹침, 다른 사용자 무관, excludeShiftId, 인접 구간(end == start) 비겹침, 비CONFIRMED 무시, 사용자 단위 조회
  • WorkspaceQueryRepositoryImplExchangeableWorkerTests(신규, @DataJpaTest) — 교환 후보 3쿼리 모두 타 업장 겹침 워커 제외
  • ManagerUpdateWorkScheduleTests, ManagerApproveSubstituteRequestTests(신규) — 겹침 시 예외·미변경, 비겹침 시 정상
  • GenerateNextMonthWorkspaceShiftTxTest — 같은 배치 내 업장 간 겹침 스킵 추가
  • 전체 ./gradlew test 431건 통과 (skip 2건은 기존 STOMP Redis 통합 테스트)

남긴 것

겹침 검사가 업장별 WorkspaceWorker 행만 봐서 두 업장에 소속된 알바생의
타 업장 근무와 겹치는 배정을 막지 못했다(QA D-03). 스케줄 시간 수정과
대타 승인 시점에는 검사 자체가 없었고, 고정 근무 월 생성은 같은 배치 안의
다른 업장과 겹칠 수 있었다.

- hasConflictingSchedule: assignedWorkspaceWorker.user 기준(타 업장·RESIGNED
  잔여 CONFIRMED 포함), excludeShiftId 오버로드 추가
- ManagerUpdateWorkSchedule: 배정자 있으면 자기 제외 재검증(B001)
- ManagerApproveSubstituteRequest: 실제 assignWorker 시점 재검증(CONFLICT)
- ManagerUpdateWorkerInSchedule: 자기 스케줄 제외(재입사 행 교체 허용)
- 교환 후보 조회 3곳: EXISTS 서브쿼리를 hasConflictingShiftForUser로 통합
- GenerateNextMonthWorkspaceShift{,Tx}: 기존 근무를 사용자 단위로 조회하고
  생성분을 맵에 즉시 반영해 배치 내 업장 간 겹침 차단

스키마 변경 없음, 신규 ErrorCode 없음.
@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository: alter-app/alter-backend/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 8062a02c-b7e3-4fab-bbb9-faacb276372c


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

shift.assignWorker(workspaceWorker);
shiftsToCreate.add(shift);
// 같은 배치에서 뒤에 처리되는 다른 업장의 고정 근무가 이 근무와 겹치지 않도록 즉시 반영한다
existingShifts.add(shift);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이 Tx는 REQUIRES_NEW라서 따로 롤백될 수 있는데, 공유 맵에는 saveAll 전에 근무를 먼저 넣습니다. 롤백돼도 맵에서 빠지지 않습니다.

예: 한 알바가 업장 A와 B에 같은 월요일 9–18시 고정 근무가 있습니다. A의 saveAll이 실패해 롤백되고, 호출하는 쪽은 예외를 잡고 다음 업장으로 넘어갑니다(execute_continuesProcessing_whenSomeWorkspacesFail이 다루는 경로). 그런데 맵에는 저장되지 않은 A의 근무가 남아 있어서 B의 월요일도 모두 건너뜁니다. 결국 그 알바는 다음 달 두 업장 어디에도 근무가 생기지 않습니다.

제안: Tx는 만든 근무 목록만 돌려주고, 호출하는 쪽이 호출이 성공한 뒤에만 맵에 합치게 하면 트랜잭션 안에서 공유 상태를 바꾸지 않아도 됩니다.

.findConfirmedByWorkerIdsAndDateRange(workerIds, from, to)
.findConfirmedByUserIdsAndDateRange(userIds, from, to)
.stream()
.collect(Collectors.groupingBy(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

이제 Tx가 이 맵 안의 리스트에 근무를 추가합니다. 그런데 Collectors.groupingBy(classifier)는 맵 종류나 리스트를 수정할 수 있는지를 문서로 약속하지 않습니다. 지금은 이 가정이 Tx의 Javadoc 한 줄에만 적혀 있습니다.

누가 나중에 여기를 toUnmodifiableList나 List.copyOf로 바꾸면, 이미 근무가 있는 사용자에서 existingShifts.add(shift)가 UnsupportedOperationException을 던지고 모든 업장이 실패합니다. 컴파일러도 잡아 주지 않습니다.

groupingBy(k, HashMap::new, Collectors.toCollection(ArrayList::new))로 명시하거나, Tx 코멘트에 적은 대로 공유 맵을 바꾸지 않는 구조로 바꾸면 해결됩니다.

existingShiftsByUserId.computeIfAbsent(workspaceWorker.getUser().getId(), k -> new ArrayList<>());

for (LocalDate currentStartDate = firstStartDate;
!currentStartDate.isAfter(endDate);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(이 루프 안의 73행 skipped++에 대한 코멘트입니다. diff 범위 밖이라 여기에 답니다.)

ALT-285에서 고른 정책 (a)는 "겹치는 일반 근무를 건너뛰고 기록만 남김"입니다. 하지만 지금은 건너뛸 때 개수만 세고, 배치가 끝날 때 합계 한 줄만 로그에 남습니다. 누구의 어느 업장 며칠 근무를 건너뛰었는지는 어디에도 남지 않습니다.

그래서 사장님과 알바는 달력에서 근무가 비어 있어야 알게 되고, QA도 로그로 D-07을 확인할 수 없습니다. 건너뛸 때마다 근무자, 업장, 시작 시각을 로그로 남기는 정도는 필요해 보입니다.


// 수락 이후 승인 전에 다른 근무가 배정됐을 수 있으므로 실제 배정 시점에 겹침을 재검증한다 (교환 대상 근무 자신은 제외)
WorkspaceShift shift = substituteRequest.getWorkspaceShift();
if (workspaceShiftQueryRepository.hasConflictingSchedule(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

검사한 다음 저장하는 구조인데, 수락한 근무자의 일정에 잠금이 없어서 동시에 들어오는 요청은 막지 못합니다.

예: 알바 C가 서로 겹치는 대타 요청 두 건(업장 A의 근무 X, 업장 B의 근무 Y)을 수락했습니다. 두 사장님이 같은 순간에 승인하면, 두 트랜잭션 모두 상대의 assignWorker가 아직 커밋되지 않았으니 "확정된 겹침 없음"으로 읽습니다. 둘 다 커밋되고, C는 겹치는 근무 두 개에 확정됩니다.

근무자 배정(ManagerAssignWorkerToSchedule), 근무 수정(ManagerUpdateWorkSchedule), 월별 생성 배치에도 같은 경합이 있습니다. 지금 막히는 것은 차례로 승인하는 경우뿐이라, PR 본문의 "동시 수락 케이스를 막습니다"와 다릅니다. 이번 PR에서 잠금까지 넣을지, 본문 설명을 고치고 따로 다룰지 정해 주세요.


// 배정된 근무자가 있으면 바뀐 시간대가 그 근무자의 다른 근무와 겹치는지 재검증한다 (자기 자신은 제외)
if (ObjectUtils.isNotEmpty(workspaceShift.getAssignedWorkspaceWorker())
&& workspaceShiftQueryRepository.hasConflictingSchedule(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

시작이 종료보다 늦거나 같으면(start >= end) 겹침 조건(existing.start < end AND existing.end > start)이 절대 참이 될 수 없어서, 이 검사를 그냥 통과합니다.

예: 근무자가 10–17시에 다른 확정 근무가 있는데, startDateTime=2026-10-05T18:00, endDateTime=2026-10-05T09:00으로 PUT /manager/schedules/{id}를 보내면 hasConflictingSchedule이 false를 돌려주고 뒤집힌 근무가 저장됩니다.

UpdateWorkScheduleRequestDto에는 NotNull만 있고, ALT-285 D-07 케이스가 "역전·겹침"입니다. PR에서는 역전 검사를 뒤로 미뤘지만, 새로 넣은 겹침 검사를 우회하는 길이 되므로 start < end 검사는 이번에 같이 넣는 게 좋겠습니다.

// 시간 겹침 확인: 새로운 스케줄이 기존 스케줄과 겹치는지 확인
workspaceShift.startDateTime.lt(endDateTime),
workspaceShift.endDateTime.gt(startDateTime),
excludeShiftId != null ? workspaceShift.id.ne(excludeShiftId) : null

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CLAUDE.md 규칙("Null checks: Prefer org.apache.commons.lang3 utilities")에 따라 ObjectUtils.isNotEmpty(excludeShiftId)로 맞춰 주세요. 같은 PR의 ManagerUpdateWorkSchedule은 이미 ObjectUtils.isNotEmpty를 씁니다.

assertThat(page).extracting(UserWorkspaceWorkerListResponse::getId).containsExactly(freeWorker.getId());
}

private User saveUser() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

saveUser, saveWorkspace, saveWorker, saveConfirmedShift와 Import 목록이 WorkspaceShiftQueryRepositoryImplTests에 한 줄씩 그대로 복사돼 있습니다. Workspace.create나 User.create 시그니처가 바뀌면 두 곳을 같이 고쳐야 하고, 한쪽만 고치면 서로 달라집니다.

공용 테스트 준비 클래스로 빼거나, 한 테스트 클래스 안에서 Nested로 묶는 방법을 제안합니다.


when(substituteRequestQueryRepository.findById(REQUEST_ID)).thenReturn(Optional.of(request));
when(workspaceWorkerQueryRepository.findById(ACCEPTED_WORKER_ID)).thenReturn(Optional.of(acceptedWorker));
when(workspaceShiftQueryRepository.hasConflictingSchedule(acceptedWorker, START, END, shift.getId())).thenReturn(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

WorkspaceShift.create로 만든 근무를 저장하지 않아서 shift.getId()가 null이고, 스텁도 hasConflictingSchedule(acceptedWorker, START, END, null)로 맞춰집니다. 실제 코드가 제외할 id로 null이나 엉뚱한 값을 넘겨도 두 테스트 모두 통과하므로, 교환 대상 근무를 제대로 제외하는지 확인하지 못합니다.

근무에 id를 넣고(ReflectionTestUtils.setField 또는 mock) 정확한 id로 스텁·검증해 주세요.


managerUpdateWorkSchedule.execute(actorOf(managerUser), SHIFT_ID, request());

verify(workspaceShiftQueryRepository, never()).hasConflictingSchedule(any(), any(), any(), anyLong());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

anyLong()은 null과 맞지 않습니다. 실제 코드가 네 번째 인자로 null을 넘기며 호출해도 이 never() 검증은 잡지 못합니다. 호출 자체가 없어야 한다는 뜻이면 any()를 쓰거나 verify(workspaceShiftQueryRepository, never()).hasConflictingSchedule(any(), any(), any(), any())로 바꿔 주세요.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants