Add auditing and logging when renaming a player identity #626

Stoolball.Data.SqlServer.IntegrationTests/Statistics/SqlServerPlayerRepositoryTests.cs

@@ -569,7 +569,7 @@ public async Task UpdatePlayerIdentity_throws_ArgumentNullException_if_playerIde
         public async Task UpdatePlayerIdentity_throws_ArgumentException_if_PlayerIdentityId_is_null()
         {
             var repo = CreateRepository();
-            var playerIdentity = new PlayerIdentity { PlayerIdentityName = "Example name", Team = new Team { TeamId = Guid.NewGuid() } };
+            var playerIdentity = new PlayerIdentity { PlayerIdentityName = "Example name", Team = new Team { TeamId = Guid.NewGuid() }, Player = new Player { PlayerId = Guid.NewGuid() } };
 
             await Assert.ThrowsAsync<ArgumentException>(async () => await repo.UpdatePlayerIdentity(playerIdentity, Guid.NewGuid(), "Member name"));
         }
@@ -581,7 +581,7 @@ public async Task UpdatePlayerIdentity_throws_ArgumentException_if_PlayerIdentit
         public async Task UpdatePlayerIdentity_throws_ArgumentException_if_PlayerIdentityName_is_missing(string? playerIdentityName)
         {
             var repo = CreateRepository();
-            var playerIdentity = new PlayerIdentity { PlayerIdentityId = Guid.NewGuid(), PlayerIdentityName = playerIdentityName, Team = new Team { TeamId = Guid.NewGuid() } };
+            var playerIdentity = new PlayerIdentity { PlayerIdentityId = Guid.NewGuid(), PlayerIdentityName = playerIdentityName, Team = new Team { TeamId = Guid.NewGuid() }, Player = new Player { PlayerId = Guid.NewGuid() } };
 
             await Assert.ThrowsAsync<ArgumentException>(async () => await repo.UpdatePlayerIdentity(playerIdentity, Guid.NewGuid(), "Member name"));
         }
@@ -590,7 +590,17 @@ public async Task UpdatePlayerIdentity_throws_ArgumentException_if_PlayerIdentit
         public async Task UpdatePlayerIdentity_throws_ArgumentException_if_TeamId_is_null()
         {
             var repo = CreateRepository();
-            var playerIdentity = new PlayerIdentity { PlayerIdentityId = Guid.NewGuid(), PlayerIdentityName = "Example name", Team = new Team() };
+            var playerIdentity = new PlayerIdentity { PlayerIdentityId = Guid.NewGuid(), PlayerIdentityName = "Example name", Team = new Team(), Player = new Player { PlayerId = Guid.NewGuid() } };
+
+            await Assert.ThrowsAsync<ArgumentException>(async () => await repo.UpdatePlayerIdentity(playerIdentity, Guid.NewGuid(), "Member name"));
+        }
+
+
+        [Fact]
+        public async Task UpdatePlayerIdentity_throws_ArgumentException_if_PlayerId_is_null()
+        {
+            var repo = CreateRepository();
+            var playerIdentity = new PlayerIdentity { PlayerIdentityId = Guid.NewGuid(), PlayerIdentityName = "Example name", Team = new Team { TeamId = Guid.NewGuid() }, Player = new Player() };
 
             await Assert.ThrowsAsync<ArgumentException>(async () => await repo.UpdatePlayerIdentity(playerIdentity, Guid.NewGuid(), "Member name"));
         }
@@ -628,6 +638,7 @@ public async Task UpdatePlayerIdentity_where_name_matches_same_player_identity_r
             var repo = CreateRepository();
 
             var playerIdentityToUpdate = _databaseFixture.TestData.PlayerIdentities[0];
+            _copier.Setup(x => x.CreateAuditableCopy(playerIdentityToUpdate.Player)).Returns(new Player());
 
             var result = await repo.UpdatePlayerIdentity(playerIdentityToUpdate, Guid.NewGuid(), "Member name");
 
@@ -654,6 +665,8 @@ public async Task UpdatePlayerIdentity_where_name_does_not_match_existing_player
                 Player = playerIdentityToUpdate.Player
             };
 
