When renaming player identities, ensure route and comparable name are…

… updated for the identity #626

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

@@ -582,7 +582,7 @@ public async Task ReadPlayerByMemberKey_returns_null_for_no_match()
         }
 
         [Fact]
-        public async Task ReadPlayerIdentityByRoute_returns_identity_and_team()
+        public async Task ReadPlayerIdentityByRoute_returns_identity_player_and_team()
         {
             var playerDataSource = new SqlServerPlayerDataSource(_databaseFixture.ConnectionFactory, _routeNormaliser.Object, _statisticsQueryBuilder.Object);
             var identity = _databaseFixture.TestData.PlayerIdentities.First();
@@ -594,6 +594,7 @@ public async Task ReadPlayerIdentityByRoute_returns_identity_and_team()
             Assert.NotNull(result?.Team);
             Assert.Equal(identity.PlayerIdentityId, result!.PlayerIdentityId);
             Assert.Equal(identity.PlayerIdentityName, result.PlayerIdentityName);
+            Assert.Equal(identity.Player?.PlayerId, result.Player?.PlayerId);
             Assert.Equal(identity.Team.TeamId, result.Team!.TeamId);
             Assert.Equal(identity.Team.TeamName, result.Team!.TeamName);
             Assert.Equal(identity.Team.TeamRoute, result.Team!.TeamRoute);

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

@@ -638,7 +638,17 @@ 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 playerIdentityToUpdateCopy = new PlayerIdentity
+            {
+                PlayerIdentityId = playerIdentityToUpdate.PlayerIdentityId,
+                PlayerIdentityName = playerIdentityToUpdate.PlayerIdentityName,
+                Team = new Team { TeamId = playerIdentityToUpdate.Team?.TeamId },
+                Player = new Player { PlayerId = playerIdentityToUpdate.Player?.PlayerId }
+            };
+
+            _copier.Setup(x => x.CreateAuditableCopy(playerIdentityToUpdate.Player)).Returns(new Player { PlayerId = playerIdentityToUpdate.Player?.PlayerId });
+            _copier.Setup(x => x.CreateAuditableCopy(playerIdentityToUpdate)).Returns(playerIdentityToUpdateCopy);
+            _routeGenerator.Setup(x => x.GenerateUniqueRoute(string.Empty, playerIdentityToUpdate.RouteSegment!, NoiseWords.PlayerRoute, It.IsAny<Func<string, Task<int>>>())).ReturnsAsync(playerIdentityToUpdate.RouteSegment!);
 
             var result = await repo.UpdatePlayerIdentity(playerIdentityToUpdate, Guid.NewGuid(), "Member name");
 
@@ -665,7 +675,17 @@ 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 updatedPlayerIdentityCopy = new PlayerIdentity
+            {
+                PlayerIdentityId = updatedPlayerIdentity.PlayerIdentityId,
+                PlayerIdentityName = updatedPlayerIdentity.PlayerIdentityName,
+                Team = new Team { TeamId = updatedPlayerIdentity.Team?.TeamId },
+                Player = new Player { PlayerId = updatedPlayerIdentity.Player?.PlayerId }
+            };
+
+            _copier.Setup(x => x.CreateAuditableCopy(updatedPlayerIdentity.Player)).Returns(new Player { PlayerId = updatedPlayerIdentityCopy.Player.PlayerId });
+            _copier.Setup(x => x.CreateAuditableCopy(updatedPlayerIdentity)).Returns(updatedPlayerIdentityCopy);
+            _routeGenerator.Setup(x => x.GenerateUniqueRoute(string.Empty, updatedPlayerIdentityCopy.PlayerIdentityName.Kebaberize(), NoiseWords.PlayerRoute, It.IsAny<Func<string, Task<int>>>())).ReturnsAsync(updatedPlayerIdentityCopy.PlayerIdentityName.Kebaberize());
 
             var result = await repo.UpdatePlayerIdentity(updatedPlayerIdentity, Guid.NewGuid(), "Member name");
             await repo.ProcessAsyncUpdatesForPlayers();
@@ -675,8 +695,11 @@ public async Task UpdatePlayerIdentity_where_name_does_not_match_existing_player
             using (var connectionForAssert = _databaseFixture.ConnectionFactory.CreateDatabaseConnection())
             {
                 connectionForAssert.Open();
-                var playerToUpdateNameAfter = await connectionForAssert.ExecuteScalarAsync<string>($"SELECT PlayerIdentityName FROM {Tables.PlayerIdentity} WHERE PlayerIdentityId = @PlayerIdentityId", playerIdentityToUpdate).ConfigureAwait(false);
-                Assert.Equal(updatedPlayerIdentity.PlayerIdentityName, playerToUpdateNameAfter);
+                var playerToUpdateNameAfter = await connectionForAssert.QuerySingleAsync<(string playerIdentityName, string comparableName, string routeSegment)>(
+                    $"SELECT PlayerIdentityName, ComparableName, RouteSegment FROM {Tables.PlayerIdentity} WHERE PlayerIdentityId = @PlayerIdentityId", playerIdentityToUpdate).ConfigureAwait(false);
+                Assert.Equal(updatedPlayerIdentity.PlayerIdentityName, playerToUpdateNameAfter.playerIdentityName);
+                Assert.Equal(updatedPlayerIdentity.ComparableName(), playerToUpdateNameAfter.comparableName);
+                Assert.Equal(updatedPlayerIdentity.PlayerIdentityName.Kebaberize(), playerToUpdateNameAfter.routeSegment);
 
                 var playerToUpdateNameInStatistics = await connectionForAssert.QueryAsync<string>($"SELECT PlayerIdentityName FROM {Tables.PlayerInMatchStatistics} WHERE PlayerIdentityId = @PlayerIdentityId", playerIdentityToUpdate).ConfigureAwait(false);
                 foreach (var nameInStatistics in playerToUpdateNameInStatistics)

Stoolball.Data.SqlServer/SqlServerPlayerDataSource.cs

@@ -274,20 +274,22 @@ WHERE LOWER(PlayerRoute) = @Route
 
             using (var connection = _databaseConnectionFactory.CreateDatabaseConnection())
             {
-                var results = await connection.QueryAsync<PlayerIdentity, Team, PlayerIdentity>(
+                var results = await connection.QueryAsync<PlayerIdentity, Player, Team, PlayerIdentity>(
                     $@"SELECT pi.PlayerIdentityId, pi.PlayerIdentityName,
+                              pi.PlayerId,
                               t.TeamId, tv.TeamName, t.TeamRoute 
                        FROM {Tables.PlayerIdentity} pi INNER JOIN {Tables.Team} t ON pi.TeamId = t.TeamId 
                        INNER JOIN {Tables.TeamVersion} tv ON t.TeamId = tv.TeamId
                        WHERE LOWER(pi.RouteSegment) = @RouteSegment AND LOWER(t.TeamRoute) = @TeamRoute
                        AND tv.TeamVersionId = (SELECT TOP 1 TeamVersionId FROM {Tables.TeamVersion} WHERE TeamId = t.TeamId ORDER BY ISNULL(UntilDate, '{SqlDateTime.MaxValue.Value.Date.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture)}') DESC)",
-                    (identity, team) =>
+                    (identity, player, team) =>
                     {
+                        identity.Player = player;
                         identity.Team = team;
                         return identity;
                     },
                     parameters,
-                    splitOn: "TeamId"
+                    splitOn: "PlayerId, TeamId"
                     ).ConfigureAwait(false);
 
                 return results.SingleOrDefault();

Stoolball.Data.SqlServer/SqlServerPlayerRepository.cs

@@ -474,22 +474,39 @@ public async Task<RepositoryResult<PlayerIdentityUpdateResult, PlayerIdentity>>
                     }
                     else
                     {
-                        _ = await _dapperWrapper.ExecuteAsync($"UPDATE {Tables.PlayerIdentity} SET PlayerIdentityName = @PlayerIdentityName WHERE PlayerIdentityId = @PlayerIdentityId", playerIdentity, transaction).ConfigureAwait(false);
+                        var auditablePlayer = _copier.CreateAuditableCopy(playerIdentity.Player)!;
+                        var auditablePlayerIdentity = _copier.CreateAuditableCopy(playerIdentity)!;
+
+                        auditablePlayerIdentity.RouteSegment = (await _routeGenerator.GenerateUniqueRoute(string.Empty, auditablePlayerIdentity.PlayerIdentityName.Kebaberize(), NoiseWords.PlayerRoute,
+                            async route => await transaction.Connection.ExecuteScalarAsync<int>($"SELECT COUNT(*) FROM {Tables.PlayerIdentity} WHERE RouteSegment = @RouteSegment AND TeamId = @TeamId", new { RouteSegment = route, auditablePlayerIdentity.Team!.TeamId }, transaction)
+                        ).ConfigureAwait(false))?.TrimStart('/');
+
+                        _ = await _dapperWrapper.ExecuteAsync($@"UPDATE {Tables.PlayerIdentity} SET 
+                                    PlayerIdentityName = @PlayerIdentityName,
+                                    ComparableName = @ComparableName,
+                                    RouteSegment = @RouteSegment
+                                    WHERE PlayerIdentityId = @PlayerIdentityId",
+                                    new
+                                    {
+                                        auditablePlayerIdentity.PlayerIdentityId,
+                                        auditablePlayerIdentity.PlayerIdentityName,
+                                        ComparableName = auditablePlayerIdentity.ComparableName(),
+                                        auditablePlayerIdentity.RouteSegment
+                                    }, 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))
+                        if (!auditablePlayer.PlayerIdentities.Any(x => x.PlayerIdentityId == auditablePlayerIdentity.PlayerIdentityId))
                         {
-                            auditablePlayer.PlayerIdentities.Add(_copier.CreateAuditableCopy(playerIdentity)!);
+                            auditablePlayer.PlayerIdentities.Add(auditablePlayerIdentity);
                         }
                         var serialisedPlayer = JsonConvert.SerializeObject(auditablePlayer);
                         await _auditRepository.CreateAudit(new AuditRecord
                         {
                             Action = AuditAction.Update,
                             MemberKey = memberKey,
                             ActorName = memberName,
-                            EntityUri = playerIdentity.Player.EntityUri,
+                            EntityUri = auditablePlayer.EntityUri,
                             State = serialisedPlayer,
                             RedactedState = serialisedPlayer,
                             AuditDate = DateTime.UtcNow
@@ -499,7 +516,7 @@ await _auditRepository.CreateAudit(new AuditRecord
 
                         transaction.Commit();
 
-                        return new RepositoryResult<PlayerIdentityUpdateResult, PlayerIdentity> { Status = PlayerIdentityUpdateResult.Success, Result = playerIdentity };
+                        return new RepositoryResult<PlayerIdentityUpdateResult, PlayerIdentity> { Status = PlayerIdentityUpdateResult.Success, Result = auditablePlayerIdentity };
                     }
                 }
             }

Stoolball.Web/Teams/RenamePlayerIdentitySurfaceController.cs

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

Stoolball.Web/Views/EditPlayersForTeam.cshtml

@@ -31,7 +31,7 @@
 
     @if (Model.Authorization.CurrentMemberIsAuthorized[AuthorizedAction.EditTeam])
     {
-        <p class="alert alert-info alert-tip">You can't add or delete players here. Players come from scorecards added to this website – see <a href="/organise/website/how-to-add-match-results/">How to add match results</a></p>
+        <p class="alert alert-info alert-tip">You can rename players here, but you can't add or delete them. Players come from scorecards added to this website – see <a href="/organise/website/how-to-add-match-results/">How to add match results</a>.</p>
 
         @if (Model.PlayerIdentities.Any())
         {