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

[FIX] mail_send_copy: work with existing BCC+adding tests #1527

Open
wants to merge 1 commit into
base: 15.0
Choose a base branch
from

Conversation

liklee-it
Copy link

@liklee-it liklee-it commented Dec 21, 2024

  1. mail_send_copy doesn't work with the module mail_composer_cc_bcc because there is a mistake in the line
    if message["Bcc"]:
    message["Bcc"] = message["Bcc"].join(COMMASPACE, message["From"])
    This PR fixes this issue.

  2. This PR adds tests as well

@liklee-it liklee-it force-pushed the 15.0-fix-mail_send_copy branch from 6bf8426 to 2b41d52 Compare December 21, 2024 03:06
@@ -4,7 +4,7 @@
{
"name": "Mail - Send Email Copy",
"summary": "Send to you a copy of each mail sent by Odoo",
"version": "15.0.1.0.0",
"version": "15.0.1.0.1",
Copy link
Member

Choose a reason for hiding this comment

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

please don't increase the version manually, we do this during merge

# Set the combined Bcc
message.replace_header(
"Bcc", all_bcc
) if "Bcc" in message else message.add_header("Bcc", all_bcc)
Copy link
Member

Choose a reason for hiding this comment

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

why don't you use a normal conditional here?

Copy link
Author

Choose a reason for hiding this comment

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

just free style :) and sure, I will update it

@liklee-it liklee-it force-pushed the 15.0-fix-mail_send_copy branch from 69e2a69 to 6f9b684 Compare December 28, 2024 17:36
@liklee-it
Copy link
Author

cc @Kev-Roche @metaminux @nilshamerlinck to review, thx!

Copy link
Contributor

@trisdoan trisdoan left a comment

Choose a reason for hiding this comment

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

I think these commits can be squashed into one.

Comment on lines +4 to +5
# TODO: find a way to test sending email with existing BCC

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, could you consider this test please?

@tagged("post_install", "-at_install")
class TestMailSendWithBcc(TestMailSendCopy):
    def test_send_email_with_existing_bcc(self):
        if not self.env["ir.module.module"].search(
            [("name", "=", "mail_composer_cc_bcc"), ("state", "=", "installed")]
        ):
            self.skipTest("mail_composer_cc_bcc module is required for this test")

        partner_bcc = self.env.ref("base.res_partner_main2")
        composer = self.env["mail.compose.message"].create(
            {
                "partner_ids": [(6, 0, [self.partner.id])],
                "subject": "Test Subject",
                "body": "<p>Test Body</p>",
                "email_from": "[email protected]",
            }
        )
        composer.partner_bcc_ids = partner_bcc

        with patch(
            "odoo.addons.base.models.ir_mail_server.IrMailServer.send_email"
        ) as mock_send_email:
            composer._action_send_mail()
            # Verify that send_email was called
            self.assertTrue(mock_send_email.called)
            call_args = mock_send_email.call_args[0]
            message = call_args[0]
            # Check existing BCC in the email message
            self.assertIn("[email protected]", message["Bcc"])

Copy link
Author

Choose a reason for hiding this comment

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

Nice idea, thanks. I have added them in the commit. It seems working, but here I cannot find the results of codecov anymore :( Have I done something wrong?

@trisdoan
Copy link
Contributor

trisdoan commented Jan 9, 2025

FYI, If you're interested in mail_composer_cc_bcc, there is an open PR to fix a bug: #1310

@liklee-it liklee-it force-pushed the 15.0-fix-mail_send_copy branch from 6f9b684 to 018a625 Compare January 9, 2025 15:27
[FIX] mail_send_copy: work with existing BCC+adding tests2

[IMP] improve code for OCA convention

[IMP] mail_send_copy: add test for sending email with bcc
@liklee-it liklee-it force-pushed the 15.0-fix-mail_send_copy branch from 018a625 to 7d3a8f5 Compare January 9, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants