IPublishedContent extension method AncestorOrSelf(string contentTypeAlias) is inconsistent with the other method overloads

[iliyan-kulishev]

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

11.4.0

Bug summary

The method returns the published model itself when no ancestors type alias match the input, even if its alias doesn't match the input.

Specifics

No response

Steps to reproduce

1. Create a variable of type IPublishedContent
2. Run AncestorOrSelf(string contentTypeAlias) on it, where contentTypeAlias is an alias of a type, that doesn't match the type aliases of the variable or any of the ancestors.
3. Store the result in a variable
4. Inspect the value of the result variable.

Expected result / actual result

Expected: the result should be null.
Actual result: the result is the value of the content itself.

[github-actions]

Hi there @iliyan-kulishev!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[nikolajlauridsen]

Hey @iliyan-kulishev I'm sorry you're having issues, however, as I see it the method is perfectly consistent. It's called AncestorOrSelf so it'll either get the ancestor if it can find it, or return itself, so it wouldn't make much sense if this method ever returned null, since it'll always fall back to itself.

[shearer3000]

I think I've just struck this same issue in Umbraco 13.x (13.2.2 and 13.3.0 to be exact)

expected: AncestorOrSelf("myContentAlias") returns an Ipc of that content type, otherwise null
actual: returns my homepage Ipc (which is not "myContentAlias")

from "Umbraco.Extensions.PublishedContentExtensions"
/// <returns>The content or its nearest (in down-top order) ancestor, of the specified content type.</returns> /// <remarks>May or may not return the content itself depending on its content type. May return <c>null</c>.</remarks> public static IPublishedContent AncestorOrSelf(this IPublishedContent content, string contentTypeAlias)

https://docs.umbraco.com/umbraco-cms/reference/templating/mvc/querying

[TwoMoreThings]

I agree with the previous comments that this is inconsistent and a breaking change from earlier versions

I would expect AncestorOrSelf("myContentAlias") should return null if there are no nodes of the specified type

[nikolajlauridsen]

Hey y'all good points about the earlier versions, I looked into this a bit again and found that the change was introduced in this PR: #12703, where similar issues were stated.

Now this put us in a bit of a predicament since this issue has been there since V11, this means that it's very likely that a good amount of people have already migrated to the new behaviour or made new solutions relying on the current behaviour, and it would be breaking for them to unbreak this now.

Given this I think a discussion with the team is warranted as to the best way to proceed, so I'll bring this up with the team, and get back to you 😄

[nikolajlauridsen]

Heya, we just had a discussion about this in the team.

We concluded that we're not changing the behaviour of the AncestorOrSelf method, mostly because this would be very subtly breaking, and we'd like to avoid that.
Additionally, we're planning to move towards a lazy-loaded content cache, and with this change, we'll have to take a good look at the IPublushedContent extensions, possibly coming up with a new approach.

With both of these in mind we've opted to leave the behaviour as is, I have made this snippet though, where you can add your own extension with your expected behaviour, thus working around the issue.

using Umbraco.Cms.Core.Models.PublishedContent;

namespace Umbraco.Extensions;

public static class CustomPublishedContentExtensions
{
    /// <summary>
    ///     Gets the content or its nearest ancestor, of a specified content type.
    /// </summary>
    /// <param name="content">The content.</param>
    /// <param name="contentTypeAlias">The content type.</param>
    /// <returns>The content or its nearest (in down-top order) ancestor, of the specified content type.</returns>
    /// <remarks>May or may not return the content itself depending on its content type. May return <c>null</c>.</remarks>
    public static IPublishedContent? NullableAncestorOrSelf(this IPublishedContent content, string contentTypeAlias) => content
        .EnumerateAncestors(true).FirstOrDefault(x => x.ContentType.Alias.InvariantEquals(contentTypeAlias));
    
    private static IEnumerable<IPublishedContent> EnumerateAncestors(this IPublishedContent content, bool orSelf)
    {
        if (orSelf)
        {
            yield return content;
        }

        while ((content = content.Parent) != null)
        {
            yield return content;
        }
    }
}

[shearer3000]

thanks for update and workaround sample @nikolajlauridsen 😊