search

umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.43k stars 2.67k forks source link

Umbraco v10 Rollback fails #13520

Open jimmierindal opened 1 year ago

jimmierindal commented 1 year ago

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

10.3.2

Bug summary

  1. Under settings, enable a language more, than default. ex Danish and English
  2. Create DocumentType with text field.
  3. On documentType enable "Allow vary by culture"
  4. Create a content page based on document type.
  5. Modify textfield - Save and publish
  6. Modify another change to text field - Save and publish
  7. On content page, go to info -> rollback. Select previous version, and trigger rollback - Save and publish

Specifics

No response

Steps to reproduce

  1. Under settings, enable a language more, than default. ex Danish and English
  2. Create DocumentType with text field.
  3. On documentType enable "Allow vary by culture"
  4. Create a content page based on document type.
  5. Modify textfield - Save and publish
  6. Modify another change to text field - Save and publish
  7. On content page, go to info -> rollback. Select previous version, and trigger rollback - Save and publish
  8. Verify missing rollback on change

Expected result / actual result

No response

nikolajlauridsen commented 1 year ago

Hey @jimmierindal unfortunately I'm unable to reproduce the issue, I've followed your steps, but it rolls back just fine, I've made a video here of it, am I missing something?

https://user-images.githubusercontent.com/16456704/205627761-136a31d8-85fd-4fc8-8b24-227e4c3a7ceb.mp4

nikolajlauridsen commented 1 year ago

Hey @jimmierindal I got some help from the friendly people in support and can now reproduce the issue.

In order to reproduce the issue I had to:

  1. Add a second language
  2. Create a document type with some property and make sure it does not allow vary by culture
  3. Create a piece of content with that document type and save and publish some value in the property type
  4. Now go and enable "allow vary by culture" on the document type
  5. Edit the property on the document type and save and publish
  6. Try and roll back to the initial value saved before allow vary by culture was enabled
  7. See how it doesn't roll back.

I also checked back on V8 and noticed that the behaviour was the same there.

I'll just leave some initial thoughts here.

This also sort of makes sense to me because when you enable vary by culture you essentially end up with two version, Language A and Language B, and how are we supposed to know what language we can roll back to the invariant version there was before?

Granted an assumption could be made that it should be possible for the default language, but what then if you change your default language afterwards? Then we'll be in trouble again.

It could also be that the language that's set as default when the change happens "inherits" all the versions from when the content node was invariant, but this might be problematic if you end up going back and forth a lot, and potentially even changing default language in between.

Nonetheless, these are just my immediate thoughts, we'll have to have a talk about it on the team and then dig a bit deeper to see what the resolution should be.

nikolajlauridsen commented 1 year ago

Hi again, we had a discussion about how this case should be handled.

As stated in the comment above, trying to roll a variant piece of content back to a state where it was invariant is quite problematic. Therefore we suggest changing the rollback dialog to no longer show the invariant versions once a piece of content is no longer invariant, and similarly, if the content becomes invariant again, only show the invariant versions.

This should be a matter of changing ContentVersionService.GetPagedContentVersions and DocumentVersionRepository.GetPagedItemsByContentId to only return the versions that have an entry in the umbracoContentVersionCultureVariation table, for variant content, and similarly, for invariant content only those that do not have an entry there.

With that said, we'd love some help with this, so I'll go ahead and leave this as up for grabs now that we have a way to proceed.

Thanks again for reporting 😄

Ambertvu commented 12 months ago

@nikolajlauridsen

Hi Nikolaj, (or anyone else reading this)

I've been looking into this, but seem to have a bit of trouble in making the distinction between variant/non variant. Do you perhaps have some pointers on that? I'm relatively new to directly working with Umbraco's database.

I've done a fresh install, created a new doctype, made 3 changes, checked allow variant, then made 2 more. When I check the DB I see the following. [umbracoContentVersion] image

[umbracoContentVersionCultureVariation] image

The only distinction I can see (as [umbracoContentVersionCultureVariation] is basically the availableUserId (which weirdly has a description to be renamed to updateDate). So theoretically I could only show the items that have null for availableUserId when languageId.Hasvalue (seems to be true when there are variants), but that really seems like a crappy fix, haha.