Skip to content

fix(tours): persist tour-finished flag in user meta to avoid wp-settings cookie desync #1268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
superdav42 merged 1 commit into from
May 25, 2026
Merged

fix(tours): persist tour-finished flag in user meta to avoid wp-settings cookie desync #1268

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 71 additions & 1 deletion inc/ui/class-tours.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,60 @@ protected function get_setting_key($id) {
return 'wu_tour_' . str_replace('-', '_', $id);
}

/**
* Returns the user meta key used to record a finished tour.
*
* We persist the finished flag in user meta (not just the WordPress
* `wp-settings-*` cookie) because `set_user_setting()` / `wp_set_all_user_settings()`
* only update the user-settings cookie on regular admin page requests via
* `wp_user_settings()` — they do NOT set the cookie during AJAX. When the
* tour dismissal arrives as an AJAX `wu_mark_tour_as_finished` call, the
* browser is never sent a `Set-Cookie` header, so the next page request
* sees a stale or missing `wp-settings-{uid}` cookie and `get_user_setting()`
* returns `false`, re-showing the tour. Storing the flag in user meta gives
* us a cookie-independent source of truth that AJAX writes can persist
* immediately and any subsequent request can read.
*
* @since 2.5.2
*
* @param string $id The tour ID.
* @return string The user meta key.
*/
protected function get_meta_key($id) {

return 'wu_tour_finished_' . str_replace('-', '_', $id);
}

/**
* Whether the tour has been finished for the current user.
*
* Checks user meta first (the authoritative cookie-independent flag) and
* falls back to the legacy `get_user_setting()` lookup for users who
* dismissed a tour before the meta-based persistence was introduced.
*
* @since 2.5.2
*
* @param string $id The tour ID.
* @param int $user_id Optional. Defaults to the current user.
* @return bool
*/
protected function is_tour_finished($id, $user_id = 0) {

$user_id = $user_id ? (int) $user_id : get_current_user_id();

if ( ! $user_id) {
return false;
}

$meta_value = get_user_meta($user_id, $this->get_meta_key($id), true);

if ($meta_value) {
return true;
}

return (bool) get_user_setting($this->get_setting_key($id), false);
}

/**
* Mark the tour as finished for a particular user.
*
Expand All @@ -79,6 +133,22 @@ public function mark_as_finished(): void {
$id = wu_request('tour_id');

if ($id) {
$user_id = get_current_user_id();

if ($user_id) {
/*
* Persist the flag in user meta so the next request can read it
* regardless of the wp-settings-* cookie state. AJAX requests do
* not pass through wp_user_settings() (which is what normally
* syncs the cookie from user meta), so writing only to
* set_user_setting() / wp_set_all_user_settings() would leave the
* browser cookie stale and the tour would re-appear on the next
* page load.
*/
update_user_meta($user_id, $this->get_meta_key($id), 1);
}

// Keep the legacy user-settings write for backward compatibility.
set_user_setting($this->get_setting_key($id), true);
if (\function_exists('save_user_settings')) {
\save_user_settings();
Expand Down Expand Up @@ -191,7 +261,7 @@ function () use ($id, $steps, $once) {
return;
}

$finished = (bool) get_user_setting($this->get_setting_key($id), false);
$finished = $this->is_tour_finished($id);

$finished = apply_filters('wu_tour_finished', $finished, $id, get_current_user_id());

Expand Down
124 changes: 122 additions & 2 deletions tests/WP_Ultimo/UI/Tours_Test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,14 @@
$reflection = new \ReflectionClass($instance);
$prop = $reflection->getProperty('tours');
$prop->setAccessible(true);
$prop->setValue($instance, ['test-tour' => [['id' => 'step1', 'text' => 'Hello']]]);
$prop->setValue($instance, [
'test-tour' => [
[
'id' => 'step1',
'text' => 'Hello',
],
],
]);

$this->assertTrue($instance->has_tours());

Expand Down Expand Up @@ -208,6 +215,112 @@
$this->assertSame('1', $parsed[ $setting_key ]);
}

/**
* Test get_meta_key normalises hyphens to underscores and uses the wu_tour_finished_ prefix.
*
* Regression test: the tour-finished flag is stored in user meta so that
* the AJAX dismissal handler can persist it without depending on the
* wp-settings-* cookie sync (which is skipped during AJAX requests by
* wp_user_settings(), leaving the browser cookie stale and causing the
* tour to re-show on the next page load — observed on the checkout form
* editor page in particular).
*/
public function test_get_meta_key_uses_wu_tour_finished_prefix(): void {

$instance = $this->get_instance();

$reflection = new \ReflectionClass($instance);
$method = $reflection->getMethod('get_meta_key');
$method->setAccessible(true);

$this->assertSame('wu_tour_finished_checkout_form_editor', $method->invoke($instance, 'checkout-form-editor'));
$this->assertSame('wu_tour_finished_wp_ultimo_dashboard', $method->invoke($instance, 'wp-ultimo-dashboard'));
$this->assertSame('wu_tour_finished_dashboard', $method->invoke($instance, 'dashboard'));
}

