Conversation
1 task
y-eonee
requested review from
a team,
soseoyo12,
sum130 and
wotjs020708
and removed request for
a team
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthrough
Changes
Sequence Diagram(s)(생성 조건에 부합하지 않아 생략) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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: 3
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/Home/HomeView.swift (1)
81-109:@EnvironmentObjectTabBarCoordinator 주입 경로 누락으로 런타임 크래시 발생
ChallengeCardEmptyView는@EnvironmentObject var tabBarCoordinator: TabBarCoordinator를 사용하지만, 뷰 계층에서tabBarCoordinator가 전달되지 않습니다.TabBarCoordinatorView는 line 38에서.environmentObject(tabBarCoordinator)를 적용하지만, line 20에서HomeCoordinatorView에는homeCoordinator만 전달하고 있습니다. 따라서HomeView및 그 자식 뷰들은tabBarCoordinator에 접근할 수 없으며, 앱 실행 시 SwiftUI 환경 객체 해석 오류로 크래시가 발생합니다.
tabBarCoordinator를HomeCoordinatorView계층까지 전달하거나,HomeView에서 다른 방식으로 네비게이션을 처리해야 합니다.
🤖 Fix all issues with AI agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift`:
- Around line 35-40: The columns GridItem currently hardcodes dimensions; update
its definition to use the existing constants instead: replace the hardcoded
40.adjustedW with calendarCellWidth and the spacing 8 with calendarRowSpacing so
the columns array references calendarCellWidth and calendarRowSpacing (symbols:
calendarCellWidth, calendarRowSpacing, columns) for consistent, maintainable
sizing.
- Line 26: The CalendarView currently declares its injected CalendarViewModel
with `@StateObject` which is incorrect for externally created DI-injected
instances; update the property declaration in CalendarView to use
`@ObservedObject` instead of `@StateObject` so the view observes the externally
created model (refer to CalendarViewModel and the factory call
makeCalendarView(viewModel:) in ViewFactory to locate the injection point).
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarViewModel.swift`:
- Around line 138-162: The method extractDate uses several force-unwraps
(TimeZone(identifier: "Asia/Seoul")!, calendar.date(... )!,
calendar.date(byAdding:...)!, etc.); change these to safe unwrapping: guard-let
the TimeZone and create a Calendar only if the TZ exists, and guard-let the
computed dates (startOfMonth, gridStart, raw dates) where applicable, returning
an empty array or handling the error path if any creation fails; update
references to getCurrentMonth, startOfMonth, and gridStart to use the safely
unwrapped values so no '!' remains in extractDate.
| struct CalendarView: View { | ||
| @EnvironmentObject private var calendarCoordinator: CalendarCoordinator | ||
| @ObservedObject var viewModel: CalendarViewModel | ||
| @StateObject var viewModel: CalendarViewModel |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for CalendarView instantiation patterns
rg -n "CalendarView\(" --type swift -C 3Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 2872
🏁 Script executed:
# Check CalendarView file for init and property declarations
fd "CalendarView.swift" --type f | xargs cat -n | head -100Repository: TEAM-Cherrish/Cherrish-iOS
Length of output: 3866
@StateObject 대신 @ObservedObject 사용 필요
CalendarViewModel이 외부의 DIContainer에서 생성되어 의존성 주입으로 전달되고 있습니다 (ViewFactory 라인 53-57, makeCalendarView(viewModel:) 호출). 이 경우 @StateObject 사용은 부적절합니다.
@StateObject: 뷰가 객체를 생성하고 소유할 때만 사용@ObservedObject: 외부에서 주입된 객체를 관찰할 때 사용
현재 @StateObject로 선언하면 뷰 재생성 시 예기치 않게 새 인스턴스가 생성될 수 있습니다. @ObservedObject로 변경하세요.
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift`
at line 26, The CalendarView currently declares its injected CalendarViewModel
with `@StateObject` which is incorrect for externally created DI-injected
instances; update the property declaration in CalendarView to use
`@ObservedObject` instead of `@StateObject` so the view observes the externally
created model (refer to CalendarViewModel and the factory call
makeCalendarView(viewModel:) in ViewFactory to locate the injection point).
Comment on lines
+35
to
+40
| private let calendarCellWidth: CGFloat = 40.adjustedW | ||
| private let calendarCellHeight: CGFloat = 40.adjustedH | ||
| private let calendarRowSpacing: CGFloat = 8.adjustedH | ||
|
|
||
| let weekdays: [String] = ["일", "월", "화", "수", "목", "금", "토"] | ||
| let columns = Array(repeating: GridItem(.fixed(40), spacing: 8), count: 7) | ||
| let columns = Array(repeating: GridItem(.fixed(40.adjustedW), spacing: 8), count: 7) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
상수 일관성 개선 권장
calendarCellWidth 상수가 정의되었지만 columns에서는 하드코딩된 40.adjustedW를 사용하고 있습니다. 유지보수성을 위해 상수를 사용하는 것이 좋습니다.
♻️ 상수 사용 권장
- let columns = Array(repeating: GridItem(.fixed(40.adjustedW), spacing: 8), count: 7)
+ let columns = Array(repeating: GridItem(.fixed(calendarCellWidth), spacing: calendarRowSpacing), count: 7)📝 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
| private let calendarCellWidth: CGFloat = 40.adjustedW | |
| private let calendarCellHeight: CGFloat = 40.adjustedH | |
| private let calendarRowSpacing: CGFloat = 8.adjustedH | |
| let weekdays: [String] = ["일", "월", "화", "수", "목", "금", "토"] | |
| let columns = Array(repeating: GridItem(.fixed(40), spacing: 8), count: 7) | |
| let columns = Array(repeating: GridItem(.fixed(40.adjustedW), spacing: 8), count: 7) | |
| private let calendarCellWidth: CGFloat = 40.adjustedW | |
| private let calendarCellHeight: CGFloat = 40.adjustedH | |
| private let calendarRowSpacing: CGFloat = 8.adjustedH | |
| let weekdays: [String] = ["일", "월", "화", "수", "목", "금", "토"] | |
| let columns = Array(repeating: GridItem(.fixed(calendarCellWidth), spacing: calendarRowSpacing), count: 7) |
🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarView.swift`
around lines 35 - 40, The columns GridItem currently hardcodes dimensions;
update its definition to use the existing constants instead: replace the
hardcoded 40.adjustedW with calendarCellWidth and the spacing 8 with
calendarRowSpacing so the columns array references calendarCellWidth and
calendarRowSpacing (symbols: calendarCellWidth, calendarRowSpacing, columns) for
consistent, maintainable sizing.
Comment on lines
138
to
162
| private func extractDate(currentMonth: Int) -> [DateValue] { | ||
| let calendar = Calendar.current | ||
| var calendar = Calendar(identifier: .gregorian) | ||
| calendar.timeZone = TimeZone(identifier: "Asia/Seoul")! | ||
|
|
||
| let currentMonth = getCurrentMonth(addingMonth: currentMonth) | ||
| var days = currentMonth.getAllDates().compactMap { date -> DateValue in | ||
| let day = calendar.component(.day, from: date) | ||
| return DateValue(day: day, date: date) | ||
| } | ||
| let targetMonthDate = getCurrentMonth(addingMonth: currentMonth) | ||
| let startOfMonth = calendar.date(from: calendar.dateComponents([.year, .month], from: targetMonthDate))! | ||
| let daysInMonth = calendar.range(of: .day, in: .month, for: startOfMonth)?.count ?? 0 | ||
|
|
||
| let firstWeekday = calendar.component(.weekday, from: days.first?.date ?? Date()) | ||
| let firstWeekday = calendar.component(.weekday, from: startOfMonth) | ||
| let leading = firstWeekday - 1 | ||
| let totalCells = leading + daysInMonth | ||
| let rows = Int(ceil(Double(totalCells) / 7.0)) | ||
|
|
||
| for _ in 0 ..< firstWeekday - 1 { | ||
| days.insert(DateValue(day: -1, date: Date()), at: 0) | ||
| } | ||
| let gridStart = calendar.date(byAdding: .day, value: -leading, to: startOfMonth)! | ||
|
|
||
| return days | ||
| return (0..<(rows * 7)).map { i in | ||
| let raw = calendar.date(byAdding: .day, value: i, to: gridStart)! | ||
| let date = calendar.startOfDay(for: raw) | ||
|
|
||
| let isInTargetMonth = calendar.isDate(date, equalTo: targetMonthDate, toGranularity: .month) | ||
| let day = isInTargetMonth ? calendar.component(.day, from: date) : -1 | ||
|
|
||
| return DateValue(day: day, date: date) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
캘린더 그리드 생성 로직 개선 승인
단일 패스로 캘린더 그리드를 생성하는 깔끔한 리팩토링입니다. 타임존 처리가 일관성 있게 Asia/Seoul로 적용되었습니다.
참고: Line 140, 143, 151, 154에서 force unwrap(!)이 사용되고 있습니다. TimeZone(identifier: "Asia/Seoul")과 캘린더 연산은 일반적으로 안전하지만, 방어적 코딩을 위해 guard let을 고려해 볼 수 있습니다.
♻️ 방어적 코딩 예시 (선택사항)
private func extractDate(currentMonth: Int) -> [DateValue] {
var calendar = Calendar(identifier: .gregorian)
- calendar.timeZone = TimeZone(identifier: "Asia/Seoul")!
+ calendar.timeZone = TimeZone(identifier: "Asia/Seoul") ?? .current
let targetMonthDate = getCurrentMonth(addingMonth: currentMonth)
- let startOfMonth = calendar.date(from: calendar.dateComponents([.year, .month], from: targetMonthDate))!
+ guard let startOfMonth = calendar.date(from: calendar.dateComponents([.year, .month], from: targetMonthDate)) else {
+ return []
+ }🤖 Prompt for AI Agents
In
`@Cherrish-iOS/Cherrish-iOS/Presentation/Feature/Calendar/CalendarMain/CalendarViewModel.swift`
around lines 138 - 162, The method extractDate uses several force-unwraps
(TimeZone(identifier: "Asia/Seoul")!, calendar.date(... )!,
calendar.date(byAdding:...)!, etc.); change these to safe unwrapping: guard-let
the TimeZone and create a Calendar only if the TZ exists, and guard-let the
computed dates (startOfMonth, gridStart, raw dates) where applicable, returning
an empty array or handling the error path if any creation fails; update
references to getCurrentMonth, startOfMonth, and gridStart to use the safely
unwrapped values so no '!' remains in extractDate.
wotjs020708
approved these changes
Jan 21, 2026
Contributor
wotjs020708
left a comment
wotjs020708
left a comment
There was a problem hiding this comment.
고생하셨습니다. 리뷰 한번만 확인해주세요!
| @@ -88,8 +96,6 @@ extension CalendarView { | |||
| } | |||
| } | |||
| .padding(.horizontal, 11) | |||
|
|
||
| let weekdays: [String] = ["일", "월", "화", "수", "목", "금", "토"] | ||
| let columns = Array(repeating: GridItem(.fixed(40), spacing: 8), count: 7) | ||
| let columns = Array(repeating: GridItem(.fixed(40.adjustedW), spacing: 8), count: 7) |
Contributor
There was a problem hiding this comment.
요기 spacing에도 기기대응 해야하나요?
sum130
approved these changes
Jan 21, 2026
| .frame(height: calendarCellHeight + calendarRowSpacing) | ||
| } | ||
| } | ||
| .padding(.horizontal, 23) |
Merged
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.
🔗 연결된 이슈
📄 작업 내용