caldav_sync bug fixes:

* Fixed an issue where accepting events from an external organizer would send emails
  to the all event attendees upon synchronization.
* Corrected the data model for events where multiple Odoo users are attendees for the
  same event. Now, only one event is created in Odoo, with all the attendees on the
  same event. This conforms to Odoo's existing calendar event data model.
* Updated the setting of the organizer field (user_id) on the calendar events upon
  synchronization. Previously, the organizer field was always set to the current user
  whose events were being synchronized. Now, it is set based on the ORGANIZER parameter
  of the iCalendar event, if present. If not present, it defaults to the current synchronizing
  user. In the case that the organizer is external, the user_id field is left blank.
This commit is contained in:
Marc Durepos 2025-02-07 10:54:15 -05:00
parent 799e944112
commit 65fbcb9b33
8 changed files with 242 additions and 46 deletions

View file

@ -8,7 +8,7 @@
{ {
"name": "CalDAV Synchronization", "name": "CalDAV Synchronization",
"version": "17.0.0.6.2", "version": "17.0.0.6.3",
"license": "LGPL-3", "license": "LGPL-3",
"category": "Productivity", "category": "Productivity",
"summary": "Synchronize Odoo Calendar Events with CalDAV Servers", "summary": "Synchronize Odoo Calendar Events with CalDAV Servers",

View file

@ -2,7 +2,8 @@ import uuid
import icalendar.cal import icalendar.cal
from odoo import models, api, fields from odoo import models, api, fields, _
from odoo.tools.misc import _logger
from odoo.addons.calendar.models.calendar_recurrence import MAX_RECURRENT_EVENT from odoo.addons.calendar.models.calendar_recurrence import MAX_RECURRENT_EVENT
import caldav import caldav
import logging import logging
@ -869,7 +870,23 @@ class CalendarEvent(models.Model):
"caldav_uid": str(component.get("uid")), "caldav_uid": str(component.get("uid")),
"partner_ids": [(6, 0, attendee_ids.ids)], "partner_ids": [(6, 0, attendee_ids.ids)],
"partner_id": organizer.id if organizer else user.partner_id.id, "partner_id": organizer.id if organizer else user.partner_id.id,
"user_id": user.id, # For user_id:
# - If there's an organizer with an Odoo user account, use that
# - If there's an organizer but no Odoo account, set to False (external)
# - If no organizer, the current user owns it
"user_id": (
# For debugging
(
_logger.info("Looking up user for organizer: %s", organizer)
or _logger.info("Organizer ID: %s", organizer.id)
or self.env["res.users"]
.search([("partner_id", "=", organizer.id)], limit=1)
.id
or False
)
if organizer
else user.id
),
} }
return values return values
@ -886,6 +903,13 @@ class CalendarEvent(models.Model):
matching Odoo event belongs to. matching Odoo event belongs to.
:return: The res.partner records who are attendees for the event.""" :return: The res.partner records who are attendees for the event."""
attendee_emails = self._get_ical_attendee_emails(component) attendee_emails = self._get_ical_attendee_emails(component)
# Add organizer to attendees if present
organizer = component.get("organizer")
if organizer:
organizer_email = _extract_vcal_email(organizer)
if organizer_email not in attendee_emails:
attendee_emails.append(organizer_email)
# Add current user if not already in attendees
if current_user_email not in attendee_emails: if current_user_email not in attendee_emails:
attendee_emails.append(current_user_email) attendee_emails.append(current_user_email)
existing_partners = self.env["res.partner"].search( existing_partners = self.env["res.partner"].search(
@ -896,14 +920,24 @@ class CalendarEvent(models.Model):
for email in attendee_emails for email in attendee_emails
if email not in [partner.email for partner in existing_partners] if email not in [partner.email for partner in existing_partners]
] ]
added_partners = self.env["res.partner"].create( # Create new partners without triggering notifications
[ added_partners = (
{ self.env["res.partner"]
"name": email, .with_context(
"email": email, mail_notify_author=False, # Don't notify the author
} mail_notify_force_send=False, # Don't force send notifications
for email in missing_emails tracking_disable=True, # Disable tracking which can trigger notifications
] no_reset_password=True, # Don't trigger password reset emails
)
.create(
[
{
"name": email,
"email": email,
}
for email in missing_emails
]
)
) )
final_attendees = {} final_attendees = {}
all_partners = existing_partners | added_partners all_partners = existing_partners | added_partners
@ -933,11 +967,29 @@ class CalendarEvent(models.Model):
organizer = component.get("organizer") organizer = component.get("organizer")
if organizer: if organizer:
partner = self.env["res.partner"].search( email = _extract_vcal_email(organizer)
[("email", "=", _extract_vcal_email(organizer))] _logger.info("Organizer email: %s", email)
) partner = self.env["res.partner"].search([("email", "=", email)], limit=1)
# TODO: prioritize partner with a user if there is one _logger.info("Found partner: %s", partner)
return partner[0] if partner else partner # partner[0] in case many matches if not partner:
# Create new partner without triggering notifications
partner = (
self.env["res.partner"]
.with_context(
mail_notify_author=False,
mail_notify_force_send=False,
tracking_disable=True,
no_reset_password=True,
)
.create(
{
"name": email,
"email": email,
}
)
)
_logger.info("Created partner: %s", partner)
return partner
else: else:
return self.env["res.partner"] return self.env["res.partner"]

