Improve document schedule #17535

Migaroez · 2024-11-14T14:04:07Z

Description

The Management API currently supports publishing OR scheduling of documents, but there is no way to cancel schedules or clear them.

This PR aims to rectify this shortcoming while still maintaining the API model/endpoint and the spirit of the endpoint.

Details

With this PR, it should be possible to

publish one or more cultures
schedule (publish and or unpublish) one or more cultures
update the schedule of one more cultures
clear the schedule for a given culture or cultures
combine any of the 4 actions above across multiple cultures (each action can not be combined within one culture)
read the schedules per culture when requesting the document

Notes

A few things came up while making the necessary changes, please keep these in mind when reviewing/testing this PR

The invariant culture string was mixed between "" and "*", where it makes sense (and there was no sign it would cause any issues) this has been aligned to "*" and placed in a constant for clarity
Regarding full truth vs partial truth: It is very hard, maybe even impossible to combine full context truth (document) vs sub context truth (per culture) in one model. To avoid breaking the endpoint signature and with the idea of allowing editors to only send the data they are working on (one or more cultures) to be updated in the future, this endpoint supports just that.

Testing

You can look for inspiration in the added integration tests, but please try to think outside the box.

# Conflicts: # src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs # src/Umbraco.Cms.Api.Management/Factories/IDocumentPresentationFactory.cs

bergmania · 2024-11-25T08:22:30Z

src/Umbraco.Cms.Api.Management/Controllers/Document/ByKeyDocumentController.cs

@@ -53,7 +71,9 @@ public async Task<IActionResult> ByKey(CancellationToken cancellationToken, Guid
            return DocumentNotFound();
        }

-        DocumentResponseModel model = await _documentPresentationFactory.CreateResponseModelAsync(content);
+        ContentScheduleCollection schedule = _contentService.GetContentScheduleByContentId(content.Id);


It's not optimal to call two services from the controller. I think we should make it a single service call and move the logic to the service layer. Furthermore, when using two services, we should wrap it in a single scope, to avoid funny things that can happen outside of a transaction

src/Umbraco.Core/Services/ContentService.cs

bergmania · 2024-11-26T09:02:07Z

src/Umbraco.Core/Services/IContentPublishingService.cs

+    async Task<Attempt<ContentPublishingResult, ContentPublishingOperationStatus>> PublishAsync(
+        Guid key,
+        ICollection<CulturePublishScheduleModel> culturesToPublishOrSchedule,
+        Guid userKey)
+    {
+        // todo remove default implementation when superseded method is removed in v17+
+        var culturesToPublishImmediately =
+            culturesToPublishOrSchedule.Where(culture => culture.Schedule is null).Select(c => c.Culture ?? Constants.System.InvariantCulture).ToHashSet();
+
+        ContentScheduleCollection schedules = StaticServiceProvider.Instance.GetRequiredService<IContentService>().GetContentScheduleByContentId(key);
+
+        foreach (CulturePublishScheduleModel cultureToSchedule in culturesToPublishOrSchedule.Where(c => c.Schedule is not null))
+        {
+            var culture = cultureToSchedule.Culture ?? Constants.System.InvariantCulture;
+
+            if (cultureToSchedule.Schedule!.PublishDate is null)
+            {
+                schedules.RemoveIfExists(culture, ContentScheduleAction.Release);
+            }
+            else
+            {
+                schedules.AddOrUpdate(culture, cultureToSchedule.Schedule!.PublishDate.Value.UtcDateTime,ContentScheduleAction.Release);
+            }
+
+            if (cultureToSchedule.Schedule!.UnpublishDate is null)
+            {
+                schedules.RemoveIfExists(culture, ContentScheduleAction.Expire);
+            }
+            else
+            {
+                schedules.AddOrUpdate(culture, cultureToSchedule.Schedule!.UnpublishDate.Value.UtcDateTime, ContentScheduleAction.Expire);
+            }
+        }
+
+        var cultureAndSchedule = new CultureAndScheduleModel
+        {
+            CulturesToPublishImmediately = culturesToPublishImmediately,
+            Schedules = schedules,
+        };
+
+#pragma warning disable CS0618 // Type or member is obsolete
+        return await PublishAsync(key, cultureAndSchedule, userKey);
+#pragma warning restore CS0618 // Type or member is obsolete
+    }


Would it be better to forward to the actual implementation?

Suggested change

async Task<Attempt<ContentPublishingResult, ContentPublishingOperationStatus>> PublishAsync(

Guid key,

ICollection<CulturePublishScheduleModel> culturesToPublishOrSchedule,

Guid userKey)

