Merge pull request #1731 from Infomaniak/optimize-print

Optimize opening of Thread to handle both ThreadFragment & PrintFragment

Author: KevinBoulongne

.idea/navEditor.xml

@@ -44,7 +44,6 @@ class MessageController @Inject constructor(private val mailboxContentRealm: Rea
             ?.messages?.query("${Message::isDeletedOnApi.name} == false")
             ?.sort(Message::date.name, Sort.ASCENDING)
     }
-
     //endregion
 
     //region Get data
@@ -134,6 +133,10 @@ class MessageController @Inject constructor(private val mailboxContentRealm: Rea
     fun getSortedAndNotDeletedMessagesAsync(threadUid: String): Flow<ResultsChange<Message>>? {
         return getSortedAndNotDeletedMessagesQuery(threadUid)?.asFlow()
     }
+
+    fun getMessagesAsync(messageUid: String): Flow<ResultsChange<Message>> {
+        return getMessagesQuery(messageUid, mailboxContentRealm()).asFlow()
+    }
     //endregion
 
     //region Edit data
@@ -162,8 +165,12 @@ class MessageController @Inject constructor(private val mailboxContentRealm: Rea
                 .limit(fibonacci)
         }
 
+        private fun getMessagesQuery(messageUid: String, realm: TypedRealm): RealmQuery<Message> {
+            return realm.query<Message>("${Message::uid.name} == $0", messageUid)
+        }
+
         private fun getMessageQuery(uid: String, realm: TypedRealm): RealmSingleQuery<Message> {
-            return realm.query<Message>("${Message::uid.name} == $0", uid).first()
+            return getMessagesQuery(uid, realm).first()
         }
         //endregion
 

app/src/main/java/com/infomaniak/mail/data/cache/mailboxContent/MessageController.kt

@@ -300,7 +300,7 @@ class Message : RealmObject {
     fun shouldBeExpanded(index: Int, lastIndex: Int) = !isDraft && (!isSeen || index == lastIndex)
 
     fun toThread() = Thread().apply {
-        uid = this@Message.uid
+        uid = this@Message.uid // TODO: Check if we can use random UUID instead ?
         folderId = this@Message.folderId
         messagesIds += this@Message.messageIds
         messages += this@Message

app/src/main/java/com/infomaniak/mail/data/models/message/Message.kt

@@ -53,18 +53,16 @@ class PrintMailFragment : Fragment() {
         return FragmentPrintMailBinding.inflate(inflater, container, false).also { binding = it }.root
     }
 
-    override fun onViewCreated(view: View, savedInstanceState: Bundle?): Unit = with(threadViewModel) {
+    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
         super.onViewCreated(view, savedInstanceState)
 
         setupAdapter()
 
-        messagesLive.observe(viewLifecycleOwner) { (items, _) ->
-            threadAdapter.submitList(listOf(items.single { it is Message && it.uid == navigationArgs.messageUid }))
+        threadViewModel.messagesLive.observe(viewLifecycleOwner) { (items, _) ->
+            threadAdapter.submitList(items)
         }
 
-        navigationArgs.openThreadUid?.let {
-            reassignMessagesLive(it, withSuperCollapsedBlock = false)
-        }
+        threadViewModel.reassignMessagesLiveWithoutSuperCollapsedBlock(navigationArgs.messageUid)
     }
 
     private fun setupAdapter() {

app/src/main/java/com/infomaniak/mail/ui/main/thread/PrintMailFragment.kt

@@ -47,6 +47,7 @@ import io.realm.kotlin.query.RealmResults
 import io.sentry.Sentry
 import io.sentry.SentryLevel
 import kotlinx.coroutines.*
+import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.map
 import javax.inject.Inject
 import kotlin.collections.set
@@ -128,95 +129,78 @@ class ThreadViewModel @Inject constructor(
         }
     }
 
-    fun reassignMessagesLive(threadUid: String, withSuperCollapsedBlock: Boolean = true) {
+    fun reassignMessagesLive(threadUid: String) {
+        reassignMessages {
+            messageController.getSortedAndNotDeletedMessagesAsync(threadUid)?.map { mapRealmMessagesResult(it.list, threadUid) }
+        }
+    }
+
+    fun reassignMessagesLiveWithoutSuperCollapsedBlock(messageUid: String) {
+        reassignMessages {
+            messageController.getMessagesAsync(messageUid).map { mapRealmMessagesResultWithoutSuperCollapsedBlock(it.list) }
+        }
+    }
+
+    private fun reassignMessages(messagesFlow: (() -> Flow<Pair<ThreadAdapterItems, MessagesWithoutHeavyData>>?)) {
         messagesLiveJob?.cancel()
         messagesLiveJob = viewModelScope.launch(ioCoroutineContext) {
-            messageController.getSortedAndNotDeletedMessagesAsync(threadUid)
-                ?.map { mapRealmMessagesResult(it.list, threadUid, withSuperCollapsedBlock) }
-                ?.collect(messagesLive::postValue)
+            messagesFlow()?.collect(messagesLive::postValue)
         }
     }
 
     private suspend fun mapRealmMessagesResult(
         messages: RealmResults<Message>,
         threadUid: String,
-        withSuperCollapsedBlock: Boolean,
     ): Pair<ThreadAdapterItems, MessagesWithoutHeavyData> {
 
         superCollapsedBlock = superCollapsedBlock ?: SuperCollapsedBlock()
 
-        val items = mutableListOf<Any>()
-        val messagesToFetch = mutableListOf<Message>()
-        val thread = messages.firstOrNull()?.threads?.firstOrNull { it.uid == threadUid } ?: return items to messagesToFetch
+        val thread = messages.first().threads.single { it.uid == threadUid }
         val firstIndexAfterBlock = computeFirstIndexAfterBlock(thread, messages)
-        superCollapsedBlock!!.shouldBeDisplayed =
-            shouldBlockBeDisplayed(messages.count(), firstIndexAfterBlock, withSuperCollapsedBlock)
+        superCollapsedBlock!!.shouldBeDisplayed = shouldBlockBeDisplayed(messages.count(), firstIndexAfterBlock)
 
-        suspend fun addMessage(message: Message) {
-            splitBody(message).let {
-                items += it
-                if (!it.isFullyDownloaded()) messagesToFetch += it
-            }
-        }
-
-        suspend fun mapListWithNewBlock() {
-            messages.forEachIndexed { index, message ->
+        suspend fun formatListWithNewBlock(): Pair<ThreadAdapterItems, MessagesWithoutHeavyData> {
+            return formatLists(messages) { index, _ ->
                 when (index) {
-                    0 -> { // First Message
-                        addMessage(message)
-                    }
-                    in 1..<firstIndexAfterBlock -> { // All Messages that should go in block
-                        superCollapsedBlock!!.messagesUids.add(message.uid)
-                    }
-                    firstIndexAfterBlock -> { // First Message not in block
-                        items += superCollapsedBlock!!
-                        addMessage(message.apply { shouldHideDivider = true })
-                    }
-                    else -> { // All following Messages
-                        addMessage(message)
-                    }
+                    0 -> MessageBehavior.DISPLAYED // First Message
+                    in 1 until firstIndexAfterBlock -> MessageBehavior.COLLAPSED // All Messages that should go in block
+                    firstIndexAfterBlock -> MessageBehavior.FIRST_AFTER_BLOCK // First Message not in block
+                    else -> MessageBehavior.DISPLAYED // All following Messages
                 }
             }
         }
 
-        suspend fun mapListWithExistingBlock() {
+        suspend fun formatListWithExistingBlock(): Pair<ThreadAdapterItems, MessagesWithoutHeavyData> {
 
             var isStillInBlock = true
             val previousBlock = superCollapsedBlock!!.messagesUids.toSet()
 
             superCollapsedBlock!!.messagesUids.clear()
 
-            messages.forEachIndexed { index, message ->
+            return formatLists(messages) { index, messageUid ->
                 when {
-                    index == 0 -> { // First Message
-                        addMessage(message)
-                    }
-                    previousBlock.contains(message.uid) && isStillInBlock -> { // All Messages already in block
-                        superCollapsedBlock!!.messagesUids.add(message.uid)
-                    }
-                    !previousBlock.contains(message.uid) && isStillInBlock -> { // First Message not in block
+                    index == 0 -> MessageBehavior.DISPLAYED // First Message
+                    previousBlock.contains(messageUid) && isStillInBlock -> MessageBehavior.COLLAPSED // All Messages already in block
+                    !previousBlock.contains(messageUid) && isStillInBlock -> { // First Message not in block
                         isStillInBlock = false
-                        items += superCollapsedBlock!!
-                        addMessage(message.apply { shouldHideDivider = true })
-                    }
-                    else -> { // All following Messages
-                        addMessage(message)
+                        MessageBehavior.FIRST_AFTER_BLOCK
                     }
+                    else -> MessageBehavior.DISPLAYED // All following Messages
                 }
             }
         }
 
-        suspend fun mapFullList() {
-            messages.forEach { addMessage(it) }
-        }
-
-        if (superCollapsedBlock!!.shouldBeDisplayed) {
-            if (superCollapsedBlock!!.isFirstTime()) mapListWithNewBlock() else mapListWithExistingBlock()
+        return if (superCollapsedBlock!!.shouldBeDisplayed) {
+            if (superCollapsedBlock!!.isFirstTime()) formatListWithNewBlock() else formatListWithExistingBlock()
         } else {
-            mapFullList()
+            formatLists(messages) { _, _ -> MessageBehavior.DISPLAYED }
         }
+    }
 
-        return items to messagesToFetch
+    private suspend fun mapRealmMessagesResultWithoutSuperCollapsedBlock(
+        messages: RealmResults<Message>,
+    ): Pair<ThreadAdapterItems, MessagesWithoutHeavyData> {
+        return formatLists(messages) { _, _ -> MessageBehavior.DISPLAYED }
     }
 
     private fun computeFirstIndexAfterBlock(thread: Thread, list: RealmResults<Message>): Int {
@@ -235,14 +219,47 @@ class ThreadViewModel @Inject constructor(
      * - If there's any unread Message in between, it will be displayed (hence, all following Messages will be displayed too).
      * After all these Messages are displayed, if there's at least 2 remaining Messages, they're gonna be collapsed in the Block.
      */
-    private fun shouldBlockBeDisplayed(messagesCount: Int, firstIndexAfterBlock: Int, withSuperCollapsedBlock: Boolean): Boolean {
-        return withSuperCollapsedBlock && // When we want to print a mail, we need the full list of Messages
-                superCollapsedBlock?.shouldBeDisplayed == true && // If the Block was hidden for any reason, we mustn't ever display it again
+    private fun shouldBlockBeDisplayed(messagesCount: Int, firstIndexAfterBlock: Int): Boolean {
+        return superCollapsedBlock?.shouldBeDisplayed == true && // If the Block was hidden for any reason, we mustn't ever display it again
                 !hasSuperCollapsedBlockBeenClicked && // Block hasn't been expanded by the user
                 messagesCount >= SUPER_COLLAPSED_BLOCK_MINIMUM_MESSAGES_LIMIT && // At least 5 Messages in the Thread
                 firstIndexAfterBlock >= SUPER_COLLAPSED_BLOCK_FIRST_INDEX_LIMIT  // At least 2 Messages in the Block
     }
 
+    // If we add a fourth case in the `when`, don't forget to add a fourth 'o' in the function name.
+    private suspend fun formatLists(
+        messages: List<Message>,
+        computeBehavior: (Int, String) -> MessageBehavior,
+    ): Pair<MutableList<Any>, MutableList<Message>> {
+
+        val items = mutableListOf<Any>()
+        val messagesToFetch = mutableListOf<Message>()
+
+        suspend fun addMessage(message: Message) {
+            splitBody(message).let {
+                items += it
+                if (!it.isFullyDownloaded()) messagesToFetch += it
+            }
+        }
+
+        messages.forEachIndexed { index, message ->
+            when (computeBehavior(index, message.uid)) {
+                MessageBehavior.DISPLAYED -> {
+                    addMessage(message)
+                }
+                MessageBehavior.COLLAPSED -> {
+                    superCollapsedBlock!!.messagesUids.add(message.uid)
+                }
+                MessageBehavior.FIRST_AFTER_BLOCK -> {
+                    items += superCollapsedBlock!!
+                    addMessage(message.apply { shouldHideDivider = true })
+                }
+            }
+        }
+
+        return items to messagesToFetch
+    }
+
     private suspend fun splitBody(message: Message): Message = withContext(ioDispatcher) {
         if (message.body == null) return@withContext message
 
@@ -437,6 +454,12 @@ class ThreadViewModel @Inject constructor(
         val menuId: Int,
     )
 
+    private enum class MessageBehavior {
+        DISPLAYED,
+        COLLAPSED,
+        FIRST_AFTER_BLOCK,
+    }
+
     companion object {
         private const val SUPER_COLLAPSED_BLOCK_MINIMUM_MESSAGES_LIMIT = 5
         private const val SUPER_COLLAPSED_BLOCK_FIRST_INDEX_LIMIT = 3

app/src/main/java/com/infomaniak/mail/ui/main/thread/ThreadViewModel.kt

@@ -156,7 +156,7 @@ class MessageActionsBottomSheetDialog : MailActionsBottomSheetDialog() {
                     trackBottomSheetMessageActionsEvent(ACTION_PRINT_NAME)
                     safeNavigate(
                         resId = R.id.printMailFragment,
-                        args = PrintMailFragmentArgs(threadUid, messageUid).toBundle(),
+                        args = PrintMailFragmentArgs(messageUid).toBundle(),
                         currentClassName = MessageActionsBottomSheetDialog::class.java.name,
                     )
                 }

app/src/main/java/com/infomaniak/mail/ui/main/thread/actions/MessageActionsBottomSheetDialog.kt

@@ -573,22 +573,14 @@
             android:id="@+id/action_attachmentActionsBottomSheetDialog_to_downloadAttachmentProgressDialog"
             app:destination="@id/downloadAttachmentProgressDialog" />
     </dialog>
+
     <fragment
         android:id="@+id/printMailFragment"
         android:name="com.infomaniak.mail.ui.main.thread.PrintMailFragment"
         android:label="PrintMailFragment">
-
-        <argument
-            android:name="openThreadUid"
-            android:defaultValue="@null"
-            app:argType="string"
-            app:nullable="true" />
-
         <argument
             android:name="messageUid"
-            android:defaultValue="@null"
-            app:argType="string"
-            app:nullable="true" />
+            app:argType="string" />
     </fragment>
 
     <dialog