+            _copier.Setup(x => x.CreateAuditableCopy(updatedPlayerIdentity.Player)).Returns(new Player());
+
             var result = await repo.UpdatePlayerIdentity(updatedPlayerIdentity, Guid.NewGuid(), "Member name");
             await repo.ProcessAsyncUpdatesForPlayers();
 

Stoolball.Data.SqlServer.UnitTests/SqlServerPlayerRepositoryUnitTests.cs

@@ -7,7 +7,9 @@
 using Stoolball.Logging;
 using Stoolball.Routing;
 using Stoolball.Statistics;
+using Stoolball.Teams;
 using Xunit;
+using static Stoolball.Constants;
 
 namespace Stoolball.Data.SqlServer.UnitTests
 {
@@ -24,10 +26,12 @@ public class SqlServerPlayerRepositoryUnitTests
         private readonly Mock<IBestRouteSelector> _routeSelector = new();
         private readonly Mock<IRedirectsRepository> _redirectsRepository = new();
         private readonly Mock<IPlayerCacheInvalidator> _playerCacheClearer = new();
+        private readonly Mock<IDbTransaction> _transaction = new();
 
         public SqlServerPlayerRepositoryUnitTests()
         {
             _connectionFactory.Setup(x => x.CreateDatabaseConnection()).Returns(_databaseConnection.Object);
+            _databaseConnection.Setup(x => x.BeginTransaction()).Returns(_transaction.Object);
         }
 
         private SqlServerPlayerRepository CreateRepository()
@@ -115,5 +119,34 @@ public async Task ProcessAsyncUpdates_logs_SQL_connection_error_and_exits()
                 SqlExceptionFactory.ERROR_ESTABLISHING_CONNECTION
             ), Times.Exactly(1));
         }
+
+        [Fact]
+        public async Task UpdatePlayerIdentity_where_name_does_not_match_existing_player_identity_audits_and_logs()
+        {
+            var repo = CreateRepository();
+
+            var playerIdentityToUpdate = new PlayerIdentity
+            {
+                PlayerIdentityId = Guid.NewGuid(),
+                PlayerIdentityName = "New name",
+                Player = new Player
+                {
+                    PlayerId = Guid.NewGuid(),
+                },
+                Team = new Team
+                {
+                    TeamId = Guid.NewGuid()
+                }
+            };
+            var memberKey = Guid.NewGuid();
+            var memberName = "Member name";
+
+            _copier.Setup(x => x.CreateAuditableCopy(playerIdentityToUpdate.Player)).Returns(new Player());
+
+            var result = await repo.UpdatePlayerIdentity(playerIdentityToUpdate, memberKey, memberName);
+
+            _auditRepository.Verify(x => x.CreateAudit(It.Is<AuditRecord>(a => a.Action == AuditAction.Update), _transaction.Object), Times.Once);
+            _logger.Verify(x => x.Info(LoggingTemplates.Updated, It.IsAny<string>(), memberName, memberKey, typeof(SqlServerPlayerRepository), nameof(SqlServerPlayerRepository.UpdatePlayerIdentity)), Times.Once);
+        }
     }
 }

Stoolball.Data.SqlServer/DapperWrapper.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Data;
 using System.Threading.Tasks;
 using Dapper;
@@ -14,6 +15,12 @@ public async Task<IEnumerable<T>> QueryAsync<T>(string sql, CommandType commandT
             return await connection.QueryAsync<T>(sql, commandType);
         }
 
