-
Notifications
You must be signed in to change notification settings - Fork 85
Feature/easy migration next #52
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
Open
ManukMinasyan
wants to merge
86
commits into
main
Choose a base branch
from
feature/easy-migration-next
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+10,342
−195
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
add comprehensive data import system for crm migration: - import center page with quick import, history, and migration wizard tabs - excel to csv conversion via phpspreadsheet - importers for companies, people, opportunities, tasks, and notes - duplicate handling strategy with user choice (skip/update/create) - custom fields integration via weakmap storage - migration batch tracking for multi-entity imports - dependency enforcement (companies before people, etc) - failed rows download for error recovery
- redesign entity cards with cleaner hover states and visual hierarchy - simplify import tips section with minimal bullet styling - improve step progress indicator with connecting lines - refine entity selection cards with subtle borders and transitions - add collapsible progress section in migration wizard step 2 - update completion step with success icon and cleaner summary - add pointer-events-none to icon containers to prevent click issues
- conditionally show logo only on guest pages (login/register/forgot) - fix svg overflow in logo container causing invisible clickable area - change sidebar nav overflow from visible to hidden
- keep brandName for page title display - use empty logo view for authenticated pages - show full logo only on guest pages
- only mark as stuck if processing actually started then stopped - show pending when processed_rows is 0 regardless of total_rows - update test to expect pending for old unprocessed imports
remove MigrationBatch model, factory, migration and migration_batch_id column as they provided no user-facing value - batch data was tracked but never displayed clean up MigrationWizard by removing unused methods (recordImportComplete, getRecentImports) and batch-related code while preserving wizard functionality simplify ImportCenter by removing dead getImportAction and getHeaderActions methods, and simplify hasImportJobFailed from 4 complex queries to 1 remove 9 batch-related tests that no longer apply, update remaining tests
- extract HasImportEntities trait for shared entity configuration - fix PeopleImporter to respect duplicate handling strategy - remove unused CreationSource import from BaseImporter - delete unused column_mappings/duplicate_strategy migration - simplify MigrationWizard with collect() helpers - remove redundant getEntityTypes() alias method
… companies, and opportunities
integrate validation service with csv analyzer to enforce custom field validation rules in the review values step. uses laravel validator instead of custom validation logic, and blocks proceeding when errors exist.
- validate comma-separated values individually for multi-value fields - add confirmation modal to proceed with validation errors - simplify error UI by removing excessive red styling - auto-skip error values when user confirms to proceed
Fixed three issues preventing email duplicate detection from working: 1. Entity type query mismatch - PeopleImporter was searching for 'people' but custom_fields table stores full class name 'App\Models\People' 2. Duplicate strategy mismatch - Import preview used default SKIP strategy but actual import was hardcoded to CREATE_NEW, causing UI to show "will update" while actual import created duplicates 3. Test setup - Test helper created custom fields with wrong entity_type format, causing tests to fail even though production code was correct Now email duplicate detection works correctly: - Preview accurately shows which records will be updated - Actual import respects duplicate handling strategy (SKIP by default) - Tests pass and validate the behavior
Add warning modal when users try to proceed from column mapping without mapping unique identifier columns (Record ID or entity-specific fields like email/name). Changes: - Add hasUniqueIdentifierMapped() to HasColumnMapping trait - Add static methods to BaseImporter for unique identifier config - Override methods in all 5 importers (People, Company, Opportunity, Task, Note) - Add proceedWithoutUniqueIdentifiersAction() modal in ImportWizard - Intercept MAP step transitions to show warning - Add documentation section explaining unique identifiers - Link to docs using absolute URL via route() helper The modal shows entity-specific guidance: - People: Email addresses or Record ID - Companies: Company name or Record ID - Opportunities: Opportunity name or Record ID - Tasks: Task title or Record ID - Notes: Skip warning (no fallback matching)
Major Performance Fixes: - Fix N+1 query in CompanyMatcher by pre-loading all companies into memory cache - Fix memory issue in CsvAnalyzer by using streaming CSV reader per column - Wrap fragile reflection usage in ImportPreviewService with clear error handling Quick Wins for SMB Users: - Add 10,000 row limit with user-friendly error message - Reduce file size limit from 100MB to 50MB (aligned with row limit) - Add loading indicators to preview step for better UX Impact: - Preview step: 10x faster for large files (caching eliminates 2000+ queries for 1000 rows) - Memory usage: Significantly reduced by streaming CSV instead of loading into arrays - Better error messages: Clear guidance when limits exceeded - Perceived performance: Loading indicators prevent "frozen app" feeling Technical Details: - CompanyMatcher: In-memory cache indexed by name and domain (1 query vs N queries) - CsvAnalyzer: Fresh CSV reader per column analysis (streaming vs array) - ImportPreviewService: Wrapped reflection in try-catch with maintenance docs - HasCsvParsing: Row count validation before processing - ImportWizard: File size validation at 50MB All tests passing (411 passed, 7 skipped)
Fixes two critical bugs that broke import functionality:
1. StreamingImportCsv: Replace manual reflection with Filament's __invoke()
- Previously attempted to call non-existent save() method
- Manually invoked pipeline methods, missing validation/hooks
- Now uses Filament's native __invoke() for complete pipeline
- Ensures all hooks execute: beforeValidate, afterValidate, beforeSave,
afterSave, beforeCreate/Update, afterCreate/Update
- Fixes custom field saving and relationship persistence
2. Wire:loading race condition in preview step
- Previously targeted both advanceToNextStep and generateImportPreview
- Created race condition since generateImportPreview called inside parent
- Now only targets advanceToNextStep method
- Loading indicator clears immediately after preview generation
All import tests passing (97 tests, 203 assertions).
Performance optimizations preserved: streaming CSV, adaptive chunking,
fast row counting, single-pass analysis, sample-based preview.
- Add 'imports' queue to StreamingImportCsv jobs - Configure Horizon supervisor-imports with optimized settings: - 5 minute timeout (vs 60s for default queue) - 256MB memory limit (vs 128MB for default queue) - 2 retry attempts (vs 1 for default queue) - 2-5 parallel processes in production - Add queue wait threshold of 120s for imports queue - Document queue architecture in .env.example This isolates import jobs from general application jobs, preventing imports from blocking other critical operations and allowing independent resource tuning for CSV processing workloads.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.