Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Articulate v5: Can filter articles by category or tag #439

Closed
khraibani opened this issue Jan 15, 2024 · 16 comments
Closed

Articulate v5: Can filter articles by category or tag #439

khraibani opened this issue Jan 15, 2024 · 16 comments

Comments

@khraibani
Copy link

I am getting the following error when try to filter posts by tag or category, I get the same internal server error
/blog/tags/ or /blog/categories/

image
@Shazwazza
Copy link
Owner

Steps to replicate please. What version are you using? You can see this is working on my site fine https://shazwazza.com/categories, https://shazwazza.com/tags

@khraibani
Copy link
Author

Just visiting the url /blog/tags/ or /blog/categories/ trigger the error
Umbraco v12.3.6 - Articulate v5.0

@gavinfaux
Copy link
Contributor

gavinfaux commented Jan 16, 2024

@khraibani is your articulate instance an upgrade from a previous version?

We see exactly the same issue/error on a site upgraded from v7 to v10 - we have disabled the category and tag routes on this instance till we resolve the issue. (We added IIS Rewrite rules to redirect /tags and /categories back to blog root short term).

Initial thoughts this may be data/migration related, not had a window to debug and dig further yet, but will try and find some time this week and will update thread with findings.

@khraibani
Copy link
Author

@gavinfaux no It was a clean install on v12.
Thank you for investigating it further.
@Shazwazza what version of Umbraco you are running?

@Shazwazza
Copy link
Owner

Are you using the latest articulate? 5.0.1 https://www.nuget.org/packages/Articulate/5.0.1

My site is running on umbraco cloud with version 12.3.6

@wvdweerd
Copy link
Contributor

I get the exact same error as @khraibani .

My current setup is: Umbraco 13.0.3, Articulate 5.0.0, two root nodes with their own domain (one is the Articulate node, the other the rest of my website).
All still running locally on IIS.

I tried to use Articulate 5.0.1, but I got the 404 errors on my website nodes like mentioned in issue #432. So I reverted to 5.0.0 which works without these 404 errors.

It is an upgrade from Umbraco 8.18 and Articulate 4.3.2.

@khraibani
Copy link
Author

@Shazwazza Umbraco version 12.3.6 Articulate 5.0, I have tried updating to 5.0.1 but I got the 404 errors mentioned in #432

@wvdweerd
Copy link
Contributor

@Shazwazza @gavinfaux
Aha...
When debugging with the current "develop" branch version of Articulate, I saw that Umbraco returned a 404 when trying to navigate to the /tags or /categories url.
Because I had a Custom404 configured in the appsettings, I now was redirected to my custom 404 page.

Because Umbraco is throwing and handling a 404 before Articulate steps in, the condition on this line in the ArticulateRouteValueTransformer will always be true, and Articulate will not process the route (/tags or /categories) any further.

After I removed my Custom404 setting, the tag and categorie pages were displayed normally again.

As my knowledge of the code of Articulate is -still- very minimal, I don't think I will be able to solve this (soon).
So hopefully someone else can take a look at this?!

@gavinfaux
Copy link
Contributor

@wvdweerd @Shazwazza I'm seeing similar locally, due to the previous PR #436 which changed route priority, it's now too high and being skipped allowing not found handlers to kick in.

Will try and figure out current route priorities and ensure correct order...

@nvisage-gf
Copy link
Contributor

I guess some of these issues depend on how site is setup, in our case we've replaced the default controller on startup and use route hijacking, likely we have overlooked something with our implementation.

  • The route order for Articulate needs to be after Umbraco else other content URL's can 404
  • The order parameter could be > 100 depending on site complexity (how many extensions that use routes installed, Umbraco Cloud, uSync, Forms, Diplo etc) .

image

  • If you have a custom 404 / last chance content finder, that will kick in before the articulate routes, for example news/tags not found (may need to handle these edge cases via the last chance handler if you need it?).

I don't think #436 breaks anything and will continue to research options/workarounds.

@nvisage-gf
Copy link
Contributor

NullReference exception resolved by #440

404 is still issue if you have you have custom content finders, e,g. Custom404 or SetContentLastChanceFinder, still looking into solutions

@nvisage-gf
Copy link
Contributor

nvisage-gf commented Jan 20, 2024

@Shazwazza @gavinfaux Aha... When debugging with the current "develop" branch version of Articulate, I saw that Umbraco returned a 404 when trying to navigate to the /tags or /categories url. Because I had a Custom404 configured in the appsettings, I now was redirected to my custom 404 page.

Because Umbraco is throwing and handling a 404 before Articulate steps in, the condition on this line in the ArticulateRouteValueTransformer will always be true, and Articulate will not process the route (/tags or /categories) any further.

After I removed my Custom404 setting, the tag and categorie pages were displayed normally again.

As my knowledge of the code of Articulate is -still- very minimal, I don't think I will be able to solve this (soon). So hopefully someone else can take a look at this?!

Even if that line was removed, if you've set a custom 404 the ShouldCheck method causes the umbracoRouteValues to be resolved by Umbraco content finders to your 404 content rather than articulate resolving, I think the Umbraco pipeline resolves it before that even.

None of the routes defined in ArticulateRouter.MapRoutes will fire if content request already resolved by Umbraco pipeline which happens when a Custom404 or last chance content finder in use currently 😞

@nvisage-gf
Copy link
Contributor

nvisage-gf commented Jan 23, 2024

As a workaround a last chance content finder can be coded to exclude the articulate routes, which will then fall through to the articulate router to handle (pseudo code):

public Task<bool> TryFindContent(IPublishedRequestBuilder contentRequest)
           //** ......trimmed........**
            var articulatePaths = new List<string>
            {
                articulatePath + "opensearch",
                articulatePath + "rsd",
                articulatePath + "metaweblog",
                articulatePath + "wlwmanifest",
                articulatePath + "rss",
                articulatePath + "author",
                articulatePath + "a-new",
                articulatePath + searchUrlName,
                articulatePath + categoriesUrlName,
                articulatePath + tagsUrlName
            };
            if (articulatePaths.Contains(path)) return Task.FromResult(false);
           //** otherwise set your 404 / not found content**
        }

@wvdweerd
Copy link
Contributor

@nvisage-gf Looks like a viable workaround, I will try this tomorrow or this weekend in my website.

And a curious noob question:
Is this something that could be used in Articulate also?
Add a Articulate contentfinder at the start of the contentfinder collection, and let it handle Articulate paths, or let the next contentfinder handle the request?!

@nvisage-gf
Copy link
Contributor

nvisage-gf commented Jan 25, 2024

Posting example IContentLastChanceFinder implementation that we are using, obviously do your own testing and modify as needed (e.g. NotFound is the name of our own 404 document type).

I don't believe this can be achieved with just an IContentFinder since we need to ignore these custom routes, not match them, so they get resolved by the Articulate router which sets the appropriate content.

using Umbraco.Cms.Core.Models.PublishedContent;
using Umbraco.Cms.Core.Routing;
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Web.Common.PublishedModels.Content;

namespace ApplicationWeb.Core.Routing;

public class DefaultLastChanceFinder : IContentLastChanceFinder
{
    private readonly IContentTypeService _contentTypeService;
    private readonly IDomainService _domainService;
    private readonly IHttpContextAccessor _httpContextAccessor;
    private readonly IUmbracoContextAccessor _umbracoContextAccessor;

    // can only have one last chance finder
    // add to startup.cs e.g.
    //    services.AddUmbraco(_env, _config)
    //   .AddBackOffice()
    //   .AddWebsite()
    //   .AddComposers()
    //   .SetContentLastChanceFinder<DefaultLastChanceFinder>()
    //   .Build();

    public DefaultLastChanceFinder(IContentTypeService contentTypeService, IDomainService domainService,
        IUmbracoContextAccessor umbracoContextAccessor, IHttpContextAccessor httpContextAccessor)
    {
        _contentTypeService = contentTypeService;
        _umbracoContextAccessor = umbracoContextAccessor;
        _domainService = domainService;
        _httpContextAccessor = httpContextAccessor;
    }

