Split transactions - Githubissues

dantuck commented 4 years ago

Enhances the transactions to include the optional use of split transactions with balancing information.

A transaction can now look like:

- date: 05/23/2020
    debit_credit: 200
    acct_offset_name: credit_card
    name: grocery store
    acct_type: expense
    acct_name:
    split:
      - amount: 20
        account: expense_general
      - amount: 100
        account: expense_food

Additional updates include:

rust.yaml updates.
Color and bold additions to balance and register reporting.
README.md updated to reflect split transaction.

ebcrowder commented 4 years ago

Overall, this looks great! Thanks for updating the GH actions yaml, as well.

I have a couple of thoughts:

this works well for split expense transactions, but does not seem to work for split income transactions as we make an assumption that we will always be dealing with debits (expenses) on the income statement side. For example, this transaction would be out of balance by 600:
```
- date: 05/23/2020
  debit_credit: 300
  acct_offset_name: checking
  name: business stuff
  acct_type: income
  acct_name:
  split:
    - amount: -180
      account: income_general
    - amount: -120
      account: income_business
```
Admittedly, the primary use case for split transactions would be expenses, but I could see this being useful for income scenarios, too. An example would be investment income - capital gains, interest and dividends, etc would need to be recorded under different income categories.
- Could we add some simple test cases for the register and balances functionality?

dantuck commented 4 years ago

Great points. I honestly didn't even consider the income side which as you point out could be very beneficial to have covered. I will try and work through it in the next week and get the income side covered and try and build in some test coverage to ensure the splits calculations work. I am new to rust so some of the testing might not be the best so chime in if you have suggestions.

ebcrowder commented 4 years ago

Sounds good. I did a bit of research re: testing CLIs and it seems like we are probably looking for unit tests to assert that the various modules are parsing things the way we expect.

The test suite is currently set up as a series of integration tests, so maybe it doesn't make sense to extend those particular tests but instead implement some unit tests for the individual modules. That should make it easier to test specific functionality. This provided some brief perspective on that: https://rust-cli.github.io/book/tutorial/testing.html#what-to-test

In summary - I think we could skip adding a test to the current suite for this feature. In the meantime, I'll work on implementing some unit tests for the modules and see if that will achieve what we are looking for.

dantuck commented 4 years ago

Thanks for looking into the test suite. I do agree with you that the current testing is entirely integration and we need testing for for the modules.

So I just updated the PR to include the ability to add income splits. With that I also noticed that there may have been an assumption that an income transaction should have the income set to a negative number. Instead of doing that the income should still be a positive but the calculations should react to the account type. You can see an example of this change in the README.md file for the split. I realize that this may be a breaking change and if it is for you I will make an update to add an absolute value conversion so that the number will be changed over to a positive number if stored in a file.

In addition to adding income split changes I also made a change to the coloring so that a line number will only show up red when not zero and on the balance side of the register. Red to me should be a error in calculations so that it should draw attention to the user.

ebcrowder commented 4 years ago

Given the nature of split transactions, I think it is fine to represent the individual split transactions as absolute values, as opposed to debits/credits, for now. A downside to this approach, however, is that individual split transactions can only be debits OR credits, but not both.

When recording investment income for a portfolio, for example, it is often the case where certain classes of income (such as capital gains/losses) are negative and others are positive (interest and dividend income). Currently, the tool does not really have the capability of doing useful investment reporting, so I don't think this is a big deal at this point. Handling investments could be a good roadmap feature to implement down the line.

TLDR - this implementation probably captures most of our use cases, so I think it is good as is. Thanks for the great work on this!

dantuck commented 4 years ago

It's great to see this moving forward. I am having fun and learning as I go. Since I am not fully versed in accounting practices I hope to learn more from you about some of the ins and outs of more advanced ledgers. In the case you pointed out above regarding investments I have little experience keeping a ledger for this case. It is likely not a good idea to use and absolute values because of this and probably not a good practice anyways. We can although create an enhancement that allows for splits to also take in the account type for each split if passed and react accordingly.

How about we take this to the matrix chat and come up with a possible direction.

Thanks again for taking the PR!

ebcrowder / rust_ledger

Split transactions #6