Skip to content

Return entire file object from DRF serializer #271

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Return entire file object from DRF serializer #271

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion s3_file_field/rest_framework.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ def to_internal_value(self, data: Union[str, File]) -> str: # type: ignore[over

# fields.S3FileField.save_form_data is not called by DRF, so the same behavior must be
# implemented here
internal_value = file_object.name
Copy link
Contributor

Choose a reason for hiding this comment

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

This change seems to violate the entire point of the above comment

fields.S3FileField.save_form_data is not called by DRF, so the same behavior must be implemented here

In particular, any FieldFile value will pass through the logic here https://github.com/django/django/blob/d4c5d2b52c897ccc07f04482d3f42f976a79223c/django/db/models/fields/files.py#L190-L202 . Changing this from a str to a S3PlaceholderFile will at least change how saves are handled.

In fact, I'm worried that this might be outright breaking the save process, but our tests just aren't specific enough to pick it up.

If you think this works, could we update the tests to add new assertions or cases to ensure that the intended save behavior (update the DB, don't touch the Storage) is still happending with this?

internal_value = file_object

return internal_value