Allow $owns on non-Versioned objects.

[sminnee]

Although we introduced it for better versioning support, "owns" can have semantics beyond that, and I don't think we need to limit its application to objects using Versioned.

The main benefit is being able to handle the case of SiteTree ---owns--> UnversionedObject ---owns--> File can be configured to ensure that the File is published with the SiteTree. This is a serious bug that occurs frequently when upgrading SS3 -> SS4, and the only current fix is to add versioning to UnversionedObject, which I agree is the "best" fix, but it's quite invasive and versioning in 4.0 might not be quite polished enough for this to be free of annoying side-effects.

In addition, owns can be used to duplicate records within has_many relations, and duplicate links to records specified in many_many relations, without special duplicate overrides. Note that has_ones already work, and the actual records in has_one / many_many shouldn't need to be duplicated, only the links to them.

Acceptance Criteria

• publishRecursive() is supported on all objects, and will publish all owned objects regardless of whether any are versioned. This necessitates that all objects can be added to a changeset (even if not versioned)
• All config and methods related to ownership (e.g. owns, owned_by, findOwned, findOwners, findOwnersRecursive
• Non-versioned objects are visible in the campaign admin area when added to a changeset
• The default "Save" action for model admin and gridfield for un-versioned objects will publish all owned objects

Exclusions:

• UX for interface of unversioned objects. See https://github.com/silverstripe/silverstripe-versioned/issues/71
• This functionality isn't necessary in absentia of the versioned module.

PRs

• [x] https://github.com/silverstripe/silverstripe-versioned/pull/74
• [x] https://github.com/silverstripe/silverstripe-framework/pull/7695 (docs update)

[tractorcow]

I think we need to be careful, since publishing a versioned object adds this object + all owned objects to a changeset.

We may need to allow non-versioned objects to be added to changesets to support this feature; Saving any un-versioned object that at least one 'owns' would trigger a new changeset.

This also shows an UX expectation risk; Why does saving an un-versioned object trigger an entire publish, which doesn't happen with other versioned objects? We'd need to ensure the UX was updated to reflect this (Save & Publish All Objects) vs (Save)

[chillu]

This just came up on the #ss4 Slack again, but in the variation of UnversionedObject ---owns--> File. In this case it was TeamMember ----has_one/owns--> ProfilePhoto, which isn't what I'd consider a "publishable" entity. That'll confuse the hell out of many devs (including myself). I think if you declare ownership, you're stating a preference for these dependencies to be published as a developer.

@clarkepaul @newleeland What do you think in terms of UX? Damian has a point about the misleading "Save" button label. In the case of TeamMember has_one/owns ProfilePhoto, I don't think anyone will be surprised by the outcome. And if that's something more "versionable" like PromoCode has_one/owns PromoCallToAction, the record should probably be versioned to begin with to avoid any UX confusion.

[Firesphere]

@kevinreynolds

[tractorcow]

This also shows an UX expectation risk; Why does saving an un-versioned object trigger an entire publish, which doesn't happen with other versioned objects? We'd need to ensure the UX was updated to reflect this (Save & Publish All Objects) vs (Save)

I would suggest maybe keeping save / publish as separate actions, but make publish available for un-versioned records, and enable adding of un-versioned records to changesets.

[clarkepaul]

What about removing the complexity of whats happening by labeling buttons differently eg. use "Update" instead of Save or Publish.

[chillu]

More discussions (around assets) in https://github.com/silverstripe/silverstripe-framework/issues/6949

[ivoba]

i just stumbled over this, FYI https://stackoverflow.com/questions/47392211/silverstripe-file-relation-in-modeladmin-doesnt-publish

[tractorcow]

Yeah, I think that non-versioned records definitely need another action.

[Save] [Save this and publish children] (wording tentative). This publishes owned objects, although the object itself doesn't get saved any differently. This begs the question what is the best "user-friendly" term to use for owned objects? Is it "children"?

I don't think we should use [Save and publish] as this implies the object itself is being published, or is versioned, which it isn't.

Non-versioned records without owned objects should only have save.

[sminnee]

I'm a little worried that this would expose to the author concerns that are really best taken care of by the developer.

Specifically: the design goal of the $owns system was to provide a way for the system to take care of the publication of related records automatically, so that the author didn't need to worry about it.

Creating two buttons puts the decision back in the author's hand, which I think runs counter to that.

As such I would probably recommend that deciding whether a save action also cascades to publishes is something that should happen on a per-UI basis, as a piece of developer config.

If there's a case where both buttons are definitely needed, I suspect that the wording of the button should be application-specific, and so providing a piece of developer config to set it would be good.

As a default, I would probably go with also publishing owned records, simply because that's what the ownership metadata indicates should happen. If there are no owned records, a simple save will do.

[tractorcow]

As a default, I would probably go with also publishing owned records, simply because that's what the ownership metadata indicates should happen. If there are no owned records, a simple save will do.

I disagree that it's always an assumption this is the expected behaviour.

I would balance this with a concern that it shouldn't be "assumed" and automatic; The knowledge that this save will effect a publish of objects the content author may not wish to publish will result from saving an object. This could be conveyed through action label, other UI components, widgets, alerts, or confirmations.

[sminnee]

I acknowledge the ambiguity. Perhaps we need to consider a more specific use-case when deciding how to approach this.

Let's say that as we have on ss.org we have use the (unversioned) Member object used as the basis of a profile page. The profile page has an avatar; as such we have Member has_one Avatar => File and Member owns Avatar.

This appears in two UIs:

• The profile editing / member details forms as powered by getCMSFields() have an Avatar UploadImage.
• Similarly, the front-end site has an "edit your profile" form has a front-end field

Seem like a reasonable example?

If so, we have 2 questions:

• What should the buttons be on the front-end?
• What should the buttons be in the CMS?

In the case of the front-end, I don't see much of a choice. The end users have no knowledge/control over the publication workflow and their image will need to get to the published stage as soon as they open it. However, there are two ways of achieving this:

• After the Member record is written, all owned records are published, OR
• Writes to related content go straight to the live site

The latter may be better as it wouldn't leave the front-end user with the ability to publish other draft content linked to the Member.

So... perhaps that's also a route to a solution in the case of CMS forms? When you're editing a non-versioned data then any changes to owned records are written straight to live, but pre-existing draft content isn't published?

[tractorcow]

How about something similar to this?

[ivoba]

Hi again :), imho this is a bigger issue. I use GridFieldOrderableRows on a DataObject that has a has_many on ChildDataObject. However when i reorder the ChildDataObjects and click publish on the owning DataObject the Children dont get published and i need to publish each ChildDataObject. I think this should happen automatically.

All DataObjects have the Versioned extensions enabled and the "owns" property is declared on the parent.

[dhensby]

All DataObjects have the Versioned extensions enabled and the "owns" property is declared on the parent.

This sounds like an unrelated issue? I think it's worth opening a ticket specifically for that problem.

[chillu]

The knowledge that this save will effect a publish of objects the content author may not wish to publish will result from saving an object

If the developer declares $owns, that decision is made for the author to a certain extent. In the particular context of the domain model, and (hopefully) embedded with some author training and messaging. I think we need to be careful to separate this out from the wider discussion of "sometimes you shouldn't declare owns on relationships". For example, owning a many_many would be just as ambiguous and confusing regardless if the owner is versioned or not.

So... perhaps that's also a route to a solution in the case of CMS forms? When you're editing a non-versioned data then any changes to owned records are written straight to live, but pre-existing draft content isn't published?

You couldn't just write to live, otherwise a later publish of pre-existing draft content would overwrite your changes again. So the whole concept of version tracking in core gets watered down, which I think we should be quite careful about taking on maintenance for (or exposing APIs to allow for that).

I don't see this as fundamentally different from publishing versioned records: You have a bunch of tabs, they might contain related objects. Those objects might have a "changed/draft" label attached (or will have at some point, separate issue).

I think "attached versioned items" isn't a good way to describe this, unless we can list which fields are actually affected by this (with their user-facing labels). We could turn it around, and add a warning to any relationship fields which aren't owned by the parent - regardless of this parent being versioned or not. This would set the right default expectation for authors (everything is published when I press the green button).

@clarkepaul @newleeland I think we need your input here :D

[sminnee]

You couldn't just write to live, otherwise a later publish of pre-existing draft content would overwrite your changes again.

Sorry, I meant "write the changes to both live and draft while leaving other draft changes unpublished"

[clarkepaul]

I know we normally have dev defined "owns" and "doesn't own" relationships (is there a better name for no ownership?), but do we have a "User defined owns" type thing? I see a "user defined" ownership being really valuable option to both dev's and users but only used when appropriate.

[tractorcow]

The intention is that the schema is set by the dev, and the user will assign owned objects by adding individual objects to that relation.

[clarkepaul]

I see there is a potential value when it comes to publishing. The user could then either decide to publish the owner by itself or all of it owned things as well.

[newleeland]

I would recommend we drop the words:

• SAVE. As this effectively published and conflicts with save draft behavioural patterns used in pages
• PUBLISH. As if we were simply updating content that's not going to be visible on the website, such as a "backup email", publishing is not the right terminology. Furthermore, the term "publish" has behavioural links to an expectation to "unpublish", which is not available.

Potential design

[kinglozzer]

Not sure about “Update”, it doesn’t suggest to me that you’re saving data, rather refreshing it from source. Could we use “Save”/“Save Now”/“Save Changes” for all unversioned DataObjects, and change versioned ones to always say “Save Draft” if they don’t already? Different button styles/icons could perhaps help make it clearer which action you’re performing.

