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

Issues in implementation of ILocalizationService #7394

Open
MatteoPiovanelli-Laser opened this issue Nov 11, 2016 · 0 comments
Open

Issues in implementation of ILocalizationService #7394

MatteoPiovanelli-Laser opened this issue Nov 11, 2016 · 0 comments
Milestone

Comments

@MatteoPiovanelli-Laser
Copy link
Contributor

related to #7386 and #7352.
As discussed with @sebastienros, I am opening an issue about the methods from ILocalizationService.

Method: GetLocalizedContentItem(IContent content, string culture, VersionOptions versionOptions)
(versionOptions is optional because there is a version of the method without that parameter)
Expected behaviour: Call a "LocalizationSet" the set of all contents that are localizations one of the other. I would expect this method to return the item from content's LocalizationSet whose Culture property matches the one given in the string culture.
Actual behaviour: That only happens if content is the MasterContentItem for its set. If that's not the case the method returns:

  • null if content's Culture does not match culture.
  • content otherwise.
    This is all down to the query used in this method:
return _contentManager
    .Query<LocalizationPart>(versionOptions, content.ContentItem.ContentType)
    .Where<LocalizationPartRecord>(l =>
        (l.Id == content.ContentItem.Id || l.MasterContentItemId == content.ContentItem.Id) // <-- I THINK THE ISSUE IS HERE
        && l.CultureId == cultureRecord.Id)
    .Slice(1)
    .FirstOrDefault();

As things are now (with #7386), given var localized = content.As<LocalizationPart>();, I would change the line I marked above to something like:

(l.Id == content.ContentItem.Id || l.MasterContentItemId == content.ContentItem.Id || (localized.MasterContentItem != null && l.MasterContentItemId == localized.MasterCotnentItem.Id))

I think that change would allow to at least look into the set of items that share content's MasterContentItem.

Method: GetContentCulture(IContent content)
Actual behaviour: This method returns content's culture if content has a LocalizationPart. It returns the Site's culture otherwise (which is debatable).

Method: SetContentCulture(IContent content, string culture)
Actual behaviour: This method is fine. If content has a LocalizationPart, it will wet its culture.

Method: GetLocalizations(IContent content, VersionOptions versionOptions)
(versionOptions is optional because there is a version of the method without that parameter)
Expected behaviour: I would expect this method to return what we called "LocalizationSet" above.
Actual behaviour: This mostly works as expected. However, because of #7386, LocalizationSets are fragmented whenever some localizations are not created directly starting from a common MasterContentItem.

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

No branches or pull requests

2 participants