From b763c2ab2f598d1ba125d6e923e5493264f44790 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 9 May 2014 15:56:45 +1000 Subject: [PATCH 1/6] Fixes deep clone of User object --- src/Umbraco.Core/Models/Membership/User.cs | 40 ++++++++++++++++------ src/Umbraco.Tests/Models/UserTests.cs | 12 +++++++ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 2e9a73ba50..2e963f5dd7 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -59,9 +59,9 @@ namespace Umbraco.Core.Models.Membership private IUserType _userType; private string _name; private Type _userTypeKey; - private readonly List _addedSections; - private readonly List _removedSections; - private readonly ObservableCollection _sectionCollection; + private List _addedSections; + private List _removedSections; + private ObservableCollection _sectionCollection; private int _sessionTimeout; private int _startContentId; private int _startMediaId; @@ -423,24 +423,44 @@ namespace Umbraco.Core.Models.Membership if (e.Action == NotifyCollectionChangedAction.Add) { + var item = e.NewItems.Cast().First(); + //remove from the removed/added sections (since people could add/remove all they want in one request) - _removedSections.RemoveAll(s => s == e.NewItems.Cast().First()); - _addedSections.RemoveAll(s => s == e.NewItems.Cast().First()); + _removedSections.Remove(item); + _addedSections.Remove(item); //add to the added sections - _addedSections.Add(e.NewItems.Cast().First()); + _addedSections.Add(item); } else if (e.Action == NotifyCollectionChangedAction.Remove) { + var item = e.OldItems.Cast().First(); + //remove from the removed/added sections (since people could add/remove all they want in one request) - _removedSections.RemoveAll(s => s == e.OldItems.Cast().First()); - _addedSections.RemoveAll(s => s == e.OldItems.Cast().First()); + _removedSections.Remove(item); + _addedSections.Remove(item); //add to the added sections - _removedSections.Add(e.OldItems.Cast().First()); + _removedSections.Add(item); } } - + + public override object DeepClone() + { + var clone = (User)base.DeepClone(); + + //need to create new collections otherwise they'll get copied by ref + clone._addedSections = new List(); + clone._removedSections = new List(); + clone._sectionCollection = new ObservableCollection(); + //re-create the event handler + clone._sectionCollection.CollectionChanged += clone.SectionCollectionChanged; + + clone.ResetDirtyProperties(false); + + return clone; + } + /// /// Internal class used to wrap the user in a profile /// diff --git a/src/Umbraco.Tests/Models/UserTests.cs b/src/Umbraco.Tests/Models/UserTests.cs index 84b0fe317d..94eadf7996 100644 --- a/src/Umbraco.Tests/Models/UserTests.cs +++ b/src/Umbraco.Tests/Models/UserTests.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using NUnit.Framework; using Umbraco.Core.Models.Membership; using Umbraco.Core.Serialization; @@ -53,6 +54,17 @@ namespace Umbraco.Tests.Models { Assert.AreEqual(propertyInfo.GetValue(clone, null), propertyInfo.GetValue(item, null)); } + + //ensure internal collections are differet + Assert.AreNotSame(item.AddedSections, clone.AddedSections); + Assert.AreNotSame(item.RemovedSections, clone.RemovedSections); + + //ensure event handlers are still wired on clone + clone.AddAllowedSection("blah"); + Assert.AreEqual(1, clone.AddedSections.Count()); + clone.RemoveAllowedSection("blah"); + Assert.AreEqual(1, clone.RemovedSections.Count()); + } [Test] From 3b424274ee356da6e3ac1e1b05d39a72c0c1205c Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 9 May 2014 15:57:17 +1000 Subject: [PATCH 2/6] ensure properties reset on deep clone of ContentType (this is just a fail safe check) --- src/Umbraco.Core/Models/ContentType.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Umbraco.Core/Models/ContentType.cs b/src/Umbraco.Core/Models/ContentType.cs index ff61f0f9a4..1cf7fc9684 100644 --- a/src/Umbraco.Core/Models/ContentType.cs +++ b/src/Umbraco.Core/Models/ContentType.cs @@ -151,6 +151,9 @@ namespace Umbraco.Core.Models clone.PropertyTypes = PropertyTypeCollection .Where(x => x.PropertyGroupId == null) .Select(x => (PropertyType)x.DeepClone()).ToList(); + + clone.ResetDirtyProperties(false); + return clone; } From d61fb5d94d1c734dae6afd53ef117adb071fe514 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 9 May 2014 15:59:30 +1000 Subject: [PATCH 3/6] Fixes user cloning issues with allowed sections --- src/Umbraco.Core/Models/Membership/User.cs | 2 +- src/Umbraco.Tests/Models/UserTests.cs | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 2e963f5dd7..88a0df14ef 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -452,7 +452,7 @@ namespace Umbraco.Core.Models.Membership //need to create new collections otherwise they'll get copied by ref clone._addedSections = new List(); clone._removedSections = new List(); - clone._sectionCollection = new ObservableCollection(); + clone._sectionCollection = new ObservableCollection(_sectionCollection.ToList()); //re-create the event handler clone._sectionCollection.CollectionChanged += clone.SectionCollectionChanged; diff --git a/src/Umbraco.Tests/Models/UserTests.cs b/src/Umbraco.Tests/Models/UserTests.cs index 94eadf7996..7fa9742870 100644 --- a/src/Umbraco.Tests/Models/UserTests.cs +++ b/src/Umbraco.Tests/Models/UserTests.cs @@ -40,6 +40,8 @@ namespace Umbraco.Tests.Models Username = "username" }; + item.AddAllowedSection("test"); + var clone = (User)item.DeepClone(); Assert.AreNotSame(clone, item); @@ -47,6 +49,7 @@ namespace Umbraco.Tests.Models Assert.AreNotSame(clone.UserType, item.UserType); Assert.AreEqual(clone.UserType, item.UserType); + Assert.AreEqual(clone.AllowedSections.Count(), item.AllowedSections.Count()); //Verify normal properties with reflection var allProps = clone.GetType().GetProperties(); From b91a06ab98e3a1764be7cce6776ca782073487ed Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 9 May 2014 16:00:30 +1000 Subject: [PATCH 4/6] Fixes: U4-4849 Cannot change a users language/sections in the back office --- .../EntityBase/TracksChangesEntityBase.cs | 10 ++++-- src/Umbraco.Core/Models/Membership/User.cs | 24 ++++++------- .../Persistence/Factories/UserFactory.cs | 2 +- .../Repositories/UserRepository.cs | 18 ++++++---- src/Umbraco.Tests/Models/ContentTests.cs | 8 +++++ src/Umbraco.Tests/Models/ContentTypeTests.cs | 20 +++++++++-- .../Services/UserServiceTests.cs | 35 +++++++++++++++++++ .../umbraco/users/EditUser.aspx.cs | 4 +-- src/umbraco.businesslogic/User.cs | 26 ++++++++++++++ 9 files changed, 119 insertions(+), 28 deletions(-) diff --git a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs index 1d23fe8385..cfc6ce8c9d 100644 --- a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs +++ b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs @@ -17,7 +17,7 @@ namespace Umbraco.Core.Models.EntityBase /// /// Tracks the properties that have changed /// - private readonly IDictionary _propertyChangedInfo = new Dictionary(); + private IDictionary _propertyChangedInfo = new Dictionary(); /// /// Tracks the properties that we're changed before the last commit (or last call to ResetDirtyProperties) @@ -86,7 +86,9 @@ namespace Umbraco.Core.Models.EntityBase /// public void ForgetPreviouslyDirtyProperties() { - _lastPropertyChangedInfo.Clear(); + //NOTE: We cannot .Clear() because when we memberwise clone this will be the SAME + // instance as the one on the clone, so we need to create a new instance. + _lastPropertyChangedInfo = new Dictionary(); } /// @@ -119,7 +121,9 @@ namespace Umbraco.Core.Models.EntityBase _lastPropertyChangedInfo = _propertyChangedInfo.ToDictionary(v => v.Key, v => v.Value); } - _propertyChangedInfo.Clear(); + //NOTE: We cannot .Clear() because when we memberwise clone this will be the SAME + // instance as the one on the clone, so we need to create a new instance. + _propertyChangedInfo = new Dictionary(); } /// diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 88a0df14ef..45bc4aa375 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -244,7 +244,10 @@ namespace Umbraco.Core.Models.Membership public void RemoveAllowedSection(string sectionAlias) { - _sectionCollection.Remove(sectionAlias); + if (_sectionCollection.Contains(sectionAlias)) + { + _sectionCollection.Remove(sectionAlias); + } } public void AddAllowedSection(string sectionAlias) @@ -425,23 +428,20 @@ namespace Umbraco.Core.Models.Membership { var item = e.NewItems.Cast().First(); - //remove from the removed/added sections (since people could add/remove all they want in one request) - _removedSections.Remove(item); - _addedSections.Remove(item); - - //add to the added sections - _addedSections.Add(item); + if (_addedSections.Contains(item) == false) + { + _addedSections.Add(item); + } } else if (e.Action == NotifyCollectionChangedAction.Remove) { var item = e.OldItems.Cast().First(); - //remove from the removed/added sections (since people could add/remove all they want in one request) - _removedSections.Remove(item); - _addedSections.Remove(item); + if (_removedSections.Contains(item) == false) + { + _removedSections.Add(item); + } - //add to the added sections - _removedSections.Add(item); } } diff --git a/src/Umbraco.Core/Persistence/Factories/UserFactory.cs b/src/Umbraco.Core/Persistence/Factories/UserFactory.cs index 080776e7fe..0f2ce2c5e6 100644 --- a/src/Umbraco.Core/Persistence/Factories/UserFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/UserFactory.cs @@ -74,7 +74,7 @@ namespace Umbraco.Core.Persistence.Factories { AppAlias = app }; - if (entity.Id != null) + if (entity.HasIdentity) { appDto.UserId = (int) entity.Id; } diff --git a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs index a2b91a8447..07714b36a7 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs @@ -211,9 +211,19 @@ namespace Umbraco.Core.Persistence.Repositories var user = (User)entity; if (user.IsPropertyDirty("AllowedSections")) { + //now we need to delete any applications that have been removed + foreach (var section in user.RemovedSections) + { + //we need to manually delete thsi record because it has a composite key + Database.Delete("WHERE app=@Section AND " + SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName("user") + "=@UserId", + new { Section = section, UserId = (int)user.Id }); + } + //for any that exist on the object, we need to determine if we need to update or insert + //NOTE: the User2AppDtos collection wil always be equal to the User.AllowedSections foreach (var sectionDto in userDto.User2AppDtos) { + //if something has been added then insert it if (user.AddedSections.Contains(sectionDto.AppAlias)) { //we need to insert since this was added @@ -227,13 +237,7 @@ namespace Umbraco.Core.Persistence.Repositories } } - //now we need to delete any applications that have been removed - foreach (var section in user.RemovedSections) - { - //we need to manually delete thsi record because it has a composite key - Database.Delete("WHERE app=@Section AND " + SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName("user") + "=@UserId", - new { Section = section, UserId = (int)user.Id }); - } + } ((ICanBeDirty)entity).ResetDirtyProperties(); diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 6f9dbc5d46..6ca360c81b 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -224,6 +224,14 @@ namespace Umbraco.Tests.Models { Assert.AreEqual(propertyInfo.GetValue(clone, null), propertyInfo.GetValue(content, null)); } + + //need to ensure the event handlers are wired + + var asDirty = (ICanBeDirty)clone; + + Assert.IsFalse(asDirty.IsPropertyDirty("Properties")); + clone.Properties.Add(new Property(1, Guid.NewGuid(), new PropertyType("test", DataTypeDatabaseType.Ntext) {Alias = "blah"}, "blah")); + Assert.IsTrue(asDirty.IsPropertyDirty("Properties")); } [Test] diff --git a/src/Umbraco.Tests/Models/ContentTypeTests.cs b/src/Umbraco.Tests/Models/ContentTypeTests.cs index 0fc1a52a28..be11337599 100644 --- a/src/Umbraco.Tests/Models/ContentTypeTests.cs +++ b/src/Umbraco.Tests/Models/ContentTypeTests.cs @@ -4,12 +4,13 @@ using NUnit.Framework; using Umbraco.Core.Models; using Umbraco.Core.Models.EntityBase; using Umbraco.Core.Serialization; +using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; namespace Umbraco.Tests.Models { [TestFixture] - public class ContentTypeTests + public class ContentTypeTests : BaseUmbracoConfigurationTest { [Test] public void Can_Deep_Clone_Content_Type_Sort() @@ -140,19 +141,21 @@ namespace Umbraco.Tests.Models Assert.AreNotSame(clone.AllowedTemplates.ElementAt(index), contentType.AllowedTemplates.ElementAt(index)); Assert.AreEqual(clone.AllowedTemplates.ElementAt(index), contentType.AllowedTemplates.ElementAt(index)); } + Assert.AreNotSame(clone.PropertyGroups, contentType.PropertyGroups); Assert.AreEqual(clone.PropertyGroups.Count, contentType.PropertyGroups.Count); for (var index = 0; index < contentType.PropertyGroups.Count; index++) { Assert.AreNotSame(clone.PropertyGroups[index], contentType.PropertyGroups[index]); Assert.AreEqual(clone.PropertyGroups[index], contentType.PropertyGroups[index]); } + Assert.AreNotSame(clone.PropertyTypes, contentType.PropertyTypes); Assert.AreEqual(clone.PropertyTypes.Count(), contentType.PropertyTypes.Count()); for (var index = 0; index < contentType.PropertyTypes.Count(); index++) { Assert.AreNotSame(clone.PropertyTypes.ElementAt(index), contentType.PropertyTypes.ElementAt(index)); Assert.AreEqual(clone.PropertyTypes.ElementAt(index), contentType.PropertyTypes.ElementAt(index)); } - + Assert.AreEqual(clone.CreateDate, contentType.CreateDate); Assert.AreEqual(clone.CreatorId, contentType.CreatorId); Assert.AreEqual(clone.Key, contentType.Key); @@ -167,13 +170,24 @@ namespace Umbraco.Tests.Models Assert.AreEqual(clone.Thumbnail, contentType.Thumbnail); Assert.AreEqual(clone.Icon, contentType.Icon); Assert.AreEqual(clone.IsContainer, contentType.IsContainer); - + //This double verifies by reflection var allProps = clone.GetType().GetProperties(); foreach (var propertyInfo in allProps) { Assert.AreEqual(propertyInfo.GetValue(clone, null), propertyInfo.GetValue(contentType, null)); } + + //need to ensure the event handlers are wired + + var asDirty = (ICanBeDirty)clone; + + Assert.IsFalse(asDirty.IsPropertyDirty("PropertyTypes")); + clone.AddPropertyType(new PropertyType("test", DataTypeDatabaseType.Nvarchar) { Alias = "blah" }); + Assert.IsTrue(asDirty.IsPropertyDirty("PropertyTypes")); + Assert.IsFalse(asDirty.IsPropertyDirty("PropertyGroups")); + clone.AddPropertyGroup("hello"); + Assert.IsTrue(asDirty.IsPropertyDirty("PropertyGroups")); } [Test] diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index f0fdc8a21d..5c41895037 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -387,6 +387,41 @@ namespace Umbraco.Tests.Services Assert.That(user.DefaultPermissions, Is.EqualTo(userType.Permissions)); } + [Test] + public void Can_Add_And_Remove_Sections_From_User() + { + var userType = ServiceContext.UserService.GetUserTypeByAlias("admin"); + + var user1 = ServiceContext.UserService.CreateUserWithIdentity("test1", "test1@test.com", userType); + + //adds some allowed sections + user1.AddAllowedSection("test1"); + user1.AddAllowedSection("test2"); + user1.AddAllowedSection("test3"); + user1.AddAllowedSection("test4"); + ServiceContext.UserService.Save(user1); + + var result1 = ServiceContext.UserService.GetUserById((int)user1.Id); + Assert.AreEqual(4, result1.AllowedSections.Count()); + + //simulate clearing the sections + foreach (var s in user1.AllowedSections) + { + result1.RemoveAllowedSection(s); + } + //now just re-add a couple + result1.AddAllowedSection("test3"); + result1.AddAllowedSection("test4"); + ServiceContext.UserService.Save(result1); + + //assert + + //re-get + result1 = ServiceContext.UserService.GetUserById((int)user1.Id); + Assert.AreEqual(2, result1.AllowedSections.Count()); + + } + [Test] public void Can_Remove_Section_From_All_Assigned_Users() { diff --git a/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs b/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs index 1f0262a1d0..b1d58bccea 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs @@ -489,10 +489,10 @@ namespace umbraco.cms.presentation.user } u.StartMediaId = mstartNode; - u.clearApplications(); + u.ClearApplications(); foreach (ListItem li in lapps.Items) { - if (li.Selected) u.addApplication(li.Value); + if (li.Selected) u.AddApplication(li.Value); } u.Save(); diff --git a/src/umbraco.businesslogic/User.cs b/src/umbraco.businesslogic/User.cs index ef1b736d73..3a135d1834 100644 --- a/src/umbraco.businesslogic/User.cs +++ b/src/umbraco.businesslogic/User.cs @@ -689,9 +689,22 @@ namespace umbraco.BusinessLogic get { return _user.Id; } } + /// + /// Clears the list of applications the user has access to, ensure to call Save afterwords + /// + public void ClearApplications() + { + if (_lazyId.HasValue) SetupUser(_lazyId.Value); + foreach (var s in _user.AllowedSections.ToArray()) + { + _user.RemoveAllowedSection(s); + } + } + /// /// Clears the list of applications the user has access to. /// + [Obsolete("This method will implicitly cause a database save and will reset the current user's dirty property, do not use this method, use the ClearApplications method instead and then call Save() when you are done performing all user changes to persist the chagnes in one transaction")] public void clearApplications() { if (_lazyId.HasValue) SetupUser(_lazyId.Value); @@ -701,19 +714,32 @@ namespace umbraco.BusinessLogic _user.RemoveAllowedSection(s); } + //For backwards compatibility this requires an implicit save ApplicationContext.Current.Services.UserService.Save(_user); } + /// + /// Adds a application to the list of allowed applications, ensure to call Save() afterwords + /// + /// + public void AddApplication(string appAlias) + { + if (_lazyId.HasValue) SetupUser(_lazyId.Value); + _user.AddAllowedSection(appAlias); + } + /// /// Adds a application to the list of allowed applications /// /// The app alias. + [Obsolete("This method will implicitly cause a multiple database saves and will reset the current user's dirty property, do not use this method, use the AddApplication method instead and then call Save() when you are done performing all user changes to persist the chagnes in one transaction")] public void addApplication(string AppAlias) { if (_lazyId.HasValue) SetupUser(_lazyId.Value); _user.AddAllowedSection(AppAlias); + //For backwards compatibility this requires an implicit save ApplicationContext.Current.Services.UserService.Save(_user); } From da0b935604f815759614f2550bd625f31a0c42d2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Fri, 9 May 2014 16:08:05 +1000 Subject: [PATCH 5/6] Fixes merge --- src/Umbraco.Tests/Models/ContentTests.cs | 2 +- src/Umbraco.Tests/Models/ContentTypeTests.cs | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests/Models/ContentTests.cs b/src/Umbraco.Tests/Models/ContentTests.cs index 6ca360c81b..a7df4ec7d5 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -230,7 +230,7 @@ namespace Umbraco.Tests.Models var asDirty = (ICanBeDirty)clone; Assert.IsFalse(asDirty.IsPropertyDirty("Properties")); - clone.Properties.Add(new Property(1, Guid.NewGuid(), new PropertyType("test", DataTypeDatabaseType.Ntext) {Alias = "blah"}, "blah")); + clone.Properties.Add(new Property(1, Guid.NewGuid(), new PropertyType(Guid.NewGuid(), DataTypeDatabaseType.Ntext) { Alias = "blah" }, "blah")); Assert.IsTrue(asDirty.IsPropertyDirty("Properties")); } diff --git a/src/Umbraco.Tests/Models/ContentTypeTests.cs b/src/Umbraco.Tests/Models/ContentTypeTests.cs index be11337599..dc2de0d6bb 100644 --- a/src/Umbraco.Tests/Models/ContentTypeTests.cs +++ b/src/Umbraco.Tests/Models/ContentTypeTests.cs @@ -10,8 +10,20 @@ using Umbraco.Tests.TestHelpers.Entities; namespace Umbraco.Tests.Models { [TestFixture] - public class ContentTypeTests : BaseUmbracoConfigurationTest + public class ContentTypeTests { + [SetUp] + public void Init() + { + TestHelper.EnsureUmbracoSettingsConfig(); + } + + [TearDown] + public void Dispose() + { + TestHelper.CleanUmbracoSettingsConfig(); + } + [Test] public void Can_Deep_Clone_Content_Type_Sort() { @@ -183,7 +195,7 @@ namespace Umbraco.Tests.Models var asDirty = (ICanBeDirty)clone; Assert.IsFalse(asDirty.IsPropertyDirty("PropertyTypes")); - clone.AddPropertyType(new PropertyType("test", DataTypeDatabaseType.Nvarchar) { Alias = "blah" }); + clone.AddPropertyType(new PropertyType(Guid.NewGuid(), DataTypeDatabaseType.Nvarchar) { Alias = "blah" }); Assert.IsTrue(asDirty.IsPropertyDirty("PropertyTypes")); Assert.IsFalse(asDirty.IsPropertyDirty("PropertyGroups")); clone.AddPropertyGroup("hello"); From 4e5019d5af2f0785e95fd1bc885e62da2e2c2448 Mon Sep 17 00:00:00 2001 From: Stephan Date: Sun, 11 May 2014 21:32:15 +0200 Subject: [PATCH 6/6] U4-4851 - Umbraco.Field legacy names --- src/Umbraco.Web/umbraco.presentation/item.cs | 26 ++++++++++++++------ 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Web/umbraco.presentation/item.cs b/src/Umbraco.Web/umbraco.presentation/item.cs index 0920618f60..9e0a41fb54 100644 --- a/src/Umbraco.Web/umbraco.presentation/item.cs +++ b/src/Umbraco.Web/umbraco.presentation/item.cs @@ -78,26 +78,38 @@ namespace umbraco else { //check for published content and get its value using that - if (publishedContent != null) + if (publishedContent != null && publishedContent.HasProperty(_fieldName)) { var pval = publishedContent.GetPropertyValue(_fieldName); var rval = pval == null ? string.Empty : pval.ToString(); _fieldContent = rval.IsNullOrWhiteSpace() ? _fieldContent : rval; } - else if (elements[_fieldName] != null && string.IsNullOrEmpty(elements[_fieldName].ToString()) == false) - { + else + { //get the vaue the legacy way (this will not parse locallinks, etc... since that is handled with ipublishedcontent) - _fieldContent = elements[_fieldName].ToString().Trim(); + var elt = elements[_fieldName]; + if (elt != null && string.IsNullOrEmpty(elt.ToString()) == false) + _fieldContent = elt.ToString().Trim(); } //now we check if the value is still empty and if so we'll check useIfEmpty if (string.IsNullOrEmpty(_fieldContent)) { - if (string.IsNullOrEmpty(helper.FindAttribute(attributes, "useIfEmpty")) == false) + var altFieldName = helper.FindAttribute(attributes, "useIfEmpty"); + if (string.IsNullOrEmpty(altFieldName) == false) { - if (elements[helper.FindAttribute(attributes, "useIfEmpty")] != null && string.IsNullOrEmpty(elements[helper.FindAttribute(attributes, "useIfEmpty")].ToString()) == false) + if (publishedContent != null && publishedContent.HasProperty(altFieldName)) { - _fieldContent = elements[helper.FindAttribute(attributes, "useIfEmpty")].ToString().Trim(); + var pval = publishedContent.GetPropertyValue(altFieldName); + var rval = pval == null ? string.Empty : pval.ToString(); + _fieldContent = rval.IsNullOrWhiteSpace() ? _fieldContent : rval; + } + else + { + //get the vaue the legacy way (this will not parse locallinks, etc... since that is handled with ipublishedcontent) + var elt = elements[altFieldName]; + if (elt != null && string.IsNullOrEmpty(elt.ToString()) == false) + _fieldContent = elt.ToString().Trim(); } } }