[Refactor/#322] 영어버전 알림시간 설정에서 TimePeriod가 바뀌지 않는 문제를 수정합니다.

[SYAAINN]

📌 ISSUE

closed #322

📄 Work Description

• 영어버전 알림시간 설정에서 TimePeriod가 바뀌지 않는 문제를 수정

✨ PR Point

바보같이 TimePeriod를 검사하는 로직에 "오전" 이라고 하드코딩을 박아놔서 ... 영어 버전에선 바뀌지 않는 이슈가 발생했네요.
온보딩 TimeReminderScreen 에서 사용되는 TimePicker와 동일한 로직으로 변경했습니다.
예전에 이걸 왜 도대체 따로 만들었는지 모르겠네요 ㅜㅜ
다음에 TimePicker 로직을 좀 손을 봐야할듯..!

📸 ScreenShot/Video

• N/A

Summary by CodeRabbit

• New Features
  • Added notification permission prompt on Home (Android 13+).
• Bug Fixes
  • Improved AM/PM selection reliability in notification time picker.
• Refactor
  • Streamlined notification handling: removed extra permission prompt in Time Reminder.
  • Unified and optimized calendar/diary data loading for faster, more responsive updates.
• Chores
  • Bumped app version to 1.5.0 (versionCode 29).

[coderabbitai]

Caution

Review failed

Failed to post review comments.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 858e7ef and d9026f8.

📒 Files selected for processing (6)

• app/build.gradle.kts (1 hunks)
• app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderScreen.kt (1 hunks)
• app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderViewModel.kt (2 hunks)
• app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt (8 hunks)
• app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt (9 hunks)
• app/src/main/java/com/sopt/clody/presentation/ui/setting/notificationsetting/component/NotificationSettingTimePicker.kt (3 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (3)

app/src/main/java/com/sopt/clody/presentation/ui/setting/notificationsetting/component/NotificationSettingTimePicker.kt (1)

app/src/main/java/com/sopt/clody/presentation/ui/component/timepicker/PickerState.kt (1)

• rememberPickerState (15-16)

app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt (2)

app/src/main/java/com/sopt/clody/presentation/ui/setting/notificationsetting/screen/NotificationSettingViewModel.kt (1)

• getNotificationInfo (62-94)

app/src/main/java/com/sopt/clody/data/repositoryimpl/NotificationRepositoryImpl.kt (1)

• getNotificationInfo (14-17)

app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt (1)

app/src/main/java/com/sopt/clody/presentation/ui/component/FailureScreen.kt (1)

• FailureScreen (27-69)

⏰ 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: build

🔇 Additional comments (19)

app/build.gradle.kts (1)

28-29: Version bump looks correct and aligned with the PR scope.

versionCode/versionName updated to 29/1.5.0. No issues.

Please ensure Play Console release notes and internal build distribution channels are updated for 1.5.0.

app/src/main/java/com/sopt/clody/presentation/ui/setting/notificationsetting/component/NotificationSettingTimePicker.kt (1)

115-116: AM/PM items now locale-safe labels: LGTM.

Switching to label items derived from TimePeriod.getLabel() removes the hardcoded "오전" dependency and fixes the English locale bug.

app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt (7)

118-120: Initialize selectedDate to now: LGTM.

Keeps UI-consistent defaults and avoids null/epoch pitfalls.

---

128-147: IO offloading and error propagation: LGTM.

withContext(Dispatchers.IO) + explicit success/error states are appropriate.

---

149-171: Daily diaries load refactor: LGTM.

Clear state transitions; data field updates (_hasDraft/_diaryCount/_isDeleted) co-located with success path are good.

---

195-219: Mutex-guarded concurrent loads + early-out heuristic: solid.

Good use of Mutex and parallel loading. One nit: alreadyLoaded requires selectedIsFirst && dailyLoaded; this always reloads when the selected date isn’t 1 even if monthly state is already loaded. If intended, fine; otherwise consider relaxing the condition.

Do we expect to always snap to day 1 for the 2-arg overload? If not, we can avoid unnecessary reloads by only checking calendarLoaded when changing month.

---

222-246: Overload for day-specific load: LGTM.

Parallel loads and lock usage mirror the month-only path correctly.

---

248-253: updateSelectedDate launches daily load: LGTM; be mindful of overlap.

Concurrent calls with month-level loaders are possible. The current state machine is resilient, but watch for redundant network calls when date changes rapidly.

If this becomes chatty under fast taps, we can debounce date changes or cancel the previous job via a dedicated scope.

---

341-354: Draft alarm send on IO and robust folding: LGTM.

Clear success/failure pathways with toast trigger. Consistent with other IO usage.

app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt (7)

90-95: Permission launcher hookup: LGTM.

The result is immediately forwarded to ViewModel, keeping UI slim.

---

105-117: Runtime POST_NOTIFICATIONS flow: LGTM.

API 33+ gating and granted/denied paths are correct. Pre-33 default-allow path calls sendNotification(true) as expected.

---

119-126: Unified initial load path: LGTM.

updateYearMonthAndLoadData(year, month, day) centralizes the first load.

---

131-136: Failure retry path uses the unified API: LGTM.

---

286-291: Delete uses ViewModel’s selectedDate: LGTM.

Avoids mismatches between UI and internal date state.

---

401-401: No-op on DeleteDiaryState.Success: confirm UX.

The success branch is empty. If a toast/snackbar or state reset was expected, it’s currently omitted.

Do we intentionally avoid user feedback after successful deletion on Home, or should we trigger a toast like on other flows?

---

445-446: YearMonthPicker now uses the consolidated loader: LGTM.

app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderViewModel.kt (2)

50-53: Confirm product intent: hard-enable isDiaryAlarm/isReplyAlarm during onboarding.

Both flags are now always true here, whereas HomeViewModel ties them to OS permission. If the onboarding flow should respect OS permission or a user toggle, consider passing that signal in or aligning behaviors.

If intentional (onboarding opts users in), consider a short comment to document the rationale.

---

36-36: Signature change OK — TimeReminder call sites updated

Confirmed: TimeReminderScreen was updated to the new signature and no remaining call sites expect the removed boolean.

• app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderViewModel.kt:36 — fun sendNotification(context: Context)
• app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderScreen.kt:79,86 — viewModel.sendNotification(context) (updated)
• app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt:365 and app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt:93,112,115 — homeViewModel.sendNotification(isGranted/true) — this is a separate HomeViewModel method (Boolean) and is unaffected.

app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderScreen.kt (1)

84-87: LGTM: event tracking followed by notification trigger

Calling AmplitudeUtils.trackEvent(...) before sendNotification(context) maintains expected sequencing. No issues spotted with the simplified API usage here.

Walkthrough

Version bumped to 1.5.0. TimeReminder notification permission flow removed; ViewModel API simplified. Home screen introduces runtime POST_NOTIFICATIONS permission handling and consolidates data loading via new HomeViewModel APIs using coroutines/IO. Notification time picker switches AM/PM handling to explicit enum mapping with default selections.

Changes

Cohort / File(s)	Summary
Version bump app/build.gradle.kts	Incremented versionCode 28→29 and versionName "1.4.0"→"1.5.0".
TimeReminder: remove permission handling & API change app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderScreen.kt, app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderViewModel.kt	Removed Android 13+ notification permission request flow and related imports/state. ViewModel sendNotification signature changed to sendNotification(Context); payload fields set to true without permission gating.
Home: permission flow + unified data loading app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt, app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt	Added POST_NOTIFICATIONS runtime permission launcher and HomeViewModel.sendNotification(isGranted). Replaced separate load calls with updateYearMonthAndLoadData(year, month[, day]); internal loads are suspend, IO-dispatched, and run in parallel with mutex guarding. Adjusted delete/retry/year-month selection paths.
Notification settings: AM/PM mapping fix app/src/main/java/com/sopt/clody/presentation/ui/setting/notificationsetting/component/NotificationSettingTimePicker.kt	Replaced TimePeriod.entries-based items with explicit [AM, PM] lists and label-to-enum mapping. Initialized default selections (PM, 9:30).

Sequence Diagram(s)

sequenceDiagram
  participant UI as HomeScreen
  participant PM as PermissionManager
  participant VM as HomeViewModel
  participant Repo as Repositories

  UI->>PM: Check POST_NOTIFICATIONS (API 33+)
  alt Granted
    UI->>VM: sendNotification(true)
  else Not granted
    UI->>PM: Request permission
    PM-->>UI: Result (granted/denied)
    UI->>VM: sendNotification(isGranted)
  end

  UI->>VM: updateYearMonthAndLoadData(year, month[, day])
  VM->>VM: mutex.withLock
  par Load calendar
    VM->>Repo: getCalendar(year, month) [IO]
    Repo-->>VM: calendar data
  and Load diaries
    VM->>Repo: getDailyDiaries(year, month, day) [IO]
    Repo-->>VM: diary data
  end
  VM-->>UI: state updated

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Assessment against linked issues

Objective	Addressed	Explanation
Fix TimePeriod not changing in English version in notification time setting (#322)	✅

Assessment against linked issues: Out-of-scope changes

Code Change	Explanation
Removed Android 13+ notification permission flow and simplified sendNotification invocation (app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderScreen.kt, various lines)	Not related to TimePeriod behavior in notification time settings.
Changed TimeReminderViewModel.sendNotification signature and hard-coded payload flags (app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderViewModel.kt, various lines)	Unrelated to the TimePeriod issue; modifies notification send logic.
Introduced POST_NOTIFICATIONS permission handling in HomeScreen and new sendNotification(isGranted) in HomeViewModel (app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt, HomeViewModel.kt, various lines)	Scope extends to home screen permissions, not tied to TimePeriod selection.
Consolidated data loading via updateYearMonthAndLoadData and coroutine refactor (app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt, various lines)	Refactor unrelated to the TimePeriod bug in settings.

Possibly related PRs

• [FIX/#290] 버전 코드 업데이트 및 사소한 수정 #291: Similar version bump and time picker handling adjustments; overlaps with AM/PM logic changes.
• [REFACTOR/#283] 하드코딩된 문자열 리소스를 추출하여 i18n 적용 준비 합니다. #284: Modifies TimeReminder components and enum-based time handling; touches same areas as this PR.
• [Feat/#266] 이어쓰기 알림설정을 추가한 UI를 구현합니다. #267: Adjusts notification sending flow and payload; related to ViewModel notification changes.

Suggested labels

🛠️ FIX

Suggested reviewers

• MoonsuKang

Poem

I thump my paws at PM and AM,
Ticking time like a carrot gem.
Permissions asked, then off I hop,
Loading months in a parallel pop.
Version up—bun drums roll—
Ding! Notifications on a scroll.
Hippity hooray, ship the goal!

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/#322-notification-time-period-change

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[MoonsuKang]

LGTM~
타임피커는 대규모 공사 한 번 쳐야될듯 ㅜ