[Feat] 사용하지 않는 Challenge 관련 기능 제거 #335
Conversation
WalkthroughReplaced UsageGoal/Challenge concepts with AppUsageGoal and removed challenge-related networking, models, and UI. Simplified repositories and use cases to local, per-app goal management. Updated ViewModels, adapters, and fragments to reflect app settings focus. Adjusted locking logic to per-app timing. Updated navigation, strings, and layouts accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant VM as MainViewModel
participant Repo as UsageGoalsRepository
participant Local as UsageGoalsLocalDataSource
participant Stats as UsageStatsProvider
VM->>Repo: getUsageGoals()
Repo->>Local: getUsageGoal()
Local-->>Repo: Flow<AppUsageGoal>
Repo-->>VM: Flow<AppUsageGoal>
VM->>Stats: getUsageStatForPackages(AppUsageGoal.appGoals)
Stats-->>VM: Usage stats
VM->>UI: Render app goals and usage
sequenceDiagram
participant Svc as AppLockManger
participant Sys as SystemUsage
participant Timer as Timer
Sys-->>Svc: usageStats(package)
Svc->>Svc: find current AppUsageGoal.App
Svc->>Svc: calc appRemainingTime
alt remainingTime > 0
Svc->>Timer: startTimer(appRemainingTime, package)
Timer-->>Svc: timeout
Svc->>Sys: lock(package)
else
Svc->>Sys: lock(package)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (42)
feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/appadd/time/SetAppGoalTimeFragment.kt (2)
41-46: Use TimeUnit for millis conversion to avoid potential Int overflow and improve readability.Current math uses Int intermediates; if the hour range increases later, overflow risk appears. Prefer TimeUnit and early Long conversion.
Apply this diff:
- binding.npUseTimeGoalHour.setOnValueChangedListener { _, _, newTime -> - viewModel.setGoalHour((newTime * 60 * 60 * 1000).toLong()) - } + binding.npUseTimeGoalHour.setOnValueChangedListener { _, _, newTime -> + viewModel.setGoalHour(TimeUnit.HOURS.toMillis(newTime.toLong())) + } ... - binding.npUseTimeGoalMinute.setOnValueChangedListener { _, _, newTime -> - viewModel.setGoalMin((newTime * 60 * 1000).toLong()) - } + binding.npUseTimeGoalMinute.setOnValueChangedListener { _, _, newTime -> + viewModel.setGoalMin(TimeUnit.MINUTES.toMillis(newTime.toLong())) + }Add this import at the top of the file:
import java.util.concurrent.TimeUnit
53-55: Apply FOCUS_BLOCK_DESCENDANTS to the minute picker as well for consistent UX.Right now only the hour picker is blocked from text focus.
npUseTimeGoalHour.descendantFocusability = NumberPicker.FOCUS_BLOCK_DESCENDANTS + npUseTimeGoalMinute.descendantFocusability = + NumberPicker.FOCUS_BLOCK_DESCENDANTSfeature/challenge/src/main/java/com/hmh/hamyeonham/challenge/appadd/AppAddViewPagerAdapter.kt (1)
13-17: Map position 1 explicitly and fail fast for out-of-range positions.Using an else makes it easy to mask future itemCount changes.
override fun createFragment(position: Int): Fragment { return when (position) { 0 -> AppSelectionFragment() - else -> SetAppGoalTimeFragment() + 1 -> SetAppGoalTimeFragment() + else -> throw IndexOutOfBoundsException("Unsupported position: $position (itemCount=$itemCount)") } }data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt (3)
6-6: Timber import is fine; confirm logging setup remains after network removals.Given the module-level dependency changes, please verify that Timber is still initialized in this app flavor so these logs don't silently drop. Also consider using a consistent tag for easier filtering.
13-17: Simplify no-opupdateDailyChallengeFailed()and deprecate in interfaceWe didn’t find any call sites besides the declaration and its default impl, so it’s safe to:
- In
DefaultChallengeRepository.kt(data/challenge/src/main/java/.../DefaultChallengeRepository.kt), replace therunCatching { … }block with an explicit success:override suspend fun updateDailyChallengeFailed(): Result<Unit> {return runCatching {Timber.d("Update daily challenge failed")}
Timber.d("updateDailyChallengeFailed() no-op")return Result.success(Unit)
- In
ChallengeRepository.kt(domain/challenge/src/main/java/.../ChallengeRepository.kt), mark the method deprecated and note planned removal:interface ChallengeRepository {- suspend fun updateDailyChallengeFailed(): Result
- @deprecated(
message = "No-op—will be removed in a future release",replaceWith = ReplaceWith(""), // no direct replacementlevel = DeprecationLevel.WARNING- )
- suspend fun updateDailyChallengeFailed(): Result
}Optional: Schedule removal of this method in the next major version. --- `35-37`: **Refactor deleteApps to avoid logging PII and return explicit success** The current implementation wraps a `Timber.d` call (including the `appCode` identifier) in `runCatching`, which both leaks an identifier in logs and can inadvertently suppress downstream work if logging ever throws. Since `deleteApps()` is effectively a no-op, update it to log a generic message and return success explicitly: ```diff --- a/data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt +++ b/data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt @@ -34,3 +34,5 @@ override suspend fun deleteApps(appCode: String): Result<Unit> { - return runCatching { Timber.d("Delete app with code: $appCode") } + Timber.d("deleteApps() no-op") + return Result.success(Unit) }Note: this method is invoked by
DeleteUsageGoalUseCase(domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/DeleteUsageGoalUseCase.kt), so it shouldn’t be deprecated—just refactored to remove sensitive logging.feature/main/src/main/java/com/hmh/hamyeonham/feature/main/home/UsageStaticsViewHolder.kt (2)
39-49: Remove redundant progress assignment and delete dead commented block.
- You already animate progress in onBind via setProgressWithAnimation; setting pbAppUsage.progress again here is redundant and may cause flicker or no-op animations.
- The tvAppTimeLeft block is intentionally disabled; prefer deleting commented-out code to reduce noise. Git history preserves it.
Apply this diff:
binding.run { - pbAppUsage.progress = usageStatusAndGoal.usedPercentage tvGoalTime.text = convertTimeToString(usageStatusAndGoal.goalTimeInMinute) -// tvAppTimeLeft.text = -// -// context.getSecondStrColoredString( -// firstStr = convertTimeToString(usageStatusAndGoal.timeLeftInMinute), -// secondStr = getString(context, R.string.all_left), -// color = com.hmh.hamyeonham.core.designsystem.R.color.gray1, -// ) }
3-11: Clean up unused imports after removing time-left rendering.getString and getSecondStrColoredString are no longer used once the time-left UI is removed.
import android.content.Context -import androidx.core.content.ContextCompat.getString import androidx.recyclerview.widget.RecyclerView import com.hmh.hamyeonham.common.context.getAppIconFromPackageName import com.hmh.hamyeonham.common.context.getAppNameFromPackageName -import com.hmh.hamyeonham.common.context.getSecondStrColoredString import com.hmh.hamyeonham.common.time.convertTimeToString import com.hmh.hamyeonham.common.view.setProgressWithAnimationfeature/main/src/main/res/layout/item_usagestatic.xml (1)
56-66: Remove the large XML comment block and clean up related commented-out codeThe XML
TextViewblock fortv_app_time_leftshould be deleted entirely. We also found commented-out references totvAppTimeLeftin the view holder—remove those to avoid dead code and confusion.• feature/main/src/main/res/layout/item_usagestatic.xml: delete lines 56–66 (the commented-out
<TextView>).
• feature/main/src/main/java/com/hmh/hamyeonham/feature/main/home/UsageStaticsViewHolder.kt: remove the commented-out lines referencingtvAppTimeLeft(around lines 43–45).feature/main/src/main/res/layout/item_usagestatic_total.xml (1)
53-61: Remove the Commented-Out Total Goal LabelNo references to
tv_total_goalor its binding remain in the codebase—only the commented lines themselves. You can safely delete the block and then verify that the layout spacing still looks correct.• File to update:
– feature/main/src/main/res/layout/item_usagestatic_total.xml (lines 53–61)feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/AppUsageGoalsViewHolder.kt (1)
15-20: Consider aligning model naming with the non-“Challenge” terminology.The ViewHolder now uses AppUsageGoalsViewHolder, but still depends on ChallengeUsageGoal. For consistency across the codebase (and to prevent cognitive overhead), consider renaming the UI model to an AppUsageGoal*-prefixed name (e.g., AppUsageGoalUiModel) in a follow-up.
core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/model/AppUsageGoal.kt (1)
3-10: Clarify time unit for goalTime or rename to goalTimeInMinute to avoid ambiguity.Downstream UI types and helpers (e.g., convertTimeToString with goalTimeInMinute) imply “minutes.” To prevent unit mismatches and future regressions, either:
- Rename goalTime → goalTimeInMinute, or
- Document the unit explicitly in KDoc.
Option A (rename for explicitness):
data class AppUsageGoal( val appGoals: List<App> = emptyList() ) { data class App( val packageName: String = "", - val goalTime: Long = 0 + val goalTimeInMinute: Long = 0 ) }Option B (keep name, add KDoc):
/** * App-level usage goals. * * @property appGoals List of per-app goals. * * Nested App: * @property packageName Android package name (e.g., "com.example.app"). * @property goalTime Time limit unit = minutes. Non-negative. */domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/Challenge.kt (1)
3-6: Avoid duplicating domain models; consider unifying with AppUsageGoal.AppIntroducing a separate AppGoal here can drift from the central domain model (core/domain/.../AppUsageGoal.App). Prefer reusing the domain type or adding a typealias to avoid divergence and future mapping boilerplate.
If this module must keep a local symbol for now, consider marking it deprecated to guide migration:
+@Deprecated( + message = "Use core.domain.usagegoal.model.AppUsageGoal.App instead", + replaceWith = ReplaceWith("AppUsageGoal.App") +) data class AppGoal( val appCode: String, val appGoalTime: Long, )Additionally, please document the unit for appGoalTime (e.g., milliseconds) to prevent misuse.
feature/main/src/main/java/com/hmh/hamyeonham/feature/main/home/UsageStaticsTotalViewHolder.kt (2)
29-37: Remove commented code or leave a clear TODO for future workThis block is fully commented out. If the “time left” UI is no longer part of the product, remove it and any now-unused imports to reduce noise. If it’s temporarily disabled, leave a TODO with owner and rationale.
39-45: Avoid double-setting progress; rely on the animated setter onlyYou set pbTotalUsage.progress here and also call setProgressWithAnimation in onBind. The direct set can neutralize or jump over the animation. Set it once via animation.
Apply:
tvTotalUsage.text = context.getString( R.string.total_used, convertMillisecondToString(totalModel.totalTimeInForeground) ) - pbTotalUsage.progress = totalModel.totalPercentagefeature/main/src/main/res/navigation/nav_graph.xml (2)
7-9: Update label to reflect App Usage Goals and use a string resourceThe destination now points to AppUsageGoalsFragment, but the label still says "Challenge Fragment". Align the label with the new UI and leverage the added string resource for localization.
Apply:
- android:label="Challenge Fragment" /> + android:label="@string/app_setting" />
7-7: Optional: Renamechallenge_desttoapp_usage_goals_destfor clarityTo keep your navigation IDs in sync with the renamed feature, you can update the ID and all its usages:
• feature/main/src/main/res/navigation/nav_graph.xml (line 7)
- android:id="@+id/challenge_dest" + android:id="@+id/app_usage_goals_dest"• feature/main/src/main/res/menu/bottom_nav_menu.xml (line 3)
- android:id="@+id/challenge_dest" + android:id="@+id/app_usage_goals_dest"• feature/main/src/main/java/com/hmh/hamyeonham/feature/main/MainActivity.kt (around line 75)
- R.id.challenge_dest -> binding.vpMain.setCurrentItem( + R.id.app_usage_goals_dest -> binding.vpMain.setCurrentItem( MainScreen.CHALLENGE.ordinal, falseDon’t forget to update any remaining references (navigation actions, tests, documentation) if you proceed with this rename.
feature/main/src/main/res/values/strings.xml (1)
21-21: Update “챌린지” wording across codebaseSeveral remaining instances of “챌린지” were found after the initial change. Please update these to the new “앱 사용 목표” (or appropriate wording) for consistency:
- feature/main/src/main/res/values/strings.xml
• dialog_description_unlock_package (already updated)- feature/onboarding/src/main/java/com/hmh/hamyeonham/feature/onboarding/viewmodel/SelectOnBoardingDataViewModel.kt
• Line 34:"챌린지 기간을 선택해주세요"
• Line 35:"첫 챌린지로 가볍게 도전하기 좋은 7일을 추천해요"- core/common/src/main/res/layout/dialog_common_two_button.xml
• tools:text at Line 26:"챌린지를 등록하시겠습니까?"
• tools:text at Line 41:"챌린지를 등록하면 포인트를 얻을 수 있어요"- core/common/src/main/res/layout/dialog_common_one_button.xml
• tools:text at Line 26:"챌린지를 등록하시겠습니까?"
• tools:text at Line 42:"챌린지를 등록하면 포인트를 얻을 수 있어요"- feature/main/src/main/java/com/hmh/hamyeonham/feature/main/home/UsageStaticsTotalViewHolder.kt
• Comments on Lines 51 (“// 챌린지 성공한 경우”) and 56 (“// 챌린지 실패한 경우”)Updating these will ensure the entire app reflects the new “앱 사용 목표” terminology.
domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetTotalUsageStatsUseCase.kt (2)
44-46: Deduplicate package names to avoid double-countingIf
appGoalscontains duplicates, you may over-sum usage. Safeguard withdistinct().- private fun getPackageNamesFromUsageGoals(appUsageGoals: AppUsageGoal): List<String> { - return appUsageGoals.appGoals.map { it.packageName } - } + private fun getPackageNamesFromUsageGoals(appUsageGoals: AppUsageGoal): List<String> { + return appUsageGoals.appGoals + .map { it.packageName } + .distinct() + }
22-29: Rename local variable for clarity (singular vs. plural)
usageGoalsis a singleAppUsageGoal. Rename to reduce cognitive load.- usageGoalsRepository.getUsageGoals().first().let { usageGoals -> + usageGoalsRepository.getUsageGoals().first().let { appUsageGoal -> val usageStatsForSelectedPackages = getUsageStatsForSelectedPackages( startTime, endTime, - getPackageNamesFromUsageGoals(usageGoals) + getPackageNamesFromUsageGoals(appUsageGoal) ) return calculateTotalUsage(usageStatsForSelectedPackages) }domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/model/UsageStatusAndGoal.kt (1)
32-41: Consider moving the extension to top-level for discoverabilityDefining
fun List<AppUsageGoal.App>.toApps()inside the companion of an inner type hides it. A top-level extension improves discoverability and reuse.Suggested top-level extension:
package com.hmh.hamyeonham.usagestats.model import com.hmh.hamyeonham.core.domain.usagegoal.model.AppUsageGoal fun List<AppUsageGoal.App>.toUsageStatusApps(): List(UsageStatusAndGoal.App) = map { UsageStatusAndGoal.App( packageName = it.packageName, goalTime = it.goalTime ) }domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetUsageGoalsUseCase.kt (1)
11-13: Avoid suspend for a function that only returns a FlowReturning a cold Flow typically shouldn’t require a suspend function. This adds friction to call sites without benefit.
Apply if you also remove suspend from the repository’s getUsageGoals():
- suspend operator fun invoke(): Flow<AppUsageGoal> { + operator fun invoke(): Flow<AppUsageGoal> { return usageGoalsRepository.getUsageGoals() }domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/AddUsageGoalsUseCase.kt (2)
13-21: Surface failure or at least log itonSuccess ignores failure paths; callers get no signal. Consider returning Result or at minimum handling onFailure for observability.
Would you like me to draft a Result-returning version of this use case and update call sites in a follow-up PR?
1-12: Consider relocating this use case out of the challenge domainThis class is about adding usage goals, not challenges. Moving it under the usage-goal/usage-stats domain would reduce confusion post-challenge removal.
core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/repository/UsageGoalsRepository.kt (1)
8-8: Consider making getUsageGoals() non-suspendingA repository method that returns a Flow generally shouldn’t be suspend. This simplifies call sites and matches common Kotlin Flow patterns.
Apply if acceptable across call sites:
- suspend fun getUsageGoals(): Flow<AppUsageGoal> + fun getUsageGoals(): Flow<AppUsageGoal>domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetUsageStatsListUseCase.kt (2)
32-41: Avoid O(n^2) lookup by pre-indexing usage by packageUsing find inside map makes it O(n^2) over app count. Precompute a map for O(n).
- val usageStatusAndGoal = UsageStatusAndGoal( + val usageByPackage = usageForSelectedApps.associateBy { it.packageName } + + val usageStatusAndGoal = UsageStatusAndGoal( totalTimeInForeground = totalUsage, apps = usageGoal.appGoals.map { UsageStatusAndGoal.App( packageName = it.packageName, - usageTime = usageForSelectedApps.find { usageStatus -> usageStatus.packageName == it.packageName }?.totalTimeInForeground ?: 0, + usageTime = usageByPackage[it.packageName]?.totalTimeInForeground ?: 0, goalTime = it.goalTime, ) }.sortedByDescending { it.usageTime }, )
55-56: Minor naming nit: parameter isn’t a listRename appUsageGoalList to appUsageGoal for clarity.
- private fun getSelectedPackageList(appUsageGoalList: AppUsageGoal): List<String> = - appUsageGoalList.appGoals.map { it.packageName }.distinct() + private fun getSelectedPackageList(appUsageGoal: AppUsageGoal): List<String> = + appUsageGoal.appGoals.map { it.packageName }.distinct()feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/AppUsageGoalsViewModel.kt (4)
34-39: Make units explicit and avoid implicit numeric widening inMAX_DELETABLE.
usageTimeis likelyLong(ms). The constant isInt, which relies on implicit conversion in comparisons. Prefer aLongwith explicit unit in the name for clarity.Apply:
companion object { - const val MAX_DELETABLE = 300000 + const val MAX_DELETABLE_MILLIS: Long = 300_000L } - val isDeletable: Boolean = usageStatusAndGoal.usageTime <= MAX_DELETABLE + val isDeletable: Boolean = usageStatusAndGoal.usageTime <= MAX_DELETABLE_MILLIS
70-79: Capture and surface failures when adding usage goals.
runCatching { addUsageGoalsUseCase(apps) }only tracks success. Logging/telemetry on failure helps diagnose issues and improves UX feedback.You could extend it like:
runCatching { addUsageGoalsUseCase(apps) - }.onSuccess { + }.onSuccess { AmplitudeUtils.trackEventWithProperties("complete_add_new") - } + }.onFailure { + AmplitudeUtils.trackEventWithProperties("add_new_failed") + // TODO: expose error state or snackbar event to UI if applicable + }
81-93: Likewise, handle failures when deleting a usage goal.Mirror success/failure tracking for deletes to aid observability.
Proposed change:
runCatching { deleteUsageGoalUseCase(usageStatusAndGoal.packageName) - }.onSuccess { + }.onSuccess { AmplitudeUtils.trackEventWithProperties("click_delete_complete") - } + }.onFailure { + AmplitudeUtils.trackEventWithProperties("delete_failed") + // TODO: propagate error to UI if needed + }
19-23: Remove or wireChallengeState.appUsageGoals– it’s never updated or consumed
appUsageGoalsis declared inChallengeStatebut isn’t referenced elsewhere in this feature module. To avoid dead state and confusion withusageStatusAndGoals, please choose one of the following:
- Wire it up in your ViewModel (e.g. add an
updateAppUsageGoals(...)method and callupdateState { copy(appUsageGoals = …) }when goals load), or- Remove the
appUsageGoalsproperty entirely until it’s actually needed by the UI.data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/mapper/UsageGoalMapper.kt (2)
6-6: Naming nit:toUsageAppGoalcan be clearer.The name reads as if it's converting to "UsageAppGoal". Consider
toAppUsageGoalApp()ortoAppGoal()to align withAppUsageGoal.Appand mirrortoUsageGoalEntity.
10-10: Consider tests for the mapping round-trip.A small unit test asserting
entity -> domain -> entitypreserves(packageName, goalTime)would guard regressions.I can draft a quick test if you confirm the test module and runner setup.
core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/MainState.kt (1)
9-11: Mutable property inside data class affects equality; confirm intent.
permissionGrantedisvar, impactingequals/hashCode/copy. If you're mutating this at runtime, this is fine. Otherwise, prefervaland updates viacopy.core/service/src/main/java/com/hmh/hamyeonham/core/lock/AppLockManger.kt (2)
81-89: Prefer a dedicated scope overProcessLifecycleOwnerfor service-scoped timers.This class is
@ServiceScoped, yet the timer runs onProcessLifecycleOwner.get().lifecycleScope. Using a dedicatedCoroutineScopetied to this manager/service gives you clearer lifecycle control and avoids coupling with process lifecycle.Example:
class AppLockManger @Inject constructor( @@ ) { - private var checkUsageJob: Job? = null - private var timerJob: Job? = null + private var checkUsageJob: Job? = null + private var timerJob: Job? = null + private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) @@ - timerJob = ProcessLifecycleOwner.get().lifecycleScope.launch { + timerJob = scope.launch { delay(appRemainingTime) moveToLock(packageName) } } @@ fun onDestroy() { releaseCheckUsageJob() releaseTimerJob() + scope.cancel() }
36-41: Nit: naming ofhandleFocusedChangedEventandGetIsUnLockUseCase.Minor readability nit: consider
handleFocusChangedEventand aligning the boolean semantics ofGetIsUnLockUseCasewith the predicate (isUnlockedvsisLocked) to reduce double negatives.feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/AppUsageGoalsAdapter.kt (1)
37-39: Avoid safe-cast on the only view type; bind directly or enforce via sealed hierarchy.Since this adapter creates only
AppUsageGoalsViewHolder.UsageGoalsViewHolder, the safe cast can conceal errors by failing silently.Apply:
- override fun onBindViewHolder(holder: AppUsageGoalsViewHolder, position: Int) { - (holder as? AppUsageGoalsViewHolder.UsageGoalsViewHolder)?.bind(getItem(position)) - } + override fun onBindViewHolder(holder: AppUsageGoalsViewHolder, position: Int) { + (holder as AppUsageGoalsViewHolder.UsageGoalsViewHolder).bind(getItem(position)) + }Optional: consider stable IDs for smoother diffing:
init { setHasStableIds(true) } override fun getItemId(position: Int) = getItem(position).usageStatusAndGoal.packageName.hashCode().toLong()core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/MainViewModel.kt (1)
109-113: Typo: Rename setUsageGaols -> setUsageGoalsMinor naming typo. Renaming improves readability and consistency with the domain model.
Apply within this file:
- private fun setUsageGaols(appUsageGoals: AppUsageGoal) { + private fun setUsageGoals(appUsageGoals: AppUsageGoal) { updateState { copy(appUsageGoals = appUsageGoals) } }Also update the call site in this file:
// In getUsageGoalAndStatList() usageGoalsRepository.getUsageGoals().collect { setUsageGoals(it) // rename here getTodayTimeAndSetUsageStatsList() }feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/AppUsageGoalsFragment.kt (1)
146-149: Avoid unsafe adapter cast by holding a typed referenceCasting the RecyclerView adapter each bind is brittle. Prefer holding a typed adapter property (e.g., private lateinit var appUsageGoalsAdapter: AppUsageGoalsAdapter) and assign it once when initializing the RecyclerView, then reference it directly here.
data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/datasource/local/UsageGoalsLocalDataSource.kt (1)
17-27: Simplify flow transformation; remove ExperimentalCoroutinesApiUse map instead of flatMapConcat+flow. This removes the ExperimentalCoroutinesApi opt-in and is more idiomatic.
Apply:
- @OptIn(ExperimentalCoroutinesApi::class) - fun getUsageGoal(): Flow<AppUsageGoal> { - return usageGoalsDao.getUsageGoal() - .flatMapConcat { goalsList -> - flow { - val result = AppUsageGoal( - appGoals = goalsList.map { it.toUsageAppGoal() } - ) - emit(result) - } - } - } + fun getUsageGoal(): Flow<AppUsageGoal> { + return usageGoalsDao.getUsageGoal() + .map { goalsList -> + AppUsageGoal(appGoals = goalsList.map { it.toUsageAppGoal() }) + } + }Also add the missing import at the top of this file:
import kotlinx.coroutines.flow.mapdata/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/repository/DefaultUsageGoalsRepository.kt (2)
13-16: Remove unused constructor deps (cleanup)usageGoalsDao and usageTotalGoalDao aren’t used anymore. Consider removing them to avoid dead dependencies and ease DI maintenance.
43-46: Drop redundant suspend from getUsageGoals
SinceFlowis cold and can be returned directly, you can remove thesuspendmodifier from the repository API and its implementation to reduce confusion and boilerplate. Update the signatures as follows:
- core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/repository/UsageGoalsRepository.kt
interface UsageGoalsRepository {suspend fun getUsageGoals(): Flow<AppUsageGoal>
fun getUsageGoals(): Flow<AppUsageGoal> // …
- data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/repository/DefaultUsageGoalsRepository.kt
class DefaultUsageGoalsRepository(...) : UsageGoalsRepository {override suspend fun getUsageGoals(): Flow<AppUsageGoal> {
override fun getUsageGoals(): Flow<AppUsageGoal> { return usageGoalsLocalDataSource.getUsageGoal() }Optionally, you can also remove the now-redundant
suspendfrom your domain use case that simply returns the flow:
- domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetUsageGoalsUseCase.kt
class GetUsageGoalsUseCase(...) {- suspend operator fun invoke(): Flow {
- operator fun invoke(): Flow {
return usageGoalsRepository.getUsageGoals()
}
}All existing call sites (e.g. in other use-cases, `DefaultDeviceRepository`, `MainViewModel`) already invoke `.first()`, `.collect()`, etc., within coroutine scopes, so they’ll continue to work without any changes. </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **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 sources in your CodeRabbit configuration. <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between be69df6e7a927f741deac8d93c09d2d9c1b291cb and d49a74e56ca25a560cdea5f742a20f04aa76486a. </details> <details> <summary>📒 Files selected for processing (69)</summary> * `core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/model/AppUsageGoal.kt` (1 hunks) * `core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/model/ChallengeStatus.kt` (0 hunks) * `core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/repository/UsageGoalsRepository.kt` (1 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/AppCodeRequest.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/ChallengeService.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/AppsRequest.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeFinishResponse.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeResponse.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeWithUsageRequest.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeWithUsageResponse.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/NewChallengeRequest.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/di/ChallengeModule.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/di/UsageGoalModule.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/usagegoal/DailyChallengeService.kt` (0 hunks) * `core/network/src/main/java/com/hmh/hamyeonham/core/network/usagegoal/model/UsageGoalResponse.kt` (0 hunks) * `core/service/src/main/java/com/hmh/hamyeonham/core/lock/AppLockManger.kt` (2 hunks) * `core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/HomeItem.kt` (0 hunks) * `core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/MainState.kt` (1 hunks) * `core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/MainViewModel.kt` (2 hunks) * `data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/AppMapper.kt` (0 hunks) * `data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/ChallengeMapper.kt` (0 hunks) * `data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/ChallengeStatusMapper.kt` (0 hunks) * `data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/NewChallengeMapper.kt` (0 hunks) * `data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt` (1 hunks) * `data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/datasource/local/UsageGoalsLocalDataSource.kt` (3 hunks) * `data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/datasource/remote/UsageGoalsRemoteDataSource.kt` (0 hunks) * `data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/mapper/UsageGoalMapper.kt` (1 hunks) * `data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/repository/DefaultUsageGoalsRepository.kt` (1 hunks) * `domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/Challenge.kt` (1 hunks) * `domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/ChallengeWithUsageInput.kt` (0 hunks) * `domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/NewChallenge.kt` (0 hunks) * `domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/repository/ChallengeRepository.kt` (0 hunks) * `domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/AddUsageGoalsUseCase.kt` (2 hunks) * `domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/NewChallengeUseCase.kt` (0 hunks) * `domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/model/UsageStatusAndGoal.kt` (2 hunks) * `domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetTotalUsageGoalUseCase.kt` (1 hunks) * `domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetTotalUsageStatsUseCase.kt` (2 hunks) * `domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetUsageGoalsUseCase.kt` (1 hunks) * `domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetUsageStatsListUseCase.kt` (2 hunks) * `feature/challenge/src/main/AndroidManifest.xml` (0 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/AppUsageGoalsFragment.kt` (4 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/AppUsageGoalsViewModel.kt` (3 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/appadd/AppAddViewPagerAdapter.kt` (2 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/appadd/time/SetAppGoalTimeFragment.kt` (1 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/calendar/ChallengeCalendarAdapter.kt` (0 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/calendar/ChallengeStatusViewHolder.kt` (0 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/AppUsageGoalsAdapter.kt` (3 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/AppUsageGoalsViewHolder.kt` (1 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/NewChallengeActivity.kt` (0 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/NewChallengeViewModel.kt` (0 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/NewChallengeViewPagerAdapter.kt` (0 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/periodselection/PeriodSelectionFragment.kt` (0 hunks) * `feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/timeselection/TimeSelectionFragment.kt` (0 hunks) * `feature/challenge/src/main/res/layout/activity_new_challenge.xml` (0 hunks) * `feature/challenge/src/main/res/layout/fragment_challenge.xml` (4 hunks) * `feature/challenge/src/main/res/layout/fragment_period_selection.xml` (0 hunks) * `feature/challenge/src/main/res/layout/fragment_time_selection.xml` (0 hunks) * `feature/challenge/src/main/res/layout/item_challenge_status.xml` (0 hunks) * `feature/challenge/src/main/res/layout/item_point.xml` (0 hunks) * `feature/challenge/src/main/res/values/strings.xml` (0 hunks) * `feature/main/src/main/java/com/hmh/hamyeonham/feature/main/MainActivity.kt` (2 hunks) * `feature/main/src/main/java/com/hmh/hamyeonham/feature/main/MainAdapter.kt` (2 hunks) * `feature/main/src/main/java/com/hmh/hamyeonham/feature/main/home/UsageStaticsTotalViewHolder.kt` (2 hunks) * `feature/main/src/main/java/com/hmh/hamyeonham/feature/main/home/UsageStaticsViewHolder.kt` (1 hunks) * `feature/main/src/main/res/layout/item_usagestatic.xml` (1 hunks) * `feature/main/src/main/res/layout/item_usagestatic_total.xml` (1 hunks) * `feature/main/src/main/res/menu/bottom_nav_menu.xml` (1 hunks) * `feature/main/src/main/res/navigation/nav_graph.xml` (1 hunks) * `feature/main/src/main/res/values/strings.xml` (1 hunks) </details> <details> <summary>💤 Files with no reviewable changes (37)</summary> * data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/NewChallengeMapper.kt * core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/HomeItem.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/NewChallengeRequest.kt * feature/challenge/src/main/res/layout/item_point.xml * data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/datasource/remote/UsageGoalsRemoteDataSource.kt * feature/challenge/src/main/res/layout/fragment_period_selection.xml * feature/challenge/src/main/res/layout/fragment_time_selection.xml * feature/challenge/src/main/AndroidManifest.xml * feature/challenge/src/main/res/layout/item_challenge_status.xml * domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/NewChallengeUseCase.kt * data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/ChallengeMapper.kt * data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/ChallengeStatusMapper.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/usagegoal/DailyChallengeService.kt * core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/model/ChallengeStatus.kt * domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/NewChallenge.kt * domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/ChallengeWithUsageInput.kt * feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/calendar/ChallengeCalendarAdapter.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeFinishResponse.kt * feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/timeselection/TimeSelectionFragment.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeWithUsageRequest.kt * data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/mapper/AppMapper.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/AppsRequest.kt * feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/calendar/ChallengeStatusViewHolder.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/ChallengeService.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeWithUsageResponse.kt * feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/periodselection/PeriodSelectionFragment.kt * feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/NewChallengeViewPagerAdapter.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/di/ChallengeModule.kt * feature/challenge/src/main/res/layout/activity_new_challenge.xml * core/network/src/main/java/com/hmh/hamyeonham/core/network/di/UsageGoalModule.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/model/ChallengeResponse.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/challenge/AppCodeRequest.kt * feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/NewChallengeViewModel.kt * feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/newchallenge/NewChallengeActivity.kt * core/network/src/main/java/com/hmh/hamyeonham/core/network/usagegoal/model/UsageGoalResponse.kt * domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/repository/ChallengeRepository.kt * feature/challenge/src/main/res/values/strings.xml </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧬 Code Graph Analysis (2)</summary> <details> <summary>feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/AppUsageGoalsViewHolder.kt (1)</summary><blockquote> <details> <summary>feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/ChallengeViewHolder.kt (1)</summary> * `binding` (15-74) </details> </blockquote></details> <details> <summary>core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/MainViewModel.kt (2)</summary><blockquote> <details> <summary>feature/onboarding/src/main/java/com/hmh/hamyeonham/feature/onboarding/viewmodel/OnBoardingViewModel.kt (1)</summary> * `updateState` (84-88) </details> <details> <summary>feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/appadd/AppAddViewModel.kt (1)</summary> * `updateState` (132-136) </details> </blockquote></details> </details> </details> <details> <summary>⏰ 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)</summary> * GitHub Check: PR Checker </details> <details> <summary>🔇 Additional comments (30)</summary><blockquote> <details> <summary>feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/appadd/time/SetAppGoalTimeFragment.kt (2)</summary> `17-17`: **Rename looks good and is consistent across usage.** Class rename to SetAppGoalTimeFragment aligns with the adapter update; no functional regressions apparent. --- `51-52`: **Confirm the intended hour range (0..1).** Limiting hours to 1 seems strict for per-app goals; if this is the new product decision, all good. Otherwise consider expanding (e.g., 0..23). </details> <details> <summary>feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/appadd/AppAddViewPagerAdapter.kt (2)</summary> `7-7`: **Adapter updated to the new fragment name.** Import and usage switched to SetAppGoalTimeFragment; wiring looks correct. Also applies to: 16-16 --- `7-7`: **No leftover `SetGoalTimeFragment` references detected** Ran `rg -nP -C2 '\bSetGoalTimeFragment\b'` across the repository and found no occurrences. The rename appears complete. </details> <details> <summary>feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/AppUsageGoalsViewHolder.kt (1)</summary> `15-20`: **Rename looks good; adapter/ViewHolder hierarchy alignment is consistent.** The sealed class rename to AppUsageGoalsViewHolder and its usage in the nested ViewHolder are consistent. No functional changes introduced. </details> <details> <summary>feature/challenge/src/main/res/layout/fragment_challenge.xml (2)</summary> `25-25`: **String/title change aligns with product rename** The title now uses @string/app_setting. This matches the navigation/menu rename away from “Challenge”. --- `79-85`: **ID rename verified – no remaining bt_goal_add references found** I ran searches for raw “bt_goal_add”, camel-case “btGoalAdd”, `R.id.bt_goal_add`, and `findViewById` usages, and found no matches. All references appear updated to `bt_app_add`. </details> <details> <summary>feature/main/src/main/res/menu/bottom_nav_menu.xml (1)</summary> `5-5`: **Menu label ‘app_setting’ verified across all locales** ✅ Confirmed `<string name="app_setting">` is present in every `res/values*/strings.xml`. Approving these changes. </details> <details> <summary>feature/main/src/main/res/values/strings.xml (2)</summary> `2-2`: **LGTM: New string added** `app_setting` looks good and aligns with the updated bottom nav label. --- `18-18`: **Confirm usage and apply correct text** I couldn’t find any references to `@string/total_time` in your layouts or code, so please verify where this string is actually used and ensure the replacement makes sense in that context. – If you intend to use it in item_usagestatic_total.xml (e.g. with `tv_total_time_left`), re-add the `android:text="@string/total_time"` attribute after applying the diff below. – If it isn’t used anywhere, consider removing or renaming the resource to match its intended purpose. Proposed diff: ```diff - <string name="total_time">헉! 블랙홀에 빠져\n별조각을 획득하지 못했어요.</string> + <string name="total_time">총 사용 시간</string>domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/model/UsageStatusAndGoal.kt (1)
33-40: LGTM: Type migration to AppUsageGoal.App and mappingThe mapping remains straightforward and correct with the new type.
domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetTotalUsageGoalUseCase.kt (1)
11-13: Verified migration to AppUsageGoal; no remaining UsageGoal references
I’ve searched the entire codebase (including imports and wildcard patterns) and found no lingeringUsageGoalusages. All callers, mappers, and tests should now be usingAppUsageGoal.domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetUsageGoalsUseCase.kt (1)
3-3: Import alignment LGTMImporting AppUsageGoal is consistent with the domain refactor.
feature/main/src/main/java/com/hmh/hamyeonham/feature/main/MainAdapter.kt (1)
15-18: Fragment swap to AppUsageGoalsFragment LGTMMapping CHALLENGE to AppUsageGoalsFragment aligns with the feature removal.
domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/AddUsageGoalsUseCase.kt (2)
17-18: AppUsageGoal mapping LGTMSwitching to AppUsageGoal.App(appCode, goalTime) is correct and consistent with the new model.
14-20: Verified no remote calls in ChallengeRepository.postAppsDefaultChallengeRepository.postApps now only logs via Timber and does not perform any network operations. The call in AddUsageGoalsUseCase is therefore safe—if you’d like to keep the logging side-effect, you can leave it as is; otherwise you may simplify by removing the redundant call:
• domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/AddUsageGoalsUseCase.kt:14-20
• data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt (postApps logs only)Optional simplification:
- challengeRepository.postApps(apps).onSuccess { - usageGoalsRepository.addUsageGoalList( - apps.apps.map { AppUsageGoal.App(it.appCode, it.goalTime) } - ) - } + usageGoalsRepository.addUsageGoalList( + apps.apps.map { AppUsageGoal.App(it.appCode, it.goalTime) } + )core/domain/src/main/java/com/hmh/hamyeonham/core/domain/usagegoal/repository/UsageGoalsRepository.kt (1)
8-10: API switch to AppUsageGoal LGTMSignatures updated to AppUsageGoal/AppUsageGoal.App look consistent across the domain.
domain/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/usecase/GetUsageStatsListUseCase.kt (1)
30-31: Confirm business rule: add “deleted usage of today” regardless of the requested time windowYou add deleteGoalRepository.getDeletedUsageOfToday() even when start/end may not be “today”. Verify this matches the intended behavior for arbitrary ranges.
data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/mapper/UsageGoalMapper.kt (1)
6-10: Symmetric, minimal mappings look good.The extensions are concise, internal, and match the new domain model. This keeps persistence<->domain conversions straightforward.
core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/MainState.kt (2)
6-11: Potential duplication betweenappGoalsandappUsageGoals.You now carry both a list of
AppGoaland an aggregateAppUsageGoal. If both model the same concept at different shapes, ensure a single source of truth to prevent divergence.
- Is
appGoalsstrictly UI projection? If so, consider deriving it fromappUsageGoalsto keep state minimal.
3-4: No layering issue:AppGoalis a domain model, not a feature
TheAppGoalclass lives indomain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/Challenge.kt, socore.viewmodelis merely depending on a domain module. Relying on domain types in your view-model layer is expected—there’s no cross-layer violation here.Likely an incorrect or invalid review comment.
core/service/src/main/java/com/hmh/hamyeonham/core/lock/AppLockManger.kt (2)
43-61: Flow firstOrNull() early-return can skip scheduling; confirm emission behavior.If
GetUsageGoalsUseCase()emits later (cold flow), usingfirstOrNull()may return null and skip monitoring. If goals are guaranteed to be present/emitted, fine; otherwise, consider waiting with a timeout.Option:
- val usageGoals = getUsageGoalsUseCase().firstOrNull() ?: return@launch + val usageGoals = withContext(Dispatchers.IO) { + kotlinx.coroutines.withTimeoutOrNull(5_000) { + getUsageGoalsUseCase().first() + } + } ?: return@launchValidate if this is needed; otherwise, keep as-is.
64-79: Lock check logic reads well after per-app simplification.Clean separation: compare app usage vs app goal, then either lock or schedule timer based on remaining time. This is much simpler than the prior global total logic.
feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/goals/AppUsageGoalsAdapter.kt (1)
10-13: Adapter renaming and type updates look consistent.Class and generic type updates are coherent with the new holder. No issues spotted.
core/viewmodel/main/src/main/java/com/hmh/hamyeonham/core/viewmodel/MainViewModel.kt (1)
6-6: AppUsageGoal import aligns with domain refactor — LGTMImport correctly reflects the shift to AppUsageGoal-based flow.
feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/AppUsageGoalsFragment.kt (3)
20-20: Adapter migration to AppUsageGoalsAdapter — LGTMImport matches the new adapter and aligns with the PR’s goal removal.
161-176: RecyclerView adapter setup and click handling — LGTMAdapter wiring and delete handling align with the new goals flow. Resource usage continues to live under feature.challenge, which is fine as long as those resources remain.
43-63: ✅ Navigation Graph Updated and No Lingering References
- The
nav_graph.xmlatfeature/main/src/main/res/navigation/nav_graph.xmlnow references
com.hmh.hamyeonham.challenge.AppUsageGoalsFragmentfor@+id/challenge_destand has no remainingChallengeFragmententries.- The layout file
fragment_challenge.xmlstill exists underfeature/challenge/src/main/res/layout, preserving theFragmentChallengeBinding.All navigation and binding references are correctly updated.
data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/datasource/local/UsageGoalsLocalDataSource.kt (2)
38-45: Bulk insert mapping — LGTMMapping to UsageGoalEntity list is correct and consistent with the single insert path.
29-36: Upsert semantics confirmed for addUsageAppGoalThe
UsageGoalsDao.insertUsageGoalmethod is annotated with@Insert(onConflict = OnConflictStrategy.REPLACE), so callingaddUsageAppGoalwill replace an existing goal for the same package rather than create duplicates. No changes needed here.
| override suspend fun postApps(request: Apps): Result<Unit> { | ||
| return runCatching { challengeService.postApps(request.toAppsRequest()) } | ||
| return runCatching { Timber.d("Post apps: $request") } | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
postApps() became a no-op and currently logs potentially sensitive details.
- Returning success unconditionally is a behavior change; ensure callers don’t depend on error propagation.
- Logging the full Apps payload can expose user app lists. Prefer minimal/no logging for PII safety.
- You can simplify to an explicit success return.
Apply this diff:
- return runCatching { Timber.d("Post apps: $request") }
+ Timber.d("postApps() no-op")
+ return Result.success(Unit)Follow-ups:
- If this method is truly unused, deprecate here and schedule removal from ChallengeRepository to reduce API surface.
- Verify usages:
🏁 Script executed:
#!/bin/bash
# Find usages of postApps with context (exclude build outputs if present)
rg -n -C3 --type=kt '\bpostApps\s*\('Length of output: 66
🏁 Script executed:
#!/bin/bash
# Retry usage search across Kotlin files without relying on file type definitions
rg -n -C3 'postApps\(' --glob '*.kt'Length of output: 2931
Remove sensitive logging and simplify postApps()
- File:
data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt(lines 31–33)
Replace therunCatching { Timber.d("Post apps: $request") }wrapper—which always returns success and logs the full payload—with an explicit success return and a minimal log message. - Verify callers: the only usage is in
domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/usecase/AddUsageGoalsUseCase.kt
Ensure that always returning success here doesn’t break any intended error handling.
Proposed diff:
--- a/data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt
+++ b/data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt
@@ -31,3 +31,5 @@
- override suspend fun postApps(request: Apps): Result<Unit> {
- return runCatching { Timber.d("Post apps: $request") }
- }
+ override suspend fun postApps(request: Apps): Result<Unit> {
+ Timber.d("postApps() no-op")
+ return Result.success(Unit)
+ }📝 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.
| override suspend fun postApps(request: Apps): Result<Unit> { | |
| return runCatching { challengeService.postApps(request.toAppsRequest()) } | |
| return runCatching { Timber.d("Post apps: $request") } | |
| } | |
| override suspend fun postApps(request: Apps): Result<Unit> { | |
| Timber.d("postApps() no-op") | |
| return Result.success(Unit) | |
| } |
🤖 Prompt for AI Agents
In
data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt
around lines 31-33, remove the runCatching wrapper that logs the full Apps
payload and always returns success; instead return Result.success(Unit) directly
and replace the Timber.d call with a minimal non-sensitive message (e.g.,
"postApps invoked"). Also confirm callers (notably
domain/challenge/.../AddUsageGoalsUseCase.kt) do not rely on errors from this
method—if they do, propagate or construct appropriate failures instead of
silently returning success.
| override suspend fun updateUsageGoal(): Result<Boolean> { | ||
| return runCatching { | ||
| usageGoalsRemoteDataSource.getUsageGoals().fold( | ||
| onSuccess = { usageGoals -> | ||
| val totalGoalTime = usageGoals.totalGoalTime | ||
| usageTotalGoalDao.insertUsageTotalGoal( | ||
| UsageTotalGoalEntity( | ||
| totalGoalTime = totalGoalTime, | ||
| status = usageGoals.status.name | ||
| ) | ||
| ) | ||
|
|
||
| // 각 앱 목표 시간 저장 | ||
| usageGoalsDao.insertUsageGoalList(usageGoals.toUsageGoalEntityList()) | ||
| usageGoals.status.name != ChallengeStatus.FAILURE.name | ||
| }, | ||
| onFailure = { | ||
| false | ||
| } | ||
| ) | ||
| // usageGoalsRemoteDataSource.getUsageGoals().fold( | ||
| // onSuccess = { usageGoals -> | ||
| // val totalGoalTime = usageGoals.totalGoalTime | ||
| // usageTotalGoalDao.insertUsageTotalGoal( | ||
| // UsageTotalGoalEntity( | ||
| // totalGoalTime = totalGoalTime, | ||
| // status = usageGoals.status.name | ||
| // ) | ||
| // ) | ||
| // | ||
| // // 각 앱 목표 시간 저장 | ||
| // usageGoalsDao.insertUsageGoalList(usageGoals.toUsageGoalEntityList()) | ||
| // usageGoals.status.name != ChallengeStatus.FAILURE.name | ||
| // }, | ||
| // onFailure = { | ||
| // false | ||
| // } | ||
| // ) | ||
| Timber.d("Updating usage goals in local data source") | ||
| return Result.success(true) | ||
| } | ||
| } |
There was a problem hiding this comment.
Bug: Misuse of runCatching with a non-local return inside the lambda
Returning Result.success(true) inside runCatching’s lambda causes a non-local return, bypassing runCatching and its exception wrapping. Either return a raw Boolean inside the block or drop runCatching.
Fix with either of the following:
Option A (keep runCatching):
- override suspend fun updateUsageGoal(): Result<Boolean> {
- return runCatching {
+ override suspend fun updateUsageGoal(): Result<Boolean> {
+ return runCatching {
Timber.d("Updating usage goals in local data source")
- return Result.success(true)
- }
+ true
+ }
}Option B (simplify, remove runCatching):
- override suspend fun updateUsageGoal(): Result<Boolean> {
- return runCatching {
- Timber.d("Updating usage goals in local data source")
- return Result.success(true)
- }
- }
+ override suspend fun updateUsageGoal(): Result<Boolean> {
+ Timber.d("Updating usage goals in local data source")
+ return Result.success(true)
+ }🤖 Prompt for AI Agents
In
data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/repository/DefaultUsageGoalsRepository.kt
around lines 18 to 41, the lambda passed to runCatching contains a non-local
return (return Result.success(true)) which escapes the lambda and bypasses
runCatching’s exception handling; fix by either A) keeping runCatching and
returning a raw Boolean from the lambda (replace the inner return
Result.success(true) with just true) so runCatching returns Result<Boolean>, or
B) remove runCatching entirely and directly return a Result (e.g.,
Result.success(true)) from the function body; choose one option and adjust the
code accordingly so no non-local return occurs inside the lambda.
| data class AppGoal( | ||
| val appCode: String, | ||
| val appGoalTime: Long, | ||
| ) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify there are no remaining references to the removed Challenge model
Given the removal of the Challenge type, ensure no lingering references (types, imports, mappers) remain.
Run:
🏁 Script executed:
#!/bin/bash
# Find usages of removed/legacy Challenge constructs and the new AppGoal
rg -n -C2 --type=kt '\bChallenge\b|ChallengeStatus|ChallengeDao|ChallengeService|updateDailyChallengeFailed|AppGoal\('
# Check if project is using the canonical domain model instead
rg -n -C2 --type=kt 'AppUsageGoal\.App\b'Length of output: 218
🏁 Script executed:
#!/bin/bash
# Retry searching Kotlin files by globbing *.kt instead of using an unrecognized type
rg -n -C2 --glob '*.kt' '\bChallenge\b|ChallengeStatus|ChallengeDao|ChallengeService|updateDailyChallengeFailed'
rg -n -C2 --glob '*.kt' 'AppGoal\('
rg -n -C2 --glob '*.kt' 'AppUsageGoal\.App\b'Length of output: 16046
Remove remaining references to the removed Challenge model
The search shows that several data‐ and infrastructure‐layer classes still reference “Challenge” constructs that must be removed or renamed now that the Challenge domain type is gone:
- domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/repository/ChallengeRepository.kt
• Methods likeupdateDailyChallengeFailed(),deleteChallengeWithUsage(...)anddeleteAllChallengeWithUsage(). - data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/datasource/ChallengeLocalDataSourceImpl.kt
• Imports ofChallengeDaoand entity types (ChallengeWithUsageEntity,DailyChallengeEntity) and the injectedchallengeDao. - data/challenge/src/main/java/com/hmh/hamyeonham/data/challenge/repository/DefaultChallengeRepository.kt
• Override ofupdateDailyChallengeFailed(). - core/database/src/main/java/com/hmh/hamyeonham/core/database/di/DaoModule.kt
•providesChallengeDao(...)provider. - core/database/src/main/java/com/hmh/hamyeonham/core/database/HMHRoomDatabase.kt
•abstract fun challengeDao(): ChallengeDaodeclaration. - core/database/src/main/java/com/hmh/hamyeonham/core/database/dao/ChallengeDao.kt
• TheChallengeDaointerface itself. - data/usagestats/src/main/java/com/hmh/hamyeonham/usagestats/repository/DefaultUsageGoalsRepository.kt
• Commented code referencingChallengeStatus(can be removed if no longer relevant).
All these references should be removed or updated to use the new AppGoal/Apps model (or your canonical domain types) to complete the cleanup.
🤖 Prompt for AI Agents
In
domain/challenge/src/main/java/com/hmh/hamyeonham/challenge/model/Challenge.kt
around lines 3 to 6, leftover references to the removed Challenge domain type
remain across the codebase; remove or replace them with the new AppGoal/Apps
model: delete or rename repository methods in
domain/challenge/.../ChallengeRepository.kt (updateDailyChallengeFailed,
deleteChallengeWithUsage, deleteAllChallengeWithUsage) to use AppGoal/Apps
semantics; remove imports and the injected challengeDao plus entity types from
data/challenge/.../ChallengeLocalDataSourceImpl.kt and adapt the data source to
the new models; update data/challenge/.../DefaultChallengeRepository.kt to
remove the override of updateDailyChallengeFailed and implement equivalent logic
using AppGoal/Apps; remove providesChallengeDao from
core/database/.../di/DaoModule.kt and the abstract fun challengeDao()
declaration from HMHRoomDatabase, and delete the ChallengeDao interface and any
related entity classes under core/database/dao; finally, remove or update
commented references to ChallengeStatus in
data/usagestats/.../DefaultUsageGoalsRepository.kt; ensure all callers are
refactored to use AppGoal/Apps types and rebuild/compile to catch remaining
references.
| binding.btAppAdd.setOnSingleClickListener { | ||
| AmplitudeUtils.trackEventWithProperties("click_add_button") | ||
| val intent = Intent(requireContext(), AppAddActivity::class.java) | ||
| appSelectionResultLauncher.launch(intent) | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm layout ID btAppAdd exists (binding uses FragmentChallengeBinding)
Since the binding class is FragmentChallengeBinding, verify that the layout defines a view with id btAppAdd; otherwise you’ll get a runtime crash.
Run to verify:
🏁 Script executed:
#!/bin/bash
# Find the view id in XML layouts
rg -nP '@\+id/btAppAdd' -g '**/*.xml' -C2
# Check for old id still referenced somewhere
rg -nP '@\+id/btGoalAdd' -g '**/*.xml' -C2Length of output: 88
🏁 Script executed:
#!/bin/bash
set -e
# Search for the view ID btAppAdd in layout XML files under res/layout
rg -n 'android:id="@\+id/btAppAdd"' -g 'res/**/layout/**/*.xml' -C2
# Fallback: search for any reference to btAppAdd in layout XML
rg -n 'btAppAdd' -g 'res/**/layout/**/*.xml' -C2
# Also check for the old ID btGoalAdd to catch any stale references
rg -n 'android:id="@\+id/btGoalAdd"' -g 'res/**/layout/**/*.xml' -C2
rg -n 'btGoalAdd' -g 'res/**/layout/**/*.xml' -C2Length of output: 227
🏁 Script executed:
#!/bin/bash
set -e
echo "Listing layout files in feature/challenge module:"
find feature/challenge/src/main/res/layout -type f -name "*.xml"
echo -e "\nSearching for btAppAdd id in these layouts:"
rg -n 'android:id="@\\+id/btAppAdd"' -g 'feature/challenge/src/main/res/layout/**/*.xml' -C2 || echo "No matches for btAppAdd id."
echo -e "\nSearching for any reference to btAppAdd in fragment_challenge.xml:"
rg -n 'btAppAdd' feature/challenge/src/main/res/layout/fragment_challenge.xml -C2 || echo "btAppAdd not found in fragment_challenge.xml."
echo -e "\nChecking for old id btGoalAdd to catch stale usages:"
rg -n 'android:id="@\\+id/btGoalAdd"' -g 'feature/challenge/src/main/res/layout/**/*.xml' -C2 || echo "No btGoalAdd id references found."
rg -n 'btGoalAdd' -g 'feature/challenge/src/main/res/layout/**/*.xml' -C2 || echo "No btGoalAdd references found."Length of output: 1527
Add missing view ID btAppAdd to fragment_challenge.xml
I searched all layout files under feature/challenge/src/main/res/layout and didn’t find any view with
android:id="@+id/btAppAdd"Without this ID, binding.btAppAdd won’t be generated and will throw at runtime. Please update fragment_challenge.xml to include a view with that ID, for example:
<Button
android:id="@+id/btAppAdd"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="Add" />• feature/challenge/src/main/res/layout/fragment_challenge.xml: add a view with @+id/btAppAdd
🤖 Prompt for AI Agents
In
feature/challenge/src/main/java/com/hmh/hamyeonham/challenge/AppUsageGoalsFragment.kt
around lines 133 to 137, binding.btAppAdd is referenced but the corresponding
view ID is missing from the layout; open
feature/challenge/src/main/res/layout/fragment_challenge.xml and add a view with
android:id="@+id/btAppAdd" (e.g., a Button or MaterialButton with appropriate
layout_width/layout_height and text), ensure its placement and styling match the
fragment UI, then rebuild to regenerate the binding so binding.btAppAdd is
available at runtime.
| app:layout_constraintEnd_toEndOf="@id/tv_modifier_button" | ||
| app:layout_constraintStart_toStartOf="@id/tv_app_lock" | ||
| app:layout_constraintTop_toBottomOf="@id/tv_app_lock" /> | ||
| app:layout_constraintTop_toBottomOf="@id/tv_modifier_button" /> |
There was a problem hiding this comment.
Constraint cycle between tv_app_lock, tv_modifier_button, and rv_app_usage_goals
rv_app_usage_goals top is constrained to tv_modifier_button bottom; tv_modifier_button bottom is constrained to tv_app_lock bottom; tv_app_lock bottom is constrained to rv_app_usage_goals top. This creates a cycle and can break the solver.
Break the cycle by anchoring rv_app_usage_goals directly under tv_app_lock:
- app:layout_constraintTop_toBottomOf="@id/tv_modifier_button" />
+ app:layout_constraintTop_toBottomOf="@id/tv_app_lock" />Optionally, if you want to keep the visual tie between tv_modifier_button and tv_app_lock, you can leave tv_modifier_button constrained to tv_app_lock as-is. No other changes are needed for a valid chain.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In feature/challenge/src/main/res/layout/fragment_challenge.xml around line 81,
there is a constraint cycle between tv_app_lock, tv_modifier_button, and
rv_app_usage_goals; fix it by changing rv_app_usage_goals’s top constraint so it
anchors directly to tv_app_lock bottom (remove or replace the existing
app:layout_constraintTop_toBottomOf="@id/tv_modifier_button" with a constraint
to @id/tv_app_lock), and keep tv_modifier_button constrained to tv_app_lock if
you want to preserve their visual relationship.
| // totalModel.challengeSuccess -> { | ||
| // BlackHoleInfo.createByPercentage(totalModel.totalPercentage) | ||
| // ?: BlackHoleInfo.LEVEL0 | ||
| // } |
There was a problem hiding this comment.
BlackHole level always shows LEVEL5; restore percentage-based level
With challengeSuccess removed, the when now always resolves to LEVEL5. Use percentage to determine the level for correct UX feedback.
Apply:
private fun bindBlackHoleInfo(totalModel: HomeItem.TotalModel) {
- val blackHoleInfo = when {
- // 챌린지 성공한 경우
-// totalModel.challengeSuccess -> {
-// BlackHoleInfo.createByPercentage(totalModel.totalPercentage)
-// ?: BlackHoleInfo.LEVEL0
-// }
- // 챌린지 실패한 경우
- else -> {
- BlackHoleInfo.LEVEL5
- }
- }
+ val blackHoleInfo =
+ BlackHoleInfo.createByPercentage(totalModel.totalPercentage) ?: BlackHoleInfo.LEVEL0📝 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.
| // totalModel.challengeSuccess -> { | |
| // BlackHoleInfo.createByPercentage(totalModel.totalPercentage) | |
| // ?: BlackHoleInfo.LEVEL0 | |
| // } | |
| private fun bindBlackHoleInfo(totalModel: HomeItem.TotalModel) { | |
| val blackHoleInfo = | |
| BlackHoleInfo.createByPercentage(totalModel.totalPercentage) ?: BlackHoleInfo.LEVEL0 | |
| // …rest of the method unchanged | |
| } |
🤖 Prompt for AI Agents
In
feature/main/src/main/java/com/hmh/hamyeonham/feature/main/home/UsageStaticsTotalViewHolder.kt
around lines 52 to 55, the when-expression currently always returns
BlackHoleInfo.LEVEL5 because the challengeSuccess branch was removed; restore
percentage-based determination by replacing the always-LEVEL5 branch with a call
to BlackHoleInfo.createByPercentage(totalModel.totalPercentage) and fallback to
BlackHoleInfo.LEVEL0 if that returns null so the black hole level is derived
from totalModel.totalPercentage.
| Timber.d("Unlock package: $packageName") | ||
| } |
There was a problem hiding this comment.
Confirm action does not clear the intent extra; dialog may reappear on next resume
On confirm you only log. Because the extra is cleared only on dismiss, the dialog can show again on future onResume calls. Clear the extra on confirm as well.
Apply:
setConfirmButtonClickListener {
- Timber.d("Unlock package: $packageName")
+ Timber.d("Unlock package: $packageName")
+ intent.removeExtra(NavigationProvider.UN_LOCK_PACKAGE_NAME)
}If additional business logic should occur here (e.g., actually unlocking the package), wire it via the view model or navigation provider.
📝 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.
| Timber.d("Unlock package: $packageName") | |
| } | |
| setConfirmButtonClickListener { | |
| Timber.d("Unlock package: $packageName") | |
| intent.removeExtra(NavigationProvider.UN_LOCK_PACKAGE_NAME) | |
| } |
🤖 Prompt for AI Agents
In feature/main/src/main/java/com/hmh/hamyeonham/feature/main/MainActivity.kt
around lines 107-108, the dialog Confirm action only logs the package and does
not clear the intent extra so the dialog may reappear on the next onResume;
update the Confirm branch to remove the intent extra that triggered the dialog
(e.g., call intent.removeExtra(EXTRA_UNLOCK_PACKAGE) or
intent.removeExtra("packageName") using the actual constant/string used), then
proceed with any business logic (wire to ViewModel or navigation provider) —
keep the dismiss behavior for the cancel/dismiss path unchanged.
개요
작업 사항
주의사항
변경 사항(optional)
스크린샷(optional)
hmh.mp4
Summary by CodeRabbit
New Features
Refactor
Chores