From 06b9f64063bbae9e35b644cf80a88a9a4b25dd9a Mon Sep 17 00:00:00 2001 From: Arthur Milchior Date: Sun, 3 Nov 2024 17:44:22 +0100 Subject: [PATCH] NF: improve SyncErrorDialog Use an enum and constants --- .../main/java/com/ichi2/anki/DeckPicker.kt | 6 +- .../src/main/java/com/ichi2/anki/Sync.kt | 2 +- .../com/ichi2/anki/dialogs/SyncErrorDialog.kt | 150 +++++++++--------- .../anki/dialogs/AsyncDialogFragmentsTest.kt | 2 +- 4 files changed, 84 insertions(+), 76 deletions(-) diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 7d96736f0106..86ebbcd4d488 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -1733,7 +1733,7 @@ open class DeckPicker : * Show a specific sync error dialog * @param dialogType id of dialog to show */ - override fun showSyncErrorDialog(dialogType: Int) { + override fun showSyncErrorDialog(dialogType: SyncErrorDialog.Type) { showSyncErrorDialog(dialogType, "") } @@ -1742,7 +1742,7 @@ open class DeckPicker : * @param dialogType id of dialog to show * @param message text to show */ - override fun showSyncErrorDialog(dialogType: Int, message: String?) { + override fun showSyncErrorDialog(dialogType: SyncErrorDialog.Type, message: String?) { val newFragment: AsyncDialogFragment = newInstance(dialogType, message) showAsyncDialogFragment(newFragment, Channel.SYNC) } @@ -1863,7 +1863,7 @@ open class DeckPicker : if (hkey!!.isEmpty()) { Timber.w("User not logged in") pullToSyncWrapper.isRefreshing = false - showSyncErrorDialog(SyncErrorDialog.DIALOG_USER_NOT_LOGGED_IN_SYNC) + showSyncErrorDialog(SyncErrorDialog.Type.DIALOG_USER_NOT_LOGGED_IN_SYNC) return } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt index 1df0ea8327d0..994d7cdfd3e8 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt @@ -261,7 +261,7 @@ private suspend fun handleNormalSync( SyncCollectionResponse.ChangesRequired.FULL_SYNC -> { deckPicker.mediaUsnOnConflict = mediaUsn - deckPicker.showSyncErrorDialog(SyncErrorDialog.DIALOG_SYNC_CONFLICT_RESOLUTION) + deckPicker.showSyncErrorDialog(SyncErrorDialog.Type.DIALOG_SYNC_CONFLICT_RESOLUTION) } SyncCollectionResponse.ChangesRequired.NORMAL_SYNC, diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/SyncErrorDialog.kt b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/SyncErrorDialog.kt index 769970226362..0c7de0e8c475 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/SyncErrorDialog.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/dialogs/SyncErrorDialog.kt @@ -21,7 +21,6 @@ import android.net.Uri import android.os.Bundle import android.os.Message import androidx.annotation.CheckResult -import androidx.annotation.VisibleForTesting import androidx.appcompat.app.AlertDialog import androidx.core.os.bundleOf import com.ichi2.anki.AnkiActivity @@ -34,21 +33,24 @@ import com.ichi2.anki.utils.ext.dismissAllDialogFragments class SyncErrorDialog : AsyncDialogFragment() { interface SyncErrorDialogListener { - fun showSyncErrorDialog(dialogType: Int) - fun showSyncErrorDialog(dialogType: Int, message: String?) + fun showSyncErrorDialog(dialogType: Type) + fun showSyncErrorDialog(dialogType: Type, message: String?) fun loginToSyncServer() fun sync(conflict: ConflictResolution? = null) fun mediaCheck() fun integrityCheck() } + /** The type of the sync error dialog*/ + private fun type() = Type.fromCode(requireArguments().getInt(SYNC_ERROR_DIALOG_TYPE_KEY)) + override fun onCreateDialog(savedInstanceState: Bundle?): Dialog { super.onCreate(savedInstanceState) val dialog = AlertDialog.Builder(requireContext()) .setTitle(title) .setMessage(message) - return when (requireArguments().getInt("dialogType")) { - DIALOG_USER_NOT_LOGGED_IN_SYNC -> { + return when (type()) { + Type.DIALOG_USER_NOT_LOGGED_IN_SYNC -> { // User not logged in; take them to login screen dialog.setIcon(R.drawable.ic_sync_problem) .setPositiveButton(R.string.log_in) { _, _ -> @@ -57,7 +59,7 @@ class SyncErrorDialog : AsyncDialogFragment() { .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - DIALOG_CONNECTION_ERROR -> { + Type.DIALOG_CONNECTION_ERROR -> { // Connection error; allow user to retry or cancel dialog.setIcon(R.drawable.ic_sync_problem) .setPositiveButton(R.string.retry) { _, _ -> @@ -69,23 +71,23 @@ class SyncErrorDialog : AsyncDialogFragment() { } .create() } - DIALOG_SYNC_CONFLICT_RESOLUTION -> { + Type.DIALOG_SYNC_CONFLICT_RESOLUTION -> { // Sync conflict; allow user to cancel, or choose between local and remote versions dialog.setIcon(R.drawable.ic_sync_problem) .setPositiveButton(R.string.sync_conflict_keep_local_new) { _, _ -> (activity as SyncErrorDialogListener?) - ?.showSyncErrorDialog(DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL) + ?.showSyncErrorDialog(Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL) } .setNegativeButton(R.string.sync_conflict_keep_remote_new) { _, _ -> (activity as SyncErrorDialogListener?) - ?.showSyncErrorDialog(DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE) + ?.showSyncErrorDialog(Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE) } .setNeutralButton(R.string.dialog_cancel) { _, _ -> activity?.dismissAllDialogFragments() } .create() } - DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL -> { + Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL -> { // Confirmation before pushing local collection to server after sync conflict dialog.setIcon(R.drawable.ic_sync_problem) .setPositiveButton(R.string.dialog_positive_replace) { _, _ -> @@ -95,7 +97,7 @@ class SyncErrorDialog : AsyncDialogFragment() { .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE -> { + Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE -> { // Confirmation before overwriting local collection with server collection after sync conflict dialog.setIcon(R.drawable.ic_sync_problem) .setPositiveButton(R.string.dialog_positive_replace) { _, _ -> @@ -105,20 +107,20 @@ class SyncErrorDialog : AsyncDialogFragment() { .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - DIALOG_SYNC_SANITY_ERROR -> { + Type.DIALOG_SYNC_SANITY_ERROR -> { // Sync sanity check error; allow user to cancel, or choose between local and remote versions dialog.setPositiveButton(R.string.sync_sanity_local) { _, _ -> (activity as SyncErrorDialogListener?) - ?.showSyncErrorDialog(DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL) + ?.showSyncErrorDialog(Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL) } .setNeutralButton(R.string.sync_sanity_remote) { _, _ -> (activity as SyncErrorDialogListener?) - ?.showSyncErrorDialog(DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE) + ?.showSyncErrorDialog(Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE) } .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL -> { + Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL -> { // Confirmation before pushing local collection to server after sanity check error dialog.setPositiveButton(R.string.dialog_positive_replace) { _, _ -> (activity as SyncErrorDialogListener).sync(ConflictResolution.FULL_UPLOAD) @@ -127,7 +129,7 @@ class SyncErrorDialog : AsyncDialogFragment() { .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE -> { + Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE -> { // Confirmation before overwriting local collection with server collection after sanity check error dialog.setPositiveButton(R.string.dialog_positive_replace) { _, _ -> (activity as SyncErrorDialogListener).sync(ConflictResolution.FULL_DOWNLOAD) @@ -136,7 +138,7 @@ class SyncErrorDialog : AsyncDialogFragment() { .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - DIALOG_MEDIA_SYNC_ERROR -> { + Type.DIALOG_MEDIA_SYNC_ERROR -> { dialog.setPositiveButton(R.string.check_media) { _, _ -> (activity as SyncErrorDialogListener).mediaCheck() activity?.dismissAllDialogFragments() @@ -144,7 +146,7 @@ class SyncErrorDialog : AsyncDialogFragment() { .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - DIALOG_SYNC_CORRUPT_COLLECTION -> { + Type.DIALOG_SYNC_CORRUPT_COLLECTION -> { dialog.setPositiveButton(R.string.dialog_ok) { _, _ -> } .setNegativeButton(R.string.help) { _, _ -> (requireActivity() as AnkiActivity).openUrl(Uri.parse(getString(R.string.repair_deck))) @@ -152,7 +154,7 @@ class SyncErrorDialog : AsyncDialogFragment() { .setCancelable(false) .create() } - DIALOG_SYNC_BASIC_CHECK_ERROR -> { + Type.DIALOG_SYNC_BASIC_CHECK_ERROR -> { dialog.setPositiveButton(R.string.check_db) { _, _ -> (activity as SyncErrorDialogListener).integrityCheck() activity?.dismissAllDialogFragments() @@ -160,16 +162,21 @@ class SyncErrorDialog : AsyncDialogFragment() { .setNegativeButton(R.string.dialog_cancel) { _, _ -> } .create() } - else -> null!! } } private val title: String - get() = when (requireArguments().getInt("dialogType")) { - DIALOG_USER_NOT_LOGGED_IN_SYNC -> res().getString(R.string.not_logged_in_title) - DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL, DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE -> res().getString(R.string.sync_conflict_replace_title) - DIALOG_SYNC_CONFLICT_RESOLUTION -> res().getString(R.string.sync_conflict_title_new) - else -> res().getString(R.string.sync_error) + get() = when (type()) { + Type.DIALOG_USER_NOT_LOGGED_IN_SYNC -> res().getString(R.string.not_logged_in_title) + Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL, Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE -> res().getString(R.string.sync_conflict_replace_title) + Type.DIALOG_SYNC_CONFLICT_RESOLUTION -> res().getString(R.string.sync_conflict_title_new) + Type.DIALOG_CONNECTION_ERROR, + Type.DIALOG_SYNC_SANITY_ERROR, + Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL, + Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE, + Type.DIALOG_MEDIA_SYNC_ERROR, + Type.DIALOG_SYNC_CORRUPT_COLLECTION, + Type.DIALOG_SYNC_BASIC_CHECK_ERROR -> res().getString(R.string.sync_error) } /** @@ -179,7 +186,7 @@ class SyncErrorDialog : AsyncDialogFragment() { */ override val notificationTitle: String get() { - return if (requireArguments().getInt("dialogType") == DIALOG_USER_NOT_LOGGED_IN_SYNC) { + return if (type() == Type.DIALOG_USER_NOT_LOGGED_IN_SYNC) { res().getString(R.string.sync_error) } else { title @@ -187,19 +194,19 @@ class SyncErrorDialog : AsyncDialogFragment() { } private val message: String? - get() = when (requireArguments().getInt("dialogType")) { - DIALOG_USER_NOT_LOGGED_IN_SYNC -> res().getString(R.string.login_create_account_message) - DIALOG_CONNECTION_ERROR -> res().getString(R.string.connection_error_message) - DIALOG_SYNC_CONFLICT_RESOLUTION -> res().getString(R.string.sync_conflict_message_new) - DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL, DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL -> res().getString(R.string.sync_conflict_local_confirm_new) - DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE, DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE -> res().getString(R.string.sync_conflict_remote_confirm_new) - DIALOG_SYNC_CORRUPT_COLLECTION -> { - val syncMessage = requireArguments().getString("dialogMessage") + get() = when (type()) { + Type.DIALOG_USER_NOT_LOGGED_IN_SYNC -> res().getString(R.string.login_create_account_message) + Type.DIALOG_CONNECTION_ERROR -> res().getString(R.string.connection_error_message) + Type.DIALOG_SYNC_CONFLICT_RESOLUTION -> res().getString(R.string.sync_conflict_message_new) + Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL, Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL -> res().getString(R.string.sync_conflict_local_confirm_new) + Type.DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE, Type.DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE -> res().getString(R.string.sync_conflict_remote_confirm_new) + Type.DIALOG_SYNC_CORRUPT_COLLECTION -> { + val syncMessage = requireArguments().getString(DIALOG_MESSAGE_KEY) val repairUrl = res().getString(R.string.repair_deck) val dialogMessage = res().getString(R.string.sync_corrupt_database, repairUrl) joinSyncMessages(dialogMessage, syncMessage) } - else -> requireArguments().getString("dialogMessage") + else -> requireArguments().getString(DIALOG_MESSAGE_KEY) } /** @@ -209,7 +216,7 @@ class SyncErrorDialog : AsyncDialogFragment() { */ override val notificationMessage: String? get() { - return if (requireArguments().getInt("dialogType") == DIALOG_USER_NOT_LOGGED_IN_SYNC) { + return if (type() == Type.DIALOG_USER_NOT_LOGGED_IN_SYNC) { res().getString(R.string.not_logged_in_title) } else { message @@ -218,39 +225,40 @@ class SyncErrorDialog : AsyncDialogFragment() { override val dialogHandlerMessage: SyncErrorDialogMessageHandler get() { - val dialogType = requireArguments().getInt("dialogType") - val dialogMessage = requireArguments().getString("dialogMessage") + val dialogType = type() + val dialogMessage = requireArguments().getString(DIALOG_MESSAGE_KEY) return SyncErrorDialogMessageHandler(dialogType, dialogMessage) } + enum class Type(val code: Int) { + DIALOG_USER_NOT_LOGGED_IN_SYNC(0), + DIALOG_CONNECTION_ERROR(1), + DIALOG_SYNC_CONFLICT_RESOLUTION(2), + DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL(3), + DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE(4), + DIALOG_SYNC_SANITY_ERROR(5), + DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL(6), + DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE(7), + DIALOG_MEDIA_SYNC_ERROR(8), + DIALOG_SYNC_CORRUPT_COLLECTION(9), + DIALOG_SYNC_BASIC_CHECK_ERROR(10); + + companion object { + fun fromCode(code: Int) = Type.entries.first { code == it.code } + } + } + companion object { - const val DIALOG_USER_NOT_LOGGED_IN_SYNC = 0 - const val DIALOG_CONNECTION_ERROR = 1 - const val DIALOG_SYNC_CONFLICT_RESOLUTION = 2 - const val DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL = 3 - const val DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE = 4 - const val DIALOG_SYNC_SANITY_ERROR = 6 - const val DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL = 7 - const val DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE = 8 - const val DIALOG_MEDIA_SYNC_ERROR = 9 - const val DIALOG_SYNC_CORRUPT_COLLECTION = 10 - const val DIALOG_SYNC_BASIC_CHECK_ERROR = 11 - @VisibleForTesting(otherwise = VisibleForTesting.NONE) - val dialogTypes = arrayOf( - DIALOG_USER_NOT_LOGGED_IN_SYNC, - DIALOG_CONNECTION_ERROR, - DIALOG_SYNC_CONFLICT_RESOLUTION, - DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_LOCAL, - DIALOG_SYNC_CONFLICT_CONFIRM_KEEP_REMOTE, - DIALOG_SYNC_SANITY_ERROR, - DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_LOCAL, - DIALOG_SYNC_SANITY_ERROR_CONFIRM_KEEP_REMOTE, - DIALOG_MEDIA_SYNC_ERROR, - DIALOG_SYNC_CORRUPT_COLLECTION, - DIALOG_SYNC_BASIC_CHECK_ERROR - ) + /** + * Key for the ordinal of the sync error in Type + */ + const val SYNC_ERROR_DIALOG_TYPE_KEY = "dialogType" + /** + * Key for the message to display in the dialog + */ + const val DIALOG_MESSAGE_KEY = "dialogMessage" /** * A set of dialogs belonging to AnkiActivity which deal with sync problems * @@ -258,18 +266,18 @@ class SyncErrorDialog : AsyncDialogFragment() { * @param dialogMessage A string which can be optionally used to set the dialog message */ @CheckResult - fun newInstance(dialogType: Int, dialogMessage: String?): SyncErrorDialog { + fun newInstance(dialogType: Type, dialogMessage: String?): SyncErrorDialog { val f = SyncErrorDialog() val args = Bundle() - args.putInt("dialogType", dialogType) - args.putString("dialogMessage", dialogMessage) + args.putInt(SYNC_ERROR_DIALOG_TYPE_KEY, dialogType.code) + args.putString(DIALOG_MESSAGE_KEY, dialogMessage) f.arguments = args return f } } class SyncErrorDialogMessageHandler( - private val dialogType: Int, + private val dialogType: Type, private val dialogMessage: String? ) : DialogHandlerMessage(WhichDialogHandler.MSG_SHOW_SYNC_ERROR_DIALOG, "SyncErrorDialog") { override fun handleAsyncMessage(activity: AnkiActivity) { @@ -289,15 +297,15 @@ class SyncErrorDialog : AsyncDialogFragment() { override fun toMessage(): Message = Message.obtain().apply { what = this@SyncErrorDialogMessageHandler.what data = bundleOf( - "dialogType" to dialogType, - "dialogMessage" to dialogMessage + SYNC_ERROR_DIALOG_TYPE_KEY to dialogType, + DIALOG_MESSAGE_KEY to dialogMessage ) } companion object { fun fromMessage(message: Message): SyncErrorDialogMessageHandler { - val dialogType = message.data.getInt("dialogType") - val dialogMessage = message.data.getString("dialogMessage") + val dialogType = Type.fromCode(message.data.getInt(SYNC_ERROR_DIALOG_TYPE_KEY)) + val dialogMessage = message.data.getString(DIALOG_MESSAGE_KEY) return SyncErrorDialogMessageHandler(dialogType, dialogMessage) } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/AsyncDialogFragmentsTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/AsyncDialogFragmentsTest.kt index 1cce476d2cd5..c05effbee65a 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/AsyncDialogFragmentsTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/AsyncDialogFragmentsTest.kt @@ -26,7 +26,7 @@ import org.junit.runner.RunWith class AsyncDialogFragmentsTest { @Test fun `SyncErrorDialog does not require context`() { - for (dialogType in SyncErrorDialog.dialogTypes) { + for (dialogType in SyncErrorDialog.Type.entries) { val instance = SyncErrorDialog.newInstance(dialogType, dialogMessage = null) assertDoesNotThrow("$dialogType message required a context") { instance.notificationMessage }