Dt Import CSV #2694
Dt Import CSV #2694
Conversation
…ple address formats, including DMS coordinates and semicolon-separated locations. Update related functions for preview mode and geocoding handling.
…id configurations
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like
Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
|
@OpenHands please fix the failing actions on PR #2694 |
|
Uh oh! There was an unexpected error starting the job :( |
- Removed unnecessary check for customizable fields in field options filtering. - Implemented total error counting for rows and added an error summary section in the import preview. - Enhanced row display to show specific errors for each record, improving user feedback during the import process.
- Implemented logic to use sampling for estimating counts when processing large CSV files (over 1000 rows) to improve performance. - Added indicators in the import preview to inform users when counts are estimated and the sample size used. - Maintained accurate counts for smaller datasets by analyzing all rows. - Updated date formatting to use gmdate for consistency.
…sor, as API will now handle all validation.
|
Fix progress numbers on mobile. |
|
Thanks for the review @squigglybob, i think i've addressed the UI fixes |
|
No worries, your location field types commit may have fixed this other thing, but I was testing the documentation, by putting in all those different location types, and the lng,lat, and degrees, and grid_id ones all worked fine. The address ones didn't work, but I think that may be because I don't have a mapbox key locally |
|
@corsacca this is Excellent! Both the flow & new look & feel are great improvements! Good work! Both Claude and I have now had a chance to review code and our findings (Claude's mostly ;-) captured below, across a number of comment blocks.
|
🎯 Overall AssessmentThis is a well-architected implementation with comprehensive functionality and good security practices. The code demonstrates excellent feature parity with the existing plugin and includes several improvements. Ready for production with a few critical fixes. 🔒 Security Analysis - GOOD✅ Strong Security Measures
|
🐛 Critical Issues Requiring Fixes1. Database Query Security - HIGH PRIORITY File: // ISSUE: Table name not properly escaped
$old_sessions = $wpdb->get_results($wpdb->prepare(
"SELECT payload FROM $wpdb->dt_reports
WHERE type = 'import_session'
AND timestamp < %d",
strtotime( '-24 hours' )
), ARRAY_A);
// FIX: Use proper table name escaping
$old_sessions = $wpdb->get_results($wpdb->prepare(
"SELECT payload FROM {$wpdb->prefix}dt_reports
WHERE type = 'import_session'
AND timestamp < %d",
strtotime( '-24 hours' )
), ARRAY_A);Also apply this fix to line 110-115 in the same file. 2. Session Ownership Validation - HIGH PRIORITY File: // ISSUE: No validation that session belongs to current user
private function get_import_session( $session_id, $load_csv_data = true ) {
// FIX: Add user ownership validation
private function get_import_session( $session_id, $load_csv_data = true ) {
global $wpdb;
$session = $wpdb->get_row($wpdb->prepare(
"SELECT * FROM {$wpdb->prefix}dt_reports
WHERE id = %d AND user_id = %d AND type = 'import_session'",
$session_id, get_current_user_id()
));
if (!$session) {
return false;
}
// Continue with existing logic...
} |
🔧 Recommended Improvements3. File Path Security Enhancement - MEDIUM PRIORITY File: // ADD: Path traversal protection
$filename = 'import_' . uniqid() . '_' . sanitize_file_name( $file_data['name'] );
$filepath = $temp_dir . $filename;
// Add this validation:
$real_path = realpath(dirname($filepath)) . '/' . basename($filepath);
if (strpos($real_path, realpath($temp_dir)) !== 0) {
return new WP_Error('invalid_path', 'Invalid file path detected');
}4. Memory Management for Large Files - MEDIUM PRIORITY File: // ADD: Always use sampling for large datasets
$total_rows = count( $csv_data );
$use_sampling = $total_rows > 500; // Lower threshold for safety5. Cleanup Process Locking - LOW PRIORITY File: private function cleanup_temp_files() {
// ADD: Prevent concurrent cleanup processes
if (get_transient('dt_import_cleanup_running')) {
return;
}
set_transient('dt_import_cleanup_running', 1, 300);
// Existing cleanup logic...
delete_transient('dt_import_cleanup_running');
} |
📝 Minor Improvements
✅ Approval RecommendationStatus: APPROVE with requested changes The implementation is solid and demonstrates excellent engineering practices. Please address the two HIGH PRIORITY database security issues before merging. The medium and low priority items can be addressed in follow-up PRs if preferred. Great work on achieving feature parity with comprehensive improvements over the existing plugin! 👏 |
…rrent executions and ensure proper resource management
|
Thanks @kodinkat
|
…MS coordinates, improving validation and error handling
…n CSV import mapping
…vent XSS vulnerabilities.
Enhanced Features in Theme Import Tool
Document Purpose: This document outlines the new features and significant improvements in the theme-based import tool that were not present or were limited in the original plugin version.
Major New Features
1. Server-Side Processing Architecture
What's New: Complete shift from client-side to server-side processing
Original Plugin: Heavy reliance on client-side JavaScript for processing imports
2. Persistent Session Management with dt_reports Table
What's New: Uses
dt_reportstable instead of WordPress transientsOriginal Plugin: Used WordPress transients which expire and don't survive server restarts
3. Inline Field and Option Creation
What's New: Create new fields and field options directly within the mapping interface
Original Plugin: Required pre-existing fields and options, no ability to create them during import
4. Name-Based Connection and User Field Handling
What's New: Handle connection fields and user assignments using names instead of requiring IDs
Original Plugin: Required database IDs for connection fields and user assignments, making CSV preparation complex
5. Enhanced User Interface and Documentation
What's New: Comprehensive UI improvements and user guidance system
Original Plugin: Basic interface with minimal guidance, requiring external documentation
Conclusion
The theme import tool maintains full feature parity with the original plugin while introducing five major new features:
These changes represent a fundamental architectural shift and enhanced workflow capabilities that improve security, reliability, performance, and user experience while maintaining all the functionality users expect from the original plugin.