Improve logic for checking valid exporters #414
Conversation
| @@ -53,7 +53,7 @@ describe( 'DefaultExporter', () => { | |||
| { path: '/path/to/site/wp-content/plugins/plugin1', name: 'plugin1.php', isFile: () => true }, | |||
| { path: '/path/to/site/wp-content/themes/theme1', name: 'index.php', isFile: () => true }, | |||
| { path: '/path/to/site/wp-includes/index.php', name: 'index.php', isFile: () => true }, | |||
| { path: '/path/to/site/wp-load.php', name: 'wp-load.php', isFile: () => true }, | |||
| { path: '/path/to/site', name: 'wp-load.php', isFile: () => true }, | |||
There was a problem hiding this comment.
We were getting false positives in the export unit tests due to the previous logic of canHandle.
| onEvent( { event: ExportEvents.EXPORT_ERROR, data: null } ); | ||
| throw new Error( 'No suitable exporter found for the site' ); |
There was a problem hiding this comment.
I noticed that we weren't raising an error when no exporter are found.
There was a problem hiding this comment.
Thanks for adding this, I had a similar concern earlier in #391 (comment) 👍
|
@fluiddot it works as expected. I'm wondering what the best behavior for the exporter would be. Export gracefully, so if any path is missing, Studio still produces the backup file? Or fail early and do not allow to product export files that miss some files or directories? |
I understand that the files we check are required so I'd lean toward fail early and mark the export as failed. However, we can re-evaluate this logic and consider some of the paths optional. |
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Related to 8409-gh-Automattic/dotcom-forge.
Proposed Changes
wp-config.php).Testing Instructions
STUDIO_IMPORT_EXPORT=true npm start.wp-config.php.Pre-merge Checklist