Django

Code

Ticket #7048 (new)

Opened 8 months ago

Last modified 2 weeks ago

Support clearing FileFields in the admin if blank=True

Reported by: jarrow Assigned to: jarrow
Milestone: post-1.0 Component: django.contrib.admin
Version: SVN Keywords:
Cc: code@vonposer.de, david@reynoldsfamily.org.uk, msaelices@yaco.es, tahooie@gmail.com, digitaljhelms@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 1 Patch needs improvement: 0

Description

Currently one can set blank=True in the model. This works in the admin: It let's you save entries without the file. But once you submit a file, you cannot get rid of it!

So blank=True on FileFields? currently means "blank first, then set forever". Which is a not the most common use case IMHO ;)

There already is a snippet implementing a RemovableFileField?: http://www.djangosnippets.org/snippets/636/

If the deletion of the file from disk isn't desired I would like to at least contribute a patch to clear the field. I'm filing this for newforms-admin so it doesn't get outdated right away.

Any thoughts on this are greatly appreciated!

Attachments

DeletableFileFields-00_code.diff (17.7 kB) - added by jarrow on 08/22/08 02:56:43.
First version of the patch, no test fixes yet, docs are separate
DeletableFileFields-00_docs.txt (1.2 kB) - added by jarrow on 08/22/08 03:02:26.
Docs as a simple text file as the docs refactor is going to land anyway …
DeletableFileFields-01.diff (19.5 kB) - added by jarrow on 11/10/08 12:15:57.
Updated patch to r9381, included docs, checking blank=True before deleting (against forged POST data)
DeletableFileFields-02.diff (19.6 kB) - added by jarrow on 11/12/08 09:34:06.
Moved documentation to the correct file

Change History

05/16/08 12:13:02 changed by garcia_marc

  • keywords changed from nfa-someday to nfa-someday fs-rf.
  • needs_better_patch changed.
  • needs_tests changed.
  • needs_docs changed.

I think that I asked it in the developers list, and the idea is not spend time in FileFields? because they are being refactored. Marking this ticket as fs-rf to be considered.

06/16/08 12:29:55 changed by Gulopine

  • keywords changed from nfa-someday fs-rf to nfa-someday.

The filestorage refactor does indeed make it easier to delete files from within Python, but it doesn't extend to the admin interface at all. Removing the fs-rf keyword.

07/12/08 04:27:25 changed by uptimebox

  • owner changed from nobody to uptimebox.
  • status changed from new to assigned.

07/13/08 07:39:29 changed by isagalaev

  • keywords changed from nfa-someday to nfa-someday yandex-sprint.

07/24/08 05:13:56 changed by Jonas

  • cc set to code@vonposer.de.

I'm very interested in a solution to this problem and would love to help out. Let me know if I can do something.

08/12/08 10:33:20 changed by jacob

  • stage changed from Unreviewed to Accepted.
  • version changed from newforms-admin to SVN.
  • milestone set to 1.0.

08/18/08 09:27:06 changed by DavidReynolds

  • cc changed from code@vonposer.de to code@vonposer.de, david@reynoldsfamily.org.uk.

08/18/08 16:32:31 changed by jarrow

  • keywords deleted.
  • owner changed from uptimebox to jarrow.
  • status changed from assigned to new.

Claiming the ticket as there hasn't been any activity in the last month. If there exists a patch from the Yandex sprint now would be a good time to post it :)

08/18/08 16:39:14 changed by isagalaev

Looks like we didn't get to it after all. Feel free to take it, thanks!

08/18/08 17:13:01 changed by garcia_marc

I don't have a patch for it, but this feature is working in a field that subclasses FileField? on the stdimage application. So it should be easy to extract from there, and create a patch. Probably the patch will need to be improved later, but at least is a starting point.

stdimage is available at http://code.google.com/p/django-stdimage/

08/18/08 17:27:16 changed by jarrow

Cool, thx! I also got code from brosner. I will probably use both sources to compile a patch :)

08/22/08 02:56:43 changed by jarrow

  • attachment DeletableFileFields-00_code.diff added.

First version of the patch, no test fixes yet, docs are separate

08/22/08 03:02:26 changed by jarrow

  • attachment DeletableFileFields-00_docs.txt added.

Docs as a simple text file as the docs refactor is going to land anyway ...

08/22/08 03:25:55 changed by jarrow

  • has_patch set to 1.
  • needs_tests set to 1.

After talking to brosner on #django-dev we decided to implement this on the ModelForm? level. The attached patch introduces file deletion functionality for ModelForms? which can be customized via its Meta class (see attached docs).

Work still to be done:

  • Check admin CSS
    • at least for the RTL design this is bound to break
    • what about inlines?
  • Fix regression tests
  • Write new tests ;)

Notes on the patch:

  • I had to add some special admin CSS to align everything properly. That can only be achieved with a fixed padding to the left ...
  • For the DeletableFileField? I started with a MultiValueField? till i had overwritten every piece of code in it ... now it just sublcasses Field. The widget still subclasses MultiWidget? though.
  • The display of the current file moved from the admin to ModelForm?. It just doesn't make sense to provide a delete option if the presence of a file isn't visible ;)

If I missed anything in this short description let me know. Please test and tell me what breaks I'll keep the patch up to date :)

08/22/08 15:38:09 changed by msaelices

  • cc changed from code@vonposer.de, david@reynoldsfamily.org.uk to code@vonposer.de, david@reynoldsfamily.org.uk, msaelices@yaco.es.

08/22/08 21:02:05 changed by brosner

  • milestone changed from 1.0 to post-1.0.

After seeing what is really involved in this ticket. jarrow and I decided it is best to avoid this pre-1.0. There is just too much design decisions that need to be made to make this work the right way. I personally appreciate jarrow's work. This will certainly make it post-1.0. :)

08/23/08 18:53:27 changed by jarrow

Just a note to myself: Maybe the CSS could be shorter as I introduced <br/>s after writing the style rules. Haven't checked though. The <br/>s are there for a sensible default display without CSS when using for example as_table().

09/24/08 15:32:26 changed by jarrow

#6405 is about making MultiValueField? pass through initial data. Maybe this could be closed if a deletable FileField? is the only use case.

10/31/08 19:23:51 changed by tahooie

  • cc changed from code@vonposer.de, david@reynoldsfamily.org.uk, msaelices@yaco.es to code@vonposer.de, david@reynoldsfamily.org.uk, msaelices@yaco.es, tahooie@gmail.com.

11/10/08 12:15:57 changed by jarrow

  • attachment DeletableFileFields-01.diff added.

Updated patch to r9381, included docs, checking blank=True before deleting (against forged POST data)

11/10/08 17:00:26 changed by jarrow

The attached patch seems to work fine. The tests still need fixing though. I wanted to wait for some feedback on the code and approach :) RTL CSS also not done yet. But there seems to be an admin redesign forthcoming anyway.

11/12/08 09:34:06 changed by jarrow

  • attachment DeletableFileFields-02.diff added.

Moved documentation to the correct file

11/12/08 09:43:08 changed by jarrow

With the current default values for the ModelForm? Meta class options the admin shows the delete checkbox and removes the file from disk. It does not remove empty directories. To enable this in the admin one currently needs to do something like this:

class TestModelAdmin(admin.ModelAdmin):
    def get_form(self, request, obj=None, **kwargs):
        defaults = {
            "files_delete_empty_dirs": True,
        }
        defaults.update(kwargs)
        return super(TestModelAdmin, self).get_form(request, obj, **defaults)

Maybe a settings option to make deletion of empty dirs the default would be nice. Or we could add a files_delete_empty_dirs option to the ModelAdmin? class and pass it to the ModelForm? (like we do with exclude etc.).

11/17/08 21:54:58 changed by digitaljhelms

  • cc changed from code@vonposer.de, david@reynoldsfamily.org.uk, msaelices@yaco.es, tahooie@gmail.com to code@vonposer.de, david@reynoldsfamily.org.uk, msaelices@yaco.es, tahooie@gmail.com, digitaljhelms@gmail.com.

Add/Change #7048 (Support clearing FileFields in the admin if blank=True)




Change Properties
Action