Improvements #72

ManukMinasyan · 2026-01-03T15:12:10Z

No description provided.

…rt preview

…in import preview

…ogic

Track unique new company names during import preview processing. Shows count of companies that will be created when importing People/Opportunities with company references. - Add newCompanies property to ImportSessionData - Accumulate unique new company names across chunks - Display new companies count in preview stats area - Remove per-row match badges (simplify UI)

…ning - Fix bug: enrichRowWithCompanyMatch was reading $row['id'] (person/opportunity ID) instead of $row['company_id'] (actual company ID) - Add hasMappingWarnings() to detect multiple warning conditions - Add hasCompanyNameWithoutId() to detect company_name without company_id mapping - Add getMappingWarningsHtml() to generate dynamic modal content - Extend existing warning modal to show both unique identifier and company warnings

…type inference

Copilot

Pull request overview

This PR introduces significant improvements to the import wizard functionality, including intelligent column mapping, session management, and cleanup mechanisms. The changes focus on enhancing the user experience during data imports and improving system reliability.

Key changes:

Added data type inference for unmapped CSV columns with confidence scoring
Implemented consolidated session tracking with heartbeat monitoring
Enhanced column matching with plural/singular variants support
Removed polymorphic entity attachment trait from Task and Note importers
Added scheduled cleanup command for orphaned import sessions
Upgraded phone number custom field type with migration

Reviewed changes

Copilot reviewed 37 out of 39 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
tests/Unit/Services/Import/DataTypeInferencerTest.php	New test suite for data type inference service
tests/Unit/Services/Import/ColumnMatcherTest.php	New test suite for column matching service
tests/Feature/Filament/App/Imports/TaskImporterTest.php	Simplified tests to focus on assignee attachment only
tests/Feature/Filament/App/Imports/ImportWizardUITest.php	Added tests for preview caching, hash-based regeneration, and session cleanup
routes/console.php	Added scheduled cleanup command for import sessions
database/migrations/2026_01_02_152157_update_phone_number_custom_fields_type.php	Migration to convert phone_number fields from text to phone type
composer.json	Changed custom-fields to local path repository with @dev version
app-modules/ImportWizard/src/Services/DataTypeInferencer.php	New service to infer field types from sample data
app-modules/ImportWizard/src/Services/ColumnMatcher.php	New service for intelligent column header matching
app-modules/ImportWizard/src/Data/ImportSessionData.php	New DTO for consolidated session cache management
app-modules/ImportWizard/src/Enums/PreviewStatus.php	New enum for preview processing states
app-modules/ImportWizard/src/Jobs/ProcessImportPreview.php	Added heartbeat monitoring and cancellation logic
app-modules/ImportWizard/src/Console/CleanupOrphanedImportsCommand.php	Enhanced with dual-check safety system and dry-run support
app-modules/ImportWizard/src/Filament/Imports/*	Expanded guess patterns for better column matching
app-modules/ImportWizard/src/Livewire/*	Implemented preview input hashing, heartbeat tracking, and field labels
app-modules/ImportWizard/resources/views/*	Added UI for inferred mappings and new company tracking
lang/vendor/custom-fields/en/custom-fields.php	Removed unused language file

Copilot · 2026-01-03T15:17:54Z

tests/Feature/Filament/App/Imports/TaskImporterTest.php

+        $assignee = User::factory()->create();
+        $assignee->teams()->attach($this->team);


This test creates a user with User::factory()->create() without explicitly attaching them to the team first. While line 19 shows the user being attached to the team, the order could cause confusion. Consider creating the user and immediately attaching them to the team in a clearer sequence to ensure the test intent is obvious.

Copilot · 2026-01-03T15:17:54Z

app-modules/ImportWizard/src/Services/DataTypeInferencer.php

+{
+    private const string EMAIL_PATTERN = '/^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/';
+
+    private const string PHONE_PATTERN = '/^[\+]?\d?[-.\s]?\(?\d{1,4}\)?[-.\s]?\d{1,4}[-.\s]?\d{1,4}[-.\s]?\d{0,4}$/';


The PHONE_PATTERN regex is very permissive and may match non-phone numbers. For example, it would match sequences like '1-2-3-4' or '....'. Consider adding minimum digit requirements or using a more specific pattern to reduce false positives. The pattern could also match dates in certain formats (e.g., '2024-01-15'), which is why date checking is done first, but this ordering dependency creates a fragile design.

Copilot · 2026-01-03T15:17:55Z

app/Providers/Filament/AppPanelProvider.php

        );

        Action::configureUsing(fn (Action $action): Action => $action->size(Size::Small)->iconPosition('before'));
+        DeleteAction::configureUsing(fn (DeleteAction $action): DeleteAction => $action->label('Delete record'));


The DeleteAction::configureUsing() sets a label 'Delete record' for all delete actions globally. This may not be appropriate for all contexts. For example, deleting a batch of records or a file attachment might need different wording. Consider if this global configuration is too broad, or if specific resources should override this label as needed.

Suggested change

DeleteAction::configureUsing(fn (DeleteAction $action): DeleteAction => $action->label('Delete record'));

Copilot · 2026-01-03T15:17:55Z

database/migrations/2026_01_02_152157_update_phone_number_custom_fields_type.php

+return new class extends Migration
+{
+    public function up(): void
+    {
+        // Get all phone_number custom field IDs that are currently text type
+        /** @var \Illuminate\Support\Collection<int, int|string> */
+        $phoneFieldIds = DB::table('custom_fields')
+            ->where('code', 'phone_number')
+            ->where('type', 'text')
+            ->pluck('id');
+
+        if ($phoneFieldIds->isEmpty()) {
+            return;
+        }
+
+        // Update the field type from text to phone
+        DB::table('custom_fields')
+            ->whereIn('id', $phoneFieldIds)
+            ->update(['type' => 'phone']);
+
+        // Migrate values from text_value to json_value (as JSON array)
+        // Phone type uses multiChoice schema which stores values in json_value
+        DB::table('custom_field_values')
+            ->whereIn('custom_field_id', $phoneFieldIds)
+            ->whereNotNull('text_value')
+            ->whereNull('json_value')
+            ->orderBy('id')
+            ->each(function (object $row): void {
+                DB::table('custom_field_values')
+                    ->where('id', $row->id)
+                    ->update([
+                        'json_value' => json_encode([$row->text_value]),
+                        'text_value' => null,
+                    ]);
+            });
+    }
+};


