Skip to content

Handle notification permission in BaseActivity #6440

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.

Sign up for GitHub

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

Open
rajvirsingh2 wants to merge 11 commits into
base: main
Choose a base branch
from
+38 −1
Open

Handle notification permission in BaseActivity #6440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0e7ba3d
Handle notification permission in BaseActivity
rajvirsingh2 Mar 24, 2026
0bab39f
Merge branch 'wikimedia:main' into main
rajvirsingh2 Mar 24, 2026
43f4387
Merge branch 'wikimedia:main' into main
rajvirsingh2 Mar 26, 2026
fe84ce4
Merge branch 'wikimedia:main' into main
rajvirsingh2 Mar 27, 2026
623dc18
Changed as requested
rajvirsingh2 Mar 27, 2026
8c54ac9
Changed as requested in DeviceUtil.kt too
rajvirsingh2 Mar 27, 2026
9ecc93d
Changed as requested in DeviceUtil.kt too
rajvirsingh2 Mar 27, 2026
8cec53e
Merge branch 'wikimedia:main' into main
rajvirsingh2 Mar 28, 2026
22431de
Merge branch 'wikimedia:main' into main
rajvirsingh2 Apr 1, 2026
4fb129e
Merge branch 'wikimedia:main' into main
rajvirsingh2 Apr 5, 2026
9b0da9d
Merge branch 'wikimedia:main' into main
rajvirsingh2 Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion app/src/main/java/org/wikipedia/activity/BaseActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.wikipedia.activity

import android.content.Intent
import android.graphics.drawable.ColorDrawable
import android.os.Build
import android.os.Bundle
import android.view.MenuItem
import android.view.MotionEvent
Expand Down Expand Up @@ -79,7 +80,25 @@ abstract class BaseActivity : AppCompatActivity(), ConnectionStateMonitor.Callba
}

private val notificationPermissionLauncher = registerForActivityResult(ActivityResultContracts.RequestPermission()) { granted ->
// TODO: Show message(s) to the user if they deny the permission
if(!granted){
if(Build.VERSION.SDK_INT>= Build.VERSION_CODES.O){
if(ActivityCompat.shouldShowRequestPermissionRationale(this,android.Manifest.permission.POST_NOTIFICATIONS)){
//User denied once->Show Snackbar
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_rationale))
.setAction(R.string.notification_permission_rationale_action){
DeviceUtil.openNotificationSettings(this)
}
.show()
}else{
//User denied (with "Don't Ask Again")
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_denied))
.setAction(R.string.app_settings){
Comment on lines +83 to +95
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The permission being handled here is POST_NOTIFICATIONS, which only exists as a runtime permission on API 33+ (TIRAMISU). The SDK_INT >= O gate is misleading and could mask future changes (e.g., if the launcher were ever invoked on 26–32). Consider gating this logic with Build.VERSION_CODES.TIRAMISU (or removing the version check entirely, since the launcher is already only triggered on 33+ in NotificationPresenter.maybeRequestPermission).

Suggested change
if(!granted){
if(Build.VERSION.SDK_INT>= Build.VERSION_CODES.O){
if(ActivityCompat.shouldShowRequestPermissionRationale(this,android.Manifest.permission.POST_NOTIFICATIONS)){
//User denied once->Show Snackbar
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_rationale))
.setAction(R.string.notification_permission_rationale_action){
DeviceUtil.openNotificationSettings(this)
}
.show()
}else{
//User denied (with "Don't Ask Again")
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_denied))
.setAction(R.string.app_settings){
if (!granted) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
if (ActivityCompat.shouldShowRequestPermissionRationale(this, android.Manifest.permission.POST_NOTIFICATIONS)) {
//User denied once->Show Snackbar
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_rationale))
.setAction(R.string.notification_permission_rationale_action) {
DeviceUtil.openNotificationSettings(this)
}
.show()
} else {
//User denied (with "Don't Ask Again")
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_denied))
.setAction(R.string.app_settings) {

Copilot uses AI. Check for mistakes.
Comment on lines +83 to +95
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Formatting: the added if/else block is missing spaces around operators and after commas (e.g., if(!granted), SDK_INT>=, this,android.Manifest...). This repository enforces Kotlin style via ktlint, so this may fail ./gradlew check; please auto-format these lines.

Suggested change
if(!granted){
if(Build.VERSION.SDK_INT>= Build.VERSION_CODES.O){
if(ActivityCompat.shouldShowRequestPermissionRationale(this,android.Manifest.permission.POST_NOTIFICATIONS)){
//User denied once->Show Snackbar
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_rationale))
.setAction(R.string.notification_permission_rationale_action){
DeviceUtil.openNotificationSettings(this)
}
.show()
}else{
//User denied (with "Don't Ask Again")
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_denied))
.setAction(R.string.app_settings){
if (!granted) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
if (ActivityCompat.shouldShowRequestPermissionRationale(this, android.Manifest.permission.POST_NOTIFICATIONS)) {
//User denied once->Show Snackbar
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_rationale))
.setAction(R.string.notification_permission_rationale_action) {
DeviceUtil.openNotificationSettings(this)
}
.show()
} else {
//User denied (with "Don't Ask Again")
FeedbackUtil.makeSnackbar(this, getString(R.string.notification_permission_denied))
.setAction(R.string.app_settings) {

Copilot uses AI. Check for mistakes.
DeviceUtil.openNotificationSettings(this)
}
.show()
}
}
}
}

