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

XPath "in-use" language retrieval issues on unpublished nodes #95

Conversation

lssweatherhead
Copy link
Contributor

- Adds content check for unpublished property
- Handles addition of root nodes where in-use languages won't have been defined yet
@lssweatherhead lssweatherhead changed the title X path in use language retrieval issues on unpublished nodes XPath "in-use" language retrieval issues on unpublished nodes Nov 9, 2017
Copy link
Collaborator

@mattbrailsford mattbrailsford left a comment

Choose a reason for hiding this comment

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

Looks good but just one item I would suggest changing. If you make the change, I'll happily merge it in. Thanks for the PR

IsoCode = x.Name,
Name = x.DisplayName,
NativeName = x.NativeName,
var currentNode = id != 0 ? ApplicationContext.Services.ContentService.GetById(id) : null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd be tempted to refactor out the use of the ContentService here as this will be performing a database hit. You could probably go one of two approaches. 1) given you are really only checking to see if the content is actually published, you could check the published content cache and if it's not their, assume it's unpublished or 2) perform a lucene query on the internal lucene index which contains unpublished nodes and check there. I reckon option 1 would be easier but should do the trick I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great, thanks for taking a look Matt - I'll update and re-commit just now :)

@mattbrailsford mattbrailsford merged commit a5d1fdd into umco:develop Nov 9, 2017
@mattbrailsford mattbrailsford added this to the 1.6 milestone Nov 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants