Add function to check Appstore application's existence #8
Conversation
WalkthroughThis pull request introduces enhancements to the application's interaction with a specific app store. A new function, Changes
Sequence Diagram(s)sequenceDiagram
participant M as MainActivity
participant C as ContextExt
participant P as PackageManager
participant D as Alert Dialog
participant U as User
M->>C: call checkIfAppstoreIsInstalled("ai.elimu.appstore.debug")
C->>P: packageManager.getPackageInfo("ai.elimu.appstore.debug", 0)
alt Package Installed
P-->>C: PackageInfo retrieved
C-->>M: returns true (logs installation status)
else Package Not Installed
P-->>C: NameNotFoundException thrown
C->>C: Log error message
C->>D: Display alert dialog with message from `appstore_needed`
D->>U: Prompt "Download" action
U->>D: Clicks "Download"
D->>C: Trigger download intent via startActivity
C-->>M: returns false
end
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
utils/src/main/java/ai/elimu/common/utils/ContextExt.kt (2)
51-51: Remove redundant exception parameter in Log.e call.The third parameter in the Log.e method call is redundant as it logs the exception twice. The message already contains the exception details.
- Log.e("checkIfAppstoreIsInstalled","getPackageInfo exception: " + e.message, e) + Log.e("checkIfAppstoreIsInstalled","getPackageInfo exception: " + e.message)
56-58: Move hardcoded URL to a constant or resource.The GitHub URL is hardcoded in the function. Consider moving it to a constant or string resource for better maintainability and flexibility.
+ // Add at the top of the file + private const val APPSTORE_DOWNLOAD_URL = "https://github.com/elimu-ai/appstore/releases" // Then in the function - val openDownloadPage = Intent(Intent.ACTION_VIEW, - "https://github.com/elimu-ai/appstore/releases".toUri()) + val openDownloadPage = Intent(Intent.ACTION_VIEW, APPSTORE_DOWNLOAD_URL.toUri())Or better yet, add it to your string resources:
<string name="appstore_download_url">https://github.com/elimu-ai/appstore/releases</string>And then use:
val openDownloadPage = Intent(Intent.ACTION_VIEW, getString(R.string.appstore_download_url).toUri())utils/src/main/res/values/strings.xml (1)
3-4: Consider extracting the app name as a separate string resource.The app name "elimu.ai Appstore" appears in the message. Consider extracting it to a separate string resource (e.g.,
<string name="appstore_name">elimu.ai Appstore</string>) and referencing it in the message using string formatting. This would make future app name changes easier to maintain.- <string name="appstore_needed">You need to download elimu.ai Appstore first.</string> + <string name="appstore_name">elimu.ai Appstore</string> + <string name="appstore_needed">You need to download %1$s first.</string>And then in code:
getString(R.string.appstore_needed, getString(R.string.appstore_name))app/src/main/java/ai/elimu/common/utils/ui/MainActivity.kt (1)
57-58: Extract hardcoded package name to a constant.The package name "ai.elimu.appstore.debug" is hardcoded in the
onCreatemethod. Consider extracting it to a constant or configuration value, especially since you're checking for a debug version which will likely be different in production.class MainActivity: AppCompatActivity() { private val TAG = "MainActivity" private lateinit var ttsViewModel: TextToSpeechViewModel + private companion object { + const val APPSTORE_PACKAGE_DEBUG = "ai.elimu.appstore.debug" + const val APPSTORE_PACKAGE_RELEASE = "ai.elimu.appstore" + } override fun onCreate(savedInstanceState: Bundle?) { // ... - val isAppstoreInstalled = checkIfAppstoreIsInstalled("ai.elimu.appstore.debug") + val isAppstoreInstalled = checkIfAppstoreIsInstalled(APPSTORE_PACKAGE_DEBUG) Log.d(TAG, "isAppstoreInstalled: $isAppstoreInstalled") } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/ai/elimu/common/utils/ui/MainActivity.kt(2 hunks)utils/src/main/AndroidManifest.xml(1 hunks)utils/src/main/java/ai/elimu/common/utils/ContextExt.kt(2 hunks)utils/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/src/main/java/ai/elimu/common/utils/ui/MainActivity.kt (1)
utils/src/main/java/ai/elimu/common/utils/ContextExt.kt (1)
checkIfAppstoreIsInstalled(43-69)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (windows-latest, 17)
- GitHub Check: build (windows-latest, 21)
🔇 Additional comments (2)
app/src/main/java/ai/elimu/common/utils/ui/MainActivity.kt (1)
57-58: What happens if the app store isn't installed?Currently, you're checking if the app store is installed but not taking any specific action based on the result besides logging. Consider what should happen if
isAppstoreInstalledisfalse:
- Should the app continue normally?
- Should certain features be disabled?
- Should the app exit after the user dismisses the dialog?
The current implementation shows a non-cancelable dialog, but the code continues executing regardless of whether the app store is installed. Consider adding logic to handle the app's behavior based on this check.
utils/src/main/AndroidManifest.xml (1)
4-7: LGTM! Package visibility properly implemented.The
<queries>element correctly declares the packages that the app needs to interact with. This is the proper implementation for package visibility restrictions introduced in Android 11+, allowing your app to check if the specified app store packages are installed.
| fun Context.checkIfAppstoreIsInstalled(appStoreId: String): Boolean { | ||
| try { | ||
| val packageInfoAppstore: PackageInfo = | ||
| packageManager.getPackageInfo(appStoreId, 0) | ||
| Log.i("checkIfAppstoreIsInstalled", | ||
| "packageInfoAppstore.versionCode: " + packageInfoAppstore.versionCode) | ||
| return true | ||
| } catch (e: PackageManager.NameNotFoundException) { | ||
| Log.e("checkIfAppstoreIsInstalled","getPackageInfo exception: " + e.message, e) | ||
| AlertDialog.Builder(this) | ||
| .setMessage(this.getString(R.string.appstore_needed)) | ||
| .setPositiveButton(this.getString(R.string.download) | ||
| ) { _, _ -> | ||
| val openDownloadPage = Intent(Intent.ACTION_VIEW, | ||
| "https://github.com/elimu-ai/appstore/releases".toUri()) | ||
| try { | ||
| startActivity(openDownloadPage) | ||
| } catch (e: Exception) { | ||
| e.printStackTrace() | ||
| Log.e("checkIfAppstoreIsInstalled", "startActivity exception: " + e.message) | ||
| } | ||
| } | ||
| .setCancelable(false) | ||
| .create().show() | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce code duplication and improve flexibility.
The new checkIfAppstoreIsInstalled function duplicates much of the logic from the existing isPackageInstalled function. Consider refactoring to reuse the existing function instead of creating a specialized version.
Here's an improved implementation:
-fun Context.checkIfAppstoreIsInstalled(appStoreId: String): Boolean {
- try {
- val packageInfoAppstore: PackageInfo =
- packageManager.getPackageInfo(appStoreId, 0)
- Log.i("checkIfAppstoreIsInstalled",
- "packageInfoAppstore.versionCode: " + packageInfoAppstore.versionCode)
- return true
- } catch (e: PackageManager.NameNotFoundException) {
- Log.e("checkIfAppstoreIsInstalled","getPackageInfo exception: " + e.message, e)
- AlertDialog.Builder(this)
- .setMessage(this.getString(R.string.appstore_needed))
- .setPositiveButton(this.getString(R.string.download)
- ) { _, _ ->
- val openDownloadPage = Intent(Intent.ACTION_VIEW,
- "https://github.com/elimu-ai/appstore/releases".toUri())
- try {
- startActivity(openDownloadPage)
- } catch (e: Exception) {
- e.printStackTrace()
- Log.e("checkIfAppstoreIsInstalled", "startActivity exception: " + e.message)
- }
- }
- .setCancelable(false)
- .create().show()
- return false
- }
+fun Context.checkIfAppstoreIsInstalled(appStoreId: String): Boolean {
+ val appstoreUrl = "https://github.com/elimu-ai/appstore/releases"
+
+ return isPackageInstalled(
+ packageName = appStoreId,
+ // Using empty strings as we're creating the intent differently in the lambda
+ launchPackage = "",
+ launchClass = "",
+ dialogMessage = getString(R.string.appstore_needed),
+ buttonText = getString(R.string.download),
+ onButtonClick = {
+ val openDownloadPage = Intent(Intent.ACTION_VIEW, appstoreUrl.toUri())
+ try {
+ startActivity(openDownloadPage)
+ } catch (e: Exception) {
+ e.printStackTrace()
+ Log.e("checkIfAppstoreIsInstalled", "startActivity exception: " + e.message)
+ }
+ }
+ )
}This would require modifying the existing isPackageInstalled function to accept a lambda for the button click action:
fun Context.isPackageInstalled(
packageName: String,
launchPackage: String,
launchClass: String,
dialogMessage: String,
buttonText: String,
onButtonClick: () -> Unit = {
val openProviderIntent = Intent().apply {
setClassName(launchPackage, launchClass)
}
try {
startActivity(openProviderIntent)
} catch (e: Exception) {
e.printStackTrace()
Log.e("isPackageInstalled", "startActivity exception: " + e.message)
}
}
): Boolean {
// Rest of the function remains the same, just call onButtonClick in the lambda
}Additionally, the hardcoded URL should be moved to a constant or resource to make it configurable.
Co-authored-by: Jo G. <1451036+jo-elimu@users.noreply.github.com>
2 participants
Add function to check Appstore application's existence.
Related thread: elimu-ai/vitabu#171
Summary by CodeRabbit