Fixes reply and forward with attachments for normal and pgp/mime

messages
This commit is contained in:
Zorica Stojchevska 2022-01-14 17:05:37 +01:00 committed by Zorica Stojchevska
parent 3fbb79fd11
commit 02eed4dc84
11 changed files with 53 additions and 48 deletions

View File

@ -2023,7 +2023,7 @@ public class ComposeMessageActivity
// draft
fillMessageFromUserInputs(localMessage, true);
localMessage.setExpirationTime(0);
composeMessageViewModel.saveDraft(localMessage, mNetworkUtil.isConnected());
composeMessageViewModel.saveDraft(localMessage);
new Handler(Looper.getMainLooper()).postDelayed(() -> disableSendButton(false), 500);
if (userAction == UserAction.SAVE_DRAFT_EXIT) {
finishActivity();

View File

@ -177,7 +177,7 @@ class IntentExtrasData(
this.attachments = att
this.embeddedImagesAttachmentsExist = true
} else {
this.attachments = ArrayList() // TODO temporary workaround for non-initialized lateinit crash
this.attachments = attachments
}
}

View File

@ -141,7 +141,11 @@ class UploadAttachmentsWorker @AssistedInject constructor(
attachmentIds.forEach { attachmentId ->
val attachment = messageDetailsRepository.findAttachmentById(attachmentId) ?: return@forEach
if (!attachment.isUploaded && (attachment.filePath == null || attachment.doesFileExist.not())) {
if (!attachment.isUploaded &&
(attachment.filePath == null || (!attachment.filePath!!.startsWith(
"data"
) && attachment.doesFileExist.not()))
) {
return Result.Failure.InvalidAttachment(
String.format(
context.getString(R.string.attachment_failed_message_drafted),

View File

@ -134,9 +134,9 @@ class ComposeMessageRepository @Inject constructor(
jobManager.addJobInBackground(FetchMessageDetailJob(messageId, labelRepository))
}
suspend fun createAttachmentList(
fun createAttachmentList(
attachmentList: List<LocalAttachment>,
) = Attachment.createAttachmentList(messageDao, attachmentList, false)
) = Attachment.createAttachmentList(messageDao, attachmentList.filter { it.doSaveInDB }, false)
fun prepareMessageData(
currentObject: MessageBuilderData,

View File

@ -65,7 +65,6 @@ import ch.protonmail.android.usecase.fetch.FetchPublicKeys
import ch.protonmail.android.usecase.model.FetchPublicKeysRequest
import ch.protonmail.android.usecase.model.FetchPublicKeysResult
import ch.protonmail.android.utils.Event
import ch.protonmail.android.utils.Logger
import ch.protonmail.android.utils.MailToData
import ch.protonmail.android.utils.MessageUtils
import ch.protonmail.android.utils.UiUtil
@ -424,7 +423,7 @@ class ComposeMessageViewModel @Inject constructor(
}
@SuppressLint("GlobalCoroutineUsage")
fun saveDraft(message: Message, hasConnectivity: Boolean) {
fun saveDraft(message: Message) {
val uploadAttachments = _messageDataResult.uploadAttachments
// This coroutine **needs** to be launched in `GlobalScope` to allow the process of saving a
@ -514,9 +513,10 @@ class ComposeMessageViewModel @Inject constructor(
watchForMessageSent()
}
_savingDraftComplete.postValue(draft)
_messageDataResult.attachmentList.map { it.doSaveInDB = false }
}
private suspend fun calculateNewAttachments(message: Message, uploadAttachments: Boolean): List<String> {
private fun calculateNewAttachments(message: Message, uploadAttachments: Boolean): List<String> {
var newAttachmentIds: List<String> = ArrayList()
val listOfAttachments = ArrayList(message.attachments)
if (uploadAttachments && listOfAttachments.isNotEmpty()) {

View File

@ -30,7 +30,6 @@ import ch.protonmail.android.data.local.model.COLUMN_ATTACHMENT_ID
import ch.protonmail.android.data.local.model.COLUMN_ATTACHMENT_MESSAGE_ID
import ch.protonmail.android.data.local.model.COLUMN_CONVERSATION_ID
import ch.protonmail.android.data.local.model.COLUMN_MESSAGE_ACCESS_TIME
import ch.protonmail.android.data.local.model.COLUMN_MESSAGE_DELETED
import ch.protonmail.android.data.local.model.COLUMN_MESSAGE_EXPIRATION_TIME
import ch.protonmail.android.data.local.model.COLUMN_MESSAGE_ID
import ch.protonmail.android.data.local.model.COLUMN_MESSAGE_IS_STARRED

View File

@ -208,10 +208,14 @@ data class Attachment constructor(
val uri = localAttachment.uri
val uriString = uri.toString()
val filePath = if (URLUtil.isNetworkUrl(uriString) || URLUtil.isDataUrl(uriString)) {
uriString
} else {
uri.path
val filePath = when {
URLUtil.isDataUrl(uriString) -> {
uriString
}
uriString.isEmpty() -> {
null
}
else -> uri.path
}
return Attachment(

View File

@ -22,8 +22,6 @@ import android.net.Uri
import android.os.Parcel
import android.os.Parcelable
import android.util.Base64
import ch.protonmail.android.api.ProtonMailApiManager
import ch.protonmail.android.core.ProtonMailApplication
import me.proton.core.util.kotlin.EMPTY_STRING
class LocalAttachment @JvmOverloads constructor(
@ -37,7 +35,8 @@ class LocalAttachment @JvmOverloads constructor(
var isUploading: Boolean = false,
val keyPackets: String? = null,
val headers: AttachmentHeaders? = null,
val isUploaded: Boolean = false
val isUploaded: Boolean = false,
var doSaveInDB: Boolean = true
) : Parcelable {
override fun describeContents() = 0
@ -54,11 +53,12 @@ class LocalAttachment @JvmOverloads constructor(
dest.writeString(keyPackets)
dest.writeString(headers?.toString() ?: "")
dest.writeInt(if (isUploaded) 1 else 0)
dest.writeInt(if (doSaveInDB) 1 else 0)
}
companion object {
fun fromAttachment(api: ProtonMailApiManager, attachment: Attachment): LocalAttachment {
private fun fromAttachment(attachment: Attachment): LocalAttachment {
val uriToParse = if (attachment.isPGPAttachment) {
val encodedMimeData = Base64.encodeToString(
attachment.mimeData,
@ -66,7 +66,7 @@ class LocalAttachment @JvmOverloads constructor(
)
"data:application/octet-stream;base64,$encodedMimeData"
} else {
api.getAttachmentUrl(attachment.attachmentId!!)
""
}
return LocalAttachment(
@ -84,10 +84,6 @@ class LocalAttachment @JvmOverloads constructor(
)
}
@Deprecated("Use with ApiManager", ReplaceWith("fromAttachment(api, attachment)"))
fun fromAttachment(attachment: Attachment): LocalAttachment =
fromAttachment(ProtonMailApplication.getApplication().api, attachment)
fun createLocalAttachmentList(attachmentList: List<Attachment>): List<LocalAttachment> =
attachmentList.map(Companion::fromAttachment)
@ -108,6 +104,7 @@ class LocalAttachment @JvmOverloads constructor(
if (serializedHeaders.isEmpty()) null
else AttachmentHeaders.fromString(serializedHeaders)
val isUploaded = parcel.readInt() == 1
val doSaveInDB = parcel.readInt() == 1
return LocalAttachment(
uri,
@ -120,7 +117,8 @@ class LocalAttachment @JvmOverloads constructor(
isUploading,
keyPackets,
headers,
isUploaded
isUploaded,
doSaveInDB
)
}

View File

@ -864,6 +864,9 @@ internal class MessageDetailsActivity : BaseStoragePermissionActivity() {
}
val attachments = editIntentExtras.attachments
if (attachments.size > 0) {
if (!editIntentExtras.isPGPMime) {
attachments.map { it.doSaveInDB = false }
}
intent.putParcelableArrayListExtra(
ComposeMessageActivity.EXTRA_MESSAGE_ATTACHMENTS,
attachments

View File

@ -31,7 +31,6 @@ import ch.protonmail.android.crypto.AddressCrypto
import ch.protonmail.android.data.local.PendingActionDao
import ch.protonmail.android.data.local.model.Message
import ch.protonmail.android.di.CurrentUserId
import ch.protonmail.android.utils.MessageUtils
import ch.protonmail.android.utils.notifier.UserNotifier
import ch.protonmail.android.worker.drafts.CreateDraftWorker
import ch.protonmail.android.worker.drafts.CreateDraftWorkerErrors

View File

@ -128,14 +128,14 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
runBlockingTest {
// Given
val message = Message()
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised("draftId")
// This indicates that saving draft was requested by the user
viewModel.setUploadAttachments(true)
coEvery { saveDraft(any()) } returns SaveDraftResult.Success("draftId")
coEvery { messageDetailsRepository.findMessageById("draftId") } returns flowOf(message)
// When
viewModel.saveDraft(message, hasConnectivity = false)
viewModel.saveDraft(message)
// Then
val parameters = SaveDraft.SaveDraftParameters(
@ -155,14 +155,14 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
runBlockingTest {
// Given
val message = Message()
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised("draftId")
// This indicates that saving draft was not requested by the user
viewModel.setUploadAttachments(false)
coEvery { saveDraft(any()) } returns SaveDraftResult.Success("draftId")
coEvery { messageDetailsRepository.findMessageById("draftId") } returns flowOf(message)
// When
viewModel.saveDraft(message, hasConnectivity = false)
viewModel.saveDraft(message)
// Then
val parameters = SaveDraft.SaveDraftParameters(
@ -185,12 +185,12 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
val createdDraftId = "newDraftId"
val createdDraft = Message(messageId = createdDraftId, localId = "local28348")
val savedDraftObserver = viewModel.savingDraftComplete.testObserver()
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised(createdDraftId)
coEvery { saveDraft(any()) } returns SaveDraftResult.Success(createdDraftId)
coEvery { messageDetailsRepository.findMessageById(createdDraftId) } returns flowOf(createdDraft)
// When
viewModel.saveDraft(message, hasConnectivity = false)
viewModel.saveDraft(message)
coVerify { messageDetailsRepository.findMessageById(createdDraftId) }
assertEquals(createdDraft, savedDraftObserver.observedValues[0])
@ -204,12 +204,12 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
val createdDraftId = "newDraftId"
val localDraftId = "localDraftId"
val createdDraft = Message(messageId = createdDraftId, localId = localDraftId)
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised(createdDraftId)
coEvery { saveDraft(any()) } returns SaveDraftResult.Success(createdDraftId)
coEvery { messageDetailsRepository.findMessageById(createdDraftId) } returns flowOf(createdDraft)
// When
viewModel.saveDraft(Message(), hasConnectivity = false)
viewModel.saveDraft(Message())
// Then
assertEquals(createdDraftId, viewModel.draftId)
@ -222,14 +222,14 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
runBlockingTest {
// Given
val message = Message()
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised("draftId")
viewModel.draftId = "non-empty-draftId"
viewModel.setUploadAttachments(true)
coEvery { saveDraft(any()) } returns SaveDraftResult.Success("draftId")
coEvery { messageDetailsRepository.findMessageById("draftId") } returns flowOf(message)
// When
viewModel.saveDraft(message, hasConnectivity = false)
viewModel.saveDraft(message)
// Then
val parameters = SaveDraft.SaveDraftParameters(
@ -257,7 +257,7 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
every { stringResourceResolver.invoke(errorResId) } returns "Error creating draft for message %s"
// When
viewModel.saveDraft(message, hasConnectivity = true)
viewModel.saveDraft(message)
val expectedError = "Error creating draft for message $messageSubject"
coVerify { stringResourceResolver.invoke(errorResId) }
@ -278,7 +278,7 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
every { stringResourceResolver.invoke(errorResId) } returns "Error uploading attachments for subject "
// When
viewModel.saveDraft(message, hasConnectivity = true)
viewModel.saveDraft(message)
val expectedError = "Error uploading attachments for subject $messageSubject"
coVerify { stringResourceResolver.invoke(errorResId) }
@ -294,13 +294,13 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
val updatedDraftId = "updatedDraftId"
val updatedDraft = Message(messageId = updatedDraftId, localId = "local82347")
val savedDraftObserver = viewModel.savingDraftComplete.testObserver()
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised(updatedDraftId)
viewModel.draftId = "non-empty draftId triggers update draft"
coEvery { saveDraft(any()) } returns SaveDraftResult.Success(updatedDraftId)
coEvery { messageDetailsRepository.findMessageById(updatedDraftId) } returns flowOf(updatedDraft)
// When
viewModel.saveDraft(message, hasConnectivity = false)
viewModel.saveDraft(message)
coVerify { messageDetailsRepository.findMessageById(updatedDraftId) }
assertEquals(updatedDraft, savedDraftObserver.observedValues[0])
@ -317,12 +317,11 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
val messageId = "draft8237472"
val message = Message(messageId, subject = "A subject")
val buildMessageObserver = viewModel.buildingMessageCompleted.testObserver()
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised(messageId)
// message was already saved once (we're updating)
viewModel.draftId = messageId
every { UiUtil.toHtml(messageBody) } returns "<html> $messageBody <html>"
coEvery { composeMessageRepository.findMessage(messageId) } returns message
coEvery { composeMessageRepository.createAttachmentList(any(), dispatchers.Io) } returns emptyList()
coEvery { composeMessageRepository.createAttachmentList(any()) } returns emptyList()
// When
viewModel.autoSaveDraft(messageBody)
@ -344,12 +343,11 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
val messageId = "draft923823"
val message = Message(messageId, subject = "Another subject")
viewModel.buildingMessageCompleted.testObserver()
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised(messageId)
// message was already saved once (we're updating)
viewModel.draftId = messageId
every { UiUtil.toHtml(messageBody) } returns "<html> $messageBody <html>"
coEvery { composeMessageRepository.findMessage(messageId) } returns message
coEvery { composeMessageRepository.createAttachmentList(any(), dispatchers.Io) } returns emptyList()
coEvery { composeMessageRepository.createAttachmentList(any()) } returns emptyList()
// When
viewModel.autoSaveDraft(messageBody)
@ -372,15 +370,14 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
val message = Message(addressID = "changedSenderAddress")
val updatedDraftId = "updatedDraftId"
val updatedDraft = Message(messageId = updatedDraftId, localId = "local82347")
givenViewModelPropertiesAreInitialised()
givenViewModelPropertiesAreInitialised("non-empty draftId triggers update draft")
// This value was set to empty during initial draft creation
viewModel.oldSenderAddressId = ""
viewModel.draftId = "non-empty draftId triggers update draft"
coEvery { saveDraft(any()) } returns SaveDraftResult.Success(updatedDraftId)
coEvery { messageDetailsRepository.findMessageById(updatedDraftId) } returns flowOf(updatedDraft)
// When
viewModel.saveDraft(message, hasConnectivity = false)
viewModel.saveDraft(message)
// Then
coVerify { messageDetailsRepository.findMessageById(updatedDraftId) }
@ -388,8 +385,9 @@ class ComposeMessageViewModelTest : ArchTest, CoroutinesTest {
}
}
private fun givenViewModelPropertiesAreInitialised() {
private fun givenViewModelPropertiesAreInitialised(draftId: String = "") {
// Needed to set class fields to the right value and allow code under test to get executed
viewModel.draftId = draftId
viewModel.prepareMessageData(false, "addressId", "mail-alias")
viewModel.setupComposingNewMessage(Constants.MessageActionType.FORWARD, "parentId823", "")
viewModel.oldSenderAddressId = "previousSenderAddressId"