From 32377b29b8e6ffa11f13039f92abf164d685d375 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 11 Jan 2016 15:04:51 +0100 Subject: [PATCH 1/7] Have content type filtering working on the compositions list so a dev is not allowed to create an illegal composition - next we need to pass in the already existing property types to further ensure an illegal composition cannot be created. --- .../Services/ContentTypeServiceExtensions.cs | 69 ++++++++++++++++--- .../components/umbgroupsbuilder.directive.js | 35 +++++++++- .../common/resources/contenttype.resource.js | 17 ++++- .../common/resources/mediatype.resource.js | 17 ++++- .../common/resources/membertype.resource.js | 17 ++++- .../services/contenttypehelper.service.js | 32 ++++++++- .../compositions/compositions.html | 15 ++-- .../Editors/ContentTypeController.cs | 20 +++++- .../Editors/ContentTypeControllerBase.cs | 18 ++++- .../Editors/MediaTypeController.cs | 20 +++++- .../Editors/MemberTypeController.cs | 20 +++++- 11 files changed, 249 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index d16e7946b9..e43818ae14 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -10,11 +10,43 @@ namespace Umbraco.Core.Services /// /// Returns the available composite content types for a given content type /// + /// + /// + /// This is normally an empty list but if additional content type aliases are passed in, any content types containing those aliases will be filtered out + /// along with any content types that have matching property types that are included in the filtered content types + /// + /// + /// + /// + /// This is normally an empty list but if additional property type aliases are passed in, any content types that have these aliases will be filtered out. + /// This is required because in the case of creating/modifying a content type because new property types being added to it are not yet persisted so cannot + /// be looked up via the db, they need to be passed in. + /// /// public static IEnumerable GetAvailableCompositeContentTypes(this IContentTypeService ctService, IContentTypeComposition source, - IContentTypeComposition[] allContentTypes) + IContentTypeComposition[] allContentTypes, + string[] filterContentTypes = null, + string[] filterPropertyTypes = null) { + + if (filterContentTypes == null) filterContentTypes = new string[] { }; + if (filterPropertyTypes == null) filterContentTypes = new string[] { }; + + //ensure the source alias is added + var filterContentTypesList = filterContentTypes.Where(x => x.IsNullOrWhiteSpace() == false).ToList(); + if (source != null && filterContentTypesList.Contains(source.Alias) == false) + filterContentTypesList.Add(source.Alias); + + //create the full list of property types to use as the filter + var filteredPropertyTypes = allContentTypes + .Where(c => filterContentTypesList.Contains(c.Alias)) + .SelectMany(c => c.PropertyTypes) + .Select(c => c.Alias) + .Union(filterPropertyTypes); + + var sourceId = source != null ? source.Id : 0; + //below is all ported from the old doc type editor and comes with the same weaknesses /insanity / magic // note: there are many sanity checks missing here and there ;-(( @@ -22,15 +54,12 @@ namespace Umbraco.Core.Services //if (allContentTypes.Any(x => x.ParentId > 0 && x.ContentTypeComposition.Any(y => y.Id == x.ParentId) == false)) // throw new Exception("A parent does not belong to a composition."); - if (source != null) + // find out if any content type uses this content type + var isUsing = allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == sourceId)).ToArray(); + if (isUsing.Length > 0) { - // find out if any content type uses this content type - var isUsing = allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == source.Id)).ToArray(); - if (isUsing.Length > 0) - { - //if already in use a composition, do not allow any composited types - return new List(); - } + //if already in use a composition, do not allow any composited types + return new List(); } // if it is not used then composition is possible @@ -62,7 +91,25 @@ namespace Umbraco.Core.Services // .ToArray(); return list - .Where(x => x.Id != (source != null ? source.Id : 0)) + //not itself + .Where(x => x.Id != sourceId) + .Where(x => + { + //need to filter any content types that are included in this list + if (filterContentTypesList.Any() == false) return true; + + return filterContentTypesList.Any(c => c.InvariantEquals(x.Alias)) == false; + }) + .Where(x => + { + //need to filter any content types that have matching property aliases that are included in this list + if (filterContentTypesList.Any() == false) return true; + + //ensure that we don't return if there's any overlapping property aliases from the filtered ones specified + return filteredPropertyTypes.Intersect( + x.PropertyTypes.Select(p => p.Alias), + StringComparer.InvariantCultureIgnoreCase).Any() == false; + }) .OrderBy(x => x.Name) .ToList(); } @@ -74,6 +121,8 @@ namespace Umbraco.Core.Services /// private static IEnumerable GetDirectOrIndirect(IContentTypeComposition ctype) { + if (ctype == null) return Enumerable.Empty(); + // hashset guarantees unicity on Id var all = new HashSet(new DelegateEqualityComparer( (x, y) => x.Id == y.Id, diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js index c8ea32110e..2657424818 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js @@ -1,7 +1,7 @@ (function() { 'use strict'; - function GroupsBuilderDirective(contentTypeHelper, contentTypeResource, mediaTypeResource, dataTypeHelper, dataTypeResource, $filter, iconHelper, $q) { + function GroupsBuilderDirective(contentTypeHelper, contentTypeResource, mediaTypeResource, dataTypeHelper, dataTypeResource, $filter, iconHelper, $q, $timeout) { function link(scope, el, attr, ctrl) { @@ -116,6 +116,36 @@ } + function filterAvailableCompositions(selectedContentType, selecting) { + + //selecting = true if the user has check the item, false if the user has unchecked the item + + var selectedContentTypeAliases = selecting ? + //the user has selected the item so add to the current list + _.union(scope.compositionsDialogModel.compositeContentTypes, [selectedContentType.alias]) : + //the user has unselected the item so remove from the current list + _.reject(scope.compositionsDialogModel.compositeContentTypes, function(i) { + return i === selectedContentType.alias; + }); + + //use a different resource lookup depending on the content type type + var resourceLookup = scope.contentType === "documentType" ? contentTypeResource.getAvailableCompositeContentTypes : mediaTypeResource.getAvailableCompositeContentTypes; + + return resourceLookup(scope.model.id, selectedContentTypeAliases).then(function (filteredAvailableCompositeTypes) { + _.each(scope.compositionsDialogModel.availableCompositeContentTypes, function (current) { + //reset first + current.disallow = false; + //see if this list item is found in the response (allowed) list + var found = _.find(filteredAvailableCompositeTypes, function (f) { + return current.alias === f.alias; + }); + //disallow if the item was not found in the response (allowed) list - + // BUT do not set to dissallowed if it is currently checked + current.disallow = (selectedContentTypeAliases.indexOf(current.alias) === -1) && (found ? false : true); + }); + }); + } + function updatePropertiesSortOrder() { angular.forEach(scope.model.groups, function(group){ @@ -187,6 +217,8 @@ // or the action has been confirmed } else { + //TODO: RE-validate compositions + // make sure that all tabs has an init property if (scope.model.groups.length !== 0) { angular.forEach(scope.model.groups, function(group) { @@ -227,6 +259,7 @@ mediaTypeResource.getById(compositeContentType.id).then(function(composition){ contentTypeHelper.mergeCompositeContentType(scope.model, composition); + } }); } diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js index a59bc83ccb..10e969226d 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js @@ -16,13 +16,26 @@ function contentTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { 'Failed to retrieve count'); }, - getAvailableCompositeContentTypes: function (contentTypeId) { + getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes) { + if (!filterContentTypes) { + filterContentTypes = []; + } + var query = ""; + _.each(filterContentTypes, function (item) { + query += "filterContentTypes=" + item + "&"; + }); + // if filterContentTypes array is empty we need a empty variable in the querystring otherwise the service returns a error + if (filterContentTypes.length === 0) { + query += "filterContentTypes=&"; + } + query += "contentTypeId=" + contentTypeId; + return umbRequestHelper.resourcePromise( $http.get( umbRequestHelper.getApiUrl( "contentTypeApiBaseUrl", "GetAvailableCompositeContentTypes", - [{ contentTypeId: contentTypeId }])), + query)), 'Failed to retrieve data for content type id ' + contentTypeId); }, diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js index 39315c6074..dff3a99667 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js @@ -16,13 +16,26 @@ function mediaTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { 'Failed to retrieve count'); }, - getAvailableCompositeContentTypes: function (contentTypeId) { + getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes) { + if (!filterContentTypes) { + filterContentTypes = []; + } + var query = ""; + _.each(filterContentTypes, function (item) { + query += "filterContentTypes=" + item + "&"; + }); + // if filterContentTypes array is empty we need a empty variable in the querystring otherwise the service returns a error + if (filterContentTypes.length === 0) { + query += "filterContentTypes=&"; + } + query += "contentTypeId=" + contentTypeId; + return umbRequestHelper.resourcePromise( $http.get( umbRequestHelper.getApiUrl( "mediaTypeApiBaseUrl", "GetAvailableCompositeMediaTypes", - [{ contentTypeId: contentTypeId }])), + query)), 'Failed to retrieve data for content type id ' + contentTypeId); }, diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js index d949844a6c..e8253e463d 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js @@ -7,13 +7,26 @@ function memberTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { return { - getAvailableCompositeContentTypes: function (contentTypeId) { + getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes) { + if (!filterContentTypes) { + filterContentTypes = []; + } + var query = ""; + _.each(filterContentTypes, function (item) { + query += "filterContentTypes=" + item + "&"; + }); + // if filterContentTypes array is empty we need a empty variable in the querystring otherwise the service returns a error + if (filterContentTypes.length === 0) { + query += "filterContentTypes=&"; + } + query += "contentTypeId=" + contentTypeId; + return umbRequestHelper.resourcePromise( $http.get( umbRequestHelper.getApiUrl( "memberTypeApiBaseUrl", "GetAvailableCompositeMemberTypes", - [{ contentTypeId: contentTypeId }])), + query)), 'Failed to retrieve data for content type id ' + contentTypeId); }, diff --git a/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js index f9ffcd4a23..3230f83331 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js @@ -43,8 +43,38 @@ function contentTypeHelper(contentTypeResource, dataTypeResource, $filter) { return newArray; }, + validateAddingComposition: function(contentType, compositeContentType) { + + //Validate that by adding this group that we are not adding duplicate property type aliases + + var propertiesAdding = _.flatten(_.map(compositeContentType.groups, function(g) { + return _.map(g.properties, function(p) { + return p.alias; + }); + })); + var propAliasesExisting = _.flatten(_.map(contentType.groups, function(g) { + return _.map(g.properties, function(p) { + return p.alias; + }); + })); + var intersec = _.intersection(propertiesAdding, propAliasesExisting); + if (intersec.length > 0) { + //return the overlapping property aliases + return intersec; + } + + //no overlapping property aliases + return []; + }, + mergeCompositeContentType: function(contentType, compositeContentType) { + //Validate that there are no overlapping aliases + var overlappingAliases = this.validateAddingComposition(contentType, compositeContentType); + if (overlappingAliases.length > 0) { + throw new Error("Cannot add this composition, these properties already exist on the content type: " + overlappingAliases.join()); + } + angular.forEach(compositeContentType.groups, function(compositionGroup) { // order composition groups based on sort order @@ -134,7 +164,7 @@ function contentTypeHelper(contentTypeResource, dataTypeResource, $filter) { // push id to array of merged composite content types compositionGroup.parentTabContentTypes.push(compositeContentType.id); - + // push group before placeholder tab contentType.groups.unshift(compositionGroup); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html b/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html index 2cdcef5ee6..49dab6151a 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html @@ -26,13 +26,18 @@
    -
  • - -
    - + +
    + + ng-change="model.selectCompositeContentType(compositeContentType)" + ng-disabled="compositeContentType.disallow" />
    diff --git a/src/Umbraco.Web/Editors/ContentTypeController.cs b/src/Umbraco.Web/Editors/ContentTypeController.cs index b70683dd93..f91844fe07 100644 --- a/src/Umbraco.Web/Editors/ContentTypeController.cs +++ b/src/Umbraco.Web/Editors/ContentTypeController.cs @@ -95,9 +95,25 @@ namespace Umbraco.Web.Editors return ApplicationContext.Services.ContentTypeService.GetAllPropertyTypeAliases(); } - public IEnumerable GetAvailableCompositeContentTypes(int contentTypeId) + /// + /// Returns the avilable compositions for this content type + /// + /// + /// + /// This is normally an empty list but if additional content type aliases are passed in, any content types containing those aliases will be filtered out + /// along with any content types that have matching property types that are included in the filtered content types + /// + /// + /// This is normally an empty list but if additional property type aliases are passed in, any content types that have these aliases will be filtered out. + /// This is required because in the case of creating/modifying a content type because new property types being added to it are not yet persisted so cannot + /// be looked up via the db, they need to be passed in. + /// + /// + public IEnumerable GetAvailableCompositeContentTypes(int contentTypeId, + [FromUri]string[] filterContentTypes, + [FromUri]string[] filterPropertyTypes) { - return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.DocumentType); + return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.DocumentType, filterContentTypes, filterPropertyTypes); } [UmbracoTreeAuthorize( diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index 975154e6e8..2f98372e77 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -49,8 +49,22 @@ namespace Umbraco.Web.Editors /// /// Returns the available composite content types for a given content type /// + /// + /// + /// This is normally an empty list but if additional content type aliases are passed in, any content types containing those aliases will be filtered out + /// along with any content types that have matching property types that are included in the filtered content types + /// + /// + /// This is normally an empty list but if additional property type aliases are passed in, any content types that have these aliases will be filtered out. + /// This is required because in the case of creating/modifying a content type because new property types being added to it are not yet persisted so cannot + /// be looked up via the db, they need to be passed in. + /// + /// /// - protected IEnumerable PerformGetAvailableCompositeContentTypes(int contentTypeId, UmbracoObjectTypes type) + protected IEnumerable PerformGetAvailableCompositeContentTypes(int contentTypeId, + UmbracoObjectTypes type, + string[] filterContentTypes, + string[] filterPropertyTypes) { IContentTypeComposition source = null; @@ -90,7 +104,7 @@ namespace Umbraco.Web.Editors throw new ArgumentOutOfRangeException("The entity type was not a content type"); } - var filtered = Services.ContentTypeService.GetAvailableCompositeContentTypes(source, allContentTypes); + var filtered = Services.ContentTypeService.GetAvailableCompositeContentTypes(source, allContentTypes, filterContentTypes, filterPropertyTypes); return filtered .Select(Mapper.Map) diff --git a/src/Umbraco.Web/Editors/MediaTypeController.cs b/src/Umbraco.Web/Editors/MediaTypeController.cs index f00f10de1e..a433bc032b 100644 --- a/src/Umbraco.Web/Editors/MediaTypeController.cs +++ b/src/Umbraco.Web/Editors/MediaTypeController.cs @@ -87,9 +87,25 @@ namespace Umbraco.Web.Editors return Request.CreateResponse(HttpStatusCode.OK); } - public IEnumerable GetAvailableCompositeMediaTypes(int contentTypeId) + /// + /// Returns the avilable compositions for this content type + /// + /// + /// + /// This is normally an empty list but if additional content type aliases are passed in, any content types containing those aliases will be filtered out + /// along with any content types that have matching property types that are included in the filtered content types + /// + /// + /// This is normally an empty list but if additional property type aliases are passed in, any content types that have these aliases will be filtered out. + /// This is required because in the case of creating/modifying a content type because new property types being added to it are not yet persisted so cannot + /// be looked up via the db, they need to be passed in. + /// + /// + public IEnumerable GetAvailableCompositeMediaTypes(int contentTypeId, + [FromUri]string[] filterContentTypes, + [FromUri]string[] filterPropertyTypes) { - return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MediaType); + return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MediaType, filterContentTypes, filterPropertyTypes); } public ContentTypeCompositionDisplay GetEmpty(int parentId) diff --git a/src/Umbraco.Web/Editors/MemberTypeController.cs b/src/Umbraco.Web/Editors/MemberTypeController.cs index 7716f6ea00..de87a0fdc9 100644 --- a/src/Umbraco.Web/Editors/MemberTypeController.cs +++ b/src/Umbraco.Web/Editors/MemberTypeController.cs @@ -79,9 +79,25 @@ namespace Umbraco.Web.Editors return Request.CreateResponse(HttpStatusCode.OK); } - public IEnumerable GetAvailableCompositeMemberTypes(int contentTypeId) + /// + /// Returns the avilable compositions for this content type + /// + /// + /// + /// This is normally an empty list but if additional content type aliases are passed in, any content types containing those aliases will be filtered out + /// along with any content types that have matching property types that are included in the filtered content types + /// + /// + /// This is normally an empty list but if additional property type aliases are passed in, any content types that have these aliases will be filtered out. + /// This is required because in the case of creating/modifying a content type because new property types being added to it are not yet persisted so cannot + /// be looked up via the db, they need to be passed in. + /// + /// + public IEnumerable GetAvailableCompositeMemberTypes(int contentTypeId, + [FromUri]string[] filterContentTypes, + [FromUri]string[] filterPropertyTypes) { - return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MemberType); + return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MemberType, filterContentTypes, filterPropertyTypes); } public ContentTypeCompositionDisplay GetEmpty() From f7b34c76f96fb7c28f444fe77c017297732a7730 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 11 Jan 2016 15:38:07 +0100 Subject: [PATCH 2/7] got the filtering all working - this ensures that no CTs are selectable if any current property type exists in one of them in the list and re-filters on each CT selection in the list to ensure no illegal comps can be created. --- .../Services/ContentTypeServiceExtensions.cs | 38 +++++++++---------- .../components/umbgroupsbuilder.directive.js | 15 ++++++-- .../common/resources/contenttype.resource.js | 13 ++++++- .../common/resources/mediatype.resource.js | 13 ++++++- .../common/resources/membertype.resource.js | 13 ++++++- .../services/contenttypehelper.service.js | 7 +++- .../compositions/compositions.html | 4 +- 7 files changed, 72 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index e43818ae14..d9aa41c02a 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -28,22 +28,22 @@ namespace Umbraco.Core.Services IContentTypeComposition[] allContentTypes, string[] filterContentTypes = null, string[] filterPropertyTypes = null) - { - - if (filterContentTypes == null) filterContentTypes = new string[] { }; - if (filterPropertyTypes == null) filterContentTypes = new string[] { }; - - //ensure the source alias is added - var filterContentTypesList = filterContentTypes.Where(x => x.IsNullOrWhiteSpace() == false).ToList(); - if (source != null && filterContentTypesList.Contains(source.Alias) == false) - filterContentTypesList.Add(source.Alias); + { + filterContentTypes = filterContentTypes == null + ? new string[] { } + : filterContentTypes.Where(x => x.IsNullOrWhiteSpace() == false).ToArray(); //create the full list of property types to use as the filter - var filteredPropertyTypes = allContentTypes - .Where(c => filterContentTypesList.Contains(c.Alias)) - .SelectMany(c => c.PropertyTypes) - .Select(c => c.Alias) - .Union(filterPropertyTypes); + //this is the combination of all property type aliases found in the content types passed in for the filter + //as well as the specific property types passed in for the filter + filterPropertyTypes = filterPropertyTypes == null + ? new string[] {} + : allContentTypes + .Where(c => filterContentTypes.Contains(c.Alias)) + .SelectMany(c => c.PropertyTypes) + .Select(c => c.Alias) + .Union(filterPropertyTypes) + .ToArray(); var sourceId = source != null ? source.Id : 0; @@ -96,17 +96,13 @@ namespace Umbraco.Core.Services .Where(x => { //need to filter any content types that are included in this list - if (filterContentTypesList.Any() == false) return true; - - return filterContentTypesList.Any(c => c.InvariantEquals(x.Alias)) == false; + return filterContentTypes.Any(c => c.InvariantEquals(x.Alias)) == false; }) .Where(x => { - //need to filter any content types that have matching property aliases that are included in this list - if (filterContentTypesList.Any() == false) return true; - + //need to filter any content types that have matching property aliases that are included in this list //ensure that we don't return if there's any overlapping property aliases from the filtered ones specified - return filteredPropertyTypes.Intersect( + return filterPropertyTypes.Intersect( x.PropertyTypes.Select(p => p.Alias), StringComparer.InvariantCultureIgnoreCase).Any() == false; }) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js index 2657424818..43b1ced6b6 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js @@ -125,13 +125,22 @@ _.union(scope.compositionsDialogModel.compositeContentTypes, [selectedContentType.alias]) : //the user has unselected the item so remove from the current list _.reject(scope.compositionsDialogModel.compositeContentTypes, function(i) { - return i === selectedContentType.alias; - }); + return i === selectedContentType && selectedContentType.alias; + }); + + //get the currently assigned property type aliases - ensure we pass these to the server side filer + var propAliasesExisting = _.filter(_.flatten(_.map(scope.model.groups, function(g) { + return _.map(g.properties, function(p) { + return p.alias; + }); + })), function (f) { + return f !== null && f !== undefined; + }); //use a different resource lookup depending on the content type type var resourceLookup = scope.contentType === "documentType" ? contentTypeResource.getAvailableCompositeContentTypes : mediaTypeResource.getAvailableCompositeContentTypes; - return resourceLookup(scope.model.id, selectedContentTypeAliases).then(function (filteredAvailableCompositeTypes) { + return resourceLookup(scope.model.id, selectedContentTypeAliases, propAliasesExisting).then(function (filteredAvailableCompositeTypes) { _.each(scope.compositionsDialogModel.availableCompositeContentTypes, function (current) { //reset first current.disallow = false; diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js index 10e969226d..c3e1368efe 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/contenttype.resource.js @@ -16,10 +16,14 @@ function contentTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { 'Failed to retrieve count'); }, - getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes) { + getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes, filterPropertyTypes) { if (!filterContentTypes) { filterContentTypes = []; } + if (!filterPropertyTypes) { + filterPropertyTypes = []; + } + var query = ""; _.each(filterContentTypes, function (item) { query += "filterContentTypes=" + item + "&"; @@ -28,6 +32,13 @@ function contentTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { if (filterContentTypes.length === 0) { query += "filterContentTypes=&"; } + _.each(filterPropertyTypes, function (item) { + query += "filterPropertyTypes=" + item + "&"; + }); + // if filterPropertyTypes array is empty we need a empty variable in the querystring otherwise the service returns a error + if (filterPropertyTypes.length === 0) { + query += "filterPropertyTypes=&"; + } query += "contentTypeId=" + contentTypeId; return umbRequestHelper.resourcePromise( diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js index dff3a99667..17a96821fd 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/mediatype.resource.js @@ -16,10 +16,14 @@ function mediaTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { 'Failed to retrieve count'); }, - getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes) { + getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes, filterPropertyTypes) { if (!filterContentTypes) { filterContentTypes = []; } + if (!filterPropertyTypes) { + filterPropertyTypes = []; + } + var query = ""; _.each(filterContentTypes, function (item) { query += "filterContentTypes=" + item + "&"; @@ -28,6 +32,13 @@ function mediaTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { if (filterContentTypes.length === 0) { query += "filterContentTypes=&"; } + _.each(filterPropertyTypes, function (item) { + query += "filterPropertyTypes=" + item + "&"; + }); + // if filterPropertyTypes array is empty we need a empty variable in the querystring otherwise the service returns a error + if (filterPropertyTypes.length === 0) { + query += "filterPropertyTypes=&"; + } query += "contentTypeId=" + contentTypeId; return umbRequestHelper.resourcePromise( diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js index e8253e463d..0649277c54 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/membertype.resource.js @@ -7,10 +7,14 @@ function memberTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { return { - getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes) { + getAvailableCompositeContentTypes: function (contentTypeId, filterContentTypes, filterPropertyTypes) { if (!filterContentTypes) { filterContentTypes = []; } + if (!filterPropertyTypes) { + filterPropertyTypes = []; + } + var query = ""; _.each(filterContentTypes, function (item) { query += "filterContentTypes=" + item + "&"; @@ -19,6 +23,13 @@ function memberTypeResource($q, $http, umbRequestHelper, umbDataFormatter) { if (filterContentTypes.length === 0) { query += "filterContentTypes=&"; } + _.each(filterPropertyTypes, function (item) { + query += "filterPropertyTypes=" + item + "&"; + }); + // if filterPropertyTypes array is empty we need a empty variable in the querystring otherwise the service returns a error + if (filterPropertyTypes.length === 0) { + query += "filterPropertyTypes=&"; + } query += "contentTypeId=" + contentTypeId; return umbRequestHelper.resourcePromise( diff --git a/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js index 3230f83331..abfa58bc72 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/contenttypehelper.service.js @@ -52,11 +52,14 @@ function contentTypeHelper(contentTypeResource, dataTypeResource, $filter) { return p.alias; }); })); - var propAliasesExisting = _.flatten(_.map(contentType.groups, function(g) { + var propAliasesExisting = _.filter(_.flatten(_.map(contentType.groups, function(g) { return _.map(g.properties, function(p) { return p.alias; }); - })); + })), function(f) { + return f !== null && f !== undefined; + }); + var intersec = _.intersection(propertiesAdding, propAliasesExisting); if (intersec.length > 0) { //return the overlapping property aliases diff --git a/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html b/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html index 49dab6151a..34e3a3d2aa 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html @@ -27,7 +27,7 @@
    • @@ -46,4 +46,4 @@
    • -
