From c3e2d2a821dfc5aa7163adec415bfbf9d8e46ca1 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 18 Oct 2018 22:52:49 +1100 Subject: [PATCH] Cleaning up the log/audit trail usage, removing duplicate and unecessary entries and adding new cols --- .../Components/RelateOnCopyComponent.cs | 5 +-- .../Components/RelateOnTrashComponent.cs | 16 ++++++---- src/Umbraco.Core/Models/IAuditItem.cs | 10 ++++++ .../Cache/DefaultCachePolicyTests.cs | 16 +++++----- .../Cache/FullDataSetCachePolicyTests.cs | 32 +++++++++---------- .../Cache/SingleItemsOnlyCachePolicyTests.cs | 6 ++-- .../Repositories/AuditRepositoryTest.cs | 18 +++++------ src/Umbraco.Web.UI.Client/package-lock.json | 15 +++------ src/Umbraco.Web/_Legacy/Packager/Installer.cs | 4 +-- .../PackageInstance/InstalledPackage.cs | 2 +- 10 files changed, 65 insertions(+), 59 deletions(-) diff --git a/src/Umbraco.Core/Components/RelateOnCopyComponent.cs b/src/Umbraco.Core/Components/RelateOnCopyComponent.cs index 4ebd309e9f..bc66dccd31 100644 --- a/src/Umbraco.Core/Components/RelateOnCopyComponent.cs +++ b/src/Umbraco.Core/Components/RelateOnCopyComponent.cs @@ -37,8 +37,9 @@ namespace Umbraco.Core.Components Current.Services.AuditService.Add( AuditType.Copy, - $"Copied content with Id: '{e.Copy.Id}' related to original content with Id: '{e.Original.Id}'", - e.Copy.WriterId, e.Copy.Id); + e.Copy.WriterId, + e.Copy.Id, ObjectTypes.GetName(UmbracoObjectTypes.Document), + $"Copied content with Id: '{e.Copy.Id}' related to original content with Id: '{e.Original.Id}'"); } } } diff --git a/src/Umbraco.Core/Components/RelateOnTrashComponent.cs b/src/Umbraco.Core/Components/RelateOnTrashComponent.cs index fffae85501..8bcce50c68 100644 --- a/src/Umbraco.Core/Components/RelateOnTrashComponent.cs +++ b/src/Umbraco.Core/Components/RelateOnTrashComponent.cs @@ -82,11 +82,12 @@ namespace Umbraco.Core.Components relationService.Save(relation); Current.Services.AuditService.Add(AuditType.Delete, + item.Entity.WriterId, + item.Entity.Id, + ObjectTypes.GetName(UmbracoObjectTypes.Document), string.Format(textService.Localize( "recycleBin/contentTrashed"), - item.Entity.Id, originalParentId), - item.Entity.WriterId, - item.Entity.Id); + item.Entity.Id, originalParentId)); } } } @@ -120,11 +121,12 @@ namespace Umbraco.Core.Components var relation = new Relation(originalParentId, item.Entity.Id, relationType); relationService.Save(relation); Current.Services.AuditService.Add(AuditType.Delete, - string.Format(textService.Localize( + item.Entity.CreatorId, + item.Entity.Id, + ObjectTypes.GetName(UmbracoObjectTypes.Media), + string.Format(textService.Localize( "recycleBin/mediaTrashed"), - item.Entity.Id, originalParentId), - item.Entity.CreatorId, - item.Entity.Id); + item.Entity.Id, originalParentId)); } } } diff --git a/src/Umbraco.Core/Models/IAuditItem.cs b/src/Umbraco.Core/Models/IAuditItem.cs index 9416e2a055..c1dfd99dbb 100644 --- a/src/Umbraco.Core/Models/IAuditItem.cs +++ b/src/Umbraco.Core/Models/IAuditItem.cs @@ -6,6 +6,16 @@ namespace Umbraco.Core.Models public interface IAuditItem : IEntity { string Comment { get; } + + /// + /// The entity type for the log entry + /// + string EntityType { get; } + + /// + /// Optional additional data parameters for the log + /// + string Parameters { get; } AuditType AuditType { get; } int UserId { get; } } diff --git a/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs b/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs index a8021055a9..37488600c7 100644 --- a/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs +++ b/src/Umbraco.Tests/Cache/DefaultCachePolicyTests.cs @@ -38,7 +38,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); - var unused = defaultPolicy.Get(1, id => new AuditItem(1, "blah", AuditType.Copy, 123), o => null); + var unused = defaultPolicy.Get(1, id => new AuditItem(1, AuditType.Copy, 123, "test", "blah"), o => null); Assert.IsTrue(isCached); } @@ -46,7 +46,7 @@ namespace Umbraco.Tests.Cache public void Get_Single_From_Cache() { var cache = new Mock(); - cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, "blah", AuditType.Copy, 123)); + cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, AuditType.Copy, 123, "test", "blah")); var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); @@ -71,8 +71,8 @@ namespace Umbraco.Tests.Cache var unused = defaultPolicy.GetAll(new object[] {}, ids => new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); Assert.AreEqual(2, cached.Count); @@ -84,8 +84,8 @@ namespace Umbraco.Tests.Cache var cache = new Mock(); cache.Setup(x => x.GetCacheItemsByKeySearch(It.IsAny())).Returns(new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); @@ -108,7 +108,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); try { - defaultPolicy.Update(new AuditItem(1, "blah", AuditType.Copy, 123), item => throw new Exception("blah!")); + defaultPolicy.Update(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => throw new Exception("blah!")); } catch { @@ -134,7 +134,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new DefaultRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); try { - defaultPolicy.Delete(new AuditItem(1, "blah", AuditType.Copy, 123), item => throw new Exception("blah!")); + defaultPolicy.Delete(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => throw new Exception("blah!")); } catch { diff --git a/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs b/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs index a275a44964..404587bcfa 100644 --- a/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs +++ b/src/Umbraco.Tests/Cache/FullDataSetCachePolicyTests.cs @@ -32,8 +32,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var isCached = false; @@ -47,7 +47,7 @@ namespace Umbraco.Tests.Cache var policy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); - var unused = policy.Get(1, id => new AuditItem(1, "blah", AuditType.Copy, 123), ids => getAll); + var unused = policy.Get(1, id => new AuditItem(1, AuditType.Copy, 123, "test", "blah"), ids => getAll); Assert.IsTrue(isCached); } @@ -56,12 +56,12 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cache = new Mock(); - cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, "blah", AuditType.Copy, 123)); + cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(new AuditItem(1, AuditType.Copy, 123, "test", "blah")); var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); @@ -114,8 +114,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cached = new List(); @@ -149,8 +149,8 @@ namespace Umbraco.Tests.Cache cache.Setup(x => x.GetCacheItem(It.IsAny())).Returns(() => new DeepCloneableList(ListCloneBehavior.CloneOnce) { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); @@ -164,8 +164,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cacheCleared = false; @@ -179,7 +179,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); try { - defaultPolicy.Update(new AuditItem(1, "blah", AuditType.Copy, 123), item => { throw new Exception("blah!"); }); + defaultPolicy.Update(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => { throw new Exception("blah!"); }); } catch { @@ -196,8 +196,8 @@ namespace Umbraco.Tests.Cache { var getAll = new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }; var cacheCleared = false; @@ -211,7 +211,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new FullDataSetRepositoryCachePolicy(cache.Object, DefaultAccessor, item => item.Id, false); try { - defaultPolicy.Delete(new AuditItem(1, "blah", AuditType.Copy, 123), item => { throw new Exception("blah!"); }); + defaultPolicy.Delete(new AuditItem(1, AuditType.Copy, 123, "test", "blah"), item => { throw new Exception("blah!"); }); } catch { diff --git a/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs b/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs index 9ab98bda7e..1c2227f79b 100644 --- a/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs +++ b/src/Umbraco.Tests/Cache/SingleItemsOnlyCachePolicyTests.cs @@ -41,8 +41,8 @@ namespace Umbraco.Tests.Cache var unused = defaultPolicy.GetAll(new object[] { }, ids => new[] { - new AuditItem(1, "blah", AuditType.Copy, 123), - new AuditItem(2, "blah2", AuditType.Copy, 123) + new AuditItem(1, AuditType.Copy, 123, "test", "blah"), + new AuditItem(2, AuditType.Copy, 123, "test", "blah2") }); Assert.AreEqual(0, cached.Count); @@ -62,7 +62,7 @@ namespace Umbraco.Tests.Cache var defaultPolicy = new SingleItemsOnlyRepositoryCachePolicy(cache.Object, DefaultAccessor, new RepositoryCachePolicyOptions()); - var unused = defaultPolicy.Get(1, id => new AuditItem(1, "blah", AuditType.Copy, 123), ids => null); + var unused = defaultPolicy.Get(1, id => new AuditItem(1, AuditType.Copy, 123, "test", "blah"), ids => null); Assert.IsTrue(isCached); } } diff --git a/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs index 6953634a31..eb85656ee4 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs @@ -27,7 +27,7 @@ namespace Umbraco.Tests.Persistence.Repositories using (var scope = sp.CreateScope()) { var repo = new AuditRepository((IScopeAccessor) sp, CacheHelper, Logger); - repo.Save(new AuditItem(-1, "This is a System audit trail", AuditType.System, -1)); + repo.Save(new AuditItem(-1, AuditType.System, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), "This is a System audit trail")); var dtos = scope.Database.Fetch("WHERE id > -1"); @@ -46,8 +46,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, $"Content {i} created", AuditType.New, -1)); - repo.Save(new AuditItem(i, $"Content {i} published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} published")); } scope.Complete(); @@ -74,8 +74,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, $"Content {i} created", AuditType.New, -1)); - repo.Save(new AuditItem(i, $"Content {i} published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} published")); } scope.Complete(); @@ -117,8 +117,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, $"Content {i} created", AuditType.New, -1)); - repo.Save(new AuditItem(i, $"Content {i} published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), $"Content {i} published")); } scope.Complete(); @@ -148,8 +148,8 @@ namespace Umbraco.Tests.Persistence.Repositories for (var i = 0; i < 100; i++) { - repo.Save(new AuditItem(i, "Content created", AuditType.New, -1)); - repo.Save(new AuditItem(i, "Content published", AuditType.Publish, -1)); + repo.Save(new AuditItem(i, AuditType.New, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), "Content created")); + repo.Save(new AuditItem(i, AuditType.Publish, -1, ObjectTypes.GetName(UmbracoObjectTypes.Document), "Content published")); } scope.Complete(); diff --git a/src/Umbraco.Web.UI.Client/package-lock.json b/src/Umbraco.Web.UI.Client/package-lock.json index 4b8cc3e6c2..6cce6d7708 100644 --- a/src/Umbraco.Web.UI.Client/package-lock.json +++ b/src/Umbraco.Web.UI.Client/package-lock.json @@ -5505,8 +5505,7 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", @@ -5516,8 +5515,7 @@ "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -5634,8 +5632,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -5647,7 +5644,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -5773,8 +5769,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -5786,7 +5781,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -5908,7 +5902,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", diff --git a/src/Umbraco.Web/_Legacy/Packager/Installer.cs b/src/Umbraco.Web/_Legacy/Packager/Installer.cs index 2581e3ad48..ae6b3e0a11 100644 --- a/src/Umbraco.Web/_Legacy/Packager/Installer.cs +++ b/src/Umbraco.Web/_Legacy/Packager/Installer.cs @@ -323,8 +323,8 @@ namespace umbraco.cms.businesslogic.packager if (_currentUserId > -1) { Current.Services.AuditService.Add(AuditType.PackagerInstall, - string.Format("Package '{0}' installed. Package guid: {1}", insPack.Data.Name, insPack.Data.PackageGuid), - _currentUserId, -1); + _currentUserId, + -1, "Package", string.Format("Package '{0}' installed. Package guid: {1}", insPack.Data.Name, insPack.Data.PackageGuid)); } insPack.Save(); diff --git a/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs b/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs index c16afa0b84..ae4d23aa9a 100644 --- a/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs +++ b/src/Umbraco.Web/_Legacy/Packager/PackageInstance/InstalledPackage.cs @@ -67,7 +67,7 @@ namespace umbraco.cms.businesslogic.packager { public void Delete(int userId) { - Current.Services.AuditService.Add(AuditType.PackagerUninstall, string.Format("Package '{0}' uninstalled. Package guid: {1}", Data.Name, Data.PackageGuid), userId, -1); + Current.Services.AuditService.Add(AuditType.PackagerUninstall, userId, -1, "Package", string.Format("Package '{0}' uninstalled. Package guid: {1}", Data.Name, Data.PackageGuid)); Delete(); }