-
Notifications
You must be signed in to change notification settings - Fork 48
Studio: Make the import process more verbose #1734
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: trunk
Are you sure you want to change the base?
Studio: Make the import process more verbose #1734
Conversation
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.
Pull Request Overview
This PR enhances the import process with verbose progress tracking to provide more detailed feedback during backup extraction and import operations. The changes make the import process more transparent by showing file counts, content types, and granular progress updates.
Key changes:
- Added granular progress tracking with file counts for extraction, database import, and WordPress content import
- Implemented content categorization (plugins, themes, uploads, other) with type-specific progress messages
- Enhanced backup handlers to emit detailed progress events with file counts and current file information
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/lib/import-export/import/types.ts | Added new interface types for detailed progress event data |
src/lib/import-export/import/events.ts | Added new progress event constants for granular tracking |
src/lib/import-export/import/importers/importer.ts | Implemented content categorization and progress events for database and WordPress content import |
src/lib/import-export/import/handlers/backup-handler-zip.ts | Enhanced ZIP handler with file-level progress tracking |
src/lib/import-export/import/handlers/backup-handler-tar-gz.ts | Enhanced tar.gz handler with file-level progress tracking |
src/hooks/use-import-export.tsx | Updated UI progress handling to display detailed progress messages with file counts |
src/lib/import-export/tests/import/importer/jetpack-importer.test.ts | Added comprehensive tests for WordPress content categorization |
src/lib/import-export/tests/import/handlers/backup-handler.test.ts | Updated test expectations for tar.gz handler options |
src/hooks/tests/use-import-export.test.tsx | Added extensive tests for verbose progress tracking scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
// Get total file count first | ||
const fileList = await this.listFiles( file ); | ||
const totalFiles = fileList.length; |
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.
This implementation requires reading the entire tar.gz file twice - once to count files and again to extract them. This could significantly impact performance for large backup files. Consider counting files during extraction or using a streaming approach to avoid the double read.
Copilot uses AI. Check for mistakes.
.pipe( | ||
tar.extract( { | ||
cwd: extractionDirectory, | ||
onwarn: ( _code, message ) => console.warn( message ), |
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.
Direct console.warn usage bypasses the application's logging system. Consider using a proper logger or emitting a warning event that can be handled consistently with other error reporting in the application.
onwarn: ( _code, message ) => console.warn( message ), | |
onwarn: ( code, message ) => { | |
this.emit(ImportEvents.BACKUP_EXTRACT_WARNING, { code, message }); | |
}, |
Copilot uses AI. Check for mistakes.
for ( const [ type, files ] of Object.entries( filesByType ) ) { | ||
for ( const file of files ) { |
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.
The nested loop structure makes progress tracking complex and could lead to inaccurate progress calculations. The outer loop iterates by type but progress is tracked globally across all files. Consider flattening this to a single loop or adjusting progress calculation to account for the nested structure.
Copilot uses AI. Check for mistakes.
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 think tracking the progress this way makes sense, because we categorized the files into these types in the first place.
if ( file.includes( '/plugins/' ) || file.includes( '\\plugins\\' ) ) { | ||
categorized.plugins.push( file ); | ||
} else if ( file.includes( '/themes/' ) || file.includes( '\\themes\\' ) ) { | ||
categorized.themes.push( file ); | ||
} else if ( file.includes( '/uploads/' ) || file.includes( '\\uploads\\' ) ) { | ||
categorized.uploads.push( file ); |
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.
The string-based path categorization is fragile and could misclassify files. For example, a file named 'plugins-backup.txt' would be categorized as a plugin. Consider using path.sep and more robust path parsing with path.normalize() or regular expressions to match directory boundaries.
Copilot uses AI. Check for mistakes.
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.
This looks good and IMO the added progress steps make the import process much more "interactive" from a UX perspective, it's great to see what's happening under the hood.
To test the changes, I configured a site on playground.wordpress.net with fake data and plugins, and exported it both as .wpress and .zip.
When importing the .wpress I ran into an SQL parse error at the end of the import process. I could recreate this error on trunk, so I don't think this is related to these changes:
CleanShot.2025-09-10.at.12.00.12.mp4
Importing the .zip file worked well without errors:
CleanShot.2025-09-10.at.11.27.37.mp4
for ( const [ type, files ] of Object.entries( filesByType ) ) { | ||
for ( const file of files ) { |
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 think tracking the progress this way makes sense, because we categorized the files into these types in the first place.
Related issues
Proposed Changes
Testing Instructions
Pre-merge Checklist