Umbraco-CMS icon indicating copy to clipboard operation
Umbraco-CMS copied to clipboard

BeginUmbracoForm doesn't work with custom umbraco routes

Open ReallyHeadlessRick opened this issue 2 years ago • 2 comments

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

10.2.0

Bug summary

Surface controller actions are not hit when rendering an Umbraco form from an Umbraco custom routes.

This used to work with the v8 equivalent setup (UmbracoVirtualNodeRouteHandler).

In v10 (and presumably v9 also) the alternate routing that takes place when a ufprt is present in the request only takes place for the dynamic controller route and not custom Umbraco routes.

Specifics

N/A

Steps to reproduce

  1. Create a new Umbraco 10 site & initialize.
  2. Create a document type (with template) named Home (no properties required).
  3. Create an instance of Home at root.
  4. Copy code samples below into project
  5. Form is handled appropriately from / but not from /demo/

Home.cshtml

@using Umbraco.Cms.Web.Common.PublishedModels;
@inherits Umbraco.Cms.Web.Common.Views.UmbracoViewPage<ContentModels.Home>
@using ContentModels = Umbraco.Cms.Web.Common.PublishedModels;
@{
    Layout = null;
}

<h1>@Model.Name</h1>

@using (Html.BeginUmbracoForm<DemoSurfaceController>("Handle"))
{
    @Html.TextBox("name")
    <input type="submit"/>
}

Demo.cs

public class DemoComposer : IComposer
{
    public void Compose(IUmbracoBuilder builder)
    {
        builder.Services.Configure<UmbracoPipelineOptions>(options =>
        {
            options.AddFilter(new UmbracoPipelineFilter(nameof(DemoPageController))
            {
                Endpoints = app => app.UseEndpoints(endpoints =>
                {
                    endpoints.MapControllerRoute(
                        "DemoPageController",
                        "demo",
                        new {Controller = "DemoPage", Action = "Index"});
                })
            });
        });
    }
}

public class DemoPageController : UmbracoPageController, IVirtualPageController
{
    private readonly UmbracoHelper _umbracoHelper;

    public DemoPageController(
        ILogger<UmbracoPageController> logger,
        ICompositeViewEngine compositeViewEngine,
        UmbracoHelper umbracoHelper) : base(logger, compositeViewEngine) =>
        _umbracoHelper = umbracoHelper;

    public IPublishedContent? FindContent(ActionExecutingContext actionExecutingContext) =>
        _umbracoHelper.ContentAtRoot().First();

    public IActionResult Index() =>
        View("~/Views/Home.cshtml", CurrentPage);
}

public class DemoSurfaceController : SurfaceController
{
    public DemoSurfaceController(
        IUmbracoContextAccessor umbracoContextAccessor,
        IUmbracoDatabaseFactory databaseFactory,
        ServiceContext services,
        AppCaches appCaches,
        IProfilingLogger profilingLogger,
        IPublishedUrlProvider publishedUrlProvider)
        : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider)
    { }

    public IActionResult Handle(string name) =>
        Ok(new {name});
}

Expected result / actual result

Should see output from DemoSurfaceController Handle() on submission of from from /demo/

ReallyHeadlessRick avatar Sep 16 '22 15:09 ReallyHeadlessRick

Hi there @ReallyHeadlessRick!

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

github-actions[bot] avatar Sep 16 '22 15:09 github-actions[bot]

Hi,

I've just hit this exact same issue. Has anybody else experienced it?

Thanks Andy

Hi @ReallyHeadlessRick

I've made a PR for this as it is due to the order the custom route endpoints are added (see https://github.com/umbraco/Umbraco-CMS/pull/13058).

There is a workaround if you need it which is to add your custom routes in Startup.cs after UseWebsiteEndpoints() is called:

            app.UseUmbraco()
                .WithMiddleware(u =>
                {
                    u.UseBackOffice();
                    u.UseWebsite();
                })
                .WithEndpoints(u =>
                {
                    u.UseInstallerEndpoints();
                    u.UseBackOfficeEndpoints();
                    u.UseWebsiteEndpoints();

                    u.EndpointRouteBuilder.MapControllerRoute(
                        "DemoPageController",
                        "demo",
                        new { Controller = "DemoPage", Action = "Index" });
                });

