Fix Lost update between last draft and sending a message

Signed-off-by: hamza221 <[email protected]>
nextcloud · Aug 17, 2023 · 1d607fd · 1d607fd
1 parent a2c9f05
commit 1d607fd
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 61 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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' => '[email protected]',
-			'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
@@ -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);
+	}
 }