[Feat/#321] 점검시간 다이얼로그 국제화 및 포맷 수정을 진행합니다. #324
Merged
Conversation
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
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR updates the inspection flow to return raw start/end times, adds localization/formatting for inspection text via a LanguageProvider, replaces literals in the inspection dialog with string resources, refactors Home loading and notification permission handling, removes notification permission flow from TimeReminder, and bumps the app version. Changes
Sequence Diagram(s)sequenceDiagram
participant Splash as SplashViewModel
participant Checker as AppUpdateChecker
participant Lang as LanguageProvider
participant UI as InspectionDialog
Splash->>Checker: getInspectionTimeText() (suspend)
Checker-->>Splash: Pair(start,end)?
alt start/end available
Splash->>Lang: getInspectionTimeText(start, end)
Lang-->>Splash: localizedText?
Splash->>UI: Show dialog with localizedText
else no inspection
Splash-->>Splash: Proceed without dialog
end
sequenceDiagram
participant HomeUI as HomeScreen
participant Perm as Permission API
participant VM as HomeViewModel
participant Repo as Repositories
HomeUI->>Perm: Check POST_NOTIFICATIONS (Android 13+)
alt Not granted
HomeUI->>Perm: Request permission
Perm-->>HomeUI: Result (isGranted)
HomeUI->>VM: sendNotification(isGranted)
else Granted or pre-13
HomeUI->>VM: sendNotification(true)
end
HomeUI->>VM: updateYearMonthAndLoadData(year, month, day)
VM->>VM: Mutex lock, set selected date
par Load calendar
VM->>Repo: fetch calendar (IO)
and Load daily
VM->>Repo: fetch daily diaries (IO)
end
Repo-->>VM: Results
VM-->>HomeUI: Update UI states
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
SYAAINN
changed the base branch from
develop
to
refactor/#317-notification-request
MoonsuKang
approved these changes
Aug 24, 2025
Contributor
MoonsuKang
left a comment
MoonsuKang
left a comment
There was a problem hiding this comment.
수고하셨슴다~ 리뷰는 그냥 읽어만 주셔도 되유
| val startText = formatDateTimeWithDayOfWeek(start) | ||
| val endText = formatDateTimeWithDayOfWeek(end) | ||
| return "$startText ~ $endText" | ||
| return nowServer.isAfter(startZ) && nowServer.isBefore(endZ) |
Contributor
There was a problem hiding this comment.
이렇게도 될 것 같아요
Suggested change
| return nowServer.isAfter(startZ) && nowServer.isBefore(endZ) | |
| return nowServer in startZ..endZ |
Contributor
Author
There was a problem hiding this comment.
기존 방식은 초과/미만이고 제안해주신 방식은 이상/이하라고 하네요! 이상/이하가 더 자연스러울 것 같아 반영했습니다!
| val now = LocalDateTime.now() | ||
| return now.isAfter(start) && now.isBefore(end) | ||
| } | ||
| val serverZone = ZoneId.of("Asia/Seoul") |
Contributor
There was a problem hiding this comment.
companion object로 분리하는게 더 좋아보입니다~
| val startLdt = LocalDateTime.parse(start, DateTimeFormatter.ISO_LOCAL_DATE_TIME) | ||
| val endLdt = LocalDateTime.parse(end, DateTimeFormatter.ISO_LOCAL_DATE_TIME) | ||
|
|
||
| if (isKorean()) { |
Contributor
There was a problem hiding this comment.
함수명이 getInspectionTimeText 이라면 formatting 하는 부분은 별도의 함수로 분리하는게 좋을 것 같아유
Contributor
Author
There was a problem hiding this comment.
단일 책임 원칙 고려하여 포맷핑 부분 분리했습니다.
| val endLdt = LocalDateTime.parse(end, DateTimeFormatter.ISO_LOCAL_DATE_TIME) | ||
|
|
||
| if (isKorean()) { | ||
| val koPattern = DateTimeFormatter.ofPattern("M/d(E) HH시mm분", Locale.KOREAN) |
Contributor
There was a problem hiding this comment.
요런 패턴들도 companion object로 분리하는게 좋습니다.
실무에선 다 분리하는 편 이에유
- isUnderInspection 함수에서 점검시간 검사를 초과/미만에서 이상/이하로 개선 + companion object 분리 - getInspectionTimeText 함수에서 단일 책임 원칙에 따라 포맷팅 함수 분리 + companion object 분리
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
📌 ISSUE
closed #321
📄 Work Description
✨ PR Point
원래는 remoteConfig에서 가져온 2025-08-11T18:00:00 형식의 점검시간을 가공 후 뷰모델로 넘겨주었는데
언어 분기 처리를 위한 LanguageProvider 때문에 layer간 의존성이 역전되어,, 방식을 조금 변경했습니다.
📸 ScreenShot/Video
Summary by CodeRabbit