11.0 mig mass_mailing_list_dynamic #284
Conversation
damcar
changed the title
|
Thanks, setting as WIP until dependency is merged. |
* [FIX+IMP] mass_mailing_list_dynamic: tests, icons, filters... * Brand new icon * Added feature of loading an existing filter as criteria * Tests as SavepointCase for optimizing times * Tests in post-install for avoiding errors on res.partner not null constraints when several modules added them. * Updated documentation. * Fix mock in test for not commiting test data. * [FIX] mass_mailing_list_dynamic: Wasn't able to create contacts in fully synced lists Syncing context was being set in the wrong object. Added to test too. * [FIX] mass_mailing_list_dynamic: Allow to write back vals from res.partner Module mass_mailing_partner writes back certain values from partner to mass_mailing_contact. Module should allow that write operation.
- Adds is_synced field to track whether a dynamic list has unsynced changes or not so the user is aware that the definitive number of contacts is yet to be determined. - It fixes an issue that made impossible deleting a res.partner filter when a list had use it to filter contacts. - It also shows only the filters available for the user (shared and belonging to self).
pedrobaeza
force-pushed
the
11.0-mig-mass_mailing_list_dynamic
branch
from
ca3ab36
to
168e1c0
Compare
There was a problem hiding this comment.
About the change you have made on the behavior, I think it should be at least configurable, having 2 options:
- Always create new mass mailing contacts
- Try to reuse existing mass mailing contacts
The first one will use the behavior on previous version, and the second will use your new method.
Why this? Because there's something important in the mass mailing contacts management: the unsubscription. If you always reuse existing contacts, then if the contact has opt_out checked, it won't receive any mailing, so the dynamic list won't serve at all.
That's why I would even say that the default behavior should be the first one.
What do you think?
| for one in self: | ||
| if any((list.dynamic and | ||
| list.sync_method == "full") for list in one.list_ids): | ||
| # if any((one.list_id.dynamic and |
| # one.list_id.sync_method == "full") for one in self): | ||
| raise ValidationError( | ||
| _("Cannot edit manually contacts in a fully " | ||
| "synchronized list. Change its sync method or execute " |
| ]).unlink() | ||
| current_contacts = Contact.search([("list_id", "=", one.id)]) | ||
| current_partners = current_contacts.mapped("partner_id") | ||
| contacts = Contact.search([("list_ids", "in", [one.id]), |
There was a problem hiding this comment.
Why changing the style in this search? This need one extra line for the same arguments.
There was a problem hiding this comment.
you're right, it's not necessary!
|
@damcar have you seen my comments? |
|
@pedrobaeza sorry for the response time! |
| for contact in contacts: | ||
| # No delete contacts with other lists | ||
| if len(contact.list_ids) > 1: | ||
| contact.write({'list_ids': [(3, one.id, False)]}) |
There was a problem hiding this comment.
You might prefer to do contact.list_ids -= one
| # Add list in existing contacts | ||
| for contact in current_contacts_not_in_list: | ||
| contact.write({ | ||
| "list_ids": [(4, one.id, False)], |
There was a problem hiding this comment.
Here you might also prefer contact.list_ids |= one
|
Hello! Thanks fo this contribution! 😄 Travis is 🔴 runbot is 🔴 this both should be fixed In this kinf of migration the most important is thinking that there are customer using this in v10 in production and when we migrate them to v11 the modules should make the same, could have improvements but never change behavior. |
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
github-actions
bot
added
the
stale
Need mass_mailing_partner (dependency) module to works.