| @@ -33,8 +33,17 @@ | ||
| 33 | 33 | * @throws \InvalidArgumentException If day exists and $allowExisting is false |
| 34 | 34 | */ |
| 35 | 35 | public function getOrCreateDay(int $tripId, int $dayNumber, ?string $dayTitle = null, ?string $dayDescription = null, bool $allowExisting = true): int |
| 36 | 36 | { |
| 37 | + // The method body uses $wpdb and $tableDays directly for the day | |
| 38 | + // update/insert below. The cached-query closures declare their own copies, | |
| 39 | + // but the body never did — so $wpdb was null ("Call to a member function | |
| 40 | + // update() on null") and $tableDays was empty ("Incorrect table name '')". | |
| 41 | + // This broke saving an activity onto an existing, titled day and silently | |
| 42 | + // mis-created new days. Declare both here. | |
| 43 | + global $wpdb; | |
| 44 | + $tableDays = TripItineraryDaysTable::getTableName(); | |
| 45 | + | |
| 37 | 46 | // Use QueryCache for caching day existence checks |
| 38 | 47 | $cacheKey = Cache::KEY_DAY_EXISTS . "_{$tripId}_day_{$dayNumber}"; |
| 39 | 48 | |
| 40 | 49 | $existingDay = $this->cacheQueryResult($cacheKey, function() use ($tripId, $dayNumber) { |
| @@ -87,9 +96,10 @@ | ||
| 87 | 96 | $existingDaysList = implode(', ', $existingDays); |
| 88 | 97 | |
| 89 | 98 | throw new \InvalidArgumentException( |
| 90 | 99 | sprintf( |
| 91 | - __('Day %s already exists for this trip. Please use day %d instead.', 'yatra'), | |
| 100 | + /* translators: 1: comma-separated list of existing day numbers, 2: next available day number. */ | |
| 101 | + __('Day %1$s already exists for this trip. Please use day %2$d instead.', 'yatra'), | |
| 92 | 102 | $existingDaysList, |
| 93 | 103 | $nextDay |
| 94 | 104 | ) |
| 95 | 105 | ); |
| @@ -143,9 +153,9 @@ | ||
| 143 | 153 | */ |
| 144 | 154 | public function createEntry(array $data): int |
| 145 | 155 | { |
| 146 | 156 | global $wpdb; |
| 147 | - $tableEntries = $this->getTableName(); // yatra_new_trip_itinerary_day_entry | |
| 157 | + $tableEntries = $this->getTableName(); // yatra_trip_itinerary_day_entry | |
| 148 | 158 | $tableDays = TripItineraryDaysTable::getTableName(); |
| 149 | 159 | |
| 150 | 160 | // Determine if this is a day entry or an activity entry |
| 151 | 161 | // Day entries have no item_type_id and item_id (or they are explicitly 0) |
| @@ -352,16 +362,27 @@ | ||
| 352 | 362 | // When mode='activity', we ONLY update the activity entry itself |
| 353 | 363 | // We do NOT touch the day table at all |
| 354 | 364 | // The day_id should remain the same unless explicitly changed |
| 355 | 365 | |
| 356 | - // Format time field | |
| 366 | + // Format time field. We must distinguish "field not present in payload" | |
| 367 | + // (don't touch the column) from "field present but empty" (the user | |
| 368 | + // cleared the time and we should write null). Without this, switching an | |
| 369 | + // activity to time_type=duration/flexible left a stale "08:00 - 17:00" | |
| 370 | + // in the legacy `time` column even though start_time/end_time got | |
| 371 | + // nulled — so list views and the public template kept showing the old | |
| 372 | + // time range. | |
| 357 | 373 | $timeField = null; |
| 358 | - if (!empty($data['start_time']) && !empty($data['end_time'])) { | |
| 359 | - $timeField = $data['start_time'] . ' - ' . $data['end_time']; | |
| 360 | - } elseif (!empty($data['start_time'])) { | |
| 361 | - $timeField = $data['start_time']; | |
| 362 | - } elseif (isset($data['time'])) { | |
| 363 | - $timeField = $data['time']; | |
| 374 | + $timeFieldExplicit = false; | |
| 375 | + if (array_key_exists('start_time', $data) || array_key_exists('end_time', $data) || array_key_exists('time', $data)) { | |
| 376 | + $timeFieldExplicit = true; | |
| 377 | + if (!empty($data['start_time']) && !empty($data['end_time'])) { | |
| 378 | + $timeField = $data['start_time'] . ' - ' . $data['end_time']; | |
| 379 | + } elseif (!empty($data['start_time'])) { | |
| 380 | + $timeField = $data['start_time']; | |
| 381 | + } elseif (!empty($data['time'])) { | |
| 382 | + $timeField = $data['time']; | |
| 383 | + } | |
| 384 | + // else: keep $timeField=null so the column gets cleared. | |
| 364 | 385 | } |
| 365 | 386 | |
| 366 | 387 | // Prepare included_items and excluded_items as JSON |
| 367 | 388 | $includedItemsJson = null; |
| @@ -395,10 +416,10 @@ | ||
| 395 | 416 | $updateData['description'] = !empty($data['description']) ? wp_kses_post($data['description']) : null; |
| 396 | 417 | $updateFormat[] = '%s'; |
| 397 | 418 | } |
| 398 | 419 | |
| 399 | - if ($timeField !== null) { | |
| 400 | - $updateData['time'] = $timeField; | |
| 420 | + if ($timeFieldExplicit) { | |
| 421 | + $updateData['time'] = $timeField; // may be null — that's the clear-on-edit case | |
| 401 | 422 | $updateFormat[] = '%s'; |
| 402 | 423 | } |
| 403 | 424 | |
| 404 | 425 | if (isset($data['start_time'])) { |
| @@ -510,8 +531,16 @@ | ||
| 510 | 531 | $updateData['status'] = sanitize_text_field($data['status']); |
| 511 | 532 | $updateFormat[] = '%s'; |
| 512 | 533 | } |
| 513 | 534 | |
| 535 | + // Activity ordering: written by the React drag-and-drop reorder UI on the | |
| 536 | + // day-edit page. Backed by the existing `order` smallint column (idx_day_order | |
| 537 | + // index covers it). We accept 0+; clamp to non-negative. | |
| 538 | + if (isset($data['order'])) { | |
| 539 | + $updateData['order'] = max(0, (int) $data['order']); | |
| 540 | + $updateFormat[] = '%d'; | |
| 541 | + } | |
| 542 | + | |
| 514 | 543 | // Note: We do NOT update day_id when mode='activity' |
| 515 | 544 | // The activity stays in its current day unless explicitly moved via different logic |
| 516 | 545 | |
| 517 | 546 | if (!empty($updateData)) { |
| @@ -863,8 +892,16 @@ | ||
| 863 | 892 | |
| 864 | 893 | return $result !== false; |
| 865 | 894 | } |
| 866 | 895 | |
| 896 | + // Explicit 'day' mode must never fall through to delete an activity. | |
| 897 | + // Day ids and activity ids come from different tables and can collide | |
| 898 | + // numerically; if the day no longer exists (e.g. already removed in | |
| 899 | + // another tab), bail rather than silently deleting a same-id activity. | |
| 900 | + if ($mode === 'day') { | |
| 901 | + return false; | |
| 902 | + } | |
| 903 | + | |
| 867 | 904 | // Check if this is an activity entry ID (from entries table) |
| 868 | 905 | $activityEntry = $wpdb->get_row( |
| 869 | 906 | $wpdb->prepare("SELECT * FROM `{$tableEntries}` WHERE id = %d", $id) |
| 870 | 907 | ); |
| @@ -883,100 +920,58 @@ | ||
| 883 | 920 | return false; |
| 884 | 921 | } |
| 885 | 922 | |
| 886 | 923 | /** |
| 887 | - * Bulk delete entries | |
| 888 | - * @param array $ids Array of entry IDs to delete | |
| 924 | + * Bulk delete itinerary days and/or activity entries. | |
| 925 | + * | |
| 926 | + * Days and activities live in two different tables with independent | |
| 927 | + * auto-increment id spaces, so a single flat list of ids is ambiguous | |
| 928 | + * (the same number can be a valid day id *and* a valid activity id). The | |
| 929 | + * caller therefore tells us which is which: `$dayIds` are day-table ids | |
| 930 | + * (deleting one removes the day row and all of its activities) and `$ids` | |
| 931 | + * are activity-entry ids. Each id is routed through the single-item | |
| 932 | + * {@see self::delete()} with an explicit mode so the correct table is | |
| 933 | + * always used. | |
| 934 | + * | |
| 935 | + * @param array $ids Activity entry ids (day_entry table) | |
| 936 | + * @param array $dayIds Day ids (days table); the day and its activities are removed | |
| 889 | 937 | * @return array ['deleted' => count, 'failed' => count] |
| 890 | 938 | */ |
| 891 | - public function bulkDelete(array $ids): array | |
| 939 | + public function bulkDelete(array $ids, array $dayIds = []): array | |
| 892 | 940 | { |
| 893 | - global $wpdb; | |
| 894 | - $tableEntries = $this->getTableName(); | |
| 895 | - | |
| 896 | - $tableDays = TripItineraryDaysTable::getTableName(); | |
| 897 | - | |
| 898 | - if (empty($ids)) { | |
| 899 | - return ['deleted' => 0, 'failed' => 0]; | |
| 900 | - } | |
| 901 | - | |
| 902 | - // Sanitize IDs | |
| 903 | - $ids = array_map('intval', $ids); | |
| 904 | - $ids = array_filter($ids, function($id) { | |
| 905 | - return $id > 0; | |
| 906 | - }); | |
| 907 | - | |
| 908 | - if (empty($ids)) { | |
| 909 | - return ['deleted' => 0, 'failed' => 0]; | |
| 910 | - } | |
| 911 | - | |
| 912 | 941 | $deleted = 0; |
| 913 | 942 | $failed = 0; |
| 914 | - $processedDayIds = []; // Track days we've already processed | |
| 915 | 943 | |
| 916 | - foreach ($ids as $id) { | |
| 944 | + // Delete whole days first — this also removes every activity that | |
| 945 | + // belongs to the day, so any of those activity ids that also appear in | |
| 946 | + // $ids become harmless no-ops below. | |
| 947 | + $dayIds = array_unique(array_filter(array_map('intval', $dayIds), static function ($id) { | |
| 948 | + return $id > 0; | |
| 949 | + })); | |
| 950 | + foreach ($dayIds as $dayId) { | |
| 917 | 951 | try { |
| 918 | - // Get the entry to check if it's a day entry | |
| 919 | - $entry = $wpdb->get_row( | |
| 920 | - $wpdb->prepare("SELECT day_id, item_type_id, item_id FROM `{$tableEntries}` WHERE id = %d", $id) | |
| 921 | - ); | |
| 922 | - | |
| 923 | - if (!$entry) { | |
| 952 | + if ($this->delete($dayId, 'day')) { | |
| 953 | + $deleted++; | |
| 954 | + } else { | |
| 924 | 955 | $failed++; |
| 925 | - continue; | |
| 926 | 956 | } |
| 957 | + } catch (\Throwable $e) { | |
| 958 | + $failed++; | |
| 959 | + } | |
| 960 | + } | |
| 927 | 961 | |
| 928 | - $dayId = (int) $entry->day_id; | |
| 929 | - $isDayEntry = ($entry->item_type_id === null || $entry->item_type_id === 0) && | |
| 930 | - ($entry->item_id === null || $entry->item_id === 0); | |
| 931 | - | |
| 932 | - // If this is a day entry, delete all entries for this day | |
| 933 | - if ($isDayEntry) { | |
| 934 | - // Skip if we've already processed this day | |
| 935 | - if (in_array($dayId, $processedDayIds)) { | |
| 936 | - continue; | |
| 937 | - } | |
| 938 | - | |
| 939 | - $processedDayIds[] = $dayId; | |
| 940 | - | |
| 941 | - // Get all entry IDs for this day | |
| 942 | - $dayEntryIds = $wpdb->get_col( | |
| 943 | - $wpdb->prepare("SELECT id FROM `{$tableEntries}` WHERE day_id = %d", $dayId) | |
| 944 | - ); | |
| 945 | - | |
| 946 | - if (!empty($dayEntryIds)) { | |
| 947 | - // Delete images for all entries | |
| 948 | - $placeholders = implode(',', array_fill(0, count($dayEntryIds), '%d')); | |
| 949 | - $wpdb->query( | |
| 950 | - $wpdb->prepare( | |
| 951 | - "DELETE FROM `{$tableImages}` WHERE entry_id IN ($placeholders)", | |
| 952 | - ...$dayEntryIds | |
| 953 | - ) | |
| 954 | - ); | |
| 955 | - | |
| 956 | - // Delete all entries for this day | |
| 957 | - $wpdb->delete($tableEntries, ['day_id' => $dayId], ['%d']); | |
| 958 | - | |
| 959 | - // Delete the day itself | |
| 960 | - $wpdb->delete($tableDays, ['id' => $dayId], ['%d']); | |
| 961 | - } | |
| 962 | - | |
| 962 | + // Delete standalone activity entries. | |
| 963 | + $ids = array_unique(array_filter(array_map('intval', $ids), static function ($id) { | |
| 964 | + return $id > 0; | |
| 965 | + })); | |
| 966 | + foreach ($ids as $id) { | |
| 967 | + try { | |
| 968 | + if ($this->delete($id, 'activity')) { | |
| 963 | 969 | $deleted++; |
| 964 | 970 | } else { |
| 965 | - // For activity entries, just delete the entry and its images | |
| 966 | - // Delete related images | |
| 967 | - $wpdb->delete($tableImages, ['entry_id' => $id], ['%d']); | |
| 968 | - | |
| 969 | - // Delete entry | |
| 970 | - $result = $wpdb->delete($tableEntries, ['id' => $id], ['%d']); | |
| 971 | - | |
| 972 | - if ($result !== false) { | |
| 973 | - $deleted++; | |
| 974 | - } else { | |
| 975 | - $failed++; | |
| 976 | - } | |
| 971 | + $failed++; | |
| 977 | 972 | } |
| 978 | - } catch (\Exception $e) { | |
| 973 | + } catch (\Throwable $e) { | |
| 979 | 974 | $failed++; |
| 980 | 975 | } |
| 981 | 976 | } |
| 982 | 977 | |
| @@ -1042,8 +1037,9 @@ | ||
| 1042 | 1037 | $tableEntries, |
| 1043 | 1038 | [ |
| 1044 | 1039 | 'trip_id' => (int) $day->trip_id, |
| 1045 | 1040 | 'day_id' => $dayId, |
| 1041 | + /* translators: %d: itinerary day number. */ | |
| 1046 | 1042 | 'title' => $day->title ?: sprintf(__('Day %d', 'yatra'), (int) $day->day_number), |
| 1047 | 1043 | 'description' => '', |
| 1048 | 1044 | 'location' => null, |
| 1049 | 1045 | 'duration' => null, |