diff --git a/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs b/src/Umbraco.Core/Models/EntityBase/TracksChangesEntityBase.cs index 9a357b3b38..8e95a1a7e2 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 30214985f7..ba9b89d779 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 51de2b2cab..305c630a71 100644 --- a/src/Umbraco.Core/Persistence/Factories/UserFactory.cs +++ b/src/Umbraco.Core/Persistence/Factories/UserFactory.cs @@ -72,7 +72,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 34775df456..67775cca54 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs @@ -210,9 +210,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 @@ -226,13 +236,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 61eda9633d..1501cf09f3 100644 --- a/src/Umbraco.Tests/Models/ContentTests.cs +++ b/src/Umbraco.Tests/Models/ContentTests.cs @@ -272,6 +272,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 8154b60ae2..0021420517 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 b1f0895022..f8c9429e80 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -390,6 +390,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 9ec1c69546..2c71e529a4 100644 --- a/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs +++ b/src/Umbraco.Web/umbraco.presentation/umbraco/users/EditUser.aspx.cs @@ -491,10 +491,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 ffa6c790e0..798e4dc46c 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); }