Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make_absolute_paths don't depend anymore on STATIC_URL #126

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dvl
Copy link

@dvl dvl commented Dec 8, 2016

Attempt to fix #103

@johnraz
Copy link
Collaborator

johnraz commented Jan 28, 2017

@dvl thanks for the contrib, can you make sure tests are passing before I start reviewing this? Thanks !

@johnraz
Copy link
Collaborator

johnraz commented Mar 5, 2017

@dvl friendly ping regarding this PR :-)

@dvl
Copy link
Author

dvl commented Mar 26, 2017

@johnraz sorry for late response, I'm was not using wkhtmltopdf for a while.

I've updated tests for when templates use static files but I'm not sure on how to deal with media files because static finders don't search for uploaded files.

Any thoughts?

@axerdan
Copy link

axerdan commented Jul 13, 2017

You can resolve MEDIA_URL url using a simple os.path.join.

9cdb573#diff-36b0c981b229128ec810acc97c68d8feR247

This row should be something like this (not tested):

if occur.startswith(settings.STATIC_URL):
    pathname = finders.find(filename, all=False)
else:
    pathname = os.path.join(settings.MEDIA_ROOT, filename)

This patch could resolve a lot of problems with rendering templates using static/media urls!

@fixmycode
Copy link
Contributor

any chance those changes be merged? the tests failed because of a connection error.

@johnraz
Copy link
Collaborator

johnraz commented Jul 17, 2017

@fixmycode there are also conflict to resolve first.

@fixmycode
Copy link
Contributor

@johnraz can I fix them? should I make a new PR?

@johnraz
Copy link
Collaborator

johnraz commented Jul 17, 2017

Sure go with a new PR

@fixmycode
Copy link
Contributor

@dvl if you merge the new master (d31e839) tests should be working now

@dvl
Copy link
Author

dvl commented Jul 20, 2017

I've updated the master but I'd like to implement @Keeper82 idea to deal with media files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doesn't work with staticfiles finders, raises error when STATIC_ROOT is not set
4 participants