-
Notifications
You must be signed in to change notification settings - Fork 338
Closed
Labels
Exp: SPP1Medium priorityMedium priorityPHPType: BugSomething isn't workingSomething isn't workingType: SupportSupport requestSupport request
Description
Bug Description
Site Kit currently adds a number of fields to its dedicated Site Health section on the "Info" tab of the core Site Health page. For dashboard sharing, additional fields are added for information related to the feature.
Currently it's possible to raise a PHP Notice for array access on an undefined index when dashboard sharing is enabled. This happens when sharing settings fields are being constructed and the implementation assumes that every shareable module will have a respective entry in the sharing settings, which isn't the case.
Steps to reproduce
- Enable
dashboardSharing - Reset the dashboard sharing settings, if any are set
- Go to the Site Health > Info tab
- See notices (in console, debug log, or Query Monitor, etc)
Screenshots
Additional Context
- No E2E coverage currently exists for the Site Health integration, and any PHP notices like this would cause it to fail if it were, so adding a basic test or two here would be a nice enhancement
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
- No PHP notices should be raised from Site Kit when viewing its info in Site Health
- All dashboard sharing specific fields should only be added to the Site Health data when the feature is enabled, including "Recoverable Modules" as well as information about shared roles and management
Implementation Brief
- Finish PR started in Fix Site Health notices and add basic E2E tests #6598
- So far this only adds E2E coverage and fixes a flaw/quirk in the debug log assertions so that the notices raised here will be caught
- Add guards for dashboard sharing fields to return early if
dashboardSharingisn't enabled - Update
Debug_Data::get_module_sharing_settings_fieldsto access keys of the sharing settings safely
Test Coverage
- Add basic E2E coverage for Site Kit's Site Health integration (basically load the screen, ensure the SK section is there, and to ensure no notices are raised which is part of every E2E test automatically)
QA Brief
- See instructions above for how to reproduce, which is simple – essentially any shareable module (module showing up in the dashboard sharing settings) but without any settings saved for it will result in notices raised when viewing the Site Health info tab
- The fix should also be covered by E2E (added here) and going forward
Changelog entry
- Prevent PHP errors on the Site Health info page when Dashboard Sharing is enabled.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
Exp: SPP1Medium priorityMedium priorityPHPType: BugSomething isn't workingSomething isn't workingType: SupportSupport requestSupport request
