| @@ -16,32 +16,23 @@ | ||
| 16 | 16 | |
| 17 | 17 | defined('ABSPATH') || exit; |
| 18 | 18 | |
| 19 | 19 | /** |
| 20 | - * Every mutation the MCP server performs on a booking, in one place. | |
| 20 | + * Every booking mutation the MCP server performs. | |
| 21 | 21 | * |
| 22 | - * The rule this class exists to enforce: an agent's write must be | |
| 23 | - * indistinguishable from the same write done by a human in wp-admin. Same | |
| 24 | - * validation, same status transitions, same hooks — so remote calendars sync, | |
| 25 | - * CRM triggers fire, webhooks deliver, and payment side effects happen exactly | |
| 26 | - * as they would otherwise. Where the plugin already has a service for the job | |
| 27 | - * (BookingService::createBooking, RescheduleService::reschedule, | |
| 28 | - * Booking::cancelMeeting) we call it rather than reimplementing it; the drift | |
| 29 | - * risk of a second implementation is not worth the convenience. | |
| 22 | + * An agent's write should behave exactly like the same write in wp-admin: same | |
| 23 | + * validation, transitions and hooks, so calendar sync, webhooks and payments | |
| 24 | + * all fire. Existing services (BookingService::createBooking, | |
| 25 | + * RescheduleService::reschedule, Booking::cancelMeeting) are called, not | |
| 26 | + * reimplemented. Each write also logs an activity row naming the acting user. | |
| 30 | 27 | * |
| 31 | - * The one thing MCP writes do that admin writes do not: every one of them lands | |
| 32 | - * an activity row tagged with the acting user and `via MCP`, so an operator | |
| 33 | - * reading a booking's timeline can always tell an agent's action from a | |
| 34 | - * human's. | |
| 35 | - * | |
| 36 | 28 | * @since 2.2.6 |
| 37 | 29 | */ |
| 38 | 30 | class BookingWriter |
| 39 | 31 | { |
| 40 | 32 | /** |
| 41 | - * Columns manage-booking's `update_details` may write, mirroring | |
| 42 | - * SchedulesController::patchBooking()'s whitelist minus the two that have | |
| 43 | - * their own actions (status, payment_status). | |
| 33 | + * Columns `update_details` may write: patchBooking()'s whitelist minus | |
| 34 | + * status and payment_status, which have their own actions. | |
| 44 | 35 | */ |
| 45 | 36 | const EDITABLE_FIELDS = ['first_name', 'last_name', 'email', 'phone', 'internal_note']; |
| 46 | 37 | |
| 47 | 38 | /** |
| @@ -49,11 +40,9 @@ | ||
| 49 | 40 | */ |
| 50 | 41 | private static $guestsDropped = []; |
| 51 | 42 | |
| 52 | 43 | /** |
| 53 | - * Statuses a booking can move to, and what each one is allowed to move from. | |
| 54 | - * Mirrors Booking::cancelMeeting()/rejectMeeting() and the admin's own | |
| 55 | - * transitions so an agent cannot reach a state the UI would refuse. | |
| 44 | + * Allowed status transitions, mirroring the model and admin rules. | |
| 56 | 45 | */ |
| 57 | 46 | public static function statusTransitions() |
| 58 | 47 | { |
| 59 | 48 | return [ |
| @@ -65,16 +54,11 @@ | ||
| 65 | 54 | ]; |
| 66 | 55 | } |
| 67 | 56 | |
| 68 | 57 | /** |
| 69 | - * Create a booking on an attendee's behalf. | |
| 58 | + * Create a booking on an attendee's behalf, following the same steps as | |
| 59 | + * BookingController::createBooking(). | |
| 70 | 60 | * |
| 71 | - * Deliberately mirrors BookingController::createBooking(): resolve the | |
| 72 | - * duration, convert the requested wall-clock time to UTC, resolve the | |
| 73 | - * location from the event's own configured locations, assign a round-robin | |
| 74 | - * host, re-check availability against the live slot engine, then hand off to | |
| 75 | - * BookingService so every downstream integration behaves normally. | |
| 76 | - * | |
| 77 | 61 | * @param CalendarSlot $event |
| 78 | 62 | * @param array $params |
| 79 | 63 | * |
| 80 | 64 | * @return Booking|\WP_Error |
| @@ -127,11 +111,10 @@ | ||
| 127 | 111 | 'email' => $email, |
| 128 | 112 | 'message' => sanitize_textarea_field(Arr::get($params, 'message', '')), |
| 129 | 113 | 'phone' => sanitize_text_field(Arr::get($params, 'phone', '')), |
| 130 | 114 | 'status' => $event->isConfirmationEnabled() ? 'pending' : 'scheduled', |
| 131 | - // Not 'admin': the admin source drives UI affordances that assume a | |
| 132 | - // human filled the form. An agent-created booking is its own thing | |
| 133 | - // and reporting should be able to tell them apart. | |
| 115 | + // Not 'admin': that source drives UI that assumes a human filled | |
| 116 | + // the form, and reports should tell the two apart. | |
| 134 | 117 | 'source' => 'mcp', |
| 135 | 118 | 'event_type' => $event->event_type, |
| 136 | 119 | 'slot_minutes' => $duration, |
| 137 | 120 | ]; |
| @@ -164,10 +147,9 @@ | ||
| 164 | 147 | ['errors' => $customFieldsData->get_error_data()] |
| 165 | 148 | ); |
| 166 | 149 | } |
| 167 | 150 | |
| 168 | - // Round robin picks the host the public page would have picked, so the | |
| 169 | - // agent's booking lands on the same person a self-service booking would. | |
| 151 | + // Pick the same round-robin host the public page would. | |
| 170 | 152 | if ($event->isRoundRobin() && !$hostUserId) { |
| 171 | 153 | $sortedHostIds = $event->getHostIdsSortedByBookings($startTime); |
| 172 | 154 | $bookingData['host_user_id'] = $sortedHostIds[0]; |
| 173 | 155 | } elseif ($hostUserId) { |
| @@ -179,20 +161,11 @@ | ||
| 179 | 161 | if (is_wp_error($service)) { |
| 180 | 162 | return MCPHelper::error('slot_service_unavailable', $service->get_error_message()); |
| 181 | 163 | } |
| 182 | 164 | |
| 183 | - // Hold the slot for the duration of the check-then-write. Availability | |
| 184 | - // is computed by a query and the booking is a separate INSERT, so | |
| 185 | - // without this two agents that both pass isSpotAvailable() before either | |
| 186 | - // writes will both write — the "re-checked at execute time" guarantee | |
| 187 | - // narrows the race, it does not remove it. An agent can fire these far | |
| 188 | - // faster than a human clicking through a booking page, and MCP hands the | |
| 189 | - // same slot to whoever asks first. | |
| 190 | - // Every host the booking would occupy, so two event types sharing an | |
| 191 | - // owner cannot both write. getHostIds() is the event's own answer: one | |
| 192 | - // id for a single or group event, all of them for a collective. | |
| 193 | - // Round robin is the exception — its host is chosen inside | |
| 194 | - // isSpotAvailable(), so it locks the event first and the host below. | |
| 165 | + // Hold the slot across check-then-write, or two requests that both pass | |
| 166 | + // isSpotAvailable() before either inserts will both book it. Round | |
| 167 | + // robin locks the event here and its host once one is chosen below. | |
| 195 | 168 | $lock = self::lockSlot($event, $startTime, $endTime, $hostUserId); |
| 196 | 169 | $hostLock = false; |
| 197 | 170 | |
| 198 | 171 | if (!$lock) { |
| @@ -216,12 +189,10 @@ | ||
| 216 | 189 | ] |
| 217 | 190 | ); |
| 218 | 191 | } |
| 219 | 192 | |
| 220 | - // isSpotAvailable() can outrun the lease on a team event with a | |
| 221 | - // cold calendar cache, and an expired lease is stolen without | |
| 222 | - // question — so re-assert it before writing rather than trusting | |
| 223 | - // that the lock taken above is still ours. | |
| 193 | + // isSpotAvailable() can outrun the lease (team event, cold calendar | |
| 194 | + // cache) and an expired lease can be taken, so re-assert it. | |
| 224 | 195 | if (!SlotLock::renewAll($lock)) { |
| 225 | 196 | return MCPHelper::error( |
| 226 | 197 | 'slot_locked', |
| 227 | 198 | __('Another booking for this slot was created while this one was being checked. Call get-available-slots before retrying.', 'fluent-booking'), |
| @@ -233,10 +204,9 @@ | ||
| 233 | 204 | $hostUserId = (int) $service->hostUserId; |
| 234 | 205 | |
| 235 | 206 | $bookingData['host_user_id'] = $hostUserId; |
| 236 | 207 | |
| 237 | - // The first lock could only name the event: round robin has no | |
| 238 | - // host until the line above settles one. Claim that host now. | |
| 208 | + // Round robin has a host only now, so claim it. | |
| 239 | 209 | $hostLock = SlotLock::acquireInterval($event->id, $startTime, $endTime, [$hostUserId]); |
| 240 | 210 | |
| 241 | 211 | if (!$hostLock) { |
| 242 | 212 | return MCPHelper::error( |
| @@ -245,11 +215,10 @@ | ||
| 245 | 215 | ['requested_start' => $startTime] |
| 246 | 216 | ); |
| 247 | 217 | } |
| 248 | 218 | |
| 249 | - // The check above settled this host while only the event was | |
| 250 | - // locked, so another event type could have taken the person in | |
| 251 | - // between. Re-check under the host lock. | |
| 219 | + // Another event type could have booked this host before the | |
| 220 | + // host lock, so re-check under it. | |
| 252 | 221 | $availableSpot = $service->isSpotAvailable($startTime, $endTime, $duration, $hostUserId); |
| 253 | 222 | |
| 254 | 223 | if (!$availableSpot) { |
| 255 | 224 | return MCPHelper::error( |
| @@ -284,13 +253,10 @@ | ||
| 284 | 253 | }; |
| 285 | 254 | |
| 286 | 255 | $booking = $notify ? $create() : NotificationGate::silently($create); |
| 287 | 256 | } catch (\Throwable $e) { |
| 288 | - // Not $e->getMessage(): an ORM or PDO failure carries table names, | |
| 289 | - // SQL fragments and absolute paths, and returning it here would walk | |
| 290 | - // straight past the scrubbing AbilitiesRegistrar does for exactly | |
| 291 | - // this reason. Catching Throwable rather than Exception also means a | |
| 292 | - // TypeError from a downstream service is handled the same way. | |
| 257 | + // Never return $e->getMessage(): DB errors leak table names, SQL | |
| 258 | + // and paths, bypassing AbilitiesRegistrar's scrubbing. | |
| 293 | 259 | self::logException('create-booking', $e); |
| 294 | 260 | |
| 295 | 261 | return MCPHelper::error( |
| 296 | 262 | 'booking_failed', |
| @@ -296,11 +262,10 @@ | ||
| 296 | 262 | 'booking_failed', |
| 297 | 263 | __('The booking could not be created. The site logged the details.', 'fluent-booking') |
| 298 | 264 | ); |
| 299 | 265 | } finally { |
| 300 | - // Every exit from the block above releases the slot, the early | |
| 301 | - // returns included — a lock left behind would block the slot for its | |
| 302 | - // whole TTL after a failure that changed nothing. | |
| 266 | + // Release on every exit, early returns included, so a failure | |
| 267 | + // doesn't block the slot for the lock's TTL. | |
| 303 | 268 | SlotLock::releaseAll($lock); |
| 304 | 269 | SlotLock::releaseAll($hostLock); |
| 305 | 270 | } |
| 306 | 271 | |
| @@ -323,11 +288,10 @@ | ||
| 323 | 288 | return $booking; |
| 324 | 289 | } |
| 325 | 290 | |
| 326 | 291 | /** |
| 327 | - * Move a booking to a new time. Delegates to the same RescheduleService the | |
| 328 | - * public booking form uses, so the two can never disagree about group | |
| 329 | - * re-assignment, round-robin hosts or which emails go out. | |
| 292 | + * Move a booking to a new time via the same RescheduleService the public | |
| 293 | + * booking form uses. | |
| 330 | 294 | * |
| 331 | 295 | * @param Booking $booking |
| 332 | 296 | * @param array $params |
| 333 | 297 | * |
| @@ -349,15 +313,10 @@ | ||
| 349 | 313 | ['status' => $booking->status] |
| 350 | 314 | ); |
| 351 | 315 | } |
| 352 | 316 | |
| 353 | - // Fall back to the ATTENDEE's zone, not the site's. This used to read | |
| 354 | - // `resolveTimezone(...) ?: $booking->person_time_zone`, and | |
| 355 | - // resolveTimezone() never returns anything falsy — its last line is | |
| 356 | - // `return 'UTC'` — so the fallback was unreachable and an omitted | |
| 357 | - // timezone silently meant "site time". For an attendee in Tokyo that | |
| 358 | - // moved the meeting and then overwrote their stored zone with the | |
| 359 | - // site's on the way out. | |
| 317 | + // Default to the attendee's zone, not the site's. resolveTimezone() | |
| 318 | + // never returns empty, so the fallback has to be chosen up front. | |
| 360 | 319 | $requested = trim((string) Arr::get($params, 'timezone', '')); |
| 361 | 320 | |
| 362 | 321 | $timezone = $requested |
| 363 | 322 | ? MCPHelper::resolveTimezone($requested) |
| @@ -383,10 +342,9 @@ | ||
| 383 | 342 | if (is_wp_error($service)) { |
| 384 | 343 | return MCPHelper::error('slot_service_unavailable', $service->get_error_message()); |
| 385 | 344 | } |
| 386 | 345 | |
| 387 | - // Same check-then-write race as create(), and the same hold over it, | |
| 388 | - // round-robin key included. | |
| 346 | + // Same check-then-write lock as create(). | |
| 389 | 347 | $lock = self::lockSlot($event, $startTime, $endTime, $hostUserId); |
| 390 | 348 | $hostLock = false; |
| 391 | 349 | |
| 392 | 350 | if (!$lock) { |
| @@ -397,10 +355,9 @@ | ||
| 397 | 355 | ); |
| 398 | 356 | } |
| 399 | 357 | |
| 400 | 358 | try { |
| 401 | - // Re-check at execute time, not just at preview time — the slot may | |
| 402 | - // have been taken during the confirm round-trip. | |
| 359 | + // The slot may have been taken during the confirm round-trip. | |
| 403 | 360 | if (!$service->isSpotAvailable($startTime, $endTime, $duration, $hostUserId)) { |
| 404 | 361 | return MCPHelper::error( |
| 405 | 362 | 'slot_unavailable', |
| 406 | 363 | __('That time is no longer available. Call get-available-slots for the current openings.', 'fluent-booking'), |
| @@ -410,10 +367,9 @@ | ||
| 410 | 367 | ] |
| 411 | 368 | ); |
| 412 | 369 | } |
| 413 | 370 | |
| 414 | - // Same reason as create(): isSpotAvailable() can outrun the lease, | |
| 415 | - // and an expired one is stolen without question. | |
| 371 | + // As in create(): isSpotAvailable() can outrun the lease. | |
| 416 | 372 | if (!SlotLock::renewAll($lock)) { |
| 417 | 373 | return MCPHelper::error( |
| 418 | 374 | 'slot_locked', |
| 419 | 375 | __('Another booking for the target slot was written while this one was being checked. Call get-available-slots before retrying.', 'fluent-booking'), |
| @@ -461,11 +417,10 @@ | ||
| 461 | 417 | return RescheduleService::reschedule($booking, $event, $startTime, $timezone, [ |
| 462 | 418 | 'reason' => Arr::get($params, 'reason', ''), |
| 463 | 419 | 'host_user_id' => $hostUserId, |
| 464 | 420 | 'source' => __('the MCP server', 'fluent-booking'), |
| 465 | - // An agent always acts for the host; it holds host | |
| 466 | - // credentials, not the attendee's booking link, so the | |
| 467 | - // guest-side reschedule window must not apply to it. | |
| 421 | + // The agent holds host credentials, so the guest-side | |
| 422 | + // reschedule window doesn't apply. | |
| 468 | 423 | 'rescheduled_by' => 'host', |
| 469 | 424 | ]); |
| 470 | 425 | }; |
| 471 | 426 | |
| @@ -485,11 +440,9 @@ | ||
| 485 | 440 | if (is_wp_error($result)) { |
| 486 | 441 | return MCPHelper::error('reschedule_failed', $result->get_error_message()); |
| 487 | 442 | } |
| 488 | 443 | |
| 489 | - // RescheduleService writes its own row, but it records the ROLE ("by | |
| 490 | - // host") rather than the person. Every other MCP write names the | |
| 491 | - // operator, and reschedule is the one most worth attributing. | |
| 444 | + // RescheduleService's own row records the role ("by host"), not the person. | |
| 492 | 445 | self::logActivity( |
| 493 | 446 | $result, |
| 494 | 447 | __('Booking Rescheduled via MCP', 'fluent-booking'), |
| 495 | 448 | /* translators: %1$s: acting user, %2$s: the new start time in UTC */ |
| @@ -529,10 +482,9 @@ | ||
| 529 | 482 | ['status' => $booking->status] |
| 530 | 483 | ); |
| 531 | 484 | } |
| 532 | 485 | |
| 533 | - // Both say a meeting already happened. Marking one three weeks out as | |
| 534 | - // completed reads to every report as a meeting that took place. | |
| 486 | + // Both statuses mean the meeting already happened. | |
| 535 | 487 | if (in_array($action, ['complete', 'no_show'], true) && $booking->end_time > gmdate('Y-m-d H:i:s')) { // phpcs:ignore WordPress.DateTime.RestrictedFunctions.date_date |
| 536 | 488 | return MCPHelper::error( |
| 537 | 489 | 'not_yet_occurred', |
| 538 | 490 | /* translators: %s: the requested status */ |
| @@ -580,11 +532,10 @@ | ||
| 580 | 532 | return Booking::with(['calendar_event'])->find($booking->id); |
| 581 | 533 | } |
| 582 | 534 | |
| 583 | 535 | /** |
| 584 | - * The transition itself. Cancel and reject go through the model methods so | |
| 585 | - * their reason handling, activity rows and hooks stay in one place; the | |
| 586 | - * others mirror patchBooking()'s own sequence. | |
| 536 | + * The transition itself. Cancel and reject go through the model methods; | |
| 537 | + * the others mirror patchBooking()'s sequence. | |
| 587 | 538 | * |
| 588 | 539 | * @return true|\WP_Error |
| 589 | 540 | */ |
| 590 | 541 | private static function transition(Booking $booking, $action, $target, $reason, $params) |
| @@ -591,13 +542,9 @@ | ||
| 591 | 542 | { |
| 592 | 543 | $from = self::statusTransitions()[$action]['from']; |
| 593 | 544 | |
| 594 | 545 | if ($action === 'cancel' || $action === 'reject') { |
| 595 | - // Re-read immediately before mutating so the model's own status | |
| 596 | - // guard runs against current data rather than whatever was loaded | |
| 597 | - // when the request started. An agent can fire these far faster than | |
| 598 | - // a human clicking in wp-admin, so the read-to-write window matters | |
| 599 | - // here in a way it does not there. | |
| 546 | + // Re-read so the model's status guard sees current data. | |
| 600 | 547 | $fresh = Booking::find($booking->id); |
| 601 | 548 | |
| 602 | 549 | if (!$fresh || !in_array($fresh->status, $from, true)) { |
| 603 | 550 | return MCPHelper::error( |
| @@ -608,15 +555,11 @@ | ||
| 608 | 555 | } |
| 609 | 556 | |
| 610 | 557 | $priorStatus = $fresh->status; |
| 611 | 558 | |
| 612 | - // The re-read above narrows the window; it does not close it. Two | |
| 613 | - // hosts cancelling the same collective booking both see `scheduled` | |
| 614 | - // and both proceed — and with refund_payment set, both fire the | |
| 615 | - // gateway's refund hook. So claim the transition atomically first, | |
| 616 | - // exactly as the non-cancel branch below does, and only let the | |
| 617 | - // winner run the side effects. cancelMeeting()/rejectMeeting() then | |
| 618 | - // do their own work on a row we already own. | |
| 559 | + // The re-read doesn't close the race: two hosts cancelling the same | |
| 560 | + // booking would both run side effects, refunds included. Claim the | |
| 561 | + // transition atomically so only the winner proceeds. | |
| 619 | 562 | $claimed = Booking::where('id', $fresh->id) |
| 620 | 563 | ->whereIn('status', $from) |
| 621 | 564 | ->update(['status' => $target]); |
| 622 | 565 | |
| @@ -627,21 +570,15 @@ | ||
| 627 | 570 | ['status' => Booking::where('id', $fresh->id)->value('status')] |
| 628 | 571 | ); |
| 629 | 572 | } |
| 630 | 573 | |
| 631 | - // Hand the model back the status it actually held — not $from[0] — | |
| 632 | - // so cancelMeeting() runs its normal transition instead of | |
| 633 | - // short-circuiting on "already cancelled", and so anything keyed on | |
| 634 | - // the prior status (pending vs scheduled) still sees the truth. | |
| 574 | + // Restore the real prior status in memory so cancelMeeting() doesn't | |
| 575 | + // short-circuit on "already cancelled" and sees pending vs scheduled. | |
| 635 | 576 | $fresh->status = $priorStatus; |
| 636 | 577 | |
| 637 | - // The claim above moved the persisted status ahead of the work that | |
| 638 | - // gives it meaning — the reason, the activity row, the hooks, the | |
| 639 | - // notification, the refund. A failure before any of it must put the | |
| 640 | - // row back. A failure after must not: the hook cancelMeeting() and | |
| 641 | - // rejectMeeting() fire mails the attendee and deletes the remote | |
| 642 | - // calendar event, so reverting there leaves a live booking whose | |
| 643 | - // attendee holds a cancellation. This marks which side it fell on. | |
| 578 | + // A failure before the cancel/reject hook fires rolls the claim back. | |
| 579 | + // After it, the attendee is already notified and the remote event | |
| 580 | + // deleted, so no rollback. $notified marks which side we're on. | |
| 644 | 581 | $notified = false; |
| 645 | 582 | |
| 646 | 583 | $marker = function () use (&$notified) { |
| 647 | 584 | $notified = true; |
| @@ -681,10 +618,9 @@ | ||
| 681 | 618 | } finally { |
| 682 | 619 | remove_action($hook, $marker, PHP_INT_MIN); |
| 683 | 620 | } |
| 684 | 621 | |
| 685 | - // The refund runs after the cancellation is already out. Failing | |
| 686 | - // to move the money is not a reason to un-cancel. | |
| 622 | + // A failed refund doesn't un-cancel the booking. | |
| 687 | 623 | try { |
| 688 | 624 | self::maybeRefund($fresh, $params); |
| 689 | 625 | } catch (\Throwable $e) { |
| 690 | 626 | self::logException('manage-booking:' . $action . ':refund', $e); |
| @@ -698,16 +634,13 @@ | ||
| 698 | 634 | |
| 699 | 635 | return true; |
| 700 | 636 | } |
| 701 | 637 | |
| 702 | - // Same read-then-claim as the cancel branch: the claim reports success, | |
| 703 | - // not which of the allowed statuses the row actually held, and a | |
| 704 | - // rollback needs the real one. | |
| 638 | + // The claim doesn't report which allowed status the row held, and a | |
| 639 | + // rollback needs it. | |
| 705 | 640 | $priorStatus = Booking::where('id', $booking->id)->value('status'); |
| 706 | 641 | |
| 707 | - // Compare-and-set. Two agents racing the same transition both pass the | |
| 708 | - // in-memory status check; only the one whose UPDATE matches a row still | |
| 709 | - // in an allowed status gets to fire the side effects. | |
| 642 | + // Compare-and-set so only one racing request fires the side effects. | |
| 710 | 643 | $claimed = Booking::where('id', $booking->id) |
| 711 | 644 | ->whereIn('status', $from) |
| 712 | 645 | ->update(['status' => $target]); |
| 713 | 646 | |
| @@ -722,15 +655,11 @@ | ||
| 722 | 655 | $booking->status = $target; |
| 723 | 656 | |
| 724 | 657 | $notified = false; |
| 725 | 658 | |
| 726 | - // The claim moved the persisted status ahead of the work that gives it | |
| 727 | - // meaning — the order, the payment status, the activity row, the hooks. | |
| 728 | - // Cancel and reject already put the row back when that work fails; this | |
| 729 | - // branch makes the same claim, so it owes the same guarantee. | |
| 659 | + // Same rollback rule as the cancel branch. | |
| 730 | 660 | try { |
| 731 | - // Confirming a booking that was paid for settles its order, exactly | |
| 732 | - // as the admin's confirm does. | |
| 661 | + // Confirming a paid booking settles its order, as the admin does. | |
| 733 | 662 | if ($action === 'confirm' && $booking->payment_method && $booking->payment_order) { |
| 734 | 663 | $settled = self::settlePayment($booking, $booking->payment_order); |
| 735 | 664 | |
| 736 | 665 | if (is_wp_error($settled)) { |
| @@ -738,11 +667,9 @@ | ||
| 738 | 667 | |
| 739 | 668 | return $settled; |
| 740 | 669 | } |
| 741 | 670 | |
| 742 | - // The admin's confirm writes this row too. Payment reporting | |
| 743 | - // reads the activity trail, so skipping it would make an | |
| 744 | - // agent-confirmed payment look like it never settled. | |
| 671 | + // Payment reporting reads this activity row. | |
| 745 | 672 | do_action('fluent_booking/log_booking_activity', [ |
| 746 | 673 | 'booking_id' => $booking->id, |
| 747 | 674 | 'status' => 'closed', |
| 748 | 675 | 'type' => 'success', |
| @@ -753,9 +680,9 @@ | ||
| 753 | 680 | |
| 754 | 681 | do_action('fluent_booking/payment/update_payment_status_paid', $booking); |
| 755 | 682 | } |
| 756 | 683 | |
| 757 | - // Same commit point as the cancel branch. | |
| 684 | + // Past this point the attendee may be notified; no rollback. | |
| 758 | 685 | $notified = true; |
| 759 | 686 | |
| 760 | 687 | do_action('fluent_booking/booking_schedule_' . $target, $booking, $booking->calendar_event); |
| 761 | 688 | |
| @@ -770,13 +697,10 @@ | ||
| 770 | 697 | if ($notified) { |
| 771 | 698 | return self::partiallyCompleted($booking->id, $target); |
| 772 | 699 | } |
| 773 | 700 | |
| 774 | - // Payment state is left as it is. A booking that is paid for but | |
| 775 | - // still awaiting approval is a normal state here, not a broken one: | |
| 776 | - // it is exactly where a gateway leaves a booking on an event that | |
| 777 | - // requires confirmation. Rolling the order back would invent a | |
| 778 | - // refund that never happened. Only the status claim is undone. | |
| 701 | + // Only the status is rolled back. Paid but pending is a normal | |
| 702 | + // state for events that need confirmation. | |
| 779 | 703 | self::rollbackStatus($booking->id, $priorStatus, $target); |
| 780 | 704 | |
| 781 | 705 | return MCPHelper::error( |
| 782 | 706 | 'transition_failed', |
| @@ -787,18 +711,11 @@ | ||
| 787 | 711 | return true; |
| 788 | 712 | } |
| 789 | 713 | |
| 790 | 714 | /** |
| 791 | - * Mark an order paid and the booking with it, as one commit. | |
| 715 | + * Mark an order and its booking paid in one transaction. Hooks run outside | |
| 716 | + * it so a listener's HTTP call doesn't hold the row locks. | |
| 792 | 717 | * |
| 793 | - * The two rows state the same fact, and they were written in sequence: a | |
| 794 | - * failure between them left the order settled while the booking still read | |
| 795 | - * unpaid, which is a divergence no later call reconciles. | |
| 796 | - * | |
| 797 | - * Only the two writes are inside the transaction. The hooks stay outside | |
| 798 | - * deliberately — a listener that makes an outbound request would otherwise | |
| 799 | - * hold both row locks for the length of someone else's HTTP call. | |
| 800 | - * | |
| 801 | 718 | * @param Booking $booking |
| 802 | 719 | * @param object $order |
| 803 | 720 | * |
| 804 | 721 | * @return true|\WP_Error |
| @@ -827,9 +744,11 @@ | ||
| 827 | 744 | return true; |
| 828 | 745 | } |
| 829 | 746 | |
| 830 | 747 | /** |
| 831 | - * Claim the slot for every host this booking would occupy. | |
| 748 | + * Claim the slot for every host this booking would occupy, so two event | |
| 749 | + * types sharing a host can't both book it. Round robin locks the event | |
| 750 | + * only, since its host isn't chosen yet. | |
| 832 | 751 | * |
| 833 | 752 | * @param CalendarSlot $event |
| 834 | 753 | * @param string $startTime |
| 835 | 754 | * @param string $endTime |
| @@ -857,13 +776,10 @@ | ||
| 857 | 776 | } |
| 858 | 777 | |
| 859 | 778 | /** |
| 860 | 779 | * Undo a claimed status transition whose side effects did not complete. |
| 780 | + * Only applies if the row still holds the claimed status. | |
| 861 | 781 | * |
| 862 | - * Conditional on the row still holding the status we claimed: if something | |
| 863 | - * downstream already moved it on, that later state is the current truth and | |
| 864 | - * stamping the old one back over it would be its own corruption. | |
| 865 | - * | |
| 866 | 782 | * @param int $bookingId |
| 867 | 783 | * @param string $priorStatus |
| 868 | 784 | * @param string $claimedStatus |
| 869 | 785 | */ |
| @@ -893,11 +809,10 @@ | ||
| 893 | 809 | ); |
| 894 | 810 | } |
| 895 | 811 | |
| 896 | 812 | /** |
| 897 | - * Cancelling or rejecting a paid booking can refund it, but only when the | |
| 898 | - * caller asks explicitly — an agent must never move money as a side effect | |
| 899 | - * of a status change. | |
| 813 | + * Refund a cancelled or rejected paid booking, only when refund_payment is | |
| 814 | + * explicitly set. | |
| 900 | 815 | */ |
| 901 | 816 | private static function maybeRefund(Booking $booking, $params) |
| 902 | 817 | { |
| 903 | 818 | if (!$booking->payment_method || !Arr::isTrue($params, 'refund_payment')) { |
| @@ -907,10 +822,9 @@ | ||
| 907 | 822 | do_action('fluent_booking/refund_payment_' . $booking->payment_method, $booking, $booking->calendar_event); |
| 908 | 823 | } |
| 909 | 824 | |
| 910 | 825 | /** |
| 911 | - * Edit an attendee's details on an existing booking. Reversible, so it is | |
| 912 | - * not gated behind a confirm token — but it still writes an activity row. | |
| 826 | + * Edit an attendee's details. Reversible, so no confirm token. | |
| 913 | 827 | * |
| 914 | 828 | * @param Booking $booking |
| 915 | 829 | * @param array $fields |
| 916 | 830 | * @param array $params |
| @@ -975,11 +889,9 @@ | ||
| 975 | 889 | $booking->fill($updates); |
| 976 | 890 | $booking->save(); |
| 977 | 891 | |
| 978 | 892 | foreach ($updates as $key => $value) { |
| 979 | - // patchBooking fires one hook per column; keeping that shape | |
| 980 | - // means existing listeners (the changed-email notification | |
| 981 | - // among them) behave identically. | |
| 893 | + // One hook per column, as patchBooking fires them. | |
| 982 | 894 | do_action('fluent_booking/after_patch_booking_' . $key, $booking, $booking->calendar_event, $before[$key]); |
| 983 | 895 | } |
| 984 | 896 | |
| 985 | 897 | return true; |
| @@ -1008,11 +920,9 @@ | ||
| 1008 | 920 | * @return array|\WP_Error |
| 1009 | 921 | */ |
| 1010 | 922 | public static function resendEmail(Booking $booking, $emailTo, $params = []) |
| 1011 | 923 | { |
| 1012 | - // The whole action is "send an email". Silently sending one after the | |
| 1013 | - // caller asked for silence — and after the dry run reported | |
| 1014 | - // notifications_requested:false — is worse than refusing. | |
| 924 | + // The action is sending an email, so refuse if notifications are off. | |
| 1015 | 925 | if (!self::wantsNotifications($params)) { |
| 1016 | 926 | return MCPHelper::error( |
| 1017 | 927 | 'notifications_disabled', |
| 1018 | 928 | __('resend_email exists to send an email, so it cannot run with send_notifications:false. Drop that parameter, or use a different action.', 'fluent-booking') |
| @@ -1018,10 +928,9 @@ | ||
| 1018 | 928 | __('resend_email exists to send an email, so it cannot run with send_notifications:false. Drop that parameter, or use a different action.', 'fluent-booking') |
| 1019 | 929 | ); |
| 1020 | 930 | } |
| 1021 | 931 | |
| 1022 | - // The template says the booking is going ahead, so sending it for a | |
| 1023 | - // cancelled or rejected one tells the attendee the opposite of the truth. | |
| 932 | + // The template says the booking is going ahead. | |
| 1024 | 933 | if (!in_array($booking->status, ['scheduled', 'rescheduled', 'pending'], true)) { |
| 1025 | 934 | return MCPHelper::error( |
| 1026 | 935 | 'not_resendable', |
| 1027 | 936 | /* translators: %s: the booking's current status */ |
| @@ -1076,11 +985,10 @@ | ||
| 1076 | 985 | return ['recipient' => $emailTo, 'sent' => true]; |
| 1077 | 986 | } |
| 1078 | 987 | |
| 1079 | 988 | /** |
| 1080 | - * Whether the caller may act on this booking. Read access is not enough: | |
| 1081 | - * writing requires being one of the booking's hosts, or holding write | |
| 1082 | - * access to its calendar, or site-wide booking management. | |
| 989 | + * Whether the caller may change this booking: site-wide booking access, | |
| 990 | + * or being a host of the booking or its event. | |
| 1083 | 991 | * |
| 1084 | 992 | * @param Booking $booking |
| 1085 | 993 | * |
| 1086 | 994 | * @return bool |
| @@ -1112,23 +1020,14 @@ | ||
| 1112 | 1020 | return $event && in_array((int) $userId, array_map('intval', (array) $event->getHostIds()), true); |
| 1113 | 1021 | } |
| 1114 | 1022 | |
| 1115 | 1023 | /** |
| 1116 | - * Notifications are on unless the caller turns them off — a booking the | |
| 1117 | - * attendee never hears about is a strange default, and matches what the | |
| 1118 | - * same action in wp-admin would do. | |
| 1024 | + * Whether the caller asked for notifications on this change. On unless | |
| 1025 | + * turned off, matching wp-admin. | |
| 1119 | 1026 | * |
| 1120 | - * @param array $params | |
| 1027 | + * Reported as `notifications_requested`, not `sent`: nothing here waits on | |
| 1028 | + * delivery. | |
| 1121 | 1029 | * |
| 1122 | - * @return bool | |
| 1123 | - */ | |
| 1124 | - /** | |
| 1125 | - * Whether the caller asked for notifications on this change. | |
| 1126 | - * | |
| 1127 | - * Reported as `notifications_requested`, not `notifications_sent`: nothing | |
| 1128 | - * here waits on SMTP, calendar sync or Twilio, so it cannot claim delivery. | |
| 1129 | - * Failures land in the booking's activity log. | |
| 1130 | - * | |
| 1131 | 1030 | * @param array $params |
| 1132 | 1031 | * |
| 1133 | 1032 | * @return bool |
| 1134 | 1033 | */ |
| @@ -1141,20 +1040,12 @@ | ||
| 1141 | 1040 | return Arr::isTrue($params, 'send_notifications'); |
| 1142 | 1041 | } |
| 1143 | 1042 | |
| 1144 | 1043 | /** |
| 1145 | - * Everything create() checks before it touches the slot engine, so a dry run | |
| 1146 | - * can run the same gauntlet. | |
| 1044 | + * The checks create() runs before the slot engine, so a dry run can't pass | |
| 1045 | + * a call the execute would reject. Availability is re-checked at execute | |
| 1046 | + * time instead. | |
| 1147 | 1047 | * |
| 1148 | - * A preview that succeeds and an execute that then fails on `location_required` | |
| 1149 | - * is worse than no preview: the agent reports "ready to book" to a human, | |
| 1150 | - * gets approval, and only then discovers the call was never valid. The | |
| 1151 | - * preview is a promise about the execute, so it has to be checked against | |
| 1152 | - * the same rules. | |
| 1153 | - * | |
| 1154 | - * Availability is deliberately NOT part of this — it is re-checked at | |
| 1155 | - * execute time by design, and the preview says so. | |
| 1156 | - * | |
| 1157 | 1048 | * @param CalendarSlot $event |
| 1158 | 1049 | * @param array $params |
| 1159 | 1050 | * |
| 1160 | 1051 | * @return true|\WP_Error |
| @@ -1250,12 +1141,10 @@ | ||
| 1250 | 1141 | do_action('fluent_booking/mcp_write_exception', $context, $e); |
| 1251 | 1142 | } |
| 1252 | 1143 | |
| 1253 | 1144 | /** |
| 1254 | - * Resolve the booking's location from the event's configured locations. An | |
| 1255 | - * agent may name a location type; when it does not, and the event offers | |
| 1256 | - * exactly one, we use it — asking a model to choose between one option is | |
| 1257 | - * a round-trip for nothing. | |
| 1145 | + * Resolve the booking's location from the event's configured locations. | |
| 1146 | + * With no location_type given, a single configured location is used. | |
| 1258 | 1147 | * |
| 1259 | 1148 | * @param CalendarSlot $event |
| 1260 | 1149 | * @param array $params |
| 1261 | 1150 | * |
| @@ -1344,10 +1233,9 @@ | ||
| 1344 | 1233 | $guests = []; |
| 1345 | 1234 | $rejected = []; |
| 1346 | 1235 | |
| 1347 | 1236 | foreach ((array) Arr::get($params, 'guests', []) as $guest) { |
| 1348 | - // Accept either shape. An agent naturally sends addresses; a group | |
| 1349 | - // event needs a name per seat, so an object is allowed too. | |
| 1237 | + // An email string or a {name, email} object. | |
| 1350 | 1238 | if (is_array($guest)) { |
| 1351 | 1239 | $email = sanitize_email((string) Arr::get($guest, 'email', '')); |
| 1352 | 1240 | $name = sanitize_text_field((string) Arr::get($guest, 'name', '')); |
| 1353 | 1241 | } else { |
| @@ -1367,14 +1255,10 @@ | ||
| 1367 | 1255 | $guests[] = $email; |
| 1368 | 1256 | continue; |
| 1369 | 1257 | } |
| 1370 | 1258 | |
| 1371 | - // A multi-guest event seats each guest as their own attendee, and | |
| 1372 | - // BookingService::prepareBookingData() reads $guest['name'] and | |
| 1373 | - // $guest['email'] off every entry. Handing it bare strings raised | |
| 1374 | - // "Cannot access offset of type string on string" on PHP 8 and | |
| 1375 | - // produced nameless attendees on 7.4 — so the group-event path, the | |
| 1376 | - // one the seat arithmetic below exists for, could never work. | |
| 1259 | + // Multi-guest events seat each guest as an attendee, and | |
| 1260 | + // BookingService::prepareBookingData() expects name and email keys. | |
| 1377 | 1261 | $guests[] = [ |
| 1378 | 1262 | 'name' => $name ?: self::nameFromEmail($email), |
| 1379 | 1263 | 'email' => $email, |
| 1380 | 1264 | ]; |
| @@ -1386,10 +1270,9 @@ | ||
| 1386 | 1270 | |
| 1387 | 1271 | if ($isMultiGuest && is_array($availableSpot)) { |
| 1388 | 1272 | $remaining = (int) Arr::get($availableSpot, 'remaining', $event->getMaxBookingPerSlot()); |
| 1389 | 1273 | |
| 1390 | - // Minus one: the attendee themself takes a seat. On a group event | |
| 1391 | - // with a single seat left this is 0, which drops every guest. | |
| 1274 | + // Minus one for the attendee's own seat. | |
| 1392 | 1275 | if (min($remaining, $limit) - 1 < $limit) { |
| 1393 | 1276 | $reason = 'no_seats_left'; |
| 1394 | 1277 | } |
| 1395 | 1278 | |
| @@ -1404,10 +1287,9 @@ | ||
| 1404 | 1287 | 'reason' => $reason, |
| 1405 | 1288 | ]; |
| 1406 | 1289 | } |
| 1407 | 1290 | |
| 1408 | - // Every one of these three drops used to be silent, so an agent asked | |
| 1409 | - // to book four people was told `created: true` for a booking with one. | |
| 1291 | + // Reported back so the agent knows which guests weren't booked. | |
| 1410 | 1292 | self::$guestsDropped = $rejected; |
| 1411 | 1293 | |
| 1412 | 1294 | return $kept; |
| 1413 | 1295 | } |
| @@ -1412,13 +1294,11 @@ | ||
| 1412 | 1294 | return $kept; |
| 1413 | 1295 | } |
| 1414 | 1296 | |
| 1415 | 1297 | /** |
| 1416 | - * How many requested guests a create could actually seat, for the preview. | |
| 1298 | + * How many requested guests a create could seat, for the preview. A ceiling: | |
| 1299 | + * group event seats are only checked at execute time. | |
| 1417 | 1300 | * |
| 1418 | - * Addresses and the guest field's own limit only — a group event's free | |
| 1419 | - * seats are re-read at execute time, so this is a ceiling, not a promise. | |
| 1420 | - * | |
| 1421 | 1301 | * @param CalendarSlot $event |
| 1422 | 1302 | * @param array $params |
| 1423 | 1303 | * |
| 1424 | 1304 | * @return int |
| @@ -1440,13 +1320,11 @@ | ||
| 1440 | 1320 | return min($valid, (int) Arr::get($guestField, 'limit', 10)); |
| 1441 | 1321 | } |
| 1442 | 1322 | |
| 1443 | 1323 | /** |
| 1444 | - * Guests the last create() was asked for and did not book. | |
| 1324 | + * Guests the last create() was asked for and did not book. Request-scoped; | |
| 1325 | + * one MCP call creates one booking. | |
| 1445 | 1326 | * |
| 1446 | - * Request-scoped: set by sanitizeGuests() during the create, read once by | |
| 1447 | - * the tool building the response. One MCP call creates one booking. | |
| 1448 | - * | |
| 1449 | 1327 | * @return array |
| 1450 | 1328 | */ |
| 1451 | 1329 | public static function droppedGuests() |
| 1452 | 1330 | { |
| @@ -1468,10 +1346,9 @@ | ||
| 1468 | 1346 | return $name ? ucwords($name) : (string) $email; |
| 1469 | 1347 | } |
| 1470 | 1348 | |
| 1471 | 1349 | /** |
| 1472 | - * Every MCP write leaves a trail naming the operator and the channel, so a | |
| 1473 | - * booking's timeline distinguishes an agent's action from a human's. | |
| 1350 | + * Log an activity row naming the operator and the MCP channel. | |
| 1474 | 1351 | * |
| 1475 | 1352 | * @param Booking $booking |
| 1476 | 1353 | * @param string $title |
| 1477 | 1354 | * @param string $description |
