Idempotent matching. Hopefully
This commit is contained in:
@@ -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,12 +1252,17 @@ 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()
|
||||
|
||||
if not was_previously_matched:
|
||||
stats["matched_to_reservation"] += 1
|
||||
|
||||
_LOGGER.info(
|
||||
@@ -1252,8 +1270,8 @@ class ConversionService:
|
||||
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
|
||||
)
|
||||
|
||||
# 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
|
||||
)
|
||||
|
||||
# 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(
|
||||
|
||||
Reference in New Issue
Block a user