private val yearInReviewLauncher = registerForActivityResult(ActivityResultContracts.StartActivityForResult()) {
Expand Down
15 changes: 15 additions & 0 deletions app/src/main/java/org/wikipedia/util/DeviceUtil.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package org.wikipedia.util

import android.app.Activity
import android.content.Context
import android.content.Intent
import android.content.res.Configuration
import android.content.res.Resources
import android.graphics.Color
import android.net.ConnectivityManager
import android.os.Build
import android.os.Handler
import android.provider.Settings
import android.view.KeyCharacterMap
import android.view.KeyEvent
import android.view.View
Expand Down Expand Up @@ -107,6 +109,19 @@ object DeviceUtil {
WindowCompat.getInsetsController(activity.window, activity.window.decorView).isAppearanceLightStatusBars = !WikipediaApp.instance.currentTheme.isDark
}

fun openNotificationSettings(context: Context){
val intent= if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
Comment on lines +112 to +113
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

PR description mentions adding openAppNotificationSettings(), but the code introduces openNotificationSettings(). Consider aligning the method name (or updating the PR description) to avoid confusion for reviewers and future grep/search.

Copilot uses AI. Check for mistakes.
Intent(Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply {
putExtra(Settings.EXTRA_APP_PACKAGE,context.packageName)
}
}else{
Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply {
data = android.net.Uri.fromParts("package", context.packageName, null)
}
}
Comment on lines +112 to +121
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Formatting: this block deviates from standard Kotlin formatting (missing spaces after fun params / before {, around = and after commas, and } else { style). Since the project runs ktlint as part of check, this is likely to fail CI; please run ktlint/auto-format on this file.

Copilot uses AI. Check for mistakes.
context.startActivity(intent as? Intent)
Comment on lines +112 to +122
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

openNotificationSettings() uses ACTION_APPLICATION_DETAILS_SETTINGS for pre-O devices, which opens the generic app details screen rather than the app’s notification settings. There’s already a pre-O approach in NotificationSettingsPreferenceLoader using the android.settings.APP_NOTIFICATION_SETTINGS action with app_package/app_uid extras; consider using that here (and only falling back to app details if the intent can’t be resolved).

Suggested change
fun openNotificationSettings(context: Context){
val intent= if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
Intent(Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply {
putExtra(Settings.EXTRA_APP_PACKAGE,context.packageName)
}
}else{
Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply {
data = android.net.Uri.fromParts("package", context.packageName, null)
}
}
context.startActivity(intent as? Intent)
fun openNotificationSettings(context: Context) {
val intent = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
Intent(Settings.ACTION_APP_NOTIFICATION_SETTINGS).apply {
putExtra(Settings.EXTRA_APP_PACKAGE, context.packageName)
}
} else {
// Try the dedicated app notification settings screen on pre-O, if available,
// and fall back to the generic app details screen otherwise.
Intent("android.settings.APP_NOTIFICATION_SETTINGS").apply {
putExtra("app_package", context.packageName)
putExtra("app_uid", context.applicationInfo.uid)
}.takeIf { it.resolveActivity(context.packageManager) != null }
?: Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS).apply {
data = android.net.Uri.fromParts("package", context.packageName, null)
}
}
context.startActivity(intent)

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

context.startActivity(intent as? Intent) is unnecessary and makes the call nullable; if that cast were ever to fail it would pass null and crash at runtime. Use context.startActivity(intent) and either (a) require an Activity parameter, or (b) add FLAG_ACTIVITY_NEW_TASK when context isn’t an Activity, since Context can be an application context.

Suggested change
context.startActivity(intent as? Intent)
if (context !is Activity) {
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
}
context.startActivity(intent)

Copilot uses AI. Check for mistakes.
}

val isOnWiFi: Boolean
get() {
val info = (WikipediaApp.instance.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager)
Expand Down
3 changes: 3 additions & 0 deletions app/src/main/res/values/strings.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,9 @@
<string name="notifications_all_types_text">All types</string>
<string name="notifications_menu_user_talk_page">%s\'s talk page</string>
<string name="notifications_offline_disable_message">The function is not available while offline.</string>
<string name="notification_permission_rationale">Enable Notification to stay updated</string>
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

User-facing copy: Enable Notification to stay updated is grammatically incorrect (singular/plural) and reads like a title rather than a sentence. Consider adjusting to something like “Enable notifications to stay up to date.” (and match capitalization/punctuation style used by nearby strings).

Suggested change
<string name="notification_permission_rationale">Enable Notification to stay updated</string>
<string name="notification_permission_rationale">Enable notifications to stay up to date.</string>

Copilot uses AI. Check for mistakes.
<string name="notification_permission_rationale_action">Settings</string>
<string name="notification_permission_denied">Notifications are disabled. Enable them in settings.</string>
<!-- /Notifications -->

<!-- The Feed -->
Expand Down