diff --git a/Stoolball.Data.SqlServer.IntegrationTests/Statistics/SqlServerPlayerRepositoryTests.cs b/Stoolball.Data.SqlServer.IntegrationTests/Statistics/SqlServerPlayerRepositoryTests.cs index 84730268..c4773fa2 100644 --- a/Stoolball.Data.SqlServer.IntegrationTests/Statistics/SqlServerPlayerRepositoryTests.cs +++ b/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(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(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(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(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(); diff --git a/Stoolball.Data.SqlServer.IntegrationTests/StoolballStatisticsMaxResultsDataSourceIntegrationTests.dacpac b/Stoolball.Data.SqlServer.IntegrationTests/StoolballStatisticsMaxResultsDataSourceIntegrationTests.dacpac index 6196a0c1..47e53c01 100644 Binary files a/Stoolball.Data.SqlServer.IntegrationTests/StoolballStatisticsMaxResultsDataSourceIntegrationTests.dacpac and b/Stoolball.Data.SqlServer.IntegrationTests/StoolballStatisticsMaxResultsDataSourceIntegrationTests.dacpac differ diff --git a/Stoolball.Data.SqlServer.UnitTests/SqlServerPlayerRepositoryUnitTests.cs b/Stoolball.Data.SqlServer.UnitTests/SqlServerPlayerRepositoryUnitTests.cs index 16ae4dd4..a8228db0 100644 --- a/Stoolball.Data.SqlServer.UnitTests/SqlServerPlayerRepositoryUnitTests.cs +++ b/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 _routeSelector = new(); private readonly Mock _redirectsRepository = new(); private readonly Mock _playerCacheClearer = new(); + private readonly Mock _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(a => a.Action == AuditAction.Update), _transaction.Object), Times.Once); + _logger.Verify(x => x.Info(LoggingTemplates.Updated, It.IsAny(), memberName, memberKey, typeof(SqlServerPlayerRepository), nameof(SqlServerPlayerRepository.UpdatePlayerIdentity)), Times.Once); + } } } diff --git a/Stoolball.Data.SqlServer/DapperWrapper.cs b/Stoolball.Data.SqlServer/DapperWrapper.cs index ae20ce31..1d1f7a53 100644 --- a/Stoolball.Data.SqlServer/DapperWrapper.cs +++ b/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> QueryAsync(string sql, CommandType commandT return await connection.QueryAsync(sql, commandType); } + /// + public async Task> QueryAsync(string sql, Func map, object param, IDbTransaction transaction, bool buffered = true, string splitOn = "Id", int? commandTimeout = null, CommandType? commandType = null) + { + return await transaction.Connection.QueryAsync(new CommandDefinition(sql, param, transaction, commandTimeout, commandType, buffered ? CommandFlags.Buffered : CommandFlags.None), map, splitOn); + } + /// public async Task> QueryAsync(string sql, object param, IDbTransaction transaction) { diff --git a/Stoolball.Data.SqlServer/IDapperWrapper.cs b/Stoolball.Data.SqlServer/IDapperWrapper.cs index fc60e47a..a9395bcc 100644 --- a/Stoolball.Data.SqlServer/IDapperWrapper.cs +++ b/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 /// Task> QueryAsync(string sql, object param, IDbTransaction transaction); + /// + /// Perform a asynchronous multi-mapping query with 2 input types. This returns a single type, combined from the raw types via map. + /// + /// The first type in the recordset. + /// The second type in the recordset. + /// The combined type to return. + /// The SQL to execute for this query. + /// The function to map row types to the return type. + /// The parameters to use for this query. + /// The transaction to use for this query. + /// Whether to buffer the results in memory. + /// The field we should split and read the second object from (default: "Id"). + /// Number of seconds before command execution timeout. + /// Is it a stored proc or a batch? + /// An enumerable of TReturn. + Task> QueryAsync(string sql, Func map, object param, IDbTransaction transaction, bool buffered = true, string splitOn = "Id", int? commandTimeout = null, CommandType? commandType = null); + /// /// Execute a command asynchronously using Task /// diff --git a/Stoolball.Data.SqlServer/SqlServerPlayerRepository.cs b/Stoolball.Data.SqlServer/SqlServerPlayerRepository.cs index 7ce9eb83..0469af26 100644 --- a/Stoolball.Data.SqlServer/SqlServerPlayerRepository.cs +++ b/Stoolball.Data.SqlServer/SqlServerPlayerRepository.cs @@ -126,7 +126,7 @@ await _auditRepository.CreateAudit(new AuditRecord return auditablePlayerIdentity; } - private static async Task MatchPlayerIdentity(PlayerIdentity playerIdentity, bool allowMatchByPlayerIdentityIdOrPlayerId, string memberName, IDbTransaction transaction) + private async Task 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( + var matchedPlayerIdentity = (await _dapperWrapper.QueryAsync( $"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> 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> } 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 { Status = PlayerIdentityUpdateResult.Success, Result = playerIdentity }; diff --git a/Stoolball.Web/Teams/RenamePlayerIdentitySurfaceController.cs b/Stoolball.Web/Teams/RenamePlayerIdentitySurfaceController.cs index d2a10f3c..3d6cd316 100644 --- a/Stoolball.Web/Teams/RenamePlayerIdentitySurfaceController.cs +++ b/Stoolball.Web/Teams/RenamePlayerIdentitySurfaceController.cs @@ -78,6 +78,7 @@ public async Task RenamePlayerIdentity([Bind(Prefix = "FormData") { PlayerIdentityId = model.PlayerIdentity.PlayerIdentityId, PlayerIdentityName = formData.PlayerSearch, + Player = model.Player, Team = model.PlayerIdentity.Team };