Skip to content

feat: add "Automatic Updates" column and related functionality to Plugins window#227

Merged
AllTerrainDeveloper merged 2 commits into
trunkfrom
autoupdate-column
May 16, 2026
Merged

feat: add "Automatic Updates" column and related functionality to Plugins window#227
AllTerrainDeveloper merged 2 commits into
trunkfrom
autoupdate-column

Conversation

@AllTerrainDeveloper

@AllTerrainDeveloper AllTerrainDeveloper commented May 16, 2026

Copy link
Copy Markdown
Collaborator

closes #226

Open WordPress Playground Preview

@AllTerrainDeveloper

Copy link
Copy Markdown
Collaborator Author

hey @claude, what about some review here? ;)

@claude

claude Bot commented May 16, 2026

Copy link
Copy Markdown

Claude finished @AllTerrainDeveloper's task in 2m 8s —— View job


Review

  • Read changed files and gather context
  • Review PHP backend changes (permissions, REST fields, window registration)
  • Review TypeScript frontend changes (types, REST client, installed-view)
  • Review tests
  • Review docs update

Overall this is a solid, well-structured PR. The implementation closely mirrors Core's WP_Plugins_List_Table behavior, the data flows are clean, and the test coverage for the new PHP is comprehensive. A few things worth addressing before merging:


Issues

1. Stale docblock — "four enrichment fields" is now five

includes/plugins-window/rest-fields.php:27:

/**
 * Register the four enrichment fields on the `plugin` REST resource.

The file header comment (lines 7–12) already lists the fifth field (desktop_mode_auto_update), but this function docblock wasn't updated to match.

Fix this →


2. tear_down() in tests doesn't clean up the new filter

tests/phpunit/tests/pluginsWindowRegistration.php:33–37:

public function tear_down() {
    remove_all_filters( 'desktop_mode_plugins_window_user_can_register' );
    remove_all_filters( 'desktop_mode_plugins_window_user_can_use' );
    parent::tear_down();
}

The individual tests that touch desktop_mode_plugins_window_auto_updates_enabled do use remove_all_filters in finally blocks, so they're safe. But for consistency with the existing pattern here — and as a safety net if someone adds a test later without a finallyremove_all_filters( 'desktop_mode_plugins_window_auto_updates_enabled' ) should be added to tear_down(). Same applies to desktop_mode_plugins_window_refresh_updates and auto_update_plugin, which are used in tests but also not currently removed in tear_down().

Fix this →


Observations (no action required)

wp_is_auto_update_enabled_for_type behind is_admin() guard

includes/plugins-window/permissions.php:157 guards the lazy-require behind is_admin(), which means desktop_mode_plugins_window_auto_updates_enabled() always returns false in REST/front-end context. This is correct — the function is only called from window.php on init (admin context). The comment on the guard explains this well. ✓

The auto_update_plugin filter payload construction

includes/plugins-window/rest-fields.php:611–628 — the two-step approach (wp_parse_args then explicitly overwriting plugin and id with $plugin_file) correctly handles the Core REST controller's .php-stripping behavior. The ordering matters here (the override after wp_parse_args wins), and the comment explains exactly why. ✓

renderAutoUpdateCell when meta is absent

src/plugins-window/installed-view.ts:498–499meta?.forced ?? null correctly evaluates to null when desktop_mode_auto_update is absent from the row, and the code falls through to the unsupported branch (em-dash). Safe degradation if REST field is missing. ✓

Optimistic toggle pattern

runToggleAutoUpdate updates state.autoUpdating, calls paintTable(), awaits the AJAX, calls mergeRow (which calls paintTable() again with row.plugin still in autoUpdating), then in finally deletes from autoUpdating and calls paintTable() a third time. The cell briefly shows spinner state right after the successful response before the finally clears it. This is correct and nearly imperceptible, matching how runUpdate handles state.updating. ✓

buildColumns() called once

The column set (including the autoUpdatesEnabled gate) is fixed at mount time, consistent with how caps.activate/caps.delete are handled. autoUpdatesEnabled is a server-generated config value that doesn't change mid-session, so this is intentional and fine. ✓


Summary

Two small fixes wanted before merge — the stale "four" → "five" docblock, and completing the tear_down() filter cleanup. The core logic — PHP gate, REST field shape, toggle-auto-updates wiring, three-state cell rendering, optimistic updates — is all correct and well-matched to Core's behavior.

@github-actions

github-actions Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

✅ WordPress Plugin Check Report

✅ Status: Passed

📊 Report

All checks passed! No errors or warnings found.


🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check

@AllTerrainDeveloper AllTerrainDeveloper enabled auto-merge (squash) May 16, 2026 13:14
@AllTerrainDeveloper AllTerrainDeveloper merged commit b381045 into trunk May 16, 2026
5 checks passed
@AllTerrainDeveloper AllTerrainDeveloper deleted the autoupdate-column branch May 16, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Plugins list does not have the enable/disable auto update

1 participant