    public Task<bool> TryFindContent(IPublishedRequestBuilder contentRequest)
    {
        if (!_umbracoContextAccessor.TryGetUmbracoContext(out var umbracoContext)) return Task.FromResult(false);

        var siteId = -1;
        IPublishedContent content = null;

        if (contentRequest.HasDomain())
            if (contentRequest.Domain != null)
                siteId = contentRequest.Domain.ContentId;

        if (siteId == -1)
        {
            var allDomains = _domainService.GetAll(true).ToList();
            var domain = allDomains.FirstOrDefault(f => f.DomainName == contentRequest.Uri.Authority || f.DomainName == $"https://{contentRequest.Uri.Authority}" || f.DomainName == $"http://{contentRequest.Uri.Authority}");
            siteId = (int)(domain != null ? domain.RootContentId : allDomains.Any() ? allDomains.FirstOrDefault()?.RootContentId : -1);
        }

        var path = contentRequest.Uri.GetAbsolutePathDecoded().Trim('/');
        var site = umbracoContext.Content?.GetById(siteId);

        var articulateRootNode = site?.DescendantOrSelf<Umbraco.Cms.Web.Common.PublishedModels.Content.Articulate>();
        if (articulateRootNode != null)
        {
            var articulatePath = RoutePathFromNodeUrl(_httpContextAccessor.HttpContext, articulateRootNode.Url());
            var searchUrlName = articulateRootNode.Value<string>("searchUrlName");
            var categoriesUrlName = articulateRootNode.Value<string>("categoriesUrlName");
            var tagsUrlName = articulateRootNode.Value<string>("tagsUrlName");
            var articulatePaths = new List<string>
            {
                articulatePath + "opensearch",
                articulatePath + "rsd",
                articulatePath + "metaweblog",
                articulatePath + "wlwmanifest",
                articulatePath + "rss",
                articulatePath + "author",
                articulatePath + "a-new",
                articulatePath + searchUrlName,
                articulatePath + categoriesUrlName,
                articulatePath + tagsUrlName
            };
            if (articulatePaths.Any(element => path.StartsWith(element.EnsureEndsWith("/")) || path.Equals(element))) return Task.FromResult(false);
        }

       // process other custom code or set your 404 content

        if (content == null && site != null) content = site.Descendant<NotFound>();
        content ??= umbracoContext.Content?.GetSingleByXPath("//notFound");

        contentRequest.SetPublishedContent(content);
        
        if (content != null && (contentRequest.Template == null || !content.TemplateId.HasValue))
        {
            var type = _contentTypeService.Get(content.ContentType.Key);
            
            if (type?.DefaultTemplate != null) contentRequest.SetTemplate(type.DefaultTemplate);

            else if (type?.AllowedTemplates != null && type.AllowedTemplates.Any()) contentRequest.SetTemplate(type.AllowedTemplates.First());
        }

        contentRequest.SetResponseStatus(content is { ContentType.Alias: "NotFound" } ? 404 : 200);
        return Task.FromResult(true);
    }

    /// <summary>
    ///     Returns a route path from a given node's URL since a node's Url might contain a domain which we can't use in our
    ///     routing.
    /// </summary>
    /// <param name="httpContext"></param>
    /// <param name="routePath"></param>
    /// <returns></returns>
    public static string RoutePathFromNodeUrl(HttpContext httpContext, string routePath)
    {
        //taken from https://github.com/Shazwazza/Articulate/blob/develop/src/Articulate/Routing/RouteCollectionExtensions.cs (marked as internal)
        var virtualPath = $"{httpContext.Request.Scheme}://{httpContext.Request.Host}{httpContext.Request.PathBase}";

        var rootRoutePath = (Uri.TryCreate(routePath, UriKind.Absolute, out var result)
            ? result.PathAndQuery
            : routePath).EnsureEndsWith('/');

        if (rootRoutePath == virtualPath)
            return string.Empty;

        return rootRoutePath.StartsWith(virtualPath)
            ? rootRoutePath.Substring(virtualPath.Length)
            : rootRoutePath.TrimStart('/');
    }
}

@wvdweerd
Copy link
Contributor

Many thanks @nvisage-gf for this elaborate code snippet.
With just a few tweaks this works like a charm for my website :-)

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

No branches or pull requests

5 participants