FIX: Source locale indicator correction.

[mfendeksilverstripe]

FIX: Source locale indicator correction.

Description

Source locale detection corrected to take into account locale inheritance settings for a specific model. Impacted methods are only used in the CMS UI to give indication to content authors on what to expect (what is the source locale of the content). Before this change the indicator could be inaccurate in a sense that it would indicate content and there would actually not be any content to display.

This change should have no impact on actual content rendering.

Change-set breakdown

• RecordLocale::getSourceLocale() now takes into account locale inheritance mode
• FluentExtension::getSourceLocale() is now only a shorthand method to the above method instead of maintaining its own implementation that doesn't cover all the cases

Manual testing steps

• assumes CMS 5 installer + Fluent
• navigate to the CMS under locales admin
• create first locale - English (Any english) and set this locale to be the global default
• create second locale - German and set the this locale to fall back to the first locale
• create a campaign page (code snippet below)

class CampaignPage extends Page
{
    private static string $frontend_publish_required = FluentExtension::INHERITANCE_MODE_EXACT;
}

• run dev/build with flush
• reload the CMS admin pages and navigate to the pages section
• make sure you are in the English locale (see locale selector in the top left)
• create a campaign page (fill any data in)
• navigate to the localisation manager tab within the campaign page edit form
• you should see two records displayed (one for each locale)
• check the source locale for the German locale for this campaign page
• the expected value is "No source" (state after the change), before this change-set it would show English locale

Most common scenarios:

• Page uses fallback inheritance mode (default)
• Assets use any inheritance mode
• Campaign pages use exact inheritance mode

The source locale indicator should be in line with what is shown on the frontend.

Issues

• Source locale doesn't respect locale inheritance mode #842

Pull request checklist

• The target branch is correct
  • See picking the right version
• All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
• The commit messages follow our commit message guidelines
• The PR follows our contribution guidelines
• Code changes follow our coding conventions
• This change is covered with tests (or tests aren't necessary for this change)
• Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
• CI is green

[mfendeksilverstripe]

This is ready for a review @GuySartorelli

[GuySartorelli]

I haven't checked anything beyond what I've requested changes for below, since these changes will require either retargetting the PR or fundamentally changing the approach.

[GuySartorelli]

Suggested change

	public function getSourceLocale(?bool $isFrontend = true): ?Locale
	public function getSourceLocale()

Additional parameters (even optional ones) and explicit typehints cannot be added outside of a major release.
See the definition of public API

[GuySartorelli]

If those changes are required, they will need to target 8 so they can be included in the next major release.

[mfendeksilverstripe]

I'll see if I can find another way 🤔

[GuySartorelli]

Please also avoid @ing me on each PR unless it is something you crucially need to be checked in the immediate future.

[mfendeksilverstripe]

New changes added: updated API so they match minor constraints, so this can be based on 7 branch.

[GuySartorelli]

Haven't reviewed properly yet. I'll add this to my list of things to try get to this week unless someone else gets to it before me

[tractorcow]

Looks good! I was trying to remind myself how inheritance worked, but this line seems to obviously fix the issue. We should never care about fallbacks if inheritance is exact mode only.

[GuySartorelli]

Can you please update the PR description to give actual step-by-step instructions for how to reproduce this from a fresh installation? Include any code samples for setting configuration, and exact steps required - assume I don't know anything about fluent :p

As it stands I'm not really sure what the behaviour was before this PR, what it should be afterward, and how to confirm either of those.

[mfendeksilverstripe]

I've added test setup details into the description of this PR as requested @GuySartorelli