Add info text argument to startFeedback (#3971)

* Add info text argument to startFeedback

* Formatting

* Update api.txt

* Make infoText CharSequence non null

Author: lfkellogg

firebase-appdistribution-api/api.txt

@@ -19,7 +19,8 @@ package com.google.firebase.appdistribution {
     method public boolean isTesterSignedIn();
     method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> signInTester();
     method public void signOutTester();
-    method public void startFeedback();
+    method public void startFeedback(int);
+    method public void startFeedback(@NonNull CharSequence);
     method @NonNull public com.google.firebase.appdistribution.UpdateTask updateApp();
     method @NonNull public com.google.firebase.appdistribution.UpdateTask updateIfNewReleaseAvailable();
   }

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

@@ -119,8 +119,27 @@ public interface FirebaseAppDistribution {
    *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
    *   <li>Starts a full screen activity for the tester to compose and submit the feedback
    * </ol>
+   *
+   * @param infoTextResourceId string resource ID of text to display to the tester before collecting
+   *     feedback data (e.g. Terms and Conditions)
+   */
+  void startFeedback(int infoTextResourceId);
+
+  /**
+   * Takes a screenshot, and starts an activity to collect and submit feedback from the tester.
+   *
+   * <p>Performs the following actions:
+   *
+   * <ol>
+   *   <li>Takes a screenshot of the current activity
+   *   <li>If tester is not signed in, presents the tester with a Google Sign-in UI
+   *   <li>Starts a full screen activity for the tester to compose and submit the feedback
+   * </ol>
+   *
+   * @param infoText text to display to the tester before collecting feedback data (e.g. Terms and
+   *     Conditions)
    */
-  void startFeedback();
+  void startFeedback(@NonNull CharSequence infoText);
 
   /** Gets the singleton {@link FirebaseAppDistribution} instance. */
   @NonNull

firebase-appdistribution-api/src/main/java/com/google/firebase/appdistribution/internal/FirebaseAppDistributionProxy.java

@@ -73,7 +73,12 @@ public UpdateTask updateApp() {
   }
 
   @Override
-  public void startFeedback() {
-    delegate.startFeedback();
+  public void startFeedback(int infoTextResourceId) {
+    delegate.startFeedback(infoTextResourceId);
+  }
+
+  @Override
+  public void startFeedback(@NonNull CharSequence infoText) {
+    delegate.startFeedback(infoText);
   }
 }

firebase-appdistribution-api/src/main/java/com/google/firebase/appdistribution/internal/FirebaseAppDistributionStub.java

@@ -74,7 +74,12 @@ public UpdateTask updateApp() {
   }
 
   @Override
-  public void startFeedback() {
+  public void startFeedback(int infoTextResourceId) {
+    return;
+  }
+
+  @Override
+  public void startFeedback(@NonNull CharSequence infoText) {
     return;
   }
 

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

@@ -16,9 +16,11 @@
 
 import android.graphics.Bitmap;
 import android.os.Bundle;
+import android.text.method.LinkMovementMethod;
 import android.view.View;
 import android.widget.EditText;
 import android.widget.ImageView;
+import android.widget.TextView;
 import android.widget.Toast;
 import androidx.annotation.Nullable;
 import androidx.appcompat.app.AppCompatActivity;
@@ -33,17 +35,21 @@ public class FeedbackActivity extends AppCompatActivity {
 
   public static final String RELEASE_NAME_EXTRA_KEY =
       "com.google.firebase.appdistribution.FeedbackActivity.RELEASE_NAME";
+  public static final String INFO_TEXT_EXTRA_KEY =
+      "com.google.firebase.appdistribution.FeedbackActivity.INFO_TEXT";
   public static final String SCREENSHOT_FILENAME_EXTRA_KEY =
       "com.google.firebase.appdistribution.FeedbackActivity.SCREENSHOT_FILE_NAME";
 
   private FeedbackSender feedbackSender;
   private String releaseName;
+  private CharSequence infoText;
   @Nullable private File screenshotFile;
 
   @Override
   protected void onCreate(Bundle savedInstanceState) {
     super.onCreate(savedInstanceState);
     releaseName = getIntent().getStringExtra(RELEASE_NAME_EXTRA_KEY);
+    infoText = getIntent().getCharSequenceExtra(INFO_TEXT_EXTRA_KEY);
     if (getIntent().hasExtra(SCREENSHOT_FILENAME_EXTRA_KEY)) {
       screenshotFile = getFileStreamPath(getIntent().getStringExtra(SCREENSHOT_FILENAME_EXTRA_KEY));
     }
@@ -53,9 +59,14 @@ protected void onCreate(Bundle savedInstanceState) {
 
   private void setupView() {
     setContentView(R.layout.activity_feedback);
+
+    TextView infoTextView = this.findViewById(R.id.infoText);
+    infoTextView.setText(infoText);
+    infoTextView.setMovementMethod(LinkMovementMethod.getInstance());
+
     Bitmap thumbnail = readThumbnail();
     if (thumbnail != null) {
-      ImageView screenshotImageView = (ImageView) this.findViewById(R.id.thumbnail);
+      ImageView screenshotImageView = this.findViewById(R.id.thumbnail);
       screenshotImageView.setImageBitmap(thumbnail);
     } else {
       View screenshotErrorLabel = this.findViewById(R.id.screenshotErrorLabel);
@@ -73,7 +84,7 @@ private Bitmap readThumbnail() {
 
   public void submitFeedback(View view) {
     setSubmittingStateEnabled(true);
-    EditText feedbackText = (EditText) findViewById(R.id.feedbackText);
+    EditText feedbackText = findViewById(R.id.feedbackText);
     feedbackSender
         .sendFeedback(releaseName, feedbackText.getText().toString(), screenshotFile)
         .addOnSuccessListener(

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

@@ -18,6 +18,7 @@
 import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.AUTHENTICATION_FAILURE;
 import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.HOST_ACTIVITY_INTERRUPTED;
 import static com.google.firebase.appdistribution.FirebaseAppDistributionException.Status.UPDATE_NOT_AVAILABLE;
+import static com.google.firebase.appdistribution.impl.FeedbackActivity.INFO_TEXT_EXTRA_KEY;
 import static com.google.firebase.appdistribution.impl.FeedbackActivity.RELEASE_NAME_EXTRA_KEY;
 import static com.google.firebase.appdistribution.impl.FeedbackActivity.SCREENSHOT_FILENAME_EXTRA_KEY;
 import static com.google.firebase.appdistribution.impl.TaskUtils.safeSetTaskException;
@@ -44,7 +45,6 @@
 import com.google.firebase.appdistribution.UpdateStatus;
 import com.google.firebase.appdistribution.UpdateTask;
 import java.util.concurrent.Executor;
-import java.util.concurrent.Executors;
 
 /**
  * This class is the "real" implementation of the Firebase App Distribution API which should only be
@@ -63,6 +63,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution {
   private final SignInStorage signInStorage;
   private final ReleaseIdentifier releaseIdentifier;
   private final ScreenshotTaker screenshotTaker;
+  private final Executor taskExecutor;
 
   private final Object updateIfNewReleaseTaskLock = new Object();
 
@@ -95,7 +96,8 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution {
       @NonNull SignInStorage signInStorage,
       @NonNull FirebaseAppDistributionLifecycleNotifier lifecycleNotifier,
       @NonNull ReleaseIdentifier releaseIdentifier,
-      @NonNull ScreenshotTaker screenshotTaker) {
+      @NonNull ScreenshotTaker screenshotTaker,
+      @NonNull Executor taskExecutor) {
     this.firebaseApp = firebaseApp;
     this.testerSignInManager = testerSignInManager;
     this.newReleaseFetcher = newReleaseFetcher;
@@ -105,6 +107,7 @@ class FirebaseAppDistributionImpl implements FirebaseAppDistribution {
     this.releaseIdentifier = releaseIdentifier;
     this.lifecycleNotifier = lifecycleNotifier;
     this.screenshotTaker = screenshotTaker;
+    this.taskExecutor = taskExecutor;
     lifecycleNotifier.addOnActivityDestroyedListener(this::onActivityDestroyed);
     lifecycleNotifier.addOnActivityPausedListener(this::onActivityPaused);
     lifecycleNotifier.addOnActivityResumedListener(this::onActivityResumed);
@@ -306,12 +309,12 @@ private UpdateTask updateApp(boolean showDownloadInNotificationManager) {
   }
 
   @Override
-  public void startFeedback() {
-    startFeedback(Executors.newSingleThreadExecutor());
+  public void startFeedback(int infoTextResourceId) {
+    startFeedback(firebaseApp.getApplicationContext().getString(infoTextResourceId));
   }
 
   @VisibleForTesting
-  public void startFeedback(Executor taskExecutor) {
+  public void startFeedback(@NonNull CharSequence infoText) {
     screenshotTaker
         .takeScreenshot()
         .onSuccessTask(
@@ -327,16 +330,19 @@ public void startFeedback(Executor taskExecutor) {
                     .onSuccessTask(taskExecutor, unused -> releaseIdentifier.identifyRelease())
                     .onSuccessTask(
                         taskExecutor,
-                        releaseName -> launchFeedbackActivity(releaseName, screenshotFilename)))
+                        releaseName ->
+                            launchFeedbackActivity(releaseName, infoText, screenshotFilename)))
         .addOnFailureListener(
             taskExecutor, e -> LogWrapper.getInstance().e("Failed to launch feedback flow", e));
   }
 
-  private Task<Void> launchFeedbackActivity(String releaseName, String screenshotFilename) {
+  private Task<Void> launchFeedbackActivity(
+      String releaseName, CharSequence infoText, String screenshotFilename) {
     return lifecycleNotifier.consumeForegroundActivity(
         activity -> {
           Intent intent = new Intent(activity, FeedbackActivity.class);
           intent.putExtra(RELEASE_NAME_EXTRA_KEY, releaseName);
+          intent.putExtra(INFO_TEXT_EXTRA_KEY, infoText);
           intent.putExtra(SCREENSHOT_FILENAME_EXTRA_KEY, screenshotFilename);
           activity.startActivity(intent);
         });

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

@@ -29,6 +29,7 @@
 import com.google.firebase.platforminfo.LibraryVersionComponent;
 import java.util.Arrays;
 import java.util.List;
+import java.util.concurrent.Executors;
 
 /**
  * Registers FirebaseAppDistribution.
@@ -92,7 +93,8 @@ private FirebaseAppDistribution buildFirebaseAppDistribution(ComponentContainer
             signInStorage,
             lifecycleNotifier,
             releaseIdentifier,
-            new ScreenshotTaker(firebaseApp, lifecycleNotifier));
+            new ScreenshotTaker(firebaseApp, lifecycleNotifier),
+            Executors.newSingleThreadExecutor());
 
     if (context instanceof Application) {
       Application firebaseApplication = (Application) context;

firebase-appdistribution/src/main/res/layout/activity_feedback.xml

@@ -4,6 +4,7 @@
     xmlns:tools="http://schemas.android.com/tools"
     android:layout_width="match_parent"
     android:layout_height="match_parent"
+    android:padding="24dp"
     tools:context=".FeedbackActivity">
 
   <TextView
@@ -34,28 +35,29 @@
       android:id="@+id/submitButton"
       android:layout_width="wrap_content"
       android:layout_height="wrap_content"
-      android:layout_marginTop="32dp"
-      android:text="Submit"
+      android:layout_marginTop="24dp"
       android:onClick="submitFeedback"
+      android:text="Submit"
       app:layout_constraintEnd_toEndOf="parent"
+      app:layout_constraintHorizontal_bias="0.5"
       app:layout_constraintStart_toStartOf="parent"
-      app:layout_constraintTop_toBottomOf="@+id/feedbackText" />
+      app:layout_constraintTop_toBottomOf="@+id/infoText" />
   <TextView
       android:id="@+id/loadingLabel"
       android:layout_width="wrap_content"
       android:layout_height="wrap_content"
-      android:layout_marginTop="44dp"
+      android:layout_marginTop="24dp"
       android:text="Submitting feedback..."
       android:visibility="invisible"
       app:layout_constraintEnd_toEndOf="parent"
       app:layout_constraintStart_toStartOf="parent"
-      app:layout_constraintTop_toBottomOf="@+id/feedbackText" />
-  <!-- Width and height should match bitmap created in setupView() -->
+      app:layout_constraintTop_toBottomOf="@+id/infoText" />
   <ImageView
       android:id="@+id/thumbnail"
       android:layout_width="wrap_content"
       android:layout_height="wrap_content"
       android:scaleType="centerInside"
+      android:layout_marginTop="24dp"
       app:layout_constraintBottom_toBottomOf="parent"
       app:layout_constraintEnd_toEndOf="parent"
       app:layout_constraintStart_toStartOf="parent"
@@ -72,5 +74,14 @@
       app:layout_constraintStart_toStartOf="parent"
       app:layout_constraintTop_toBottomOf="@+id/submitButton"
       app:layout_constraintVertical_bias="0.403" />
+  <TextView
+      android:id="@+id/infoText"
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:ems="5"
+      android:layout_marginTop="24dp"
+      app:layout_constraintEnd_toEndOf="parent"
+      app:layout_constraintStart_toStartOf="parent"
+      app:layout_constraintTop_toBottomOf="@+id/feedbackText" />
 
 </androidx.constraintlayout.widget.ConstraintLayout>

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

@@ -27,6 +27,7 @@
 import static com.google.firebase.appdistribution.impl.ErrorMessages.NETWORK_ERROR;
 import static com.google.firebase.appdistribution.impl.ErrorMessages.RELEASE_NOT_FOUND_ERROR;
 import static com.google.firebase.appdistribution.impl.ErrorMessages.UPDATE_CANCELED;
+import static com.google.firebase.appdistribution.impl.FeedbackActivity.INFO_TEXT_EXTRA_KEY;
 import static com.google.firebase.appdistribution.impl.FeedbackActivity.RELEASE_NAME_EXTRA_KEY;
 import static com.google.firebase.appdistribution.impl.FeedbackActivity.SCREENSHOT_FILENAME_EXTRA_KEY;
 import static com.google.firebase.appdistribution.impl.TestUtils.assertTaskFailure;
@@ -122,6 +123,7 @@ public class FirebaseAppDistributionServiceImplTest {
   private FirebaseAppDistributionImpl firebaseAppDistribution;
   private TestActivity activity;
   private FirebaseApp firebaseApp;
+  private ExecutorService taskExecutor = Executors.newSingleThreadExecutor();
 
   @Mock private InstallationTokenResult mockInstallationTokenResult;
   @Mock private TesterSignInManager mockTesterSignInManager;
@@ -162,7 +164,8 @@ public void setup() throws FirebaseAppDistributionException {
                 mockSignInStorage,
                 mockLifecycleNotifier,
                 mockReleaseIdentifier,
-                mockScreenshotTaker));
+                mockScreenshotTaker,
+                taskExecutor));
 
     when(mockTesterSignInManager.signInTester()).thenReturn(Tasks.forResult(null));
     when(mockSignInStorage.getSignInStatus()).thenReturn(true);
@@ -632,11 +635,9 @@ public void updateApp_withApkReleaseAvailable_returnsSameApkTask() {
 
   @Test
   public void startFeedback_signsInTesterAndStartsActivity() throws InterruptedException {
-    ExecutorService testExecutor = Executors.newSingleThreadExecutor();
     when(mockReleaseIdentifier.identifyRelease()).thenReturn(Tasks.forResult("release-name"));
-
-    firebaseAppDistribution.startFeedback(testExecutor);
-    TestUtils.awaitAsyncOperations(testExecutor);
+    firebaseAppDistribution.startFeedback("Some terms and conditions");
+    TestUtils.awaitAsyncOperations(taskExecutor);
 
     ArgumentCaptor<Intent> argument = ArgumentCaptor.forClass(Intent.class);
     verify(activity).startActivity(argument.capture());
@@ -645,5 +646,7 @@ public void startFeedback_signsInTesterAndStartsActivity() throws InterruptedExc
         .isEqualTo("release-name");
     assertThat(argument.getValue().getStringExtra(SCREENSHOT_FILENAME_EXTRA_KEY))
         .isEqualTo(TEST_SCREENSHOT_FILE_NAME);
+    assertThat(argument.getValue().getStringExtra(INFO_TEXT_EXTRA_KEY))
+        .isEqualTo("Some terms and conditions");
   }
 }

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

@@ -169,7 +169,7 @@ class MainActivity : AppCompatActivity() {
         }
 
         feedbackButton.setOnClickListener {
-            firebaseAppDistribution.startFeedback()
+            firebaseAppDistribution.startFeedback(R.string.terms_and_conditions)
         }
     }
 

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

@@ -1,3 +1,4 @@
 <resources>
     <string name="app_name">App Distro Sample</string>
+    <string name="terms_and_conditions"><b>Before giving feedback</b> you might want to check out the <a href="http://google.com">Terms and Conditions</a></string>
 </resources>