Skip to content

[Feat/#268] 이어쓰기 알림 설정을 유도하는 기능을 구현합니다.#272

Merged
SYAAINN merged 12 commits intodevelopfrom
feat/#268-draft-alarm-home
Jun 6, 2025
Merged

[Feat/#268] 이어쓰기 알림 설정을 유도하는 기능을 구현합니다.#272
SYAAINN merged 12 commits intodevelopfrom
feat/#268-draft-alarm-home

Conversation

@SYAAINN
Copy link
Contributor

@SYAAINN SYAAINN commented Jun 4, 2025

📌 ISSUE

closed #268

📄 Work Description

1. 임시저장 최초 여부 판별

📱 일기쓰기 화면 내 로직

  1. 유저가 일기를 작성하다가 홈화면으로 이동하기 위해 뒤로가기를 누른다.

  2. 임시저장 의사를 묻는 다이얼로그가 출력된다.

  3. 임시저장 하고 나가기를 선택하였을 경우

    isDraftUsed == false : 최초 사용을 의미하므로 isDraftUsed, isFirstUsetrue 로 변경

    isDraftUsed == true : 아무 추가 동작없이 저장 후 홈화면으로 이동한다.

📱 홈 화면 내 로직

  1. 홈 화면에 진입 시 isFirstUse 값을 검사한다.

    true → 이어쓰기 알림설정을 위한 바텀시트를 노출시킨다.

    false → 아무 추가 동작도 하지 않는다.

위 로직 구현을 위해 SharedPreferences로 flag 관리하도록 했습니다.


2. 이어쓰기 알림설정 바텀시트

사용자의 알림 권한 설정 여부와 상관없이 무조건 이어쓰기 알림설정을 활성화하는 방향으로 구현한다.

  • 유저가 하나의 알림이라도 받고 있을 경우

    → 해당 시간과 동일하게 이어쓰기 알림설정 활성화

  • 유저가 어떤 알림도 받지 않고 있을 경우

    → 이어쓰기 알림 활성화 + 시간은 21:30 으로 설정

✨ PR Point

  1. FirstDraftLocalDataSource에서 SharedPreferences를 주입받아야해서 DataSourceModule에서 @BINDS로 진행했던 작업이 아닌 @provides를 사용해야 했습니다. TokenLocalDataSource는 별도의 모듈이 있었으나 이것 때문에 또 별도의 모듈을 만들어야하나 싶어서 일단 SharedPreferencesModule에 집어넣어놨는데.. 얘는 어떤 위치에 있으면 좋을까요?
  2. 어떤 Repository로 연결시켜야 할지 (Notification..? Diary..? User..?) 애매하다고 생각돼서 DataSource에서 바로 ViewModel로 연결하여 무작정 갖다쓰는 바람에 레이어 간 의존성이 조금 마음에 들지 않습니다. (원래 Clean Architecture를 사용하고 있진 않았지만 ..!)
  3. String 추출은 다음에 영어 앱 출시를 위해 공부해서 한번에 진행하겠습니다.

📸 ScreenShot/Video

268-draft.mp4

Summary by CodeRabbit

  • New Features

    • Added a popup bottom sheet in the Home screen to inform users about enabling draft alarm notifications, with options to enable or dismiss.
    • Introduced a toast message to confirm successful activation of the draft alarm notification.
    • Implemented local tracking for the first use and usage state of the draft feature, enhancing personalized user experience.
  • Enhancements

    • Improved exit dialog behavior in the diary writing screen to update draft usage status and navigate users appropriately.
  • Chores

    • Updated dependency management and internal state handling for notification and draft usage features.
    • Added new local data source and dependency injection qualifiers for managing draft-related preferences.
    • Refined notification request logic to disable draft alarm flag in time reminder notifications.
    • Consolidated local data source providers into a single module and removed redundant modules.

@SYAAINN SYAAINN requested a review from MoonsuKang June 4, 2025 08:34
@coderabbitai
Copy link

coderabbitai bot commented Jun 4, 2025

"""

Walkthrough

This update introduces a feature to prompt users to enable draft notifications after their first use of the draft functionality. It adds new local data source interfaces and implementations, dependency injection qualifiers, and ViewModel logic to manage the draft usage state and notification prompt. UI components are updated to display a popup and toast for this feature.

Changes

Files/Paths Change Summary
.../di/qualifier/Qualifier.kt Added @TokenPrefs and @FirstDraftPrefs Dagger qualifier annotations.
.../di/SharedPreferencesModule.kt Added providers for FirstDraftPrefs SharedPreferences and FirstDraftLocalDataSource. Updated TokenPrefs annotation usage.
.../data/local/datasource/FirstDraftLocalDataSource.kt
.../data/local/datasourceimpl/FirstDraftLocalDataSourceImpl.kt
Introduced FirstDraftLocalDataSource interface and its implementation using SharedPreferences.
.../data/datastore/TokenDataStoreImpl.kt
.../di/TokenDataStoreModule.kt
Applied @TokenPrefs annotation to injected SharedPreferences in constructor and provider.
.../presentation/ui/home/screen/HomeScreen.kt Added popup bottom sheet and toast for draft alarm notification prompt in UI.
.../presentation/ui/home/screen/HomeViewModel.kt Added state and logic for managing first draft popup and enabling draft alarm notifications.
.../presentation/ui/writediary/screen/WriteDiaryScreen.kt Modified exit dialog logic to update draft usage state.
.../presentation/ui/writediary/screen/WriteDiaryViewModel.kt Injected DraftRepository and added updateDraftUsage() method.
.../presentation/ui/auth/timereminder/TimeReminderViewModel.kt Hardcoded isDraftAlarm to false in notification request payload.
.../di/LocalDataSourceModule.kt Added new module providing TokenDataStore and FirstDraftLocalDataSource with qualifiers.
.../di/RemoteDataSourceModule.kt Renamed class from DataSourceModule to RemoteDataSourceModule.
.../data/repositoryimpl/DraftRepositoryImpl.kt
.../domain/repository/DraftRepository.kt
Added DraftRepository interface and DraftRepositoryImpl implementation.
.../di/RepositoryModule.kt Added binding for DraftRepository to DraftRepositoryImpl.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant WriteDiaryScreen
    participant WriteDiaryViewModel
    participant DraftRepository
    participant FirstDraftLocalDataSource
    participant HomeScreen
    participant HomeViewModel
    participant NotificationRepository

    User->>WriteDiaryScreen: Dismiss Exit Dialog
    WriteDiaryScreen->>WriteDiaryViewModel: updateDraftUsage()
    WriteDiaryViewModel->>DraftRepository: getIsDraftUsed()
    DraftRepository->>FirstDraftLocalDataSource: get/set isDraftUsed, isFirstUse

    User->>HomeScreen: Navigates to Home
    HomeScreen->>HomeViewModel: Check showFirstDraftPopup
    HomeViewModel->>DraftRepository: getIsFirstUse()
    HomeScreen->>User: Show popup if first use

    User->>HomeScreen: Click "Enable Draft Alarm"
    HomeScreen->>HomeViewModel: enableDraftAlarm(context)
    HomeViewModel->>NotificationRepository: getNotificationInfo, sendDraftAlarmRequest
    HomeViewModel->>HomeScreen: Show toast on success
Loading

Assessment against linked issues

Objective Addressed Explanation
임시저장 최초 여부 검사, 최초 일시 바텀시트를 통한 이어쓰기 알림 설정 유도 (#268)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes detected.

Suggested labels

🌊 문수

Suggested reviewers

  • MoonsuKang

Poem

A bunny hops with code so bright,
Drafts and popups now in sight!
First use tracked, a toast appears,
Enable alarms, dismiss your fears.
With every hop, a feature new—
Clody’s home is fresh for you!
🐇✨
"""

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 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.
    • Explain this complex logic.
    • 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. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • 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 src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

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

Documentation and Community

  • 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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

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

127-180: Consider accessibility improvements for the popup.

The popup implementation is functionally correct and provides clear information to users. However, there's a potential accessibility concern:

The "다음에 하기" (Skip for now) text uses a generic clickable modifier without proper semantic information.

Consider improving accessibility:

Text(
    text = "다음에 하기",
    modifier = Modifier
-       .clickable(onClick = { homeViewModel.updateFirstDraftUse(false) })
+       .clickable(
+           onClick = { homeViewModel.updateFirstDraftUse(false) },
+           role = Role.Button
+       )
+       .semantics { contentDescription = "다음에 하기 버튼" }
        .padding(12.dp),
    color = ClodyTheme.colors.gray05,
    style = ClodyTheme.typography.body4Medium,
)

Also import the necessary semantics:

import androidx.compose.foundation.semantics.Role
import androidx.compose.foundation.semantics.contentDescription
import androidx.compose.foundation.semantics.semantics
app/src/main/java/com/sopt/clody/data/local/datasourceimpl/FirstDraftLocalDataSourceImpl.kt (1)

15-17: Consider renaming isFirstUse for clarity.

The property name isFirstUse is ambiguous. Based on the usage in HomeViewModel, it appears to track whether the user should see the first draft notification prompt. Consider a more descriptive name like shouldShowFirstDraftPrompt or hasSeenDraftPrompt (with inverted logic).

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 91e35d0 and f2ca24a.

📒 Files selected for processing (10)
  • app/src/main/java/com/sopt/clody/data/datastore/TokenDataStoreImpl.kt (1 hunks)
  • app/src/main/java/com/sopt/clody/data/local/datasource/FirstDraftLocalDataSource.kt (1 hunks)
  • app/src/main/java/com/sopt/clody/data/local/datasourceimpl/FirstDraftLocalDataSourceImpl.kt (1 hunks)
  • app/src/main/java/com/sopt/clody/di/SharedPreferencesModule.kt (2 hunks)
  • app/src/main/java/com/sopt/clody/di/TokenDataStoreModule.kt (2 hunks)
  • app/src/main/java/com/sopt/clody/di/qualifier/Qualifier.kt (1 hunks)
  • app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt (5 hunks)
  • app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt (4 hunks)
  • app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryScreen.kt (1 hunks)
  • app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryViewModel.kt (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt (3)
app/src/main/java/com/sopt/clody/presentation/ui/component/popup/ClodyPopupBottomSheet.kt (1)
  • ClodyPopupBottomSheet (28-77)
app/src/main/java/com/sopt/clody/presentation/ui/component/button/ClodyButton.kt (1)
  • ClodyButton (14-43)
app/src/main/java/com/sopt/clody/presentation/ui/component/toast/ClodyToastMessage.kt (1)
  • ClodyToastMessage (27-66)
app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt (3)
app/src/main/java/com/sopt/clody/presentation/ui/setting/notificationsetting/screen/NotificationSettingViewModel.kt (2)
  • getNotificationInfo (59-91)
  • getTokenFromPreferences (190-193)
app/src/main/java/com/sopt/clody/data/repositoryimpl/NotificationRepositoryImpl.kt (1)
  • getNotificationInfo (14-17)
app/src/main/java/com/sopt/clody/presentation/ui/auth/timereminder/TimeReminderViewModel.kt (1)
  • getTokenFromPreferences (75-78)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (15)
app/src/main/java/com/sopt/clody/di/SharedPreferencesModule.kt (3)

5-8: Clean dependency injection setup for the first draft feature.

The new imports and qualifier usage follow proper DI patterns and maintain clear separation between different SharedPreferences instances.


20-20: Good addition of @TokenPrefs qualifier for consistency.

This ensures proper disambiguation between different SharedPreferences instances in the dependency injection graph.


27-37: Well-structured provider methods for first draft functionality.

The naming convention and dependency chain from SharedPreferences to FirstDraftLocalDataSource is clear and follows good practices.

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

141-145: Excellent integration of draft usage tracking.

The placement of updateDraftUsage() call in the dismiss callback is logical - it properly tracks when users choose to save their work as a draft before exiting.

app/src/main/java/com/sopt/clody/data/datastore/TokenDataStoreImpl.kt (1)

4-4: Proper qualifier usage for dependency injection consistency.

The addition of @TokenPrefs qualifier aligns with the refactored dependency injection setup and ensures the correct SharedPreferences instance is injected.

Also applies to: 8-8

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

10-10: Clean integration of FirstDraftLocalDataSource dependency.

The import and constructor injection follow proper patterns and maintain the ViewModel's dependency management approach.

Also applies to: 26-26


174-179:

Details

✅ Verification successful

Verify the logic for isFirstUse flag setting.

The current implementation sets isFirstUse = true every time isDraftUsed is false. This might not align with the intended behavior if isFirstUse should only be set once on the very first draft usage and then remain true.

Please verify that this logic correctly implements the first draft detection requirement. Consider whether isFirstUse should be set only once or reset for each new draft session.


🏁 Script executed:

#!/bin/bash
# Description: Check how isFirstUse is consumed in the codebase to understand intended behavior
# Expected: Find usages that show whether isFirstUse should be set once or multiple times

echo "Searching for isFirstUse property usage..."
rg -A 3 -B 3 "isFirstUse"

echo -e "\nSearching for FirstDraftLocalDataSource interface definition..."
ast-grep --pattern $'interface FirstDraftLocalDataSource {
  $$$
}'

Length of output: 5282


No action needed on isFirstUse logic

The flag is only set when isDraftUsed is false, and since isDraftUsed is persisted in SharedPreferences, this happens only once—exactly as intended. The HomeViewModel’s updateFirstDraftUse() call then clears isFirstUse after the popup is shown, preventing it from re-triggering.

app/src/main/java/com/sopt/clody/di/qualifier/Qualifier.kt (1)

1-12: LGTM! Clean qualifier implementation for dependency injection.

The qualifiers are properly implemented following Dagger best practices. Using BINARY retention is appropriate for runtime dependency injection, and the separation between token and first draft preferences will enable type-safe injection throughout the app.

app/src/main/java/com/sopt/clody/data/local/datasource/FirstDraftLocalDataSource.kt (1)

8-11: Well-designed interface for draft state management.

The interface provides a clean abstraction with clear property names and good documentation. Using var properties is appropriate for this SharedPreferences-backed data source pattern.

app/src/main/java/com/sopt/clody/di/TokenDataStoreModule.kt (1)

18-18: Correct application of the new qualifier system.

Adding the @TokenPrefs qualifier ensures the correct SharedPreferences instance is injected, preventing potential conflicts with other preference instances and making the dependency injection more explicit.

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

6-6: New imports properly organized for UI enhancements.

The imported components (ClodyButton, ClodyPopupBottomSheet, ClodyToastMessage) are appropriately added to support the new first draft notification feature.

Also applies to: 9-9, 14-14, 21-21, 24-24, 28-28, 33-33, 35-37


67-69: State management correctly implemented for new UI features.

The new state flows for showFirstDraftPopup and draftAlarmEnableToast are properly collected and will reactively update the UI when changed by the ViewModel.


182-191: Toast implementation provides good user feedback.

The toast correctly confirms the draft alarm enablement with appropriate styling, duration (3 seconds), and auto-dismiss functionality.


263-263: Clean formatting improvement for empty when branches.

The explicit empty block syntax {} improves readability compared to having completely empty branches.

Also applies to: 291-291

app/src/main/java/com/sopt/clody/data/local/datasourceimpl/FirstDraftLocalDataSourceImpl.kt (1)

1-23: LGTM! Clean SharedPreferences implementation.

The implementation follows Android best practices for SharedPreferences usage with proper dependency injection and encapsulation.

Comment on lines +206 to +213
fun enableDraftAlarm(context: Context) {
viewModelScope.launch {
val fcmToken = getFcmToken(context) ?: return@launch
val notificationInfo = getNotificationInfo() ?: return@launch
val request = buildDraftAlarmRequest(notificationInfo, fcmToken)
sendDraftAlarmRequest(request)
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add network availability check for consistency.

The enableDraftAlarm function makes API calls but doesn't check network availability, unlike other functions in this ViewModel (loadCalendarData, loadDailyDiariesData). Add a network check for consistency and better error handling.

 fun enableDraftAlarm(context: Context) {
     viewModelScope.launch {
+        if (!networkUtil.isNetworkAvailable()) {
+            _draftAlarmChangeState.value = NotificationChangeState.Failure("네트워크 연결을 확인해주세요.")
+            return@launch
+        }
         val fcmToken = getFcmToken(context) ?: return@launch
         val notificationInfo = getNotificationInfo() ?: return@launch
         val request = buildDraftAlarmRequest(notificationInfo, fcmToken)
         sendDraftAlarmRequest(request)
     }
 }
📝 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
fun enableDraftAlarm(context: Context) {
viewModelScope.launch {
val fcmToken = getFcmToken(context) ?: return@launch
val notificationInfo = getNotificationInfo() ?: return@launch
val request = buildDraftAlarmRequest(notificationInfo, fcmToken)
sendDraftAlarmRequest(request)
}
}
fun enableDraftAlarm(context: Context) {
viewModelScope.launch {
if (!networkUtil.isNetworkAvailable()) {
_draftAlarmChangeState.value = NotificationChangeState.Failure("네트워크 연결을 확인해주세요.")
return@launch
}
val fcmToken = getFcmToken(context) ?: return@launch
val notificationInfo = getNotificationInfo() ?: return@launch
val request = buildDraftAlarmRequest(notificationInfo, fcmToken)
sendDraftAlarmRequest(request)
}
}
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt
around lines 206 to 213, the enableDraftAlarm function lacks a network
availability check before making API calls. Add a check for network connectivity
at the start of the function, similar to the approach used in loadCalendarData
and loadDailyDiariesData, and handle the case where the network is unavailable
by returning early or notifying the user appropriately.

Comment on lines +215 to +222
private suspend fun getFcmToken(context: Context): String? {
val token = getTokenFromPreferences(context)
if (token.isNullOrBlank()) {
_draftAlarmChangeState.value = NotificationChangeState.Failure("FCM Token을 가져오는데 실패했습니다.")
return null
}
return token
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Implement token retrieval locally to reduce coupling.

Instead of importing getTokenFromPreferences from a companion object, implement it locally like other ViewModels in the codebase do (see TimeReminderViewModel and NotificationSettingViewModel).

-private suspend fun getFcmToken(context: Context): String? {
-    val token = getTokenFromPreferences(context)
+private fun getFcmToken(context: Context): String? {
+    val sharedPreferences = context.getSharedPreferences("fcm_prefs", Context.MODE_PRIVATE)
+    val token = sharedPreferences.getString("fcm_token", null)
     if (token.isNullOrBlank()) {
         _draftAlarmChangeState.value = NotificationChangeState.Failure("FCM Token을 가져오는데 실패했습니다.")
         return null
     }
     return token
 }

Also note that this function doesn't need to be suspend as it doesn't perform any suspending operations.

📝 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 suspend fun getFcmToken(context: Context): String? {
val token = getTokenFromPreferences(context)
if (token.isNullOrBlank()) {
_draftAlarmChangeState.value = NotificationChangeState.Failure("FCM Token을 가져오는데 실패했습니다.")
return null
}
return token
}
private fun getFcmToken(context: Context): String? {
val sharedPreferences = context.getSharedPreferences("fcm_prefs", Context.MODE_PRIVATE)
val token = sharedPreferences.getString("fcm_token", null)
if (token.isNullOrBlank()) {
_draftAlarmChangeState.value = NotificationChangeState.Failure("FCM Token을 가져오는데 실패했습니다.")
return null
}
return token
}
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt
around lines 215 to 222, the getFcmToken function currently calls
getTokenFromPreferences from a companion object, creating unnecessary coupling,
and is marked suspend without needing to be. Refactor by implementing the token
retrieval logic locally within this ViewModel, similar to the approach in
TimeReminderViewModel and NotificationSettingViewModel, and remove the suspend
modifier since no suspending operations are performed.

@SYAAINN SYAAINN self-assigned this Jun 5, 2025
@SYAAINN SYAAINN added ✨ FEAT 기능 개발 🔥 민재 민재 labels Jun 5, 2025
Copy link
Contributor

@MoonsuKang MoonsuKang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

민재야 가보자 가보자잇🚀

private val diaryRepository: DiaryRepository,
private val notificationRepository: NotificationRepository,
private val networkUtil: NetworkUtil,
private val firstDraftLocalDataSource: FirstDraftLocalDataSource,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2
이렇게 DataSource를 가져다써도 큰 문제는 없을 것 같긴 한데
viewmodel - repository pattern을 지키고 싶으면 아래와 같은 방법으로도 할 수 있을 것 같아요

interface DraftRepository {
    fun isFirstUse(): Boolean
    fun setFirstUse(value:Boolean)
}

class DraftRepositoryImpl @Inject Constructor
override fun isFirstUse(): Boolean= firstDraftLocalDataSource.isFirstUse

Copy link
Contributor Author

@SYAAINN SYAAINN Jun 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사실 DataSource를 가져다 쓰는 구조는 지금까지의 코드 구조(Repository Pattern)를 많이 해치는 거 같아 조금 찝찝하긴합니다. 그래서 패턴을 따르도록 개선을 해야할 것 같기는 한데,

여기서 고민이 되는 것은 현재 FirstDraftLocalDataSource는 1. 일기2. 임시저장하는 기능 처음 사용할 경우 3. 알림 설정을 돕기 위한 기능이다보니 .. DiaryRepository, NotificationRepository, DraftRepository(신규 생성) 중 어디로 들어가야할 지에 대한 것입니다!

DraftRepository를 새로 만들기에는 기존 두개에 충분히 들어갈만 하지 않을까하는 생각이 들고, Repository의 개수가 늘어날수록 의미가 퇴색된다고 생각이 돼서 혹시 어디로 들어가는게 좋을 것 같다고 생각하시나요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

흠...저는 Repository구분을 할때 "책임" 단위로 분리를 해는 방법이 좋은 방법이라고 생각합니다.
그래서 의미 있는 독립된 도메인이라면 Repository 수는 늘어나도 괜찮다고 생각하거덩여.
DiaryRepository, NotificationRepository 에 넣는 케이스일 경우 서버API와 로컬 SharedPreferences가 섞이는 이슈가 발생 하겠죠 아마도...?
FirstDraftLocalDataSource는 기기 내부적 플래그 역할을 해서 독립적인 도메인의 의미를 갖는다. 라고 생각해서 분리하는 쪽으로 생각을 했습니다...!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DraftRepository를 만들면 뭔가 이름상 임시저장 조회/쓰기 API도 이쪽으로 옮겨야하지 않을까요? 이름을 바꿔야하나 ㅜ

제가 이해한 Repository Pattern으로는 로컬과 서버API가 섞이는 것은 필연적이라고 생각을 합니다. 데이터의 출처와 관계없이 하나의 Repository(저장소)에서 오는 data라고 속이는(?) 패턴이라고 생각해서요! 섞이면 발생하는 문제가 있을까요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 맞다 리포지토리는 local, remote datasource가 섞여도 된다는걸 간과하고 있었습니다ㅋㅋㅋ...
말씀하신대로 "임시"fetch/save를 아예 분리 할까요 그냥?

class WriteDiaryViewModel @Inject constructor(
private val diaryRepository: DiaryRepository,
private val networkUtil: NetworkUtil,
private val firstDraftLocalDataSource: FirstDraftLocalDataSource,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위와 동일합니다잉

)

if (showFirstDraftPopup) {
ClodyPopupBottomSheet(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P4
Route는 UI 상태 구독과 조립만 담당하는게 좋지 않을까요? 물론 방학시즌에 홈화면 개편이 들어갈 예정이니 요건 고려해보시고 바꿔주시면 될 것 같습니다~

Comment on lines +182 to +183
if (draftAlarmEnableToast) {
ClodyToastMessage(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P4
요 친구를 Route에 둔 이유가 있을까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI 상태 구독과 더불어 SideEffect 적인 것들은 Route에서 처리하는 것이 자연스럽다고 생각했습니다.

추후 MVI 구조를 적용한다고 생각했을때,
Screen에서 SideEffect를 트리거 -> Route에서 해당 SideEffect를 수집해서 토스트 노출

이 토스트 코드의 책임(?)을 다시 Screen에서 지는게 왔다리갔다리..? 같이 생각이 돼서 UiState가 성공한 화면에 대해서만 Screen에 정의하고 그 외는 Route에 위치시켰습니다. 그런데 지금 생각하니 또 확신이 잘 안서기는 하네요. HomeScreen이 리팩토링할 부분이 워낙 많아서 ..! Route에서 져야할 책임과 Screen에서 져야하는 책임은 무엇일까요?

Copy link

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (1)
app/src/main/java/com/sopt/clody/domain/repository/DraftRepository.kt (1)

3-8: Add documentation and consider more domain-focused naming.

The interface lacks documentation explaining what these boolean flags represent. Additionally, consider if the interface name DraftRepository accurately reflects its responsibility of managing usage flags rather than draft content.

Consider adding KDoc documentation:

+/**
+ * Repository for managing draft usage state and first-time use tracking.
+ */
 interface DraftRepository {
+    /**
+     * Checks if draft functionality has been used before.
+     * @return true if draft has been used, false otherwise
+     */
     fun getIsDraftUsed(): Boolean
+    /**
+     * Sets the draft usage state.
+     * @param state true to mark draft as used, false otherwise
+     */
     fun setIsDraftUsed(state: Boolean)
+    /**
+     * Checks if this is the first time using the feature.
+     * @return true if first use, false otherwise
+     */
     fun getIsFirstUse(): Boolean
+    /**
+     * Sets the first use state.
+     * @param state true to mark as first use, false otherwise
+     */
     fun setIsFirstUse(state: Boolean)
 }
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 79c9a3e and 116ac9e.

📒 Files selected for processing (5)
  • app/src/main/java/com/sopt/clody/data/repositoryimpl/DraftRepositoryImpl.kt (1 hunks)
  • app/src/main/java/com/sopt/clody/di/RepositoryModule.kt (2 hunks)
  • app/src/main/java/com/sopt/clody/domain/repository/DraftRepository.kt (1 hunks)
  • app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt (4 hunks)
  • app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryViewModel.kt (3 hunks)
✅ Files skipped from review due to trivial changes (1)
  • app/src/main/java/com/sopt/clody/data/repositoryimpl/DraftRepositoryImpl.kt
🚧 Files skipped from review as they are similar to previous changes (2)
  • app/src/main/java/com/sopt/clody/presentation/ui/writediary/screen/WriteDiaryViewModel.kt
  • app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (1)
app/src/main/java/com/sopt/clody/di/RepositoryModule.kt (1)

7-7: LGTM: Consistent dependency injection pattern.

The import additions and repository binding follow the established pattern used by other repositories in this module. The implementation correctly uses @Binds and @Singleton annotations for proper dependency injection.

Also applies to: 15-15, 70-74

Comment on lines +1 to +8
package com.sopt.clody.domain.repository

interface DraftRepository {
fun getIsDraftUsed(): Boolean
fun setIsDraftUsed(state: Boolean)
fun getIsFirstUse(): Boolean
fun setIsFirstUse(state: Boolean)
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider making repository methods suspend functions for better async handling.

The synchronous methods may cause ANR issues if the underlying SharedPreferences operations block the main thread. Consider making these suspend functions to align with modern Android development practices.

Apply this diff to make the methods asynchronous:

 interface DraftRepository {
-    fun getIsDraftUsed(): Boolean
-    fun setIsDraftUsed(state: Boolean)
-    fun getIsFirstUse(): Boolean
-    fun setIsFirstUse(state: Boolean)
+    suspend fun getIsDraftUsed(): Boolean
+    suspend fun setIsDraftUsed(state: Boolean)
+    suspend fun getIsFirstUse(): Boolean
+    suspend fun setIsFirstUse(state: Boolean)
 }
📝 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
package com.sopt.clody.domain.repository
interface DraftRepository {
fun getIsDraftUsed(): Boolean
fun setIsDraftUsed(state: Boolean)
fun getIsFirstUse(): Boolean
fun setIsFirstUse(state: Boolean)
}
package com.sopt.clody.domain.repository
interface DraftRepository {
suspend fun getIsDraftUsed(): Boolean
suspend fun setIsDraftUsed(state: Boolean)
suspend fun getIsFirstUse(): Boolean
suspend fun setIsFirstUse(state: Boolean)
}
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/domain/repository/DraftRepository.kt lines 1
to 8, the repository methods are synchronous which can block the main thread and
cause ANR issues. Modify all methods in the DraftRepository interface to be
suspend functions by adding the suspend modifier before each function
declaration to enable asynchronous execution and better align with modern
Android development practices.

Copy link
Contributor

@MoonsuKang MoonsuKang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

private val diaryRepository: DiaryRepository,
private val notificationRepository: NotificationRepository,
private val networkUtil: NetworkUtil,
private val firstDraftLocalDataSource: FirstDraftLocalDataSource,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 맞다 리포지토리는 local, remote datasource가 섞여도 된다는걸 간과하고 있었습니다ㅋㅋㅋ...
말씀하신대로 "임시"fetch/save를 아예 분리 할까요 그냥?

@SYAAINN SYAAINN merged commit e3d4eaa into develop Jun 6, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

✨ FEAT 기능 개발 🔥 민재 민재

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] 이어쓰기 알림 설정을 유도하는 기능을 구현합니다.

2 participants