Skip to content

Commit

Permalink
[FIX] mail: try to parse scheduled date input
Browse files Browse the repository at this point in the history
``scheduled_date`` field of ``mail.mail`` is a char field since its addition
in 2015 (see 364b4ba). As its first usage was in combination with
mail templates, a char field was used to simplify its implementation.

However this technically allows to store whatever value in that field. Using
it in a filter with a datetime argument is quite strange. A workaround is
to try to parse as much as possible the inputs, remove timezone information,
try to localize it, and have it in regular server format to enable filtering
on it.

We consider value should be set in UTC. If we have a specific timezone set
on the input we localize it to UTC. Otherwise we consider the input was done
in UTC, as all datetime fields. It is the role of the business code generating
mail.mail to either give the timezone, either already convert into UTC.

This might solve the following bug
  Step to reproduce:
	activate the developer mode
	go to settings - technical - emails
	create a new email and set the Scheduled Send Date in the future
	run the scheduled action Mail: Email Queue Manager
  Current behavior:
    the email is sent and the action does not consider the filter
  Expected behavior:
    the email is not sent and will only be sent when the scheduler detects
    that scheduled_date exceeds the current time.

Task-2833300
opw-2823106

closes #94770

Signed-off-by: Warnon Aurélien (awa) <awa@odoo.com>
  • Loading branch information
mafo-odoo authored and tde-banana-odoo committed Jun 28, 2022
1 parent 9d3d0d0 commit 582ac8d
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 9 deletions.
5 changes: 5 additions & 0 deletions addons/mail/i18n/mail.pot
Expand Up @@ -7467,6 +7467,11 @@ msgstr ""
msgid "Wrong operation name (%s)"
msgstr ""

#. module: mail
#: model_terms:ir.ui.view,arch_db:mail.view_mail_form
msgid "YYYY-MM-DD HH:MM:SS"
msgstr ""

#. module: mail
#. openerp-web
#: code:addons/mail/static/src/js/activity.js:0
Expand Down
63 changes: 57 additions & 6 deletions addons/mail/models/mail_mail.py
Expand Up @@ -9,8 +9,10 @@
import smtplib
import threading
import re
import pytz

from collections import defaultdict
from dateutil.parser import parse

from odoo import _, api, fields, models
from odoo import tools
Expand Down Expand Up @@ -87,7 +89,12 @@ def create(self, values_list):
for values in values_list:
if 'is_notification' not in values and values.get('mail_message_id'):
values['is_notification'] = True

if values.get('scheduled_date'):
parsed_datetime = self._parse_scheduled_datetime(values['scheduled_date'])
if parsed_datetime:
values['scheduled_date'] = parsed_datetime.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT)
else:
values['scheduled_date'] = False
new_mails = super(MailMail, self).create(values_list)

new_mails_w_attach = self
Expand All @@ -100,6 +107,12 @@ def create(self, values_list):
return new_mails

