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

WL-320: fixed get_template_path to work with absolute path also #11548

Merged
merged 1 commit into from
Feb 16, 2016

Conversation

ziafazal
Copy link
Contributor

It has been discovered while PR #11076 that get_template_path in BaseMicrositeTemplateBackend fails in case of absolute path. This PR has fix for get_template_path in BaseMicrositeTemplateBackend to accept both absolute and relative paths.
@saleem-latif please review.
@mattdrayer @strannikk FYI.

@ziafazal
Copy link
Contributor Author

jenkins run python

@ziafazal ziafazal force-pushed the ziafazal/fix-get-template-path branch from b6e8ae6 to 070a142 Compare February 15, 2016 11:57
@ziafazal
Copy link
Contributor Author

jenkins run bokchoy

@@ -312,6 +312,7 @@ def get_template_path(self, relative_path, **kwargs):
from microsite_configuration.microsite import get_value as microsite_get_value

microsite_template_path = microsite_get_value('template_dir', None)
relative_path = template_path[1:] if template_path.startswith('/') else template_path

if not microsite_template_path:
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for readability, let's move this block up to line 315 and move relative_path closer to search_path

fixed quality violation

skipped test in CMS

changes after feedback from mattd
@ziafazal ziafazal force-pushed the ziafazal/fix-get-template-path branch from a98e748 to 6cd2657 Compare February 16, 2016 06:52
@ziafazal
Copy link
Contributor Author

@mattdrayer made changes as you suggested. @felipemontoya would you like to review this PR too?

@mattdrayer
Copy link
Contributor

Great, LGTM! 👍

@saleem-latif
Copy link
Contributor

LGTM 👍

ziafazal added a commit that referenced this pull request Feb 16, 2016
WL-320: fixed get_template_path to work with absolute path also
@ziafazal ziafazal merged commit 57af11c into master Feb 16, 2016
@felipemontoya
Copy link
Member

I had a look at this, and its alright. In our fork we sometimes do need the leading slash when the path is a microsite template, but we have extended the BaseMicrositeTemplateBackend to support our usecase, so 👍

@benpatterson benpatterson deleted the ziafazal/fix-get-template-path branch August 2, 2016 13:06
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.

4 participants