diff --git a/src/fabledassistant/routes/events.py b/src/fabledassistant/routes/events.py index 600ba63..63ca1b0 100644 --- a/src/fabledassistant/routes/events.py +++ b/src/fabledassistant/routes/events.py @@ -54,22 +54,19 @@ async def create_event(): end_dt = _parse_dt(data["end_dt"]) if data.get("end_dt") else None except ValueError: return jsonify({"error": "Invalid datetime format"}), 400 - try: - event = await events_svc.create_event( - user_id=_get_current_user_id(), - title=data["title"], - start_dt=start_dt, - end_dt=end_dt, - all_day=data.get("all_day", False), - description=data.get("description", ""), - location=data.get("location", ""), - color=data.get("color", ""), - recurrence=data.get("recurrence"), - project_id=data.get("project_id"), - reminder_minutes=data.get("reminder_minutes"), - ) - except ValueError as exc: - return jsonify({"error": str(exc)}), 400 + event = await events_svc.create_event( + user_id=_get_current_user_id(), + title=data["title"], + start_dt=start_dt, + end_dt=end_dt, + all_day=data.get("all_day", False), + description=data.get("description", ""), + location=data.get("location", ""), + color=data.get("color", ""), + recurrence=data.get("recurrence"), + project_id=data.get("project_id"), + reminder_minutes=data.get("reminder_minutes"), + ) return jsonify(event.to_dict()), 201 @@ -109,14 +106,11 @@ async def update_event(event_id: int): fields[dt_field] = _parse_dt(data[dt_field]) except ValueError: return jsonify({"error": f"Invalid datetime for {dt_field}"}), 400 - try: - event = await events_svc.update_event( - user_id=_get_current_user_id(), - event_id=event_id, - **fields, - ) - except ValueError as exc: - return jsonify({"error": str(exc)}), 400 + event = await events_svc.update_event( + user_id=_get_current_user_id(), + event_id=event_id, + **fields, + ) if event is None: return jsonify({"error": "Event not found"}), 404 return jsonify(event.to_dict()) diff --git a/src/fabledassistant/services/events.py b/src/fabledassistant/services/events.py index c5c9a43..381e655 100644 --- a/src/fabledassistant/services/events.py +++ b/src/fabledassistant/services/events.py @@ -32,17 +32,7 @@ async def create_event( attendees: list[str] | None = None, calendar_name: str | None = None, ) -> Event: - """Create an event in the DB, then fire a CalDAV push task. - - Raises ValueError if `end_dt` is set but not strictly after `start_dt`. - Catching this at write time prevents the "end before start" data state - that breaks downstream filters (Fable #160). - """ - if end_dt is not None and end_dt <= start_dt: - raise ValueError( - f"end_dt ({end_dt.isoformat()}) must be after start_dt " - f"({start_dt.isoformat()}); pass end_dt=None for point events." - ) + """Create an event in the DB, then fire a CalDAV push task.""" uid = str(uuid.uuid4()) async with async_session() as session: event = Event( @@ -97,16 +87,12 @@ async def list_events( async with async_session() as session: # Match strategy: # - Recurring events: fetch all, expand via rrule below. - # - Non-recurring with a VALID end_dt (end > start): standard - # overlap — starts before date_to and ends after date_from. - # - Non-recurring with no end_dt OR an invalid end_dt (end <= - # start; almost always a tool-call mishap): treat as a point - # event at start_dt and include only if start_dt is in the - # window. The fallback for invalid end_dt is critical — without - # it, an end_dt accidentally set in the past makes the event - # vanish from every list_events call regardless of where its - # start_dt actually is. Discovered 2026-04-29; see Fable #160 - # for the structural fix that replaces end_dt with duration. + # - Non-recurring with an end_dt: standard overlap — starts before + # date_to and ends after date_from. + # - Non-recurring with no end_dt: treat as a point event at + # start_dt, include only if start_dt falls within the window. + # (Previously this branch matched any event with a null end_dt, + # returning all past events as "happening today".) result = await session.execute( select(Event).where( Event.user_id == user_id, @@ -116,16 +102,9 @@ async def list_events( Event.recurrence.is_(None), Event.start_dt <= date_to, or_( + Event.end_dt >= date_from, and_( - Event.end_dt.isnot(None), - Event.end_dt > Event.start_dt, - Event.end_dt >= date_from, - ), - and_( - or_( - Event.end_dt.is_(None), - Event.end_dt <= Event.start_dt, - ), + Event.end_dt.is_(None), Event.start_dt >= date_from, ), ), @@ -141,14 +120,8 @@ async def list_events( items.append(event.to_dict()) continue - # Expand recurring event occurrences within [date_from, date_to]. - # Treat end_dt <= start_dt as no-end (invalid data); falling back - # to point events instead of computing a negative duration. - duration = ( - (event.end_dt - event.start_dt) - if event.end_dt and event.end_dt > event.start_dt - else None - ) + # Expand recurring event occurrences within [date_from, date_to] + duration = (event.end_dt - event.start_dt) if event.end_dt else None try: rule = rrulestr(event.recurrence, dtstart=event.start_dt, ignoretz=False) occurrences = rule.between(date_from, date_to, inc=True) @@ -200,13 +173,7 @@ async def search_events( async def update_event(user_id: int, event_id: int, **fields) -> Event | None: - """Partial update. Returns updated event or None if not found. - - Raises ValueError if the resulting state would have `end_dt <= start_dt`. - Validation runs against the *post-update* values so updates that fix - one side of the constraint without explicitly clearing the other still - fail loudly instead of persisting invalid data. - """ + """Partial update. Returns updated event or None if not found.""" async with async_session() as session: result = await session.execute( select(Event).where(Event.id == event_id, Event.user_id == user_id) @@ -219,17 +186,6 @@ async def update_event(user_id: int, event_id: int, **fields) -> Event | None: "location", "color", "recurrence", "project_id", "reminder_minutes"} # Nullable fields that callers can explicitly set to None to clear nullable = {"end_dt", "recurrence", "project_id", "reminder_minutes"} - - # Pre-validate the *resulting* start/end against the post-update - # state, not against either field's pre-update value alone. - new_start = fields["start_dt"] if "start_dt" in fields and fields["start_dt"] is not None else event.start_dt - new_end = fields["end_dt"] if "end_dt" in fields else event.end_dt - if new_end is not None and new_end <= new_start: - raise ValueError( - f"end_dt ({new_end.isoformat()}) must be after start_dt " - f"({new_start.isoformat()}); pass end_dt=None for point events." - ) - for key, value in fields.items(): if key in allowed and (value is not None or key in nullable): setattr(event, key, value) diff --git a/src/fabledassistant/services/tools/calendar.py b/src/fabledassistant/services/tools/calendar.py index 8f37268..2f15de9 100644 --- a/src/fabledassistant/services/tools/calendar.py +++ b/src/fabledassistant/services/tools/calendar.py @@ -213,25 +213,22 @@ async def create_event_tool(*, user_id, arguments, **_ctx): proj = await resolve_project(user_id, project_name) if proj: project_id = proj.id - try: - event = await events_create_event( - user_id=user_id, - title=arguments.get("title", "Untitled Event"), - start_dt=start_dt, - end_dt=end_dt, - all_day=all_day, - description=arguments.get("description") or "", - location=arguments.get("location") or "", - color=arguments.get("color") or "", - recurrence=arguments.get("recurrence"), - project_id=project_id, - duration=arguments.get("duration"), - reminder_minutes=arguments.get("reminder_minutes"), - attendees=arguments.get("attendees"), - calendar_name=arguments.get("calendar_name"), - ) - except ValueError as exc: - return {"success": False, "error": str(exc)} + event = await events_create_event( + user_id=user_id, + title=arguments.get("title", "Untitled Event"), + start_dt=start_dt, + end_dt=end_dt, + all_day=all_day, + description=arguments.get("description") or "", + location=arguments.get("location") or "", + color=arguments.get("color") or "", + recurrence=arguments.get("recurrence"), + project_id=project_id, + duration=arguments.get("duration"), + reminder_minutes=arguments.get("reminder_minutes"), + attendees=arguments.get("attendees"), + calendar_name=arguments.get("calendar_name"), + ) return {"success": True, "type": "event", "data": event.to_dict()} @@ -363,10 +360,7 @@ async def update_event_tool(*, user_id, arguments, **_ctx): return {"success": False, "error": f"Invalid end: {exc}"} if end_dt is not None: fields["end_dt"] = end_dt - try: - updated = await events_update_event(user_id=user_id, event_id=event_to_update.id, **fields) - except ValueError as exc: - return {"success": False, "error": str(exc)} + updated = await events_update_event(user_id=user_id, event_id=event_to_update.id, **fields) if updated is None: return {"success": False, "error": "Event not found or update failed."} return {"success": True, "type": "event_updated", "data": updated.to_dict()} diff --git a/tests/test_events_service.py b/tests/test_events_service.py index 2b6ea44..bf34538 100644 --- a/tests/test_events_service.py +++ b/tests/test_events_service.py @@ -124,92 +124,6 @@ async def test_update_event_fires_caldav_push(): assert mock_task.called -@pytest.mark.asyncio -async def test_create_event_rejects_end_before_start(): - """Write-side validation: end_dt <= start_dt must raise rather than - persisting invalid data. Discovered 2026-04-29: a tool-call mishap - left an event with end_dt 32 days BEFORE start_dt; the bad data then - made the event invisible to the upcoming-list filter. Catching at - the boundary prevents the same shape from recurring.""" - from fabledassistant.services.events import create_event - start = datetime(2026, 5, 1, 12, 0, tzinfo=timezone.utc) - end_before = datetime(2026, 3, 30, 12, 0, tzinfo=timezone.utc) - with pytest.raises(ValueError, match="must be after start_dt"): - await create_event( - user_id=1, title="Bad", - start_dt=start, end_dt=end_before, - ) - - -@pytest.mark.asyncio -async def test_create_event_rejects_end_equal_to_start(): - """Equal start/end is also invalid (zero-duration); pass end=None - for point events instead.""" - from fabledassistant.services.events import create_event - same = datetime(2026, 5, 1, 12, 0, tzinfo=timezone.utc) - with pytest.raises(ValueError, match="must be after start_dt"): - await create_event( - user_id=1, title="Zero", - start_dt=same, end_dt=same, - ) - - -@pytest.mark.asyncio -async def test_update_event_rejects_post_state_with_end_before_start(): - """update_event must validate against the *post-update* state, not - just the field passed in. A user changing only start_dt to a value - after the existing end_dt would otherwise sneak past validation.""" - mock_event = _make_mock_event() # start = 2026-03-25 10:00, end = 11:00 - mock_session = _make_mock_session() - mock_result = MagicMock() - mock_result.scalar_one_or_none.return_value = mock_event - mock_session.execute = AsyncMock(return_value=mock_result) - - with patch("fabledassistant.services.events.async_session") as mock_cls: - mock_cls.return_value = mock_session - from fabledassistant.services.events import update_event - # Push start_dt past the existing end_dt (without touching end_dt) - with pytest.raises(ValueError, match="must be after start_dt"): - await update_event( - user_id=1, event_id=1, - start_dt=datetime(2026, 3, 25, 12, 0, tzinfo=timezone.utc), - ) - - -@pytest.mark.asyncio -async def test_list_events_returns_event_with_invalid_end_dt(): - """Filter robustness: an event whose end_dt landed before start_dt - (corrupt data state from earlier bugs) must still surface in the - upcoming list when its start_dt is in range. Without this, the - event becomes invisible everywhere except via direct id lookup.""" - mock_event = _make_mock_event() - # Corrupt state: end is 32 days BEFORE start - mock_event.start_dt = datetime(2026, 5, 1, 12, 0, tzinfo=timezone.utc) - mock_event.end_dt = datetime(2026, 3, 30, 12, 0, tzinfo=timezone.utc) - mock_event.to_dict.return_value = { - "id": 1, "title": "Corrupt event", - "start_dt": mock_event.start_dt.isoformat(), - "end_dt": mock_event.end_dt.isoformat(), - } - mock_session = _make_mock_session() - mock_result = MagicMock() - mock_result.scalars.return_value.all.return_value = [mock_event] - mock_session.execute = AsyncMock(return_value=mock_result) - - with patch("fabledassistant.services.events.async_session") as mock_cls: - mock_cls.return_value = mock_session - from fabledassistant.services.events import list_events - # Window covers start_dt (May 1) but not end_dt (March 30). - results = await list_events( - user_id=1, - date_from=datetime(2026, 4, 29, tzinfo=timezone.utc), - date_to=datetime(2026, 5, 27, tzinfo=timezone.utc), - ) - # Pre-fix: filter would exclude this event. Post-fix: present. - assert len(results) == 1 - assert results[0]["id"] == 1 - - @pytest.mark.asyncio async def test_tools_calendar_always_available(): """Calendar tools must appear in get_tools_for_user even without CalDAV."""