Add import export settings feature #310
Conversation
📝 WalkthroughWalkthroughA comprehensive export/import workflow is added to the Settings admin page, enabling administrators to export settings as JSON files and import them with validation. The implementation includes form registration, file upload handling, nonce-protected endpoints, and a settings UI section with export/import buttons. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin as Administrator
participant UI as Settings Page UI
participant Handler as Settings_Admin_Page
participant Files as File Upload
participant Settings as WP_Ultimo::settings
participant WordPress as WordPress Core
rect rgb(200, 220, 255)
note over Admin,WordPress: Export Flow
Admin->>UI: Clicks Export Settings
UI->>Handler: Hits handle_export via nonce URL
Handler->>WordPress: Verify admin capability
Handler->>Settings: Retrieve all settings
Handler->>Handler: Serialize to JSON with metadata
Handler->>WordPress: Set no-cache headers
Handler->>Admin: Return JSON download
end
rect rgb(220, 255, 200)
note over Admin,WordPress: Import Flow
Admin->>UI: Opens Import Settings modal
UI->>Admin: Display file upload + confirmation
Admin->>Files: Upload JSON file
Files->>Handler: Trigger handle_import_settings_modal
Handler->>Files: Validate file & parse JSON
Handler->>Handler: Validate structure (plugin, settings)
Handler->>Settings: Save imported settings
Handler->>WordPress: Fire wu_settings_imported action
Handler->>Admin: Return redirect URL
Admin->>UI: Handle redirect to import-export tab
Handler->>UI: Display success notice
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
inc/admin-pages/class-settings-admin-page.phpinc/class-settings.phpinc/helpers/class-settings-porter.php
🧰 Additional context used
🧬 Code graph analysis (2)
inc/helpers/class-settings-porter.php (3)
inc/functions/settings.php (1)
wu_get_all_settings(23-26)inc/functions/helper.php (1)
wu_clean(312-318)inc/class-settings.php (1)
save_settings(250-305)
inc/class-settings.php (1)
inc/functions/url.php (1)
wu_get_current_url(18-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: PHP 8.5
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (8)
inc/helpers/class-settings-porter.php (1)
65-130: LGTM - Comprehensive file validation.The validation logic properly checks upload errors, file type, size limits, JSON validity, and data structure. The use of
file_get_contents()with phpcs:ignore is appropriate for uploaded files.inc/class-settings.php (3)
1517-1530: LGTM - Well-structured section registration.The import/export section is properly registered with appropriate order (995) to appear near the end of settings, and includes a clear description for users.
1557-1573: Export button implementation is correct.The inline onclick handler that navigates to the nonce-protected export URL is an appropriate approach for triggering a file download. The
return false;prevents form submission, and the nonce protection viawp_nonce_url()ensures security.
1609-1623: Excellent use of visual warning for destructive action.The prominent red-bordered warning clearly communicates the irreversible nature of the import operation and recommends creating a backup first. This follows UX best practices for destructive actions.
inc/admin-pages/class-settings-admin-page.php (4)
621-628: LGTM - Proper orchestration of export/import flow.The
page_loaded()override correctly handles export requests, import redirects, and form registration before delegating to the parent implementation. The order of operations is logical.
684-733: Excellent modal UX with confirmation gate.The import modal properly gates the destructive action behind a confirmation toggle (lines 696-704), with the submit button disabled until confirmed (line 712). This prevents accidental imports and follows best practices for dangerous operations.
782-798: LGTM - Clean success notification.The redirect handler appropriately checks for the correct tab and updated parameter before displaying the success notice using the existing admin notice hook.
741-774: Nonce handling is correctly managed by the form framework.The
wu_register_form()system automatically handles nonce validation. TheForm_Manager::display_form()method adds the nonce field (line 130 in class-form-manager.php), andForm_Manager::handle_form()explicitly verifies it viawp_verify_nonce()at line 151 before invoking the handler. The handler receives control only after nonce verification succeeds, making the phpcs:ignore comments on lines 744 and 753 appropriate—the handler itself doesn't verify the nonce because the framework enforces it at a higher level.
| public static function export_settings() { | ||
|
|
||
| $settings = wu_get_all_settings(); | ||
|
|
||
| $export_data = [ | ||
| 'version' => '1.0.0', | ||
| 'plugin' => 'ultimate-multisite', | ||
| 'timestamp' => time(), | ||
| 'site_url' => get_site_url(), | ||
| 'wp_version' => get_bloginfo('version'), | ||
| 'settings' => $settings, | ||
| ]; | ||
|
|
||
| $filename = sprintf( | ||
| 'ultimate-multisite-settings-export-%s-%s.json', | ||
| gmdate('Y-m-d-His'), | ||
| get_current_site()->cookie_domain, | ||
| ); | ||
|
|
||
| return [ | ||
| 'success' => true, | ||
| 'data' => $export_data, | ||
| 'filename' => $filename, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Potential null reference issue with cookie_domain.
Line 47 accesses get_current_site()->cookie_domain without checking if get_current_site() returns null. In edge cases or certain network configurations, this could cause a fatal error.
🔎 Proposed fix
+ $current_site = get_current_site();
+ $domain = $current_site ? $current_site->cookie_domain : 'export';
+
$filename = sprintf(
'ultimate-multisite-settings-export-%s-%s.json',
gmdate('Y-m-d-His'),
- get_current_site()->cookie_domain,
+ $domain,
);🤖 Prompt for AI Agents
In inc/helpers/class-settings-porter.php around lines 31-55, the code calls
get_current_site()->cookie_domain directly which can be null and cause a fatal
error; fix by first assigning $current_site = get_current_site(); then derive a
safe $cookie_domain fallback (e.g. empty string or parse a fallback domain)
using a null/empty check before passing into sprintf so the filename generation
never dereferences a null object.
| public static function import_settings($data) { | ||
|
|
||
| $settings = $data['settings']; | ||
|
|
||
| // Sanitize settings before import | ||
|
|
||
| $sanitized_settings = array_map( | ||
| function ($value) { | ||
| return wu_clean($value); | ||
| }, | ||
| $settings | ||
| ); | ||
|
|
||
| // Use the Settings class save_settings method for proper validation | ||
| WP_Ultimo()->settings->save_settings($sanitized_settings); | ||
|
|
||
| do_action('wu_settings_imported', $sanitized_settings, $data); | ||
| } |
There was a problem hiding this comment.
Import may bypass field validation logic.
The import process sanitizes settings with wu_clean() then passes them directly to save_settings(). However, save_settings() (lines 250-305 in inc/class-settings.php) performs field-specific validation via the Field class, including type checking and value transformations. By passing already-sanitized raw values, the import may bypass these validations, potentially allowing invalid data or missing required transformations.
Consider one of these approaches:
- Have
save_settings()accept a flag to indicate import mode and handle validation accordingly - Pre-process imported settings through field validation before calling
save_settings() - Document that imports bypass validation and add a filter to allow custom validation hooks
| public static function send_download($json, $filename) { | ||
|
|
||
| nocache_headers(); | ||
|
|
||
| header('Content-Disposition: attachment; filename=' . $filename); | ||
| header('Content-Length: ' . strlen($json)); | ||
| header('Pragma: no-cache'); | ||
| header('Expires: 0'); | ||
| wp_send_json($json, null, JSON_PRETTY_PRINT); | ||
|
|
||
| exit; | ||
| } |
There was a problem hiding this comment.
Incorrect usage of wp_send_json() breaks JSON formatting.
Line 176 attempts to pass JSON_PRETTY_PRINT as a third parameter to wp_send_json(), but this function only accepts $response and $status_code parameters. The flag will be ignored, and the JSON output won't be pretty-printed. Additionally, wp_send_json() expects an array/object to encode, but receives already-encoded JSON data from export_settings().
🔎 Proposed fix
public static function send_download($json, $filename) {
nocache_headers();
+ header('Content-Type: application/json; charset=utf-8');
header('Content-Disposition: attachment; filename=' . $filename);
- header('Content-Length: ' . strlen($json));
header('Pragma: no-cache');
header('Expires: 0');
- wp_send_json($json, null, JSON_PRETTY_PRINT);
+
+ $json_output = wp_json_encode($json, JSON_PRETTY_PRINT);
+ header('Content-Length: ' . strlen($json_output));
+
+ echo $json_output; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
exit;
}🤖 Prompt for AI Agents
In inc/helpers/class-settings-porter.php around lines 168 to 179, the function
misuses wp_send_json() by passing a JSON string and a third parameter for
JSON_PRETTY_PRINT; wp_send_json() only accepts response and status and expects
an array/object. Replace the wp_send_json() call with sending the
already-encoded JSON directly: set the Content-Type header to application/json;
charset=utf-8, remove the JSON_PRETTY_PRINT parameter, echo the $json string (or
decode and re-encode with wp_json_encode($data, JSON_PRETTY_PRINT) if you really
need pretty output), then exit.
🔨 Build Complete - Ready for Testing!📦 Download Build Artifact (Recommended)Download the zip build, upload to WordPress and test:
🌐 Test in WordPress Playground (Very Experimental)Click the link below to instantly test this PR in your browser - no installation needed! Login credentials: |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
inc/admin-pages/class-settings-admin-page.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/admin-pages/class-settings-admin-page.php (2)
inc/exception/class-runtime-exception.php (1)
Runtime_Exception(19-19)inc/admin-pages/class-base-admin-page.php (2)
page_loaded(697-697)register_forms(738-738)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
🔇 Additional comments (6)
inc/admin-pages/class-settings-admin-page.php (6)
617-630: LGTM!The orchestration of export handling, import redirect, and form registration before delegating to the parent is well-structured.
677-693: LGTM!The form registration follows the framework pattern correctly with appropriate render callback, handler, and capability check.
695-750: LGTM!The import modal implementation is well-structured with proper file upload handling, a confirmation toggle for safety, and reactive form state management.
825-847: LGTM!The import redirect handler correctly displays a success notice using WordPress standard markup.
849-880: Note: Return value not utilized.This private method is well-structured and returns the export data correctly. However, as noted in the earlier comment on
handle_export(), the return value is currently ignored, leading to code duplication. Once the refactor is applied, this method will be properly utilized.
752-823: Settings validation is already performed by the Field class.The imported settings values are sanitized before being saved. The
save_settings()method creates Field objects for each defined setting and callsset_value(), which triggers field-specific sanitization via thesanitize()method. Additionally, only settings that are defined in the configuration sections are processed; unknown fields from the imported JSON are automatically discarded, which further prevents injection of malicious data.
|
|
||
| namespace WP_Ultimo\Admin_Pages; | ||
|
|
||
| use SebastianBergmann\Template\RuntimeException; |
There was a problem hiding this comment.
Remove unused and incorrect import.
Line 12 imports SebastianBergmann\Template\RuntimeException, which is from the PHPUnit test library and should not be used in production code. This import is unused throughout the file—the code correctly uses WP_Ultimo\Exception\Runtime_Exception from line 13.
🔎 Proposed fix
-use SebastianBergmann\Template\RuntimeException;
use WP_Ultimo\Exception\Runtime_Exception;📝 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.
| use SebastianBergmann\Template\RuntimeException; | |
| use WP_Ultimo\Exception\Runtime_Exception; |
🤖 Prompt for AI Agents
In inc/admin-pages/class-settings-admin-page.php around line 12, remove the
incorrect and unused import "use SebastianBergmann\Template\RuntimeException;" —
delete that line so the file relies on the correct
WP_Ultimo\Exception\Runtime_Exception import on the following line and avoid
pulling PHPUnit test libs into production code.
| protected function handle_export() { | ||
|
|
||
| if ( ! isset($_GET['wu_export_settings'])) { | ||
| return; | ||
| } | ||
| check_admin_referer('wu_export_settings'); | ||
|
|
||
| // Check permissions | ||
| if ( ! current_user_can('wu_edit_settings')) { | ||
| wp_die(esc_html__('You do not have permission to export settings.', 'ultimate-multisite')); | ||
| } | ||
|
|
||
| $this->export_settings(); | ||
| $settings = wu_get_all_settings(); | ||
|
|
||
| $export_data = [ | ||
| 'version' => \WP_Ultimo::VERSION, | ||
| 'plugin' => 'ultimate-multisite', | ||
| 'timestamp' => time(), | ||
| 'site_url' => get_site_url(), | ||
| 'wp_version' => get_bloginfo('version'), | ||
| 'settings' => $settings, | ||
| ]; | ||
|
|
||
| $filename = sprintf( | ||
| 'ultimate-multisite-settings-export-%s-%s.json', | ||
| gmdate('Y-m-d'), | ||
| get_current_site()->cookie_domain, | ||
| ); | ||
| nocache_headers(); | ||
|
|
||
| header('Content-Disposition: attachment; filename=' . $filename); | ||
| header('Pragma: no-cache'); | ||
| header('Expires: 0'); | ||
| wp_send_json($export_data, null, JSON_PRETTY_PRINT); | ||
|
|
||
| exit; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Eliminate code duplication with export_settings() method.
Line 650 calls $this->export_settings() but ignores the return value, then lines 651–660 duplicate the exact same logic found in the export_settings() method (lines 858–867). This duplication creates a maintenance burden.
🔎 Proposed fix
Refactor to use the return value from export_settings():
- $this->export_settings();
- $settings = wu_get_all_settings();
-
- $export_data = [
- 'version' => \WP_Ultimo::VERSION,
- 'plugin' => 'ultimate-multisite',
- 'timestamp' => time(),
- 'site_url' => get_site_url(),
- 'wp_version' => get_bloginfo('version'),
- 'settings' => $settings,
- ];
-
- $filename = sprintf(
- 'ultimate-multisite-settings-export-%s-%s.json',
- gmdate('Y-m-d'),
- get_current_site()->cookie_domain,
- );
+ $result = $this->export_settings();
+ $export_data = $result['data'];
+ $filename = $result['filename'];
+
nocache_headers();
header('Content-Disposition: attachment; filename=' . $filename);Note: You may also want to update the timestamp format in export_settings() (line 871) from 'Y-m-d-His' to 'Y-m-d' to match the original format, or keep the more precise timestamp.
📝 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.
| protected function handle_export() { | |
| if ( ! isset($_GET['wu_export_settings'])) { | |
| return; | |
| } | |
| check_admin_referer('wu_export_settings'); | |
| // Check permissions | |
| if ( ! current_user_can('wu_edit_settings')) { | |
| wp_die(esc_html__('You do not have permission to export settings.', 'ultimate-multisite')); | |
| } | |
| $this->export_settings(); | |
| $settings = wu_get_all_settings(); | |
| $export_data = [ | |
| 'version' => \WP_Ultimo::VERSION, | |
| 'plugin' => 'ultimate-multisite', | |
| 'timestamp' => time(), | |
| 'site_url' => get_site_url(), | |
| 'wp_version' => get_bloginfo('version'), | |
| 'settings' => $settings, | |
| ]; | |
| $filename = sprintf( | |
| 'ultimate-multisite-settings-export-%s-%s.json', | |
| gmdate('Y-m-d'), | |
| get_current_site()->cookie_domain, | |
| ); | |
| nocache_headers(); | |
| header('Content-Disposition: attachment; filename=' . $filename); | |
| header('Pragma: no-cache'); | |
| header('Expires: 0'); | |
| wp_send_json($export_data, null, JSON_PRETTY_PRINT); | |
| exit; | |
| } | |
| protected function handle_export() { | |
| if ( ! isset($_GET['wu_export_settings'])) { | |
| return; | |
| } | |
| check_admin_referer('wu_export_settings'); | |
| // Check permissions | |
| if ( ! current_user_can('wu_edit_settings')) { | |
| wp_die(esc_html__('You do not have permission to export settings.', 'ultimate-multisite')); | |
| } | |
| $result = $this->export_settings(); | |
| $export_data = $result['data']; | |
| $filename = $result['filename']; | |
| nocache_headers(); | |
| header('Content-Disposition: attachment; filename=' . $filename); | |
| header('Pragma: no-cache'); | |
| header('Expires: 0'); | |
| wp_send_json($export_data, null, JSON_PRETTY_PRINT); | |
| exit; | |
| } |
🤖 Prompt for AI Agents
In inc/admin-pages/class-settings-admin-page.php around lines 638 to 675, the
handler calls $this->export_settings() then rebuilds the same export payload
inline, duplicating logic from export_settings(); instead, change
handle_export() to capture and use the return value from
$this->export_settings() (e.g. $export_data = $this->export_settings();) and
remove the duplicated payload construction so the single export_settings()
implementation supplies the data, then keep the existing
headers/filename/response logic (ensure filename generation and timestamp format
remain correct or adjust export_settings() timestamp to 'Y-m-d' if you want
exact parity), and call wp_send_json($export_data, null, JSON_PRETTY_PRINT)
before exit.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.