Skip to content
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

Add outbox backend implementation #6031

Merged
merged 5 commits into from
Apr 7, 2022

Conversation

miaulalala
Copy link
Contributor

@miaulalala miaulalala commented Feb 11, 2022

Closes #6139

  • Adapt to Add outbox REST API stubs #6052
  • OutboxService::getMessages
  • OutboxService::getMessage
  • OutboxService::sendMessage
  • OutboxService::deleteMessage
  • LocalAttachmentMapperTest (Integration)
  • RecipientMapperTest (Integration)
  • LocalMessageMapperTest (Integration)
  • OutboxServiceIntegrationTest (Integration)
  • Fix OutboxControllerTest (Unit)
  • Fix OutboxServiceTest (Unit)
  • AttachmentServiceTest (Unit)
  • MailTransmissionUnitTest
  • RepiledMessageData in Transmission::sendLocalMessage
    • Pass Message ID only and let the SentMessageEvent handle the mark replied
  • change the Event Order to delete a draft immediately on scheduling send.
  • MailTransmissionIntegrationTest - cover sendLocalMessage
  • Debug fwrite error
  • Attachments: add an array of attachments and process them per type

@miaulalala miaulalala self-assigned this Feb 11, 2022
@ChristophWurst ChristophWurst changed the title rest api changes Add local mailbox REST API Feb 11, 2022
@ChristophWurst ChristophWurst changed the title Add local mailbox REST API Add outbox REST API Feb 11, 2022
@st3iny st3iny added this to the v1.12.0 milestone Feb 15, 2022
@ChristophWurst ChristophWurst changed the title Add outbox REST API Add outbox backend implementation Feb 18, 2022
@ChristophWurst ChristophWurst marked this pull request as draft February 18, 2022 16:21
@miaulalala

This comment was marked as outdated.

@ChristophWurst
Copy link
Member

CI should run but doesn't. I hope this is just a Github hiccup. This PR doesn't even change/touch the workflow files.

@miaulalala miaulalala marked this pull request as ready for review March 28, 2022 15:07
@miaulalala
Copy link
Contributor Author

StatAna isn't liking the event dispatcher, otherwise it looks ok

@ChristophWurst
Copy link
Member

ChristophWurst commented Mar 29, 2022

StatAna isn't liking the event dispatcher, otherwise it looks ok

Use the interface instead of the implementation

lib/Db/LocalMessageMapper.php Outdated Show resolved Hide resolved
lib/Service/AutoConfig/ConnectivityTester.php Outdated Show resolved Hide resolved
lib/Service/MailTransmission.php Outdated Show resolved Hide resolved
lib/Service/OutboxService.php Show resolved Hide resolved
@ChristophWurst
Copy link
Member

$mail->addAttachment('thisIsMyFileContentAsAString', 'test.txt', 'attachment');

The first argument should be a path to your file, not the contents as string

@miaulalala
Copy link
Contributor Author

miaulalala commented Apr 6, 2022

$mail->addAttachment('thisIsMyFileContentAsAString', 'test.txt', 'attachment');

The first argument should be a path to your file, not the contents as string

I did try this for sure, I'm not Jared who's 19 and can't read ;)

But it doesn't matter anyway:

    public function getRaw($stream = true)
    {
        if ($stream) {
            $hdr = new Horde_Stream();
            $hdr->add($this->_headers->toString(), true);
            return Horde_Stream_Wrapper_Combine::getStream(
                array($hdr->stream,
                      $this->getBasePart()->toString(
                        array('stream' => true, 'encode' => Horde_Mime_Part::ENCODE_7BIT | Horde_Mime_Part::ENCODE_8BIT | Horde_Mime_Part::ENCODE_BINARY))
                )
            );
        }

        return $this->_headers->toString() . $this->getBasePart()->toString();
    }

There is no way to get the additional body parts from a raw message.

@ChristophWurst
Copy link
Member

Bildschirmfoto von 2022-04-06 20-25-16

Sending messages with attachments fails successfully

@ChristophWurst
Copy link
Member

Bildschirmfoto von 2022-04-06 20-27-48

lib/Contracts/ILocalMailboxService.php Outdated Show resolved Hide resolved
} catch (ClientException $e) {
$this->logger->info("Message in reply " . $messageId . " could not be loaded: " . $e->getMessage());
}
$repliedMessageData = new RepliedMessageData($account, $repliedMessage);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what the old data structure and identification allowed was to receive a message with account A, reply with account B and still get the flag updates on account A. This will break now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raised as #6189

lib/Service/OutboxService.php Show resolved Hide resolved
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works

Code looks good but couldn't grasp all the changes due to the size of the patch

miaulalala and others added 5 commits April 7, 2022 09:20
Co-Authored-By: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Anna Larch <anna@nextcloud.com>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants