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

[8.0][ADD][website_snippet_mass_mailing_partner] Snippet + name #201

Closed

Conversation

yajo
Copy link
Member

@yajo yajo commented May 24, 2016

Let you have the name field in the mass mailing subscription snippet.

When somebody tells his name, a partner is linked (if a match is found) or created.

@Tecnativa.

@coveralls
Copy link

coveralls commented May 24, 2016

Coverage Status

Coverage decreased (-0.2%) to 43.17% when pulling 74cd693 on Tecnativa:8.0-website_snippet_mass_mailing_partner into 92a85d5 on OCA:8.0.

crm
Copy link
Member Author

Choose a reason for hiding this comment

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

@gurneyalex Runbot fails because it lacks this repo, could you add it please?

Copy link
Member

Choose a reason for hiding this comment

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

I have added it.

Let you have the name field in the mass mailing subscription snippet.

When somebody tells his name, a partner is linked (if a match is found) or created.
@yajo yajo force-pushed the 8.0-website_snippet_mass_mailing_partner branch from 74cd693 to 2034582 Compare May 25, 2016 07:33
@yajo
Copy link
Member Author

yajo commented May 25, 2016

Rebased to get update.

@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage decreased (-0.2%) to 43.17% when pulling 2034582 on Tecnativa:8.0-website_snippet_mass_mailing_partner into 92a85d5 on OCA:8.0.

@rafaelbn rafaelbn added this to the 8.0 milestone May 28, 2016
@rafaelbn
Copy link
Member

Tested. Something is wrong. Let's explain.

1 - Go to http://3147655-201-203458.runbot1.odoo-community.org/, in home page there is a snippet for Mailing List "Imported Contacts"
2 - Go to Sales -> Sales -> Customers and create a new one called AAA with email: email@email.com
3 - Log out
4 - Go to homepage and suscribe to Pepe (email@email.com)
5 - Again go to homepage and suscribe to Paco (email@email.com)
6 - Again (3 is OK) go to homepage and suscribe to Ana (email@email.com)
7 - Now, log ing as admin and goto Marketing -> Mailing list -> Imported contacts -> Recipients. You will just the last suscribed person, Ana (email@email.com). OK!
8 - Go to Sales -> Sales -> Customer and filter by 'email', you will chech that there are 4 res.partners with the same email.
BUG: It must be just one
2016-05-28_17-01-04

@pedrobaeza
Copy link
Member

No, @rafaelbn, that task is subrogated to the module mass_mailing_partner, that filters and matches partner_id by email, and avoid these duplicates.

("opt_out", "=", False),
])
partner = Partner.search(
[("email", "=ilike", email), ("name", "=ilike", name)],
Copy link
Member

Choose a reason for hiding this comment

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

Only match by email. The name can be easily twisted or typoed

Copy link
Member

Choose a reason for hiding this comment

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

This is not done by mass_mailing_partner?

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I was working around something that it should do, and now you have it in OCA/crm#96. Thanks for the point.

@pedrobaeza
Copy link
Member

This module should go on OCA/social

@yajo yajo closed this May 30, 2016
@yajo yajo deleted the 8.0-website_snippet_mass_mailing_partner branch May 30, 2016 11:28
@pedrobaeza
Copy link
Member

Why do you close this? Actually, the patch is not correct.

@coveralls
Copy link

coveralls commented May 30, 2016

Coverage Status

Coverage increased (+0.04%) to 43.4% when pulling fa8459e on Tecnativa:8.0-website_snippet_mass_mailing_partner into 92a85d5 on OCA:8.0.

@yajo
Copy link
Member Author

yajo commented May 30, 2016

You just told me to put it in OCA/social

@pedrobaeza
Copy link
Member

Sorry, I didn't remember, hehe...

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

4 participants