Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion partner_email_check/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

{
"name": "Email Format Checker",
"version": "19.0.1.0.0",
"version": "19.0.1.1.0",
"summary": "Validate email address field",
"author": "Komit, Odoo Community Association (OCA)",
"website": "https://github.com/OCA/partner-contact",
Expand Down
20 changes: 20 additions & 0 deletions partner_email_check/models/res_partner.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,32 @@ def _normalize_email(self, email):
return result.normalized.lower()

def _should_check_syntax(self):
"""Whether to validate the address, and so also normalize it.

``partner_email_check_skip_syntax`` in the context turns this off for a
single operation, for the same reason as
``_should_check_deliverability``. It skips normalization too, exactly as
disabling the company setting does.
"""
if self.env.context.get("partner_email_check_skip_syntax"):
return False
return self.env.company.partner_email_check_syntax

def _should_filter_duplicates(self):
return self.env.company.partner_email_check_filter_duplicates

def _should_check_deliverability(self):
"""Whether to require that the address' domain accepts mail.

``partner_email_check_skip_deliverability`` in the context turns this
off for a single operation. Code that stores addresses it did not
collect from a user needs that: the incoming mail gateway creates a
partner for the sender of every message, and bulk senders routinely send
from a subdomain that publishes no MX record, so a hard check there
discards the message rather than improving anyone's data.
"""
if self.env.context.get("partner_email_check_skip_deliverability"):
return False
Comment on lines +88 to +89

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe it's a personal take, but IMO it's better to use positive flags

Suggested change
if self.env.context.get("partner_email_check_skip_deliverability"):
return False
if not self.env.context.get("partner_email_check_deliverability", True):
return False

Meaning: by default it's True, but you can set it to False explicitly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I did weigh the positive form. I kept skip_ because a company setting is also in play here. With partner_email_check_deliverability, True wouldn't mean "check deliverability". It would mean "defer to the company setting", so a caller who writes with_context(partner_email_check_deliverability=True) to force the DNS check on a company that has it off would quietly get no check. A skip_ key only promises what it does: it can turn a check off for one operation, never on. It also matches how core names one-operation bypasses (skip_account_move_synchronization, skip_sms, skip_activity). The default-True flags like active_test have no setting competing with them.

If you'd still prefer a positive form, I think it should be a real override (unset means the company setting, True forces the check on, False turns it off), so that True means what it says. Happy to switch to that if you'd rather.

return self.env.company.partner_email_check_check_deliverability

@api.model_create_multi
Expand Down
23 changes: 23 additions & 0 deletions partner_email_check/readme/USAGE.md
Original file line number Diff line number Diff line change
@@ -1 +1,24 @@
This module integrate automatically in all of the view `res.partner`

## Bypassing the checks for a single operation

The checks run on every `res.partner` create and write. That is what you want
for an address a user typed, but not for one your code did not collect from a
user: the incoming mail gateway creates a partner for the sender of every
message it accepts, and bulk senders routinely send from a subdomain that
publishes no MX record, so a deliverability failure there discards the message
rather than improving anyone's data.

Two context keys turn a check off for one operation:

```python
# Store the address even though its domain accepts no mail.
partner.with_context(partner_email_check_skip_deliverability=True).write(vals)

# Store it without validating or normalizing it at all.
partner.with_context(partner_email_check_skip_syntax=True).write(vals)
```

`partner_email_check_skip_deliverability` keeps validation and normalization and
skips only the DNS lookup. `partner_email_check_skip_syntax` skips both, exactly
as disabling the company setting does.
36 changes: 36 additions & 0 deletions partner_email_check/tests/test_partner_email_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,39 @@ def test_invalid_email_addresses_allowed(self):
self.env.company.partner_email_check_syntax = False
self.test_partner.email = "bad@email@domain..com"
self.assertTrue(self.test_partner.email)

def test_nondeliverable_addresses_allowed_by_context(self):
"""The check is bypassable for one operation, and only that one.

Mixed case in, lowercase out: skipping deliverability must not also skip
validation, so callers keep the normalization they came for.
"""
self.check_deliverability()
partner = self.test_partner.with_context(
partner_email_check_skip_deliverability=True
)
partner.email = "Cezrik@ACOA.nrdkt"
self.assertEqual(partner.email, "cezrik@acoa.nrdkt")

def test_syntax_still_checked_when_deliverability_skipped(self):
"""Bypassing one check must not quietly bypass the other."""
self.check_deliverability()
with self.assertRaisesRegex(ValidationError, "is an invalid email"):
self.test_partner.with_context(
partner_email_check_skip_deliverability=True
).email = "bad@email@domain..com"

def test_deliverability_still_checked_outside_the_context(self):
"""The bypass does not persist onto the record's own environment."""
self.check_deliverability()
self.test_partner.with_context(
partner_email_check_skip_deliverability=True
).email = "cezrik@acoa.nrdkt"
with self.assertRaisesRegex(ValidationError, "Cannot deliver to email address"):
self.test_partner.email = "othercezrik@acoa.nrdkt"

def test_invalid_email_addresses_allowed_by_context(self):
self.assertTrue(self.env.company.partner_email_check_syntax)
partner = self.test_partner.with_context(partner_email_check_skip_syntax=True)
partner.email = "bad@email@domain..com"
self.assertEqual(partner.email, "bad@email@domain..com")
Loading