[WTH-142] attendance 도메인 코틀린 마이그레이션#9
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughJava 기반의 Attendance 도메인을 Kotlin으로 마이그레이션하면서 Lombok 플러그인을 추가하고, 계층화된 아키텍처(usecase, service, repository 등)를 재구조화했습니다. 기존 Java 클래스들을 삭제하고 Kotlin으로 재구현하여 언어 일관성을 확보합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AttendanceController
participant CheckInAttendanceUseCase
participant UserGetService
participant AttendanceRepository
participant User
participant Attendance
Client->>AttendanceController: PATCH /api/v1/attendances
AttendanceController->>CheckInAttendanceUseCase: checkIn(userId, code)
CheckInAttendanceUseCase->>UserGetService: find(userId)
UserGetService-->>CheckInAttendanceUseCase: User
CheckInAttendanceUseCase->>AttendanceRepository: findCurrentByUserId(userId, now, checkInEnd)
AttendanceRepository-->>CheckInAttendanceUseCase: Attendance (오늘 현재)
CheckInAttendanceUseCase->>Attendance: isWrong(code)
Attendance-->>CheckInAttendanceUseCase: boolean (코드 검증)
alt 코드 불일치
CheckInAttendanceUseCase-->>AttendanceController: AttendanceCodeMismatchException
AttendanceController-->>Client: Error Response
else 코드 일치 및 상태 미변경
CheckInAttendanceUseCase->>Attendance: attend()
CheckInAttendanceUseCase->>User: attend()
AttendanceRepository->>Attendance: save()
AttendanceController-->>Client: ATTENDANCE_CHECKIN_SUCCESS
end
sequenceDiagram
participant Admin
participant AttendanceAdminController
participant CloseAttendanceUseCase
participant MeetingGetService
participant AttendanceRepository
participant Attendance
participant User
Admin->>AttendanceAdminController: PATCH /api/v1/admin/attendances
AttendanceAdminController->>CloseAttendanceUseCase: close(now, cardinal)
CloseAttendanceUseCase->>MeetingGetService: find(cardinal)
MeetingGetService-->>CloseAttendanceUseCase: List<Meeting>
CloseAttendanceUseCase->>AttendanceRepository: findAllByMeetingAndUserStatus(meeting, ACTIVE)
AttendanceRepository-->>CloseAttendanceUseCase: List<Attendance>
loop 보류중 출석 처리
CloseAttendanceUseCase->>Attendance: close()
CloseAttendanceUseCase->>User: absent()
end
AttendanceAdminController-->>Admin: ATTENDANCE_CLOSE_SUCCESS
Estimated code review effort🎯 4 (복잡함) | ⏱️ ~45분 Suggested reviewers
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. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 9
🧹 Nitpick comments (2)
src/main/kotlin/com/weeth/domain/attendance/domain/service/AttendanceSaveService.kt (1)
9-29: 도메인 서비스로서의 역할 검토 필요코딩 가이드라인에 따르면 "UseCases call Repositories directly, no GetService/SaveService pattern"입니다.
saveAll메서드는 Repository의 thin wrapper로 보이며, UseCase에서 직접 Repository를 호출하는 것이 권장됩니다.
init메서드는 비즈니스 로직(user.add)을 포함하고 있어 도메인 서비스로서 타당하지만,saveAll은 단순 위임이므로 UseCase에서 직접 처리하는 것을 고려해 주세요.As per coding guidelines: "Do not create thin wrapper services — UseCases call Repositories directly, no GetService/SaveService pattern"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/attendance/domain/service/AttendanceSaveService.kt` around lines 9 - 29, AttendanceSaveService contains a thin-wrapper method saveAll that simply delegates to attendanceRepository.saveAll; per the guideline "UseCases call Repositories directly, no GetService/SaveService pattern" remove the saveAll method from AttendanceSaveService and move calls to attendanceRepository.saveAll into the calling UseCase(s); keep the init(user, meetings) method in AttendanceSaveService because it contains domain logic (user.add) and only use attendanceRepository.save within init as-is (refer to AttendanceSaveService, saveAll, init, and attendanceRepository.saveAll when making the change).src/main/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt (1)
17-25: Cross-domain 읽기에 Reader 인터페이스 사용 검토코딩 가이드라인에 따르면 cross-domain 읽기는 Reader 인터페이스를 통해 수행해야 합니다. 현재
UserGetService,MeetingGetService등의 GetService를 직접 사용하고 있습니다.향후 리팩터링 시
UserReader,MeetingReader인터페이스를 도입하여 도메인 간 의존성을 명확히 하는 것을 고려해 주세요.As per coding guidelines: "Cross-domain reads via Reader interfaces; cross-domain writes via Repository directly"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt` around lines 17 - 25, GetAttendanceQueryService currently injects cross-domain GetServices (UserGetService, MeetingGetService, UserCardinalGetService); change these to read-only Reader interfaces (e.g., UserReader, MeetingReader, UserCardinalReader) and inject those instead while leaving domain repository usage (AttendanceRepository) unchanged: define or reuse Reader interfaces exposing only the read methods used by GetAttendanceQueryService, update the constructor parameters of GetAttendanceQueryService to accept the Reader interfaces, and refactor internal calls to use the new readers so cross-domain reads follow the Reader pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/com/weeth/domain/attendance/application/mapper/AttendanceMapper.kt`:
- Around line 3-6: The import declarations used by AttendanceMapper (the imports
for AttendanceDetailResponse, AttendanceInfoResponse, AttendanceResponse,
AttendanceSummaryResponse) are not alphabetically ordered; reorder these import
lines into lexicographic (A–Z) order so imports are sorted consistently (or run
your Kotlin formatter/organize-imports/ktlint to apply the fix automatically)
and ensure AttendanceMapper's imports follow the same rule.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCase.kt`:
- Around line 13-15: CheckInAttendanceUseCase currently depends on
UserGetService which appears to be a thin read-only wrapper; change the
dependency to the user repository/port used for data access (e.g., replace the
UserGetService constructor parameter with the appropriate UserRepository or
UserPort interface) and update all usages inside CheckInAttendanceUseCase to
call the repository/port methods directly; keep attendanceRepository as-is and
ensure constructor signature and imports are adjusted to the repository/port
type and its methods.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/command/CloseAttendanceUseCase.kt`:
- Around line 13-21: CloseAttendanceUseCase currently depends on
MeetingGetService; remove this thin wrapper by having
CloseAttendanceUseCase.call/execute use the MeetingRepository (or MeetingPort)
directly or inline the small retrieval logic into the use case. Replace the
MeetingGetService constructor parameter with the repository/port interface used
to fetch meetings (or move the find(cardinal) logic into
CloseAttendanceUseCase.execute), update uses of meetingGetService.find(cardinal)
to call the repository method (e.g., meetingRepository.findByCardinal or
port.find), and adjust imports and tests accordingly so CloseAttendanceUseCase
only depends on domain ports/repositories and attendanceRepository.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt`:
- Around line 44-46: The chained call violates ktlint rule requiring the dot to
start the next line; update the expression using
attendanceRepository.findAllByUserIdAndCardinal(userId,
currentCardinal.cardinalNumber) so the subsequent .map(mapper::toResponse) is on
its own indented line with the dot at the start (e.g., break the line before
.map) to satisfy ktlint.
In `@src/main/kotlin/com/weeth/domain/attendance/domain/entity/Attendance.kt`:
- Around line 36-39: The `@PrePersist` init() method on Attendance currently
unconditionally sets status = Status.PENDING which can overwrite an already-set
status; change init() to only assign Status.PENDING when status is null (e.g.,
check status == null before setting) so existing statuses aren't overwritten;
references: the Attendance class, the init() method, the status property, and
Status.PENDING.
In
`@src/main/kotlin/com/weeth/domain/attendance/domain/service/scheduler/AttendanceScheduler.kt`:
- Around line 3-8: Imports in AttendanceScheduler.kt are not ordered per ktlint;
reorder the import block containing AttendanceRepository, MeetingGetService,
Status, Transactional, Scheduled, and Service so they are grouped and
alphabetized per ktlint rules (standard library/third-party then project
packages) and ensure blank-line separation between groups; update the
top-of-file imports where AttendanceScheduler is defined to match this ordering.
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.kt`:
- Around line 38-39: The test fails to compile because MockK matcher functions
eq() and any() used in calls like userGetService.find(userId) and
attendanceRepository.findCurrentByUserId(eq(userId), any(), any()) are not
imported; fix by adding the MockK matcher imports (e.g., import io.mockk.eq and
import io.mockk.any) to the test file so eq() and any() resolve correctly for
the lines using those matchers (lines referencing userGetService.find and
attendanceRepository.findCurrentByUserId).
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryServiceTest.kt`:
- Around line 51-56: The mock setup for attendanceMapper.toSummaryResponse is
currently matching only two parameters but the real call in
GetAttendanceQueryService uses three (user, todayAttendance, isAdmin); update
the test in GetAttendanceQueryServiceTest to stub and verify
attendanceMapper.toSummaryResponse with the third boolean argument (isAdmin =
user.role == Role.ADMIN) so the every { ... } returns mapped and the verify {
... } matches the actual three-argument invocation when calling
queryService.find(userId).
- Around line 65-70: The mock setup for the null-attendance case is missing the
isAdmin parameter on attendanceMapper.toSummaryResponse; update the mock and
verification to include the same isAdmin value used in the first test (e.g.,
false) so the stub matches the actual call from queryService.find — change every
{ attendanceMapper.toSummaryResponse(user, null) } returns mapped and verify {
attendanceMapper.toSummaryResponse(user, null) } to include the isAdmin argument
(attendanceMapper.toSummaryResponse(user, null, false)) and verify the same
signature.
---
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt`:
- Around line 17-25: GetAttendanceQueryService currently injects cross-domain
GetServices (UserGetService, MeetingGetService, UserCardinalGetService); change
these to read-only Reader interfaces (e.g., UserReader, MeetingReader,
UserCardinalReader) and inject those instead while leaving domain repository
usage (AttendanceRepository) unchanged: define or reuse Reader interfaces
exposing only the read methods used by GetAttendanceQueryService, update the
constructor parameters of GetAttendanceQueryService to accept the Reader
interfaces, and refactor internal calls to use the new readers so cross-domain
reads follow the Reader pattern.
In
`@src/main/kotlin/com/weeth/domain/attendance/domain/service/AttendanceSaveService.kt`:
- Around line 9-29: AttendanceSaveService contains a thin-wrapper method saveAll
that simply delegates to attendanceRepository.saveAll; per the guideline
"UseCases call Repositories directly, no GetService/SaveService pattern" remove
the saveAll method from AttendanceSaveService and move calls to
attendanceRepository.saveAll into the calling UseCase(s); keep the init(user,
meetings) method in AttendanceSaveService because it contains domain logic
(user.add) and only use attendanceRepository.save within init as-is (refer to
AttendanceSaveService, saveAll, init, and attendanceRepository.saveAll when
making the change).
src/main/kotlin/com/weeth/domain/attendance/application/mapper/AttendanceMapper.kt
Show resolved
Hide resolved
Comment on lines
+13
to
+15
| class CheckInAttendanceUseCase( | ||
| private val userGetService: UserGetService, | ||
| private val attendanceRepository: AttendanceRepository, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
UserGetService 의존이 얇은 래퍼라면 Repository/Port로 변경 필요
Line 13-15에서 UserGetService를 사용하고 있습니다. 단순 조회 서비스라면 UseCase가 Repository/Port를 직접 호출하도록 조정하는 것을 권장합니다.
As per coding guidelines, "Do not create thin wrapper services — UseCases call Repositories directly, no GetService/SaveService pattern".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCase.kt`
around lines 13 - 15, CheckInAttendanceUseCase currently depends on
UserGetService which appears to be a thin read-only wrapper; change the
dependency to the user repository/port used for data access (e.g., replace the
UserGetService constructor parameter with the appropriate UserRepository or
UserPort interface) and update all usages inside CheckInAttendanceUseCase to
call the repository/port methods directly; keep attendanceRepository as-is and
ensure constructor signature and imports are adjusted to the repository/port
type and its methods.
Collaborator
Author
There was a problem hiding this comment.
schedule, user 도메인에서 cross-domain 참조 중이라 이번 PR에서는 유지하고 해당 도메인 마이그레이션 시 Reader 인터페이스로 전환 예정이야
There was a problem hiding this comment.
@soo0711, 이해했습니다! schedule과 user 도메인이 아직 마이그레이션되지 않은 상태에서 cross-domain 의존성을 변경하면 여러 도메인에 걸친 수정이 필요하니, 관련 도메인 마이그레이션 시 함께 Reader 인터페이스로 전환하는 것이 합리적입니다.
향후 schedule/user 도메인 마이그레이션 PR에서 UserReader 인터페이스로 전환하는 것으로 계획하시면 되겠습니다. 👍
🧠 Learnings used
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/application/usecase/**/*.{java,kt} : Add Transactional on UseCase classes only — Domain Services must not have transaction annotations
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/application/**/*.{java,kt} : Do not create thin wrapper services — UseCases call Repositories directly, no GetService/SaveService pattern
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/domain/repository/**/*.{java,kt} : Cross-domain reads via Reader interfaces; cross-domain writes via Repository directly
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/*.{java,kt} : New code should be written in Kotlin; ~271 existing Java files remain and should be migrated incrementally
Comment on lines
13
to
21
| private val meetingGetService: MeetingGetService, | ||
| private val attendanceRepository: AttendanceRepository, | ||
| ) { | ||
| @Transactional | ||
| fun execute( | ||
| now: LocalDate, | ||
| cardinal: Int, | ||
| ) { | ||
| val meetings = meetingGetService.find(cardinal) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
UseCase에서 GetService 의존 제거 검토
Line 13-21에서 MeetingGetService에 직접 의존합니다. 얇은 조회 래퍼라면 Repository/Port로 대체하거나 UseCase로 흡수해 가이드라인에 맞춰주세요.
As per coding guidelines, "Do not create thin wrapper services — UseCases call Repositories directly, no GetService/SaveService pattern".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/command/CloseAttendanceUseCase.kt`
around lines 13 - 21, CloseAttendanceUseCase currently depends on
MeetingGetService; remove this thin wrapper by having
CloseAttendanceUseCase.call/execute use the MeetingRepository (or MeetingPort)
directly or inline the small retrieval logic into the use case. Replace the
MeetingGetService constructor parameter with the repository/port interface used
to fetch meetings (or move the find(cardinal) logic into
CloseAttendanceUseCase.execute), update uses of meetingGetService.find(cardinal)
to call the repository method (e.g., meetingRepository.findByCardinal or
port.find), and adjust imports and tests accordingly so CloseAttendanceUseCase
only depends on domain ports/repositories and attendanceRepository.
There was a problem hiding this comment.
@hyxklee, 알겠습니다! 마이그레이션 작업 중이시군요. 필요하시면 언제든지 도움을 요청해 주세요. 👍
🧠 Learnings used
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/application/**/*.{java,kt} : Do not create thin wrapper services — UseCases call Repositories directly, no GetService/SaveService pattern
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/application/usecase/**/*.{java,kt} : Add Transactional on UseCase classes only — Domain Services must not have transaction annotations
Learnt from: CR
Repo: Team-Weeth/weeth-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-17T02:40:40.740Z
Learning: Applies to src/main/**/*.{java,kt} : New code should be written in Kotlin; ~271 existing Java files remain and should be migrated incrementally
...in/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt
Show resolved
Hide resolved
src/main/kotlin/com/weeth/domain/attendance/domain/service/scheduler/AttendanceScheduler.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+38
to
+39
| every { userGetService.find(userId) } returns user | ||
| every { attendanceRepository.findCurrentByUserId(eq(userId), any(), any()) } returns attendance |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 파일 존재 및 크기 확인
stat src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.kt 2>/dev/null || echo "파일을 찾을 수 없습니다"
# 전체 파일 읽기 (라인 수 먼저 확인)
wc -l src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.ktRepository: Team-Weeth/weeth-server
Length of output: 187
🏁 Script executed:
#!/bin/bash
# 매처 사용 및 import 확인
echo "=== eq() 및 any() 사용 확인 ==="
rg -n "eq\(|any\(" src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.kt
echo "=== MockK import 확인 ==="
rg -n "import io.mockk" src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.ktRepository: Team-Weeth/weeth-server
Length of output: 663
🏁 Script executed:
#!/bin/bash
# 파일 전체 내용 확인
cat -n src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.ktRepository: Team-Weeth/weeth-server
Length of output: 4712
MockK 매처 eq와 any의 import 누락으로 컴파일 실패
39, 53, 68, 84줄에서 eq(), any() 매처를 사용하지만 import가 없어 컴파일 오류가 발생합니다. 다음 import를 추가하세요:
import io.kotest.assertions.throwables.shouldNotThrowAny
import io.kotest.assertions.throwables.shouldThrow
import io.kotest.core.spec.style.DescribeSpec
+import io.mockk.any
+import io.mockk.eq
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify📝 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.
Suggested change
| every { userGetService.find(userId) } returns user | |
| every { attendanceRepository.findCurrentByUserId(eq(userId), any(), any()) } returns attendance | |
| import io.kotest.assertions.throwables.shouldNotThrowAny | |
| import io.kotest.assertions.throwables.shouldThrow | |
| import io.kotest.core.spec.style.DescribeSpec | |
| import io.mockk.any | |
| import io.mockk.eq | |
| import io.mockk.every | |
| import io.mockk.mockk | |
| import io.mockk.verify |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.kt`
around lines 38 - 39, The test fails to compile because MockK matcher functions
eq() and any() used in calls like userGetService.find(userId) and
attendanceRepository.findCurrentByUserId(eq(userId), any(), any()) are not
imported; fix by adding the MockK matcher imports (e.g., import io.mockk.eq and
import io.mockk.any) to the test file so eq() and any() resolve correctly for
the lines using those matchers (lines referencing userGetService.find and
attendanceRepository.findCurrentByUserId).
...otlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryServiceTest.kt
Outdated
Show resolved
Hide resolved
...otlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryServiceTest.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt`:
- Around line 20-23: GetAttendanceQueryService currently depends on
UserGetService, UserCardinalGetService and MeetingGetService; refactor it to
call the corresponding repository/port interfaces directly instead of these thin
"GetService" wrappers. Remove the constructor dependencies userGetService,
userCardinalGetService and meetingGetService, inject the underlying
UserRepository/UserCardinalRepository/MeetingRepository (or their ports)
alongside attendanceRepository, and update methods in GetAttendanceQueryService
to use those repository methods (look for usages of userGetService.*,
userCardinalGetService.* and meetingGetService.*) so the UseCase calls
repositories/ports directly per the guideline.
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryServiceTest.kt`:
- Around line 17-22: The test fails to compile because MockK matcher functions
are used but not imported; add explicit imports for eq, any, and match from
io.mockk: import io.mockk.eq, import io.mockk.any, and import io.mockk.match so
the usages of eq(...), any(), and match(...) in GetAttendanceQueryServiceTest
(e.g., where you stub or verify interactions with mocked repositories/services)
resolve correctly.
---
Duplicate comments:
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryServiceTest.kt`:
- Around line 49-70: The tests mock and verify
attendanceMapper.toSummaryResponse but omit the third isAdmin parameter; update
both cases to stub and verify attendanceMapper.toSummaryResponse with three
arguments (user, todayAttendance or null, isAdmin) to match the real call from
queryService.find, and ensure the same isAdmin value used in every { ... }
returns and verify { ... } checks so the stubbing and verification align with
attendanceRepository.findTodayByUserId and userGetService.find interactions.
Comment on lines
+20
to
+23
| private val userGetService: UserGetService, | ||
| private val userCardinalGetService: UserCardinalGetService, | ||
| private val meetingGetService: MeetingGetService, | ||
| private val attendanceRepository: AttendanceRepository, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
GetService 의존성 대신 Repository/Port 직접 호출로 정리 필요
UseCase가 UserGetService/MeetingGetService 등에 의존하면 “GetService/SaveService” 패턴이 고착됩니다. 해당 로직을 UseCase에서 Repository/Port로 직접 호출하도록 구조를 정리해 주세요.
As per coding guidelines: "Do not create thin wrapper services — UseCases call Repositories directly, no GetService/SaveService pattern".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt`
around lines 20 - 23, GetAttendanceQueryService currently depends on
UserGetService, UserCardinalGetService and MeetingGetService; refactor it to
call the corresponding repository/port interfaces directly instead of these thin
"GetService" wrappers. Remove the constructor dependencies userGetService,
userCardinalGetService and meetingGetService, inject the underlying
UserRepository/UserCardinalRepository/MeetingRepository (or their ports)
alongside attendanceRepository, and update methods in GetAttendanceQueryService
to use those repository methods (look for usages of userGetService.*,
userCardinalGetService.* and meetingGetService.*) so the UseCase calls
repositories/ports directly per the guideline.
Comment on lines
+17
to
+22
| import io.kotest.core.spec.style.DescribeSpec | ||
| import io.kotest.matchers.shouldBe | ||
| import io.mockk.every | ||
| import io.mockk.mockk | ||
| import io.mockk.verify | ||
|
|
There was a problem hiding this comment.
MockK 매처 함수 import 누락으로 컴파일 실패
eq, any, match를 사용하고 있는데 import가 없어 컴파일이 깨집니다. 아래처럼 매처를 명시적으로 import 해주세요.
🛠️ 수정 제안
-import io.mockk.every
-import io.mockk.mockk
-import io.mockk.verify
+import io.mockk.any
+import io.mockk.eq
+import io.mockk.match
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.verify🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryServiceTest.kt`
around lines 17 - 22, The test fails to compile because MockK matcher functions
are used but not imported; add explicit imports for eq, any, and match from
io.mockk: import io.mockk.eq, import io.mockk.any, and import io.mockk.match so
the usages of eq(...), any(), and match(...) in GetAttendanceQueryServiceTest
(e.g., where you stub or verify interactions with mocked repositories/services)
resolve correctly.
hyxklee
reviewed
Feb 18, 2026
Contributor
hyxklee
left a comment
hyxklee
left a comment
There was a problem hiding this comment.
고생하셨습니다!
전반적으로 잘 작업된 것 같아용 클로드 코드가 작업 잘 해주나요??
2가지 커멘트를 달았는데 한 번 씩만 확인 부탁드릴게요!
| ATTENDANCE_EVENT_TYPE_NOT_MATCH(2202, HttpStatus.BAD_REQUEST, "출석일정은 직접 수정할 수 없습니다."), | ||
| ; | ||
|
|
||
| override fun getCode(): Int = code |
Contributor
There was a problem hiding this comment.
해당 코드들은 Global 마이그레이션 할 때 ErrorCodeInterface를 코틀린으로 변경할 때 깔끔하게 지워두겠습니당
| private val attendanceRepository: AttendanceRepository, | ||
| ) { | ||
| @Transactional | ||
| fun execute( |
Contributor
There was a problem hiding this comment.
클래스 명에서도 이미 동작이 들어나긴 하지만, 추후에 메서드가 늘어날 가능성도 있으니, 의미를담은 메서드 명으로 바꿔주는 것은 어떨까요??
checkIn 등의 느낌으로.. (checkIn도 살짝 애매하긴 하지만..)
Collaborator
Author
There was a problem hiding this comment.
넵넵 네이밍 고민해서 적용해보겠습니다!
Collaborator
Author
There was a problem hiding this comment.
attend로 할까도 고민해봤는데 entity랑 이름이 겹쳐서 checkIn으로 수정하겟습니당
다른 usecase의 execute도 수정해두겟습니다!
Comment on lines
13
to
21
| private val meetingGetService: MeetingGetService, | ||
| private val attendanceRepository: AttendanceRepository, | ||
| ) { | ||
| @Transactional | ||
| fun execute( | ||
| now: LocalDate, | ||
| cardinal: Int, | ||
| ) { | ||
| val meetings = meetingGetService.find(cardinal) |
| private val attendanceRepository: AttendanceRepository, | ||
| private val mapper: AttendanceMapper, | ||
| ) { | ||
| fun find(userId: Long): AttendanceSummaryResponse { |
| import com.weeth.domain.attendance.domain.entity.enums.Status | ||
| import org.springframework.stereotype.Service | ||
|
|
||
| @Service |
Collaborator
Author
There was a problem hiding this comment.
네넵 지금 다른 파일에서도 참조중이라서 다른 도메인들 마이그레이션하면서 바꿀 예정입니다!
| import org.springframework.transaction.annotation.Transactional | ||
|
|
||
| @Service | ||
| class AttendanceScheduler( |
Contributor
There was a problem hiding this comment.
좀 찾아보니 저희 아키텍처 기준에서는 스케줄러는 Infra 계층이 적절할 것 같아요!
또한 해당 스케줄러는 말 그대로 "정해진 시간에 호출한다"의 로직을 실행하고, 내부 로직은 Usecase로 분리하면 더 좋을 것 같아요!
그럼 Infra -> application(Usecase) -> domain의 흐름을 타게 되니 책임 분리나 아키텍처를 지키는 면에서도 좋아보입니다!
Contributor
There was a problem hiding this comment.
Infra가 application을 의존해도 괜찮다구 하네용
Contributor
There was a problem hiding this comment.
┌─────────────────────────────┐
│ Inbound Adapter │
│ (Controller / Scheduler ...)│
└───────────────┬─────────────┘
│
▼
┌───────────────┐
│ Application │
│ (UseCase) │
└───────┬───────┘
│
▼
┌───────────────┐
│ Domain │
└───────┬───────┘
▲
│
┌───────────────┴─────────────┐
│ Outbound Adapter │
│ (JPA / Redis / API Client) │
└─────────────────────────────┘
Collaborator
Author
There was a problem hiding this comment.
넵! 안 그래도 scheduler를 어디에 둬야 할 지 고민 중이었는데
이대로 수정해보겠습니다 감사합니다!!
Collaborator
Author
There was a problem hiding this comment.
클로드가 전체적인 틀은 잘 작성해주는데 가끔 놓치는 규칙들이 있어서 리마인드를 종종 해줍니다 ㅎㅎ..
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/command/UpdateAttendanceStatusUseCase.kt`:
- Around line 15-22: UpdateAttendanceStatusUseCase.updateStatus currently calls
Status.valueOf(update.status) which trusts external validation; change this to a
safe conversion that does not throw unchecked exceptions by: locate the call to
Status.valueOf in updateStatus (processing UpdateAttendanceStatusRequest),
replace it with a safe lookup (e.g., Status.values().firstOrNull { it.name ==
update.status } or a Status.fromName(String) helper) and if not found throw a
clear domain exception such as InvalidAttendanceStatusException (or
IllegalArgumentException) with the invalid value included so the domain
explicitly handles invalid status inputs.
---
Duplicate comments:
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/command/CloseAttendanceUseCase.kt`:
- Around line 14-22: The constructor still depends on MeetingGetService; remove
that dependency from CloseAttendanceUseCase and call the appropriate
repository/port directly from the close method instead of meetingGetService.find
— replace the meetingGetService.find(cardinal) call with a direct repository
call (e.g., use an existing MeetingRepository or add a MeetingPort method like
findByCardinal and invoke it from CloseAttendanceUseCase.close), and update the
constructor to only accept AttendanceRepository (and any new repository/port you
add) so the UseCase no longer relies on a thin GetService wrapper.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/query/GetAttendanceQueryService.kt`:
- Around line 20-23: GetAttendanceQueryService currently depends on thin wrapper
services (UserGetService, UserCardinalGetService, MeetingGetService); remove
those dependencies and have the use case call the underlying repositories/ports
directly instead. Update the constructor to accept the appropriate
repository/port interfaces (e.g., UserRepository/UserPort,
UserCardinalRepository/UserCardinalPort, MeetingRepository/MeetingPort) in place
of UserGetService/UserCardinalGetService/MeetingGetService, replace calls to the
wrapper methods (e.g., getUserById/getUserCardinalByUserId/getMeetingById) with
direct repository/port calls within GetAttendanceQueryService, adjust imports
and any mapping logic to live in this use case (or delegate to domain mappers),
and update tests/consumers to mock the repositories/ports instead of the removed
GetService classes.
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.kt`:
- Around line 16-18: Tests fail to compile because MockK matchers eq() and any()
used in CheckInAttendanceUseCaseTest (at usages around lines 39, 53, 68, 84) are
not imported; add the missing imports for io.mockk.eq and io.mockk.any at the
top of the test file (alongside existing mockk/every/verify imports) so the
matcher calls resolve, then rebuild/run the tests to confirm compilation passes.
Comment on lines
+15
to
+22
| fun updateStatus(attendanceUpdates: List<UpdateAttendanceStatusRequest>) { | ||
| attendanceUpdates.forEach { update -> | ||
| val attendance = | ||
| attendanceRepository.findByIdWithUser(update.attendanceId) | ||
| ?: throw AttendanceNotFoundException() | ||
| val user = attendance.user | ||
| val newStatus = Status.valueOf(update.status) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# UpdateAttendanceStatusRequest 클래스 찾기
fd -a 'UpdateAttendanceStatusRequest\.kt' src
# UpdateAttendanceStatusRequest 정의 검색
rg -n 'data class UpdateAttendanceStatusRequest|class UpdateAttendanceStatusRequest' -A 5 -g '*.kt'
# Status enum 찾기
fd -a 'Status\.kt' src
rg -n 'enum class Status' -A 10 -g '*.kt'
# UpdateAttendanceStatusUseCase 전체 내용 확인
rg -n 'class UpdateAttendanceStatusUseCase' -A 30 -g '*.kt'Repository: Team-Weeth/weeth-server
Length of output: 9734
출석 상태 변환 시 안전 예외 처리 필요
Line 21의 Status.valueOf(update.status)는 요청 레이어의 @Pattern 검증에만 의존합니다. 하지만 도메인 로직은 입력 유효성을 명시적으로 처리해야 하며, 외부 검증에 의존해서는 안 됩니다. 정상적인 데이터로 유입되더라도 코드의 명확성과 안정성을 위해 다음과 같이 개선하세요:
- val newStatus = Status.valueOf(update.status)
+ val newStatus =
+ Status.entries.firstOrNull { it.name == update.status }
+ ?: throw IllegalArgumentException("Invalid status: ${update.status}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/attendance/application/usecase/command/UpdateAttendanceStatusUseCase.kt`
around lines 15 - 22, UpdateAttendanceStatusUseCase.updateStatus currently calls
Status.valueOf(update.status) which trusts external validation; change this to a
safe conversion that does not throw unchecked exceptions by: locate the call to
Status.valueOf in updateStatus (processing UpdateAttendanceStatusRequest),
replace it with a safe lookup (e.g., Status.values().firstOrNull { it.name ==
update.status } or a Status.fromName(String) helper) and if not found throw a
clear domain exception such as InvalidAttendanceStatusException (or
IllegalArgumentException) with the invalid value included so the domain
explicitly handles invalid status inputs.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@src/test/kotlin/com/weeth/domain/attendance/application/usecase/command/CheckInAttendanceUseCaseTest.kt`:
- Around line 10-14: The test is missing MockK matcher imports used in
assertions (eq() and any()), which causes compilation failures; add the MockK
matcher imports (import io.mockk.eq and import io.mockk.any) to the imports
block in CheckInAttendanceUseCaseTest so calls to eq(...) and any() resolve
(look for usages of eq and any in the test and add the corresponding imports).
Contributor
|
리뷰 사항들 다 적용 완료 된 걸까용?? |
Collaborator
Author
|
넵!! 다 반영해뒀습니당 |
hyxklee
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 Summary
📝 Changes
What
Why
@ManyToOne기본 EAGER 페치로 인한 불필요한 조인 및 N+1 쿼리 발생How
📸 Screenshots / Logs
💡 Reviewer 참고사항
✅ Checklist
Summary by CodeRabbit
New Features
Chores