diff --git a/viator-collector.zip b/viator-collector.zip index 65ab1eb..7a24480 100644 Binary files a/viator-collector.zip and b/viator-collector.zip differ diff --git a/viator-collector/assets/js/vc-structure-admin.js b/viator-collector/assets/js/vc-structure-admin.js index 1547fa3..bf7dea7 100644 --- a/viator-collector/assets/js/vc-structure-admin.js +++ b/viator-collector/assets/js/vc-structure-admin.js @@ -43,7 +43,7 @@ var row = fromTemplate('vc-tpl-city'); row.querySelector('[data-f="id"]').value = item.id; row.querySelector('[data-f="name"]').value = item.name; - row.querySelector('[data-f="slug"]').value = slugify(item.name); + row.querySelector('[data-f="slug"]').value = item.slug || slugify(item.name); row.querySelector('.vc-city__name').textContent = item.name; row.querySelector('.vc-city__id').textContent = item.id; region.querySelector('.vc-cities tbody').appendChild(row); @@ -147,8 +147,27 @@ region.querySelector('[data-f="name"]').focus(); }); - form.addEventListener('submit', function () { - list.querySelectorAll('.vc-region').forEach(function (region, i) { + function hidden(name) { + var input = document.createElement('input'); + input.type = 'hidden'; + input.name = name; + input.value = '1'; + form.appendChild(input); + } + + form.addEventListener('submit', function (e) { + var regions = list.querySelectorAll('.vc-region'); + if (!regions.length) { + if (!window.confirm(i18n.confirmEmpty)) { + e.preventDefault(); + return; + } + hidden('vc_confirm_empty'); + } + // Proves the editor ran: the server refuses a save without it, so a + // failed script can never submit an empty structure by accident. + hidden('vc_structure_present'); + regions.forEach(function (region, i) { region.querySelectorAll('.vc-region__head [data-f]').forEach(function (input) { input.name = 'vc_structure[' + i + '][' + input.dataset.f + ']'; }); diff --git a/viator-collector/includes/class-vc-plugin.php b/viator-collector/includes/class-vc-plugin.php index b067e0f..388d6cd 100644 --- a/viator-collector/includes/class-vc-plugin.php +++ b/viator-collector/includes/class-vc-plugin.php @@ -60,6 +60,9 @@ final class VC_Plugin { VC_Taxonomies::register(); VC_Cron::maybe_schedule(); flush_rewrite_rules(); + // Structure routes are added on init, which has not run for this + // request; flush again on the next one so they are included. + update_option( 'vc_flush_rewrite', 1 ); } public static function deactivate() { diff --git a/viator-collector/includes/class-vc-routing.php b/viator-collector/includes/class-vc-routing.php index 67569dd..b308f0b 100644 --- a/viator-collector/includes/class-vc-routing.php +++ b/viator-collector/includes/class-vc-routing.php @@ -99,6 +99,13 @@ class VC_Routing { $term = get_queried_object(); if ( self::is_configured_term( $term ) ) { $canonical = get_term_link( $term ); + // Keep the page: /tour-destination/x/page/2/ and /x/?paged=2 + // both land on /x/page/2/ (the form core's redirect_canonical + // would produce anyway). + $paged = (int) get_query_var( 'paged' ); + if ( $paged > 1 && ! is_wp_error( $canonical ) ) { + $canonical = trailingslashit( $canonical ) . 'page/' . $paged . '/'; + } } } if ( ! $canonical || is_wp_error( $canonical ) ) { @@ -114,6 +121,7 @@ class VC_Routing { } if ( isset( $parts[1] ) && '' !== $parts[1] ) { $canonical .= '?' . $parts[1]; + $canonical = remove_query_arg( 'paged', $canonical ); } wp_safe_redirect( $canonical, 301 ); exit; diff --git a/viator-collector/includes/class-vc-settings.php b/viator-collector/includes/class-vc-settings.php index 9ce82b1..71bdc63 100644 --- a/viator-collector/includes/class-vc-settings.php +++ b/viator-collector/includes/class-vc-settings.php @@ -63,6 +63,33 @@ class VC_Settings { } } + // Suggested URL slug for the Viator Structure screen: keep an existing + // term's slug so live /tour-destination/{slug}/ links still resolve + // (and 301), otherwise WordPress's own slug for the name. + $existing = array(); + if ( $results ) { + $terms = get_terms( + array( + 'taxonomy' => 'viator_destination', + 'hide_empty' => false, + 'meta_query' => array( // phpcs:ignore WordPress.DB.SlowDBQuery + array( + 'key' => '_vc_viator_destination_id', + 'value' => array_column( $results, 'id' ), + 'compare' => 'IN', + ), + ), + ) + ); + foreach ( is_wp_error( $terms ) ? array() : $terms as $term ) { + $existing[ (string) get_term_meta( $term->term_id, '_vc_viator_destination_id', true ) ] = $term->slug; + } + } + foreach ( $results as &$result ) { + $result['slug'] = isset( $existing[ $result['id'] ] ) ? $existing[ $result['id'] ] : sanitize_title( $result['name'] ); + } + unset( $result ); + wp_send_json_success( array( 'results' => $results ) ); } diff --git a/viator-collector/includes/class-vc-structure-admin.php b/viator-collector/includes/class-vc-structure-admin.php index 590ba76..ac0d60a 100644 --- a/viator-collector/includes/class-vc-structure-admin.php +++ b/viator-collector/includes/class-vc-structure-admin.php @@ -10,6 +10,8 @@ defined( 'ABSPATH' ) || exit; class VC_Structure_Admin { const PAGE = 'viator-collector-structure'; + private static $sync_errors = array(); + public static function init() { add_action( 'admin_menu', array( __CLASS__, 'admin_menu' ) ); add_action( 'admin_post_vc_save_structure', array( __CLASS__, 'handle_save' ) ); @@ -44,7 +46,8 @@ class VC_Structure_Admin { 'failed' => __( 'Search failed.', 'viator-collector' ), 'add' => __( 'Add', 'viator-collector' ), 'used' => __( 'Already added', 'viator-collector' ), - 'confirm' => __( 'Remove this region and all its cities from the structure?', 'viator-collector' ), + 'confirm' => __( 'Remove this region and all its cities from the structure?', 'viator-collector' ), + 'confirmEmpty' => __( 'Save with no regions at all? Every region and city URL will stop working.', 'viator-collector' ), ), ) ); @@ -66,6 +69,11 @@ class VC_Structure_Admin { $errors = (array) $draft['errors']; $structure = (array) $draft['structure']; } + $warnings = get_transient( self::warnings_key() ); + $warnings = is_array( $warnings ) ? $warnings : array(); + if ( $warnings ) { + delete_transient( self::warnings_key() ); + } $notice = isset( $_GET['vc_notice'] ) ? sanitize_text_field( wp_unslash( $_GET['vc_notice'] ) ) : ''; // phpcs:ignore WordPress.Security.NonceVerification ?>
@@ -73,7 +81,16 @@ class VC_Structure_Admin {

-

+

+ + +
+ +
@@ -190,7 +207,11 @@ class VC_Structure_Admin { unset( $region ); $map = ( new VC_API() )->get_destination_map(); - if ( is_wp_error( $map ) ) { + if ( empty( $_POST['vc_structure_present'] ) ) { + $errors = array( __( 'The structure editor did not load correctly, so nothing was saved. Reload the page and try again.', 'viator-collector' ) ); + } elseif ( ! $structure && empty( $_POST['vc_confirm_empty'] ) ) { + $errors = array( __( 'Saving an empty structure needs confirmation; nothing was saved.', 'viator-collector' ) ); + } elseif ( is_wp_error( $map ) ) { $errors = array( __( 'Could not load Viator destinations; nothing was saved.', 'viator-collector' ) ); } else { $errors = VC_Structure::validate( $structure, (array) $map, self::taken_slugs( $structure ) ); @@ -211,21 +232,39 @@ class VC_Structure_Admin { self::draft_key(), array( 'errors' => $errors, - 'structure' => $structure, + // Without the editor's fields there is no draft worth keeping. + 'structure' => empty( $_POST['vc_structure_present'] ) ? VC_Structure::get() : $structure, ), 10 * MINUTE_IN_SECONDS ); self::redirect(); } - $structure = self::sync_terms( VC_Structure::get(), $structure ); + self::$sync_errors = array(); + $structure = self::sync_terms( VC_Structure::get(), $structure ); update_option( VC_Structure::OPTION, $structure ); VC_Structure::flush_cache(); update_option( 'vc_flush_rewrite', 1 ); VC_Logger::info( sprintf( 'Structure saved: %d regions, %d cities.', count( $structure ), count( VC_Structure::cities( $structure ) ) ) ); + if ( self::$sync_errors ) { + set_transient( self::warnings_key(), self::$sync_errors, 10 * MINUTE_IN_SECONDS ); + self::redirect( __( 'Structure saved, but some pages could not be set up (see below).', 'viator-collector' ) ); + } self::redirect( __( 'Structure saved.', 'viator-collector' ) ); } + private static function warnings_key() { + return 'vc_structure_warnings_' . get_current_user_id(); + } + + /** + * Record a term-sync failure: logged, and shown to the admin after save. + */ + private static function sync_error( $message ) { + VC_Logger::error( 'Structure: ' . $message ); + self::$sync_errors[] = $message; + } + private static function redirect( $notice = '' ) { $args = array( 'page' => self::PAGE ); if ( '' !== $notice ) { @@ -335,7 +374,7 @@ class VC_Structure_Admin { $result = wp_insert_term( $region['name'], $tax, array( 'slug' => $region['slug'] ) ); } if ( is_wp_error( $result ) ) { - VC_Logger::error( 'Structure: region "' . $region['name'] . '": ' . $result->get_error_message() ); + self::sync_error( 'region "' . $region['name'] . '": ' . $result->get_error_message() ); continue; } $region['term_id'] = (int) $result['term_id']; @@ -344,7 +383,7 @@ class VC_Structure_Admin { foreach ( $region['cities'] as $city ) { $city_term = VC_Taxonomies::get_or_create_destination_by_id( $city['id'], $city['name'] ); if ( ! $city_term ) { - VC_Logger::error( 'Structure: could not create city term for ' . $city['id'] ); + self::sync_error( 'could not create the page for city ' . $city['id'] ); continue; } $result = wp_update_term( @@ -356,7 +395,7 @@ class VC_Structure_Admin { ) ); if ( is_wp_error( $result ) ) { - VC_Logger::error( 'Structure: city "' . $city['name'] . '": ' . $result->get_error_message() ); + self::sync_error( 'city "' . $city['name'] . '": ' . $result->get_error_message() ); } } } diff --git a/viator-collector/includes/class-vc-structure.php b/viator-collector/includes/class-vc-structure.php index fadf350..97c3300 100644 --- a/viator-collector/includes/class-vc-structure.php +++ b/viator-collector/includes/class-vc-structure.php @@ -193,15 +193,28 @@ class VC_Structure { /** * Explicit per-slug rules (no catch-alls), regions first, then cities. + * Only regions synced to a term (term_id set) are routed, so the unsaved + * presets never claim /europe/ etc. over existing pages. Archives get a + * /page/N/ rule because core's redirect_canonical turns ?paged=N into it. */ public static function rewrite_rules( array $structure ) { - $rules = array(); - foreach ( $structure as $region ) { - $rules[ '^' . $region['slug'] . '/?$' ] = 'index.php?viator_destination=' . $region['slug']; + $synced = array_values( + array_filter( + $structure, + function ( $region ) { + return (int) $region['term_id'] > 0; + } + ) + ); + $rules = array(); + foreach ( $synced as $region ) { + $rules[ '^' . $region['slug'] . '/page/([0-9]+)/?$' ] = 'index.php?viator_destination=' . $region['slug'] . '&paged=$matches[1]'; + $rules[ '^' . $region['slug'] . '/?$' ] = 'index.php?viator_destination=' . $region['slug']; } - foreach ( self::cities( $structure ) as $city ) { - $rules[ '^' . $city['slug'] . '/?$' ] = 'index.php?viator_destination=' . $city['slug']; - $rules[ '^' . $city['slug'] . '/([^/]+)/?$' ] = 'index.php?viator_tour=$matches[1]'; + foreach ( self::cities( $synced ) as $city ) { + $rules[ '^' . $city['slug'] . '/page/([0-9]+)/?$' ] = 'index.php?viator_destination=' . $city['slug'] . '&paged=$matches[1]'; + $rules[ '^' . $city['slug'] . '/?$' ] = 'index.php?viator_destination=' . $city['slug']; + $rules[ '^' . $city['slug'] . '/([^/]+)/?$' ] = 'index.php?viator_tour=$matches[1]'; } return $rules; } diff --git a/viator-collector/tests/test-structure.php b/viator-collector/tests/test-structure.php index 3acdc9f..08f4866 100644 --- a/viator-collector/tests/test-structure.php +++ b/viator-collector/tests/test-structure.php @@ -127,23 +127,47 @@ function test_primary_destination_id() { vc_assert_same( '', VC_Structure::primary_destination_id( array() ) ); } +function vc_fixture_synced() { + $s = vc_fixture_structure(); + $s[0]['term_id'] = 11; + $s[1]['term_id'] = 12; + return $s; +} + function test_rewrite_rules() { vc_assert_same( array( - '^europe/?$' => 'index.php?viator_destination=europe', - '^americas/?$' => 'index.php?viator_destination=americas', - '^amsterdam/?$' => 'index.php?viator_destination=amsterdam', - '^amsterdam/([^/]+)/?$' => 'index.php?viator_tour=$matches[1]', - '^paris/?$' => 'index.php?viator_destination=paris', - '^paris/([^/]+)/?$' => 'index.php?viator_tour=$matches[1]', - '^boston/?$' => 'index.php?viator_destination=boston', - '^boston/([^/]+)/?$' => 'index.php?viator_tour=$matches[1]', + '^europe/page/([0-9]+)/?$' => 'index.php?viator_destination=europe&paged=$matches[1]', + '^europe/?$' => 'index.php?viator_destination=europe', + '^americas/page/([0-9]+)/?$' => 'index.php?viator_destination=americas&paged=$matches[1]', + '^americas/?$' => 'index.php?viator_destination=americas', + '^amsterdam/page/([0-9]+)/?$' => 'index.php?viator_destination=amsterdam&paged=$matches[1]', + '^amsterdam/?$' => 'index.php?viator_destination=amsterdam', + '^amsterdam/([^/]+)/?$' => 'index.php?viator_tour=$matches[1]', + '^paris/page/([0-9]+)/?$' => 'index.php?viator_destination=paris&paged=$matches[1]', + '^paris/?$' => 'index.php?viator_destination=paris', + '^paris/([^/]+)/?$' => 'index.php?viator_tour=$matches[1]', + '^boston/page/([0-9]+)/?$' => 'index.php?viator_destination=boston&paged=$matches[1]', + '^boston/?$' => 'index.php?viator_destination=boston', + '^boston/([^/]+)/?$' => 'index.php?viator_tour=$matches[1]', ), - VC_Structure::rewrite_rules( vc_fixture_structure() ) + VC_Structure::rewrite_rules( vc_fixture_synced() ) ); } function test_rewrite_rules_empty_region() { - $rules = VC_Structure::rewrite_rules( array( array( 'name' => 'Europe', 'slug' => 'europe', 'term_id' => 0, 'cities' => array() ) ) ); - vc_assert_same( array( '^europe/?$' => 'index.php?viator_destination=europe' ), $rules ); + $rules = VC_Structure::rewrite_rules( array( array( 'name' => 'Europe', 'slug' => 'europe', 'term_id' => 11, 'cities' => array() ) ) ); + vc_assert_same( + array( + '^europe/page/([0-9]+)/?$' => 'index.php?viator_destination=europe&paged=$matches[1]', + '^europe/?$' => 'index.php?viator_destination=europe', + ), + $rules + ); +} + +// Before the structure is ever saved, get() returns the unsynced presets: +// they must not claim /uk/, /europe/, … over existing pages. +function test_rewrite_rules_skip_unsynced_regions() { + vc_assert_same( array(), VC_Structure::rewrite_rules( VC_Structure::default_regions() ) ); }