I like the wording “Publishes all underlying content” 👍

[cansozeri]

I have tried bulkupload and no success with owns or anything. Maybe it is not related about this topic but I think bulkupload might be a standard and publishing the images from dataobject would be an auto behaviour. Maybe there would be an option passive - active option for images or attachments and if the statement of the item is passive, then it is not gonna be published ...

I have tried many_many relation with images but no luck. Yes I have uploaded more than one images with uploader component - but the relation is broken with template.. I think the uploader component must be redesigned with a bulk upload feature.

[clarkepaul]

Bit of a strange concept with site settings in this example @newleeland but I understand that it's just placeholder.

I think the wording of the action could either work with "Update" or "Publish", the benefit of using update means there is something to differentiate whether something has a history or not, even if it isn't super obvious, it still closely identifies with a single version of something which can be updated as opposed to creating another version. "Publish" is better than using "save" in my opinion as it uses the same thinking that when I click the button, this information becomes live on the site in whatever form it takes.

The problem I see with the second concept for users is that they can't publish the parent item without forcing them to publish all things below it (there's also no preview or way to see everything that's happening), even listing all the things that are changing isn't really visual enough for users to understand.

@newleeland and I have also talked about being able to see a list of changes in a similar view to campaigns (perhaps in modal) showing the relationships and complexity of whats happening where you might be able to preview and protect items from being published.

In addition to a forced publish of owned items, I am in favor of adding a user defined publishing model where they don't need to publish all owned items unless they are comfortable with the action but probably not having the option checked by default.

[tractorcow]

@newleeland looking good. I feel the only missing piece of the solution is the UX component. I wouldn't be against either of the ideas you've posted (wording withstanding).

[chillu]

"Publishes all underlying content" is slightly inaccurate, it only publishes content which is declared as an "owned" relationship (by a dev in code) and is versioned. Relationships which aren't versioned are visible when they are saved, regardless of any actions on the parent object. Relationships which are versioned and not "owned" shouldn't exist, since there's no way for the author to trigger a publish.

I talked to @newleeland about indicating which relationships are versioned and owned on individual field. That could work for larger fields like GridField, but would get messy for compact fields like UploadField, TagField etc.

Not keen on "Update", for the reasons cited by @kinglozzer. "Publish" is problematic as well, since it implies that this content is visible on the website somewhere. You'd "Save" a member, but it's never "published" on the website. How about sticking with "Save", and conditionally adding "Publishes all underlying content" only when ownership relationships exist?

Allow authors to opt-out of publishing owned relationships seems like a pretty advanced use case that would require the author to understand details that aren't obvious in the UI (which relationships are owned). I'd rather rely on developers setting good defaults here.

[clarkepaul]

@chillu Opt-in/out of publishing owned items isn't any more complicated for a user than forcing them to publish things that they can't see.

Imagine you want to change the name of a page but don't want to publish any of the blocks or something hidden in a content block, the user has no way of doing that. I'm saying that as a developer you have three options: no ownership, forced ownership, or has ownership but user has to opt for it.

By the way, we have dropped "draft" from the action "save draft" so its just "save" (as per files area). I think using just save as a label for things that impact the live site feels wrong as we use it as a safety action at the moment. If we applied the word "changes" to say "Update changes" or "Apply changes" it might work out better.

[clarkepaul]

We'll take a look around and see what other systems use but from my knowledge no one has such an advanced publishing as we do.

Experience manager uses "Done" for saving images. Concrete 5 uses "Save" and "Publish changes" Sitecore has advanced options when it comes to publishing:

[sminnee]

You know what this highlights? The person who raised the issue the 2nd comment on this thread by Ingo should refactor their app to use Versioned. :-P Like, seriously it's the only way to get a consistent user experience.

Perhaps for this case we should be aiming to have some warning lights to show that it's an odd situation rather than creating the most streamlined experience.

[chillu]

OK, after another discussion with the team, I'm trimming this issue back to the immediate problem at hand: Providing a default behaviour that's unsurprising to most authors and devs. This issue mostly surfaces on File relationships, which weren't versioned in 3.x. Let's fix the ORM behaviour for this to be less surprising (non-versioned objects publish $owns by default). And deal with the author comms and UX impact as a separate issue: https://github.com/silverstripe/silverstripe-versioned/issues/71. I've summarised the proposed approaches there, please continue UX focused discussions over there.

[tractorcow]

I would like to add that I feel the most appropriate place to implement this is at the controller level, rather than the model level; Baking this into DataObject::save() rather than LeftAndMain::save() would remove control from the developer.

[tractorcow]

WIP at:

• https://github.com/open-sausages/silverstripe-framework/tree/pulls/4/versions-of-atlantis
• https://github.com/open-sausages/silverstripe-versioned/tree/pulls/1/versions-of-atlantis

[tractorcow]

Pull requests up (see top-most comment).