| @@ -80,16 +80,36 @@ | ||
| 80 | 80 | } |
| 81 | 81 | break; |
| 82 | 82 | |
| 83 | 83 | case 'monthly': |
| 84 | - if (empty($data['week_of_month'])) { | |
| 84 | + // week_of_month may come from the admin UI as a string (first/second/...) | |
| 85 | + // or from persisted data as an int (1..5). Accept both and normalize for checks. | |
| 86 | + $weekValue = $data['week_of_month'] ?? null; | |
| 87 | + if ($weekValue === null || $weekValue === '') { | |
| 85 | 88 | throw new \InvalidArgumentException('Week of month is required for monthly rules'); |
| 86 | 89 | } |
| 90 | + if (is_string($weekValue)) { | |
| 91 | + $weekValue = strtolower(trim($weekValue)); | |
| 92 | + } | |
| 93 | + if (is_numeric($weekValue)) { | |
| 94 | + $weekInt = (int) $weekValue; | |
| 95 | + $map = [ | |
| 96 | + 1 => 'first', | |
| 97 | + 2 => 'second', | |
| 98 | + 3 => 'third', | |
| 99 | + 4 => 'fourth', | |
| 100 | + 5 => 'last', | |
| 101 | + ]; | |
| 102 | + if (isset($map[$weekInt])) { | |
| 103 | + $weekValue = $map[$weekInt]; | |
| 104 | + $data['week_of_month'] = $weekValue; | |
| 105 | + } | |
| 106 | + } | |
| 87 | 107 | if (!isset($data['day_of_week']) || $data['day_of_week'] === '') { |
| 88 | 108 | throw new \InvalidArgumentException('Day of week is required for monthly rules'); |
| 89 | 109 | } |
| 90 | 110 | $validWeeks = ['first', 'second', 'third', 'fourth', 'last']; |
| 91 | - if (!in_array($data['week_of_month'], $validWeeks, true)) { | |
| 111 | + if (!is_string($weekValue) || !in_array($weekValue, $validWeeks, true)) { | |
| 92 | 112 | throw new \InvalidArgumentException('Invalid week of month'); |
| 93 | 113 | } |
| 94 | 114 | break; |
| 95 | 115 | |
| @@ -129,14 +149,20 @@ | ||
| 129 | 149 | */ |
| 130 | 150 | public function create(array $data): int |
| 131 | 151 | { |
| 132 | 152 | $this->validate($data); |
| 133 | - | |
| 134 | - // Convert days array to string if needed | |
| 135 | - if (isset($data['days_of_week']) && is_array($data['days_of_week'])) { | |
| 136 | - $data['days_of_week'] = implode(',', $data['days_of_week']); | |
| 153 | + | |
| 154 | + // The `days_of_week` column is JSON. Leave the value as an array so | |
| 155 | + // the repository can JSON-encode it; if a caller passes a legacy CSV | |
| 156 | + // string, normalise it to an array of ints here so the repo always | |
| 157 | + // sees a single shape. | |
| 158 | + $data = $this->normaliseDaysOfWeek($data); | |
| 159 | + | |
| 160 | + // Normalise UI strings to match DB column types. | |
| 161 | + if (isset($data['week_of_month']) && is_string($data['week_of_month'])) { | |
| 162 | + $data['week_of_month'] = strtolower(trim($data['week_of_month'])); | |
| 137 | 163 | } |
| 138 | - | |
| 164 | + | |
| 139 | 165 | return $this->repository->create($data); |
| 140 | 166 | } |
| 141 | 167 | |
| 142 | 168 | /** |
| @@ -144,18 +170,55 @@ | ||
| 144 | 170 | */ |
| 145 | 171 | public function update(int $id, array $data): bool |
| 146 | 172 | { |
| 147 | 173 | $this->validate($data, $id); |
| 148 | - | |
| 149 | - // Convert days array to string if needed | |
| 150 | - if (isset($data['days_of_week']) && is_array($data['days_of_week'])) { | |
| 151 | - $data['days_of_week'] = implode(',', $data['days_of_week']); | |
| 174 | + | |
| 175 | + $data = $this->normaliseDaysOfWeek($data); | |
| 176 | + | |
| 177 | + if (isset($data['week_of_month']) && is_string($data['week_of_month'])) { | |
| 178 | + $data['week_of_month'] = strtolower(trim($data['week_of_month'])); | |
| 152 | 179 | } |
| 153 | - | |
| 180 | + | |
| 154 | 181 | return $this->repository->update($id, $data); |
| 155 | 182 | } |
| 156 | 183 | |
| 157 | 184 | /** |
| 185 | + * Coerce `days_of_week` to an array of ints (0..6) regardless of how the | |
| 186 | + * caller passed it (array of mixed scalars, comma-separated string, JSON | |
| 187 | + * string). The repository is responsible for JSON-encoding it for the | |
| 188 | + * database column. | |
| 189 | + */ | |
| 190 | + private function normaliseDaysOfWeek(array $data): array | |
| 191 | + { | |
| 192 | + if (!array_key_exists('days_of_week', $data)) { | |
| 193 | + return $data; | |
| 194 | + } | |
| 195 | + | |
| 196 | + $value = $data['days_of_week']; | |
| 197 | + | |
| 198 | + if (is_string($value)) { | |
| 199 | + $trimmed = trim($value); | |
| 200 | + if ($trimmed === '') { | |
| 201 | + $value = []; | |
| 202 | + } else { | |
| 203 | + $decoded = json_decode($trimmed, true); | |
| 204 | + $value = is_array($decoded) ? $decoded : explode(',', $trimmed); | |
| 205 | + } | |
| 206 | + } | |
| 207 | + | |
| 208 | + if (!is_array($value)) { | |
| 209 | + $value = []; | |
| 210 | + } | |
| 211 | + | |
| 212 | + $value = array_values(array_unique(array_map(static fn($d) => (int) $d, $value))); | |
| 213 | + $value = array_values(array_filter($value, static fn(int $d) => $d >= 0 && $d <= 6)); | |
| 214 | + | |
| 215 | + $data['days_of_week'] = $value; | |
| 216 | + | |
| 217 | + return $data; | |
| 218 | + } | |
| 219 | + | |
| 220 | + /** | |
| 158 | 221 | * Delete a rule |
| 159 | 222 | */ |
| 160 | 223 | public function delete(int $id): bool |
| 161 | 224 | { |
| @@ -287,9 +350,9 @@ | ||
| 287 | 350 | } |
| 288 | 351 | |
| 289 | 352 | if (in_array($dayOfWeek, $targetDays, true)) { |
| 290 | 353 | // Check if not excluded |
| 291 | - if (!in_array($dateStr, $excludedDates, true)) { | |
| 354 | + if (!$this->isDateExcluded($dateStr, $excludedDates)) { | |
| 292 | 355 | // Check cutoff |
| 293 | 356 | if ($this->isBookable($current, $rule)) { |
| 294 | 357 | $generatedDates = $this->createAvailabilityFromRule($rule, $dateStr, $dayOfWeek); |
| 295 | 358 | $dates = array_merge($dates, $generatedDates); |
| @@ -308,11 +371,21 @@ | ||
| 308 | 371 | */ |
| 309 | 372 | private function generateMonthlyDates(object $rule, string $fromDate, string $toDate): array |
| 310 | 373 | { |
| 311 | 374 | $dates = []; |
| 312 | - $weekOfMonth = $rule->week_of_month; | |
| 313 | - $dayOfWeek = (int) $rule->day_of_week; | |
| 314 | - $excludedDates = $rule->excluded_dates; | |
| 375 | + | |
| 376 | + // Legacy rows (migrated from the pre-3.x schema) can land here | |
| 377 | + // with `week_of_month` NULL or missing entirely — the column is | |
| 378 | + // nullable in the DB but {@see self::getNthWeekdayOfMonth()}'s | |
| 379 | + // signature requires `string`, so calling it with NULL was | |
| 380 | + // crashing the trip page on PHP 7.4+. Bail out cleanly instead. | |
| 381 | + $weekOfMonth = $rule->week_of_month ?? null; | |
| 382 | + if (!is_string($weekOfMonth) || $weekOfMonth === '') { | |
| 383 | + return $dates; | |
| 384 | + } | |
| 385 | + | |
| 386 | + $dayOfWeek = (int) ($rule->day_of_week ?? 0); | |
| 387 | + $excludedDates = $rule->excluded_dates ?? []; | |
| 315 | 388 | $selectedMonths = !empty($rule->months) ? $rule->months : []; |
| 316 | 389 | |
| 317 | 390 | // Start from the first day of the starting month |
| 318 | 391 | $current = strtotime(date('Y-m-01', strtotime($fromDate))); |
| @@ -335,9 +408,9 @@ | ||
| 335 | 408 | |
| 336 | 409 | // Check if within range |
| 337 | 410 | if ($targetTimestamp >= strtotime($fromDate) && $targetTimestamp <= $end) { |
| 338 | 411 | // Check if not excluded |
| 339 | - if (!in_array($targetDate, $excludedDates, true)) { | |
| 412 | + if (!$this->isDateExcluded($targetDate, $excludedDates)) { | |
| 340 | 413 | // Check cutoff |
| 341 | 414 | if ($this->isBookable($targetTimestamp, $rule)) { |
| 342 | 415 | $generatedDates = $this->createAvailabilityFromRule($rule, $targetDate, $dayOfWeek); |
| 343 | 416 | $dates = array_merge($dates, $generatedDates); |
| @@ -353,43 +426,72 @@ | ||
| 353 | 426 | return $dates; |
| 354 | 427 | } |
| 355 | 428 | |
| 356 | 429 | /** |
| 357 | - * Generate interval recurring dates (every X days) | |
| 430 | + * Generate interval recurring dates (every X days). | |
| 431 | + * | |
| 432 | + * Hardened against malformed legacy data (rules carried over from the | |
| 433 | + * pre-3.x schema may land here after the legacy→new heal-step in | |
| 434 | + * {@see \Yatra\Services\InstallerService::maybeNormalizeAvailabilityRulesLegacyData()}): | |
| 435 | + * - `interval_days` defaults to 1 when 0/NULL so we never divide by zero | |
| 436 | + * or loop forever. | |
| 437 | + * - `interval_start_date`/`start_date` may be NULL or unparseable; we | |
| 438 | + * bail out rather than feed `false` into a chain of strtotime() calls, | |
| 439 | + * which on PHP 8.1+ raises a TypeError ($baseTimestamp must be ?int). | |
| 440 | + * - The previous "+N * M days" string was never a valid strtotime | |
| 441 | + * expression (strtotime doesn't multiply); we now compute the skip | |
| 442 | + * arithmetic in PHP and pass a single, well-formed relative format. | |
| 358 | 443 | */ |
| 359 | 444 | private function generateIntervalDates(object $rule, string $fromDate, string $toDate): array |
| 360 | 445 | { |
| 361 | 446 | $dates = []; |
| 362 | - $intervalDays = (int) $rule->interval_days; | |
| 363 | - $excludedDates = $rule->excluded_dates; | |
| 447 | + | |
| 448 | + $intervalDays = (int) ($rule->interval_days ?? 0); | |
| 449 | + if ($intervalDays <= 0) { | |
| 450 | + $intervalDays = 1; | |
| 451 | + } | |
| 452 | + | |
| 453 | + $excludedDates = $rule->excluded_dates ?? []; | |
| 364 | 454 | $selectedMonths = !empty($rule->months) ? $rule->months : []; |
| 365 | - | |
| 366 | - // Start from interval_start_date or rule start_date | |
| 367 | - $referenceDate = $rule->interval_start_date ?? $rule->start_date; | |
| 368 | - $reference = strtotime($referenceDate); | |
| 455 | + | |
| 456 | + $referenceDate = !empty($rule->interval_start_date) | |
| 457 | + ? $rule->interval_start_date | |
| 458 | + : ($rule->start_date ?? null); | |
| 459 | + | |
| 460 | + if (empty($referenceDate)) { | |
| 461 | + return $dates; | |
| 462 | + } | |
| 463 | + | |
| 464 | + $reference = strtotime((string) $referenceDate); | |
| 369 | 465 | $from = strtotime($fromDate); |
| 370 | 466 | $end = strtotime($toDate); |
| 371 | - | |
| 372 | - // Find first occurrence on or after fromDate | |
| 467 | + | |
| 468 | + if ($reference === false || $from === false || $end === false) { | |
| 469 | + return $dates; | |
| 470 | + } | |
| 471 | + | |
| 472 | + // Snap reference forward to the first occurrence on/after $from. | |
| 373 | 473 | if ($reference < $from) { |
| 374 | - $daysDiff = floor(($from - $reference) / 86400); | |
| 375 | - $intervalsToSkip = ceil($daysDiff / $intervalDays); | |
| 376 | - $reference = strtotime("+{$intervalsToSkip} * {$intervalDays} days", $reference); | |
| 474 | + $daysDiff = (int) floor(($from - $reference) / 86400); | |
| 475 | + $intervalsToSkip = (int) ceil($daysDiff / $intervalDays); | |
| 476 | + $skipDays = $intervalsToSkip * $intervalDays; | |
| 477 | + $advanced = strtotime("+{$skipDays} days", $reference); | |
| 478 | + if ($advanced === false) { | |
| 479 | + return $dates; | |
| 480 | + } | |
| 481 | + $reference = $advanced; | |
| 377 | 482 | } |
| 378 | - | |
| 483 | + | |
| 379 | 484 | $current = $reference; |
| 380 | - | |
| 381 | - while ($current <= $end) { | |
| 485 | + | |
| 486 | + while ($current !== false && $current <= $end) { | |
| 382 | 487 | if ($current >= $from) { |
| 383 | 488 | $dateStr = date('Y-m-d', $current); |
| 384 | 489 | $dayOfWeek = (int) date('w', $current); |
| 385 | 490 | $month = (int) date('n', $current); // 1-12 |
| 386 | - | |
| 387 | - // Check if month is allowed (if months filter is set) | |
| 491 | + | |
| 388 | 492 | if (empty($selectedMonths) || in_array($month, $selectedMonths, true)) { |
| 389 | - // Check if not excluded | |
| 390 | - if (!in_array($dateStr, $excludedDates, true)) { | |
| 391 | - // Check cutoff | |
| 493 | + if (!$this->isDateExcluded($dateStr, $excludedDates)) { | |
| 392 | 494 | if ($this->isBookable($current, $rule)) { |
| 393 | 495 | $generatedDates = $this->createAvailabilityFromRule($rule, $dateStr, $dayOfWeek); |
| 394 | 496 | $dates = array_merge($dates, $generatedDates); |
| 395 | 497 | } |
| @@ -395,12 +497,12 @@ | ||
| 395 | 497 | } |
| 396 | 498 | } |
| 397 | 499 | } |
| 398 | 500 | } |
| 399 | - | |
| 501 | + | |
| 400 | 502 | $current = strtotime("+{$intervalDays} days", $current); |
| 401 | 503 | } |
| 402 | - | |
| 504 | + | |
| 403 | 505 | return $dates; |
| 404 | 506 | } |
| 405 | 507 | |
| 406 | 508 | /** |
| @@ -477,9 +579,14 @@ | ||
| 477 | 579 | private function createAvailabilityFromRule(object $rule, string $date, int $dayOfWeek): array |
| 478 | 580 | { |
| 479 | 581 | // Check for day-specific overrides |
| 480 | 582 | $dayOverrides = $rule->day_overrides[$dayOfWeek] ?? []; |
| 481 | - | |
| 583 | + | |
| 584 | + // Preview flows pass a pseudo-rule that has no persisted id; fall back | |
| 585 | + // to the string "preview" so we still emit a deterministic synthetic | |
| 586 | + // availability id without triggering PHP 8 undefined-property warnings. | |
| 587 | + $ruleId = $rule->id ?? 'preview'; | |
| 588 | + | |
| 482 | 589 | // If rule has time_slots, create separate availability for each slot |
| 483 | 590 | if (!empty($rule->time_slots) && is_array($rule->time_slots)) { |
| 484 | 591 | $availabilities = []; |
| 485 | 592 | foreach ($rule->time_slots as $index => $slot) { |
| @@ -485,12 +592,12 @@ | ||
| 485 | 592 | foreach ($rule->time_slots as $index => $slot) { |
| 486 | 593 | $slotPrice = $slot['price'] ?? $dayOverrides['original_price'] ?? $rule->original_price; |
| 487 | 594 | $slotSeats = $slot['seats'] ?? $dayOverrides['seats_total'] ?? $rule->seats_total; |
| 488 | 595 | $slotTravelerPricing = $slot['traveler_pricing'] ?? $rule->traveler_pricing ?? []; |
| 489 | - | |
| 596 | + | |
| 490 | 597 | $availabilities[] = [ |
| 491 | - 'id' => 'rule_' . $rule->id . '_' . $date . '_slot_' . $index, | |
| 492 | - 'rule_id' => $rule->id, | |
| 598 | + 'id' => 'rule_' . $ruleId . '_' . $date . '_slot_' . $index, | |
| 599 | + 'rule_id' => $ruleId, | |
| 493 | 600 | 'trip_id' => $rule->trip_id, |
| 494 | 601 | 'departure_date' => $date, |
| 495 | 602 | 'departure_time' => $slot['departure_time'] ?? null, |
| 496 | 603 | 'arrival_time' => $slot['arrival_time'] ?? null, |
| @@ -523,10 +630,10 @@ | ||
| 523 | 630 | $seats = $dayOverrides['seats_total'] ?? $rule->seats_total; |
| 524 | 631 | $travelerPricing = $rule->traveler_pricing ?? []; |
| 525 | 632 | |
| 526 | 633 | return [[ |
| 527 | - 'id' => 'rule_' . $rule->id . '_' . $date, | |
| 528 | - 'rule_id' => $rule->id, | |
| 634 | + 'id' => 'rule_' . $ruleId . '_' . $date, | |
| 635 | + 'rule_id' => $ruleId, | |
| 529 | 636 | 'trip_id' => $rule->trip_id, |
| 530 | 637 | 'departure_date' => $date, |
| 531 | 638 | 'departure_time' => $rule->departure_time, |
| 532 | 639 | 'arrival_time' => $rule->arrival_time, |
| @@ -552,13 +659,94 @@ | ||
| 552 | 659 | |
| 553 | 660 | /** |
| 554 | 661 | * Preview generated dates (for admin UI) |
| 555 | 662 | */ |
| 663 | + /** | |
| 664 | + * Has the operator explicitly closed this date on any of the trip's active | |
| 665 | + * rules — a holiday, or a period such as a business vacation? | |
| 666 | + * | |
| 667 | + * Only answers for dates the operator deliberately listed. A date that is | |
| 668 | + * simply not generated by a rule (a Tuesday on a Mon/Wed/Fri pattern) is | |
| 669 | + * not "excluded" and is left to the rest of the resolution chain. | |
| 670 | + */ | |
| 671 | + public function isDateExcludedForTrip(int $tripId, string $date): bool | |
| 672 | + { | |
| 673 | + if ($tripId < 1 || $date === '') { | |
| 674 | + return false; | |
| 675 | + } | |
| 676 | + | |
| 677 | + foreach ($this->repository->getActiveRulesForDateRange($tripId, $date, $date) as $rule) { | |
| 678 | + if ($this->isDateExcluded($date, $rule->excluded_dates ?? [])) { | |
| 679 | + return true; | |
| 680 | + } | |
| 681 | + } | |
| 682 | + | |
| 683 | + return false; | |
| 684 | + } | |
| 685 | + | |
| 686 | + /** | |
| 687 | + * Is this date excluded by the rule? | |
| 688 | + * | |
| 689 | + * An entry in `excluded_dates` is either a single 'Y-m-d' string (what the | |
| 690 | + * admin has always been able to add one day at a time) or an inclusive | |
| 691 | + * period ['start' => 'Y-m-d', 'end' => 'Y-m-d'] — a business vacation or | |
| 692 | + * seasonal closure, added in one go instead of day by day. | |
| 693 | + * | |
| 694 | + * Both shapes share the one column so existing rules keep working | |
| 695 | + * untouched: a stored list of plain strings behaves exactly as before. | |
| 696 | + * Dates are ISO, so plain string comparison is chronological. | |
| 697 | + * | |
| 698 | + * @param string $date Date being generated, 'Y-m-d'. | |
| 699 | + * @param mixed $excluded Stored exclusions (a list; anything else = none). | |
| 700 | + */ | |
| 701 | + public function isDateExcluded(string $date, $excluded): bool | |
| 702 | + { | |
| 703 | + if (!is_array($excluded)) { | |
| 704 | + return false; | |
| 705 | + } | |
| 706 | + | |
| 707 | + foreach ($excluded as $entry) { | |
| 708 | + if (is_string($entry)) { | |
| 709 | + if ($entry === $date) { | |
| 710 | + return true; | |
| 711 | + } | |
| 712 | + | |
| 713 | + continue; | |
| 714 | + } | |
| 715 | + | |
| 716 | + if (!is_array($entry)) { | |
| 717 | + continue; | |
| 718 | + } | |
| 719 | + | |
| 720 | + $start = isset($entry['start']) ? (string) $entry['start'] : ''; | |
| 721 | + $end = isset($entry['end']) ? (string) $entry['end'] : ''; | |
| 722 | + | |
| 723 | + if ($start === '' || $end === '') { | |
| 724 | + continue; | |
| 725 | + } | |
| 726 | + | |
| 727 | + // Tolerate a reversed period rather than silently ignoring it. | |
| 728 | + if ($start > $end) { | |
| 729 | + [$start, $end] = [$end, $start]; | |
| 730 | + } | |
| 731 | + | |
| 732 | + if ($date >= $start && $date <= $end) { | |
| 733 | + return true; | |
| 734 | + } | |
| 735 | + } | |
| 736 | + | |
| 737 | + return false; | |
| 738 | + } | |
| 739 | + | |
| 556 | 740 | public function previewDates(array $ruleData, int $limit = 20): array |
| 557 | 741 | { |
| 558 | 742 | // Create a temporary rule object |
| 559 | 743 | $rule = (object) $ruleData; |
| 560 | - $rule->excluded_dates = $rule->excluded_dates ?? []; | |
| 744 | + // Normalise once: a malformed value (e.g. a bare string from a | |
| 745 | + // hand-rolled API call) must not reach count() below as a fatal. | |
| 746 | + $rule->excluded_dates = is_array($rule->excluded_dates ?? null) | |
| 747 | + ? $rule->excluded_dates | |
| 748 | + : []; | |
| 561 | 749 | $rule->day_overrides = $rule->day_overrides ?? []; |
| 562 | 750 | $rule->days_of_week_array = isset($rule->days_of_week) |
| 563 | 751 | ? (is_array($rule->days_of_week) ? $rule->days_of_week : array_map('intval', explode(',', (string) $rule->days_of_week))) |
| 564 | 752 | : []; |
