umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.45k stars 2.68k forks source link

Info Content App - Links - Lots of 'This document is published but is not in the cache' messages one for every 'language' in non variant site #15361

Open marcemarc opened 10 months ago

marcemarc commented 10 months ago

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

12.3.3

Bug summary

Well, this is a curious thing.

I have an Umbraco site with multiple Territories, set up as different sites within Umbraco, those Territories have different culture/languages, and so these are added to the Languages section of Umbraco and applied using the Cultures and Hostnames option for each root node.

The site doesn't use variants, it's not a 1-1 scenario, and there are pages on one territory that aren't in another territory - hence the separate branches.

But when I visit one of the pages on the site, switch to the Info content app, and have a look at the Link presented to the editors I see this:

image

This is confusing editors, as it seems like there is 'something wrong'.

People often mixup territories and cultures in their heads (let's not even mention territories with multiple languages - looking at you Switzerland!) - but although here the message is saying IF you requested the Url of this page on a page which had the culture of Canada, then although the underlying page is published, it's not available in the cache...

... but the editor thinks - how is this published in Canada? there is no Canadian Destinations page...

This kind of information would I think make sense to editors and be super useful too, if the site was using Variants, if there was 1-1, for languages rather than territories, then it would be useful to see all the urls in one place for a node and report which ones were published etc...

... but I'm not using Variants!

I thought this might be easily turned off with a custom UrlProvider - eg these 'other urls' were being added in via the 'Other Urls' functionality of a Url Provider, but it turns out this is not the case.

Apologies if I am doing something stupid here!

Specifics

Digging a bit deeper into this:

The schenanigans are occurring in the GetContentUrlsByCultureAsync UrlProviderExtension:

https://github.com/umbraco/Umbraco-CMS/blob/14a64857df3edbd23558e7d741316db0ddccc098/src/Umbraco.Core/Routing/UrlProviderExtensions.cs#L107

This is running the UrlProvider multiple times, once for each culture:

image

It looks like this is 'by design'

   // build a list of URLs, for the back-office
        // which will contain
        // - the 'main' URLs, which is what .Url would return, for each culture
        // - the 'other' URLs we know (based upon domains, etc)
        //
        // need to work through each installed culture:
        // on invariant nodes, each culture returns the same URL segment but,
        // we don't know if the branch to this content is invariant, so we need to ask
        // for URLs for all cultures.
        // and, not only for those assigned to domains in the branch, because we want
        // to show what GetUrl() would return, for every culture.
        var urls = new HashSet<UrlInfo>();
        var cultures = localizationService.GetAllLanguages().Select(x => x.IsoCode).ToList();

        // get all URLs for all cultures
        // in a HashSet, so de-duplicates too
        foreach (UrlInfo cultureUrl in await GetContentUrlsByCultureAsync(content, cultures, publishedRouter, umbracoContext, contentService, textService, variationContextAccessor, logger, uriUtility, publishedUrlProvider))

My contention...

My contention is that for a non-variant site, this presents a lot of unnecessary and confusing information for editors, which promotes blindness to when there is an actual routing problem!

Also the information is incorrect

image

Here it thinks the node would collide with /Home/Destinations page... but it won't... because the first node in Umbraco is routed as / and not as it's Url Name, but also because the url starts with /us that will be route via culture and hostnames to the US site, so it is untrue to say this will clash!

Bit that needs thought

The assumption which I think needs a bit of thought is this:

 // we don't know if the branch to this content is invariant, so we need to ask
        // for URLs for all cultures.

I think firstly for both variant and non-variant sites - you probably only want to do this for cultures that have been assigned to nodes in the branch of the site, rather than 'all languages in Umbraco'. (you can see in the screenshot I added 'en' language and it immediately appeared as a Url, even though it's not setup to be routed anywhere...)

There is a database table called umbracoDomain that stores the mapping of languageId to NodeId...

image

(it also seem to have entries for the nodes *48036 for the context of the backoffice)

I wonder if it could be used, along with the Path property of the Umbraco item in the backoffice, eg -1, 1234, 134566 - to lookup the cultures available to be varied upon on the current branch rather than using all languages?

good default behaviour for non variant sites?

But if this is actually the desired display for Variant-based sites... could we do something different for non-variant sites?

Could we make it configurable or perhaps only triggered if there is a Document Type on the site set to 'vary by culture' ?

Workaround for future non-variant Umbracians coming here with a similar fear of these extraneous Url messages

The way I'm working around this, for anyone coming to this issue with the same problem is to handle the EditorSendingContentNotification and get the parentNode and use the DomainUtility GetCultureFromDomains() to find the current branches culture, and then remove all the other entries for other cultures... but this, I think will only work if a single culture is assigned to a branch... but if it's useful it looks something like this:

public class EditorSendingContentNotificationHandler : INotificationHandler<SendingContentNotification>
{
    private readonly IUmbracoContextAccessor _umbracoContextAccessor;

    public EditorSendingContentNotificationHandler(IUmbracoContextAccessor umbracoContextAccessor)
    {
        _umbracoContextAccessor = umbracoContextAccessor;
    }
    public void Handle(SendingContentNotification notification)
    {
        if (notification != null && notification.Content != null)
        {
            // get culture of parent?
            string? branchCulture = "";
            if (_umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext umbracoContext))
            {
                var parentNode = umbracoContext?.Content?.GetById(notification.Content.ParentId ?? 0);
                if (parentNode != null)
                {
                    branchCulture = parentNode.GetCultureFromDomains();
                }
            }
            if (!string.IsNullOrEmpty(branchCulture) && notification.Content.Urls != null)
            {
                List<UrlInfo> filteredUrlInfos = new List<UrlInfo>();
                foreach (var url in notification.Content.Urls)
                {
                    if (url.Culture == branchCulture)
                    {
                        filteredUrlInfos.Add(url);
                    }
                }

                notification.Content.Urls = filteredUrlInfos.ToArray();
            }
            if (notification.Content.ContentTypeAlias == "destinationRegion")
            {
                var messages = new List<UrlInfo>();
                messages.Add(UrlInfo.Message("Destination Region pages have no URL."));
                notification.Content.Urls = messages.ToArray();
            }

        }

    }
}

Et Voila! the editor sees one Url that is relevant for the territory they are on, as you'd expect.

Steps to reproduce

Create a New Umbraco Site with a single domain pointing at it Create Four homepages at the root of Umbraco and some pages underneath DO NOT make any of the document types vary by culture - in fact don't even whisper variants to anyone when you are creating them, don't think about variants, just hum beatles songs or something. Add Four languages to the Umbraco Site Assign one of each of those languages to one of each of the four homepages, and use one-level paths for each language site other than the first to define the difference image

Now visit a page underneath any of the sites, switch to the Info Content App, and examine the links... ... see how you have a link for every language in the site!

Is it confusing?

Add more languages to Umbraco - see how they immediately appear as links,even though they are not assigned to domains.

Expected result / actual result

You shold only see Urls on Links for Cultures that have been assigned to the root branch.

github-actions[bot] commented 10 months ago

Hi there @marcemarc!

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 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 :robot: :slightly_smiling_face:

Zeegaan commented 10 months ago

Wierd one, I am seeing to links always (the default language, and then the given culture i assigned via domain) But the other links aren't there for me šŸ˜…

image

marcemarc commented 10 months ago

@Zeegaan so for your TwoLanguage site, and your ChildTo page - why are you seeing 'This document is published but is not in the cache' for the ChildTwo page for da / sv/ am languages?

image

Those cultures arent' added to the TwoLanguage page, and your Umbraco site isn't using Variants... so that is what I'm reporting it's really confusing for the editor that only edits the 'Two language' branch, to have these additional messages about cultures! that sound like something is wrong... but it is not!

regards

Marc

Zeegaan commented 10 months ago

Aha good points, will take this up with the team on what to do about this šŸ˜ šŸ’Ŗ

marcemarc commented 10 months ago

thanks @Zeegaan !! - I think it might have been like this since V8, it's only now that we are migrating all these V7 sites that are coming across these anomalies for non variant sites

But appreciate you taking a look!

bjarnef commented 10 months ago

I noticed something similar in a multi-site solutions where we have 9 languages, but use the old language structure as each site needed its own site structure.

But on each node we then had 8 empty links šŸ˜…

The workaround we have for now.

public class EditorSendingContentNotificationHandler : INotificationHandler<SendingContentNotification>
{
    private readonly IPublishedSnapshotAccessor _publishedSnapshotAccessor;

    public EditorSendingContentNotificationHandler(
        IPublishedSnapshotAccessor publishedSnapshotAccessor)
    {
        _publishedSnapshotAccessor = publishedSnapshotAccessor;
    }

    public void Handle(SendingContentNotification notification)
    {
        if (notification.Content.ContentTypeAlias.Equals(EmployeeArchive.ModelTypeAlias) ||
            notification.Content.ContentTypeAlias.Equals(Employee.ModelTypeAlias) ||
            notification.Content.ContentTypeAlias.Equals(LocationArchive.ModelTypeAlias) ||
            notification.Content.ContentTypeAlias.Equals(Location.ModelTypeAlias) ||
            notification.Content.ContentTypeAlias.Equals(TestimonialArchive.ModelTypeAlias) ||
            notification.Content.ContentTypeAlias.Equals(Testimonial.ModelTypeAlias) ||
            notification.Content.ContentTypeAlias.Equals(TopicsArchive.ModelTypeAlias) ||
            notification.Content.ContentTypeAlias.Equals(Topic.ModelTypeAlias))
        {
            notification.Content.Urls = null;
        }
        else
        {
            notification.Content.Urls = notification.Content.Urls?.Where(x => x.IsUrl).ToArray();
        }
    }
}
marcemarc commented 10 months ago

@bjarnef Thanks, but I think that will hide all the non url messages, which might be genuine warnings!!! here we want messages that aren't necessarily Urls, just not for the cultures that don't apply - if you look at my workaround above I use GetCultureFromDomains on the parent Item, so that only the language specific version is shown and it can be either a message or a url!

bjarnef commented 10 months ago

@marcemarc yes, I think this was a quick workaround and better UI/UX to the customer. It does show the "This item is not published" message, but I haven't checked the "This document is published but is not in the cache" or other error related message.

I always find it a bit odd to show error message inline here. Could probably be redesigned with a warning/error icon instead opening a details overlay with more error/warning details or an inline warning/error notification, similar to languages when changing default language?

image