diff --git a/BarakoCMS.Accounting/AccountService.cs b/BarakoCMS.Accounting/AccountService.cs index 479f131..5d22181 100644 --- a/BarakoCMS.Accounting/AccountService.cs +++ b/BarakoCMS.Accounting/AccountService.cs @@ -60,14 +60,28 @@ public async Task CountAsync(CancellationToken ct = default) => public async Task UpsertAsync(Account account, CancellationToken ct = default) { var session = WriteSession; + var data = ToData(account); + + // Accounts staged earlier in this same unit of work are not in the database yet, so a query + // cannot see them. Seeding a whole chart in one transaction is the ordinary way to reach + // that, and without this the second appearance of a code becomes a second account — one code + // split across two documents, with lookups picking between them arbitrarily. + var staged = session.PendingChanges.AllChangedFor() + .FirstOrDefault(c => c.ContentType == AccountingContentTypes.Account && HasCode(c, account.Code)); + + if (staged is not null) + { + staged.Data = data; + staged.UpdatedAt = DateTime.UtcNow; + session.Store(staged); + return; + } + var existing = await session.Query() .Where(c => c.ContentType == AccountingContentTypes.Account) .ToListAsync(ct); - var match = existing.FirstOrDefault(c => string.Equals( - ContentData.AsString(ContentData.Get(c.Data, "Code")), account.Code, StringComparison.OrdinalIgnoreCase)); - - var data = ToData(account); + var match = existing.FirstOrDefault(c => HasCode(c, account.Code)); if (match is not null) { @@ -96,6 +110,9 @@ public async Task UpsertManyAsync(IEnumerable accounts, CancellationTok await UpsertAsync(account, ct); } + private static bool HasCode(barakoCMS.Models.Content c, string code) => string.Equals( + ContentData.AsString(ContentData.Get(c.Data, "Code")), code, StringComparison.OrdinalIgnoreCase); + private static Dictionary ToData(Account a) => new() { ["Code"] = a.Code, diff --git a/BarakoCMS.Accounting/BarakoCMS.Accounting.csproj b/BarakoCMS.Accounting/BarakoCMS.Accounting.csproj index c95822e..820a280 100644 --- a/BarakoCMS.Accounting/BarakoCMS.Accounting.csproj +++ b/BarakoCMS.Accounting/BarakoCMS.Accounting.csproj @@ -5,7 +5,7 @@ true BarakoCMS.Accounting - 0.2.1 + 0.2.2 Optional double-entry accounting module for barakoCMS: accounts, balanced journal entries, and reporting. diff --git a/BarakoCMS.Tests/Features/Accounting/AccountServiceTests.cs b/BarakoCMS.Tests/Features/Accounting/AccountServiceTests.cs new file mode 100644 index 0000000..4782596 --- /dev/null +++ b/BarakoCMS.Tests/Features/Accounting/AccountServiceTests.cs @@ -0,0 +1,206 @@ +using Xunit; +using FluentAssertions; +using BarakoCMS.Accounting; +using BarakoCMS.Accounting.Domain; +using Marten; +using Microsoft.Extensions.DependencyInjection; +using barakoCMS.Models; + +namespace BarakoCMS.Tests.Features.Accounting; + +/// +/// The chart-of-accounts API that hosts actually use. +/// +/// It had no tests at all, which is the wrong way round: nothing inside barakoCMS calls it, so it +/// looked like dead code, but BaryoClub uses it in seven places — seeding the chart, creating member +/// accounts, batch charging, delisting, and reminders. Being consumer-only means a break here shows +/// up in someone else's repository, after a release, rather than in this one's CI. +/// +[Collection("Sequential")] +public class AccountServiceTests +{ + private readonly IntegrationTestFixture _factory; + + public AccountServiceTests(IntegrationTestFixture factory) => _factory = factory; + + private IDocumentStore Store() + { + using var scope = _factory.Services.CreateScope(); + return scope.ServiceProvider.GetRequiredService(); + } + + private static Account Acct(string code, string name = "Account", AccountType type = AccountType.Asset) => + new() { Code = code, Name = name, Type = type, IsActive = true }; + + private static string Tag() => Guid.NewGuid().ToString("N")[..8]; + + [Fact] + public async Task An_upserted_account_can_be_read_back() + { + var tag = Tag(); + using var s = Store().LightweightSession(); + var svc = new AccountService(s); + + await svc.UpsertAsync(Acct($"1000-{tag}", "Cash on hand")); + await s.SaveChangesAsync(); + + var found = await new AccountService(s).GetByCodeAsync($"1000-{tag}"); + found.Should().NotBeNull(); + found!.Name.Should().Be("Cash on hand"); + found.Type.Should().Be(AccountType.Asset); + found.IsActive.Should().BeTrue(); + } + + [Fact] + public async Task Lookup_by_code_ignores_case() + { + var tag = Tag(); + using var s = Store().LightweightSession(); + await new AccountService(s).UpsertAsync(Acct($"ab-{tag}")); + await s.SaveChangesAsync(); + + // Hosts pass codes through URLs and spreadsheets; a case-sensitive miss would read as + // "account not found" and send a charge to the wrong place, or nowhere. + (await new AccountService(s).GetByCodeAsync($"AB-{tag}")).Should().NotBeNull(); + } + + [Fact] + public async Task Upserting_an_existing_code_updates_in_place_rather_than_duplicating() + { + var tag = Tag(); + // Letters, so the second write can differ only in case and actually exercise the + // case-insensitive match. A digits-only code would compare equal either way. + var code = $"dues-{tag}"; + + using (var s1 = Store().LightweightSession()) + { + await new AccountService(s1).UpsertAsync(Acct(code, "Dues", AccountType.Income)); + await s1.SaveChangesAsync(); + } + + using (var s2 = Store().LightweightSession()) + { + await new AccountService(s2).UpsertAsync( + Acct(code.ToUpperInvariant(), "Membership dues", AccountType.Income)); + await s2.SaveChangesAsync(); + } + + // Two accounts sharing a code means every balance for that code is split across two + // documents, and which one a lookup returns is arbitrary. + using var q = Store().QuerySession(); + var all = await new AccountService(q).GetAllAsync(); + var matching = all.Where(a => string.Equals(a.Code, code, StringComparison.OrdinalIgnoreCase)).ToList(); + matching.Should().HaveCount(1, "a code differing only in case is the same account"); + matching.Single().Name.Should().Be("Membership dues", "the later write wins"); + } + + [Fact] + public async Task The_chart_comes_back_ordered_by_code() + { + var tag = Tag(); + using var s = Store().LightweightSession(); + await new AccountService(s).UpsertManyAsync(new[] + { + Acct($"5000-{tag}"), Acct($"1000-{tag}"), Acct($"3000-{tag}"), + }); + await s.SaveChangesAsync(); + + var mine = (await new AccountService(s).GetAllAsync()) + .Where(a => a.Code.EndsWith(tag)).Select(a => a.Code).ToList(); + + mine.Should().Equal($"1000-{tag}", $"3000-{tag}", $"5000-{tag}"); + } + + [Fact] + public async Task A_read_only_service_refuses_to_write_instead_of_silently_dropping_it() + { + using var q = Store().QuerySession(); + var svc = new AccountService(q); + + // Silently doing nothing here would be the worst outcome: a seeding run that reports success + // and leaves the chart empty. + var act = async () => await svc.UpsertAsync(Acct("1000-readonly")); + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task Member_metadata_survives_the_round_trip() + { + var tag = Tag(); + var memberId = Guid.NewGuid(); + + using var s = Store().LightweightSession(); + await new AccountService(s).UpsertAsync(new Account + { + Code = $"1200-{tag}", Name = "Receivable — J. Cruz", Type = AccountType.Asset, + MemberId = memberId, PayeeName = "J. Cruz", ParentCode = $"1000-{tag}", IsActive = true, + }); + await s.SaveChangesAsync(); + + // BaryoClub keys a member's statement off MemberId. Losing it detaches the member from their + // own receivable account, which is the cross-member read the project treats as a security bug. + var found = (await new AccountService(s).GetByCodeAsync($"1200-{tag}"))!; + found.MemberId.Should().Be(memberId); + found.PayeeName.Should().Be("J. Cruz"); + found.ParentCode.Should().Be($"1000-{tag}"); + } + + [Fact] + public async Task Deactivating_an_account_is_persisted() + { + var tag = Tag(); + var code = $"1300-{tag}"; + + using var s = Store().LightweightSession(); + await new AccountService(s).UpsertAsync(Acct(code)); + await s.SaveChangesAsync(); + + var acct = (await new AccountService(s).GetByCodeAsync(code))!; + acct.IsActive = false; + await new AccountService(s).UpsertAsync(acct); + await s.SaveChangesAsync(); + + // The journal hook refuses postings to an inactive account, so an ignored deactivation means + // a delisted member keeps accruing charges. + (await new AccountService(s).GetByCodeAsync(code))!.IsActive.Should().BeFalse(); + } + + [Fact] + public async Task Count_matches_what_the_chart_returns() + { + using var s = Store().LightweightSession(); + var svc = new AccountService(s); + (await svc.CountAsync()).Should().Be((await svc.GetAllAsync()).Count); + } + + /// + /// Repeating a code inside one uncommitted unit of work. + /// + /// looks for an existing account with + /// session.Query, which reads the database — so accounts stored earlier in the same + /// uncommitted batch are invisible to it. is a loop + /// over that method and is what a host uses to seed a whole chart in one transaction, which is + /// exactly where a repeated code is most likely to appear. + /// + [Fact] + public async Task Repeating_a_code_within_one_unit_of_work_does_not_create_two_accounts() + { + var tag = Tag(); + var code = $"levy-{tag}"; + + using var s = Store().LightweightSession(); + await new AccountService(s).UpsertManyAsync(new[] + { + Acct(code, "First spelling"), + Acct(code.ToUpperInvariant(), "Second spelling"), + }); + await s.SaveChangesAsync(); + + using var q = Store().QuerySession(); + var matching = (await new AccountService(q).GetAllAsync()) + .Where(a => string.Equals(a.Code, code, StringComparison.OrdinalIgnoreCase)).ToList(); + matching.Should().HaveCount(1, "one code is one account, whichever transaction it arrived in"); + matching.Single().Name.Should().Be("Second spelling", + "the staged account must be replaced, not merely left alone"); + } +} diff --git a/BarakoCMS.Tests/Features/Accounting/AccountingApiTests.cs b/BarakoCMS.Tests/Features/Accounting/AccountingApiTests.cs new file mode 100644 index 0000000..fcdcda2 --- /dev/null +++ b/BarakoCMS.Tests/Features/Accounting/AccountingApiTests.cs @@ -0,0 +1,278 @@ +using Xunit; +using FluentAssertions; +using System.Net; +using System.Net.Http.Json; +using System.Net.Http.Headers; +using System.Text.Json; +using barakoCMS.Models; +using BarakoCMS.Accounting; +using BarakoCMS.Accounting.Domain; +using Marten; +using Microsoft.Extensions.DependencyInjection; + +namespace BarakoCMS.Tests.Features.Accounting; + +/// +/// The module's own HTTP surface: POST /api/accounting/journal-entries and the accounts endpoints. +/// +/// These had no tests at all, while carrying real money: BaryoClub posts a treasurer's entries through +/// the journal-entries route. Accounting moved to content types, and the content path is well covered, +/// but this route is still live, still registered, and still what an existing consumer calls — so a +/// change that satisfied the content tests could break the thing actually in use. +/// +/// The invariants worth stating: an unbalanced entry never reaches the store, a rejected post consumes +/// no entry number, amounts survive as decimal, and posting requires an accounting role. +/// +[Collection("Sequential")] +public class AccountingApiTests +{ + private readonly IntegrationTestFixture _factory; + private readonly HttpClient _client; + private Guid _userId; + private bool _seeded; + + public AccountingApiTests(IntegrationTestFixture factory) + { + _factory = factory; + _client = factory.CreateClient(); + } + + private async Task SeedAsync() + { + if (_seeded) return; + + using var scope = _factory.Services.CreateScope(); + var store = scope.ServiceProvider.GetRequiredService(); + using var session = store.LightweightSession(); + + foreach (var def in new[] + { + AccountingContentTypes.AccountDefinition(), + AccountingContentTypes.JournalEntryDefinition(), + }) + { + var existing = await session.Query().FirstOrDefaultAsync(t => t.Name == def.Name); + if (existing is null) session.Store(def); + } + + if (await session.LoadAsync(barakoCMS.Data.DataSeeder.SuperAdminRoleId) is null) + { + session.Store(new Role + { + Id = barakoCMS.Data.DataSeeder.SuperAdminRoleId, + Name = "SuperAdmin", + Description = "Full system access", + }); + } + + // The endpoint loads the caller by their UserId claim, so the user has to exist, not just the token. + _userId = Guid.NewGuid(); + session.Store(new User + { + Id = _userId, + Username = $"ledger_{Guid.NewGuid():N}", + Email = $"{Guid.NewGuid():N}@example.com", + RoleIds = new List { barakoCMS.Data.DataSeeder.SuperAdminRoleId }, + }); + await session.SaveChangesAsync(); + + _client.DefaultRequestHeaders.Authorization = + new AuthenticationHeaderValue("Bearer", _factory.CreateToken(new[] { "SuperAdmin" }, _userId.ToString())); + _seeded = true; + } + + private async Task AccountAsync(string code, string type = "Asset") + { + await SeedAsync(); + var res = await _client.PostAsJsonAsync("/api/contents", new + { + contentType = AccountingContentTypes.Account, + status = 1, + sensitivity = 0, + data = new { Code = code, Name = $"Account {code}", Type = type, IsActive = true }, + }); + res.StatusCode.Should().Be(HttpStatusCode.OK, await res.Content.ReadAsStringAsync()); + return code; + } + + private static object Line(string code, decimal debit, decimal credit) => + new { AccountCode = code, Debit = debit, Credit = credit }; + + private Task PostEntryAsync(object body) => + _client.PostAsJsonAsync("/api/accounting/journal-entries", body); + + private static object Entry(string memo, params object[] lines) => + new { Date = "2026-03-01", Memo = memo, Lines = lines }; + + private async Task EntryCountAsync() + { + using var scope = _factory.Services.CreateScope(); + var s = scope.ServiceProvider.GetRequiredService(); + var all = await s.Query() + .Where(c => c.ContentType == AccountingContentTypes.JournalEntry) + .ToListAsync(); + return all.Count; + } + + [Fact] + public async Task A_balanced_entry_posts_and_is_numbered() + { + var suffix = Guid.NewGuid().ToString("N")[..6]; + var cash = await AccountAsync($"1000-{suffix}"); + var income = await AccountAsync($"4000-{suffix}", "Income"); + + var res = await PostEntryAsync(Entry("membership dues", Line(cash, 1500.50m, 0m), Line(income, 0m, 1500.50m))); + + res.IsSuccessStatusCode.Should().BeTrue(await res.Content.ReadAsStringAsync()); + var body = await res.Content.ReadFromJsonAsync(); + body.GetProperty("entryNumber").GetString().Should().NotBeNullOrWhiteSpace( + "an entry without a number cannot be referred to in a statement"); + body.GetProperty("amount").GetDecimal().Should().Be(1500.50m); + } + + [Fact] + public async Task An_unbalanced_entry_is_refused_and_stores_nothing() + { + var suffix = Guid.NewGuid().ToString("N")[..6]; + var cash = await AccountAsync($"1000-{suffix}"); + var income = await AccountAsync($"4000-{suffix}", "Income"); + var before = await EntryCountAsync(); + + // One peso out. The whole point of double entry is that this cannot be stored. + var res = await PostEntryAsync(Entry("off by one", Line(cash, 100m, 0m), Line(income, 0m, 99m))); + + res.IsSuccessStatusCode.Should().BeFalse(await res.Content.ReadAsStringAsync()); + (await EntryCountAsync()).Should().Be(before, "a rejected entry must not reach the ledger"); + } + + [Fact] + public async Task A_rejected_entry_does_not_consume_an_entry_number() + { + var suffix = Guid.NewGuid().ToString("N")[..6]; + var cash = await AccountAsync($"1000-{suffix}"); + var income = await AccountAsync($"4000-{suffix}", "Income"); + + var first = await PostEntryAsync(Entry("first", Line(cash, 10m, 0m), Line(income, 0m, 10m))); + var firstNumber = (await first.Content.ReadFromJsonAsync()).GetProperty("entryNumber").GetString(); + + // A failed post in between must not burn a number, or the ledger shows a gap and a treasurer + // has to explain a missing entry that never existed. + (await PostEntryAsync(Entry("rejected", Line(cash, 10m, 0m), Line(income, 0m, 9m)))) + .IsSuccessStatusCode.Should().BeFalse(); + + var third = await PostEntryAsync(Entry("second", Line(cash, 20m, 0m), Line(income, 0m, 20m))); + var thirdNumber = (await third.Content.ReadFromJsonAsync()).GetProperty("entryNumber").GetString(); + + var firstSeq = int.Parse(new string(firstNumber!.Where(char.IsDigit).ToArray())[^4..]); + var thirdSeq = int.Parse(new string(thirdNumber!.Where(char.IsDigit).ToArray())[^4..]); + (thirdSeq - firstSeq).Should().Be(1, "the sequence should advance once, not twice"); + } + + [Fact] + public async Task An_entry_against_an_unknown_account_is_refused() + { + var suffix = Guid.NewGuid().ToString("N")[..6]; + var cash = await AccountAsync($"1000-{suffix}"); + var before = await EntryCountAsync(); + + var res = await PostEntryAsync(Entry("typo", Line(cash, 50m, 0m), Line("no-such-account", 0m, 50m))); + + res.IsSuccessStatusCode.Should().BeFalse(await res.Content.ReadAsStringAsync()); + (await EntryCountAsync()).Should().Be(before); + } + + [Fact] + public async Task An_entry_with_too_few_lines_is_refused() + { + var suffix = Guid.NewGuid().ToString("N")[..6]; + var cash = await AccountAsync($"1000-{suffix}"); + + // One line is refused, though the balance rule would have caught it anyway. + (await PostEntryAsync(Entry("one leg", Line(cash, 100m, 0m)))) + .IsSuccessStatusCode.Should().BeFalse(); + + // No lines is the case where the minimum is the only thing standing in the way: debits and + // credits are both zero, so the entry balances, and the "total must exceed zero" rule is + // itself conditioned on there being lines. Drop the minimum and an empty entry posts. + (await PostEntryAsync(Entry("nothing at all"))) + .IsSuccessStatusCode.Should().BeFalse("an entry with no lines is not an entry"); + } + + [Fact] + public async Task Fractional_amounts_survive_the_round_trip_exactly() + { + var suffix = Guid.NewGuid().ToString("N")[..6]; + var cash = await AccountAsync($"1000-{suffix}"); + var income = await AccountAsync($"4000-{suffix}", "Income"); + + // 0.1 + 0.2 is 0.30000000000000004 as double. If any part of this path is double, the entry + // either fails to balance or stores a number a treasurer cannot reconcile. + var res = await PostEntryAsync(Entry("thirds", + Line(cash, 0.1m, 0m), Line(cash, 0.2m, 0m), Line(income, 0m, 0.3m))); + + res.IsSuccessStatusCode.Should().BeTrue(await res.Content.ReadAsStringAsync()); + (await res.Content.ReadFromJsonAsync()).GetProperty("amount").GetDecimal() + .Should().Be(0.3m); + } + + [Fact] + public async Task A_large_amount_keeps_its_centavos() + { + var suffix = Guid.NewGuid().ToString("N")[..6]; + var cash = await AccountAsync($"1000-{suffix}"); + var income = await AccountAsync($"4000-{suffix}", "Income"); + + // Far beyond float's exact-integer range, with centavos that a double would round away. + const decimal big = 12_345_678.91m; + var res = await PostEntryAsync(Entry("annual", Line(cash, big, 0m), Line(income, 0m, big))); + + res.IsSuccessStatusCode.Should().BeTrue(await res.Content.ReadAsStringAsync()); + (await res.Content.ReadFromJsonAsync()).GetProperty("amount").GetDecimal() + .Should().Be(big); + } + + [Fact] + public async Task Posting_requires_an_accounting_role() + { + await SeedAsync(); + var anon = _factory.CreateClient(); + (await anon.PostAsJsonAsync("/api/accounting/journal-entries", Entry("anon", Line("1000", 1m, 0m)))) + .StatusCode.Should().Be(HttpStatusCode.Unauthorized); + + var editor = _factory.CreateClient(); + editor.DefaultRequestHeaders.Authorization = + new AuthenticationHeaderValue("Bearer", _factory.CreateToken(new[] { "Editor" }, Guid.NewGuid().ToString())); + (await editor.PostAsJsonAsync("/api/accounting/journal-entries", Entry("editor", Line("1000", 1m, 0m)))) + .StatusCode.Should().Be(HttpStatusCode.Forbidden, "an editor has no business posting to the ledger"); + } + + [Fact] + public async Task Accounts_can_be_created_and_listed_through_the_module_endpoints() + { + await SeedAsync(); + var code = $"5000-{Guid.NewGuid().ToString("N")[..6]}"; + + // Type is the AccountType enum and no string-enum converter is registered, so this endpoint + // takes the ordinal. The content-type path takes "Expense" as a string for the same concept — + // worth knowing before writing a client against either. + var created = await _client.PostAsJsonAsync("/api/accounting/accounts", new + { + Code = code, + Name = "Office supplies", + Type = (int)AccountType.Expense, + }); + created.IsSuccessStatusCode.Should().BeTrue(await created.Content.ReadAsStringAsync()); + + var listed = await _client.GetAsync("/api/accounting/accounts"); + listed.IsSuccessStatusCode.Should().BeTrue(); + (await listed.Content.ReadAsStringAsync()).Should().Contain(code); + } + + [Fact] + public async Task Listing_accounts_requires_a_role() + { + await SeedAsync(); + var anon = _factory.CreateClient(); + (await anon.GetAsync("/api/accounting/accounts")).StatusCode.Should().Be(HttpStatusCode.Unauthorized); + } +} diff --git a/BarakoCMS.Tests/Features/Accounting/AccountingMigrationTests.cs b/BarakoCMS.Tests/Features/Accounting/AccountingMigrationTests.cs new file mode 100644 index 0000000..0b05eba --- /dev/null +++ b/BarakoCMS.Tests/Features/Accounting/AccountingMigrationTests.cs @@ -0,0 +1,219 @@ +using Xunit; +using FluentAssertions; +using BarakoCMS.Accounting; +using BarakoCMS.Accounting.Domain; +using Marten; +using Microsoft.Extensions.DependencyInjection; +using barakoCMS.Models; + +namespace BarakoCMS.Tests.Features.Accounting; + +/// +/// The one-shot move of an existing deployment's ledger from the old strongly-typed documents onto +/// content types. +/// +/// This had no tests, and it is the single most dangerous piece of code in the module: it runs once, +/// against a real club's books, usually by someone following a runbook. A silent drop or a +/// double-post here is not a crash — it is a treasurer's balance being quietly wrong afterwards, with +/// the run already finished and the operator moving on. +/// +/// Its doc comment makes two promises, so those are what gets pinned: it copies rather than moves, +/// and running it twice does not duplicate anything. +/// +[Collection("Sequential")] +public class AccountingMigrationTests +{ + private readonly IntegrationTestFixture _factory; + + public AccountingMigrationTests(IntegrationTestFixture factory) => _factory = factory; + + private IDocumentStore Store() + { + using var scope = _factory.Services.CreateScope(); + return scope.ServiceProvider.GetRequiredService(); + } + + /// Old-shape data, tagged with a unique code prefix so parallel-safe assertions are possible. + private static (Account cash, Account income, JournalEntry entry) Legacy(string tag) + { + var cash = new Account + { + Code = $"1000-{tag}", Name = "Cash on hand", Type = AccountType.Asset, + IsActive = true, CreatedAt = new DateTime(2021, 5, 4, 9, 30, 0, DateTimeKind.Utc), + }; + var income = new Account + { + Code = $"4000-{tag}", Name = "Dues", Type = AccountType.Income, + IsActive = true, CreatedAt = new DateTime(2021, 5, 4, 9, 30, 0, DateTimeKind.Utc), + }; + var entry = new JournalEntry + { + EntryNumber = $"JE-2021-{tag}", + Date = new DateOnly(2021, 6, 1), + Memo = "Q2 dues", + Status = JournalStatus.Posted, + Amount = 2500.25m, + CreatedAt = new DateTime(2021, 6, 1, 8, 0, 0, DateTimeKind.Utc), + Lines = new List + { + new() { AccountCode = cash.Code, Debit = 2500.25m, Credit = 0m }, + new() { AccountCode = income.Code, Debit = 0m, Credit = 2500.25m }, + }, + }; + return (cash, income, entry); + } + + private async Task SeedLegacyAsync() + { + var tag = Guid.NewGuid().ToString("N")[..8]; + var (cash, income, entry) = Legacy(tag); + using var s = Store().LightweightSession(); + s.Store(cash, income); + s.Store(entry); + await s.SaveChangesAsync(); + return tag; + } + + private async Task> ContentAsync(string type, string tag) + { + using var s = Store().QuerySession(); + var all = await s.Query().Where(c => c.ContentType == type).ToListAsync(); + return all.Where(c => System.Text.Json.JsonSerializer.Serialize(c.Data).Contains(tag)).ToList(); + } + + [Fact] + public async Task It_copies_accounts_and_entries_onto_content_types() + { + var tag = await SeedLegacyAsync(); + + using var s = Store().LightweightSession(); + var result = await AccountingMigration.RunAsync(s, Guid.NewGuid()); + + result.AccountsCopied.Should().BeGreaterThanOrEqualTo(2); + result.EntriesCopied.Should().BeGreaterThanOrEqualTo(1); + + var accounts = await ContentAsync(AccountingContentTypes.Account, tag); + accounts.Should().HaveCount(2); + + var entries = await ContentAsync(AccountingContentTypes.JournalEntry, tag); + entries.Should().HaveCount(1); + } + + [Fact] + public async Task It_leaves_the_original_documents_in_place() + { + var tag = await SeedLegacyAsync(); + + using var s = Store().LightweightSession(); + await AccountingMigration.RunAsync(s, Guid.NewGuid()); + + // "Copy, not move" is the safety property the whole design rests on: if the converted shape + // turns out wrong, the books are still on disk and the migration can be re-run. A version + // that deleted as it went would pass every other test here. + using var q = Store().QuerySession(); + (await q.Query().Where(a => a.Code == $"1000-{tag}").ToListAsync()) + .Should().HaveCount(1, "the original account must survive the migration"); + (await q.Query().Where(e => e.EntryNumber == $"JE-2021-{tag}").ToListAsync()) + .Should().HaveCount(1, "the original entry must survive the migration"); + } + + [Fact] + public async Task Running_it_twice_copies_nothing_the_second_time() + { + var tag = await SeedLegacyAsync(); + + using (var s1 = Store().LightweightSession()) + await AccountingMigration.RunAsync(s1, Guid.NewGuid()); + + using var s2 = Store().LightweightSession(); + var second = await AccountingMigration.RunAsync(s2, Guid.NewGuid()); + + // An operator who is unsure whether the first run finished will run it again. If that + // double-posts the ledger, every balance afterwards is wrong and nothing announces it. + second.EntriesCopied.Should().Be(0, "a second run must not re-post the ledger"); + second.AccountsCopied.Should().Be(0, "a second run must not duplicate the chart"); + second.EntriesSkipped.Should().BeGreaterThan(0, "the entries should be recognised, not invisible"); + + (await ContentAsync(AccountingContentTypes.JournalEntry, tag)).Should().HaveCount(1); + (await ContentAsync(AccountingContentTypes.Account, tag)).Should().HaveCount(2); + } + + [Fact] + public async Task Amounts_and_line_values_survive_as_decimal() + { + var tag = await SeedLegacyAsync(); + + using var s = Store().LightweightSession(); + await AccountingMigration.RunAsync(s, Guid.NewGuid()); + + var entry = (await ContentAsync(AccountingContentTypes.JournalEntry, tag)).Single(); + + // The centavos are the point. A migration that round-trips money through double moves a + // real club's books by amounts too small for anyone to notice on the day. + Dec(entry.Data, "Amount").Should().Be(2500.25m); + + var lines = ((System.Collections.IEnumerable)entry.Data["Lines"]).Cast().Select(AsDict).ToList(); + var debits = lines.Select(l => Dec(l, "Debit")).ToList(); + var credits = lines.Select(l => Dec(l, "Credit")).ToList(); + + debits.Sum().Should().Be(2500.25m); + credits.Sum().Should().Be(2500.25m); + debits.Sum().Should().Be(credits.Sum(), "a migrated entry that no longer balances is a corrupted ledger"); + } + + [Fact] + public async Task The_original_dates_are_preserved() + { + var tag = await SeedLegacyAsync(); + + using var s = Store().LightweightSession(); + await AccountingMigration.RunAsync(s, Guid.NewGuid()); + + // Stamping migration day onto the records would file every historical entry under the date + // the move happened, which quietly rewrites every period report. + var entry = (await ContentAsync(AccountingContentTypes.JournalEntry, tag)).Single(); + Str(entry.Data, "Date").Should().Be("2021-06-01"); + entry.CreatedAt.Should().BeCloseTo(new DateTime(2021, 6, 1, 8, 0, 0, DateTimeKind.Utc), TimeSpan.FromSeconds(1)); + + var account = (await ContentAsync(AccountingContentTypes.Account, tag)) + .Single(a => Str(a.Data, "Code") == $"1000-{tag}"); + account.CreatedAt.Should().BeCloseTo(new DateTime(2021, 5, 4, 9, 30, 0, DateTimeKind.Utc), TimeSpan.FromSeconds(1)); + } + + [Fact] + public async Task Migrated_entries_are_published_and_not_marked_sensitive() + { + var tag = await SeedLegacyAsync(); + + using var s = Store().LightweightSession(); + await AccountingMigration.RunAsync(s, Guid.NewGuid()); + + // A migrated ledger that landed as Draft would be invisible to every report, which reads as + // "the migration lost my data" even though it is all there. + var migrated = (await ContentAsync(AccountingContentTypes.JournalEntry, tag)).Single(); + migrated.Status.Should().Be(ContentStatus.Published); + + // The other half of the name, which went unchecked at first: sensitivity decides who may + // read the record at all, so a migration that changed it would move the whole ledger across + // an access boundary without anything reporting a failure. + migrated.Sensitivity.Should().Be(SensitivityLevel.Public, + "the migration must not alter who can read an entry"); + } + + // ContentData is internal to the module, so the bag is read directly here rather than widening + // production visibility to suit a test. + private static decimal Dec(Dictionary d, string key) => + d[key] is System.Text.Json.JsonElement je ? je.GetDecimal() : Convert.ToDecimal(d[key]); + + private static string Str(Dictionary d, string key) => + d[key] is System.Text.Json.JsonElement je ? je.GetString() ?? "" : Convert.ToString(d[key]) ?? ""; + + private static Dictionary AsDict(object o) => o switch + { + Dictionary d => d, + System.Text.Json.JsonElement je => System.Text.Json.JsonSerializer + .Deserialize>(je.GetRawText())!, + _ => System.Text.Json.JsonSerializer + .Deserialize>(System.Text.Json.JsonSerializer.Serialize(o))!, + }; +} diff --git a/CHANGELOG.md b/CHANGELOG.md index dfe3f02..08a34d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,35 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed: seeding a chart of accounts could create two accounts sharing one code + +`AccountService.UpsertAsync` looked for an existing account with a database query, so accounts stored +earlier in the *same uncommitted* unit of work were invisible to it. `UpsertManyAsync` is a loop over +that method and is how a host seeds a whole chart in one transaction — precisely where a repeated +code is most likely to appear. The second appearance became a second account: one code split across +two documents, with lookups picking between them arbitrarily and balances divided between them. + +It now checks the session's pending changes before the database. Accounting module `0.2.2`. + +### Accounting test coverage: 49.6% → 85.4% + +The module's own HTTP surface (`POST /api/accounting/journal-entries`, the accounts endpoints), the +one-shot `AccountingMigration`, and `AccountService` had no tests between them, while carrying the +money. Three new suites cover them, each checked by reintroducing the bug it claims to catch — +balance tolerance, totals accumulated through `double`, a migration that moves instead of copies, a +dropped idempotency guard, and a widened role gate. + +Two of those checks found weak tests rather than weak code, and both were rewritten: a one-line +journal entry is rejected for being unbalanced, not for having too few lines, so the line-minimum +rule was only pinned once an entry with *no* lines was tested; and a `(decimal)(double)` round trip +is lossless at these magnitudes, so the shape that actually bites — the running totals declared as +`double` — is what the fractional-amount test now pins. + +`AccountService` was the surprise. Nothing inside barakoCMS calls it, so it read as dead code, but +BaryoClub uses it in seven places. Whole suite: 71.1% → 74.4%. + ## [3.19.0] - 2026-08-09 ### Fixed: the Next.js upgrade that was never actually broken