Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

Split transactions #6

Merged
merged 9 commits into from
Jun 2, 2020

Conversation

dantuck
Copy link
Collaborator

@dantuck dantuck commented May 24, 2020

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
Copy link
Owner

ebcrowder commented May 27, 2020

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
Copy link
Collaborator Author

dantuck commented May 28, 2020

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
Copy link
Owner

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.

Daniel Tucker added 2 commits May 29, 2020 21:25
- Minor changes to that negative amounts on the balanced side of a register will show red.
@dantuck
Copy link
Collaborator Author

dantuck commented Jun 1, 2020

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
Copy link
Owner

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!

@ebcrowder ebcrowder merged commit 696226d into ebcrowder:master Jun 2, 2020
@dantuck
Copy link
Collaborator Author

dantuck commented Jun 2, 2020

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!

@dantuck dantuck deleted the feature/split-transactions branch June 2, 2020 23:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants