search

KalvadTech / django-cloudflare-images

Django library to add Cloudflare Images support for ImageField
MIT License
14 stars 6 forks source link

Filename not preserved #11

Open mschfh opened 1 month ago

mschfh commented 1 month ago

The current implementation only persists the UUID, although the filename is successfully stored by Cloudflare images.

https://github.com/KalvadTech/django-cloudflare-images/blob/b7451e23e4b74558c0211e873ba5400dd4997ded/cloudflare_images/service.py#L49

The preferable solution would be custom paths to use a filename instead of the uuid (this would also allows custom subpaths for e.g. separating environments).

mschfh commented 1 month ago

@hnb2 WDYT about a config setting (e.g. CLOUDFLARE_IMAGES_FILENAME_AS_ID) and adding content.id = content.name in _save? https://github.com/KalvadTech/django-cloudflare-images/blob/b7451e23e4b74558c0211e873ba5400dd4997ded/cloudflare_images/storage.py#L38-L45

hnb2 commented 1 month ago

Hi @mschfh can you elaborate what this setting would do ? Like a boolean to choose whether we save the ID or the name ?

My main concern here is backward compatibility, I do not want to break existing implementations

mschfh commented 1 month ago

Yes, if set to true, it would set content.id to the filename as mentioned.

Backwards compatibility shouldn't be an issue, as it would just use the different id, which should also be returned by the API and stored in the DB, similar to the uuid

hnb2 commented 1 month ago

Hi @mschfh, maybe we are thinking of a different content variable, but File does not have an id attribute.

mschfh commented 1 month ago

Hi @mschfh, maybe we are thinking of a different content variable, but File does not have an id attribute.

true, but it should be possible to add it as a POST variable here from my understanding: https://github.com/KalvadTech/django-cloudflare-images/blob/30833e17ff32f5a117bf72658939a635f3b6539f/cloudflare_images/service.py#L40-L42

Would you accept a patch with an env var to override the default behaviour?