From be4ea93d1277b53146cd9c7ae4b8c68ab0754806 Mon Sep 17 00:00:00 2001 From: Shannon Date: Tue, 1 May 2018 18:17:07 +1000 Subject: [PATCH 1/4] U4-11289 Tracking the currently selected language in the main tree --- .../Implement/EntityRepository.cs | 13 +++-- .../application/umbsections.directive.js | 9 ++-- .../components/content/edit.controller.js | 12 ++--- .../editor/umbeditorheader.directive.js | 2 +- .../src/common/resources/content.resource.js | 6 +-- .../services/contenteditinghelper.service.js | 8 +-- .../src/common/services/navigation.service.js | 19 +++++++ .../services/umbdataformatter.service.js | 4 +- .../src/controllers/navigation.controller.js | 52 +++++++++++++------ src/Umbraco.Web.UI.Client/src/routes.js | 1 - .../confirmroutechange.controller.js | 4 +- .../common/overlays/user/user.controller.js | 2 +- .../views/content/content.edit.controller.js | 2 +- .../src/views/content/edit.html | 2 +- src/Umbraco.Web/Editors/ContentController.cs | 52 ++++++++----------- .../Models/ContentEditing/ContentItemSave.cs | 4 +- .../ContentEditing/ContentVariationPublish.cs | 4 +- .../Trees/ContentTreeController.cs | 2 +- .../Trees/ContentTreeControllerBase.cs | 18 +++---- .../WebApi/Binders/ContentItemBinder.cs | 6 +-- 20 files changed, 130 insertions(+), 92 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs index 09711cf9e1..0dd7a25798 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs @@ -24,9 +24,12 @@ namespace Umbraco.Core.Persistence.Repositories.Implement internal class EntityRepository : IEntityRepository { private readonly IScopeAccessor _scopeAccessor; - public EntityRepository(IScopeAccessor scopeAccessor) + private readonly ILanguageRepository _langRepository; + + public EntityRepository(IScopeAccessor scopeAccessor, ILanguageRepository langRepository) { _scopeAccessor = scopeAccessor; + _langRepository = langRepository; } protected IUmbracoDatabase Database => _scopeAccessor.AmbientScope.Database; @@ -889,17 +892,19 @@ namespace Umbraco.Core.Persistence.Repositories.Implement /// /// /// - private static EntitySlim BuildDocumentEntity(ContentEntityDto dto) + private EntitySlim BuildDocumentEntity(ContentEntityDto dto) { // EntitySlim does not track changes var entity = new DocumentEntitySlim(); BuildDocumentEntity(entity, dto); - var variantInfo = new Dictionary(); + var variantInfo = new Dictionary(StringComparer.InvariantCultureIgnoreCase); if (dto.VariationInfo != null) { foreach (var info in dto.VariationInfo) { - variantInfo[info.LanguageId] = info.Name; + var isoCode = _langRepository.GetIsoCodeById(info.LanguageId); + if (isoCode != null) + variantInfo[isoCode] = info.Name; } entity.AdditionalData["CultureNames"] = variantInfo; } diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbsections.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbsections.directive.js index 0896f986e6..23664ed842 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbsections.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/application/umbsections.directive.js @@ -38,7 +38,7 @@ function sectionsDirective($timeout, $window, navigationService, treeService, se calculateWidth(); }); } - + function calculateWidth(){ $timeout(function(){ //total width minus room for avatar, search, and help icon @@ -119,9 +119,10 @@ function sectionsDirective($timeout, $window, navigationService, treeService, se } else { var lastAccessed = historyService.getLastAccessedItemForSection(section.alias); - var path = lastAccessed != null ? lastAccessed.link : section.alias; - $location.path(path).search(''); - } + var path = lastAccessed != null ? lastAccessed.link : section.alias; + $location.path(path); + } + navigationService.clearSearch(); }; diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js index ffac0bf965..63bdb9c92e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/edit.controller.js @@ -20,7 +20,7 @@ $scope.page.listViewPath = null; $scope.page.isNew = $scope.isNew ? true : false; $scope.page.buttonGroupState = "init"; - $scope.page.languageId = $scope.languageId; + $scope.page.culture = $scope.culture; $scope.allowOpen = true; // add all editors to an editors array to support split view @@ -122,14 +122,14 @@ /** * This does the content loading and initializes everything, called on load and changing variants - * @param {any} languageId + * @param {any} culture */ - function getNode(languageId) { + function getNode(culture) { $scope.page.loading = true; //we are editing so get the content item from the server - $scope.getMethod()($scope.contentId, languageId) + $scope.getMethod()($scope.contentId, culture) .then(function (data) { $scope.content = data; @@ -258,7 +258,7 @@ else { //Browse content nodes based on the selected tree language variant - $scope.page.languageId ? getNode($scope.page.languageId) : getNode(); + $scope.page.culture ? getNode($scope.page.culture) : getNode(); } @@ -527,7 +527,7 @@ saveMethod: "&", getMethod: "&", getScaffoldMethod: "&?", - languageId: "=?" + culture: "=?" } }; diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js index 6a15b0baba..9b0e0d810b 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js @@ -253,7 +253,7 @@ Use this directive to construct a header inside the main editor window. scope.selectVariant = function (event, variant) { scope.vm.dropdownOpen = false; - $location.search({ languageId: variant.language.id }); + $location.search("cculture", variant.language.culture); }; scope.openIconPicker = function() { diff --git a/src/Umbraco.Web.UI.Client/src/common/resources/content.resource.js b/src/Umbraco.Web.UI.Client/src/common/resources/content.resource.js index 7d560d7b43..1f2a611fe5 100644 --- a/src/Umbraco.Web.UI.Client/src/common/resources/content.resource.js +++ b/src/Umbraco.Web.UI.Client/src/common/resources/content.resource.js @@ -314,17 +314,17 @@ function contentResource($q, $http, umbDataFormatter, umbRequestHelper) { * * * @param {Int} id id of content item to return - * @param {Int} languageId optional ID of the language to retrieve the item in + * @param {Int} culture optional culture to retrieve the item in * @returns {Promise} resourcePromise object containing the content item. * */ - getById: function (id, languageId) { + getById: function (id, culture) { return umbRequestHelper.resourcePromise( $http.get( umbRequestHelper.getApiUrl( "contentApiBaseUrl", "GetById", - { id: id, languageId: languageId })), + { id: id, culture: culture })), 'Failed to retrieve data for content id ' + id); }, diff --git a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js index 36a184cd95..e8e10c5d63 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/contenteditinghelper.service.js @@ -5,7 +5,7 @@ * @description A helper service for most editors, some methods are specific to content/media/member model types but most are used by * all editors to share logic and reduce the amount of replicated code among editors. **/ -function contentEditingHelper(fileManager, $q, $location, $routeParams, notificationsService, localizationService, serverValidationManager, dialogService, formHelper, appState) { +function contentEditingHelper(fileManager, $q, $location, $routeParams, notificationsService, navigationService, localizationService, serverValidationManager, dialogService, formHelper, appState) { function isValidIdentifier(id){ //empty id <= 0 @@ -596,7 +596,7 @@ function contentEditingHelper(fileManager, $q, $location, $routeParams, notifica // /belle/#/content/edit/9876 (where 9876 is the new id) //clear the query strings - $location.search(""); + navigationService.clearSearch(); //change to new path $location.path("/" + $routeParams.section + "/" + $routeParams.tree + "/" + $routeParams.method + "/" + id); @@ -617,9 +617,9 @@ function contentEditingHelper(fileManager, $q, $location, $routeParams, notifica * For some editors like scripts or entites that have names as ids, these names can change and we need to redirect * to their new paths, this is helper method to do that. */ - redirectToRenamedContent: function (id) { + redirectToRenamedContent: function (id) { //clear the query strings - $location.search(""); + navigationService.clearSearch(); //change to new path $location.path("/" + $routeParams.section + "/" + $routeParams.tree + "/" + $routeParams.method + "/" + id); //don't add a browser history for this diff --git a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js index 257d36af31..9a19304288 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js @@ -102,6 +102,25 @@ function navigationService($rootScope, $routeParams, $log, $location, $q, $timeo }, + /** + * @ngdoc method + * @name umbraco.services.navigationService#clearSearch + * @methodOf umbraco.services.navigationService + * + * @description + * utility to clear the querystring/search params while maintaining a known list of parameters that should be maintained throughout the app + */ + clearSearch: function () { + var retainKeys = ["mculture"]; + var currentSearch = $location.search(); + $location.search(''); + _.each(retainKeys, function (k) { + if (currentSearch[k]) { + $location.search(k, currentSearch[k]); + } + }); + }, + /** * @ngdoc method * @name umbraco.services.navigationService#load 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 5ef6eae293..1888ff623d 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 @@ -333,11 +333,11 @@ function (d) { //set the selected variant if this is current if (d.current === true) { - saveModel.languageId = d.language.id; + saveModel.culture = d.language.culture; } if (d.publish === true) { saveModel.publishVariations.push({ - languageId: d.language.id, + culture: d.language.culture, segment: d.segment }); } diff --git a/src/Umbraco.Web.UI.Client/src/controllers/navigation.controller.js b/src/Umbraco.Web.UI.Client/src/controllers/navigation.controller.js index b1810ca683..00458aa6b4 100644 --- a/src/Umbraco.Web.UI.Client/src/controllers/navigation.controller.js +++ b/src/Umbraco.Web.UI.Client/src/controllers/navigation.controller.js @@ -96,10 +96,12 @@ function NavigationController($scope, $rootScope, $location, $log, $q, $routePar appState.setMenuState("currentNode", args.node); //not legacy, lets just set the route value and clear the query string if there is one. - $location.path(n.routePath).search(""); + $location.path(n.routePath); + navigationService.clearSearch(); } else if (n.section) { - $location.path(n.section).search(""); + $location.path(n.section); + navigationService.clearSearch(); } navigationService.hideNavigation(); @@ -236,24 +238,37 @@ function NavigationController($scope, $rootScope, $location, $log, $q, $routePar languageResource.getAll().then(function(languages) { $scope.languages = languages; - // make the default language selected - $scope.languages.forEach(function (language) { - if (language.isDefault) { - $scope.selectedLanguage = language; + if ($scope.languages.length > 1) { + var defaultLang = _.find($scope.languages, function (l) { + return l.isDefault; + }); + if (defaultLang) { + //set the route param + $location.search("mculture", defaultLang.culture); } - }); - }); + } + init(); + }); })); - /** - * Updates the tree's query parameters - */ - function initTree() { + function init() { + //select the current language if set in the query string + var mainCulture = $location.search().mculture; + if (mainCulture && $scope.languages && $scope.languages.length > 1) { + var found = _.find($scope.languages, function (l) { + return l.culture === mainCulture; + }); + if (found) { + //set the route param + $scope.selectedLanguage = found; + } + } + //create the custom query string param for this tree var queryParams = {}; - if ($scope.selectedLanguage && $scope.selectedLanguage.id) { - queryParams["languageId"] = $scope.selectedLanguage.id; + if ($scope.selectedLanguage && $scope.selectedLanguage.culture) { + queryParams["culture"] = $scope.selectedLanguage.culture; } var queryString = $.param(queryParams); //create the query string from the params object @@ -266,6 +281,7 @@ function NavigationController($scope, $rootScope, $location, $log, $q, $routePar } } + function nodeExpandedHandler(args) { //store the reference to the expanded node path if (args.node) { @@ -274,11 +290,15 @@ function NavigationController($scope, $rootScope, $location, $log, $q, $routePar } $scope.selectLanguage = function(language) { - $scope.selectedLanguage = language; + + $location.search("mculture", language.culture); + + //$scope.selectedLanguage = language; + // close the language selector $scope.page.languageSelectorIsOpen = false; - initTree(); //this will reset the tree params and the tree directive will pick up the changes in a $watch + init(); //re-bind language to the query string and update the tree params //execute after next digest because the internal watch on the customtreeparams needs to be bound now that we've changed it $timeout(function () { diff --git a/src/Umbraco.Web.UI.Client/src/routes.js b/src/Umbraco.Web.UI.Client/src/routes.js index 5f660cf212..e4f9852a11 100644 --- a/src/Umbraco.Web.UI.Client/src/routes.js +++ b/src/Umbraco.Web.UI.Client/src/routes.js @@ -104,7 +104,6 @@ app.config(function ($routeProvider) { resolve: doLogout() }) .when('/:section', { - //This allows us to dynamically change the template for this route since you cannot inject services into the templateUrl method. template: "
", //This controller will execute for this route, then we can execute some code in order to set the template Url diff --git a/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js index c9f3cc347c..5851d30610 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js @@ -13,7 +13,7 @@ angular.module("umbraco").controller("Umbraco.Notifications.ConfirmRouteChangeCo // when no callback is added run the normal functionality of the discard button not.args.listener(); - $location.search(""); + navigationService.clearSearch(); //we need to break the path up into path and query var parts = not.args.path.split("?"); @@ -33,4 +33,4 @@ angular.module("umbraco").controller("Umbraco.Notifications.ConfirmRouteChangeCo notificationsService.remove(not); }; - }); \ No newline at end of file + }); diff --git a/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js index 700b18b518..e8b1bb0f68 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/overlays/user/user.controller.js @@ -36,7 +36,7 @@ angular.module("umbraco") //perform the path change, if it is successful then the promise will resolve otherwise it will fail $scope.model.close(); - $location.path("/logout"); + $location.path("/logout").search(''); }; $scope.gotoHistory = function (link) { diff --git a/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js index e8a384c2a9..1bd924e9f0 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/content.edit.controller.js @@ -21,7 +21,7 @@ function ContentEditController($scope, $routeParams, contentResource) { $scope.getScaffoldMethod = $routeParams.blueprintId ? scaffoldBlueprint : scaffoldEmpty; $scope.page = $routeParams.page; $scope.isNew = $routeParams.create; - $scope.languageId = $routeParams.languageId; + $scope.culture = $routeParams.cculture; } angular.module("umbraco").controller("Umbraco.Editors.Content.EditController", ContentEditController); diff --git a/src/Umbraco.Web.UI.Client/src/views/content/edit.html b/src/Umbraco.Web.UI.Client/src/views/content/edit.html index 96d80fb151..84537672d4 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/edit.html +++ b/src/Umbraco.Web.UI.Client/src/views/content/edit.html @@ -7,6 +7,6 @@ get-scaffold-method="getScaffoldMethod" tree-alias="content" is-new="isNew" - language-id="languageId"> + culture="culture"> diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index b4483fc09a..efdc6c23ab 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -253,11 +253,11 @@ namespace Umbraco.Web.Editors /// Gets the content json for the content id /// /// - /// + /// /// [OutgoingEditorModelEvent] [EnsureUserPermissionForContent("id")] - public ContentItemDisplay GetById(int id, int? languageId = null) + public ContentItemDisplay GetById(int id, string culture = null) { var foundContent = GetObjectFromRequest(() => Services.ContentService.GetById(id)); if (foundContent == null) @@ -266,7 +266,7 @@ namespace Umbraco.Web.Editors return null;//irrelevant since the above throws } - var content = MapToDisplay(foundContent, GetLanguageCulture(languageId)); + var content = MapToDisplay(foundContent, culture); return content; } @@ -573,12 +573,12 @@ namespace Umbraco.Web.Editors public ContentItemDisplay PostSave([ModelBinder(typeof(ContentItemBinder))] ContentItemSave contentItem) { var contentItemDisplay = PostSaveInternal(contentItem, content => Services.ContentService.Save(contentItem.PersistedContent, Security.CurrentUser.Id)); - //ensure the active language is still selected - if (contentItem.LanguageId.HasValue) + //ensure the active culture is still selected + if (!contentItem.Culture.IsNullOrWhiteSpace()) { foreach (var contentVariation in contentItemDisplay.Variants) { - contentVariation.IsCurrent = contentVariation.Language.Id == contentItem.LanguageId; + contentVariation.IsCurrent = contentVariation.Language.IsoCode.InvariantEquals(contentItem.Culture); } } return contentItemDisplay; @@ -606,7 +606,7 @@ namespace Umbraco.Web.Editors { //ok, so the absolute mandatory data is invalid and it's new, we cannot actually continue! // add the modelstate to the outgoing object and throw a validation message - var forDisplay = MapToDisplay(contentItem.PersistedContent, GetLanguageCulture(contentItem.LanguageId)); + var forDisplay = MapToDisplay(contentItem.PersistedContent, contentItem.Culture); forDisplay.Errors = ModelState.ToErrorDictionary(); throw new HttpResponseException(Request.CreateValidationErrorResponse(forDisplay)); @@ -643,30 +643,30 @@ namespace Umbraco.Web.Editors else { //publish the item and check if it worked, if not we will show a diff msg below - contentItem.PersistedContent.TryPublishValues(GetLanguageCulture(contentItem.LanguageId)); //we are not checking for a return value here because we've already pre-validated the property values + contentItem.PersistedContent.TryPublishValues(contentItem.Culture); //we are not checking for a return value here because we've already pre-validated the property values //check if we are publishing other variants and validate them - var allLangs = Services.LocalizationService.GetAllLanguages().ToDictionary(x => x.Id, x => x); - var variantsToValidate = contentItem.PublishVariations.Where(x => x.LanguageId != contentItem.LanguageId).ToList(); + var allLangs = Services.LocalizationService.GetAllLanguages().ToDictionary(x => x.IsoCode, x => x, StringComparer.InvariantCultureIgnoreCase); + var variantsToValidate = contentItem.PublishVariations.Where(x => !x.Culture.InvariantEquals(contentItem.Culture)).ToList(); foreach (var publishVariation in variantsToValidate) { - if (!contentItem.PersistedContent.TryPublishValues(GetLanguageCulture(publishVariation.LanguageId))) + if (!contentItem.PersistedContent.TryPublishValues(publishVariation.Culture)) { - var errMsg = Services.TextService.Localize("speechBubbles/contentLangValidationError", new[] {allLangs[publishVariation.LanguageId].CultureName}); - ModelState.AddModelError("publish_variant_" + publishVariation.LanguageId + "_", errMsg); + var errMsg = Services.TextService.Localize("speechBubbles/contentLangValidationError", new[] {allLangs[publishVariation.Culture].CultureName}); + ModelState.AddModelError("publish_variant_" + publishVariation.Culture + "_", errMsg); } } //validate any mandatory variants that are not in the list var mandatoryLangs = Mapper.Map, IEnumerable>(allLangs.Values) - .Where(x => variantsToValidate.All(v => v.LanguageId != x.Id)) //don't include variants above - .Where(x => x.Id != contentItem.LanguageId) //don't include the current variant + .Where(x => variantsToValidate.All(v => !v.Culture.InvariantEquals(x.IsoCode))) //don't include variants above + .Where(x => !x.IsoCode.InvariantEquals(contentItem.Culture)) //don't include the current variant .Where(x => x.Mandatory); foreach (var lang in mandatoryLangs) { - if (contentItem.PersistedContent.Validate(GetLanguageCulture(lang.Id)).Length > 0) + if (contentItem.PersistedContent.Validate(lang.IsoCode).Length > 0) { - var errMsg = Services.TextService.Localize("speechBubbles/contentReqLangValidationError", new[]{allLangs[lang.Id].CultureName}); + var errMsg = Services.TextService.Localize("speechBubbles/contentReqLangValidationError", new[]{allLangs[lang.IsoCode].CultureName}); ModelState.AddModelError("publish_variant_" + lang.Id + "_", errMsg); } } @@ -676,7 +676,7 @@ namespace Umbraco.Web.Editors } //get the updated model - var display = MapToDisplay(contentItem.PersistedContent, GetLanguageCulture(contentItem.LanguageId)); + var display = MapToDisplay(contentItem.PersistedContent, contentItem.Culture); //lasty, if it is not valid, add the modelstate to the outgoing object and throw a 403 HandleInvalidModelState(display); @@ -954,10 +954,9 @@ namespace Umbraco.Web.Editors if (contentItem.Name.IsNullOrWhiteSpace() == false) { //set the name according to the culture settings - if (contentItem.LanguageId.HasValue && contentItem.PersistedContent.ContentType.Variations.HasFlag(ContentVariation.CultureNeutral)) + if (!contentItem.Culture.IsNullOrWhiteSpace() && contentItem.PersistedContent.ContentType.Variations.HasFlag(ContentVariation.CultureNeutral)) { - var culture = Services.LocalizationService.GetLanguageById(contentItem.LanguageId.Value).IsoCode; - contentItem.PersistedContent.SetName(culture, contentItem.Name); + contentItem.PersistedContent.SetName(contentItem.Culture, contentItem.Name); } else { @@ -990,8 +989,8 @@ namespace Umbraco.Web.Editors base.MapPropertyValues( contentItem, - (save, property) => property.GetValue(GetLanguageCulture(save.LanguageId)), //get prop val - (save, property, v) => property.SetValue(v, GetLanguageCulture(save.LanguageId))); //set prop val + (save, property) => property.GetValue(save.Culture), //get prop val + (save, property, v) => property.SetValue(v, save.Culture)); //set prop val } /// @@ -1199,11 +1198,6 @@ namespace Umbraco.Web.Editors return display; } - - private string GetLanguageCulture(int? languageId) - { - if (languageId == null) return null; - return Core.Composing.Current.Services.LocalizationService.GetLanguageById(languageId.Value).IsoCode; // fixme optimize! - } + } } diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs b/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs index ca410100ec..01d1a50460 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentItemSave.cs @@ -15,8 +15,8 @@ namespace Umbraco.Web.Models.ContentEditing /// /// The language Id for the content variation being saved /// - [DataMember(Name = "languageId")] - public int? LanguageId { get; set; } //TODO: Change this to ContentVariationPublish, but this will all change anyways when we can edit all variants at once + [DataMember(Name = "culture")] + public string Culture { get; set; } //TODO: Change this to ContentVariationPublish, but this will all change anyways when we can edit all variants at once /// /// The template alias to save diff --git a/src/Umbraco.Web/Models/ContentEditing/ContentVariationPublish.cs b/src/Umbraco.Web/Models/ContentEditing/ContentVariationPublish.cs index aefc487e7c..71c4672ccb 100644 --- a/src/Umbraco.Web/Models/ContentEditing/ContentVariationPublish.cs +++ b/src/Umbraco.Web/Models/ContentEditing/ContentVariationPublish.cs @@ -8,9 +8,9 @@ namespace Umbraco.Web.Models.ContentEditing /// public class ContentVariationPublish { - [DataMember(Name = "languageId", IsRequired = true)] + [DataMember(Name = "culture", IsRequired = true)] [Required] - public int LanguageId { get; set; } + public string Culture { get; set; } [DataMember(Name = "segment")] public string Segment { get; set; } diff --git a/src/Umbraco.Web/Trees/ContentTreeController.cs b/src/Umbraco.Web/Trees/ContentTreeController.cs index 53528484d1..ad190fe8ed 100644 --- a/src/Umbraco.Web/Trees/ContentTreeController.cs +++ b/src/Umbraco.Web/Trees/ContentTreeController.cs @@ -47,7 +47,7 @@ namespace Umbraco.Web.Trees /// protected override TreeNode GetSingleTreeNode(IEntitySlim entity, string parentId, FormDataCollection queryStrings) { - var langId = queryStrings["languageId"].TryConvertTo(); + var langId = queryStrings["culture"].TryConvertTo(); var allowedUserOptions = GetAllowedUserMenuItemsForNode(entity); if (CanUserAccessNode(entity, allowedUserOptions, langId.Success ? langId.Result : null)) diff --git a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs index 4e7b78efe9..0c0308a471 100644 --- a/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs +++ b/src/Umbraco.Web/Trees/ContentTreeControllerBase.cs @@ -149,8 +149,8 @@ namespace Umbraco.Web.Trees // get child entities - if id is root, but user's start nodes do not contain the // root node, this returns the start nodes instead of root's children - var langId = queryStrings["languageId"].TryConvertTo(); - var entities = GetChildEntities(id, langId.Success ? langId.Result : null).ToList(); + var culture = queryStrings["culture"].TryConvertTo(); + var entities = GetChildEntities(id, culture.Success ? culture.Result : null).ToList(); nodes.AddRange(entities.Select(x => GetSingleTreeNodeWithAccessCheck(x, id, queryStrings)).Where(x => x != null)); // if the user does not have access to the root node, what we have is the start nodes, @@ -182,7 +182,7 @@ namespace Umbraco.Web.Trees protected abstract UmbracoObjectTypes UmbracoObjectType { get; } - protected IEnumerable GetChildEntities(string id, int? langId) + protected IEnumerable GetChildEntities(string id, string culture) { // try to parse id as an integer else use GetEntityFromId // which will grok Guids, Udis, etc and let use obtain the id @@ -211,7 +211,7 @@ namespace Umbraco.Web.Trees } //This should really never be null, but we'll error check anyways - var currLangId = langId ?? Services.LocalizationService.GetDefaultLanguageId(); + culture = culture ?? Services.LocalizationService.GetDefaultLanguageIsoCode(); //Try to see if there is a variant name for the current language for the item and set the name accordingly. //If any of this fails, the tree node name will remain the default invariant culture name. @@ -219,14 +219,14 @@ namespace Umbraco.Web.Trees //fixme - what if there is no name found at all ? This could occur if the doc type is variant and the user fills in all language values, then creates a new lang and sets it as the default //fixme - what if the user changes this document type to not allow culture variants after it's already been created with culture variants, this means we'll be displaying the culture variant name when in fact we should be displaying the invariant name... but that would be null - if (currLangId.HasValue) + if (!culture.IsNullOrWhiteSpace()) { foreach (var e in result) { if (e.AdditionalData.TryGetValue("CultureNames", out var cultureNames) - && cultureNames is IDictionary cnd) + && cultureNames is IDictionary cnd) { - if (cnd.TryGetValue(currLangId.Value, out var name)) + if (cnd.TryGetValue(culture, out var name)) { e.Name = name; } @@ -393,9 +393,9 @@ namespace Umbraco.Web.Trees /// A list of MenuItems that the user has permissions to execute on the current document /// By default the user must have Browse permissions to see the node in the Content tree /// - internal bool CanUserAccessNode(IUmbracoEntity doc, IEnumerable allowedUserOptions, int? langId) + internal bool CanUserAccessNode(IUmbracoEntity doc, IEnumerable allowedUserOptions, string culture) { - //TODO: At some stage when we implement permissions on languages we'll need to take care of langId + //TODO: At some stage when we implement permissions on languages we'll need to take care of culture return allowedUserOptions.Select(x => x.Action).OfType().Any(); } diff --git a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs index 525b1fcf2c..8c3b705bb6 100644 --- a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs @@ -27,14 +27,14 @@ namespace Umbraco.Web.WebApi.Binders protected override ContentItemDto MapFromPersisted(ContentItemSave model) { - return MapFromPersisted(model.PersistedContent, model.LanguageId); + return MapFromPersisted(model.PersistedContent, model.Culture); } - internal static ContentItemDto MapFromPersisted(IContent content, int? languageId) + internal static ContentItemDto MapFromPersisted(IContent content, string culture) { return ContextMapper.Map>(content, new Dictionary { - [ContextMapper.CultureKey] = languageId + [ContextMapper.CultureKey] = culture }); } } From adfc8769e626879160d39d3e7fcaabe5dd2ec8db Mon Sep 17 00:00:00 2001 From: Robert Date: Tue, 1 May 2018 13:16:13 +0200 Subject: [PATCH 2/4] Adding missing service injection which broke discard changes dialog --- .../views/common/notifications/confirmroutechange.controller.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js b/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js index 5851d30610..eb2b0e5930 100644 --- a/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/common/notifications/confirmroutechange.controller.js @@ -1,6 +1,6 @@ //used for the media picker dialog angular.module("umbraco").controller("Umbraco.Notifications.ConfirmRouteChangeController", - function ($scope, $location, $log, notificationsService) { + function ($scope, $location, $log, notificationsService, navigationService) { $scope.discard = function(not){ From 9843f3a5fd6fb2f4e2d017a37e6d149984254595 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 2 May 2018 14:52:00 +1000 Subject: [PATCH 3/4] Ensures that domains assigned to non-published nodes are filtered when routing, ensures caches are cleared when content is deleted or langauges are changed, updates logic for dealing with null invariant content names, adds more validation for saving cultre variants. --- src/Umbraco.Core/Models/ContentBase.cs | 2 - src/Umbraco.Core/Models/ContentExtensions.cs | 15 +----- .../Implement/DocumentRepository.cs | 35 ++++++++++++++ .../Services/Implement/ContentService.cs | 27 ++++++----- .../services/umbdataformatter.service.js | 4 +- .../components/editor/umb-editor-header.html | 2 +- .../Cache/ContentCacheRefresher.cs | 36 +++++++++++++-- src/Umbraco.Web/Cache/DomainCacheRefresher.cs | 3 +- .../Cache/LanguageCacheRefresher.cs | 46 +++++++++++++++---- src/Umbraco.Web/Editors/ContentController.cs | 35 ++++++-------- .../NuCache/PublishedSnapshotService.cs | 1 + .../XmlPublishedCache/DomainCache.cs | 11 +++++ src/Umbraco.Web/Routing/PublishedRouter.cs | 6 ++- .../WebApi/Binders/ContentItemBinder.cs | 34 +++++++++++++- .../WebApi/Binders/MemberBinder.cs | 6 +-- .../Filters/ContentItemValidationHelper.cs | 12 +++-- 16 files changed, 200 insertions(+), 75 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentBase.cs b/src/Umbraco.Core/Models/ContentBase.cs index f0cb8c0574..ff1925c72d 100644 --- a/src/Umbraco.Core/Models/ContentBase.cs +++ b/src/Umbraco.Core/Models/ContentBase.cs @@ -55,8 +55,6 @@ namespace Umbraco.Core.Models Id = 0; // no identity VersionId = 0; // no versions - //fixme we always need to set the invariant name else an exception will throw if we try to persist - Name = name; SetName(culture, name); _contentTypeId = contentType.Id; diff --git a/src/Umbraco.Core/Models/ContentExtensions.cs b/src/Umbraco.Core/Models/ContentExtensions.cs index c0e4214881..0c232ff1e5 100644 --- a/src/Umbraco.Core/Models/ContentExtensions.cs +++ b/src/Umbraco.Core/Models/ContentExtensions.cs @@ -159,20 +159,7 @@ namespace Umbraco.Core.Models return Current.Services.MediaService.GetById(media.ParentId); } #endregion - - #region Variants - - /// - /// Returns true if the content has any property type that allows language variants - /// - public static bool HasPropertyTypeVaryingByCulture(this IContent content) - { - // fixme - what about CultureSegment? what about content.ContentType.Variations? - return content.PropertyTypes.Any(x => x.Variations.Has(ContentVariation.CultureNeutral)); - } - - #endregion - + /// /// Removes characters that are not valide XML characters from all entity properties /// of type string. See: http://stackoverflow.com/a/961504/5018 diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 76321abe6a..7f79788091 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -177,9 +177,11 @@ namespace Umbraco.Core.Persistence.Repositories.Implement "DELETE FROM " + Constants.DatabaseSchema.Tables.TagRelationship + " WHERE nodeId = @id", "DELETE FROM " + Constants.DatabaseSchema.Tables.Domain + " WHERE domainRootStructureID = @id", "DELETE FROM " + Constants.DatabaseSchema.Tables.Document + " WHERE nodeId = @id", + "DELETE FROM " + Constants.DatabaseSchema.Tables.DocumentCultureVariation + " WHERE nodeId = @id", "DELETE FROM " + Constants.DatabaseSchema.Tables.DocumentVersion + " WHERE id IN (SELECT id FROM " + Constants.DatabaseSchema.Tables.ContentVersion + " WHERE nodeId = @id)", "DELETE FROM " + Constants.DatabaseSchema.Tables.PropertyData + " WHERE versionId IN (SELECT id FROM " + Constants.DatabaseSchema.Tables.ContentVersion + " WHERE nodeId = @id)", "DELETE FROM cmsPreviewXml WHERE nodeId = @id", + "DELETE FROM " + Constants.DatabaseSchema.Tables.ContentVersionCultureVariation + " WHERE versionId IN (SELECT id FROM " + Constants.DatabaseSchema.Tables.ContentVersion + " WHERE nodeId = @id)", "DELETE FROM " + Constants.DatabaseSchema.Tables.ContentVersion + " WHERE nodeId = @id", "DELETE FROM cmsContentXml WHERE nodeId = @id", "DELETE FROM " + Constants.DatabaseSchema.Tables.Content + " WHERE nodeId = @id", @@ -229,6 +231,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement protected override void PersistNewItem(IContent entity) { + //fixme - stop doing this just so we have access to AddingEntity var content = (Content) entity; content.AddingEntity(); @@ -238,6 +241,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (entity.Template == null) entity.Template = entity.ContentType.DefaultTemplate; + entity.Name = FormatInvariantNameValue(entity); // ensure unique name on the same level entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name); @@ -416,6 +420,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Execute(Sql().Update(u => u.Set(x => x.Published, false)).Where(x => x.Id == content.PublishedVersionId)); } + entity.Name = FormatInvariantNameValue(entity); // ensure unique name on the same level entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name, entity.Id); @@ -1074,6 +1079,36 @@ namespace Umbraco.Core.Persistence.Repositories.Implement #region Utilities + /// + /// This ensures that the Name property exists and validates if all names are null + /// + /// + /// + private string FormatInvariantNameValue(IContent content) + { + if (content.Name.IsNullOrWhiteSpace() && content.Names.Count == 0) + throw new InvalidOperationException("Cannot save content with empty name."); + + if (content.Name.IsNullOrWhiteSpace() && content.Names.Count > 0) + { + //if we are saving a variant, we current need to have the invariant name set too + //fixme - this needs to be discussed! http://issues.umbraco.org/issue/U4-11286 + + var defaultCulture = LanguageRepository.GetDefaultIsoCode(); + if (!defaultCulture.IsNullOrWhiteSpace() && content.Names.TryGetValue(defaultCulture, out var cultureName)) + { + return cultureName; + } + else + { + //our only option is to take the first + return content.Names.First().Value; + } + } + + return content.Name; + } + protected override string EnsureUniqueNodeName(int parentId, string nodeName, int id = 0) { return EnsureUniqueNaming == false ? nodeName : base.EnsureUniqueNodeName(parentId, nodeName, id); diff --git a/src/Umbraco.Core/Services/Implement/ContentService.cs b/src/Umbraco.Core/Services/Implement/ContentService.cs index fc37000e13..919ca73273 100644 --- a/src/Umbraco.Core/Services/Implement/ContentService.cs +++ b/src/Umbraco.Core/Services/Implement/ContentService.cs @@ -163,6 +163,8 @@ namespace Umbraco.Core.Services.Implement /// public IContent Create(string name, Guid parentId, string contentTypeAlias, int userId = 0) { + //fixme - what about culture? + var parent = GetById(parentId); return Create(name, parent, contentTypeAlias, userId); } @@ -181,6 +183,8 @@ namespace Umbraco.Core.Services.Implement /// The content object. public IContent Create(string name, int parentId, string contentTypeAlias, int userId = 0) { + //fixme - what about culture? + var contentType = GetContentType(contentTypeAlias); if (contentType == null) throw new ArgumentException("No content type with that alias.", nameof(contentTypeAlias)); @@ -212,6 +216,8 @@ namespace Umbraco.Core.Services.Implement /// The content object. public IContent Create(string name, IContent parent, string contentTypeAlias, int userId = 0) { + //fixme - what about culture? + if (parent == null) throw new ArgumentNullException(nameof(parent)); using (var scope = ScopeProvider.CreateScope()) @@ -241,6 +247,8 @@ namespace Umbraco.Core.Services.Implement /// The content object. public IContent CreateAndSave(string name, int parentId, string contentTypeAlias, int userId = 0) { + //fixme - what about culture? + using (var scope = ScopeProvider.CreateScope()) { // locking the content tree secures content types too @@ -273,6 +281,8 @@ namespace Umbraco.Core.Services.Implement /// The content object. public IContent CreateAndSave(string name, IContent parent, string contentTypeAlias, int userId = 0) { + //fixme - what about culture? + if (parent == null) throw new ArgumentNullException(nameof(parent)); using (var scope = ScopeProvider.CreateScope()) @@ -864,11 +874,6 @@ namespace Umbraco.Core.Services.Implement return OperationResult.Cancel(evtMsgs); } - if (string.IsNullOrWhiteSpace(content.Name)) - { - throw new ArgumentException("Cannot save content with empty name."); - } - var isNew = content.IsNewEntity(); scope.WriteLock(Constants.Locks.ContentTree); @@ -1217,7 +1222,7 @@ namespace Umbraco.Core.Services.Implement using (var scope = ScopeProvider.CreateScope()) { var deleteEventArgs = new DeleteEventArgs(content, evtMsgs); - if (scope.Events.DispatchCancelable(Deleting, this, deleteEventArgs)) + if (scope.Events.DispatchCancelable(Deleting, this, deleteEventArgs, nameof(Deleting))) { scope.Complete(); return OperationResult.Cancel(evtMsgs); @@ -1229,7 +1234,7 @@ namespace Umbraco.Core.Services.Implement // but... UnPublishing event makes no sense (not going to cancel?) and no need to save // just raise the event if (content.Trashed == false && content.Published) - scope.Events.Dispatch(UnPublished, this, new PublishEventArgs(content, false, false), "UnPublished"); + scope.Events.Dispatch(UnPublished, this, new PublishEventArgs(content, false, false), nameof(UnPublished)); DeleteLocked(scope, content); @@ -1265,7 +1270,7 @@ namespace Umbraco.Core.Services.Implement _documentRepository.Delete(c); var args = new DeleteEventArgs(c, false); // raise event & get flagged files - scope.Events.Dispatch(Deleted, this, args); + scope.Events.Dispatch(Deleted, this, args, nameof(Deleted)); // fixme not going to work, do it differently _mediaFileSystem.DeleteFiles(args.MediaFilesToDelete, // remove flagged files @@ -2149,7 +2154,7 @@ namespace Umbraco.Core.Services.Implement var query = Query().WhereIn(x => x.ContentTypeId, contentTypeIdsA); var contents = _documentRepository.Get(query).ToArray(); - if (scope.Events.DispatchCancelable(Deleting, this, new DeleteEventArgs(contents))) + if (scope.Events.DispatchCancelable(Deleting, this, new DeleteEventArgs(contents), nameof(Deleting))) { scope.Complete(); return; @@ -2296,7 +2301,7 @@ namespace Umbraco.Core.Services.Implement { scope.WriteLock(Constants.Locks.ContentTree); _documentBlueprintRepository.Delete(content); - scope.Events.Dispatch(DeletedBlueprint, this, new DeleteEventArgs(content), "DeletedBlueprint"); + scope.Events.Dispatch(DeletedBlueprint, this, new DeleteEventArgs(content), nameof(DeletedBlueprint)); scope.Complete(); } } @@ -2357,7 +2362,7 @@ namespace Umbraco.Core.Services.Implement _documentBlueprintRepository.Delete(blueprint); } - scope.Events.Dispatch(DeletedBlueprint, this, new DeleteEventArgs(blueprints), "DeletedBlueprint"); + scope.Events.Dispatch(DeletedBlueprint, this, new DeleteEventArgs(blueprints), nameof(DeletedBlueprint)); scope.Complete(); } } 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 1888ff623d..70004900f9 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 @@ -327,8 +327,8 @@ //get the selected variant and build the additional published variants saveModel.publishVariations = []; - //if there's more than 1 variant than we need to set the language and include the variants to publish - if (displayModel.variants.length > 1) { + //if there's any variants than we need to set the language and include the variants to publish + if (displayModel.variants.length > 0) { _.each(displayModel.variants, function (d) { //set the selected variant if this is current diff --git a/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html b/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html index 41d44114a0..dfe235753c 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/editor/umb-editor-header.html @@ -50,7 +50,7 @@ server-validation-field="Alias"> - + {{vm.currentVariant.language.name}}   diff --git a/src/Umbraco.Web/Cache/ContentCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentCacheRefresher.cs index f44781886d..58c5732650 100644 --- a/src/Umbraco.Web/Cache/ContentCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentCacheRefresher.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Linq; using Umbraco.Core; using Umbraco.Core.Cache; @@ -17,12 +18,14 @@ namespace Umbraco.Web.Cache { private readonly IPublishedSnapshotService _publishedSnapshotService; private readonly IdkMap _idkMap; + private readonly IDomainService _domainService; - public ContentCacheRefresher(CacheHelper cacheHelper, IPublishedSnapshotService publishedSnapshotService, IdkMap idkMap) + public ContentCacheRefresher(CacheHelper cacheHelper, IPublishedSnapshotService publishedSnapshotService, IdkMap idkMap, IDomainService domainService) : base(cacheHelper) { _publishedSnapshotService = publishedSnapshotService; _idkMap = idkMap; + _domainService = domainService; } #region Define @@ -49,6 +52,8 @@ namespace Umbraco.Web.Cache runtimeCache.ClearCacheByKeySearch(CacheKeys.IdToKeyCacheKey); runtimeCache.ClearCacheByKeySearch(CacheKeys.KeyToIdCacheKey); + var idsRemoved = new HashSet(); + foreach (var payload in payloads) { // remove that one @@ -62,6 +67,31 @@ namespace Umbraco.Web.Cache var pathid = "," + payload.Id + ","; runtimeCache.ClearCacheObjectTypes((k, v) => v.Path.Contains(pathid)); } + + //if the item is being completely removed, we need to refresh the domains cache if any domain was assigned to the content + if (payload.ChangeTypes.HasTypesAny(TreeChangeTypes.Remove)) + { + idsRemoved.Add(payload.Id); + } + } + + if (idsRemoved.Count > 0) + { + var assignedDomains = _domainService.GetAll(true).Where(x => x.RootContentId.HasValue && idsRemoved.Contains(x.RootContentId.Value)).ToList(); + + if (assignedDomains.Count > 0) + { + //fixme - this is duplicating the logic in DomainCacheRefresher BUT we cannot inject that into this because it it not registered explicitly in the container, + // and we cannot inject the CacheRefresherCollection since that would be a circular reference, so what is the best way to call directly in to the + // DomainCacheRefresher? + + ClearAllIsolatedCacheByEntityType(); + // note: must do what's above FIRST else the repositories still have the old cached + // content and when the PublishedCachesService is notified of changes it does not see + // the new content... + // notify + _publishedSnapshotService.Notify(assignedDomains.Select(x => new DomainCacheRefresher.JsonPayload(x.Id, DomainChangeTypes.Remove)).ToArray()); + } } // note: must do what's above FIRST else the repositories still have the old cached @@ -130,10 +160,6 @@ namespace Umbraco.Web.Cache #endregion - #region Events - - #endregion - #region Indirect public static void RefreshContentTypes(CacheHelper cacheHelper) diff --git a/src/Umbraco.Web/Cache/DomainCacheRefresher.cs b/src/Umbraco.Web/Cache/DomainCacheRefresher.cs index 5e3c0220a8..5520d72cd2 100644 --- a/src/Umbraco.Web/Cache/DomainCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/DomainCacheRefresher.cs @@ -84,6 +84,7 @@ namespace Umbraco.Web.Cache public DomainChangeTypes ChangeType { get; } } - #endregion + #endregion + } } diff --git a/src/Umbraco.Web/Cache/LanguageCacheRefresher.cs b/src/Umbraco.Web/Cache/LanguageCacheRefresher.cs index 2270f6471c..d523d3686f 100644 --- a/src/Umbraco.Web/Cache/LanguageCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/LanguageCacheRefresher.cs @@ -1,21 +1,30 @@ using System; +using System.Linq; using Umbraco.Core.Cache; using Umbraco.Core.Models; - +using Umbraco.Core.Services; +using Umbraco.Core.Services.Changes; +using Umbraco.Web.PublishedCache; + namespace Umbraco.Web.Cache { public sealed class LanguageCacheRefresher : CacheRefresherBase { - public LanguageCacheRefresher(CacheHelper cacheHelper) + public LanguageCacheRefresher(CacheHelper cacheHelper, IPublishedSnapshotService publishedSnapshotService, IDomainService domainService) : base(cacheHelper) - { } + { + _publishedSnapshotService = publishedSnapshotService; + _domainService = domainService; + } #region Define protected override LanguageCacheRefresher This => this; - public static readonly Guid UniqueId = Guid.Parse("3E0F95D8-0BE5-44B8-8394-2B8750B62654"); - + public static readonly Guid UniqueId = Guid.Parse("3E0F95D8-0BE5-44B8-8394-2B8750B62654"); + private readonly IPublishedSnapshotService _publishedSnapshotService; + private readonly IDomainService _domainService; + public override Guid RefresherUniqueId => UniqueId; public override string Name => "Language Cache Refresher"; @@ -26,7 +35,8 @@ namespace Umbraco.Web.Cache public override void Refresh(int id) { - ClearAllIsolatedCacheByEntityType(); + ClearAllIsolatedCacheByEntityType(); + RefreshDomains(id); base.Refresh(id); } @@ -34,10 +44,30 @@ namespace Umbraco.Web.Cache { ClearAllIsolatedCacheByEntityType(); //if a language is removed, then all dictionary cache needs to be removed - ClearAllIsolatedCacheByEntityType(); + ClearAllIsolatedCacheByEntityType(); + RefreshDomains(id); base.Remove(id); } - #endregion + #endregion + + private void RefreshDomains(int langId) + { + var assignedDomains = _domainService.GetAll(true).Where(x => x.LanguageId.HasValue && x.LanguageId.Value == langId).ToList(); + + if (assignedDomains.Count > 0) + { + //fixme - this is duplicating the logic in DomainCacheRefresher BUT we cannot inject that into this because it it not registered explicitly in the container, + // and we cannot inject the CacheRefresherCollection since that would be a circular reference, so what is the best way to call directly in to the + // DomainCacheRefresher? + + ClearAllIsolatedCacheByEntityType(); + // note: must do what's above FIRST else the repositories still have the old cached + // content and when the PublishedCachesService is notified of changes it does not see + // the new content... + // notify + _publishedSnapshotService.Notify(assignedDomains.Select(x => new DomainCacheRefresher.JsonPayload(x.Id, DomainChangeTypes.Remove)).ToArray()); + } + } } } diff --git a/src/Umbraco.Web/Editors/ContentController.cs b/src/Umbraco.Web/Editors/ContentController.cs index efdc6c23ab..691938c771 100644 --- a/src/Umbraco.Web/Editors/ContentController.cs +++ b/src/Umbraco.Web/Editors/ContentController.cs @@ -72,7 +72,9 @@ namespace Umbraco.Web.Editors /// [FilterAllowedOutgoingContent(typeof(IEnumerable))] public IEnumerable GetByIds([FromUri]int[] ids) - { + { + //fixme what about cultures? + var foundContent = Services.ContentService.GetByIds(ids); return foundContent.Select(x => MapToDisplay(x)); } @@ -217,7 +219,8 @@ namespace Umbraco.Web.Editors return display; } - + + //fixme what about cultures? public ContentItemDisplay GetBlueprintById(int id) { var foundContent = Services.ContentService.GetBlueprintById(id); @@ -270,19 +273,6 @@ namespace Umbraco.Web.Editors return content; } - [EnsureUserPermissionForContent("id")] - public ContentItemDisplay GetWithTreeDefinition(int id) - { - var foundContent = GetObjectFromRequest(() => Services.ContentService.GetById(id)); - if (foundContent == null) - { - HandleContentNotFound(id); - } - - var content = MapToDisplay(foundContent); - return content; - } - /// /// Gets an empty content item for the /// @@ -954,8 +944,9 @@ namespace Umbraco.Web.Editors if (contentItem.Name.IsNullOrWhiteSpace() == false) { //set the name according to the culture settings - if (!contentItem.Culture.IsNullOrWhiteSpace() && contentItem.PersistedContent.ContentType.Variations.HasFlag(ContentVariation.CultureNeutral)) - { + if (contentItem.PersistedContent.ContentType.Variations.HasFlag(ContentVariation.CultureNeutral)) + { + if (contentItem.Culture.IsNullOrWhiteSpace()) throw new InvalidOperationException($"Cannot save a content item that is {ContentVariation.CultureNeutral} with a culture specified"); contentItem.PersistedContent.SetName(contentItem.Culture, contentItem.Name); } else @@ -1185,11 +1176,11 @@ namespace Umbraco.Web.Editors /// private ContentItemDisplay MapToDisplay(IContent content, string culture = null) { - //a languageId must exist in the mapping context if this content item has any property type that can be varied by language - //otherwise the property validation will fail since it's expecting to be get/set with a language ID. If a languageId is not explicitly - //sent up, then it means that the user is editing the default variant language. - if (culture == null && content.HasPropertyTypeVaryingByCulture()) - { + //A culture must exist in the mapping context if this content type is CultureNeutral since for a culture variant to be edited, + // the Cuture property of ContentItemDisplay must exist (at least currently). + if (culture == null && content.ContentType.Variations.Has(ContentVariation.CultureNeutral)) + { + //If a culture is not explicitly sent up, then it means that the user is editing the default variant language. culture = Services.LocalizationService.GetDefaultLanguageIsoCode(); } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index b498a1a42b..06eb14898e 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -815,6 +815,7 @@ namespace Umbraco.Web.PublishedCache.NuCache switch (payload.ChangeType) { case DomainChangeTypes.RefreshAll: + //fixme why this check? if (!(_serviceContext.DomainService is DomainService)) throw new Exception("oops"); diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DomainCache.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DomainCache.cs index 9a82840024..0f40d78225 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DomainCache.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/DomainCache.cs @@ -17,6 +17,11 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache DefaultCulture = systemDefaultCultureProvider.DefaultCulture; } + /// + /// Returns all in the current domain cache including any domains that may be referenced by content items that are no longer published + /// + /// + /// public IEnumerable GetAll(bool includeWildcards) { return _domainService.GetAll(includeWildcards) @@ -24,6 +29,12 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache .Select(x => new Domain(x.Id, x.DomainName, x.RootContentId.Value, CultureInfo.GetCultureInfo(x.LanguageIsoCode), x.IsWildcard)); } + /// + /// Returns all assigned for the content id specified even if the content item is not published + /// + /// + /// + /// public IEnumerable GetAssigned(int contentId, bool includeWildcards) { return _domainService.GetAssignedDomains(contentId, includeWildcards) diff --git a/src/Umbraco.Web/Routing/PublishedRouter.cs b/src/Umbraco.Web/Routing/PublishedRouter.cs index 35e5ab1af4..f85d2d6c28 100644 --- a/src/Umbraco.Web/Routing/PublishedRouter.cs +++ b/src/Umbraco.Web/Routing/PublishedRouter.cs @@ -266,7 +266,11 @@ namespace Umbraco.Web.Routing _logger.Debug(() => $"{tracePrefix}Uri=\"{request.Uri}\""); var domainsCache = request.UmbracoContext.PublishedSnapshot.Domains; - var domains = domainsCache.GetAll(includeWildcards: false); + + //get the domains but filter to ensure that any referenced content is actually published + var domains = domainsCache.GetAll(includeWildcards: false) + .Where(x => request.UmbracoContext.PublishedSnapshot.Content.GetById(x.ContentId) != null); + var defaultCulture = domainsCache.DefaultCulture; // try to find a domain matching the current request diff --git a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs index 8c3b705bb6..6863a267b1 100644 --- a/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/ContentItemBinder.cs @@ -1,15 +1,24 @@ using System; using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Web.Http.Controllers; using AutoMapper; using Umbraco.Core; using Umbraco.Core.Models; using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.Models.Mapping; - +using Umbraco.Web.WebApi.Filters; + namespace Umbraco.Web.WebApi.Binders { internal class ContentItemBinder : ContentItemBaseBinder - { + { + protected override ContentItemValidationHelper GetValidationHelper() + { + return new ContentValidationHelper(); + } + protected override IContent GetExisting(ContentItemSave model) { return Services.ContentService.GetById(Convert.ToInt32(model.Id)); @@ -36,6 +45,27 @@ namespace Umbraco.Web.WebApi.Binders { [ContextMapper.CultureKey] = culture }); + } + + internal class ContentValidationHelper : ContentItemValidationHelper + { + /// + /// Validates that the correct information is in the request for saving a culture variant + /// + /// + /// + /// + protected override bool ValidateCultureVariant(ContentItemSave postedItem, HttpActionContext actionContext) + { + var contentType = postedItem.PersistedContent.GetContentType(); + if (contentType.Variations.Has(Core.Models.ContentVariation.CultureNeutral) && postedItem.Culture.IsNullOrWhiteSpace()) + { + //we cannot save a content item that is culture variant if no culture was specified in the request! + actionContext.Response = actionContext.Request.CreateValidationErrorResponse($"No 'Culture' found in request. Cannot save a content item that is of a {Core.Models.ContentVariation.CultureNeutral} content type without a specified culture."); + return false; + } + return true; + } } } } diff --git a/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs b/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs index 57215ab5ee..45a1947c50 100644 --- a/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs @@ -186,9 +186,9 @@ namespace Umbraco.Web.WebApi.Binders /// /// /// - public override bool ValidatePropertyData(ContentItemBasic postedItem, ContentItemDto dto, ModelStateDictionary modelState) + public override bool ValidatePropertyData(MemberSave postedItem, ContentItemDto dto, ModelStateDictionary modelState) { - var memberSave = (MemberSave)postedItem; + var memberSave = postedItem; if (memberSave.Username.IsNullOrWhiteSpace()) { @@ -234,7 +234,7 @@ namespace Umbraco.Web.WebApi.Binders /// /// /// - protected override bool ValidateProperties(ContentItemBasic postedItem, HttpActionContext actionContext) + protected override bool ValidateProperties(MemberSave postedItem, HttpActionContext actionContext) { var propertiesToValidate = postedItem.Properties.ToList(); var defaultProps = Constants.Conventions.Member.GetStandardPropertyTypeStubs(); diff --git a/src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs b/src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs index c1a8c989e5..f0e23fcadf 100644 --- a/src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs +++ b/src/Umbraco.Web/WebApi/Filters/ContentItemValidationHelper.cs @@ -48,9 +48,15 @@ namespace Umbraco.Web.WebApi.Filters { //now do each validation step if (ValidateExistingContent(contentItem, actionContext) == false) return; + if (ValidateCultureVariant(contentItem, actionContext) == false) return; if (ValidateProperties(contentItem, actionContext) == false) return; if (ValidatePropertyData(contentItem, contentItem.ContentDto, actionContext.ModelState) == false) return; } + + protected virtual bool ValidateCultureVariant(TModelSave postedItem, HttpActionContext actionContext) + { + return true; + } /// /// Ensure the content exists @@ -58,7 +64,7 @@ namespace Umbraco.Web.WebApi.Filters /// /// /// - protected virtual bool ValidateExistingContent(ContentItemBasic postedItem, HttpActionContext actionContext) + protected virtual bool ValidateExistingContent(TModelSave postedItem, HttpActionContext actionContext) { if (postedItem.PersistedContent == null) { @@ -76,7 +82,7 @@ namespace Umbraco.Web.WebApi.Filters /// /// /// - protected virtual bool ValidateProperties(ContentItemBasic postedItem, HttpActionContext actionContext) + protected virtual bool ValidateProperties(TModelSave postedItem, HttpActionContext actionContext) { return ValidateProperties(postedItem.Properties.ToList(), postedItem.PersistedContent.Properties.ToList(), actionContext); } @@ -116,7 +122,7 @@ namespace Umbraco.Web.WebApi.Filters /// /// All property data validation goes into the modelstate with a prefix of "Properties" /// - public virtual bool ValidatePropertyData(ContentItemBasic postedItem, ContentItemDto dto, ModelStateDictionary modelState) + public virtual bool ValidatePropertyData(TModelSave postedItem, ContentItemDto dto, ModelStateDictionary modelState) { foreach (var p in dto.Properties) { From 6fe9c4d4dfbacba978d2b040f0471dc504327722 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 2 May 2018 15:35:26 +1000 Subject: [PATCH 4/4] Fixes the null PublishName problem, ensures the quey strings are not replaced when selecting a doc type --- src/Umbraco.Core/Models/Content.cs | 24 +++++++------- .../Implement/DocumentRepository.cs | 31 +++++++++++-------- src/Umbraco.Core/StringExtensions.cs | 8 +++-- .../content/content.create.controller.js | 9 +++--- 4 files changed, 41 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Core/Models/Content.cs b/src/Umbraco.Core/Models/Content.cs index 57ad512616..c096b3dc70 100644 --- a/src/Umbraco.Core/Models/Content.cs +++ b/src/Umbraco.Core/Models/Content.cs @@ -207,20 +207,20 @@ namespace Umbraco.Core.Models { if (string.IsNullOrWhiteSpace(name)) throw new ArgumentNullOrEmptyException(nameof(name)); + + //fixme - we always need to set the invariant values since these cannot be null! discuss http://issues.umbraco.org/issue/U4-11286 + PublishName = name; + PublishDate = date; - if (culture == null) - { - PublishName = name; - PublishDate = date; - return; + if (culture != null) + { + // private method, assume that culture is valid + + if (_publishInfos == null) + _publishInfos = new Dictionary(StringComparer.OrdinalIgnoreCase); + + _publishInfos[culture] = (name, date); } - - // private method, assume that culture is valid - - if (_publishInfos == null) - _publishInfos = new Dictionary(StringComparer.OrdinalIgnoreCase); - - _publishInfos[culture] = (name, date); } /// diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs index 7f79788091..2e8de4afa4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs @@ -241,7 +241,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement if (entity.Template == null) entity.Template = entity.ContentType.DefaultTemplate; - entity.Name = FormatInvariantNameValue(entity); + EnsureInvariantNameValues(entity, publishing); // ensure unique name on the same level entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name); @@ -309,7 +309,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement content.PublishedVersionId = content.VersionId; contentVersionDto.Id = 0; contentVersionDto.Current = true; - contentVersionDto.Text = content.Name; + contentVersionDto.Text = content.PublishName; Database.Insert(contentVersionDto); content.VersionId = contentVersionDto.Id; @@ -420,7 +420,7 @@ namespace Umbraco.Core.Persistence.Repositories.Implement Database.Execute(Sql().Update(u => u.Set(x => x.Published, false)).Where(x => x.Id == content.PublishedVersionId)); } - entity.Name = FormatInvariantNameValue(entity); + EnsureInvariantNameValues(entity, publishing); // ensure unique name on the same level entity.Name = EnsureUniqueNodeName(entity.ParentId, entity.Name, entity.Id); @@ -1080,33 +1080,38 @@ namespace Umbraco.Core.Persistence.Repositories.Implement #region Utilities /// - /// This ensures that the Name property exists and validates if all names are null + /// This ensures that the Name/PublishName properties are filled in and validates if all names are null /// /// /// - private string FormatInvariantNameValue(IContent content) + private void EnsureInvariantNameValues(IContent content, bool publishing) { + //fixme - if we are saving a variant, we current need to have the invariant name set too, discuss! http://issues.umbraco.org/issue/U4-11286 + //NOTE - this doesn't deal with the PublishName since that is readonly, instead this scenario is dealt with in `Content.SetPublishInfos` + + //validate + if (content.Name.IsNullOrWhiteSpace() && content.Names.Count == 0) - throw new InvalidOperationException("Cannot save content with empty name."); + throw new InvalidOperationException("Cannot save content with an empty name."); + + if (publishing && content.PublishName.IsNullOrWhiteSpace() && content.PublishNames.Count == 0) + throw new InvalidOperationException("Cannot publish content with an empty name."); + + //ensure the invariant Name if (content.Name.IsNullOrWhiteSpace() && content.Names.Count > 0) { - //if we are saving a variant, we current need to have the invariant name set too - //fixme - this needs to be discussed! http://issues.umbraco.org/issue/U4-11286 - var defaultCulture = LanguageRepository.GetDefaultIsoCode(); if (!defaultCulture.IsNullOrWhiteSpace() && content.Names.TryGetValue(defaultCulture, out var cultureName)) { - return cultureName; + content.Name = cultureName; } else { //our only option is to take the first - return content.Names.First().Value; + content.Name = content.Names.First().Value; } } - - return content.Name; } protected override string EnsureUniqueNodeName(int parentId, string nodeName, int id = 0) diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index afdb4c3646..a399c82f0c 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -1147,7 +1147,8 @@ namespace Umbraco.Core /// The text to filter. /// The safe url segment. public static string ToUrlSegment(this string text) - { + { + if (string.IsNullOrWhiteSpace(text)) throw new ArgumentException("message", nameof(text)); return Current.ShortStringHelper.CleanStringForUrlSegment(text); } @@ -1158,7 +1159,10 @@ namespace Umbraco.Core /// The culture. /// The safe url segment. public static string ToUrlSegment(this string text, CultureInfo culture) - { + { + if (string.IsNullOrWhiteSpace(text)) throw new ArgumentException("message", nameof(text)); + if (culture == null) throw new ArgumentNullException(nameof(culture)); + return Current.ShortStringHelper.CleanStringForUrlSegment(text, culture); } diff --git a/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js b/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js index ace6058b1d..199976f3fa 100644 --- a/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/content/content.create.controller.js @@ -29,9 +29,10 @@ function contentCreateController($scope, } function createBlank(docType) { - $location - .path("/content/content/edit/" + $scope.currentNode.id) - .search("doctype=" + docType.alias + "&create=true"); + $location + .path("/content/content/edit/" + $scope.currentNode.id) + .search("doctype", docType.alias) + .search("create", true); close(); } @@ -69,4 +70,4 @@ angular.module("umbraco").controller("Umbraco.Editors.Content.CreateController", angular.module("umbraco").value("blueprintConfig", { skipSelect: false, allowBlank: true -}); \ No newline at end of file +});