PR E: ChartService cleanup
Remove the settings-mutating year-hopping in GetAccountExpensesPerMonth that discarded the user's header year selection and silently forced the Spendings-over-time chart back to DateTime.Now.Year. The chart now honours settingsService.GetYear(), skips accounts with no transactions instead of returning null, and no longer depends on IAccountRepository or ISubclassRepository. Also reorder the plan: async becomes PR F, tests PR G. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
1c7ba7f416
commit
2eedd2fd20
2 changed files with 55 additions and 80 deletions
|
|
@ -99,5 +99,6 @@ Analysis and phased plan produced 2026-07-02. See conversation history for full
|
||||||
- **PR B** — Phase 1 item 8 (atomic writes + lock).
|
- **PR B** — Phase 1 item 8 (atomic writes + lock).
|
||||||
- **PR C** — Phase 2 mechanical cleanup: items 11, 13, 15, 16, 17.
|
- **PR C** — Phase 2 mechanical cleanup: items 11, 13, 15, 16, 17.
|
||||||
- **PR D** — Phase 2 item 9: per-request unit of work / cache. Split off from PR C because it is invasive enough to warrant its own review.
|
- **PR D** — Phase 2 item 9: per-request unit of work / cache. Split off from PR C because it is invasive enough to warrant its own review.
|
||||||
- **PR E** — Phase 3 (async).
|
- **PR E** — Phase 3 item 22: ChartService cleanup. Promoted ahead of async because it fixes an active correctness bug — `GetAccountExpensesPerMonth(accountId, year)` was mutating `ISettingsService` on the singleton to hop years, with the year parameter always hardcoded to `DateTime.Now.Year`, so the Spendings-over-time chart discarded the user's header year selection.
|
||||||
- **PR F** — Phase 4 (tests) — done alongside PR D to lock in behavior.
|
- **PR F** — Phase 3 items 19 + 21: async I/O across the repository stack, and precomputed dictionaries for view assembly.
|
||||||
|
- **PR G** — Phase 4 (tests) — locks in behavior for PR F.
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
using System;
|
using System;
|
||||||
using System.Collections.Generic;
|
using System.Collections.Generic;
|
||||||
using System.Linq;
|
using System.Linq;
|
||||||
|
using Schaad.Accounting.Datasets;
|
||||||
using Schaad.Accounting.Datasets.Charts;
|
using Schaad.Accounting.Datasets.Charts;
|
||||||
using Schaad.Accounting.Interfaces;
|
using Schaad.Accounting.Interfaces;
|
||||||
|
|
||||||
|
|
@ -8,115 +9,88 @@ namespace Schaad.Accounting.Services
|
||||||
{
|
{
|
||||||
public class ChartService : IChartService
|
public class ChartService : IChartService
|
||||||
{
|
{
|
||||||
private readonly IAccountRepository accountRepository;
|
|
||||||
private readonly ISettingsService settingsService;
|
private readonly ISettingsService settingsService;
|
||||||
private readonly ISubclassRepository subclassRepository;
|
|
||||||
private readonly IViewService viewService;
|
private readonly IViewService viewService;
|
||||||
|
|
||||||
public ChartService(
|
public ChartService(ISettingsService settingsService, IViewService viewService)
|
||||||
ISettingsService settingsService,
|
|
||||||
IViewService viewService,
|
|
||||||
IAccountRepository accountRepository,
|
|
||||||
ISubclassRepository subclassRepository)
|
|
||||||
{
|
{
|
||||||
this.settingsService = settingsService;
|
this.settingsService = settingsService;
|
||||||
this.viewService = viewService;
|
this.viewService = viewService;
|
||||||
this.accountRepository = accountRepository;
|
|
||||||
this.subclassRepository = subclassRepository;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public IReadOnlyList<DataSerie> GetExpensesPerMonth()
|
public IReadOnlyList<DataSerie> GetExpensesPerMonth()
|
||||||
{
|
{
|
||||||
var transactions = viewService.GetTransactionViewList().Where(a => a.TargetAccount.Class == ClassIds.Expenses);
|
var expenseTransactions = viewService.GetTransactionViewList()
|
||||||
|
.Where(t => t.TargetAccount.Class == ClassIds.Expenses)
|
||||||
|
.ToList();
|
||||||
|
|
||||||
// More than one subclass xx
|
var distinctSubClasses = expenseTransactions
|
||||||
if (transactions.Select(t => t.TargetAccount.SubClass).Distinct().Count() > 1)
|
.Select(t => t.TargetAccount.SubClass)
|
||||||
{
|
.Distinct()
|
||||||
return GetSubClassExpensesPerMonth();
|
.Count();
|
||||||
|
|
||||||
|
return distinctSubClasses > 1
|
||||||
|
? GetSubClassExpensesPerMonth(expenseTransactions)
|
||||||
|
: GetAccountExpensesPerMonth();
|
||||||
}
|
}
|
||||||
|
|
||||||
// only one subclass xx (z.B. Mandant Mannenbach)
|
private List<DataSerie> GetSubClassExpensesPerMonth(List<TransactionDataset> expenseTransactions)
|
||||||
return GetAccountExpensesPerMonth();
|
|
||||||
}
|
|
||||||
|
|
||||||
private List<DataSerie> GetSubClassExpensesPerMonth()
|
|
||||||
{
|
{
|
||||||
//var subclasses = subclassRepository.GetSubClassList();
|
|
||||||
var transactions = viewService.GetTransactionViewList().Where(a => a.TargetAccount.Class == ClassIds.Expenses).ToList();
|
|
||||||
var list = new List<DataSerie>();
|
|
||||||
|
|
||||||
var newestTransaction = transactions.OrderByDescending(t => t.ValueDate).FirstOrDefault();
|
|
||||||
var maxMonth = newestTransaction?.ValueDate.Month ?? 12;
|
|
||||||
var year = settingsService.GetYear();
|
var year = settingsService.GetYear();
|
||||||
|
var maxMonth = expenseTransactions
|
||||||
|
.OrderByDescending(t => t.ValueDate)
|
||||||
|
.FirstOrDefault()?.ValueDate.Month ?? 12;
|
||||||
|
|
||||||
// Group by subclass
|
var list = new List<DataSerie>();
|
||||||
foreach (var grp in transactions.GroupBy(a => a.TargetAccount.SubClass).Select(a => new {Key = a.Key, List = a.ToList()}))
|
foreach (var grp in expenseTransactions.GroupBy(t => t.TargetAccount.SubClass))
|
||||||
{
|
{
|
||||||
// Sum subclass transactions per month
|
var groupedByMonth = grp
|
||||||
var groupedByMonth = grp.List.GroupBy(g => g.ValueDate.Month).ToDictionary(g => g.Key, g => g.ToList().Sum(s => s.Value));
|
.GroupBy(t => t.ValueDate.Month)
|
||||||
|
.ToDictionary(g => g.Key, g => g.Sum(t => t.Value));
|
||||||
EnsureEntryForEveryMonth(groupedByMonth, maxMonth);
|
EnsureEntryForEveryMonth(groupedByMonth, maxMonth);
|
||||||
|
|
||||||
//var subClass = subclasses.FirstOrDefault(s => s.Number == grp.List.First().TargetAccount.SubClass);
|
list.Add(new DataSerie(
|
||||||
list.Add(
|
Id: grp.Key.ToString(),
|
||||||
new DataSerie(
|
Name: grp.First().TargetAccount.Name,
|
||||||
Id: grp.List.First().TargetAccount.SubClass.ToString(),
|
|
||||||
Name: grp.List.First().TargetAccount.Name,
|
|
||||||
X: groupedByMonth.OrderBy(g => g.Key).Select(g => new DateOnly(year, g.Key, 1)).ToList(),
|
X: groupedByMonth.OrderBy(g => g.Key).Select(g => new DateOnly(year, g.Key, 1)).ToList(),
|
||||||
Y: groupedByMonth.OrderBy(g => g.Key).Select(g => g.Value).ToList()
|
Y: groupedByMonth.OrderBy(g => g.Key).Select(g => g.Value).ToList()
|
||||||
)
|
));
|
||||||
);
|
|
||||||
}
|
}
|
||||||
return list;
|
return list;
|
||||||
}
|
}
|
||||||
|
|
||||||
private List<DataSerie> GetAccountExpensesPerMonth()
|
private List<DataSerie> GetAccountExpensesPerMonth()
|
||||||
{
|
{
|
||||||
|
var year = settingsService.GetYear();
|
||||||
|
var allTransactions = viewService.GetTransactionViewList();
|
||||||
|
var expenseAccounts = viewService.GetAccountViewList().Where(a => a.Class == ClassIds.Expenses);
|
||||||
|
|
||||||
var list = new List<DataSerie>();
|
var list = new List<DataSerie>();
|
||||||
var expensesAccounts = viewService.GetAccountViewList().Where(a => a.Class == ClassIds.Expenses);
|
foreach (var account in expenseAccounts)
|
||||||
foreach (var account in expensesAccounts)
|
|
||||||
{
|
{
|
||||||
var serie = GetAccountExpensesPerMonth(account.Id, DateTime.Now.Year);
|
var transactions = allTransactions.Where(t => t.TargetAccountId == account.Id).ToList();
|
||||||
if (serie != null)
|
if (transactions.Count == 0)
|
||||||
{
|
{
|
||||||
list.Add(serie);
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
var maxMonth = transactions.Max(t => t.ValueDate.Month);
|
||||||
|
var groupedByMonth = transactions
|
||||||
|
.GroupBy(t => t.ValueDate.Month)
|
||||||
|
.ToDictionary(g => g.Key, g => g.Sum(t => t.Value));
|
||||||
|
EnsureEntryForEveryMonth(groupedByMonth, maxMonth);
|
||||||
|
|
||||||
|
list.Add(new DataSerie(
|
||||||
|
Id: account.Id,
|
||||||
|
Name: account.Name,
|
||||||
|
X: groupedByMonth.OrderBy(g => g.Key).Select(g => new DateOnly(year, g.Key, 1)).ToList(),
|
||||||
|
Y: groupedByMonth.OrderBy(g => g.Key).Select(g => g.Value).ToList()
|
||||||
|
));
|
||||||
}
|
}
|
||||||
return list;
|
return list;
|
||||||
}
|
}
|
||||||
|
|
||||||
private DataSerie GetAccountExpensesPerMonth(string accountId, int year)
|
private static void EnsureEntryForEveryMonth(Dictionary<int, decimal> values, int maxMonth = 12)
|
||||||
{
|
|
||||||
if (settingsService.TrySetYear(year))
|
|
||||||
{
|
|
||||||
var accountList = accountRepository.GetAccountList();
|
|
||||||
var account = accountList.SingleOrDefault(a => a.Id == accountId);
|
|
||||||
// perhaps we dont have the account for last year
|
|
||||||
if (account == null)
|
|
||||||
{
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
var transactions = viewService.GetTransactionViewList().Where(t => t.TargetAccountId == accountId).ToList();
|
|
||||||
|
|
||||||
var newestTransaction = transactions.OrderByDescending(t => t.ValueDate).FirstOrDefault();
|
|
||||||
var maxMonth = newestTransaction?.ValueDate.Month ?? 12;
|
|
||||||
|
|
||||||
var groupedByMonth = transactions.GroupBy(g => g.ValueDate.Month).ToDictionary(g => g.Key, g => g.ToList().Sum(s => s.Value));
|
|
||||||
EnsureEntryForEveryMonth(groupedByMonth, maxMonth);
|
|
||||||
|
|
||||||
settingsService.SetYear(DateTime.Now.Year);
|
|
||||||
|
|
||||||
return new DataSerie(
|
|
||||||
Id: accountId,
|
|
||||||
Name: account.Name,
|
|
||||||
X: groupedByMonth.OrderBy(g => g.Key).Select(g => new DateOnly(year, g.Key, 1)).ToList(),
|
|
||||||
Y: groupedByMonth.OrderBy(g => g.Key).Select(g => g.Value).ToList()
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
|
|
||||||
private void EnsureEntryForEveryMonth(Dictionary<int, decimal> values, int maxMonth = 12)
|
|
||||||
{
|
{
|
||||||
for (int i = 1; i <= maxMonth; i++)
|
for (int i = 1; i <= maxMonth; i++)
|
||||||
{
|
{
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue