-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat/#273] 인앱 리뷰 유도 기능을 구현합니다. #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change implements an in-app review prompt feature that triggers when a user returns to the home screen after checking a reply diary. It introduces new data sources, repository, dependency injection bindings, and UI logic to manage and display the review prompt using Google Play's In-App Review API, with persistent flag management via shared preferences. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ReplyDiaryScreen
participant HomeScreen
participant HomeViewModel
participant InAppReviewManager
User->>ReplyDiaryScreen: Completes reply diary
ReplyDiaryScreen->>HomeScreen: Navigate to Home (isFromReplyDiary = true)
HomeScreen->>HomeViewModel: Collect showInAppReviewPopup state
HomeViewModel-->>HomeScreen: showInAppReviewPopup = true
HomeScreen->>InAppReviewManager: showPopup(activity)
InAppReviewManager->>User: Show in-app review prompt
HomeScreen->>HomeViewModel: updateShowInAppReviewPopup(false)
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (2)
✨ Finishing Touches
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. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/sopt/clody/data/local/datasource/AppReviewLocalDataSource.kt (1)
3-5: LGTM! Clean and focused interface design.The interface follows the single responsibility principle with a clear, descriptive property name. The mutable Boolean property is appropriate for managing the review popup state.
Consider adding KDoc documentation to clarify the behavior:
+/** + * Local data source for managing app review popup state. + */ interface AppReviewLocalDataSource { + /** + * Flag indicating whether the in-app review popup should be shown. + * Set to false after displaying the popup to prevent repeated prompts. + */ var shouldShowPopup: Boolean }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
app/build.gradle.kts(1 hunks)app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt(1 hunks)app/src/main/java/com/sopt/clody/data/datastore/TokenDataStoreImpl.kt(1 hunks)app/src/main/java/com/sopt/clody/data/local/datasource/AppReviewLocalDataSource.kt(1 hunks)app/src/main/java/com/sopt/clody/data/local/datasourceimpl/AppReviewLocalDataSourceImpl.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(6 hunks)app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt(4 hunks)gradle/libs.versions.toml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (18)
gradle/libs.versions.toml (2)
127-129: LGTM! Clean dependency organization.The Play Store in-app review libraries are properly configured with both core and KTX variants, and the bundle organization follows the existing pattern in the file.
Also applies to: 203-206
53-53:Details
✅ Verification successful
Verify the Play Core review library version is current.
The version 2.0.2 should be verified to ensure it's the latest stable version and doesn't have known security vulnerabilities.
🌐 Web query:
What is the latest version of Google Play Core in-app review library for Android?💡 Result:
Based on the search results, the latest version of the Google Play In-App Reviews Library for Android is 2.0.2, which was released in October 2024[6]. This update fixed an Activity crash issue, which is referenced in the Android issue tracker (https://issuetracker.google.com/343086058)[6].
The Play In-App Review Library is a separate component that was extracted from the original monolithic Play Core library. Google has stopped releasing new versions of the original Play Core library in early 2022[3], with the last version being released in January 2022 according to Maven Repository information[10].
To integrate the latest Play In-App Review Library into your Android project, you would need to add the following dependencies to your app's build.gradle file:
For Groovy:
implementation 'com.google.android.play:review:2.0.2' // For Kotlin users, also add the Kotlin extensions library: implementation 'com.google.android.play:review-ktx:2.0.2'For Kotlin:
implementation("com.google.android.play:review:2.0.2") // For Kotlin users, also import the Kotlin extensions library: implementation("com.google.android.play:review-ktx:2.0.2") ```[2] It's worth noting that Google has been moving away from the monolithic Play Core library since 2022, transitioning to individual libraries for specific functionalities like in-app reviews, in-app updates, and feature delivery[3]. This is part of their migration strategy from the original Play Core library. Citations: - 1: https://developer.android.com/reference/com/google/android/play/core/release-notes - 2: https://developer.android.com/guide/playcore - 3: https://www.droidcon.com/2024/07/01/migrating-from-play-core-library/ - 4: https://support.google.com/googleplay/android-developer/thread/285447378/google-play-sending-the-warning-of-google-play-core-library-for-sdk-33 - 5: https://play.google.com/console/about/whats-new/ - 6: https://developer.android.com/reference/com/google/android/play/core/release-notes-in_app_reviews - 7: https://developers.google.com/android/guides/releases - 8: https://github.com/google/play-unity-plugins/blob/master/GooglePlayPlugins/com.google.play.core/CHANGELOG.md - 9: https://github.com/google/play-in-app-reviews-unity/releases - 10: https://mvnrepository.com/artifact/com.google.android.play --- </details> **Play In-App Review Library Version Verified as Latest (2.0.2)** The dependency in gradle/libs.versions.toml matches the current stable release (Oct 2024) and includes the crash fix referenced in Google’s issue tracker. No further changes are needed. - File: gradle/libs.versions.toml (line 53): play-review = "2.0.2" </details> <details> <summary>app/build.gradle.kts (1)</summary> `126-128`: **LGTM! Proper dependency integration.** The Play Store dependencies are correctly added using the bundle pattern and properly organized with a descriptive comment. </details> <details> <summary>app/src/main/java/com/sopt/clody/data/datastore/TokenDataStoreImpl.kt (1)</summary> `4-4`: <details> <summary>✅ Verification successful</summary> **LGTM! Proper DI qualifier implementation.** The addition of `@TokenPrefs` qualifier correctly distinguishes the token-specific `SharedPreferences` instance, supporting the multi-preferences architecture needed for the review feature. Verify that the `@TokenPrefs` qualifier is properly defined: Also applies to: 8-8 --- 🏁 Script executed: ```shell #!/bin/bash # Description: Verify that the @TokenPrefs qualifier is defined and properly configured in DI modules # Search for TokenPrefs qualifier definition rg -A 3 "@Qualifier.*TokenPrefs|@TokenPrefs" --type kt # Search for DI module providing TokenPrefs SharedPreferences rg -A 5 "@Provides.*TokenPrefs|@TokenPrefs.*SharedPreferences" --type ktLength of output: 185
To locate the
@TokenPrefsqualifier and its provider without relying on unrecognized file-type filters:
🏁 Script executed:
#!/bin/bash # Search for @TokenPrefs qualifier definition in all Kotlin files rg '@Qualifier.*TokenPrefs|@TokenPrefs' -g '*.kt' -n -A3 # Search for DI module providing @TokenPrefs SharedPreferences rg '@Provides.*TokenPrefs|@TokenPrefs.*SharedPreferences' -g '*.kt' -n -A5Length of output: 2621
To confirm the
@TokenPrefsannotation itself is defined, let’s search for its declaration:
🏁 Script executed:
#!/bin/bash # Search for the TokenPrefs annotation declaration rg "annotation class TokenPrefs" -g '*.kt' -n -A3 # Fallback: locate any file named TokenPrefs.kt rg -l "TokenPrefs" -g '*/TokenPrefs.kt'Length of output: 564
Approved:
@TokenPrefsQualifier Correctly Defined and UsedAll checks confirm the
@TokenPrefsqualifier is declared and properly wired:
- Qualifier declaration in
app/src/main/java/com/sopt/clody/di/qualifier/Qualifier.kt(lines 7–10)SharedPreferencesprovider annotated with@TokenPrefsinSharedPreferencesModule.kt(lines 21–24)TokenDataStoreprovider and implementation constructor parameter both correctly annotated inTokenDataStoreModule.ktandTokenDataStoreImpl.ktNo further changes needed.
app/src/main/java/com/sopt/clody/di/qualifier/Qualifier.kt (1)
1-11: LGTM! Well-implemented qualifier annotations.The qualifier annotations follow Dagger/Hilt best practices with proper retention policy. This design effectively separates concerns between token and review preferences.
app/src/main/java/com/sopt/clody/di/TokenDataStoreModule.kt (2)
6-6: Good addition of qualifier import.The import correctly brings in the new
@TokenPrefsqualifier annotation.
18-18: Proper use of qualifier annotation.The
@TokenPrefsqualifier correctly distinguishes this SharedPreferences instance from others, ensuring proper dependency injection separation.app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt (3)
23-23: Good addition of InAppReviewManager import.The import correctly brings in the new review management functionality.
61-61: Good state collection.The state is properly collected from the ViewModel using
collectAsStateWithLifecycle.
195-195: Minor formatting improvement applied.The empty when branches are now consistently formatted.
Also applies to: 223-223
app/src/main/java/com/sopt/clody/data/local/datasourceimpl/AppReviewLocalDataSourceImpl.kt (2)
8-14: LGTM! Clean SharedPreferences implementation.The implementation correctly uses dependency injection with the
@ReviewPrefsqualifier, properly handles async writes withapply(), and follows Android best practices. The default value oftrueis appropriate for first-time users.
16-18: Good use of companion object for constants.The constant is properly named and scoped within a companion object, following Kotlin conventions.
app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeViewModel.kt (3)
23-23: Good dependency injection integration.The
AppReviewLocalDataSourceis properly injected following the existing pattern in the constructor.
67-68: Proper state flow initialization.The state flow is correctly initialized from the data source, ensuring consistency between the local storage and the UI state.
188-191: Well-implemented state update method.The method correctly updates both the data source and the state flow, maintaining consistency between persistence and UI state.
app/src/main/java/com/sopt/clody/di/SharedPreferencesModule.kt (3)
21-24: Good addition of qualifier annotation.Adding the
@TokenPrefsqualifier to the existing provider ensures proper disambiguation between different SharedPreferences instances.
26-31: Well-structured provider for review preferences.The new provider correctly uses the
@ReviewPrefsqualifier and follows the same pattern as the existing token preferences provider, maintaining consistency.
33-36: Clean data source provider implementation.The provider correctly injects the qualified SharedPreferences and returns the implementation as the interface type, following DI best practices.
app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt
Outdated
Show resolved
Hide resolved
MoonsuKang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| object InAppReviewManager { | ||
| fun showPopup(activity: Activity) { | ||
| val reviewManager = ReviewManagerFactory.create(activity) | ||
| val request = reviewManager.requestReviewFlow() | ||
|
|
||
| request.addOnCompleteListener { task -> | ||
| if (task.isSuccessful) { | ||
| val reviewInfo = task.result | ||
| reviewManager.launchReviewFlow(activity, reviewInfo) | ||
| } else { | ||
| // fallback logic: 예를 들어 마켓 링크 열기 | ||
| val uri = Uri.parse("market://details?id=${activity.packageName}") | ||
| val intent = Intent(Intent.ACTION_VIEW, uri) | ||
| activity.startActivity(intent) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P4
object AppUpdateUtils의navigateToMarket() 함수를 쓰면 더 좋을 것 같습니다.
AppUpdateUtils라는 이름이 애매하니 MarketUtils? 이런걸로 바꿔서 쓰면 좋을라나?
| override var shouldShowPopup: Boolean | ||
| get() = sharedPreferences.getBoolean(SHOULD_SHOW_POPUP, true) | ||
| set(value) = sharedPreferences.edit { putBoolean(SHOULD_SHOW_POPUP, value) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P5
인터페이스를 get/set 함수로 안하고 이렇게 하는 방식 좋네욤. 저장/읽기만 잇으니 더 간결한듯!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt (2)
10-10: Consider removing redundant Context parameter.Since
ActivityextendsContext, the separatecontextparameter appears redundant. You could simplify the method signature and use the activity directly.- fun showPopup(activity: Activity, context: Context) { + fun showPopup(activity: Activity) {Then update line 22:
- AppUpdateUtils.navigateToMarket(context) + AppUpdateUtils.navigateToMarket(activity)
16-19: Consider adding additional activity state validation in the callback.While the initial activity state check is good, the activity could become invalid between the initial check and when the callback executes. Consider adding another validation inside the callback.
request.addOnCompleteListener { task -> + if (activity.isFinishing || activity.isDestroyed) return@addOnCompleteListener + if (task.isSuccessful) { val reviewInfo = task.result reviewManager.launchReviewFlow(activity, reviewInfo)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt (2)
21-26: Previous feedback has been successfully addressed.The error handling implementation looks excellent:
- Uses
AppUpdateUtils.navigateToMarket()as suggested by MoonsuKang- Includes proper exception handling with try-catch
- Implements Timber logging for debugging as suggested in previous reviews
9-30: Well-structured singleton implementation for the use case.The object declaration is appropriate for this utility class that doesn't need to maintain state. The placement in the core package aligns well with the PR objectives, especially since it's utility functionality that doesn't involve complex business logic.
There was a problem hiding this 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 (2)
app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt (2)
21-26: Remove redundant error logging.You're logging the same error twice - both with
printStackTrace()andTimber.e(). The Timber logging is sufficient and more appropriate for production apps.try { AppUpdateUtils.navigateToMarket(activity) } catch (e: Exception) { - e.printStackTrace() Timber.e(e, "Failed to open store for app review") }
16-28: Consider re-validating activity state in async callback.The activity state could change between the initial check and when the async callback executes. Consider adding another activity state validation inside the callback for extra safety.
request.addOnCompleteListener { task -> + if (activity.isFinishing || activity.isDestroyed) return@addOnCompleteListener + if (task.isSuccessful) { val reviewInfo = task.result if (reviewInfo != null) { reviewManager.launchReviewFlow(activity, reviewInfo) } } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt(1 hunks)app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/sopt/clody/presentation/ui/home/screen/HomeScreen.kt
🔇 Additional comments (2)
app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt (2)
9-11: Good implementation of activity state validation.The activity state check prevents crashes when the activity is no longer valid. This addresses the previous review feedback effectively.
22-22: Good adoption of AppUpdateUtils.navigateToMarket().This addresses the previous feedback from MoonsuKang about reusing existing utility functions instead of duplicating market navigation logic.
| request.addOnCompleteListener { task -> | ||
| if (task.isSuccessful) { | ||
| val reviewInfo = task.result | ||
| reviewManager.launchReviewFlow(activity, reviewInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add null check for reviewInfo before launching review flow.
While the task success check is good, you should also validate that reviewInfo is not null before launching the review flow to prevent potential crashes.
request.addOnCompleteListener { task ->
if (task.isSuccessful) {
val reviewInfo = task.result
- reviewManager.launchReviewFlow(activity, reviewInfo)
+ if (reviewInfo != null) {
+ reviewManager.launchReviewFlow(activity, reviewInfo)
+ }
} else {📝 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.
| request.addOnCompleteListener { task -> | |
| if (task.isSuccessful) { | |
| val reviewInfo = task.result | |
| reviewManager.launchReviewFlow(activity, reviewInfo) | |
| object InAppReviewManager { | |
| fun showPopup(activity: Activity) { | |
| if (activity.isFinishing || activity.isDestroyed) { | |
| return | |
| } | |
| val reviewManager = ReviewManagerFactory.create(activity) | |
| val request = reviewManager.requestReviewFlow() | |
| request.addOnCompleteListener { task -> | |
| if (task.isSuccessful) { | |
| val reviewInfo = task.result | |
| if (reviewInfo != null) { | |
| reviewManager.launchReviewFlow(activity, reviewInfo) | |
| } | |
| } else { | |
| // Log the failure for debugging | |
| Timber.w("In-app review request failed: ${task.exception?.message}") | |
| // Fallback logic with error handling | |
| try { | |
| val uri = Uri.parse("market://details?id=${activity.packageName}") | |
| val intent = Intent(Intent.ACTION_VIEW, uri) | |
| if (intent.resolveActivity(activity.packageManager) != null) { | |
| activity.startActivity(intent) | |
| } else { | |
| // Fallback to web version if Play Store app isn't available | |
| val webUri = Uri.parse("https://play.google.com/store/apps/details?id=${activity.packageName}") | |
| val webIntent = Intent(Intent.ACTION_VIEW, webUri) | |
| activity.startActivity(webIntent) | |
| } | |
| } catch (e: Exception) { | |
| Timber.e(e, "Failed to open store for app review") | |
| } | |
| } | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/src/main/java/com/sopt/clody/core/review/InAppReviewManager.kt around
lines 16 to 19, add a null check for the variable reviewInfo before calling
reviewManager.launchReviewFlow. Ensure that reviewInfo is not null to prevent
potential crashes by wrapping the launchReviewFlow call inside an if statement
that verifies reviewInfo is non-null.

📌 ISSUE
closed #273
📄 Work Description
1. 기능목표
로컬에서 flag를 관리하여, 유저가 앱 진입 후 처음으로 답장을 조회하고 홈화면으로 돌아오면 인앱 리뷰 팝업을 노출시킵니다.
2. 구현방식
홈 화면에서 위 두 가지 사항을 검사하여 충족할 경우 인앱 리뷰 팝업을 노출하고,
shouldShowPopup = false로 업데이트 합니다.✨ PR Point
1. InAppReviewManager
: 애드몹 관련한 함수들과 비슷하다고 생각되어 core 패키지에 위치시켰습니다. 딱히 서버 통신이 있는 것도 아니라 data layer는 탈락, 비즈니스 로직적인가도 애매한 것 같아 domain layer도 탈락. presentation의 util 쪽과 고민하다가 지금처럼 위치 시키게 되었습니다.
: 워낙 쓰이는 시점이 한정적(첫 답장 본 후 홈화면 복귀)이다보니 앱에서 초기화되어 메모리를 잡아먹는 시간이 최소화 되어야한다고 생각이 되어서 싱글톤으로 유지되는 object로 선언하고, 실제 메서드가 호출될 때만 초기화되도록 할 수 있는 object 키워드를 사용했습니다. adShower처럼 interface, impl 분리가 좋을까요, 지금 구조도 괜찮을까요? 개선할 수 있는 부분이 있을까요?
2. 답장확인으로부터 홈화면으로 진입하였는지 검사 로직
navigateToHome 메서드에
isFromReplyDiary: Boolean파라미터를 추가하여, 답장확인으로부터 왔다면 true가 되도록 했습니다. Serializable 네비게이션을 쓰는 만큼 Route 클래스를 넘겨야하나 생각했지만, 그렇게 되면 홈으로 이동하는 화면들을 다 만져야해서 .. 좀 편히 가려고 꼼수를 썼습니다 ㅎㅎ.. 괜찮을까요??!📸 ScreenShot/Video
273-in-app-review-final.mp4
Summary by CodeRabbit