-
Notifications
You must be signed in to change notification settings - Fork 1
fix: 목표없는 기록 조회(Fitrun 186) #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "FITRUN-186-Server-\uBAA9\uD45C\uC5C6\uB294-\uAE30\uB85D-\uC870\uD68C"
Conversation
Walkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant Controller
participant RunningRecordService
participant UserGoalManager
participant UserGoalDao
Controller->>RunningRecordService: getRecords(user)
RunningRecordService->>UserGoalManager: hasUserGoal(user)
UserGoalManager->>UserGoalDao: findUserGoal(user)
UserGoalDao-->>UserGoalManager: UserGoal/null
UserGoalManager-->>RunningRecordService: Boolean
alt User has goal
RunningRecordService->>UserGoalManager: getUserGoal(user)
RunningRecordService->>RunningRecordService: Count achieved goals
RunningRecordService-->>Controller: RunningRecordListResponse(카운트 포함)
else User has no goal
RunningRecordService-->>Controller: RunningRecordListResponse(카운트 미포함)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/kotlin/com/yapp/yapp/record/api/response/RunningRecordListResponse.kt
(1 hunks)src/main/kotlin/com/yapp/yapp/record/domain/RunningRecordService.kt
(1 hunks)src/main/kotlin/com/yapp/yapp/user/domain/goal/UserGoalManager.kt
(1 hunks)src/test/kotlin/com/yapp/yapp/record/RunningRecordControllerTest.kt
(6 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: jhon3242
PR: YAPP-Github/26th-App-Team-4-BE#32
File: src/main/kotlin/com/yapp/yapp/record/domain/record/RunningRecord.kt:62-68
Timestamp: 2025-07-04T07:35:18.584Z
Learning: YAPP-Github/26th-App-Team-4-BE 프로젝트의 RunningRecord.updateInfoByRunningPoints 메서드에서 빈 runningPoints 리스트에 대한 검증이 필요하다. runningPoints.last() 호출 전에 isEmpty() 체크를 통해 NoSuchElementException을 방지해야 한다.
src/main/kotlin/com/yapp/yapp/user/domain/goal/UserGoalManager.kt (1)
Learnt from: jhon3242
PR: #43
File: src/main/kotlin/com/yapp/yapp/user/domain/goal/UserGoalRepository.kt:6-8
Timestamp: 2025-07-10T14:16:05.498Z
Learning: UserGoal과 User 간의 관계는 1:1이고 단방향입니다 (UserGoal -> User). 이는 UserGoal이 User에 대한 외래키를 가지지만, User는 UserGoal을 직접 참조하지 않음을 의미합니다.
src/main/kotlin/com/yapp/yapp/record/domain/RunningRecordService.kt (1)
Learnt from: jhon3242
PR: #32
File: src/main/kotlin/com/yapp/yapp/record/domain/record/RunningRecord.kt:62-68
Timestamp: 2025-07-04T07:35:18.584Z
Learning: YAPP-Github/26th-App-Team-4-BE 프로젝트의 RunningRecord.updateInfoByRunningPoints 메서드에서 빈 runningPoints 리스트에 대한 검증이 필요하다. runningPoints.last() 호출 전에 isEmpty() 체크를 통해 NoSuchElementException을 방지해야 한다.
src/test/kotlin/com/yapp/yapp/record/RunningRecordControllerTest.kt (2)
Learnt from: jhon3242
PR: #32
File: src/main/kotlin/com/yapp/yapp/record/domain/record/RunningRecord.kt:62-68
Timestamp: 2025-07-04T07:35:18.584Z
Learning: YAPP-Github/26th-App-Team-4-BE 프로젝트의 RunningRecord.updateInfoByRunningPoints 메서드에서 빈 runningPoints 리스트에 대한 검증이 필요하다. runningPoints.last() 호출 전에 isEmpty() 체크를 통해 NoSuchElementException을 방지해야 한다.
Learnt from: jhon3242
PR: #44
File: src/test/kotlin/com/yapp/yapp/document/user/UserDocumentTest.kt:0-0
Timestamp: 2025-07-11T02:24:46.949Z
Learning: YAPP-Github/26th-App-Team-4-BE 프로젝트에서는 API 문서 테스트(Spring REST Docs)의 목적을 문서화에 집중하고, 세부 로직에 대한 테스트는 진행하지 않는 것으로 통일한다. 이는 테스트의 관심사 분리를 위한 아키텍처 결정이다.
⏰ 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: Run tests and generate API docs
🔇 Additional comments (6)
src/main/kotlin/com/yapp/yapp/user/domain/goal/UserGoalManager.kt (1)
74-76
: 새로운 메서드 구현이 적절합니다.사용자 목표 존재 여부를 확인하는
hasUserGoal
메서드가 명확하고 간결하게 구현되었습니다. 기존 DAO의findUserGoal
메서드를 재사용하여 일관성을 유지하고 있습니다.src/main/kotlin/com/yapp/yapp/record/api/response/RunningRecordListResponse.kt (1)
14-14
: 기본값 추가가 적절합니다.목표 달성 카운트 파라미터에 기본값 0을 추가하여 목표가 없는 사용자의 경우를 우아하게 처리하고 있습니다. 기존 코드와의 호환성도 유지됩니다.
src/main/kotlin/com/yapp/yapp/record/domain/RunningRecordService.kt (2)
48-58
: 조건부 로직이 효율적으로 구현되었습니다.사용자 목표 존재 여부를 확인하여 필요한 경우에만 목표 달성 카운트를 계산하는 로직이 적절합니다. 불필요한 데이터베이스 조회를 방지하고 성능을 최적화합니다.
59-62
: 목표가 없는 경우의 처리가 적절합니다.Response 객체의 기본값 기능을 활용하여 목표가 없는 사용자의 경우를 깔끔하게 처리하고 있습니다.
src/test/kotlin/com/yapp/yapp/record/RunningRecordControllerTest.kt (2)
8-8
: 서비스 레이어 테스트로의 변경이 적절합니다.
RunningRecordRepository
에서RunningService
로 변경하여 더 높은 레벨의 통합 테스트를 수행하고 있습니다.
24-24
: 테스트 메서드명 개선이 좋습니다.기존 테스트 메서드명에 "리스트"를 추가하여 테스트 목적을 더 명확히 표현하고 있습니다.
Also applies to: 55-55, 85-85, 115-115
@Test | ||
fun `목표가 없는 사용자가 러닝 기록 리스트를 조회한다`() { | ||
// given | ||
val user = userFixture.create() | ||
val startResponse = runningService.start(userId = user.id, request = RequestFixture.runningStartRequest()) | ||
val recordId = startResponse.recordId | ||
runningService.done( | ||
userId = user.id, | ||
recordId = recordId, | ||
request = RequestFixture.runningDoneRequest(), | ||
imageFile = runningFixture.multipartFile(), | ||
) | ||
|
||
// when | ||
// then | ||
RestAssured.given().log().all() | ||
.header(HttpHeaders.AUTHORIZATION, getAccessToken(user.email)) | ||
.param("type", RecordsSearchType.ALL.name) | ||
.param("page", 0) | ||
.param("size", 10) | ||
.`when`() | ||
.get("/api/v1/records") | ||
.then().log().all() | ||
.statusCode(200) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
새 테스트 케이스 개선이 필요합니다.
목표가 없는 사용자의 기록 조회 테스트가 추가되었습니다. 하지만 몇 가지 개선점이 있습니다.
다음과 같은 개선을 권장합니다:
@Test
fun `목표가 없는 사용자가 러닝 기록 리스트를 조회한다`() {
// given
+ val now = TimeProvider.now().toStartOfDay()
val user = userFixture.create()
val startResponse = runningService.start(userId = user.id, request = RequestFixture.runningStartRequest())
val recordId = startResponse.recordId
runningService.done(
userId = user.id,
recordId = recordId,
request = RequestFixture.runningDoneRequest(),
imageFile = runningFixture.multipartFile(),
)
// when
- // then
- RestAssured.given().log().all()
+ val result = RestAssured.given().log().all()
.header(HttpHeaders.AUTHORIZATION, getAccessToken(user.email))
.param("type", RecordsSearchType.ALL.name)
+ .param("targetDate", now.toString())
.param("page", 0)
.param("size", 10)
.`when`()
.get("/api/v1/records")
.then().log().all()
.statusCode(200)
+ .extract().`as`(ApiResponse::class.java)
+ .result
+ val response = convert(result, RunningRecordListResponse::class.java)
+
+ // then
+ Assertions.assertThat(response.records.size).isEqualTo(1)
+ Assertions.assertThat(response.timeGoalAchievedCount).isEqualTo(0)
+ Assertions.assertThat(response.distanceGoalAchievedCount).isEqualTo(0)
}
주요 개선점:
targetDate
파라미터 추가로 다른 테스트와 일관성 유지- 응답 내용 검증 추가로 목표 달성 카운트가 0인지 확인
- 레코드가 정상적으로 생성되었는지 확인
📝 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.
@Test | |
fun `목표가 없는 사용자가 러닝 기록 리스트를 조회한다`() { | |
// given | |
val user = userFixture.create() | |
val startResponse = runningService.start(userId = user.id, request = RequestFixture.runningStartRequest()) | |
val recordId = startResponse.recordId | |
runningService.done( | |
userId = user.id, | |
recordId = recordId, | |
request = RequestFixture.runningDoneRequest(), | |
imageFile = runningFixture.multipartFile(), | |
) | |
// when | |
// then | |
RestAssured.given().log().all() | |
.header(HttpHeaders.AUTHORIZATION, getAccessToken(user.email)) | |
.param("type", RecordsSearchType.ALL.name) | |
.param("page", 0) | |
.param("size", 10) | |
.`when`() | |
.get("/api/v1/records") | |
.then().log().all() | |
.statusCode(200) | |
} | |
@Test | |
fun `목표가 없는 사용자가 러닝 기록 리스트를 조회한다`() { | |
// given | |
val now = TimeProvider.now().toStartOfDay() | |
val user = userFixture.create() | |
val startResponse = runningService.start(userId = user.id, request = RequestFixture.runningStartRequest()) | |
val recordId = startResponse.recordId | |
runningService.done( | |
userId = user.id, | |
recordId = recordId, | |
request = RequestFixture.runningDoneRequest(), | |
imageFile = runningFixture.multipartFile(), | |
) | |
// when | |
val result = RestAssured.given().log().all() | |
.header(HttpHeaders.AUTHORIZATION, getAccessToken(user.email)) | |
.param("type", RecordsSearchType.ALL.name) | |
.param("targetDate", now.toString()) | |
.param("page", 0) | |
.param("size", 10) | |
.`when`() | |
.get("/api/v1/records") | |
.then().log().all() | |
.statusCode(200) | |
.extract().`as`(ApiResponse::class.java) | |
.result | |
val response = convert(result, RunningRecordListResponse::class.java) | |
// then | |
Assertions.assertThat(response.records.size).isEqualTo(1) | |
Assertions.assertThat(response.timeGoalAchievedCount).isEqualTo(0) | |
Assertions.assertThat(response.distanceGoalAchievedCount).isEqualTo(0) | |
} |
🤖 Prompt for AI Agents
In src/test/kotlin/com/yapp/yapp/record/RunningRecordControllerTest.kt around
lines 167 to 191, the test for fetching running records of a user without a goal
lacks consistency and thoroughness. Add the 'targetDate' parameter to the
request to align with other tests, include assertions to verify the response
content ensuring the goal achievement count is zero, and add checks confirming
that the running record was created successfully before making the GET request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다! 페이징 관련해서 별도의 문제는 없을 것 같지만 1~2케이스 정도는 데이터 페이징도 테스트 하면 좋을 것 같습니다!!
원준님 페이징 테스트 하는 부분 size 10으로 고정 되어 있는거 같아요!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/kotlin/com/yapp/yapp/document/record/RecordDocumentTest.kt
(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: jhon3242
PR: YAPP-Github/26th-App-Team-4-BE#32
File: src/main/kotlin/com/yapp/yapp/record/domain/record/RunningRecord.kt:62-68
Timestamp: 2025-07-04T07:35:18.584Z
Learning: YAPP-Github/26th-App-Team-4-BE 프로젝트의 RunningRecord.updateInfoByRunningPoints 메서드에서 빈 runningPoints 리스트에 대한 검증이 필요하다. runningPoints.last() 호출 전에 isEmpty() 체크를 통해 NoSuchElementException을 방지해야 한다.
Learnt from: jhon3242
PR: YAPP-Github/26th-App-Team-4-BE#44
File: src/test/kotlin/com/yapp/yapp/document/user/UserDocumentTest.kt:0-0
Timestamp: 2025-07-11T02:24:46.949Z
Learning: YAPP-Github/26th-App-Team-4-BE 프로젝트에서는 API 문서 테스트(Spring REST Docs)의 목적을 문서화에 집중하고, 세부 로직에 대한 테스트는 진행하지 않는 것으로 통일한다. 이는 테스트의 관심사 분리를 위한 아키텍처 결정이다.
📚 Learning: yapp-github/26th-app-team-4-be 프로젝트에서는 api 문서 테스트(spring rest docs)의 목적을 문서화에 집중하고, 세부 로직에 대한 테스트는 진...
Learnt from: jhon3242
PR: YAPP-Github/26th-App-Team-4-BE#44
File: src/test/kotlin/com/yapp/yapp/document/user/UserDocumentTest.kt:0-0
Timestamp: 2025-07-11T02:24:46.949Z
Learning: YAPP-Github/26th-App-Team-4-BE 프로젝트에서는 API 문서 테스트(Spring REST Docs)의 목적을 문서화에 집중하고, 세부 로직에 대한 테스트는 진행하지 않는 것으로 통일한다. 이는 테스트의 관심사 분리를 위한 아키텍처 결정이다.
Applied to files:
src/test/kotlin/com/yapp/yapp/document/record/RecordDocumentTest.kt
📚 Learning: xml 응답을 테스트하는 spring rest docs에서는 응답 필드 문서화(`restdocsresponse`)를 추가하면 테스트가 실패할 수 있다. xml api 테스트에서는 ...
Learnt from: jhon3242
PR: YAPP-Github/26th-App-Team-4-BE#32
File: src/test/kotlin/com/yapp/yapp/document/running/RunningDocumentTest.kt:130-167
Timestamp: 2025-07-02T06:05:46.573Z
Learning: XML 응답을 테스트하는 Spring REST Docs에서는 응답 필드 문서화(`restDocsResponse`)를 추가하면 테스트가 실패할 수 있다. XML API 테스트에서는 응답 필드 문서화를 제외하더라도 API 문서에 예시 값이 정상적으로 표시된다.
Applied to files:
src/test/kotlin/com/yapp/yapp/document/record/RecordDocumentTest.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: Run tests and generate API docs
🔇 Additional comments (3)
src/test/kotlin/com/yapp/yapp/document/record/RecordDocumentTest.kt (3)
29-29
: API 문서 명확성 개선이 좋습니다.type 파라미터의 설명을 "ALL, TODAY etc."에서 구체적인 값들 "ALL, DAILY, WEEKLY, MONTHLY, YEARLY"로 명시한 것이 API 사용자에게 더 명확한 정보를 제공합니다.
39-39
: sort 파라미터 설명이 크게 개선되었습니다.기본 정렬 기준과 선택사항임을 명시하고, 예시를 "createdAt,DESC"에서 "startAt,DESC"로 변경한 것이 실제 구현과 일치하여 좋습니다.
88-88
: 테스트에 sort 파라미터 추가가 적절합니다.문서화된 파라미터를 실제 테스트에서도 사용하여 API 문서와 테스트의 일관성을 보장하는 것이 좋습니다.
@@ -85,6 +85,7 @@ class RecordDocumentTest : BaseDocumentTest() { | |||
.param("targetDate", now.toString()) | |||
.param("page", 0) | |||
.param("size", 10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
페이지 크기 고정값에 대한 검토가 필요합니다.
PR 댓글에서 지적된 대로 페이지 크기가 10으로 고정되어 있습니다. 다양한 페이지 크기에 대한 테스트 케이스를 추가하는 것을 고려해보세요.
다양한 페이지 크기 값들이 실제로 테스트되고 있는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# 다른 테스트 파일들에서 페이지 크기 파라미터 사용 현황 확인
rg -A 3 -B 3 'param.*size' --type kotlin src/test/
Length of output: 6251
다양한 페이지 크기 테스트 추가 필요
현재 모든 테스트에서 size
파라미터가 10으로 고정되어 있어, 다양한 페이지 크기에 대한 동작을 검증할 수 없습니다. 아래 위치에서 확인된 고정 값들을 참고하여 여러 케이스(예: 1, 5, 20 등)에 대한 테스트를 추가해 보세요.
- src/test/kotlin/com/yapp/yapp/document/record/RecordDocumentTest.kt:87 (
.param("size", 10)
) - src/test/kotlin/com/yapp/yapp/record/RunningRecordControllerTest.kt (여러 군데에서
.param("size", 10)
)
🤖 Prompt for AI Agents
In src/test/kotlin/com/yapp/yapp/document/record/RecordDocumentTest.kt at line
87, the page size parameter "size" is fixed at 10, limiting test coverage for
different page sizes. Add additional test cases with varying "size" values such
as 1, 5, and 20 to verify the behavior of pagination under different conditions.
Duplicate or parameterize the existing test to cover these multiple page sizes.
📎 관련 이슈
📄 추가 작업 내용
✅ Check List
💬 기타 참고 사항
Summary by CodeRabbit
New Features
Bug Fixes
Tests