adamcharnock / django-hordak

Double entry accounting in Django
http://django-hordak.readthedocs.io
MIT License
238 stars 55 forks source link

Fix Liability to Liability in `accounting_transfer_to` #101

Closed nitsujri closed 3 months ago

nitsujri commented 1 year ago

My understanding of liability transfers was wrong and this thread was correct in the solution.

This remained hidden because we only just started using it and the accounting was flagging issues.

This is backed by ChatGPT and other examples:

Transfer_Between_Income_Accounts

image

codecov-commenter commented 1 year ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (698842f) 92.97% compared to head (51ae940) 93.00%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #101 +/- ## ========================================== + Coverage 92.97% 93.00% +0.02% ========================================== Files 59 59 Lines 3828 3844 +16 Branches 248 247 -1 ========================================== + Hits 3559 3575 +16 Misses 224 224 Partials 45 45 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

PetrDlouhy commented 1 year ago

@nitsujri Thank you for your contribution. I have tried to update use this code in my project and run the tests with it. I have got lot errors like this:

Traceback (most recent call last):
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/moneyed/classes.py", line 392, in get_currency
    return CURRENCIES[code]  # type: ignore[index]
           ~~~~~~~~~~^^^^^^
KeyError: '['

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/project/blenderhub/apps/redistribution/tests/test_models.py", line 134, in test_add_redistribution_items_different_account
    .balance()
     ^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/hordak/models/core.py", line 318, in balance
    balances = [
               ^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/hordak/models/core.py", line 319, in <listcomp>
    account.simple_balance(as_of=as_of, raw=raw, leg_query=leg_query, **kwargs)
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/hordak/models/core.py", line 346, in simple_balance
    return legs.sum_to_balance() * (1 if raw else self.sign) + self._zero_balance()
                                                               ^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/hordak/models/core.py", line 350, in _zero_balance
    return Balance([Money("0", currency) for currency in self.currencies])
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/hordak/models/core.py", line 350, in <listcomp>
    return Balance([Money("0", currency) for currency in self.currencies])
                    ^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/moneyed/classes.py", line 186, in __init__
    else get_currency(str(currency).upper())
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/moneyed/classes.py", line 394, in get_currency
    raise CurrencyDoesNotExist(code)
moneyed.classes.CurrencyDoesNotExist: No currency with code [ is defined.

And this:

Traceback (most recent call last):
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 87, in _execute
    return self.cursor.execute(sql)
           ^^^^^^^^^^^^^^^^^^^^^^^^
psycopg2.errors.RaiseException: Destination Account#2 does not support currency USD. Account currencies: "[\"USD\"]"
CONTEXT:  PL/pgSQL function check_leg_and_account_currency_match() line 15 at RAISE

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/test/testcases.py", line 419, in _setup_and_call
    self._post_teardown()
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/test/testcases.py", line 1279, in _post_teardown
    self._fixture_teardown()
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/test/testcases.py", line 1509, in _fixture_teardown
    connections[db_name].check_constraints()
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/db/backends/postgresql/base.py", line 383, in check_constraints
    cursor.execute("SET CONSTRAINTS ALL IMMEDIATE")
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/cachalot/monkey_patch.py", line 137, in inner
    return original(cursor, sql, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/sentry_sdk/integrations/django/__init__.py", line 582, in execute
    return real_execute(self, sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django_read_only/__init__.py", line 79, in blocker
    return execute(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/nginx/.cache/pypoetry/virtualenvs/blenderkit-server-6gE1vKXj-py3.11/lib/python3.11/site-packages/django/db/backends/utils.py", line 87, in _execute
    return self.cursor.execute(sql)
           ^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.InternalError: Destination Account#2 does not support currency USD. Account currencies: "[\"USD\"]"
CONTEXT:  PL/pgSQL function check_leg_and_account_currency_match() line 15 at RAISE

Have you any idea what could have caused them? Should the code be fully backward compatible or am I expected to make some changes to my project?

nitsujri commented 1 year ago

There's in the code that is changing the data model in this PR, purely how the calculation of accounting_transfer_to is performed. So the code is fully backwards compatible.

Without the full code, I'm taking a lot of guesses, so please bare with me.

Based on Error

The error is complaining "[\"USD\"]" isn't a valid currency. This looks really similar to testing a view w/ a self.client.post (I'm guessing as to when a JSON str is being used).

What's being stored in the database/object isn't being json.loadsd, it's being kept as a str. Either the currencies' field is not a JSONField anymore or the encoder isn't being used?

Recommendation

Do you still see this problem if you go back one commit? i.e. master's 0108808204fd41e40f7f4d7dfa05ec440e1070cc

PetrDlouhy commented 8 months ago

@nitsujri Can you please rebase to the current master? I will try to test this again, I expect that I have those errors fixed.

nitsujri commented 8 months ago

@PetrDlouhy done!

adamcharnock commented 3 months ago

Merged. Thank you @nitsujri!