search

barseghyanartur / django-fobi

Form generator/builder application for Django done right: customisable, modular, user- and developer- friendly.
https://pypi.python.org/pypi/django-fobi
485 stars 112 forks source link

Incorrect urls for files in emails #264

Closed corentinbettiol closed 2 years ago

corentinbettiol commented 2 years ago

The bug is triggered when using a full url in settings.MEDIA_URL instead of only a folder name.

The previous check would add base_url before our settings.MEDIA_URL if our url started with settings.MEDIA_URL, and now it add it only if our url don't start with "http" (prevent adding "https://example.ext" before "https://media-example.ext").

corentinbettiol commented 2 years ago

@barseghyanartur Hello, sorry to mention you like this but we need a correct support of urls for files in the mails, as we're currently using our own fork and it's not always a good practice to stick on a fork for too long.

Is there anything else to do on this PR before it can be merged?

barseghyanartur commented 2 years ago

@corentinbettiol:

Thanks for reminding! I'll pick it ASAP.

corentinbettiol commented 1 year ago

@barseghyanartur Hi, sorry to mention you one more time here, but I can't see the changes of this PR on the "main" branch (they are only available on branch "old_master"). I didn't found an explanation (in issues & readme) about the differences between commits in main & old_master, so sorry if it has already been answered elsewhere.

barseghyanartur commented 1 year ago

Ah, snap! I think they were lost during the introduction of class-based views.

Could you make another PR, please? :)

corentinbettiol commented 1 year ago

Using the same branch (kapt-labs/master)? To merge on branch main?

barseghyanartur commented 1 year ago

Please, branch from main. Submit PR to main.