From 502e9af9ce732766d647018be856322c893b0c88 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Thu, 27 Jun 2024 16:43:34 +0200 Subject: [PATCH 01/15] Add bimi in FeatureFlag enum class --- app/src/main/java/com/infomaniak/mail/data/models/FeatureFlag.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/app/src/main/java/com/infomaniak/mail/data/models/FeatureFlag.kt b/app/src/main/java/com/infomaniak/mail/data/models/FeatureFlag.kt index 7472218efd..48cb42bebd 100644 --- a/app/src/main/java/com/infomaniak/mail/data/models/FeatureFlag.kt +++ b/app/src/main/java/com/infomaniak/mail/data/models/FeatureFlag.kt @@ -20,4 +20,5 @@ package com.infomaniak.mail.data.models // The field apiName is also used to store the enum in Realm enum class FeatureFlag(val apiName: String) { AI("ai-mail-composer"), + BIMI("bimi"), } From 17a163b12fc7118e756dd681e3a62af51383cd37 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Mon, 1 Jul 2024 14:43:45 +0200 Subject: [PATCH 02/15] implement feature flag --- .../infomaniak/mail/data/api/ApiRepository.kt | 4 +- .../com/infomaniak/mail/data/api/ApiRoutes.kt | 4 +- .../com/infomaniak/mail/data/models/Bimi.kt | 2 +- .../mail/data/models/mailbox/Mailbox.kt | 9 ++-- .../com/infomaniak/mail/ui/MainViewModel.kt | 2 +- .../mail/ui/newMessage/AiViewModel.kt | 2 +- .../com/infomaniak/mail/utils/SharedUtils.kt | 14 +++-- .../com/infomaniak/mail/views/AvatarView.kt | 51 ++++++++++++------- .../itemViews/AvatarMergedContactData.kt | 14 +++++ 9 files changed, 67 insertions(+), 35 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt b/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt index 3558ae0a7b..827f3fd66a 100644 --- a/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt +++ b/app/src/main/java/com/infomaniak/mail/data/api/ApiRepository.kt @@ -364,8 +364,8 @@ object ApiRepository : ApiRepositoryCore() { "output" to "mail", ) - fun checkFeatureFlag(featureFlag: FeatureFlag, currentMailboxUuid: String): ApiResponse> { - return callApi(ApiRoutes.featureFlag(featureFlag.apiName, currentMailboxUuid), GET) + fun getFeatureFlags(currentMailboxUuid: String): ApiResponse> { + return callApi(ApiRoutes.featureFlags(currentMailboxUuid), GET) } fun getCredentialsPassword(): ApiResponse = runCatching { diff --git a/app/src/main/java/com/infomaniak/mail/data/api/ApiRoutes.kt b/app/src/main/java/com/infomaniak/mail/data/api/ApiRoutes.kt index 82b523508b..9ba43adb16 100644 --- a/app/src/main/java/com/infomaniak/mail/data/api/ApiRoutes.kt +++ b/app/src/main/java/com/infomaniak/mail/data/api/ApiRoutes.kt @@ -250,8 +250,8 @@ object ApiRoutes { return "?mailbox_uuid=$mailboxUuid" } - fun featureFlag(featureName: String, mailboxUuid: String): String { - return "$MAIL_API/api/feature-flag/check/${featureName}${mailboxUuidParameter(mailboxUuid)}" + fun featureFlags(mailboxUuid: String): String { + return "$MAIL_API/api/feature-flag/check${mailboxUuidParameter(mailboxUuid)}" } fun ping(): String { diff --git a/app/src/main/java/com/infomaniak/mail/data/models/Bimi.kt b/app/src/main/java/com/infomaniak/mail/data/models/Bimi.kt index 314dd9c166..09d1a0838a 100644 --- a/app/src/main/java/com/infomaniak/mail/data/models/Bimi.kt +++ b/app/src/main/java/com/infomaniak/mail/data/models/Bimi.kt @@ -41,7 +41,7 @@ class Bimi() : EmbeddedRealmObject, Parcelable { this.isCertified = isCertified } - fun isDisplayable(): Boolean = isCertified && svgContentUrl?.isNotEmpty() == true + fun isDisplayable(isBimiEnabled: Boolean): Boolean = isBimiEnabled && isCertified && svgContentUrl?.isNotEmpty() == true companion object : Parceler { diff --git a/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt b/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt index e4c464654f..77b934f8c2 100644 --- a/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt +++ b/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt @@ -124,12 +124,9 @@ class Mailbox : RealmObject { return _featureFlags.contains(featureFlag.apiName) } - fun add(featureFlag: FeatureFlag): Boolean { - return _featureFlags.add(featureFlag.apiName) - } - - fun remove(featureFlag: FeatureFlag): Boolean { - return _featureFlags.remove(featureFlag.apiName) + fun overrideFeatureFlags(featureFlags: List) { + _featureFlags.clear() + _featureFlags.addAll(featureFlags) } } } diff --git a/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt b/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt index 8eb2eecb6f..34c0e2859a 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt @@ -345,7 +345,7 @@ class MainViewModel @Inject constructor( private fun updateFeatureFlag(mailbox: Mailbox) = viewModelScope.launch(ioCoroutineContext) { SentryLog.d(TAG, "Force refresh Features flags") - sharedUtils.updateAiFeatureFlag(mailbox.objectId, mailbox.uuid) + sharedUtils.updateFeatureFlags(mailbox.objectId, mailbox.uuid) } private fun updateExternalMailInfo(mailbox: Mailbox) = viewModelScope.launch(ioCoroutineContext) { diff --git a/app/src/main/java/com/infomaniak/mail/ui/newMessage/AiViewModel.kt b/app/src/main/java/com/infomaniak/mail/ui/newMessage/AiViewModel.kt index ec5122ce29..e1c08edcb0 100644 --- a/app/src/main/java/com/infomaniak/mail/ui/newMessage/AiViewModel.kt +++ b/app/src/main/java/com/infomaniak/mail/ui/newMessage/AiViewModel.kt @@ -136,7 +136,7 @@ class AiViewModel @Inject constructor( } fun updateFeatureFlag(mailboxObjectId: String, mailboxUuid: String) = viewModelScope.launch(ioCoroutineContext) { - sharedUtils.updateAiFeatureFlag(mailboxObjectId, mailboxUuid) + sharedUtils.updateFeatureFlags(mailboxObjectId, mailboxUuid) } fun isHistoryEmpty(): Boolean = history.excludingContextMessage().isEmpty() diff --git a/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt b/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt index 16c90239e9..1bdbcf65b7 100644 --- a/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt +++ b/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt @@ -38,6 +38,7 @@ import com.infomaniak.mail.ui.main.settings.SettingRadioGroupView import com.infomaniak.mail.utils.extensions.getApiException import com.infomaniak.mail.utils.extensions.getFoldersIds import com.infomaniak.mail.utils.extensions.getUids +import com.infomaniak.mail.views.itemViews.AvatarMergedContactData import io.realm.kotlin.Realm import io.sentry.Sentry import org.jsoup.Jsoup @@ -49,6 +50,7 @@ class SharedUtils @Inject constructor( private val refreshController: RefreshController, private val messageController: MessageController, private val mailboxController: MailboxController, + private val avatarMergedContactData: AvatarMergedContactData, ) { @Inject @@ -118,12 +120,14 @@ class SharedUtils @Inject constructor( } } - fun updateAiFeatureFlag(mailboxObjectId: String, mailboxUuid: String) { - with(ApiRepository.checkFeatureFlag(FeatureFlag.AI, mailboxUuid)) { + fun updateFeatureFlags( + mailboxObjectId: String, + mailboxUuid: String, + ) { + with(ApiRepository.getFeatureFlags(mailboxUuid)) { if (isSuccess()) { - val isEnabled = data?.get("is_enabled") == true - mailboxController.updateMailbox(mailboxObjectId) { - if (isEnabled) it.featureFlags.add(FeatureFlag.AI) else it.featureFlags.remove(FeatureFlag.AI) + mailboxController.updateMailbox(mailboxObjectId) { mailbox -> + mailbox.featureFlags.overrideFeatureFlags(featureFlags = data ?: emptyList()) } } } diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index b6c298bfb3..6abdb66099 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -40,6 +40,7 @@ import com.infomaniak.mail.data.models.correspondent.Correspondent import com.infomaniak.mail.data.models.correspondent.MergedContact import com.infomaniak.mail.databinding.ViewAvatarBinding import com.infomaniak.mail.utils.AccountUtils +import com.infomaniak.mail.utils.Utils import com.infomaniak.mail.utils.extensions.MergedContactDictionary import com.infomaniak.mail.utils.extensions.getColorOrNull import com.infomaniak.mail.utils.extensions.getTransparentColor @@ -60,14 +61,19 @@ class AvatarView @JvmOverloads constructor( private var currentCorrespondent: Correspondent? = null private var currentBimi: Bimi? = null - private val mergedContactObserver = Observer { contacts -> - val displayType = getAvatarDisplayType(currentCorrespondent, currentBimi) + private val liveData = Utils.waitInitMediator( + avatarMergedContactData.mergedContactLiveData, + avatarMergedContactData.isBimiEnabledLiveData, + ) - if (displayType == AvatarDisplayType.CUSTOM_AVATAR || displayType == AvatarDisplayType.INITIALS) { - loadAvatarUsingDictionary(currentCorrespondent!!, contacts) - } + private val mediatorObserver = Observer> { (contacts, isBimiEnabled) -> + val displayType = getAvatarDisplayType(currentCorrespondent, currentBimi, isBimiEnabled) + if (displayType == AvatarDisplayType.UNKNOWN_CORRESPONDENT) return@Observer + + handleDisplayType(displayType, currentCorrespondent, currentBimi, contacts) } + @Inject lateinit var avatarMergedContactData: AvatarMergedContactData @@ -111,13 +117,15 @@ class AvatarView @JvmOverloads constructor( override fun onAttachedToWindow() { super.onAttachedToWindow() if (isInEditMode) return // Avoid lateinit property has not been initialized in preview - avatarMergedContactData.mergedContactLiveData.observeForever(mergedContactObserver) + + liveData.observeForever(mediatorObserver) } override fun onDetachedFromWindow() { super.onDetachedFromWindow() if (isInEditMode) return // Avoid lateinit property has not been initialized in preview - avatarMergedContactData.mergedContactLiveData.removeObserver(mergedContactObserver) + + liveData.removeObserver(mediatorObserver) } override fun setOnClickListener(onClickListener: OnClickListener?) = binding.root.setOnClickListener(onClickListener) @@ -138,27 +146,36 @@ class AvatarView @JvmOverloads constructor( } fun loadAvatar(correspondent: Correspondent?, bimi: Bimi? = null) { + currentBimi = bimi - fun loadSimpleCorrespondent(correspondent: Correspondent) { - loadAvatarUsingDictionary(correspondent, contacts = contactsFromViewModel) - currentCorrespondent = correspondent - } + val isBimiEnabled = avatarMergedContactData.isBimiEnabledLiveData.value ?: false + val avatarDisplayType = getAvatarDisplayType(correspondent, bimi, isBimiEnabled) - currentBimi = bimi + handleDisplayType(avatarDisplayType, correspondent, bimi, contactsFromViewModel) + } - when (getAvatarDisplayType(correspondent, bimi)) { + private fun handleDisplayType( + avatarDisplayType: AvatarDisplayType, + correspondent: Correspondent?, + bimi: Bimi?, + contacts: MergedContactDictionary, + ) { + when (avatarDisplayType) { AvatarDisplayType.UNKNOWN_CORRESPONDENT -> loadUnknownUserAvatar() - AvatarDisplayType.CUSTOM_AVATAR -> loadSimpleCorrespondent(correspondent!!) + AvatarDisplayType.CUSTOM_AVATAR, + AvatarDisplayType.INITIALS -> { + loadAvatarUsingDictionary(correspondent!!, contacts) + currentCorrespondent = correspondent + } AvatarDisplayType.BIMI -> loadBimiAvatar(ApiRoutes.bimi(bimi!!.svgContentUrl!!), correspondent!!) - AvatarDisplayType.INITIALS -> loadSimpleCorrespondent(correspondent!!) } } - private fun getAvatarDisplayType(correspondent: Correspondent?, bimi: Bimi?): AvatarDisplayType { + private fun getAvatarDisplayType(correspondent: Correspondent?, bimi: Bimi?, isBimiEnabled: Boolean): AvatarDisplayType { return when { correspondent == null -> AvatarDisplayType.UNKNOWN_CORRESPONDENT correspondent.hasMergedContactAvatar(contactsFromViewModel) -> AvatarDisplayType.CUSTOM_AVATAR - bimi?.isDisplayable() == true -> AvatarDisplayType.BIMI + bimi?.isDisplayable(isBimiEnabled) == true -> AvatarDisplayType.BIMI else -> AvatarDisplayType.INITIALS } } diff --git a/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt b/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt index 486405db6a..cafa121dd9 100644 --- a/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt +++ b/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt @@ -18,14 +18,18 @@ package com.infomaniak.mail.views.itemViews import androidx.lifecycle.asLiveData +import com.infomaniak.mail.data.cache.mailboxInfo.MailboxController import com.infomaniak.mail.data.cache.userInfo.MergedContactController +import com.infomaniak.mail.data.models.FeatureFlag import com.infomaniak.mail.di.IoDispatcher +import com.infomaniak.mail.utils.AccountUtils import com.infomaniak.mail.utils.ContactUtils import com.infomaniak.mail.utils.coroutineContext import io.realm.kotlin.ext.copyFromRealm import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.mapLatest import javax.inject.Inject import javax.inject.Singleton @@ -34,6 +38,7 @@ import javax.inject.Singleton @Singleton class AvatarMergedContactData @Inject constructor( mergedContactController: MergedContactController, + mailboxController: MailboxController, globalCoroutineScope: CoroutineScope, @IoDispatcher ioDispatcher: CoroutineDispatcher, ) { @@ -43,4 +48,13 @@ class AvatarMergedContactData @Inject constructor( .getMergedContactsAsync() .mapLatest { ContactUtils.arrangeMergedContacts(it.list.copyFromRealm()) } .asLiveData(ioCoroutineContext) + + val isBimiEnabledLiveData = mailboxController + .getMailboxAsync(AccountUtils.currentUserId, AccountUtils.currentMailboxId) + .mapLatest { + val mailbox = it.obj + mailbox?.featureFlags?.contains(FeatureFlag.BIMI) + } + .filterNotNull() + .asLiveData(ioCoroutineContext) } From d63b96fbbc6b9bb5054025a420b221b2f28043dd Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Mon, 1 Jul 2024 14:45:35 +0200 Subject: [PATCH 03/15] move function after all loading avatar function --- .../com/infomaniak/mail/views/AvatarView.kt | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index 6abdb66099..b03cf81cba 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -154,6 +154,30 @@ class AvatarView @JvmOverloads constructor( handleDisplayType(avatarDisplayType, correspondent, bimi, contactsFromViewModel) } + fun loadAvatar(mergedContact: MergedContact) { + binding.avatarImage.baseLoadAvatar(mergedContact) + } + + fun loadUnknownUserAvatar() { + currentCorrespondent = null + binding.avatarImage.load(R.drawable.ic_unknown_user_avatar) + } + + private fun loadBimiAvatar(bimiUrl: String, correspondent: Correspondent) = with(binding.avatarImage) { + contentDescription = correspondent.email + currentCorrespondent = null + loadAvatar( + backgroundColor = context.getBackgroundColorBasedOnId( + correspondent.email.hashCode(), + R.array.AvatarColors, + ), + avatarUrl = bimiUrl, + initials = correspondent.initials, + imageLoader = svgImageLoader, + initialsColor = context.getColor(R.color.onColorfulBackground), + ) + } + private fun handleDisplayType( avatarDisplayType: AvatarDisplayType, correspondent: Correspondent?, @@ -180,30 +204,6 @@ class AvatarView @JvmOverloads constructor( } } - fun loadAvatar(mergedContact: MergedContact) { - binding.avatarImage.baseLoadAvatar(mergedContact) - } - - fun loadUnknownUserAvatar() { - currentCorrespondent = null - binding.avatarImage.load(R.drawable.ic_unknown_user_avatar) - } - - private fun loadBimiAvatar(bimiUrl: String, correspondent: Correspondent) = with(binding.avatarImage) { - contentDescription = correspondent.email - currentCorrespondent = null - loadAvatar( - backgroundColor = context.getBackgroundColorBasedOnId( - correspondent.email.hashCode(), - R.array.AvatarColors, - ), - avatarUrl = bimiUrl, - initials = correspondent.initials, - imageLoader = svgImageLoader, - initialsColor = context.getColor(R.color.onColorfulBackground), - ) - } - fun setImageDrawable(drawable: Drawable?) = binding.avatarImage.setImageDrawable(drawable) private fun searchInMergedContact(correspondent: Correspondent, contacts: MergedContactDictionary): MergedContact? { From 0ca7a401f5ce359c9c4636b52af353cea0a92d92 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Mon, 1 Jul 2024 15:32:22 +0200 Subject: [PATCH 04/15] Rename mediator and add comment to better comprehension --- app/src/main/java/com/infomaniak/mail/views/AvatarView.kt | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index b03cf81cba..cc1c36ac98 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -61,7 +61,8 @@ class AvatarView @JvmOverloads constructor( private var currentCorrespondent: Correspondent? = null private var currentBimi: Bimi? = null - private val liveData = Utils.waitInitMediator( + // We use waitInitMediator over MediatorLiveData because we know both live data will be initialized very quickly anyway + private val avatarMediatorLiveData = Utils.waitInitMediator( avatarMergedContactData.mergedContactLiveData, avatarMergedContactData.isBimiEnabledLiveData, ) @@ -118,14 +119,14 @@ class AvatarView @JvmOverloads constructor( super.onAttachedToWindow() if (isInEditMode) return // Avoid lateinit property has not been initialized in preview - liveData.observeForever(mediatorObserver) + avatarMediatorLiveData.observeForever(mediatorObserver) } override fun onDetachedFromWindow() { super.onDetachedFromWindow() if (isInEditMode) return // Avoid lateinit property has not been initialized in preview - liveData.removeObserver(mediatorObserver) + avatarMediatorLiveData.removeObserver(mediatorObserver) } override fun setOnClickListener(onClickListener: OnClickListener?) = binding.root.setOnClickListener(onClickListener) From 5d92db98ff6fb54ebc83e6da7368fa589dae268e Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 08:32:01 +0200 Subject: [PATCH 05/15] removed unused value and import --- app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt b/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt index 1bdbcf65b7..b090eb71e1 100644 --- a/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt +++ b/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt @@ -30,7 +30,6 @@ import com.infomaniak.mail.data.cache.mailboxContent.RefreshController.RefreshCa import com.infomaniak.mail.data.cache.mailboxContent.RefreshController.RefreshMode import com.infomaniak.mail.data.cache.mailboxContent.SignatureController import com.infomaniak.mail.data.cache.mailboxInfo.MailboxController -import com.infomaniak.mail.data.models.FeatureFlag import com.infomaniak.mail.data.models.mailbox.Mailbox import com.infomaniak.mail.data.models.message.Message import com.infomaniak.mail.data.models.thread.Thread @@ -38,7 +37,6 @@ import com.infomaniak.mail.ui.main.settings.SettingRadioGroupView import com.infomaniak.mail.utils.extensions.getApiException import com.infomaniak.mail.utils.extensions.getFoldersIds import com.infomaniak.mail.utils.extensions.getUids -import com.infomaniak.mail.views.itemViews.AvatarMergedContactData import io.realm.kotlin.Realm import io.sentry.Sentry import org.jsoup.Jsoup @@ -50,7 +48,6 @@ class SharedUtils @Inject constructor( private val refreshController: RefreshController, private val messageController: MessageController, private val mailboxController: MailboxController, - private val avatarMergedContactData: AvatarMergedContactData, ) { @Inject From ef00ba08795cc7a6574337469641d250c9caee62 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 09:14:09 +0200 Subject: [PATCH 06/15] Correctly save internal avatar state Avoid missing correspondant when Bimi is deactivated --- app/src/main/java/com/infomaniak/mail/views/AvatarView.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index cc1c36ac98..a0dea3cfd2 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -166,7 +166,7 @@ class AvatarView @JvmOverloads constructor( private fun loadBimiAvatar(bimiUrl: String, correspondent: Correspondent) = with(binding.avatarImage) { contentDescription = correspondent.email - currentCorrespondent = null + currentCorrespondent = correspondent loadAvatar( backgroundColor = context.getBackgroundColorBasedOnId( correspondent.email.hashCode(), From b31ab4645cc63f7b7e491925236022299f396b52 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 09:57:01 +0200 Subject: [PATCH 07/15] Add internal state to define avatar view state --- .../com/infomaniak/mail/views/AvatarView.kt | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index a0dea3cfd2..874bac3a24 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -58,8 +58,7 @@ class AvatarView @JvmOverloads constructor( private val binding by lazy { ViewAvatarBinding.inflate(LayoutInflater.from(context), this, true) } - private var currentCorrespondent: Correspondent? = null - private var currentBimi: Bimi? = null + private var internalState = InternalState(null, null) // We use waitInitMediator over MediatorLiveData because we know both live data will be initialized very quickly anyway private val avatarMediatorLiveData = Utils.waitInitMediator( @@ -68,10 +67,11 @@ class AvatarView @JvmOverloads constructor( ) private val mediatorObserver = Observer> { (contacts, isBimiEnabled) -> - val displayType = getAvatarDisplayType(currentCorrespondent, currentBimi, isBimiEnabled) - if (displayType == AvatarDisplayType.UNKNOWN_CORRESPONDENT) return@Observer + val (correspondent, bimi) = internalState + val displayType = getAvatarDisplayType(correspondent, bimi, isBimiEnabled) - handleDisplayType(displayType, currentCorrespondent, currentBimi, contacts) + if (displayType == AvatarDisplayType.UNKNOWN_CORRESPONDENT) return@Observer + handleDisplayType(displayType, correspondent, bimi, contacts) } @@ -147,7 +147,6 @@ class AvatarView @JvmOverloads constructor( } fun loadAvatar(correspondent: Correspondent?, bimi: Bimi? = null) { - currentBimi = bimi val isBimiEnabled = avatarMergedContactData.isBimiEnabledLiveData.value ?: false val avatarDisplayType = getAvatarDisplayType(correspondent, bimi, isBimiEnabled) @@ -160,19 +159,19 @@ class AvatarView @JvmOverloads constructor( } fun loadUnknownUserAvatar() { - currentCorrespondent = null + internalState.setInternalState(null, null) binding.avatarImage.load(R.drawable.ic_unknown_user_avatar) } - private fun loadBimiAvatar(bimiUrl: String, correspondent: Correspondent) = with(binding.avatarImage) { + private fun loadBimiAvatar(bimi: Bimi, correspondent: Correspondent) = with(binding.avatarImage) { + internalState.setInternalState(correspondent, bimi) contentDescription = correspondent.email - currentCorrespondent = correspondent loadAvatar( backgroundColor = context.getBackgroundColorBasedOnId( correspondent.email.hashCode(), R.array.AvatarColors, ), - avatarUrl = bimiUrl, + avatarUrl = ApiRoutes.bimi(bimi.svgContentUrl!!), initials = correspondent.initials, imageLoader = svgImageLoader, initialsColor = context.getColor(R.color.onColorfulBackground), @@ -188,11 +187,8 @@ class AvatarView @JvmOverloads constructor( when (avatarDisplayType) { AvatarDisplayType.UNKNOWN_CORRESPONDENT -> loadUnknownUserAvatar() AvatarDisplayType.CUSTOM_AVATAR, - AvatarDisplayType.INITIALS -> { - loadAvatarUsingDictionary(correspondent!!, contacts) - currentCorrespondent = correspondent - } - AvatarDisplayType.BIMI -> loadBimiAvatar(ApiRoutes.bimi(bimi!!.svgContentUrl!!), correspondent!!) + AvatarDisplayType.INITIALS -> loadAvatarUsingDictionary(correspondent!!, contacts, bimi) + AvatarDisplayType.BIMI -> loadBimiAvatar(bimi!!, correspondent!!) } } @@ -216,7 +212,8 @@ class AvatarView @JvmOverloads constructor( return searchInMergedContact(correspondent = this, contacts)?.avatar != null } - private fun loadAvatarUsingDictionary(correspondent: Correspondent, contacts: MergedContactDictionary) { + private fun loadAvatarUsingDictionary(correspondent: Correspondent, contacts: MergedContactDictionary, bimi: Bimi?) { + internalState.setInternalState(correspondent, bimi) val mergedContact = searchInMergedContact(correspondent, contacts) binding.avatarImage.baseLoadAvatar(correspondent = mergedContact ?: correspondent) } @@ -237,6 +234,13 @@ class AvatarView @JvmOverloads constructor( } } + private data class InternalState(var correspondent: Correspondent?, var bimi: Bimi?) { + fun setInternalState(correspondent: Correspondent?, bimi: Bimi?) { + this.correspondent = correspondent + this.bimi = bimi + } + } + enum class AvatarDisplayType { UNKNOWN_CORRESPONDENT, CUSTOM_AVATAR, From 82b31c954eb3e70cf2e668eff15ede6802a2a30d Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 09:59:15 +0200 Subject: [PATCH 08/15] Avoid live data to be notify for the same value --- .../infomaniak/mail/views/itemViews/AvatarMergedContactData.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt b/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt index cafa121dd9..3320d7cec1 100644 --- a/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt +++ b/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt @@ -29,6 +29,7 @@ import io.realm.kotlin.ext.copyFromRealm import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.mapLatest import javax.inject.Inject @@ -56,5 +57,6 @@ class AvatarMergedContactData @Inject constructor( mailbox?.featureFlags?.contains(FeatureFlag.BIMI) } .filterNotNull() + .distinctUntilChanged() .asLiveData(ioCoroutineContext) } From 5055ea7d1a47d4236c6da2e588bd0baf5e9208bc Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 10:17:59 +0200 Subject: [PATCH 09/15] Apply suggestion from code review --- .../mail/data/models/mailbox/Mailbox.kt | 6 +++--- .../com/infomaniak/mail/utils/SharedUtils.kt | 7 ++----- .../java/com/infomaniak/mail/views/AvatarView.kt | 16 ++++++++-------- .../views/itemViews/AvatarMergedContactData.kt | 5 +---- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt b/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt index 77b934f8c2..8abe33d21c 100644 --- a/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt +++ b/app/src/main/java/com/infomaniak/mail/data/models/mailbox/Mailbox.kt @@ -124,9 +124,9 @@ class Mailbox : RealmObject { return _featureFlags.contains(featureFlag.apiName) } - fun overrideFeatureFlags(featureFlags: List) { - _featureFlags.clear() - _featureFlags.addAll(featureFlags) + fun setFeatureFlags(featureFlags: List) = with(_featureFlags){ + clear() + addAll(featureFlags) } } } diff --git a/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt b/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt index b090eb71e1..686e2c77ad 100644 --- a/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt +++ b/app/src/main/java/com/infomaniak/mail/utils/SharedUtils.kt @@ -117,14 +117,11 @@ class SharedUtils @Inject constructor( } } - fun updateFeatureFlags( - mailboxObjectId: String, - mailboxUuid: String, - ) { + fun updateFeatureFlags(mailboxObjectId: String, mailboxUuid: String) { with(ApiRepository.getFeatureFlags(mailboxUuid)) { if (isSuccess()) { mailboxController.updateMailbox(mailboxObjectId) { mailbox -> - mailbox.featureFlags.overrideFeatureFlags(featureFlags = data ?: emptyList()) + mailbox.featureFlags.setFeatureFlags(featureFlags = data ?: emptyList()) } } } diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index 874bac3a24..094ae2710a 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -66,12 +66,12 @@ class AvatarView @JvmOverloads constructor( avatarMergedContactData.isBimiEnabledLiveData, ) - private val mediatorObserver = Observer> { (contacts, isBimiEnabled) -> + private val avatarUpdateObserver = Observer> { (contacts, isBimiEnabled) -> val (correspondent, bimi) = internalState val displayType = getAvatarDisplayType(correspondent, bimi, isBimiEnabled) if (displayType == AvatarDisplayType.UNKNOWN_CORRESPONDENT) return@Observer - handleDisplayType(displayType, correspondent, bimi, contacts) + loadAvatarByDisplayType(displayType, correspondent, bimi, contacts) } @@ -84,6 +84,8 @@ class AvatarView @JvmOverloads constructor( return if (isInEditMode) emptyMap() else avatarMergedContactData.mergedContactLiveData.value ?: emptyMap() } + private val isBimiEnabled: Boolean get() = !isInEditMode && avatarMergedContactData.isBimiEnabledLiveData.value == true + @Inject lateinit var svgImageLoader: ImageLoader @@ -119,14 +121,14 @@ class AvatarView @JvmOverloads constructor( super.onAttachedToWindow() if (isInEditMode) return // Avoid lateinit property has not been initialized in preview - avatarMediatorLiveData.observeForever(mediatorObserver) + avatarMediatorLiveData.observeForever(avatarUpdateObserver) } override fun onDetachedFromWindow() { super.onDetachedFromWindow() if (isInEditMode) return // Avoid lateinit property has not been initialized in preview - avatarMediatorLiveData.removeObserver(mediatorObserver) + avatarMediatorLiveData.removeObserver(avatarUpdateObserver) } override fun setOnClickListener(onClickListener: OnClickListener?) = binding.root.setOnClickListener(onClickListener) @@ -147,11 +149,9 @@ class AvatarView @JvmOverloads constructor( } fun loadAvatar(correspondent: Correspondent?, bimi: Bimi? = null) { - - val isBimiEnabled = avatarMergedContactData.isBimiEnabledLiveData.value ?: false val avatarDisplayType = getAvatarDisplayType(correspondent, bimi, isBimiEnabled) - handleDisplayType(avatarDisplayType, correspondent, bimi, contactsFromViewModel) + loadAvatarByDisplayType(avatarDisplayType, correspondent, bimi, contactsFromViewModel) } fun loadAvatar(mergedContact: MergedContact) { @@ -178,7 +178,7 @@ class AvatarView @JvmOverloads constructor( ) } - private fun handleDisplayType( + private fun loadAvatarByDisplayType( avatarDisplayType: AvatarDisplayType, correspondent: Correspondent?, bimi: Bimi?, diff --git a/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt b/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt index 3320d7cec1..d64a67c941 100644 --- a/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt +++ b/app/src/main/java/com/infomaniak/mail/views/itemViews/AvatarMergedContactData.kt @@ -52,10 +52,7 @@ class AvatarMergedContactData @Inject constructor( val isBimiEnabledLiveData = mailboxController .getMailboxAsync(AccountUtils.currentUserId, AccountUtils.currentMailboxId) - .mapLatest { - val mailbox = it.obj - mailbox?.featureFlags?.contains(FeatureFlag.BIMI) - } + .mapLatest { it.obj?.featureFlags?.contains(FeatureFlag.BIMI) } .filterNotNull() .distinctUntilChanged() .asLiveData(ioCoroutineContext) From 3aa1c22daeff4429124c5ec4fc4d521a23895d54 Mon Sep 17 00:00:00 2001 From: Kevin Boulongne Date: Tue, 2 Jul 2024 11:40:23 +0200 Subject: [PATCH 10/15] Rewrite AvatarView `InternalState` class --- .../com/infomaniak/mail/views/AvatarView.kt | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index 094ae2710a..1e802b89c2 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -58,7 +58,7 @@ class AvatarView @JvmOverloads constructor( private val binding by lazy { ViewAvatarBinding.inflate(LayoutInflater.from(context), this, true) } - private var internalState = InternalState(null, null) + private var state = State() // We use waitInitMediator over MediatorLiveData because we know both live data will be initialized very quickly anyway private val avatarMediatorLiveData = Utils.waitInitMediator( @@ -67,14 +67,13 @@ class AvatarView @JvmOverloads constructor( ) private val avatarUpdateObserver = Observer> { (contacts, isBimiEnabled) -> - val (correspondent, bimi) = internalState + val (correspondent, bimi) = state val displayType = getAvatarDisplayType(correspondent, bimi, isBimiEnabled) if (displayType == AvatarDisplayType.UNKNOWN_CORRESPONDENT) return@Observer loadAvatarByDisplayType(displayType, correspondent, bimi, contacts) } - @Inject lateinit var avatarMergedContactData: AvatarMergedContactData @@ -159,12 +158,12 @@ class AvatarView @JvmOverloads constructor( } fun loadUnknownUserAvatar() { - internalState.setInternalState(null, null) + state.update(correspondent = null, bimi = null) binding.avatarImage.load(R.drawable.ic_unknown_user_avatar) } private fun loadBimiAvatar(bimi: Bimi, correspondent: Correspondent) = with(binding.avatarImage) { - internalState.setInternalState(correspondent, bimi) + state.update(correspondent, bimi) contentDescription = correspondent.email loadAvatar( backgroundColor = context.getBackgroundColorBasedOnId( @@ -213,7 +212,7 @@ class AvatarView @JvmOverloads constructor( } private fun loadAvatarUsingDictionary(correspondent: Correspondent, contacts: MergedContactDictionary, bimi: Bimi?) { - internalState.setInternalState(correspondent, bimi) + state.update(correspondent, bimi) val mergedContact = searchInMergedContact(correspondent, contacts) binding.avatarImage.baseLoadAvatar(correspondent = mergedContact ?: correspondent) } @@ -234,8 +233,11 @@ class AvatarView @JvmOverloads constructor( } } - private data class InternalState(var correspondent: Correspondent?, var bimi: Bimi?) { - fun setInternalState(correspondent: Correspondent?, bimi: Bimi?) { + private data class State( + var correspondent: Correspondent? = null, + var bimi: Bimi? = null, + ) { + fun update(correspondent: Correspondent?, bimi: Bimi?) { this.correspondent = correspondent this.bimi = bimi } From 339664459b525fb037b1f0bc80c9c1ddb6c37f03 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 12:42:36 +0200 Subject: [PATCH 11/15] Add edit mode --- .../java/com/infomaniak/mail/views/AvatarView.kt | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index 1e802b89c2..ef4b491242 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -24,6 +24,7 @@ import android.util.AttributeSet import android.view.LayoutInflater import android.widget.FrameLayout import android.widget.ImageView +import androidx.lifecycle.MutableLiveData import androidx.lifecycle.Observer import coil.ImageLoader import coil.imageLoader @@ -61,10 +62,16 @@ class AvatarView @JvmOverloads constructor( private var state = State() // We use waitInitMediator over MediatorLiveData because we know both live data will be initialized very quickly anyway - private val avatarMediatorLiveData = Utils.waitInitMediator( - avatarMergedContactData.mergedContactLiveData, - avatarMergedContactData.isBimiEnabledLiveData, - ) + private val avatarMediatorLiveData = + if (isInEditMode) { + MutableLiveData>(null) + } else { + Utils.waitInitMediator( + avatarMergedContactData.mergedContactLiveData, + avatarMergedContactData.isBimiEnabledLiveData, + ) + } + private val avatarUpdateObserver = Observer> { (contacts, isBimiEnabled) -> val (correspondent, bimi) = state From fefe750a0bee203f5538a5a1f7eae4f492a591a1 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 12:50:29 +0200 Subject: [PATCH 12/15] Apply suggestion from code review --- app/src/main/java/com/infomaniak/mail/views/AvatarView.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index ef4b491242..58a3732056 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -59,7 +59,7 @@ class AvatarView @JvmOverloads constructor( private val binding by lazy { ViewAvatarBinding.inflate(LayoutInflater.from(context), this, true) } - private var state = State() + private val state = State() // We use waitInitMediator over MediatorLiveData because we know both live data will be initialized very quickly anyway private val avatarMediatorLiveData = From d3ee2d233693ea214a0fa533c278bb9534cedb56 Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Tue, 2 Jul 2024 12:59:11 +0200 Subject: [PATCH 13/15] Better definition of avatarMediatorLiveData --- app/src/main/java/com/infomaniak/mail/views/AvatarView.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index 58a3732056..1f9b44f2fb 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -62,9 +62,9 @@ class AvatarView @JvmOverloads constructor( private val state = State() // We use waitInitMediator over MediatorLiveData because we know both live data will be initialized very quickly anyway - private val avatarMediatorLiveData = + private val avatarMediatorLiveData: MutableLiveData> = if (isInEditMode) { - MutableLiveData>(null) + MutableLiveData() } else { Utils.waitInitMediator( avatarMergedContactData.mergedContactLiveData, From b8aedb16bbc506c3ea9fa7ed3b43b95451c7e95d Mon Sep 17 00:00:00 2001 From: Nicolas Bourdin <167090505+NicolasBourdin88@users.noreply.github.com> Date: Tue, 2 Jul 2024 13:05:13 +0200 Subject: [PATCH 14/15] Update app/src/main/java/com/infomaniak/mail/views/AvatarView.kt Co-authored-by: Kevin Boulongne <1788629+KevinBoulongne@users.noreply.github.com> --- app/src/main/java/com/infomaniak/mail/views/AvatarView.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index 1f9b44f2fb..407619e463 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -72,7 +72,6 @@ class AvatarView @JvmOverloads constructor( ) } - private val avatarUpdateObserver = Observer> { (contacts, isBimiEnabled) -> val (correspondent, bimi) = state val displayType = getAvatarDisplayType(correspondent, bimi, isBimiEnabled) From ecbc35b5d6e9b9e0f2ecda586e6936ccb2d491cd Mon Sep 17 00:00:00 2001 From: NicolasBourdin88 Date: Wed, 3 Jul 2024 09:10:41 +0200 Subject: [PATCH 15/15] Use LiveData instead of MutableLiveData --- app/src/main/java/com/infomaniak/mail/views/AvatarView.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt index 407619e463..eac59b3375 100644 --- a/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt +++ b/app/src/main/java/com/infomaniak/mail/views/AvatarView.kt @@ -24,6 +24,7 @@ import android.util.AttributeSet import android.view.LayoutInflater import android.widget.FrameLayout import android.widget.ImageView +import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import androidx.lifecycle.Observer import coil.ImageLoader @@ -62,14 +63,11 @@ class AvatarView @JvmOverloads constructor( private val state = State() // We use waitInitMediator over MediatorLiveData because we know both live data will be initialized very quickly anyway - private val avatarMediatorLiveData: MutableLiveData> = + private val avatarMediatorLiveData: LiveData> = if (isInEditMode) { MutableLiveData() } else { - Utils.waitInitMediator( - avatarMergedContactData.mergedContactLiveData, - avatarMergedContactData.isBimiEnabledLiveData, - ) + Utils.waitInitMediator(avatarMergedContactData.mergedContactLiveData, avatarMergedContactData.isBimiEnabledLiveData) } private val avatarUpdateObserver = Observer> { (contacts, isBimiEnabled) ->