justin-nevitech avatar Sep 24 '22 13:09 justin-nevitech

Hey @justin-nevitech - Cheers but there are still a few more issues to resolve.

If the surface controller is updated to the following, the response to the post on the /demo route is a 404 but works as expected for /.

// Demo.cs
public class DemoSurfaceController : SurfaceController
{
    public IActionResult Handle(string name)
    {
        ViewData["name"] = name;
        return CurrentUmbracoPage();
    }
}
// Home.cshtml
@using (Html.BeginUmbracoForm<DemoSurfaceController>("Handle"))
{
    @Html.TextBox("name")
    <input type="submit"/>
}

@ViewData["name"]

ReallyHeadlessRick avatar Sep 26 '22 08:09 ReallyHeadlessRick

Hi @ReallyHeadlessRick

Ok, I will take another look.

justin-nevitech avatar Sep 26 '22 08:09 justin-nevitech

Hi @ReallyHeadlessRick

I'm not getting far on this, the problem is that the surface controller doesn't know about the virtual page controller due to the order the controllers are executed which is dictated by the routing.

I will have to leave this for someone else to look into (possibly HQ) as changing the routing could have lots of unintended consequences!

I can get something working by adding HttpGet/HttpPost to the actions so the surface controller handles the POST but then it still has an issue displaying the results using CurrentUmbracoPage() as it thinks the current page is from a normal render controller not a virtual page controller. A redirect to the current URL works but then you loose temp data and model, etc.

Not sure if any of this helps of not as a workaround?:

    public class DemoComposer : IComposer
    {
        public void Compose(IUmbracoBuilder builder)
        {
            builder.Services.Configure<UmbracoPipelineOptions>(options =>
            {
                options.AddFilter(new UmbracoPipelineFilter(nameof(DemoPageController))
                {
                    Endpoints = app => app.UseEndpoints(endpoints =>
                    {
                        endpoints.MapControllerRoute(
                            "DemoPageController",
                            "demo",
                            new { Controller = "DemoPage", Action = "Index" });
                    })
                });
            });
        }
    }

    public class DemoPageController : UmbracoPageController, IVirtualPageController
    {
        private readonly UmbracoHelper _umbracoHelper;

        public DemoPageController(
            ILogger<UmbracoPageController> logger,
            ICompositeViewEngine compositeViewEngine,
            UmbracoHelper umbracoHelper) : base(logger, compositeViewEngine) =>
            _umbracoHelper = umbracoHelper;

        public IPublishedContent? FindContent(ActionExecutingContext actionExecutingContext) =>
            _umbracoHelper.ContentAtRoot().First();

        [HttpGet]
        public IActionResult Index() =>
            View("~/Views/HomePage.cshtml", CurrentPage);
    }

    public class DemoSurfaceController : SurfaceController
    {
        public DemoSurfaceController(
            IUmbracoContextAccessor umbracoContextAccessor,
            IUmbracoDatabaseFactory databaseFactory,
            ServiceContext services,
            AppCaches appCaches,
            IProfilingLogger profilingLogger,
            IPublishedUrlProvider publishedUrlProvider)
            : base(umbracoContextAccessor, databaseFactory, services, appCaches, profilingLogger, publishedUrlProvider)
        { }

        //public IActionResult Handle(string name) =>
        //    Ok(new { name });

        [HttpPost]
        public IActionResult Handle(string name)
        {
            var currentPage = CurrentPage;
            ViewData["name"] = name;
            TempData["name"] = name;
            return RedirectToCurrentUmbracoUrl();
        }

    }

justin-nevitech avatar Sep 27 '22 15:09 justin-nevitech

@justin-nevitech thanks for your efforts - I have a workaround that works fine for now (middleware that switches endpoint when ufprt is present and current endpoint matches some criteria).

ReallyHeadlessRick avatar Sep 28 '22 21:09 ReallyHeadlessRick

Hi @ReallyHeadlessRick

Is it worth sharing your solution on here for others that may have the same issue?

justin-nevitech avatar Sep 29 '22 07:09 justin-nevitech

Hi @ReallyHeadlessRick

I've had a another look at this issue (as it was bugging me!) and I think I've got a fully working solutions tested across normal Umbraco routes, hijacked routes and virtual pages. Fingers crossed!

justin-nevitech avatar Oct 03 '22 20:10 justin-nevitech