Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#2413 closed defect (wontfix)

[patch] FileField edit_inline broken

Reported by: Gacha Owned by: adrian
Component: contrib.admin Version: master
Severity: critical Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

When adding multiple files as inline elements, then the next file overwrites first file. To better understand, reed this post http://groups.google.com/group/django-users/browse_thread/thread/bad034520e28c2c8/3bcb3350d4a19bea?lnk=gst&q=FileField&rnum=9#3bcb3350d4a19bea

Attachments (1)

inline_files_bug.diff (5.9 KB) - added by Joel Heenan <joelh-django@…> 9 years ago.
bug fix for ticket 2413

Download all attachments as: .zip

Change History (13)

comment:1 Changed 9 years ago by josephdrose@…

  • priority changed from normal to highest
  • Severity changed from normal to critical

Here is a little more detail to help understand the error:

If a model 'foo' is created that has a one-to-many relationship with 'foo-files' (foo-files has a column of the filefield type, of course), and if the 'foo-files' has the edit_inline=models.TABULAR or edit_inline=modles.STACKED, when the 'foo' object is editid and saved, the 'foo-file' objects associated with it are deleted. If a user is creating a foo object and adds foo-files at the bottom, the admin panel exhibits the correct behavior. The error only seems to occur when one edits, and then saves a foo object.

I think the severity of this error should be increased. I think any time an error causes a loss of data, it is severe. Accordingly, I have also increased its priority.

comment:2 Changed 9 years ago by josephdrose@…

  • Version changed from new-admin to SVN

Changed version to SVN. This appears in the .95 release as well.

comment:3 Changed 9 years ago by Darnell Brawner

I am also having problems with this bug.

comment:4 Changed 9 years ago by anonymous

  • Resolution set to worksforme
  • Status changed from new to closed

Could not replicate.

comment:5 Changed 9 years ago by mtredinnick

  • Resolution worksforme deleted
  • Status changed from closed to reopened

We have three people reporting the problem (including the original mailing list poster), quite recently. Closing this as "worksforme" by an anonymous person with no further information is probably not correct. For example, was the process in the initial email thread followed precisely? Reopening for more investigation.

comment:6 Changed 9 years ago by Joel Heenan <joelh-django@…>

Ok so I investigated this and there were a number of issues. The most glaring is that manipulators can set all_cores_given=True and all_cores_blank=True at the same time in django/db/models/manipulators around line 169. I'm not sure why two variables were used here when one would suffice given that the code says that if some but not all were given that would cause an earlier error. So that caused some nasty bugs where files were added then deleted again.

The second issue I found was that there is currently no checking for file type fields that are core. I added a new function for this purpose but its a bit of a hack. This check had the (good) side-effect of elimating these weird invalid objects that were being created from blank files.

I've made a patch that appears to me to fix the error you have encountered. I fiddled with the admin interface and I ran the included tests and it doesn't appear to have broken anything else that I can see - but I would imagine there are other corner cases around files like this one. Ideally we could do away with the hack of treating files so differently and have a better way of discovering whether they are provided or not - e.g. fields could return whether they are to be deleted or not? There should not be any field specific code inside the manipulators.

Disclaimer: this patch appears to work for me, don't apply it on anything you care about

Changed 9 years ago by Joel Heenan <joelh-django@…>

bug fix for ticket 2413

comment:7 Changed 9 years ago by bp@…

I can report a "metoo" here - had the exact same problem with an object that's "edit_inline=models.TABULAR" containing a FileField being blown away whenever the parent object is updated through the admin interface. This is on a version from SVN trunk as of a short time ago.

Tried the attached patch, and it does seem to fix the problem (thanks Joel).

comment:8 Changed 9 years ago by clelland@…

Joel -- I wish I had seen this before I submitted ticket #2534 -- There is a big problem right now, with all_cores_given and all_cores_blank both being possible. As far as I can tell, it can only happen when FileFields are the only core fields in the model. If it happens, you get effects like files being uploaded and then immediately deleted, or three records being created the first time you save the model, and then all deleted the next time, with one new one being created. Odd stuff, but all generally traceable back to this bit of code.

The patch that I have uploaded for #2534 overlaps yours a bit; it specifically makes FileFields deletable, by adding a checkbox to their manipulator form field, and does away with the logic that treats them as special cases.

Also, instead of adding a get_manipulator_new_file_data method, I fixed the get_manipulator_new_data method for FileFields specifically, thinking that get_manipulator_new_data should always return the value that the field will have, if it is saved, regardless of the type of field. That alone eliminates a lot of special-case logic.

Perhaps the two tickets (and code) should be merged?

comment:9 Changed 9 years ago by ilia

spent half-day trying to make ImageField editable inline.. Finally found autodelete bug.

I guess, django really needs a "removeme" checkbox together with file field... Otherwise, not only item gets autodeleted, but there is no way to detach a file from e.g letter.

comment:10 Changed 9 years ago by grimboy

  • Summary changed from FileField edit_inline broken to [patch] FileField edit_inline broken

Changing the title to reflect this having a patch. (I get rejected as spam by Akismet if I don't put any text here)

comment:11 Changed 8 years ago by SmileyChris

  • Resolution set to wontfix
  • Status changed from reopened to closed

This is rather out of date with the upcoming new-admin rejigging. If anyone still has this issue after new-admin is merged then reopen.

comment:12 Changed 7 years ago by ecir.hana@…

Hello,
I run latest SVN but I guess I'm having similar problems:

--- from http://groups.google.com/group/django-users/browse_thread/thread/aa0b83e3aa91fb63

Hello,
I'm trying to write simple blog tool but I ran into some problems. I
would like to attach multiple images to a post. Here's my models:

class Entry(models.Model):

title = models.CharField(max_length=200)
date = models.DateTimeField(auto_now=True)
body = models.TextField()

class Photo(models.Model):

entry = models.ForeignKey(Entry, edit_inline=models.TABULAR,

num_in_admin=10)

photo = models.ImageField(upload_to='photos/%Y/%m/%d', core=True,

blank=True)

Now, when I create very new Entry via admin, I can add 10 images, ok.
So I add some. Then, when I would like to add another ones or to
change the ones already in, it goes wild: all the pictures are deleted
and it is not even possible to add anything. Please, any help?

I read somewhere that perhaps edit_inline is broken or something, is
there another way how to attach multiple images to a post via admin?

Thanks!

--- /from

Any hint on this new-admin and its merge?

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