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

[10.0][FIX][mass_mailing_list_dynamic] Fixed sync domain problem with escape characters. Added context… #384

Open
wants to merge 1 commit into
base: 10.0
from

Conversation

@wpichler
Copy link

commented May 21, 2019

… variable auto_created=True when creating contacts. So a possible send double opt in mail function can check for this context variable

@wpichler wpichler force-pushed the Callino:10.0-fix_escape_char_mail_mailing_list_dynamic-WP branch 2 times, most recently from 29b4585 to 5333997 May 21, 2019
@pedrobaeza pedrobaeza added this to the 10.0 milestone May 21, 2019
@@ -3,7 +3,7 @@
# License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl).

from odoo import api, fields, models
from odoo.tools import safe_eval
from odoo.tools.safe_eval import safe_eval

This comment has been minimized.

Copy link
@pedrobaeza

pedrobaeza May 21, 2019

Member

Why changing this? It was working before.

This comment has been minimized.

Copy link
@wpichler

wpichler Sep 5, 2019

Author

correct - was working before - i thought it would be better to directly only import the function... - i will undo this

@@ -55,7 +55,7 @@ def action_sync(self):
current_partners = current_contacts.mapped("partner_id")
# Add new contacts
for partner in desired_partners - current_partners:
Contact.create({
Contact.with_context(auto_created=True).create({

This comment has been minimized.

Copy link
@pedrobaeza

pedrobaeza May 21, 2019

Member

Can you provide here a comment pointing to why it's done this way. I have read your main comment and I still don't understand it...

This comment has been minimized.

Copy link
@wpichler

wpichler Sep 5, 2019

Author

in an other module i extend the create function of mail.mass_mailing.contact to automatically send a double opt in mail to the new created contact. To be able to not send it, in the case the contact got created in some other way - i've added this context var.

This comment has been minimized.

Copy link
@Yajo

Yajo Sep 9, 2019

Member

Which module?

@hbrunn

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

please rebase

@rafaelbn rafaelbn changed the title [FIX] Fixed sync domain problem with escape characters. Added context… [10.0][FIX][mass_mailing_list_dynamic] Fixed sync domain problem with escape characters. Added context… Sep 4, 2019
@rafaelbn

This comment has been minimized.

Copy link
Member

commented Sep 4, 2019

H @wpichler , thanks for this contribution. Could you please rebase and answer questions from the review please? Best!

@rafaelbn rafaelbn requested review from Yajo and chienandalu Sep 4, 2019
@rafaelbn rafaelbn closed this Sep 4, 2019
@rafaelbn rafaelbn reopened this Sep 4, 2019
@rafaelbn rafaelbn requested a review from OCA-git-bot Sep 4, 2019
@wpichler wpichler force-pushed the Callino:10.0-fix_escape_char_mail_mailing_list_dynamic-WP branch from 5333997 to a16dd5f Sep 5, 2019
… variable auto_created=True when creating contacts. So a possible send double opt in mail function can check for this context variable
@wpichler wpichler force-pushed the Callino:10.0-fix_escape_char_mail_mailing_list_dynamic-WP branch from a16dd5f to 8f8b0c5 Sep 5, 2019
@@ -43,7 +42,7 @@ def action_sync(self):
# Skip non-dynamic lists
dynamic = self.filtered("dynamic")
for one in dynamic:
sync_domain = safe_eval(one.sync_domain) + [("email", "!=", False)]
sync_domain = safe_eval(one.sync_domain.replace('u\'', '\'')) + [("email", "!=", False)]

This comment has been minimized.

Copy link
@Yajo

Yajo Sep 9, 2019

Member

Nothing forbids you to use unicode strings to search AFAIK, so this change doesn't seem to be actually fixing something.

OTOH, it's introducing a new real bug, where a domain like [('name', '=', 'Maru')] would be converted to [('name', '=', 'Mar')] and produce unexpected results.

I bet you're trying to fix a bug in the wrong place.

@@ -55,7 +55,7 @@ def action_sync(self):
current_partners = current_contacts.mapped("partner_id")
# Add new contacts
for partner in desired_partners - current_partners:
Contact.create({
Contact.with_context(auto_created=True).create({

This comment has been minimized.

Copy link
@Yajo

Yajo Sep 9, 2019

Member

Which module?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.