| @@ -20,16 +20,13 @@ | ||
| 20 | 20 | |
| 21 | 21 | /** |
| 22 | 22 | * The `scheduling` toolset: configuration work, off by default. |
| 23 | 23 | * |
| 24 | - * Most agent sessions read bookings and availability and never touch setup, so | |
| 25 | - * these four tools stay out of the default context budget until an operator | |
| 26 | - * turns them on. When they are on they cost about as much again as the core | |
| 27 | - * nine — which is exactly why they are a separate switch rather than always | |
| 28 | - * present. | |
| 24 | + * Most sessions never touch setup, and these four tools cost about as much | |
| 25 | + * context as the core nine, so they sit behind their own switch. | |
| 29 | 26 | * |
| 30 | - * Writes here go through RestBridge so the admin's own validation runs. Reads | |
| 31 | - * are projected by hand, because the admin's responses are shaped for a UI. | |
| 27 | + * Writes go through RestBridge so the admin's validation runs. Reads are | |
| 28 | + * projected by hand, because admin responses are shaped for a UI. | |
| 32 | 29 | * |
| 33 | 30 | * @see \FluentBooking\App\Modules\MCP\Support\RestBridge |
| 34 | 31 | */ |
| 35 | 32 | class SchedulingTools |
| @@ -36,11 +33,10 @@ | ||
| 36 | 33 | { |
| 37 | 34 | const REFERENCE_KINDS = ['hosts', 'calendars', 'location_providers', 'booking_fields', 'availability_schedules']; |
| 38 | 35 | |
| 39 | 36 | /** |
| 40 | - * Ceiling on any one reference list. Never applied silently — every list | |
| 41 | - * that hits it says so and reports the real total, because a list that | |
| 42 | - * stops at 100 with no note reads as a complete list of 100. | |
| 37 | + * Cap on any one reference list. A list that hits it says so and reports | |
| 38 | + * the real total. | |
| 43 | 39 | */ |
| 44 | 40 | const LIST_LIMIT = 100; |
| 45 | 41 | |
| 46 | 42 | public static function definitions() |
| @@ -234,11 +230,10 @@ | ||
| 234 | 230 | } |
| 235 | 231 | |
| 236 | 232 | $projected = self::projectSchedule($schedule, $timezone, true); |
| 237 | 233 | |
| 238 | - // The row already resolved which zone its hours are really in — a | |
| 239 | - // requested one only if valid, the schedule's own otherwise. Read | |
| 240 | - // it back rather than restating the request. | |
| 234 | + // Report the zone the row resolved, not the requested one, which | |
| 235 | + // may have been invalid. | |
| 241 | 236 | return MCPHelper::success($projected, ['timezone' => $projected['timezone']]); |
| 242 | 237 | } |
| 243 | 238 | |
| 244 | 239 | $query = Availability::orderBy('id', 'desc'); |
| @@ -252,10 +247,9 @@ | ||
| 252 | 247 | $total = (clone $query)->count(); |
| 253 | 248 | |
| 254 | 249 | $schedules = []; |
| 255 | 250 | |
| 256 | - // Fetch one past the cap so a truncated list can say so. A list that | |
| 257 | - // silently stops at 100 reads as a complete list of 100. | |
| 251 | + // Fetch one past the cap so a truncated list can say so. | |
| 258 | 252 | $rows = []; |
| 259 | 253 | |
| 260 | 254 | foreach ($query->limit(self::LIST_LIMIT + 1)->get() as $schedule) { |
| 261 | 255 | if (count($rows) >= self::LIST_LIMIT) { |
| @@ -309,10 +303,9 @@ | ||
| 309 | 303 | return self::manageAvailabilityAction($params); |
| 310 | 304 | }, function ($params) { |
| 311 | 305 | $scheduleId = absint(Arr::get($params, 'schedule_id')); |
| 312 | 306 | |
| 313 | - // A create has no id yet, so the title stands in for one: a retry | |
| 314 | - // that means the same schedule names it the same way. | |
| 307 | + // A create has no id yet, so the title stands in for one. | |
| 315 | 308 | return 'availability:' . ($scheduleId ?: 'new:' . md5(strtolower(trim((string) Arr::get($params, 'title', ''))))) . ':' . sanitize_text_field(Arr::get($params, 'action', '')); |
| 316 | 309 | }); |
| 317 | 310 | } |
| 318 | 311 | |
| @@ -331,11 +324,10 @@ | ||
| 331 | 324 | if (!$title) { |
| 332 | 325 | return MCPHelper::error('missing_title', __('create needs a title.', 'fluent-booking')); |
| 333 | 326 | } |
| 334 | 327 | |
| 335 | - // create takes a title and a timezone and lays down the stock | |
| 336 | - // Mon–Fri grid; it has nowhere to put hours, so refuse rather than | |
| 337 | - // report success on a schedule with the wrong ones. | |
| 328 | + // create always lays down the stock Mon-Fri grid. Refuse hours | |
| 329 | + // rather than report success on a schedule without them. | |
| 338 | 330 | if (Arr::get($params, 'weekly_schedules') || Arr::get($params, 'date_overrides')) { |
| 339 | 331 | return MCPHelper::error( |
| 340 | 332 | 'hours_not_accepted_on_create', |
| 341 | 333 | __('create makes a schedule with the default weekly hours; it cannot set them. Create it first, then call update with weekly_schedules to replace the grid.', 'fluent-booking') |
| @@ -427,12 +419,10 @@ | ||
| 427 | 419 | __('update replaces the whole weekly grid, so weekly_schedules is required. Read the schedule with get-availability first and send it back changed.', 'fluent-booking') |
| 428 | 420 | ); |
| 429 | 421 | } |
| 430 | 422 | |
| 431 | - // `update` replaces the entire grid, so the hours it overwrites are | |
| 432 | - // gone — there is no per-day merge and no undo. That makes it | |
| 433 | - // destructive in every sense that matters, and it is gated the same | |
| 434 | - // way delete is: preview, then a token bound to these exact hours. | |
| 423 | + // update replaces the whole grid with no merge and no undo, so it | |
| 424 | + // is gated like delete: preview, then a token bound to these hours. | |
| 435 | 425 | $tool = 'fluent-booking/manage-availability'; |
| 436 | 426 | $entityKey = 'availability:' . $scheduleId . ':update'; |
| 437 | 427 | $fingerprint = self::scheduleFingerprint($schedule); |
| 438 | 428 | $digest = WriteGuard::paramsDigest($params); |
| @@ -471,16 +461,13 @@ | ||
| 471 | 461 | return MCPHelper::error('unsupported_action', __('Unknown action.', 'fluent-booking')); |
| 472 | 462 | } |
| 473 | 463 | |
| 474 | 464 | /** |
| 475 | - * Preview shape for an action that only ever adds something. | |
| 465 | + * Preview shape for an action that only adds something. | |
| 476 | 466 | * |
| 477 | - * `dry_run` has to mean "changed nothing" for EVERY action a tool exposes, | |
| 478 | - * not only the ones that happen to need a confirm token. An agent trained to | |
| 479 | - * preview first — and the prompts shipped with this plugin train exactly | |
| 480 | - * that — would otherwise find that its cautious path was the destructive | |
| 481 | - * one. Additive and reversible actions still answer a dry run; they just | |
| 482 | - * hand back no token, because none is needed to proceed. | |
| 467 | + * dry_run must change nothing for every action, not just the token-gated | |
| 468 | + * ones, or an agent's cautious preview would be the write. Additive and | |
| 469 | + * reversible actions answer a dry run without a token. | |
| 483 | 470 | * |
| 484 | 471 | * @param string $action |
| 485 | 472 | * @param array $preview |
| 486 | 473 | * @return array |
| @@ -611,14 +598,11 @@ | ||
| 611 | 598 | }); |
| 612 | 599 | } |
| 613 | 600 | |
| 614 | 601 | /** |
| 615 | - * Give the scheduling writes the same retry safety the booking writes have. | |
| 602 | + * Idempotency for the scheduling writes, so a retried create or clone | |
| 603 | + * doesn't leave two live bookable records. | |
| 616 | 604 | * |
| 617 | - * Without it a retried create or clone leaves two live bookable records | |
| 618 | - * and nothing detects it — and because wrapExecuteCallback() rejects | |
| 619 | - * undeclared parameters, an agent could not even opt in. | |
| 620 | - * | |
| 621 | 605 | * @param string $tool |
| 622 | 606 | * @param array $params |
| 623 | 607 | * @param callable $fn |
| 624 | 608 | * @param callable $entityKey |
| @@ -628,10 +612,9 @@ | ||
| 628 | 612 | private static function deduped($tool, $params, callable $fn, callable $entityKey) |
| 629 | 613 | { |
| 630 | 614 | $key = (string) Arr::get($params, 'idempotency_key', ''); |
| 631 | 615 | |
| 632 | - // A dry run changes nothing, so there is nothing to deduplicate — and | |
| 633 | - // recording one would replay a preview in place of the real write. | |
| 616 | + // Recording a dry run would replay the preview in place of the real write. | |
| 634 | 617 | if (!$key || Arr::isTrue($params, 'dry_run')) { |
| 635 | 618 | return $fn(); |
| 636 | 619 | } |
| 637 | 620 | |
| @@ -698,14 +681,10 @@ | ||
| 698 | 681 | if (!$event) { |
| 699 | 682 | return MCPHelper::error('not_found', __('No event type with that id.', 'fluent-booking')); |
| 700 | 683 | } |
| 701 | 684 | |
| 702 | - // Gate here, not at the bridge. RestBridge's policy does refuse the | |
| 703 | - // write, but a dry_run returns its preview before ever reaching the | |
| 704 | - // bridge — so without this check the preview would hand an event | |
| 705 | - // type's title, status and upcoming booking count to a caller with no | |
| 706 | - // write access to it, along with a confirm_token implying they may | |
| 707 | - // proceed. | |
| 685 | + // Gate here, not only at the bridge: a dry_run returns its preview | |
| 686 | + // (and a confirm_token) before it ever reaches RestBridge's policy. | |
| 708 | 687 | if (!PermissionManager::canWriteCalendar($event->calendar_id)) { |
| 709 | 688 | return MCPHelper::error( |
| 710 | 689 | 'permission_denied', |
| 711 | 690 | __('You do not have permission to change this event type.', 'fluent-booking'), |
| @@ -725,11 +704,9 @@ | ||
| 725 | 704 | 'status' => ['from' => $event->status, 'to' => $status], |
| 726 | 705 | ]; |
| 727 | 706 | |
| 728 | 707 | if ($action === 'deactivate') { |
| 729 | - // Deactivating is reversible as a database change, but it | |
| 730 | - // takes a live booking page offline, so the count of what is | |
| 731 | - // about to stop being bookable belongs in the preview. | |
| 708 | + // Reversible, but it takes a live booking page offline. | |
| 732 | 709 | $preview['effect'] = __('The public booking page stops offering slots immediately. Existing bookings are untouched.', 'fluent-booking'); |
| 733 | 710 | } |
| 734 | 711 | |
| 735 | 712 | return self::reversiblePreview($action, $preview); |
| @@ -740,10 +717,9 @@ | ||
| 740 | 717 | |
| 741 | 718 | if ($action === 'duplicate') { |
| 742 | 719 | $targetCalendar = absint(Arr::get($params, 'calendar_id')) ?: $calendarId; |
| 743 | 720 | |
| 744 | - // The duplicate lands on whatever calendar_id was passed, which is | |
| 745 | - // not necessarily the one the permission check above covered. | |
| 721 | + // The target calendar may differ from the one checked above. | |
| 746 | 722 | if ($targetCalendar !== $calendarId && !PermissionManager::canWriteCalendar($targetCalendar)) { |
| 747 | 723 | return MCPHelper::error( |
| 748 | 724 | 'permission_denied', |
| 749 | 725 | __('You do not have permission to add event types to the destination calendar.', 'fluent-booking'), |
| @@ -794,11 +770,11 @@ | ||
| 794 | 770 | if (is_wp_error($payload)) { |
| 795 | 771 | return $payload; |
| 796 | 772 | } |
| 797 | 773 | |
| 798 | - // These two rebuild a whole settings block, and an availability | |
| 799 | - // write replaces the weekly grid outright — the shape | |
| 800 | - // manage-availability update is already gated for. | |
| 774 | + // These two rebuild a whole settings block (availability replaces | |
| 775 | + // the weekly grid), so they need a confirm token like | |
| 776 | + // manage-availability update. | |
| 801 | 777 | if (!in_array($section, ['availability', 'limits'], true)) { |
| 802 | 778 | if ($dryRun) { |
| 803 | 779 | return self::reversiblePreview('update', [ |
| 804 | 780 | 'event' => self::eventSummary($event), |
| @@ -838,20 +814,13 @@ | ||
| 838 | 814 | return MCPHelper::error('unsupported_action', __('Unknown action.', 'fluent-booking')); |
| 839 | 815 | } |
| 840 | 816 | |
| 841 | 817 | /** |
| 842 | - * Translate a section write from the vocabulary get-event-types PROJECTS | |
| 843 | - * into the shape the admin controller reads, over the event's current | |
| 844 | - * values. | |
| 818 | + * Map a section write from get-event-types' names (`buffer_before_minutes`) | |
| 819 | + * to the admin controller's shape (`settings.buffer_time_before`), seeded | |
| 820 | + * from stored values. The controllers rebuild their whole key set on every | |
| 821 | + * POST, so seeding is what keeps a partial write partial. | |
| 845 | 822 | * |
| 846 | - * The tool tells the agent to read a section and send it back changed, but | |
| 847 | - * the projection uses friendly names (`buffer_before_minutes`) and the | |
| 848 | - * controllers read the admin SPA's shape (`settings.buffer_time_before`). | |
| 849 | - * Forwarded verbatim, the controller found none of its keys and wrote its | |
| 850 | - * own defaults over all of them — dropping the requested change and | |
| 851 | - * resetting the rest. Seeding from stored values also keeps a partial write | |
| 852 | - * partial, since the controllers rebuild their whole key set on every POST. | |
| 853 | - * | |
| 854 | 823 | * @param CalendarSlot $event |
| 855 | 824 | * @param string $section |
| 856 | 825 | * @param array $fields |
| 857 | 826 | * @return array|\WP_Error |
| @@ -896,16 +865,11 @@ | ||
| 896 | 865 | } |
| 897 | 866 | |
| 898 | 867 | /** |
| 899 | 868 | * Every key updateEventDetails() rebuilds, seeded from what is stored. |
| 869 | + * The controller defaults anything missing, including location_settings, | |
| 870 | + * which would leave the event unbookable. | |
| 900 | 871 | * |
| 901 | - * It validates only title, duration and status and absorbs the rest with | |
| 902 | - * defaults, so a write sending just those three set max_book_per_slot to 0, | |
| 903 | - * reset color_schema, emptied description and — worst — wiped | |
| 904 | - * location_settings, leaving an event that cannot be booked at all. | |
| 905 | - * Seeding also makes a partial write possible: the three "required" fields | |
| 906 | - * come from storage when the caller does not send them. | |
| 907 | - * | |
| 908 | 872 | * @return array|\WP_Error |
| 909 | 873 | */ |
| 910 | 874 | private static function detailsPayload(CalendarSlot $event, $fields) |
| 911 | 875 | { |
| @@ -936,11 +900,10 @@ | ||
| 936 | 900 | 'available_durations' => [], |
| 937 | 901 | ]), |
| 938 | 902 | ]; |
| 939 | 903 | |
| 940 | - // Only when the caller SENDS it: omitting it keeps what is stored, but | |
| 941 | - // sending an empty list would clear the last location, and create | |
| 942 | - // refuses that same state. | |
| 904 | + // Only when sent: omitting it keeps what is stored, but an empty list | |
| 905 | + // would clear the last location. | |
| 943 | 906 | if (array_key_exists('location_settings', $fields) |
| 944 | 907 | && is_wp_error($locationError = self::validateLocations($fields['location_settings']))) { |
| 945 | 908 | return $locationError; |
| 946 | 909 | } |
| @@ -1019,13 +982,11 @@ | ||
| 1019 | 982 | } |
| 1020 | 983 | |
| 1021 | 984 | /** |
| 1022 | 985 | * Rebuild a cap block from the by-unit map the projection returns. |
| 986 | + * capLimits() turns the stored {unit, value} list into a map; the | |
| 987 | + * controller expects the list. | |
| 1023 | 988 | * |
| 1024 | - * capLimits() re-keys the stored {unit, value} LIST into a map so an agent | |
| 1025 | - * can look up `per_day`; the controller reads the list back. Without the | |
| 1026 | - * inverse, sending a read cap block back writes an empty cap. | |
| 1027 | - * | |
| 1028 | 989 | * @param mixed $cap |
| 1029 | 990 | * @return array |
| 1030 | 991 | */ |
| 1031 | 992 | private static function capPayload($cap) |
| @@ -1070,11 +1031,10 @@ | ||
| 1070 | 1031 | |
| 1071 | 1032 | $settings = (array) $event->settings; |
| 1072 | 1033 | $timezone = $event->calendar ? $event->calendar->author_timezone : 'UTC'; |
| 1073 | 1034 | |
| 1074 | - // The controller converts the grid it receives from the author timezone | |
| 1075 | - // into UTC, and what is stored is already UTC. Send it back through the | |
| 1076 | - // inverse first, or every slot slides by the offset. | |
| 1035 | + // Stored hours are UTC, but the controller converts what it receives | |
| 1036 | + // from the author timezone. Convert back first or every slot shifts. | |
| 1077 | 1037 | $weekly = SanitizeService::weeklySchedules( |
| 1078 | 1038 | (array) Arr::get($settings, 'weekly_schedules', []), |
| 1079 | 1039 | 'UTC', |
| 1080 | 1040 | $timezone, |
| @@ -1099,14 +1059,11 @@ | ||
| 1099 | 1059 | |
| 1100 | 1060 | if (array_key_exists('schedule_id', $fields)) { |
| 1101 | 1061 | $scheduleId = absint($fields['schedule_id']); |
| 1102 | 1062 | |
| 1103 | - // Bind only a schedule the caller may read. The controller assigns | |
| 1104 | - // availability_id with no ownership test and Availability has no | |
| 1105 | - // owner scope, so an unchecked id here binds another host's | |
| 1106 | - // schedule to this event — after which diagnose-availability and | |
| 1107 | - // get-available-slots read its weekly grid and timezone straight | |
| 1108 | - // back out, through a tool that would have refused the id. | |
| 1063 | + // The controller does no ownership check on availability_id, so | |
| 1064 | + // an unchecked id would bind another host's schedule and expose its | |
| 1065 | + // hours through the slot tools. | |
| 1109 | 1066 | if ($scheduleId) { |
| 1110 | 1067 | $schedule = Availability::find($scheduleId); |
| 1111 | 1068 | |
| 1112 | 1069 | if (!$schedule) { |
| @@ -1153,12 +1110,11 @@ | ||
| 1153 | 1110 | } |
| 1154 | 1111 | |
| 1155 | 1112 | $stored = (array) $event->getBookingFields(); |
| 1156 | 1113 | |
| 1157 | - // Match on NAME and edit in place. saveEventBookingFields() replaces the | |
| 1158 | - // whole set and mints a name for any entry lacking one, so returning the | |
| 1159 | - // projection — which renames `name` to `key` — appended a second copy of | |
| 1160 | - // every question instead of updating the originals. | |
| 1114 | + // Match on name and edit in place. saveEventBookingFields() replaces | |
| 1115 | + // the whole set and names any unnamed entry, and the projection exposes | |
| 1116 | + // `name` as `key`, so unmatched fields would be duplicated. | |
| 1161 | 1117 | $byName = []; |
| 1162 | 1118 | |
| 1163 | 1119 | foreach ($stored as $key => $field) { |
| 1164 | 1120 | if (is_array($field)) { |
| @@ -1221,16 +1177,11 @@ | ||
| 1221 | 1177 | ]; |
| 1222 | 1178 | } |
| 1223 | 1179 | |
| 1224 | 1180 | /** |
| 1225 | - * An event type is not three fields. The controller reads a full settings | |
| 1226 | - * block — schedule type, weekly hours, range, buffers — straight out of the | |
| 1227 | - * payload, and an agent that sent only a title would create a broken event | |
| 1228 | - * or trip an undefined-index. So start from exactly what the admin's own | |
| 1229 | - * "new event type" screen starts from, `CalendarSlot::getEventSchema()`, | |
| 1230 | - * and lay the agent's fields over it. An agent can then create a working | |
| 1231 | - * event type with a title, a duration and a location, which is what it | |
| 1232 | - * would expect to need. | |
| 1181 | + * The controller reads a full settings block from the payload, so start | |
| 1182 | + * from the admin's "new event type" defaults (getEventSchema()) and lay the | |
| 1183 | + * agent's fields over them. A title, duration and location are then enough. | |
| 1233 | 1184 | * |
| 1234 | 1185 | * @return array|\WP_Error |
| 1235 | 1186 | */ |
| 1236 | 1187 | private static function createPayload($calendarId, $fields) |
| @@ -1242,10 +1193,9 @@ | ||
| 1242 | 1193 | } |
| 1243 | 1194 | |
| 1244 | 1195 | $schema = (new CalendarSlot())->getEventSchema($calendar); |
| 1245 | 1196 | |
| 1246 | - // The schema embeds the calendar for the UI to render; it is not a | |
| 1247 | - // field on the event type. | |
| 1197 | + // Embedded for the UI, not an event type field. | |
| 1248 | 1198 | unset($schema['calendar']); |
| 1249 | 1199 | |
| 1250 | 1200 | if (is_wp_error($locationError = self::validateLocations(Arr::get($fields, 'location_settings', [])))) { |
| 1251 | 1201 | return $locationError; |
| @@ -1250,12 +1200,10 @@ | ||
| 1250 | 1200 | if (is_wp_error($locationError = self::validateLocations(Arr::get($fields, 'location_settings', [])))) { |
| 1251 | 1201 | return $locationError; |
| 1252 | 1202 | } |
| 1253 | 1203 | |
| 1254 | - // The admin controller refuses these too, but in its own vocabulary — | |
| 1255 | - // "Event type field is required" names neither the tool's parameter nor | |
| 1256 | - // what a valid value looks like. Refuse here, in the terms the schema | |
| 1257 | - // uses, before bridging. | |
| 1204 | + // The controller refuses these too, but its messages don't name the | |
| 1205 | + // tool's parameters or valid values. | |
| 1258 | 1206 | $required = [ |
| 1259 | 1207 | 'title' => __('a name for the event type', 'fluent-booking'), |
| 1260 | 1208 | 'duration' => __('its length in minutes, e.g. 30', 'fluent-booking'), |
| 1261 | 1209 | 'event_type' => sprintf( |
| @@ -1277,10 +1225,10 @@ | ||
| 1277 | 1225 | } |
| 1278 | 1226 | |
| 1279 | 1227 | $payload = array_merge($schema, $fields); |
| 1280 | 1228 | |
| 1281 | - // Merge one level into settings rather than replacing it, so an agent | |
| 1282 | - // changing a buffer does not wipe the weekly hours it never saw. | |
| 1229 | + // Merge settings one level deep so a partial settings block keeps the | |
| 1230 | + // default weekly hours. | |
| 1283 | 1231 | $payload['settings'] = array_merge( |
| 1284 | 1232 | (array) Arr::get($schema, 'settings', []), |
| 1285 | 1233 | (array) Arr::get($fields, 'settings', []) |
| 1286 | 1234 | ); |
| @@ -1288,22 +1236,18 @@ | ||
| 1288 | 1236 | return $payload; |
| 1289 | 1237 | } |
| 1290 | 1238 | |
| 1291 | 1239 | /** |
| 1292 | - * Deleting an event type takes its bookings with it. The admin has no guard | |
| 1293 | - * against that — a human doing it has the schedule on screen and knows what | |
| 1294 | - * they are throwing away. An agent does not, so it has to be told, and has | |
| 1295 | - * to say `force` to proceed anyway. | |
| 1240 | + * Deleting an event type deletes its bookings. The admin doesn't guard | |
| 1241 | + * this, so an agent must pass `force` when any exist. | |
| 1296 | 1242 | * |
| 1297 | 1243 | * @return array|\WP_Error |
| 1298 | 1244 | */ |
| 1299 | 1245 | private static function deleteEventType(CalendarSlot $event, $params) |
| 1300 | 1246 | { |
| 1301 | - // CalenderEventCleaner deletes EVERY booking on the event — no status | |
| 1302 | - // filter, no date filter — along with their activities and, in pro, | |
| 1303 | - // their orders and transactions. Counting only the upcoming ones let an | |
| 1304 | - // event with years of completed bookings delete without force, under a | |
| 1305 | - // preview that said nothing would be affected. | |
| 1247 | + // CalenderEventCleaner deletes every booking on the event, any status | |
| 1248 | + // or date, with their activities and (in pro) orders and transactions. | |
| 1249 | + // So count them all, not just upcoming ones. | |
| 1306 | 1250 | $byStatus = Booking::where('event_id', $event->id) |
| 1307 | 1251 | ->groupBy('status') |
| 1308 | 1252 | ->selectRaw('status, COUNT(*) AS total') |
| 1309 | 1253 | ->pluck('total', 'status') |
| @@ -1402,12 +1346,8 @@ | ||
| 1402 | 1346 | |
| 1403 | 1347 | /** |
| 1404 | 1348 | * Give every reference list the same shape, and say when it was cut short. |
| 1405 | 1349 | * |
| 1406 | - * The cap used to be applied bare: a 150-calendar site got 100 rows with | |
| 1407 | - * nothing marking them as a page, so an agent concluded the other 50 did | |
| 1408 | - * not exist. | |
| 1409 | - * | |
| 1410 | 1350 | * @param array $result [$rows, $total] |
| 1411 | 1351 | * |
| 1412 | 1352 | * @return array |
| 1413 | 1353 | */ |
| @@ -1438,13 +1378,10 @@ | ||
| 1438 | 1378 | */ |
| 1439 | 1379 | private static function referenceKind($kind, $params) |
| 1440 | 1380 | { |
| 1441 | 1381 | if ($kind === 'calendars') { |
| 1442 | - // PermissionGate::scopeToReadableCalendars() rather than a local | |
| 1443 | - // user_id filter: the event-type tools gate on canReadCalendar(), | |
| 1444 | - // which also admits shared team calendars, and a reference list | |
| 1445 | - // that omits a calendar those tools will happily read leaves an | |
| 1446 | - // agent unable to name an id it is allowed to use. | |
| 1382 | + // Same scope as canReadCalendar(), which admits shared team | |
| 1383 | + // calendars; a user_id filter would hide ids the agent may use. | |
| 1447 | 1384 | $query = PermissionGate::scopeToReadableCalendars(Calendar::orderBy('id', 'asc'), 'id'); |
| 1448 | 1385 | |
| 1449 | 1386 | $total = (clone $query)->count(); |
| 1450 | 1387 | $rows = []; |
| @@ -1463,24 +1400,18 @@ | ||
| 1463 | 1400 | return [$rows, $total]; |
| 1464 | 1401 | } |
| 1465 | 1402 | |
| 1466 | 1403 | if ($kind === 'hosts') { |
| 1467 | - // Hosts are whoever owns a calendar. Enumerating WP users instead | |
| 1468 | - // would leak every account on the site into an agent's context. | |
| 1404 | + // Hosts are calendar owners. Listing WP users would leak every account. | |
| 1469 | 1405 | $query = PermissionGate::scopeToReadableCalendars(Calendar::query(), 'id'); |
| 1470 | 1406 | |
| 1471 | - // A calendar can outlive the user who owned it, and a deleted owner | |
| 1472 | - // is not a host. Excluded in SQL so the total stays honest without | |
| 1473 | - // reading every calendar into PHP to find out. | |
| 1407 | + // Skip calendars whose owner was deleted, in SQL so the total stays right. | |
| 1474 | 1408 | $query->whereIn('user_id', User::select('ID')); |
| 1475 | 1409 | |
| 1476 | 1410 | $total = (clone $query)->distinct()->count('user_id'); |
| 1477 | 1411 | |
| 1478 | - // Distinct owners, capped in SQL and ordered by the first calendar | |
| 1479 | - // each appears on: deriving hosts from a capped page of calendars | |
| 1480 | - // could drop one entirely and still read as a complete list, but | |
| 1481 | - // reading every calendar to find LIST_LIMIT hosts scales with the | |
| 1482 | - // table rather than the answer. | |
| 1412 | + // Distinct owners, capped in SQL. Deriving hosts from a capped page | |
| 1413 | + // of calendars could silently drop one. | |
| 1483 | 1414 | $userIds = array_map('intval', (clone $query) |
| 1484 | 1415 | ->groupBy('user_id') |
| 1485 | 1416 | ->orderByRaw('min(id) asc') |
| 1486 | 1417 | ->limit(self::LIST_LIMIT) |
| @@ -1536,11 +1467,9 @@ | ||
| 1536 | 1467 | |
| 1537 | 1468 | if ($kind === 'location_providers') { |
| 1538 | 1469 | $rows = []; |
| 1539 | 1470 | |
| 1540 | - // The registry is grouped for the admin's location picker; flatten | |
| 1541 | - // it, and keep `disabled` so an agent can see which providers are | |
| 1542 | - // Pro-only rather than trying one and getting a validation error. | |
| 1471 | + // Flatten the picker's groups. `disabled` marks Pro-only providers. | |
| 1543 | 1472 | foreach ((new CalendarSlot())->getLocationFields() as $group) { |
| 1544 | 1473 | foreach ((array) Arr::get($group, 'options', []) as $type => $option) { |
| 1545 | 1474 | $rows[] = [ |
| 1546 | 1475 | 'type' => $type, |
| @@ -1572,11 +1501,10 @@ | ||
| 1572 | 1501 | } |
| 1573 | 1502 | |
| 1574 | 1503 | $rows = []; |
| 1575 | 1504 | |
| 1576 | - // getBookingFields(), not getMeta('booking_fields'): the accessor | |
| 1577 | - // merges the built-in fields over the stored ones, and get-event-types | |
| 1578 | - // already reads it. Two sources for one answer drift. | |
| 1505 | + // getBookingFields() merges built-in fields over stored ones, and is | |
| 1506 | + // what get-event-types reads too. | |
| 1579 | 1507 | foreach ((array) $event->getBookingFields() as $field) { |
| 1580 | 1508 | if (!is_array($field)) { |
| 1581 | 1509 | continue; |
| 1582 | 1510 | } |
| @@ -1593,10 +1521,9 @@ | ||
| 1593 | 1521 | return [$rows, count($rows)]; |
| 1594 | 1522 | } |
| 1595 | 1523 | |
| 1596 | 1524 | /** |
| 1597 | - * Usage counts for a page of schedules in one grouped query rather than one | |
| 1598 | - * count per row: the list returns up to LIST_LIMIT of them. | |
| 1525 | + * Usage counts for a page of schedules in one grouped query. | |
| 1599 | 1526 | * |
| 1600 | 1527 | * @param array $scheduleIds |
| 1601 | 1528 | * |
| 1602 | 1529 | * @return array schedule id => count |
| @@ -1628,13 +1555,10 @@ | ||
| 1628 | 1555 | private static function projectSchedule(Availability $schedule, $timezone, $full, $usageCount = null) |
| 1629 | 1556 | { |
| 1630 | 1557 | $own = Arr::get($schedule, 'value.timezone', 'UTC'); |
| 1631 | 1558 | |
| 1632 | - // Only honour a requested zone we can actually render in. Reporting a | |
| 1633 | - // timezone the hours are NOT expressed in is worse than ignoring the | |
| 1634 | - // parameter: the list used to stamp every schedule with whatever the | |
| 1635 | - // caller asked for while returning each one's own stored hours, so an | |
| 1636 | - // agent asking for Asia/Tokyo was told the whole site ran on Tokyo time. | |
| 1559 | + // Only honour a valid zone, and never label hours with a zone they | |
| 1560 | + // aren't expressed in. | |
| 1637 | 1561 | $requested = $timezone && in_array($timezone, timezone_identifiers_list(), true) ? $timezone : ''; |
| 1638 | 1562 | |
| 1639 | 1563 | $row = [ |
| 1640 | 1564 | 'id' => (int) $schedule->id, |
| @@ -1640,10 +1564,10 @@ | ||
| 1640 | 1564 | 'id' => (int) $schedule->id, |
| 1641 | 1565 | 'title' => $schedule->key, |
| 1642 | 1566 | 'host_id' => (int) $schedule->object_id, |
| 1643 | 1567 | 'default' => Arr::isTrue($schedule, 'value.default'), |
| 1644 | - // The zone the hours below are in. On the summary row there are no | |
| 1645 | - // hours, so this is always the schedule's own. | |
| 1568 | + // The zone the hours are in. Summary rows have no hours, so it is | |
| 1569 | + // always the schedule's own there. | |
| 1646 | 1570 | 'timezone' => $full && $requested ? $requested : $own, |
| 1647 | 1571 | // The list hands its count in; a single schedule looks its own up. |
| 1648 | 1572 | 'usage_count' => $usageCount === null |
| 1649 | 1573 | ? AvailabilityService::getAvailabilityUsageCount($schedule->id) |
| @@ -1653,12 +1577,10 @@ | ||
| 1653 | 1577 | if (!$full) { |
| 1654 | 1578 | return $row; |
| 1655 | 1579 | } |
| 1656 | 1580 | |
| 1657 | - // Hours are stored in UTC; getFormattedSchedule() renders them in the | |
| 1658 | - // schedule's own zone. Rendering them in the caller's is the same | |
| 1659 | - // conversion with a different target, so do it properly rather than | |
| 1660 | - // return a mislabelled grid. | |
| 1581 | + // Hours are stored in UTC. getFormattedSchedule() only renders the | |
| 1582 | + // schedule's own zone, so convert for any other. | |
| 1661 | 1583 | if ($requested && $requested !== $own) { |
| 1662 | 1584 | $row['weekly_schedules'] = SanitizeService::weeklySchedules(Arr::get($schedule, 'value.weekly_schedules', []), 'UTC', $requested); |
| 1663 | 1585 | $row['date_overrides'] = SanitizeService::slotDateOverrides(Arr::get($schedule, 'value.date_overrides', []), 'UTC', $requested); |
| 1664 | 1586 | $row['stored_timezone'] = $own; |
| @@ -1673,10 +1595,9 @@ | ||
| 1673 | 1595 | } |
| 1674 | 1596 | |
| 1675 | 1597 | $formatted = AvailabilityService::getFormattedSchedule($schedule); |
| 1676 | 1598 | |
| 1677 | - // The admin shape carries a gravatar URL — a hundred bytes of nothing, | |
| 1678 | - // on every row, forever. | |
| 1599 | + // Pick the grids out; the admin shape also carries a gravatar URL. | |
| 1679 | 1600 | $row['weekly_schedules'] = Arr::get($formatted, 'settings.weekly_schedules', []); |
| 1680 | 1601 | $row['date_overrides'] = Arr::get($formatted, 'settings.date_overrides', []); |
| 1681 | 1602 | |
| 1682 | 1603 | return $row; |
| @@ -1682,11 +1603,10 @@ | ||
| 1682 | 1603 | return $row; |
| 1683 | 1604 | } |
| 1684 | 1605 | |
| 1685 | 1606 | /** |
| 1686 | - * Reading someone else's schedule is a lesser privilege than editing it: | |
| 1687 | - * `read_and_use_other_availabilities` exists precisely so a host can point | |
| 1688 | - * an event type at a colleague's hours without being able to change them. | |
| 1607 | + * Stricter than canReadSchedule(): `read_and_use_other_availabilities` | |
| 1608 | + * lets a host use a colleague's hours but not edit them. | |
| 1689 | 1609 | * |
| 1690 | 1610 | * @return bool |
| 1691 | 1611 | */ |
| 1692 | 1612 | private static function canWriteSchedule(Availability $schedule) |
| @@ -1731,10 +1651,9 @@ | ||
| 1731 | 1651 | if (!$record) { |
| 1732 | 1652 | continue; |
| 1733 | 1653 | } |
| 1734 | 1654 | |
| 1735 | - // Models come back as objects, not arrays; casting one to an array | |
| 1736 | - // does not surface its attributes, so read the property first. | |
| 1655 | + // Models come back as objects, and an array cast hides their attributes. | |
| 1737 | 1656 | $out['id'] = is_object($record) ? (int) $record->id : (int) Arr::get((array) $record, 'id', 0); |
| 1738 | 1657 | break; |
| 1739 | 1658 | } |
| 1740 | 1659 | |
