Fix dashboard status widget not showing after task list completion#63522
Merged
Fix dashboard status widget not showing after task list completion#63522
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRoutes reads/writes for deprecated task-list options through DeprecatedOptions (init now final static and typed), removes several PHPStan baseline entries for DeprecatedOptions, and adds unit tests verifying WC_Admin_Dashboard widget visibility for task-list complete/hidden states and capability checks. Changes
Sequence DiagramsequenceDiagram
participant WP as WordPress Hook System
participant DepOpts as DeprecatedOptions
participant Meta as TaskList Metadata
participant Dashboard as WC_Admin_Dashboard
WP->>DepOpts: pre_option_woocommerce_task_list_complete
activate DepOpts
DepOpts->>Meta: read woocommerce_task_list_completed_lists
Meta-->>DepOpts: completed lists
alt 'setup' present
DepOpts-->>WP: return 'yes'
else
DepOpts-->>WP: return 'no'
end
deactivate DepOpts
WP->>DepOpts: pre_update_option_woocommerce_task_list_complete(value)
activate DepOpts
DepOpts->>Meta: add/remove 'setup' in completed lists
DepOpts->>Meta: delete deprecated option key
DepOpts-->>WP: return previous value
deactivate DepOpts
WP->>Dashboard: option values (possibly provided via DepOpts)
activate Dashboard
Dashboard->>Dashboard: should_display_widget() checks completion/hidden and user capabilities
Dashboard-->>WP: widget visibility decision
deactivate Dashboard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
c8aa094 to
02c8487
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php (1)
122-147: Consider cleaning up the subscriber user in tearDown.The subscriber user created at lines 131-138 is not explicitly deleted. While WordPress test framework may handle transactional cleanup, explicitly deleting or tracking test users for cleanup improves test isolation.
Option: Store and clean up subscriber user
public function tearDown(): void { delete_option( 'woocommerce_task_list_completed_lists' ); delete_option( 'woocommerce_task_list_hidden' ); delete_option( 'woocommerce_task_list_hidden_lists' ); delete_option( 'woocommerce_task_list_complete' ); remove_all_filters( 'pre_option_woocommerce_task_list_complete' ); remove_all_filters( 'pre_option_woocommerce_task_list_hidden' ); + wp_set_current_user( 0 ); parent::tearDown(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php` around lines 122 - 147, The test creates a subscriber user in test_widget_does_not_show_without_capabilities() ($subscriber) but never deletes it; update the test class (e.g., add a tearDown method or track created users in a property like $this->created_users) to call wp_delete_user($subscriber) (or loop over $this->created_users) after each test so the subscriber created in test_widget_does_not_show_without_capabilities() is removed and test isolation is preserved; reference the $subscriber variable and the test method name when adding cleanup.plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php (1)
85-85: Remove unused$updatevariable assignment.The
hide()/unhide()calls are needed for their side effects, but the return value is never used. This applies here and at line 93.Proposed fix
- $update = 'yes' === $value ? $task_list->hide() : $task_list->unhide(); + 'yes' === $value ? $task_list->hide() : $task_list->unhide();Apply the same pattern at line 93.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php` at line 85, In DeprecatedOptions (class DeprecatedOptions) remove the unused assignment to $update and call the side-effecting methods directly — replace "$update = 'yes' === $value ? $task_list->hide() : $task_list->unhide();" with a direct conditional that invokes $task_list->hide() or $task_list->unhide() without assigning the result; do the same for the second occurrence of this pattern elsewhere in the file (the other hide()/unhide() call).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php`:
- Line 85: In DeprecatedOptions (class DeprecatedOptions) remove the unused
assignment to $update and call the side-effecting methods directly — replace
"$update = 'yes' === $value ? $task_list->hide() : $task_list->unhide();" with a
direct conditional that invokes $task_list->hide() or $task_list->unhide()
without assigning the result; do the same for the second occurrence of this
pattern elsewhere in the file (the other hide()/unhide() call).
In
`@plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php`:
- Around line 122-147: The test creates a subscriber user in
test_widget_does_not_show_without_capabilities() ($subscriber) but never deletes
it; update the test class (e.g., add a tearDown method or track created users in
a property like $this->created_users) to call wp_delete_user($subscriber) (or
loop over $this->created_users) after each test so the subscriber created in
test_widget_does_not_show_without_capabilities() is removed and test isolation
is preserved; reference the $subscriber variable and the test method name when
adding cleanup.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/woocommerce/phpstan-baseline.neonplugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.phpplugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php
💤 Files with no reviewable changes (1)
- plugins/woocommerce/phpstan-baseline.neon
- Use add_filter instead of add_action for pre_update_option_* hooks - Add void return type to init(), mixed types to filter callbacks - Return $old_value to skip DB writes instead of false (which stored literal false) - Add is_array() guards before in_array() calls for defensive safety - Add default cases to switch statements for complete code paths - Remove 6 resolved PHPStan baseline entries for DeprecatedOptions.php - Rename misleading $author variable to $subscriber in dashboard test
02c8487 to
ff2cefd
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php (1)
85-85: Remove unused$updatelocal variable.The assigned value is never used; call
hide()/unhide()directly to avoid dead stores and keep PHPMD clean.Suggested cleanup
- $update = 'yes' === $value ? $task_list->hide() : $task_list->unhide(); + 'yes' === $value ? $task_list->hide() : $task_list->unhide(); ... - $update = 'yes' === $value ? $task_list->hide() : $task_list->unhide(); + 'yes' === $value ? $task_list->hide() : $task_list->unhide();Also applies to: 93-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php` at line 85, Remove the unused local variable $update in DeprecatedOptions.php by replacing the assignment $update = 'yes' === $value ? $task_list->hide() : $task_list->unhide(); with a direct conditional call so you invoke $task_list->hide() or $task_list->unhide() without assigning the result; do the same for the similar statement at the other occurrence (line shown as 93) to eliminate the dead store and satisfy PHPMD.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php`:
- Line 85: Remove the unused local variable $update in DeprecatedOptions.php by
replacing the assignment $update = 'yes' === $value ? $task_list->hide() :
$task_list->unhide(); with a direct conditional call so you invoke
$task_list->hide() or $task_list->unhide() without assigning the result; do the
same for the similar statement at the other occurrence (line shown as 93) to
eliminate the dead store and satisfy PHPMD.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/woocommerce/phpstan-baseline.neonplugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.phpplugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php
💤 Files with no reviewable changes (1)
- plugins/woocommerce/phpstan-baseline.neon
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php
Contributor
Testing GuidelinesHi @kraftbj , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
Add final and @internal to init() for PHPCS InternalInjectionMethod rule. Use $sut convention in dashboard tests. Add changelog entry.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php (1)
87-97: Remove unused$updateassignments in hidden/unhidden branches.Line 87 and Line 95 assign
$updatebut never use it. This adds noise and can be simplified.Refactor suggestion
- $update = 'yes' === $value ? $task_list->hide() : $task_list->unhide(); + if ( 'yes' === $value ) { + $task_list->hide(); + } else { + $task_list->unhide(); + } delete_option( 'woocommerce_task_list_hidden' ); return $old_value; @@ - $update = 'yes' === $value ? $task_list->hide() : $task_list->unhide(); + if ( 'yes' === $value ) { + $task_list->hide(); + } else { + $task_list->unhide(); + } delete_option( 'woocommerce_extended_task_list_hidden' ); return $old_value;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php` around lines 87 - 97, Remove the unused $update assignments in the branches that call TaskLists::get_list('default'/'extended') and then call $task_list->hide() or $task_list->unhide(); the result is not used, so delete the `$update =` assignment and just call `$task_list->hide()` or `$task_list->unhide()` directly before calling delete_option('woocommerce_task_list_hidden') / delete_option('woocommerce_extended_task_list_hidden') and returning $old_value; ensure you update both cases that reference $task_list, hide(), unhide(), and the corresponding delete_option calls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php`:
- Around line 77-130: The tests currently stub
pre_option_woocommerce_task_list_* filters and bypass the DeprecatedOptions
bridge; update the three tests (test_widget_shows_when_task_list_complete,
test_widget_shows_when_task_list_hidden, and
test_widget_does_not_show_without_capabilities) to set the real underlying
option arrays (update_option('woocommerce_task_list_completed_lists', ...) and
update_option('woocommerce_task_list_hidden_lists', ...)) that DeprecatedOptions
maps from, then assert via get_option('woocommerce_task_list_complete') /
get_option('woocommerce_task_list_hidden') that the bridge returns the expected
values and finally call invoke_should_display_widget($this->sut) to verify
widget visibility; keep invoke_should_display_widget and the DeprecatedOptions
mapping logic intact so regressions in the bridge are caught.
---
Nitpick comments:
In
`@plugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.php`:
- Around line 87-97: Remove the unused $update assignments in the branches that
call TaskLists::get_list('default'/'extended') and then call $task_list->hide()
or $task_list->unhide(); the result is not used, so delete the `$update =`
assignment and just call `$task_list->hide()` or `$task_list->unhide()` directly
before calling delete_option('woocommerce_task_list_hidden') /
delete_option('woocommerce_extended_task_list_hidden') and returning $old_value;
ensure you update both cases that reference $task_list, hide(), unhide(), and
the corresponding delete_option calls.
ℹ️ Review info
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/woocommerce/changelog/fix-55137-dashboard-status-widget-task-list-completeplugins/woocommerce/src/Admin/Features/OnboardingTasks/DeprecatedOptions.phpplugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php
✅ Files skipped from review due to trivial changes (1)
- plugins/woocommerce/changelog/fix-55137-dashboard-status-widget-task-list-complete
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php (1)
77-84:⚠️ Potential issue | 🟠 MajorTests bypass the DeprecatedOptions bridge they intend to protect.
Three tests (
test_widget_shows_when_task_list_complete,test_widget_shows_when_task_list_hidden,test_widget_does_not_show_without_capabilities) add their ownpre_option_*filters that return'yes'directly, which circumvents theDeprecatedOptions::get_deprecated_options()mapping logic. These tests will pass even if the bridge is broken.To actually test the bridge, set the underlying options instead:
// For "task list complete": update_option( 'woocommerce_task_list_completed_lists', array( 'setup' ) ); // For "task list hidden": update_option( 'woocommerce_task_list_hidden_lists', array( 'setup' ) );Then assert via
get_option( 'woocommerce_task_list_complete' )returning'yes'before checking widget visibility. This ensures regressions in the bridge are caught.,
Also applies to: 89-96, 114-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php` around lines 77 - 84, The tests (test_widget_shows_when_task_list_complete, test_widget_shows_when_task_list_hidden, test_widget_does_not_show_without_capabilities) currently bypass the DeprecatedOptions bridge by adding pre_option_* filters; instead set the underlying options using update_option('woocommerce_task_list_completed_lists', array('setup')) for the "complete" case and update_option('woocommerce_task_list_hidden_lists', array('setup')) for the "hidden" case, then assert get_option('woocommerce_task_list_complete') returns 'yes' (or the expected value) before calling invoke_should_display_widget($this->sut) so the DeprecatedOptions::get_deprecated_options() mapping is exercised and regressions are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@plugins/woocommerce/tests/php/includes/admin/class-wc-admin-dashboard-test.php`:
- Around line 77-84: The tests (test_widget_shows_when_task_list_complete,
test_widget_shows_when_task_list_hidden,
test_widget_does_not_show_without_capabilities) currently bypass the
DeprecatedOptions bridge by adding pre_option_* filters; instead set the
underlying options using update_option('woocommerce_task_list_completed_lists',
array('setup')) for the "complete" case and
update_option('woocommerce_task_list_hidden_lists', array('setup')) for the
"hidden" case, then assert get_option('woocommerce_task_list_complete') returns
'yes' (or the expected value) before calling
invoke_should_display_widget($this->sut) so the
DeprecatedOptions::get_deprecated_options() mapping is exercised and regressions
are caught.
Collaborator
Author
|
Response for CodeRabbit's concerns: These tests intentionally use The suggested You're right that bridge regressions aren't caught here — but that's a case for a dedicated |
…a loss - Tests now use DeprecatedOptions::init() and set the underlying modern options (woocommerce_task_list_completed_lists / hidden_lists) instead of adding pre_option filters that bypass the bridge entirely. - Add two direct bridge tests: one verifying the option mapping and one for non-array (corrupt) option data. - Move delete_option inside the is_array guard in update_deprecated_options so a corrupt completed_lists value does not cause silent data loss.
WC_INSTALLING is permanently true in the test environment (set by WC_Install::install() in the bootstrap), so the DeprecatedOptions bridge bails out when called through get_option(). The widget tests correctly use pre_option filters to test should_display_widget logic in isolation. The two bridge-specific tests now call DeprecatedOptions::get_deprecated_options() directly, bypassing the WC_INSTALLING guard while still verifying the mapping logic and corrupt-data handling.
WC_INSTALLING is permanently true in the test bootstrap, and the DeprecatedOptions::get_deprecated_options guard checks this constant even on direct calls. Bridge testing is not possible in this environment. The widget tests via pre_option filters adequately cover the fix.
kraftbj
approved these changes
Mar 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submission Review Guidelines:
Changes proposed in this Pull Request:
After completing the onboarding task list, the Dashboard Status widget disappears because
WC_Admin_Dashboard::status_widget()checksget_option( 'woocommerce_task_list_complete' ), but this option was removed and its value is now stored inwoocommerce_task_list_complete_or_hidden(an option managed byTaskLists). The old option name is never populated, so the widget never renders.This PR adds a
DeprecatedOptionsbridge inOnboardingTasksthat registerswoocommerce_task_list_completeas a deprecated option, mapping reads to the underlyingwoocommerce_task_list_complete_or_hiddenoption viapre_option_anddefault_option_filters. This restores the widget without modifying the legacy dashboard code.This issue was discovered while testing the colour contrast fix in #63521.
Closes #55137.
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
woocommerce_task_list_complete_or_hiddenoption to"yes"in the database).get_option( 'woocommerce_task_list_complete' )returns"yes"whenwoocommerce_task_list_complete_or_hiddenis"yes".Testing that has already taken place:
DeprecatedOptionsbridge (WC_Admin_Dashboard_Test).Milestone
Changelog entry
Changelog Entry Details
Significance
Type
Message
Fix dashboard status widget not appearing after completing the onboarding task list.