Add support for logging of WordPress Automatic Updates#16
Open
Add support for logging of WordPress Automatic Updates#16
Conversation
WalkthroughWP Cron tracking now prefers Changes
Sequence Diagram(s)sequenceDiagram
participant WP as WordPress Updater
participant CI as Connector_Installer
participant LOG as Log
WP->>CI: do_action('pre_auto_update', $type)
CI->>CI: if $type == 'core' set current_wordpress_version
WP->>CI: do_action('automatic_updates_complete', $update_results)
CI->>CI: validate $update_results is array & non-empty
alt plugin/theme entries present
CI->>CI: loop plugin/theme entries, validate fields, add auto_updated:true
CI->>LOG: emit plugin/theme logs (forced)
end
alt core entries present
CI->>CI: iterate core entries, skip invalid, use current_wordpress_version as old, item->version as new
CI->>LOG: emit core logs only for forward upgrades
CI->>CI: clear current_wordpress_version
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@connectors/class-connector-installer.php`:
- Around line 602-605: The theme update loop over $update_results['theme']
accesses $result->item->theme without ensuring $result->item exists; update the
defensive check in the foreach handling $update_results['theme'] to verify
isset($result->item) (or ! empty($result->item)) before checking ->theme so you
don't trigger notices or errors when $result->item is missing; modify the
condition currently using isset($result->result) / true !== $result->result /
empty($result->item->theme) to include a check for $result->item first.
- Around line 574-577: The plugin update loop is accessing $result->item->plugin
without ensuring $result->item exists; update the conditional inside the foreach
over $update_results['plugin'] (the block handling $result) to first check
isset($result->item) (e.g. add ! isset($result->item) to the existing guard) or
reorder checks so isset($result->item) is verified before accessing ->plugin,
and then continue when the item or plugin is missing to avoid PHP warnings.
- Around line 532-543: The loop over $update_results['core'] accesses
$info->item->current without ensuring $info->item exists; add a defensive check
inside the foreach (before reading ->current or ->version) to skip when
$info->item is empty or not an object (e.g. if ( empty($info->item) || !
is_object($info->item) ) continue;), then proceed to set $old_version,
$new_version and $auto_updated as currently done in the foreach body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e315c785-4202-4d93-8be8-9c19164bdc92
📒 Files selected for processing (2)
classes/class-log.phpconnectors/class-connector-installer.php
There was a problem hiding this comment.
🧹 Nitpick comments (1)
connectors/class-connector-installer.php (1)
573-599: Defensive check properly added; consider optional file existence validation.The guard for
$result->itemon line 575 correctly prevents PHP warnings when accessing$result->item->plugin. This addresses the previous review feedback.Minor edge case:
get_plugin_data()on line 591 may generate a PHP warning if the plugin file doesn't exist (e.g., removed between update completion and logging). While line 595's check prevents invalid log entries, the warning would still occur.🛡️ Optional: Add file existence check
$slug = $result->item->plugin; $old_version = isset( $result->item->current_version ) ? $result->item->current_version : ''; +$plugin_file = WP_PLUGIN_DIR . '/' . $slug; +if ( ! file_exists( $plugin_file ) ) { + continue; +} -$plugin_data = get_plugin_data( WP_PLUGIN_DIR . '/' . $slug ); +$plugin_data = get_plugin_data( $plugin_file );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/class-connector-installer.php` around lines 573 - 599, The code may call get_plugin_data( WP_PLUGIN_DIR . '/' . $slug ) even if the plugin file no longer exists, causing a PHP warning; before calling get_plugin_data in the block that processes $result->item->plugin (use $slug from $result->item->plugin), check that the computed file path (WP_PLUGIN_DIR . '/' . $slug) exists/is a file (e.g., file_exists() or is_file()) and only call get_plugin_data and populate $name/$version if the file is present; if the file is missing, skip adding the log entry (or set $name/$version to empty) so the later conditional prevents a log and no warning is emitted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@connectors/class-connector-installer.php`:
- Around line 573-599: The code may call get_plugin_data( WP_PLUGIN_DIR . '/' .
$slug ) even if the plugin file no longer exists, causing a PHP warning; before
calling get_plugin_data in the block that processes $result->item->plugin (use
$slug from $result->item->plugin), check that the computed file path
(WP_PLUGIN_DIR . '/' . $slug) exists/is a file (e.g., file_exists() or
is_file()) and only call get_plugin_data and populate $name/$version if the file
is present; if the file is missing, skip adding the log entry (or set
$name/$version to empty) so the later conditional prevents a log and no warning
is emitted.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8312d3a9-b576-40f2-9f20-9c4ca8471a6b
📒 Files selected for processing (1)
connectors/class-connector-installer.php
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@connectors/class-connector-installer.php`:
- Around line 705-713: The call to $this->log(...) is not forcing core-update
logs when wp_cron tracking is disabled; update the $this->log invocation (in the
same block that sets $this->current_wordpress_version = '') to pass
$auto_updated as the forced_log parameter (i.e. add $auto_updated as the final
argument) so core auto-update events are always logged the same way as in
callback_automatic_updates_complete(); leave the subsequent
$this->current_wordpress_version = '' line unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6baad8b-14b3-44d2-8a41-baf13537f4c6
📒 Files selected for processing (1)
connectors/class-connector-installer.php
|
There was a problem hiding this comment.
♻️ Duplicate comments (2)
connectors/class-connector-installer.php (2)
731-737:⚠️ Potential issue | 🟠 MajorForce logging for auto core updates in
_core_updated_successfully.When
$auto_updatedis true, this call still relies on defaultforced_log = false. If cron tracking suppresses normal logs, the core-update event can be dropped. Pass$auto_updatedas the final argument.🛠️ Proposed fix
$this->log( $message, compact( 'old_version', 'new_version', 'auto_updated' ), null, 'wordpress', // phpcs:ignore -- fix format text. - 'updated' + 'updated', + null, + $auto_updated );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/class-connector-installer.php` around lines 731 - 737, The call to $this->log in _core_updated_successfully currently omits the $forced_log parameter so auto core updates can be suppressed by cron tracking; update the $this->log invocation inside _core_updated_successfully to pass the $auto_updated boolean as the final argument (forced_log) so when $auto_updated is true the event is always forced to be logged, leaving the existing message, context (compact('old_version','new_version','auto_updated')), level/null and category ('wordpress','updated') arguments unchanged.
572-575:⚠️ Potential issue | 🟡 MinorGuard
$info->itembefore accessing->version.Line 573 dereferences
$info->item->versionbefore confirmingitemexists and is an object, which can emit notices on malformed update payloads.🛠️ Proposed fix
- foreach ( $update_results['core'] as $info ) { - if ( ! is_object( $info ) || empty( $info->item->version ) ) { + foreach ( $update_results['core'] as $info ) { + if ( ! is_object( $info ) || ! isset( $info->item ) || ! is_object( $info->item ) || empty( $info->item->version ) ) { continue; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@connectors/class-connector-installer.php` around lines 572 - 575, The loop over $update_results['core'] dereferences $info->item->version without ensuring $info->item exists or is an object; update the guard in the foreach ( $update_results['core'] as $info ) to verify $info is an object and that $info->item exists and is an object (or at least non-empty) before accessing ->version (e.g., check isset/empty and is_object on $info->item), and skip the iteration if those checks fail so $info->item->version is only accessed when safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@connectors/class-connector-installer.php`:
- Around line 731-737: The call to $this->log in _core_updated_successfully
currently omits the $forced_log parameter so auto core updates can be suppressed
by cron tracking; update the $this->log invocation inside
_core_updated_successfully to pass the $auto_updated boolean as the final
argument (forced_log) so when $auto_updated is true the event is always forced
to be logged, leaving the existing message, context
(compact('old_version','new_version','auto_updated')), level/null and category
('wordpress','updated') arguments unchanged.
- Around line 572-575: The loop over $update_results['core'] dereferences
$info->item->version without ensuring $info->item exists or is an object; update
the guard in the foreach ( $update_results['core'] as $info ) to verify $info is
an object and that $info->item exists and is an object (or at least non-empty)
before accessing ->version (e.g., check isset/empty and is_object on
$info->item), and skip the iteration if those checks fail so
$info->item->version is only accessed when safe.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9da87136-4e88-4334-8427-13df2c92130a
📒 Files selected for processing (1)
connectors/class-connector-installer.php
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
2 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.
Summary by CodeRabbit
Bug Fixes
Improvements