Update valid reasons to not have a template on a content node to include having umbracoRedirect

[CodeBunTes]

Prerequisites

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

If there's an existing issue for this PR then this fixes #15724

Description

In Umbraco 8 it used to be possible to have a doctype with no template assigned and an 'umbracoRedirect' field present that when hit would redirect a user to a chosen page. In Umbraco 10 onwards, this functionality will only work if a template has also been assigned to the doctype. This PR restores the original behaviour.

This appears to happen because:-

• In Umbraco 8, as part of the 'GetHandlerForRoute()' method in 'Mvc\RenderRouteHandler.cs' a check is initially performed to see if the request has no template. If so it would begin to update it to a 404 via 'UpdateRequestToNotFound', before promptly exiting if it is detected it is a redirect and not following through any further on the 404 behaviour.
• In the newer Umbraco versions, the equivalent 'CheckNoTemplateAsync' method in Routing/UmbracoRouteValuesFactory.cs begins the same, calling an 'UpdateRequestAsync' method to begin updating this to a 404. However due to changes with a fresh copy of the request being created and null content entry now being passed in, the original 'IsRedirect' value is reset so it will no longer know it needs to do a redirect and continues through standard 404 behaviour.

To replicate the Umbraco 8 behaviour, I've moved an additional check for 'IsRedirect' to the outer point in UmbracoRouteValuesFactory. As having a redirect field should be a valid reason to not have a template in the first place, this avoids it unnecessarily performing any further checks for the presence of the template and it'll then follow through the redirect behaviour instead.

An alternative way of approaching this could have been to update the inner behaviour on PublishedRouter/UpdateRequestAsync() to copy back in the redirect status after it is cleared. However as this method is used in multiple places changing this could have more of a regression impact.

Testing

To test this:-

1. Create a new document type without a template
2. Give it a property named umbracoRedirect with a content picker editor.
3. Allow it wherever you need in the node tree.
4. Add a node of this type, choose a page to be redirected to in the field, and save & publish.
5. Visit the node address of the original node and you should now be redirected despite no template.

Some additional variations validated with this PR:-

• Content node with umbracoRedirect set and no template assigned. This would return a 404 'no template' error previously, but now will follow the redirect as per older Umbraco behaviour
• Content node with umbracoRedirect set and a template assigned. This would redirect fine previously, and still does so
• Content node with umbracoRedirect field present, but nothing picked and no template assigned. This would return the 404 'no template' error previously, and still does so as we are not able to redirect anywhere else
• Content node with umbracoRedirect field present, but nothing picked and a template assigned. This would return the content page itself and still does so.
• Ensured unit tests for UpdateRequestAsync still pass correctly

Thanks,
Terence

[github-actions]

Hi there @CodeBunTes, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[georgebid]

Hey, @CodeBunTes! Thanks a lot for your PR to fix #15724, someone on the team will review this soon 😄

[emmagarland]

Hi @CodeBunTes

Cheers for this PR to resolve #15724. This looks decent to me and I can replicate the behaviour (and the template one still works). There is an "increasing cyclomatic complexity warning" but I think to resolve it would be a larger refactoring piece and a separate issue, which I don't think is in the scope of this fix.

I've merged into contrib so it will be fixed in the latest v14 🙌

Best wishes

Emma