From f65a496b0359de3a2c146a1a79339c2ad0914df8 Mon Sep 17 00:00:00 2001 From: Audrine Whrite Date: Thu, 15 Oct 2026 16:03:00 +0300 Subject: [PATCH 1/3] fix: include approved leave in shift updates --- .../dev/whrite/opsgrid/shift/ShiftService.kt | 75 +++++++++++++++---- 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/src/main/kotlin/dev/whrite/opsgrid/shift/ShiftService.kt b/src/main/kotlin/dev/whrite/opsgrid/shift/ShiftService.kt index ac0a4ba..7e11789 100644 --- a/src/main/kotlin/dev/whrite/opsgrid/shift/ShiftService.kt +++ b/src/main/kotlin/dev/whrite/opsgrid/shift/ShiftService.kt @@ -11,45 +11,92 @@ data class CreateShiftCommand(val employeeId: UUID, val startsAt: OffsetDateTime data class UpdateShiftCommand(val startsAt: OffsetDateTime, val endsAt: OffsetDateTime) class ShiftConflictException(message: String) : RuntimeException(message) class EmployeeNotFoundException(message: String) : RuntimeException(message) +class EmployeeInactiveException(message: String) : RuntimeException(message) class ShiftNotFoundException(message: String) : RuntimeException(message) @Service -class ShiftService(private val employees: EmployeeRepository, private val shifts: ShiftRepository, private val leaves: EmployeeLeaveRepository) { +class ShiftService( + private val employees: EmployeeRepository, + private val shifts: ShiftRepository, + private val leaves: EmployeeLeaveRepository +) { @Transactional fun schedule(command: CreateShiftCommand): Shift { validateWindow(command.startsAt, command.endsAt) - val employee = employees.findById(command.employeeId).orElseThrow { EmployeeNotFoundException("Employee "+command.employeeId+" was not found") } - check(employee.active) { "Inactive employees cannot be scheduled" } - if (shifts.hasConflict(employee.id, command.startsAt, command.endsAt) || leaves.hasConflict(employee.id, command.startsAt, command.endsAt)) { - throw ShiftConflictException("Employee "+employee.employeeNumber+" has a scheduling conflict in this time window") + val employee = employees.findById(command.employeeId) + .orElseThrow { EmployeeNotFoundException("Employee ${command.employeeId} was not found") } + if (!employee.active) { + throw EmployeeInactiveException("Inactive employees cannot be scheduled") + } + if (shifts.hasConflict(employee.id, command.startsAt, command.endsAt) || + leaves.hasConflict(employee.id, command.startsAt, command.endsAt) + ) { + throw ShiftConflictException( + "Employee ${employee.employeeNumber} has a scheduling conflict in this time window" + ) } return shifts.save(Shift(UUID.randomUUID(), employee, command.startsAt, command.endsAt)) } + @Transactional fun update(id: UUID, command: UpdateShiftCommand): Shift { validateWindow(command.startsAt, command.endsAt) - val shift = shifts.findById(id).orElseThrow { ShiftNotFoundException("Shift "+id+" was not found") } + val shift = shifts.findById(id) + .orElseThrow { ShiftNotFoundException("Shift $id was not found") } check(shift.status == ShiftStatus.SCHEDULED) { "Cancelled shifts cannot be updated" } - if (shifts.hasConflict(shift.employee.id, command.startsAt, command.endsAt, shift.id)) { - throw ShiftConflictException("Employee "+shift.employee.employeeNumber+" already has a shift overlapping this time window") + + val hasShiftConflict = shifts.hasConflict( + shift.employee.id, + command.startsAt, + command.endsAt, + shift.id + ) + val hasLeaveConflict = leaves.hasConflict( + shift.employee.id, + command.startsAt, + command.endsAt + ) + if (hasShiftConflict || hasLeaveConflict) { + throw ShiftConflictException( + "Employee ${shift.employee.employeeNumber} has a scheduling conflict in this time window" + ) } + shift.startsAt = command.startsAt shift.endsAt = command.endsAt return shifts.save(shift) } + @Transactional fun cancel(id: UUID): Shift { - val shift = shifts.findById(id).orElseThrow { ShiftNotFoundException("Shift "+id+" was not found") } + val shift = shifts.findById(id) + .orElseThrow { ShiftNotFoundException("Shift $id was not found") } shift.status = ShiftStatus.CANCELLED return shifts.save(shift) } + @Transactional(readOnly = true) - fun find(id: UUID): Shift = shifts.findById(id).orElseThrow { ShiftNotFoundException("Shift "+id+" was not found") } + fun find(id: UUID): Shift = shifts.findById(id) + .orElseThrow { ShiftNotFoundException("Shift $id was not found") } + @Transactional(readOnly = true) - fun findEmployeeSchedule(employeeId: UUID, startsAt: OffsetDateTime, endsAt: OffsetDateTime): List { + fun findEmployeeSchedule( + employeeId: UUID, + startsAt: OffsetDateTime, + endsAt: OffsetDateTime + ): List { validateWindow(startsAt, endsAt) - if (!employees.existsById(employeeId)) throw EmployeeNotFoundException("Employee "+employeeId+" was not found") - return shifts.findAllByEmployeeIdAndStartsAtLessThanAndEndsAtGreaterThanOrderByStartsAtAsc(employeeId, endsAt, startsAt) + if (!employees.existsById(employeeId)) { + throw EmployeeNotFoundException("Employee $employeeId was not found") + } + return shifts.findAllByEmployeeIdAndStartsAtLessThanAndEndsAtGreaterThanOrderByStartsAtAsc( + employeeId, + endsAt, + startsAt + ) + } + + private fun validateWindow(startsAt: OffsetDateTime, endsAt: OffsetDateTime) { + require(endsAt.isAfter(startsAt)) { "Shift end time must be after start time" } } - private fun validateWindow(startsAt: OffsetDateTime, endsAt: OffsetDateTime) { require(endsAt.isAfter(startsAt)) { "Shift end time must be after start time" } } } From 1b07b13cd90b74636c30456d5a9b509887d9cfed Mon Sep 17 00:00:00 2001 From: Audrine Whrite Date: Thu, 15 Oct 2026 16:03:25 +0300 Subject: [PATCH 2/3] fix: return conflict response for inactive employees --- .../whrite/opsgrid/api/ApiExceptionHandler.kt | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt b/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt index fdcab2f..07e5eff 100644 --- a/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt +++ b/src/main/kotlin/dev/whrite/opsgrid/api/ApiExceptionHandler.kt @@ -1,35 +1,52 @@ package dev.whrite.opsgrid.api -import dev.whrite.opsgrid.shift.EmployeeNotFoundException -import dev.whrite.opsgrid.shift.ShiftNotFoundException + import dev.whrite.opsgrid.leave.LeaveConflictException import dev.whrite.opsgrid.leave.LeaveNotFoundException +import dev.whrite.opsgrid.shift.EmployeeInactiveException +import dev.whrite.opsgrid.shift.EmployeeNotFoundException import dev.whrite.opsgrid.shift.ShiftConflictException +import dev.whrite.opsgrid.shift.ShiftNotFoundException import org.springframework.http.HttpStatus import org.springframework.web.bind.annotation.ExceptionHandler import org.springframework.web.bind.annotation.ResponseStatus import org.springframework.web.bind.annotation.RestControllerAdvice + data class ApiError(val code: String, val message: String) + @RestControllerAdvice class ApiExceptionHandler { @ExceptionHandler(ShiftConflictException::class) @ResponseStatus(HttpStatus.CONFLICT) - fun conflict(ex: ShiftConflictException) = ApiError("SHIFT_CONFLICT", ex.message ?: "Shift conflict") + fun conflict(ex: ShiftConflictException) = + ApiError("SHIFT_CONFLICT", ex.message ?: "Shift conflict") + + @ExceptionHandler(EmployeeInactiveException::class) + @ResponseStatus(HttpStatus.CONFLICT) + fun inactiveEmployee(ex: EmployeeInactiveException) = + ApiError("EMPLOYEE_INACTIVE", ex.message ?: "Inactive employees cannot be scheduled") + @ExceptionHandler(EmployeeNotFoundException::class) @ResponseStatus(HttpStatus.NOT_FOUND) - fun notFound(ex: EmployeeNotFoundException) = ApiError("EMPLOYEE_NOT_FOUND", ex.message ?: "Employee not found") + fun notFound(ex: EmployeeNotFoundException) = + ApiError("EMPLOYEE_NOT_FOUND", ex.message ?: "Employee not found") + @ExceptionHandler(ShiftNotFoundException::class) @ResponseStatus(HttpStatus.NOT_FOUND) - fun shiftNotFound(ex: ShiftNotFoundException) = ApiError("SHIFT_NOT_FOUND", ex.message ?: "Shift not found") + fun shiftNotFound(ex: ShiftNotFoundException) = + ApiError("SHIFT_NOT_FOUND", ex.message ?: "Shift not found") @ExceptionHandler(LeaveNotFoundException::class) @ResponseStatus(HttpStatus.NOT_FOUND) - fun leaveNotFound(ex: LeaveNotFoundException) = ApiError("LEAVE_NOT_FOUND", ex.message ?: "Leave not found") + fun leaveNotFound(ex: LeaveNotFoundException) = + ApiError("LEAVE_NOT_FOUND", ex.message ?: "Leave not found") @ExceptionHandler(LeaveConflictException::class) @ResponseStatus(HttpStatus.CONFLICT) - fun leaveConflict(ex: LeaveConflictException) = ApiError("LEAVE_CONFLICT", ex.message ?: "Leave conflict") + fun leaveConflict(ex: LeaveConflictException) = + ApiError("LEAVE_CONFLICT", ex.message ?: "Leave conflict") @ExceptionHandler(IllegalArgumentException::class) @ResponseStatus(HttpStatus.BAD_REQUEST) - fun badRequest(ex: IllegalArgumentException) = ApiError("INVALID_REQUEST", ex.message ?: "Invalid request") + fun badRequest(ex: IllegalArgumentException) = + ApiError("INVALID_REQUEST", ex.message ?: "Invalid request") } From 1b21c1868c8eb54eec0f204be03642aeb33205aa Mon Sep 17 00:00:00 2001 From: Audrine Whrite Date: Thu, 15 Oct 2026 16:04:15 +0300 Subject: [PATCH 3/3] test: cover shift scheduling conflict matrix --- .../whrite/opsgrid/shift/ShiftServiceTest.kt | 123 +++++++++++++++--- 1 file changed, 108 insertions(+), 15 deletions(-) diff --git a/src/test/kotlin/dev/whrite/opsgrid/shift/ShiftServiceTest.kt b/src/test/kotlin/dev/whrite/opsgrid/shift/ShiftServiceTest.kt index 9abd13c..17a63ad 100644 --- a/src/test/kotlin/dev/whrite/opsgrid/shift/ShiftServiceTest.kt +++ b/src/test/kotlin/dev/whrite/opsgrid/shift/ShiftServiceTest.kt @@ -1,4 +1,5 @@ package dev.whrite.opsgrid.shift + import dev.whrite.opsgrid.employee.Employee import dev.whrite.opsgrid.employee.EmployeeRepository import dev.whrite.opsgrid.leave.EmployeeLeaveRepository @@ -16,38 +17,130 @@ class ShiftServiceTest { private val leaves = mock() private val service = ShiftService(employees, shifts, leaves) + private val morning = OffsetDateTime.parse("2026-10-06T08:00:00+03:00") + private val afternoon = OffsetDateTime.parse("2026-10-06T16:00:00+03:00") + private val evening = OffsetDateTime.parse("2026-10-06T20:00:00+03:00") + @Test fun rejectsOverlappingScheduledShift() { val employeeId = UUID.randomUUID() - val employee = Employee(employeeId, "EMP-001", "Ada", "Lovelace", "Operations") + val employee = employee(employeeId, "EMP-001") whenever(employees.findById(employeeId)).thenReturn(Optional.of(employee)) whenever(shifts.hasConflict(any(), any(), any())).thenReturn(true) - whenever(leaves.hasConflict(any(), any(), any())).thenReturn(false) + val ex = assertThrows(ShiftConflictException::class.java) { - service.schedule(CreateShiftCommand( - employeeId, - OffsetDateTime.parse("2026-10-06T08:00:00+03:00"), - OffsetDateTime.parse("2026-10-06T16:00:00+03:00") - )) + service.schedule(CreateShiftCommand(employeeId, morning, afternoon)) } + assertEquals("Employee EMP-001 has a scheduling conflict in this time window", ex.message) verify(shifts, never()).save(any()) + verify(leaves, never()).hasConflict(any(), any(), any()) + } + + @Test + fun rejectsShiftThatOverlapsApprovedLeave() { + val employeeId = UUID.randomUUID() + whenever(employees.findById(employeeId)).thenReturn(Optional.of(employee(employeeId, "EMP-002"))) + whenever(shifts.hasConflict(any(), any(), any())).thenReturn(false) + whenever(leaves.hasConflict(any(), any(), any())).thenReturn(true) + + assertThrows(ShiftConflictException::class.java) { + service.schedule(CreateShiftCommand(employeeId, morning, afternoon)) + } + + verify(shifts, never()).save(any()) + } + + @Test + fun rejectsUpdateThatWouldOverlapApprovedLeave() { + val employeeId = UUID.randomUUID() + val shiftId = UUID.randomUUID() + val employee = employee(employeeId, "EMP-003") + val existing = Shift(shiftId, employee, morning, afternoon) + whenever(shifts.findById(shiftId)).thenReturn(Optional.of(existing)) + whenever(shifts.hasConflict(any(), any(), any(), any())).thenReturn(false) + whenever(leaves.hasConflict(employeeId, morning, evening)).thenReturn(true) + + assertThrows(ShiftConflictException::class.java) { + service.update(shiftId, UpdateShiftCommand(morning, evening)) + } + + verify(shifts, never()).save(any()) + } + + @Test + fun rejectsSchedulingForInactiveEmployee() { + val employeeId = UUID.randomUUID() + whenever(employees.findById(employeeId)).thenReturn( + Optional.of(employee(employeeId, "EMP-004", active = false)) + ) + + assertThrows(EmployeeInactiveException::class.java) { + service.schedule(CreateShiftCommand(employeeId, morning, afternoon)) + } + + verify(shifts, never()).save(any()) + verify(shifts, never()).hasConflict(any(), any(), any()) + verify(leaves, never()).hasConflict(any(), any(), any()) + } + + @Test + fun rejectsInvalidTimeWindowBeforeQueryingRepositories() { + val employeeId = UUID.randomUUID() + + assertThrows(IllegalArgumentException::class.java) { + service.schedule(CreateShiftCommand(employeeId, afternoon, morning)) + } + + verifyNoInteractions(employees, shifts, leaves) + } + + @Test + fun allowsAdjacentNonOverlappingWindow() { + val employeeId = UUID.randomUUID() + whenever(employees.findById(employeeId)).thenReturn(Optional.of(employee(employeeId, "EMP-005"))) + whenever(shifts.hasConflict(employeeId, afternoon, evening)).thenReturn(false) + whenever(leaves.hasConflict(employeeId, afternoon, evening)).thenReturn(false) + whenever(shifts.save(any())).thenAnswer { it.arguments[0] as Shift } + + val shift = service.schedule(CreateShiftCommand(employeeId, afternoon, evening)) + + assertEquals(afternoon, shift.startsAt) + assertEquals(evening, shift.endsAt) + verify(shifts).save(any()) + } + + @Test + fun allowsOverlappingWindowForDifferentEmployee() { + val employeeId = UUID.randomUUID() + whenever(employees.findById(employeeId)).thenReturn(Optional.of(employee(employeeId, "EMP-006"))) + whenever(shifts.hasConflict(employeeId, morning, afternoon)).thenReturn(false) + whenever(leaves.hasConflict(employeeId, morning, afternoon)).thenReturn(false) + whenever(shifts.save(any())).thenAnswer { it.arguments[0] as Shift } + + val shift = service.schedule(CreateShiftCommand(employeeId, morning, afternoon)) + + assertEquals(employeeId, shift.employee.id) + verify(shifts).save(any()) } @Test fun createsShiftWhenThereIsNoConflict() { val employeeId = UUID.randomUUID() - val employee = Employee(employeeId, "EMP-002", "Grace", "Hopper", "Engineering") - whenever(employees.findById(employeeId)).thenReturn(Optional.of(employee)) + whenever(employees.findById(employeeId)).thenReturn(Optional.of(employee(employeeId, "EMP-007"))) whenever(shifts.hasConflict(any(), any(), any())).thenReturn(false) whenever(leaves.hasConflict(any(), any(), any())).thenReturn(false) whenever(shifts.save(any())).thenAnswer { it.arguments[0] as Shift } - val shift = service.schedule(CreateShiftCommand( - employeeId, - OffsetDateTime.parse("2026-10-06T08:00:00+03:00"), - OffsetDateTime.parse("2026-10-06T16:00:00+03:00") - )) + + val shift = service.schedule(CreateShiftCommand(employeeId, morning, afternoon)) + assertEquals(employeeId, shift.employee.id) verify(shifts).save(any()) } -} \ No newline at end of file + + private fun employee( + id: UUID, + employeeNumber: String, + active: Boolean = true + ) = Employee(id, employeeNumber, "Test", "Employee", "Operations", active) +}