From 26382ab8d567d009a50df4ae5ad372d4bc602c09 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Mon, 27 Sep 2021 14:36:17 +0100 Subject: [PATCH 01/14] Add EntityController GetUrlsByUdis Enables loading multiple URLs in a single request for Media & Documents --- src/Umbraco.Web/Editors/EntityController.cs | 49 +++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/Umbraco.Web/Editors/EntityController.cs b/src/Umbraco.Web/Editors/EntityController.cs index b6bef3fd9651..66173634185b 100644 --- a/src/Umbraco.Web/Editors/EntityController.cs +++ b/src/Umbraco.Web/Editors/EntityController.cs @@ -235,6 +235,55 @@ public HttpResponseMessage GetUrl(Udi udi, string culture = "*") return GetUrl(intId.Result, entityType, culture); } + /// + /// Get entity URLs by UDIs + /// + /// + /// A list of UDIs to lookup items by + /// + /// The culture to fetch the URL for + /// Dictionary mapping Udi -> Url + /// + /// We allow for POST because there could be quite a lot of Ids. + /// + [HttpGet] + [HttpPost] + public IDictionary GetUrlsByUdis([FromJsonPath] Udi[] ids, string culture = null) + { + if (ids == null) + { + throw new HttpResponseException(HttpStatusCode.NotFound); + } + + if (ids.Length == 0) + { + return new Dictionary(); + } + + // TODO: PMJ 2021-09-27 - Should GetUrl(Udi) exist as an extension method on UrlProvider/IUrlProvider (in v9) + string MediaOrDocumentUrl(Udi udi) + { + if (udi is not GuidUdi guidUdi) + { + return null; + } + + return guidUdi.EntityType switch + { + Constants.UdiEntityType.Document => UmbracoContext.UrlProvider.GetUrl(guidUdi.Guid, culture: culture ?? ClientCulture()), + // NOTE: If culture is passed here we get an empty string rather than a media item URL WAT + Constants.UdiEntityType.Media => UmbracoContext.UrlProvider.GetMediaUrl(guidUdi.Guid, culture: null), + _ => null + }; + } + + return ids + .Select(udi => new { + Udi = udi, + Url = MediaOrDocumentUrl(udi) + }).ToDictionary(x => x.Udi, x => x.Url); + } + /// /// Gets the URL of an entity /// From 4eb75799e7458ddebec05958b53e80b2030ad8d7 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Mon, 27 Sep 2021 14:39:04 +0100 Subject: [PATCH 02/14] Update content picker to use GetUrlsByUdis --- .../common/mocks/resources/entity.mocks.js | 13 +++++++ .../src/common/resources/entity.resource.js | 15 ++++++++ .../contentpicker/contentpicker.controller.js | 36 +++++++------------ 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/mocks/resources/entity.mocks.js b/src/Umbraco.Web.UI.Client/src/common/mocks/resources/entity.mocks.js index 2c2007dd91b0..05594115e1d2 100644 --- a/src/Umbraco.Web.UI.Client/src/common/mocks/resources/entity.mocks.js +++ b/src/Umbraco.Web.UI.Client/src/common/mocks/resources/entity.mocks.js @@ -34,6 +34,15 @@ angular.module('umbraco.mocks'). return [200, nodes, null]; } + function returnUrlsbyUdis(status, data, headers) { + + if (!mocksUtils.checkAuth()) { + return [401, null, null]; + } + + return [200, {}, null]; + } + function returnEntitybyIdsPost(method, url, data, headers) { if (!mocksUtils.checkAuth()) { @@ -73,6 +82,10 @@ angular.module('umbraco.mocks'). .whenPOST(mocksUtils.urlRegex('/umbraco/UmbracoApi/Entity/GetByIds')) .respond(returnEntitybyIdsPost); + $httpBackend + .whenPOST(mocksUtils.urlRegex('/umbraco/UmbracoApi/Entity/GetUrlsByUdis')) + .respond(returnUrlsbyUdis); + $httpBackend .whenGET(mocksUtils.urlRegex('/umbraco/UmbracoApi/Entity/GetAncestors')) .respond(returnEntitybyIds); diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js index 0b060da34bed..1c29a69c2dda 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js @@ -127,6 +127,21 @@ function entityResource($q, $http, umbRequestHelper) { 'Failed to retrieve url for id:' + id); }, + getUrlsByUdis: function(ids, culture) { + var query = "culture=" + (culture || ""); + + return umbRequestHelper.resourcePromise( + $http.post( + umbRequestHelper.getApiUrl( + "entityApiBaseUrl", + "GetUrlsByUdis", + query), + { + ids: ids + }), + 'Failed to retrieve url map for ids ' + ids); + }, + getUrlByUdi: function (udi, culture) { if (!udi) { diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js index d8c7b3e76a62..7c8c1e64fbd7 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/contentpicker/contentpicker.controller.js @@ -413,8 +413,13 @@ function contentPickerController($scope, $q, $routeParams, $location, entityReso var missingIds = _.difference(valueIds, renderModelIds); if (missingIds.length > 0) { - return entityResource.getByIds(missingIds, entityType).then(function (data) { + var requests = [ + entityResource.getByIds(missingIds, entityType), + entityResource.getUrlsByUdis(missingIds) + ]; + + return $q.all(requests).then(function ([data, urlMap]) { _.each(valueIds, function (id, i) { var entity = _.find(data, function (d) { @@ -422,7 +427,12 @@ function contentPickerController($scope, $q, $routeParams, $location, entityReso }); if (entity) { - addSelectedItem(entity); + + entity.url = entity.trashed + ? vm.labels.general_recycleBin + : urlMap[id]; + + addSelectedItem(entity); } }); @@ -469,26 +479,6 @@ function contentPickerController($scope, $q, $routeParams, $location, entityReso } - function setEntityUrl(entity) { - - // get url for content and media items - if (entityType !== "Member") { - entityResource.getUrl(entity.id, entityType).then(function (data) { - // update url - $scope.renderModel.forEach(function (item) { - if (item.id === entity.id) { - if (entity.trashed) { - item.url = vm.labels.general_recycleBin; - } else { - item.url = data; - } - } - }); - }); - } - - } - function addSelectedItem(item) { // set icon @@ -523,8 +513,6 @@ function contentPickerController($scope, $q, $routeParams, $location, entityReso "published": (item.metaData && item.metaData.IsPublished === false && entityType === "Document") ? false : true // only content supports published/unpublished content so we set everything else to published so the UI looks correct }); - - setEntityUrl(item); } function setSortingState(items) { From 20b9db87d091e7d53f9f25d4d11744afcffa2584 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Mon, 4 Oct 2021 09:45:33 +0200 Subject: [PATCH 03/14] Remove inherited property group id/key when local properties are added (#11231) * Remove inherited property group id/key when local properties are added * Rebind saved content type values * Remove inherited from save group --- .../services/contenttypehelper.service.js | 39 +++++++++++++++++ .../services/umbdataformatter.service.js | 40 +++++++++-------- .../views/documenttypes/edit.controller.js | 32 ++------------ .../src/views/mediatypes/edit.controller.js | 35 ++------------- .../src/views/membertypes/edit.controller.js | 43 +++---------------- 5 files changed, 73 insertions(+), 116 deletions(-) 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 d9157093a7b9..cceb67689fed 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 @@ -439,6 +439,45 @@ function contentTypeHelper(contentTypeResource, dataTypeResource, $filter, $inje array.push(placeholder); + }, + + rebindSavedContentType: function (contentType, savedContentType) { + // The saved content type might have updated values (eg. new IDs/keys), so make sure the view model is updated + contentType.ModelState = savedContentType.ModelState; + contentType.id = savedContentType.id; + contentType.groups.forEach(function (group) { + if (!group.alias) return; + + var k = 0; + while (k < savedContentType.groups.length && savedContentType.groups[k].alias != group.alias) + k++; + + if (k == savedContentType.groups.length) { + group.id = 0; + return; + } + + var savedGroup = savedContentType.groups[k]; + group.id = savedGroup.id; + group.key = savedGroup.key; + group.contentTypeId = savedGroup.contentTypeId; + + group.properties.forEach(function (property) { + if (property.id || !property.alias) return; + + k = 0; + while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) + k++; + + if (k == savedGroup.properties.length) { + property.id = 0; + return; + } + + var savedProperty = savedGroup.properties[k]; + property.id = savedProperty.id; + }); + }); } }; diff --git a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js index 226fabeae41d..7270ae5bbf06 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/umbdataformatter.service.js @@ -37,7 +37,7 @@ return { - formatChangePasswordModel: function(model) { + formatChangePasswordModel: function (model) { if (!model) { return null; } @@ -59,26 +59,23 @@ }, formatContentTypePostData: function (displayModel, action) { - - //create the save model from the display model + // Create the save model from the display model var saveModel = _.pick(displayModel, 'compositeContentTypes', 'isContainer', 'allowAsRoot', 'allowedTemplates', 'allowedContentTypes', 'alias', 'description', 'thumbnail', 'name', 'id', 'icon', 'trashed', 'key', 'parentId', 'alias', 'path', 'allowCultureVariant', 'allowSegmentVariant', 'isElement'); - // TODO: Map these saveModel.allowedTemplates = _.map(displayModel.allowedTemplates, function (t) { return t.alias; }); saveModel.defaultTemplate = displayModel.defaultTemplate ? displayModel.defaultTemplate.alias : null; var realGroups = _.reject(displayModel.groups, function (g) { - //do not include these tabs + // Do not include groups with init state return g.tabState === "init"; }); saveModel.groups = _.map(realGroups, function (g) { - - var saveGroup = _.pick(g, 'inherited', 'id', 'sortOrder', 'name', 'key', 'alias', 'type'); + var saveGroup = _.pick(g, 'id', 'sortOrder', 'name', 'key', 'alias', 'type'); var realProperties = _.reject(g.properties, function (p) { - //do not include these properties + // Do not include properties with init state or inherited from a composition return p.propertyState === "init" || p.inherited === true; }); @@ -89,16 +86,21 @@ saveGroup.properties = saveProperties; - //if this is an inherited group and there are not non-inherited properties on it, then don't send up the data - if (saveGroup.inherited === true && saveProperties.length === 0) { - return null; + if (g.inherited === true) { + if (saveProperties.length === 0) { + // All properties are inherited from the compositions, no need to save this group + return null; + } else if (g.contentTypeId != saveModel.id) { + // We have local properties, but the group id is not local, ensure a new id/key is generated on save + saveGroup = _.omit(saveGroup, 'id', 'key'); + } } return saveGroup; }); - //we don't want any null groups saveModel.groups = _.reject(saveModel.groups, function (g) { + // Do not include empty/null groups return !g; }); @@ -127,17 +129,17 @@ }, /** formats the display model used to display the dictionary to the model used to save the dictionary */ - formatDictionaryPostData : function(dictionary, nameIsDirty) { + formatDictionaryPostData: function (dictionary, nameIsDirty) { var saveModel = { parentId: dictionary.parentId, id: dictionary.id, name: dictionary.name, nameIsDirty: nameIsDirty, translations: [], - key : dictionary.key + key: dictionary.key }; - for(var i = 0; i < dictionary.translations.length; i++) { + for (var i = 0; i < dictionary.translations.length; i++) { saveModel.translations.push({ isoCode: dictionary.translations[i].isoCode, languageId: dictionary.translations[i].languageId, @@ -362,7 +364,7 @@ parentId: displayModel.parentId, //set the action on the save model action: action, - variants: _.map(displayModel.variants, function(v) { + variants: _.map(displayModel.variants, function (v) { return { name: v.name || "", //if its null/empty,we must pass up an empty string else we get json converter errors properties: getContentProperties(v.tabs), @@ -392,7 +394,7 @@ * @param {} displayModel * @returns {} */ - formatContentGetData: function(displayModel) { + formatContentGetData: function (displayModel) { // We need to check for invariant properties among the variant variants, // as the value of an invariant property is shared between different variants. @@ -458,12 +460,12 @@ * Formats the display model used to display the relation type to a model used to save the relation type. * @param {Object} relationType */ - formatRelationTypePostData : function(relationType) { + formatRelationTypePostData: function (relationType) { var saveModel = { id: relationType.id, name: relationType.name, alias: relationType.alias, - key : relationType.key, + key: relationType.key, isBidirectional: relationType.isBidirectional, parentObjectType: relationType.parentObjectType, childObjectType: relationType.childObjectType diff --git a/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js index 3672af900c4d..a5e49163f9e5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/documenttypes/edit.controller.js @@ -324,35 +324,9 @@ scope: $scope, content: vm.contentType, infiniteMode: infiniteMode, - // we need to rebind... the IDs that have been created! - rebindCallback: function (origContentType, savedContentType) { - vm.contentType.ModelState = savedContentType.ModelState; - vm.contentType.id = savedContentType.id; - vm.contentType.groups.forEach(function (group) { - if (!group.name) return; - var k = 0; - while (k < savedContentType.groups.length && savedContentType.groups[k].name != group.name) - k++; - if (k == savedContentType.groups.length) { - group.id = 0; - return; - } - var savedGroup = savedContentType.groups[k]; - if (!group.id) group.id = savedGroup.id; - - group.properties.forEach(function (property) { - if (property.id || !property.alias) return; - k = 0; - while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) - k++; - if (k == savedGroup.properties.length) { - property.id = 0; - return; - } - var savedProperty = savedGroup.properties[k]; - property.id = savedProperty.id; - }); - }); + rebindCallback: function (_, savedContentType) { + // we need to rebind... the IDs that have been created! + contentTypeHelper.rebindSavedContentType(vm.contentType, savedContentType); } }).then(function (data) { // allow UI to access server validation state diff --git a/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js index ecf2aec30c0b..32fac36baaee 100644 --- a/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/mediatypes/edit.controller.js @@ -295,38 +295,9 @@ saveMethod: mediaTypeResource.save, scope: $scope, content: vm.contentType, - // we need to rebind... the IDs that have been created! - rebindCallback: function (origContentType, savedContentType) { - vm.contentType.id = savedContentType.id; - vm.contentType.groups.forEach(function (group) { - if (!group.name) return; - - var k = 0; - while (k < savedContentType.groups.length && savedContentType.groups[k].name != group.name) - k++; - if (k == savedContentType.groups.length) { - group.id = 0; - return; - } - - var savedGroup = savedContentType.groups[k]; - if (!group.id) group.id = savedGroup.id; - - group.properties.forEach(function (property) { - if (property.id || !property.alias) return; - - k = 0; - while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) - k++; - if (k == savedGroup.properties.length) { - property.id = 0; - return; - } - - var savedProperty = savedGroup.properties[k]; - property.id = savedProperty.id; - }); - }); + rebindCallback: function (_, savedContentType) { + // we need to rebind... the IDs that have been created! + contentTypeHelper.rebindSavedContentType(vm.contentType, savedContentType); } }).then(function (data) { //success diff --git a/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js index 53bb4adb9b99..2eddc71924be 100644 --- a/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/membertypes/edit.controller.js @@ -175,11 +175,11 @@ //we are creating so get an empty data type item memberTypeResource.getScaffold(memberTypeId) - .then(function (dt) { - init(dt); + .then(function (dt) { + init(dt); - vm.page.loading = false; - }); + vm.page.loading = false; + }); } else { loadMemberType(); @@ -215,38 +215,9 @@ saveMethod: memberTypeResource.save, scope: $scope, content: vm.contentType, - // we need to rebind... the IDs that have been created! - rebindCallback: function (origContentType, savedContentType) { - vm.contentType.id = savedContentType.id; - vm.contentType.groups.forEach(function (group) { - if (!group.name) return; - - var k = 0; - while (k < savedContentType.groups.length && savedContentType.groups[k].name != group.name) - k++; - if (k == savedContentType.groups.length) { - group.id = 0; - return; - } - - var savedGroup = savedContentType.groups[k]; - if (!group.id) group.id = savedGroup.id; - - group.properties.forEach(function (property) { - if (property.id || !property.alias) return; - - k = 0; - while (k < savedGroup.properties.length && savedGroup.properties[k].alias != property.alias) - k++; - if (k == savedGroup.properties.length) { - property.id = 0; - return; - } - - var savedProperty = savedGroup.properties[k]; - property.id = savedProperty.id; - }); - }); + rebindCallback: function (_, savedContentType) { + // we need to rebind... the IDs that have been created! + contentTypeHelper.rebindSavedContentType(vm.contentType, savedContentType); } }).then(function (data) { //success From 106f6dce25fbf5be2c7fb6de27e47c645b683253 Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Tue, 5 Oct 2021 13:45:43 +0100 Subject: [PATCH 04/14] Rename parameter for clarity --- .../src/common/resources/entity.resource.js | 6 +++--- src/Umbraco.Web/Editors/EntityController.cs | 13 ++++--------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js index 1c29a69c2dda..44be85b8fdd9 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/entity.resource.js @@ -127,7 +127,7 @@ function entityResource($q, $http, umbRequestHelper) { 'Failed to retrieve url for id:' + id); }, - getUrlsByUdis: function(ids, culture) { + getUrlsByUdis: function(udis, culture) { var query = "culture=" + (culture || ""); return umbRequestHelper.resourcePromise( @@ -137,9 +137,9 @@ function entityResource($q, $http, umbRequestHelper) { "GetUrlsByUdis", query), { - ids: ids + udis: udis }), - 'Failed to retrieve url map for ids ' + ids); + 'Failed to retrieve url map for udis ' + udis); }, getUrlByUdi: function (udi, culture) { diff --git a/src/Umbraco.Web/Editors/EntityController.cs b/src/Umbraco.Web/Editors/EntityController.cs index 66173634185b..0b6273e79da8 100644 --- a/src/Umbraco.Web/Editors/EntityController.cs +++ b/src/Umbraco.Web/Editors/EntityController.cs @@ -238,7 +238,7 @@ public HttpResponseMessage GetUrl(Udi udi, string culture = "*") /// /// Get entity URLs by UDIs /// - /// + /// /// A list of UDIs to lookup items by /// /// The culture to fetch the URL for @@ -248,14 +248,9 @@ public HttpResponseMessage GetUrl(Udi udi, string culture = "*") /// [HttpGet] [HttpPost] - public IDictionary GetUrlsByUdis([FromJsonPath] Udi[] ids, string culture = null) + public IDictionary GetUrlsByUdis([FromJsonPath] Udi[] udis, string culture = null) { - if (ids == null) - { - throw new HttpResponseException(HttpStatusCode.NotFound); - } - - if (ids.Length == 0) + if (udis == null || udis.Length == 0) { return new Dictionary(); } @@ -277,7 +272,7 @@ string MediaOrDocumentUrl(Udi udi) }; } - return ids + return udis .Select(udi => new { Udi = udi, Url = MediaOrDocumentUrl(udi) From f6f5723b7be0639cb4ff3ef0961db4f7d716725d Mon Sep 17 00:00:00 2001 From: Paul Johnson Date: Tue, 12 Oct 2021 09:18:42 +0100 Subject: [PATCH 05/14] Resolve incorrect ContentSavedState for failed publish Closes #11290 (for v8) --- .../Implement/DocumentRepository.cs | 14 +++++ .../Services/ContentServiceTests.cs | 63 ++++++++++++++++++- 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index f5d993070c04..5716fbe129c4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -520,6 +520,7 @@ protected override void PersistNewItem(IContent entity) protected override void PersistUpdatedItem(IContent entity) { var isEntityDirty = entity.IsDirty(); + var editedSnapshot = entity.Edited; // check if we need to make any database changes at all if ((entity.PublishedState == PublishedState.Published || entity.PublishedState == PublishedState.Unpublished) @@ -621,6 +622,19 @@ protected override void PersistUpdatedItem(IContent entity) if (!publishing && entity.PublishName != entity.Name) edited = true; + // To establish the new value of "edited" we compare all properties publishedValue to editedValue and look + // for differences. + // + // If we SaveAndPublish but the publish fails (e.g. already scheduled for release) + // we have lost the publishedValue on IContent (in memory vs database) so we cannot correctly make that comparison. + // + // This is a slight change to behaviour, historically a publish, followed by change & save, followed by undo change & save + // would change edited back to false. + if (!publishing && editedSnapshot) + { + edited = true; + } + if (entity.ContentType.VariesByCulture()) { // bump dates to align cultures to version diff --git a/src/Umbraco.Tests/Services/ContentServiceTests.cs b/src/Umbraco.Tests/Services/ContentServiceTests.cs index d9a90b74d808..64e88b94131d 100644 --- a/src/Umbraco.Tests/Services/ContentServiceTests.cs +++ b/src/Umbraco.Tests/Services/ContentServiceTests.cs @@ -1375,6 +1375,65 @@ public void Cannot_Publish_Content_Awaiting_Release() Assert.AreEqual(PublishResultType.FailedPublishAwaitingRelease, published.Result); } + // V9 - Tests.Integration + [Test] + public void Failed_Publish_Should_Not_Update_Edited_State_When_Edited_True() + { + const int rootNodeId = NodeDto.NodeIdSeed + 2; + + // Arrange + var contentService = ServiceContext.ContentService; + var content = contentService.GetById(rootNodeId); + contentService.SaveAndPublish(content); + + content.Properties[0].SetValue("Foo", culture: string.Empty); + content.ContentSchedule.Add(DateTime.Now.AddHours(2), null); + contentService.Save(content); + + // Act + var result = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); + + // Assert + Assert.Multiple(() => + { + Assert.IsFalse(result.Success); + Assert.IsTrue(result.Content.Published); + Assert.AreEqual(PublishResultType.FailedPublishAwaitingRelease, result.Result); + + // We changed property data + Assert.IsTrue(result.Content.Edited, "result.Content.Edited"); + }); + } + + // V9 - Tests.Integration + [Test] + public void Failed_Publish_Should_Not_Update_Edited_State_When_Edited_False() + { + const int rootNodeId = NodeDto.NodeIdSeed + 2; + + // Arrange + var contentService = ServiceContext.ContentService; + var content = contentService.GetById(rootNodeId); + contentService.SaveAndPublish(content); + + content.ContentSchedule.Add(DateTime.Now.AddHours(2), null); + contentService.Save(content); + + // Act + var result = contentService.SaveAndPublish(content, userId: Constants.Security.SuperUserId); + + // Assert + Assert.Multiple(() => + { + Assert.IsFalse(result.Success); + Assert.IsTrue(result.Content.Published); + Assert.AreEqual(PublishResultType.FailedPublishAwaitingRelease, result.Result); + + // We didn't change any property data + Assert.IsFalse(result.Content.Edited, "result.Content.Edited"); + }); + } + [Test] public void Cannot_Publish_Culture_Awaiting_Release() { @@ -2176,7 +2235,7 @@ public void Can_Rollback_Version_On_Content() contentService.Save(rollback2); Assert.IsTrue(rollback2.Published); - Assert.IsFalse(rollback2.Edited); // all changes cleared! + Assert.IsTrue(rollback2.Edited); // Still edited, change of behaviour Assert.AreEqual("Jane Doe", rollback2.GetValue("author")); Assert.AreEqual("Text Page 2 ReReUpdated", rollback2.Name); @@ -2195,7 +2254,7 @@ public void Can_Rollback_Version_On_Content() content.CopyFrom(rollto); content.Name = rollto.PublishName; // must do it explicitely AND must pick the publish one! contentService.Save(content); - Assert.IsFalse(content.Edited); + Assert.IsTrue(content.Edited); // Still edited, change of behaviour Assert.AreEqual("Text Page 2 ReReUpdated", content.Name); Assert.AreEqual("Jane Doe", content.GetValue("author")); } From 3d53c72205f33c93bd0109eb5879aeb347fd6386 Mon Sep 17 00:00:00 2001 From: Mads Rasmussen Date: Tue, 12 Oct 2021 11:52:01 +0200 Subject: [PATCH 06/14] add modelValue validation for server to correctly update validation errors --- .../mediapicker3/umb-media-picker3-property-editor.html | 4 ++++ .../mediapicker3/umbMediaPicker3PropertyEditor.component.js | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html index daf9566e2d4d..f40c6bc437e5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umb-media-picker3-property-editor.html @@ -69,4 +69,8 @@ + + + + diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js index 02b9f0b92813..ff0b0030e6b5 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/mediapicker3/umbMediaPicker3PropertyEditor.component.js @@ -137,6 +137,10 @@ if (vm.propertyForm) { vm.propertyForm.$setDirty(); } + + if (vm.modelValueForm) { + vm.modelValueForm.modelValue.$setDirty(); + } } function addMediaAt(createIndex, $event) { From d02440d4111b59e560cfe49eae7be2bf0366bf7b Mon Sep 17 00:00:00 2001 From: patrickdemooij9 Date: Tue, 12 Oct 2021 14:47:03 +0200 Subject: [PATCH 07/14] 11048: Bugfix for groups and properties that get replaced (#11257) (cherry picked from commit 1605dc10bd91caa46d4bec1946a392f1d47c993d) --- .../src/common/services/contenttypehelper.service.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 cceb67689fed..ba08739676b6 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 @@ -96,7 +96,7 @@ function contentTypeHelper(contentTypeResource, dataTypeResource, $filter, $inje group.convertingToTab = true; group.type = this.TYPE_TAB; - + const newAlias = this.generateLocalAlias(group.name); // when checking for alias uniqueness we need to exclude the current group or the alias would get a + 1 const otherGroups = [...groups].filter(groupCopy => !groupCopy.convertingToTab); @@ -445,6 +445,12 @@ function contentTypeHelper(contentTypeResource, dataTypeResource, $filter, $inje // The saved content type might have updated values (eg. new IDs/keys), so make sure the view model is updated contentType.ModelState = savedContentType.ModelState; contentType.id = savedContentType.id; + + // Prevent rebinding if there was an error: https://github.com/umbraco/Umbraco-CMS/pull/11257 + if (savedContentType.ModelState) { + return; + } + contentType.groups.forEach(function (group) { if (!group.alias) return; From 3f3262eb0115a2b92fad8d9b8d1bc78cc24d399e Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Thu, 14 Oct 2021 09:54:14 +0200 Subject: [PATCH 08/14] Adding property group aliases to ex.message --- .../Exceptions/InvalidCompositionException.cs | 51 ++++++++++++++----- 1 file changed, 39 insertions(+), 12 deletions(-) diff --git a/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs b/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs index 95cd27b2cd45..f1bb4d2dd59c 100644 --- a/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs +++ b/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs @@ -85,18 +85,45 @@ public InvalidCompositionException(string contentTypeAlias, string addedComposit private static string FormatMessage(string contentTypeAlias, string addedCompositionAlias, string[] propertyTypeAliases, string[] propertyGroupAliases) { - // TODO Add property group aliases to message - return addedCompositionAlias.IsNullOrWhiteSpace() - ? string.Format( - "ContentType with alias '{0}' has an invalid composition " + - "and there was a conflict on the following PropertyTypes: '{1}'. " + - "PropertyTypes must have a unique alias across all Compositions in order to compose a valid ContentType Composition.", - contentTypeAlias, string.Join(", ", propertyTypeAliases)) - : string.Format( - "ContentType with alias '{0}' was added as a Composition to ContentType with alias '{1}', " + - "but there was a conflict on the following PropertyTypes: '{2}'. " + - "PropertyTypes must have a unique alias across all Compositions in order to compose a valid ContentType Composition.", - addedCompositionAlias, contentTypeAlias, string.Join(", ", propertyTypeAliases)); + // list both propertyTypeAliases and propertyGroupAliases + var customMsg = string.Format("PropertyTypes: '{0}' and PropertyGroups: '{1}'. PropertyTypes and PropertyGroups", + string.Join(", ", propertyTypeAliases), string.Join(", ", propertyGroupAliases)); + + var endMsg = " must have a unique alias across all Compositions in order to compose a valid ContentType Composition."; + + // list only propertyGroupAliases when there are no property type aliases + if (propertyTypeAliases.Length == 0) + { + customMsg = string.Format("PropertyGroups: '{0}'. PropertyGroups", + string.Join(", ", propertyGroupAliases)); + } + else + { + // list only propertyTypeAliases when there are no property group aliases + if (propertyGroupAliases.Length == 0) + { + customMsg = string.Format("PropertyTypes: '{0}'. PropertyTypes", + string.Join(", ", propertyTypeAliases)); + } + } + + string message; + if (addedCompositionAlias.IsNullOrWhiteSpace()) + { + var startMsg = "ContentType with alias '{0}' has an invalid composition " + + "and there was a conflict on the following "; + + message = string.Format(startMsg + customMsg + endMsg, contentTypeAlias); + } + else + { + var startMsg = "ContentType with alias '{0}' was added as a Composition to ContentType with alias '{1}', " + + "but there was a conflict on the following "; + + message = string.Format(startMsg + customMsg + endMsg, addedCompositionAlias, contentTypeAlias); + } + + return message; } /// From ccd9013d30accdb1ea69d3f0eb0b332d03bc6141 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Thu, 14 Oct 2021 09:57:13 +0200 Subject: [PATCH 09/14] Adding invalid prop group aliases as ModelState errors, so we don't introduce breaking changes --- .../Editors/ContentTypeControllerBase.cs | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index 6ac936961f10..e4a5b2e41751 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -454,10 +454,12 @@ private HttpResponseException CreateCompositionValidationExceptionIfInvalid(contentTypeSave, invalidPropertyAliases); + // if it's not successful then we need to return some model state for the property type and property group + // aliases that are duplicated + var invalidPropertyTypeAliases = validateAttempt.Result.Distinct(); + var invalidPropertyGroupAliases = (validateAttempt.Exception as InvalidCompositionException)?.PropertyGroupAliases ?? Array.Empty(); + + AddCompositionValidationErrors(contentTypeSave, invalidPropertyTypeAliases, invalidPropertyGroupAliases); var display = Mapper.Map(composition); //map the 'save' data on top @@ -472,22 +474,32 @@ private HttpResponseException CreateCompositionValidationExceptionIfInvalid /// - /// + /// + /// /// - private void AddCompositionValidationErrors(TContentTypeSave contentTypeSave, IEnumerable invalidPropertyAliases) + private void AddCompositionValidationErrors(TContentTypeSave contentTypeSave, IEnumerable invalidPropertyTypeAliases, IEnumerable invalidPropertyGroupAliases) where TContentTypeSave : ContentTypeSave where TPropertyType : PropertyTypeBasic { - foreach (var propertyAlias in invalidPropertyAliases) + foreach (var propertyTypeAlias in invalidPropertyTypeAliases) { - // Find the property relating to these - var property = contentTypeSave.Groups.SelectMany(x => x.Properties).Single(x => x.Alias == propertyAlias); + // Find the property type relating to these + var property = contentTypeSave.Groups.SelectMany(x => x.Properties).Single(x => x.Alias == propertyTypeAlias); var group = contentTypeSave.Groups.Single(x => x.Properties.Contains(property)); var propertyIndex = group.Properties.IndexOf(property); var groupIndex = contentTypeSave.Groups.IndexOf(group); var key = $"Groups[{groupIndex}].Properties[{propertyIndex}].Alias"; - ModelState.AddModelError(key, "Duplicate property aliases not allowed between compositions"); + ModelState.AddModelError(key, "Duplicate property aliases are not allowed between compositions"); + } + + foreach (var propertyGroupAlias in invalidPropertyGroupAliases) + { + // Find the property group relating to these + var group = contentTypeSave.Groups.Single(x => x.Alias == propertyGroupAlias); + var groupIndex = contentTypeSave.Groups.IndexOf(group); + var key = $"Groups[{groupIndex}].Name"; + ModelState.AddModelError(key, "Duplicate property group aliases are not allowed between compositions"); } } @@ -519,7 +531,7 @@ private HttpResponseException CreateInvalidCompositionResponseException(contentTypeSave, invalidCompositionException.PropertyTypeAliases); + AddCompositionValidationErrors(contentTypeSave, invalidCompositionException.PropertyTypeAliases, invalidCompositionException.PropertyGroupAliases); return CreateModelStateValidationException(ctId, contentTypeSave, ct); } return null; From b8ecc17140f93e9b528445609a5ea185b87e30ca Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Thu, 14 Oct 2021 13:41:56 +0200 Subject: [PATCH 10/14] Pointing the actual reason for invalidating composition --- src/Umbraco.Web/Editors/ContentTypeControllerBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index e4a5b2e41751..60e10313ca05 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -499,7 +499,7 @@ private void AddCompositionValidationErrors(TCo var group = contentTypeSave.Groups.Single(x => x.Alias == propertyGroupAlias); var groupIndex = contentTypeSave.Groups.IndexOf(group); var key = $"Groups[{groupIndex}].Name"; - ModelState.AddModelError(key, "Duplicate property group aliases are not allowed between compositions"); + ModelState.AddModelError(key, "Same alias for tab and group is not allowed between compositions"); } } From 1b06db8d5cc18c7813afbf0d1505d7d4bb1982e3 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 14 Oct 2021 14:47:57 +0200 Subject: [PATCH 11/14] Validate all content type dependencies and throw a single InvalidCompositionException --- ...entTypeServiceBaseOfTRepositoryTItemTService.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs index 11142ad96eca..385bfec92325 100644 --- a/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentTypeServiceBaseOfTRepositoryTItemTService.cs @@ -97,18 +97,22 @@ protected void ValidateLocked(TItem compositionContentType) stack.Push(c); } + var duplicatePropertyTypeAliases = new List(); + var invalidPropertyGroupAliases = new List(); + foreach (var dependency in dependencies) { if (dependency.Id == compositionContentType.Id) continue; var contentTypeDependency = allContentTypes.FirstOrDefault(x => x.Alias.Equals(dependency.Alias, StringComparison.InvariantCultureIgnoreCase)); if (contentTypeDependency == null) continue; - var duplicatePropertyTypeAliases = contentTypeDependency.PropertyTypes.Select(x => x.Alias).Intersect(propertyTypeAliases, StringComparer.InvariantCultureIgnoreCase).ToArray(); - var invalidPropertyGroupAliases = contentTypeDependency.PropertyGroups.Where(x => propertyGroupAliases.TryGetValue(x.Alias, out var type) && type != x.Type).Select(x => x.Alias).ToArray(); - - if (duplicatePropertyTypeAliases.Length == 0 && invalidPropertyGroupAliases.Length == 0) continue; + duplicatePropertyTypeAliases.AddRange(contentTypeDependency.PropertyTypes.Select(x => x.Alias).Intersect(propertyTypeAliases, StringComparer.InvariantCultureIgnoreCase)); + invalidPropertyGroupAliases.AddRange(contentTypeDependency.PropertyGroups.Where(x => propertyGroupAliases.TryGetValue(x.Alias, out var type) && type != x.Type).Select(x => x.Alias)); + } - throw new InvalidCompositionException(compositionContentType.Alias, null, duplicatePropertyTypeAliases, invalidPropertyGroupAliases); + if (duplicatePropertyTypeAliases.Count > 0 || invalidPropertyGroupAliases.Count > 0) + { + throw new InvalidCompositionException(compositionContentType.Alias, null, duplicatePropertyTypeAliases.Distinct().ToArray(), invalidPropertyGroupAliases.Distinct().ToArray()); } } From 6666f6aab7c3f62eefb52609a2889148c3517c53 Mon Sep 17 00:00:00 2001 From: Elitsa Marinovska Date: Thu, 14 Oct 2021 14:57:52 +0200 Subject: [PATCH 12/14] Rename based on review comments --- src/Umbraco.Web/Editors/ContentTypeControllerBase.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index 60e10313ca05..b38e2b619f46 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -456,10 +456,10 @@ private HttpResponseException CreateCompositionValidationExceptionIfInvalid(); - AddCompositionValidationErrors(contentTypeSave, invalidPropertyTypeAliases, invalidPropertyGroupAliases); + AddCompositionValidationErrors(contentTypeSave, duplicatePropertyTypeAliases, invalidPropertyGroupAliases); var display = Mapper.Map(composition); //map the 'save' data on top @@ -474,14 +474,14 @@ private HttpResponseException CreateCompositionValidationExceptionIfInvalid /// - /// + /// /// /// - private void AddCompositionValidationErrors(TContentTypeSave contentTypeSave, IEnumerable invalidPropertyTypeAliases, IEnumerable invalidPropertyGroupAliases) + private void AddCompositionValidationErrors(TContentTypeSave contentTypeSave, IEnumerable duplicatePropertyTypeAliases, IEnumerable invalidPropertyGroupAliases) where TContentTypeSave : ContentTypeSave where TPropertyType : PropertyTypeBasic { - foreach (var propertyTypeAlias in invalidPropertyTypeAliases) + foreach (var propertyTypeAlias in duplicatePropertyTypeAliases) { // Find the property type relating to these var property = contentTypeSave.Groups.SelectMany(x => x.Properties).Single(x => x.Alias == propertyTypeAlias); From 705a3ed4e86cc2df630eab239fc93bea9ad613f3 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 14 Oct 2021 15:06:28 +0200 Subject: [PATCH 13/14] Update composition validation error messages --- src/Umbraco.Web/Editors/ContentTypeControllerBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs index 60e10313ca05..232ca922346c 100644 --- a/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs +++ b/src/Umbraco.Web/Editors/ContentTypeControllerBase.cs @@ -490,7 +490,7 @@ private void AddCompositionValidationErrors(TCo var groupIndex = contentTypeSave.Groups.IndexOf(group); var key = $"Groups[{groupIndex}].Properties[{propertyIndex}].Alias"; - ModelState.AddModelError(key, "Duplicate property aliases are not allowed between compositions"); + ModelState.AddModelError(key, "Duplicate property aliases aren't allowed between compositions"); } foreach (var propertyGroupAlias in invalidPropertyGroupAliases) @@ -499,7 +499,7 @@ private void AddCompositionValidationErrors(TCo var group = contentTypeSave.Groups.Single(x => x.Alias == propertyGroupAlias); var groupIndex = contentTypeSave.Groups.IndexOf(group); var key = $"Groups[{groupIndex}].Name"; - ModelState.AddModelError(key, "Same alias for tab and group is not allowed between compositions"); + ModelState.AddModelError(key, "Different group types aren't allowed between compositions"); } } From b73ab255e47fe1a821a2b214c5fe9efeaca203c1 Mon Sep 17 00:00:00 2001 From: Ronald Barendse Date: Thu, 14 Oct 2021 15:50:29 +0200 Subject: [PATCH 14/14] Update InvalidCompositionException message --- .../Exceptions/InvalidCompositionException.cs | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs b/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs index f1bb4d2dd59c..5d2beaa38793 100644 --- a/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs +++ b/src/Umbraco.Core/Exceptions/InvalidCompositionException.cs @@ -1,5 +1,6 @@ using System; using System.Runtime.Serialization; +using System.Text; namespace Umbraco.Core.Exceptions { @@ -85,45 +86,28 @@ public InvalidCompositionException(string contentTypeAlias, string addedComposit private static string FormatMessage(string contentTypeAlias, string addedCompositionAlias, string[] propertyTypeAliases, string[] propertyGroupAliases) { - // list both propertyTypeAliases and propertyGroupAliases - var customMsg = string.Format("PropertyTypes: '{0}' and PropertyGroups: '{1}'. PropertyTypes and PropertyGroups", - string.Join(", ", propertyTypeAliases), string.Join(", ", propertyGroupAliases)); + var sb = new StringBuilder(); - var endMsg = " must have a unique alias across all Compositions in order to compose a valid ContentType Composition."; - - // list only propertyGroupAliases when there are no property type aliases - if (propertyTypeAliases.Length == 0) + if (addedCompositionAlias.IsNullOrWhiteSpace()) { - customMsg = string.Format("PropertyGroups: '{0}'. PropertyGroups", - string.Join(", ", propertyGroupAliases)); + sb.AppendFormat("Content type with alias '{0}' has an invalid composition.", contentTypeAlias); } else { - // list only propertyTypeAliases when there are no property group aliases - if (propertyGroupAliases.Length == 0) - { - customMsg = string.Format("PropertyTypes: '{0}'. PropertyTypes", - string.Join(", ", propertyTypeAliases)); - } + sb.AppendFormat("Content type with alias '{0}' was added as a composition to content type with alias '{1}', but there was a conflict.", addedCompositionAlias, contentTypeAlias); } - string message; - if (addedCompositionAlias.IsNullOrWhiteSpace()) + if (propertyTypeAliases.Length > 0) { - var startMsg = "ContentType with alias '{0}' has an invalid composition " + - "and there was a conflict on the following "; - - message = string.Format(startMsg + customMsg + endMsg, contentTypeAlias); + sb.AppendFormat(" Property types must have a unique alias across all compositions, these aliases are duplicate: {0}.", string.Join(", ", propertyTypeAliases)); } - else - { - var startMsg = "ContentType with alias '{0}' was added as a Composition to ContentType with alias '{1}', " + - "but there was a conflict on the following "; - message = string.Format(startMsg + customMsg + endMsg, addedCompositionAlias, contentTypeAlias); + if (propertyGroupAliases.Length > 0) + { + sb.AppendFormat(" Property groups with the same alias must also have the same type across all compositions, these aliases have different types: {0}.", string.Join(", ", propertyGroupAliases)); } - return message; + return sb.ToString(); } ///