Skip to content

Commit 79d23a2

Browse files
frasermolyneuxfrasermolyneux
committed
feat: Enhance file transport functionality with improved handling and UI updates
1 parent 6796c83 commit 79d23a2

8 files changed

Lines changed: 388 additions & 68 deletions

File tree

‎src/XtremeIdiots.Portal.Web.Tests/Controllers/GameServersControllerTests.cs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,23 @@
33
using Microsoft.AspNetCore.Authorization;
44
using Microsoft.AspNetCore.Http;
55
using Microsoft.AspNetCore.Mvc;
6+
using Microsoft.AspNetCore.Mvc.ViewFeatures;
67
using Microsoft.Extensions.Configuration;
78
using Microsoft.Extensions.Logging;
89
using Moq;
910
using MX.Observability.ApplicationInsights.Auditing;
1011
using MX.Api.Abstractions;
12+
using XtremeIdiots.Portal.Repository.Abstractions.Constants.V1;
1113
using Newtonsoft.Json;
1214
using System.Net;
1315
using System.Reflection;
1416
using System.Security.Claims;
1517
using XtremeIdiots.Portal.Repository.Abstractions.Models.V1.Configurations;
18+
using XtremeIdiots.Portal.Repository.Abstractions.Models.V1.GameServers;
1619
using XtremeIdiots.Portal.Repository.Api.Client.V1;
20+
using XtremeIdiots.Portal.Web.Auth.Constants;
1721
using XtremeIdiots.Portal.Web.Controllers;
22+
using XtremeIdiots.Portal.Web.Models;
1823
using XtremeIdiots.Portal.Web.ViewModels;
1924

2025
namespace XtremeIdiots.Portal.Web.Tests.Controllers;
@@ -43,6 +48,7 @@ private GameServersController CreateSut(ClaimsPrincipal? user = null)
4348
User = user ?? new ClaimsPrincipal(new ClaimsIdentity("TestAuth"))
4449
};
4550
controller.ControllerContext = new ControllerContext { HttpContext = httpContext };
51+
controller.TempData = new TempDataDictionary(httpContext, Mock.Of<ITempDataProvider>());
4652

4753
return controller;
4854
}
@@ -195,4 +201,108 @@ private static MethodInfo GetPrivateInstanceMethod(string name)
195201
Assert.NotNull(method);
196202
return method;
197203
}
204+
205+
[Fact]
206+
public async Task Edit_WhenUserCannotEditFileTransport_PreservesExistingFileTransportValues()
207+
{
208+
// Arrange
209+
var existingServer = CreateGameServerDto(ftpEnabled: true, fileTransportEnabled: true, fileTransportType: "Ftp");
210+
var updateResultDto = JsonConvert.DeserializeObject<GameServerDto>(JsonConvert.SerializeObject(new
211+
{
212+
GameServerId = existingServer.GameServerId,
213+
Title = existingServer.Title,
214+
GameType = existingServer.GameType,
215+
Hostname = existingServer.Hostname,
216+
QueryPort = existingServer.QueryPort,
217+
AgentEnabled = existingServer.AgentEnabled,
218+
FtpEnabled = existingServer.FtpEnabled,
219+
RconEnabled = existingServer.RconEnabled,
220+
BanFileSyncEnabled = existingServer.BanFileSyncEnabled,
221+
BanFileRootPath = existingServer.BanFileRootPath,
222+
ServerListEnabled = existingServer.ServerListEnabled,
223+
ServerListPosition = existingServer.ServerListPosition
224+
}))!;
225+
226+
mockRepositoryApiClient
227+
.Setup(x => x.GameServers.V1.GetGameServer(existingServer.GameServerId, It.IsAny<CancellationToken>()))
228+
.ReturnsAsync(new ApiResult<GameServerDto>(HttpStatusCode.OK, new ApiResponse<GameServerDto>(existingServer)));
229+
230+
EditGameServerDto? capturedUpdate = null;
231+
mockRepositoryApiClient
232+
.Setup(x => x.GameServers.V1.UpdateGameServer(It.IsAny<EditGameServerDto>(), It.IsAny<CancellationToken>()))
233+
.Callback<EditGameServerDto, CancellationToken>((dto, _) => capturedUpdate = dto)
234+
.ReturnsAsync(new ApiResult<GameServerDto>(HttpStatusCode.OK, new ApiResponse<GameServerDto>(updateResultDto)));
235+
236+
mockAuthorizationService
237+
.Setup(x => x.AuthorizeAsync(It.IsAny<ClaimsPrincipal>(), It.IsAny<object>(), AuthPolicies.GameServers_Write))
238+
.ReturnsAsync(AuthorizationResult.Success());
239+
240+
mockAuthorizationService
241+
.Setup(x => x.AuthorizeAsync(It.IsAny<ClaimsPrincipal>(), It.IsAny<object>(), AuthPolicies.GameServers_Credentials_FileTransport_Write))
242+
.ReturnsAsync(AuthorizationResult.Failed());
243+
244+
mockAuthorizationService
245+
.Setup(x => x.AuthorizeAsync(It.IsAny<ClaimsPrincipal>(), It.IsAny<object>(), AuthPolicies.GameServers_Credentials_Rcon_Write))
246+
.ReturnsAsync(AuthorizationResult.Failed());
247+
248+
var model = new GameServerEditViewModel
249+
{
250+
GameServer = new GameServerViewModel
251+
{
252+
GameServerId = existingServer.GameServerId,
253+
Title = existingServer.Title,
254+
GameType = existingServer.GameType,
255+
Hostname = existingServer.Hostname,
256+
QueryPort = existingServer.QueryPort,
257+
AgentEnabled = existingServer.AgentEnabled,
258+
FileTransportEnabled = true,
259+
FileTransportType = FileTransportType.Sftp,
260+
RconEnabled = existingServer.RconEnabled,
261+
BanFileSyncEnabled = existingServer.BanFileSyncEnabled,
262+
BanFileRootPath = existingServer.BanFileRootPath,
263+
ServerListEnabled = existingServer.ServerListEnabled
264+
}
265+
};
266+
267+
var sut = CreateSut();
268+
269+
// Act
270+
var result = await sut.Edit(model, CancellationToken.None);
271+
272+
// Assert
273+
var redirect = Assert.IsType<RedirectToActionResult>(result);
274+
Assert.Equal("Index", redirect.ActionName);
275+
Assert.NotNull(capturedUpdate);
276+
Assert.True(capturedUpdate!.FtpEnabled);
277+
278+
var optionalTransportType = capturedUpdate.GetType().GetProperty("FileTransportType", BindingFlags.Public | BindingFlags.Instance);
279+
if (optionalTransportType is not null)
280+
{
281+
var value = optionalTransportType.GetValue(capturedUpdate);
282+
Assert.Equal("Ftp", value?.ToString());
283+
}
284+
}
285+
286+
private static GameServerDto CreateGameServerDto(bool ftpEnabled, bool fileTransportEnabled, string fileTransportType)
287+
{
288+
var json = JsonConvert.SerializeObject(new
289+
{
290+
GameServerId = Guid.NewGuid(),
291+
Title = "Test Server",
292+
GameType = GameType.CallOfDuty4,
293+
Hostname = "127.0.0.1",
294+
QueryPort = 28960,
295+
AgentEnabled = false,
296+
FileTransportEnabled = fileTransportEnabled,
297+
FileTransportType = fileTransportType,
298+
FtpEnabled = ftpEnabled,
299+
RconEnabled = false,
300+
BanFileSyncEnabled = false,
301+
BanFileRootPath = "/",
302+
ServerListEnabled = false,
303+
ServerListPosition = 1
304+
});
305+
306+
return JsonConvert.DeserializeObject<GameServerDto>(json)!;
307+
}
198308
}

‎src/XtremeIdiots.Portal.Web.Tests/Extensions/FileTransportCompatibilityExtensionsTests.cs‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
using XtremeIdiots.Portal.Web.Extensions;
2+
using XtremeIdiots.Portal.Web.Models;
3+
4+
namespace XtremeIdiots.Portal.Web.Tests.Extensions;
5+
6+
public class FileTransportCompatibilityExtensionsTests
7+
{
8+
private enum ExternalFileTransportType
9+
{
10+
Unknown = 0,
11+
Ftp = 1,
12+
Sftp = 2
13+
}
14+
15+
private sealed class CompatibilitySource
16+
{
17+
public bool? FileTransportEnabled { get; set; }
18+
public ExternalFileTransportType? FileTransportType { get; set; }
19+
}
20+
21+
[Fact]
22+
public void GetFileTransportType_WhenTypeMissingAndLegacyFtpEnabled_InfersFtp()
23+
{
24+
var source = new CompatibilitySource
25+
{
26+
FileTransportEnabled = null,
27+
FileTransportType = null
28+
};
29+
30+
var result = source.GetFileTransportType(fileTransportEnabled: false, fallbackFtpEnabled: true);
31+
32+
Assert.Equal(FileTransportType.Ftp, result);
33+
}
34+
35+
[Fact]
36+
public void GetFileTransportType_WhenTypeMissingAndFileTransportEnabledWithoutLegacyFtp_InfersSftp()
37+
{
38+
var source = new CompatibilitySource
39+
{
40+
FileTransportEnabled = true,
41+
FileTransportType = null
42+
};
43+
44+
var result = source.GetFileTransportType(fileTransportEnabled: true, fallbackFtpEnabled: false);
45+
46+
Assert.Equal(FileTransportType.Sftp, result);
47+
}
48+
49+
[Fact]
50+
public void GetFileTransportType_WhenTypeMissingAndBothFlagsFalse_InfersUnknown()
51+
{
52+
var source = new CompatibilitySource
53+
{
54+
FileTransportEnabled = false,
55+
FileTransportType = null
56+
};
57+
58+
var result = source.GetFileTransportType(fileTransportEnabled: false, fallbackFtpEnabled: false);
59+
60+
Assert.Equal(FileTransportType.Unknown, result);
61+
}
62+
63+
[Fact]
64+
public void GetFileTransportType_WhenTypeProvided_UsesProvidedType()
65+
{
66+
var source = new CompatibilitySource
67+
{
68+
FileTransportEnabled = true,
69+
FileTransportType = ExternalFileTransportType.Sftp
70+
};
71+
72+
var result = source.GetFileTransportType(fileTransportEnabled: true, fallbackFtpEnabled: true);
73+
74+
Assert.Equal(FileTransportType.Sftp, result);
75+
}
76+
}

‎src/XtremeIdiots.Portal.Web.Tests/Extensions/GameServerDtoExtensionsTests.cs‎

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,32 @@ public class GameServerDtoExtensionsTests
88
{
99
private static GameServerDto CreateGameServerDto(bool agentEnabled = false,
1010
bool ftpEnabled = false, bool rconEnabled = false, bool banFileSyncEnabled = false, bool serverListEnabled = false,
11-
int serverListPosition = 0, string banFileRootPath = "/")
11+
int serverListPosition = 0, string banFileRootPath = "/", bool? fileTransportEnabled = null, string? fileTransportType = null)
1212
{
1313
// GameServerDto uses internal setters, so we serialize/deserialize to set values
14-
var json = System.Text.Json.JsonSerializer.Serialize(new
14+
var payload = new Dictionary<string, object?>
1515
{
16-
GameServerId = Guid.NewGuid(),
17-
Title = "Test Server",
18-
GameType = GameType.CallOfDuty4,
19-
Hostname = "127.0.0.1",
20-
QueryPort = 28960,
21-
AgentEnabled = agentEnabled,
22-
FtpEnabled = ftpEnabled,
23-
RconEnabled = rconEnabled,
24-
BanFileSyncEnabled = banFileSyncEnabled,
25-
BanFileRootPath = banFileRootPath,
26-
ServerListEnabled = serverListEnabled,
27-
ServerListPosition = serverListPosition
28-
});
16+
["GameServerId"] = Guid.NewGuid(),
17+
["Title"] = "Test Server",
18+
["GameType"] = GameType.CallOfDuty4,
19+
["Hostname"] = "127.0.0.1",
20+
["QueryPort"] = 28960,
21+
["AgentEnabled"] = agentEnabled,
22+
["FtpEnabled"] = ftpEnabled,
23+
["RconEnabled"] = rconEnabled,
24+
["BanFileSyncEnabled"] = banFileSyncEnabled,
25+
["BanFileRootPath"] = banFileRootPath,
26+
["ServerListEnabled"] = serverListEnabled,
27+
["ServerListPosition"] = serverListPosition
28+
};
29+
30+
if (fileTransportEnabled.HasValue)
31+
payload["FileTransportEnabled"] = fileTransportEnabled.Value;
32+
33+
if (!string.IsNullOrWhiteSpace(fileTransportType))
34+
payload["FileTransportType"] = fileTransportType;
35+
36+
var json = System.Text.Json.JsonSerializer.Serialize(payload);
2937

3038
return Newtonsoft.Json.JsonConvert.DeserializeObject<GameServerDto>(json)!;
3139
}
@@ -70,4 +78,30 @@ public void ToViewModel_MapsAgentEnabled(bool agentEnabled)
7078
// Assert
7179
Assert.Equal(agentEnabled, viewModel.AgentEnabled);
7280
}
81+
82+
[Fact]
83+
public void ToViewModel_WhenLegacyFtpEnabledWithoutFileTransportType_InfersFtp()
84+
{
85+
// Arrange
86+
var dto = CreateGameServerDto(ftpEnabled: true);
87+
88+
// Act
89+
var viewModel = dto.ToViewModel();
90+
91+
// Assert
92+
Assert.Equal(XtremeIdiots.Portal.Web.Models.FileTransportType.Ftp, viewModel.FileTransportType);
93+
}
94+
95+
[Fact]
96+
public void ToViewModel_WhenFileTransportDisabledAndFtpDisabledWithoutFileTransportType_InfersUnknown()
97+
{
98+
// Arrange
99+
var dto = CreateGameServerDto(ftpEnabled: false, fileTransportEnabled: false);
100+
101+
// Act
102+
var viewModel = dto.ToViewModel();
103+
104+
// Assert
105+
Assert.Equal(XtremeIdiots.Portal.Web.Models.FileTransportType.Unknown, viewModel.FileTransportType);
106+
}
73107
}

‎src/XtremeIdiots.Portal.Web/Controllers/GameServersController.cs‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,20 @@ public async Task<IActionResult> Edit(GameServerEditViewModel model, Cancellatio
430430
}
431431

432432
editGameServerDto.AgentEnabled = model.GameServer.AgentEnabled;
433-
editGameServerDto.SetFileTransportProperties(model.GameServer.FileTransportEnabled, model.GameServer.FileTransportType);
434-
editGameServerDto.FtpEnabled = model.GameServer.FileTransportEnabled && model.GameServer.FileTransportType == FileTransportType.Ftp;
433+
434+
var existingFileTransportEnabled = gameServerData.GetFileTransportEnabled(gameServerData.FtpEnabled);
435+
var existingFileTransportType = gameServerData.GetFileTransportType(existingFileTransportEnabled, gameServerData.FtpEnabled);
436+
437+
var selectedFileTransportEnabled = canEditFileTransport.Succeeded
438+
? model.GameServer.FileTransportEnabled
439+
: existingFileTransportEnabled;
440+
441+
var selectedFileTransportType = canEditFileTransport.Succeeded
442+
? model.GameServer.FileTransportType
443+
: existingFileTransportType;
444+
445+
editGameServerDto.SetFileTransportProperties(selectedFileTransportEnabled, selectedFileTransportType);
446+
editGameServerDto.FtpEnabled = selectedFileTransportEnabled && selectedFileTransportType == FileTransportType.Ftp;
435447
editGameServerDto.RconEnabled = model.GameServer.RconEnabled;
436448
editGameServerDto.BanFileSyncEnabled = model.GameServer.BanFileSyncEnabled;
437449
editGameServerDto.BanFileRootPath = string.IsNullOrWhiteSpace(model.GameServer.BanFileRootPath) ? "/" : model.GameServer.BanFileRootPath;
@@ -456,9 +468,8 @@ public async Task<IActionResult> Edit(GameServerEditViewModel model, Cancellatio
456468

457469
// Track toggle changes for audit trail
458470
var serverTitle = model.GameServer.Title ?? "";
459-
var existingFileTransportEnabled = gameServerData.GetFileTransportEnabled(gameServerData.FtpEnabled);
460471
TrackToggleChange(gameServerData.GameServerId, serverTitle, nameof(GameServerDto.AgentEnabled), gameServerData.AgentEnabled, model.GameServer.AgentEnabled);
461-
TrackToggleChange(gameServerData.GameServerId, serverTitle, "FileTransportEnabled", existingFileTransportEnabled, model.GameServer.FileTransportEnabled);
472+
TrackToggleChange(gameServerData.GameServerId, serverTitle, "FileTransportEnabled", existingFileTransportEnabled, selectedFileTransportEnabled);
462473
TrackToggleChange(gameServerData.GameServerId, serverTitle, nameof(GameServerDto.RconEnabled), gameServerData.RconEnabled, model.GameServer.RconEnabled);
463474
TrackToggleChange(gameServerData.GameServerId, serverTitle, nameof(GameServerDto.BanFileSyncEnabled), gameServerData.BanFileSyncEnabled, model.GameServer.BanFileSyncEnabled);
464475
TrackToggleChange(gameServerData.GameServerId, serverTitle, nameof(GameServerDto.ServerListEnabled), gameServerData.ServerListEnabled, model.GameServer.ServerListEnabled);

‎src/XtremeIdiots.Portal.Web/Extensions/FileTransportCompatibilityExtensions.cs‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,20 @@ public static FileTransportType GetFileTransportType(this object source, bool fi
2020
return value.Value;
2121
}
2222

23-
return fileTransportEnabled || fallbackFtpEnabled ? FileTransportType.Ftp : FileTransportType.Unknown;
23+
// Backward compatibility inference when FileTransportType is missing:
24+
// - Legacy records only had FtpEnabled
25+
// - Newer records can have FileTransportEnabled=true with FtpEnabled=false, which implies SFTP
26+
if (!fileTransportEnabled && !fallbackFtpEnabled)
27+
{
28+
return FileTransportType.Unknown;
29+
}
30+
31+
if (fallbackFtpEnabled)
32+
{
33+
return FileTransportType.Ftp;
34+
}
35+
36+
return FileTransportType.Sftp;
2437
}
2538

2639
public static void SetFileTransportProperties(this object target, bool fileTransportEnabled, FileTransportType fileTransportType)

0 commit comments

Comments
 (0)