{

// todo remove default implementation when superseded method is removed in v17+

var culturesToPublishImmediately =

culturesToPublishOrSchedule.Where(culture => culture.Schedule is null).Select(c => c.Culture ?? Constants.System.InvariantCulture).ToHashSet();

ContentScheduleCollection schedules = StaticServiceProvider.Instance.GetRequiredService<IContentService>().GetContentScheduleByContentId(key);

foreach (CulturePublishScheduleModel cultureToSchedule in culturesToPublishOrSchedule.Where(c => c.Schedule is not null))

{

var culture = cultureToSchedule.Culture ?? Constants.System.InvariantCulture;

if (cultureToSchedule.Schedule!.PublishDate is null)

{

schedules.RemoveIfExists(culture, ContentScheduleAction.Release);

}

else

{

schedules.AddOrUpdate(culture, cultureToSchedule.Schedule!.PublishDate.Value.UtcDateTime,ContentScheduleAction.Release);

}

if (cultureToSchedule.Schedule!.UnpublishDate is null)

{

schedules.RemoveIfExists(culture, ContentScheduleAction.Expire);

}

else

{

schedules.AddOrUpdate(culture, cultureToSchedule.Schedule!.UnpublishDate.Value.UtcDateTime, ContentScheduleAction.Expire);

}

}

var cultureAndSchedule = new CultureAndScheduleModel

{

CulturesToPublishImmediately = culturesToPublishImmediately,

Schedules = schedules,

};

#pragma warning disable CS0618 // Type or member is obsolete

return await PublishAsync(key, cultureAndSchedule, userKey);

#pragma warning restore CS0618 // Type or member is obsolete

}

Task<Attempt<ContentPublishingResult, ContentPublishingOperationStatus>> PublishAsync(

Guid key,

ICollection<CulturePublishScheduleModel> culturesToPublishOrSchedule,

Guid userKey) => StaticServiceProvider.Instance.GetRequiredService<ContentPublishingService>()

.PublishAsync(key, culturesToPublishOrSchedule, userKey);

bergmania · 2024-11-26T09:09:22Z

src/Umbraco.Core/Services/IContentService.cs

+    ContentScheduleCollection GetContentScheduleByContentId(Guid contentId)
+    {
+        // Todo clean up default implementation in v17
+        Attempt<int> idAttempt = StaticServiceProvider.Instance.GetRequiredService<IIdKeyMap>()
+            .GetIdForKey(contentId, UmbracoObjectTypes.Document);
+        if (idAttempt.Success is false)
+        {
+            throw new ArgumentException("Invalid contentId");
+        }
+        return GetContentScheduleByContentId(idAttempt.Result);
+    }


Suggested change

ContentScheduleCollection GetContentScheduleByContentId(Guid contentId)

{

// Todo clean up default implementation in v17

Attempt<int> idAttempt = StaticServiceProvider.Instance.GetRequiredService<IIdKeyMap>()

.GetIdForKey(contentId, UmbracoObjectTypes.Document);

if (idAttempt.Success is false)

{

throw new ArgumentException("Invalid contentId");

}

return GetContentScheduleByContentId(idAttempt.Result);

}

ContentScheduleCollection GetContentScheduleByContentId(Guid contentId) => StaticServiceProvider.Instance

.GetRequiredService<ContentService>().GetContentScheduleByContentId(contentId);

Maybe?

Migaroez added 12 commits November 7, 2024 11:27

Expose schedule date for on document get endpoint

49760f6

typo fix

366d2ff

stupid stuff

4b457c0

Enable content scheduling features in the publishing service

114d53d

Replace obsoleted non async calls

dcf8e86

Add content scheduling test

7094855

Publush and schedule combination test

c05aaeb

More invariantCulture notation allignment and more tests

73b9786

Link up api with updated document scheduling

dd0bd78

Merge branch 'v15/dev' into v15/task/improve-document-schedule

7d5779b

# Conflicts: # src/Umbraco.Cms.Api.Management/Factories/DocumentPresentationFactory.cs # src/Umbraco.Cms.Api.Management/Factories/IDocumentPresentationFactory.cs

More invariant culture notation allignment

e98f25b

Fix breaking change

2b93255

Migaroez marked this pull request as ready for review November 18, 2024 13:15

Migaroez requested a review from kjac November 18, 2024 13:15

Migaroez and others added 3 commits November 19, 2024 13:59

Merge branch 'v15/dev' into v15/task/improve-document-schedule

9130fb2

Merge branch 'v15/dev' into v15/task/improve-document-schedule

3b9f14d

Return expected status codes.

75079b2

bergmania reviewed Nov 25, 2024

View reviewed changes

bergmania added 2 commits November 25, 2024 14:11

Merge branch 'v15/dev' into v15/task/improve-document-schedule

dbb78a8

Fix constructor

afabd09

Migaroez changed the title ~~V15/task/improve document schedule~~ Improve document schedule Nov 26, 2024

bergmania reviewed Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve document schedule #17535

Improve document schedule #17535

Migaroez commented Nov 14, 2024 •

edited

Loading

bergmania Nov 25, 2024

bergmania Nov 26, 2024

bergmania Nov 26, 2024

Improve document schedule #17535

Are you sure you want to change the base?

Improve document schedule #17535

Conversation

Migaroez commented Nov 14, 2024 • edited Loading

Description

Details

Notes

Testing

bergmania Nov 25, 2024

Choose a reason for hiding this comment

bergmania Nov 26, 2024

Choose a reason for hiding this comment

bergmania Nov 26, 2024

Choose a reason for hiding this comment

Migaroez commented Nov 14, 2024 •

edited

Loading