Ticket #2413: inline_files_bug.diff

django/db/models/manipulators.py

@@ -169 +169 @@
                     for f in related.opts.fields:
                         if f.core and not isinstance(f, FileField) and f.get_manipulator_new_data(rel_new_data, rel=True) in (None, ''):
                             all_cores_given = False
+                        # if the new file is blank and the old file is blank
+                        if f.core and isinstance(f,FileField) and f.get_manipulator_new_file_data(rel_new_data, rel=True) in (None, '') and \
+                                                                  f.get_manipulator_new_data(rel_new_data, rel=True) in (None,''):
+                            all_cores_given = False
                         elif f.core and not isinstance(f, FileField) and f.get_manipulator_new_data(rel_new_data, rel=True) not in (None, ''):
                             all_cores_blank = False
                         # If this field isn't editable, give it the same value it had
@@ -200 +204 @@
                         # Save any uploaded files.
                         for f in related.opts.fields:
                             if child_follow.get(f.name, None):
-                                if isinstance(f, FileField) and rel_new_data.get(f.name, False):
+                                if isinstance(f, FileField):
                                     f.save_file(rel_new_data, new_rel_obj, self.change and old_rel_obj or None, old_rel_obj is not None, rel=True)
 
                         # Calculate whether any fields have changed.
@@ -221 +225 @@
 
                     # If, in the change stage, all of the core fields were blank and
                     # the primary key (ID) was provided, delete the item.
-                    if self.change and all_cores_blank and old_rel_obj:
+                    elif self.change and all_cores_blank and old_rel_obj:
                         new_rel_obj.delete()
                         self.fields_deleted.append('%s "%s"' % (related.opts.verbose_name, old_rel_obj))
 

django/db/models/fields/__init__.py

@@ -271 +271 @@
     def get_validator_unique_lookup_type(self):
         return '%s__exact' % self.name
 
+    def get_manipulator_new_file_data(self, new_data, rel=False):
+        """
+        Return the file data in order to overcome some nasty 
+        corner cases
+        """
+        if rel:
+            val = new_data.get(self.name+"_file", [self.get_default()])
+            try:
+                return val[0]
+            except(IndexError):
+                return self.get_default()
+        val = new_data.get(self.name+"_file",self.get_default())
+        if not self.empty_strings_allowed and val == '' and self.null:
+            val = None
+        return val
+
     def get_manipulator_new_data(self, new_data, rel=False):
         """
         Given the full new_data dictionary (from the manipulator), returns this

tests/modeltests/inline_files/models.py

@@ +1 @@
+"""
+This tests an inline file in an object when saved via a dict
+(normally called through HTML say in the admin interface) will 
+not delete the file. This comes about because to the manipulator
+there is confusion about whether you have ignored a file or want
+to delete it. 
+
+Also note that while its not primarily designed to do so it tests
+a number of other things in the code. It tests the case of an 
+object that has only a file field and a foreign key - this is 
+a particularly nasty case because the file is the only core field
+yet its always unclear whether it has been submitted (because people
+don't resubmit entire files). 
+
+Author: Joel Heenan
+"""
+
+from django.db import models
+
+class Foo(models.Model):
+    name = models.CharField(maxlength=200)
+    class Admin:
+        pass
+
+class FooFiles(models.Model):
+    file = models.FileField(verbose_name="The File",core=True,upload_to='foo/')
+    foo = models.ForeignKey(Foo,edit_inline=models.TABULAR)
+    class Admin:
+        pass
+    def __str__(self):
+        return self.file
+
+API_TESTS = """
+>>> from django.db import models
+
+# lets submit something that looks like a normal submit
+# with a bunch of empty files and one filled in with some content
+>>> dict = {'foofiles.1.id': [''], 'foofiles.0.id': [''], 'foofiles.2.id': [''], 'foofiles.1.file': [''], 'foofiles.2.file': [''], 'foofiles.0.file': [''], 'foofiles.0.file_file': [{'content': 'FILE CONTENT GOES HERE', 'content-type': 'text/x-patch', 'filename': 'test_file'}], 'name': 'testfoo3'}
+
+>>> manipulator = Foo.AddManipulator()
+
+>>> f = manipulator.save(dict)
+
+# as we expect our file is there
+>>> print f.foofiles_set.filter()[0]
+foo/test_file
+
+# previous versions of the code would create empty file records
+>>> print f.foofiles_set.count()
+1
+
+# ok lets resubmit to the change manipulator. note we must 
+# re-use the objects id
+>>> dict = {'foofiles.1.id': [''], 'foofiles.0.id': [f.foofiles_set.filter()[0].id], 'foofiles.2.id': [''], 'foofiles.1.file': [''], 'foofiles.2.file': [''], 'foofiles.0.file': ['test_file'], 'name': 'testfoo3'}
+
+>>> manipulator = Foo.ChangeManipulator(f.id)
+
+>>> f = manipulator.save(dict)
+
+# yay or file still exists. previous versions of the code would delete it
+>>> print f.foofiles_set.filter()[0]
+foo/test_file
+
+# no blank files added
+>>> print f.foofiles_set.count()
+1
+
+# cleanup
+>>> f.delete()
+>>>
+"""