From dea05c85a2b63e5ca4371ac42afbba8b30bba2ec Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Mon, 18 Apr 2022 10:16:38 -0300
Subject: [PATCH] Improve MailboxStatusFragment and record check failures as
 well

---
 .../bramble/mailbox/MailboxManagerImpl.java   | 32 ++++++++++++-------
 .../mailbox/MailboxStatusFragment.java        |  2 +-
 .../res/layout/fragment_mailbox_status.xml    | 11 ++++---
 briar-android/src/main/res/values/strings.xml |  2 +-
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java
index 39b4d7fa0..ecdf5291f 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java
@@ -103,22 +103,32 @@ class MailboxManagerImpl implements MailboxManager {
 			MailboxProperties props = db.transactionWithNullableResult(true,
 					mailboxSettingsManager::getOwnMailboxProperties);
 			success = api.checkStatus(props);
-		} catch (DbException | IOException | MailboxApi.ApiException e) {
+		} catch (DbException e) {
+			logException(LOG, WARNING, e);
+			// we don't treat this is a failure to record
+			return false;
+		} catch (IOException | MailboxApi.ApiException e) {
+			// we record this as a failure
 			success = false;
 			logException(LOG, WARNING, e);
 		}
-		if (success) {
-			try {
-				// we are only recording successful connections here
-				// as those update the UI and failures might be false negatives
-				db.transaction(false, txn ->
-						mailboxSettingsManager.recordSuccessfulConnection(txn,
-								clock.currentTimeMillis()));
-			} catch (DbException e) {
-				logException(LOG, WARNING, e);
-			}
+		try {
+			recordCheckResult(success);
+		} catch (DbException e) {
+			logException(LOG, WARNING, e);
 		}
 		return success;
 	}
 
+	private void recordCheckResult(boolean success) throws DbException {
+		long now = clock.currentTimeMillis();
+		db.transaction(false, txn -> {
+			if (success) {
+				mailboxSettingsManager.recordSuccessfulConnection(txn, now);
+			} else {
+				mailboxSettingsManager.recordFailedConnectionAttempt(txn, now);
+			}
+		});
+	}
+
 }
diff --git a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxStatusFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxStatusFragment.java
index 45f3456db..921081964 100644
--- a/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxStatusFragment.java
+++ b/briar-android/src/main/java/org/briarproject/briar/android/mailbox/MailboxStatusFragment.java
@@ -100,7 +100,7 @@ public class MailboxStatusFragment extends Fragment {
 				.observe(getViewLifecycleOwner(), this::onMailboxStateChanged);
 
 		// TODO
-		//  * detect problems and show them #2175
+		//  * Implement UI for warning user when mailbox is unreachable #2175
 		//  * add "Unlink" button confirmation dialog and functionality #2173
 		Button unlinkButton = v.findViewById(R.id.unlinkButton);
 		unlinkButton.setOnClickListener(view -> Toast.makeText(requireContext(),
diff --git a/briar-android/src/main/res/layout/fragment_mailbox_status.xml b/briar-android/src/main/res/layout/fragment_mailbox_status.xml
index 58096f487..098c71d7c 100644
--- a/briar-android/src/main/res/layout/fragment_mailbox_status.xml
+++ b/briar-android/src/main/res/layout/fragment_mailbox_status.xml
@@ -19,22 +19,23 @@
 		app:layout_constraintTop_toTopOf="parent"
 		app:layout_constraintVertical_bias="0.25"
 		app:layout_constraintVertical_chainStyle="packed"
-		app:srcCompat="@drawable/ic_check_circle_outline"
-		app:tint="@color/briar_brand_green"
-		tools:ignore="ContentDescription" />
+		tools:ignore="ContentDescription"
+		tools:srcCompat="@drawable/ic_help_outline_white"
+		tools:tint="@color/briar_orange_500" />
 
 	<TextView
 		android:id="@+id/statusTitleView"
 		android:layout_width="wrap_content"
 		android:layout_height="wrap_content"
 		android:layout_margin="16dp"
-		android:text="@string/mailbox_status_connected_title"
+		android:gravity="center"
 		android:textAppearance="@style/TextAppearance.MaterialComponents.Headline6"
 		app:layout_constrainedWidth="true"
 		app:layout_constraintBottom_toTopOf="@+id/checkButton"
 		app:layout_constraintEnd_toEndOf="parent"
 		app:layout_constraintStart_toStartOf="parent"
-		app:layout_constraintTop_toBottomOf="@+id/imageView" />
+		app:layout_constraintTop_toBottomOf="@+id/imageView"
+		tools:text="@string/mailbox_status_problem_title" />
 
 	<Button
 		android:id="@+id/checkButton"
diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml
index f3d2ec0ab..f38a3639f 100644
--- a/briar-android/src/main/res/values/strings.xml
+++ b/briar-android/src/main/res/values/strings.xml
@@ -643,7 +643,7 @@
 	<string name="tor_offline_button_check">Check connection settings</string>
 	<string name="mailbox_status_title">Mailbox status</string>
 	<string name="mailbox_status_connected_title">Mailbox is running</string>
-	<string name="mailbox_status_problem_title">We are having trouble connecting to the mailbox</string>
+	<string name="mailbox_status_problem_title">Briar is having trouble connecting to the Mailbox</string>
 	<string name="mailbox_status_failure_title">Mailbox is unavailable</string>
 	<string name="mailbox_status_check_button">Check Connection</string>
 	<!-- Example for string substitution: Last connection: 3min ago-->