Skip to content
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.

Sign up for GitHub

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

file upload max size feature #328

Merged
merged 3 commits into from
Mar 12, 2024
Merged

file upload max size feature #328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
89d23fd
file upload max size feature
madassdev Feb 28, 2024
a8a0327
Change label
JhumanJ Mar 12, 2024
59898de
Merge branch 'main' into 7fd29-customizable-file-upload-size-limit
JhumanJ Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion app/Http/Requests/AnswerFormRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ public function __construct(Request $request)
$this->maxFileSize = $this->form->workspace->max_file_size;
}

private function getFieldMaxFileSize($fieldProps)
{
return array_key_exists('max_file_size', $fieldProps) ?
min($fieldProps['max_file_size'] * 1000000, $this->maxFileSize) : $this->maxFileSize;
}

/**
* Validate form before use it
*
Expand Down Expand Up @@ -180,7 +186,7 @@ private function getPropertyRules($property): array
if (! empty($property['allowed_file_types'])) {
$allowedFileTypes = explode(',', $property['allowed_file_types']);
}
$this->requestRules[$property['id'].'.*'] = [new StorageFile($this->maxFileSize, $allowedFileTypes, $this->form)];
$this->requestRules[$property['id'] . '.*'] = [new StorageFile($this->getFieldMaxFileSize($property), $allowedFileTypes, $this->form)];

return ['array'];
case 'email':
Expand Down
3 changes: 3 additions & 0 deletions app/Http/Requests/UserFormRequest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ public function rules()
'properties.*.generates_uuid' => 'boolean|nullable',
'properties.*.generates_auto_increment_id' => 'boolean|nullable',

// For file (min and max)
'properties.*.max_file_size' => 'min:1|numeric',
Copy link
Contributor

Choose a reason for hiding this comment

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

The validation rule for max_file_size correctly ensures that the value is numeric and at least 1. However, to prevent potential abuse or unintentional configuration errors, it might be beneficial to also enforce an upper limit. Consider modifying the validation rule to include a reasonable maximum value, such as numeric|min:1|max:100 (assuming the maximum file size is 100MB as an example). This ensures that file sizes are within a reasonable range.

- 'properties.*.max_file_size' => 'min:1|numeric',
+ 'properties.*.max_file_size' => 'numeric|min:1|max:100', // Adjust the max value as appropriate
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
'properties.*.max_file_size' => 'min:1|numeric',
'properties.*.max_file_size' => 'numeric|min:1|max:100', // Adjust the max value as appropriate


// Security & Privacy
'can_be_indexed' => 'boolean',
'password' => 'sometimes|nullable',
Expand Down
2 changes: 1 addition & 1 deletion client/components/open/forms/OpenFormField.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ export default {
}
} else if (field.type === 'files' || (field.type === 'url' && field.file_upload)) {
inputProperties.multiple = (field.multiple !== undefined && field.multiple)
inputProperties.mbLimit = this.form?.max_file_size ?? this.currentWorkspace?.max_file_size
inputProperties.mbLimit = Math.min(Math.max(field.max_file_size, 1), this.form?.max_file_size ?? this.currentWorkspace?.max_file_size)
inputProperties.accept = (this.form.is_pro && field.allowed_file_types) ? field.allowed_file_types : ''
} else if (field.type === 'number' && field.is_rating) {
inputProperties.numberOfStars = parseInt(field.rating_max_value)
Expand Down
16 changes: 16 additions & 0 deletions client/components/open/forms/fields/components/FieldOptions.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@
label="Allowed file types" placeholder="jpg,jpeg,png,gif"
help="Comma separated values, leave blank to allow all file types"
/>

<text-input name="max_file_size" class="mt-3" :form="field" native-type="number"
:min="1"
:max="mbLimit"
label="Maximum file size (in MB)" :placeholder="`1MB - ${mbLimit}MB`"
help="Set the maximum file size that can be uploaded"
/>
</div>

<!-- Number Options -->
Expand Down Expand Up @@ -428,6 +435,9 @@ export default {
required: false
}
},
setup() {
return { currentWorkspace: computed(() => useWorkspacesStore().getCurrent), }
},
data () {
return {
typesWithoutPlaceholder: ['date', 'checkbox', 'files'],
Expand All @@ -442,6 +452,9 @@ export default {
hasPlaceholder () {
return !this.typesWithoutPlaceholder.includes(this.field.type)
},
mbLimit() {
return this.form?.max_file_size ?? this.currentWorkspace?.max_file_size
},
prefillSelectsOptions () {
if (!['select', 'multi_select'].includes(this.field.type)) return {}

Expand Down Expand Up @@ -504,6 +517,9 @@ export default {
if (['text', 'number', 'url', 'email'].includes(this.field?.type) && !this.field?.max_char_limit) {
this.field.max_char_limit = 2000
}
if (this.field.type == 'files') {
this.field.max_file_size = Math.min((this.field.max_file_size ?? this.mbLimit), this.mbLimit)
}
Comment on lines +520 to +522
Copy link
Contributor

Choose a reason for hiding this comment

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

The logic to set max_file_size for file type fields based on the minimum of form and workspace limits is a crucial update. It ensures that the file size limit is consistently enforced across different levels of settings. However, as previously commented, consider adding a user notification or confirmation step if the max_file_size is adjusted to a lower value than a previously set one. This would enhance user awareness and control over the changes.

+ if (this.field.max_file_size > this.mbLimit) {
+   // Notify the user that the max_file_size has been adjusted to the mbLimit
+ }
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (this.field.type == 'files') {
this.field.max_file_size = Math.min((this.field.max_file_size ?? this.mbLimit), this.mbLimit)
}
if (this.field.type == 'files') {
this.field.max_file_size = Math.min((this.field.max_file_size ?? this.mbLimit), this.mbLimit)
if (this.field.max_file_size > this.mbLimit) {
// Notify the user that the max_file_size has been adjusted to the mbLimit
}
}

},

methods: {
Expand Down
Loading