This migration is missing a down() method to reverse the changes. Migrations should be reversible when possible. The down method should convert phone fields back to text type and migrate json_value back to text_value.

Copilot · 2026-01-03T15:17:55Z

app-modules/ImportWizard/src/Data/ImportSessionData.php

+        Cache::put(
+            self::cacheKey($sessionId),
+            [...$this->toArray(), 'heartbeat' => (int) now()->timestamp],


The refresh() method updates the cache with a spread of $this->toArray() plus the new heartbeat, but it doesn't update the current object's $this->heartbeat property. This could lead to inconsistency if the same ImportSessionData instance is used after calling refresh(). Consider updating the object's heartbeat property as well, or making it clear through naming/documentation that this method only updates the cache.

Suggested change

Cache::put(

self::cacheKey($sessionId),

[...$this->toArray(), 'heartbeat' => (int) now()->timestamp],

$heartbeat = (int) now()->timestamp;

$this->heartbeat = $heartbeat;

Cache::put(

self::cacheKey($sessionId),

[...$this->toArray(), 'heartbeat' => $heartbeat],

Copilot · 2026-01-03T15:17:56Z

app-modules/ImportWizard/src/Data/ImportSessionData.php

+    public static function update(string $sessionId, array $changes): void
+    {
+        $data = self::find($sessionId);
+
+        if (! $data instanceof self) {
+            return;
+        }
+
+        Cache::put(
+            self::cacheKey($sessionId),
+            [...$data->toArray(), ...$changes],
+            now()->addHours(self::ttlHours())
+        );
+    }


The update() method has a race condition. Between calling find() (line 64) and Cache::put() (line 70-73), another process could update the cache, and those changes would be overwritten. For example, if two job chunks update progress simultaneously, one update could be lost. Consider using atomic cache operations like Cache::lock() or designing the update to merge changes more safely.

Copilot · 2026-01-03T15:17:56Z

app/Filament/Resources/PeopleResource/Pages/ViewPeople.php

                        ->label('Company')
                        ->color('primary')
-                        ->url(fn (People $record): ?string => $record->company ? CompanyResource::getUrl('view', [$record->company]) : null),
+                        ->url(fn(People $record): ?string => $record->company ? CompanyResource::getUrl('view', [$record->company]) : null),


The inline lambda function syntax fn(People $record): ?string => is missing a space after fn. According to Laravel Pint (which this project uses), there should be a space between fn and the parameter list. The existing code on line 39 has the correct format with fn (...), but this line uses fn(...) without the space.

Suggested change

->url(fn(People $record): ?string => $record->company ? CompanyResource::getUrl('view', [$record->company]) : null),

->url(fn (People $record): ?string => $record->company ? CompanyResource::getUrl('view', [$record->company]) : null),

Copilot · 2026-01-03T15:17:56Z

app-modules/ImportWizard/src/Services/ColumnMatcher.php

+        $normalized = preg_replace('/[\s_\-\.]+/', '_', trim($value));
+
+        return Str::lower($normalized ?? $value);


The normalize() method uses a regex pattern that replaces multiple separators with underscores, but the pattern [\s_\-\.]+ will match sequences of mixed separators (e.g., ".- _") and replace them with a single underscore. While this is likely the intended behavior, the subsequent call to collapse multiple underscores may not be necessary since the regex already handles this. Additionally, if the preg_replace fails and returns null, the fallback to $value may not be properly normalized. Consider handling this edge case more explicitly.

Suggested change

$normalized = preg_replace('/[\s_\-\.]+/', '_', trim($value));

return Str::lower($normalized ?? $value);

$trimmed = trim($value);

$normalized = preg_replace('/[\s_\-\.]+/', '_', $trimmed);

if ($normalized === null) {

$normalized = $trimmed;

}

return Str::lower($normalized);

Copilot · 2026-01-03T15:17:57Z

app-modules/ImportWizard/src/Services/PreviewChunkService.php

    private function enrichRowWithCompanyMatch(array $row, string $teamId): array
    {
-        $companyId = (string) ($row['id'] ?? '');
+        $companyId = (string) ($row['company_id'] ?? '');


The company_id is read from $row['company_id'] but this field was previously read as $row['id'] (see diff line removing it). This change might break existing functionality if the row structure hasn't been updated elsewhere. Verify that all places populating this row data have been updated to use 'company_id' instead of 'id'.

Suggested change

$companyId = (string) ($row['company_id'] ?? '');

$companyId = (string) ($row['company_id'] ?? $row['id'] ?? '');

Copilot · 2026-01-03T15:17:57Z

routes/console.php

+// Clean up orphaned import sessions hourly
+// Deletes temp files from abandoned imports (user left without completing)
+Schedule::command('import:cleanup')->everyFifteenMinutes();


The comment says the cleanup runs hourly, but the code specifies everyFifteenMinutes(). The comment should be updated to match the actual schedule frequency.

ManukMinasyan added 7 commits January 2, 2026 01:43

refactor: remove unused processing and readiness indicators from impo…

725e74e

…rt preview

refactor: adjust layout for loading indicator and navigation buttons …

614fb01

…in import preview

Merge branch 'feature/import-wizard-error-filter' into 3.x

a5d4115

refactor: consolidate import session management and enhance cleanup l…

54bb02d

…ogic

feat: enhance import functionality with improved column matching and …

3eafe29

…type inference

Copilot AI review requested due to automatic review settings January 3, 2026 15:12

ManukMinasyan merged commit 327f08a into 3.x Jan 3, 2026
2 of 10 checks passed

Copilot started reviewing on behalf of ManukMinasyan January 3, 2026 15:12 View session

Copilot AI reviewed Jan 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improvements #72

Improvements #72

Uh oh!

ManukMinasyan commented Jan 3, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Copilot AI Jan 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		$assignee = User::factory()->create();
		$assignee->teams()->attach($this->team);

	->url(fn(People $record): ?string => $record->company ? CompanyResource::getUrl('view', [$record->company]) : null),
	->url(fn (People $record): ?string => $record->company ? CompanyResource::getUrl('view', [$record->company]) : null),

		$normalized = preg_replace('/[\s_\-\.]+/', '_', trim($value));

		return Str::lower($normalized ?? $value);

-        $normalized = preg_replace('/[\s_\-\.]+/', '_', trim($value));
-        return Str::lower($normalized ?? $value);
+        $trimmed = trim($value);
+        $normalized = preg_replace('/[\s_\-\.]+/', '_', $trimmed);
+        if ($normalized === null) {
+            $normalized = $trimmed;
+        }
+        return Str::lower($normalized);

	$companyId = (string) ($row['company_id'] ?? '');
	$companyId = (string) ($row['company_id'] ?? $row['id'] ?? '');

Improvements #72

Improvements #72

Uh oh!

Conversation

ManukMinasyan commented Jan 3, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants