diff --git a/src/alpine_bits_python/conversion_service.py b/src/alpine_bits_python/conversion_service.py index 7ccf05d..f0cec19 100644 --- a/src/alpine_bits_python/conversion_service.py +++ b/src/alpine_bits_python/conversion_service.py @@ -1127,18 +1127,24 @@ class ConversionService: Returns a mapping of guest_id -> HashedCustomer for all unique guests found in unmatched conversions. Only processes each guest once. + This includes: + 1. Conversions with no customer match at all (reservation_id IS NULL AND customer_id IS NULL) + 2. Conversions matched to a customer but not a reservation (reservation_id IS NULL AND customer_id IS NOT NULL) + - These may have been matched in a previous run and need re-evaluation for reservation linking + Args: session: AsyncSession for database queries Returns: Dictionary mapping guest_id to matched HashedCustomer (or None if no match) """ - # Find all conversions that have no reservation/customer match yet + # Find all conversions that either: + # - Have no match at all (reservation_id IS NULL AND customer_id IS NULL), OR + # - Have a customer but no reservation (for re-linking in case new reservations were added) result = await session.execute( select(Conversion) .where( (Conversion.reservation_id.is_(None)) - & (Conversion.customer_id.is_(None)) & (Conversion.guest_id.isnot(None)) ) .options(selectinload(Conversion.guest)) @@ -1201,6 +1207,13 @@ class ConversionService: that don't have a reservation yet, and try to link them to reservations based on date matching. + This includes: + 1. Conversions with no customer match (will link customer first) + 2. Conversions already linked to a customer from a previous run (will try to link to reservation) + + After all conversions for a guest are processed, check if the guest is a regular + by looking at whether they have paying conversions that predate any reservations. + Args: guest_to_hashed_customer: Mapping from guest_id to matched HashedCustomer session: AsyncSession for database queries @@ -1210,15 +1223,15 @@ class ConversionService: if not matched_hashed_customer or not matched_hashed_customer.customer_id: continue - # Find all unmatched conversions from this guest + # Find all conversions from this guest that don't have a reservation + # (whether or not they have a customer match - we might be re-running after new reservations added) result = await session.execute( select(Conversion) .where( (Conversion.guest_id == guest_id) & (Conversion.reservation_id.is_(None)) - & (Conversion.customer_id.is_(None)) ) - .options(selectinload(Conversion.conversion_rooms)) + .options(selectinload(Conversion.conversion_rooms), selectinload(Conversion.guest)) ) conversions = result.scalars().all() @@ -1239,21 +1252,26 @@ class ConversionService: ) if matched_reservation and is_attributable: + # Only update stats if this is a NEW match (wasn't matched before) + was_previously_matched = conversion.customer_id is not None + conversion.reservation_id = matched_reservation.id conversion.customer_id = matched_hashed_customer.customer_id conversion.hashed_customer_id = matched_hashed_customer.id - conversion.directly_attributable = False + conversion.directly_attributable = True conversion.guest_matched = True conversion.updated_at = datetime.now() - stats["matched_to_reservation"] += 1 + + if not was_previously_matched: + stats["matched_to_reservation"] += 1 _LOGGER.info( "Phase 3c: Linked conversion (pms_id=%s) to reservation %d via guest matching", conversion.pms_reservation_id, matched_reservation.id, ) - elif matched_hashed_customer: - # No attributable reservation found, but link to customer/hashed customer + elif matched_hashed_customer and conversion.customer_id is None: + # Only count new customer matches (conversions that didn't have a customer before) conversion.customer_id = matched_hashed_customer.customer_id conversion.hashed_customer_id = matched_hashed_customer.id conversion.directly_attributable = False @@ -1261,6 +1279,63 @@ class ConversionService: conversion.updated_at = datetime.now() stats["matched_to_customer"] += 1 + # After all conversions for this guest are processed, check if guest is regular + # Look at ALL conversions from this guest to see if there are pre-dated payments + if conversions and conversions[0].guest: + await self._check_if_guest_is_regular( + guest_id, matched_hashed_customer.customer_id, session + ) + + async def _check_regularity_for_all_matched_guests( + self, session: AsyncSession + ) -> None: + """Phase 3d: Check regularity for ALL matched guests (both ID-matched and guest-detail-matched). + + This is called after all matching is complete to evaluate every guest that has been + matched to a customer, regardless of match type. This ensures consistent regularity + evaluation across all matched conversions. + + This is run on ALL matched guests, not just newly matched ones, to ensure that if + the regularity logic changes, it gets re-applied to all guests on the next run. + This maintains idempotency of the matching process. + + Args: + session: AsyncSession for database queries + """ + # Get all ConversionGuests that have ANY customer link + # This includes: + # 1. Guests matched via guest-details (hashed_customer_id is not null) + # 2. Guests matched via ID-based matching (customer_id is not null via conversion) + result = await session.execute( + select(ConversionGuest).where( + ConversionGuest.hashed_customer_id.isnot(None) + ) + ) + matched_guests = result.scalars().all() + + if not matched_guests: + _LOGGER.debug("Phase 3d: No matched guests to check for regularity") + return + + _LOGGER.debug("Phase 3d: Checking regularity for %d matched guests", len(matched_guests)) + + for conversion_guest in matched_guests: + if not conversion_guest.hashed_customer_id: + continue + + # Get the customer ID from the hashed_customer + hashed_customer_result = await session.execute( + select(HashedCustomer).where( + HashedCustomer.id == conversion_guest.hashed_customer_id + ) + ) + hashed_customer = hashed_customer_result.scalar_one_or_none() + + if hashed_customer and hashed_customer.customer_id: + await self._check_if_guest_is_regular( + conversion_guest.guest_id, hashed_customer.customer_id, session + ) + async def _match_conversions_from_db_sequential( self, pms_reservation_ids: list[str], stats: dict[str, int] ) -> None: @@ -1291,10 +1366,14 @@ class ConversionService: await self._link_matched_guests_to_reservations( guest_to_hashed_customer, session, stats ) - await session.commit() + + # Phase 3d: Check regularity for all matched guests (both ID and guest-detail matched) + await self._check_regularity_for_all_matched_guests(session) + + await session.commit() except Exception as e: await session.rollback() - _LOGGER.exception("Error in Phase 3b/3c guest matching: %s", e) + _LOGGER.exception("Error in Phase 3b/3c/3d guest matching: %s", e) finally: if self.session_maker: await session.close() @@ -1338,10 +1417,14 @@ class ConversionService: await self._link_matched_guests_to_reservations( guest_to_hashed_customer, session, stats ) - await session.commit() + + # Phase 3d: Check regularity for all matched guests (both ID and guest-detail matched) + await self._check_regularity_for_all_matched_guests(session) + + await session.commit() except Exception as e: await session.rollback() - _LOGGER.exception("Error in Phase 3b/3c guest matching: %s", e) + _LOGGER.exception("Error in Phase 3b/3c/3d guest matching: %s", e) finally: await session.close() @@ -1494,10 +1577,6 @@ class ConversionService: conversion.directly_attributable = True conversion.guest_matched = False - # Check if guest is regular - if matched_reservation: - await self._check_if_regular(conversion, matched_reservation, session) - # Update conversion_guest with hashed_customer reference if matched if conversion_guest and matched_hashed_customer: conversion_guest.hashed_customer_id = matched_hashed_customer.id @@ -1515,33 +1594,41 @@ class ConversionService: else: stats["unmatched"] += 1 - async def _check_if_regular( + async def _check_if_guest_is_regular( self, - conversion: Conversion, - matched_reservation: Reservation, + guest_id: str, + customer_id: int, session: AsyncSession, ) -> None: - """Check if guest is a regular customer and update is_regular flag. + """Check if a guest is a regular customer based on conversion and reservation history. - A guest is regular if they have conversions with dates before their first completed reservation. - Otherwise, is_regular is set to False. + A guest is regular if they have conversions with paying bookings that predate their first + reservation sent by us. This indicates they were already a customer before we started tracking. + + This check is done AFTER all conversions for a guest have been processed and matched, + so it can evaluate the complete picture of their payment history vs our reservation history. Args: - conversion: The Conversion record being evaluated - matched_reservation: The matched Reservation record + guest_id: The guest ID to evaluate + customer_id: The matched customer ID session: AsyncSession for database queries """ - if not conversion.guest or not matched_reservation.customer_id: + # Get the ConversionGuest record + guest_result = await session.execute( + select(ConversionGuest).where(ConversionGuest.guest_id == guest_id) + ) + conversion_guest = guest_result.scalar_one_or_none() + + if not conversion_guest: return - # Find the earliest paying conversion for this customer - # (booked reservations from hotel with actual revenue) + # Find the earliest paying conversion for this guest (across all hotels) + # Look for conversions with actual revenue earliest_paying_conversion_result = await session.execute( select(Conversion) .join(ConversionRoom, Conversion.id == ConversionRoom.conversion_id) .where( - Conversion.hotel_id == conversion.hotel_id, - Conversion.guest_id == conversion.guest_id, + Conversion.guest_id == guest_id, ConversionRoom.total_revenue.isnot(None), ConversionRoom.total_revenue > Decimal(0), ) @@ -1551,32 +1638,44 @@ class ConversionService: earliest_paying_conversion = earliest_paying_conversion_result.scalar_one_or_none() if not earliest_paying_conversion: - conversion.guest.is_regular = False + # No paying conversions found for this guest + conversion_guest.is_regular = False return - # Find the earliest reservation (booking request we sent) for this customer + # Find the earliest reservation sent to this customer earliest_reservation_result = await session.execute( select(Reservation) - .where(Reservation.customer_id == matched_reservation.customer_id) + .where(Reservation.customer_id == customer_id) .order_by(Reservation.start_date.asc()) .limit(1) ) earliest_reservation = earliest_reservation_result.scalar_one_or_none() if not earliest_reservation: - conversion.guest.is_regular = False + # No reservations for this customer yet, can't determine regularity + conversion_guest.is_regular = False return - # Guest is regular if their earliest paying conversion predates all their reservations - # (meaning they were already a customer before we started tracking reservations) - is_regular = earliest_paying_conversion.reservation_date < earliest_reservation.start_date - conversion.guest.is_regular = is_regular + # Guest is regular if their earliest paying conversion predates our first reservation + # (meaning they were already a customer before we sent them a reservation) + # Compare against the reservation's creation date (when WE created/sent it), not check-in date + # Convert created_at to date for comparison with reservation_date (both are dates) + is_regular = earliest_paying_conversion.reservation_date < earliest_reservation.created_at.date() + conversion_guest.is_regular = is_regular if is_regular: _LOGGER.info( - "Marking guest as regular: earliest paying conversion date %s is before first reservation %s", + "Marking guest %s as regular: earliest paying conversion %s predates first reservation created at %s", + guest_id, earliest_paying_conversion.reservation_date, - earliest_reservation.start_date, + earliest_reservation.created_at, + ) + else: + _LOGGER.debug( + "Guest %s is not regular: first paying conversion %s is after/equal to first reservation created at %s", + guest_id, + earliest_paying_conversion.reservation_date, + earliest_reservation.created_at, ) async def _check_if_attributable(