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

Specializing static and media based on tenants #108

Merged
merged 6 commits into from
Nov 27, 2016

Conversation

mateuspadua
Copy link
Contributor

New Feature to be possible have different static files for each tenant or upload media files to separate directories for each tenant. More information, please read the documentation on this commit. :)

@kosz85
Copy link
Contributor

kosz85 commented Nov 20, 2016

Nice addition, but... From my personal story :) Using schema_name in path can be a bit problematic, especially when you can (as app feature) or have to (as client request) rename schema_name. I have less advance solution of this (just constructing path where it's needed), but really using tenant.id is much better option, as even after renaming it stays the same. It could be an option if you want to use schema_name or id, it's small change but it will save you problems in future.

Copy link
Member

@tomturner tomturner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you I will look an merge later

@mateuspadua
Copy link
Contributor Author

mateuspadua commented Nov 20, 2016

Thank you for your considerations. I followed the same idea of what already existed for the templates based on tenants, so I used schema_name, but later, we can put a flag in the settings where we can choose between using ID or schema_name, as you said :). Thanks

@@ -26,7 +32,9 @@ def process_request(self, request):
domain = get_object_or_404(get_tenant_domain_model().objects.select_related('tenant'),
domain=hostname)

domain.tenant.domain_url = hostname
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mateuspadua Is this line needed as i don't think it is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont add the line 35, this line already existed, see here. :)
https://github.com/tomturner/django-tenants/blob/master/django_tenants/middleware.py

I searched here and saw that line was added in this commit 7020e63

Is about this line your question?

@mateuspadua
Copy link
Contributor Author

Hi @tomturner, i'm here if you need any help to merge my pull request. Thanks

@tomturner
Copy link
Member

@mateuspadua Thank you I will try and merge over the weekend

@tomturner tomturner merged commit edc9e33 into django-tenants:master Nov 27, 2016
@mateuspadua
Copy link
Contributor Author

@tomturner thanks for merge, i believe that it is going to help many people. :)

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.

None yet

3 participants