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.

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

Avoid the creation of empty file annotations #466

Merged
merged 2 commits into from
May 4, 2023

Conversation

sbesson
Copy link
Member

@sbesson sbesson commented May 4, 2023

Related to #464

Summary

The default contract of the FileField used in the files annotation form is to "validate that non-empty file data has been bound to the form." - see https://docs.djangoproject.com/en/3.2/ref/forms/fields/#django.forms.FileField
Interestingly this check is not happening when adding file annotations. However, the emptiness of the file is definitely checked when uploading a photo in the user settings.

The difference comes from the fact the uploaded files are not bound to the files annotation form. Thus the is_valid() check is largely a no-op and this UI can create annotations with empty files.

This PR fixes this issue by properly binding the uploaded files to the form - see https://docs.djangoproject.com/en/3.2/ref/forms/api/#binding-uploaded-files

Testing

There are a few scenarios to test here:

  • using OMERO.web 5.19.0 and Django 3.2.18, it should be possible to create empty file annotation using the UI. This can be done using the file attachment form and selecting one of multiple files. The annotation should display 0B
  • using this PR and Django 3.2.18, attaching empty files via the dialog UI should be:
  • using this PR, Add MultipleFileField to handle and clean multiple files #465 and Django 3.2.19, the creation of empty file annotations should be disallowed in all cases

Presently, files uploaded in the UI are not bound to the form and
thus the is_valid() check is largely a no-op. As per the default
validation contract of FileField, this means that this UI can
create annotations with empty files.

See https://docs.djangoproject.com/en/3.2/ref/forms/api/#binding-uploaded-files
@snoopycrimecop
Copy link
Member

snoopycrimecop commented May 4, 2023

Conflicting PR. Removed from build OMERO-python-superbuild-push#1436. See the console output for more details.
Possible conflicts:

--conflicts Conflict resolved in build OMERO-python-superbuild-push#1437. See the console output for more details.

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

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

Testing on merge-ci, this successfully prevents the upload and annotation of Objects with empty Files.

@will-moore will-moore merged commit d53e790 into ome:master May 4, 2023
@knabar knabar added this to the 5.20.0 milestone May 5, 2023
@jburel jburel mentioned this pull request May 5, 2023
@sbesson sbesson deleted the annotation_files_validation branch May 11, 2023 05:02
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.

4 participants