Skip to content

[FW][FIX] mail: multi recipient duplication (16.0)#189071

Closed
fw-bot wants to merge 3 commits intoodoo:masterfrom
odoo-dev:master-16.0-mail_semicolon_dup-juc-4Wyl-fw
Closed

[FW][FIX] mail: multi recipient duplication (16.0)#189071
fw-bot wants to merge 3 commits intoodoo:masterfrom
odoo-dev:master-16.0-mail_semicolon_dup-juc-4Wyl-fw

Conversation

@fw-bot
Copy link
Copy Markdown
Contributor

@fw-bot fw-bot commented Nov 28, 2024

Install mass-mailing and crm. Change the email address of Brandon to a@example.com;b@example.com. In the list view of crm, add the select all leads and change their customer to Brandon (the customer column is not displayed by default, just make it visible). Create a new mass-mailing with the recipient list as "Lead/Opportunity". Start the campaign. Brandon receives as many emails as there are leads but he shall only receive one.

The system has a known limitation when it comes to filtering duplicates: it skips all records that have multiple recipients. In this case Brandon has two: a@example.com and b@example.com. The de-duplication mechanism was skipped for every lead he was the customer of and each time a new email was sent, spamming him.

In this work we make it possible to also process records with multiple recipients. It is a best-effort and will still let some duplicates through. Nonetheless it solves the current problem with minimal changes.

Note: any([]) and any(['']) are both False while all([]) is True, hence we now check for empty list / empty email first otherwise an empty list would be considered to be opt-out instead of empty.

Task-3927361

Forward-Port-Of: #188469
Forward-Port-Of: #186149

@robodoo
Copy link
Copy Markdown
Contributor

robodoo commented Nov 28, 2024

Pull request status dashboard

@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels Nov 28, 2024
@fw-bot
Copy link
Copy Markdown
Contributor Author

fw-bot commented Nov 28, 2024

@Julien00859 @tde-banana-odoo cherrypicking of pull request #186149 failed.

stdout:

Auto-merging addons/mail/wizard/mail_compose_message.py
Auto-merging addons/test_mail/tests/test_mail_composer.py
Auto-merging addons/test_mail_full/tests/test_mass_mailing.py
CONFLICT (content): Merge conflict in addons/test_mail_full/tests/test_mass_mailing.py

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@C3POdoo C3POdoo added the RD research & development, internal work label Nov 28, 2024
@fw-bot
Copy link
Copy Markdown
Contributor Author

fw-bot commented Dec 14, 2024

@Julien00859 @tde-banana-odoo this forward port of #186149 is awaiting action (not merged or closed).

@tde-banana-odoo tde-banana-odoo force-pushed the master-16.0-mail_semicolon_dup-juc-4Wyl-fw branch from 3e1a1d3 to 85774eb Compare December 16, 2024 09:21
@C3POdoo C3POdoo requested review from a team December 16, 2024 09:24
@tde-banana-odoo tde-banana-odoo force-pushed the master-16.0-mail_semicolon_dup-juc-4Wyl-fw branch 2 times, most recently from 1a63c09 to 7c443b7 Compare December 16, 2024 12:58
Julien00859 and others added 3 commits December 16, 2024 15:18
It is faster to check first if the emails are empty than to look them up
inside the optout/done lists. But frankly this commit is only here to
simplify the diff with the next commit.

Task-3927361

X-original-commit: d38aeef
Part-of: odoo#188469
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
Install mass-mailing and crm. Change the email address of Brandon to
`a@example.com;b@example.com`. In the list view of crm, add the select
all leads and change their customer to Brandon (the customer column is
not displayed by default, just make it visible). Create a new mass-
mailing with the recipient list as "Lead/Opportunity". Start the
campaign. Brandon receives as many emails as there are leads but he
shall only receive one.

The system has a known limitation when it comes to filtering duplicates:
it skips all records that have multiple recipients. In this case Brandon
has two: a@example.com and b@example.com. The de-duplication mechanism
was skipped for every lead he was the customer of and each time a new
email was sent, spamming him.

In this work we make it possible to also process records with multiple
recipients. It is a best-effort and will still let some duplicates
through. Nonetheless it solves the current problem with minimal changes.

Note: `any([])` and `any([''])` are both False while `all([])` is True,
hence we now check for empty list / empty email first otherwise an empty
list would be considered to be opt-out instead of empty.

Task-3927361

X-original-commit: 66b9b43
Part-of: odoo#188469
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
This context key is not used anymore. It was used to help determining duplicates
in mailing mode. However we now check for the content (body, attachments, ...)
which is considered as complete.
@tde-banana-odoo tde-banana-odoo force-pushed the master-16.0-mail_semicolon_dup-juc-4Wyl-fw branch from 7c443b7 to 6640d70 Compare December 16, 2024 14:19
@tde-banana-odoo
Copy link
Copy Markdown
Contributor

@robodoo r+ rebase-merge

@robodoo
Copy link
Copy Markdown
Contributor

robodoo commented Dec 16, 2024

Merge method set to rebase and merge, using the PR as merge commit message.

robodoo pushed a commit that referenced this pull request Dec 16, 2024
It is faster to check first if the emails are empty than to look them up
inside the optout/done lists. But frankly this commit is only here to
simplify the diff with the next commit.

Task-3927361

X-original-commit: d38aeef
Part-of: #189071
Related: odoo/enterprise#75667
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo pushed a commit that referenced this pull request Dec 16, 2024
Install mass-mailing and crm. Change the email address of Brandon to
`a@example.com;b@example.com`. In the list view of crm, add the select
all leads and change their customer to Brandon (the customer column is
not displayed by default, just make it visible). Create a new mass-
mailing with the recipient list as "Lead/Opportunity". Start the
campaign. Brandon receives as many emails as there are leads but he
shall only receive one.

The system has a known limitation when it comes to filtering duplicates:
it skips all records that have multiple recipients. In this case Brandon
has two: a@example.com and b@example.com. The de-duplication mechanism
was skipped for every lead he was the customer of and each time a new
email was sent, spamming him.

In this work we make it possible to also process records with multiple
recipients. It is a best-effort and will still let some duplicates
through. Nonetheless it solves the current problem with minimal changes.

Note: `any([])` and `any([''])` are both False while `all([])` is True,
hence we now check for empty list / empty email first otherwise an empty
list would be considered to be opt-out instead of empty.

Task-3927361

X-original-commit: 66b9b43
Part-of: #189071
Related: odoo/enterprise#75667
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo pushed a commit that referenced this pull request Dec 16, 2024
This context key is not used anymore. It was used to help determining duplicates
in mailing mode. However we now check for the content (body, attachments, ...)
which is considered as complete.

Part-of: #189071
Related: odoo/enterprise#75667
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo added a commit that referenced this pull request Dec 16, 2024
Install mass-mailing and crm. Change the email address of Brandon to a@example.com;b@example.com. In the list view of crm, add the select all leads and change their customer to Brandon (the customer column is not displayed by default, just make it visible). Create a new mass-mailing with the recipient list as "Lead/Opportunity". Start the campaign. Brandon receives as many emails as there are leads but he shall only receive one.

The system has a known limitation when it comes to filtering duplicates: it skips all records that have multiple recipients. In this case Brandon has two: [a@example.com](mailto:a@example.com) and [b@example.com](mailto:b@example.com). The de-duplication mechanism was skipped for every lead he was the customer of and each time a new email was sent, spamming him.

In this work we make it possible to also process records with multiple recipients. It is a best-effort and will still let some duplicates through. Nonetheless it solves the current problem with minimal changes.

Note: any([]) and any(['']) are both False while all([]) is True, hence we now check for empty list / empty email first otherwise an empty list would be considered to be opt-out instead of empty.

Task-3927361

closes #189071

Forward-port-of: #188469
Forward-port-of: #186149
Related: odoo/enterprise#75667
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo pushed a commit that referenced this pull request Dec 17, 2024
It is faster to check first if the emails are empty than to look them up
inside the optout/done lists. But frankly this commit is only here to
simplify the diff with the next commit.

Task-3927361

