Improve auto update functionality with suggestions from coderabbit#69
Conversation
WalkthroughThe changes to 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
📝 Coding Plan
Comment Tip CodeRabbit can use your project's PHP CodeSniffer (phpcs) configuration to improve the quality of PHP code reviews.Add a |
gin0115
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugin.php (1)
124-130: Make ZIP asset selection deterministic by asset name, not only URL suffix.Line 126 picks the first URL ending in
.zip. If a release includes multiple zip assets, this can still select the wrong package. Prefer matching expected assetname(e.g., plugin slug/version pattern) first, then fallback.♻️ Suggested refactor
$package_url = ''; foreach ( $latest_release_info['assets'] as $asset ) { - if ( ! empty( $asset['browser_download_url'] ) && str_ends_with( $asset['browser_download_url'], '.zip' ) ) { - $package_url = $asset['browser_download_url']; + $asset_name = isset( $asset['name'] ) ? (string) $asset['name'] : ''; + $asset_url = isset( $asset['browser_download_url'] ) ? (string) $asset['browser_download_url'] : ''; + + if ( + '' !== $asset_url && + '' !== $asset_name && + preg_match( '/^simple-events.*\.zip$/i', $asset_name ) + ) { + $package_url = $asset_url; break; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin.php` around lines 124 - 130, The current loop that sets $package_url by checking asset['browser_download_url'] suffix can pick the wrong ZIP when multiple zips exist; update the logic in the loop over $latest_release_info['assets'] to first look for an asset whose asset['name'] matches the expected plugin slug/version pattern (e.g., "your-plugin-slug-<version>.zip") and set $package_url to that asset['browser_download_url'], and only if no name match is found fall back to the existing check for assets whose asset['browser_download_url'] ends with '.zip'; keep the variable $package_url and the same break behavior once a match is found.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin.php`:
- Around line 102-104: The code currently returns $update immediately when
is_wp_error($response) or non-200 from
wp_remote_retrieve_response_code($response) occurs, causing repeated GitHub
hits; instead, record a transient failure state (e.g., set_transient with a
brief TTL like 5–15 minutes) when that error branch is taken so subsequent
update checks short-circuit without re-querying GitHub. Modify the error branch
that checks is_wp_error($response) || 200 !==
wp_remote_retrieve_response_code($response) to set a transient (unique key
related to the plugin/update) describing the failure before returning $update,
and ensure other code paths consult that transient to skip remote requests while
it exists.
---
Nitpick comments:
In `@plugin.php`:
- Around line 124-130: The current loop that sets $package_url by checking
asset['browser_download_url'] suffix can pick the wrong ZIP when multiple zips
exist; update the logic in the loop over $latest_release_info['assets'] to first
look for an asset whose asset['name'] matches the expected plugin slug/version
pattern (e.g., "your-plugin-slug-<version>.zip") and set $package_url to that
asset['browser_download_url'], and only if no name match is found fall back to
the existing check for assets whose asset['browser_download_url'] ends with
'.zip'; keep the variable $package_url and the same break behavior once a match
is found.
| if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { | ||
| return $update; | ||
| } |
There was a problem hiding this comment.
Cache failure states briefly to avoid repeated GitHub retries during outages.
Line 102 returns immediately on error, but no transient is written. That means every subsequent update check re-hits GitHub until one succeeds, which can amplify latency/rate-limit issues.
💡 Suggested fix
- if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) {
- return $update;
- }
+ if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) {
+ // Cache negative result briefly to avoid hammering GitHub on repeated checks.
+ set_site_transient( 'se_latest_release_info', '', 5 * MINUTE_IN_SECONDS );
+ return $update;
+ }📝 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 ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { | |
| return $update; | |
| } | |
| if ( is_wp_error( $response ) || 200 !== wp_remote_retrieve_response_code( $response ) ) { | |
| // Cache negative result briefly to avoid hammering GitHub on repeated checks. | |
| set_site_transient( 'se_latest_release_info', '', 5 * MINUTE_IN_SECONDS ); | |
| return $update; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin.php` around lines 102 - 104, The code currently returns $update
immediately when is_wp_error($response) or non-200 from
wp_remote_retrieve_response_code($response) occurs, causing repeated GitHub
hits; instead, record a transient failure state (e.g., set_transient with a
brief TTL like 5–15 minutes) when that error branch is taken so subsequent
update checks short-circuit without re-querying GitHub. Modify the error branch
that checks is_wp_error($response) || 200 !==
wp_remote_retrieve_response_code($response) to set a transient (unique key
related to the plugin/update) describing the failure before returning $update,
and ensure other code paths consult that transient to skip remote requests while
it exists.
1 participant
Changes proposed in this Pull Request
This pull request improves the plugin update mechanism by adding caching and more robust handling of GitHub release data. The main focus is on reducing unnecessary API calls and ensuring the correct release asset is used for updates.
Performance and reliability improvements:
se_latest_release_info) to reduce API requests and improve performance.Update asset selection:
.zipasset from the release's assets array, instead of assuming the first asset is always the correct one.Testing instructions
Mentions #
Summary by CodeRabbit
Bug Fixes
Performance