\ No newline at end of file + From 7389d343e4d2c6cf2a1de794a54b21204b579681 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 11 Jan 2016 15:53:20 +0100 Subject: [PATCH 3/7] adds more unit tests --- .../Services/ContentTypeServiceExtensions.cs | 8 +- .../ContentTypeServiceExtensionsTests.cs | 113 ++++++++++++++++++ 2 files changed, 117 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index d9aa41c02a..7f7c74fb37 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -33,13 +33,13 @@ namespace Umbraco.Core.Services ? new string[] { } : filterContentTypes.Where(x => x.IsNullOrWhiteSpace() == false).ToArray(); + if (filterPropertyTypes == null) filterPropertyTypes = new string[] {}; + //create the full list of property types to use as the filter //this is the combination of all property type aliases found in the content types passed in for the filter //as well as the specific property types passed in for the filter - filterPropertyTypes = filterPropertyTypes == null - ? new string[] {} - : allContentTypes - .Where(c => filterContentTypes.Contains(c.Alias)) + filterPropertyTypes = allContentTypes + .Where(c => filterContentTypes.InvariantContains(c.Alias)) .SelectMany(c => c.PropertyTypes) .Select(c => c.Alias) .Union(filterPropertyTypes) diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs index fbdb45114e..b4054e1669 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs @@ -1,6 +1,9 @@ +using System; using System.Linq; using Moq; using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Models; using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; @@ -10,6 +13,116 @@ namespace Umbraco.Tests.Services [TestFixture] public class ContentTypeServiceExtensionsTests : BaseUmbracoApplicationTest { + [Test] + public void GetAvailableCompositeContentTypes_No_Overlap_By_Content_Type_And_Property_Type_Alias() + { + Action addPropType = (alias, ct) => + { + var contentCollection = new PropertyTypeCollection + { + new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) {Alias = alias, Name = "Title", Description = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88} + }; + var pg = new PropertyGroup(contentCollection) { Name = "test", SortOrder = 1 }; + ct.PropertyGroups.Add(pg); + }; + + var ct1 = MockedContentTypes.CreateBasicContentType("ct1", "CT1", null); + var ct2 = MockedContentTypes.CreateBasicContentType("ct2", "CT2", null); + addPropType("title", ct2); + var ct3 = MockedContentTypes.CreateBasicContentType("ct3", "CT3", null); + addPropType("title", ct3); + var ct4 = MockedContentTypes.CreateBasicContentType("ct4", "CT4", null); + var ct5 = MockedContentTypes.CreateBasicContentType("ct5", "CT5", null); + addPropType("blah", ct5); + ct1.Id = 1; + ct2.Id = 2; + ct3.Id = 3; + ct4.Id = 4; + ct5.Id = 4; + + var service = new Mock(); + + var availableTypes = service.Object.GetAvailableCompositeContentTypes( + ct1, + new[] { ct1, ct2, ct3, ct4, ct5 }, + new[] { ct2.Alias }, + new[] { "blah" }); + + Assert.AreEqual(1, availableTypes.Count()); + Assert.AreEqual(ct4.Id, availableTypes.ElementAt(0).Id); + } + + [Test] + public void GetAvailableCompositeContentTypes_No_Overlap_By_Property_Type_Alias() + { + Action addPropType = ct => + { + var contentCollection = new PropertyTypeCollection + { + new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) {Alias = "title", Name = "Title", Description = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88} + }; + var pg = new PropertyGroup(contentCollection) { Name = "test", SortOrder = 1 }; + ct.PropertyGroups.Add(pg); + }; + + var ct1 = MockedContentTypes.CreateBasicContentType("ct1", "CT1", null); + var ct2 = MockedContentTypes.CreateBasicContentType("ct2", "CT2", null); + addPropType(ct2); + var ct3 = MockedContentTypes.CreateBasicContentType("ct3", "CT3", null); + addPropType(ct3); + var ct4 = MockedContentTypes.CreateBasicContentType("ct4", "CT4", null); + ct1.Id = 1; + ct2.Id = 2; + ct3.Id = 3; + ct4.Id = 4; + + var service = new Mock(); + + var availableTypes = service.Object.GetAvailableCompositeContentTypes( + ct1, + new[] { ct1, ct2, ct3, ct4 }, + new string[] { }, + new[] { "title" }); + + Assert.AreEqual(1, availableTypes.Count()); + Assert.AreEqual(ct4.Id, availableTypes.ElementAt(0).Id); + } + + [Test] + public void GetAvailableCompositeContentTypes_No_Overlap_By_Content_Type() + { + Action addPropType = ct => + { + var contentCollection = new PropertyTypeCollection + { + new PropertyType(Constants.PropertyEditors.TextboxAlias, DataTypeDatabaseType.Ntext) {Alias = "title", Name = "Title", Description = "", Mandatory = false, SortOrder = 1, DataTypeDefinitionId = -88} + }; + var pg = new PropertyGroup(contentCollection) { Name = "test", SortOrder = 1 }; + ct.PropertyGroups.Add(pg); + }; + + var ct1 = MockedContentTypes.CreateBasicContentType("ct1", "CT1", null); + var ct2 = MockedContentTypes.CreateBasicContentType("ct2", "CT2", null); + addPropType(ct2); + var ct3 = MockedContentTypes.CreateBasicContentType("ct3", "CT3", null); + addPropType(ct3); + var ct4 = MockedContentTypes.CreateBasicContentType("ct4", "CT4", null); + ct1.Id = 1; + ct2.Id = 2; + ct3.Id = 3; + ct4.Id = 4; + + var service = new Mock(); + + var availableTypes = service.Object.GetAvailableCompositeContentTypes( + ct1, + new[] { ct1, ct2, ct3, ct4 }, + new [] {ct2.Alias}); + + Assert.AreEqual(1, availableTypes.Count()); + Assert.AreEqual(ct4.Id, availableTypes.ElementAt(0).Id); + } + [Test] public void GetAvailableCompositeContentTypes_Not_Itself() { From ee8405ccd01577ab5302987f2c2e06471450f265 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 11 Jan 2016 17:29:41 +0100 Subject: [PATCH 4/7] fixes rebase/merge --- .../components/umbgroupsbuilder.directive.js | 53 +++++++++++-------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js index 43b1ced6b6..c984f182a1 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js @@ -225,9 +225,7 @@ // submit overlay if no compositions has been removed // or the action has been confirmed } else { - - //TODO: RE-validate compositions - + // make sure that all tabs has an init property if (scope.model.groups.length !== 0) { angular.forEach(scope.model.groups, function(group) { @@ -252,32 +250,43 @@ scope.compositionsDialogModel = null; }, - selectCompositeContentType: function(compositeContentType) { + selectCompositeContentType: function (selectedContentType) { - if (scope.model.compositeContentTypes.indexOf(compositeContentType.alias) === -1) { - //merge composition with content type + //first check if this is a new selection - we need to store this value here before any further digests/async + // because after that the scope.model.compositeContentTypes will be populated with the selected value. + var newSelection = scope.model.compositeContentTypes.indexOf(selectedContentType.alias) === -1; - if(scope.contentType === "documentType") { + //based on the selection, we need to filter the available composite types list + filterAvailableCompositions(selectedContentType, newSelection).then(function () { - contentTypeResource.getById(compositeContentType.id).then(function(composition){ - contentTypeHelper.mergeCompositeContentType(scope.model, composition); - }); + if (newSelection) { + //merge composition with content type + //use a different resource lookup depending on the content type type + var resourceLookup = scope.contentType === "documentType" ? contentTypeResource.getById : mediaTypeResource.getById; - } else if(scope.contentType === "mediaType") { - - mediaTypeResource.getById(compositeContentType.id).then(function(composition){ - contentTypeHelper.mergeCompositeContentType(scope.model, composition); + resourceLookup(selectedContentType.id).then(function (composition) { + //based on the above filtering we shouldn't be able to select an invalid one, but let's be safe and + // double check here. + var overlappingAliases = contentTypeHelper.validateAddingComposition(scope.model, composition); + if (overlappingAliases.length > 0) { + //this will create an invalid composition, need to uncheck it + scope.compositionsDialogModel.compositeContentTypes.splice( + scope.compositionsDialogModel.compositeContentTypes.indexOf(composition.alias), 1); + //dissallow this until something else is unchecked + selectedContentType.disallow = true; } - }); + else { + contentTypeHelper.mergeCompositeContentType(scope.model, composition); + } + }); + } + else { + // split composition from content type + contentTypeHelper.splitCompositeContentType(scope.model, selectedContentType); + } - } - - - } else { - // split composition from content type - contentTypeHelper.splitCompositeContentType(scope.model, compositeContentType); - } + }); } }; From 67994c630f943e8d4306c22f6159b5103db7683d Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 Jan 2016 13:46:18 +0100 Subject: [PATCH 5/7] fixes null checking and ensures that the current property type aliases are passed up when first launching the compositions dialog. --- .../Services/ContentTypeServiceExtensions.cs | 4 +++- .../components/umbgroupsbuilder.directive.js | 23 +++++++++++++------ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index 7f7c74fb37..5fbf2a2c74 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -33,7 +33,9 @@ namespace Umbraco.Core.Services ? new string[] { } : filterContentTypes.Where(x => x.IsNullOrWhiteSpace() == false).ToArray(); - if (filterPropertyTypes == null) filterPropertyTypes = new string[] {}; + filterPropertyTypes = filterPropertyTypes == null + ? new string[] {} + : filterPropertyTypes.Where(x => x.IsNullOrWhiteSpace() == false).ToArray(); //create the full list of property types to use as the filter //this is the combination of all property type aliases found in the content types passed in for the filter diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js index c984f182a1..1a9367cf5a 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js @@ -293,21 +293,30 @@ var availableContentTypeResource = scope.contentType === "documentType" ? contentTypeResource.getAvailableCompositeContentTypes : mediaTypeResource.getAvailableCompositeContentTypes; var countContentTypeResource = scope.contentType === "documentType" ? contentTypeResource.getCount : mediaTypeResource.getCount; - $q.all([ + + //get the currently assigned property type aliases - ensure we pass these to the server side filer + var propAliasesExisting = _.filter(_.flatten(_.map(scope.model.groups, function(g) { + return _.map(g.properties, function(p) { + return p.alias; + }); + })), function(f) { + return f !== null && f !== undefined; + }); + $q.all([ //get available composite types - availableContentTypeResource(scope.model.id).then(function (result) { + availableContentTypeResource(scope.model.id, [], propAliasesExisting).then(function (result) { scope.compositionsDialogModel.availableCompositeContentTypes = result; // convert icons for composite content types iconHelper.formatContentTypeIcons(scope.compositionsDialogModel.availableCompositeContentTypes); }), //get content type count - countContentTypeResource().then(function (result) { + countContentTypeResource().then(function(result) { scope.compositionsDialogModel.totalContentTypes = parseInt(result, 10); }) - ]).then(function () { - //resolves when both other promises are done, now show it - scope.compositionsDialogModel.show = true; - }); + ]).then(function() { + //resolves when both other promises are done, now show it + scope.compositionsDialogModel.show = true; + }); }; From 85178bbad8aad69b75823b27826c0aa894fb229a Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 12 Jan 2016 15:44:37 +0100 Subject: [PATCH 6/7] Changes GetAvailableCompositeContentTypes to return a list of composite candidates and then whether they are actually allowed or not based on the filters. Updates the controllers response to be a list of which content types are allowed or not, updates the view accordingly, changes when the filtering happens so that it happens after the content type has been updated with new properties. --- .../Services/ContentTypeServiceExtensions.cs | 38 ++++------ .../ContentTypeServiceExtensionsTests.cs | 21 +++-- .../components/umbgroupsbuilder.directive.js | 76 +++++++++++-------- .../compositions/compositions.html | 10 +-- .../Editors/ContentTypeController.cs | 10 ++- .../Editors/ContentTypeControllerBase.cs | 6 +- .../Editors/MediaTypeController.cs | 10 ++- .../Editors/MemberTypeController.cs | 10 ++- 8 files changed, 105 insertions(+), 76 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index 5fbf2a2c74..97fa199e97 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -23,7 +23,7 @@ namespace Umbraco.Core.Services /// be looked up via the db, they need to be passed in. /// /// - public static IEnumerable GetAvailableCompositeContentTypes(this IContentTypeService ctService, + public static IEnumerable> GetAvailableCompositeContentTypes(this IContentTypeService ctService, IContentTypeComposition source, IContentTypeComposition[] allContentTypes, string[] filterContentTypes = null, @@ -48,20 +48,13 @@ namespace Umbraco.Core.Services .ToArray(); var sourceId = source != null ? source.Id : 0; - - //below is all ported from the old doc type editor and comes with the same weaknesses /insanity / magic - - // note: there are many sanity checks missing here and there ;-(( - // make sure once and for all - //if (allContentTypes.Any(x => x.ParentId > 0 && x.ContentTypeComposition.Any(y => y.Id == x.ParentId) == false)) - // throw new Exception("A parent does not belong to a composition."); - + // find out if any content type uses this content type var isUsing = allContentTypes.Where(x => x.ContentTypeComposition.Any(y => y.Id == sourceId)).ToArray(); if (isUsing.Length > 0) { //if already in use a composition, do not allow any composited types - return new List(); + return new List>(); } // if it is not used then composition is possible @@ -81,18 +74,10 @@ namespace Umbraco.Core.Services foreach (var x in indirectContentTypes) list.Add(x); - //// directContentTypes are those we use directly - //// they are already in indirectContentTypes, no need to add to the list - //var directContentTypes = source.ContentTypeComposition.ToArray(); - - //var enabled = usableContentTypes.Select(x => x.Id) // those we can use - // .Except(indirectContentTypes.Select(x => x.Id)) // except those that are indirectly used - // .Union(directContentTypes.Select(x => x.Id)) // but those that are directly used - // .Where(x => x != source.ParentId) // but not the parent - // .Distinct() - // .ToArray(); - - return list + //At this point we have a list of content types that 'could' be compositions + + //now we'll filter this list based on the filters requested + var filtered = list //not itself .Where(x => x.Id != sourceId) .Where(x => @@ -110,6 +95,15 @@ namespace Umbraco.Core.Services }) .OrderBy(x => x.Name) .ToList(); + + //now we can create our result based on what is still available + var result = list + .OrderBy(x => x.Name) + .Select(composition => filtered.Contains(composition) + ? new Tuple(composition, true) + : new Tuple(composition, false)).ToList(); + + return result; } /// diff --git a/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs b/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs index b4054e1669..6057443fa0 100644 --- a/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs +++ b/src/Umbraco.Tests/Services/ContentTypeServiceExtensionsTests.cs @@ -46,7 +46,8 @@ namespace Umbraco.Tests.Services ct1, new[] { ct1, ct2, ct3, ct4, ct5 }, new[] { ct2.Alias }, - new[] { "blah" }); + new[] { "blah" }) + .Where(x => x.Item2).Select(x => x.Item1).ToArray(); Assert.AreEqual(1, availableTypes.Count()); Assert.AreEqual(ct4.Id, availableTypes.ElementAt(0).Id); @@ -82,7 +83,8 @@ namespace Umbraco.Tests.Services ct1, new[] { ct1, ct2, ct3, ct4 }, new string[] { }, - new[] { "title" }); + new[] { "title" }) + .Where(x => x.Item2).Select(x => x.Item1).ToArray(); Assert.AreEqual(1, availableTypes.Count()); Assert.AreEqual(ct4.Id, availableTypes.ElementAt(0).Id); @@ -117,7 +119,8 @@ namespace Umbraco.Tests.Services var availableTypes = service.Object.GetAvailableCompositeContentTypes( ct1, new[] { ct1, ct2, ct3, ct4 }, - new [] {ct2.Alias}); + new [] {ct2.Alias}) + .Where(x => x.Item2).Select(x => x.Item1).ToArray(); Assert.AreEqual(1, availableTypes.Count()); Assert.AreEqual(ct4.Id, availableTypes.ElementAt(0).Id); @@ -137,7 +140,8 @@ namespace Umbraco.Tests.Services var availableTypes = service.Object.GetAvailableCompositeContentTypes( ct1, - new[] {ct1, ct2, ct3}); + new[] {ct1, ct2, ct3}) + .Where(x => x.Item2).Select(x => x.Item1).ToArray(); Assert.AreEqual(2, availableTypes.Count()); Assert.AreEqual(ct2.Id, availableTypes.ElementAt(0).Id); @@ -202,7 +206,8 @@ namespace Umbraco.Tests.Services var availableTypes = service.Object.GetAvailableCompositeContentTypes( ct1, - new[] { ct1, ct2, ct3 }); + new[] { ct1, ct2, ct3 }) + .Where(x => x.Item2).Select(x => x.Item1).ToArray(); Assert.AreEqual(1, availableTypes.Count()); Assert.AreEqual(ct3.Id, availableTypes.Single().Id); @@ -224,7 +229,8 @@ namespace Umbraco.Tests.Services var availableTypes = service.Object.GetAvailableCompositeContentTypes( ct1, - new[] { ct1, ct2, ct3 }); + new[] { ct1, ct2, ct3 }) + .Where(x => x.Item2).Select(x => x.Item1).ToArray(); Assert.AreEqual(2, availableTypes.Count()); Assert.AreEqual(ct2.Id, availableTypes.ElementAt(0).Id); @@ -250,7 +256,8 @@ namespace Umbraco.Tests.Services var availableTypes = service.Object.GetAvailableCompositeContentTypes( ct1, - new[] { ct1, ct2, ct3 }); + new[] { ct1, ct2, ct3 }) + .Where(x => x.Item2).Select(x => x.Item1).ToArray(); Assert.AreEqual(3, availableTypes.Count()); Assert.AreEqual(ct2.Id, availableTypes.ElementAt(0).Id); diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js index 1a9367cf5a..7dadee5e09 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js @@ -125,7 +125,7 @@ _.union(scope.compositionsDialogModel.compositeContentTypes, [selectedContentType.alias]) : //the user has unselected the item so remove from the current list _.reject(scope.compositionsDialogModel.compositeContentTypes, function(i) { - return i === selectedContentType && selectedContentType.alias; + return i === selectedContentType.alias; }); //get the currently assigned property type aliases - ensure we pass these to the server side filer @@ -143,14 +143,16 @@ return resourceLookup(scope.model.id, selectedContentTypeAliases, propAliasesExisting).then(function (filteredAvailableCompositeTypes) { _.each(scope.compositionsDialogModel.availableCompositeContentTypes, function (current) { //reset first - current.disallow = false; + current.allowed = true; //see if this list item is found in the response (allowed) list var found = _.find(filteredAvailableCompositeTypes, function (f) { - return current.alias === f.alias; + return current.contentType.alias === f.contentType.alias; }); - //disallow if the item was not found in the response (allowed) list - - // BUT do not set to dissallowed if it is currently checked - current.disallow = (selectedContentTypeAliases.indexOf(current.alias) === -1) && (found ? false : true); + //allow if the item was found in the response (allowed) list - + // and ensure its set to allowed if it is currently checked + current.allowed = (selectedContentTypeAliases.indexOf(current.contentType.alias) !== -1) || + ((found !== null && found !== undefined) ? found.allowed : false); + }); }); } @@ -256,37 +258,42 @@ // because after that the scope.model.compositeContentTypes will be populated with the selected value. var newSelection = scope.model.compositeContentTypes.indexOf(selectedContentType.alias) === -1; - //based on the selection, we need to filter the available composite types list - filterAvailableCompositions(selectedContentType, newSelection).then(function () { + if (newSelection) { + //merge composition with content type - if (newSelection) { - //merge composition with content type + //use a different resource lookup depending on the content type type + var resourceLookup = scope.contentType === "documentType" ? contentTypeResource.getById : mediaTypeResource.getById; - //use a different resource lookup depending on the content type type - var resourceLookup = scope.contentType === "documentType" ? contentTypeResource.getById : mediaTypeResource.getById; + resourceLookup(selectedContentType.id).then(function (composition) { + //based on the above filtering we shouldn't be able to select an invalid one, but let's be safe and + // double check here. + var overlappingAliases = contentTypeHelper.validateAddingComposition(scope.model, composition); + if (overlappingAliases.length > 0) { + //this will create an invalid composition, need to uncheck it + scope.compositionsDialogModel.compositeContentTypes.splice( + scope.compositionsDialogModel.compositeContentTypes.indexOf(composition.alias), 1); + //dissallow this until something else is unchecked + selectedContentType.allowed = false; + } + else { + contentTypeHelper.mergeCompositeContentType(scope.model, composition); + } - resourceLookup(selectedContentType.id).then(function (composition) { - //based on the above filtering we shouldn't be able to select an invalid one, but let's be safe and - // double check here. - var overlappingAliases = contentTypeHelper.validateAddingComposition(scope.model, composition); - if (overlappingAliases.length > 0) { - //this will create an invalid composition, need to uncheck it - scope.compositionsDialogModel.compositeContentTypes.splice( - scope.compositionsDialogModel.compositeContentTypes.indexOf(composition.alias), 1); - //dissallow this until something else is unchecked - selectedContentType.disallow = true; - } - else { - contentTypeHelper.mergeCompositeContentType(scope.model, composition); - } + //based on the selection, we need to filter the available composite types list + filterAvailableCompositions(selectedContentType, newSelection).then(function () { + //TODO: Here we could probably re-enable selection if we previously showed a throbber or something }); - } - else { - // split composition from content type - contentTypeHelper.splitCompositeContentType(scope.model, selectedContentType); - } + }); + } + else { + // split composition from content type + contentTypeHelper.splitCompositeContentType(scope.model, selectedContentType); - }); + //based on the selection, we need to filter the available composite types list + filterAvailableCompositions(selectedContentType, newSelection).then(function () { + //TODO: Here we could probably re-enable selection if we previously showed a throbber or something + }); + } } }; @@ -306,8 +313,11 @@ //get available composite types availableContentTypeResource(scope.model.id, [], propAliasesExisting).then(function (result) { scope.compositionsDialogModel.availableCompositeContentTypes = result; + var contentTypes = _.map(scope.compositionsDialogModel.availableCompositeContentTypes, function(c) { + return c.contentType; + }); // convert icons for composite content types - iconHelper.formatContentTypeIcons(scope.compositionsDialogModel.availableCompositeContentTypes); + iconHelper.formatContentTypeIcons(contentTypes); }), //get content type count countContentTypeResource().then(function(result) { diff --git a/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html b/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html index 34e3a3d2aa..b8719f029f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html +++ b/src/Umbraco.Web.UI.Client/src/views/common/overlays/contenttypeeditor/compositions/compositions.html @@ -35,14 +35,14 @@ ng-class="{ '-selected': model.compositeContentTypes.indexOf(compositeContentType.alias)+1 }"> + checklist-value="compositeContentType.contentType.alias" + ng-change="model.selectCompositeContentType(compositeContentType.contentType)" + ng-disabled="compositeContentType.allowed===false" />
- - {{ compositeContentType.name }} + + {{ compositeContentType.contentType.name }}
diff --git a/src/Umbraco.Web/Editors/ContentTypeController.cs b/src/Umbraco.Web/Editors/ContentTypeController.cs index f91844fe07..45fc01e55c 100644 --- a/src/Umbraco.Web/Editors/ContentTypeController.cs +++ b/src/Umbraco.Web/Editors/ContentTypeController.cs @@ -109,11 +109,17 @@ namespace Umbraco.Web.Editors /// be looked up via the db, they need to be passed in. /// /// - public IEnumerable GetAvailableCompositeContentTypes(int contentTypeId, + public HttpResponseMessage GetAvailableCompositeContentTypes(int contentTypeId, [FromUri]string[] filterContentTypes, [FromUri]string[] filterPropertyTypes) { - return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.DocumentType, filterContentTypes, filterPropertyTypes); + var result = PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.DocumentType, filterContentTypes, filterPropertyTypes) + .Select(x => new + { + contentType = x.Item1, + allowed = x.Item2 + }); + return Request.CreateResponse(result); } [UmbracoTreeAuthorize( diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index 2f98372e77..17a3b01666 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -61,7 +61,7 @@ namespace Umbraco.Web.Editors /// /// /// - protected IEnumerable PerformGetAvailableCompositeContentTypes(int contentTypeId, + protected IEnumerable> PerformGetAvailableCompositeContentTypes(int contentTypeId, UmbracoObjectTypes type, string[] filterContentTypes, string[] filterPropertyTypes) @@ -107,10 +107,10 @@ namespace Umbraco.Web.Editors var filtered = Services.ContentTypeService.GetAvailableCompositeContentTypes(source, allContentTypes, filterContentTypes, filterPropertyTypes); return filtered - .Select(Mapper.Map) + .Select(x => new Tuple(Mapper.Map(x.Item1), x.Item2)) .Select(x => { - x.Name = TranslateItem(x.Name); + x.Item1.Name = TranslateItem(x.Item1.Name); return x; }) .ToList(); diff --git a/src/Umbraco.Web/Editors/MediaTypeController.cs b/src/Umbraco.Web/Editors/MediaTypeController.cs index a433bc032b..8b88d99a33 100644 --- a/src/Umbraco.Web/Editors/MediaTypeController.cs +++ b/src/Umbraco.Web/Editors/MediaTypeController.cs @@ -101,11 +101,17 @@ namespace Umbraco.Web.Editors /// be looked up via the db, they need to be passed in. /// /// - public IEnumerable GetAvailableCompositeMediaTypes(int contentTypeId, + public HttpResponseMessage GetAvailableCompositeMediaTypes(int contentTypeId, [FromUri]string[] filterContentTypes, [FromUri]string[] filterPropertyTypes) { - return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MediaType, filterContentTypes, filterPropertyTypes); + var result = PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MediaType, filterContentTypes, filterPropertyTypes) + .Select(x => new + { + contentType = x.Item1, + allowed = x.Item2 + }); + return Request.CreateResponse(result); } public ContentTypeCompositionDisplay GetEmpty(int parentId) diff --git a/src/Umbraco.Web/Editors/MemberTypeController.cs b/src/Umbraco.Web/Editors/MemberTypeController.cs index de87a0fdc9..27941a38df 100644 --- a/src/Umbraco.Web/Editors/MemberTypeController.cs +++ b/src/Umbraco.Web/Editors/MemberTypeController.cs @@ -93,11 +93,17 @@ namespace Umbraco.Web.Editors /// be looked up via the db, they need to be passed in. /// /// - public IEnumerable GetAvailableCompositeMemberTypes(int contentTypeId, + public HttpResponseMessage GetAvailableCompositeMemberTypes(int contentTypeId, [FromUri]string[] filterContentTypes, [FromUri]string[] filterPropertyTypes) { - return PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MemberType, filterContentTypes, filterPropertyTypes); + var result = PerformGetAvailableCompositeContentTypes(contentTypeId, UmbracoObjectTypes.MemberType, filterContentTypes, filterPropertyTypes) + .Select(x => new + { + contentType = x.Item1, + allowed = x.Item2 + }); + return Request.CreateResponse(result); } public ContentTypeCompositionDisplay GetEmpty() From 3131096ffb236f47706741c7bd71d0a5b06f95ef Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 13 Jan 2016 11:33:46 +0100 Subject: [PATCH 7/7] Ensures that itself is never returned in the list, not even disabled (since you can never select yourself), ensures that any comp already selected is enabled for selection in the UI. --- .../Services/ContentTypeServiceExtensions.cs | 10 +++++----- .../components/umbgroupsbuilder.directive.js | 2 +- src/Umbraco.Web/Editors/ContentTypeControllerBase.cs | 11 +++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs index 97fa199e97..0a5cff6d4b 100644 --- a/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs +++ b/src/Umbraco.Core/Services/ContentTypeServiceExtensions.cs @@ -77,9 +77,7 @@ namespace Umbraco.Core.Services //At this point we have a list of content types that 'could' be compositions //now we'll filter this list based on the filters requested - var filtered = list - //not itself - .Where(x => x.Id != sourceId) + var filtered = list .Where(x => { //need to filter any content types that are included in this list @@ -95,9 +93,11 @@ namespace Umbraco.Core.Services }) .OrderBy(x => x.Name) .ToList(); - - //now we can create our result based on what is still available + + //now we can create our result based on what is still available var result = list + //not itself + .Where(x => x.Id != sourceId) .OrderBy(x => x.Name) .Select(composition => filtered.Contains(composition) ? new Tuple(composition, true) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js index 7dadee5e09..df92ae1c90 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/umbgroupsbuilder.directive.js @@ -317,7 +317,7 @@ return c.contentType; }); // convert icons for composite content types - iconHelper.formatContentTypeIcons(contentTypes); + iconHelper.formatContentTypeIcons(contentTypes); }), //get content type count countContentTypeResource().then(function(result) { diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index 17a3b01666..a1082639e8 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -106,11 +106,22 @@ namespace Umbraco.Web.Editors var filtered = Services.ContentTypeService.GetAvailableCompositeContentTypes(source, allContentTypes, filterContentTypes, filterPropertyTypes); + var currCompositions = source == null ? new string[] { } : source.ContentTypeComposition.Select(x => x.Alias).ToArray(); + return filtered .Select(x => new Tuple(Mapper.Map(x.Item1), x.Item2)) .Select(x => { + //translate the name x.Item1.Name = TranslateItem(x.Item1.Name); + + //we need to ensure that the item is enabled if it is already selected + if (currCompositions.Contains(x.Item1.Alias)) + { + //re-set x to be allowed (NOTE: I didn't know you could set an enumerable item in a lambda!) + x = new Tuple(x.Item1, true); + } + return x; }) .ToList();