Conversation
y-eonee
requested review from
soseoyo12,
sum130 and
wotjs020708
and removed request for
a team
Closed
1 task
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough여러 화면에 공통 로딩 상태( Changes
Sequence Diagram(s)(생략) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25분 Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/MyPage/MyPageView.swift (1)
29-35: 중복 에러 처리: ViewModel과 View 양쪽에서 에러를 처리하고 있습니다.
fetchUserInfo()는 내부에서 이미 에러를catch하고 로깅합니다. 하지만 함수 시그니처가throws이고 View에서도try-catch로 감싸고 있어 혼란스럽습니다. ViewModel에서 에러를 삼키므로 실제로 View의catch블록은 실행되지 않습니다.일관성을 위해
throws를 제거하거나, ViewModel에서 에러를 다시 던지도록 수정하는 것을 권장합니다.
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift`:
- Around line 26-29: The SwiftUI state properties are missing access control;
mark the `@StateObject` and `@State` properties as private to satisfy SwiftLint and
encapsulate internal state—update the declarations for viewModel,
homeCalendarFlowState, calendarMode, and selectedProcedureID in CalendarView
(e.g., change "@StateObject var viewModel" to "private `@StateObject` var
viewModel", and similarly make homeCalendarFlowState, calendarMode, and
selectedProcedureID private).
- Around line 44-52: The two sequential calls
viewModel.fetchProcedureCountsOfMonth and viewModel.fetchTodayProcedureList
cause isLoading toggles and potential flicker and also prevent the second call
if the first fails; fix by either (A) adding a ViewModel wrapper like
fetchMonthData() that sets isLoading = true once, starts both operations (e.g.,
using async let or TaskGroup inside the wrapper), awaits both, aggregates
errors, then sets isLoading = false, and call that from the .task, or (B) change
the .task body to run both in parallel (use async let for
viewModel.fetchProcedureCountsOfMonth and viewModel.fetchTodayProcedureList) and
handle errors so both run and you control a single loading flag (ensure you
reference viewModel.isLoading or the new wrapper method names to locate the
changes).
- Around line 71-72: The properties viewModel and homeCalendarFlowState in
CalendarView (declared as `@StateObject` var viewModel and `@StateObject` var
homeCalendarFlowState) are being passed in from a parent and should be observed,
not owned; change both declarations to use `@ObservedObject` instead of
`@StateObject` (so CalendarView/CalendarContentView observes externally-owned
CalendarViewModel and HomeCalendarFlowState) and ensure you do not reinitialize
them in this view's init.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarViewModel.swift`:
- Around line 127-132: The catch block currently logs errors but fails to reset
isLoading, causing the UI to remain in a loading state; update the error
handling for fetchTodayProcedureListUseCase.execute (used to assign
procedureList) so that isLoading is set to false in all paths — either by
setting isLoading = false inside the catch block alongside
CherrishLogger.error(error) or by restructuring the async call with a
defer/finally-like pattern to guarantee isLoading is cleared after the await
completes or errors.
- Around line 113-119: The do-catch in CalendarViewModel around
fetchProcedureCountOfMonthUseCase.execute currently sets isLoading = false only
on success, causing infinite loading on errors; update the function so isLoading
is always set to false after the await (either add isLoading = false in the
catch block or use a defer/finally-style pattern) and keep assigning
procedureCountOfMonth from response only on success—refer to CalendarViewModel
and the fetchProcedureCountOfMonthUseCase.execute call to locate the code to
change.
- Around line 139-146: The catch path for the
fetchProcedureDowntimeUseCase.execute call doesn't reset isLoading, causing the
view to remain stuck loading; update the error handling so isLoading is set to
false on failure (either add isLoading = false inside the catch before
CherrishLogger.error(error) or refactor the surrounding logic to use a defer
that sets isLoading = false after starting the async work), keeping the rest of
the flow (selectedDowntime, mapToDowntimeDays, CherrishLogger.error) unchanged.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/ChallengeView/View/ChallengeProgressView.swift`:
- Around line 70-73: The ChallengeProgressContentView is incorrectly declaring
its injected viewModel as `@StateObject`; change the property declaration in
ChallengeProgressContentView from "@StateObject var viewModel:
ChallengeProgressViewModel" to use "@ObservedObject" so the view observes the
externally created ChallengeProgressViewModel passed in from
ChallengeProgressView; ensure the parent (ChallengeProgressView) remains the
owner (uses `@StateObject`) and only pass the viewModel instance into
ChallengeProgressContentView.
In `@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Home/HomeView.swift`:
- Around line 27-29: HomeContentView currently declares its injected view model
as `@StateObject` which incorrectly claims ownership; change the property wrapper
on viewModel from `@StateObject` to `@ObservedObject` so HomeContentView observes
the parent-provided HomeViewModel instead (update the declaration of viewModel
in the HomeContentView struct to use `@ObservedObject` and keep the type
HomeViewModel).
In `@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/MyPage/MyPageViewModel.swift`:
- Around line 25-32: The catch block in MyPageViewModel leaves isLoading true on
errors causing infinite loading; update the error path so isLoading is set to
false after fetchUserInfoUseCase.execute() fails — either add isLoading = false
inside the catch that currently calls CherrishLogger.error(error), or refactor
the async call in the function (the method containing
fetchUserInfoUseCase.execute()) to use a defer that sets isLoading = false to
guarantee it is cleared on both success and failure.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishLoadingView.swift`:
- Around line 10-15: 현재 CherrishLoadingView의 ProgressView가 부모 레이아웃에 따라 위치가 달라질 수
있으니 CherrishLoadingView의 body(ProgressView)를 화면 전체를 차지하도록 확장하고 중앙에 정렬되게 변경하세요;
구체적으로 struct CherrishLoadingView의 body에서 ProgressView를 감싸는 컨테이너(예: ZStack 또는
VStack)를 사용하거나 ProgressView에 frame(maxWidth: .infinity, maxHeight: .infinity) 및
.frame 정렬/alignment를 적용하여 수평·수직 중앙 정렬을 보장하도록 수정하세요.
Comment on lines
26
to
29
| @StateObject var viewModel: CalendarViewModel | ||
| @StateObject var homeCalendarFlowState: HomeCalendarFlowState | ||
| @State var calendarMode: CalendarMode = .none | ||
| @State var selectedProcedureID: Int? = nil |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
SwiftUI state 속성에 private 접근 제어자를 추가하세요.
SwiftLint 경고에 따르면, SwiftUI의 @StateObject와 @State 속성은 private으로 선언해야 합니다. 특히 calendarMode와 selectedProcedureID는 내부 상태이므로 반드시 private이어야 합니다.
♻️ 권장 수정안
struct CalendarView: View {
`@EnvironmentObject` private var calendarCoordinator: CalendarCoordinator
- `@StateObject` var viewModel: CalendarViewModel
- `@StateObject` var homeCalendarFlowState: HomeCalendarFlowState
- `@State` var calendarMode: CalendarMode = .none
- `@State` var selectedProcedureID: Int? = nil
+ `@StateObject` private var viewModel: CalendarViewModel
+ `@StateObject` private var homeCalendarFlowState: HomeCalendarFlowState
+ `@State` private var calendarMode: CalendarMode = .none
+ `@State` private var selectedProcedureID: Int? = nil📝 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
| @StateObject var viewModel: CalendarViewModel | |
| @StateObject var homeCalendarFlowState: HomeCalendarFlowState | |
| @State var calendarMode: CalendarMode = .none | |
| @State var selectedProcedureID: Int? = nil | |
| `@StateObject` private var viewModel: CalendarViewModel | |
| `@StateObject` private var homeCalendarFlowState: HomeCalendarFlowState | |
| `@State` private var calendarMode: CalendarMode = .none | |
| `@State` private var selectedProcedureID: Int? = nil |
🧰 Tools
🪛 SwiftLint (0.57.0)
[Warning] 26-26: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 27-27: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 28-28: SwiftUI state properties should be private
(private_swiftui_state)
[Warning] 29-29: SwiftUI state properties should be private
(private_swiftui_state)
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift`
around lines 26 - 29, The SwiftUI state properties are missing access control;
mark the `@StateObject` and `@State` properties as private to satisfy SwiftLint and
encapsulate internal state—update the declarations for viewModel,
homeCalendarFlowState, calendarMode, and selectedProcedureID in CalendarView
(e.g., change "@StateObject var viewModel" to "private `@StateObject` var
viewModel", and similarly make homeCalendarFlowState, calendarMode, and
selectedProcedureID private).
Comment on lines
+44
to
+52
| .task (id: viewModel.currentMonth){ | ||
| if calendarMode == .none { | ||
| do { | ||
| try await viewModel.fetchProcedureCountsOfMonth() | ||
| try await viewModel.fetchTodayProcedureList() | ||
| } catch { | ||
| CherrishLogger.error(error) | ||
| } | ||
| } |
There was a problem hiding this comment.
순차적 fetch 호출로 인한 로딩 상태 깜빡임(flicker) 가능성이 있습니다.
fetchProcedureCountsOfMonth와 fetchTodayProcedureList 모두 내부에서 isLoading = true로 설정 후 완료 시 false로 설정합니다. 순차 호출 시 true → false → true → false 순서로 변경되어 로딩 뷰가 깜빡일 수 있습니다.
또한, 첫 번째 호출이 실패하면 두 번째 호출이 실행되지 않아 불완전한 상태가 될 수 있습니다.
🔧 권장 수정안
ViewModel에서 로딩 상태를 별도로 관리하거나, 두 호출을 병렬로 실행하고 외부에서 로딩 상태를 제어하는 방식을 고려하세요:
.task (id: viewModel.currentMonth){
if calendarMode == .none {
+ viewModel.isLoading = true
do {
- try await viewModel.fetchProcedureCountsOfMonth()
- try await viewModel.fetchTodayProcedureList()
+ async let counts: () = viewModel.fetchProcedureCountsOfMonthWithoutLoading()
+ async let list: () = viewModel.fetchTodayProcedureListWithoutLoading()
+ _ = try await (counts, list)
} catch {
CherrishLogger.error(error)
}
+ viewModel.isLoading = false
}
}또는 ViewModel에 두 작업을 래핑하는 단일 메서드를 추가하세요.
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift`
around lines 44 - 52, The two sequential calls
viewModel.fetchProcedureCountsOfMonth and viewModel.fetchTodayProcedureList
cause isLoading toggles and potential flicker and also prevent the second call
if the first fails; fix by either (A) adding a ViewModel wrapper like
fetchMonthData() that sets isLoading = true once, starts both operations (e.g.,
using async let or TaskGroup inside the wrapper), awaits both, aggregates
errors, then sets isLoading = false, and call that from the .task, or (B) change
the .task body to run both in parallel (use async let for
viewModel.fetchProcedureCountsOfMonth and viewModel.fetchTodayProcedureList) and
handle errors so both run and you control a single loading flag (ensure you
reference viewModel.isLoading or the new wrapper method names to locate the
changes).
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift
Outdated
Show resolved
Hide resolved
Comment on lines
+113
to
+119
| do { | ||
| let response = try await fetchProcedureCountOfMonthUseCase.execute(year: year, month: month) | ||
| isLoading = false | ||
| procedureCountOfMonth = response.dailyProcedureCounts | ||
| } catch { | ||
| CherrishLogger.error(error) | ||
| } |
There was a problem hiding this comment.
에러 발생 시 isLoading이 false로 설정되지 않아 무한 로딩 상태가 됩니다.
catch 블록에서 isLoading = false가 누락되어 있습니다. 네트워크 오류 발생 시 사용자는 영원히 로딩 화면을 보게 됩니다. HomeViewModel.loadDashboard()의 패턴처럼 isLoading = false를 do-catch 이후 또는 catch 블록 내에서 호출해야 합니다.
🐛 수정 제안
do {
let response = try await fetchProcedureCountOfMonthUseCase.execute(year: year, month: month)
- isLoading = false
procedureCountOfMonth = response.dailyProcedureCounts
} catch {
CherrishLogger.error(error)
}
+ isLoading = false🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarViewModel.swift`
around lines 113 - 119, The do-catch in CalendarViewModel around
fetchProcedureCountOfMonthUseCase.execute currently sets isLoading = false only
on success, causing infinite loading on errors; update the function so isLoading
is always set to false after the await (either add isLoading = false in the
catch block or use a defer/finally-style pattern) and keep assigning
procedureCountOfMonth from response only on success—refer to CalendarViewModel
and the fetchProcedureCountOfMonthUseCase.execute call to locate the code to
change.
Comment on lines
+127
to
+132
| do { | ||
| procedureList = try await fetchTodayProcedureListUseCase.execute(date: treatmentDate) | ||
| isLoading = false | ||
| } catch { | ||
| CherrishLogger.error(error) | ||
| } |
There was a problem hiding this comment.
동일한 이슈: 에러 시 isLoading이 초기화되지 않습니다.
🐛 수정 제안
do {
procedureList = try await fetchTodayProcedureListUseCase.execute(date: treatmentDate)
- isLoading = false
} catch {
CherrishLogger.error(error)
}
+ isLoading = false📝 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
| do { | |
| procedureList = try await fetchTodayProcedureListUseCase.execute(date: treatmentDate) | |
| isLoading = false | |
| } catch { | |
| CherrishLogger.error(error) | |
| } | |
| do { | |
| procedureList = try await fetchTodayProcedureListUseCase.execute(date: treatmentDate) | |
| } catch { | |
| CherrishLogger.error(error) | |
| } | |
| isLoading = false |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarViewModel.swift`
around lines 127 - 132, The catch block currently logs errors but fails to reset
isLoading, causing the UI to remain in a loading state; update the error
handling for fetchTodayProcedureListUseCase.execute (used to assign
procedureList) so that isLoading is set to false in all paths — either by
setting isLoading = false inside the catch block alongside
CherrishLogger.error(error) or by restructuring the async call with a
defer/finally-like pattern to guarantee isLoading is cleared after the await
completes or errors.
Comment on lines
+139
to
+146
| do { | ||
| let downtimeList = try await fetchProcedureDowntimeUseCase.execute(id: procedureId) | ||
| isLoading = false | ||
| selectedDowntime = downtimeList | ||
| mapToDowntimeDays(procedure: downtimeList) | ||
| } catch { | ||
| CherrishLogger.error(error) | ||
| } |
There was a problem hiding this comment.
동일한 이슈: 에러 시 isLoading이 초기화되지 않습니다.
🐛 수정 제안
do {
let downtimeList = try await fetchProcedureDowntimeUseCase.execute(id: procedureId)
- isLoading = false
selectedDowntime = downtimeList
mapToDowntimeDays(procedure: downtimeList)
} catch {
CherrishLogger.error(error)
}
+ isLoading = false📝 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
| do { | |
| let downtimeList = try await fetchProcedureDowntimeUseCase.execute(id: procedureId) | |
| isLoading = false | |
| selectedDowntime = downtimeList | |
| mapToDowntimeDays(procedure: downtimeList) | |
| } catch { | |
| CherrishLogger.error(error) | |
| } | |
| do { | |
| let downtimeList = try await fetchProcedureDowntimeUseCase.execute(id: procedureId) | |
| selectedDowntime = downtimeList | |
| mapToDowntimeDays(procedure: downtimeList) | |
| } catch { | |
| CherrishLogger.error(error) | |
| } | |
| isLoading = false |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarViewModel.swift`
around lines 139 - 146, The catch path for the
fetchProcedureDowntimeUseCase.execute call doesn't reset isLoading, causing the
view to remain stuck loading; update the error handling so isLoading is set to
false on failure (either add isLoading = false inside the catch before
CherrishLogger.error(error) or refactor the surrounding logic to use a defer
that sets isLoading = false after starting the async work), keeping the rest of
the flow (selectedDowntime, mapToDowntimeDays, CherrishLogger.error) unchanged.
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/ChallengeView/View/ChallengeProgressView.swift
Show resolved
Hide resolved
Comment on lines
+25
to
+32
| do { | ||
| let response = try await fetchUserInfoUseCase.execute() | ||
| isLoading = false | ||
| name = response.name | ||
| day = response.daysSinceSignup | ||
| } catch { | ||
| CherrishLogger.error(error) | ||
| } |
There was a problem hiding this comment.
에러 발생 시 isLoading이 false로 설정되지 않습니다.
CalendarViewModel과 동일한 이슈입니다. catch 블록 실행 후에도 isLoading이 true로 유지되어 에러 발생 시 무한 로딩 상태가 됩니다.
🐛 수정 제안
do {
let response = try await fetchUserInfoUseCase.execute()
- isLoading = false
name = response.name
day = response.daysSinceSignup
} catch {
CherrishLogger.error(error)
}
+ isLoading = false📝 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
| do { | |
| let response = try await fetchUserInfoUseCase.execute() | |
| isLoading = false | |
| name = response.name | |
| day = response.daysSinceSignup | |
| } catch { | |
| CherrishLogger.error(error) | |
| } | |
| do { | |
| let response = try await fetchUserInfoUseCase.execute() | |
| name = response.name | |
| day = response.daysSinceSignup | |
| } catch { | |
| CherrishLogger.error(error) | |
| } | |
| isLoading = false |
🤖 Prompt for AI Agents
In `@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/MyPage/MyPageViewModel.swift`
around lines 25 - 32, The catch block in MyPageViewModel leaves isLoading true
on errors causing infinite loading; update the error path so isLoading is set to
false after fetchUserInfoUseCase.execute() fails — either add isLoading = false
inside the catch that currently calls CherrishLogger.error(error), or refactor
the async call in the function (the method containing
fetchUserInfoUseCase.execute()) to use a defer that sets isLoading = false to
guarantee it is cleared on both success and failure.
Comment on lines
+10
to
+15
| struct CherrishLoadingView: View { | ||
| var body: some View { | ||
| ProgressView() | ||
| .progressViewStyle(.circular) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
로딩 뷰가 화면 중앙에 배치되도록 frame 설정을 고려해주세요.
현재 구현은 부모 뷰의 레이아웃에 따라 로딩 인디케이터 위치가 달라질 수 있습니다. 전체 화면을 차지하고 중앙에 배치되도록 설정하면 일관된 UX를 제공할 수 있습니다.
♻️ 개선 제안
struct CherrishLoadingView: View {
var body: some View {
ProgressView()
.progressViewStyle(.circular)
+ .frame(maxWidth: .infinity, maxHeight: .infinity)
}
}🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Global/Components/CherrishLoadingView.swift`
around lines 10 - 15, 현재 CherrishLoadingView의 ProgressView가 부모 레이아웃에 따라 위치가 달라질
수 있으니 CherrishLoadingView의 body(ProgressView)를 화면 전체를 차지하도록 확장하고 중앙에 정렬되게 변경하세요;
구체적으로 struct CherrishLoadingView의 body에서 ProgressView를 감싸는 컨테이너(예: ZStack 또는
VStack)를 사용하거나 ProgressView에 frame(maxWidth: .infinity, maxHeight: .infinity) 및
.frame 정렬/alignment를 적용하여 수평·수직 중앙 정렬을 보장하도록 수정하세요.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift (1)
73-76: 중첩된Task를 제거하고.task의 비동기 컨텍스트에서 직접 호출하세요
.task내부에 새로운Task { }를 생성하면 구조화되지 않은 최상위 작업이 만들어져.task의 취소 신호를 받지 않습니다. 뷰가 제거되어도 작업이 계속 실행될 수 있으므로, 작업을.task클로저 내에 직접 유지하세요.♻️ 수정 제안
.task { - Task { - try await viewModel.fetchTreatments() - } + try? await viewModel.fetchTreatments() }
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/ViewModel/NoTreatment/NoTreatmentViewModel.swift`:
- Around line 73-81: The fetchCategories() method sets isLoading = true but only
clears it on the success path, so failures leave the loading state stuck; change
the method to set isLoading = true then immediately use a defer { isLoading =
false } so isLoading is cleared on both success and error (leave the await call
to fetchCategoriesUseCase.execute() and the catch logging as-is). Apply the same
pattern to the analogous function in the 84-93 range so every async fetch in
this ViewModel uses defer to always reset isLoading.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/ViewModel/Treatment/TreatmentViewModel.swift`:
- Around line 65-74: The fetchTreatments() method sets isLoading = true but only
clears it on the success path; wrap the cleanup in a defer to ensure isLoading =
false runs on both success and failure: set isLoading = true at the start of
fetchTreatments(), immediately add defer { isLoading = false }, then remove the
manual isLoading = false in the do block; keep the existing treatments
assignment from fetchTreatmentsUseCase.execute(id: nil, keyword: searchText) and
the catch block that sets treatments = [] and logs via
CherrishLogger.network(error) so failures still clear data and are logged.
Comment on lines
73
to
81
| @MainActor | ||
| func fetchCategories() async { | ||
| isLoading = true | ||
| do { | ||
| categories = try await fetchCategoriesUseCase.execute() | ||
| isLoading = false | ||
| } catch { | ||
| CherrishLogger.debug(error) | ||
| } |
There was a problem hiding this comment.
에러 경로에서도 isLoading을 반드시 해제하세요.
현재 catch에서 isLoading = false가 없어 실패 시 로딩 뷰가 영구 표시됩니다. defer로 성공/실패 모두에서 해제되도록 보장해주세요.
🔧 제안 수정(diff)
`@MainActor`
func fetchCategories() async {
isLoading = true
+ defer { isLoading = false }
do {
categories = try await fetchCategoriesUseCase.execute()
- isLoading = false
} catch {
CherrishLogger.debug(error)
}
}
`@MainActor`
func fetchNoTreatments() async {
isLoading = true
+ defer { isLoading = false }
do {
treatments = try await fetchTreatmentsUseCase.execute(id: selectedCategory?.id, keyword: "")
- isLoading = false
} catch {
treatments = []
CherrishLogger.debug(error)
}
}Also applies to: 84-93
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/ViewModel/NoTreatment/NoTreatmentViewModel.swift`
around lines 73 - 81, The fetchCategories() method sets isLoading = true but
only clears it on the success path, so failures leave the loading state stuck;
change the method to set isLoading = true then immediately use a defer {
isLoading = false } so isLoading is cleared on both success and error (leave the
await call to fetchCategoriesUseCase.execute() and the catch logging as-is).
Apply the same pattern to the analogous function in the 84-93 range so every
async fetch in this ViewModel uses defer to always reset isLoading.
Comment on lines
65
to
74
| @MainActor | ||
| func fetchTreatments() async throws { | ||
| isLoading = true | ||
| do { | ||
| treatments = try await fetchTreatmentsUseCase.execute(id: nil, keyword: searchText) | ||
| isLoading = false | ||
| } catch { | ||
| treatments = [] | ||
| CherrishLogger.network(error) | ||
| } |
There was a problem hiding this comment.
에러 경로에서 로딩이 해제되지 않습니다.
실패 시 isLoading이 false로 돌아오지 않아 로딩 뷰가 고정될 수 있습니다. 성공/실패 모두에서 해제되도록 defer로 정리하세요.
🔧 수정 제안
`@MainActor`
func fetchTreatments() async throws {
isLoading = true
+ defer { isLoading = false }
do {
treatments = try await fetchTreatmentsUseCase.execute(id: nil, keyword: searchText)
- isLoading = false
} catch {
treatments = []
CherrishLogger.network(error)
}
}📝 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
| @MainActor | |
| func fetchTreatments() async throws { | |
| isLoading = true | |
| do { | |
| treatments = try await fetchTreatmentsUseCase.execute(id: nil, keyword: searchText) | |
| isLoading = false | |
| } catch { | |
| treatments = [] | |
| CherrishLogger.network(error) | |
| } | |
| `@MainActor` | |
| func fetchTreatments() async throws { | |
| isLoading = true | |
| defer { isLoading = false } | |
| do { | |
| treatments = try await fetchTreatmentsUseCase.execute(id: nil, keyword: searchText) | |
| } catch { | |
| treatments = [] | |
| CherrishLogger.network(error) | |
| } | |
| } |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/ViewModel/Treatment/TreatmentViewModel.swift`
around lines 65 - 74, The fetchTreatments() method sets isLoading = true but
only clears it on the success path; wrap the cleanup in a defer to ensure
isLoading = false runs on both success and failure: set isLoading = true at the
start of fetchTreatments(), immediately add defer { isLoading = false }, then
remove the manual isLoading = false in the do block; keep the existing
treatments assignment from fetchTreatmentsUseCase.execute(id: nil, keyword:
searchText) and the catch block that sets treatments = [] and logs via
CherrishLogger.network(error) so failures still clear data and are logged.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift (1)
82-86:.task내부에 중복된Task가 있습니다.
.task수정자는 이미 async 컨텍스트를 제공하므로 내부에Task를 중첩할 필요가 없습니다. 중첩된Task는 에러 처리를 복잡하게 만들고,try로 던져진 에러가 처리되지 않습니다.🐛 수정 제안
.task { - Task { - try await viewModel.fetchTreatments() - } + do { + try await viewModel.fetchTreatments() + } catch { + // 에러 처리 필요 + } }
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift`:
- Around line 61-74: The horizontal padding modifier is applied to the ForEach
container instead of each row; remove `.padding(.horizontal, 24.adjustedW)` from
the ForEach and move it onto each TreatmentRowView instance so each row gets the
padding (update the TreatmentRowView call inside the ForEach to append
`.padding(.horizontal, 24.adjustedW)`). Keep the existing TreatmentRowView
parameters (displayMode, treatmentEntity, isSelected, action) and ensure the
viewModel methods (`isSelected`, `addTreatment`, `removeTreatment`) continue to
be used as before.
♻️ Duplicate comments (11)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/MyPage/MyPageViewModel.swift (1)
23-32: 에러 경로에서도 isLoading 해제가 필요합니다.현재 실패 시 isLoading이 true로 남아 무한 로딩이 될 수 있습니다. 성공/실패 공통으로 해제되도록 defer를 권장합니다.
🔧 수정 제안
`@MainActor` func fetchUserInfo() async throws { isLoading = true + defer { isLoading = false } do { let response = try await fetchUserInfoUseCase.execute() - isLoading = false name = response.name day = response.daysSinceSignup } catch { CherrishLogger.error(error) } }Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/ViewModel/Treatment/TreatmentViewModel.swift (1)
65-74: 에러 경로에서 isLoading 해제가 누락됩니다.실패 시 로딩 뷰가 고정될 수 있으니 defer로 정리하세요.
🔧 수정 제안
`@MainActor` func fetchTreatments() async throws { isLoading = true + defer { isLoading = false } do { treatments = try await fetchTreatmentsUseCase.execute(id: nil, keyword: searchText) - isLoading = false } catch { treatments = [] CherrishLogger.network(error) } }Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Home/HomeView.swift (1)
27-29: 주입받는 ViewModel은@ObservedObject가맞습니다.HomeContentView는 부모로부터 viewModel을 전달받으므로
@StateObject사용 시 소유권 충돌/라이프사이클 문제가 발생할 수 있습니다. 또한 SwiftLint 경고(private_swiftui_state)를 피하려면 private 지정이 필요합니다.🔧 수정 제안
private struct HomeContentView: View { - `@StateObject` var viewModel: HomeViewModel + `@ObservedObject` private var viewModel: HomeViewModelCherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/ViewModel/NoTreatment/NoTreatmentViewModel.swift (1)
73-92: 에러 경로에서도 로딩 해제가 필요합니다.두 fetch 함수 모두 실패 시 isLoading이 true로 남아 로딩이 고정될 수 있습니다. defer로 공통 해제를 보장하세요.
🔧 수정 제안
`@MainActor` func fetchCategories() async { isLoading = true + defer { isLoading = false } do { categories = try await fetchCategoriesUseCase.execute() - isLoading = false } catch { CherrishLogger.debug(error) } } `@MainActor` func fetchNoTreatments() async { isLoading = true + defer { isLoading = false } do { treatments = try await fetchTreatmentsUseCase.execute(id: selectedCategory?.id, keyword: "") - isLoading = false } catch { treatments = [] CherrishLogger.debug(error) } }Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarViewModel.swift (3)
113-119: 에러 발생 시isLoading이false로 설정되지 않아 무한 로딩 상태가 됩니다.
catch블록에서isLoading = false가 누락되어 있습니다. 네트워크 오류 발생 시 사용자는 영원히 로딩 화면을 보게 됩니다.🐛 수정 제안
do { let response = try await fetchProcedureCountOfMonthUseCase.execute(year: year, month: month) - isLoading = false procedureCountOfMonth = response.dailyProcedureCounts } catch { CherrishLogger.error(error) } + isLoading = false
127-132: 동일한 이슈: 에러 시isLoading이 초기화되지 않습니다.🐛 수정 제안
do { procedureList = try await fetchTodayProcedureListUseCase.execute(date: treatmentDate) - isLoading = false } catch { CherrishLogger.error(error) } + isLoading = false
139-146: 동일한 이슈: 에러 시isLoading이 초기화되지 않습니다.🐛 수정 제안
do { let downtimeList = try await fetchProcedureDowntimeUseCase.execute(id: procedureId) - isLoading = false selectedDowntime = downtimeList mapToDowntimeDays(procedure: downtimeList) } catch { CherrishLogger.error(error) } + isLoading = falseCherrish-iOS/Cherrish-iOS/Presentation/Feature/ChallengeView/View/ChallengeProgressView.swift (1)
30-33:@StateObject대신@ObservedObject를 사용해야 합니다.
ChallengeProgressContentView는 부모 뷰(ChallengeProgressView)로부터 이미 생성된viewModel을 전달받습니다.@StateObject는 뷰가 해당 객체의 소유권을 가지고 생성할 때 사용하고, 외부에서 전달받는 경우에는@ObservedObject를 사용해야 합니다.🐛 수정 제안
private struct ChallengeProgressContentView: View { `@EnvironmentObject` private var challengeCoordinator: ChallengeCoordinator - `@StateObject` var viewModel: ChallengeProgressViewModel + `@ObservedObject` var viewModel: ChallengeProgressViewModelCherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift (3)
26-29: SwiftUI state 속성에private접근 제어자를 추가하세요.SwiftLint 경고에 따르면, SwiftUI의
@StateObject와@State속성은private으로 선언해야 합니다.♻️ 수정 제안
struct CalendarView: View { `@EnvironmentObject` private var calendarCoordinator: CalendarCoordinator - `@StateObject` var viewModel: CalendarViewModel - `@StateObject` var homeCalendarFlowState: HomeCalendarFlowState - `@State` var calendarMode: CalendarMode = .none - `@State` var selectedProcedureID: Int? = nil + `@StateObject` private var viewModel: CalendarViewModel + `@StateObject` private var homeCalendarFlowState: HomeCalendarFlowState + `@State` private var calendarMode: CalendarMode = .none + `@State` private var selectedProcedureID: Int? = nil
44-53: 순차적 fetch 호출로 인한 로딩 상태 깜빡임(flicker) 가능성이 있습니다.
fetchProcedureCountsOfMonth와fetchTodayProcedureList모두 내부에서isLoading = true로 설정 후 완료 시false로 설정합니다. 순차 호출 시true → false → true → false순서로 변경되어 로딩 뷰가 깜빡일 수 있습니다.ViewModel에 두 작업을 래핑하는 단일 메서드를 추가하거나, 병렬 실행을 고려하세요.
71-72:@StateObject대신@ObservedObject를 사용해야 합니다.
CalendarContentView는 부모 뷰(CalendarView)로부터viewModel과homeCalendarFlowState를 전달받습니다. 외부에서 전달받는 객체는@ObservedObject를 사용해야 합니다.🐛 수정 제안
private struct CalendarContentView: View { `@EnvironmentObject` private var calendarCoordinator: CalendarCoordinator - `@StateObject` var viewModel: CalendarViewModel - `@StateObject` var homeCalendarFlowState: HomeCalendarFlowState + `@ObservedObject` var viewModel: CalendarViewModel + `@ObservedObject` var homeCalendarFlowState: HomeCalendarFlowState
Comment on lines
+61
to
+74
| ForEach(viewModel.treatments, id: \.id) { treatment in | ||
| TreatmentRowView( | ||
| displayMode: .checkBoxView, | ||
| treatmentEntity: treatment, | ||
| isSelected: .constant(viewModel.isSelected(treatment)), | ||
| action: { if viewModel.isSelected(treatment) { | ||
| viewModel.removeTreatment(treatment) | ||
| } else { | ||
| viewModel.addTreatment(treatment) | ||
|
|
||
| } } | ||
| ) | ||
| } | ||
| .padding(.horizontal, 24.adjustedW) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
.padding 수정자가 ForEach에 적용되어 있습니다.
ForEach에 적용된 .padding(.horizontal, 24.adjustedW)는 각 행이 아닌 컨테이너에 적용됩니다. NoTreatmentFilterView.swift에서는 TreatmentRowView에 직접 패딩을 적용하는 패턴을 사용합니다(Line 49: .padding(.horizontal, 25.adjustedW)). 일관성을 위해 동일한 패턴을 적용하세요.
♻️ 수정 제안
ForEach(viewModel.treatments, id: \.id) { treatment in
TreatmentRowView(
displayMode: .checkBoxView,
treatmentEntity: treatment,
isSelected: .constant(viewModel.isSelected(treatment)),
action: { if viewModel.isSelected(treatment) {
viewModel.removeTreatment(treatment)
} else {
viewModel.addTreatment(treatment)
} }
)
+ .padding(.horizontal, 24.adjustedW)
}
- .padding(.horizontal, 24.adjustedW)📝 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
| ForEach(viewModel.treatments, id: \.id) { treatment in | |
| TreatmentRowView( | |
| displayMode: .checkBoxView, | |
| treatmentEntity: treatment, | |
| isSelected: .constant(viewModel.isSelected(treatment)), | |
| action: { if viewModel.isSelected(treatment) { | |
| viewModel.removeTreatment(treatment) | |
| } else { | |
| viewModel.addTreatment(treatment) | |
| } } | |
| ) | |
| } | |
| .padding(.horizontal, 24.adjustedW) | |
| ForEach(viewModel.treatments, id: \.id) { treatment in | |
| TreatmentRowView( | |
| displayMode: .checkBoxView, | |
| treatmentEntity: treatment, | |
| isSelected: .constant(viewModel.isSelected(treatment)), | |
| action: { if viewModel.isSelected(treatment) { | |
| viewModel.removeTreatment(treatment) | |
| } else { | |
| viewModel.addTreatment(treatment) | |
| } } | |
| ) | |
| .padding(.horizontal, 24.adjustedW) | |
| } |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift`
around lines 61 - 74, The horizontal padding modifier is applied to the ForEach
container instead of each row; remove `.padding(.horizontal, 24.adjustedW)` from
the ForEach and move it onto each TreatmentRowView instance so each row gets the
padding (update the TreatmentRowView call inside the ForEach to append
`.padding(.horizontal, 24.adjustedW)`). Keep the existing TreatmentRowView
parameters (displayMode, treatmentEntity, isSelected, action) and ensure the
viewModel methods (`isSelected`, `addTreatment`, `removeTreatment`) continue to
be used as before.
sum130
approved these changes
Jan 23, 2026
Comment on lines
+113
to
+119
| do { | ||
| let response = try await fetchProcedureCountOfMonthUseCase.execute(year: year, month: month) | ||
| isLoading = false | ||
| procedureCountOfMonth = response.dailyProcedureCounts | ||
| } catch { | ||
| CherrishLogger.error(error) | ||
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift`:
- Around line 54-56: The loading view is currently inside the ScrollView so user
can still interact while viewModel.isLoading; change the layout to present
CherrishLoadingView as a full-screen overlay and disable interactions on the
underlying ScrollView when viewModel.isLoading. Locate the ScrollView and wrap
content in a ZStack (or apply .disabled(viewModel.isLoading) /
.allowsHitTesting(!viewModel.isLoading) to the ScrollView) so touches are
blocked, and also ensure any trigger methods check viewModel.isLoading (e.g.,
search/fetch handlers) to avoid duplicate calls while loading.
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/ChallengeView/View/ChallengeProgressView.swift`:
- Around line 14-26: The view uses .task in ChallengeProgressView which calls
await viewModel.loadChallenge() every time the view appears, risking duplicate
loads on re-entry; if repeated loads are not intended, add a one-time load guard
(e.g., a didLoad Bool on the viewModel or ChallengeProgressView) and check it
before calling loadChallenge(), set the flag after the first successful load,
and update ChallengeProgressContentView logic to rely on
viewModel.isLoading/didLoad to prevent re-entry; alternatively, move the load
call to an initializer or a higher-level lifecycle controller if you want
strictly one-time loading.
♻️ Duplicate comments (3)
Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift (2)
26-29: SwiftUI 상태 프로퍼티는private로 캡슐화하세요.SwiftLint 경고 대상이며 내부 상태 노출을 줄일 수 있습니다.
♻️ 권장 수정안
- `@StateObject` var viewModel: CalendarViewModel - `@StateObject` var homeCalendarFlowState: HomeCalendarFlowState - `@State` var calendarMode: CalendarMode = .none - `@State` var selectedProcedureID: Int? = nil + `@StateObject` private var viewModel: CalendarViewModel + `@StateObject` private var homeCalendarFlowState: HomeCalendarFlowState + `@State` private var calendarMode: CalendarMode = .none + `@State` private var selectedProcedureID: Int? = nil
44-52: 순차 fetch로 로딩 깜빡임과 부분 실패 위험이 있습니다.두 메서드가 각각
isLoading을 토글하면true→false→true로 깜빡이고, 첫 호출이 실패하면 두 번째가 실행되지 않습니다. ViewModel에서 단일 로딩 제어 + 병렬/통합 호출로 묶어 주세요.🔧 권장 수정안(예시)
.task (id: viewModel.currentMonth){ if calendarMode == .none { do { - try await viewModel.fetchProcedureCountsOfMonth() - try await viewModel.fetchTodayProcedureList() + try await viewModel.fetchMonthData() } catch { CherrishLogger.error(error) } } }Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift (1)
62-75: 행 패딩 적용 위치 일관화 필요
이전 리뷰에서 이미 지적된 항목과 동일합니다.ForEach가 아니라 각TreatmentRowView에 패딩을 적용하는 쪽이 의도에 더 맞습니다.
Comment on lines
+54
to
56
| if viewModel.isLoading { | ||
| CherrishLoadingView() | ||
| } else { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
로딩 중 인터랙션 차단 여부를 확인해 주세요.
현재 로딩 뷰가 ScrollView 내부에만 표시되어 검색/스크롤이 계속 가능합니다. 로딩이 중복 호출 방지 목적이라면 비활성화를 고려해 주세요.
♻️ 제안 (로딩 중 입력 차단)
- ScrollView(.vertical, showsIndicators: false) {
+ ScrollView(.vertical, showsIndicators: false) {
...
}
+ .disabled(viewModel.isLoading)🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/Treatment/View/Treatment/TreatmentFilterView.swift`
around lines 54 - 56, The loading view is currently inside the ScrollView so
user can still interact while viewModel.isLoading; change the layout to present
CherrishLoadingView as a full-screen overlay and disable interactions on the
underlying ScrollView when viewModel.isLoading. Locate the ScrollView and wrap
content in a ZStack (or apply .disabled(viewModel.isLoading) /
.allowsHitTesting(!viewModel.isLoading) to the ScrollView) so touches are
blocked, and also ensure any trigger methods check viewModel.isLoading (e.g.,
search/fetch handlers) to avoid duplicate calls while loading.
Comment on lines
+14
to
+26
| var body: some View { | ||
| ZStack { | ||
| if viewModel.isLoading { | ||
| CherrishLoadingView() | ||
| } | ||
| else { | ||
| ChallengeProgressContentView(viewModel: viewModel) | ||
| } | ||
| } | ||
| .task { | ||
| await viewModel.loadChallenge() | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
로딩 재진입 시 중복 로드 의도 확인 필요
.task는 뷰가 다시 나타날 때마다 실행되어 loadChallenge()가 재호출될 수 있습니다. 이 동작이 의도된 것인지 확인해 주세요. 아니라면 1회 로드 가드(didLoad 플래그 등)를 고려하는 편이 안전합니다.
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/ChallengeView/View/ChallengeProgressView.swift`
around lines 14 - 26, The view uses .task in ChallengeProgressView which calls
await viewModel.loadChallenge() every time the view appears, risking duplicate
loads on re-entry; if repeated loads are not intended, add a one-time load guard
(e.g., a didLoad Bool on the viewModel or ChallengeProgressView) and check it
before calling loadChallenge(), set the flag after the first successful load,
and update ChallengeProgressContentView logic to rely on
viewModel.isLoading/didLoad to prevent re-entry; alternatively, move the load
call to an initializer or a higher-level lifecycle controller if you want
strictly one-time loading.
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
4 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.
🔗 연결된 이슈
📄 작업 내용