-
-
Notifications
You must be signed in to change notification settings - Fork 68
Add import export settings feature #313
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
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
📝 WalkthroughWalkthroughAdds end-to-end settings import/export: UI sections and controls, export endpoint generating a versioned JSON download, an import modal and multipart form, server-side JSON validation/filtering and save, and post-import redirect/notice handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant UI as Settings UI
participant AdminPage as Settings Admin Page
participant Exporter as Export Handler
participant Settings as Settings Manager
participant Browser as Browser/File
User->>UI: Click "Export Settings"
UI->>AdminPage: Request export (nonce)
AdminPage->>Exporter: handle_export()
Exporter->>Exporter: verify nonce & permissions
Exporter->>Settings: fetch current settings
Settings-->>Exporter: settings payload
Exporter->>Exporter: export_settings() adds metadata, filename
Exporter->>Browser: send JSON with download headers
Browser-->>User: prompt file download
sequenceDiagram
autonumber
participant User
participant UI as Settings UI
participant Modal as Import Modal
participant AdminPage as Settings Admin Page
participant Validator as File/JSON Validator
participant Settings as Settings Manager
participant Redirect as Redirect Handler
User->>UI: Click "Import Settings"
UI->>Modal: open import modal (file input + confirm)
User->>Modal: attach JSON file and confirm
User->>Modal: submit form
Modal->>AdminPage: handle_import_settings_modal()
AdminPage->>Validator: validate nonce, file presence, extension, upload errors
Validator-->>AdminPage: validation result
AdminPage->>AdminPage: parse JSON, validate structure, filter allowed keys
AdminPage->>Settings: persist imported settings
Settings-->>AdminPage: success
AdminPage->>Redirect: handle_import_redirect() (redirect with success notice)
Redirect-->>User: show success message on settings tab
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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: |
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.
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.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
🔨 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
inc/admin-pages/class-settings-admin-page.php (2)
654-659: Consider quoting the filename in Content-Disposition header.If
cookie_domaincontains spaces or special characters, the filename should be properly quoted to ensure correct browser handling.🔎 Proposed fix
- header('Content-Disposition: attachment; filename=' . $filename); + header('Content-Disposition: attachment; filename="' . $filename . '"');
869-873: Consider sanitizing the cookie_domain for safe filename usage.
get_current_site()->cookie_domaincould contain characters that are problematic for filenames or HTTP headers (e.g., colons in certain edge cases).🔎 Proposed fix
$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 (1)
inc/admin-pages/class-settings-admin-page.php
🧰 Additional context used
🧬 Code graph analysis (1)
inc/admin-pages/class-settings-admin-page.php (3)
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)inc/functions/settings.php (1)
wu_get_all_settings(23-26)
🪛 GitHub Check: Code Quality Checks
inc/admin-pages/class-settings-admin-page.php
[failure] 792-792:
Line indented incorrectly; expected 2 tabs, found 1
[failure] 791-791:
Line indented incorrectly; expected at least 3 tabs, found 2
[failure] 790-790:
Line indented incorrectly; expected 2 tabs, found 1
[failure] 789-789:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 788-788:
Line indented incorrectly; expected at least 4 tabs, found 3
[failure] 787-787:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 785-785:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 784-784:
Line indented incorrectly; expected at least 4 tabs, found 3
[failure] 783-783:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 782-782:
Line indented incorrectly; expected at least 3 tabs, found 2
⏰ 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). (3)
- GitHub Check: cypress (8.1, chrome)
- GitHub Check: cypress (8.2, chrome)
- GitHub Check: Build Plugin for Testing
🔇 Additional comments (5)
inc/admin-pages/class-settings-admin-page.php (5)
12-12: LGTM!The
Runtime_Exceptionimport is appropriate for the new import validation error handling.
616-629: LGTM!The
page_loaded()override correctly orchestrates export/import flows before delegating to the parent implementation.
664-680: LGTM!Form registration is clean with appropriate capability enforcement matching other permission checks in the class.
682-737: LGTM!The modal provides good UX with client-side JSON filtering, confirmation toggle, and proper multipart encoding for file uploads.
825-847: LGTM!The redirect handler correctly displays a success notice after import without performing any state-changing operations.
| // 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)); |
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.
Fix inconsistent indentation in validation and filtering logic.
Lines 782-803 have mixed indentation (1-2 tabs instead of the expected 2-3 tabs), breaking code style consistency. The static analysis tool flagged this as well.
🔎 Proposed fix for indentation
- // Validate structure
- if ( ! isset($data['plugin']) || 'ultimate-multisite' !== $data['plugin']) {
- throw new Runtime_Exception('invalid_format');
- }
+ // 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')));
- }
+ 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']));
+ // 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));
+ $filtered_settings = array_intersect_key($data['settings'], array_flip($allowed_fields));
- WP_Ultimo()->settings->save_settings($filtered_settings);
+ WP_Ultimo()->settings->save_settings($filtered_settings);📝 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'))); | |
| } | |
| // 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)); | |
| // 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)); |
🧰 Tools
🪛 GitHub Check: Code Quality Checks
[failure] 792-792:
Line indented incorrectly; expected 2 tabs, found 1
[failure] 791-791:
Line indented incorrectly; expected at least 3 tabs, found 2
[failure] 790-790:
Line indented incorrectly; expected 2 tabs, found 1
[failure] 789-789:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 788-788:
Line indented incorrectly; expected at least 4 tabs, found 3
[failure] 787-787:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 785-785:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 784-784:
Line indented incorrectly; expected at least 4 tabs, found 3
[failure] 783-783:
Line indented incorrectly; expected 3 tabs, found 2
[failure] 782-782:
Line indented incorrectly; expected at least 3 tabs, found 2
🤖 Prompt for AI Agents
inc/admin-pages/class-settings-admin-page.php lines 782-803: this block uses
mixed indentation (1-2 tabs) which breaks project style; reformat the entire
try/catch and subsequent validation/filtering lines to use the same tab depth as
surrounding code (use the file's standard 2-3 tabs for nested blocks), ensuring
consistent indentation for the if statements, throw calls, catch block, and the
foreach and array operations; run the repo linter/PHPCS after changes to confirm
style compliance.
| WP_Ultimo()->settings->save_settings($filtered_settings); | ||
|
|
||
| do_action('wu_settings_imported', $data['settings'], $data); | ||
|
|
||
| do_action('wu_settings_imported', $data['settings'], $data); |
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.
Critical: Duplicate action hook call.
do_action('wu_settings_imported', ...) is called twice (lines 807 and 809), which will trigger all registered callbacks twice.
🔎 Proposed fix - remove duplicate line
$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);
-
do_action('wu_settings_imported', $data['settings'], $data);
// Success📝 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.
| WP_Ultimo()->settings->save_settings($filtered_settings); | |
| do_action('wu_settings_imported', $data['settings'], $data); | |
| do_action('wu_settings_imported', $data['settings'], $data); | |
| WP_Ultimo()->settings->save_settings($filtered_settings); | |
| do_action('wu_settings_imported', $data['settings'], $data); |
🤖 Prompt for AI Agents
In inc/admin-pages/class-settings-admin-page.php around lines 805 to 809, there
is a duplicate do_action('wu_settings_imported', $data['settings'], $data) call
which will fire registered callbacks twice; remove the redundant second
do_action so the hook is invoked only once (keep the single call immediately
after WP_Ultimo()->settings->save_settings($filtered_settings)) and ensure
spacing/indentation is consistent.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.