X-original-commit: d38aeef
Part-of: #189071
Related: odoo/enterprise#75667
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo pushed a commit that referenced this pull request Dec 17, 2024
Install mass-mailing and crm. Change the email address of Brandon to
`a@example.com;b@example.com`. In the list view of crm, add the select
all leads and change their customer to Brandon (the customer column is
not displayed by default, just make it visible). Create a new mass-
mailing with the recipient list as "Lead/Opportunity". Start the
campaign. Brandon receives as many emails as there are leads but he
shall only receive one.

The system has a known limitation when it comes to filtering duplicates:
it skips all records that have multiple recipients. In this case Brandon
has two: a@example.com and b@example.com. The de-duplication mechanism
was skipped for every lead he was the customer of and each time a new
email was sent, spamming him.

In this work we make it possible to also process records with multiple
recipients. It is a best-effort and will still let some duplicates
through. Nonetheless it solves the current problem with minimal changes.

Note: `any([])` and `any([''])` are both False while `all([])` is True,
hence we now check for empty list / empty email first otherwise an empty
list would be considered to be opt-out instead of empty.

Task-3927361

X-original-commit: 66b9b43
Part-of: #189071
Related: odoo/enterprise#75667
Signed-off-by: Julien Castiaux (juc) <juc@odoo.com>
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo pushed a commit that referenced this pull request Dec 17, 2024
This context key is not used anymore. It was used to help determining duplicates
in mailing mode. However we now check for the content (body, attachments, ...)
which is considered as complete.

Part-of: #189071
Related: odoo/enterprise#75667
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
robodoo added a commit that referenced this pull request Dec 17, 2024
Install mass-mailing and crm. Change the email address of Brandon to a@example.com;b@example.com. In the list view of crm, add the select all leads and change their customer to Brandon (the customer column is not displayed by default, just make it visible). Create a new mass-mailing with the recipient list as "Lead/Opportunity". Start the campaign. Brandon receives as many emails as there are leads but he shall only receive one.

The system has a known limitation when it comes to filtering duplicates: it skips all records that have multiple recipients. In this case Brandon has two: [a@example.com](mailto:a@example.com) and [b@example.com](mailto:b@example.com). The de-duplication mechanism was skipped for every lead he was the customer of and each time a new email was sent, spamming him.

In this work we make it possible to also process records with multiple recipients. It is a best-effort and will still let some duplicates through. Nonetheless it solves the current problem with minimal changes.

Note: any([]) and any(['']) are both False while all([]) is True, hence we now check for empty list / empty email first otherwise an empty list would be considered to be opt-out instead of empty.

Task-3927361

closes #189071

Forward-port-of: #188469
Forward-port-of: #186149
Related: odoo/enterprise#75667
Signed-off-by: Thibault Delavallee (tde) <tde@openerp.com>
@robodoo robodoo added the 18.1 label Dec 17, 2024
@robodoo robodoo closed this Dec 17, 2024
@fw-bot fw-bot deleted the master-16.0-mail_semicolon_dup-juc-4Wyl-fw branch December 31, 2024 05:46
xmo-odoo added a commit to odoo-dev/odoo that referenced this pull request Jul 16, 2025
Not sure why odoo#186799 used a super complicated lookup and matching with
inconsistent types just to find the recipient for the subtest, or why
it was replaced by something straightforward in odoo#189071 but that was
never backported.

In 3.13 this triggers a warning from the ORM, not sure why not
before (can't see anything in the changelog, but it might be from a
side-effect e.g. `locals()` ordering changed and now a non-empty dict
is tested before we find our target recipient, or since
`BaseModel.__eq__` ignores falsy objects maybe a dict which was falsy
in previous versions isn't anymore).
robodoo pushed a commit that referenced this pull request Jul 16, 2025
Not sure why #186799 used a super complicated lookup and matching with
inconsistent types just to find the recipient for the subtest, or why
it was replaced by something straightforward in #189071 but that was
never backported.

In 3.13 this triggers a warning from the ORM, not sure why not
before (can't see anything in the changelog, but it might be from a
side-effect e.g. `locals()` ordering changed and now a non-empty dict
is tested before we find our target recipient, or since
`BaseModel.__eq__` ignores falsy objects maybe a dict which was falsy
in previous versions isn't anymore).

Part-of: #219270
Related: odoo/enterprise#90352
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

18.1 conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot RD research & development, internal work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants