diff --git a/viator-collector/docs/superpowers/plans/2026-10-29-region-city-urls.md b/viator-collector/docs/superpowers/plans/2026-10-29-region-city-urls.md new file mode 100644 index 0000000..6b22c6e --- /dev/null +++ b/viator-collector/docs/superpowers/plans/2026-10-29-region-city-urls.md @@ -0,0 +1,293 @@ +# Region / City / Product URLs Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Serve regions at `/{region}/`, cities at `/{city}/` and products at `/{city}/{product}/`. The client manages the region → city structure from a validated settings screen. + +**Architecture:** A pure-PHP `VC_Structure` class holds the structure, validation, canonical-city choice and rewrite-rule generation, and is unit-tested without WordPress. Thin WordPress layers sit on top of it: +- `VC_Structure_Admin`: settings screen, save, sync to `viator_destination` terms +- `VC_Routing`: rewrite rules, link filters, 301s +- `VC_Menu`: region items in the header menu + +These layers are verified by `php -l` and a manual QA pass on a WordPress install. + +**Tech Stack:** WordPress, PHP 8 (local CLI is PHP 8.4), plugin `viator-collector`, child theme `astra-child-hoponhopoff`. No Composer/PHPUnit: tests use a tiny runner at `viator-collector/tests/run.php`. + +**Spec:** `viator-collector/docs/superpowers/specs/2026-10-29-region-city-urls-design.md` + +## Global Constraints + +- Preset regions, in order: UK `uk`, Europe `europe`, Asia `asia`, Middle East and Africa `middle-east-and-africa`, Americas `americas`, Australasia `australasia`. +- Valid slug: `^[a-z0-9]+(?:-[a-z0-9]+)*$`. +- Reserved slugs: `wp-admin`, `wp-content`, `wp-includes`, `wp-json`, `feed`, `page`, `search`, `tag`, `category`, `author`, `tours`, `tour-destination`, `tour-tag`, `tour-category`, `blog`, `about`, `contact`, `faq`, `embed`, `attachment`. +- Term meta: region flag `_vc_region` = `1`; city link `_vc_viator_destination_id` (already exists). +- Post meta: `_vc_primary_destination` (Viator destination ref string). +- Options: `vc_structure` (structure array), `vc_flush_rewrite` (flag). +- All redirects are 301 and keep the query string. +- Match plugin style: static classes, `VC_` prefix, tabs, `vc_get_option`, `VC_Logger`, text domain `viator-collector`. +- Work on branch `feature/region-city-urls`, not `main`. +- `tests/` and `docs/` never ship in the zip. + +**Structure array shape** (used everywhere): +```php +array( + array( + 'name' => 'Europe', + 'slug' => 'europe', + 'term_id' => 12, // 0 until synced + 'cities' => array( + array( 'id' => '737', 'name' => 'Amsterdam', 'slug' => 'amsterdam' ), + ), + ), +) +``` + +## Review Focus + +1. A city slug equal to an **unconfigured** existing destination term's slug (e.g. two Viator "Victoria"s) must be a validation error, not a silent `wp_update_term` failure. Covered by `test_validate_taken_slug` (Task 1). +2. A product whose Viator primary destination is not configured but which belongs to two configured cities must use the first city in settings order. Covered by `test_canonical_city_fallback_order` (Task 1). +3. A product in no configured city keeps `/tours/{slug}/`: `canonical_city()` returns `''`. Covered by `test_canonical_city_none` (Task 1). +4. An admin-typed slug like `New York` or `amsterdam!` must be rejected, not silently changed. Covered by `test_validate_bad_slug` (Task 1). +5. Filter and sort query strings (`?dest=…&sort=…&paged=2`) must survive every 301. Covered by manual check M6 (Task 7). + +--- + +### Task 1: `VC_Structure` core and test runner + +**Files:** +- Create: `viator-collector/includes/class-vc-structure.php` +- Create: `viator-collector/tests/run.php`, `viator-collector/tests/test-structure.php` +- Modify: `build-zip.php` (add `'tests'` to `$excluded_dirs`) + +**Interfaces (Produces):** class `VC_Structure`, all `public static`: +- `const OPTION = 'vc_structure';` +- `default_regions(): array`: the six presets, `term_id` 0, no cities. +- `get(): array`: `get_option( self::OPTION, self::default_regions() )`, cached in a static per request. Untested; WordPress only. +- `normalize( array $raw ): array` + - Shape POST data into the structure shape: trim names, `trim(strtolower())` slugs, cast `term_id` to int and `id` to a digit string, drop city rows with an empty `id`, reindex. + - An empty region slug becomes `slugify( name )`. An empty city slug becomes `slugify( city name )`. +- `slugify( string $s ): string`: lowercase, runs of non `[a-z0-9]` become `-`, trim `-`. +- `reserved_slugs(): array`: the list in Global Constraints. +- `validate( array $structure, array $destination_map, array $taken ): array` + - `$destination_map` is Viator `id => name`. `$taken` is `slug => human label` for slugs owned by something outside the structure. + - Returns a list of error strings, empty when valid. Messages are exact: + - `Region %d has no name.` (1-based) + - `City ID %s is not a Viator destination.` + - `%s is listed more than once.` (city name) + - `Slug "%s" is invalid: use only a-z, 0-9 and -.` + - `Slug "%s" is reserved.` + - `Slug "%s" is used more than once.` + - `Slug "%s" is already used by %s.` +- `cities( array $structure ): array`: ordered `viator_id => city + array( 'region' => region slug )`. +- `canonical_city( array $structure, array $dest_ids, string $primary_id ): string` + - Returns the city slug for `$primary_id` if it is configured. + - Otherwise returns the slug of the first configured city (in `cities()` order) that appears in `$dest_ids`. + - Otherwise returns `''`. +- `primary_destination_id( array $destinations ): string`: `ref` (or `destinationId`) of the entry with truthy `primary`, else `''`. +- `rewrite_rules( array $structure ): array`, regex => query, in this order: + - each region: `'^europe/?$' => 'index.php?viator_destination=europe'` + - each city: `'^amsterdam/?$' => 'index.php?viator_destination=amsterdam'` and `'^amsterdam/([^/]+)/?$' => 'index.php?viator_tour=$matches[1]'` + +- [ ] **Step 1: Write the runner.** `tests/run.php` defines `ABSPATH`, requires `includes/class-vc-structure.php` and every `tests/test-*.php`, then calls each function whose name starts with `test_`. It provides `vc_assert_same( $expected, $actual, string $msg )`, which throws on mismatch, and `vc_assert_contains( string $needle, array $errors )`. It prints `N passed, M failed` and exits 1 on any failure. + +- [ ] **Step 2: Write the failing tests** in `tests/test-structure.php`. Use the fixture `$s` = Europe(`amsterdam` id 737, `paris` id 479) + Americas(`boston` id 678) and `$map = array('737'=>'Amsterdam','479'=>'Paris','678'=>'Boston','22'=>'Victoria')`. +```php +test_default_regions // 6 regions; slugs exactly the Global Constraints list, in order +test_normalize_fills_slugs // region name 'Middle East and Africa', slug '' -> 'middle-east-and-africa'; city row with id '' dropped +test_validate_ok // validate($s,$map,[]) === [] +test_validate_unknown_city // city id '999' -> contains 'City ID 999 is not a Viator destination.' +test_validate_duplicate_city // 737 in Europe and Americas -> contains 'Amsterdam is listed more than once.' +test_validate_duplicate_slug // region slug 'paris' + city 'paris' -> contains 'Slug "paris" is used more than once.' +test_validate_bad_slug // 'New York' and 'amsterdam!' -> both 'Slug "…" is invalid: use only a-z, 0-9 and -.' +test_validate_reserved // city slug 'contact' -> contains 'Slug "contact" is reserved.' +test_validate_taken_slug // $taken=['victoria'=>'destination "Victoria"'], city slug 'victoria' -> contains 'Slug "victoria" is already used by destination "Victoria".' +test_validate_empty_region // region name '' -> contains 'Region 1 has no name.' +test_canonical_city_primary // canonical_city($s,['479','737'],'479') === 'paris' +test_canonical_city_fallback_order // canonical_city($s,['678','479','737'],'5555') === 'amsterdam' +test_canonical_city_none // canonical_city($s,['5555'],'') === '' +test_primary_destination_id // [['ref'=>'1'],['ref'=>'2','primary'=>true]] -> '2'; [] -> '' +test_rewrite_rules // exact array: europe, americas, amsterdam, amsterdam product, paris, paris product, boston, boston product (region rules first, then cities) +test_rewrite_rules_empty_region // region with no cities still yields '^europe/?$' +``` + +- [ ] **Step 3: Run and confirm the failure.** Run `php tests/run.php` from `viator-collector/`. Expected: a fatal error saying class `VC_Structure` was not found. + +- [ ] **Step 4: Implement `VC_Structure`** with the interface above. Only `get()` may call WordPress functions. + +- [ ] **Step 5: Run and confirm the pass.** Run `php tests/run.php`. Expected: `16 passed, 0 failed`. + +- [ ] **Step 6: Exclude tests from the zip.** Add `'tests'` to `$excluded_dirs` in `build-zip.php`. Then run `php build-zip.php viator-collector "$TMP/vc.zip"` and confirm no `tests/` entries are in the zip. + +- [ ] **Step 7: Commit.** Commit message: `feat(structure): add region/city structure model with validation`. + +### Task 2: Importer stores the primary destination; crawl follows the structure + +**Files:** +- Modify: `viator-collector/includes/class-vc-importer.php` (the `$meta` array in `save_tour_meta`, ~line 370) +- Modify: `viator-collector/includes/class-vc-cron.php` (`start_run`, ~lines 159-166) +- Modify: `viator-collector/includes/class-vc-settings.php` (description of the crawl destinations field) +- Modify: `viator-collector/includes/class-vc-plugin.php` (load `class-vc-structure.php` before the importer) + +**Interfaces (Consumes):** `VC_Structure::primary_destination_id()`, `VC_Structure::cities()`, `VC_Structure::get()`. + +- [ ] **Step 1: Store the primary destination.** Add `'_vc_primary_destination' => VC_Structure::primary_destination_id( (array) vc_array_get( $product, 'destinations', array() ) )` to the `$meta` array. + +- [ ] **Step 2: Build the crawl queue from the structure.** In `start_run`: + - If `array_keys( VC_Structure::cities( VC_Structure::get() ) )` is non-empty, the queue is exactly that list. + - Otherwise keep the existing allowlist/full-catalogue logic. + - Log `Sync run started from structure — %d cities queued.` in the structure case. + +- [ ] **Step 3: Relabel the old allowlist.** Append `Used only when no cities are set on the Viator Structure screen.` to the crawl destinations field's description. + +- [ ] **Step 4: Lint.** Run `php -l` on each changed file. Expected: `No syntax errors detected` for each. Then run `php tests/run.php`, which should still pass. + +- [ ] **Step 5: Commit.** Commit message: `feat(import): store primary destination; crawl configured cities`. + +### Task 3: Structure settings screen, validated save, and term sync + +**Files:** +- Create: `viator-collector/includes/class-vc-structure-admin.php` +- Create: `viator-collector/assets/js/vc-structure-admin.js` +- Modify: `viator-collector/includes/class-vc-plugin.php` (load the file and call `VC_Structure_Admin::init()`) + +**Interfaces:** +- Consumes: all of `VC_Structure`; existing AJAX action `vc_search_destinations` (nonce `vc_search_destinations`, POST `q`, returns `{results:[{id,name}]}`); `VC_Taxonomies::get_or_create_destination_by_id( $id, $name )`; `VC_API::get_destination_map()`. +- Produces: + - `VC_Structure_Admin::init()` + - `VC_Structure_Admin::taken_slugs( array $structure ): array`, slug => label + - `VC_Structure_Admin::sync_terms( array $old, array $new ): array`, returns `$new` with region `term_id`s filled + - Sets option `vc_flush_rewrite` = 1 after a successful save + +- [ ] **Step 1: Add the screen.** Register a submenu under Settings with `add_options_page`: title `Viator Structure`, slug `viator-collector-structure`, capability `manage_options`. + - The page renders one block per region: name input, slug input, a hidden `term_id`, and up/down/remove buttons. + - Each region block holds its city rows. A city row shows the name read-only, an editable slug, and a hidden `id` and `name`, with up/down/remove buttons. + - Each region has a city search box that calls `vc_search_destinations`. Results whose id is already used anywhere in the form are shown disabled. + - An "Add region" button sits at the end. + - Field names: `vc_structure[i][name|slug|term_id]` and `vc_structure[i][cities][j][id|name|slug]`. The JS renumbers indexes before submit. + - The form posts to `admin-post.php`, action `vc_save_structure`, nonce `vc_save_structure`. + +- [ ] **Step 2: Implement `taken_slugs( $structure )`.** It returns these as `slug => label`: + - top-level Pages (`post_parent` 0), labelled `page "Title"` + - posts of every public post type in any status except trash and auto-draft, labelled `post "Title"` + - `viator_destination` terms that are neither a structure region (`term_id`) nor a structure city (matched through `_vc_viator_destination_id`), labelled `destination "Name"` + +- [ ] **Step 3: Implement the save handler.** + 1. Check the capability and nonce. + 2. Run `normalize( wp_unslash( $_POST['vc_structure'] ?? array() ) )`. + 3. Load the destination map. If that returns a `WP_Error`, treat it as the single error `Could not load Viator destinations; nothing was saved.` + 4. Run `validate( $s, $map, self::taken_slugs( $s ) )`. + 5. On errors, store `array( 'errors' => …, 'structure' => $s )` in transient `vc_structure_draft_{user_id}` (10 minutes) and redirect back. The screen re-renders that draft with a red notice listing every error. Nothing is saved. + 6. On success: `$s = self::sync_terms( VC_Structure::get(), $s )`, then `update_option( VC_Structure::OPTION, $s )`, then `update_option( 'vc_flush_rewrite', 1 )`, then redirect with the notice `Structure saved.` + +- [ ] **Step 4: Implement `sync_terms( $old, $new )`.** + - **Region:** update the term by `term_id` if it exists (name, slug, parent 0). Otherwise `wp_insert_term( name, 'viator_destination', array( 'slug' => slug ) )`. In both cases set `_vc_region` = 1 and store the `term_id` back. + - **City:** `$tid = VC_Taxonomies::get_or_create_destination_by_id( id, name )`, then `wp_update_term( $tid, 'viator_destination', array( 'slug' => slug, 'parent' => region term_id ) )`. + - **Removed regions** (old `term_id`s that are no longer present): `wp_delete_term`. + - **Removed cities** (old ids that are no longer present): set `parent` to 0. + - Log every `WP_Error` with `VC_Logger::error`. + +- [ ] **Step 5: Add the page-slug clash notice.** On `admin_notices`, on the page edit screen, if the current page's `post_name` is a structure region or city slug, show the warning `This page's URL is also used by the Viator region/city "%s"; the Viator page will take priority.` + +- [ ] **Step 6: Lint.** Run `php -l includes/class-vc-structure-admin.php` and `node --check assets/js/vc-structure-admin.js`. Expected: no errors. + +- [ ] **Step 7: Commit.** Commit message: `feat(structure): admin screen with validated save and term sync`. + +### Task 4: Routing — rewrite rules, links, redirects + +**Files:** +- Create: `viator-collector/includes/class-vc-routing.php` +- Modify: `viator-collector/includes/class-vc-plugin.php` (load the file and call `VC_Routing::init()`) + +**Interfaces:** +- Consumes: `VC_Structure::get()`, `::rewrite_rules()`, `::cities()`, `::canonical_city()`; option `vc_flush_rewrite`. +- Produces: `VC_Routing::init()`, `VC_Routing::is_configured_term( WP_Term $term ): bool`, `VC_Routing::product_city( int $post_id ): string` + +- [ ] **Step 1: Register the rules.** On `init` at priority 20, call `add_rewrite_rule( $regex, $query, 'top' )` for each `rewrite_rules( get() )` entry. Then, if `get_option( 'vc_flush_rewrite' )` is set, delete it and call `flush_rewrite_rules( false )`. + +- [ ] **Step 2: Implement `is_configured_term`.** It is true when the term is in `viator_destination` and either has `_vc_region` = 1 or its `_vc_viator_destination_id` is a key of `cities()`. + +- [ ] **Step 3: Implement `product_city`.** Build `$dest_ids` from the `ref` / `destinationId` values in the post's `_vc_destinations` JSON meta. Return `canonical_city( get(), $dest_ids, (string) _vc_primary_destination )`. Cache the result per `post_id` in a static. + +- [ ] **Step 4: Add the link filters.** + - `term_link`: for a configured term, return `home_url( '/' . $term->slug . '/' )`. + - `post_type_link`: for a `viator_tour` with a non-empty `post_name` and a non-empty `product_city`, return `home_url( "/{city}/{post_name}/" )`. + - Everything else is returned unchanged. + +- [ ] **Step 5: Add the 301s.** On `template_redirect`, for `is_singular( 'viator_tour' )` (published, not a preview) and for configured `is_tax( 'viator_destination' )`: + - Compute the canonical URL (`get_permalink()` / `get_term_link()`). + - Compare `untrailingslashit` paths of the canonical URL and `REQUEST_URI` with the query string removed. + - If they differ, `wp_safe_redirect( canonical + '?' + original query string if any, 301 )` and exit. + +- [ ] **Step 6: Add the term canonical tag.** On `wp_head`, for a configured term archive, print ``. Skip this when `WPSEO_VERSION` or `RANK_MATH_VERSION` is defined. Singular products already get core's canonical from `get_permalink()`. + +- [ ] **Step 7: Lint and commit.** Run `php -l includes/class-vc-routing.php`. Expected: no errors. Commit message: `feat(routing): region/city/product URLs with 301s from old paths`. + +### Task 5: Region landing page + +**Files:** +- Create: `viator-collector/templates/region.php` +- Modify: `viator-collector/includes/class-vc-display.php` (`taxonomy_template`, ~line 534) +- Modify: `viator-collector/templates/listing.php` (new optional arg `$vc_after_hero`) + +**Interfaces (Consumes):** `VC_Routing::is_configured_term()`; existing `vc_get_template()`, `card-destination.php` (arg `term`), `listing.php`. + +- [ ] **Step 1: Route region terms.** In `taxonomy_template`, when the queried term has `_vc_region` = 1, return `get_stylesheet_directory() . '/viator-collector/region.php'` if it exists, else `VC_PLUGIN_DIR . 'templates/region.php'`. + +- [ ] **Step 2: Add `$vc_after_hero` to `listing.php`.** It is an optional HTML string, default `''`, echoed immediately after the listing hero section closes. + +- [ ] **Step 3: Write `templates/region.php`.** + - Copy the structure of `taxonomy-viator.php`. + - Build `$vc_after_hero` as `
`. It holds the heading `Cities in %s` and a `vc-destination-grid` of `card-destination.php` cards. + - The cards are the child terms from `get_terms( array( 'taxonomy' => 'viator_destination', 'parent' => term_id, 'hide_empty' => false ) )`, ordered as in the structure (the order of `cities()`). + - The listing's main query already includes child terms, so it shows every tour in the region. + +- [ ] **Step 4: Lint and commit.** Run `php -l` on the three files. Commit message: `feat(display): region landing page with city grid`. + +### Task 6: Regions in the header menu + +**Files:** +- Create: `viator-collector/includes/class-vc-menu.php` +- Modify: `viator-collector/includes/class-vc-plugin.php` (load the file and call `VC_Menu::init()`) +- Modify: `astra-child-hoponhopoff/inc/nav-walker.php` (`hoho_nav_fallback`) + +**Interfaces:** +- Consumes: `VC_Structure::get()`; region and city terms (`term_id`, links via `get_term_link`). +- Produces: `VC_Menu::items(): array` of menu-item objects. + +- [ ] **Step 1: Implement `VC_Menu::items()`.** Build `stdClass` items with these fields: `ID`, `db_id`, `menu_item_parent` (string), `title`, `url`, `classes` (array), `type` `'custom'`, `object` `'custom'`, `object_id`, `target` `''`, `xfn` `''`, `attr_title` `''`, `description` `''`, `current` false. + - Use negative unique `ID`/`db_id`: `-1000 - n`. + - Each region gets a top-level item. + - A region with cities gets these children, in order: `All of %s` (the region URL, because the theme walker turns a parent with children into a toggle button with no link), then one item per city. + - Regions whose term no longer exists are skipped. + +- [ ] **Step 2: Append to the header menu.** Add the filter `wp_nav_menu_objects` (`$items`, `$args`). When `$args->theme_location === 'hoho_primary'`, return `array_merge( $items, self::items() )`. Set `menu_order` sequentially after the existing items. + +- [ ] **Step 3: Update the theme fallback.** In `hoho_nav_fallback`, after the Tours link and when `class_exists( 'VC_Menu' )`, echo `walk_nav_menu_tree( VC_Menu::items(), 2, (object) array( 'walker' => new Hoho_Nav_Walker() ) )` inside the same `