search

stephenmcd / filebrowser-safe

File manager for Mezzanine
Other
42 stars 104 forks source link

Make FileBrowseField return a FieldFile #69

Closed ryneeverett closed 8 years ago

ryneeverett commented 8 years ago

An issue was opened for this a couple years ago in django-filebrowser, but it was closed without being understood.

See the docs on FileField and FieldFile:

When you access a FileField on a model, you are given an instance of FieldFile as a proxy for accessing the underlying file.

The motivation is primarily that reusable django apps tend to have functionality that depends on file fields returning a FieldFile instance. Examples that I'm aware of include easy-thumnails (as mentioned in the django-filebrowser issue) and django-downloadview (my use case).

I'm not yet sure if this is realistic -- it's not just a matter of changing the base class.

Potential Resolutions:

  1. Make FileBrowseField a subclass of django.db.models.FileField.
  2. Have FileBrowseField return a FieldFile instance.
  3. Make FileObject a subclass of FieldFile.
  4. Have FileBrowseField implement the FieldFile API.
stephenmcd commented 8 years ago

What needs to happen other than changing the base class, do a bunch of methods and attributes need to be implemented? Are there a minimal set of those that would satisfy your use case? Maybe start off with those and we can just implement it incrementally - presumably it'd be backward compatible with what we have now, which is nothing.

ryneeverett commented 8 years ago

What needs to happen other than changing the base class, do a bunch of methods and attributes need to be implemented?

Well a lot of filebrowser-safe assumes that a string will be returned and not a FieldFile. I haven't fixed all the resulting TypeError's yet, but with luck that may be the only issue.

Are there a minimal set of those that would satisfy your use case? Maybe start off with those and we can just implement it incrementally - presumably it'd be backward compatible with what we have now, which is nothing.

Good idea. I hadn't considered reimplementing the FieldFile API. My first choice however would be offloading responsibility to django. My hope is that this decreases filebrowser-safe's responsibility rather than increasing it.

stephenmcd commented 8 years ago

Well a lot of filebrowser-safe assumes that a string will be returned and not a FieldFile. I haven't fixed all the resulting TypeError's yet, but with luck that may be the only issue.

Makes sense, didn't think of that.

Good idea. I hadn't considered reimplementing the FieldFile API.

I figured that's what'd actually be needed, but my preference is as little code as possible - so let's only implement what's needed to get it working.