From 2f0f43e60560ffe948d00d2119dbd596e4e718ae Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 10 Aug 2018 00:39:20 +1000 Subject: [PATCH] Gets image cropper working between variants, even with invariant properties, adds unit test to ensure invariant properties in the model are the same reference --- .../upload/umbpropertyfileupload.directive.js | 26 +++++- .../src/common/resources/content.resource.js | 15 ++-- .../services/umbdataformatter.service.js | 46 ++++++++++ .../upload/umb-property-file-upload.html | 4 +- .../fileupload/fileupload.controller.js | 4 +- .../imagecropper/imagecropper.controller.js | 22 +++-- .../imagecropper/imagecropper.html | 1 + .../umb-data-formatter-service.spec.js | 88 +++++++++++++++++++ 8 files changed, 185 insertions(+), 21 deletions(-) create mode 100644 src/Umbraco.Web.UI.Client/test/unit/common/services/umb-data-formatter-service.spec.js diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js index 52983efd87..b8afc4b35e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/upload/umbpropertyfileupload.directive.js @@ -51,6 +51,19 @@ vm.fileUploadForm.$setDirty(); } + function notifyInit(val, files) { + if (!val) { + val = null; + } + if (!files) { + files = null; + } + + if (vm.onInit) { + vm.onInit({ value: val, files: files }); + } + } + /** Called when the component initializes */ function onInit() { $scope.$on("filesSelected", onFilesSelected); @@ -85,7 +98,7 @@ if (existingClientFiles.length > 0) { updateModelFromSelectedFiles(existingClientFiles).then(function(newVal) { //notify the callback - vm.onValueChanged({ value: newVal, files: vm.files }); + notifyInit(newVal, vm.files); }); } else if (vm.value) { @@ -101,12 +114,16 @@ f.fileSrc = getThumbnail(f); return f; }); + + //notify the callback + notifyInit(); } else { vm.files = []; - } - //vm.clearFiles = false; + //notify the callback + notifyInit(); + } } ///** Method required by the valPropertyValidator directive (returns true if the property editor has at least one file selected) */ @@ -250,8 +267,9 @@ culture: "@?", propertyAlias: "@", value: "<", + hideSelection: "<", onValueChanged: "&", - hideSelection: "<" + onInit: "&" }, transclude: true, controllerAs: 'vm', 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 206ca08dac..33840e87d9 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 @@ -332,12 +332,15 @@ function contentResource($q, $http, umbDataFormatter, umbRequestHelper) { */ getById: function (id) { return umbRequestHelper.resourcePromise( - $http.get( - umbRequestHelper.getApiUrl( - "contentApiBaseUrl", - "GetById", - { id: id })), - 'Failed to retrieve data for content id ' + id); + $http.get( + umbRequestHelper.getApiUrl( + "contentApiBaseUrl", + "GetById", + { id: id })), + 'Failed to retrieve data for content id ' + id) + .then(function(result) { + return $q.when(umbDataFormatter.formatContentGetData(result)); + }); }, getBlueprintById: function (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 38c02338ee..77ca2b6157 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 @@ -383,6 +383,52 @@ saveModel.templateAlias = propTemplate ? propTemplate : null; return saveModel; + }, + + /** + * This formats the server GET response for a content display item + * @param {} displayModel + * @returns {} + */ + formatContentGetData: function(displayModel) { + + //We need to check for invariant properties among the variant variants. + //When we detect this, we want to make sure that the property object instance is the + //same reference object between all variants instead of a copy (which it will be when + //return from the JSON structure). + + if (displayModel.variants && displayModel.variants.length > 1) { + + var invariantProperties = []; + + //collect all invariant properties on the first first variant + var firstVariant = displayModel.variants[0]; + _.each(firstVariant.tabs, function(tab, tabIndex) { + _.each(tab.properties, function (property, propIndex) { + //in theory if there's more than 1 variant, that means they would all have a language + //but we'll do our safety checks anyways here + if (firstVariant.language && !property.culture) { + invariantProperties.push({ + tabIndex: tabIndex, + propIndex: propIndex, + property: property + }); + } + }); + }); + + + //now assign this same invariant property instance to the same index of the other variants property array + for (var j = 1; j < displayModel.variants.length; j++) { + var variant = displayModel.variants[j]; + + _.each(invariantProperties, function (invProp) { + variant.tabs[invProp.tabIndex].properties[invProp.propIndex] = invProp.property; + }); + } + } + + return displayModel; } }; } diff --git a/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-property-file-upload.html b/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-property-file-upload.html index 5729bf0908..f205dffa57 100644 --- a/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-property-file-upload.html +++ b/src/Umbraco.Web.UI.Client/src/views/components/upload/umb-property-file-upload.html @@ -8,9 +8,7 @@

Click to upload

- - 
{{vm.files.length}}
- +
diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js index d005cbedad..a5ebc92c5f 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/fileupload/fileupload.controller.js @@ -12,7 +12,7 @@ */ function fileUploadController($scope) { - $scope.fileChanged = fileChanged; + $scope.fileChanged = onFileChanged; //declare a special method which will be called whenever the value has changed from the server $scope.model.onValueChanged = onValueChanged; @@ -20,7 +20,7 @@ * Called when the file selection value changes * @param {any} value */ - function fileChanged(value) { + function onFileChanged(value) { $scope.model.value = value; //if the value is empty, then tell the server to clear the files diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js index d4b68d98e3..1c9f835142 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.controller.js @@ -4,7 +4,8 @@ angular.module('umbraco') var config = angular.copy($scope.model.config); - $scope.fileChanged = fileChanged; + $scope.fileChanged = onFileChanged; + $scope.fileUploaderInit = onFileUploaderInit; $scope.crop = crop; $scope.done = done; $scope.clear = clear; @@ -40,7 +41,7 @@ angular.module('umbraco') * Called when the file selection value changes * @param {any} value */ - function fileChanged(value, files) { + function onFileChanged(value, files) { setModelValueWithSrc(value); if (files && files[0]) { @@ -50,7 +51,11 @@ angular.module('umbraco') } } - function onInit() { + /** + * Called when the file uploader initializes + * @param {any} value + */ + function onFileUploaderInit(value, files) { //move previously saved value to the editor if ($scope.model.value) { //backwards compat with the old file upload (incase some-one swaps them..) @@ -74,7 +79,14 @@ angular.module('umbraco') } } - $scope.imageSrc = $scope.model.value.src; + //if there are already files in the client assigned then set the src + if (files && files[0]) { + $scope.imageSrc = files[0].fileSrc; + } + else { + $scope.imageSrc = $scope.model.value.src; + } + } } @@ -120,8 +132,6 @@ angular.module('umbraco') //set form to dirty to track changes $scope.imageCropperForm.$setDirty(); }; - - onInit(); var unsubscribe = $scope.$on("formSubmitting", function () { $scope.currentCrop = null; diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.html b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.html index a4c06b40c4..bc41218592 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.html +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/imagecropper/imagecropper.html @@ -7,6 +7,7 @@ property-alias="{{model.alias}}" value="model.value.src" on-value-changed="fileChanged(value, files)" + on-init="fileUploaderInit(value, files)" hide-selection="true">
diff --git a/src/Umbraco.Web.UI.Client/test/unit/common/services/umb-data-formatter-service.spec.js b/src/Umbraco.Web.UI.Client/test/unit/common/services/umb-data-formatter-service.spec.js new file mode 100644 index 0000000000..7c307a96e1 --- /dev/null +++ b/src/Umbraco.Web.UI.Client/test/unit/common/services/umb-data-formatter-service.spec.js @@ -0,0 +1,88 @@ +describe('umbDataFormatter service tests', function () { + var umbDataFormatter; + + beforeEach(module('umbraco.services')); + + beforeEach(inject(function ($injector) { + umbDataFormatter = $injector.get('umbDataFormatter'); + })); + + describe('formatting GET content data', function () { + + it('will set the same invariant property instance reference between all variants', function () { + + var model = { + variants: [ + { + language: { culture: "en-US" }, + tabs: [ + { + properties: [ + { alias: "test1", culture: null, value: "test1" }, + { alias: "test2", culture: "en-US", value: "test2" } + ] + }, + { + properties: [ + { alias: "test3", culture: "en-US", value: "test3" }, + { alias: "test4", culture: null, value: "test4" } + ] + } + ] + + }, + { + language: { culture: "es-ES" }, + tabs: [ + { + properties: [ + { alias: "test1", culture: null, value: "test5" }, + { alias: "test2", culture: "en-US", value: "test6" } + ] + }, + { + properties: [ + { alias: "test3", culture: "en-US", value: "test7" }, + { alias: "test4", culture: null, value: "test8" } + ] + } + ] + }, + { + language: { culture: "fr-FR" }, + tabs: [ + { + properties: [ + { alias: "test1", culture: null, value: "test9" }, + { alias: "test2", culture: "en-US", value: "test10" } + ] + }, + { + properties: [ + { alias: "test3", culture: "en-US", value: "test11" }, + { alias: "test4", culture: null, value: "test12" } + ] + } + ] + } + ] + }; + var result = umbDataFormatter.formatContentGetData(model); + + //make sure the same property reference exists for property 0 and 3 for each variant + for (var i = 1; i < result.variants.length; i++) { + expect(result.variants[0].tabs[0].properties[0]).toBe(result.variants[i].tabs[0].properties[0]); + expect(result.variants[0].tabs[1].properties[3]).toBe(result.variants[i].tabs[1].properties[3]); + } + + //test that changing a property value in one variant is definitely updating the same object reference and therefor + //is done on all variants. + result.variants[0].tabs[0].properties[0].value = "hello"; + for (var i = 1; i < result.variants.length; i++) { + expect(result.variants[0].tabs[0].properties[0].value).toBe("hello"); + } + + }); + + }); +});