View file

@ -25,14 +25,24 @@ class ResUsers(models.Model):
@api.depends("caldav_username", "caldav_password", "caldav_calendar_url") @api.depends("caldav_username", "caldav_password", "caldav_calendar_url")
def _compute_is_caldav_enabled(self): def _compute_is_caldav_enabled(self):
"""This is a bit of an odd way of computing the field, but it works since any """Compute whether CalDAV is enabled for each user by validating their credentials.
failed attempt to get the events from the server should mark the user as We only check if we can connect to the server and access the principal, without
CalDAV being disabled. We just make sure to mute the logger in the case that fetching any events to avoid timeouts with large calendars."""
an exception is raised because we are just computing the field, not actually
attempting to synchronize anything."""
for rec in self: for rec in self:
with mute_logger("odoo.addons.caldav_sync.models.res_users"): # If any required field is empty, CalDAV is disabled
rec._get_caldav_events() if not (
rec.caldav_username and rec.caldav_password and rec.caldav_calendar_url
):
rec.is_caldav_enabled = False
continue
try:
client = rec._get_caldav_client()
# Just try to access the principal, which is a lightweight operation
client.principal()
rec.is_caldav_enabled = True
except Exception as e:
rec.is_caldav_enabled = False
_logger.error("Failed to validate CalDAV credentials: %s", e)
def _get_caldav_client(self): def _get_caldav_client(self):
self.ensure_one() self.ensure_one()

View file

@ -1,2 +1,3 @@
from . import test_res_users from . import test_res_users
from . import test_calendar from . import test_calendar
from . import test_external_organizer

View file

@ -0,0 +1,14 @@
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Odoo//CalDAV Client//EN
BEGIN:VEVENT
UID:external-organizer-test-123
DTSTART:20250207T143000Z
DTEND:20250207T153000Z
DTSTAMP:20250207T143000Z
ORGANIZER;CN=External Person:mailto:external.person@otherdomain.com
ATTENDEE;PARTSTAT=ACCEPTED;CN=Test User:mailto:test@example.com
SUMMARY:Meeting with External Organizer
DESCRIPTION:This is a test event with an external organizer
END:VEVENT
END:VCALENDAR

View file

