Skip to content

Commit

Permalink
NF: improve SyncErrorDialog
Browse files Browse the repository at this point in the history 
Use an enum and constants
  • Loading branch information
@Arthur-Milchior
Arthur-Milchior committed Dec 7, 2024
1 parent e98fe44 commit 06b9f64
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 76 deletions.
6 changes: 3 additions & 3 deletions AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, "")
}

Expand All @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion AnkiDroid/src/main/java/com/ichi2/anki/Sync.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
150 changes: 79 additions & 71 deletions AnkiDroid/src/main/java/com/ichi2/anki/dialogs/SyncErrorDialog.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) { _, _ ->
Expand All @@ -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) { _, _ ->
Expand All @@ -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) { _, _ ->
Expand All @@ -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) { _, _ ->
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -136,40 +138,45 @@ 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()
}
.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)))
}
.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()
}
.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)
}

/**
Expand All @@ -179,27 +186,27 @@ 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
}
}

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)
}

/**
Expand All @@ -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
Expand All @@ -218,58 +225,59 @@ 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
*
* @param dialogType An integer which specifies which of the sub-dialogs to show
* @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) {
Expand All @@ -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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion AnkiDroid/src/test/java/com/ichi2/anki/dialogs/AsyncDialogFragmentsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down

0 comments on commit 06b9f64

Please sign in to comment.