From 13e404d07cc1ecefb0c66c7e8620d20cd064359d Mon Sep 17 00:00:00 2001 From: Jonas Linter <{email_address}> Date: Tue, 9 Dec 2025 14:06:00 +0100 Subject: [PATCH 1/4] Only update updated_at timestamps if something actually changes. --- src/alpine_bits_python/conversion_service.py | 79 ++++++++++++++------ 1 file changed, 58 insertions(+), 21 deletions(-) diff --git a/src/alpine_bits_python/conversion_service.py b/src/alpine_bits_python/conversion_service.py index 16a9258..6cad096 100644 --- a/src/alpine_bits_python/conversion_service.py +++ b/src/alpine_bits_python/conversion_service.py @@ -119,6 +119,28 @@ class ConversionService: f"session must be AsyncSession or SessionMaker, got {type(session)}" ) + @staticmethod + def _update_timestamp_if_modified( + obj: Conversion | ConversionRoom, session: AsyncSession + ) -> bool: + """Update the updated_at timestamp only if the object has been modified. + + Uses SQLAlchemy's change tracking to determine if any scalar attributes + have changed. Only updates the timestamp if actual changes were detected. + + Args: + obj: The ORM object to check and potentially update + session: The session managing this object + + Returns: + True if the object was modified and timestamp was updated, False otherwise + + """ + if session.is_modified(obj, include_collections=False): + obj.updated_at = datetime.now() + return True + return False + @staticmethod def _parse_required_int(value: str | None, field_name: str) -> int: """Parse an integer attribute that must be present.""" @@ -897,26 +919,39 @@ class ConversionService: existing_conversion = existing_result.scalar_one_or_none() if existing_conversion: - # Update existing conversion - only update reservation metadata and advertising data + # Update existing conversion using Pydantic validation # Guest info is stored in ConversionGuest table, not here - # Don't clear reservation/customer links (matching logic will update if needed) - existing_conversion.reservation_number = ( - parsed_reservation.reservation_number + # Preserve reservation/customer links (matching logic will update if needed) + conversion_data = ConversionData( + hotel_id=hotel_id, + pms_reservation_id=pms_reservation_id, + guest_id=parsed_reservation.guest_id, + reservation_number=parsed_reservation.reservation_number, + reservation_date=parsed_reservation.reservation_date, + creation_time=parsed_reservation.creation_time, + reservation_type=parsed_reservation.reservation_type, + booking_channel=parsed_reservation.booking_channel, + advertising_medium=parsed_reservation.advertising_medium, + advertising_partner=parsed_reservation.advertising_partner, + advertising_campagne=parsed_reservation.advertising_campagne, + # Preserve existing values (managed separately) + created_at=existing_conversion.created_at, + reservation_id=existing_conversion.reservation_id, + customer_id=existing_conversion.customer_id, + directly_attributable=existing_conversion.directly_attributable, + guest_matched=existing_conversion.guest_matched, ) - existing_conversion.reservation_date = parsed_reservation.reservation_date - existing_conversion.creation_time = parsed_reservation.creation_time - existing_conversion.reservation_type = parsed_reservation.reservation_type - existing_conversion.booking_channel = parsed_reservation.booking_channel - existing_conversion.advertising_medium = ( - parsed_reservation.advertising_medium + + # Apply validated data, excluding managed fields + validated_dict = conversion_data.model_dump( + exclude={'created_at', 'updated_at', 'reservation_id', 'customer_id', + 'directly_attributable', 'guest_matched'} ) - existing_conversion.advertising_partner = ( - parsed_reservation.advertising_partner - ) - existing_conversion.advertising_campagne = ( - parsed_reservation.advertising_campagne - ) - existing_conversion.updated_at = datetime.now() + for key, value in validated_dict.items(): + setattr(existing_conversion, key, value) + + # Only update timestamp if something actually changed + self._update_timestamp_if_modified(existing_conversion, session) conversion = existing_conversion _LOGGER.debug( "Updated conversion %s (pms_id=%s)", @@ -998,7 +1033,8 @@ class ConversionService: else None ) existing_room_reservation.total_revenue = room_reservation.total_revenue - existing_room_reservation.updated_at = datetime.now() + # Only update timestamp if something actually changed + self._update_timestamp_if_modified(existing_room_reservation, session) _LOGGER.debug( "Updated room reservation %s (pms_id=%s, room=%s)", existing_room_reservation.id, @@ -1358,8 +1394,8 @@ class ConversionService: # ID-based matches are always directly attributable conversion.directly_attributable = True conversion.guest_matched = False - - conversion.updated_at = datetime.now() + # Only update timestamp if something actually changed + self._update_timestamp_if_modified(conversion, session) # Update stats if provided if stats is not None: @@ -1506,7 +1542,8 @@ class ConversionService: elif conversion.reservation_id is None: conversion.directly_attributable = False - conversion.updated_at = datetime.now() + # Only update timestamp if something actually changed + self._update_timestamp_if_modified(conversion, session) return matched_reservation, matched_customer -- 2.49.1 From c73747e02debf8145d654e30650a5ea3cc3b7478 Mon Sep 17 00:00:00 2001 From: Jonas Linter <{email_address}> Date: Tue, 9 Dec 2025 14:45:22 +0100 Subject: [PATCH 2/4] Update free_rooms is_closing season detection. Should also accept 1 as True --- src/alpine_bits_python/alpinebits_server.py | 2 +- src/alpine_bits_python/free_rooms_action.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/alpine_bits_python/alpinebits_server.py b/src/alpine_bits_python/alpinebits_server.py index ee8c12c..b5533d3 100644 --- a/src/alpine_bits_python/alpinebits_server.py +++ b/src/alpine_bits_python/alpinebits_server.py @@ -842,4 +842,4 @@ class AlpineBitsServer: # Ensure FreeRoomsAction is registered with ServerCapabilities discovery -#from .free_rooms_action import FreeRoomsAction +from .free_rooms_action import FreeRoomsAction diff --git a/src/alpine_bits_python/free_rooms_action.py b/src/alpine_bits_python/free_rooms_action.py index ca7e5a2..c5b102b 100644 --- a/src/alpine_bits_python/free_rooms_action.py +++ b/src/alpine_bits_python/free_rooms_action.py @@ -586,7 +586,12 @@ class FreeRoomsAction(AlpineBitsAction): self, sac: OtaHotelInvCountNotifRq.Inventories.Inventory.StatusApplicationControl, ) -> bool: - return (sac.all_inv_code or "").strip().lower() == "true" + """Check if AllInvCode is a truthy boolean value. + + Accepts: "true", "True", "TRUE", "1", "yes", "Yes", "YES", etc. + """ + value = (sac.all_inv_code or "").strip().lower() + return value in ("true", "1", "yes") def _extract_counts( self, -- 2.49.1 From f6929ca7cc96ff2b8fe78260c961f09e4835a164 Mon Sep 17 00:00:00 2001 From: Jonas Linter Date: Tue, 9 Dec 2025 14:13:58 +0000 Subject: [PATCH 3/4] Small logging improvement --- src/alpine_bits_python/free_rooms_action.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/alpine_bits_python/free_rooms_action.py b/src/alpine_bits_python/free_rooms_action.py index c5b102b..645c411 100644 --- a/src/alpine_bits_python/free_rooms_action.py +++ b/src/alpine_bits_python/free_rooms_action.py @@ -290,8 +290,10 @@ class FreeRoomsAction(AlpineBitsAction): # Validate standard inventory entries inv_type_code = (sac.inv_type_code or "").strip() if not inv_type_code: + error_message = "InvTypeCode is required unless AllInvCode=\"true\" or similar truthy values" + _LOGGER.info(error_message) raise FreeRoomsProcessingError( - "InvTypeCode is required unless AllInvCode=\"true\"", + error_message, HttpStatusCode.BAD_REQUEST, ) @@ -301,8 +303,10 @@ class FreeRoomsAction(AlpineBitsAction): # Check for overlap with closing seasons for closing_start, closing_end in closing_season_ranges: if self._date_ranges_overlap(start_date, end_date, closing_start, closing_end): + error_message = f"Inventory entry ({start_date} to {end_date}) overlaps with closing season ({closing_start} to {closing_end})" + _LOGGER.info(error_message) raise FreeRoomsProcessingError( - f"Inventory entry ({start_date} to {end_date}) overlaps with closing season ({closing_start} to {closing_end})", + error_message, HttpStatusCode.BAD_REQUEST, ) -- 2.49.1 From fce2dbc8de5464c16b6f58749d1e17c11cf81c2d Mon Sep 17 00:00:00 2001 From: Jonas Linter <{email_address}> Date: Tue, 9 Dec 2025 15:29:35 +0100 Subject: [PATCH 4/4] Fixed incorrect overlap detection --- src/alpine_bits_python/free_rooms_action.py | 23 +- tests/test_data/ClosingSeasons.xml | 549 ++++++++++++++++++++ tests/test_free_rooms_action.py | 31 ++ 3 files changed, 595 insertions(+), 8 deletions(-) create mode 100644 tests/test_data/ClosingSeasons.xml diff --git a/src/alpine_bits_python/free_rooms_action.py b/src/alpine_bits_python/free_rooms_action.py index 645c411..8285773 100644 --- a/src/alpine_bits_python/free_rooms_action.py +++ b/src/alpine_bits_python/free_rooms_action.py @@ -300,15 +300,22 @@ class FreeRoomsAction(AlpineBitsAction): # Validate date range start_date, end_date = self._parse_date_range(sac.start, sac.end) + # Check if this inventory entry has any counts (available rooms) + # Entries without counts represent unavailable rooms + has_availability = inventory.inv_counts is not None and inventory.inv_counts.inv_count + # Check for overlap with closing seasons - for closing_start, closing_end in closing_season_ranges: - if self._date_ranges_overlap(start_date, end_date, closing_start, closing_end): - error_message = f"Inventory entry ({start_date} to {end_date}) overlaps with closing season ({closing_start} to {closing_end})" - _LOGGER.info(error_message) - raise FreeRoomsProcessingError( - error_message, - HttpStatusCode.BAD_REQUEST, - ) + # Only entries with availability (counts) cannot overlap with closing seasons + # Entries without counts (unavailable rooms) can overlap with closing seasons + if has_availability: + for closing_start, closing_end in closing_season_ranges: + if self._date_ranges_overlap(start_date, end_date, closing_start, closing_end): + error_message = f"Inventory entry ({start_date} to {end_date}) overlaps with closing season ({closing_start} to {closing_end})" + _LOGGER.info(error_message) + raise FreeRoomsProcessingError( + error_message, + HttpStatusCode.BAD_REQUEST, + ) # Check for overlap with other inventory entries for the same room/category inv_code = sac.inv_code.strip() if sac.inv_code else None diff --git a/tests/test_data/ClosingSeasons.xml b/tests/test_data/ClosingSeasons.xml new file mode 100644 index 0000000..c96303a --- /dev/null +++ b/tests/test_data/ClosingSeasons.xml @@ -0,0 +1,549 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/tests/test_free_rooms_action.py b/tests/test_free_rooms_action.py index a1215b1..988827f 100644 --- a/tests/test_free_rooms_action.py +++ b/tests/test_free_rooms_action.py @@ -3,6 +3,7 @@ from __future__ import annotations from datetime import UTC, datetime +from pathlib import Path import pytest import pytest_asyncio @@ -231,6 +232,36 @@ async def test_closing_season_entries_marked_correctly(db_session: AsyncSession) assert len(closing_rows) == 2 assert all(row.bookable_type_2 is None for row in closing_rows) +@pytest.mark.asyncio +async def test_closing_seasons_test_file(db_session: AsyncSession): + await insert_test_hotel(db_session) + action = make_action() + + Path(__file__).parent / "test_data" / "ClosingSeasons.xml" + + xml = (Path(__file__).parent / "test_data" / "ClosingSeasons.xml").read_text() + + response = await action.handle( + "OTA_HotelInvCountNotif:FreeRooms", + xml, + Version.V2024_10, + make_client_info(), + db_session, + ) + assert response.status_code == HttpStatusCode.OK, f"Response was not OK {response.xml_content}" + + inventories = (await db_session.execute(select(HotelInventory))).scalars().all() + closing_inventory = next(inv for inv in inventories if inv.inv_type_code == "__CLOSE") + assert closing_inventory.inv_code is None + + rows = ( + await db_session.execute(select(RoomAvailability).order_by(RoomAvailability.date)) + ).scalars().all() + closing_rows = [row for row in rows if row.is_closing_season] + # Closing season from 2025-12-20 to 2025-12-23 = 4 days + assert len(closing_rows) == 4 + assert all(row.bookable_type_2 is None for row in closing_rows) + @pytest.mark.asyncio async def test_closing_season_not_allowed_in_delta(db_session: AsyncSession): -- 2.49.1