Add import export settings feature #313
Conversation
- Remove incorrect SebastianBergmann\Template\RuntimeException import that pulls in PHPUnit test dependencies - Refactor handle_export() to use export_settings() return value instead of duplicating payload construction logic - Maintain exact same filename format (Y-m-d) for backward compatibility
📝 WalkthroughWalkthroughThis change introduces import/export functionality to the settings management system. New methods handle exporting current settings as a JSON file and importing previously exported settings via validated file upload. The settings UI is extended with dedicated import/export controls and sections, including permission checks and post-import feedback. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AdminPage as Settings Admin Page
participant Handler as Export Handler
participant Settings as Settings Manager
participant File as File System
User->>AdminPage: Click "Export Settings" button
AdminPage->>Handler: handle_export() triggered
Handler->>Handler: Check permissions & nonce
Handler->>Settings: Retrieve current settings
Settings-->>Handler: Return settings data
Handler->>Handler: export_settings() - package with metadata<br/>(version, plugin, timestamp, URL, WP version)
Handler->>Handler: Generate filename
Handler->>File: Send JSON with download headers
File-->>User: Download JSON export file
sequenceDiagram
participant User
participant AdminPage as Settings Admin Page
participant Modal as Import Modal
participant Handler as Import Handler
participant FileValidation as File Validator
participant Settings as Settings Manager
participant Redirect as Redirect Handler
User->>Modal: Click "Import Settings" link
Modal-->>User: Display import modal with<br/>file input & confirmation toggle
User->>Modal: Select JSON file & confirm
Modal->>Modal: Client-side confirmation gate
User->>Modal: Submit import form
Modal->>Handler: handle_import_settings_modal()
Handler->>FileValidation: Validate file presence<br/>upload errors, .json extension
FileValidation-->>Handler: Validation result
Handler->>Handler: Read & parse JSON content
Handler->>FileValidation: Validate required structure
FileValidation-->>Handler: Structure valid
Handler->>Settings: Save imported settings
Settings-->>Handler: Settings persisted
Handler->>Redirect: Trigger import-export tab redirect
Redirect-->>User: Redirect with success notice
User-->>User: Display success message
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The review spans two files with new logic for file validation, JSON parsing, and permission checks, plus form registration and modal rendering. Mixed complexity with some straightforward configuration alongside validation logic that requires careful attention to security (nonce verification, file handling, JSON structure validation) and error handling patterns. 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: 1
🧹 Nitpick comments (3)
inc/admin-pages/class-settings-admin-page.php (3)
790-792: Consider providing more specific error messages for debugging.All error types result in the same generic message. Since this is an admin-only feature with proper capability checks, providing more specific error messages would improve the user experience and help administrators diagnose issues with their export files.
🔎 Proposed improvement for error messages
} catch ( Runtime_Exception $e ) { - wp_send_json_error(new \WP_Error($e->getMessage(), __('Something is wrong with the uploaded file.', 'ultimate-multisite'))); + $error_messages = [ + 'no_file' => __('No file was uploaded.', 'ultimate-multisite'), + 'upload_error' => __('File upload failed. Please try again.', 'ultimate-multisite'), + 'invalid_file_type' => __('Invalid file type. Please upload a JSON file.', 'ultimate-multisite'), + 'read_error' => __('Could not read the uploaded file.', 'ultimate-multisite'), + 'invalid_json' => __('The file contains invalid JSON.', 'ultimate-multisite'), + 'invalid_format' => __('The file is not a valid Ultimate Multisite export.', 'ultimate-multisite'), + 'invalid_structure' => __('The file structure is invalid or corrupted.', 'ultimate-multisite'), + ]; + $message = $error_messages[ $e->getMessage() ] ?? __('Something is wrong with the uploaded file.', 'ultimate-multisite'); + wp_send_json_error(new \WP_Error($e->getMessage(), $message)); }
762-767: Consider adding MIME type validation for defense in depth.The current validation only checks the file extension. While the JSON parsing will reject non-JSON content, adding MIME type validation provides an additional layer of defense. Also consider adding a file size limit to prevent potential DoS with very large files.
🔎 Proposed enhancement for file validation
// Check file extension $file_ext = strtolower(pathinfo($file['name'], PATHINFO_EXTENSION)); if ('json' !== $file_ext) { throw new Runtime_Exception('invalid_file_type'); } + // Check MIME type + $finfo = new \finfo(FILEINFO_MIME_TYPE); + $mime_type = $finfo->file($file['tmp_name']); + if ( ! in_array($mime_type, ['application/json', 'text/plain', 'text/json'], true)) { + throw new Runtime_Exception('invalid_file_type'); + } + + // Check file size (limit to 1MB) + if ($file['size'] > 1048576) { + throw new Runtime_Exception('file_too_large'); + } +
856-860: Sanitizecookie_domainfor safe filename usage.The
cookie_domainvalue is used directly in the filename. While typically a valid domain string, it should be sanitized to ensure it's safe for filesystem use (e.g., removing any characters that might be invalid in filenames on certain OS).🔎 Proposed fix to sanitize the filename
$filename = sprintf( 'ultimate-multisite-settings-export-%s-%s.json', gmdate('Y-m-d'), - get_current_site()->cookie_domain, + sanitize_file_name(get_current_site()->cookie_domain), );
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
inc/admin-pages/class-settings-admin-page.phpinc/class-settings.php
🧰 Additional context used
🧬 Code graph analysis (1)
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). (6)
- GitHub Check: PHP 7.4
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: PHP 8.0
- GitHub Check: PHP 8.1
- GitHub Check: PHP 8.2
🔇 Additional comments (10)
inc/class-settings.php (4)
1519-1532: LGTM!The new Import/Export section is well-structured and follows the established pattern for section registration. The order of 995 correctly positions it before "Other Options" (order 1000).
1558-1575: LGTM!The export button implementation is secure with proper nonce verification via
wp_nonce_url. The inline onclick redirect approach is appropriate for initiating a file download.
1589-1609: LGTM!The import button correctly uses
wu_get_form_urlto open a modal dialog, consistent with other modal interactions in the plugin. Thewuboxclass ensures proper modal behavior.
1611-1627: LGTM!The warning note appropriately emphasizes the destructive nature of the import operation with clear visual styling. The
do_action('wu_settings_import_export')hook follows the established pattern for section extensibility.inc/admin-pages/class-settings-admin-page.php (6)
12-12: LGTM!The
Runtime_Exceptionimport is correctly added to support error handling in the import flow.
616-629: LGTM!The
page_loaded()override correctly orchestrates the export/import handling. The order is appropriate: export handling must occur early (before any output), and the parent call at the end ensures normal page loading continues.
664-680: LGTM!The form registration follows the established pattern with proper capability check (
wu_edit_settings).
682-737: LGTM!The modal form is well-structured with a confirmation toggle that prevents accidental imports. The Vue.js bindings correctly disable the submit button until the user explicitly confirms understanding of the destructive action.
812-834: LGTM!The redirect handler appropriately displays a success message after import. The lack of nonce verification on the
updatedparameter is acceptable here since it only triggers a non-sensitive display message with no security implications.
654-662: No changes needed. Thewp_send_json($export_data, null, JSON_PRETTY_PRINT)call is correct. The WordPress function signature iswp_send_json( mixed $response, int $status_code = null, int $flags = 0 ), where the third parameter ($flags) accepts json_encode flags likeJSON_PRETTY_PRINT. The null for status code is appropriate when no specific HTTP status code needs to be set.
| // Validate structure | ||
| if ( ! isset($data['plugin']) || 'ultimate-multisite' !== $data['plugin']) { | ||
| throw new Runtime_Exception('invalid_format'); | ||
| } | ||
|
|
||
| if ( ! isset($data['settings']) || ! is_array($data['settings'])) { | ||
| throw new Runtime_Exception('invalid_structure'); | ||
| } | ||
| } catch ( Runtime_Exception $e ) { | ||
| wp_send_json_error(new \WP_Error($e->getMessage(), __('Something is wrong with the uploaded file.', 'ultimate-multisite'))); | ||
| } | ||
|
|
||
| WP_Ultimo()->settings->save_settings($data['settings']); |
There was a problem hiding this comment.
Imported settings should be validated against allowed fields.
The default_handler() method (lines 526-549) explicitly validates that only fields defined in sections are saved. However, handle_import_settings_modal() directly passes $data['settings'] to save_settings() without this validation. This inconsistency could allow importing settings that aren't registered, potentially causing unexpected behavior.
Consider extracting the field validation logic from default_handler() into a reusable method and applying it here:
🔎 Proposed fix to validate imported settings
if ( ! isset($data['settings']) || ! is_array($data['settings'])) {
throw new Runtime_Exception('invalid_structure');
}
} catch ( Runtime_Exception $e ) {
wp_send_json_error(new \WP_Error($e->getMessage(), __('Something is wrong with the uploaded file.', 'ultimate-multisite')));
}
+ // Validate imported settings against allowed fields (same as default_handler)
+ $sections = WP_Ultimo()->settings->get_sections();
+ $allowed_fields = [];
+ foreach ($sections as $section) {
+ if (isset($section['fields'])) {
+ $allowed_fields = array_merge($allowed_fields, array_keys($section['fields']));
+ }
+ }
+
+ $filtered_settings = array_intersect_key($data['settings'], array_flip($allowed_fields));
+
- WP_Ultimo()->settings->save_settings($data['settings']);
+ WP_Ultimo()->settings->save_settings($filtered_settings);
do_action('wu_settings_imported', $data['settings'], $data);📝 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.
| // Validate structure | |
| if ( ! isset($data['plugin']) || 'ultimate-multisite' !== $data['plugin']) { | |
| throw new Runtime_Exception('invalid_format'); | |
| } | |
| if ( ! isset($data['settings']) || ! is_array($data['settings'])) { | |
| throw new Runtime_Exception('invalid_structure'); | |
| } | |
| } catch ( Runtime_Exception $e ) { | |
| wp_send_json_error(new \WP_Error($e->getMessage(), __('Something is wrong with the uploaded file.', 'ultimate-multisite'))); | |
| } | |
| WP_Ultimo()->settings->save_settings($data['settings']); | |
| // Validate structure | |
| if ( ! isset($data['plugin']) || 'ultimate-multisite' !== $data['plugin']) { | |
| throw new Runtime_Exception('invalid_format'); | |
| } | |
| if ( ! isset($data['settings']) || ! is_array($data['settings'])) { | |
| throw new Runtime_Exception('invalid_structure'); | |
| } | |
| } catch ( Runtime_Exception $e ) { | |
| wp_send_json_error(new \WP_Error($e->getMessage(), __('Something is wrong with the uploaded file.', 'ultimate-multisite'))); | |
| } | |
| // Validate imported settings against allowed fields (same as default_handler) | |
| $sections = WP_Ultimo()->settings->get_sections(); | |
| $allowed_fields = []; | |
| foreach ($sections as $section) { | |
| if (isset($section['fields'])) { | |
| $allowed_fields = array_merge($allowed_fields, array_keys($section['fields'])); | |
| } | |
| } | |
| $filtered_settings = array_intersect_key($data['settings'], array_flip($allowed_fields)); | |
| WP_Ultimo()->settings->save_settings($filtered_settings); | |
| do_action('wu_settings_imported', $data['settings'], $data); |
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.