Skip to content
Permalink
Browse files

[IMP] mass_mailing_(sms): Add unblacklisting button

Previously a blacklisted phone number or email address could only be
removed from their respective blacklist by going to the corresponding
blacklist view [in mass_mailing_(sms)] and manually
archiving/deactiviting the entry. All users could see that an email was
blacklisted via an fa-ban icon added next to their corresponding field
in the modules: crm, mass_mailing, mass_mailing_sms, and contacts (via
extension).

This commit adds additional fa-ban icons next to blacklisted phone
numbers and makes all instances of these icons clickable to remove them
from their corresponding blacklist using a wizard. There are several
limitations to this implementation including:

1. If both a mobile and phone number field appear within a crm lead
instance, then only the mobile field will indicate if it is blacklisted
(due to current implementation of PhoneMixin which only checks 1 phone
value against the blacklist and "sms" which returns mobile numbers first).
I.e. if a phone field value is blacklisted, but a value is typed into the
mobile field, then the phone field will never indicate that it is blacklisted.

2. If someone clicks on a unblacklisting icon after changing the
corresponding field value without clicking off the field input, then the
latest typed in value will be the one submitted for unblacklisting,
which may not actually be blacklisted (due to "is_blacklisted" flag
being a computed field and it not having a chance to re-compute to hide
the button).

3. If someone changes values in the blacklist, then someone who already
has a form open will not see icon disappear until something triggers a
re-compute of the "phone_sanitized_blacklisted" flag (this was already the
case with the icon, but now a user may try to unblacklist a value that is
already unblacklisted.)

4. Since the icon needs to be visible by everyone to indicate whether or
not a phone/email is blacklisted, users without corresponding unblacklist
permissions will be able to click on the icon. When they click on it, they
will be informed they do not have permission to unblacklist. A wizard was
determined to be the best way to implement this unblacklisting for the
following reasons:
  - A "Unblacklisting Reason" is needed and will not be stored as a field.
  - A field widget is unable to do this due to security settings +
    inability to refresh view after unblacklisting with a
    "Unblacklisting Reason" without wiping unsaved changes.

Additionally, to better align with GDPR, the corresponding form view for
the phone/email blacklist has been updated to use the same wizard to
track "Reason for unblacklisting". Unfortunately there is no
straightforward way to prevent direct "Archive" action, so users are
still able to bypass unblacklisting without being asked for a "reason
for unblacklisting".

Supports task: 2117635
Upgrade PR: odoo/upgrade#939
COM PR: #45315
  • Loading branch information
ticodoo committed Feb 13, 2020
1 parent d8e883f commit b5eecba8816e646261c581dd39b6b320bed980f5
Showing with 303 additions and 56 deletions.
  1. +1 −0 addons/crm/models/crm_lead.py
  2. +54 −11 addons/crm/views/crm_lead_views.xml
  3. +1 −0 addons/mail/__manifest__.py
  4. +34 −1 addons/mail/models/mail_blacklist.py
  5. +1 −0 addons/mail/security/ir.model.access.csv
  6. +10 −1 addons/mail/views/mail_blacklist_views.xml
  7. +4 −2 addons/mail/views/res_partner_views.xml
  8. +1 −0 addons/mail/wizard/__init__.py
  9. +15 −0 addons/mail/wizard/mail_blacklist_remove.py
  10. +19 −0 addons/mail/wizard/mail_blacklist_remove_view.xml
  11. +1 −0 addons/mass_mailing/security/ir.model.access.csv
  12. +5 −3 addons/mass_mailing/views/mailing_contact_views.xml
  13. +2 −2 addons/mass_mailing_sms/models/mailing_mailing.py
  14. +1 −0 addons/mass_mailing_sms/security/ir.model.access.csv
  15. +1 −1 addons/mass_mailing_sms/tests/test_mailing_internals.py
  16. +8 −6 addons/mass_mailing_sms/views/mailing_contact_views.xml
  17. +1 −0 addons/phone_validation/__init__.py
  18. +1 −0 addons/phone_validation/__manifest__.py
  19. +40 −7 addons/phone_validation/models/mail_thread_phone.py
  20. +25 −4 addons/phone_validation/models/phone_blacklist.py
  21. +1 −0 addons/phone_validation/security/ir.model.access.csv
  22. +3 −3 addons/phone_validation/tools/phone_validation.py
  23. +10 −1 addons/phone_validation/views/phone_blacklist_views.xml
  24. +4 −0 addons/phone_validation/wizard/__init__.py
  25. +15 −0 addons/phone_validation/wizard/phone_blacklist_remove.py
  26. +19 −0 addons/phone_validation/wizard/phone_blacklist_remove_view.xml
  27. +17 −5 addons/sms/views/res_partner_views.xml
  28. +9 −9 addons/test_mail_full/tests/test_phone_blacklist.py
