Skip to content

Commit 9fc267e

Browse files
tagboolalfkellogg
authored andcommitted
Pass feedback trigger type to create feedback call (#4631)
* Pipe through feedback trigger to API client calls and pass it as a header in call to create feedback * Make FeedbackTrigger package private and add comment * Fix file formatting * Add check before casting interface to impl * Fix formatting of TakeScreenshotAndStartFeedbackActivity.java * Add copyright information to FeedbackTrigger.java * Update FeedbackTrigger and remove instances of valueOf * Fix formatting of java files * Inject FirebaseAppDistributionImpl directly iun TakeScreenshotAndStartFeedbackActivity * Remove FeedbackTrigger dependency from TesterApiHttpClient * Fix formatting
1 parent 14322c3 commit 9fc267e

File tree

10 files changed

+145
-35
lines changed

10 files changed

+145
-35
lines changed

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FeedbackActivity.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ public class FeedbackActivity extends AppCompatActivity {
5353
"com.google.firebase.appdistribution.FeedbackActivity.INFO_TEXT";
5454
public static final String SCREENSHOT_URI_KEY =
5555
"com.google.firebase.appdistribution.FeedbackActivity.SCREENSHOT_URI";
56+
public static final String FEEDBACK_TRIGGER_KEY =
57+
"com.google.firebase.appdistribution.FeedbackActivity.FEEDBACK_TRIGGER";
5658

5759
private final ActivityResultLauncher<Intent> chooseScreenshotLauncher =
5860
registerForActivityResult(new StartActivityForResult(), this::handleChooseScreenshotResult);
@@ -64,6 +66,7 @@ public class FeedbackActivity extends AppCompatActivity {
6466
@Nullable private String releaseName; // in development-mode the releaseName might be null
6567
private CharSequence infoText;
6668
@Nullable private Uri screenshotUri;
69+
private FeedbackTrigger feedbackTrigger;
6770

6871
@Override
6972
protected void onCreate(Bundle savedInstanceState) {
@@ -75,13 +78,17 @@ protected void onCreate(Bundle savedInstanceState) {
7578
if (savedInstanceState != null) {
7679
releaseName = savedInstanceState.getString(RELEASE_NAME_KEY);
7780
infoText = savedInstanceState.getCharSequence(INFO_TEXT_KEY);
81+
feedbackTrigger =
82+
FeedbackTrigger.fromString(savedInstanceState.getString(FEEDBACK_TRIGGER_KEY));
7883
String screenshotUriString = savedInstanceState.getString(SCREENSHOT_URI_KEY);
7984
if (screenshotUriString != null) {
8085
screenshotUri = Uri.parse(screenshotUriString);
8186
}
8287
} else {
8388
releaseName = getIntent().getStringExtra(RELEASE_NAME_KEY);
8489
infoText = getIntent().getCharSequenceExtra(INFO_TEXT_KEY);
90+
feedbackTrigger =
91+
FeedbackTrigger.fromString(getIntent().getStringExtra(FEEDBACK_TRIGGER_KEY));
8592
if (getIntent().hasExtra(SCREENSHOT_URI_KEY)) {
8693
screenshotUri = Uri.parse(getIntent().getStringExtra(SCREENSHOT_URI_KEY));
8794
}
@@ -197,7 +204,8 @@ public void submitFeedback(View view) {
197204
.sendFeedback(
198205
releaseName,
199206
feedbackText.getText().toString(),
200-
screenshotCheckBox.isChecked() ? screenshotUri : null)
207+
screenshotCheckBox.isChecked() ? screenshotUri : null,
208+
feedbackTrigger)
201209
.addOnSuccessListener(
202210
uiThreadExecutor,
203211
unused -> {

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FeedbackSender.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,13 @@ class FeedbackSender {
3636
}
3737

3838
/** Send feedback text and optionally a screenshot to the Tester API for the given release. */
39-
Task<Void> sendFeedback(String releaseName, String feedbackText, @Nullable Uri screenshotUri) {
39+
Task<Void> sendFeedback(
40+
String releaseName,
41+
String feedbackText,
42+
@Nullable Uri screenshotUri,
43+
FeedbackTrigger trigger) {
4044
return testerApiClient
41-
.createFeedback(releaseName, feedbackText)
45+
.createFeedback(releaseName, feedbackText, trigger)
4246
.onSuccessTask(
4347
lightweightExecutor, feedbackName -> attachScreenshot(feedbackName, screenshotUri))
4448
.onSuccessTask(lightweightExecutor, testerApiClient::commitFeedback);
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright 2021 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package com.google.firebase.appdistribution.impl;
16+
17+
/** Feedback trigger type */
18+
enum FeedbackTrigger {
19+
NOTIFICATION("notification"),
20+
CUSTOM("custom"),
21+
UNKNOWN("unknown");
22+
23+
private final String value;
24+
25+
FeedbackTrigger(String value) {
26+
this.value = value;
27+
}
28+
29+
@Override
30+
public String toString() {
31+
return value;
32+
}
33+
34+
public static FeedbackTrigger fromString(String value) {
35+
if (value.equals(NOTIFICATION.value)) {
36+
return NOTIFICATION;
37+
} else if (value.equals(CUSTOM.value)) {
38+
return CUSTOM;
39+
} else {
40+
return UNKNOWN;
41+
}
42+
}
43+
}

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionImpl.java

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,15 @@ private UpdateTask updateApp(boolean showDownloadInNotificationManager) {
340340

341341
@Override
342342
public void startFeedback(@StringRes int infoTextResourceId) {
343-
startFeedback(applicationContext.getText(infoTextResourceId));
343+
startFeedback(applicationContext.getText(infoTextResourceId), FeedbackTrigger.CUSTOM);
344344
}
345345

346346
@Override
347347
public void startFeedback(@NonNull CharSequence infoText) {
348+
startFeedback(infoText, FeedbackTrigger.CUSTOM);
349+
}
350+
351+
void startFeedback(@NonNull CharSequence infoText, FeedbackTrigger trigger) {
348352
if (!feedbackInProgress.compareAndSet(/* expect= */ false, /* update= */ true)) {
349353
LogWrapper.i(TAG, "Ignoring startFeedback() call because feedback is already in progress");
350354
return;
@@ -356,24 +360,30 @@ public void startFeedback(@NonNull CharSequence infoText) {
356360
lightweightExecutor,
357361
e -> {
358362
LogWrapper.w(TAG, "Failed to take screenshot for feedback", e);
359-
doStartFeedback(infoText, null);
363+
doStartFeedback(infoText, null, trigger);
360364
})
361365
.addOnSuccessListener(
362-
lightweightExecutor, screenshotUri -> doStartFeedback(infoText, screenshotUri));
366+
lightweightExecutor,
367+
screenshotUri -> doStartFeedback(infoText, screenshotUri, trigger));
363368
}
364369

365370
@Override
366371
public void startFeedback(@StringRes int infoTextResourceId, @Nullable Uri screenshotUri) {
367-
startFeedback(getText(infoTextResourceId), screenshotUri);
372+
startFeedback(getText(infoTextResourceId), screenshotUri, FeedbackTrigger.CUSTOM);
368373
}
369374

370375
@Override
371376
public void startFeedback(@NonNull CharSequence infoText, @Nullable Uri screenshotUri) {
377+
startFeedback(infoText, screenshotUri, FeedbackTrigger.CUSTOM);
378+
}
379+
380+
private void startFeedback(
381+
@NonNull CharSequence infoText, @Nullable Uri screenshotUri, FeedbackTrigger trigger) {
372382
if (!feedbackInProgress.compareAndSet(/* expect= */ false, /* update= */ true)) {
373383
LogWrapper.i(TAG, "Ignoring startFeedback() call because feedback is already in progress");
374384
return;
375385
}
376-
doStartFeedback(infoText, screenshotUri);
386+
doStartFeedback(infoText, screenshotUri, trigger);
377387
}
378388

379389
@Override
@@ -393,7 +403,8 @@ public void cancelFeedbackNotification() {
393403
notificationsManager.cancelFeedbackNotification();
394404
}
395405

396-
private void doStartFeedback(CharSequence infoText, @Nullable Uri screenshotUri) {
406+
private void doStartFeedback(
407+
CharSequence infoText, @Nullable Uri screenshotUri, FeedbackTrigger trigger) {
397408
testerSignInManager
398409
.signInTester()
399410
.addOnFailureListener(
@@ -422,7 +433,7 @@ private void doStartFeedback(CharSequence infoText, @Nullable Uri screenshotUri)
422433
lightweightExecutor,
423434
releaseName ->
424435
// in development-mode the releaseName might be null
425-
launchFeedbackActivity(releaseName, infoText, screenshotUri)
436+
launchFeedbackActivity(releaseName, infoText, screenshotUri, trigger)
426437
.addOnFailureListener(
427438
uiThreadExecutor,
428439
e -> {
@@ -437,14 +448,18 @@ private void doStartFeedback(CharSequence infoText, @Nullable Uri screenshotUri)
437448
}
438449

439450
private Task<Void> launchFeedbackActivity(
440-
@Nullable String releaseName, CharSequence infoText, @Nullable Uri screenshotUri) {
451+
@Nullable String releaseName,
452+
CharSequence infoText,
453+
@Nullable Uri screenshotUri,
454+
FeedbackTrigger trigger) {
441455
return lifecycleNotifier.consumeForegroundActivity(
442456
activity -> {
443457
LogWrapper.i(TAG, "Launching feedback activity");
444458
Intent intent = new Intent(activity, FeedbackActivity.class);
445459
// in development-mode the releaseName might be null
446460
intent.putExtra(FeedbackActivity.RELEASE_NAME_KEY, releaseName);
447461
intent.putExtra(FeedbackActivity.INFO_TEXT_KEY, infoText);
462+
intent.putExtra(FeedbackActivity.FEEDBACK_TRIGGER_KEY, trigger.toString());
448463
if (screenshotUri != null) {
449464
intent.putExtra(FeedbackActivity.SCREENSHOT_URI_KEY, screenshotUri.toString());
450465
}

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/FirebaseAppDistributionTesterApiClient.java

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@
2828
import com.google.firebase.inject.Provider;
2929
import com.google.firebase.installations.FirebaseInstallationsApi;
3030
import com.google.firebase.installations.InstallationTokenResult;
31+
import java.util.HashMap;
32+
import java.util.Map;
3133
import java.util.concurrent.Executor;
3234
import javax.inject.Inject;
3335
import org.json.JSONArray;
@@ -60,6 +62,7 @@ private interface FidDependentJob<TResult> {
6062
private static final String CREATE_FEEDBACK_TAG = "Creating feedback";
6163
private static final String COMMIT_FEEDBACK_TAG = "Committing feedback";
6264
private static final String UPLOAD_SCREENSHOT_TAG = "Uploading screenshot";
65+
private static final String X_APP_DISTRO_FEEDBACK_TRIGGER = "X-APP-DISTRO-FEEDBACK-TRIGGER";
6366

6467
private final FirebaseOptions firebaseOptions;
6568
private final Provider<FirebaseInstallationsApi> firebaseInstallationsApiProvider;
@@ -136,14 +139,18 @@ private String findRelease(String fid, String token, String binaryIdParam, Strin
136139

137140
/** Creates a new feedback from the given text, and returns the feedback name. */
138141
@NonNull
139-
Task<String> createFeedback(String testerReleaseName, String feedbackText) {
142+
Task<String> createFeedback(
143+
String testerReleaseName, String feedbackText, FeedbackTrigger trigger) {
140144
return runWithFidAndToken(
141145
(unused, token) -> {
142146
LogWrapper.i(TAG, "Creating feedback for release: " + testerReleaseName);
143147
String path = String.format("v1alpha/%s/feedbackReports", testerReleaseName);
144148
String requestBody = buildCreateFeedbackBody(feedbackText).toString();
149+
Map<String, String> extraHeaders = new HashMap<>();
150+
extraHeaders.put(X_APP_DISTRO_FEEDBACK_TRIGGER, trigger.toString());
145151
JSONObject responseBody =
146-
testerApiHttpClient.makePostRequest(CREATE_FEEDBACK_TAG, path, token, requestBody);
152+
testerApiHttpClient.makePostRequest(
153+
CREATE_FEEDBACK_TAG, path, token, requestBody, extraHeaders);
147154
return parseJsonFieldFromResponse(responseBody, "name");
148155
});
149156
}

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TakeScreenshotAndStartFeedbackActivity.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
import android.app.Activity;
1818
import android.os.Bundle;
19-
import com.google.firebase.appdistribution.FirebaseAppDistribution;
2019
import javax.inject.Inject;
2120

2221
public class TakeScreenshotAndStartFeedbackActivity extends Activity {
@@ -26,7 +25,7 @@ public class TakeScreenshotAndStartFeedbackActivity extends Activity {
2625
public static final String INFO_TEXT_EXTRA_KEY =
2726
"com.google.firebase.appdistribution.TakeScreenshotAndStartFeedbackActivity.INFO_TEXT";
2827

29-
@Inject FirebaseAppDistribution firebaseAppDistribution;
28+
@Inject FirebaseAppDistributionImpl firebaseAppDistribution;
3029

3130
private CharSequence infoText;
3231

@@ -38,7 +37,8 @@ public void onCreate(Bundle savedInstanceState) {
3837
super.onCreate(savedInstanceState);
3938
infoText = getIntent().getCharSequenceExtra(INFO_TEXT_EXTRA_KEY);
4039
LogWrapper.i(TAG, "Capturing screenshot and starting feedback");
41-
firebaseAppDistribution.startFeedback(infoText);
40+
firebaseAppDistribution.startFeedback(infoText, FeedbackTrigger.NOTIFICATION);
41+
4242
finish();
4343
}
4444
}

firebase-appdistribution/src/main/java/com/google/firebase/appdistribution/impl/TesterApiHttpClient.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,12 @@ JSONObject makeGetRequest(String tag, String path, String token)
104104
*/
105105
JSONObject makePostRequest(String tag, String path, String token, String requestBody)
106106
throws FirebaseAppDistributionException {
107+
return makePostRequest(tag, path, token, requestBody, new HashMap<>());
108+
}
109+
110+
JSONObject makePostRequest(
111+
String tag, String path, String token, String requestBody, Map<String, String> extraHeaders)
112+
throws FirebaseAppDistributionException {
107113
byte[] bytes;
108114
try {
109115
bytes = requestBody.getBytes(UTF_8);
@@ -112,7 +118,7 @@ JSONObject makePostRequest(String tag, String path, String token, String request
112118
"Unsupported encoding: " + UTF_8, Status.UNKNOWN, e);
113119
}
114120
return makePostRequest(
115-
tag, path, token, new HashMap<>(), outputStream -> outputStream.write(bytes));
121+
tag, path, token, extraHeaders, outputStream -> outputStream.write(bytes));
116122
}
117123

118124
/**

firebase-appdistribution/src/test/java/com/google/firebase/appdistribution/impl/FeedbackSenderTest.java

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,32 +59,38 @@ public void setup() {
5959

6060
@Test
6161
public void sendFeedback_success() throws Exception {
62-
when(mockTesterApiClient.createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT))
62+
when(mockTesterApiClient.createFeedback(
63+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, FeedbackTrigger.CUSTOM))
6364
.thenReturn(Tasks.forResult(TEST_FEEDBACK_NAME));
6465
when(mockTesterApiClient.attachScreenshot(TEST_FEEDBACK_NAME, TEST_SCREENSHOT_URI))
6566
.thenReturn(Tasks.forResult(TEST_FEEDBACK_NAME));
6667
when(mockTesterApiClient.commitFeedback(TEST_FEEDBACK_NAME)).thenReturn(Tasks.forResult(null));
6768

6869
Task<Void> task =
69-
feedbackSender.sendFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI);
70+
feedbackSender.sendFeedback(
71+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI, FeedbackTrigger.CUSTOM);
7072
TestUtils.awaitTask(task);
7173

72-
verify(mockTesterApiClient).createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT);
74+
verify(mockTesterApiClient)
75+
.createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, FeedbackTrigger.CUSTOM);
7376
verify(mockTesterApiClient).attachScreenshot(TEST_FEEDBACK_NAME, TEST_SCREENSHOT_URI);
7477
verify(mockTesterApiClient).commitFeedback(TEST_FEEDBACK_NAME);
7578
}
7679

7780
@Test
7881
public void sendFeedback_withoutScreenshot_success() throws Exception {
79-
when(mockTesterApiClient.createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT))
82+
when(mockTesterApiClient.createFeedback(
83+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, FeedbackTrigger.CUSTOM))
8084
.thenReturn(Tasks.forResult(TEST_FEEDBACK_NAME));
8185
when(mockTesterApiClient.commitFeedback(TEST_FEEDBACK_NAME)).thenReturn(Tasks.forResult(null));
8286

8387
Task<Void> task =
84-
feedbackSender.sendFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, /* screenshot= */ null);
88+
feedbackSender.sendFeedback(
89+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, /* screenshot= */ null, FeedbackTrigger.CUSTOM);
8590
TestUtils.awaitTask(task);
8691

87-
verify(mockTesterApiClient).createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT);
92+
verify(mockTesterApiClient)
93+
.createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, FeedbackTrigger.CUSTOM);
8894
verify(mockTesterApiClient).commitFeedback(TEST_FEEDBACK_NAME);
8995
verify(mockTesterApiClient, never()).attachScreenshot(any(), any());
9096
}
@@ -93,33 +99,38 @@ public void sendFeedback_withoutScreenshot_success() throws Exception {
9399
public void sendFeedback_createFeedbackFails_failsTask() {
94100
FirebaseAppDistributionException cause =
95101
new FirebaseAppDistributionException("test ex", Status.AUTHENTICATION_FAILURE);
96-
when(mockTesterApiClient.createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT))
102+
when(mockTesterApiClient.createFeedback(
103+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, FeedbackTrigger.CUSTOM))
97104
.thenReturn(Tasks.forException(cause));
98105

99106
Task<Void> task =
100-
feedbackSender.sendFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI);
107+
feedbackSender.sendFeedback(
108+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI, FeedbackTrigger.CUSTOM);
101109

102110
TestUtils.awaitTaskFailure(task, Status.AUTHENTICATION_FAILURE, "test ex");
103111
}
104112

105113
@Test
106114
public void sendFeedback_attachScreenshotFails_failsTask() {
107-
when(mockTesterApiClient.createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT))
115+
when(mockTesterApiClient.createFeedback(
116+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, FeedbackTrigger.CUSTOM))
108117
.thenReturn(Tasks.forResult(TEST_FEEDBACK_NAME));
109118
FirebaseAppDistributionException cause =
110119
new FirebaseAppDistributionException("test ex", Status.AUTHENTICATION_FAILURE);
111120
when(mockTesterApiClient.attachScreenshot(TEST_FEEDBACK_NAME, TEST_SCREENSHOT_URI))
112121
.thenReturn(Tasks.forException(cause));
113122

114123
Task<Void> task =
115-
feedbackSender.sendFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI);
124+
feedbackSender.sendFeedback(
125+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI, FeedbackTrigger.CUSTOM);
116126

117127
TestUtils.awaitTaskFailure(task, Status.AUTHENTICATION_FAILURE, "test ex");
118128
}
119129

120130
@Test
121131
public void sendFeedback_commitFeedbackFails_failsTask() {
122-
when(mockTesterApiClient.createFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT))
132+
when(mockTesterApiClient.createFeedback(
133+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, FeedbackTrigger.CUSTOM))
123134
.thenReturn(Tasks.forResult(TEST_FEEDBACK_NAME));
124135
when(mockTesterApiClient.attachScreenshot(TEST_FEEDBACK_NAME, TEST_SCREENSHOT_URI))
125136
.thenReturn(Tasks.forResult(TEST_FEEDBACK_NAME));
@@ -129,7 +140,8 @@ public void sendFeedback_commitFeedbackFails_failsTask() {
129140
.thenReturn(Tasks.forException(cause));
130141

131142
Task<Void> task =
132-
feedbackSender.sendFeedback(TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI);
143+
feedbackSender.sendFeedback(
144+
TEST_RELEASE_NAME, TEST_FEEDBACK_TEXT, TEST_SCREENSHOT_URI, FeedbackTrigger.CUSTOM);
133145

134146
TestUtils.awaitTaskFailure(task, Status.AUTHENTICATION_FAILURE, "test ex");
135147
}

0 commit comments

Comments
 (0)