-
Notifications
You must be signed in to change notification settings - Fork 21
feat: add FieldDataType::FILE to fix file upload validation #101
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,9 @@ | |
| namespace Relaticle\CustomFields\Services; | ||
|
|
||
| use Relaticle\CustomFields\Data\ValidationRuleData; | ||
| use Relaticle\CustomFields\Enums\FieldDataType; | ||
| use Relaticle\CustomFields\Enums\ValidationRule; | ||
| use Relaticle\CustomFields\Facades\CustomFieldsType; | ||
| use Relaticle\CustomFields\FieldTypeSystem\FieldManager; | ||
| use Relaticle\CustomFields\Models\CustomField; | ||
| use Relaticle\CustomFields\Models\CustomFieldValue; | ||
|
|
@@ -97,6 +99,13 @@ private function convertUserRulesToValidatorFormat(?DataCollection $rules, Custo | |
| */ | ||
| public function getDatabaseValidationRules(string $fieldType, bool $isEncrypted = false): array | ||
| { | ||
| // File types validate the uploaded file, not the stored path | ||
| $fieldTypeData = CustomFieldsType::getFieldType($fieldType); | ||
|
|
||
| if ($fieldTypeData?->dataType === FieldDataType::FILE) { | ||
| return []; | ||
| } | ||
|
|
||
| // Determine the database column for this field type | ||
| $columnName = CustomFieldValue::getValueColumn($fieldType); | ||
|
|
||
|
|
@@ -202,6 +211,13 @@ private function mergeAllValidationRules(array $fieldTypeDefaults, array $userRu | |
| // Add user rules (can override or supplement defaults) | ||
| $mergedRules = $this->combineRules($mergedRules, $userRules); | ||
|
|
||
| // File types validate the uploaded file, not the stored path — skip DB constraints | ||
| $fieldTypeData = CustomFieldsType::getFieldType($fieldType); | ||
|
|
||
| if ($fieldTypeData?->dataType === FieldDataType::FILE) { | ||
| return $mergedRules; | ||
| } | ||
|
Comment on lines
+214
to
+219
|
||
|
|
||
| // Apply database constraint rules using existing logic | ||
| $columnName = CustomFieldValue::getValueColumn($fieldType); | ||
| $dbConstraints = DatabaseFieldConstraints::getConstraintsForColumn($columnName); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| use Relaticle\CustomFields\Enums\FieldDataType; | ||
| use Relaticle\CustomFields\FieldTypeSystem\FieldTypeConfigurator; | ||
| use Relaticle\CustomFields\Models\CustomField; | ||
| use Relaticle\CustomFields\Models\CustomFieldSection; | ||
| use Relaticle\CustomFields\Models\CustomFieldValue; | ||
| use Relaticle\CustomFields\Services\ValidationService; | ||
| use Relaticle\CustomFields\Tests\Fixtures\Models\Post; | ||
| use Relaticle\CustomFields\Tests\Fixtures\Models\User; | ||
|
|
||
| beforeEach(function (): void { | ||
| $this->user = User::factory()->create(); | ||
| $this->actingAs($this->user); | ||
|
|
||
| config()->set('custom-fields.field_type_configuration', FieldTypeConfigurator::configure() | ||
| ->enabled([]) | ||
| ->disabled([]) | ||
| ->discover(true) | ||
| ->cache(enabled: false)); | ||
|
|
||
| $this->section = CustomFieldSection::factory() | ||
| ->forEntityType(Post::class) | ||
| ->create(); | ||
| }); | ||
|
|
||
| it('resolves FILE data type to string_value column', function (): void { | ||
| $field = CustomField::factory()->create([ | ||
| 'custom_field_section_id' => $this->section->getKey(), | ||
| 'entity_type' => Post::class, | ||
| 'code' => 'document', | ||
| 'type' => 'file-upload', | ||
| ]); | ||
|
|
||
| $column = CustomFieldValue::getValueColumn($field->type); | ||
|
|
||
| expect($column)->toBe('string_value'); | ||
| }); | ||
|
|
||
| it('does not apply string database constraints to file upload fields', function (): void { | ||
| $field = CustomField::factory()->create([ | ||
| 'custom_field_section_id' => $this->section->getKey(), | ||
| 'entity_type' => Post::class, | ||
| 'code' => 'attachment', | ||
| 'type' => 'file-upload', | ||
| ]); | ||
|
|
||
| $service = app(ValidationService::class); | ||
| $rules = $service->getValidationRules($field); | ||
|
|
||
| expect($rules) | ||
| ->toContain('file') | ||
| ->not->toContain('string') | ||
| ->not->toContain('max:255'); | ||
| }); | ||
|
|
||
| it('returns empty database validation rules for file types', function (): void { | ||
| $service = app(ValidationService::class); | ||
| $dbRules = $service->getDatabaseValidationRules('file-upload'); | ||
|
|
||
| expect($dbRules)->toBe([]); | ||
| }); | ||
|
|
||
| it('has FILE case in FieldDataType enum', function (): void { | ||
| $file = FieldDataType::FILE; | ||
|
|
||
| expect($file->value)->toBe('file') | ||
| ->and($file->isChoiceField())->toBeFalse() | ||
| ->and($file->isMultiChoiceField())->toBeFalse(); | ||
| }); |
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.
CustomFieldsType::getFieldType() delegates to FieldManager::getFieldType(), which rebuilds the full field type collection via toCollection() each call (instantiates all field type definitions). Adding this lookup here introduces a potentially expensive operation on every validation call. Consider avoiding CustomFieldsType::getFieldType() in this hot path by reusing the FieldManager instance/field type instance already accessed in getFieldTypeDefaultRules(), or by passing the resolved dataType/FieldTypeData into getDatabaseValidationRules so this method can decide FILE vs non-FILE without another full rebuild.