From 1d607fdc2e82da37f12c3d4d80dad15e9e99166e Mon Sep 17 00:00:00 2001 From: hamza221 Date: Mon, 14 Aug 2023 18:41:58 +0200 Subject: [PATCH] Fix Lost update between last draft and sending a message Signed-off-by: hamza221 --- lib/Controller/DraftsController.php | 8 +--- lib/Service/DraftsService.php | 5 --- lib/Service/OutboxService.php | 3 ++ src/components/NewMessageModal.vue | 6 ++- tests/Unit/Service/DraftsServiceTest.php | 48 ------------------------ tests/Unit/Service/OutboxServiceTest.php | 16 ++++++++ 6 files changed, 25 insertions(+), 61 deletions(-) diff --git a/lib/Controller/DraftsController.php b/lib/Controller/DraftsController.php index 6697116b63..47a7f558c7 100644 --- a/lib/Controller/DraftsController.php +++ b/lib/Controller/DraftsController.php @@ -121,9 +121,6 @@ public function create( $message->setSendAt($sendAt); $message->setSmimeSign($smimeSign); $message->setSmimeEncrypt($smimeEncrypt); - if ($sendAt !== null) { - $message->setType(LocalMessage::TYPE_OUTGOING); - } if (!empty($smimeCertificateId)) { $smimeCertificate = $this->smimeService->findCertificate($smimeCertificateId, $this->userId); @@ -175,9 +172,8 @@ public function update(int $id, $message = $this->service->getMessage($id, $this->userId); $account = $this->accountService->find($this->userId, $accountId); - ($sendAt !== null) - ? $message->setType(LocalMessage::TYPE_OUTGOING) - : $message->setType(LocalMessage::TYPE_DRAFT); + + $message->setType(LocalMessage::TYPE_DRAFT); $message->setAccountId($accountId); $message->setAliasId($aliasId); $message->setSubject($subject); diff --git a/lib/Service/DraftsService.php b/lib/Service/DraftsService.php index 2d7f996471..f3c9d596b4 100644 --- a/lib/Service/DraftsService.php +++ b/lib/Service/DraftsService.php @@ -151,11 +151,6 @@ public function updateMessage(Account $account, LocalMessage $message, array $to $ccRecipients = self::convertToRecipient($cc, Recipient::TYPE_CC); $bccRecipients = self::convertToRecipient($bcc, Recipient::TYPE_BCC); - // if this message has a "sent at" datestamp and the type is set as Type Outbox - // check for a valid recipient - if ($message->getType() === LocalMessage::TYPE_OUTGOING && $toRecipients === []) { - throw new ClientException('Cannot convert message to outbox message without at least one recipient'); - } $message = $this->mapper->updateWithRecipients($message, $toRecipients, $ccRecipients, $bccRecipients); diff --git a/lib/Service/OutboxService.php b/lib/Service/OutboxService.php index e112e714dd..8afc6b1309 100644 --- a/lib/Service/OutboxService.php +++ b/lib/Service/OutboxService.php @@ -276,6 +276,9 @@ public function flush(): void { } public function convertDraft(LocalMessage $draftMessage, int $sendAt): LocalMessage { + if (empty($draftMessage->getRecipients())) { + throw new ClientException('Cannot convert message to outbox message without at least one recipient'); + } $outboxMessage = clone $draftMessage; $outboxMessage->setType(LocalMessage::TYPE_OUTGOING); $outboxMessage->setSendAt($sendAt); diff --git a/src/components/NewMessageModal.vue b/src/components/NewMessageModal.vue index 59655ab72a..1a49ceccda 100644 --- a/src/components/NewMessageModal.vue +++ b/src/components/NewMessageModal.vue @@ -310,8 +310,9 @@ export default { // This is a new message const { id } = await saveDraft(dataForServer) dataForServer.id = id - await this.$store.dispatch('outbox/enqueueMessage', { - message: dataForServer, + await this.$store.dispatch('outbox/enqueueFromDraft', { + draftMessage: dataForServer, + id, }) } else if (this.composerData.type === 0) { // This is an outbox message @@ -322,6 +323,7 @@ export default { }) } else { // This is a draft + await updateDraft(dataForServer) dataForServer.id = this.composerData.id await this.$store.dispatch('outbox/enqueueFromDraft', { draftMessage: dataForServer, diff --git a/tests/Unit/Service/DraftsServiceTest.php b/tests/Unit/Service/DraftsServiceTest.php index 03720867ae..536397feb6 100644 --- a/tests/Unit/Service/DraftsServiceTest.php +++ b/tests/Unit/Service/DraftsServiceTest.php @@ -373,54 +373,6 @@ public function testConvertToOutboxMessage(): void { $this->draftsService->updateMessage($account, $message, $to, $cc, $bcc, $attachments); } - public function testConvertToOutboxMessageNoRecipients(): void { - $message = new LocalMessage(); - $message->setId(10); - $message->setAccountId(1); - $message->setSendAt($this->time->getTime()); - $message->setSubject('Test'); - $message->setBody('Test Test Test'); - $message->setHtml(true); - $message->setInReplyToMessageId('abcd'); - $message->setType(LocalMessage::TYPE_OUTGOING); - $old = Recipient::fromParams([ - 'label' => 'Pam', - 'email' => 'BuyMeAnAle@startdewvalley.com', - 'type' => Recipient::TYPE_TO, - ]); - $message->setRecipients([$old]); - $to = []; - $cc = []; - $bcc = []; - $attachments = [['type' => '']]; - $attachmentIds = [3]; - $message2 = $message; - $message2->setRecipients([]); - $account = $this->createConfiguredMock(Account::class, [ - 'getUserId' => $this->userId - ]); - $client = $this->createMock(\Horde_Imap_Client_Socket::class); - - $this->mapper->expects(self::never()) - ->method('updateWithRecipients') - ->with($message, [], $cc, $bcc) - ->willReturn($message2); - $this->clientFactory->expects(self::never()) - ->method('getClient') - ->with($account) - ->willReturn($client); - $this->attachmentService->expects(self::never()) - ->method('handleAttachments') - ->with($account, $attachments, $client) - ->willReturn($attachmentIds); - $this->attachmentService->expects(self::never()) - ->method('updateLocalMessageAttachments') - ->with($this->userId, $message2, $attachmentIds); - - $this->expectException(ClientException::class); - $this->draftsService->updateMessage($account, $message, $to, $cc, $bcc, $attachments); - } - public function testUpdateMessageNoAttachments(): void { $message = new LocalMessage(); $message->setId(10); diff --git a/tests/Unit/Service/OutboxServiceTest.php b/tests/Unit/Service/OutboxServiceTest.php index f5262b157d..44f15baef4 100644 --- a/tests/Unit/Service/OutboxServiceTest.php +++ b/tests/Unit/Service/OutboxServiceTest.php @@ -496,4 +496,20 @@ public function testSendMessageTransmissionError(): void { $this->expectException(ClientException::class); $this->outboxService->sendMessage($message, $account); } + + public function testConvertToOutboxMessageNoRecipients(): void { + $message = new LocalMessage(); + $message->setId(10); + $message->setAccountId(1); + $sentAt = $this->time->getTime(); + $message->setSendAt($sentAt); + $message->setSubject('Test'); + $message->setBody('Test Test Test'); + $message->setHtml(true); + $message->setInReplyToMessageId('abcd'); + $message->setType(LocalMessage::TYPE_DRAFT); + + $this->expectException(ClientException::class); + $this->outboxService->convertDraft($message, $sentAt); + } }