From 5382820e584c79dbe2a5d0e4793ea14a744709e7 Mon Sep 17 00:00:00 2001 From: Nathan Woulfe Date: Mon, 3 Jul 2023 22:17:24 +1000 Subject: [PATCH] Ensure package view path is generated with consideration the current section (#14268) * check section when determining package template path * return sectionAlias as packageFolder and the route segment won't always match * fix test * adds PluginController attribute to StylesheetsTreeController fixes actionUrl generation to ensure valid URL find controller by fullname to avoid incorrect controller selection when multiple exist with same name * Fix frontend breaking changes * Adding missing PluginController attribute for ScriptsTreeController --------- Co-authored-by: Elitsa --- .../Controllers/BackOfficeServerVariables.cs | 5 ++- .../Trees/ApplicationTreeController.cs | 2 +- .../Trees/ScriptsTreeController.cs | 2 + .../Trees/StylesheetsTreeController.cs | 2 + .../Trees/UrlHelperExtensions.cs | 39 ++++++++++++++----- .../common/mocks/umbraco.servervariables.js | 2 +- .../src/common/services/navigation.service.js | 7 ++-- .../src/common/services/tree.service.js | 30 +++++++------- src/Umbraco.Web.UI.Client/src/routes.js | 18 +++++---- .../unit/common/services/tree-service.spec.js | 4 +- 10 files changed, 72 insertions(+), 39 deletions(-) diff --git a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs index 6dbd5f1e79..9325f9e8ae 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/BackOfficeServerVariables.cs @@ -704,6 +704,9 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers [DataMember(Name = "packageFolder")] public string? PackageFolder { get; set; } + + [DataMember(Name = "sectionAlias")] + public string? SectionAlias { get; set; } } private IEnumerable GetPluginTrees() @@ -735,7 +738,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers continue; } - yield return new PluginTree { Alias = tree.TreeAlias, PackageFolder = pluginController.AreaName }; + yield return new PluginTree { Alias = tree.TreeAlias, PackageFolder = pluginController.AreaName, SectionAlias = tree.SectionAlias }; } } diff --git a/src/Umbraco.Web.BackOffice/Trees/ApplicationTreeController.cs b/src/Umbraco.Web.BackOffice/Trees/ApplicationTreeController.cs index 461d1fc82f..28f685bf0d 100644 --- a/src/Umbraco.Web.BackOffice/Trees/ApplicationTreeController.cs +++ b/src/Umbraco.Web.BackOffice/Trees/ApplicationTreeController.cs @@ -360,7 +360,7 @@ public class ApplicationTreeController : UmbracoAuthorizedApiController ControllerActionDescriptor? actionDescriptor = _actionDescriptorCollectionProvider.ActionDescriptors.Items .Cast() .First(x => - x.ControllerName.Equals(controllerName) && + (x.ControllerTypeInfo.FullName ?? string.Empty).Equals(controllerType.FullName) && x.ActionName == action); var actionContext = new ActionContext(HttpContext, routeData, actionDescriptor); diff --git a/src/Umbraco.Web.BackOffice/Trees/ScriptsTreeController.cs b/src/Umbraco.Web.BackOffice/Trees/ScriptsTreeController.cs index 630584a839..0c46e809d2 100644 --- a/src/Umbraco.Web.BackOffice/Trees/ScriptsTreeController.cs +++ b/src/Umbraco.Web.BackOffice/Trees/ScriptsTreeController.cs @@ -3,10 +3,12 @@ using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Trees; +using Umbraco.Cms.Web.Common.Attributes; namespace Umbraco.Cms.Web.BackOffice.Trees; [CoreTree] +[PluginController(Constants.Web.Mvc.BackOfficeTreeArea)] [Tree(Constants.Applications.Settings, Constants.Trees.Scripts, TreeTitle = "Scripts", SortOrder = 10, TreeGroup = Constants.Trees.Groups.Templating)] public class ScriptsTreeController : FileSystemTreeController { diff --git a/src/Umbraco.Web.BackOffice/Trees/StylesheetsTreeController.cs b/src/Umbraco.Web.BackOffice/Trees/StylesheetsTreeController.cs index 3ff7a7ecfc..32f2a3e465 100644 --- a/src/Umbraco.Web.BackOffice/Trees/StylesheetsTreeController.cs +++ b/src/Umbraco.Web.BackOffice/Trees/StylesheetsTreeController.cs @@ -3,10 +3,12 @@ using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.IO; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Core.Trees; +using Umbraco.Cms.Web.Common.Attributes; namespace Umbraco.Cms.Web.BackOffice.Trees; [CoreTree] +[PluginController(Constants.Web.Mvc.BackOfficeTreeArea)] [Tree(Constants.Applications.Settings, Constants.Trees.Stylesheets, TreeTitle = "Stylesheets", SortOrder = 9, TreeGroup = Constants.Trees.Groups.Templating)] public class StylesheetsTreeController : FileSystemTreeController { diff --git a/src/Umbraco.Web.BackOffice/Trees/UrlHelperExtensions.cs b/src/Umbraco.Web.BackOffice/Trees/UrlHelperExtensions.cs index 1688a99ec2..d8506f5692 100644 --- a/src/Umbraco.Web.BackOffice/Trees/UrlHelperExtensions.cs +++ b/src/Umbraco.Web.BackOffice/Trees/UrlHelperExtensions.cs @@ -47,14 +47,15 @@ public static class UrlHelperExtensions string nodeId, FormCollection? queryStrings) { - var actionUrl = urlHelper.GetUmbracoApiService(umbracoApiControllerTypeCollection, "GetNodes", treeType)? - .EnsureEndsWith('?'); + var actionName = "GetNodes"; + var actionUrl = urlHelper.GetUmbracoApiService(umbracoApiControllerTypeCollection, actionName, treeType); + actionUrl = StartOrContinueQueryString(actionUrl, actionName); + + // Now we need to append the query strings + // Always ignore the custom start node id when generating URLs for tree nodes since this is a custom once-only parameter + // that should only ever be used when requesting a tree to render (root), not a tree node + actionUrl += "id=" + nodeId.EnsureEndsWith('&') + queryStrings?.ToQueryString("id", TreeQueryStringParameters.StartNodeId); - //now we need to append the query strings - actionUrl += "id=" + nodeId.EnsureEndsWith('&') + queryStrings?.ToQueryString("id", - //Always ignore the custom start node id when generating URLs for tree nodes since this is a custom once-only parameter - // that should only ever be used when requesting a tree to render (root), not a tree node - TreeQueryStringParameters.StartNodeId); return actionUrl; } @@ -65,11 +66,29 @@ public static class UrlHelperExtensions string nodeId, FormCollection? queryStrings) { - var actionUrl = urlHelper.GetUmbracoApiService(umbracoApiControllerTypeCollection, "GetMenu", treeType)? - .EnsureEndsWith('?'); + var actionName = "GetMenu"; + var actionUrl = urlHelper.GetUmbracoApiService(umbracoApiControllerTypeCollection, actionName, treeType); + actionUrl = StartOrContinueQueryString(actionUrl, actionName); - //now we need to append the query strings + // now we need to append the query strings actionUrl += "id=" + nodeId.EnsureEndsWith('&') + queryStrings?.ToQueryString("id"); return actionUrl; } + + /// + /// Check the provided string already includes a querystring fragment + /// If so, result has "&" appended, else has "?" appended. + /// + /// + /// + /// + private static string? StartOrContinueQueryString(string? actionUrl, string? delimiter) + { + if (actionUrl is null) + { + return actionUrl; + } + + return actionUrl.EnsureEndsWith(actionUrl.Contains($"{delimiter}?") ? "&" : "?"); + } } diff --git a/src/Umbraco.Web.UI.Client/src/common/mocks/umbraco.servervariables.js b/src/Umbraco.Web.UI.Client/src/common/mocks/umbraco.servervariables.js index 51029234f5..4b8e8fa146 100644 --- a/src/Umbraco.Web.UI.Client/src/common/mocks/umbraco.servervariables.js +++ b/src/Umbraco.Web.UI.Client/src/common/mocks/umbraco.servervariables.js @@ -32,7 +32,7 @@ Umbraco.Sys.ServerVariables = { }, umbracoPlugins: { trees: [ - { alias: "myTree", packageFolder: "MyPackage" } + { alias: "myTree", packageFolder: "MyPackage", sectionAlias: "myPackageSectionAlias" } ] }, isDebuggingEnabled: true, diff --git a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js index 77b97545b6..248e78880a 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js @@ -616,7 +616,7 @@ function navigationService($routeParams, $location, $q, $injector, eventsService if (!treeAlias) { throw "Could not get tree alias for node " + args.node.id; } - templateUrl = this.getTreeTemplateUrl(treeAlias, args.action.alias); + templateUrl = this.getTreeTemplateUrl(treeAlias, args.action.alias, args.node.section); } setMode("dialog"); @@ -633,6 +633,7 @@ function navigationService($routeParams, $location, $q, $injector, eventsService * * @param {string} treeAlias the alias of the tree to look up * @param {string} action the view file name + * @param {string} sectionAlias the alias of the current section * @description * creates the templateUrl based on treeAlias and action * by convention we will look into the /views/{treetype}/{action}.html @@ -640,8 +641,8 @@ function navigationService($routeParams, $location, $q, $injector, eventsService * we will also check for a 'packageName' for the current tree, if it exists then the convention will be: * for example: /App_Plugins/{mypackage}/backoffice/{treetype}/create.html */ - getTreeTemplateUrl: function (treeAlias, action) { - var packageTreeFolder = treeService.getTreePackageFolder(treeAlias); + getTreeTemplateUrl: function (treeAlias, action, sectionAlias) { + var packageTreeFolder = treeService.getTreePackageFolder(treeAlias, sectionAlias); if (packageTreeFolder) { return (Umbraco.Sys.ServerVariables.umbracoSettings.appPluginsPath + "/" + packageTreeFolder + diff --git a/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js b/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js index ba9ebc1b00..fa5d297a88 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/tree.service.js @@ -166,23 +166,27 @@ function treeService($q, treeResource, iconHelper, notificationsService, eventsS * * @description * Determines if the current tree is a plugin tree and if so returns the package folder it has declared - * so we know where to find it's views, otherwise it will just return undefined. + * so we know where to find its views, otherwise it will just return undefined. * * @param {String} treeAlias The tree alias to check + * @param {String} sectionAlias The current section */ - getTreePackageFolder: function (treeAlias) { + getTreePackageFolder: function (treeAlias, sectionAlias) { //we determine this based on the server variables - if (Umbraco.Sys.ServerVariables.umbracoPlugins && - Umbraco.Sys.ServerVariables.umbracoPlugins.trees && - Utilities.isArray(Umbraco.Sys.ServerVariables.umbracoPlugins.trees)) { - - var found = _.find(Umbraco.Sys.ServerVariables.umbracoPlugins.trees, function (item) { - return invariantEquals(item.alias, treeAlias); - }); - - return found ? found.packageFolder : undefined; + if (!Umbraco.Sys.ServerVariables.umbracoPlugins || !Utilities.isArray(Umbraco.Sys.ServerVariables.umbracoPlugins.trees)) { + return undefined; } - return undefined; + + let found; + if (sectionAlias !== undefined) { + found = Umbraco.Sys.ServerVariables.umbracoPlugins.trees.find(item => + invariantEquals(item.alias, treeAlias) && invariantEquals(item.sectionAlias, sectionAlias)); + } else { + found = Umbraco.Sys.ServerVariables.umbracoPlugins.trees.find(item => + invariantEquals(item.alias, treeAlias)); + } + + return found ? found.packageFolder : undefined; }, /** @@ -868,7 +872,7 @@ function treeService($q, treeResource, iconHelper, notificationsService, eventsS //start var wrappedPromise = doSync(); - //then wrap it + //then wrap it wrappedPromise.then(function (args) { deferred.resolve(args); }, function (args) { diff --git a/src/Umbraco.Web.UI.Client/src/routes.js b/src/Umbraco.Web.UI.Client/src/routes.js index 7e65346d1b..e6d97ea18f 100644 --- a/src/Umbraco.Web.UI.Client/src/routes.js +++ b/src/Umbraco.Web.UI.Client/src/routes.js @@ -1,5 +1,5 @@ window.app.config(function ($routeProvider) { - + /** * This determines if the route can continue depending on authentication and initialization requirements * @param {boolean} authRequired If true, it checks if the user is authenticated and will resolve successfully @@ -117,9 +117,9 @@ window.app.config(function ($routeProvider) { template: "
", //This controller will execute for this route, then we can execute some code in order to set the template Url controller: function ($scope, $route, $routeParams, $location, sectionService) { - + //We are going to check the currently loaded sections for the user and if the section we are navigating - //to has a custom route path we'll use that + //to has a custom route path we'll use that sectionService.getSectionsForUser().then(function(sections) { //find the one we're requesting var found = _.find(sections, function(s) { @@ -175,8 +175,9 @@ window.app.config(function ($routeProvider) { if ($routeParams.section.toLowerCase() === "users" && $routeParams.tree.toLowerCase() === "users" && usersPages.indexOf($routeParams.method.toLowerCase()) === -1) { $scope.templateUrl = "views/users/overview.html"; return; - } - $scope.templateUrl = navigationService.getTreeTemplateUrl($routeParams.tree, $routeParams.method); + } + + $scope.templateUrl = navigationService.getTreeTemplateUrl($routeParams.tree, $routeParams.method, $routeParams.section); }, reloadOnSearch: false, resolve: canRoute(true) @@ -190,8 +191,9 @@ window.app.config(function ($routeProvider) { if (!$routeParams.tree || !$routeParams.method) { $scope.templateUrl = "views/common/dashboard.html"; return; - } - $scope.templateUrl = navigationService.getTreeTemplateUrl($routeParams.tree, $routeParams.method); + } + + $scope.templateUrl = navigationService.getTreeTemplateUrl($routeParams.tree, $routeParams.method, $routeParams.section); }, reloadOnSearch: false, reloadOnUrl: false, @@ -199,7 +201,7 @@ window.app.config(function ($routeProvider) { }) .otherwise({ redirectTo: '/login' }); }).config(function ($locationProvider) { - + $locationProvider.html5Mode(false); //turn html5 mode off $locationProvider.hashPrefix(''); }); diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js index 4d19cf557a..63800a9e12 100644 --- a/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/tree-service.spec.js @@ -298,13 +298,13 @@ describe('tree service tests', function () { it('can find a plugin based tree', function () { //we know this exists in the mock umbraco server vars - var found = treeService.getTreePackageFolder("myTree"); + var found = treeService.getTreePackageFolder("myTree", "MyPackageSectionAlias"); expect(found).toBe("MyPackage"); }); it('returns undefined for a not found tree', function () { //we know this does not exist in the mock umbraco server vars - var found = treeService.getTreePackageFolder("asdfasdf"); + var found = treeService.getTreePackageFolder("asdfasdf", "fdsafdsa"); expect(found).not.toBeDefined(); });