#3297 (newforms FileField/ImageField) - Feedback requested
Russell Keith-Magee
I've just uploaded a potential fix for ticket #3297 - an
implementation of FileField and ImageField for newforms - and I'm
looking for some feedback.
Details for those interested (repeated from the description of the ticket):
* Modifies forms to take a "files" argument, for storage of file
data. This means you no longer have to copy and update your POST
dictionary on submit. You just call MyForm?(request.POST,
request.FILES)
* Introduces an "UploadedFile?" temporary object to store the
submitted file data.
* Modifies the internals of the form_for_model/instance save
method to make field saving more generic, rather than making a special
case of FileField?. This should make writing custom fields with
unusual save requirements much easier.
This patch has 2 potential backwards incompatibilities:
1. If you are relying upon auto_id being the 2nd argument on a form
(rather than explicitly naming auto_id=False), your rendered forms
will start adding label tags.
2. If you have written any custom code using value_from_datadict,
you will need to modify those implementations to handle the new files
argument.
Yours,
Russ Magee %-)
oyvind....@gmail.com
Files cannot be saved with different names based on other data i the
model, since fields after the filefield does not set values in the
model. This also causes a problem since save is called in
save_FIELD_file before all values are set on the model.
Not sure how this should be fixed.
Except for this +1 for me.
Wrote a patch for admin intergration and solving the problem with save
being called in save_FIELD_file.
Made the view pass request.FILES to the form only if it is needed by a
FileField, not sure if it should be passed no matter what to keep the
code cleaner.
Would like feedback this patch.
On Jun 7, 1:36 pm, "Russell Keith-Magee" <freakboy3...@gmail.com>
wrote:
Malcolm Tredinnick
> I like this patch except for some issues.
>
> Files cannot be saved with different names based on other data i the
> model, since fields after the filefield does not set values in the
> model. This also causes a problem since save is called in
> save_FIELD_file before all values are set on the model.
>
> Not sure how this should be fixed.
>
> Except for this +1 for me.
>
> Wrote a patch for admin intergration
Just one note: any admin changes now should be against newforms-admin.
We aren't adding anything to existing admin any longer.
> and solving the problem with save
> being called in save_FIELD_file.
>
> Made the view pass request.FILES to the form only if it is needed by a
> FileField, not sure if it should be passed no matter what to keep the
> code cleaner.
>
> Would like feedback this patch.
Remember that Russell is on a critical missions helping to repair the
international space station for a few weeks, so there may be a small
delay on this.
Regards,
Malcolm
oyvind....@gmail.com
later than regular fields?
Like doing a second for loop over the fields that have a custom save
method, or has a attribute that says it should be handled later.
So a model object can be saved in the save handler and have a pk, and
foreignkeys/slugs before those spechial case save handlers are called.
On Jun 12, 1:05 pm, "oyvind.salt...@gmail.com"
<oyvind.salt...@gmail.com> wrote:
> I like this patch except for some issues.
>
> Files cannot be saved with different names based on other data i the
> model, since fields after thefilefielddoes not set values in the
> model. This also causes a problem since save is called in
> save_FIELD_file before all values are set on the model.
>
> Not sure how this should be fixed.
>
> Except for this +1 for me.
>
> Wrote a patch for admin intergration and solving the problem with save
> being called in save_FIELD_file.
>
> Made the view pass request.FILES to the form only if it is needed by aFileField, not sure if it should be passed no matter what to keep the
> code cleaner.
>
> Would like feedback this patch.
>
> On Jun 7, 1:36 pm, "Russell Keith-Magee" <freakboy3...@gmail.com>
> wrote:
>
> > Hi all,
>
> > I've just uploaded a potential fix for ticket #3297 - an
> > implementation ofFileFieldand ImageField for newforms - and I'm
> > looking for some feedback.
>
> > Details for those interested (repeated from the description of the ticket):
>
> > * Modifies forms to take a "files" argument, for storage of file
> > data. This means you no longer have to copy and update your POST
> > dictionary on submit. You just call MyForm?(request.POST,
> > request.FILES)
>
> > * Introduces an "UploadedFile?" temporary object to store the
> > submitted file data.
>
> > * Modifies the internals of the form_for_model/instance save
> > method to make field saving more generic, rather than making a special
> > case ofFileField?. This should make writing custom fields with
oyvind....@gmail.com
backwards compatibility for those with special file naming
requirements.
Please give some feedback and tell me if i'm way off course. :)
On Jul 16, 3:42 pm, "oyvind.salt...@gmail.com"
Russell Keith-Magee
>
> I tried some tricks to get this working as I think it should to keep
> backwards compatibility for those with special file naming
> requirements.
>
> Please give some feedback and tell me if i'm way off course. :)
Hi Oyvind
Apologies for not getting back to you on this. I've been clening out a
few bugs in the serialization and test systems while I wait for some
feedback from Adrian on some other tickets; looking at FileField got
pushed off my plate for a bit. Hopefully I will get a chance to look
at this ticket again in the very near future (maybe even tonight).
Yours,
Russ Magee %-)
carole...@gmail.com
one of the main reasons I didn't use FileField in my current code
base... and thought others might have a similar need going forward.
When I upload files, I'm required to rename them and then move them to
different directories. Basically I create a uuid for the file name,
rename it, then create a directory (if it doesn't exist) using the
first character of the uuid, I then create a sub directory using
characters 2 and 3 of the uuid, and move the file there from the
upload location. I didn't see a way to do this with FileField or
ImageField in old forms ( I could have been mistaken, just tell me if
I am ) ... so I used 'old school' forms and uploaded the files that
way... and made the file name field a CharField.
I know that this is a pretty specific example, but I thought it was a
bit restricting to always assume that all files in a FileField for a
model would be stored in the same directory off of Media. It would be
nice if there was an option to not set the upload_to path in the model
code, and have the capability of making it different for each entry.
Figured I'd mention this since you were asking for feedback.
On Jul 23, 11:01 pm, "Russell Keith-Magee" <freakboy3...@gmail.com>
wrote:
oyvind....@gmail.com
It will allow you to subclass and override parts of FileField to store
using other values in the model.
On Jul 24, 7:11 pm, "carole.zie...@gmail.com"
oyvind....@gmail.com
On Jul 24, 7:42 pm, "oyvind.salt...@gmail.com"
Russell Keith-Magee
> backwards compatibility for those with special file naming
> requirements.
>
> Please give some feedback and tell me if i'm way off course. :)
Hi Oyvind,
Apologies for taking so long to get back to you on this issue.
Regarding your suggestions for 'special file naming requirements': I
agree that broad use case exists, but I can't say I particularly like
any of the the implementations you propose. More design work is
required here.
On an organizational level, it would be better to open this as a new
ticket, rather than piggyback it on #3297. #3297 is tracking a
specific problem - the absence of FileFields in newforms. This
problem needs to be fixed.
Allowing 'special file naming requirements' is a feature addition that
is separate to File Fields. While it is a nice feature, it is a
separate feature, and no decision has been made on what this feature
should look like - the plethora of options you provide shows that it
is not a simple proposition. This feature will need to go through a
separate design and review process.
As such, it should be on a different ticket. This would also make
following the patches much easier. It took me a while to work out
exactly which parts of your latest patches were bugfixes to my
FileField patch, and which were new and unrelated features.
Yours,
Russ Magee %-)
oyvind....@gmail.com
The implementations seemed a bit hackish to me aswell.
Starting a new ticket for this #5029
On Jul 31, 1:48 pm, "Russell Keith-Magee" <freakboy3...@gmail.com>
wrote: