| @@ -12,29 +12,20 @@ | ||
| 12 | 12 | |
| 13 | 13 | defined('ABSPATH') || exit; |
| 14 | 14 | |
| 15 | 15 | /** |
| 16 | - * Reading bookings — the surface an agent spends most of its calls on. | |
| 16 | + * Read-only booking tools. Attendees, hosts etc. are `include` values on | |
| 17 | + * get-booking rather than separate tools, to save schema tokens. | |
| 17 | 18 | * |
| 18 | - * Two tools rather than five. Cal.com ships separate tools for a booking's | |
| 19 | - * attendees; here that is an `include` value on `get-booking`, because the | |
| 20 | - * parameter shape is identical and a separate tool would cost another ~500 | |
| 21 | - * tokens of permanently-resident schema to save one round-trip nobody makes. | |
| 22 | - * | |
| 23 | - * Scoping is done in the query, never in the response. A host without | |
| 24 | - * read-all-bookings permission gets a query that cannot see other hosts' rows | |
| 25 | - * at all, so counts, pagination totals and results are all consistent with what | |
| 26 | - * they are allowed to know. Filtering after the fact leaks the totals. | |
| 19 | + * Scoping happens in the query, not the response, so totals and pagination | |
| 20 | + * never leak rows the caller can't see. | |
| 27 | 21 | */ |
| 28 | 22 | class BookingTools |
| 29 | 23 | { |
| 30 | 24 | const DEFAULT_PER_PAGE = 20; |
| 31 | 25 | |
| 32 | - /** | |
| 33 | - * `include` values get-booking accepts. Each one costs a query or an | |
| 34 | - * unserialize, which is exactly why none of them are on by default. | |
| 35 | - */ | |
| 36 | - const INCLUDABLE = ['custom_fields', 'attendees', 'hosts', 'activities']; | |
| 26 | + // `include` values for get-booking. Each costs a query, so none are default. | |
| 27 | + const INCLUDABLE = ['custom_fields', 'attendees', 'hosts', 'activities', 'notes']; | |
| 37 | 28 | |
| 38 | 29 | public static function definitions() |
| 39 | 30 | { |
| 40 | 31 | return [ |
| @@ -113,9 +104,9 @@ | ||
| 113 | 104 | ], |
| 114 | 105 | |
| 115 | 106 | 'fluent-booking/get-booking' => [ |
| 116 | 107 | 'label' => __('Get booking', 'fluent-booking'), |
| 117 | - 'description' => __('Full detail for one booking by id or hash, including attendee contact details, location, status history and cancellation reason. Use include to add form answers, guests, hosts or the activity timeline.', 'fluent-booking'), | |
| 108 | + 'description' => __('Full detail for one booking by id or hash, including attendee contact details, location, status history and cancellation reason. Use include to add form answers, guests, hosts or the activity timeline. include notes is a Pro section: host notes, oldest first.', 'fluent-booking'), | |
| 118 | 109 | 'input_schema' => [ |
| 119 | 110 | 'type' => 'object', |
| 120 | 111 | 'properties' => [ |
| 121 | 112 | 'booking_id' => ['type' => 'integer'], |
| @@ -162,17 +153,13 @@ | ||
| 162 | 153 | if (is_wp_error($query)) { |
| 163 | 154 | return $query; |
| 164 | 155 | } |
| 165 | 156 | |
| 166 | - // The admin list collapses group bookings on group_id so a ten-attendee | |
| 167 | - // group event reads as one booking rather than ten. Default to the same | |
| 168 | - // thing: an agent that reports a different number than the operator's | |
| 169 | - // screen is worse than useless. | |
| 157 | + // Collapse group bookings by default, like the admin list, so counts match. | |
| 170 | 158 | $grouped = (bool) Arr::get($params, 'group_bookings', true); |
| 171 | 159 | |
| 172 | - // ...except when searching. GROUP BY keeps one arbitrary row per group, | |
| 173 | - // so a term matching two attendees of the same group could drop the | |
| 174 | - // exact match in favour of the weaker one beside it. | |
| 160 | + // Not when searching: GROUP BY keeps an arbitrary row per group and | |
| 161 | + // could hide the attendee that actually matched. | |
| 175 | 162 | $searchCollapsed = $grouped && trim((string) Arr::get($params, 'search', '')) !== ''; |
| 176 | 163 | |
| 177 | 164 | if ($searchCollapsed) { |
| 178 | 165 | $grouped = false; |
| @@ -180,13 +167,10 @@ | ||
| 180 | 167 | |
| 181 | 168 | $perPage = MCPHelper::perPage(Arr::get($params, 'per_page'), self::DEFAULT_PER_PAGE); |
| 182 | 169 | $page = max(1, absint(Arr::get($params, 'page', 1))); |
| 183 | 170 | |
| 184 | - // Count BEFORE the groupBy is applied. COUNT() over a grouped query | |
| 185 | - // returns the size of the first group, not the number of groups — which | |
| 186 | - // reads as a plausible small number rather than an error, so an agent | |
| 187 | - // would report "1 booking" over a page of seventeen and never know. | |
| 188 | - // Mirrors SchedulesController::addCountsForFirstPage(). | |
| 171 | + // Count before groupBy: COUNT() on a grouped query returns the first | |
| 172 | + // group's size. Mirrors SchedulesController::addCountsForFirstPage(). | |
| 189 | 173 | $total = $grouped |
| 190 | 174 | ? (clone $query)->withoutEagerLoads()->distinct('group_id')->count('group_id') |
| 191 | 175 | : (clone $query)->withoutEagerLoads()->count(); |
| 192 | 176 | |
| @@ -198,11 +182,10 @@ | ||
| 198 | 182 | ->skip(($page - 1) * $perPage) |
| 199 | 183 | ->take($perPage) |
| 200 | 184 | ->get(); |
| 201 | 185 | |
| 202 | - // Unmasked emails are a read-all-bookings privilege. Asking for them | |
| 203 | - // without that permission is not an error — the rows are still useful — | |
| 204 | - // so the request is downgraded and the response says it was. | |
| 186 | + // Unmasked emails need read-all-bookings. Without it, downgrade and | |
| 187 | + // flag pii_masked rather than erroring. | |
| 205 | 188 | $wantsPii = (bool) Arr::get($params, 'include_pii', false); |
| 206 | 189 | $includePii = $wantsPii && $seesAll; |
| 207 | 190 | |
| 208 | 191 | $rows = []; |
| @@ -277,10 +260,18 @@ | ||
| 277 | 260 | |
| 278 | 261 | $include = Arr::get($params, 'include', []); |
| 279 | 262 | $include = is_array($include) ? array_intersect($include, self::INCLUDABLE) : []; |
| 280 | 263 | |
| 264 | + $bookingData = apply_filters( | |
| 265 | + 'fluent_booking/mcp_booking_full', | |
| 266 | + BookingProjector::full($booking, $timezone, $include), | |
| 267 | + $booking, | |
| 268 | + $timezone, | |
| 269 | + $include | |
| 270 | + ); | |
| 271 | + | |
| 281 | 272 | return MCPHelper::success( |
| 282 | - BookingProjector::full($booking, $timezone, $include), | |
| 273 | + $bookingData, | |
| 283 | 274 | [ |
| 284 | 275 | 'timezone' => $timezone, |
| 285 | 276 | 'scope' => PermissionGate::currentScope(), |
| 286 | 277 | ] |
| @@ -287,23 +278,12 @@ | ||
| 287 | 278 | ); |
| 288 | 279 | } |
| 289 | 280 | |
| 290 | 281 | /** |
| 291 | - * True when the caller may read this specific booking. | |
| 282 | + * Whether the caller may read this booking. Must match the scope | |
| 283 | + * list-bookings uses (Booking::whereHostAccess()), so a booking hidden from | |
| 284 | + * the list can't be opened by guessing its id. | |
| 292 | 285 | * |
| 293 | - * Deliberately the SAME test `list-bookings` scopes its query with — | |
| 294 | - * `Booking::whereHostAccess()`, i.e. own the calendar or be a host on this | |
| 295 | - * booking. It used to fall back to `PermissionManager::canReadCalendar()`, | |
| 296 | - * which is a broader question than it sounds: `canReadCalendar()` treats a | |
| 297 | - * calendar as readable if the caller is a team member on *any one* of its | |
| 298 | - * event types, so on a shared team calendar it returned true for every | |
| 299 | - * booking on every other event type too. | |
| 300 | - * | |
| 301 | - * The effect was a single-record read that was wider than the list beside | |
| 302 | - * it, on sequential integer ids, while still stamping the response | |
| 303 | - * `scope: own_calendars`. An agent that cannot see a booking in | |
| 304 | - * `list-bookings` must not be able to open it by guessing its id. | |
| 305 | - * | |
| 306 | 286 | * @param Booking $booking |
| 307 | 287 | * @return bool |
| 308 | 288 | */ |
| 309 | 289 | private static function canReadBooking(Booking $booking) |
| @@ -321,10 +301,10 @@ | ||
| 321 | 301 | if (in_array($userId, array_map('intval', (array) $booking->getHostIds()), true)) { |
| 322 | 302 | return true; |
| 323 | 303 | } |
| 324 | 304 | |
| 325 | - // Calendar ownership, matching whereHostAccess()'s first branch. Not | |
| 326 | - // canReadCalendar(): that also admits shared calendars. | |
| 305 | + // Calendar ownership, as in whereHostAccess(). Not canReadCalendar(): | |
| 306 | + // it admits any team member on a shared calendar. | |
| 327 | 307 | return (bool) Calendar::where('id', $booking->calendar_id) |
| 328 | 308 | ->where('user_id', $userId) |
| 329 | 309 | ->exists(); |
| 330 | 310 | } |
| @@ -329,14 +309,11 @@ | ||
| 329 | 309 | ->exists(); |
| 330 | 310 | } |
| 331 | 311 | |
| 332 | 312 | /** |
| 333 | - * Compose the list query from the filters, scoped to what the caller may see. | |
| 313 | + * Build the list query, scoped to what the caller may see. Filters reuse | |
| 314 | + * the Booking scopes the admin list uses, so the two agree. | |
| 334 | 315 | * |
| 335 | - * Every filter here delegates to an existing Booking scope, so MCP results | |
| 336 | - * and the admin schedules list are produced by the same code — the two | |
| 337 | - * cannot drift into disagreeing about what "upcoming" or "cancelled" means. | |
| 338 | - * | |
| 339 | 316 | * @param array $params |
| 340 | 317 | * @param bool $seesAll |
| 341 | 318 | * @param string $timezone resolved IANA identifier the from/to dates are read in |
| 342 | 319 | * @return object|\WP_Error |
| @@ -388,13 +365,9 @@ | ||
| 388 | 365 | if ($range) { |
| 389 | 366 | $query->applyDateRangeFilter($range); |
| 390 | 367 | } |
| 391 | 368 | |
| 392 | - // Applied before the status/period branch below, because that branch | |
| 393 | - // returns early: a search silently dropped whenever `status` was also | |
| 394 | - // passed produced a full unfiltered result set that reads exactly like | |
| 395 | - // a successful search, which is the one failure mode this module is | |
| 396 | - // least able to recover from. | |
| 369 | + // Before the status branch below, which returns early. | |
| 397 | 370 | $search = sanitize_text_field((string) Arr::get($params, 'search', '')); |
| 398 | 371 | |
| 399 | 372 | if ($search) { |
| 400 | 373 | $query->searchBy($search); |
| @@ -399,12 +372,10 @@ | ||
| 399 | 372 | if ($search) { |
| 400 | 373 | $query->searchBy($search); |
| 401 | 374 | } |
| 402 | 375 | |
| 403 | - // A raw status filter and a period bucket answer different questions | |
| 404 | - // ("rows whose column says cancelled" vs "rows the admin shows under | |
| 405 | - // Cancelled"), so an explicit status list wins rather than being ANDed | |
| 406 | - // into a contradiction that silently returns nothing. | |
| 376 | + // An explicit status list replaces the period bucket. ANDing the two | |
| 377 | + // can contradict and silently return nothing. | |
| 407 | 378 | $statuses = Arr::get($params, 'status', []); |
| 408 | 379 | $statuses = is_array($statuses) ? array_filter(array_map('sanitize_text_field', $statuses)) : []; |
| 409 | 380 | |
| 410 | 381 | if ($statuses) { |
| @@ -426,18 +397,11 @@ | ||
| 426 | 397 | return $query; |
| 427 | 398 | } |
| 428 | 399 | |
| 429 | 400 | /** |
| 430 | - * Translate the caller's from/to dates into the UTC window they mean. | |
| 401 | + * Convert from/to dates into a UTC window. The dates are local calendar | |
| 402 | + * days in $timezone, while `start_time` is stored in UTC. | |
| 431 | 403 | * |
| 432 | - * `start_time` is stored in UTC, but "bookings on 2026-08-24" is a question | |
| 433 | - * about a local calendar day. Matching the UTC column against a bare | |
| 434 | - * '2026-08-24 00:00:00'–'23:59:59' answers a question up to fourteen hours | |
| 435 | - * out of alignment with the one asked: in America/Los_Angeles it silently | |
| 436 | - * drops everything from 5pm Monday onward and folds in Sunday evening | |
| 437 | - * instead. The dates are therefore read in the same timezone the response | |
| 438 | - * renders its *_local times in. | |
| 439 | - * | |
| 440 | 404 | * @param array $params |
| 441 | 405 | * @param string $timezone resolved IANA identifier |
| 442 | 406 | * @return array|\WP_Error [] when unbounded |
| 443 | 407 | */ |
| @@ -459,10 +423,9 @@ | ||
| 459 | 423 | ); |
| 460 | 424 | } |
| 461 | 425 | } |
| 462 | 426 | |
| 463 | - // An open-ended bound is a usable question; fill the other end rather | |
| 464 | - // than rejecting it. | |
| 427 | + // Open-ended ranges are allowed. | |
| 465 | 428 | $start = $from ? MCPHelper::dayBoundaryToUtc($from, $timezone, false) : '1970-01-01 00:00:00'; |
| 466 | 429 | $end = $to ? MCPHelper::dayBoundaryToUtc($to, $timezone, true) : '2999-12-31 23:59:59'; |
| 467 | 430 | |
| 468 | 431 | if ($end < $start) { |
