Separate "stages" logic and concept from "versions" logic and concept

[GuySartorelli]

Right now most of the logic for both stages (draft, published) and versioning is held inside the same extension - Versioned.

With that extension, you can have the following:

1. Records which record their version history, but do not have a separation between "Draft" and "Published"
2. Records which both record their version history, and also have a separation between "Draft" and "Published"

It isn't possible to have a separation between "Draft" and "Published" without recording version history.

What's more, the way these two separate concepts have been weaved together results in a lot of complexity both in the code itself and in the way developers need to reason about the code.

Related issues

• Versioned.versioned vs Versioned.versionedstaged is confusing #79
• Refactor Versioned to use more composition #421

Acceptance criteria

• Versioned and Publishable (think about naming, "Staged" is probably better) have their own separate extensions
• It's decided where the Publishable extension should live, e.g. in silverstripe/framework or silverstripe/versioned
• All core classes with Versioned extension have the Publishable extension added too
• Consideration is given to alleviate upgrade pain for projects since the Publishable extension must be added in various places e.g. has_extension(Versioned::class) calls
• The Versioned extension no longer has "modes" it is either applied or it is not.
• Any DataObject can have one, both, or neither of the Publishable and Versioned extensions, i.e. the two extensions combine nicely (i.e. when you publish you create a new version), and can be used separately

Notes

• Current modes are "use stages and version history", and, "only use version history". There is currently a lack of an "only use stages " mode"

[emteknetnz]

I've spent a decent chunk of time looking at this, probably a goods days worth. IMO this is in the "too hard" basket, I'm not convinced the effort to benefit ratio is worthwhile here, and there are other things that we could spend our time on that would yield more for the time invested

The Versioned class is really a large tangled knot of string and splitting it all apart into a pair of extensions, which, while it should make for cleaner code, it's a lot of work. I also can't help thinking that the split won't actually be that clean anyway as you have things like the "WasPublished" column on the _Versions table, which seems like it will naturally result in lots of if ($this->owner::has_extension(Publishable::class)) ... within the Versioned extension, somewhat negating the point of splitting them, and possibly increasing complexity rather than reducing it.

I made a crude attempt to split everything into Versioned / Staged / Mode (temporary thing with both Versioned + Staged code), and split the giant Versioned class into a bunch of traits so that I could getting a better view of what's going on, before eventually giving up which link to this issue. The silverstripe/versioned PR is the relevant one in case anyone's interested

[GuySartorelli]

Having taken a good look at this, I agree that for CMS 6 this is just too much work, and too large a chance of regression for the time we have left.

For future reference the approach I was taking is:

1. Take all of the global functionality (e.g. set_reading_mode()) out of Versioned and put it into a new injectable service.
   • The static methods become instance methods on the new service, which is used as a singleton
   • Collapse "reading mode" and "reading stage" into a single concept. This reduces mental load when deciding which to use.
   • Tidy up that service as much as possible. This includes but is not limited to:
     • possible enum for the current Versioned::DRAFT and Versioned::LIVE consts, plus archived, so we can use the enum in type hints and avoid scenarios with invalid values
     • Consider blending ReadingMode into the new enum or into the new injectable service
2. Split out staging logic from Versioned into a new extension.
   • Archiving is a versioning concept, not a staging concept, since it relies on version history being kept.
   • All methods pertaining to stages move to the new extension, along with several of the augment methods

I didn't even finish step 1 above before I decided the chance for regressions is just too high for the time we have left for CMS 6.

We should consider doing this early in the cycle for CMS 7.

[michalkleiner]

Just a question here - would staging be dependent on versioning? If we made that call that for staging you need versioning, perhaps some of the dependencies would become clearer/less unexpected and we wouldn't need to invent a separate staging mechanism to mimic 2 versions.

Otherwise I would agree that this might be an inefficient effort to benefit ratio.

[GuySartorelli]

would staging be dependent on versioning?

No. It currently is, but there's no reason for it to be. Draft lives in the main table, and published lives in the *_Live table. Neither of those rely on the *_Versions table. So you have draft and published stages of content without retaining versioned history of all changes to a record.

Versioning is "I want to retain history of the changes to my record" which includes the ability to archive and restore.

Staging is "I want to keep my draft content hidden from public eyes until I publish it." which doesn't have any reliance on historical versions at all.

There is some minimal interplay between the two (e.g. restoring a version which was published should restore it to the published stage) but having them be the same thing or be reliant on each other is unnecessary coupling which in my eyes makes it harder to implement, maintain, and reason about rather than easier.

[michalkleiner]

Fair enough — nice explanation. Thanks!