@@ -54,6 +54,7 @@ class Lead(models.Model):
_order = "priority desc, id desc"
_inherit = ['mail.thread.cc',
'mail.thread.blacklist',
'mail.thread.phone',
'mail.activity.mixin',
'utm.mixin',
'format.address.mixin',
@@ -130,20 +130,33 @@
/>
<field name="is_blacklisted" invisible="1"/>
<field name="partner_is_blacklisted" invisible="1"/>
<field name="phone_blacklisted" invisible="1"/>
<field name="mobile_blacklisted" invisible="1"/>
<field name="email_state" invisible="1"/>
<field name="phone_state" invisible="1"/>
<label for="email_from" class="oe_inline"/>
<div class="o_row o_row_readonly">
<i class="fa fa-ban" style="color: red;" role="img" title="This email is blacklisted for mass mailings"
aria-label="Blacklisted" attrs="{'invisible': ['|', ('is_blacklisted', '=', False), ('partner_address_email', '!=', False)]}" groups="base.group_user"/>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': email_from}" groups="base.group_user"
attrs="{'invisible': ['|', ('is_blacklisted', '=', False), ('partner_address_email', '!=', False)]}"/>
<field name="email_from"
attrs="{'invisible': [('partner_address_email', '!=', False)]}" string="Email" widget="email"/>
<i class="fa fa-ban" style="color: red;" role="img" title="This email is blacklisted for mass mailings"
aria-label="Blacklisted" attrs="{'invisible': ['|', ('partner_is_blacklisted', '=', False), ('partner_address_email', '=', False)]}" groups="base.group_user"/>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': partner_address_email}" groups="base.group_user"
attrs="{'invisible': ['|', ('partner_is_blacklisted', '=', False), ('partner_address_email', '=', False)]}"/>
<field name="partner_address_email"
attrs="{'invisible': [('partner_address_email', '=', False)]}" widget="email" string="Email"/>
</div>
<field name="phone" widget="phone"/>
<label for="phone" class="oe_inline"/>
<div class="o_row o_row_readonly">
<button name="phone_action_blacklist_remove" class="fa fa-ban text-danger"
title="This phone number is blacklisted for SMS Marketing. Click to unblacklist."
type="object" context="{'default_phone': phone}" groups="base.group_user"
attrs="{'invisible': [('phone_blacklisted', '=', False)]}"/>
<field name="phone" widget="phone"/>
</div>
</group>
<group name="lead_info" attrs="{'invisible': [('type', '=', 'opportunity')]}">
<label for="contact_name"/>
@@ -156,14 +169,37 @@
<field name="phone_state" invisible="1"/>
<label for="email_from" class="oe_inline"/>
<div class="o_row o_row_readonly">
<i class="fa fa-ban" style="color: red;" role="img" title="This email is blacklisted for mass mailings"
aria-label="Blacklisted" attrs="{'invisible': [('is_blacklisted', '=', False)]}" groups="base.group_user"/>
<field name="email_from" widget="email"/>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': email_from}" groups="base.group_user"
attrs="{'invisible': ['|', ('is_blacklisted', '=', False), ('partner_address_email', '!=', False)]}"/>
<field name="email_from"
attrs="{'invisible': [('partner_address_email', '!=', False)]}" string="Email" widget="email"/>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': partner_address_email}" groups="base.group_user"
attrs="{'invisible': ['|', ('partner_is_blacklisted', '=', False), ('partner_address_email', '=', False)]}"/>
<field name="partner_address_email"
attrs="{'invisible': [('partner_address_email', '=', False)]}" widget="email" string="Email"/>
</div>
<field name="email_cc" groups="base.group_no_one"/>
<field name="function"/>
<field name="phone" widget="phone"/>
<field name="mobile" widget="phone"/>
<label for="phone" class="oe_inline"/>
<div class="o_row o_row_readonly">
<button name="phone_action_blacklist_remove" class="fa fa-ban text-danger"
title="This phone number is blacklisted for SMS Marketing. Click to unblacklist."
type="object" context="{'default_phone': phone}" groups="base.group_user"
attrs="{'invisible': [('phone_blacklisted', '=', False)]}"/>
<field name="phone" widget="phone"/>
</div>
<label for="mobile" class="oe_inline"/>
<div class="o_row o_row_readonly">
<button name="phone_action_blacklist_remove" class="fa fa-ban text-danger"
title="This phone number is blacklisted for SMS Marketing. Click to unblacklist."
type="object" context="{'default_phone': mobile}" groups="base.group_user"
attrs="{'invisible': [('mobile_blacklisted', '=', False)]}"/>
<field name="mobile" widget="phone" string="Mobile"/>
</div>
</group>
<group attrs="{'invisible': [('type', '=', 'lead')]}">
<field name="date_deadline"/>
@@ -234,7 +270,14 @@
<field name="title" placeholder="Title" domain="[]" options='{"no_open": True}'/>
</div>
<field name="function"/>
<field name="mobile" widget="phone"/>
<label for="mobile" class="oe_inline"/>
<div class="o_row o_row_readonly">
<button name="phone_action_blacklist_remove" class="fa fa-ban text-danger"
title="This phone number is blacklisted for SMS Marketing. Click to unblacklist."
type="object" context="{'default_phone': mobile}" groups="base.group_user"
attrs="{'invisible': [('mobile_blacklisted', '=', False)]}"/>
<field name="mobile" widget="phone"/>
</div>
</group>
<group string="Marketing">
<field name="campaign_id" />
@@ -11,6 +11,7 @@
'data': [
'views/mail_menus.xml',
'wizard/invite_view.xml',
'wizard/mail_blacklist_remove_view.xml',
'wizard/mail_compose_message_view.xml',
'wizard/mail_resend_cancel_views.xml',
'wizard/mail_resend_message_views.xml',
@@ -2,7 +2,7 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from odoo import api, fields, models, tools, _
from odoo.exceptions import UserError
from odoo.exceptions import AccessError, UserError


class MailBlackList(models.Model):
@@ -78,6 +78,13 @@ def _add(self, email):
record = self.create({'email': email})
return record

def action_remove_with_reason(self, email, reason=None):
record = self._remove(email)
if reason:
record.message_post(body=_("Unblacklisting Reason: %s" % (reason)))

return record

def _remove(self, email):
normalized = tools.email_normalize(email)
record = self.env["mail.blacklist"].with_context(active_test=False).search([('email', '=', normalized)])
@@ -87,6 +94,17 @@ def _remove(self, email):
record = record.create({'email': email, 'active': False})
return record

def mail_action_blacklist_remove(self):
return {
'name': 'Are you sure you want to unblacklist this Email Address?',
'type': 'ir.actions.act_window',
'view_mode': 'form',
'res_model': 'mail.blacklist.remove',
'target': 'new',
}

def action_add(self):
self._add(self.email)

class MailBlackListMixin(models.AbstractModel):
""" Mixin that is inherited by all model with opt out. This mixin stores a normalized
@@ -192,3 +210,18 @@ def _message_reset_bounce(self, email):
bounce counter of the record. """
super(MailBlackListMixin, self)._message_reset_bounce(email)
self.write({'message_bounce': 0})

def mail_action_blacklist_remove(self):
# wizard access rights currently not working as expected and allows users without access to
# open this wizard, therefore we check to make sure they have access before the wizard opens.
can_access = self.env['mail.blacklist'].check_access_rights('write', raise_exception=False)
if can_access:
return {
'name': 'Are you sure you want to unblacklist this Email Address?',
'type': 'ir.actions.act_window',
'view_mode': 'form',
'res_model': 'mail.blacklist.remove',
'target': 'new',
}
else:
raise AccessError("You do not have the access right to unblacklist emails. Please contact your administrator.")
@@ -45,3 +45,4 @@ access_mail_resend_cancel,access.mail.resend.cancel,model_mail_resend_cancel,bas
access_mail_resend_message,access.mail.resend.message,model_mail_resend_message,base.group_user,1,1,1,0
access_mail_resend_partner,access.mail.resend.partner,model_mail_resend_partner,base.group_user,1,1,1,0
access_mail_template_preview,access.mail.template.preview,model_mail_template_preview,base.group_user,1,1,1,0
access_mail_blacklist_remove_system,acesss.mail.blacklist.remove.system,model_mail_blacklist_remove,base.group_system,1,1,1,1
@@ -16,12 +16,21 @@
<field name="model">mail.blacklist</field>
<field name="arch" type="xml">
<form string="Add Email Blacklist" duplicate="false" edit="false">
<header>
<button name="mail_action_blacklist_remove" string="Unblacklist"
type="object" class="oe_highlight" context="{'default_email': email}"
attrs="{'invisible': ['|', ('active', '=', False), ('email', '=', False)]}"/>
<button name="action_add" string="Blacklist"
type="object" class="oe_highlight"
attrs="{'invisible': ['|', ('active', '=', True), ('email', '=', False)]}"/>
</header>
<sheet>
<widget name="web_ribbon" title="Archived" bg_color="bg-danger" attrs="{'invisible': [('active', '=', True)]}"/>
<group>
<group>
<field name="email"/>
<field name="active" widget="boolean_toggle"/>
<field name="active" readonly="1"/>
<br/>
</group>
</group>
</sheet>
@@ -11,8 +11,10 @@
<field name="is_blacklisted" invisible="1"/>
<label for="email" class="oe_inline"/>
<div class="o_row o_row_readonly">
<i class="fa fa-ban" style="color: red;" role="img" title="This email is blacklisted for mass mailings"
aria-label="Blacklisted" attrs="{'invisible': [('is_blacklisted', '=', False)]}" groups="base.group_user"></i>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': email}" groups="base.group_user"
attrs="{'invisible': [('is_blacklisted', '=', False)]}"/>
<field name="email" widget="email"/>
</div>
</xpath>
@@ -2,6 +2,7 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.

from . import invite
from . import mail_blacklist_remove
from . import mail_compose_message
from . import mail_resend_cancel
from . import mail_resend_message
@@ -0,0 +1,15 @@
# -*- coding: utf-8 -*-

from odoo import fields, models


class MailBlacklistRemove(models.TransientModel):
_name = 'mail.blacklist.remove'
_description = 'Remove email from blacklist wizard'

email = fields.Char(name="Email", readonly=True, required=True)
reason = fields.Char(name="Reason")

def action_unblacklist_apply(self):
blacklist = self.env['mail.blacklist']
return blacklist.action_remove_with_reason(self.email, self.reason)
@@ -0,0 +1,19 @@
<?xml version="1.0"?>
<odoo>
<record id="mail_blacklist_remove_view_form" model="ir.ui.view">
<field name="name">mail.blacklist.remove.form</field>
<field name="model">mail.blacklist.remove</field>
<field name="arch" type="xml">
<form string="mail_blacklist_removal">
<group class="oe_title">
<field name="email" string="Email Address"/>
<field name="reason" string="Reason"/>
</group>
<footer>
<button name="action_unblacklist_apply" string="Confirm" type="object" class="btn-primary"/>
<button string="Discard" class="btn-secondary" special="cancel"/>
</footer>
</form>
</field>
</record>
</odoo>
@@ -13,6 +13,7 @@ access_mailing_trace_report_mm_user,access.mailing.trace.report.mm.user,model_ma
access_utm_source,access_utm_source,utm.model_utm_source,mass_mailing.group_mass_mailing_user,1,1,1,0
access_ir_mail_server,access_ir_mail_server,base.model_ir_mail_server,mass_mailing.group_mass_mailing_user,1,0,0,0
access_mail_blacklist_mass_mailing_user,access.mail.blacklist.mass_mailing_user,mail.model_mail_blacklist,mass_mailing.group_mass_mailing_user,1,1,1,1
access_mail_blacklist_remove_mass_mailing_user,acesss.mail.blacklist.remove.mass_mailing_user,mail.model_mail_blacklist_remove,mass_mailing.group_mass_mailing_user,1,1,1,1
access_link_tracker_mailing,access.link.tracker.mailing,link_tracker.model_link_tracker,mass_mailing.group_mass_mailing_user,1,1,1,1
access_mailing_list_merge,access.mailing.list.merge,model_mailing_list_merge,mass_mailing.group_mass_mailing_user,1,1,1,0
access_mailing_mailing_schedule_date,access.mailing.mailing.schedule.date,model_mailing_mailing_schedule_date,mass_mailing.group_mass_mailing_user,1,1,1,0
@@ -13,7 +13,7 @@
<field name="is_blacklisted" invisible="1"/>
<label for="contact_id" class="oe_inline"/>
<div class="o_row o_row_readonly">
<i class="fa fa-ban" style="color: red;" role="img" title="This email is blacklisted for mass mailings"
<i class="fa fa-ban text-danger" role="img" title="This email is blacklisted for mass mailings"
aria-label="Blacklisted" attrs="{'invisible': [('is_blacklisted', '=', False)]}" groups="base.group_user"></i>
<field name="contact_id"/>
</div>
@@ -164,8 +164,10 @@
<group>
<label for="email" class="oe_inline"/>
<div class="o_row o_row_readonly" name="email_details">
<i class="fa fa-ban" style="color: red;" role="img" title="This email is blacklisted for mass mailings"
aria-label="Blacklisted" attrs="{'invisible': [('is_blacklisted', '=', False)]}" groups="base.group_user"></i>
<button name="mail_action_blacklist_remove" class="fa fa-ban text-danger"
title="This email is blacklisted for mass mailings. Click to unblacklist."
type="object" context="{'default_email': email}" groups="base.group_user"
attrs="{'invisible': [('is_blacklisted', '=', False)]}"/>
<field name="email" widget="email"/>
<field name="is_blacklisted" invisible="1"/>
</div>
@@ -217,7 +217,7 @@ def action_send_sms(self, res_ids=None):

def _get_default_mailing_domain(self):
mailing_domain = super(Mailing, self)._get_default_mailing_domain()
if self.mailing_type == 'sms' and 'phone_blacklisted' in self.env[self.mailing_model_name]._fields:
mailing_domain = expression.AND([mailing_domain, [('phone_blacklisted', '=', False)]])
if self.mailing_type == 'sms' and 'phone_sanitized_blacklisted' in self.env[self.mailing_model_name]._fields:
mailing_domain = expression.AND([mailing_domain, [('phone_sanitized_blacklisted', '=', False)]])

return mailing_domain

0 comments on commit b5eecba

Please sign in to comment.
You can’t perform that action at this time.