vargha/hoponhopofftickets
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: paginated region/city pages, no routes before first save, safer structure save

Browse Source 
- Add /{slug}/page/N/ rules and keep the page number through 301s
- Route only regions synced to a term, so unsaved presets never claim /europe/ etc.
- Flush rewrites on the request after activation so structure routes survive
- Suggest the existing term slug for new cities (keeps live /tour-destination/ URLs)
- Refuse saves without the editor sentinel; confirm before saving an empty structure
- Show term-sync failures to the admin instead of reporting success

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Amir Bagheri
2026-10-29 15:33:01 +03:30
parent f5c4c89f1a
commit 930b7e0534
8 changed files with 161 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
viator-collector/assets/js/vc-structure-admin.js
View File

@@ -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 + ']';
});

3
viator-collector/includes/class-vc-plugin.php
View File

@@ -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() {

8
viator-collector/includes/class-vc-routing.php
View File

@@ -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;

27
viator-collector/includes/class-vc-settings.php
View File

@@ -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 ) );
}

55
viator-collector/includes/class-vc-structure-admin.php
View File

@@ -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
?>
<div class="wrap">
@@ -73,7 +81,16 @@ class VC_Structure_Admin {
<p><?php echo esc_html__( 'Regions appear in the header menu and at /{region}/. Each city is a Viator destination: its tours are imported automatically and listed at /{city}/, with tour pages at /{city}/{tour}/.', 'viator-collector' ); ?></p>
<?php if ( $notice ) : ?>
<div class="notice notice-success is-dismissible"><p><?php echo esc_html( $notice ); ?></p></div>
<div class="notice <?php echo $warnings ? 'notice-warning' : 'notice-success'; ?> is-dismissible"><p><?php echo esc_html( $notice ); ?></p></div>
<?php endif; ?>
<?php if ( $warnings ) : ?>
<div class="notice notice-warning">
<ul style="list-style:disc;margin-left:20px;">
<?php foreach ( $warnings as $warning ) : ?>
<li><?php echo esc_html( $warning ); ?></li>
<?php endforeach; ?>
</ul>
</div>
<?php endif; ?>
<?php if ( $errors ) : ?>
<div class="notice notice-error">
@@ -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() );
}
}
}

25
viator-collector/includes/class-vc-structure.php
View File

@@ -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;
}

46
viator-collector/tests/test-structure.php
View File

@@ -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() ) );
}