Skip to content

Commit 94fb7e4

Browse files
kaibolaykaibolay
committed
Simplify requesting permissions
1 parent 4d64675 commit 94fb7e4

File tree

2 files changed

+87
-94
lines changed

2 files changed

+87
-94
lines changed

‎firebase-appdistribution/test-app/src/main/java/com/googletest/firebase/appdistribution/testapp/MainActivity.kt

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import android.widget.ArrayAdapter
1313
import android.widget.AutoCompleteTextView
1414
import android.widget.ProgressBar
1515
import android.widget.TextView
16+
import androidx.activity.result.ActivityResultLauncher
1617
import androidx.appcompat.app.AppCompatActivity
1718
import androidx.appcompat.widget.AppCompatButton
1819
import androidx.core.widget.doOnTextChanged
@@ -46,7 +47,7 @@ class MainActivity : AppCompatActivity() {
4647
lateinit var feedbackTriggerMenu: TextInputLayout
4748

4849
var updateTask: Task<Void>? = null
49-
var release: AppDistributionRelease? = null
50+
var notificationPermissionLauncher: ActivityResultLauncher<String>? = null
5051

5152
override fun onCreate(savedInstanceState: Bundle?) {
5253
super.onCreate(savedInstanceState)
@@ -66,6 +67,7 @@ class MainActivity : AppCompatActivity() {
6667
progressPercent = findViewById(R.id.progress_percentage)
6768
signInStatus = findViewById(R.id.sign_in_status)
6869
progressBar = findViewById(R.id.progress_bar)
70+
notificationPermissionLauncher = NotificationFeedbackTrigger.registerPermissionLauncher(this)
6971

7072
// Set up feedback trigger menu
7173
feedbackTriggerMenu = findViewById(R.id.feedbackTriggerMenu)
@@ -98,7 +100,7 @@ class MainActivity : AppCompatActivity() {
98100
FeedbackTrigger.NOTIFICATION.label -> {
99101
disableAllFeedbackTriggers()
100102
Log.i(TAG, "Enabling notification trigger")
101-
NotificationFeedbackTrigger.enable(this)
103+
NotificationFeedbackTrigger.enable(this, notificationPermissionLauncher)
102104
}
103105
}
104106
}
@@ -194,11 +196,10 @@ class MainActivity : AppCompatActivity() {
194196
firebaseAppDistribution
195197
.checkForNewRelease()
196198
.addOnSuccessListener {
197-
release = it
198199
setupUI(
199200
isSignedIn = firebaseAppDistribution.isTesterSignedIn,
200-
isUpdateAvailable = release != null,
201-
release = release)
201+
isUpdateAvailable = it != null,
202+
release = it)
202203
}
203204
.addOnFailureListener { failureListener(it) }
204205
}

‎firebase-appdistribution/test-app/src/main/java/com/googletest/firebase/appdistribution/testapp/NotificationFeedbackTrigger.kt

Lines changed: 81 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import android.util.Log
1515
import androidx.activity.result.ActivityResultCaller
1616
import androidx.activity.result.ActivityResultLauncher
1717
import androidx.activity.result.contract.ActivityResultContracts
18-
import androidx.appcompat.app.AppCompatActivity
1918
import androidx.core.app.NotificationCompat
2019
import androidx.core.app.NotificationManagerCompat
2120
import androidx.core.content.ContextCompat
@@ -24,7 +23,6 @@ import com.google.firebase.ktx.Firebase
2423
import com.googletest.firebase.appdistribution.testapp.NotificationFeedbackTrigger.SCREENSHOT_FILE_NAME
2524
import com.googletest.firebase.appdistribution.testapp.NotificationFeedbackTrigger.takeScreenshot
2625
import java.io.IOException
27-
import java.util.*
2826

2927
@SuppressLint("StaticFieldLeak") // Reference to Activity is set to null in onActivityDestroyed
3028
object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
@@ -36,9 +34,6 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
3634
private var isEnabled = false
3735
private var hasRequestedPermission = false
3836
private var currentActivity: Activity? = null
39-
// TODO(lkellogg): this is getting too complex - simplify it by, for example, only enabling this
40-
// trigger on a per-activity basis instead of app-wide
41-
private var requestPermissionLaunchers: WeakHashMap<Activity, ActivityResultLauncher<String>?> = WeakHashMap()
4237

4338
fun initialize(application: Application) {
4439
// Create the NotificationChannel, but only on API 26+ because
@@ -59,11 +54,43 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
5954
application.registerActivityLifecycleCallbacks(this)
6055
}
6156

62-
fun enable(currentActivity: Activity? = null) {
63-
this.currentActivity = currentActivity
57+
fun <T> registerPermissionLauncher(activity: T): ActivityResultLauncher<String>? where
58+
T : Activity,
59+
T : ActivityResultCaller {
60+
if (hasRequestedPermission) {
61+
Log.i(TAG, "Already request permission; Not trying again.")
62+
return null
63+
}
64+
65+
return activity.registerForActivityResult(ActivityResultContracts.RequestPermission()) {
66+
isGranted: Boolean ->
67+
if (!isEnabled) {
68+
Log.w(TAG, "Trigger disabled after permission check. Abandoning notification.")
69+
} else if (isGranted) {
70+
showNotification(activity)
71+
} else {
72+
Log.i(TAG, "Permission not granted")
73+
// TODO: Ideally we would show a message indicating the impact of not
74+
// enabling the permission, but there's no way to know if they've
75+
// permanently denied the permission, and we don't want to show them a
76+
// message after each time we try to post a notification.
77+
}
78+
}
79+
}
80+
81+
fun enable(activity: Activity? = null, launcher: ActivityResultLauncher<String>? = null) {
82+
currentActivity = activity
6483
isEnabled = true
65-
if (currentActivity != null) {
66-
showNotification(currentActivity)
84+
if (activity != null) {
85+
if (hasPermission(activity)) {
86+
showNotification(activity)
87+
} else {
88+
if (launcher != null) {
89+
requestPermission(activity, launcher)
90+
} else {
91+
Log.i(TAG, "Not requesting permission, because of no launcher was provided.")
92+
}
93+
}
6794
}
6895
}
6996

@@ -77,62 +104,52 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
77104
}
78105

79106
private fun showNotification(activity: Activity) {
80-
if (ContextCompat.checkSelfPermission(activity, POST_NOTIFICATIONS) == PERMISSION_GRANTED) {
81-
val intent = Intent(activity, TakeScreenshotAndTriggerFeedbackActivity::class.java)
82-
intent.addFlags(Intent.FLAG_ACTIVITY_NO_HISTORY)
83-
val pendingIntent =
84-
PendingIntent.getActivity(
85-
activity,
86-
/* requestCode= */ 0,
87-
intent,
88-
PendingIntent.FLAG_IMMUTABLE
89-
)
90-
val builder =
91-
NotificationCompat.Builder(activity, FEEBACK_NOTIFICATION_CHANNEL_ID)
92-
.setSmallIcon(R.mipmap.ic_launcher)
93-
.setContentTitle(activity.getText(R.string.feedback_notification_title))
94-
.setContentText(activity.getText(R.string.feedback_notification_text))
95-
.setPriority(NotificationCompat.PRIORITY_LOW)
96-
.setContentIntent(pendingIntent)
97-
val notificationManager = NotificationManagerCompat.from(activity)
98-
Log.i(TAG, "Showing notification")
99-
notificationManager.notify(FEEDBACK_NOTIFICATION_ID, builder.build())
100-
} else {
101-
// no permission to post notifications - try to get it
102-
if (hasRequestedPermission) {
103-
Log.i(
104-
TAG,
105-
"We've already request permission. Not requesting again for the life of the activity."
106-
)
107-
} else {
108-
requestPermission(activity)
109-
}
110-
}
107+
val intent = Intent(activity, TakeScreenshotAndTriggerFeedbackActivity::class.java)
108+
intent.addFlags(Intent.FLAG_ACTIVITY_NO_HISTORY)
109+
val pendingIntent =
110+
PendingIntent.getActivity(
111+
activity,
112+
/* requestCode = */ 0,
113+
intent,
114+
PendingIntent.FLAG_IMMUTABLE
115+
)
116+
val builder =
117+
NotificationCompat.Builder(activity, FEEBACK_NOTIFICATION_CHANNEL_ID)
118+
.setSmallIcon(R.mipmap.ic_launcher)
119+
.setContentTitle(activity.getText(R.string.feedback_notification_title))
120+
.setContentText(activity.getText(R.string.feedback_notification_text))
121+
.setPriority(NotificationCompat.PRIORITY_LOW)
122+
.setContentIntent(pendingIntent)
123+
val notificationManager = NotificationManagerCompat.from(activity)
124+
Log.i(TAG, "Showing notification")
125+
notificationManager.notify(FEEDBACK_NOTIFICATION_ID, builder.build())
111126
}
112127

113-
private fun requestPermission(activity: Activity) {
114-
var launcher = requestPermissionLaunchers[activity]
115-
if (launcher == null) {
116-
Log.i(TAG, "Not requesting permission, because of inability to register for result.")
128+
private fun hasPermission(activity: Activity) =
129+
ContextCompat.checkSelfPermission(activity, POST_NOTIFICATIONS) == PERMISSION_GRANTED
130+
131+
private fun requestPermission(activity: Activity, launcher: ActivityResultLauncher<String>) {
132+
if (hasRequestedPermission) {
133+
Log.i(TAG, "Already request permission; Not trying again.")
134+
return
135+
}
136+
if (activity.shouldShowRequestPermissionRationale(POST_NOTIFICATIONS)) {
137+
Log.i(TAG, "Showing customer rationale for requesting permission.")
138+
AlertDialog.Builder(activity)
139+
.setMessage(
140+
"Using a notification to initiate feedback to the developer. " +
141+
"To enable this feature, allow the app to post notifications."
142+
)
143+
.setPositiveButton("OK") { _, _ ->
144+
Log.i(TAG, "Launching request for permission.")
145+
launcher.launch(POST_NOTIFICATIONS)
146+
}
147+
.show()
117148
} else {
118-
if (activity.shouldShowRequestPermissionRationale(POST_NOTIFICATIONS)) {
119-
Log.i(TAG, "Showing customer rationale for requesting permission.")
120-
AlertDialog.Builder(activity)
121-
.setMessage(
122-
"Using a notification to initiate feedback to the developer. " +
123-
"To enable this feature, allow the app to post notifications."
124-
)
125-
.setPositiveButton("OK") { _, _ ->
126-
Log.i(TAG, "Launching request for permission.")
127-
launcher.launch(POST_NOTIFICATIONS)
128-
}
129-
.show()
130-
} else {
131-
Log.i(TAG, "Launching request for permission without rationale.")
132-
launcher.launch(POST_NOTIFICATIONS)
133-
}
134-
hasRequestedPermission = true
149+
Log.i(TAG, "Launching request for permission without rationale.")
150+
launcher.launch(POST_NOTIFICATIONS)
135151
}
152+
hasRequestedPermission = true
136153
}
137154

138155
private fun cancelNotification(context: Context) {
@@ -160,35 +177,10 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
160177
Log.d(TAG, "clearing current activity")
161178
currentActivity = null
162179
}
163-
requestPermissionLaunchers[activity] = null
164-
}
165-
166-
override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {
167-
if (activity is ActivityResultCaller && !hasRequestedPermission) {
168-
requestPermissionLaunchers[activity] =
169-
activity.registerForActivityResult(ActivityResultContracts.RequestPermission()) {
170-
isGranted: Boolean ->
171-
if (!isEnabled) {
172-
Log.w(TAG, "Trigger disabled after permission check. Abandoning notification.")
173-
} else if (isGranted) {
174-
showNotification(activity)
175-
} else {
176-
Log.i(TAG, "Permission not granted")
177-
// TODO: Ideally we would show a message indicating the impact of not
178-
// enabling the permission, but there's no way to know if they've
179-
// permanently denied the permission, and we don't want to show them a
180-
// message after each time we try to post a notification.
181-
}
182-
}
183-
} else {
184-
Log.w(
185-
TAG,
186-
"Not showing notification because this activity can't register for permission request results: $activity"
187-
)
188-
}
189180
}
190181

191182
// Other lifecycle methods
183+
override fun onActivityCreated(activity: Activity, savedInstanceState: Bundle?) {}
192184
override fun onActivityStarted(activity: Activity) {}
193185
override fun onActivityStopped(activity: Activity) {}
194186
override fun onActivitySaveInstanceState(activity: Activity, outState: Bundle) {}
@@ -215,10 +207,10 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
215207
}
216208
}
217209

218-
class TakeScreenshotAndTriggerFeedbackActivity : AppCompatActivity() {
210+
class TakeScreenshotAndTriggerFeedbackActivity : Activity() {
219211
override fun onCreate(savedInstanceState: Bundle?) {
220212
super.onCreate(savedInstanceState)
221-
takeScreenshot()
213+
takeScreenshot() // at this point currentActivity still points to the previous activity
222214
}
223215

224216
override fun onResume() {

0 commit comments

Comments
 (0)