+        /// <inheritdoc/>
+        public async Task<IEnumerable<TReturn>> QueryAsync<TFirst, TSecond, TReturn>(string sql, Func<TFirst, TSecond, TReturn> map, object param, IDbTransaction transaction, bool buffered = true, string splitOn = "Id", int? commandTimeout = null, CommandType? commandType = null)
+        {
+            return await transaction.Connection.QueryAsync<TFirst, TSecond, TReturn>(new CommandDefinition(sql, param, transaction, commandTimeout, commandType, buffered ? CommandFlags.Buffered : CommandFlags.None), map, splitOn);
+        }
+
         /// <inheritdoc/>
         public async Task<IEnumerable<T>> QueryAsync<T>(string sql, object param, IDbTransaction transaction)
         {

Stoolball.Data.SqlServer/IDapperWrapper.cs

@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Data;
 using System.Threading.Tasks;
 
@@ -29,6 +30,23 @@ public interface IDapperWrapper
         /// <returns></returns>
         Task<IEnumerable<T>> QueryAsync<T>(string sql, object param, IDbTransaction transaction);
 
+        /// <summary>
+        /// Perform a asynchronous multi-mapping query with 2 input types. This returns a single type, combined from the raw types via map.
+        /// </summary>
+        /// <typeparam name="TFirst">The first type in the recordset.</typeparam>
+        /// <typeparam name="TSecond">The second type in the recordset.</typeparam>
+        /// <typeparam name="TReturn">The combined type to return.</typeparam>
+        /// <param name="sql">The SQL to execute for this query.</param>
+        /// <param name="map">The function to map row types to the return type.</param>
+        /// <param name="param">The parameters to use for this query.</param>
+        /// <param name="transaction">The transaction to use for this query.</param>
+        /// <param name="buffered">Whether to buffer the results in memory.</param>
+        /// <param name="splitOn">The field we should split and read the second object from (default: "Id").</param>
+        /// <param name="commandTimeout">Number of seconds before command execution timeout.</param>
+        /// <param name="commandType"> Is it a stored proc or a batch?</param>
+        /// <returns>An enumerable of TReturn.</returns>
+        Task<IEnumerable<TReturn>> QueryAsync<TFirst, TSecond, TReturn>(string sql, Func<TFirst, TSecond, TReturn> map, object param, IDbTransaction transaction, bool buffered = true, string splitOn = "Id", int? commandTimeout = null, CommandType? commandType = null);
+
         /// <summary>
         /// Execute a command asynchronously using Task
         /// </summary>

Stoolball.Data.SqlServer/SqlServerPlayerRepository.cs

@@ -126,7 +126,7 @@ await _auditRepository.CreateAudit(new AuditRecord
             return auditablePlayerIdentity;
         }
 
-        private static async Task<PlayerIdentity?> MatchPlayerIdentity(PlayerIdentity playerIdentity, bool allowMatchByPlayerIdentityIdOrPlayerId, string memberName, IDbTransaction transaction)
+        private async Task<PlayerIdentity?> MatchPlayerIdentity(PlayerIdentity playerIdentity, bool allowMatchByPlayerIdentityIdOrPlayerId, string memberName, IDbTransaction transaction)
         {
             if (playerIdentity is null)
             {
@@ -158,7 +158,7 @@ await _auditRepository.CreateAudit(new AuditRecord
                 throw new ArgumentNullException(nameof(transaction));
             }
 
-            var matchedPlayerIdentity = (await transaction.Connection.QueryAsync<PlayerIdentity, Player, PlayerIdentity>(
+            var matchedPlayerIdentity = (await _dapperWrapper.QueryAsync<PlayerIdentity, Player, PlayerIdentity>(
                     $"SELECT PlayerIdentityId, PlayerIdentityName, PlayerId FROM {Tables.PlayerIdentity} WHERE ComparableName = @ComparableName AND TeamId = @TeamId",
                     (pi, p) =>
                     {
@@ -255,7 +255,7 @@ await _auditRepository.CreateAudit(new AuditRecord
                         await connection.ExecuteAsync($"UPDATE {Tables.PlayerIdentity} SET PlayerId = @ExistingPlayerId WHERE PlayerId = @PlayerId", replaceWithExistingPlayer, transaction);
 
                         // We also need to update statistics, and delete the now-unused player that the identity has been moved away from. 
-                        // However this is done asynchronously by ProcessAsyncUpdatesForLinkingAndUnlinkingPlayersToMemberAccounts, so we just need to mark the player as safe to delete.
+                        // However this is done asynchronously by ProcessAsyncUpdatesForPlayers, so we just need to mark the player as safe to delete.
                         await connection.ExecuteAsync($"UPDATE {Tables.Player} SET ForAsyncDelete = 1 WHERE PlayerId = @PlayerId", auditablePlayer, transaction);
 
                         var serialisedDeletedPlayer = JsonConvert.SerializeObject(auditablePlayer);
@@ -363,7 +363,7 @@ await transaction.Connection.ExecuteAsync(
                         await connection.ExecuteAsync($"UPDATE {Tables.PlayerIdentity} SET PlayerId = @PlayerId WHERE PlayerIdentityId = @PlayerIdentityId", new { player.PlayerId, playerIdentity.PlayerIdentityId }, transaction);
 
                         // We also need to update statistics to point to new player and new player route.
-                        // However this is done asynchronously by ProcessAsyncUpdatesForLinkingAndUnlinkingPlayersToMemberAccounts, so there is nothing to do here.
+                        // However this is done asynchronously by ProcessAsyncUpdatesForPlayers, so there is nothing to do here.
 
                         var serialisedPlayer = JsonConvert.SerializeObject(_copier.CreateAuditableCopy(player));
                         await _auditRepository.CreateAudit(new AuditRecord
@@ -454,6 +454,11 @@ public async Task<RepositoryResult<PlayerIdentityUpdateResult, PlayerIdentity>>
                 throw new ArgumentException("TeamId must have a value");
             }
 
+            if (playerIdentity.Player?.PlayerId == null)
+            {
+                throw new ArgumentException("PlayerId must have a value");
+            }
+
             using (var connection = _databaseConnectionFactory.CreateDatabaseConnection())
             {
                 connection.Open();
@@ -469,7 +474,29 @@ public async Task<RepositoryResult<PlayerIdentityUpdateResult, PlayerIdentity>>
                     }
                     else
                     {
-                        _ = await connection.ExecuteAsync($"UPDATE {Tables.PlayerIdentity} SET PlayerIdentityName = @PlayerIdentityName WHERE PlayerIdentityId = @PlayerIdentityId", playerIdentity, transaction).ConfigureAwait(false);
+                        _ = await _dapperWrapper.ExecuteAsync($"UPDATE {Tables.PlayerIdentity} SET PlayerIdentityName = @PlayerIdentityName WHERE PlayerIdentityId = @PlayerIdentityId", playerIdentity, transaction).ConfigureAwait(false);
+
+                        // We also need to update statistics to point to new player identity name and new player route.
+                        // However this is done asynchronously by ProcessAsyncUpdatesForPlayers, so there is nothing to do here.
+                        var auditablePlayer = _copier.CreateAuditableCopy(playerIdentity.Player)!;
+                        if (!auditablePlayer.PlayerIdentities.Any(x => x.PlayerIdentityId == playerIdentity.PlayerIdentityId))
+                        {
+                            auditablePlayer.PlayerIdentities.Add(_copier.CreateAuditableCopy(playerIdentity)!);
+                        }
+                        var serialisedPlayer = JsonConvert.SerializeObject(auditablePlayer);
+                        await _auditRepository.CreateAudit(new AuditRecord
+                        {
+                            Action = AuditAction.Update,
+                            MemberKey = memberKey,
+                            ActorName = memberName,
+                            EntityUri = playerIdentity.Player.EntityUri,
+                            State = serialisedPlayer,
+                            RedactedState = serialisedPlayer,
+                            AuditDate = DateTime.UtcNow
+                        }, transaction);
+
+                        _logger.Info(LoggingTemplates.Updated, serialisedPlayer, memberName, memberKey, GetType(), nameof(UpdatePlayerIdentity));
+
                         transaction.Commit();
 
                         return new RepositoryResult<PlayerIdentityUpdateResult, PlayerIdentity> { Status = PlayerIdentityUpdateResult.Success, Result = playerIdentity };

Stoolball.Web/Teams/RenamePlayerIdentitySurfaceController.cs

@@ -78,6 +78,7 @@ public async Task<IActionResult> RenamePlayerIdentity([Bind(Prefix = "FormData")
                 {
                     PlayerIdentityId = model.PlayerIdentity.PlayerIdentityId,
                     PlayerIdentityName = formData.PlayerSearch,
+                    Player = model.Player,
                     Team = model.PlayerIdentity.Team
                 };