From ca5a5288ca5044b8fcebb6d5eb5fa08a65f87df2 Mon Sep 17 00:00:00 2001 From: Tom Raikes Date: Tue, 7 Oct 2025 16:41:08 +0100 Subject: [PATCH 1/4] Clearbank tech test --- .../ClearBank.DeveloperTest.Tests.csproj | 12 ++ .../PaymentServiceTests.cs | 179 ++++++++++++++++++ .../ClearBank.DeveloperTest.csproj | 8 +- .../Controllers/PaymentServiceController.cs | 24 +++ .../Data/AccountDataStore.cs | 7 +- .../Data/BackupAccountDataStore.cs | 7 +- .../Data/IAccountDataStore.cs | 11 ++ ClearBank.DeveloperTest/Program.cs | 41 ++++ .../Services/PaymentService.cs | 109 +++++------ .../Types/PaymentScheme.cs | 6 +- .../appsettings.Development.json | 8 + ClearBank.DeveloperTest/appsettings.json | 10 + 12 files changed, 351 insertions(+), 71 deletions(-) create mode 100644 ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs create mode 100644 ClearBank.DeveloperTest/Controllers/PaymentServiceController.cs create mode 100644 ClearBank.DeveloperTest/Data/IAccountDataStore.cs create mode 100644 ClearBank.DeveloperTest/Program.cs create mode 100644 ClearBank.DeveloperTest/appsettings.Development.json create mode 100644 ClearBank.DeveloperTest/appsettings.json diff --git a/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj b/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj index b21ab73..83e3c10 100644 --- a/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj +++ b/ClearBank.DeveloperTest.Tests/ClearBank.DeveloperTest.Tests.csproj @@ -5,6 +5,18 @@ true + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + diff --git a/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs new file mode 100644 index 0000000..2bb4d0f --- /dev/null +++ b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs @@ -0,0 +1,179 @@ +using ClearBank.DeveloperTest.Data; +using ClearBank.DeveloperTest.Services; +using ClearBank.DeveloperTest.Types; +using Moq; +using System.Collections.Generic; +using Xunit; + +namespace ClearBank.DeveloperTest.Tests +{ + public class PaymentServiceTests + { + private readonly Mock _accountDataStoreMock; + + public PaymentServiceTests() + { + _accountDataStoreMock = new Mock(); + } + + //Define special condition accounts to be used in tests + private readonly Account _accountNoBacs = new Account + { + AccountNumber = "123456", + Balance = 500, + Status = AccountStatus.Live, + AllowedPaymentSchemes = AllowedPaymentSchemes.FasterPayments | AllowedPaymentSchemes.Chaps + }; + + private readonly Account _noBalanceFasterPaymentAccount = new Account + { + AccountNumber = "111222", + Balance = 0, + Status = AccountStatus.Live, + AllowedPaymentSchemes = AllowedPaymentSchemes.FasterPayments + }; + + private readonly Account _disabledChapsAccount = new Account + { + AccountNumber = "111222", + Balance = 500, + Status = AccountStatus.Disabled, + AllowedPaymentSchemes = AllowedPaymentSchemes.Chaps + }; + + [Fact] + public void MakePayment_ShouldReturnFalse_WhenAccountIsNull() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.Bacs + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns((Account)null); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Fact] + public void MakeBacsPayment_ShouldReturnFalse_WhenAccountBacsPaymentSchemeIsDisallowed() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.Bacs + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(_accountNoBacs); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Fact] + public void MakeFasterPaymentsPayment_ShouldReturnFalse_WhenAccountHasZeroBalance() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.FasterPayments + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(_noBalanceFasterPaymentAccount); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Fact] + public void MakeChapsPayment_ShouldReturnFalse_WhenAccountAccountStatusIsDisabled() + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object); + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = PaymentScheme.Chaps + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(_disabledChapsAccount); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + + [Theory] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.Bacs)] + [InlineData(PaymentScheme.FasterPayments, AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Chaps)] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.Bacs | AllowedPaymentSchemes.Chaps | AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Chaps | AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.FasterPayments, AllowedPaymentSchemes.Bacs | AllowedPaymentSchemes.FasterPayments)] + public void MakePayment_ShouldReturnTrue_WhenAccountIsActive_AndHasCorrectPaymentScheme(PaymentScheme paymentScheme, AllowedPaymentSchemes allowedPaymentSchemes) + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object); + + var account = new Account + { + AccountNumber = "12345678", + Balance = 500, + Status = AccountStatus.Live, + AllowedPaymentSchemes = allowedPaymentSchemes + }; + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = paymentScheme + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(account); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.True(result.Success); + } + + [Theory] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Bacs)] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.FasterPayments, AllowedPaymentSchemes.Chaps)] + [InlineData(PaymentScheme.Bacs, AllowedPaymentSchemes.Chaps | AllowedPaymentSchemes.FasterPayments)] + [InlineData(PaymentScheme.Chaps, AllowedPaymentSchemes.Bacs | AllowedPaymentSchemes.FasterPayments)] + public void MakePayment_ShouldReturnFalse_WhenAccountIsActive_AndHasIncorrectPaymentScheme(PaymentScheme paymentScheme, AllowedPaymentSchemes allowedPaymentSchemes) + { + // Arrange + var paymentService = new PaymentService(_accountDataStoreMock.Object); + + var account = new Account + { + AccountNumber = "12345678", + Balance = 500, + Status = AccountStatus.Live, + AllowedPaymentSchemes = allowedPaymentSchemes + }; + var request = new MakePaymentRequest + { + DebtorAccountNumber = "12345678", + Amount = 100, + PaymentScheme = paymentScheme + }; + _accountDataStoreMock.Setup(x => x.GetAccount(request.DebtorAccountNumber)).Returns(account); + // Act + var result = paymentService.MakePayment(request); + // Assert + Assert.False(result.Success); + } + } +} diff --git a/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj b/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj index 6aac7db..f6a33a8 100644 --- a/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj +++ b/ClearBank.DeveloperTest/ClearBank.DeveloperTest.csproj @@ -1,11 +1,13 @@ - + net8.0 + enable + enable - + - \ No newline at end of file + diff --git a/ClearBank.DeveloperTest/Controllers/PaymentServiceController.cs b/ClearBank.DeveloperTest/Controllers/PaymentServiceController.cs new file mode 100644 index 0000000..20c78b0 --- /dev/null +++ b/ClearBank.DeveloperTest/Controllers/PaymentServiceController.cs @@ -0,0 +1,24 @@ +using ClearBank.DeveloperTest.Services; +using ClearBank.DeveloperTest.Types; +using Microsoft.AspNetCore.Mvc; +using System.Threading.Tasks; + +namespace ClearBank.DeveloperTest.Controllers +{ + [ApiController] + [Route("[controller]")] + public class PaymentServiceController : ControllerBase + { + private readonly IPaymentService _paymentService; + public PaymentServiceController(IPaymentService paymentService) + { + _paymentService = paymentService; + } + + [HttpPost] + public MakePaymentResult Post(MakePaymentRequest request) + { + return _paymentService.MakePayment(request); + } + } +} diff --git a/ClearBank.DeveloperTest/Data/AccountDataStore.cs b/ClearBank.DeveloperTest/Data/AccountDataStore.cs index 89e49dd..800deb0 100644 --- a/ClearBank.DeveloperTest/Data/AccountDataStore.cs +++ b/ClearBank.DeveloperTest/Data/AccountDataStore.cs @@ -2,8 +2,13 @@ namespace ClearBank.DeveloperTest.Data { - public class AccountDataStore + public class AccountDataStore : IAccountDataStore { + public AccountDataStore() + { + // I'd add here the database context, or the account service provided its an external service + } + public Account GetAccount(string accountNumber) { // Access database to retrieve account, code removed for brevity diff --git a/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs b/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs index 170362c..e1746da 100644 --- a/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs +++ b/ClearBank.DeveloperTest/Data/BackupAccountDataStore.cs @@ -2,8 +2,13 @@ namespace ClearBank.DeveloperTest.Data { - public class BackupAccountDataStore + public class BackupAccountDataStore : IAccountDataStore { + public BackupAccountDataStore() + { + // I'd add here the backup database context if needed, or the backup account service provided its an external service + } + public Account GetAccount(string accountNumber) { // Access backup data base to retrieve account, code removed for brevity diff --git a/ClearBank.DeveloperTest/Data/IAccountDataStore.cs b/ClearBank.DeveloperTest/Data/IAccountDataStore.cs new file mode 100644 index 0000000..ff14fb3 --- /dev/null +++ b/ClearBank.DeveloperTest/Data/IAccountDataStore.cs @@ -0,0 +1,11 @@ +using ClearBank.DeveloperTest.Types; + +namespace ClearBank.DeveloperTest.Data +{ + public interface IAccountDataStore + { + Account GetAccount(string accountNumber); + + void UpdateAccount(Account account); + } +} diff --git a/ClearBank.DeveloperTest/Program.cs b/ClearBank.DeveloperTest/Program.cs new file mode 100644 index 0000000..8f098fa --- /dev/null +++ b/ClearBank.DeveloperTest/Program.cs @@ -0,0 +1,41 @@ +using ClearBank.DeveloperTest.Services; +using ClearBank.DeveloperTest.Data; +using ClearBank.DeveloperTest.Types; + +var builder = WebApplication.CreateBuilder(args); + +// Add services to the container. + +builder.Services.AddControllers(); +// Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle +builder.Services.AddEndpointsApiExplorer(); +builder.Services.AddSwaggerGen(); + +bool useBackupDataStore = (bool)builder.Configuration.GetValue(typeof(bool), "UseBackupDataStore"); +if (useBackupDataStore) +{ + builder.Services.AddSingleton(); +} +else +{ + builder.Services.AddSingleton(); +} + +builder.Services.AddSingleton(); + +var app = builder.Build(); + +// Configure the HTTP request pipeline. +if (app.Environment.IsDevelopment()) +{ + app.UseSwagger(); + app.UseSwaggerUI(); +} + +app.UseHttpsRedirection(); + +app.UseAuthorization(); + +app.MapControllers(); + +app.Run(); diff --git a/ClearBank.DeveloperTest/Services/PaymentService.cs b/ClearBank.DeveloperTest/Services/PaymentService.cs index 573713d..9592ba5 100644 --- a/ClearBank.DeveloperTest/Services/PaymentService.cs +++ b/ClearBank.DeveloperTest/Services/PaymentService.cs @@ -1,93 +1,76 @@ using ClearBank.DeveloperTest.Data; using ClearBank.DeveloperTest.Types; -using System.Configuration; namespace ClearBank.DeveloperTest.Services { public class PaymentService : IPaymentService { + private readonly IAccountDataStore _accountDataStore; + + public PaymentService(IAccountDataStore accountDataStore) + { + _accountDataStore = accountDataStore; + } + public MakePaymentResult MakePayment(MakePaymentRequest request) { - var dataStoreType = ConfigurationManager.AppSettings["DataStoreType"]; + Account account = _accountDataStore.GetAccount(request.DebtorAccountNumber); - Account account = null; + var result = new MakePaymentResult(); - if (dataStoreType == "Backup") + result.Success = true; + if (account == null) { - var accountDataStore = new BackupAccountDataStore(); - account = accountDataStore.GetAccount(request.DebtorAccountNumber); + result.Success = false; + return result; } else { - var accountDataStore = new AccountDataStore(); - account = accountDataStore.GetAccount(request.DebtorAccountNumber); - } - - var result = new MakePaymentResult(); + switch (request.PaymentScheme) + { + case PaymentScheme.Bacs: + if (!IsPaymentSchemeAllowed(account.AllowedPaymentSchemes, PaymentScheme.Bacs)) + { + result.Success = false; + return result; + } + break; - result.Success = true; - - switch (request.PaymentScheme) - { - case PaymentScheme.Bacs: - if (account == null) - { - result.Success = false; - } - else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.Bacs)) - { - result.Success = false; - } - break; + case PaymentScheme.FasterPayments: + if (!IsPaymentSchemeAllowed(account.AllowedPaymentSchemes, PaymentScheme.FasterPayments) || account.Balance < request.Amount) + { + result.Success = false; + return result; + } + break; - case PaymentScheme.FasterPayments: - if (account == null) - { - result.Success = false; - } - else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.FasterPayments)) - { + case PaymentScheme.Chaps: + if (!IsPaymentSchemeAllowed(account.AllowedPaymentSchemes, PaymentScheme.Chaps) || account.Status != AccountStatus.Live) + { + result.Success = false; + return result; + } + break; + //Add default case to handle unexpected PaymentScheme values + default: result.Success = false; - } - else if (account.Balance < request.Amount) - { - result.Success = false; - } - break; - - case PaymentScheme.Chaps: - if (account == null) - { - result.Success = false; - } - else if (!account.AllowedPaymentSchemes.HasFlag(AllowedPaymentSchemes.Chaps)) - { - result.Success = false; - } - else if (account.Status != AccountStatus.Live) - { - result.Success = false; - } - break; + return result; + } } if (result.Success) { account.Balance -= request.Amount; - if (dataStoreType == "Backup") - { - var accountDataStore = new BackupAccountDataStore(); - accountDataStore.UpdateAccount(account); - } - else - { - var accountDataStore = new AccountDataStore(); - accountDataStore.UpdateAccount(account); - } + _accountDataStore.UpdateAccount(account); } return result; } + + private bool IsPaymentSchemeAllowed(AllowedPaymentSchemes accountSchemes, PaymentScheme paymentScheme) + { + return accountSchemes.HasFlag((AllowedPaymentSchemes)paymentScheme); + } } } diff --git a/ClearBank.DeveloperTest/Types/PaymentScheme.cs b/ClearBank.DeveloperTest/Types/PaymentScheme.cs index d521384..ae74cb5 100644 --- a/ClearBank.DeveloperTest/Types/PaymentScheme.cs +++ b/ClearBank.DeveloperTest/Types/PaymentScheme.cs @@ -2,8 +2,8 @@ { public enum PaymentScheme { - FasterPayments, - Bacs, - Chaps + FasterPayments = 1 << 0, + Bacs = 1 << 1, + Chaps = 1 << 2 } } diff --git a/ClearBank.DeveloperTest/appsettings.Development.json b/ClearBank.DeveloperTest/appsettings.Development.json new file mode 100644 index 0000000..0c208ae --- /dev/null +++ b/ClearBank.DeveloperTest/appsettings.Development.json @@ -0,0 +1,8 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning" + } + } +} diff --git a/ClearBank.DeveloperTest/appsettings.json b/ClearBank.DeveloperTest/appsettings.json new file mode 100644 index 0000000..8ab1ca7 --- /dev/null +++ b/ClearBank.DeveloperTest/appsettings.json @@ -0,0 +1,10 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning" + } + }, + "AllowedHosts": "*", + "UseBackupDataStore": false +} From 4178936734054756edad8ea0d15b772ac1698152 Mon Sep 17 00:00:00 2001 From: Tom Raikes Date: Tue, 7 Oct 2025 17:49:39 +0100 Subject: [PATCH 2/4] 2nd commit, added logging to payment service --- .../PaymentServiceTests.cs | 16 ++-- .../Services/PaymentService.cs | 87 ++++++++++--------- .../Types/PaymentScheme.cs | 7 +- 3 files changed, 61 insertions(+), 49 deletions(-) diff --git a/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs index 2bb4d0f..49c651d 100644 --- a/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs +++ b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs @@ -1,6 +1,7 @@ using ClearBank.DeveloperTest.Data; using ClearBank.DeveloperTest.Services; using ClearBank.DeveloperTest.Types; +using Microsoft.Extensions.Logging; using Moq; using System.Collections.Generic; using Xunit; @@ -11,9 +12,12 @@ public class PaymentServiceTests { private readonly Mock _accountDataStoreMock; + private readonly Mock _loggerMock = new Mock(); + public PaymentServiceTests() { _accountDataStoreMock = new Mock(); + _loggerMock = new Mock(); } //Define special condition accounts to be used in tests @@ -45,7 +49,7 @@ public PaymentServiceTests() public void MakePayment_ShouldReturnFalse_WhenAccountIsNull() { // Arrange - var paymentService = new PaymentService(_accountDataStoreMock.Object); + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); var request = new MakePaymentRequest { DebtorAccountNumber = "12345678", @@ -63,7 +67,7 @@ public void MakePayment_ShouldReturnFalse_WhenAccountIsNull() public void MakeBacsPayment_ShouldReturnFalse_WhenAccountBacsPaymentSchemeIsDisallowed() { // Arrange - var paymentService = new PaymentService(_accountDataStoreMock.Object); + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); var request = new MakePaymentRequest { DebtorAccountNumber = "12345678", @@ -81,7 +85,7 @@ public void MakeBacsPayment_ShouldReturnFalse_WhenAccountBacsPaymentSchemeIsDisa public void MakeFasterPaymentsPayment_ShouldReturnFalse_WhenAccountHasZeroBalance() { // Arrange - var paymentService = new PaymentService(_accountDataStoreMock.Object); + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); var request = new MakePaymentRequest { DebtorAccountNumber = "12345678", @@ -99,7 +103,7 @@ public void MakeFasterPaymentsPayment_ShouldReturnFalse_WhenAccountHasZeroBalanc public void MakeChapsPayment_ShouldReturnFalse_WhenAccountAccountStatusIsDisabled() { // Arrange - var paymentService = new PaymentService(_accountDataStoreMock.Object); + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); var request = new MakePaymentRequest { DebtorAccountNumber = "12345678", @@ -123,7 +127,7 @@ public void MakeChapsPayment_ShouldReturnFalse_WhenAccountAccountStatusIsDisable public void MakePayment_ShouldReturnTrue_WhenAccountIsActive_AndHasCorrectPaymentScheme(PaymentScheme paymentScheme, AllowedPaymentSchemes allowedPaymentSchemes) { // Arrange - var paymentService = new PaymentService(_accountDataStoreMock.Object); + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); var account = new Account { @@ -154,7 +158,7 @@ public void MakePayment_ShouldReturnTrue_WhenAccountIsActive_AndHasCorrectPaymen public void MakePayment_ShouldReturnFalse_WhenAccountIsActive_AndHasIncorrectPaymentScheme(PaymentScheme paymentScheme, AllowedPaymentSchemes allowedPaymentSchemes) { // Arrange - var paymentService = new PaymentService(_accountDataStoreMock.Object); + var paymentService = new PaymentService(_accountDataStoreMock.Object, _loggerMock.Object); var account = new Account { diff --git a/ClearBank.DeveloperTest/Services/PaymentService.cs b/ClearBank.DeveloperTest/Services/PaymentService.cs index 9592ba5..7e994f8 100644 --- a/ClearBank.DeveloperTest/Services/PaymentService.cs +++ b/ClearBank.DeveloperTest/Services/PaymentService.cs @@ -1,15 +1,18 @@ using ClearBank.DeveloperTest.Data; using ClearBank.DeveloperTest.Types; +using System.Security.Principal; namespace ClearBank.DeveloperTest.Services { public class PaymentService : IPaymentService { private readonly IAccountDataStore _accountDataStore; + private readonly ILogger _logger; - public PaymentService(IAccountDataStore accountDataStore) + public PaymentService(IAccountDataStore accountDataStore, ILogger logger) { _accountDataStore = accountDataStore; + _logger = logger; } public MakePaymentResult MakePayment(MakePaymentRequest request) @@ -18,59 +21,59 @@ public MakePaymentResult MakePayment(MakePaymentRequest request) var result = new MakePaymentResult(); - result.Success = true; - if (account == null) - { - result.Success = false; - return result; - } - else - { - switch (request.PaymentScheme) - { - case PaymentScheme.Bacs: - if (!IsPaymentSchemeAllowed(account.AllowedPaymentSchemes, PaymentScheme.Bacs)) - { - result.Success = false; - return result; - } - break; - - case PaymentScheme.FasterPayments: - if (!IsPaymentSchemeAllowed(account.AllowedPaymentSchemes, PaymentScheme.FasterPayments) || account.Balance < request.Amount) - { - result.Success = false; - return result; - } - break; - - case PaymentScheme.Chaps: - if (!IsPaymentSchemeAllowed(account.AllowedPaymentSchemes, PaymentScheme.Chaps) || account.Status != AccountStatus.Live) - { - result.Success = false; - return result; - } - break; - //Add default case to handle unexpected PaymentScheme values - default: - result.Success = false; - return result; - } - } + result.Success = IsAccountStatusValid(account, request); if (result.Success) { account.Balance -= request.Amount; _accountDataStore.UpdateAccount(account); + + _logger.LogInformation($"Payment of {request.Amount} from account {account.AccountNumber} successful."); } return result; } - private bool IsPaymentSchemeAllowed(AllowedPaymentSchemes accountSchemes, PaymentScheme paymentScheme) + private bool IsAccountStatusValid(Account account, MakePaymentRequest request) { - return accountSchemes.HasFlag((AllowedPaymentSchemes)paymentScheme); + if (account == null) + { + return false; + } + else { + if (!account.AllowedPaymentSchemes.HasFlag((AllowedPaymentSchemes)request.PaymentScheme)) + { + _logger.LogWarning($"{0} payment scheme not allowed for account {1}.", request.PaymentScheme, account.AccountNumber); + return false; + } + + if (account.Balance < request.Amount) + { + _logger.LogWarning($"Payment failed for account {account.AccountNumber} as the balance is less than the amount"); + return false; + } + + if (account.Status != AccountStatus.Live) + { + _logger.LogWarning($"Payment failed as account {account.AccountNumber} is currently disabled"); + return false; + } + + //This follows the original logic however I couldn't help feeling that the same logic should apply to all transactions, not just Chaps or Faster Payments, + //hence why this has been commented out. Would appreciate your thoughts on this. + //if (account.Status != AccountStatus.Live && request.PaymentScheme == PaymentScheme.Chaps) + //{ + // _logger.LogWarning($"Chaps payment failed for account {account.AccountNumber} as the account status is not live"); + // return false; + //} + //else if (account.Balance < request.Amount && request.PaymentScheme == PaymentScheme.FasterPayments) + //{ + // _logger.LogWarning($"Faster Payments payment failed for account {account.AccountNumber} as the balance is less than the amount"); + // return false; + //} + } + return true; } } } diff --git a/ClearBank.DeveloperTest/Types/PaymentScheme.cs b/ClearBank.DeveloperTest/Types/PaymentScheme.cs index ae74cb5..027be9c 100644 --- a/ClearBank.DeveloperTest/Types/PaymentScheme.cs +++ b/ClearBank.DeveloperTest/Types/PaymentScheme.cs @@ -1,9 +1,14 @@ -namespace ClearBank.DeveloperTest.Types +using System.ComponentModel; + +namespace ClearBank.DeveloperTest.Types { public enum PaymentScheme { + [Description("Faster Payments")] FasterPayments = 1 << 0, + [Description("Bacs")] Bacs = 1 << 1, + [Description("Chaps")] Chaps = 1 << 2 } } From 3293f4c99a443a713f80c272becd90f6de7742a2 Mon Sep 17 00:00:00 2001 From: Tom Raikes Date: Tue, 7 Oct 2025 17:57:38 +0100 Subject: [PATCH 3/4] Added request property validation --- ClearBank.DeveloperTest/Types/MakePaymentRequest.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs b/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs index 4e582b6..3ca1c54 100644 --- a/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs +++ b/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs @@ -1,17 +1,24 @@ using System; +using System.ComponentModel.DataAnnotations; namespace ClearBank.DeveloperTest.Types { public class MakePaymentRequest { + [Required(ErrorMessage = "Please provide a Creditor Account Number")] public string CreditorAccountNumber { get; set; } + [Required(ErrorMessage = "Please provide a Debtor Account Number")] public string DebtorAccountNumber { get; set; } + [Required] + [Range(0.01, double.MaxValue, ErrorMessage = "Amount must be greater than zero")] public decimal Amount { get; set; } + [Required(ErrorMessage = "Please provide a Payment Date")] public DateTime PaymentDate { get; set; } + [EnumDataType(typeof(PaymentScheme), ErrorMessage = "Please provide a valid Payment Scheme")] public PaymentScheme PaymentScheme { get; set; } } } From ca01262372483bde1517357fdb597dc8f025a1fc Mon Sep 17 00:00:00 2001 From: Tom Raikes Date: Tue, 7 Oct 2025 18:50:47 +0100 Subject: [PATCH 4/4] Tidying up --- ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs | 5 ++--- ClearBank.DeveloperTest/Program.cs | 3 ++- ClearBank.DeveloperTest/Services/PaymentService.cs | 2 +- ClearBank.DeveloperTest/Types/MakePaymentRequest.cs | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs index 49c651d..61bb4d4 100644 --- a/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs +++ b/ClearBank.DeveloperTest.Tests/PaymentServiceTests.cs @@ -3,7 +3,6 @@ using ClearBank.DeveloperTest.Types; using Microsoft.Extensions.Logging; using Moq; -using System.Collections.Generic; using Xunit; namespace ClearBank.DeveloperTest.Tests @@ -12,12 +11,12 @@ public class PaymentServiceTests { private readonly Mock _accountDataStoreMock; - private readonly Mock _loggerMock = new Mock(); + private readonly Mock> _loggerMock; public PaymentServiceTests() { _accountDataStoreMock = new Mock(); - _loggerMock = new Mock(); + _loggerMock = new Mock>(); } //Define special condition accounts to be used in tests diff --git a/ClearBank.DeveloperTest/Program.cs b/ClearBank.DeveloperTest/Program.cs index 8f098fa..4311aa2 100644 --- a/ClearBank.DeveloperTest/Program.cs +++ b/ClearBank.DeveloperTest/Program.cs @@ -10,6 +10,7 @@ // Learn more about configuring Swagger/OpenAPI at https://aka.ms/aspnetcore/swashbuckle builder.Services.AddEndpointsApiExplorer(); builder.Services.AddSwaggerGen(); +builder.Services.AddLogging(); bool useBackupDataStore = (bool)builder.Configuration.GetValue(typeof(bool), "UseBackupDataStore"); if (useBackupDataStore) @@ -21,7 +22,7 @@ builder.Services.AddSingleton(); } -builder.Services.AddSingleton(); +builder.Services.AddScoped(); var app = builder.Build(); diff --git a/ClearBank.DeveloperTest/Services/PaymentService.cs b/ClearBank.DeveloperTest/Services/PaymentService.cs index 7e994f8..4ad5233 100644 --- a/ClearBank.DeveloperTest/Services/PaymentService.cs +++ b/ClearBank.DeveloperTest/Services/PaymentService.cs @@ -9,7 +9,7 @@ public class PaymentService : IPaymentService private readonly IAccountDataStore _accountDataStore; private readonly ILogger _logger; - public PaymentService(IAccountDataStore accountDataStore, ILogger logger) + public PaymentService(IAccountDataStore accountDataStore, ILogger logger) { _accountDataStore = accountDataStore; _logger = logger; diff --git a/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs b/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs index 3ca1c54..b9595bd 100644 --- a/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs +++ b/ClearBank.DeveloperTest/Types/MakePaymentRequest.cs @@ -11,7 +11,7 @@ public class MakePaymentRequest [Required(ErrorMessage = "Please provide a Debtor Account Number")] public string DebtorAccountNumber { get; set; } - [Required] + [Required(ErrorMessage = "Please provide a payment amount")] [Range(0.01, double.MaxValue, ErrorMessage = "Amount must be greater than zero")] public decimal Amount { get; set; }