U4-6322 - Items with delayed publishing get published too early

Created by Sandor Voordes 25 Feb 2015, 10:58:51 Updated by Shannon Deminick 13 Jan 2016, 16:11:29

Relates to: U4-7709

Relates to: U4-7710

Relates to: U4-7711

We found out that using the following lines of code can result in unwanted publication of content.

        var contentService = ApplicationContext.Current.Services.ContentService;
        contentService.RePublishAll();
        umbraco.library.RefreshContent();

Please check if you use these methods in your Umbraco project and remove them! (if they are needed for your solution please contact me to find an alternative solution)

The issue has been reported to Umbraco

Read below for a full explanation of the issue:

Perform these steps

• Put a future publication date on an item • Save the item using the “Save and Publish” button and not the alternative “Save”. • The item is stored in the database marked with “published”, but is rightfully not published. • The feedback to the editor also rightfully shows that the item is not published because it contains a future publication date. ligt. Dit zien wij ook terug in de logs. • Recycle the application pool (as it does every 29 hours by default). • Run these lines of code in your startup procedure. var contentService = ApplicationContext.Current.Services.ContentService; contentService.RePublishAll(); umbraco.library.RefreshContent(); • This will result in the item being published even when it has a future publication date.

It seems that these calls only check if the item is marked as published and if so publishes the item.

In the source code of Umbraco we found out that this is in fact the case.

public void RebuildXmlStructures(Func<IContent, XElement> serializer, int groupSize = 5000, IEnumerable contentTypeIds = null) {

        //Ok, now we need to remove the data and re-insert it, we'll do this all in one transaction too.
        using (var tr = Database.GetTransaction())
        {
            //Remove all the data first, if anything fails after this it's no problem the transaction will be reverted
            if (contentTypeIds == null)
            {
                var subQuery = new Sql()
                        .Select("DISTINCT cmsContentXml.nodeId")
                        .From<ContentXmlDto>()
                        .InnerJoin<DocumentDto>()
                        .On<ContentXmlDto, DocumentDto>(left => left.NodeId, right => right.NodeId);

                var deleteSql = SqlSyntaxContext.SqlSyntaxProvider.GetDeleteSubquery("cmsContentXml", "nodeId", subQuery);
                Database.Execute(deleteSql);
            }
            else
            {
                foreach (var id in contentTypeIds)
                {
                    var id1 = id;
                    var subQuery = new Sql()
                        .Select("cmsDocument.nodeId")
                        .From<DocumentDto>()
                        .InnerJoin<ContentDto>()
                        .On<DocumentDto, ContentDto>(left => left.NodeId, right => right.NodeId)
                        .Where<DocumentDto>(dto => dto.Published)
                        .Where<ContentDto>(dto => dto.ContentTypeId == id1);

                    var deleteSql = SqlSyntaxContext.SqlSyntaxProvider.GetDeleteSubquery("cmsContentXml", "nodeId", subQuery);
                    Database.Execute(deleteSql);
                }
            }

            //now insert the data, again if something fails here, the whole transaction is reversed
            if (contentTypeIds == null)
            {
                var query = Query<IContent>.Builder.Where(x => x.Published == true);
                RebuildXmlStructuresProcessQuery(serializer, query, tr, groupSize);
            }
            else
            {
                foreach (var contentTypeId in contentTypeIds)
                {
                    //copy local
                    var id = contentTypeId;

//THIS IS THE LINE THAT SHOULD CHECK FOR THE DELAYED PUBLISHING VALUE var query = Query.Builder.Where(x => x.Published == true && x.ContentTypeId == id && x.Trashed == false); RebuildXmlStructuresProcessQuery(serializer, query, tr, groupSize); } }

            tr.Complete();
        }
    }

Comments

Jordan Lane 29 Oct 2015, 15:41:19

Took a while but fixed a few other issues while investigating! https://github.com/umbraco/Umbraco-CMS/pull/865


Shannon Deminick 25 Nov 2015, 09:21:03

Hi, firstly to note that you should never use the legacy call:

umbraco.library.RefreshContent();

There is no reason for this and is probably the cause of some of the issues you are seeing.


Shannon Deminick 25 Nov 2015, 09:24:47

@JJCLane Can you also please describe what the 'other issues' that you fixed are so these details can become part of any release notes.


Shannon Deminick 25 Nov 2015, 09:27:20

I'd also like to know why you are using these API calls:

contentService.RePublishAll();
umbraco.library.RefreshContent();


Jordan Lane 25 Nov 2015, 13:53:15

@Shandem I described them in the commits of the PR here: https://github.com/umbraco/Umbraco-CMS/pull/865/commits but let me know if you need any more information.

Out of interest, is there a replacement for .RefreshrhContent() now or is that handled within the .RePublishAll() method?


Shannon Deminick 25 Nov 2015, 15:00:06

Well, i can see why you'd think you'd want to use that method if you are just doing those two calls. But I still don't understand why you are making a call to: contentService.RePublishAll(); That is really more of an internal services method.


Shannon Deminick 13 Jan 2016, 11:19:57

Internal note: New issues to create (for tracking purposes):

  • Fix a bug where PublishStatusType would fallback to FailedContentInvalid (and break) if it was expired or trashed and add error message for FailedHasExpired
  • Checks if content has expired or is awaiting release before publishing
  • Set the publish status of new content version to saved if validation of publication fails e.g. a release date of the future or past unpublish date


Shannon Deminick 13 Jan 2016, 11:20:45

@JJCLane Now i understand the issue you fixed - thanks a ton, it makes sense and i'll pull this in and update/add tests today - this should also fix: http://issues.umbraco.org/issue/U4-5982


Jordan Lane 13 Jan 2016, 11:38:45

@Shandem Brilliant! I'm not sure duplicating the function was the best solution [https://github.com/JJCLane/Umbraco-CMS/commit/a6b56db2f542a3a9e971f5798cc9fe879d31e211#diff-d0975eccc13244396f1b4e9181030669R790 here], but I was unsure how method overloading could be used and still keep backwards compatibility with the method signature. Would be interested to hear your thoughts.

Thanks,

Jordan.


Shannon Deminick 13 Jan 2016, 11:40:44

I've changed some of the code (not much), not sure what breaking changes you are referring too ?


Shannon Deminick 13 Jan 2016, 11:41:55

btw, the PR for internal review is here if you want to look: https://github.com/umbraco/Umbraco-CMS/pull/1018


Jordan Lane 13 Jan 2016, 11:48:53

@Shandem Just that I couldn't think of a way to alter GetPagedResultsByQuery to remove the filter and keep backwards compatibility, so created the GetPagedResultsByQueryNoQuery instead.


Shannon Deminick 13 Jan 2016, 16:11:02

@JJCLane Ah, at the repository level it doesn't matter, the repositories are all internal. I removed that method though in favor of just making the call inline ( you can see in that PR)


Shannon Deminick 13 Jan 2016, 16:11:27

@Claus.Jensen I'm closing this issue since you pulled in the PR - which relates to this one and the other one.


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category: Architecture

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions: 7.1.8

Due in version: 7.3.5

Sprint: Sprint 6

Story Points:

Cycle: