Skip to content

Commit 8d70c68

Browse files
kaibolaylfkellogg
authored andcommitted
Handle the case where we can't identify the release. (#4331)
Use `adb shell setprop debug.firebase.appdistro.devmode true` for development mode. This mode doesn't verify that a release is present and doesn't actually submit feedback - it just demonstrates the UI,
1 parent 8d9b701 commit 8d70c68

File tree

5 files changed

+83
-19
lines changed

5 files changed

+83
-19
lines changed

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class FeedbackActivity extends AppCompatActivity {
4646
"com.google.firebase.appdistribution.FeedbackActivity.SCREENSHOT_URI";
4747

4848
private FeedbackSender feedbackSender;
49-
private String releaseName;
49+
@Nullable private String releaseName; // in development-mode the releaseName might be null
5050
private CharSequence infoText;
5151
@Nullable private Uri screenshotUri;
5252

@@ -112,6 +112,12 @@ private Bitmap readScreenshot() {
112112
}
113113

114114
public void submitFeedback(View view) {
115+
if (releaseName == null) {
116+
// Don't actually send feedback in development-mode
117+
Toast.makeText(this, R.string.feedback_no_release, Toast.LENGTH_LONG).show();
118+
finish();
119+
return;
120+
}
115121
setSubmittingStateEnabled(true);
116122
EditText feedbackText = findViewById(R.id.feedbackText);
117123
CheckBox screenshotCheckBox = findViewById(R.id.screenshotCheckBox);

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

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import android.content.Context;
2727
import android.content.Intent;
2828
import android.net.Uri;
29+
import android.widget.Toast;
2930
import androidx.annotation.GuardedBy;
3031
import androidx.annotation.NonNull;
3132
import androidx.annotation.Nullable;
@@ -313,8 +314,6 @@ private UpdateTask updateApp(boolean showDownloadInNotificationManager) {
313314

314315
@Override
315316
public void startFeedback(@StringRes int infoTextResourceId) {
316-
// TODO(lkellogg): Once we have the real FeedbackActivity view implemented, we should write a
317-
// test that checks that <a> tags are preserved
318317
startFeedback(firebaseApp.getApplicationContext().getText(infoTextResourceId));
319318
}
320319

@@ -375,26 +374,51 @@ private void doStartFeedback(CharSequence infoText, @Nullable Uri screenshotUri)
375374
.signInTester()
376375
.addOnFailureListener(
377376
taskExecutor,
378-
e ->
379-
LogWrapper.getInstance()
380-
.e("Failed to sign in tester. Could not collect feedback.", e))
381-
.onSuccessTask(taskExecutor, unused -> releaseIdentifier.identifyRelease())
382-
.onSuccessTask(
383-
taskExecutor,
384-
releaseName -> launchFeedbackActivity(releaseName, infoText, screenshotUri))
385-
.addOnFailureListener(
386377
e -> {
387-
LogWrapper.getInstance().e("Failed to launch feedback flow", e);
388378
feedbackInProgress.set(false);
389-
});
379+
LogWrapper.getInstance()
380+
.e("Failed to sign in tester. Could not collect feedback.", e);
381+
})
382+
.onSuccessTask(
383+
taskExecutor,
384+
unused ->
385+
releaseIdentifier
386+
.identifyRelease()
387+
.addOnFailureListener(
388+
e -> {
389+
feedbackInProgress.set(false);
390+
LogWrapper.getInstance().e("Failed to identify release", e);
391+
Toast.makeText(
392+
firebaseApp.getApplicationContext(),
393+
R.string.feedback_unidentified_release,
394+
Toast.LENGTH_LONG)
395+
.show();
396+
})
397+
.onSuccessTask(
398+
taskExecutor,
399+
releaseName ->
400+
// in development-mode the releaseName might be null
401+
launchFeedbackActivity(releaseName, infoText, screenshotUri)
402+
.addOnFailureListener(
403+
e -> {
404+
feedbackInProgress.set(false);
405+
LogWrapper.getInstance()
406+
.e("Failed to launch feedback flow", e);
407+
Toast.makeText(
408+
firebaseApp.getApplicationContext(),
409+
R.string.feedback_launch_failed,
410+
Toast.LENGTH_LONG)
411+
.show();
412+
})));
390413
}
391414

392415
private Task<Void> launchFeedbackActivity(
393-
String releaseName, CharSequence infoText, @Nullable Uri screenshotUri) {
416+
@Nullable String releaseName, CharSequence infoText, @Nullable Uri screenshotUri) {
394417
return lifecycleNotifier.consumeForegroundActivity(
395418
activity -> {
396419
LogWrapper.getInstance().i("Launching feedback activity");
397420
Intent intent = new Intent(activity, FeedbackActivity.class);
421+
// in development-mode the releaseName might be null
398422
intent.putExtra(FeedbackActivity.RELEASE_NAME_EXTRA_KEY, releaseName);
399423
intent.putExtra(FeedbackActivity.INFO_TEXT_EXTRA_KEY, infoText);
400424
if (screenshotUri != null) {

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

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import com.google.firebase.appdistribution.FirebaseAppDistributionException.Status;
2828
import java.io.File;
2929
import java.io.IOException;
30+
import java.lang.reflect.Method;
3031
import java.nio.ByteBuffer;
3132
import java.security.MessageDigest;
3233
import java.security.NoSuchAlgorithmException;
@@ -46,17 +47,26 @@ class ReleaseIdentifier {
4647
static final String IAS_ARTIFACT_ID_METADATA_KEY = "com.android.vending.internal.apk.id";
4748

4849
private final ConcurrentMap<String, String> cachedApkHashes = new ConcurrentHashMap<>();
49-
private FirebaseApp firebaseApp;
50-
FirebaseAppDistributionTesterApiClient testerApiClient;
50+
private final FirebaseApp firebaseApp;
51+
private final FirebaseAppDistributionTesterApiClient testerApiClient;
5152

5253
ReleaseIdentifier(
5354
FirebaseApp firebaseApp, FirebaseAppDistributionTesterApiClient testerApiClient) {
5455
this.firebaseApp = firebaseApp;
5556
this.testerApiClient = testerApiClient;
5657
}
5758

58-
/** Identify the currently installed release, returning the release name. */
59+
/**
60+
* Identify the currently installed release, returning the release name.
61+
*
62+
* <p>Will return {@code Task} with a {@code null} result in "developer mode" which allows the UI
63+
* to be used, but no actual feedback to be submitted.
64+
*/
5965
Task<String> identifyRelease() {
66+
if (developmentModeEnabled()) {
67+
return Tasks.forResult(null);
68+
}
69+
6070
// Attempt to find release using IAS artifact ID, which identifies app bundle releases
6171
String iasArtifactId = null;
6272
try {
@@ -193,4 +203,25 @@ private static byte[] arrayListToByteArray(ArrayList<Byte> list) {
193203
}
194204
return result;
195205
}
206+
207+
private static boolean developmentModeEnabled() {
208+
return Boolean.valueOf(getSystemProperty("debug.firebase.appdistro.devmode"));
209+
}
210+
211+
@Nullable
212+
@SuppressWarnings({"unchecked", "PrivateApi"})
213+
private static String getSystemProperty(String propertyName) {
214+
String className = "android.os.SystemProperties";
215+
try {
216+
Class<?> sysProps = Class.forName(className);
217+
Method method = sysProps.getDeclaredMethod("get", String.class);
218+
Object result = method.invoke(null, propertyName);
219+
if (result != null && String.class.isAssignableFrom(result.getClass())) {
220+
return (String) result;
221+
}
222+
} catch (Exception e) {
223+
// do nothing
224+
}
225+
return null;
226+
}
196227
}

firebase-appdistribution/src/main/res/values/strings.xml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
<string name="notifications_group_name">Firebase App Distribution</string>
3232
<string name="app_update_notification_channel_name">App updates</string>
3333
<string name="app_update_notification_channel_description">Shows progress of in-app updates</string>
34+
<string name="feedback_launch_failed">Failed to launch feedback</string>
35+
<string name="feedback_unidentified_release">Release not found. This app may not have been installed by App Distribution, or you may no longer have access.</string>
36+
<string name="feedback_no_release">Would have sent feedback, but did not due to development mode.</string>
3437
<string name="feedback_notification_channel_name">Feedback</string>
3538
<string name="feedback_notification_channel_description">Tap to leave feedback</string>
3639
<string name="feedback_notification_title">We want your feedback!</string>

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -756,7 +756,7 @@ public void startFeedback_signInTesterFails_logsAndSetsInProgressToFalse()
756756
TestUtils.awaitAsyncOperations(taskExecutor);
757757

758758
assertThat(firebaseAppDistribution.isFeedbackInProgress()).isFalse();
759-
assertLoggedError("Failed to launch feedback flow", exception);
759+
assertLoggedError("Failed to sign in tester", exception);
760760
}
761761

762762
@Test
@@ -770,7 +770,7 @@ public void startFeedback_cantIdentifyRelease_logsAndSetsInProgressToFalse()
770770
TestUtils.awaitAsyncOperations(taskExecutor);
771771

772772
assertThat(firebaseAppDistribution.isFeedbackInProgress()).isFalse();
773-
assertLoggedError("Failed to launch feedback flow", exception);
773+
assertLoggedError("Failed to identify release", exception);
774774
}
775775

776776
private static void assertLoggedError(String partialMessage, Throwable e) {

0 commit comments

Comments
 (0)