@ -31,18 +31,20 @@ def _patch_caldav_with_events_from_ics(ics_paths, user, last_modified=None):
patch("caldav.Calendar") as MockCalendar, patch("caldav.Calendar") as MockCalendar,
): ):
mock_client = MockDAVClient.return_value mock_client = MockDAVClient.return_value
mock_calendar = MockCalendar.return_value
mock_client.calendar = mock_calendar
mock_calendars = {} mock_calendars = {}
def calendar_side_effect(url): def calendar_side_effect(url):
if url not in mock_calendars: if url not in mock_calendars:
mock_calendars[url] = MockCalendar() mock_cal = MagicMock()
if url == user.caldav_calendar_url: mock_cal.events = MagicMock(return_value=[])
return mock_calendars[url] mock_cal.event_by_uid = MagicMock()
raise Exception("Calendar does not exist.") mock_calendars[url] = mock_cal
return mock_calendars[url]
mock_calendar.side_effect = calendar_side_effect mock_client.calendar = calendar_side_effect
# Get or create the mock calendar for this user
mock_calendar = calendar_side_effect(user.caldav_calendar_url)
def event_by_uid_side_effect(self, uid): def event_by_uid_side_effect(self, uid):
for event in self.events(): for event in self.events():
@ -89,11 +91,26 @@ class TestCalendarEvent(TransactionCase, CaldavTestCommon):
super().setUpClass() super().setUpClass()
cls.env["res.users"].search([])._compute_is_caldav_enabled() cls.env["res.users"].search([])._compute_is_caldav_enabled()
cls.user_1_url = "https://mycaldav.test.com/test1calendar" cls.user_1_url = "https://mycaldav.test.com/test1calendar"
cls.user_1 = cls._generate_user("test1", "test1", cls.user_1_url) cls.user_1 = cls._generate_user(
"test1",
caldav_username="user1",
caldav_password="pass1",
caldav_url=cls.user_1_url,
)
cls.user_2_url = "https://mycaldav.test.com/test2calendar" cls.user_2_url = "https://mycaldav.test.com/test2calendar"
cls.user_2 = cls._generate_user("test2", "test2", cls.user_2_url) cls.user_2 = cls._generate_user(
"test2",
caldav_username="user2",
caldav_password="pass2",
caldav_url=cls.user_2_url,
)
cls.user_3_url = "https://mycaldav.test.com/test3calendar" cls.user_3_url = "https://mycaldav.test.com/test3calendar"
cls.user_3 = cls._generate_user("test3", "test3", cls.user_3_url) cls.user_3 = cls._generate_user(
"test3",
caldav_username="user3",
caldav_password="pass3",
caldav_url=cls.user_3_url,
)
def test_basic_event_from_server_create(self): def test_basic_event_from_server_create(self):
user = self.user_1 user = self.user_1

View file

@ -0,0 +1,55 @@
from odoo.tests import TransactionCase
from unittest.mock import patch, MagicMock
from .common import CaldavTestCommon
from .test_calendar import _get_ics_path, _patch_caldav_with_events_from_ics
class TestExternalOrganizer(TransactionCase, CaldavTestCommon):
@classmethod
def setUpClass(cls):
super().setUpClass()
cls.user = cls._generate_user(
"test",
caldav_username="test",
caldav_password="test",
caldav_url="https://example.com/calendar",
)
def test_external_organizer_event_sync(self):
"""Test that events with external organizers are handled correctly."""
# Setup mock for CalDAV client with our test ICS file
with _patch_caldav_with_events_from_ics(
[_get_ics_path("test_external_organizer.ics")], self.user
):
# Ensure caldav is enabled
self.user._compute_is_caldav_enabled()
# Sync events from the mock server
self.env["calendar.event"].poll_caldav_server()
# Find the synced event
event = self.env["calendar.event"].search(
[("caldav_uid", "=", "external-organizer-test-123")]
)
# Verify event was created
self.assertTrue(event, "Event should be created")
# Verify user_id is False for external organizer
self.assertFalse(
event.user_id,
"Event with external organizer should have user_id set to False",
)
# Verify the organizer's email is preserved in attendees
external_attendee = event.attendee_ids.filtered(
lambda a: a.email == "external.person@otherdomain.com"
)
self.assertTrue(
external_attendee,
"External organizer should be present in attendees",
)
# The external organizer should be in the attendees list
self.assertTrue(
external_attendee,
"External organizer should be present in attendees list",
)

