dmitry-merzlyakov / nledger

.Net Ledger: Double-Entry Accounting System
Other
182 stars 51 forks source link

Fix inverted conditions in DateSpecifier #39

Closed aaubry closed 8 months ago

aaubry commented 8 months ago

The conditions in the DateSpecifier constructor are inverted, causing queries such as --begin 'since 7 days ago' reg to be filtered incorrectly. When the traits are null (or omitted), the intention is to include every component of the date, but currently no components are used. In the original source, the conditions are different:

  date_specifier_t(const date_t& date,
                   const optional<date_traits_t>& traits = none) {
    if (! traits || traits->has_year)
      year = date.year();
    if (! traits || traits->has_month)
      month = date.month();
    if (! traits || traits->has_day)
      day = date.day();

    TRACE_CTOR(date_specifier_t, "date_t, date_traits_t");
  }

This fix inverts the logic to match the original source code, and makes the above query work as intended.

I have added a test that checks that the behaviour is now correct. Before this fix it failed:

PS> .\nledger-tools.ps1 test nl-issues-2.test
NLedger Testing Framework Console
Getting test information... Collected 1 test cases in 25 tests
[Test 1 out of 1; Case 1] nledger\nl-issues-2.test. Done (282 msec) [FAIL]
 >Reason: (different output) 
Executed 1 test cases in 1 tests; total execution time is 00:00:00,282
None test cases passed
1 test case(s) failed (100%)
None test cases ignored

And after the fix it passes:

PS> .\nledger-tools.ps1 test nl-issues-2.test
NLedger Testing Framework Console
Getting test information... Collected 1 test cases in 25 tests
[Test 1 out of 1; Case 1] nledger\nl-issues-2.test. Done (285 msec) [OK]
Executed 1 test cases in 1 tests; total execution time is 00:00:00,285
1 test case(s) passed (100%)
None test cases failed
None test cases ignored

(all other tests still pass, of course :) )

dmitry-merzlyakov commented 8 months ago

@aaubry Thank you for your work!