618 조기귀가 type 추가 #628
618 조기귀가 type 추가 #628
Conversation
|
Warning Rate limit exceeded@imhanseong has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 3 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Walkthrough출석 변환 로직을 ApplicationType 요청값 기반으로 분기 처리하도록 변경하고, 신청 종류(applicationKind)가 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Controller as EarlyReturnController
participant Service as CreateEarlyReturnService
participant Processor as EarlyReturnApprovalProcessor
participant Attendance as AttendanceService
participant Notifier as NotificationService
Client->>Controller: CreateEarlyReturnRequest(start, applicationType, ...)
Controller->>Service: validate request (format, applicationType) / create Application
Service->>Processor: submit application for approval
Processor->>Attendance: updateAttendanceToEarlyReturn(start, applicationType, attendanceId)
Attendance-->>Processor: updated attendance / periods
Processor->>Notifier: compute device tokens & send notifications
Notifier-->>Client: push notifications (async)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20분 Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/dsm/pick2024/domain/attendance/domain/service/AttendanceService.kt (1)
103-111: 조기귀가 신청 시 1~5교시 처리 누락 버그
updateAttendanceToEarlyReturn의 switch 문이 periods[5]~9만 처리하며, periods[0]~4는else문으로 빠져 조용히 무시됩니다.문제점:
- FormatValidator는 모든 교시를 허용하며 범위 제한이 없음 (line 38-39)
- 사용자가 "1교시" 또는 오전 시간을 지정하면, line 94-95 또는 99에서 계산된 list에 periods[0-4]가 포함됨
- 하지만 switch 문 (line 104-111)은 periods[5-9]만 처리하므로 periods[0-4]는 변경되지 않음
translateEarlyReturn(line 116-120)은 모든 교시를 반환하지만, 실제 출석 기록은 6~10교시만 업데이트되어 신청 기록과 출석 기록이 불일치같은 버그가
updateAttendanceToApplication(line 74-82)에도 존재합니다.해결 방안:
- 조기귀가가 6
10교시로 정책상 제한되면, 요청 검증 단계에서 15교시를 명시적으로 거부- 또는 period1~5 케이스를 switch 문에 추가하여 모든 교시 처리
🤖 Fix all issues with AI agents
In @buildSrc/src/main/kotlin/PluginVersions.kt:
- Around line 4-7: The Kotlin version constants (JVM_VERSION,
SPRING_PLUGIN_VERSION, JPA_PLUGIN_VERSION, KAPT_VERSION) are set to 1.9.24 which
is not compatible with Spring Boot 2.7.16; either pin these constants to the
Kotlin version that Spring Boot 2.7.16 expects (e.g., 1.6.21) or upgrade your
Spring Boot management to 3.x and then set these constants to a Spring-supported
Kotlin release (e.g., 2.3.0); update the four constants accordingly so the build
uses a matching Kotlin/Spring Boot pair.
In
@src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt:
- Around line 30-34: The code in ChangeApplicationStatusService currently throws
ApplicationNotFoundException when an application's applicationKind is not
ApplicationKind.APPLICATION; rename or replace that exception with a clearer one
(e.g., InvalidApplicationKindException) and throw it (or construct it) where the
check is performed; also merge the two separate applications.forEach loops (the
applicationKind validation and the subsequent loop starting at line 35) into a
single loop to perform both validations and any per-application processing in
one pass, referencing the applications.forEach block and the
applicationKind/property checks to locate the logic.
In
@src/main/kotlin/dsm/pick2024/domain/attendance/domain/service/AttendanceService.kt:
- Around line 92-101: The TIME branch in AttendanceService.kt (inside
AttendanceService when building list) uses periods.filter {
it.first.isAfter(startTime) || it.first == startTime }, which conflicts with
translateEarlyReturn/getMatchPeriods that checks startTime < period.second;
change the TIME filtering to match translateEarlyReturn (e.g., include periods
whose end time is after the early-return time, using period.second > startTime
or startTime < it.second) so updateAttendanceToEarlyReturn and
translateEarlyReturn return consistent period sets.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
buildSrc/src/main/kotlin/PluginVersions.ktsrc/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.ktsrc/main/kotlin/dsm/pick2024/domain/attendance/domain/service/AttendanceService.ktsrc/main/kotlin/dsm/pick2024/domain/earlyreturn/presentation/dto/request/CreateEarlyReturnRequest.ktsrc/main/kotlin/dsm/pick2024/domain/earlyreturn/service/CreateEarlyReturnService.ktsrc/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.ktsrc/main/kotlin/dsm/pick2024/global/annotation/common/FormatValidator.kt
🔇 Additional comments (7)
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt (1)
4-4: LGTM!새로운 유효성 검사 로직에 필요한 import가 올바르게 추가되었습니다.
Also applies to: 6-6
src/main/kotlin/dsm/pick2024/global/annotation/common/FormatValidator.kt (1)
31-35: 변경사항 승인
applicationType에 따라PERIOD는 교시 형식,TIME은 시간 형식으로 분기하여 검증하는 로직이 올바르게 구현되었습니다.ApplicationRequest검증 로직과 일관성 있게 작성되었습니다.src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/CreateEarlyReturnService.kt (1)
51-51: 변경사항 승인하드코딩된
ApplicationType.TIME에서request.applicationType으로 변경하여 동적으로 신청 타입을 처리할 수 있게 되었습니다. PR의 목적에 부합하는 올바른 수정입니다.src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.kt (2)
40-40: 변경사항 승인
updateAttendanceToEarlyReturn호출 시applicationType을 전달하도록 올바르게 수정되었습니다.
50-50: 변경사항 승인
translateEarlyReturn메서드를 사용하여applicationType을 기반으로 신청 스토리를 생성하도록 올바르게 수정되었습니다.src/main/kotlin/dsm/pick2024/domain/attendance/domain/service/AttendanceService.kt (1)
116-127: 새 메서드 추가 확인
translateEarlyReturn메서드가 추가되어applicationType에 따라 교시 이름 목록을 반환합니다. 로직은updateAttendanceToEarlyReturn과 유사하게 구성되었습니다.다만, 앞서 지적한 TIME 타입 필터링 불일치 문제(Line 99 vs 132)와 6~10교시 제한 이슈를 함께 해결해야 합니다.
src/main/kotlin/dsm/pick2024/domain/earlyreturn/presentation/dto/request/CreateEarlyReturnRequest.kt (1)
9-10: API 호출 클라이언트 검증 필요
CreateEarlyReturnRequest의 필수 필드applicationType추가로 인한 API 호환성 변경입니다. 서버 구현은 완료되었으나, 이 API를 호출하는 모든 클라이언트가applicationType필드를 제공하도록 업데이트되었는지 확인이 필요합니다.
- 서버 구현: ✓ @Valid 어노테이션으로 필드 필수 여부 강제됨
- 내부 검증: ✓ FormatValidator에서 applicationType 검증 로직 포함됨
- 확인 대상: 외부 API 호출자, API 호출 문서/Swagger 업데이트 여부
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt
Show resolved
Hide resolved
src/main/kotlin/dsm/pick2024/domain/attendance/domain/service/AttendanceService.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/kotlin/dsm/pick2024/global/error/exception/ErrorCode.kt (1)
34-34: HTTP 상태 코드의 의미론적 고려사항
INVALID_APPLICATION_KIND가 404 상태 코드를 사용하고 있습니다. 일반적으로 404는 리소스가 존재하지 않음을 의미하며, 유효성 검사 오류는 보통 400 (Bad Request)을 사용합니다. 그러나 코드베이스의 다른 유효성 검사 오류들(Line 11-13의INVALID_PERIOD,INVALID_TIME,INVALID_SUBJECT)도 404를 사용하고 있어 일관성은 유지되고 있습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
src/main/kotlin/dsm/pick2024/domain/application/exception/InvalidApplicationKindException.ktsrc/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.ktsrc/main/kotlin/dsm/pick2024/global/error/exception/ErrorCode.kt
🔇 Additional comments (3)
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt (2)
4-4: 적절한 import 추가
ApplicationKind와InvalidApplicationKindException을 import하여 신청 종류 유효성 검사를 지원합니다. 변경 사항이 적절합니다.Also applies to: 6-6
30-37: 이전 리뷰 피드백이 성공적으로 반영되었습니다두 가지 개선 사항이 모두 구현되었습니다:
- 더 명확한 예외명인
InvalidApplicationKindException사용- 두 개의
forEach루프를 하나로 병합하여 성능과 가독성 향상유효성 검사 로직이 명확하고 효율적입니다.
src/main/kotlin/dsm/pick2024/domain/application/exception/InvalidApplicationKindException.kt (1)
1-8: 깔끔한 예외 구현
InvalidApplicationKindException이 Kotlin 싱글톤 객체 패턴을 사용하여 올바르게 구현되었습니다.PickException을 확장하고 적절한ErrorCode와 연결되어 있습니다.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.kt:
- Line 52: translateEarlyReturn returns a List<String> but
EarlyReturnApprovalProcessor currently only uses .first() and never sets the
ApplicationStory.end field, causing lost periods for PERIOD application types;
update the code in EarlyReturnApprovalProcessor (where
attendanceService.translateEarlyReturn(application.start,
application.applicationType) is called) to assign ApplicationStory.start =
translated.first() and ApplicationStory.end = translated.last() (or the same
value if only one element) so both start and end are persisted; ensure you
handle empty lists defensively and preserve existing behavior for non-PERIOD
types.
🧹 Nitpick comments (1)
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt (1)
30-37: 검증 로직이 올바르게 구현되었습니다.이전 리뷰 코멘트의 제안사항이 모두 반영되어 두 개의 검증이 하나의 forEach 루프로 통합되었고, 적절한 예외명(
InvalidApplicationKindException)이 사용되고 있습니다. 로직의 순서와 구현이 정확합니다.♻️ 선택적 개선 제안
제안 1: 모든 검증 오류 수집
현재는 첫 번째 유효하지 않은 신청에서 예외가 발생하여 중단됩니다. 모든 유효하지 않은 신청을 수집하여 한 번에 보고하면 사용자 경험이 향상될 수 있습니다. (단, 현재 구조로도 충분히 동작하므로 선택사항입니다)
제안 2: 스타일 개선
Line 37의 빈 줄을 제거하면 코드가 더 간결해집니다.
applications.forEach { if (it.applicationKind != ApplicationKind.APPLICATION) { throw InvalidApplicationKindException } if (it.status != Status.QUIET) { throw AlreadyApplyingForPicnicException } - }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.ktsrc/main/kotlin/dsm/pick2024/domain/earlyreturn/service/CreateEarlyReturnService.ktsrc/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check Code Quality
🔇 Additional comments (3)
src/main/kotlin/dsm/pick2024/domain/application/service/ChangeApplicationStatusService.kt (1)
4-4: 필요한 import가 정확하게 추가되었습니다.새로운 검증 로직에 필요한
ApplicationKind와InvalidApplicationKindExceptionimport가 적절하게 추가되었습니다.Also applies to: 6-6
src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/CreateEarlyReturnService.kt (1)
48-48: 동적 applicationType 적용이 올바르게 구현되었습니다.하드코딩된
ApplicationType.TIME대신 요청 객체에서applicationType을 동적으로 받아오도록 변경되었습니다.CreateEarlyReturnRequest는@ValidFormat어노테이션을 통해applicationType값에 따라 적절한 유효성 검증을 수행합니다:
PERIOD타입:start필드가\d+교시형식(예: "1교시")으로 검증- 그 외:
start필드가 유효한LocalTime형식으로 검증TIME과 PERIOD 등 다양한 신청 유형을 유연하게 지원하도록 구현되었으며, 각 타입에 맞는 검증 로직이 이미 적절히 구현되어 있습니다.
src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.kt (1)
39-39: 조기귀가 신청 시 applicationType 파라미터 전달 확인됨
updateAttendanceToEarlyReturn메서드가applicationType파라미터를 올바르게 받아 처리합니다. AttendanceService는 TIME과 PERIOD 유형 모두에 대해 적절히 구현되어 있습니다.
- TIME: 시작 시간을 파싱하여 해당 시간 이후의 모든 교시 필터링
- PERIOD: 시작 교시 인덱스부터 마지막 교시까지의 범위 반환
|
|
||
| private fun createApplicationStory(application: Application): ApplicationStory { | ||
| val start = attendanceService.translateApplication(application.start, null, ApplicationType.TIME) | ||
| val start = attendanceService.translateEarlyReturn(application.start, application.applicationType) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find translateEarlyReturn method implementation
ast-grep --pattern $'fun translateEarlyReturn($$$): List<String> {
$$$
}'Repository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 1615
🏁 Script executed:
# Find ApplicationStory entity and start field
rg -n "class ApplicationStory" -A 20 --type kotlinRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 13881
🏁 Script executed:
# Look for the actual usage of start variable and find .first() call
rg -B 5 -A 5 "translateEarlyReturn" --type kotlinRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 2901
🏁 Script executed:
# Check ApplicationStory usage - is the 'end' field being populated elsewhere for PERIOD types?
rg -B 5 -A 10 "ApplicationStory(" src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 153
🏁 Script executed:
# Check if translateEarlyReturn is used anywhere else in the codebase
rg "translateEarlyReturn" --type kotlinRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 450
🏁 Script executed:
# Get the full createApplicationStory method with proper escaping
rg -A 15 "private fun createApplicationStory" src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 547
🏁 Script executed:
# Check if there's any usage of 'end' field in ApplicationStory creation for PERIOD types
rg -B 10 -A 10 "end.*=" src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.ktRepository: DSM-PICK/PiCK_CORE_SERVER
Length of output: 1058
PERIOD 타입의 경우 여러 교시가 반환되지만 첫 번째 값만 저장되고 있습니다 - 데이터 손실 우려
translateEarlyReturn이 반환하는 List<String>에서 .first()로만 값을 취하고 있습니다. ApplicationStory 엔티티는 start와 end 두 필드를 모두 보유하고 있으나, 이 메서드에서는 end 필드를 설정하지 않고 있습니다.
PERIOD 타입의 경우 시작 교시부터 하루 종료 시간까지의 모든 교시가 반환되는데, 현재 구현에서는 첫 번째 교시만 저장되고 나머지는 버려집니다. 이것이 의도된 동작인지, 아니면 end 필드에 마지막 교시를 저장해야 하는지 확인이 필요합니다.
🤖 Prompt for AI Agents
In
@src/main/kotlin/dsm/pick2024/domain/earlyreturn/service/processor/EarlyReturnApprovalProcessor.kt
at line 52, translateEarlyReturn returns a List<String> but
EarlyReturnApprovalProcessor currently only uses .first() and never sets the
ApplicationStory.end field, causing lost periods for PERIOD application types;
update the code in EarlyReturnApprovalProcessor (where
attendanceService.translateEarlyReturn(application.start,
application.applicationType) is called) to assign ApplicationStory.start =
translated.first() and ApplicationStory.end = translated.last() (or the same
value if only one element) so both start and end are persisted; ensure you
handle empty lists defensively and preserve existing behavior for non-PERIOD
types.
There was a problem hiding this comment.
그게 조기귀가는 집 가는거라 end 시간이 필요가 없어서 일부러 저렇게 저장하는거임
There was a problem hiding this comment.
@imhanseong 아, 이해했습니다! 조기귀가는 학생이 학교를 떠나는 시점만 중요하고, 귀가 후에는 돌아오지 않기 때문에 종료 시간이 필요 없다는 점을 고려하면 현재 구현이 맞네요.
비즈니스 로직을 명확히 설명해주셔서 감사합니다. start 필드에 시작 교시만 저장하는 것이 조기귀가의 특성에 맞는 올바른 구현입니다. 👍
Summary by CodeRabbit
새 기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.