From 461ec8d7226764a58a7882f286efb687c11ff7bc Mon Sep 17 00:00:00 2001 From: Martin Marconcini Date: Sat, 19 Aug 2023 17:36:00 +0200 Subject: [PATCH 01/11] Prompt user before leaving edit profile when any field has been modified. --- .../tusky/EditProfileActivity.kt | 48 ++++-- .../tusky/viewmodel/EditProfileViewModel.kt | 138 +++++++++++------- app/src/main/res/values/strings.xml | 2 + 3 files changed, 124 insertions(+), 64 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index 190421e69..b734d92a8 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -25,7 +25,9 @@ import android.view.Menu import android.view.MenuItem import android.view.View import android.widget.ImageView +import androidx.activity.OnBackPressedCallback import androidx.activity.viewModels +import androidx.appcompat.app.AlertDialog import androidx.core.view.isVisible import androidx.lifecycle.LiveData import androidx.lifecycle.lifecycleScope @@ -46,9 +48,11 @@ import com.keylesspalace.tusky.di.ViewModelFactory import com.keylesspalace.tusky.util.Error import com.keylesspalace.tusky.util.Loading import com.keylesspalace.tusky.util.Success +import com.keylesspalace.tusky.util.await import com.keylesspalace.tusky.util.show import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.viewmodel.EditProfileViewModel +import com.keylesspalace.tusky.viewmodel.ProfileData import com.mikepenz.iconics.IconicsDrawable import com.mikepenz.iconics.typeface.library.googlematerial.GoogleMaterial import com.mikepenz.iconics.utils.colorInt @@ -200,20 +204,37 @@ class EditProfileActivity : BaseActivity(), Injectable { } } } + + val onBackCallback = object : OnBackPressedCallback(enabled = true) { + override fun handleOnBackPressed() { + if (!viewModel.hasUnsavedChanges(gatherProfileData())) finish() + + lifecycleScope.launch { + when(showConfirmationDialog()) { + AlertDialog.BUTTON_POSITIVE -> save() + else -> finish() + } + } + } + } + + onBackPressedDispatcher.addCallback(this, onBackCallback) } override fun onStop() { super.onStop() if (!isFinishing) { - viewModel.updateProfile( - binding.displayNameEditText.text.toString(), - binding.noteEditText.text.toString(), - binding.lockedCheckBox.isChecked, - accountFieldEditAdapter.getFieldData() - ) + viewModel.updateProfile(gatherProfileData()) } } + private fun gatherProfileData() = ProfileData( + displayName = binding.displayNameEditText.text.toString(), + note = binding.noteEditText.text.toString(), + locked = binding.lockedCheckBox.isChecked, + fields = accountFieldEditAdapter.getFieldData(), + ) + private fun observeImage( liveData: LiveData, imageView: ImageView, @@ -287,14 +308,7 @@ class EditProfileActivity : BaseActivity(), Injectable { return super.onOptionsItemSelected(item) } - private fun save() { - viewModel.save( - binding.displayNameEditText.text.toString(), - binding.noteEditText.text.toString(), - binding.lockedCheckBox.isChecked, - accountFieldEditAdapter.getFieldData() - ) - } + private fun save() = viewModel.save(gatherProfileData()) private fun onSaveFailure(msg: String?) { val errorMsg = msg ?: getString(R.string.error_media_upload_sending) @@ -306,4 +320,10 @@ class EditProfileActivity : BaseActivity(), Injectable { Log.w("EditProfileActivity", "failed to pick media", throwable) Snackbar.make(binding.avatarButton, R.string.error_media_upload_sending, Snackbar.LENGTH_LONG).show() } + + private suspend fun showConfirmationDialog() = AlertDialog.Builder(this) + .setTitle(getString(R.string.title_edit_profile_save_changes_prompt)) + .setMessage(getString(R.string.message_edit_profile_save_changes_prompt)) + .create() + .await(R.string.action_save, R.string.action_discard) } diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt index 21a8a01d7..46b16d5ed 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt @@ -51,6 +51,17 @@ import javax.inject.Inject private const val HEADER_FILE_NAME = "header.png" private const val AVATAR_FILE_NAME = "avatar.png" + +/** + * Conveniently groups Profile Data users can modify in the UI. + */ +internal data class ProfileData( + val displayName: String, + val note: String, + val locked: Boolean, + val fields: List, +) + class EditProfileViewModel @Inject constructor( private val mastodonApi: MastodonApi, private val eventHub: EventHub, @@ -96,29 +107,73 @@ class EditProfileViewModel @Inject constructor( headerData.value = getHeaderUri() } - fun save(newDisplayName: String, newNote: String, newLocked: Boolean, newFields: List) { + internal fun save(newProfileData: ProfileData) { if (saveData.value is Loading || profileData.value !is Success) { return } saveData.value = Loading() - val displayName = if (oldProfileData?.displayName == newDisplayName) { - null - } else { - newDisplayName.toRequestBody(MultipartBody.FORM) + val encoded = encodeChangedProfileFields(newProfileData) + if (encoded.allFieldsAreNull()) { + // if nothing has changed, there is no need to make a network request + saveData.postValue(Success()) + return } - val note = if (oldProfileData?.source?.note == newNote) { + viewModelScope.launch { + mastodonApi.accountUpdateCredentials( + encoded.displayName, encoded.note, encoded.locked, encoded.avatar, encoded.header, + encoded.field1?.first, encoded.field1?.second, encoded.field2?.first, encoded.field2?.second, encoded.field3?.first, encoded.field3?.second, encoded.field4?.first, encoded.field4?.second + ).fold( + { newProfileData -> + saveData.postValue(Success()) + eventHub.dispatch(ProfileEditedEvent(newProfileData)) + }, + { throwable -> + saveData.postValue(Error(errorMessage = throwable.getServerErrorMessage())) + } + ) + } + } + + // cache activity state for rotation change + internal fun updateProfile(newProfileData: ProfileData) { + if (profileData.value is Success) { + val newProfileSource = profileData.value?.data?.source?.copy(note = newProfileData.note, fields = newProfileData.fields) + val newProfile = profileData.value?.data?.copy( + displayName = newProfileData.displayName, + locked = newProfileData.locked, + source = newProfileSource + ) + + profileData.postValue(Success(newProfile)) + } + } + + internal fun hasUnsavedChanges(newProfileData: ProfileData) : Boolean { + val encoded = encodeChangedProfileFields(newProfileData) + // If all fields are null, there are no changes. + return !encoded.allFieldsAreNull() + } + + private fun encodeChangedProfileFields(newProfileData: ProfileData): EncodedProfileData { + val displayName = if (oldProfileData?.displayName == newProfileData.displayName) { null } else { - newNote.toRequestBody(MultipartBody.FORM) + newProfileData.displayName.toRequestBody(MultipartBody.FORM) } - val locked = if (oldProfileData?.locked == newLocked) { + val note = if (oldProfileData?.source?.note == newProfileData.note) { null } else { - newLocked.toString().toRequestBody(MultipartBody.FORM) + newProfileData.note.toRequestBody(MultipartBody.FORM) + } + + val locked = if (oldProfileData?.locked == newProfileData.locked) { + null + } else { + newProfileData.locked.toString().toRequestBody(MultipartBody.FORM) } val avatar = if (avatarData.value != null) { @@ -136,48 +191,15 @@ class EditProfileViewModel @Inject constructor( } // when one field changed, all have to be sent or they unchanged ones would get overridden - val fieldsUnchanged = oldProfileData?.source?.fields == newFields - val field1 = calculateFieldToUpdate(newFields.getOrNull(0), fieldsUnchanged) - val field2 = calculateFieldToUpdate(newFields.getOrNull(1), fieldsUnchanged) - val field3 = calculateFieldToUpdate(newFields.getOrNull(2), fieldsUnchanged) - val field4 = calculateFieldToUpdate(newFields.getOrNull(3), fieldsUnchanged) + val fieldsUnchanged = oldProfileData?.source?.fields == newProfileData.fields + val field1 = calculateFieldToUpdate(newProfileData.fields.getOrNull(0), fieldsUnchanged) + val field2 = calculateFieldToUpdate(newProfileData.fields.getOrNull(1), fieldsUnchanged) + val field3 = calculateFieldToUpdate(newProfileData.fields.getOrNull(2), fieldsUnchanged) + val field4 = calculateFieldToUpdate(newProfileData.fields.getOrNull(3), fieldsUnchanged) - if (displayName == null && note == null && locked == null && avatar == null && header == null && - field1 == null && field2 == null && field3 == null && field4 == null - ) { - /** if nothing has changed, there is no need to make a network request */ - saveData.postValue(Success()) - return - } - - viewModelScope.launch { - mastodonApi.accountUpdateCredentials( - displayName, note, locked, avatar, header, - field1?.first, field1?.second, field2?.first, field2?.second, field3?.first, field3?.second, field4?.first, field4?.second - ).fold( - { newProfileData -> - saveData.postValue(Success()) - eventHub.dispatch(ProfileEditedEvent(newProfileData)) - }, - { throwable -> - saveData.postValue(Error(errorMessage = throwable.getServerErrorMessage())) - } - ) - } - } - - // cache activity state for rotation change - fun updateProfile(newDisplayName: String, newNote: String, newLocked: Boolean, newFields: List) { - if (profileData.value is Success) { - val newProfileSource = profileData.value?.data?.source?.copy(note = newNote, fields = newFields) - val newProfile = profileData.value?.data?.copy( - displayName = newDisplayName, - locked = newLocked, - source = newProfileSource - ) - - profileData.postValue(Success(newProfile)) - } + return EncodedProfileData( + displayName, note, locked, field1, field2, field3, field4, header, avatar + ) } private fun calculateFieldToUpdate(newField: StringField?, fieldsUnchanged: Boolean): Pair? { @@ -193,4 +215,20 @@ class EditProfileViewModel @Inject constructor( private fun getCacheFileForName(filename: String): File { return File(application.cacheDir, filename) } + + private data class EncodedProfileData( + val displayName: RequestBody?, + val note: RequestBody?, + val locked: RequestBody?, + val field1: Pair?, + val field2: Pair?, + val field3: Pair?, + val field4: Pair?, + val header: MultipartBody.Part?, + val avatar: MultipartBody.Part?, + ) { + fun allFieldsAreNull() = displayName == null && note == null && locked == null + && avatar == null && header == null && field1 == null && field2 == null + && field3 == null && field4 == null + } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index fdf776200..b43965fbc 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -820,4 +820,6 @@ Playback failed: %s Delete filter \'%1$s\'?" Delete + Unsaved Changes + Do you want to save your profile changes? From 634f020ffa9adae79f413c792ec9dcc3fb128a81 Mon Sep 17 00:00:00 2001 From: Martin Marconcini Date: Sat, 19 Aug 2023 17:57:25 +0200 Subject: [PATCH 02/11] Apply klint recommendations. --- .../com/keylesspalace/tusky/EditProfileActivity.kt | 4 ++-- .../tusky/viewmodel/EditProfileViewModel.kt | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index b734d92a8..2fa1d3ae7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -210,7 +210,7 @@ class EditProfileActivity : BaseActivity(), Injectable { if (!viewModel.hasUnsavedChanges(gatherProfileData())) finish() lifecycleScope.launch { - when(showConfirmationDialog()) { + when (showConfirmationDialog()) { AlertDialog.BUTTON_POSITIVE -> save() else -> finish() } @@ -232,7 +232,7 @@ class EditProfileActivity : BaseActivity(), Injectable { displayName = binding.displayNameEditText.text.toString(), note = binding.noteEditText.text.toString(), locked = binding.lockedCheckBox.isChecked, - fields = accountFieldEditAdapter.getFieldData(), + fields = accountFieldEditAdapter.getFieldData() ) private fun observeImage( diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt index 46b16d5ed..aa5ff44c8 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt @@ -51,7 +51,6 @@ import javax.inject.Inject private const val HEADER_FILE_NAME = "header.png" private const val AVATAR_FILE_NAME = "avatar.png" - /** * Conveniently groups Profile Data users can modify in the UI. */ @@ -59,7 +58,7 @@ internal data class ProfileData( val displayName: String, val note: String, val locked: Boolean, - val fields: List, + val fields: List ) class EditProfileViewModel @Inject constructor( @@ -151,7 +150,7 @@ class EditProfileViewModel @Inject constructor( } } - internal fun hasUnsavedChanges(newProfileData: ProfileData) : Boolean { + internal fun hasUnsavedChanges(newProfileData: ProfileData): Boolean { val encoded = encodeChangedProfileFields(newProfileData) // If all fields are null, there are no changes. return !encoded.allFieldsAreNull() @@ -225,10 +224,10 @@ class EditProfileViewModel @Inject constructor( val field3: Pair?, val field4: Pair?, val header: MultipartBody.Part?, - val avatar: MultipartBody.Part?, + val avatar: MultipartBody.Part? ) { - fun allFieldsAreNull() = displayName == null && note == null && locked == null - && avatar == null && header == null && field1 == null && field2 == null - && field3 == null && field4 == null + fun allFieldsAreNull() = displayName == null && note == null && locked == null && + avatar == null && header == null && field1 == null && field2 == null && + field3 == null && field4 == null } } From 06239bb8a1755f848d872d6a0aa721b87f34899e Mon Sep 17 00:00:00 2001 From: Martin Marconcini Date: Tue, 22 Aug 2023 12:05:11 +0200 Subject: [PATCH 03/11] Fix synthetic accessor lint error. --- .../tusky/EditProfileActivity.kt | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index 2fa1d3ae7..2e7695aba 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -207,7 +207,7 @@ class EditProfileActivity : BaseActivity(), Injectable { val onBackCallback = object : OnBackPressedCallback(enabled = true) { override fun handleOnBackPressed() { - if (!viewModel.hasUnsavedChanges(gatherProfileData())) finish() + if (!viewModel.hasUnsavedChanges(profileData)) finish() lifecycleScope.launch { when (showConfirmationDialog()) { @@ -224,16 +224,17 @@ class EditProfileActivity : BaseActivity(), Injectable { override fun onStop() { super.onStop() if (!isFinishing) { - viewModel.updateProfile(gatherProfileData()) + viewModel.updateProfile(profileData) } } - private fun gatherProfileData() = ProfileData( - displayName = binding.displayNameEditText.text.toString(), - note = binding.noteEditText.text.toString(), - locked = binding.lockedCheckBox.isChecked, - fields = accountFieldEditAdapter.getFieldData() - ) + private val profileData + get() = ProfileData( + displayName = binding.displayNameEditText.text.toString(), + note = binding.noteEditText.text.toString(), + locked = binding.lockedCheckBox.isChecked, + fields = accountFieldEditAdapter.getFieldData()) + private fun observeImage( liveData: LiveData, @@ -308,7 +309,7 @@ class EditProfileActivity : BaseActivity(), Injectable { return super.onOptionsItemSelected(item) } - private fun save() = viewModel.save(gatherProfileData()) + private fun save() = viewModel.save(profileData) private fun onSaveFailure(msg: String?) { val errorMsg = msg ?: getString(R.string.error_media_upload_sending) From c446d510e4c3c26ab315f7bbaa1947b3bb63e0b2 Mon Sep 17 00:00:00 2001 From: Martin Marconcini Date: Tue, 22 Aug 2023 12:08:13 +0200 Subject: [PATCH 04/11] Fix lint double space. --- .../main/java/com/keylesspalace/tusky/EditProfileActivity.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index 2e7695aba..d15d0d1ab 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -233,8 +233,8 @@ class EditProfileActivity : BaseActivity(), Injectable { displayName = binding.displayNameEditText.text.toString(), note = binding.noteEditText.text.toString(), locked = binding.lockedCheckBox.isChecked, - fields = accountFieldEditAdapter.getFieldData()) - + fields = accountFieldEditAdapter.getFieldData() + ) private fun observeImage( liveData: LiveData, From 8edc8d6422cba5d1da3d85b346fd7750766d1438 Mon Sep 17 00:00:00 2001 From: Martin Marconcini Date: Tue, 22 Aug 2023 12:19:38 +0200 Subject: [PATCH 05/11] Make profileData internal so there's no synthetic accessor required. --- .../main/java/com/keylesspalace/tusky/EditProfileActivity.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index d15d0d1ab..3d02f940d 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -228,7 +228,7 @@ class EditProfileActivity : BaseActivity(), Injectable { } } - private val profileData + internal val profileData get() = ProfileData( displayName = binding.displayNameEditText.text.toString(), note = binding.noteEditText.text.toString(), From e56c0cb5a327256427eb7bd544ca196c286ef707 Mon Sep 17 00:00:00 2001 From: Martin Marconcini Date: Tue, 22 Aug 2023 12:49:33 +0200 Subject: [PATCH 06/11] Avoid synthetic accessors. --- .../tusky/EditProfileActivity.kt | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index 3d02f940d..87df58fb7 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -206,21 +206,20 @@ class EditProfileActivity : BaseActivity(), Injectable { } val onBackCallback = object : OnBackPressedCallback(enabled = true) { - override fun handleOnBackPressed() { - if (!viewModel.hasUnsavedChanges(profileData)) finish() - - lifecycleScope.launch { - when (showConfirmationDialog()) { - AlertDialog.BUTTON_POSITIVE -> save() - else -> finish() - } - } - } + override fun handleOnBackPressed() = checkForPotentialUnsavedChanges() } onBackPressedDispatcher.addCallback(this, onBackCallback) } + fun checkForPotentialUnsavedChanges() { + if (hasUnsavedChanges()) { + showUnsavedChangesDialog() + } else { + finish() + } + } + override fun onStop() { super.onStop() if (!isFinishing) { @@ -228,7 +227,7 @@ class EditProfileActivity : BaseActivity(), Injectable { } } - internal val profileData + private val profileData get() = ProfileData( displayName = binding.displayNameEditText.text.toString(), note = binding.noteEditText.text.toString(), @@ -322,7 +321,16 @@ class EditProfileActivity : BaseActivity(), Injectable { Snackbar.make(binding.avatarButton, R.string.error_media_upload_sending, Snackbar.LENGTH_LONG).show() } - private suspend fun showConfirmationDialog() = AlertDialog.Builder(this) + private fun showUnsavedChangesDialog() = lifecycleScope.launch { + when (launchAlertDialog()) { + AlertDialog.BUTTON_POSITIVE -> save() + else -> finish() + } + } + + private fun hasUnsavedChanges() = viewModel.hasUnsavedChanges(profileData) + + private suspend fun launchAlertDialog() = AlertDialog.Builder(this) .setTitle(getString(R.string.title_edit_profile_save_changes_prompt)) .setMessage(getString(R.string.message_edit_profile_save_changes_prompt)) .create() From f09f464667f74ba37d14907e7d3bc27b57f6ec96 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Tue, 22 Aug 2023 15:52:09 +0200 Subject: [PATCH 07/11] 3486: Rename stuff --- .../tusky/EditProfileActivity.kt | 38 +++++++++---------- .../tusky/viewmodel/EditProfileViewModel.kt | 38 +++++++++---------- 2 files changed, 36 insertions(+), 40 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index 87df58fb7..ea55bd810 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -100,6 +100,14 @@ class EditProfileActivity : BaseActivity(), Injectable { } } + private val currentProfileData + get() = ProfileData( + displayName = binding.displayNameEditText.text.toString(), + note = binding.noteEditText.text.toString(), + locked = binding.lockedCheckBox.isChecked, + fields = accountFieldEditAdapter.getFieldData() + ) + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) @@ -206,35 +214,25 @@ class EditProfileActivity : BaseActivity(), Injectable { } val onBackCallback = object : OnBackPressedCallback(enabled = true) { - override fun handleOnBackPressed() = checkForPotentialUnsavedChanges() + override fun handleOnBackPressed() { + if (viewModel.hasUnsavedChanges(currentProfileData)) { + showUnsavedChangesDialog() + } else { + finish() + } + } } onBackPressedDispatcher.addCallback(this, onBackCallback) } - fun checkForPotentialUnsavedChanges() { - if (hasUnsavedChanges()) { - showUnsavedChangesDialog() - } else { - finish() - } - } - override fun onStop() { super.onStop() if (!isFinishing) { - viewModel.updateProfile(profileData) + viewModel.updateProfile(currentProfileData) } } - private val profileData - get() = ProfileData( - displayName = binding.displayNameEditText.text.toString(), - note = binding.noteEditText.text.toString(), - locked = binding.lockedCheckBox.isChecked, - fields = accountFieldEditAdapter.getFieldData() - ) - private fun observeImage( liveData: LiveData, imageView: ImageView, @@ -308,7 +306,7 @@ class EditProfileActivity : BaseActivity(), Injectable { return super.onOptionsItemSelected(item) } - private fun save() = viewModel.save(profileData) + private fun save() = viewModel.save(currentProfileData) private fun onSaveFailure(msg: String?) { val errorMsg = msg ?: getString(R.string.error_media_upload_sending) @@ -328,8 +326,6 @@ class EditProfileActivity : BaseActivity(), Injectable { } } - private fun hasUnsavedChanges() = viewModel.hasUnsavedChanges(profileData) - private suspend fun launchAlertDialog() = AlertDialog.Builder(this) .setTitle(getString(R.string.title_edit_profile_save_changes_prompt)) .setMessage(getString(R.string.message_edit_profile_save_changes_prompt)) diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt index aa5ff44c8..4a8782d58 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt @@ -76,7 +76,7 @@ class EditProfileViewModel @Inject constructor( val instanceData: Flow = instanceInfoRepo::getInstanceInfo.asFlow() .shareIn(viewModelScope, SharingStarted.Eagerly, replay = 1) - private var oldProfileData: Account? = null + private var apiProfileAccount: Account? = null fun obtainProfile() = viewModelScope.launch { if (profileData.value == null || profileData.value is Error) { @@ -84,7 +84,7 @@ class EditProfileViewModel @Inject constructor( mastodonApi.accountVerifyCredentials().fold( { profile -> - oldProfileData = profile + apiProfileAccount = profile profileData.postValue(Success(profile)) }, { @@ -113,21 +113,21 @@ class EditProfileViewModel @Inject constructor( saveData.value = Loading() - val encoded = encodeChangedProfileFields(newProfileData) - if (encoded.allFieldsAreNull()) { - // if nothing has changed, there is no need to make a network request + val diff = getProfileDiff(apiProfileAccount, newProfileData) + if (diff.hasNoChanges()) { + // if nothing has changed, there is no need to make an api call saveData.postValue(Success()) return } viewModelScope.launch { mastodonApi.accountUpdateCredentials( - encoded.displayName, encoded.note, encoded.locked, encoded.avatar, encoded.header, - encoded.field1?.first, encoded.field1?.second, encoded.field2?.first, encoded.field2?.second, encoded.field3?.first, encoded.field3?.second, encoded.field4?.first, encoded.field4?.second + diff.displayName, diff.note, diff.locked, diff.avatar, diff.header, + diff.field1?.first, diff.field1?.second, diff.field2?.first, diff.field2?.second, diff.field3?.first, diff.field3?.second, diff.field4?.first, diff.field4?.second ).fold( - { newProfileData -> + { newAccountData -> saveData.postValue(Success()) - eventHub.dispatch(ProfileEditedEvent(newProfileData)) + eventHub.dispatch(ProfileEditedEvent(newAccountData)) }, { throwable -> saveData.postValue(Error(errorMessage = throwable.getServerErrorMessage())) @@ -151,25 +151,25 @@ class EditProfileViewModel @Inject constructor( } internal fun hasUnsavedChanges(newProfileData: ProfileData): Boolean { - val encoded = encodeChangedProfileFields(newProfileData) + val diff = getProfileDiff(apiProfileAccount, newProfileData) // If all fields are null, there are no changes. - return !encoded.allFieldsAreNull() + return !diff.hasNoChanges() } - private fun encodeChangedProfileFields(newProfileData: ProfileData): EncodedProfileData { - val displayName = if (oldProfileData?.displayName == newProfileData.displayName) { + private fun getProfileDiff(oldProfileAccount: Account?, newProfileData: ProfileData): DiffProfileData { + val displayName = if (oldProfileAccount?.displayName == newProfileData.displayName) { null } else { newProfileData.displayName.toRequestBody(MultipartBody.FORM) } - val note = if (oldProfileData?.source?.note == newProfileData.note) { + val note = if (oldProfileAccount?.source?.note == newProfileData.note) { null } else { newProfileData.note.toRequestBody(MultipartBody.FORM) } - val locked = if (oldProfileData?.locked == newProfileData.locked) { + val locked = if (oldProfileAccount?.locked == newProfileData.locked) { null } else { newProfileData.locked.toString().toRequestBody(MultipartBody.FORM) @@ -190,13 +190,13 @@ class EditProfileViewModel @Inject constructor( } // when one field changed, all have to be sent or they unchanged ones would get overridden - val fieldsUnchanged = oldProfileData?.source?.fields == newProfileData.fields + val fieldsUnchanged = oldProfileAccount?.source?.fields == newProfileData.fields val field1 = calculateFieldToUpdate(newProfileData.fields.getOrNull(0), fieldsUnchanged) val field2 = calculateFieldToUpdate(newProfileData.fields.getOrNull(1), fieldsUnchanged) val field3 = calculateFieldToUpdate(newProfileData.fields.getOrNull(2), fieldsUnchanged) val field4 = calculateFieldToUpdate(newProfileData.fields.getOrNull(3), fieldsUnchanged) - return EncodedProfileData( + return DiffProfileData( displayName, note, locked, field1, field2, field3, field4, header, avatar ) } @@ -215,7 +215,7 @@ class EditProfileViewModel @Inject constructor( return File(application.cacheDir, filename) } - private data class EncodedProfileData( + private data class DiffProfileData( val displayName: RequestBody?, val note: RequestBody?, val locked: RequestBody?, @@ -226,7 +226,7 @@ class EditProfileViewModel @Inject constructor( val header: MultipartBody.Part?, val avatar: MultipartBody.Part? ) { - fun allFieldsAreNull() = displayName == null && note == null && locked == null && + fun hasNoChanges() = displayName == null && note == null && locked == null && avatar == null && header == null && field1 == null && field2 == null && field3 == null && field4 == null } From ba50ff5686901fab541ae5be74d9025bc372d769 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Tue, 22 Aug 2023 16:12:03 +0200 Subject: [PATCH 08/11] 3486: Separate diff and encoding --- .../tusky/viewmodel/EditProfileViewModel.kt | 89 +++++++++++-------- 1 file changed, 54 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt index 4a8782d58..f3564ad33 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt @@ -114,16 +114,37 @@ class EditProfileViewModel @Inject constructor( saveData.value = Loading() val diff = getProfileDiff(apiProfileAccount, newProfileData) - if (diff.hasNoChanges()) { + if (!diff.hasChanges()) { // if nothing has changed, there is no need to make an api call saveData.postValue(Success()) return } viewModelScope.launch { + var avatarFileBody: MultipartBody.Part? = null + diff.avatarFile?.let { + avatarFileBody = MultipartBody.Part.createFormData("avatar", randomAlphanumericString(12), it.asRequestBody("image/png".toMediaTypeOrNull())) + } + + var headerFileBody: MultipartBody.Part? = null + diff.headerFile?.let { + headerFileBody = MultipartBody.Part.createFormData("header", randomAlphanumericString(12), it.asRequestBody("image/png".toMediaTypeOrNull())) + } + mastodonApi.accountUpdateCredentials( - diff.displayName, diff.note, diff.locked, diff.avatar, diff.header, - diff.field1?.first, diff.field1?.second, diff.field2?.first, diff.field2?.second, diff.field3?.first, diff.field3?.second, diff.field4?.first, diff.field4?.second + diff.displayName?.toRequestBody(MultipartBody.FORM), + diff.note?.toRequestBody(MultipartBody.FORM), + diff.locked?.toString()?.toRequestBody(MultipartBody.FORM), + avatarFileBody, + headerFileBody, + diff.field1?.first?.toRequestBody(MultipartBody.FORM), + diff.field1?.second?.toRequestBody(MultipartBody.FORM), + diff.field2?.first?.toRequestBody(MultipartBody.FORM), + diff.field2?.second?.toRequestBody(MultipartBody.FORM), + diff.field3?.first?.toRequestBody(MultipartBody.FORM), + diff.field3?.second?.toRequestBody(MultipartBody.FORM), + diff.field4?.first?.toRequestBody(MultipartBody.FORM), + diff.field4?.second?.toRequestBody(MultipartBody.FORM), ).fold( { newAccountData -> saveData.postValue(Success()) @@ -152,62 +173,60 @@ class EditProfileViewModel @Inject constructor( internal fun hasUnsavedChanges(newProfileData: ProfileData): Boolean { val diff = getProfileDiff(apiProfileAccount, newProfileData) - // If all fields are null, there are no changes. - return !diff.hasNoChanges() + + return diff.hasChanges() } private fun getProfileDiff(oldProfileAccount: Account?, newProfileData: ProfileData): DiffProfileData { val displayName = if (oldProfileAccount?.displayName == newProfileData.displayName) { null } else { - newProfileData.displayName.toRequestBody(MultipartBody.FORM) + newProfileData.displayName } val note = if (oldProfileAccount?.source?.note == newProfileData.note) { null } else { - newProfileData.note.toRequestBody(MultipartBody.FORM) + newProfileData.note } val locked = if (oldProfileAccount?.locked == newProfileData.locked) { null } else { - newProfileData.locked.toString().toRequestBody(MultipartBody.FORM) + newProfileData.locked } - val avatar = if (avatarData.value != null) { - val avatarBody = getCacheFileForName(AVATAR_FILE_NAME).asRequestBody("image/png".toMediaTypeOrNull()) - MultipartBody.Part.createFormData("avatar", randomAlphanumericString(12), avatarBody) + val avatarFile = if (avatarData.value != null) { + getCacheFileForName(AVATAR_FILE_NAME) } else { null } - val header = if (headerData.value != null) { - val headerBody = getCacheFileForName(HEADER_FILE_NAME).asRequestBody("image/png".toMediaTypeOrNull()) - MultipartBody.Part.createFormData("header", randomAlphanumericString(12), headerBody) + val headerFile = if (headerData.value != null) { + getCacheFileForName(HEADER_FILE_NAME) } else { null } // when one field changed, all have to be sent or they unchanged ones would get overridden - val fieldsUnchanged = oldProfileAccount?.source?.fields == newProfileData.fields - val field1 = calculateFieldToUpdate(newProfileData.fields.getOrNull(0), fieldsUnchanged) - val field2 = calculateFieldToUpdate(newProfileData.fields.getOrNull(1), fieldsUnchanged) - val field3 = calculateFieldToUpdate(newProfileData.fields.getOrNull(2), fieldsUnchanged) - val field4 = calculateFieldToUpdate(newProfileData.fields.getOrNull(3), fieldsUnchanged) + val allFieldsUnchanged = oldProfileAccount?.source?.fields == newProfileData.fields + val field1 = calculateFieldToUpdate(newProfileData.fields.getOrNull(0), allFieldsUnchanged) + val field2 = calculateFieldToUpdate(newProfileData.fields.getOrNull(1), allFieldsUnchanged) + val field3 = calculateFieldToUpdate(newProfileData.fields.getOrNull(2), allFieldsUnchanged) + val field4 = calculateFieldToUpdate(newProfileData.fields.getOrNull(3), allFieldsUnchanged) return DiffProfileData( - displayName, note, locked, field1, field2, field3, field4, header, avatar + displayName, note, locked, field1, field2, field3, field4, headerFile, avatarFile ) } - private fun calculateFieldToUpdate(newField: StringField?, fieldsUnchanged: Boolean): Pair? { + private fun calculateFieldToUpdate(newField: StringField?, fieldsUnchanged: Boolean): Pair? { if (fieldsUnchanged || newField == null) { return null } return Pair( - newField.name.toRequestBody(MultipartBody.FORM), - newField.value.toRequestBody(MultipartBody.FORM) + newField.name, + newField.value, ) } @@ -216,18 +235,18 @@ class EditProfileViewModel @Inject constructor( } private data class DiffProfileData( - val displayName: RequestBody?, - val note: RequestBody?, - val locked: RequestBody?, - val field1: Pair?, - val field2: Pair?, - val field3: Pair?, - val field4: Pair?, - val header: MultipartBody.Part?, - val avatar: MultipartBody.Part? + val displayName: String?, + val note: String?, + val locked: Boolean?, + val field1: Pair?, + val field2: Pair?, + val field3: Pair?, + val field4: Pair?, + val headerFile: File?, + val avatarFile: File? ) { - fun hasNoChanges() = displayName == null && note == null && locked == null && - avatar == null && header == null && field1 == null && field2 == null && - field3 == null && field4 == null + fun hasChanges() = displayName != null || note != null || locked != null || + avatarFile != null || headerFile != null || field1 != null || field2 != null || + field3 != null || field4 != null } } From 3a402740031fbde683cc5dccb1fcc6d21c70fec5 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Tue, 22 Aug 2023 21:17:22 +0200 Subject: [PATCH 09/11] 3486: Re-introduce separate check method to not need a synthetic accessor (lint error) --- .../keylesspalace/tusky/EditProfileActivity.kt | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index ea55bd810..a24ccc10a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -214,18 +214,21 @@ class EditProfileActivity : BaseActivity(), Injectable { } val onBackCallback = object : OnBackPressedCallback(enabled = true) { - override fun handleOnBackPressed() { - if (viewModel.hasUnsavedChanges(currentProfileData)) { - showUnsavedChangesDialog() - } else { - finish() - } - } + override fun handleOnBackPressed() = checkForUnsavedChanges() } onBackPressedDispatcher.addCallback(this, onBackCallback) } + fun checkForUnsavedChanges() { + if (viewModel.hasUnsavedChanges(currentProfileData)) { + showUnsavedChangesDialog() + } else { + finish() + } + } + + override fun onStop() { super.onStop() if (!isFinishing) { From 45d2fa1570e677e15119e129a5cf54c9c03e7c63 Mon Sep 17 00:00:00 2001 From: Lakoja Date: Wed, 23 Aug 2023 15:06:08 +0200 Subject: [PATCH 10/11] 3486: (Appease linter) --- .../java/com/keylesspalace/tusky/EditProfileActivity.kt | 1 - .../tusky/viewmodel/EditProfileViewModel.kt | 9 ++++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index a24ccc10a..f373521ac 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -228,7 +228,6 @@ class EditProfileActivity : BaseActivity(), Injectable { } } - override fun onStop() { super.onStop() if (!isFinishing) { diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt index f3564ad33..2e1a8d43c 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt @@ -42,7 +42,6 @@ import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.launch import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.MultipartBody -import okhttp3.RequestBody import okhttp3.RequestBody.Companion.asRequestBody import okhttp3.RequestBody.Companion.toRequestBody import java.io.File @@ -123,12 +122,12 @@ class EditProfileViewModel @Inject constructor( viewModelScope.launch { var avatarFileBody: MultipartBody.Part? = null diff.avatarFile?.let { - avatarFileBody = MultipartBody.Part.createFormData("avatar", randomAlphanumericString(12), it.asRequestBody("image/png".toMediaTypeOrNull())) + avatarFileBody = MultipartBody.Part.createFormData("avatar", randomAlphanumericString(12), it.asRequestBody("image/png".toMediaTypeOrNull())) } var headerFileBody: MultipartBody.Part? = null diff.headerFile?.let { - headerFileBody = MultipartBody.Part.createFormData("header", randomAlphanumericString(12), it.asRequestBody("image/png".toMediaTypeOrNull())) + headerFileBody = MultipartBody.Part.createFormData("header", randomAlphanumericString(12), it.asRequestBody("image/png".toMediaTypeOrNull())) } mastodonApi.accountUpdateCredentials( @@ -144,7 +143,7 @@ class EditProfileViewModel @Inject constructor( diff.field3?.first?.toRequestBody(MultipartBody.FORM), diff.field3?.second?.toRequestBody(MultipartBody.FORM), diff.field4?.first?.toRequestBody(MultipartBody.FORM), - diff.field4?.second?.toRequestBody(MultipartBody.FORM), + diff.field4?.second?.toRequestBody(MultipartBody.FORM) ).fold( { newAccountData -> saveData.postValue(Success()) @@ -226,7 +225,7 @@ class EditProfileViewModel @Inject constructor( } return Pair( newField.name, - newField.value, + newField.value ) } From 387c3989d7339a88086311d35577b3d4634c4bd5 Mon Sep 17 00:00:00 2001 From: Martin Marconcini Date: Thu, 24 Aug 2023 10:06:25 +0200 Subject: [PATCH 11/11] Apply PR suggestions: * Remove dialog title and string * Rename Data Class * Use liveData.value directly when possible --- .../keylesspalace/tusky/EditProfileActivity.kt | 11 +++++------ .../tusky/viewmodel/EditProfileViewModel.kt | 17 +++++++---------- app/src/main/res/values/strings.xml | 3 +-- 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt index f373521ac..94c160f74 100644 --- a/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt +++ b/app/src/main/java/com/keylesspalace/tusky/EditProfileActivity.kt @@ -52,7 +52,7 @@ import com.keylesspalace.tusky.util.await import com.keylesspalace.tusky.util.show import com.keylesspalace.tusky.util.viewBinding import com.keylesspalace.tusky.viewmodel.EditProfileViewModel -import com.keylesspalace.tusky.viewmodel.ProfileData +import com.keylesspalace.tusky.viewmodel.ProfileDataInUi import com.mikepenz.iconics.IconicsDrawable import com.mikepenz.iconics.typeface.library.googlematerial.GoogleMaterial import com.mikepenz.iconics.utils.colorInt @@ -101,7 +101,7 @@ class EditProfileActivity : BaseActivity(), Injectable { } private val currentProfileData - get() = ProfileData( + get() = ProfileDataInUi( displayName = binding.displayNameEditText.text.toString(), note = binding.noteEditText.text.toString(), locked = binding.lockedCheckBox.isChecked, @@ -322,15 +322,14 @@ class EditProfileActivity : BaseActivity(), Injectable { } private fun showUnsavedChangesDialog() = lifecycleScope.launch { - when (launchAlertDialog()) { + when (launchSaveDialog()) { AlertDialog.BUTTON_POSITIVE -> save() else -> finish() } } - private suspend fun launchAlertDialog() = AlertDialog.Builder(this) - .setTitle(getString(R.string.title_edit_profile_save_changes_prompt)) - .setMessage(getString(R.string.message_edit_profile_save_changes_prompt)) + private suspend fun launchSaveDialog() = AlertDialog.Builder(this) + .setMessage(getString(R.string.dialog_save_profile_changes_message)) .create() .await(R.string.action_save, R.string.action_discard) } diff --git a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt index 2e1a8d43c..55de04be4 100644 --- a/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt +++ b/app/src/main/java/com/keylesspalace/tusky/viewmodel/EditProfileViewModel.kt @@ -50,10 +50,7 @@ import javax.inject.Inject private const val HEADER_FILE_NAME = "header.png" private const val AVATAR_FILE_NAME = "avatar.png" -/** - * Conveniently groups Profile Data users can modify in the UI. - */ -internal data class ProfileData( +internal data class ProfileDataInUi( val displayName: String, val note: String, val locked: Boolean, @@ -105,7 +102,7 @@ class EditProfileViewModel @Inject constructor( headerData.value = getHeaderUri() } - internal fun save(newProfileData: ProfileData) { + internal fun save(newProfileData: ProfileDataInUi) { if (saveData.value is Loading || profileData.value !is Success) { return } @@ -115,7 +112,7 @@ class EditProfileViewModel @Inject constructor( val diff = getProfileDiff(apiProfileAccount, newProfileData) if (!diff.hasChanges()) { // if nothing has changed, there is no need to make an api call - saveData.postValue(Success()) + saveData.value = Success() return } @@ -157,7 +154,7 @@ class EditProfileViewModel @Inject constructor( } // cache activity state for rotation change - internal fun updateProfile(newProfileData: ProfileData) { + internal fun updateProfile(newProfileData: ProfileDataInUi) { if (profileData.value is Success) { val newProfileSource = profileData.value?.data?.source?.copy(note = newProfileData.note, fields = newProfileData.fields) val newProfile = profileData.value?.data?.copy( @@ -166,17 +163,17 @@ class EditProfileViewModel @Inject constructor( source = newProfileSource ) - profileData.postValue(Success(newProfile)) + profileData.value = Success(newProfile) } } - internal fun hasUnsavedChanges(newProfileData: ProfileData): Boolean { + internal fun hasUnsavedChanges(newProfileData: ProfileDataInUi): Boolean { val diff = getProfileDiff(apiProfileAccount, newProfileData) return diff.hasChanges() } - private fun getProfileDiff(oldProfileAccount: Account?, newProfileData: ProfileData): DiffProfileData { + private fun getProfileDiff(oldProfileAccount: Account?, newProfileData: ProfileDataInUi): DiffProfileData { val displayName = if (oldProfileAccount?.displayName == newProfileData.displayName) { null } else { diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 2233b2dea..a04f6ca5b 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -819,6 +819,5 @@ Playback failed: %s Delete filter \'%1$s\'?" Delete - Unsaved Changes - Do you want to save your profile changes? + Do you want to save your profile changes?