/**
* Test is_tour_finished returns true once the user meta flag is set.
*
* Regression test for the checkout-form-editor tour re-showing every
* visit: the AJAX `wu_mark_tour_as_finished` handler writes to user meta,
* so subsequent requests must see the flag without depending on the
* wp-settings-* cookie (which is not updated during AJAX requests).
*/
public function test_is_tour_finished_reads_user_meta(): void {

$instance = $this->get_instance();

$user_id = self::factory()->user->create(['role' => 'administrator']);
wp_set_current_user($user_id);

$reflection = new \ReflectionClass($instance);
$is_finished = $reflection->getMethod('is_tour_finished');
$is_finished->setAccessible(true);
$get_meta_key = $reflection->getMethod('get_meta_key');
$get_meta_key->setAccessible(true);

// Not finished initially.
$this->assertFalse($is_finished->invoke($instance, 'checkout-form-editor'));

// Mark as finished via the same meta key the AJAX handler writes.
update_user_meta($user_id, $get_meta_key->invoke($instance, 'checkout-form-editor'), 1);

$this->assertTrue($is_finished->invoke($instance, 'checkout-form-editor'));

// Different tour ID still false.
$this->assertFalse($is_finished->invoke($instance, 'dashboard'));
}

/**
* Test is_tour_finished falls back to legacy get_user_setting().
*
* Users who dismissed a tour before this release have their flag stored
* only in the WordPress user-settings cookie (`wp-settings-{uid}`), not in
* the new wu_tour_finished_* meta key. The legacy fallback prevents those
* users from seeing tours they already dismissed.
*
* get_user_setting() reads from $_COOKIE (or its in-memory cache), so the
* test populates $_COOKIE directly to emulate a returning browser whose
* cookie still carries the pre-upgrade dismissal flag.
*/
public function test_is_tour_finished_falls_back_to_legacy_user_setting(): void {

$instance = $this->get_instance();

$user_id = self::factory()->user->create(['role' => 'administrator']);
wp_set_current_user($user_id);

$reflection = new \ReflectionClass($instance);
$is_finished = $reflection->getMethod('is_tour_finished');
$is_finished->setAccessible(true);
$get_setting = $reflection->getMethod('get_setting_key');
$get_setting->setAccessible(true);

$cookie_name = 'wp-settings-' . $user_id;
$setting_key = $get_setting->invoke($instance, 'legacy-tour');
$prior_cookie = $_COOKIE[ $cookie_name ] ?? null; // phpcs:ignore WordPress.Security.ValidatedSanitizedInput -- test stash, no user input.
$prior_updated_settings = $GLOBALS['_updated_user_settings'] ?? null;
$GLOBALS['_updated_user_settings'] = null;
unset($_COOKIE[ $cookie_name ]);

try {
$this->assertFalse($is_finished->invoke($instance, 'legacy-tour'));

// Simulate the legacy user-settings cookie value that get_user_setting() reads.
$_COOKIE[ $cookie_name ] = $setting_key . '=1';
$GLOBALS['_updated_user_settings'] = null;

$this->assertTrue($is_finished->invoke($instance, 'legacy-tour'));
} finally {
if (null === $prior_cookie) {
unset($_COOKIE[ $cookie_name ]);
} else {
$_COOKIE[ $cookie_name ] = $prior_cookie;
}
$GLOBALS['_updated_user_settings'] = $prior_updated_settings;
}
}

/**
* Test enqueue_scripts uses wp_add_inline_script on 'underscore', not wu-admin.
*
Expand All @@ -223,14 +336,21 @@

// Register 'underscore' if not already registered (test environment may not have it).
if ( ! wp_script_is('underscore', 'registered')) {
wp_register_script('underscore', false, [], false, false);

Check warning on line 339 in tests/WP_Ultimo/UI/Tours_Test.php

View workflow job for this annotation

GitHub Actions / Code Quality Checks 

Version parameter is not explicitly set or has been set to an equivalent of "false" for wp_register_script; This means that the WordPress core version will be used which is not recommended for plugin or theme development.
}

// Inject a tour so enqueue_scripts() proceeds.
$reflection = new \ReflectionClass($instance);
$prop = $reflection->getProperty('tours');
$prop->setAccessible(true);
$prop->setValue($instance, ['test-tour' => [['id' => 'step1', 'text' => 'Hello']]]);
$prop->setValue($instance, [
'test-tour' => [
[
'id' => 'step1',
'text' => 'Hello',
],
],
]);

$instance->enqueue_scripts();

Expand Down
Loading