Relevant comment: https://github.com/tractorcow/silverstripe-fluent/issues/313#issuecomment-366563803

Ok, fair enough.

At the moment this is quite a confusing interface though - especially considering the top version is always selected, even when it's not the current version for the Locale you're in.

An indicator + the correct version (relative to your Locale) being selected when you load the page is probably reasonable. Bonus would be the ability to allow/disallow authors from reverting to a version from another Locale.

CC: @frankmullenger

Yeah I agree. I think the logic in the CMS for generating the DataList of those versions is not built in a way that allows Fluent to modify it. Perhaps using the history viewer with SilverStripe 4.2 onwards might help here

@robbieaverill Can you please classify this so it doesn't get lost? Are you using the impact/* labels to go through things on bug day? Ideally this gets looked at for your next bug day

We need to discuss with @tractorcow when he's back, he's previously classed this as a feature

The long and short of it is:

• a version number may be saved in a locale, but it doesn't belong to that locale per se; It's a little ambiguous at times, but every time you save in any locale, it does write to the base record, and that base content (e.g. the "test 3" in the above screenshot) is what's being shown to the user.
• We could just hide content saved in other locales, but there are cases where the user may need to see "all" versions despite. I would probably want a "show only this locale" option that you can turn on. However, that may be a pain to build.
• Filtering the history may be challenging, since you could unintentionally affect other internal API. Versioned::allVersions() does NOT let fluent extend it; It happens via extremely low level custom filters and table renames. CMSPageHistoryController itself has issues; The VersionsForm() method is extremely un-extendable. You couldn't easily do it without subclassing it and replacing it with DI.

tl;dr it's hard and may require core changes to fix.

Oh look, 5.0 uses Versioned::all_versions() instead, which IS extendable.

That means we can do it in 5.0 without core changes. :)

Is this only an issue in 4.1 which doesn't have the new GraphQL-based history viewer yet (due to be released in 4.2)? In this case, it's not worth fixing.

@tractorcow What module version are you referring to with "5.0" here?

It might actually be susceptible to the same problem if it's using the same Versioned API. We've already found blocking problems with not being able to paginate or filter the versions that GraphQL gives you for versioned DataObjects (silverstripe/silverstripe-versioned#147). We might need to backport the functionality in order to move forward with this kind of thing in 4.x

@tractorcow What module version are you referring to with "5.0" here?

Recipe 5.x.

It might actually be susceptible to the same problem if it's using the same Versioned API. We've already found blocking problems with not being able to paginate or filter the versions that GraphQL gives you for versioned DataObjects (silverstripe/silverstripe-versioned#147). We might need to backport the functionality in order to move forward with this kind of thing in 4.x

It seems that the limitation on that versioned API is affecting both fluent and graphql. Maybe we do need to back port it?

For recipe 4.2 we may be able to build a custom "lazy" list, a subclass of DataList, that returns the same Versioned_Version object. That would let us include the new functionality without breaking API / SemVer.

Let's treat this as a bug I think

So I've re-read this issue again after a few months, and basically the problem is that Versioned::allVersions() doesn't return a DataList, so Fluent can't modify the underlying queries.

SS5 has had a (breaking) rewrite and does return a DataList (silverstripe/silverstripe-versioned#81). I think the fix here is to backport that change in a non-breaking way then use that from SilverStripe\Versioned\GraphQL\Operations\ReadVersions::resolve().

This would be only to allow Fluent to adjust the query for history viewer. From here we'd need to make sure that the returned list through Fluent is sensible.

PR at silverstripe/silverstripe-versioned#182 to backport the versioned DataList changes from 5.x to 4.x. I don't think I can justify suggesting it's a patch change unfortunately.

Yep so with that change in place we still get all versions in all locales. We may need to do as @tractorcow suggested an add a filter to history viewer for "Only show versions from this locale" or something like that. I'm looking into how we'd filter this list too - as @tractorcow also mentioned, it doesn't look like the locales actually own the versions

Hi team (@robbieaverill),

I'm not sure if this is entirely related or not. Please let me know and I'll open up a separate issue if need be.

Archive view shows the latest version (not Locale specific)

I have 2 Locales (neither fallback to anything):

• International
• Japan

I publish my Page in both Locales (International first, Japan second).

I then "Unpublish and archive" the Page in both Locales (International first, Japan second).

Result

When you view the record, you get (I assume) a view of the most recent version (not Locale specific). Using "Restore draft" will also restore the non-Locale specific version that you are viewing.

@chrispenny I think it would be easier to track that issue separately - I've moved it to #471

Weeee this is going to be tricky. It looks like Versioned adds an inner join to the versions table in order to get version information, but FluentVersionedExtension isn't rewriting that join.

What Fluent is doing at the moment is replacing SiteTree_Localised to SiteTree_Localised_Versions, which is fine, but when the table is SiteTree_Versions it's not being replaced with SiteTree_Localised_Versions.

I quickly tried making this replacement, but many of the columns required are in the base _Versions table, so we get errors like "WasDeleted" column not existing.

I think we need to put an inner join on any SiteTree_Versions tables to SiteTree_Localised_Versions, and fallback for any columns that don't exist to the SiteTree_Versions table. This is pretty tricky because the original inner join that we need to modify is added by Versioned as raw SQL, e.g. in Versioned::augmentSQLVersionedLatest:

https://github.com/silverstripe/silverstripe-versioned/blob/1/src/Versioned.php#L672-L689

Perhaps one option is to replace this with a SQLSelect implementation, which Fluent might automatically be able to rewrite

Perhaps one option is to replace this with a SQLSelect implementation, which Fluent might automatically be able to rewrite

It doesn't look like our DB layer allows this to happen. I think this might be a "can't fix" issue...

@tractorcow any suggestions?

Hmm, the SQL query has a SourceLocale field added by FluentExtension::augmentSQL - if we can filter that by the current locale then it might work

Alright so I've made some PRs that should hopefully resolve this by adding a filter to the DataList that the GraphQL resolver for version lists now returns (see the issue description for the PRs).

This now filters the versions in the history viewer by the correct locale that created them.

Version numbers don't start from 1 - they're incremented from the latest global version number, so the third or fourth locale you publish a page in will get version 8 and 9 for example.

Apart from a fix for descending versions (silverstripe/silverstripe-cms#2244) where the tests are still running, this has been merged.

All merged 🎉

Massive! Awesome effort. Thanks, Robbie (and everyone)!