Skip to content

Commit 50f4148

Browse files
lfkellogglfkellogg
committed
Address Kai's feedback
1 parent ae8db00 commit 50f4148

File tree

11 files changed

+74
-57
lines changed

11 files changed

+74
-57
lines changed

‎firebase-appdistribution-api/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -205,10 +205,10 @@ public interface FirebaseAppDistribution {
205205
*
206206
* @param infoTextResourceId string resource ID of text to display to the tester before collecting
207207
* feedback data (e.g. Terms and Conditions)
208-
* @param importance the amount the user should be interrupted by notifications from the feedback
209-
* notification channel. Once the channel's importance is set it cannot be changed except by
210-
* the user. See {@link NotificationChannel#setImportance}. On platforms below Android 8, the
211-
* importance will be translated into a comparable notification priority (see {@link
208+
* @param importance the level of interruption for the feedback notification channel. Once the
209+
* channel's importance is set it cannot be changed except by the user. See {@link
210+
* NotificationChannel#setImportance}. On platforms below Android 8, the importance will be
211+
* translated into a comparable notification priority (see {@link
212212
* NotificationCompat.Builder#setPriority}).
213213
*/
214214
void showFeedbackNotification(int infoTextResourceId, int importance);
@@ -236,10 +236,10 @@ public interface FirebaseAppDistribution {
236236
*
237237
* @param infoText text to display to the tester before collecting feedback data (e.g. Terms and
238238
* Conditions)
239-
* @param importance the amount the user should be interrupted by notifications from the feedback
240-
* notification channel. Once the channel's importance is set it cannot be changed except by
241-
* the user. See {@link NotificationChannel#setImportance}. On platforms below Android 8, the
242-
* importance will be translated into a comparable notification priority (see {@link
239+
* @param importance the level of interruption for the feedback notification channel. Once the
240+
* channel's importance is set it cannot be changed except by the user. See {@link
241+
* NotificationChannel#setImportance}. On platforms below Android 8, the importance will be
242+
* translated into a comparable notification priority (see {@link
243243
* NotificationCompat.Builder#setPriority}).
244244
*/
245245
void showFeedbackNotification(@NonNull CharSequence infoText, int importance);

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionLifecycleNotifier.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,10 @@ public void onActivitySaveInstanceState(@NonNull Activity activity, @NonNull Bun
334334
@Override
335335
public void onActivityDestroyed(@NonNull Activity activity) {
336336
synchronized (lock) {
337+
// If an activity is destroyed, delete all references to it, including the previous activity
337338
if (currentActivity == activity) {
338339
updateCurrentActivity(null);
339340
}
340-
341-
// If an activity is destroyed, delete all references to it, including the previous activity
342341
if (previousActivity == activity) {
343342
previousActivity = null;
344343
}

‎firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionNotificationsManager.java

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,29 +33,24 @@
3333
class FirebaseAppDistributionNotificationsManager {
3434
private static final String TAG = "FirebaseAppDistributionNotificationsManager";
3535

36+
private static final String PACKAGE_PREFIX = "com.google.firebase.appdistribution";
37+
3638
@VisibleForTesting
37-
static final String CHANNEL_GROUP_ID =
38-
"com.google.firebase.appdistribution.notification_channel_group_id";
39+
static final String CHANNEL_GROUP_ID = prependPackage("notification_channel_group_id");
3940

4041
@VisibleForTesting
4142
enum Notification {
42-
APP_UPDATE(
43-
"com.google.firebase.appdistribution.notification_channel_id",
44-
"com.google.firebase.appdistribution.app_update_notification_tag",
45-
0),
46-
FEEDBACK(
47-
"com.google.firebase.appdistribution.feedback_notification_channel_id",
48-
"com.google.firebase.appdistribution.feedback_notification_tag",
49-
1);
43+
APP_UPDATE("notification_channel_id", "app_update_notification_tag"),
44+
FEEDBACK("feedback_notification_channel_id", "feedback_notification_tag");
5045

5146
final String channelId;
5247
final String tag;
5348
final int id;
5449

55-
Notification(String channelId, String tag, int id) {
56-
this.channelId = channelId;
57-
this.tag = tag;
58-
this.id = id;
50+
Notification(String channelId, String tag) {
51+
this.channelId = prependPackage(channelId);
52+
this.tag = prependPackage(tag);
53+
this.id = ordinal();
5954
}
6055
}
6156

@@ -213,4 +208,8 @@ private void createChannel(Notification notification, int name, int description,
213208
// or other notification behaviors after this
214209
notificationManager.createNotificationChannel(channel);
215210
}
211+
212+
private static String prependPackage(String id) {
213+
return String.format("%s.%s", PACKAGE_PREFIX, id);
214+
}
216215
}

‎firebase-appdistribution/test-app/src/main/AndroidManifest.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
<application
77
android:allowBackup="true"
88
android:icon="@mipmap/ic_launcher"
9-
android:label="@string/app_name"
9+
android:label="@string/appName"
1010
android:roundIcon="@mipmap/ic_launcher_round"
1111
android:supportsRtl="true"
1212
android:theme="@style/Theme.AppDistributionTestAppSample"

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ class AppDistroTestApplication : Application() {
77
super.onCreate()
88

99
// Perform any required trigger initialization here
10-
ScreenshotDetectionFeedbackTrigger.initialize(this, R.string.terms_and_conditions)
11-
// NotificationFeedbackTrigger.initialize(this);
10+
ScreenshotDetectionFeedbackTrigger.initialize(this, R.string.termsAndConditions)
11+
CustomNotificationFeedbackTrigger.initialize(this);
1212

1313
// Default feedback triggers can optionally be enabled application-wide here
1414
// ShakeForFeedback.enable(this)

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import com.google.firebase.ktx.Firebase
2424
import java.io.IOException
2525

2626
@SuppressLint("StaticFieldLeak") // Reference to Activity is set to null in onActivityDestroyed
27-
object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
27+
object CustomNotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
2828
private const val TAG: String = "NotificationFeedbackTrigger"
2929
private const val FEEBACK_NOTIFICATION_CHANNEL_ID = "InAppFeedbackNotification"
3030
private const val FEEDBACK_NOTIFICATION_ID = 1
@@ -49,11 +49,11 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
4949
val channel =
5050
NotificationChannel(
5151
FEEBACK_NOTIFICATION_CHANNEL_ID,
52-
application.getString(R.string.feedback_trigger_notification_channel_name),
52+
application.getString(R.string.feedbackTriggerNotificationChannelName),
5353
NotificationManager.IMPORTANCE_HIGH
5454
)
5555
channel.description =
56-
application.getString(R.string.feedback_trigger_notification_channel_description)
56+
application.getString(R.string.feedbackTriggerNotificationChannelDescription)
5757
application
5858
.getSystemService(NotificationManager::class.java)
5959
.createNotificationChannel(channel)
@@ -156,8 +156,8 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
156156
val builder =
157157
NotificationCompat.Builder(context, FEEBACK_NOTIFICATION_CHANNEL_ID)
158158
.setSmallIcon(R.mipmap.ic_launcher)
159-
.setContentTitle(context.getText(R.string.feedback_trigger_notification_title))
160-
.setContentText(context.getText(R.string.feedback_trigger_notification_text))
159+
.setContentTitle(context.getText(R.string.feedbackTriggerNotificationTitle))
160+
.setContentText(context.getText(R.string.feedbackTriggerNotificationText))
161161
.setPriority(NotificationCompat.PRIORITY_HIGH)
162162
.setContentIntent(pendingIntent)
163163
val notificationManager = NotificationManagerCompat.from(context)
@@ -198,7 +198,7 @@ object NotificationFeedbackTrigger : Application.ActivityLifecycleCallbacks {
198198
class TakeScreenshotAndTriggerFeedbackActivity : Activity() {
199199
override fun onCreate(savedInstanceState: Bundle?) {
200200
super.onCreate(savedInstanceState)
201-
val activity = NotificationFeedbackTrigger.activityToScreenshot
201+
val activity = CustomNotificationFeedbackTrigger.activityToScreenshot
202202
if (activity == null) {
203203
Log.e(TAG, "Can't take screenshot because activity is unknown")
204204
return
@@ -209,7 +209,8 @@ class TakeScreenshotAndTriggerFeedbackActivity : Activity() {
209209
override fun onResume() {
210210
super.onResume()
211211
val screenshotUri = Uri.fromFile(getFileStreamPath(SCREENSHOT_FILE_NAME))
212-
Firebase.appDistribution.startFeedback(R.string.terms_and_conditions, screenshotUri)
212+
Firebase.appDistribution.startFeedback(R.string.termsAndConditions, screenshotUri)
213+
finish()
213214
}
214215

215216
fun takeScreenshot(activity: Activity) {

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

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,16 @@ class MainActivity : AppCompatActivity() {
6767
signInStatus = findViewById(R.id.sign_in_status)
6868
progressBar = findViewById(R.id.progress_bar)
6969

70-
firebaseAppDistribution.showFeedbackNotification(
71-
R.string.terms_and_conditions, NotificationManagerCompat.IMPORTANCE_HIGH);
70+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
71+
CustomNotificationFeedbackTrigger.requestPermission(this)
72+
}
7273

7374
// Set up feedback trigger menu
7475
feedbackTriggerMenu = findViewById(R.id.feedbackTriggerMenu)
7576
val items = listOf(
7677
FeedbackTrigger.NONE.label,
78+
FeedbackTrigger.SDK_NOTIFICATION.label,
79+
FeedbackTrigger.CUSTOM_NOTIFICATION.label,
7780
FeedbackTrigger.SHAKE.label,
7881
FeedbackTrigger.SCREENSHOT.label,
7982
)
@@ -87,6 +90,17 @@ class MainActivity : AppCompatActivity() {
8790
FeedbackTrigger.NONE.label -> {
8891
disableAllFeedbackTriggers()
8992
}
93+
FeedbackTrigger.SDK_NOTIFICATION.label -> {
94+
disableAllFeedbackTriggers()
95+
Log.i(TAG, "Enabling notification trigger (SDK)")
96+
firebaseAppDistribution.showFeedbackNotification(
97+
R.string.termsAndConditions, NotificationManagerCompat.IMPORTANCE_HIGH)
98+
}
99+
FeedbackTrigger.CUSTOM_NOTIFICATION.label -> {
100+
disableAllFeedbackTriggers()
101+
Log.i(TAG, "Enabling notification trigger (custom)")
102+
CustomNotificationFeedbackTrigger.enable(this)
103+
}
90104
FeedbackTrigger.SHAKE.label -> {
91105
disableAllFeedbackTriggers()
92106
Log.i(TAG, "Enabling shake for feedback trigger")
@@ -108,6 +122,8 @@ class MainActivity : AppCompatActivity() {
108122

109123
private fun disableAllFeedbackTriggers() {
110124
Log.i(TAG, "Disabling all feedback triggers")
125+
firebaseAppDistribution.cancelFeedbackNotification()
126+
CustomNotificationFeedbackTrigger.disable()
111127
ShakeForFeedback.disable(application)
112128
ScreenshotDetectionFeedbackTrigger.disable()
113129
}
@@ -121,7 +137,7 @@ class MainActivity : AppCompatActivity() {
121137
override fun onOptionsItemSelected(item: MenuItem): Boolean {
122138
return when (item.itemId) {
123139
R.id.startFeedbackMenuItem -> {
124-
Firebase.appDistribution.startFeedback(R.string.terms_and_conditions)
140+
Firebase.appDistribution.startFeedback(R.string.termsAndConditions)
125141
true
126142
}
127143
else -> super.onOptionsItemSelected(item)
@@ -206,18 +222,18 @@ class MainActivity : AppCompatActivity() {
206222
}
207223
}
208224

209-
signOutButton.setOnClickListener {
210-
firebaseAppDistribution.signOutTester()
211-
setupUI(
212-
isSignedIn = firebaseAppDistribution.isTesterSignedIn, isUpdateAvailable = false)
213-
}
225+
signOutButton.setOnClickListener {
226+
firebaseAppDistribution.signOutTester()
227+
setupUI(
228+
isSignedIn = firebaseAppDistribution.isTesterSignedIn, isUpdateAvailable = false)
229+
}
214230

215-
signOutButtonBackground.setOnClickListener {
216-
executorService.execute { firebaseAppDistribution.signOutTester() }
231+
signOutButtonBackground.setOnClickListener {
232+
executorService.execute { firebaseAppDistribution.signOutTester() }
217233

218-
setupUI(
219-
isSignedIn = firebaseAppDistribution.isTesterSignedIn, isUpdateAvailable = false)
220-
}
234+
setupUI(
235+
isSignedIn = firebaseAppDistribution.isTesterSignedIn, isUpdateAvailable = false)
236+
}
221237

222238
updateAppButton.setOnClickListener {
223239
setProgressBar()
@@ -312,7 +328,9 @@ class MainActivity : AppCompatActivity() {
312328
enum class FeedbackTrigger(val label: String) {
313329
NONE("None"),
314330
SHAKE("Shake the device"),
315-
SCREENSHOT("Take a screenshot")
331+
SCREENSHOT("Take a screenshot"),
332+
SDK_NOTIFICATION("Tap a notification (SDK)"),
333+
CUSTOM_NOTIFICATION("Tap a notification (custom)")
316334
}
317335
}
318336
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class SecondActivity : AppCompatActivity() {
3434
override fun onOptionsItemSelected(item: MenuItem): Boolean {
3535
return when (item.itemId) {
3636
R.id.startFeedbackMenuItem -> {
37-
Firebase.appDistribution.startFeedback(R.string.terms_and_conditions)
37+
Firebase.appDistribution.startFeedback(R.string.termsAndConditions)
3838
true
3939
}
4040
else -> super.onOptionsItemSelected(item)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ object ShakeForFeedback : ShakeDetector.Listener, Application.ActivityLifecycleC
5050

5151
override fun hearShake() {
5252
Log.i(TAG, "Shake detected")
53-
Firebase.appDistribution.startFeedback(R.string.terms_and_conditions)
53+
Firebase.appDistribution.startFeedback(R.string.termsAndConditions)
5454
}
5555

5656
override fun onActivityResumed(activity: Activity) {

‎firebase-appdistribution/test-app/src/main/res/values/strings.xml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
<resources>
2-
<string name="app_name">App Distro Sample</string>
3-
<string name="terms_and_conditions"><b>Before giving feedback</b> you might want to check out the <a href="https://policies.google.com/terms">Terms and Conditions</a></string>
2+
<string name="appName">App Distro Sample</string>
3+
<string name="termsAndConditions"><b>Before giving feedback</b> you might want to check out the <a href="https://policies.google.com/terms">Terms and Conditions</a></string>
44
<string name="feedbackTriggerMenuLabel">Choose a feedback trigger</string>
55
<string name="startFeedbackLabel">Start feedback</string>
6-
<string name="feedback_trigger_notification_title">We want your Feedback!</string>
7-
<string name="feedback_trigger_notification_text">Click to leave feedback for the App Distro Sample App</string>
8-
<string name="feedback_trigger_notification_channel_name">Feedback Notification</string>
9-
<string name="feedback_trigger_notification_channel_description">Used to trigger Feedback</string>
6+
<string name="feedbackTriggerNotificationTitle">We want your Feedback!</string>
7+
<string name="feedbackTriggerNotificationText">This is a custom notification shown by the App Distro Sample App</string>
8+
<string name="feedbackTriggerNotificationChannelName">Feedback notification</string>
9+
<string name="feedbackTriggerNotificationChannelDescription">Used to trigger feedback</string>
1010
</resources>

‎firebase-appdistribution/test-app/test-app.gradle

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ android {
2424
defaultConfig {
2525
applicationId "com.googletest.firebase.appdistribution.testapp"
2626
minSdkVersion 23
27-
targetSdkVersion 33
27+
targetSdkVersion 32
2828
versionName "2.1"
2929
versionCode 3
3030

0 commit comments

Comments
 (0)