diff --git a/config/custom-fields.php b/config/custom-fields.php index 77a06808..b353181f 100644 --- a/config/custom-fields.php +++ b/config/custom-fields.php @@ -159,6 +159,25 @@ 'currencies' => null, ], + /* + |-------------------------------------------------------------------------- + | Imports + |-------------------------------------------------------------------------- + | + | How CSV cells are read during import. Dates and numbers are parsed against a + | declared convention rather than guessed, so an ambiguous cell like 3/4/2024 has + | exactly one meaning. The defaults match how this package has always behaved. + | + */ + 'imports' => [ + // 'iso' (Y-m-d only), 'european' (day first), or 'american' (month first). + // Every convention also accepts ISO, so picking one only widens what is read. + 'date_format' => env('CUSTOM_FIELDS_IMPORT_DATE_FORMAT', 'iso'), + + // 'point' (1,234.56) or 'comma' (1.234,56). + 'number_format' => env('CUSTOM_FIELDS_IMPORT_NUMBER_FORMAT', 'point'), + ], + 'database' => [ 'migrations_path' => database_path('custom-fields'), 'table_names' => [ diff --git a/src/CustomFields.php b/src/CustomFields.php index 6d625846..6f68b1d6 100644 --- a/src/CustomFields.php +++ b/src/CustomFields.php @@ -5,6 +5,8 @@ namespace Relaticle\CustomFields; use Closure; +use Relaticle\CustomFields\Enums\ImportDateFormat; +use Relaticle\CustomFields\Enums\ImportNumberFormat; use Relaticle\CustomFields\Models\CustomField; use Relaticle\CustomFields\Models\CustomFieldOption; use Relaticle\CustomFields\Models\CustomFieldSection; @@ -226,6 +228,26 @@ public static function dateTimeDisplayFormat(): ?string return self::$dateTimeDisplayFormat; } + /** + * Date convention used when reading CSV cells during import. + */ + public static function importDateFormat(): ImportDateFormat + { + return ImportDateFormat::tryFrom( + (string) config('custom-fields.imports.date_format', 'iso') + ) ?? ImportDateFormat::ISO; + } + + /** + * Decimal separator convention used when reading CSV cells during import. + */ + public static function importNumberFormat(): ImportNumberFormat + { + return ImportNumberFormat::tryFrom( + (string) config('custom-fields.imports.number_format', 'point') + ) ?? ImportNumberFormat::POINT; + } + /** * Register a custom tenant resolver callback. * diff --git a/src/Enums/ImportDateFormat.php b/src/Enums/ImportDateFormat.php new file mode 100644 index 00000000..d3aa81ce --- /dev/null +++ b/src/Enums/ImportDateFormat.php @@ -0,0 +1,137 @@ + + */ + private const array TEXTUAL_FORMATS = ['j F Y', 'j M Y', 'F j, Y', 'F jS Y', 'M j, Y', 'M jS Y']; + + public function getLabel(): string + { + return match ($this) { + self::ISO => 'ISO standard', + self::EUROPEAN => 'European (day first)', + self::AMERICAN => 'American (month first)', + }; + } + + /** + * @return array + */ + public function getExamples(bool $withTime = false): array + { + if ($withTime) { + return match ($this) { + self::ISO => ['2024-05-15 16:00:00'], + self::EUROPEAN => ['2024-05-15 16:00:00', '15/05/2024 16:00:00'], + self::AMERICAN => ['2024-05-15 16:00:00', '05/15/2024 16:00:00'], + }; + } + + return match ($this) { + self::ISO => ['2024-05-15'], + self::EUROPEAN => ['2024-05-15', '15/05/2024', '15 May 2024'], + self::AMERICAN => ['2024-05-15', '05/15/2024', 'May 15, 2024'], + }; + } + + public function parse(string $value, bool $withTime = false): ?CarbonImmutable + { + $value = trim($value); + + if ($value === '') { + return null; + } + + foreach ($this->getParseFormats($withTime) as $format) { + $parsed = $this->parseStrictly($format, $value); + + if ($parsed instanceof CarbonImmutable) { + return $parsed; + } + } + + return null; + } + + /** + * `createFromFormat` overflows silently: `d/m/Y` turns 31/02/2024 into 2 March and + * `Y-m-d` turns 2024-02-31 into the same. The `!` prefix does not prevent it and + * `Carbon::hasFormat()` does not detect it. `getLastErrors()` reports a warning for + * every overflow, so that is the gate. + */ + private function parseStrictly(string $format, string $value): ?CarbonImmutable + { + $parsed = DateTimeImmutable::createFromFormat('!'.$format, $value); + $errors = DateTimeImmutable::getLastErrors(); + + if ($parsed === false) { + return null; + } + + if ($errors !== false && (($errors['warning_count'] ?? 0) > 0 || ($errors['error_count'] ?? 0) > 0)) { + return null; + } + + if ((int) $parsed->format('Y') < self::MINIMUM_YEAR) { + return null; + } + + return CarbonImmutable::instance($parsed); + } + + /** + * ISO forms come first in every list so a `Y-m-d` cell is never re-read as something + * else, and every list carries the textual and datetime forms the previous + * `Carbon::parse` fallback accepted, so nothing that imports today stops importing. + * + * @return array + */ + private function getParseFormats(bool $withTime): array + { + $iso = $withTime + ? ['Y-m-d H:i:s', 'Y-m-d\TH:i:s', 'Y-m-d\TH:i', 'Y-m-d H:i'] + : ['Y-m-d', 'Y-m-d H:i:s', 'Y-m-d\TH:i:s', 'Y-m-d H:i']; + + $localised = match ($this) { + self::ISO => [], + self::EUROPEAN => $withTime + ? ['d/m/Y H:i:s', 'd-m-Y H:i:s', 'd.m.Y H:i:s', 'j/n/Y H:i:s', 'd/m/Y H:i', 'j/n/Y H:i'] + : ['d/m/Y', 'd-m-Y', 'd.m.Y', 'j/n/Y', 'j-n-Y', 'j.n.Y', ...self::TEXTUAL_FORMATS], + self::AMERICAN => $withTime + ? ['m/d/Y H:i:s', 'm-d-Y H:i:s', 'n/j/Y H:i:s', 'm/d/Y H:i', 'n/j/Y H:i'] + : ['m/d/Y', 'm-d-Y', 'n/j/Y', 'n-j-Y', ...self::TEXTUAL_FORMATS], + }; + + return [...$iso, ...$localised]; + } +} diff --git a/src/Enums/ImportNumberFormat.php b/src/Enums/ImportNumberFormat.php new file mode 100644 index 00000000..7a30e759 --- /dev/null +++ b/src/Enums/ImportNumberFormat.php @@ -0,0 +1,77 @@ + 'Point', + self::COMMA => 'Comma', + }; + } + + public function getExample(): string + { + return match ($this) { + self::POINT => '1,234.56', + self::COMMA => '1.234,56', + }; + } + + /** + * `$stripCurrencySymbol` keeps currency columns accepting `$1,234.56` and + * `1234.56 EUR`, which they accept today. It is not applied to plain number + * columns, which reject anything with non-numeric characters. + */ + public function parse(string $value, bool $stripCurrencySymbol = false): ?float + { + $value = trim($value); + + if ($value === '') { + return null; + } + + if ($stripCurrencySymbol) { + // Keep the sign, drop a symbol or code on either side: -$5.50, $1,234.56, 1234.56 EUR. + $value = preg_replace('/^([+-]?)[^\d]*/u', '$1', $value) ?? ''; + $value = preg_replace('/[^\d]*$/u', '', $value) ?? ''; + } + + $decimalSeparator = match ($this) { + self::POINT => '.', + self::COMMA => ',', + }; + + $otherSeparator = match ($this) { + self::POINT => ',', + self::COMMA => '.', + }; + + $value = str_replace([' ', "\u{00A0}", $otherSeparator], '', $value); + + if ($decimalSeparator === ',') { + $value = str_replace(',', '.', $value); + } + + if (! is_numeric($value)) { + return null; + } + + return (float) $value; + } +} diff --git a/src/FieldTypeSystem/Definitions/CurrencyFieldType.php b/src/FieldTypeSystem/Definitions/CurrencyFieldType.php index b4c73d49..8e9a05e1 100644 --- a/src/FieldTypeSystem/Definitions/CurrencyFieldType.php +++ b/src/FieldTypeSystem/Definitions/CurrencyFieldType.php @@ -9,12 +9,14 @@ use Filament\Schemas\Components\Fieldset; use Filament\Schemas\Components\Utilities\Get; use NumberFormatter; +use Relaticle\CustomFields\CustomFields; use Relaticle\CustomFields\Data\Settings\CurrencyFieldSettingsData; use Relaticle\CustomFields\FieldTypeSystem\BaseFieldType; use Relaticle\CustomFields\FieldTypeSystem\FieldSchema; use Relaticle\CustomFields\Filament\Integration\Components\Forms\CurrencyComponent; use Relaticle\CustomFields\Filament\Integration\Components\Infolists\CurrencyEntry; use Relaticle\CustomFields\Filament\Integration\Components\Tables\Columns\CurrencyColumn; +use Relaticle\CustomFields\Imports\UnresolvedValue; use Relaticle\CustomFields\Support\CurrencyProvider; use Relaticle\CustomFields\Validation\Capabilities\MaxValueCapability; use Relaticle\CustomFields\Validation\Capabilities\MinValueCapability; @@ -40,16 +42,23 @@ public function configure(): FieldSchema fn (): array => $this->settingsSchema(), ) ->importExample('99.99') - ->importTransformer(function (mixed $state): ?float { + ->importTransformer(function (mixed $state): float|UnresolvedValue|null { if (blank($state)) { return null; } - if (is_string($state)) { - $state = preg_replace('/[^0-9.-]/', '', $state); + $format = CustomFields::importNumberFormat(); + $parsed = $format->parse((string) $state, stripCurrencySymbol: true); + + if ($parsed === null) { + return UnresolvedValue::make($state, sprintf( + "'%s' is not a valid amount. Expected format: %s.", + $state, + $format->getExample(), + )); } - return (float) $state; + return $parsed; }) ->exportTransformer(function (mixed $value): ?string { if ($value === null) { diff --git a/src/Filament/Integration/Support/Imports/ImportColumnConfigurator.php b/src/Filament/Integration/Support/Imports/ImportColumnConfigurator.php index 283b1366..b5155633 100644 --- a/src/Filament/Integration/Support/Imports/ImportColumnConfigurator.php +++ b/src/Filament/Integration/Support/Imports/ImportColumnConfigurator.php @@ -7,16 +7,17 @@ namespace Relaticle\CustomFields\Filament\Integration\Support\Imports; -use Carbon\Carbon; -use Exception; -use Filament\Actions\Imports\Exceptions\RowImportFailedException; +use Carbon\CarbonImmutable; +use Closure; use Filament\Actions\Imports\ImportColumn; use Relaticle\CustomFields\CustomFields; use Relaticle\CustomFields\Enums\FieldDataType; use Relaticle\CustomFields\Facades\CustomFieldsType; use Relaticle\CustomFields\Facades\Entities; +use Relaticle\CustomFields\Imports\UnresolvedValue; use Relaticle\CustomFields\Models\CustomField; use Relaticle\CustomFields\Models\CustomFieldOption; +use Relaticle\CustomFields\Rules\RejectsUnresolvedValue; use Relaticle\CustomFields\Services\ValidationService; use Throwable; @@ -42,10 +43,10 @@ public function configure(ImportColumn $column, CustomField $customField): Impor match ($customField->typeData->dataType) { FieldDataType::SINGLE_CHOICE => $this->configureSingleChoice($column, $customField), FieldDataType::MULTI_CHOICE => $this->configureMultiChoice($column, $customField), - FieldDataType::DATE => $this->configureDate($column), - FieldDataType::DATE_TIME => $this->configureDateTime($column), - FieldDataType::NUMERIC, FieldDataType::FLOAT => $column->numeric(), - FieldDataType::BOOLEAN => $this->configureBoolean($column), + FieldDataType::DATE => $this->configureDate($column, $customField), + FieldDataType::DATE_TIME => $this->configureDateTime($column, $customField), + FieldDataType::NUMERIC, FieldDataType::FLOAT => $this->configureNumeric($column, $customField), + FieldDataType::BOOLEAN => $this->configureBoolean($column, $customField), default => $this->configureText($column, $customField), }; @@ -70,7 +71,7 @@ private function configureViaFieldType(ImportColumn $column, CustomField $custom return false; } - $column->castStateUsing($transformer); + $column->castStateUsing($this->wrapTransformer($transformer, $customField)); $example = $schema->getImportExample(); if ($example !== null) { @@ -130,7 +131,7 @@ private function configureMultiChoice(ImportColumn $column, CustomField $customF */ private function configureLookup(ImportColumn $column, CustomField $customField, bool $multiple): void { - $column->castStateUsing(function (mixed $state) use ($customField, $multiple): array|null|int { + $column->castStateUsing(function (mixed $state) use ($customField, $multiple): array|int|UnresolvedValue|null { if (blank($state)) { return $multiple ? [] : null; } @@ -150,14 +151,13 @@ private function configureLookup(ImportColumn $column, CustomField $customField, /** * Resolve a single lookup value. */ - private function resolveLookupValue(CustomField $customField, mixed $value): int + private function resolveLookupValue(CustomField $customField, mixed $value): int|UnresolvedValue { try { $entity = Entities::getEntity($customField->lookup_type); $modelInstance = $entity->createModelInstance(); $primaryAttribute = $entity->getPrimaryAttribute(); - // Try to find by primary attribute $record = $modelInstance->newQuery() ->where($primaryAttribute, $value) ->first(); @@ -166,7 +166,6 @@ private function resolveLookupValue(CustomField $customField, mixed $value): int return (int) $record->getKey(); } - // Try to find by ID if numeric if (is_numeric($value)) { $record = $modelInstance->newQuery() ->where($modelInstance->getKeyName(), $value) @@ -177,51 +176,62 @@ private function resolveLookupValue(CustomField $customField, mixed $value): int } } - throw new RowImportFailedException( - sprintf("No %s record found matching '%s'", $customField->lookup_type, $value) - ); + return UnresolvedValue::make($value, sprintf( + "No %s found matching '%s' for %s.", + $this->lookupRecordLabel($customField), + is_scalar($value) ? (string) $value : gettype($value), + $customField->name, + )); } catch (Throwable $throwable) { - if ($throwable instanceof RowImportFailedException) { - throw $throwable; - } - - throw new RowImportFailedException('Error resolving lookup value: '.$throwable->getMessage(), $throwable->getCode(), $throwable); + return UnresolvedValue::make($value, 'Error resolving lookup value: '.$throwable->getMessage()); } } /** * Resolve multiple lookup values. */ - private function resolveLookupValues(CustomField $customField, array $values): array + private function resolveLookupValues(CustomField $customField, array $values): array|UnresolvedValue { $foundIds = []; $missingValues = []; foreach ($values as $value) { - try { - $id = $this->resolveLookupValue($customField, $value); - $foundIds[] = $id; - } catch (RowImportFailedException) { + $id = $this->resolveLookupValue($customField, $value); + + if ($id instanceof UnresolvedValue) { $missingValues[] = $value; + + continue; } + + $foundIds[] = $id; } if ($missingValues !== []) { - throw new RowImportFailedException( - sprintf('Could not find %s records: ', $customField->lookup_type). - implode(', ', $missingValues) - ); + return UnresolvedValue::make($values, sprintf( + 'Could not find a %s for %s: %s', + $this->lookupRecordLabel($customField), + $customField->name, + implode(', ', $missingValues), + )); } return $foundIds; } + private function lookupRecordLabel(CustomField $customField): string + { + return filled($customField->lookup_type) + ? $customField->lookup_type.' record' + : 'record'; + } + /** * Configure choice-based fields. */ private function configureChoices(ImportColumn $column, CustomField $customField, bool $multiple): void { - $column->castStateUsing(function (mixed $state) use ($customField, $multiple): array|null|int|string { + $column->castStateUsing(function (mixed $state) use ($customField, $multiple): array|int|string|UnresolvedValue|null { if (blank($state)) { return $multiple ? [] : null; } @@ -241,7 +251,7 @@ private function configureChoices(ImportColumn $column, CustomField $customField /** * Resolve a single choice value. */ - private function resolveChoiceValue(CustomField $customField, mixed $value): int|string|null + private function resolveChoiceValue(CustomField $customField, mixed $value): int|string|UnresolvedValue|null { // If already numeric, assume it's a choice ID if (is_numeric($value)) { @@ -254,15 +264,17 @@ private function resolveChoiceValue(CustomField $customField, mixed $value): int // Try case-insensitive match if (! $choice) { $choice = $customField->options->first( - fn (CustomFieldOption $opt): bool => strtolower((string) $opt->name) === strtolower($value) + fn (CustomFieldOption $opt): bool => strtolower((string) $opt->name) === strtolower((string) $value) ); } if (! $choice) { - throw new RowImportFailedException( - sprintf("Invalid choice '%s' for %s. Valid choices: ", $value, $customField->name). - $customField->options->pluck('name')->implode(', ') - ); + return UnresolvedValue::make($value, sprintf( + "Invalid choice '%s' for %s. Valid choices: %s", + is_scalar($value) ? (string) $value : gettype($value), + $customField->name, + $customField->options->pluck('name')->implode(', '), + )); } $key = $choice->getKey(); @@ -270,34 +282,32 @@ private function resolveChoiceValue(CustomField $customField, mixed $value): int return CustomFields::optionModelUsesStringKeys() ? (string) $key : $key; } - /** - * Resolve multiple choice values. - * - * @throws RowImportFailedException - */ - private function resolveChoiceValues(CustomField $customField, array $values): array + private function resolveChoiceValues(CustomField $customField, array $values): array|UnresolvedValue { $foundIds = []; $missingValues = []; foreach ($values as $value) { - try { - $id = $this->resolveChoiceValue($customField, $value); - if ($id !== null) { - $foundIds[] = $id; - } - } catch (RowImportFailedException) { + $id = $this->resolveChoiceValue($customField, $value); + + if ($id instanceof UnresolvedValue) { $missingValues[] = $value; + + continue; + } + + if ($id !== null) { + $foundIds[] = $id; } } if ($missingValues !== []) { - throw new RowImportFailedException( - sprintf('Invalid choices for %s: ', $customField->name). - implode(', ', $missingValues). - '. Valid choices: '. - $customField->options->pluck('name')->implode(', ') - ); + return UnresolvedValue::make($values, sprintf( + 'Invalid choices for %s: %s. Valid choices: %s', + $customField->name, + implode(', ', $missingValues), + $customField->options->pluck('name')->implode(', '), + )); } return $foundIds; @@ -306,9 +316,9 @@ private function resolveChoiceValues(CustomField $customField, array $values): a /** * Configure boolean fields with string coercion for CSV values. */ - private function configureBoolean(ImportColumn $column): void + private function configureBoolean(ImportColumn $column, CustomField $customField): void { - $column->castStateUsing(function (mixed $state): ?bool { + $column->castStateUsing(function (mixed $state) use ($customField): bool|UnresolvedValue|null { if (blank($state)) { return null; } @@ -317,58 +327,108 @@ private function configureBoolean(ImportColumn $column): void return $state; } - $normalized = strtolower(trim((string) $state)); - - return match ($normalized) { + return match (strtolower(trim((string) $state))) { '1', 'true', 'yes', 'on' => true, '0', 'false', 'no', 'off' => false, - default => null, + default => UnresolvedValue::make($state, sprintf( + "'%s' is not a valid value for %s. Use true or false (accepted: true, false, 1, 0, yes, no, on, off).", + $state, + $customField->name, + )), }; }); $column->example('true or false'); } - /** - * Configure date fields. - */ - private function configureDate(ImportColumn $column): void + private function configureDate(ImportColumn $column, CustomField $customField): void + { + $column->castStateUsing(fn (mixed $state): string|UnresolvedValue|null => $this->parseDate($state, false, $customField)); + + $column->example(CustomFields::importDateFormat()->getExamples()[0]); + } + + private function configureDateTime(ImportColumn $column, CustomField $customField): void { - $column->castStateUsing(function (mixed $state): ?string { + $column->castStateUsing(fn (mixed $state): string|UnresolvedValue|null => $this->parseDate($state, true, $customField)); + + $column->example(CustomFields::importDateFormat()->getExamples(withTime: true)[0]); + } + + private function parseDate(mixed $state, bool $withTime, CustomField $customField): string|UnresolvedValue|null + { + if (blank($state)) { + return null; + } + + $format = CustomFields::importDateFormat(); + $parsed = $format->parse((string) $state, $withTime); + + if (! $parsed instanceof CarbonImmutable) { + return UnresolvedValue::make($state, sprintf( + "'%s' is not a valid date for %s. Expected format: %s.", + $state, + $customField->name, + implode(' or ', $format->getExamples($withTime)), + )); + } + + return $parsed->format($withTime ? 'Y-m-d H:i:s' : 'Y-m-d'); + } + + private function configureNumeric(ImportColumn $column, CustomField $customField): void + { + $column->castStateUsing(function (mixed $state) use ($customField): float|UnresolvedValue|null { if (blank($state)) { return null; } - try { - // Try to parse DD/MM/YYYY format first - if (preg_match('#^(\d{1,2})/(\d{1,2})/(\d{4})$#', $state, $matches)) { - return Carbon::createFromFormat('d/m/Y', $state)->format('Y-m-d'); - } + $format = CustomFields::importNumberFormat(); + $parsed = $format->parse((string) $state); - // Fall back to Carbon's default parsing - return Carbon::parse($state)->format('Y-m-d'); - } catch (Exception) { - return null; + if ($parsed === null) { + return UnresolvedValue::make($state, sprintf( + "'%s' is not a valid number for %s. Expected format: %s.", + $state, + $customField->name, + $format->getExample(), + )); } + + return $parsed; }); + + $column->example('99.99'); } /** - * Configure datetime fields. + * Field types registered by the host application supply their own import + * transformers. A throw from one would abort the whole row inside Filament's cast + * loop, taking every other column's error with it, so it is converted into a value + * the validator can report alongside them. */ - private function configureDateTime(ImportColumn $column): void + public function wrapTransformer(Closure $transformer, CustomField $customField): Closure { - $column->castStateUsing(function (mixed $state): ?string { - if (blank($state)) { - return null; + return function (mixed $state) use ($transformer, $customField): mixed { + try { + $result = $transformer($state); + } catch (Throwable $throwable) { + $result = UnresolvedValue::make($state, $throwable->getMessage()); } - try { - return Carbon::parse($state)->format('Y-m-d H:i:s'); - } catch (Exception) { - return null; + if (! $result instanceof UnresolvedValue) { + return $result; } - }); + + // A field type does not know which field it is configuring, and ImportCsv + // flattens the error bag to message text, so an unnamed reason reaches the + // user with no way to tell which column to fix. + if (str_contains($result->reason, (string) $customField->name)) { + return $result; + } + + return UnresolvedValue::make($result->raw, $customField->name.': '.$result->reason); + }; } /** @@ -443,16 +503,24 @@ private function setChoiceExamples(ImportColumn $column, CustomField $customFiel /** * Finalize column configuration. + * + * `bail` plus the sentinel rule go first so an unresolvable cell reports its own + * reason once, instead of also tripping the type rule behind it with a message that + * describes the cast's fallback rather than the user's mistake. */ private function finalize(ImportColumn $column, CustomField $customField): ImportColumn { - $rules = app(ValidationService::class)->getValidationRules($customField); - - if ($rules !== []) { - $column->rules($rules); - } + $column->rules([ + 'bail', + new RejectsUnresolvedValue, + ...app(ValidationService::class)->getValidationRules($customField), + ]); $column->fillRecordUsing(function (mixed $state, mixed $record) use ($customField): void { + if ($state instanceof UnresolvedValue) { + return; + } + ImportDataStorage::set($record, $customField->code, $state); }); diff --git a/src/Imports/UnresolvedValue.php b/src/Imports/UnresolvedValue.php new file mode 100644 index 00000000..361f6534 --- /dev/null +++ b/src/Imports/UnresolvedValue.php @@ -0,0 +1,33 @@ +raw) ? (string) $this->raw : ''; + } +} diff --git a/src/Rules/RejectsUnresolvedValue.php b/src/Rules/RejectsUnresolvedValue.php new file mode 100644 index 00000000..2c09d434 --- /dev/null +++ b/src/Rules/RejectsUnresolvedValue.php @@ -0,0 +1,21 @@ +reason); + } +} diff --git a/tests/Feature/Imports/ImportArchitectureTest.php b/tests/Feature/Imports/ImportArchitectureTest.php index 5aa6e704..9f267fe2 100644 --- a/tests/Feature/Imports/ImportArchitectureTest.php +++ b/tests/Feature/Imports/ImportArchitectureTest.php @@ -2,12 +2,12 @@ declare(strict_types=1); -use Filament\Actions\Imports\Exceptions\RowImportFailedException; use Filament\Actions\Imports\ImportColumn; use Illuminate\Database\Eloquent\Model; use Relaticle\CustomFields\Enums\FieldDataType; use Relaticle\CustomFields\Filament\Integration\Support\Imports\ImportColumnConfigurator; use Relaticle\CustomFields\Filament\Integration\Support\Imports\ImportDataStorage; +use Relaticle\CustomFields\Imports\UnresolvedValue; use Relaticle\CustomFields\Models\CustomField; use Relaticle\CustomFields\Models\CustomFieldOption; @@ -203,17 +203,53 @@ $castCallback = $property->getValue($column); if ($castCallback) { - // Test various date formats + // The default convention is ISO, so the Y-m-d family parses. expect($castCallback('2024-01-15'))->toBe('2024-01-15'); - expect($castCallback('15/01/2024'))->toBe('2024-01-15'); - expect($castCallback('January 15, 2024'))->toBe('2024-01-15'); expect($castCallback('2024-01-15 10:30:00'))->toBe('2024-01-15'); expect($castCallback(''))->toBeNull(); expect($castCallback(null))->toBeNull(); - expect($castCallback('invalid-date'))->toBeNull(); + + // Day-first and textual forms are ambiguous or localised, so ISO does not + // guess at them. Declare `european` or `american` to accept them. + expect($castCallback('15/01/2024'))->toBeInstanceOf(UnresolvedValue::class); + expect($castCallback('January 15, 2024'))->toBeInstanceOf(UnresolvedValue::class); + + // A cell the cast cannot honestly convert is reported, not swallowed. + // 13/45/2024 used to become 2027-09-13. + expect($castCallback('invalid-date'))->toBeInstanceOf(UnresolvedValue::class); + expect($castCallback('13/45/2024'))->toBeInstanceOf(UnresolvedValue::class); } }); +it('reads day-first and textual dates once a convention is declared', function (): void { + config()->set('custom-fields.imports.date_format', 'european'); + + $configurator = new ImportColumnConfigurator; + + $field = new CustomField([ + 'name' => 'Date Field', + 'code' => 'date_field', + 'type' => 'date', + ]); + + $field->validation_rules = collect([]); + $field->options = collect([]); + + $column = ImportColumn::make('test_date'); + $configurator->configure($column, $field); + + $reflection = new ReflectionObject($column); + $property = $reflection->getProperty('castStateUsing'); + $property->setAccessible(true); + + $castCallback = $property->getValue($column); + + expect($castCallback('15/01/2024'))->toBe('2024-01-15') + ->and($castCallback('January 15, 2024'))->toBe('2024-01-15') + ->and($castCallback('2024-01-15'))->toBe('2024-01-15') + ->and($castCallback('13/45/2024'))->toBeInstanceOf(UnresolvedValue::class); +}); + /** * Test option resolution with case-insensitive matching */ @@ -258,9 +294,9 @@ expect($castCallback('GREEN'))->toBe(3); expect($castCallback('2'))->toBe(2); // Numeric should work - // Test invalid option throws exception - expect(fn () => $castCallback('Yellow')) - ->toThrow(RowImportFailedException::class); + // An invalid option is reported through the validator instead of throwing, + // so the rest of the row's columns still get to report their own errors. + expect($castCallback('Yellow'))->toBeInstanceOf(UnresolvedValue::class); } }); diff --git a/tests/Feature/Imports/ImportContractConformanceTest.php b/tests/Feature/Imports/ImportContractConformanceTest.php new file mode 100644 index 00000000..cd3ba4a0 --- /dev/null +++ b/tests/Feature/Imports/ImportContractConformanceTest.php @@ -0,0 +1,178 @@ +actingAs(User::factory()->create()); + + config()->set('custom-fields.field_type_configuration', FieldTypeConfigurator::configure() + ->enabled([]) + ->disabled([]) + ->discover(true) + ->cache(enabled: false)); + + $this->section = CustomFieldSection::factory()->forEntityType(Post::class)->create(); +}); + +function conformanceCast(string $type, mixed $cell): mixed +{ + $field = CustomField::factory()->create([ + 'custom_field_section_id' => test()->section->getKey(), + 'entity_type' => Post::class, + 'name' => 'Conformance '.$type, + 'code' => 'conformance_'.str_replace('-', '_', $type), + 'type' => $type, + ]); + + foreach (['Alpha', 'Beta'] as $name) { + $field->options()->create(['name' => $name]); + } + + return app(ImportColumnConfigurator::class) + ->configure(ImportColumn::make('custom_fields_'.$field->code), $field->refresh()) + ->castState($cell); +} + +/** @return array */ +function conformanceFieldTypes(): array +{ + return CustomFieldsType::toCollection() + ->map(fn (mixed $fieldType): string => $fieldType->key) + ->values() + ->all(); +} + +it('never throws out of a cast, for any registered field type', function (): void { + $threw = []; + + foreach (conformanceFieldTypes() as $type) { + try { + conformanceCast($type, 'Q/A'); + } catch (Throwable $throwable) { + $threw[$type] = $throwable->getMessage(); + } + } + + expect($threw)->toBe([]); +}); + +it('never silently coerces a garbage cell, for any registered field type', function (): void { + $swallowed = []; + + foreach (conformanceFieldTypes() as $type) { + $state = conformanceCast($type, 'Q/A'); + + if ($state instanceof UnresolvedValue) { + continue; + } + + // Text-shaped columns legitimately accept any string. They may wrap or + // normalise it (rich editor emits `

Q/A

`), but the value must survive. + $flattened = is_array($state) ? implode(' ', array_map(strval(...), $state)) : $state; + + if (is_string($flattened) && str_contains($flattened, 'Q/A')) { + continue; + } + + $swallowed[$type] = var_export($state, true); + } + + expect($swallowed)->toBe([]); +}); + +it('names the offending field in every rejection, since ImportCsv drops attribute names', function (): void { + $unnamed = []; + + foreach (conformanceFieldTypes() as $type) { + $state = conformanceCast($type, 'Q/A'); + + if (! $state instanceof UnresolvedValue) { + continue; + } + + if ($state->reason !== '' && str_contains($state->reason, 'Conformance '.$type)) { + continue; + } + + $unnamed[$type] = $state->reason; + } + + expect($unnamed)->toBe([]); +}); + +it('still treats a blank cell as null, for any registered field type', function (): void { + $rejected = []; + + foreach (conformanceFieldTypes() as $type) { + $state = conformanceCast($type, ''); + + if ($state === null || $state === []) { + continue; + } + + $rejected[$type] = var_export($state, true); + } + + expect($rejected)->toBe([]); +}); + +function conformanceFieldFor(string $name): CustomField +{ + return CustomField::factory()->create([ + 'custom_field_section_id' => test()->section->getKey(), + 'entity_type' => Post::class, + 'name' => $name, + 'code' => str($name)->snake()->toString(), + 'type' => 'text', + ]); +} + +it('contains a throwing field-type transformer instead of letting it kill the row', function (): void { + $wrapped = app(ImportColumnConfigurator::class)->wrapTransformer( + fn (): never => throw new RuntimeException('transformer exploded'), + conformanceFieldFor('Monthly Stipend'), + ); + + $state = $wrapped('anything'); + + expect($state)->toBeInstanceOf(UnresolvedValue::class) + ->and($state->reason)->toBe('Monthly Stipend: transformer exploded') + ->and($state->raw)->toBe('anything'); +}); + +it('names the field when a transformer returns an unnamed rejection', function (): void { + $wrapped = app(ImportColumnConfigurator::class)->wrapTransformer( + fn (mixed $state): UnresolvedValue => UnresolvedValue::make($state, 'is not a valid amount.'), + conformanceFieldFor('Monthly Stipend'), + ); + + expect($wrapped('Q/A')->reason)->toBe('Monthly Stipend: is not a valid amount.'); +}); + +it('leaves a rejection that already names the field alone', function (): void { + $wrapped = app(ImportColumnConfigurator::class)->wrapTransformer( + fn (mixed $state): UnresolvedValue => UnresolvedValue::make($state, "'Q/A' is not valid for Monthly Stipend."), + conformanceFieldFor('Monthly Stipend'), + ); + + expect($wrapped('Q/A')->reason)->toBe("'Q/A' is not valid for Monthly Stipend."); +}); + +it('leaves a well-behaved transformer untouched', function (): void { + $wrapped = app(ImportColumnConfigurator::class)->wrapTransformer( + fn (mixed $state): string => strtoupper((string) $state), + conformanceFieldFor('Referral Source'), + ); + + expect($wrapped('street outreach'))->toBe('STREET OUTREACH'); +}); diff --git a/tests/Feature/Imports/ImportDateFormatTest.php b/tests/Feature/Imports/ImportDateFormatTest.php new file mode 100644 index 00000000..d2053672 --- /dev/null +++ b/tests/Feature/Imports/ImportDateFormatTest.php @@ -0,0 +1,77 @@ +parse($cell)?->format('Y-m-d'))->toBe('2024-01-15'); +})->with([ + '2024-01-15', + '15/01/2024', + '15-01-2024', + '15.01.2024', + 'January 15, 2024', + '15 January 2024', + '2024-01-15 10:30:00', +]); + +it('rejects calendar-invalid and out-of-range dates', function (string $cell): void { + expect(ImportDateFormat::EUROPEAN->parse($cell))->toBeNull(); +})->with([ + '31/02/2024', + '13/45/2024', + '2024-02-31', + '2024-13-01', + '99/99/9999', +]); + +it('rejects free text, two-digit years and blanks', function (string $cell): void { + expect(ImportDateFormat::EUROPEAN->parse($cell))->toBeNull(); +})->with([ + 'invalid-date', + 'Q/A', + 'next tuesday', + '1/1/24', + '', + ' ', +]); + +it('reads an ambiguous date according to the declared convention', function (): void { + expect(ImportDateFormat::EUROPEAN->parse('3/4/2024')?->format('Y-m-d'))->toBe('2024-04-03') + ->and(ImportDateFormat::AMERICAN->parse('3/4/2024')?->format('Y-m-d'))->toBe('2024-03-04') + ->and(ImportDateFormat::ISO->parse('3/4/2024'))->toBeNull(); +}); + +it('accepts ISO under every convention so a Y-m-d cell is never re-read', function (ImportDateFormat $format): void { + expect($format->parse('2024-01-15')?->format('Y-m-d'))->toBe('2024-01-15'); +})->with(ImportDateFormat::cases()); + +it('parses every example it advertises', function (ImportDateFormat $format, bool $withTime): void { + foreach ($format->getExamples($withTime) as $example) { + expect($format->parse($example, $withTime)) + ->not->toBeNull("{$format->value} advertises '{$example}' but cannot parse it"); + } +})->with([ + 'iso date' => [ImportDateFormat::ISO, false], + 'iso datetime' => [ImportDateFormat::ISO, true], + 'european date' => [ImportDateFormat::EUROPEAN, false], + 'european datetime' => [ImportDateFormat::EUROPEAN, true], + 'american date' => [ImportDateFormat::AMERICAN, false], + 'american datetime' => [ImportDateFormat::AMERICAN, true], +]); + +it('keeps ISO to the Y-m-d family so a declared ISO column means exactly that', function (string $cell): void { + expect(ImportDateFormat::ISO->parse($cell))->toBeNull(); +})->with(['15/01/2024', '01/15/2024', 'January 15, 2024', '15 January 2024']); + +it('parses datetimes when asked', function (): void { + expect(ImportDateFormat::ISO->parse('2024-01-15 10:30:00', withTime: true)?->format('Y-m-d H:i:s')) + ->toBe('2024-01-15 10:30:00') + ->and(ImportDateFormat::EUROPEAN->parse('15/01/2024 10:30:00', withTime: true)?->format('Y-m-d H:i:s')) + ->toBe('2024-01-15 10:30:00'); +}); + +it('rejects an overflowing datetime', function (): void { + expect(ImportDateFormat::EUROPEAN->parse('31/02/2024 10:30:00', withTime: true))->toBeNull(); +}); diff --git a/tests/Feature/Imports/ImportNumberFormatTest.php b/tests/Feature/Imports/ImportNumberFormatTest.php new file mode 100644 index 00000000..e58f51e8 --- /dev/null +++ b/tests/Feature/Imports/ImportNumberFormatTest.php @@ -0,0 +1,49 @@ +parse($cell))->toBe($expected); +})->with([ + ['1234.56', 1234.56], + ['1,234.56', 1234.56], + ['1 234.56', 1234.56], + ['0', 0.0], + ['-5.5', -5.5], + ['12abc', null], + ['Q/A', null], + ['1.2.3', null], + ['$1,234.56', null], + ['', null], +]); + +it('parses comma-decimal numbers', function (string $cell, ?float $expected): void { + expect(ImportNumberFormat::COMMA->parse($cell))->toBe($expected); +})->with([ + ['1234,56', 1234.56], + ['1.234,56', 1234.56], + ['Q/A', null], +]); + +it('strips a currency symbol or code at either edge', function (string $cell, ?float $expected): void { + expect(ImportNumberFormat::POINT->parse($cell, stripCurrencySymbol: true))->toBe($expected); +})->with([ + ['$1,234.56', 1234.56], + ['-$5.50', -5.5], + ['1234.56 EUR', 1234.56], + ['USD 99.99', 99.99], + ['12$34', null], + ['Q/A', null], +]); + +/** + * `12abc` and `1234.56 EUR` are the same shape, so a currency column cannot reject one + * and accept the other without a currency allowlist. This matches what the currency + * field did before, and a plain number column still rejects it. + */ +it('accepts trailing text on a currency column, the price of accepting a currency code', function (): void { + expect(ImportNumberFormat::POINT->parse('12abc', stripCurrencySymbol: true))->toBe(12.0) + ->and(ImportNumberFormat::POINT->parse('12abc'))->toBeNull(); +}); diff --git a/tests/Feature/Imports/ImportValidationContractTest.php b/tests/Feature/Imports/ImportValidationContractTest.php new file mode 100644 index 00000000..e040839f --- /dev/null +++ b/tests/Feature/Imports/ImportValidationContractTest.php @@ -0,0 +1,277 @@ +up(); + } + + $this->actingAs($user = User::factory()->create()); + $this->user = $user; + $this->section = CustomFieldSection::factory()->forEntityType(Post::class)->create(); +}); + +function contractField(string $type, string $code, array $options = [], bool $required = false): CustomField +{ + $field = CustomField::factory()->create([ + 'custom_field_section_id' => test()->section->getKey(), + 'entity_type' => Post::class, + 'name' => str($code)->headline()->toString(), + 'code' => $code, + 'type' => $type, + 'validation_rules' => $required ? ['required' => true] : [], + ]); + + foreach ($options as $name) { + $field->options()->create(['name' => $name]); + } + + return $field->refresh(); +} + +/** + * Runs rows through the importer the way `ImportCsv` does: a ValidationException fails + * the row and is recorded, anything else is a successful import. + * + * @return array{failures: array, imported: int} + */ +function runPostImport(array $rows): array +{ + $import = Import::create([ + 'user_id' => test()->user->getKey(), + 'file_name' => 'contract.csv', + 'file_path' => 'imports/contract.csv', + 'importer' => PostImporter::class, + 'total_rows' => count($rows), + 'processed_rows' => 0, + 'successful_rows' => 0, + ]); + + $columnMap = collect(PostImporter::getColumns()) + ->mapWithKeys(fn ($column): array => [$column->getName() => $column->getName()]) + ->all(); + + $importer = new PostImporter($import, $columnMap, []); + + $failures = []; + $imported = 0; + + foreach ($rows as $row) { + try { + $importer($row); + $imported++; + } catch (ValidationException $exception) { + $message = collect($exception->errors())->flatten()->implode(' '); + + FailedImportRow::create([ + 'import_id' => $import->getKey(), + 'data' => $row, + 'validation_error' => $message, + ]); + + $failures[] = $message; + } + } + + return ['failures' => $failures, 'imported' => $imported]; +} + +it('reports every invalid custom field in the row, not just the first', function (): void { + contractField('select', 'housing_program', ['Rapid Rehousing', 'Permanent Supportive']); + contractField('toggle', 'ra_eligible'); + contractField('currency', 'monthly_stipend'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_housing_program' => 'Q/A', + 'custom_fields_ra_eligible' => 'Q/A', + 'custom_fields_monthly_stipend' => 'Q/A', + ]]); + + expect($result['imported'])->toBe(0) + ->and($result['failures'])->toHaveCount(1); + + $message = $result['failures'][0]; + + expect($message)->toContain('Housing Program') + ->and($message)->toContain('Ra Eligible') + ->and($message)->toContain('Monthly Stipend'); +}); + +it('reports custom field errors together with ordinary column errors', function (): void { + contractField('select', 'housing_program', ['Rapid Rehousing']); + + $result = runPostImport([[ + 'title' => '', + 'custom_fields_housing_program' => 'Q/A', + ]]); + + expect($result['failures'][0])->toContain('title') + ->and($result['failures'][0])->toContain('Housing Program'); +}); + +it('keeps the message quality it had for a single invalid choice', function (): void { + contractField('select', 'housing_program', ['Rapid Rehousing', 'Permanent Supportive']); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_housing_program' => 'Q/A', + ]]); + + expect($result['failures'][0]) + ->toContain("Invalid choice 'Q/A' for Housing Program") + ->toContain('Rapid Rehousing') + ->toContain('Permanent Supportive'); +}); + +it('no longer imports a garbage toggle as a definite no', function (): void { + $toggle = contractField('toggle', 'ra_eligible'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_ra_eligible' => 'Q/A', + ]]); + + expect($result['imported'])->toBe(0) + ->and(Post::count())->toBe(0) + ->and($result['failures'][0])->toContain('true or false'); +}); + +it('no longer imports a garbage amount as zero', function (): void { + contractField('currency', 'monthly_stipend'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_monthly_stipend' => 'Q/A', + ]]); + + expect($result['imported'])->toBe(0) + ->and(Post::count())->toBe(0); +}); + +it('no longer fabricates a date from an impossible one', function (): void { + contractField('date', 'move_in_target'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_move_in_target' => '13/45/2024', + ]]); + + expect($result['imported'])->toBe(0) + ->and($result['failures'][0])->toContain('not a valid date'); +}); + +it('imports a clean row and stores every value', function (): void { + $program = contractField('select', 'housing_program', ['Rapid Rehousing', 'Permanent Supportive']); + $toggle = contractField('toggle', 'ra_eligible'); + $stipend = contractField('currency', 'monthly_stipend'); + $moveIn = contractField('date', 'move_in_target'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_housing_program' => 'Permanent Supportive', + 'custom_fields_ra_eligible' => 'yes', + 'custom_fields_monthly_stipend' => '$1,234.56', + 'custom_fields_move_in_target' => '2024-01-15', + ]]); + + expect($result['failures'])->toBe([]) + ->and($result['imported'])->toBe(1); + + $post = Post::latest('id')->first()->load('customFieldValues'); + + expect($post->getCustomFieldValue($program)) + ->toBe($program->options->firstWhere('name', 'Permanent Supportive')->getKey()) + ->and($post->getCustomFieldValue($toggle))->toBeTrue() + ->and($post->getCustomFieldValue($stipend))->toBe(1234.56) + ->and($post->getCustomFieldValue($moveIn)->format('Y-m-d'))->toBe('2024-01-15'); +}); + +it('reads a day-first date once the convention is declared', function (): void { + config()->set('custom-fields.imports.date_format', 'european'); + + $moveIn = contractField('date', 'move_in_target'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_move_in_target' => '15/01/2024', + ]]); + + expect($result['failures'])->toBe([]) + ->and(Post::latest('id')->first()->load('customFieldValues')->getCustomFieldValue($moveIn)->format('Y-m-d')) + ->toBe('2024-01-15'); +}); + +it('does not guess at a day-first date under the ISO default', function (): void { + contractField('date', 'move_in_target'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_move_in_target' => '15/01/2024', + ]]); + + expect($result['imported'])->toBe(0) + ->and($result['failures'][0])->toContain('not a valid date'); +}); + +it('still imports a row whose optional custom fields are blank', function (): void { + contractField('select', 'housing_program', ['Rapid Rehousing']); + contractField('toggle', 'ra_eligible'); + contractField('currency', 'monthly_stipend'); + contractField('date', 'move_in_target'); + + $result = runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_housing_program' => '', + 'custom_fields_ra_eligible' => '', + 'custom_fields_monthly_stipend' => '', + 'custom_fields_move_in_target' => '', + ]]); + + expect($result['failures'])->toBe([]) + ->and($result['imported'])->toBe(1); +}); + +it('fails only the bad rows and keeps importing the rest', function (): void { + contractField('currency', 'monthly_stipend'); + + $result = runPostImport([ + ['title' => 'Good one', 'custom_fields_monthly_stipend' => '100.00'], + ['title' => 'Bad one', 'custom_fields_monthly_stipend' => 'Q/A'], + ['title' => 'Good two', 'custom_fields_monthly_stipend' => '200.00'], + ]); + + expect($result['imported'])->toBe(2) + ->and($result['failures'])->toHaveCount(1) + ->and(Post::pluck('title')->all())->toBe(['Good one', 'Good two']); +}); + +it('preserves the failed row so the user can correct and re-upload', function (): void { + contractField('toggle', 'ra_eligible'); + + runPostImport([[ + 'title' => 'Smith household', + 'custom_fields_ra_eligible' => 'Q/A', + ]]); + + expect(FailedImportRow::first()->data)->toBe([ + 'title' => 'Smith household', + 'custom_fields_ra_eligible' => 'Q/A', + ]); +}); diff --git a/tests/Fixtures/Imports/PostImporter.php b/tests/Fixtures/Imports/PostImporter.php new file mode 100644 index 00000000..308956c8 --- /dev/null +++ b/tests/Fixtures/Imports/PostImporter.php @@ -0,0 +1,52 @@ + + */ + public static function getColumns(): array + { + return [ + ImportColumn::make('title') + ->requiredMapping() + ->rules(['required', 'string']), + ...CustomFields::importer()->forModel(new Post)->columns()->all(), + ]; + } + + public function resolveRecord(): ?Model + { + $post = new Post; + $post->author_id = $this->import->user_id; + + return $post; + } + + public static function getCompletedNotificationBody(Import $import): string + { + return 'done'; + } + + protected function afterSave(): void + { + CustomFields::importer()->forModel($this->record)->saveValues(); + } +}