-
Notifications
You must be signed in to change notification settings - Fork 108
feat: Implement version switching functionality in plugin settings #251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: core
Are you sure you want to change the base?
Conversation
I followed the testing steps, but at step 4 the version didn’t switch. The version I selected was downloaded, and I received positive feedback confirming the switch. However, the new version did not replace the previous one — it was only downloaded, that plugin version was listed in the plugins but was disabled, keeping the previous version activated. |
Download and install |
@CarolinaOP I have now adjusted our build action workflow and this shouldnt be an issue. please proceed with testing further :) |
Review completed — everything is working well overall, though the error messages could be refined. |
.notice { | ||
&.notice-success { | ||
border-left-color: #00a32a; | ||
} | ||
|
||
&.notice-error { | ||
border-left-color: #d63638; | ||
} | ||
|
||
&.notice-warning { | ||
border-left-color: #dba617; | ||
} | ||
|
||
&.notice-info { | ||
border-left-color: #72aee6; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a big deal, but some of the classes can be more nested.
.notice {
&.notice {
&-success{}
&-error{}
}
}
const VERSION_CACHE_DURATION = HOUR_IN_SECONDS; | ||
const PROGRESS_TIMEOUT = 5 * MINUTE_IN_SECONDS; | ||
const WORDPRESS_API_ENDPOINT = 'https://api.wordpress.org/plugins/info/1.2/?action=plugin_information&slug=code-snippets'; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we perhaps move all the code in this file to a class?
/** | ||
* Check if a version switch is in progress | ||
* | ||
* @return bool True if switch is in progress | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove all these comments. For example is_version_switch_in_progress()
is readable enough, and the function signature already declares the return type as bool
.
// Check if already on target version | ||
if ( get_current_version() === $target_version ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Check if already on target version | |
if ( get_current_version() === $target_version ) { | |
$is_already_on_target_version = get_current_version() === $target_version; | |
if ( $is_already_on_target_version ) { |
Please remove all the redundant comments from the file.
<script type="text/javascript"> | ||
jQuery(document).ready(function($) { | ||
var currentVersion = '<?php echo esc_js( $current_version ); ?>'; | ||
var $button = $('#switch-version-btn'); | ||
var $dropdown = $('#target_version'); | ||
var $result = $('#version-switch-result'); | ||
|
||
// Handle dropdown changes - enable/disable button and show/hide warning | ||
$dropdown.on('change', function() { | ||
var selectedVersion = $(this).val(); | ||
|
||
if (!selectedVersion || selectedVersion === currentVersion) { | ||
// Current version or no selection - disable button and hide warning | ||
$button.prop('disabled', true); | ||
$('#version-switch-warning').hide(); | ||
} else { | ||
// Different version selected - enable button and show warning | ||
$button.prop('disabled', false); | ||
$('#version-switch-warning').show(); | ||
} | ||
}); | ||
|
||
$button.on('click', function() { | ||
var targetVersion = $dropdown.val(); | ||
|
||
if (!targetVersion || targetVersion === currentVersion) { | ||
$result.removeClass('notice-success notice-error').addClass('notice-warning') | ||
.html('<p><?php esc_html_e( 'Please select a different version to switch to.', 'code-snippets' ); ?></p>') | ||
.show(); | ||
return; | ||
} | ||
|
||
// Disable button and show loading | ||
$button.prop('disabled', true).text('<?php esc_html_e( 'Switching...', 'code-snippets' ); ?>'); | ||
$result.removeClass('notice-success notice-error notice-warning').addClass('notice-info') | ||
.html('<p><?php esc_html_e( 'Processing version switch. Please wait...', 'code-snippets' ); ?></p>') | ||
.show(); | ||
|
||
// Make AJAX request | ||
$.post(ajaxurl, { | ||
action: 'code_snippets_switch_version', | ||
target_version: targetVersion, | ||
nonce: '<?php echo esc_js( wp_create_nonce( 'code_snippets_version_switch' ) ); ?>' | ||
}) | ||
.done(function(response) { | ||
if (response.success) { | ||
$result.removeClass('notice-info notice-error').addClass('notice-success') | ||
.html('<p>' + response.data.message + '</p>'); | ||
|
||
// Refresh page after 3 seconds | ||
setTimeout(function() { | ||
window.location.reload(); | ||
}, 3000); | ||
} else { | ||
$result.removeClass('notice-info notice-success').addClass('notice-error') | ||
.html('<p>' + response.data.message + '</p>'); | ||
$button.prop('disabled', false).text('<?php esc_html_e( 'Switch Version', 'code-snippets' ); ?>'); | ||
} | ||
}) | ||
.fail(function() { | ||
$result.removeClass('notice-info notice-success').addClass('notice-error') | ||
.html('<p><?php esc_html_e( 'An error occurred while switching versions. Please try again.', 'code-snippets' ); ?></p>'); | ||
$button.prop('disabled', false).text('<?php esc_html_e( 'Switch Version', 'code-snippets' ); ?>'); | ||
}); | ||
}); | ||
}); | ||
</script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Can we rather add this script to a separate JS file and enqueue it?
- Do we have to use jQuery?
- Use
let/const
instead ofvar
🔄 Version Switching Functionality Implementation
This PR introduces comprehensive version switching functionality in the Code Snippets plugin settings, enabling users to safely switch between different plugin versions directly from the WordPress admin interface.
✨ Features Implemented
🎛️ New Settings Section
Settings > Snippets Settings > Version
)🔄 Version Switching Interface
🚀 Performance & Caching
🔐 Security & Safety
update_plugins
)🎨 User Experience
🧪 Testing Steps
📋 Prerequisites
1️⃣ Settings Access
2️⃣ Version Selection Flow
3️⃣ Cache Refresh Mechanism
4️⃣ Successful Version Switch
5️⃣ Error Handling
6️⃣ Concurrent Operation Prevention
7️⃣ Input Validation
# Test Steps: 1. Use dev tools to modify dropdown values 2. Submit invalid version 3. Verify rejection Expected: Server-side validation rejects invalid versions with error message.
8️⃣ CSRF Protection
📊 Expected Results
✅ Success Indicators
🔍 Code Review Focus Areas
📁 File Changes
src/php/settings/version-switch.php
: Complete version switching logicsrc/css/settings.scss
: UI styling and responsive design