[13.0][MIG] partner_email_check: Migration to 13.0 #989

qgroulard · 2020-10-15T15:48:12Z

No description provided.

* Add new module partner-email-check Validate email input

…en importing validate_email in partner_email_check

- partner_email_check now uses email-validator instead of validate_email - Email addresses are normalized - There is a setting to enforce uniqueness of partner email addresses - There is a setting to check whether emails are deliverable (i.e. whether the domain resolves)

portal's tests set an invalid email address, `c@c`, that's rejected by the current version of partner_email_check. If partner_email_check is installed before portal's tests run then they fail. To solve this, skip email validation if tests are running, unless a key in the context is set to make the checks run anyway. That lets tests in other addons opt in to the checks as well. The previous update to partner_email_check didn't bump the version number, so do that now.

kos94ok-3D · 2021-03-21T16:39:52Z

Not allowed duplicate partner with filled email so maybe need set copy=False or if _should_filter_duplicates remove email from copy_data

simahawk · 2021-07-02T08:36:56Z

partner_email_check/models/res_config_settings.py

+
+    partner_email_check_filter_duplicates = fields.Boolean(
+        string="Filter duplicate partner email addresses",
+        help="Don't allow multiple partners to have the same email address.",


Suggested change

help="Don't allow multiple partners to have the same email address.",

help="Don't allow multiple partners to have the same email address.",

config_parameter="partner_email_check_filter_duplicates"

partner_email_check/models/res_config_settings.py

partner_email_check/models/res_partner.py

partner_email_check/tests/test_partner_email_check.py

rousseldenis · 2021-07-02T13:51:54Z

@qgroulard Could you attend comments?

qgroulard · 2021-07-02T14:10:45Z

@qgroulard Could you attend comments?

Yes I am trying to squeeze this in today's agenda but I'm afraid it will be for another day.
I'll work on this asap.

NL66278 · 2021-07-06T11:17:06Z

partner_email_check/models/res_partner.py

+        if self._should_filter_duplicates():
+            for copy_vals in res:
+                if "email" in copy_vals:
+                    copy_vals["email"] = ""


Why not delete the value from the dictionary, or set it to False, so email will have a NULL value in the database.

NL66278

👍 LGTM

rousseldenis · 2021-07-07T07:07:17Z

@simahawk Could you update your review?

simahawk · 2021-07-07T07:24:21Z

partner_email_check/models/res_partner.py

+            for copy_vals in res:
+                if "email" in copy_vals:
+                    del copy_vals["email"]


not a blocker but you could do:

Suggested change

for copy_vals in res:

if "email" in copy_vals:

del copy_vals["email"]

for copy_vals in res:

copy_vals.pop("email", None)

simahawk · 2021-07-07T07:25:10Z

partner_email_check/models/res_partner.py

+
+    @api.model
+    def email_check(self, emails):
+        if config["test_enable"] and not self.env.context.get(


do we really need this? What's the main reason?

simahawk · 2021-07-07T07:26:04Z

partner_email_check/tests/test_partner_email_check.py

+    def setUpClass(cls):
+        super().setUpClass()
+        cls.env = cls.env(context=dict(cls.env.context, tracking_disable=True))
+        # Checks are disabled during tests unless this key is set


I don't get why we need this ctx key 🤔
Checks should be disabled by the flags set on the company.
If you want to keep them ON by default you can set them OFF in tests for the rest of the world by adding demo data 😉

This is actually the same comment as #989 (comment) since this key is only used there.

I think I get it now...

There are actually 3 checks performed:

Check email unique: only if set on company;

Check deliverability: only if set on company, performed with https://pypi.org/project/email-validator/ thus can't run without checking emails syntax as well;

Check syntax: currently always performed unless in test mode without this key (test_partner_email_check), performed with https://pypi.org/project/email-validator/

So I guess the idea is not to break other modules' tests since not having this context key set (and being in test mode) is the only way not to check emails syntax.

Maybe the solution would be to add a third flag for the syntax check.
It should be off by default otherwise other modules' test may fail. Thus users must be aware that behavior changes with v13 since beforehand this check used to always be performed.

What do you think ?

nope, the solution is simpler and I reported it above 😉

Just do this:

add demo data file

in that file call write function on all companies

set the flags to false

in your test class enable the flags

This way for the rest of the test suites (referred as "rest of the world" above) won't be impacted.

of course, the alternative is that you make the flag false by default and you state in the README that it should be enabled

NL66278 · 2021-07-07T09:00:24Z

partner_email_check/migrations/13.0.1.0.0/post-migration.py

@@ -0,0 +1,24 @@
+# Copyright 2021 ACSONE SA/NV
+# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).


Make it https:

[REF] Partner email chack params on company [IMP] email_check [IMP] test partner_email_check use SavepointCase

simahawk

thanks for your changes!

simahawk · 2021-07-08T06:10:56Z

/ocabot merge minor

OCA-git-bot · 2021-07-08T06:11:06Z

This PR looks fantastic, let's merge it!
Prepared branch 13.0-ocabot-merge-pr-989-by-simahawk-bump-minor, awaiting test results.

OCA-git-bot · 2021-07-08T06:48:54Z

Congratulations, your PR was merged at 5578b86. Thanks a lot for contributing to OCA. ❤️

Signed-off-by pedrobaeza

denislour and others added 13 commits October 15, 2020 17:01

Add new module partner-email-check (OCA#496)

361c472

* Add new module partner-email-check Validate email input

OCA Transbot updated translations from Transifex

e7f4252

Migrate module partner_email_check to version 11.0

a79215f

Remove copyright in init + ref code.

44e4a11

[ADD] icon.png

2353acb

[UPD] Update partner_email_check.pot

811239e

[FIX][11.0] Make debugger record a debug message instead of error wh…

d951873

…en importing validate_email in partner_email_check

partner_email_check 11.0.1.0.1

6bd4839

[UPD] Update partner_email_check.pot

b5a7bc1

partner_email_check 11.0.2.0.0

c76c325

[12.0][MIG] partner_email_check: Migration to 12.0

b330b96

qgroulard changed the title ~~[MIG] partner_email_check: Migration to 13.0~~ [13.0][MIG] partner_email_check: Migration to 13.0 Oct 15, 2020

qgroulard force-pushed the 13.0-mig-partner_email_check-qgr branch 2 times, most recently from 41c0e13 to 4bbf9a7 Compare October 16, 2020 07:46

qgroulard mentioned this pull request Oct 16, 2020

Migration to version 13.0 #783

Open

47 tasks

qgroulard force-pushed the 13.0-mig-partner_email_check-qgr branch 4 times, most recently from 117fdd9 to e40fa9c Compare October 16, 2020 09:34

[MIG] partner_email_check: Migration to 13.0

e356c0f

qgroulard force-pushed the 13.0-mig-partner_email_check-qgr branch from e40fa9c to e356c0f Compare October 16, 2020 09:40

simahawk requested changes Jul 2, 2021

View reviewed changes

simahawk mentioned this pull request Jul 2, 2021

[IMP] partner_email_check: Migration to v13, also adding Mx.po transl… #1097

Closed

rousseldenis added migration needs review labels Jul 2, 2021

rousseldenis added this to the 13.0 milestone Jul 2, 2021

NL66278 reviewed Jul 6, 2021

View reviewed changes

qgroulard force-pushed the 13.0-mig-partner_email_check-qgr branch 2 times, most recently from 3864e32 to e3dfcc8 Compare July 6, 2021 15:33

NL66278 approved these changes Jul 7, 2021

View reviewed changes

simahawk reviewed Jul 7, 2021

View reviewed changes

NL66278 reviewed Jul 7, 2021

View reviewed changes

[FIX] Remove email from copy data if duplicates not allowed

050a5f8

[REF] Partner email chack params on company [IMP] email_check [IMP] test partner_email_check use SavepointCase

qgroulard force-pushed the 13.0-mig-partner_email_check-qgr branch from e3dfcc8 to 1aa74d9 Compare July 7, 2021 16:01

[IMP] Add partner email check syntax flag

f42ad2f

qgroulard force-pushed the 13.0-mig-partner_email_check-qgr branch from 1aa74d9 to f42ad2f Compare July 7, 2021 16:07

simahawk approved these changes Jul 8, 2021

View reviewed changes

OCA-git-bot added the approved label Jul 8, 2021

OCA-git-bot added the bot is merging ⏳ label Jul 8, 2021

OCA-git-bot merged commit 9f45f5a into OCA:13.0 Jul 8, 2021

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Jul 8, 2021

CLaurelB pushed a commit to vauxoo-dev/partner-contact that referenced this pull request Mar 6, 2024

Merge PR OCA#989 into 15.0

34d0e71

Signed-off-by pedrobaeza

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[13.0][MIG] partner_email_check: Migration to 13.0 #989

[13.0][MIG] partner_email_check: Migration to 13.0 #989

qgroulard commented Oct 15, 2020

kos94ok-3D commented Mar 21, 2021

simahawk Jul 2, 2021

rousseldenis commented Jul 2, 2021

qgroulard commented Jul 2, 2021

NL66278 Jul 6, 2021

NL66278 left a comment

rousseldenis commented Jul 7, 2021

simahawk Jul 7, 2021

simahawk Jul 7, 2021

simahawk Jul 7, 2021 •

edited

Loading

qgroulard Jul 7, 2021 •

edited

Loading

simahawk Jul 7, 2021

simahawk Jul 7, 2021

NL66278 Jul 7, 2021

simahawk left a comment

simahawk commented Jul 8, 2021

OCA-git-bot commented Jul 8, 2021

OCA-git-bot commented Jul 8, 2021

	help="Don't allow multiple partners to have the same email address.",
	help="Don't allow multiple partners to have the same email address.",
	config_parameter="partner_email_check_filter_duplicates"

		@@ -0,0 +1,24 @@
		# Copyright 2021 ACSONE SA/NV
		# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).

[13.0][MIG] partner_email_check: Migration to 13.0 #989

[13.0][MIG] partner_email_check: Migration to 13.0 #989

Conversation

qgroulard commented Oct 15, 2020

kos94ok-3D commented Mar 21, 2021

simahawk Jul 2, 2021

Choose a reason for hiding this comment

rousseldenis commented Jul 2, 2021

qgroulard commented Jul 2, 2021

NL66278 Jul 6, 2021

Choose a reason for hiding this comment

NL66278 left a comment

Choose a reason for hiding this comment

rousseldenis commented Jul 7, 2021

simahawk Jul 7, 2021

Choose a reason for hiding this comment

simahawk Jul 7, 2021

Choose a reason for hiding this comment

simahawk Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

qgroulard Jul 7, 2021 • edited Loading

Choose a reason for hiding this comment

simahawk Jul 7, 2021

Choose a reason for hiding this comment

simahawk Jul 7, 2021

Choose a reason for hiding this comment

NL66278 Jul 7, 2021

Choose a reason for hiding this comment

simahawk left a comment

Choose a reason for hiding this comment

simahawk commented Jul 8, 2021

OCA-git-bot commented Jul 8, 2021

OCA-git-bot commented Jul 8, 2021

simahawk Jul 7, 2021 •

edited

Loading

qgroulard Jul 7, 2021 •

edited

Loading