Opened 18 years ago

Closed 17 years ago

Last modified 14 years ago

#2534 closed defect (invalid)

[Patch] Enable FileField/ImageField to be used as core field

Reported by: clelland@… Owned by: nobody
Component: Core (Other) Version: dev
Severity: normal Keywords:
Cc: gary.wilson@…, nesta.campbell@…, tomi.kyostila@… Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

This patch gives FileField (and ImageField) objects the ability to be used as 'core' fields in related models. Also added is the ability to clear the field, deleting the uploaded file from the filesytem.

FileField and ImageField objects do not work properly as core fields. This seems to be acknowledged in manipulators.py, where they are treated as a special case, but as a result, if a FileField or ImageField is the only field marked core=True in a related model, then that record will always be saved, even if every field is empty. This patch corrects this issue by providing a get_manipulator_new_data method for FileField which returns the new value that the field will take, rather than the pre-existing value.

Since core=True fields aren't very useful without the ability to clear them, this patch also adds the optional ability to delete files after they've been uploaded. This has been long-standing issue in Django -- see, for example, Ticket #22. The patch adds a new optional constructor argument, can_delete, which, when set, will place a checkbox labeled Delete on the HTML form. When the user checks this checkbox and saves the form, the FileField will be cleared and the file associated with the field will be removed from the filesystem. (Unlike the patch in ticket #22 that provides an HTML link which redirects the user away from the change form, the Delete checkbox in this patch does not get processed until the user saves the object. This also means that the user can choose to clear a FileField, make other changes to an object, and save all of the changes atomically.)

In creating this patch, we have strived to change as little code as possible. We hope that it will be accepted into Django as it immediately addresses a significant shortcoming while still maintaining compatibility with existing models and apps.

This patch is largely based on our work at http://www.verdjn.com/wiki/FileField, but goes further by actually allowing FileField objects to be used as core fields.

Attachments (3)

filefield_core_fixes.diff (6.6 KB ) - added by anonymous 18 years ago.
Patch to allow FileFields to be used as core fields
stylers.xml (79.8 KB ) - added by anonymous 17 years ago.
filefield_core_fixes_6635.diff (6.6 KB ) - added by james.utter@… 17 years ago.
updated patch to point to oldforms instead of forms

Download all attachments as: .zip

Change History (17)

by anonymous, 18 years ago

Attachment: filefield_core_fixes.diff added

Patch to allow FileFields to be used as core fields

comment:1 by ilia, 18 years ago

Thanks a lot! Eager to see it in django.

comment:2 by anonymous, 18 years ago

very useful patch, I have used provided functionality in a few projects already. would be good to see it commited in trunk

comment:3 by Gary Wilson <gary.wilson@…>, 18 years ago

Cc: gary.wilson@… added

comment:4 by Peter Bowyer, 18 years ago

It would be great to have this added to core... are there important reasons why this hasn't happened in the 5 months since submission?

comment:5 by Nesta Campbell, 17 years ago

Cc: nesta.campbell@… added

I was just bitten by this. Any idea what's up with the patch? Would be nice to see it integrated.

comment:6 by Simon G. <dev@…>, 17 years ago

Has patch: set
Triage Stage: UnreviewedAccepted
Version: SVN

comment:7 by Tomi Kyöstilä <tomi.kyostila@…>, 17 years ago

Cc: tomi.kyostila@… added

comment:8 by atomekk@…, 17 years ago

I guess this patch still isnt merged with trunk. The question remains ... when ?

comment:9 by Ionuț Ciocîrlan, 17 years ago

Hate to post a useless comment. But really... when will it get merged? The patch is already old and doesn't apply cleanly anymore.

comment:10 by Malcolm Tredinnick, 17 years ago

Can everybody please stop posting "when will it be merged" comments. They add nothing to getting the ticket resolved. This is just one of many open tickets -- no more special than most others.

Before it can be merged, though, a few things have to happen: the patch has to apply cleanly, it needs to work against newforms (we are not adding any more functionality to oldforms at this time), we need to really think about whether allowing Django to delete files is a good idea (that's not a given -- needs discussion on django-developers) and, if so, ensuring this cannot be exploited to delete arbitrary files in any way at all.

comment:11 by anonymous, 17 years ago

In relation to the file deletion question, might I suggest we at least allow one to select existing files? For example, if I want to use the same image in two different objects, the current image field uploads another copy and renames it rather than noticing the file already exists and asking to overwrite or use the existing file. Seems like a slightly complex issue. Probably worth doing some use cases?

by anonymous, 17 years ago

Attachment: stylers.xml added

comment:12 by John Shaffer <jshaffer2112@…>, 17 years ago

Patch needs improvement: set
Triage Stage: AcceptedDesign decision needed

Marking "needs better patch" and "design decision needed" based on mtreddinick's comments.

by james.utter@…, 17 years ago

updated patch to point to oldforms instead of forms

comment:13 by Jacob, 17 years ago

Resolution: invalid
Status: newclosed

To my delight, core is going away in newforms-admin!

comment:14 by , 16 years ago

Deletion of FileFields is now beeing addressed in #7048

Note: See TracTickets for help on using tickets.
Back to Top