Skip to content

Content: Ensure correct variant change tracking when unpublishing variant content - #22799

Merged
kjac merged 3 commits into
v17/devfrom
v17/bugfix/published-state-tracking-when-unpublishing
May 12, 2026
Merged

Content: Ensure correct variant change tracking when unpublishing variant content#22799
kjac merged 3 commits into
v17/devfrom
v17/bugfix/published-state-tracking-when-unpublishing

Conversation

@kjac

@kjac kjac commented May 11, 2026

Copy link
Copy Markdown
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Description

This PR addresses a couple of issues with our tracking of published status for variants, specifically tied to unpublishing variant content:

Invariant remains published after unpublishing cultures in turn

After unpublishing all cultures one at a time, the IPublishStatusQueryService still reports the now fully unpublished content as being published in the invariant culture.

Publish state tracking fails when unpublishing all cultures

When "bulk" unpublishing all cultures in a single operation (IContentService.Unpublish() with no culture specified), IPublishStatusQueryService continues to report all culture variants of the content as published.

Testing this PR

The first issue can be reproduced using the backoffice, because this is exclusively how the backoffice works - it only ever unpublishes specific cultures (likely this is also why this issue has remained undiscovered).

The second requires a bit of custom code to perform the unpublishing. I have used this controller:

using Microsoft.AspNetCore.Mvc;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Services;

namespace Umbraco.Cms.Web.UI.Controllers;

public class ReproduceIssuesController : Controller
{
    private readonly IContentService _contentService;

    public ReproduceIssuesController(IContentService contentService)
        => _contentService = contentService;

    [HttpGet("/repro/unpublish-variants-tracking")]
    public IActionResult Run(Guid id)
    {
        IContent? content = _contentService.GetById(id);
        if (content is null)
        {
            return NotFound("The content was not found.");
        }

        if (content.Published is false)
        {
            return BadRequest("The content was not published.");
        }

        if (content.ContentType.VariesByCulture() is false)
        {
            return BadRequest("The content type does not vary by culture.");
        }

        PublishResult result = _contentService.Unpublish(content);
        return result.Success
            ? Ok("Success.")
            : BadRequest($"Could not unpublish: {result.Result}");
    }
}

Now, given this content structure, where all pages are published in en-US and da-DK:

image

...and given this template:

@inject IContentService ContentService;
@inject IPublishStatusQueryService PublishStatusQueryService;
@using Umbraco.Cms.Core
@using Umbraco.Cms.Core.Services
@using Umbraco.Cms.Core.Services.Navigation
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage
@{
	Layout = null;
    var children = ContentService
        .GetPagedChildren(Model.Id, 0, 1000, out _)
        .ToArray();
}
<html>
<body>
<ul>
    @foreach (var child in children)
    {
        <li><b>@child.Name</b></li>
        <ul>
            <li>Is published (en-US): @PublishStatusQueryService.IsDocumentPublished(child.Key, "en-US")</li>
            <li>Is published (da-DK): @PublishStatusQueryService.IsDocumentPublished(child.Key, "da-DK")</li>
            <li>Is published (invariant): @PublishStatusQueryService.IsDocumentPublished(child.Key, Constants.System.InvariantCulture)</li>
        </ul>
    }
</ul>
</body>
</html>

...this is the initial rendering:

image

When unpublishing all cultures of "Child 1" from the backoffice, the output still claims that "Child 1" is published for the invariant culture:

image

When unpublishing "Child 2" using the custom controller, the output continues to claim that "Child 2" is fully published:

image

...when in fact it is not - which the backoffice correctly displays:

image

@kjac
kjac marked this pull request as ready for review May 11, 2026 15:40
@claude

claude Bot commented May 11, 2026

Copy link
Copy Markdown

Claude finished @kjac's task in 4m 25s —— View job


PR Review

Target: origin/v17/dev · Based on commit: 09015d52
Skipped: 0 noise files out of 4 total

Fixes two related bugs in variant publish-status tracking: after all cultures are individually unpublished the in-memory status cache retained a stale entry (causing IsDocumentPublished(..., "*") to return true), and a bulk Unpublish() call with no culture specified left the cache fully un-cleared. Both root causes are addressed — per-culture dirty tracking in ClearPublishInfos() and cache-key removal in UpdatePublishedCultures() when the published-cultures set is empty.

  • Modified public API: ClearPublishInfos(this IContent content) — signature unchanged; behaviour change: now iterates each culture through ClearPublishInfo(culture) (triggering TouchCulture per culture) before setting PublishCultureInfos = null.
  • Other changes: IPublishStatusManagementService.AddOrUpdateStatusAsync now removes a document key from the in-memory cache when its published-culture set is empty (previously set an empty set, causing false-positive invariant hits).

Suggestions

  • tests/Umbraco.Tests.Integration/Umbraco.Core/Services/PublishStatusServiceTests.Query.cs:248: Assert.IsEmpty(child.PublishCultureInfos!) — the null-forgiving ! is a compile-time annotation only; if the repository returns null for PublishCultureInfos after a full unpublish, Assert.IsEmpty(null) throws ArgumentNullException instead of a meaningful assertion failure. Consider Assert.That(child.PublishCultureInfos, Is.Null.Or.Empty) to cover both outcomes cleanly.

  • src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs:176: publishedCultures.Any() — minor inconsistency with IsDocumentPublishedInAnyCulture (same file) which uses publishedCultures.Count > 0. Prefer .Count > 0 for consistency and to avoid the enumerator allocation on hot paths. Fix this →


Approved with Suggestions for improvement

Good to go — the bug fixes are correct, well-targeted, and backed by a well-structured integration test covering both unpublish paths. Please carefully consider the importance of the suggestions.

@claude claude Bot added the area/backend label May 11, 2026

@AndyButland AndyButland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good and tests out as expected @kjac. I've checked the before and after on v17/dev and this branch and see the issue and resolution.

Left a couple of nit-pick comments but then all good to merge.

Will leave you to label and cherry-pick once that done, but seems to me this should be:

  • Cherry-picked to release/17.5.0 and labelled with release/17.5.0.
  • v17/dev merged to main.
  • Cherry-picked to release/18.0 and labelled with release/18.0.0.

Comment thread src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs Outdated
Comment thread src/Umbraco.Core/Models/ContentRepositoryExtensions.cs
kjac and others added 2 commits May 12, 2026 08:02
@kjac
kjac merged commit c784858 into v17/dev May 12, 2026
26 of 27 checks passed
@kjac
kjac deleted the v17/bugfix/published-state-tracking-when-unpublishing branch May 12, 2026 07:58
kjac added a commit that referenced this pull request May 12, 2026
…iant content (#22799)

* Ensure correct change tracking when unpublishing

* Update src/Umbraco.Core/Services/PublishStatus/PublishStatusService.cs

Co-authored-by: Andy Butland <abutland73@gmail.com>

* Add comment

---------

Co-authored-by: Andy Butland <abutland73@gmail.com>
@kjac

kjac commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-picked to 17.5 in fc9ca86

kjac added a commit that referenced this pull request May 12, 2026
@kjac

kjac commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-picked to main in 6766eb9 (adapted to the changes from Elements)

kjac added a commit that referenced this pull request May 12, 2026
@kjac

kjac commented May 12, 2026

Copy link
Copy Markdown
Contributor Author

Cherry-picked to 18.0 in 3daf727

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants