diff --git a/caldav_sync/__manifest__.py b/caldav_sync/__manifest__.py index 3fea042..e8d7afb 100644 --- a/caldav_sync/__manifest__.py +++ b/caldav_sync/__manifest__.py @@ -8,7 +8,7 @@ { "name": "CalDAV Synchronization", - "version": "17.0.0.6.2", + "version": "17.0.0.6.3", "license": "LGPL-3", "category": "Productivity", "summary": "Synchronize Odoo Calendar Events with CalDAV Servers", diff --git a/caldav_sync/models/calendar_event.py b/caldav_sync/models/calendar_event.py index d00b2a8..f1c9c5a 100644 --- a/caldav_sync/models/calendar_event.py +++ b/caldav_sync/models/calendar_event.py @@ -2,7 +2,8 @@ import uuid 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 import caldav import logging @@ -869,7 +870,23 @@ class CalendarEvent(models.Model): "caldav_uid": str(component.get("uid")), "partner_ids": [(6, 0, attendee_ids.ids)], "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 @@ -886,6 +903,13 @@ class CalendarEvent(models.Model): matching Odoo event belongs to. :return: The res.partner records who are attendees for the event.""" 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: attendee_emails.append(current_user_email) existing_partners = self.env["res.partner"].search( @@ -896,14 +920,24 @@ class CalendarEvent(models.Model): for email in attendee_emails if email not in [partner.email for partner in existing_partners] ] - added_partners = self.env["res.partner"].create( - [ - { - "name": email, - "email": email, - } - for email in missing_emails - ] + # Create new partners without triggering notifications + added_partners = ( + self.env["res.partner"] + .with_context( + mail_notify_author=False, # Don't notify the author + mail_notify_force_send=False, # Don't force send notifications + 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 = {} all_partners = existing_partners | added_partners @@ -933,11 +967,29 @@ class CalendarEvent(models.Model): organizer = component.get("organizer") if organizer: - partner = self.env["res.partner"].search( - [("email", "=", _extract_vcal_email(organizer))] - ) - # TODO: prioritize partner with a user if there is one - return partner[0] if partner else partner # partner[0] in case many matches + email = _extract_vcal_email(organizer) + _logger.info("Organizer email: %s", email) + partner = self.env["res.partner"].search([("email", "=", email)], limit=1) + _logger.info("Found partner: %s", partner) + 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: return self.env["res.partner"] diff --git a/caldav_sync/models/res_users.py b/caldav_sync/models/res_users.py index defefcb..0088848 100644 --- a/caldav_sync/models/res_users.py +++ b/caldav_sync/models/res_users.py @@ -25,14 +25,24 @@ class ResUsers(models.Model): @api.depends("caldav_username", "caldav_password", "caldav_calendar_url") def _compute_is_caldav_enabled(self): - """This is a bit of an odd way of computing the field, but it works since any - failed attempt to get the events from the server should mark the user as - CalDAV being disabled. We just make sure to mute the logger in the case that - an exception is raised because we are just computing the field, not actually - attempting to synchronize anything.""" + """Compute whether CalDAV is enabled for each user by validating their credentials. + We only check if we can connect to the server and access the principal, without + fetching any events to avoid timeouts with large calendars.""" for rec in self: - with mute_logger("odoo.addons.caldav_sync.models.res_users"): - rec._get_caldav_events() + # If any required field is empty, CalDAV is disabled + 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): self.ensure_one() diff --git a/caldav_sync/tests/__init__.py b/caldav_sync/tests/__init__.py index 3908f85..4137ec6 100644 --- a/caldav_sync/tests/__init__.py +++ b/caldav_sync/tests/__init__.py @@ -1,2 +1,3 @@ from . import test_res_users from . import test_calendar +from . import test_external_organizer diff --git a/caldav_sync/tests/data/test_external_organizer.ics b/caldav_sync/tests/data/test_external_organizer.ics new file mode 100644 index 0000000..70866ce --- /dev/null +++ b/caldav_sync/tests/data/test_external_organizer.ics @@ -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 diff --git a/caldav_sync/tests/test_calendar.py b/caldav_sync/tests/test_calendar.py index fde0324..5eec38a 100644 --- a/caldav_sync/tests/test_calendar.py +++ b/caldav_sync/tests/test_calendar.py @@ -31,18 +31,20 @@ def _patch_caldav_with_events_from_ics(ics_paths, user, last_modified=None): patch("caldav.Calendar") as MockCalendar, ): mock_client = MockDAVClient.return_value - mock_calendar = MockCalendar.return_value - mock_client.calendar = mock_calendar mock_calendars = {} def calendar_side_effect(url): if url not in mock_calendars: - mock_calendars[url] = MockCalendar() - if url == user.caldav_calendar_url: - return mock_calendars[url] - raise Exception("Calendar does not exist.") + mock_cal = MagicMock() + mock_cal.events = MagicMock(return_value=[]) + mock_cal.event_by_uid = MagicMock() + 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): for event in self.events(): @@ -89,11 +91,26 @@ class TestCalendarEvent(TransactionCase, CaldavTestCommon): super().setUpClass() cls.env["res.users"].search([])._compute_is_caldav_enabled() 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 = 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 = 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): user = self.user_1 diff --git a/caldav_sync/tests/test_external_organizer.py b/caldav_sync/tests/test_external_organizer.py new file mode 100644 index 0000000..d50236c --- /dev/null +++ b/caldav_sync/tests/test_external_organizer.py @@ -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", + ) diff --git a/caldav_sync/tests/test_res_users.py b/caldav_sync/tests/test_res_users.py index f00c58d..3c42316 100644 --- a/caldav_sync/tests/test_res_users.py +++ b/caldav_sync/tests/test_res_users.py @@ -1,6 +1,10 @@ from odoo.tests import TransactionCase from unittest.mock import patch, MagicMock from .common import CaldavTestCommon +import caldav +import logging + +_logger = logging.getLogger(__name__) class TestUsers(TransactionCase, CaldavTestCommon): @@ -9,24 +13,67 @@ class TestUsers(TransactionCase, CaldavTestCommon): super().setUpClass() 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) + 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") - def test_caldav_connection_succeeds_but_not_calendar(self, MockDAVClient): - user = self._generate_user("test", "test", "https://example.com/abc123") - # Create a mock client and mock calendar + def test_caldav_enabled_success(self, MockDAVClient): + """Test that is_caldav_enabled is True when connection succeeds.""" + # 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_calendar = MagicMock() - mock_client.calendar.return_value = mock_calendar + mock_principal = MagicMock() + mock_client.principal.return_value = mock_principal - # Mock the events method to raise an exception - mock_calendar.events.side_effect = Exception("Failed to get events") + # Compute should succeed and set is_caldav_enabled to True + 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 - # users' calendars. Instead the exception message is simply logged to error. + @patch("caldav.DAVClient") + 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"): - user._get_caldav_events() - - # Ensure the is_caldav_enabled is set to False + user._compute_is_caldav_enabled() self.assertFalse(user.is_caldav_enabled)