From 7287dbd440e1e0f4418ec276b2e4d4975543ab97 Mon Sep 17 00:00:00 2001 From: Claudio Schaad Date: Thu, 2 Jul 2026 21:30:41 +0200 Subject: [PATCH] PR H: log bank-statement imports via ILogger FileService.ImportAccountStatementFile now emits Information on the file being processed and on each account's import count, Warning on mandator-mismatch skips, and Error on vendor-parser failures. UI-facing MessageDataset behaviour is unchanged. Pulls Microsoft.Extensions.Logging.Abstractions into the Services project; FileServiceTestShould uses NullLogger.Instance. Rest of item 18 (repository logging, MatchOpenBankTransactions summary) tracked as a follow-up. Co-Authored-By: Claude Opus 4.7 --- IMPROVEMENT_PLAN.md | 1 + Schaad.Accounting.Services/FileService.cs | 14 +++++++++++++- .../Schaad.Accounting.Services.csproj | 1 + Schaad.Accounting.Tests/FileServiceTestShould.cs | 3 ++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/IMPROVEMENT_PLAN.md b/IMPROVEMENT_PLAN.md index ad9d464..d0769cf 100644 --- a/IMPROVEMENT_PLAN.md +++ b/IMPROVEMENT_PLAN.md @@ -103,3 +103,4 @@ Analysis and phased plan produced 2026-07-02. See conversation history for full - **PR F** — Phase 3 item 21: precomputed dictionaries in `ViewService` to eliminate O(N·M) `Single(...)` scans and per-account transaction filtering. - **Item 19 (async I/O)** — deferred. After PR D, each XML file is loaded at most once per SignalR circuit, and this app is single-user local Blazor Server. Converting every repository/service method to async would touch ~40 files for negligible user-visible benefit and real regression risk. Revisit if the app is ever hosted for multiple concurrent users. - **PR G** — Phase 4 items 23 + 24: `Schaad.Accounting.Tests` xUnit + NSubstitute + Shouldly project. Adopts the `xxxTestShould.DoThisWhenThat` naming convention with Shouldly assertions (no xUnit `Assert.*`). Covers `ViewService`, `TransactionRepository`, `Formatting`, `RepositoryCache`, `AccountRepository`, `ChartService`, and `FileService.GetTransactionListCsv` — 38 tests locking in the earlier PRs' behavior. Note: originally shipped as three separate commits (initial project, expanded coverage, Shouldly + naming conversion) and later squashed into one commit at the user's request. +- **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. diff --git a/Schaad.Accounting.Services/FileService.cs b/Schaad.Accounting.Services/FileService.cs index e6b3b8f..4882278 100644 --- a/Schaad.Accounting.Services/FileService.cs +++ b/Schaad.Accounting.Services/FileService.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.IO.Compression; using System.Linq; using System.Text; +using Microsoft.Extensions.Logging; using Schaad.Accounting.Datasets; using Schaad.Accounting.Interfaces; using Schaad.Finance.Api; @@ -17,6 +18,7 @@ namespace Schaad.Accounting.Services private readonly ISettingsService settingsService; private readonly IAccountStatementService accountStatementService; private readonly ICreditCardStatementService creditCardStatementService; + private readonly ILogger logger; public FileService( ISettingsService settingsService, @@ -24,7 +26,8 @@ namespace Schaad.Accounting.Services ITransactionRepository transactionsRepository, IBankTransactionRepository bankTransactionRepository, IAccountStatementService accountStatementService, - ICreditCardStatementService creditCardStatementService) + ICreditCardStatementService creditCardStatementService, + ILogger logger) { this.settingsService = settingsService; this.accountRepository = accountRepository; @@ -32,6 +35,7 @@ namespace Schaad.Accounting.Services this.bankTransactionRepository = bankTransactionRepository; this.accountStatementService = accountStatementService; this.creditCardStatementService = creditCardStatementService; + this.logger = logger; } public string Backup() @@ -46,6 +50,8 @@ namespace Schaad.Accounting.Services // http://www.mikesdotnetting.com/article/288/asp-net-5-uploading-files-with-asp-net-mvc-6 public IReadOnlyList ImportAccountStatementFile(string filePath) { + logger.LogInformation("Importing account statement file {FilePath}", filePath); + var messages = new List(); var accountStatementResults = accountStatementService.ReadFile(filePath, Encoding.UTF8); @@ -56,6 +62,7 @@ namespace Schaad.Accounting.Services if (accountStatementResult.IsSuccess == false) { + logger.LogError("Statement parse failed for account {AccountNumber}: {Error}", account.AccountNumber, accountStatementResult.Error); messages.Add(new MessageDataset($"Account {account.AccountNumber} NICHT importiert: {accountStatementResult.Error}", MessageStatus.Error)); } @@ -87,6 +94,9 @@ namespace Schaad.Accounting.Services var count = bankTransactionRepository.SaveBankTransactionList(account.AccountNumber, transactionsThisYear); accountRepository.SaveBankAccountBalance(account.AccountNumber, (decimal)account.EndBalance.Value); + logger.LogInformation("Imported {Imported} of {Total} transactions for account {AccountNumber}", + count, account.Transactions.Count, account.AccountNumber); + var status = count == account.Transactions.Count ? MessageStatus.Success : MessageStatus.Info; message.Add($"{count} von {account.Transactions.Count} Transaktion(en) importiert.", status); @@ -99,6 +109,8 @@ namespace Schaad.Accounting.Services } else { + logger.LogWarning("Skipping account {AccountNumber}: not part of the currently selected mandator ({Skipped} transactions ignored)", + account.AccountNumber, account.Transactions.Count); message.Add($"Falscher Mandant: {account.Transactions.Count} Transaktion(en) nicht importiert.", MessageStatus.Warning); } } diff --git a/Schaad.Accounting.Services/Schaad.Accounting.Services.csproj b/Schaad.Accounting.Services/Schaad.Accounting.Services.csproj index 2def6e6..ce5ed5b 100644 --- a/Schaad.Accounting.Services/Schaad.Accounting.Services.csproj +++ b/Schaad.Accounting.Services/Schaad.Accounting.Services.csproj @@ -6,6 +6,7 @@ + diff --git a/Schaad.Accounting.Tests/FileServiceTestShould.cs b/Schaad.Accounting.Tests/FileServiceTestShould.cs index 242be65..3d3f966 100644 --- a/Schaad.Accounting.Tests/FileServiceTestShould.cs +++ b/Schaad.Accounting.Tests/FileServiceTestShould.cs @@ -1,4 +1,5 @@ using System.Text; +using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using Schaad.Accounting.Interfaces; using Schaad.Accounting.Models; @@ -26,7 +27,7 @@ public class FileServiceTestShould private FileService BuildService() => new( settingsService, accountRepo, transactionRepo, bankTransactionRepo, - accountStatementService, creditCardStatementService); + accountStatementService, creditCardStatementService, NullLogger.Instance); [Fact] public void EmitHeaderRowAndRunningBalanceWhenExportingTransactionsCsv()