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

Using Default Language to look up base templates. #39

Merged
merged 3 commits into from
Mar 21, 2017

Conversation

kmazzoni
Copy link
Member

When using the GlassInterfaceFactory in a non-default language, not all properties were getting mapped properly. This was because the Base Template lookup was using the current language instead of the Default language.

@smithc
Copy link
Contributor

smithc commented Mar 17, 2017

Hey Kevin,

I've added a change to your PR to use Glass' VersionCountDisabler instead of explicitly resolving the glass items in the 'default' language.

The core issue you discovered looks to arise when an item being requested does not have a version in the current language context. While your change probably works in most cases, I don't think we can necessarily rely on all items having versions in the default language.

My change to use the VersionCountDisabler should ensure that we are returned a version of the item with the Shared fields populated - and this should include the 'Template' and '__Base tempaltes' fields.

You may want to check the change out, and verify my assumptions, but I'm hopeful this should solve the issue.

Thanks!
Chris

@kmazzoni
Copy link
Member Author

Hey Chris,

I think for this specific situation we are looking up templates which will likely always have a version in the default language, I suspect. However, if this a more fail-safe approach I'm all for it. The fix appears to work from my testing. I just checked in a minor change to reduce a little bit of nesting.

Thanks,

Kevin

@smithc smithc merged commit cd2dc31 into Velir:v1-LTS Mar 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants