From aaee5dab51b78225f319771a9950ae86b2d567ff Mon Sep 17 00:00:00 2001 From: Leonardo Pavan Rocha Date: Wed, 3 Jan 2024 13:06:45 +0100 Subject: [PATCH] [FIX] calendar{_sms}: alarm manager not creating cron trigger in recurrences After https://github.com/odoo/odoo/pull/118738, we start generating a single cron trigger per recurrence, however it fails to create the cron trigger for daily recurrences. The main issue happens when the cron is run after the first trigger of the recurrence. Imagine we have an alarm of 5 minutes before the event, the call for_setup_alarm from _send_reminder will try to get the next event in the recurrency to notify. However, the SQL query uses `WHERE start > now` and if the cron trigger runs before the event is started (most cases) this query will return the event that we're notifying. This commit fixes this by checking event by event which is the next alarm tha should be triggered and passing that date in the context of the `_setup_alarms` call. How to reproduce? Install calendar - Create a recurring event with daily recurrence repeating for 3 days. Invite Demo, set an email alarm for 5 min before the event and create the event - A cron trigger will be created for 5 min before the event - Change your computer's time so that it is 5min before the event - The cron trigger will be called and it should create a new cron trigger for the other day, however no cron trigger is created Issue: SQL query in _setup_alarms in calendar.recurrence is getting the first event with start > now, however it returns the event that was already notified Solution: process event by event and check if its last alarm was already set. If it was, we'll generate an alarm for the next event. If not, we'll use the same event for the _setup_alarms call For SMS alarms, we would not even create triggers for recurring events, so this commit also fixes that. task-3663455 closes odoo/odoo#151623 X-original-commit: 2da5bf6e9184db23836ea458a1fc53e27be49a1e Signed-off-by: Arnaud Joset (arj) Signed-off-by: Leonardo Pavan Rocha --- .../calendar/models/calendar_alarm_manager.py | 15 +++++--- addons/calendar/models/calendar_event.py | 13 +++++++ addons/calendar/models/calendar_recurrence.py | 10 ++--- .../tests/test_event_notifications.py | 37 +++++++++++++++++++ .../models/calendar_alarm_manager.py | 3 ++ 5 files changed, 66 insertions(+), 12 deletions(-) diff --git a/addons/calendar/models/calendar_alarm_manager.py b/addons/calendar/models/calendar_alarm_manager.py index a568d877c5e..b21efc74ed7 100644 --- a/addons/calendar/models/calendar_alarm_manager.py +++ b/addons/calendar/models/calendar_alarm_manager.py @@ -2,8 +2,9 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import logging -from datetime import timedelta +from datetime import timedelta, datetime from dateutil.relativedelta import relativedelta +from pytz import UTC from odoo import api, fields, models from odoo.tools import plaintext2html @@ -152,6 +153,7 @@ class AlarmManager(models.AbstractModel): already. """ lastcall = self.env.context.get('lastcall', False) or fields.date.today() - relativedelta(weeks=1) + now = datetime.now(tz=UTC) self.env.cr.execute(''' SELECT "alarm"."id", "event"."id" FROM "calendar_event" AS "event" @@ -163,8 +165,8 @@ class AlarmManager(models.AbstractModel): "alarm"."alarm_type" = %s AND "event"."active" AND "event"."start" - CAST("alarm"."duration" || ' ' || "alarm"."interval" AS Interval) >= %s - AND "event"."start" - CAST("alarm"."duration" || ' ' || "alarm"."interval" AS Interval) < now() at time zone 'utc' - )''', [alarm_type, lastcall]) + AND "event"."start" - CAST("alarm"."duration" || ' ' || "alarm"."interval" AS Interval) < %s + )''', [alarm_type, lastcall, now]) events_by_alarm = {} for alarm_id, event_id in self.env.cr.fetchall(): @@ -190,8 +192,11 @@ class AlarmManager(models.AbstractModel): alarm.mail_template_id, force_send=True ) - # Create cron trigger for next recurring events - events.recurrence_id._setup_alarms(recurrence_update=True) + + for event in events: + if event.recurrence_id: + next_date = event.get_next_alarm_date(events_by_alarm) + event.recurrence_id.with_context(date=next_date)._setup_alarms() @api.model def get_next_notif(self): diff --git a/addons/calendar/models/calendar_event.py b/addons/calendar/models/calendar_event.py index e4123fa054f..ab855016bc3 100644 --- a/addons/calendar/models/calendar_event.py +++ b/addons/calendar/models/calendar_event.py @@ -1005,6 +1005,19 @@ class Meeting(models.Model): self.env['calendar.alarm_manager']._notify_next_alarm(events_to_notify.partner_ids.ids) return triggers_by_events + def get_next_alarm_date(self, events_by_alarm): + self.ensure_one() + now = fields.datetime.now() + sorted_alarms = self.alarm_ids.sorted("duration_minutes") + triggered_alarms = sorted_alarms.filtered(lambda alarm: alarm.id in events_by_alarm)[0] + event_has_future_alarms = sorted_alarms[0] != triggered_alarms + next_date = None + if self.recurrence_id.trigger_id and self.recurrence_id.trigger_id.call_at <= now: + next_date = self.start - timedelta(minutes=sorted_alarms[0].duration_minutes) \ + if event_has_future_alarms \ + else self.start + return next_date + # ------------------------------------------------------------ # RECURRENCY # ------------------------------------------------------------ diff --git a/addons/calendar/models/calendar_recurrence.py b/addons/calendar/models/calendar_recurrence.py index d99fb242134..fad03b3a101 100644 --- a/addons/calendar/models/calendar_recurrence.py +++ b/addons/calendar/models/calendar_recurrence.py @@ -254,14 +254,10 @@ class RecurrenceRule(models.Model): :param recurrence_update: boolean: if true, update all recurrences in self, else only the recurrences without trigger """ - now = fields.Datetime.now() + now = self.env.context.get('date') or fields.Datetime.now() # get next events self.env['calendar.event'].flush_model(fnames=['recurrence_id', 'start']) - if recurrence_update: - recurrence = self - else: - recurrence = self.filtered(lambda rec: not rec.trigger_id) - if not recurrence.calendar_event_ids.ids: + if not self.calendar_event_ids.ids: return self.env.cr.execute(""" @@ -270,7 +266,7 @@ class RecurrenceRule(models.Model): WHERE start > %s AND id IN %s ORDER BY recurrence_id,start ASC; - """, (now, tuple(recurrence.calendar_event_ids.ids))) + """, (now, tuple(self.calendar_event_ids.ids))) result = self.env.cr.dictfetchall() if not result: return diff --git a/addons/calendar/tests/test_event_notifications.py b/addons/calendar/tests/test_event_notifications.py index e696226203c..a0c811446c5 100644 --- a/addons/calendar/tests/test_event_notifications.py +++ b/addons/calendar/tests/test_event_notifications.py @@ -268,6 +268,43 @@ class TestEventNotifications(TransactionCase, MailCase, CronMixinCase): self.env.flush_all() self.assertEqual(len(capt.records), 1) + def test_email_alarm_daily_recurrence(self): + # test email alarm is sent correctly on daily recurrence + alarm = self.env['calendar.alarm'].create({ + 'name': 'Alarm', + 'alarm_type': 'email', + 'interval': 'minutes', + 'duration': 5, + }) + cron = self.env.ref('calendar.ir_cron_scheduler_alarm') + cron.lastcall = False + with self.capture_triggers('calendar.ir_cron_scheduler_alarm') as capt: + with freeze_time('2022-04-13 10:00+0000'): + now = fields.Datetime.now() + self.env['calendar.event'].create({ + 'name': "Recurring Event", + 'start': now + relativedelta(minutes=15), + 'stop': now + relativedelta(minutes=20), + 'recurrency': True, + 'rrule_type': 'daily', + 'count': 3, + 'alarm_ids': [fields.Command.link(alarm.id)], + }).with_context(mail_notrack=True) + self.env.flush_all() + self.assertEqual(len(capt.records), 1, "1 trigger should have been created for the whole recurrence (1)") + self.assertEqual(capt.records.call_at, datetime(2022, 4, 13, 10, 10)) + + with self.capture_triggers('calendar.ir_cron_scheduler_alarm') as capt: + with freeze_time('2022-04-13 10:11+0000'): + self.env['calendar.alarm_manager']._send_reminder() + self.assertEqual(len(capt.records), 1) + + with self.capture_triggers('calendar.ir_cron_scheduler_alarm') as capt: + with freeze_time('2022-04-14 10:11+0000'): + self.env['calendar.alarm_manager']._send_reminder() + self.assertEqual(len(capt.records), 1, "1 trigger should have been created for the whole recurrence (2)") + self.assertEqual(capt.records.call_at, datetime(2022, 4, 15, 10, 10)) + def test_notification_event_timezone(self): """ Check the domain that decides when calendar events should be notified to the user. diff --git a/addons/calendar_sms/models/calendar_alarm_manager.py b/addons/calendar_sms/models/calendar_alarm_manager.py index 461de5ea796..98fb8f4a40d 100644 --- a/addons/calendar_sms/models/calendar_alarm_manager.py +++ b/addons/calendar_sms/models/calendar_alarm_manager.py @@ -22,3 +22,6 @@ class AlarmManager(models.AbstractModel): for event in events: alarm = event.alarm_ids.filtered(lambda alarm: alarm.id in alarms.ids) event._do_sms_reminder(alarm) + if event.recurrence_id: + next_date = event.get_next_alarm_date(events_by_alarm) + event.recurrence_id.with_context(date=next_date)._setup_alarms()