bugfix: normalise tour IDs to underscores in user settings keys#1051
Conversation
Tour IDs containing hyphens (wp-ultimo-dashboard, checkout-form-editor,
checkout-form-list) could not round-trip through WordPress's user settings
system, causing every tour to re-show on every session.
Root cause: WordPress's wp_user_settings() cookie sanitizer strips hyphens
(preg_replace('/[^A-Za-z0-9=&_]/', '', ...)), and wp_set_all_user_settings()
persists settings through parse_str() which can also mangle hyphenated keys
depending on WP version and storage path. The result: set_user_setting() writes
'wu_tour_wp-ultimo-dashboard=1' but get_user_setting('wu_tour_wp-ultimo-dashboard')
cannot find it in any subsequent request, so the tour always shows again.
Tours with underscore-only IDs (dashboard, new_product_warning,
new_site_template_warning) were unaffected.
Fix: add get_setting_key($id) which replaces hyphens with underscores before
using the tour ID as a user-settings key. Both mark_as_finished() and
create_tour() call it, keeping write and read in sync across all WP versions
and storage paths.
Tests: two new assertions in Tours_Test —
- test_get_setting_key_replaces_hyphens_with_underscores: pins normalisation
for all real-world tour IDs
- test_normalised_key_survives_user_settings_round_trip: verifies the
normalised key passes through parse_str() unchanged (the failure mechanism)
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 1 minute and 22 seconds.Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
|
Performance Test Results Performance test results for 595aaa0 are in 🛎️! Note: the numbers in parentheses show the difference to the previous (baseline) test run. Differences below 2% or 0.5 in absolute values are not shown. URL:
|
…peating on every page load (#1281) * fix(tours): mark one-shot tours as finished on render to stop them repeating on every page load When a tour was rendered without the user explicitly clicking through to the last step (e.g. they refreshed the page, navigated away mid-walkthrough, closed the browser tab, or hit the back button before reaching the close button), the wu_tour_finished_* meta flag was never written. The next page load would re-render the same tour, producing the user-visible symptom of "the same admin tour keeps appearing on every page load". Previous fixes (#1051, #1268, #1271, #1277) ensured the dismissal *could* persist across cookie / id-normalization / user-scoping edge cases — but all of them still depended on the AJAX dismissal triggered by Shepherd's complete / cancel events. If the user never reached the end of the tour, no AJAX call was made and no flag was stored. Persist the finished flag synchronously, server-side, the moment a one-shot tour is queued for display. The Shepherd event handlers in tours.js still fire markTourFinished for completeness; update_user_meta is idempotent so the double-write is harmless. Tours registered with $once = false continue to render on every page load. Verified on https://ruling-sable.jurassic.ninja (Ultimate Multisite v2.12.0 deploy) by: - Resetting wu_tour_* user meta and wp_user-settings for the demo user in a brand new agent-browser session. - Loading /wp-admin/network/admin.php?page=wp-ultimo — tour renders once, wu_tour_finished_wp_ultimo_dashboard = 1 written immediately. - Reloading the same URL — tour no longer renders. - Repeating for /wp-admin/network/admin.php?page=wp-ultimo-checkout-forms (checkout-form-list) — same one-shot behaviour confirmed. * fix(tours): avoid rewriting filter-forced tour state * fix(e2e): stabilize Cypress login and password reset fixture
1 participant
Summary
Tours with hyphenated IDs (
wp-ultimo-dashboard,checkout-form-editor,checkout-form-list) were shown again on every session because the IDs could not round-trip through WordPress's user settings system.Root Cause
WordPress's user settings system mangles hyphenated keys in two ways depending on the code path taken:
wp_user_settings()sanitizes the cookie withpreg_replace('/[^A-Za-z0-9=&_]/', '', ...)which strips hyphens entirely:wu_tour_wp-ultimo-dashboard=1→wu_tour_wpultimodashboard=1wp_set_all_user_settings()→parse_str()may convert hyphens to underscores depending on WP versionEither way,
get_user_setting("wu_tour_wp-ultimo-dashboard")returnsfalseon the next page load, socreate_tour()always adds the tour to the queue and it re-shows.Tours with underscore-only IDs (
dashboard,new_product_warning,new_site_template_warning) were never affected.Fix
Added
get_setting_key($id)toTours— a protected helper that replaces hyphens with underscores before using the tour ID as a user-settings key. Bothmark_as_finished()(write) andcreate_tour()(read) call it, so the stored key and the lookup key are always identical.Files Changed
EDIT: inc/ui/class-tours.php— addget_setting_key()helper; updatemark_as_finished()andcreate_tour()to use itEDIT: tests/WP_Ultimo/UI/Tours_Test.php— two new regression tests:test_get_setting_key_replaces_hyphens_with_underscores— pins normalisation for all real-world IDstest_normalised_key_survives_user_settings_round_trip— verifies the normalised key passes throughparse_str()unchanged (the exact failure mechanism)Verification
vendor/bin/phpunit --filter Tours_Test # OK (9 tests, 22 assertions)aidevops.sh v3.13.17 plugin for OpenCode v1.3.17 with claude-sonnet-4-6 spent 27m and 59,378 tokens on this with the user in an interactive session.