From 9dadecdcc14bf7cf9b9ed87b2101e8ce1f797db4 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 23 Oct 2018 18:28:55 +1100 Subject: [PATCH] Fixes boolean conversion logic in ColorPickerConfigurationEditor, Fixes tree grouping logic and moves groups cache to the ApplicationTreeService, fix other merge issues --- .../Implement/DocumentRepository.cs | 2 +- .../Services/IApplicationTreeService.cs | 16 ++-- .../imaging/umbimagegravity.directive.js | 2 +- .../imagecropper/imagecropper.controller.js | 3 +- .../ColorPickerConfigurationEditor.cs | 8 +- .../Services/ApplicationTreeService.cs | 48 ++++++++++-- .../Trees/ApplicationTreeController.cs | 75 +++++++------------ 7 files changed, 90 insertions(+), 64 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index de7d0927de..1ffc03c11f 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -985,7 +985,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (properties.ContainsKey(temp.VersionId)) temp.Content.Properties = properties[temp.VersionId]; else - throw new InvalidOperationException($"No property data found for version: '{cc.Version}'."); + throw new InvalidOperationException($"No property data found for version: '{temp.VersionId}'."); // reset dirty initial properties (U4-1946) temp.Content.ResetDirtyProperties(false); diff --git a/src/Umbraco.Core/Services/IApplicationTreeService.cs b/src/Umbraco.Core/Services/IApplicationTreeService.cs index 98c3047fea..691a3a0b63 100644 --- a/src/Umbraco.Core/Services/IApplicationTreeService.cs +++ b/src/Umbraco.Core/Services/IApplicationTreeService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using Umbraco.Core.Models; namespace Umbraco.Core.Services @@ -41,10 +42,7 @@ namespace Umbraco.Core.Services /// /// Returns a ApplicationTree Array IEnumerable GetAll(); - - - IEnumerable GetAllTypes(); - + /// /// Gets the application tree for the applcation with the specified alias /// @@ -59,6 +57,14 @@ namespace Umbraco.Core.Services /// /// Returns a ApplicationTree Array IEnumerable GetApplicationTrees(string applicationAlias, bool onlyInitialized); + + /// + /// Gets the grouped application trees for the application with the specified alias + /// + /// + /// + /// + IDictionary> GetGroupedApplicationTrees(string applicationAlias, bool onlyInitialized); } /// @@ -117,7 +123,7 @@ namespace Umbraco.Core.Services throw new System.NotImplementedException(); } - public IEnumerable GetAllTypes() + public IDictionary> GetGroupedApplicationTrees(string applicationAlias, bool onlyInitialized) { throw new System.NotImplementedException(); } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js index e7d2540fe6..a70b8ca33e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/imaging/umbimagegravity.directive.js @@ -108,7 +108,7 @@ } else { // From: https://stackoverflow.com/a/51789597/5018 - var type = vm.src.substring(vm.src.indexOf("/") + 1, scope.src.indexOf(";base64")); + var type = vm.src.substring(vm.src.indexOf("/") + 1, vm.src.indexOf(";base64")); if (type.startsWith("svg")) { vm.isCroppable = false; vm.hasDimensions = false; diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js index 31bd4fd563..e4a7bd5267 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js @@ -130,7 +130,8 @@ angular.module('umbraco') * @param {any} crop */ function crop(crop) { - $scope.currentCrop = crop; + // clone the crop so we can discard the changes + $scope.currentCrop = angular.copy(crop); $scope.currentPoint = null; //set form to dirty to track changes diff --git a/src/Umbraco.Web/PropertyEditors/ColorPickerConfigurationEditor.cs b/src/Umbraco.Web/PropertyEditors/ColorPickerConfigurationEditor.cs index 943180bb20..a4d894c551 100644 --- a/src/Umbraco.Web/PropertyEditors/ColorPickerConfigurationEditor.cs +++ b/src/Umbraco.Web/PropertyEditors/ColorPickerConfigurationEditor.cs @@ -105,7 +105,11 @@ namespace Umbraco.Web.PropertyEditors // handle useLabel if (editorValues.TryGetValue("useLabel", out var useLabelObj)) - output.UseLabel = useLabelObj.TryConvertTo(); + { + var convertBool = useLabelObj.TryConvertTo(); + if (convertBool.Success) + output.UseLabel = convertBool.Result; + } // auto-assigning our ids, get next id from existing values var nextId = 1; @@ -124,7 +128,7 @@ namespace Umbraco.Web.PropertyEditors var id = item.Property("id")?.Value?.Value() ?? 0; if (id >= nextId) nextId = id + 1; - + var label = item.Property("label")?.Value?.Value(); value = JsonConvert.SerializeObject(new { value, label }); diff --git a/src/Umbraco.Web/Services/ApplicationTreeService.cs b/src/Umbraco.Web/Services/ApplicationTreeService.cs index 08a17187e7..f13c0547b6 100644 --- a/src/Umbraco.Web/Services/ApplicationTreeService.cs +++ b/src/Umbraco.Web/Services/ApplicationTreeService.cs @@ -21,15 +21,16 @@ namespace Umbraco.Web.Services private readonly ILogger _logger; private readonly CacheHelper _cache; private Lazy> _allAvailableTrees; - private IEnumerable _treeTypes; internal const string TreeConfigFileName = "trees.config"; private static string _treeConfig; private static readonly object Locker = new object(); + private readonly Lazy>> _groupedTrees; public ApplicationTreeService(ILogger logger, CacheHelper cache) { _logger = logger; _cache = cache; + _groupedTrees = new Lazy>>(InitGroupedTrees); } /// @@ -252,11 +253,6 @@ namespace Umbraco.Web.Services return GetAppTrees().OrderBy(x => x.SortOrder); } - public IEnumerable GetAllTypes() - { - return _treeTypes ?? (_treeTypes = GetAll().Select(x => x.GetRuntimeType())); - } - /// /// Gets the application tree for the applcation with the specified alias /// @@ -287,6 +283,46 @@ namespace Umbraco.Web.Services return list.OrderBy(x => x.SortOrder).ToArray(); } + public IDictionary> GetGroupedApplicationTrees(string applicationAlias, bool onlyInitialized) + { + var result = new Dictionary>(); + var foundTrees = GetApplicationTrees(applicationAlias, onlyInitialized); + foreach(var treeGroup in _groupedTrees.Value) + { + List resultGroup = null; + foreach(var tree in foundTrees) + { + foreach(var treeAliasInGroup in treeGroup) + { + if (tree.Alias == treeAliasInGroup) + { + if (resultGroup == null) resultGroup = new List(); + resultGroup.Add(tree); + } + } + } + if (resultGroup != null) + result[treeGroup.Key ?? string.Empty] = resultGroup; //key cannot be null so make empty string + } + return result; + } + + /// + /// Creates a group of all tree groups and their tree aliases + /// + /// + /// + /// Used to initialize the field + /// + private IReadOnlyCollection> InitGroupedTrees() + { + var result = GetAll() + .Select(x => (treeAlias: x.Alias, treeGroup: x.GetRuntimeType().GetCustomAttribute(false)?.TreeGroup)) + .GroupBy(x => x.treeGroup, x => x.treeAlias) + .ToList(); + return result; + } + /// /// Loads in the xml structure from disk if one is found, otherwise loads in an empty xml structure, calls the /// callback with the xml document and saves the structure back to disk if saveAfterCallback is true. diff --git a/src/Umbraco.Web/Trees/ApplicationTreeController.cs b/src/Umbraco.Web/Trees/ApplicationTreeController.cs index fd669a1b07..273a7afb37 100644 --- a/src/Umbraco.Web/Trees/ApplicationTreeController.cs +++ b/src/Umbraco.Web/Trees/ApplicationTreeController.cs @@ -21,20 +21,7 @@ namespace Umbraco.Web.Trees [AngularJsonOnlyConfiguration] [PluginController("UmbracoTrees")] public class ApplicationTreeController : UmbracoAuthorizedApiController - { - /// - /// Fetches all registered trees and groups them together if they have a [CoreTree] - /// Attribute with a 'TreeGroup' property set - /// This allows the settings section trees to be grouped by Settings, Templating & Other - /// - private static readonly Lazy>> CoreTrees - = new Lazy>>(() => - Current.Services.ApplicationTreeService.GetAllTypes() - .Select(x => (TreeType: x, TreeGroup: x.GetCustomAttribute(false)?.TreeGroup)) - .GroupBy(x => x.TreeGroup) - .ToList()); - - + { /// /// Returns the tree nodes for an application /// @@ -51,13 +38,14 @@ namespace Umbraco.Web.Trees if (string.IsNullOrEmpty(application)) throw new HttpResponseException(HttpStatusCode.NotFound); //find all tree definitions that have the current application alias - var appTrees = Services.ApplicationTreeService.GetApplicationTrees(application, onlyInitialized).ToArray(); + var groupedTrees = Services.ApplicationTreeService.GetGroupedApplicationTrees(application, onlyInitialized); + var allTrees = groupedTrees.Values.SelectMany(x => x).ToList(); - if (string.IsNullOrEmpty(tree) == false || appTrees.Length <= 1) + if (string.IsNullOrEmpty(tree) == false || allTrees.Count <= 1) { - var apptree = string.IsNullOrEmpty(tree) == false - ? appTrees.SingleOrDefault(x => x.Alias == tree) - : appTrees.SingleOrDefault(); + var apptree = !tree.IsNullOrWhiteSpace() + ? allTrees.FirstOrDefault(x => x.Alias == tree) + : allTrees.FirstOrDefault(); if (apptree == null) throw new HttpResponseException(HttpStatusCode.NotFound); @@ -74,22 +62,22 @@ namespace Umbraco.Web.Trees } } - var collection = new TreeNodeCollection(); - foreach (var apptree in appTrees) - { - //return the root nodes for each tree in the app - var rootNode = await GetRootForMultipleAppTree(apptree, queryStrings); - //This could be null if the tree decides not to return it's root (i.e. the member type tree does this when not in umbraco membership mode) - if (rootNode != null) - { - collection.Add(rootNode); - } - } - //Don't apply fancy grouping logic futher down, if we only have one group of items - var hasGroups = CoreTrees.Value.Count > 0; + var hasGroups = groupedTrees.Count > 1; if (!hasGroups) { + var collection = new TreeNodeCollection(); + foreach (var apptree in allTrees) + { + //return the root nodes for each tree in the app + var rootNode = await GetRootForMultipleAppTree(apptree, queryStrings); + //This could be null if the tree decides not to return it's root (i.e. the member type tree does this when not in umbraco membership mode) + if (rootNode != null) + { + collection.Add(rootNode); + } + } + var multiTree = TreeRootNode.CreateMultiTreeRoot(collection); multiTree.Name = Services.TextService.Localize("sections/" + application); @@ -99,32 +87,23 @@ namespace Umbraco.Web.Trees var rootNodeGroups = new List(); //Group trees by [CoreTree] attribute with a TreeGroup property - foreach (var treeSectionGroup in CoreTrees.Value) + foreach (var treeSectionGroup in groupedTrees) { var treeGroupName = treeSectionGroup.Key; var groupNodeCollection = new TreeNodeCollection(); - foreach (var treeItem in treeSectionGroup) + foreach (var appTree in treeSectionGroup.Value) { - //Item1 tuple - is the type from all tree types - var treeItemType = treeItem.Item1; - - var findAppTree = appTrees.FirstOrDefault(x => x.GetRuntimeType() == treeItemType); - if (findAppTree != null) + var rootNode = await GetRootForMultipleAppTree(appTree, queryStrings); + if (rootNode != null) { - //Now we need to get the 'TreeNode' which is in 'collection' - var treeItemNode = collection.FirstOrDefault(x => x.AdditionalData["treeAlias"].ToString() == findAppTree.Alias); - - if (treeItemNode != null) - { - //Add to a new list/collection - groupNodeCollection.Add(treeItemNode); - } + //Add to a new list/collection + groupNodeCollection.Add(rootNode); } } //If treeGroupName == null then its third party - if (treeGroupName == null) + if (treeGroupName.IsNullOrWhiteSpace()) { //This is used for the localisation key //treeHeaders/thirdPartyGroup