-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Make culture specific update dates work again #19145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amends and tests all look good to me. Updates all seem to work as described too.
Approving based on what you've put in the PR description, but noting a couple of things we may want to consider (either in this PR or another).
-
As you'll see in my testing, I also thought the behaviour of having the "other" variant's published culture date update on publish is a bit weird. You say, "While the latter kinda seems like a mistake, it really is the V13 behaviour, and we need to align with it for V16.", So just querying this - should we really take V13 as "correct" here or rather should we fix for 16 (and maybe even consider fixing for 13 too)?
-
Likely unrelated, but nonetheless jumping out when I'm testing - we seem to have a date kind discrepancy, with the
IPublishedContentdate in UTC but theIContentin server time.
Test Method and Results
To test the date updates I've used this code (similar to yours):
@inject Umbraco.Cms.Core.Services.IContentService ContentService
@{
var contentId = Guid.Parse("22a90a48-b517-472b-9312-a79eab9ffe2c");
var content = ContentService.GetById(contentId)!;
var publishedContent = Umbraco.Content(contentId)!;
<div>Published culture date (en): @publishedContent.CultureDate("en-US")</div>
<div>Published culture date (it): @publishedContent.CultureDate("it")</div>
<div>Content update date (en): @content.GetUpdateDate("en-US")</div>
<div>Content update date (it): @content.GetUpdateDate("it")</div>
<div>Content update date: @content.UpdateDate</div>
}
And get these results, which look correct based on what you've described.
Starting point:
Published culture date (en): 4/25/2025 2:19:35 PM
Published culture date (it): 4/25/2025 2:19:35 PM
Content update date (en): 4/25/2025 4:19:35 PM
Content update date (it): 4/25/2025 4:19:35 PM
Content update date: 4/25/2025 4:19:35 PM
Saved English only:
Published culture date (en): 4/25/2025 2:19:35 PM
Published culture date (it): 4/25/2025 2:19:35 PM
Content update date (en): 4/25/2025 4:20:16 PM
Content update date (it): 4/25/2025 4:19:35 PM
Content update date: 4/25/2025 4:20:16 PM
Saved English and Italian but only updated English:
Published culture date (en): 4/25/2025 2:19:35 PM
Published culture date (it): 4/25/2025 2:19:35 PM
Content update date (en): 4/25/2025 4:20:41 PM
Content update date (it): 4/25/2025 4:19:35 PM
Content update date: 4/25/2025 4:20:41 PM
Saved English and Italian with updates in both:
Published culture date (en): 4/25/2025 2:19:35 PM
Published culture date (it): 4/25/2025 2:19:35 PM
Content update date (en): 4/25/2025 4:21:08 PM
Content update date (it): 4/25/2025 4:21:08 PM
Content update date: 4/25/2025 4:21:08 PM
Published English only:
Published culture date (en): 4/25/2025 2:21:35 PM
Published culture date (it): 4/25/2025 2:21:08 PM
Content update date (en): 4/25/2025 4:21:35 PM
Content update date (it): 4/25/2025 4:21:08 PM
Content update date: 4/25/2025 4:21:35 PM
This looks odd - wouldn't have expected Italian to have updated, but I see from your description this is actually "correct" in the sense of aligned with 13.
Published both:
Published culture date (en): 4/25/2025 2:23:13 PM
Published culture date (it): 4/25/2025 2:23:13 PM
Content update date (en): 4/25/2025 4:23:13 PM
Content update date (it): 4/25/2025 4:23:13 PM
Content update date: 4/25/2025 4:23:13 PM
|
Thanks for testing! Awesome job!
While I would love to fix this, I do believe we need to align as closely as possible with V13 here. Changing this behaviour can have a lot of unforeseen consequences for a lot of people relying on update dates. I'll make a note of it. Let's discuss it further 👍
I saw that too while testing. We've had our fair bit of weirdness with draft content dates being server time, not UTC time. Actually, this area has been improved (stabilised) since V13, but yes - the draft content dates are still in server time. |
Prerequisites
Description
While working on another task, I realized we have introduced a behavioural change for variant update dates in V14 (and beyond).
In V14, the
CultureDateof a published culture variant updates when any variant is published on the document.In V13, the
CultureDateof a specific published variant only updates when:CultureDatebecomes the datetime of the publish action.CultureDatebecomes the datetime of the name edit action.While the latter kinda seems like a mistake, it really is the V13 behaviour, and we need to align with it for V16. So this PR ensures that.
The published
CultureDateis tied to the update date of the draft variant (IContent). And this is where the reason for this change in behaviour is found:To get around this, I have changed the behaviour of updates at service layer level, so updates only register as changes if something actually changed (name or property values).
In effect, this means that
GetUpdateDatefor a draft variant only updates whenThis is a slight behavioural change compared to V13, where the draft variant update date would always change when the variant was saved, regardless of changes having been made or not.
I think we can live with this behavioural change in the grand scheme of things; effectively it won't matter to the editors, since the update date is still updated correctly when publishing. And comparatively, this behavioural change is much preferrable to the current difference between V13 and V14 (as described above).
Breaking change
This is a breaking change, in part due to the behavioural change described above, and in part due to the signature changes of
IProperty.SetValue()andBeingDirtyBase.DetectChanges().Fortuantely, these are minimal changes and they're not behaviourally breaking on their own. In effect they will likely not incur any issues, since consumers don't have to care about the changed return values.
Any consumers going through
IContentfor updates won't notice a difference.Testing this PR
Verify that the
CultureDateof published content andUpdateDateof draft content works as described above - both for variant and invariant content.The following template might help with the testing - at least it's the one I've used 😂