The report page was reachable from the map and the device list and from nowhere else, so + * the one screen whose failures most need diagnosing was the one screen that could not hand a + * user any evidence. Anyone whose sign-in failed had adb on a laptop or had nothing. + * + *
Asserted on the cause reaching the page rather than merely on the page opening: the + * point is that the report names the failure that was on screen, and an intent that opened a + * blank report would pass a check that only looked at the component. + */ + @Test + public void afailedSignInOffersTheLogWithoutADeveloperMachine() { + this.apple = FakeAppleAuthService.rejectsTheSignIn("Bad password"); + AppDependencies.replaceAuthService(this.apple); + + launch(); + signIn(); + + Eventually.check(() -> onView(withId(R.id.login_error_export_logs)) + .check(matches(isDisplayed()))); + onView(withId(R.id.login_error_export_logs)).perform(click()); + + Eventually.check(() -> intended(allOf( + hasComponent(ErrorReportActivity.class.getName()), + hasExtra(ErrorReportActivity.EXTRA_CAUSE, containsString("Bad password"))))); + } + + /** + * And it is only offered once there is something to report. + * + *
A button sitting under an empty error box invites a log of a sign-in that has not been + * attempted, which is a report with nothing in it. + */ + @Test + public void thelogButtonIsNotOfferedBeforeAnythingHasFailed() { + this.apple = FakeAppleAuthService.rejectsTheSignIn("Bad password"); + AppDependencies.replaceAuthService(this.apple); + + launch(); + + onView(withId(R.id.login_error_export_logs)).check(matches(not(isDisplayed()))); + } + /** A rejected code says so, and gives the boxes back rather than stranding them. */ @Test public void aWrongCodeIsReportedAndTheBoxesComeBack() { diff --git a/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java index 24045d64..59986dab 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/AppleLoginActivity.java @@ -33,6 +33,7 @@ import androidx.core.os.LocaleListCompat; import androidx.databinding.DataBindingUtil; +import dev.wander.android.opentagviewer.ui.error.ErrorReportActivity; import dev.wander.android.opentagviewer.ui.compat.WindowPaddingUtil; import androidx.lifecycle.ViewModelProvider; @@ -264,6 +265,9 @@ public void handleOnBackPressed() { this.loginButton = this.findViewById(R.id.login_button_main); this.twoFactorAuthChoiceBackButton = this.findViewById(R.id.twofactorauthchoice_back_button); + this.findViewById(R.id.login_error_export_logs) + .setOnClickListener(v -> this.openTheReportPageForTheLoginFailure()); + this.findViewById(R.id.login_terms_agree_button) .setOnClickListener(v -> this.onAgreeToTerms()); @@ -581,17 +585,13 @@ public void onClickLoginButton(View view) { // undo loading and allow user to try again, basically. Backwards, because // that is what it is: the step the user just left, handed back to them. - this.hideLoading(); - this.showPage(R.id.login_maininfo_container, Direction.BACK); - emailOrPhoneInput.setEnabled(true); - passwordInput.setEnabled(true); - loginButton.setClickable(true); - - FrameLayout loginErrorMessage = this.findViewById(R.id.login_error_container); - loginErrorMessage.setVisibility(VISIBLE); - - TextView loginErrorText = this.findViewById(R.id.login_error_message_text); - loginErrorText.setText(this.describeLoginFailure(error)); + // + // **Through showLoginFailure rather than inline.** This block did the same five + // things by hand, and the copy drifted the moment that method grew a sixth - + // recording the failure so the Export logs button can name it. The report opened + // saying `cause=unknown`, which reads as the button being broken rather than as + // one of two paths having been missed. + this.showLoginFailure(error); }); } @@ -698,6 +698,15 @@ private void onAgreeToTerms() { }); } + /** + * The failure the error box is currently showing, for the report page to name. + * + *
Kept rather than re-derived: {@link #describeLoginFailure} produces a sentence for a + * person, and the report page wants the exception - class and message - which is a different + * string and the one worth pasting into an issue. + */ + private Throwable lastLoginFailure; + /** Hand the sign-in step back to the user with the failure on it. */ private void showLoginFailure(final Throwable error) { this.hideLoading(); @@ -707,11 +716,26 @@ private void showLoginFailure(final Throwable error) { this.findViewById(R.id.password_input_field).setEnabled(true); this.findViewById(R.id.login_button_main).setClickable(true); + this.lastLoginFailure = error; this.findViewById(R.id.login_error_container).setVisibility(VISIBLE); ((TextView) this.findViewById(R.id.login_error_message_text)) .setText(this.describeLoginFailure(error)); } + /** + * Open the report page on the failure just shown. + * + *
A separate activity on purpose. Closing it returns here with the error box still + * up and the typed Apple ID still in place, so looking at the log costs the user nothing - + * which matters because the alternative was a second machine running adb. + */ + private void openTheReportPageForTheLoginFailure() { + this.startActivity(ErrorReportActivity.intentFor( + this, + ErrorReportActivity.describe(ErrorReportActivity.rootOf(this.lastLoginFailure)), + R.string.error_report_body_login)); + } + /** * What to put on screen when a sign-in fails. * diff --git a/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java index e8626c91..e89a06ef 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/MyDevicesListActivity.java @@ -583,19 +583,10 @@ private void onBundleExportFailed(final Throwable error) { // **The cause, not the wrapper.** Rx wraps what a `map` throws, so `describe(error)` here // would put "RuntimeException" on the page and bury the sentence the reporter needs. this.startActivity(ErrorReportActivity.intentFor( - this, ErrorReportActivity.describe(rootOf(error)), + this, ErrorReportActivity.describe(ErrorReportActivity.rootOf(error)), R.string.error_report_body_export)); } - /** The innermost cause, which is the one that says what actually happened. */ - private static Throwable rootOf(final Throwable error) { - Throwable cause = error; - while (cause.getCause() != null && cause.getCause() != cause) { - cause = cause.getCause(); - } - return cause; - } - private void exportHistoryForSelection() { this.pendingExport = this.deviceListAdaptor.getSelectedBeacons(); @@ -744,7 +735,7 @@ private void historyImportFailed(@NonNull final Throwable error) { || failure.getReason() == HistoryImportException.Reason.UNEXPECTED) { this.startActivity(ErrorReportActivity.intentFor( this, - ErrorReportActivity.describe(rootOf(error)), + ErrorReportActivity.describe(ErrorReportActivity.rootOf(error)), R.string.error_report_body_history_import)); return; } diff --git a/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java b/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java index dc0d4e8c..46aa8bce 100644 --- a/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java +++ b/app/src/main/java/dev/wander/android/opentagviewer/ui/error/ErrorReportActivity.java @@ -93,6 +93,22 @@ public static Intent intentFor( * exactly this string and two of them writing it slightly differently makes two reports of * one bug look like two bugs. */ + /** + * The innermost cause, which is the one that says what actually happened. + * + *
Here beside {@link #describe} because every caller of that needs this first, and for a
+ * reason that is easy to miss: Rx wraps what a {@code map} throws, so describing the error as
+ * it arrives puts "RuntimeException" on the page and buries the sentence the reporter needs.
+ * Two screens writing their own copy of this is two reports of one bug looking like two.
+ */
+ public static Throwable rootOf(final Throwable error) {
+ Throwable cause = error;
+ while (cause != null && cause.getCause() != null && cause.getCause() != cause) {
+ cause = cause.getCause();
+ }
+ return cause;
+ }
+
public static String describe(final Throwable error) {
if (error == null) {
return "unknown";
diff --git a/app/src/main/res/layout/activity_apple_login.xml b/app/src/main/res/layout/activity_apple_login.xml
index 6ea5bd97..9964a512 100644
--- a/app/src/main/res/layout/activity_apple_login.xml
+++ b/app/src/main/res/layout/activity_apple_login.xml
@@ -131,16 +131,42 @@
android:visibility="invisible"
tools:visibility="visible">
-