Skip to content

feat: add FieldDataType::FILE to fix file upload validation#101

Merged
ManukMinasyan merged 1 commit into2.xfrom
fix/file-upload-validation-2x
Mar 6, 2026
Merged

feat: add FieldDataType::FILE to fix file upload validation#101
ManukMinasyan merged 1 commit into2.xfrom
fix/file-upload-validation-2x

Conversation

@ManukMinasyan
Copy link
Collaborator

Summary

Backport of #100 to 2.x.

  • Adds FieldDataType::FILE enum case with FieldSchema::file() factory method
  • FILE maps to string_value for storage but skips database column constraints in validation
  • Fixes file upload fields rejecting files >255 KB due to string_value column's max:255 being interpreted as kilobytes when combined with the file validation rule

Test plan

  • New FileUploadValidationTest with 4 tests
  • No test regressions (pre-existing 2.x failures unchanged)

File upload fields stored paths in string_value, causing
DatabaseFieldConstraints to add max:255 (characters) which Laravel
interpreted as 255 KB when combined with the 'file' rule.

Adds FieldDataType::FILE with FieldSchema::file() factory method.
FILE maps to string_value for storage but skips database column
constraints, since validation runs against the uploaded file,
not the stored path.
Copilot AI review requested due to automatic review settings March 6, 2026 18:08
@ManukMinasyan ManukMinasyan merged commit 18a87dd into 2.x Mar 6, 2026
3 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Backports the “FILE” custom field data type to 2.x to prevent file uploads from being incorrectly constrained by string_value (VARCHAR 255) database validation rules (notably max:255 being interpreted as KB when combined with Laravel’s file rule).

Changes:

  • Adds FieldDataType::FILE and FieldSchema::file() for semantic file-upload field configuration.
  • Maps FILE storage to string_value while skipping DB column constraint validation for file upload validation flows.
  • Updates file-upload field type and scaffolding to use the new FILE data type; adds integration tests covering the behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/Feature/Integration/FileUploadValidationTest.php Adds regression tests asserting DB constraints aren’t applied to file uploads and FILE enum exists.
src/Services/ValidationService.php Skips DB constraint rules for FILE data types during validation rule assembly.
src/Models/CustomFieldValue.php Maps FieldDataType::FILE to string_value for persistence.
src/FieldTypeSystem/FieldSchema.php Introduces FieldSchema::file() factory mapping to FieldDataType::FILE.
src/FieldTypeSystem/Definitions/FileUploadFieldType.php Switches file upload field definition to FieldSchema::file().
src/Enums/FieldDataType.php Adds FILE enum case and compatible visibility operators.
src/Console/Commands/MakeFieldTypeCommand.php Enables scaffolding of FILE types using file() and Filament FileUpload.

Comment on lines +102 to +105
// File types validate the uploaded file, not the stored path
$fieldTypeData = CustomFieldsType::getFieldType($fieldType);

if ($fieldTypeData?->dataType === FieldDataType::FILE) {
Copy link

Copilot AI Mar 6, 2026

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.

Suggested change
// File types validate the uploaded file, not the stored path
$fieldTypeData = CustomFieldsType::getFieldType($fieldType);
if ($fieldTypeData?->dataType === FieldDataType::FILE) {
// Cache the FILE/non-FILE decision per field type to avoid repeated expensive lookups.
static $isFileTypeCache = [];
if (! array_key_exists($fieldType, $isFileTypeCache)) {
$fieldTypeData = CustomFieldsType::getFieldType($fieldType);
$isFileTypeCache[$fieldType] = $fieldTypeData?->dataType === FieldDataType::FILE;
}
// File types validate the uploaded file, not the stored path
if ($isFileTypeCache[$fieldType]) {

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +219
// File types validate the uploaded file, not the stored path — skip DB constraints
$fieldTypeData = CustomFieldsType::getFieldType($fieldType);

if ($fieldTypeData?->dataType === FieldDataType::FILE) {
return $mergedRules;
}
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repeats a CustomFieldsType::getFieldType() lookup (which rebuilds the field type collection) even though getDatabaseValidationRules() already performs the same FILE check. To reduce overhead and duplication, consider doing the FILE/skip-DB-constraints decision once (e.g., pass a resolved dataType/flag into mergeAllValidationRules, or have getDatabaseValidationRules return enough information to skip DatabaseFieldConstraints::getConstraintsForColumn() without another field type resolution).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants