Skip to content

[16.0][FIX] pms: skip cancel-by-modification reservations on confirm_all_reservations#400

Merged
OCA-git-bot merged 1 commit into
OCA:16.0from
commitsun:16.0-fix-skip-cancel-by-modification-on-confirm-all
May 24, 2026
Merged

[16.0][FIX] pms: skip cancel-by-modification reservations on confirm_all_reservations#400
OCA-git-bot merged 1 commit into
OCA:16.0from
commitsun:16.0-fix-skip-cancel-by-modification-on-confirm-all

Conversation

@DarioLodeiros
Copy link
Copy Markdown
Member

Bug

pms.folio.action_confirm() called with confirm_all_reservations=True in the context unconditionally re-confirms every reservation of the folio, including those previously cancelled.

This breaks reservations cancelled because of a modification chain (state='cancel', cancelled_reason='modified'):

  • The cancelled reservation is a historical artefact of a prior version of the same booking.
  • Re-confirming it re-occupies the room of the superseded version.
  • The new, current version of the same booking typically targets the same room.
  • _compute_avail_id then raises ValidationError: There is no availability for the room type <X> on <YYYY-MM-DD> from the still-occupying superseded line, even though the booking itself is consistent.

The flag is set today by an external channel-manager connector when re-importing a folio whose external status has been reactivated after a cancellation — exactly the case where modification chains accumulate.

Fix

Skip reservations with state='cancel' AND cancelled_reason='modified' when iterating reservation_ids to call action_confirm(). They are leftovers from earlier versions of the same booking and must not be revived.

Other cancelled reservations (cancelled_reason in late / intime / noshow / empty) keep being re-confirmed: those represent cases where an external system cancels and later re-confirms the same booking, and the caller of confirm_all_reservations=True expects them to come back to life.

Scope

confirm_all_reservations=True is set by exactly one caller in known downstream connectors: the channel-manager folio importer reactivating a previously cancelled folio. The blast radius is contained to that import path.

Test plan

  • Import a folio modification whose previous version is cancel/modified on the same property and dates — folio should re-confirm without ValidationError.
  • Import a folio re-activation (cancel→confirm without modification) — previous cancel/noshow (or similar) reservation should still be reactivated.

…servations

pms.folio.action_confirm() called with confirm_all_reservations=True in
the context unconditionally re-confirms every reservation of the folio,
including those previously cancelled.

This is problematic for reservations cancelled because of a
modification chain (state='cancel', cancelled_reason='modified'):
re-confirming them re-occupies the room of a superseded version of the
same booking and triggers a ValidationError from _compute_avail_id
("There is no availability for the room type X on YYYY-MM-DD") even
though the booking itself is consistent.

Skip those historical artefacts. Other cancelled reservations
(cancelled_reason in late/intime/noshow, or empty) keep being
re-confirmed: those represent cases where an external system cancels
and later re-confirms the same booking, and the caller of
confirm_all_reservations=True expects them to come back to life.

Today this flag is set only by an external channel-manager connector
import path, so the blast radius of the change is contained.
@DarioLodeiros
Copy link
Copy Markdown
Member Author

/ocabot merge minor

@OCA-git-bot
Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-400-by-DarioLodeiros-bump-minor, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 6b760f5 into OCA:16.0 May 24, 2026
6 of 7 checks passed
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 29545e9. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants