diff --git a/IMPROVEMENT_PLAN.md b/IMPROVEMENT_PLAN.md index b48d461..6340fb7 100644 --- a/IMPROVEMENT_PLAN.md +++ b/IMPROVEMENT_PLAN.md @@ -106,3 +106,4 @@ Analysis and phased plan produced 2026-07-02. See conversation history for full - **PR H** — Phase 2 item 18 (first slice): add `ILogger` to `FileService.ImportAccountStatementFile` so bank-statement imports emit `Information` for the file being processed and each account's import count, `Warning` when an account is skipped because it belongs to a different mandator, and `Error` when the vendor parser reports a failure. Rest of the logging (BaseRepository save failures, MatchOpenBankTransactions summary) tracked as a follow-up because it requires threading loggers through all seven repositories. - **PR I** — Phase 2 item 12: move the service interfaces (`IViewService`, `IFileService`, `IChartService`) from `Schaad.Accounting.Common` into `Schaad.Accounting.Services/Interfaces/`. Namespaces are unchanged (`Schaad.Accounting.Interfaces`), so no consumer needs a `using` update. Drops the `Schaad.Finance.Api` `` from `Common.csproj` — Common is now vendor-free and matches its documented role as the "shared models, DTOs, interfaces" layer. - **PR J** — Phase 2 item 18 (second slice): thread `ILogger` through `BaseRepository` and every concrete repository so `BaseRepository.Save`'s catch block logs the failing file path and exception before rethrowing (previously the exception's origin was silently swallowed and only the stack trace at the callsite survived). Pulls `Microsoft.Extensions.Logging.Abstractions` into the Db project. `TransactionRepositoryTestShould` and `AccountRepositoryTestShould` use `NullLogger.Instance`. Remaining item 18 work (MatchOpenBankTransactions match-count summary in `ViewService`) still open. +- **PR K** — Phase 5 item 27: introduce `IFxConverter.ConvertToChf(amount, fromCurrency)` and its `FxConverter` implementation. The vendor `IFxService` and the FixerIo API key are now hidden inside `FxConverter`; callers stop threading the API key through every method call. `ViewService` drops both `IFxService` and `ISettingsService` from its constructor and takes `IFxConverter` instead. Simplifies `GetAccountViewList` (no more per-call `settingsService.GetSettings()` reads) and `GetBalanceSheetView`. Two-test `FxConverterTestShould` locks in the target-currency and API-key routing. diff --git a/Schaad.Accounting.Services/FxConverter.cs b/Schaad.Accounting.Services/FxConverter.cs new file mode 100644 index 0000000..d7a1e59 --- /dev/null +++ b/Schaad.Accounting.Services/FxConverter.cs @@ -0,0 +1,21 @@ +using Schaad.Accounting.Datasets; +using Schaad.Accounting.Interfaces; +using Schaad.Finance.Api; + +namespace Schaad.Accounting.Services +{ + public class FxConverter : IFxConverter + { + private readonly IFxService fxService; + private readonly SettingsDataset settings; + + public FxConverter(IFxService fxService, SettingsDataset settings) + { + this.fxService = fxService; + this.settings = settings; + } + + public decimal ConvertToChf(decimal amount, string fromCurrency) + => fxService.ConvertCurrency(amount, fromCurrency, "CHF", settings.FixerIoApiKey); + } +} diff --git a/Schaad.Accounting.Services/Interfaces/IFxConverter.cs b/Schaad.Accounting.Services/Interfaces/IFxConverter.cs new file mode 100644 index 0000000..15a8fba --- /dev/null +++ b/Schaad.Accounting.Services/Interfaces/IFxConverter.cs @@ -0,0 +1,11 @@ +namespace Schaad.Accounting.Interfaces +{ + /// + /// Thin wrapper over the vendor IFxService that hides the FixerIo API key + /// and the base currency, so callers just ask "convert this amount to CHF". + /// + public interface IFxConverter + { + decimal ConvertToChf(decimal amount, string fromCurrency); + } +} diff --git a/Schaad.Accounting.Services/ViewService.cs b/Schaad.Accounting.Services/ViewService.cs index e4fb22c..067a4e2 100644 --- a/Schaad.Accounting.Services/ViewService.cs +++ b/Schaad.Accounting.Services/ViewService.cs @@ -5,7 +5,6 @@ using Schaad.Accounting.Datasets; using Schaad.Accounting.Datasets.Reports; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; -using Schaad.Finance.Api; using Schaad.Finance.Api.Datasets; namespace Schaad.Accounting.Services @@ -17,8 +16,7 @@ namespace Schaad.Accounting.Services private readonly IBookingRuleRepository bookingRuleRepository; private readonly ISubclassRepository subclassRepository; private readonly ITransactionRepository transactionRepository; - private readonly IFxService fxService; - private readonly ISettingsService settingsService; + private readonly IFxConverter fxConverter; public ViewService( IAccountRepository accountRepository, @@ -26,16 +24,14 @@ namespace Schaad.Accounting.Services ITransactionRepository transactionRepository, ISubclassRepository subclassRepository, IBookingRuleRepository bookingRuleRepository, - IFxService fxService, - ISettingsService settingsService) + IFxConverter fxConverter) { this.accountRepository = accountRepository; this.bankTransactionRepository = bankTransactionRepository; this.transactionRepository = transactionRepository; this.subclassRepository = subclassRepository; this.bookingRuleRepository = bookingRuleRepository; - this.fxService = fxService; - this.settingsService = settingsService; + this.fxConverter = fxConverter; } /// @@ -55,7 +51,6 @@ namespace Schaad.Accounting.Services var accounts = accountRepository.GetAccountList(); var transactionList = GetTransactionViewList(); var subClassNameByNumber = subclassRepository.GetSubClassList().ToDictionary(s => s.Number, s => s.Name); - var settings = settingsService.GetSettings(); // Group transactions by account so per-account balance is O(k) instead of O(M). var creditsByAccount = transactionList.GroupBy(t => t.TargetAccountId).ToDictionary(g => g.Key, g => g.ToList()); @@ -67,8 +62,8 @@ namespace Schaad.Accounting.Services return new AccountDataset( account: a, balance: balance, - balanceCHF: fxService.ConvertCurrency(balance, a.Currency, "CHF", settings.FixerIoApiKey), - startBalanceCHF: fxService.ConvertCurrency(a.StartBalance, a.Currency, "CHF", settings.FixerIoApiKey), + balanceCHF: fxConverter.ConvertToChf(balance, a.Currency), + startBalanceCHF: fxConverter.ConvertToChf(a.StartBalance, a.Currency), className: subclassRepository.GetClass(a.Class), subClassName: subClassNameByNumber[a.SubClass]); }) @@ -111,9 +106,8 @@ namespace Schaad.Accounting.Services { var accountList = GetAccountViewList(); - var settings = settingsService.GetSettings(); - var profit = Math.Abs(accountList.Where(m => m.Class == ClassIds.Income).Sum(m =>m.BalanceCHF)); - profit += Math.Abs(accountList.Where(m => m.Class == ClassIds.Activa).Sum(m => fxService.ConvertCurrency(m.StartBalance, m.Currency, "CHF", settings.FixerIoApiKey))); + var profit = Math.Abs(accountList.Where(m => m.Class == ClassIds.Income).Sum(m => m.BalanceCHF)); + profit += Math.Abs(accountList.Where(m => m.Class == ClassIds.Activa).Sum(m => fxConverter.ConvertToChf(m.StartBalance, m.Currency))); var loss = Math.Abs(accountList.Where(m => m.Class == ClassIds.Expenses).Sum(m => m.BalanceCHF)); var balanceView = new BalanceSheetDataset( diff --git a/Schaad.Accounting.Tests/FxConverterTestShould.cs b/Schaad.Accounting.Tests/FxConverterTestShould.cs new file mode 100644 index 0000000..2b3e0a6 --- /dev/null +++ b/Schaad.Accounting.Tests/FxConverterTestShould.cs @@ -0,0 +1,34 @@ +using NSubstitute; +using Schaad.Accounting.Datasets; +using Schaad.Accounting.Services; +using Schaad.Finance.Api; +using Shouldly; + +namespace Schaad.Accounting.Tests; + +public class FxConverterTestShould +{ + private readonly IFxService fxService = Substitute.For(); + private readonly SettingsDataset settings = new() { DataPath = "", FixerIoApiKey = "test-key" }; + + private FxConverter BuildConverter() => new(fxService, settings); + + [Fact] + public void DelegateToFxServiceWithChfAsTargetAndConfiguredApiKeyWhenConverting() + { + fxService.ConvertCurrency(100m, "USD", "CHF", "test-key").Returns(90m); + + var result = BuildConverter().ConvertToChf(100m, "USD"); + + result.ShouldBe(90m); + fxService.Received(1).ConvertCurrency(100m, "USD", "CHF", "test-key"); + } + + [Fact] + public void PassNegativeAmountsThroughWhenConverting() + { + fxService.ConvertCurrency(-50m, "EUR", "CHF", Arg.Any()).Returns(-48m); + + BuildConverter().ConvertToChf(-50m, "EUR").ShouldBe(-48m); + } +} diff --git a/Schaad.Accounting.Tests/ViewServiceTestShould.cs b/Schaad.Accounting.Tests/ViewServiceTestShould.cs index d8aa4de..7637ce3 100644 --- a/Schaad.Accounting.Tests/ViewServiceTestShould.cs +++ b/Schaad.Accounting.Tests/ViewServiceTestShould.cs @@ -1,9 +1,7 @@ using NSubstitute; -using Schaad.Accounting.Datasets; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; using Schaad.Accounting.Services; -using Schaad.Finance.Api; using Shouldly; namespace Schaad.Accounting.Tests; @@ -15,8 +13,7 @@ public class ViewServiceTestShould private readonly IBankTransactionRepository bankTransactionRepo = Substitute.For(); private readonly IBookingRuleRepository bookingRuleRepo = Substitute.For(); private readonly ISubclassRepository subclassRepo = Substitute.For(); - private readonly IFxService fxService = Substitute.For(); - private readonly ISettingsService settingsService = Substitute.For(); + private readonly IFxConverter fxConverter = Substitute.For(); public ViewServiceTestShould() { @@ -24,15 +21,14 @@ public class ViewServiceTestShould subclassRepo.GetSubClassList().Returns( Enumerable.Range(10, 50).Select(n => new SubClass { Number = n, Name = "sub-" + n }).ToList()); subclassRepo.GetClass(Arg.Any()).Returns(""); - settingsService.GetSettings().Returns(new SettingsDataset { DataPath = "", FixerIoApiKey = "" }); // Passthrough FX by default (CHF-only). Individual tests can override. - fxService.ConvertCurrency(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + fxConverter.ConvertToChf(Arg.Any(), Arg.Any()) .Returns(ci => ci.ArgAt(0)); } private ViewService BuildService() => - new(accountRepo, bankTransactionRepo, transactionRepo, subclassRepo, bookingRuleRepo, fxService, settingsService); + new(accountRepo, bankTransactionRepo, transactionRepo, subclassRepo, bookingRuleRepo, fxConverter); // --- Balance math --------------------------------------------------------- @@ -82,7 +78,7 @@ public class ViewServiceTestShould transactionRepo.GetTransactionList().Returns(new List()); // 1 USD = 0.90 CHF - fxService.ConvertCurrency(Arg.Any(), "USD", "CHF", Arg.Any()) + fxConverter.ConvertToChf(Arg.Any(), "USD") .Returns(ci => ci.ArgAt(0) * 0.9m); var account = BuildService().GetAccountViewList().Single(); diff --git a/Schaad.Accounting.UI/Extensions.cs b/Schaad.Accounting.UI/Extensions.cs index 384173b..5605f6a 100644 --- a/Schaad.Accounting.UI/Extensions.cs +++ b/Schaad.Accounting.UI/Extensions.cs @@ -31,6 +31,7 @@ namespace Schaad.Accounting.UI services.AddScoped(); services.AddScoped(); services.AddSingleton(); + services.AddScoped(); services.AddSingleton(); return services;