fix(tours): mark one-shot tours as finished on render to stop them repeating on every page load#1281
Conversation
…peating 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.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthrough
ChangesTour Finished Flag Early Persistence
Possibly related PRs
Suggested labels
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. 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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
inc/ui/class-tours.php (1)
336-365:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard early persistence from filter-forced visibility overrides.
The new
update_user_meta()path runs whenever$onceis true, even ifwu_tour_finishedforced the tour to show (false). That can mark tours as finished without real completion and alter behavior after the filter is removed.Consider persisting only when the pre-filter state indicates a genuine first-time render path.
Suggested adjustment
- $finished = $this->is_tour_finished($id); - - $finished = apply_filters('wu_tour_finished', $finished, $id, get_current_user_id()); + $raw_finished = $this->is_tour_finished($id); + $finished = apply_filters('wu_tour_finished', $raw_finished, $id, get_current_user_id()); if ( ! $finished || ! $once) { ... - if ($once) { + if ($once && ! $raw_finished && $finished === $raw_finished) { $user_id = get_current_user_id(); if ($user_id) { update_user_meta($user_id, $this->get_meta_key($id), 1); } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@inc/ui/class-tours.php` around lines 336 - 365, Capture the tour's finished state before calling apply_filters('wu_tour_finished', ...) (e.g. $pre_filter_finished = $finished) and use that pre-filter value when deciding to persist; only call update_user_meta($user_id, $this->get_meta_key($id), 1) when $once is true AND the pre-filter value indicated the tour was not finished (i.e. !$pre_filter_finished), so filter-forced visibility (where the filter flips $finished to false) won't incorrectly mark a tour finished.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@inc/ui/class-tours.php`:
- Around line 336-365: Capture the tour's finished state before calling
apply_filters('wu_tour_finished', ...) (e.g. $pre_filter_finished = $finished)
and use that pre-filter value when deciding to persist; only call
update_user_meta($user_id, $this->get_meta_key($id), 1) when $once is true AND
the pre-filter value indicated the tour was not finished (i.e.
!$pre_filter_finished), so filter-forced visibility (where the filter flips
$finished to false) won't incorrectly mark a tour finished.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ae1a20b-9bed-4357-bd79-d65443ec55f4
📒 Files selected for processing (2)
inc/ui/class-tours.phptests/WP_Ultimo/UI/Tours_Test.php
|
Performance Test Results Performance test results for bdb79b1 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:
|
MERGE_SUMMARYSummary
Verification
Commit: c6743e4 aidevops.sh v3.19.5 plugin for OpenCode v1.15.10 with gpt-5.5 spent 9m and 143,881 tokens on this as a headless worker. |
🔨 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: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@inc/ui/class-tours.php`:
- Line 359: Change the non‑Yoda conditional "if ($once && ! $pre_filter_finished
&& ! $finished)" to use Yoda-style comparisons: check literals on the left with
strict equality (e.g., true === $once and false === $pre_filter_finished / false
=== $finished) so the if uses true/false === variable form for $once,
$pre_filter_finished and $finished; update the conditional in the if statement
inside the persistence guard accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22952b76-4084-44e0-bb1d-3f7a2e96382b
📒 Files selected for processing (2)
inc/ui/class-tours.phptests/WP_Ultimo/UI/Tours_Test.php
| * fire markTourFinished for completeness (the operation is | ||
| * idempotent). | ||
| */ | ||
| if ($once && ! $pre_filter_finished && ! $finished) { |
There was a problem hiding this comment.
Use Yoda conditions in the new persistence guard.
Line 359 introduces a non-Yoda conditional in production code. Please rewrite it to the repository’s required Yoda style.
Suggested change
- if ($once && ! $pre_filter_finished && ! $finished) {
+ if (true === $once && false === $pre_filter_finished && false === $finished) {As per coding guidelines: Use Yoda conditions in production code (e.g., 'value' === $var, not $var === 'value').
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ($once && ! $pre_filter_finished && ! $finished) { | |
| if (true === $once && false === $pre_filter_finished && false === $finished) { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@inc/ui/class-tours.php` at line 359, Change the non‑Yoda conditional "if
($once && ! $pre_filter_finished && ! $finished)" to use Yoda-style comparisons:
check literals on the left with strict equality (e.g., true === $once and false
=== $pre_filter_finished / false === $finished) so the if uses true/false ===
variable form for $once, $pre_filter_finished and $finished; update the
conditional in the if statement inside the persistence guard accordingly.
CI follow-upThe PR regression from CodeRabbit was fixed in Terminal CI result after the push:
The Cypress failure matches the pre-fix run's unrelated failures, not the tour files changed in this PR:
Local verification for the pushed tour change:
aidevops.sh v3.19.5 plugin for OpenCode v1.15.10 with gpt-5.5 spent 16m and 186,335 tokens on this as a headless worker. |
🔨 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: |
SummaryFinal fix for "the same admin tour keeps appearing on every page load" — the Root cause
FixPersist the finished flag synchronously, server-side, the moment a one-shot if ($once) {
$user_id = get_current_user_id();
if ($user_id) {
update_user_meta($user_id, $this->get_meta_key($id), 1);
}
}The Shepherd event handlers in VerificationLocal
Live (https://ruling-sable.jurassic.ninja, Ultimate Multisite v2.12.0)Deployed patched
Tests added
Related
aidevops.sh v3.19.5 plugin for OpenCode v1.15.10 with gpt-5.5 spent 1d 1h and 14,327 tokens on this as a headless worker. Merged via PR #1281 to main. |
superdav42
added
the
review-feedback-scanned
1 participant
Summary
Final fix for "the same admin tour keeps appearing on every page load" — the
user-visible symptom that survived #1051, #1268, #1271, and #1277. All prior
fixes ensured the dismissal could persist across cookie / id-normalization
/ user-scoping edge cases, but every one of them still depended on the AJAX
dismissal triggered by Shepherd's
complete/cancelevents. If the usernever reached the end of the tour, no AJAX call was made and no flag was
stored, so the next page load showed the same tour again.
Root cause
assets/js/tours.jsonly firesmarkTourFinishedon Shepherd'scompleteand
cancelevents. Real-world dismissal modes that bypass both events:sendBeaconis blocked or fails silently.In all of these cases
wu_tour_finished_*is never written, sois_tour_finished()returnsfalseon the next request and the tourre-renders.
Fix
Persist the finished flag synchronously, server-side, the moment a one-shot
tour is queued for display in
inc/ui/class-tours.php::create_tour():The Shepherd event handlers in
tours.jsstill firemarkTourFinishedfor completeness;
update_user_metais idempotent so the double-writeis harmless. Tours registered with
$once = false(none in the currentcodebase, but the option is part of the public contract) continue to
render on every page load.
Verification
Local
vendor/bin/phpcs inc/ui/class-tours.php tests/WP_Ultimo/UI/Tours_Test.php-> 0 errors / 0 warnings.vendor/bin/phpstan analyse inc/ui/class-tours.php-> 0 errors.vendor/bin/phpunit --filter "test_create_tour_(marks_finished_on_render|does_not_mark_finished_when_once_is_false)|test_is_tour_finished_reads_user_meta|test_is_tour_finished_falls_back_to_legacy_user_setting|test_is_tour_finished_reads_stripped_legacy_user_settings_meta" --no-coverage->OK (5 tests, 13 assertions).Live (https://ruling-sable.jurassic.ninja, Ultimate Multisite v2.12.0)
Deployed patched
inc/ui/class-tours.phpand resetwu_tour_*user metafor the
demosuper-admin (uid 1) in a brand new agent-browser session.wu_tour_finished_wp_ultimo_dashboardwu_tour_finished_checkout_form_list/wp-admin/network/admin.php?page=wp-ultimo1(written by render)1/wp-admin/network/admin.php?page=wp-ultimo-checkout-forms11(written by render)11/wp-admin/network/admin.php?page=wp-ultimoagain11Each tour renders exactly once, then never again — including when the user
refreshes immediately or never clicks the last-step button.
Tests added
test_create_tour_marks_finished_on_render— verifies the meta iswritten as soon as
in_admin_headerfires for a$once = truetour.test_create_tour_does_not_mark_finished_when_once_is_false— guardsthe public contract for non-one-shot tours.
Related
wp-settingscookie desync.aidevops.sh v3.19.5 plugin for OpenCode v1.15.10 with gpt-5.5 spent 1d 1h and 14,327 tokens on this as a headless worker.
Summary by CodeRabbit
Bug Fixes
Tests