-
Notifications
You must be signed in to change notification settings - Fork 91
3x/import wizard date formats #73
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
Conversation
add intelligent date format detection that analyzes csv values to determine whether dates are iso, european (dd/mm), or american (mm/dd) format based on unambiguous evidence (values where day > 12). - add DateFormat and TimestampFormat enums with parsing support - add DateFormatResult dto for detection results with confidence - add DateValidator service for validating dates with format awareness - add date format detection to DataTypeInferencer - integrate date validation into CsvAnalyzer for date/datetime fields - add date format selector dropdown to review step ui - extend ColumnAnalysis with date format properties and helpers - extend ValueIssue with issue type for date ambiguity warnings
completely rewrites the import documentation to: - fix incorrect required fields (people: company_name is not required) - correct duplicate detection methods (only companies/people have non-id matching; opportunities, tasks, notes use id-only matching) - add comprehensive date format detection section explaining iso, european, and american formats with ambiguity handling - document company matching priority for people imports (id > domain > name) - simplify structure using tables instead of verbose prose - reduce file size by 40% while improving accuracy also fixes config/custom-fields.php to remove non-existent feature enum
add commonmark tableextension to markdown config so tables render as html instead of raw pipe-separated text also moves system_sections_enabled to disabled features in custom-fields
- Replace 5 overlapping docs with 4 focused guides - Add getting-started.md (actionable user onboarding) - Add developer-guide.md (streamlined technical docs) - Update api-guide.md with Coming Soon banner - Keep import-guide.md (already accurate) - Delete business-guide.md, quick-start-guide.md, technical-guide.md - Fix inaccuracies: auth is Jetstream not Fortify - Fix inaccuracies: Task/Note use morphToMany relationships - Remove non-existent keyboard shortcuts - Update config and icon mappings
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 implements comprehensive date format detection and handling for the Import Wizard, enabling automatic recognition of ISO, European (DD/MM/YYYY), and American (MM/DD/YYYY) date formats with ambiguity detection and user-selectable format correction.
Key Changes:
- Automatic date format detection with confidence scoring for CSV imports
- Support for multiple date and timestamp formats (ISO, European, American)
- Ambiguity detection and warning system for dates like "01/02/2024"
- User-selectable format dropdown in the review step when confidence is low
- Comprehensive test coverage for all date format parsing and validation scenarios
Reviewed changes
Copilot reviewed 28 out of 30 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/Services/Import/TimestampFormatTest.php | New test file covering timestamp parsing with time-first (European/American) and time-last (ISO) formats |
| tests/Unit/Services/Import/DateValidatorTest.php | New test file for date validation, ambiguity detection, and column-level validation |
| tests/Unit/Services/Import/DateFormatTest.php | New comprehensive test file covering date parsing, format detection, and multi-pattern support |
| tests/Unit/Services/Import/DataTypeInferencerTest.php | Extended existing tests to include date format detection with confidence scoring |
| config/markdown.php | Added TableExtension for markdown rendering support |
| config/custom-fields.php | Updated feature flags for custom fields configuration |
| composer.lock | Updated custom-fields package reference |
| app/Filament/Resources/PeopleResource/Pages/ViewPeople.php | Code style fix: spacing in closure parameter |
| app-modules/ImportWizard/src/Services/DateValidator.php | New service for validating date values, detecting ambiguity, and generating validation issues |
| app-modules/ImportWizard/src/Services/DataTypeInferencer.php | Extended with date format detection logic using unambiguous value analysis |
| app-modules/ImportWizard/src/Services/CsvAnalyzer.php | Integrated date field detection and DateValidator for date columns |
| app-modules/ImportWizard/src/Livewire/ImportWizard.php | Added selectedDateFormats property for tracking user-selected formats |
| app-modules/ImportWizard/src/Livewire/Concerns/HasValueAnalysis.php | Added changeDateFormat and getSelectedDateFormat methods for format selection |
| app-modules/ImportWizard/src/Enums/TimestampFormat.php | New enum for timestamp formats with parsing and display methods |
| app-modules/ImportWizard/src/Enums/DateFormat.php | New enum for date formats with multi-pattern parsing and ambiguity detection |
| app-modules/ImportWizard/src/Data/ValueIssue.php | Extended with issueType property and isDateAmbiguous helper method |
| app-modules/ImportWizard/src/Data/DateFormatResult.php | New data object for date format detection results |
| app-modules/ImportWizard/src/Data/ColumnAnalysis.php | Extended with date format properties and helper methods |
| app-modules/ImportWizard/resources/views/livewire/partials/value-row.blade.php | New partial for rendering individual value rows with date preview |
| app-modules/ImportWizard/resources/views/livewire/partials/step-review.blade.php | Refactored to support date format selection dropdown and grouped value display |
| app-modules/ImportWizard/resources/views/components/format-select.blade.php | New dropdown component for date format selection with examples |
| app-modules/Documentation/resources/views/index.blade.php | Updated document icon mappings for documentation reorganization |
| app-modules/Documentation/resources/markdown/technical-guide.md | Removed file - content reorganized into other guides |
| app-modules/Documentation/resources/markdown/quick-start-guide.md | Removed file - replaced with getting-started.md |
| app-modules/Documentation/resources/markdown/import-guide.md | Comprehensive rewrite with date format detection documentation and simplified structure |
| app-modules/Documentation/resources/markdown/getting-started.md | New streamlined getting started guide |
| app-modules/Documentation/resources/markdown/developer-guide.md | New focused developer guide with installation and architecture |
| app-modules/Documentation/resources/markdown/business-guide.md | Removed file - content consolidated |
| app-modules/Documentation/resources/markdown/api-guide.md | Simplified to reflect API coming soon status |
| app-modules/Documentation/config/documentation.php | Updated document configuration to match new guide structure |
| /** | ||
| * Get the primary PHP date format string for Carbon::createFromFormat(). | ||
| */ | ||
| #[\Deprecated(message: 'Use getPhpFormats() for multi-pattern support')] |
Copilot
AI
Jan 3, 2026
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 Deprecated attribute uses a lowercase 'd' but should use uppercase 'D' to match PHP's built-in Deprecated attribute syntax: #[\Deprecated(message: '...')]
|
|
||
| // For date fields, show parsed preview | ||
| $parsedDatePreview = null; | ||
| if ($isDateField && $effectiveDateFormat !== null && $value !== '' && !$hasError) { |
Copilot
AI
Jan 3, 2026
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 Blade comparison uses $value !== '' which compares against an empty string, but earlier in the file at line 4 the display value uses $value !== '' ? $value : '(blank)'. This inconsistency could cause confusion. Consider using a consistent check, such as a helper variable defined once.
| /** | ||
| * Validate a date value against the specified format. | ||
| * | ||
| * @return array{valid: bool, parsed: ?Carbon, issue: ?ValueIssue, isAmbiguous: bool} |
Copilot
AI
Jan 3, 2026
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 PHPDoc return type annotation uses \Carbon\Carbon while the type hint uses just Carbon. For consistency and since Carbon\Carbon is already imported at the top of the file, the PHPDoc should use the imported alias without the leading backslash.
| // Try to parse with the specified format | ||
| $parsed = $format->parse($value); | ||
|
|
||
| if (! $parsed instanceof \Carbon\Carbon) { |
Copilot
AI
Jan 3, 2026
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 condition checks ! $parsed instanceof \Carbon\Carbon but should use the imported Carbon alias without the leading backslash for consistency with the rest of the codebase and the import statement at the top of the file.
| $alternativeParsed = $alternativeFormat->parse($value); | ||
|
|
||
| // Only mark as ambiguous if the alternative also parses to a different date | ||
| if ($alternativeParsed instanceof \Carbon\Carbon && ! $parsed->isSameDay($alternativeParsed)) { |
Copilot
AI
Jan 3, 2026
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 condition checks $alternativeParsed instanceof \Carbon\Carbon but should use the imported Carbon alias without the leading backslash for consistency with the import statement and other parts of the codebase.
| */ | ||
| public function formatForPreview(?Carbon $date): string | ||
| { | ||
| if (! $date instanceof \Carbon\Carbon) { |
Copilot
AI
Jan 3, 2026
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 condition checks ! $date instanceof \Carbon\Carbon but should use the imported Carbon alias without the leading backslash for consistency with the import statement at the top of the file.
| if (! $date instanceof \Carbon\Carbon) { | |
| if (! $date instanceof Carbon) { |
No description provided.