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

[MIG][12.0] mass_mailing_newsletter_welcome_mail #452

Merged

Conversation

ernestotejeda
Copy link
Member

Cc @Tecnativa TT19367

cristinamartinrod and others added 4 commits October 24, 2019 11:06
- Allow user to customize template.
- Only react when someone subscribes from the website. Not when creating a contact from the backend.
- Proper usage of context values.
- Full instructions.
@ernestotejeda ernestotejeda force-pushed the 12.0-mig-mass_mailing_newsletter_welcome_mail branch from f1df1bc to 33eda91 Compare October 24, 2019 19:43
@pedrobaeza pedrobaeza added this to the 12.0 milestone Oct 25, 2019
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 25, 2019
24 tasks
@pedrobaeza pedrobaeza requested a review from Tardo October 25, 2019 12:17
('email', '=', email),
("opt_out", "=", False),
contact_obj = request.env["mail.mass_mailing.contact"].with_context(
default_list_ids=[list_id])
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

Copy link
Member

Choose a reason for hiding this comment

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

Can you please answer about this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now in v12 the contacts are not opted out by themselves, but they are opted out or not from a particular list. One way to search contacts that are opted out from a particular list is that, by searching by the opt_oup field of mass.mailing.contact (which is now a calculated field) and passing in the context the list id.
check this
https://github.com/odoo/odoo/blob/fe35870fa0024a1e8adebb386f343f25fdea2c7b/addons/mass_mailing/models/mass_mailing.py#L250
On the other hand, I assumed that so in v11 we didn't want to send mail to contacts that had been opted out from the list, now in v12 that blacklisting exists, we don't whant either to send mail to those who are blacklisted

Copy link
Member

Choose a reason for hiding this comment

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

Well, if they are subscribing just now, how they are going to be opted-out?

If you keep it (and for the rest of cases), please always explain this not easy to deduce things as a comment in the code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe in Odoo 11.0 this is not necessary but in v12 yes because the re-subscription of a contact doesn't work well. I made a issue to Odoo for this: odoo/odoo#39604

Copy link
Member

Choose a reason for hiding this comment

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

OK, then please add that as comment:

# Needed for being re-subscribed while odoo/odoo#39604 is not fixed

mass_mailing_newsletter_welcome_mail/controllers/main.py Outdated Show resolved Hide resolved
Copy link
Member

@Tardo Tardo left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@ernestotejeda ernestotejeda force-pushed the 12.0-mig-mass_mailing_newsletter_welcome_mail branch from 33eda91 to 735b9e7 Compare October 25, 2019 19:40
@ernestotejeda
Copy link
Member Author

Changes done

@ernestotejeda ernestotejeda force-pushed the 12.0-mig-mass_mailing_newsletter_welcome_mail branch from 735b9e7 to 950518a Compare October 30, 2019 21:02
@pedrobaeza
Copy link
Member

/ocabot merge

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 12.0-ocabot-merge-pr-452-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Oct 31, 2019
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit 950518a into OCA:12.0 Oct 31, 2019
@OCA-git-bot
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants