-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Lost update between last draft and sending a message #8743
Conversation
5ac4d7c
to
fb9b3a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 lost update is no longer lost
but I found a new logic flaw 🥲
? $message->setType(LocalMessage::TYPE_OUTGOING) | ||
: $message->setType(LocalMessage::TYPE_DRAFT); | ||
|
||
$message->setType(LocalMessage::TYPE_DRAFT); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
\OCA\Mail\Controller\DraftsController::create
still has the same logic and it is in use
- Compose a new message
- Send it before the first draft
There is a POST to create the draft but it results in implicit conversion to an outbox message.
Expectation is that the draft is created, then converted to an outbox message.
The removed test doesn't make sense anymore in DraftServiceTest, because if I didn't make any mistakes or forgot an edge case. LocalMessages will never have type outbox in the draft service |
so, ready to get merged? :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Messages that are sent right away and those that are a draft first are sent as expected 🙏
@@ -373,54 +373,6 @@ public function testConvertToOutboxMessage(): void { | |||
$this->draftsService->updateMessage($account, $message, $to, $cc, $bcc, $attachments); | |||
} | |||
|
|||
public function testConvertToOutboxMessageNoRecipients(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, the test makes no sense anymore because we just dropped the implicit conversion 👍
Signed-off-by: hamza221 <[email protected]>
a9c698f
to
1d607fd
Compare
/backport to stable3.3 |
fixes #8734