Skip to content

Commit

Permalink
refractor: Apply suggestion from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
NicolasBourdin88 committed Nov 22, 2024
1 parent c707760 commit c8d9269
Show file tree
Hide file tree
Showing 14 changed files with 73 additions and 51 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ object ApiRepository : ApiRepositoryCore() {

fun getDownloadedMessage(mailboxUuid: String, folderId: String, shortUid: Int): Response {
val request = Request.Builder().url(ApiRoutes.downloadMessage(mailboxUuid, folderId, shortUid))
.headers(HttpUtils.getHeaders(null))
.headers(HttpUtils.getHeaders())
.get()
.build()

Expand Down
9 changes: 5 additions & 4 deletions app/src/main/java/com/infomaniak/mail/ui/MainViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -1195,17 +1195,18 @@ class MainViewModel @Inject constructor(
}
}

fun getMessagesUidsFromThreadUids(selectedThreadsUuids: List<String>): List<String> {
fun getMessagesUidsFromThreadUids(selectedThreadsUids: List<String>): List<String> {
val messageUids = mutableListOf<String>()
selectedThreadsUuids.forEach { threadUuid ->
selectedThreadsUids.forEach { threadUuid ->
val thread = threadController.getThread(threadUuid) ?: return@forEach
messageUids.addAll(thread.messages.map { it.uid })
}

return messageUids
}

fun getSubject(threadUuid: String): String? {
return threadController.getThread(threadUuid)?.subject
fun getSubject(threadUid: String): String {
return threadController.getThread(threadUid)?.subject ?: ""
}

companion object {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
*/
package com.infomaniak.mail.ui.main.thread.actions

import android.os.Bundle
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.appcompat.content.res.AppCompatResources
import androidx.core.view.isVisible
import androidx.fragment.app.viewModels
import androidx.navigation.fragment.findNavController
import androidx.navigation.fragment.navArgs
Expand All @@ -25,13 +31,17 @@ import com.infomaniak.mail.utils.extensions.AttachmentExtensions
import com.infomaniak.mail.utils.extensions.AttachmentExtensions.getIntentOrGoToPlayStore
import dagger.hilt.android.AndroidEntryPoint

@AndroidEntryPoint
class DownloadAttachmentProgressDialog : DownloadProgressDialog() {
private val navigationArgs: DownloadAttachmentProgressDialogArgs by navArgs()
private val downloadAttachmentViewModel: DownloadAttachmentViewModel by viewModels()

override val dialogTitle: String? by lazy { navigationArgs.attachmentName }
override val dialogIconDrawableRes: Int? by lazy { navigationArgs.attachmentType.icon }

override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?, savedInstanceState: Bundle?): View? {
binding.icon.isVisible = true
binding.icon.setImageDrawable(AppCompatResources.getDrawable(requireContext(), navigationArgs.attachmentType.icon))
return super.onCreateView(inflater, container, savedInstanceState)
}

override fun download() {
downloadAttachmentViewModel.downloadAttachment().observe(this) { cachedAttachment ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,14 @@ import androidx.navigation.fragment.navArgs
import com.infomaniak.lib.core.utils.goToPlayStore
import com.infomaniak.lib.core.utils.setBackNavigationResult
import com.infomaniak.mail.R
import com.infomaniak.mail.utils.KDriveUtils.DRIVE_PACKAGE
import com.infomaniak.mail.utils.KDriveUtils.SAVE_EXTERNAL_ACTIVITY_CLASS
import com.infomaniak.mail.utils.KDriveUtils.canSaveOnKDrive
import dagger.hilt.android.AndroidEntryPoint
import com.infomaniak.mail.utils.SaveOnKDriveUtils.DRIVE_PACKAGE
import com.infomaniak.mail.utils.SaveOnKDriveUtils.SAVE_EXTERNAL_ACTIVITY_CLASS
import com.infomaniak.mail.utils.SaveOnKDriveUtils.canSaveOnKDrive

@AndroidEntryPoint
class DownloadMessagesProgressDialog : DownloadProgressDialog() {
private val downloadThreadsViewModel: DownloadMessagesViewModel by viewModels()
private val navigationArgs: DownloadMessagesProgressDialogArgs by navArgs()
override val dialogTitle: String? by lazy { getDialogTitleFromArgs() }
override val dialogIconDrawableRes: Int? by lazy { null }

override fun download() {
downloadThreadsViewModel.downloadThreads(mainViewModel.currentMailbox.value).observe(this) { threadUris ->
Expand All @@ -54,7 +51,11 @@ class DownloadMessagesProgressDialog : DownloadProgressDialog() {
private fun getDialogTitleFromArgs() = if (navigationArgs.messageUids.size == 1) {
navigationArgs.nameFirstMessage
} else {
requireContext().resources.getQuantityString(R.plurals.downloadingEmailsTitle, 1, navigationArgs.messageUids.size)
requireContext().resources.getQuantityString(
R.plurals.downloadingEmailsTitle,
navigationArgs.messageUids.size,
navigationArgs.messageUids.size
)
}

private fun ArrayList<Uri>.openKDriveOrPlayStore(context: Context): Intent? {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import androidx.lifecycle.SavedStateHandle
import androidx.lifecycle.liveData
import androidx.lifecycle.viewModelScope
import com.infomaniak.lib.core.utils.DownloadManagerUtils
import com.infomaniak.mail.R
import com.infomaniak.mail.data.api.ApiRepository
import com.infomaniak.mail.data.cache.mailboxContent.MessageController
import com.infomaniak.mail.data.models.mailbox.Mailbox
Expand All @@ -47,23 +48,28 @@ class DownloadMessagesViewModel @Inject constructor(

private val ioCoroutineContext = viewModelScope.coroutineContext(ioDispatcher)

private val messageLocalUids
inline get() = savedStateHandle.get<Array<String>>(DownloadMessagesProgressDialogArgs::messageUids.name)!!
private val messageLocalUids: Array<String>
inline get() = savedStateHandle[DownloadMessagesProgressDialogArgs::messageUids.name]!!

fun downloadThreads(currentMailbox: Mailbox?) = liveData(ioCoroutineContext) {
val downloadedThreadUris: List<Uri>? = runCatching {
val mailbox = currentMailbox ?: return@runCatching null
val listUri = mutableListOf<Uri>()
val listFileName = mutableSetOf<String>().also { it.addAll(getAllFileNameInExportEmlDir(appContext)) }
val listFileName = getAllFileNameInExportEmlDir(appContext).toMutableSet()

messageLocalUids.forEach { messageUid ->
val message = messageController.getMessage(messageUid) ?: return@runCatching null
val response = ApiRepository.getDownloadedMessage(mailbox.uuid, message.folderId, message.shortUid)
val response = ApiRepository.getDownloadedMessage(
mailboxUuid = mailbox.uuid,
folderId = message.folderId,
shortUid = message.shortUid,
contentType = EML_CONTENT_TYPE,
)

if (!response.isSuccessful || response.body == null) return@runCatching null

val messageSubject: String = message.subject?.removeIllegalFileNameCharacter() ?: NO_SUBJECT_FILE
createOriginalFileName(messageSubject, listFileName.toList()).let { fileName ->
createUniqueFileName(messageSubject, listFileName.toList()).let { fileName ->
listFileName.add(fileName)
saveEmlToFile(appContext, response.body!!.bytes(), fileName)?.let { listUri.add(it) }
}
Expand All @@ -75,7 +81,7 @@ class DownloadMessagesViewModel @Inject constructor(
}

// TODO Extract this code in core2
private fun createOriginalFileName(originalFileName: String, listFileName: List<String>): String {
private fun createUniqueFileName(originalFileName: String, listFileName: List<String>): String {
var postfix = 1
var fileName = originalFileName

Expand All @@ -85,21 +91,21 @@ class DownloadMessagesViewModel @Inject constructor(
}

private fun getAllFileNameInExportEmlDir(context: Context): List<String> {
val fileDir = File(context.cacheDir, context.getString(com.infomaniak.mail.R.string.EXPOSED_EML_PATH))
val fileDir = File(context.cacheDir, context.getString(R.string.EXPOSED_EML_PATH))
if (!fileDir.exists()) fileDir.mkdirs()
return fileDir.listFiles()?.map { it.name.removeSuffix(".eml") } ?: emptyList()
}

private fun saveEmlToFile(context: Context, emlByteArray: ByteArray, fileName: String): Uri? {
val fileNameWithExtension = "${fileName.removeIllegalFileNameCharacter()}.eml"
val fileDir = File(context.cacheDir, context.getString(com.infomaniak.mail.R.string.EXPOSED_EML_PATH))
val fileDir = File(context.cacheDir, context.getString(R.string.EXPOSED_EML_PATH))

if (!fileDir.exists()) fileDir.mkdirs()

runCatching {
val file = File(fileDir, fileNameWithExtension)
file.outputStream().use { it.write(emlByteArray) }
return FileProvider.getUriForFile(context, context.getString(com.infomaniak.mail.R.string.EML_AUTHORITY), file)
return FileProvider.getUriForFile(context, context.getString(R.string.EML_AUTHORITY), file)
}.onFailure { exception ->
exception.printStackTrace()
}
Expand All @@ -109,7 +115,7 @@ class DownloadMessagesViewModel @Inject constructor(
private fun String.removeIllegalFileNameCharacter(): String = this.replace(DownloadManagerUtils.regexInvalidSystemChar, "")

companion object {
private const val EML_CONTENT_TYPE = "message/rfc822"
private const val NO_SUBJECT_FILE = "message"
private const val EML_CONTENT_TYPE = "message/rfc822"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ package com.infomaniak.mail.ui.main.thread.actions
import android.app.Dialog
import android.os.Bundle
import android.view.KeyEvent
import androidx.annotation.DrawableRes
import androidx.appcompat.content.res.AppCompatResources
import androidx.core.view.isVisible
import androidx.fragment.app.DialogFragment
import androidx.fragment.app.activityViewModels
import androidx.lifecycle.lifecycleScope
Expand All @@ -38,10 +35,13 @@ import kotlinx.coroutines.launch

@AndroidEntryPoint
abstract class DownloadProgressDialog : DialogFragment() {

protected val binding by lazy { DialogDownloadProgressBinding.inflate(layoutInflater) }
protected val mainViewModel: MainViewModel by activityViewModels()

abstract val dialogTitle: String?
@get:DrawableRes abstract val dialogIconDrawableRes: Int?

protected abstract fun download()

override fun onStart() {
download()
Expand All @@ -50,10 +50,6 @@ abstract class DownloadProgressDialog : DialogFragment() {

override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
val context = requireContext()

dialogIconDrawableRes?.let { binding.icon.setImageDrawable(AppCompatResources.getDrawable(requireContext(), it)) }
binding.icon.isVisible = dialogIconDrawableRes == null

isCancelable = false

return MaterialAlertDialogBuilder(context)
Expand All @@ -68,8 +64,6 @@ abstract class DownloadProgressDialog : DialogFragment() {
.create()
}

abstract fun download()

protected fun popBackStackWithError() {
lifecycleScope.launch {
mainViewModel.isNetworkAvailable.first { it != null }?.let { isNetworkAvailable ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,9 @@ class MessageActionsBottomSheetDialog : MailActionsBottomSheetDialog() {
override fun onSaveKDrive() {
trackBottomSheetThreadActionsEvent(ACTION_SAVE_KDRIVE_NAME)
navigateToDownloadMessagesProgressDialog(
listOf(messageUid),
mainViewModel.getSubject(threadUid),
MessageActionsBottomSheetDialog::class.java.name
messageUids = listOf(messageUid),
nameFirstMessage = mainViewModel.getSubject(threadUid),
currentClassName = MessageActionsBottomSheetDialog::class.java.name,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import androidx.fragment.app.activityViewModels
import com.infomaniak.lib.core.utils.SentryLog
import com.infomaniak.lib.core.utils.safeBinding
import com.infomaniak.mail.MatomoMail.ACTION_ARCHIVE_NAME
import com.infomaniak.mail.MatomoMail.ACTION_DELETE_NAME
Expand Down Expand Up @@ -118,13 +119,18 @@ class MultiSelectBottomSheetDialog : ActionsBottomSheetDialog() {
toggleThreadsFavoriteStatus(selectedThreadsUids, shouldFavorite)
isMultiSelectOn = false
}

binding.saveKDrive.setClosingOnClickListener(shouldCloseMultiSelection = true) {
trackMultiSelectActionEvent(ACTION_SAVE_KDRIVE_NAME, selectedThreadsCount, isFromBottomSheet = true)
navigateToDownloadMessagesProgressDialog(
messageUids = mainViewModel.getMessagesUidsFromThreadUids(selectedThreadsUids),
nameFirstMessage = selectedThreads.firstOrNull()?.subject,
MultiSelectBottomSheetDialog::class.java.name,
)
runCatching {
navigateToDownloadMessagesProgressDialog(
messageUids = mainViewModel.getMessagesUidsFromThreadUids(selectedThreadsUids),
nameFirstMessage = mainViewModel.getSubject(selectedThreadsUids.first()),
currentClassName = MultiSelectBottomSheetDialog::class.java.name,
)
}.onFailure {
SentryLog.e(TAG, "SelectedThreadUids is empty, it should not happened")
}
isMultiSelectOn = false
}
}
Expand Down Expand Up @@ -154,4 +160,8 @@ class MultiSelectBottomSheetDialog : ActionsBottomSheetDialog() {
private fun getSpamIconAndText(isFromSpam: Boolean): Pair<Int, Int> {
return if (isFromSpam) R.drawable.ic_non_spam to R.string.actionNonSpam else R.drawable.ic_spam to R.string.actionSpam
}

companion object {
private val TAG = this::class.java.simpleName
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class ThreadActionsBottomSheetDialog : MailActionsBottomSheetDialog() {
trackBottomSheetThreadActionsEvent(ACTION_SAVE_KDRIVE_NAME)
navigateToDownloadMessagesProgressDialog(
messageUids = thread.messages.map { it.uid },
nameFirstMessage = thread.subject,
nameFirstMessage = thread.subject ?: "",
ThreadActionsBottomSheetDialog::class.java.name
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import android.content.Context
import android.content.pm.PackageManager
import io.sentry.Sentry

object KDriveUtils {
object SaveOnKDriveUtils {
const val DRIVE_PACKAGE = "com.infomaniak.drive"
const val SAVE_EXTERNAL_ACTIVITY_CLASS = "com.infomaniak.drive.ui.SaveExternalFilesActivity"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ import com.infomaniak.mail.data.models.mailbox.Mailbox
import com.infomaniak.mail.ui.main.SnackbarManager
import com.infomaniak.mail.ui.main.thread.actions.DownloadAttachmentProgressDialogArgs
import com.infomaniak.mail.utils.AccountUtils
import com.infomaniak.mail.utils.KDriveUtils.DRIVE_PACKAGE
import com.infomaniak.mail.utils.KDriveUtils.SAVE_EXTERNAL_ACTIVITY_CLASS
import com.infomaniak.mail.utils.KDriveUtils.canSaveOnKDrive
import com.infomaniak.mail.utils.SaveOnKDriveUtils.DRIVE_PACKAGE
import com.infomaniak.mail.utils.SaveOnKDriveUtils.SAVE_EXTERNAL_ACTIVITY_CLASS
import com.infomaniak.mail.utils.SaveOnKDriveUtils.canSaveOnKDrive
import com.infomaniak.mail.utils.SentryDebug
import com.infomaniak.mail.utils.WorkerUtils.UploadMissingLocalFileException
import com.infomaniak.mail.utils.extensions.AttachmentExtensions.AttachmentIntentType.OPEN_WITH
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ fun Fragment.navigateToDownloadProgressDialog(

fun Fragment.navigateToDownloadMessagesProgressDialog(
messageUids: List<String>,
nameFirstMessage: String?,
currentClassName: String
nameFirstMessage: String,
currentClassName: String,
) {
safeNavigate(
resId = R.id.downloadMessagesProgressDialog,
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/dialog_download_progress.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
android:layout_marginStart="@dimen/marginStandardMedium"
android:importantForAccessibility="no"
android:src="@drawable/ic_file_unknown"
android:visibility="gone"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toTopOf="parent" />
Expand Down
3 changes: 1 addition & 2 deletions app/src/main/res/navigation/main_navigation.xml
Original file line number Diff line number Diff line change
Expand Up @@ -583,8 +583,7 @@
app:argType="string[]" />
<argument
android:name="nameFirstMessage"
app:argType="string"
app:nullable="true" />
app:argType="string" />
</dialog>

<dialog
Expand Down

0 comments on commit c8d9269

Please sign in to comment.