def write(self, vals):
if vals.get('scheduled_date'):
parsed_datetime = self._parse_scheduled_datetime(vals['scheduled_date'])
if parsed_datetime:
vals['scheduled_date'] = parsed_datetime.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT)
else:
vals['scheduled_date'] = False
res = super(MailMail, self).write(vals)
if vals.get('attachment_ids'):
for mail in self:
Expand Down Expand Up @@ -138,11 +151,13 @@ def process_email_queue(self, ids=None):
messages to send (by default all 'outgoing'
messages are sent).
"""
filters = ['&',
('state', '=', 'outgoing'),
'|',
('scheduled_date', '<', datetime.datetime.now()),
('scheduled_date', '=', False)]
filters = [
'&',
('state', '=', 'outgoing'),
'|',
('scheduled_date', '=', False),
('scheduled_date', '<=', datetime.datetime.utcnow()),
]
if 'filters' in self._context:
filters.extend(self._context['filters'])
# TODO: make limit configurable
Expand Down Expand Up @@ -201,6 +216,42 @@ def _postprocess_sent_message(self, success_pids, failure_reason=False, failure_
self.browse(mail_to_delete_ids).sudo().unlink()
return True

def _parse_scheduled_datetime(self, scheduled_datetime):
""" Taking an arbitrary datetime (either as a date, a datetime or a string)
try to parse it and return a datetime timezoned to UTC.
If no specific timezone information is given, we consider it as being
given in UTC, as all datetime values given to the server. Trying to
guess its timezone based on user or flow would be strange as this is
not standard. When manually creating datetimes for mail.mail scheduled
date, business code should ensure either a timezone info is set, either
it is converted into UTC.
Using yearfirst when parsing str datetimes eases parser's job when
dealing with the hard-to-parse trio (01/04/09 -> ?). In most use cases
year will be given first as this is the expected default formatting.
:return datetime: parsed datetime (or False if parser failed)
"""
if isinstance(scheduled_datetime, datetime.datetime):
parsed_datetime = scheduled_datetime
elif isinstance(scheduled_datetime, datetime.date):
parsed_datetime = datetime.combine(scheduled_datetime, datetime.time.min)
else:
try:
parsed_datetime = parse(scheduled_datetime, yearfirst=True)
except (ValueError, TypeError):
parsed_datetime = False
if parsed_datetime:
if not parsed_datetime.tzinfo:
parsed_datetime = pytz.utc.localize(parsed_datetime)
else:
try:
parsed_datetime = parsed_datetime.astimezone(pytz.utc)
except Exception:
pass
return parsed_datetime

# ------------------------------------------------------
# mail_mail formatting, tools and send mechanism
# ------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion addons/mail/views/mail_mail_views.xml
Expand Up @@ -27,7 +27,7 @@
<field name="recipient_ids" widget="many2many_tags"/>
<field name="email_cc"/>
<field name="reply_to"/>
<field name="scheduled_date"/>
<field name="scheduled_date" placeholder="YYYY-MM-DD HH:MM:SS"/>
</group>
<notebook>
<page string="Body" name="body">
Expand Down
60 changes: 58 additions & 2 deletions addons/test_mail/tests/test_mail_mail.py
Expand Up @@ -2,13 +2,17 @@
# Part of Odoo. See LICENSE file for full copyright and licensing details.

import psycopg2
import pytz

from datetime import datetime, timedelta
from freezegun import freeze_time
from unittest.mock import call

from odoo import api
from odoo import api, tools
from odoo.addons.base.tests.common import MockSmtplibCase
from odoo.addons.test_mail.tests.common import TestMailCommon
from odoo.tests import common, tagged
from odoo.tools import mute_logger
from odoo.tools import mute_logger, DEFAULT_SERVER_DATETIME_FORMAT


@tagged('mail_mail')
Expand Down Expand Up @@ -44,6 +48,7 @@ def test_mail_message_notify_from_mail_mail(self):
self.assertSentEmail(mail.env.user.partner_id, ['test@example.com'])
self.assertEqual(len(self._mails), 1)

@mute_logger('odoo.addons.mail.models.mail_mail')
def test_mail_mail_return_path(self):
# mail without thread-enabled record
base_values = {
Expand All @@ -65,6 +70,57 @@ def test_mail_mail_return_path(self):
mail.send()
self.assertEqual(self._mails[0]['headers']['Return-Path'], '%s@%s' % (self.alias_bounce, self.alias_domain))

@mute_logger('odoo.addons.mail.models.mail_mail', 'odoo.tests')
def test_mail_mail_schedule(self):
"""Test that a mail scheduled in the past/future are sent or not"""
now = datetime(2022, 6, 28, 14, 0, 0)
scheduled_datetimes = [
# falsy values
False, '', 'This is not a date format',
# datetimes (UTC/GMT +10 hours for Australia/Brisbane)
now, pytz.timezone('Australia/Brisbane').localize(now),
# string
(now - timedelta(days=1)).strftime(DEFAULT_SERVER_DATETIME_FORMAT),
(now + timedelta(days=1)).strftime(DEFAULT_SERVER_DATETIME_FORMAT),
(now + timedelta(days=1)).strftime("%H:%M:%S %d-%m-%Y"),
# tz: is actually 1 hour before now in UTC
(now + timedelta(hours=3)).strftime("%H:%M:%S %d-%m-%Y") + " +0400",
# tz: is actually 1 hour after now in UTC
(now + timedelta(hours=-3)).strftime("%H:%M:%S %d-%m-%Y") + " -0400",
]
expected_datetimes = [
False, '', False,
now, now - pytz.timezone('Australia/Brisbane').utcoffset(now),
now - timedelta(days=1), now + timedelta(days=1), now + timedelta(days=1),
now + timedelta(hours=-1),
now + timedelta(hours=1),
]
expected_states = [
# falsy values = send now
'sent', 'sent', 'sent',
'sent', 'sent',
'sent', 'outgoing', 'outgoing',
'sent', 'outgoing'
]

mails = self.env['mail.mail'].create([
{'body_html': '<p>Test</p>',
'email_to': 'test@example.com',
'scheduled_date': scheduled_datetime,
} for scheduled_datetime in scheduled_datetimes
])

for mail, expected_datetime, scheduled_datetime in zip(mails, expected_datetimes, scheduled_datetimes):
expected = expected_datetime.strftime(tools.DEFAULT_SERVER_DATETIME_FORMAT) if expected_datetime else expected_datetime
self.assertEqual(mail.scheduled_date, expected,
'Scheduled date: %s should be stored as %s, received %s' % (scheduled_datetime, expected, mail.scheduled_date))
self.assertEqual(mail.state, 'outgoing')

with freeze_time(now):
self.env['mail.mail'].process_email_queue()
for mail, expected_state in zip(mails, expected_states):
self.assertEqual(mail.state, expected_state)

@mute_logger('odoo.addons.mail.models.mail_mail')
def test_mail_mail_send_server(self):
"""Test that the mails are send in batch.
Expand Down

0 comments on commit 582ac8d

Please sign in to comment.