View file

@ -1,6 +1,10 @@
from odoo.tests import TransactionCase from odoo.tests import TransactionCase
from unittest.mock import patch, MagicMock from unittest.mock import patch, MagicMock
from .common import CaldavTestCommon from .common import CaldavTestCommon
import caldav
import logging
_logger = logging.getLogger(__name__)
class TestUsers(TransactionCase, CaldavTestCommon): class TestUsers(TransactionCase, CaldavTestCommon):
@ -9,24 +13,67 @@ class TestUsers(TransactionCase, CaldavTestCommon):
super().setUpClass() super().setUpClass()
def test_caldav_enabled_false_without_url(self): def test_caldav_enabled_false_without_url(self):
user = self._generate_user("test", "test") # Create user with no CalDAV credentials
user = self._generate_user("test")
self.assertFalse(user.is_caldav_enabled) self.assertFalse(user.is_caldav_enabled)
def test_caldav_enabled_false_without_credentials(self):
"""Test that is_caldav_enabled is False when any required field is missing."""
# Test with missing URL - has username and password only
user1 = self._generate_user(
"test1", caldav_username="user1", caldav_password="pass1"
)
self.assertFalse(user1.is_caldav_enabled)
# Test with missing username - has password and URL only
user2 = self._generate_user(
"test2", caldav_password="pass2", caldav_url="https://example.com"
)
self.assertFalse(user2.is_caldav_enabled)
# Test with missing password - has username and URL only
user3 = self._generate_user(
"test3", caldav_username="user3", caldav_url="https://example.com"
)
self.assertFalse(user3.is_caldav_enabled)
@patch("caldav.DAVClient") @patch("caldav.DAVClient")
def test_caldav_connection_succeeds_but_not_calendar(self, MockDAVClient): def test_caldav_enabled_success(self, MockDAVClient):
user = self._generate_user("test", "test", "https://example.com/abc123") """Test that is_caldav_enabled is True when connection succeeds."""
# Create a mock client and mock calendar # Create user with name 'test' and set CalDAV credentials
user = self._generate_user(
"test",
caldav_username="user",
caldav_password="pass",
caldav_url="https://example.com/abc123",
)
# Mock successful connection
mock_client = MockDAVClient.return_value mock_client = MockDAVClient.return_value
mock_calendar = MagicMock() mock_principal = MagicMock()
mock_client.calendar.return_value = mock_calendar mock_client.principal.return_value = mock_principal
# Mock the events method to raise an exception # Compute should succeed and set is_caldav_enabled to True
mock_calendar.events.side_effect = Exception("Failed to get events") user._compute_is_caldav_enabled()
self.assertTrue(user.is_caldav_enabled)
# An exception should not be raised because we need to continue to sync other @patch("caldav.DAVClient")
# users' calendars. Instead the exception message is simply logged to error. def test_caldav_enabled_connection_fails(self, MockDAVClient):
"""Test that is_caldav_enabled is False when connection fails."""
user = self._generate_user(
"test",
caldav_username="user",
caldav_password="pass",
caldav_url="https://example.com/abc123",
)
# Mock failed connection
mock_client = MockDAVClient.return_value
mock_client.principal.side_effect = caldav.error.AuthorizationError(
"Invalid credentials"
)
# Should handle the error gracefully and set is_caldav_enabled to False
with self.assertLogs("odoo.addons.caldav_sync.models.res_users", "ERROR"): with self.assertLogs("odoo.addons.caldav_sync.models.res_users", "ERROR"):
user._get_caldav_events() user._compute_is_caldav_enabled()
# Ensure the is_caldav_enabled is set to False
self.assertFalse(user.is_caldav_enabled) self.assertFalse(user.is_caldav_enabled)