Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Moving Account.amount to Account.credit & Account.debit #133

Merged
merged 39 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
f446ee6
Adding migrations and early work on code changes
adamcharnock Jul 1, 2024
2970b15
Code changes to support PR: Experiment in moving Account.amount to Ac…
adamcharnock Jul 1, 2024
2698f43
Test changes to support PR: Experiment in moving Account.amount to Ac…
adamcharnock Jul 1, 2024
ca60528
Now supporting legacy `amount` argument to Leg, with deprecation warning
adamcharnock Jul 1, 2024
ee63c76
Tests now pass for mysql
adamcharnock Jul 1, 2024
341bcfb
Setting mysql error code correctly
adamcharnock Jul 1, 2024
3cb4419
Adding tests and errors for invalid Leg credit/debit values
adamcharnock Jul 1, 2024
a365d6a
Merge branch 'master' into experiment/amount-to-debit-credit
adamcharnock Jul 1, 2024
800b6e1
Now storing creation migrations for debugging
adamcharnock Jul 1, 2024
61d812d
Now storing creation migrations for debugging
adamcharnock Jul 1, 2024
41415ac
Now storing creation migrations for debugging
adamcharnock Jul 1, 2024
5e9a71b
Now storing creation migrations for debugging
adamcharnock Jul 1, 2024
b2c6575
Now storing creation migrations for debugging
adamcharnock Jul 1, 2024
62cfa2a
Requiring djmoney>3 due to migration creation problem with djmoney 3.…
adamcharnock Jul 1, 2024
bec5fdb
Formatting fix
adamcharnock Jul 1, 2024
4ffc107
Fixing typo in deprecation warning
adamcharnock Jul 3, 2024
cc407b7
Expanding out the changelog
adamcharnock Jul 3, 2024
a4e74f4
Making model choices available using the old syntax
adamcharnock Jul 15, 2024
c0f03d5
Fixing SQL error message
adamcharnock Jul 15, 2024
ef658e6
Fixing postgres reverse migration issue
adamcharnock Jul 15, 2024
ab11c27
Migrations now correctly set number of decimal places for decimal fields
adamcharnock Jul 15, 2024
be40944
Fixing error in changelog, thanks to @nitsujri for catching it
adamcharnock Jul 18, 2024
26f28e8
Including migration sql files in build
adamcharnock Jul 22, 2024
c7e68e6
GET_BALANCE() now sets sign correctly (postgres)
adamcharnock Jul 28, 2024
133c401
GET_BALANCE() now sets sign correctly mysql & postgres)
adamcharnock Jul 28, 2024
e8c0088
Updating orm-based balance calculations to calculate correct sign
adamcharnock Jul 28, 2024
6024b46
Fixes to balance calculations given updates to sum_to_balance(). Remo…
adamcharnock Jul 28, 2024
41c07ac
Renaming balance() to get_balance() (as per deprecation in 1.17.0)
adamcharnock Jul 28, 2024
eeee60e
Renaming all uses of balance() to get_balance() (as per deprecation i…
adamcharnock Jul 28, 2024
00d3cd2
Updating admin to use the balance property now it is available
adamcharnock Jul 28, 2024
05b5ab7
Moving account_balance_after() and account_balance_before() to use da…
adamcharnock Jul 28, 2024
c0ef2cc
Moving account_balance_after() and account_balance_before() to use an…
adamcharnock Jul 28, 2024
ef8cf08
get_balance() db function now uses leg_id rather than transcation ID …
adamcharnock Jul 28, 2024
e75321d
Simplifying tests as it was a bit flaky between postgres and mysql
adamcharnock Jul 28, 2024
9e025e3
Docs improvements
adamcharnock Jul 28, 2024
f419477
Docs improvements
adamcharnock Jul 28, 2024
4bf9c55
Adding warning in docs re views
adamcharnock Jul 28, 2024
06e1337
Work on changelog
adamcharnock Jul 28, 2024
ea90bd0
Fixing error in credit/debit data migration, #133
adamcharnock Aug 29, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,20 @@ jobs:

- name: Check All Migrations Exist
run: |
pip install "py-moneyed<3" --upgrade
PYTHONPATH=`pwd` ./manage.py makemigrations --check hordak
pip install "py-moneyed>=3" --upgrade # Different version of py-moneyed should not create different migrations
PYTHONPATH=`pwd` ./manage.py makemigrations --check hordak

- name: Create missing migrations
if: ${{ failure() }}
run: |
PYTHONPATH=`pwd` ./manage.py makemigrations hordak

- name: Archive created migrations for debugging
if: ${{ failure() }}
uses: actions/upload-artifact@v1
with:
name: created-migrations
path: hordak/migrations

- name: Testing (PostgreSQL)
run: |
PYTHONPATH=`pwd` python -Wall -W error::DeprecationWarning -m coverage run ./manage.py test hordak
Expand Down
43 changes: 37 additions & 6 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,13 +1,29 @@
Version 2.0.0 (unreleased)
==========================

* **Breaking**: New account currencies now default to ``DEFAULT_CURRENCY`` rather than all available currencies (trading accounts should be the only accounts with multiple currencies).
.. note::

**Upgrade to Hordak 1.17 before upgrading to 2.0**

Hordak 1.17 will issue deprecation warnings for most issues you may encounter in the
2.0 upgrade. Fix those warnings and your upgrade should be smooth.

* **Breaking**: ``transfer_to()`` was deprecated in 1.16. This old implementation has now been removed
and the newer ``transfer_to_accounting()`` has been renamed to take its place.
Existing uses of ``transfer_to_accounting()`` should be updated to
point to ``transfer_to()``. This change standardises the behaviour of this method on industry norms.
* **Breaking**: ``Leg.account_balance_after()`` and ``Leg.account_balance_before()`` have been removed and replaced
with annotations which populate properties of the same name. Enable this annotations using
``Leg.objects.with_account_balance_after()`` and ``Leg.objects.with_account_balance_before()``
* **Breaking**: New account currencies now default to ``DEFAULT_CURRENCY`` rather than all available currencies
(trading accounts should be the only accounts with multiple currencies).
* **Breaking**: Removed ``django-smalluuid``. UUIDs in URLs will now be rendered in the regular UUID format.
* **Breaking**: Removed use of ``django-model-utils``. Model choices have the same values, but the data structures have changed to use the Django-native ``models.TextChoices``:
* **Breaking**: Balance methods have be renamed to make way for (more performant) annotated properties.
Deprecation notices are issued by Hordak 1.17:

* ``Account.TYPES`` is now ``AccountType``
* ``TransactionCsvImportColumn.TO_FIELDS`` is now ``ToField``
* ``TransactionCsvImport.STATES`` is now ``TransactionCsvImportState``
* ``Account.balance()`` -> ``Account.get_balance()``
* ``Account.simple_balance()`` -> ``Account.get_simple_balance()``
* ``Transaction.balance()`` -> ``Transaction.get_balance()``

* **Feature:** New accounting-oriented database view on the Legs table (keep your accountants happier). Adds columns

Expand All @@ -23,10 +39,25 @@ Version 2.0.0 (unreleased)
* Shows the transaction amount (JSON list, one value per currency)
* Shows credited/debited account IDs & names

* **Feature:** Added ``GET_BALANCE(account_id: BIGINT, as_of: DATE = NULL)`` database function. Will get the balance of an account.
* **Feature:** Added ``GET_BALANCE(account_id: BIGINT, as_of: DATE = NULL, as_of_leg_id: INT = NULL)`` database function. Will get the balance of an account.
* **Feature**: Many balance calculations are now available as query annotations, and are performed in-database. This represents a
significant performance improvement. See the ``AccountQueryset`` and ``LegQueryset``.
* **Enhancement:** Account code max length increased from 3 to 6
* ``Balance.__eq__()`` now returns False rather than raising an exception if the other object is not a ``Balance``
* Removed used of ``django-sql-utils``
* Removed use of ``django-model-utils``. Model choices have the same values, but the data
structures have changed to use the Django-native ``models.TextChoices``. The old syntax will continue to work until Hordak 3.0:

* ``Account.TYPES`` is now ``AccountType``
* ``TransactionCsvImportColumn.TO_FIELDS`` is now ``ToField``
* ``TransactionCsvImport.STATES`` is now ``TransactionCsvImportState``

Version 1.17.0 (unreleased)
===========================

* Deprecated Account.balance(), now renamed to Account.get_balance().
The `balance` property can be pre-populated in Hordak 2.0 using `Account.objects.with_balances()`.
* Deprecated Transaction.balance(), now renamed to Transaction.get_balance().

Version 1.16.0, Fri, 25 June 2024
=================================
Expand Down
1 change: 1 addition & 0 deletions MANIFEST.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ include *.txt
include *.rst
recursive-include docs *
include VERSION
include hordak/migrations/*.sql

# added by check_manifest.py
include *.py
Expand Down
1 change: 1 addition & 0 deletions docs/.gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/_build_html
/_build
/html
1 change: 1 addition & 0 deletions docs/api/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ API Documentation
:maxdepth: 2

models
models_statements
views
forms
utilities_money
Expand Down
19 changes: 7 additions & 12 deletions docs/api/models.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.. _api_models:

Models
======
Models (Core)
=============

.. contents::

Expand All @@ -13,6 +13,10 @@ Account
.. autoclass:: hordak.models.Account
:members:


.. autoclass:: hordak.models.AccountQuerySet
:members:

Transaction
-----------

Expand All @@ -25,16 +29,7 @@ Leg
.. autoclass:: hordak.models.Leg
:members:

StatementImport
---------------

.. autoclass:: hordak.models.StatementImport
:members:

StatementLine
-------------

.. autoclass:: hordak.models.StatementLine
.. autoclass:: hordak.models.LegQuerySet
:members:


Expand Down
16 changes: 16 additions & 0 deletions docs/api/models_statements.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.. _api_models_statements:

Models (Statements)
===================

StatementImport
---------------

.. autoclass:: hordak.models.StatementImport
:members:

StatementLine
-------------

.. autoclass:: hordak.models.StatementLine
:members:
5 changes: 3 additions & 2 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#
import os
import sys
from datetime import datetime


# sys.path.insert(0, os.path.abspath('.'))
Expand Down Expand Up @@ -56,7 +57,7 @@

# General information about the project.
project = "Django Hordak"
copyright = "2016, Adam Charnock"
copyright = f"2016-{datetime.today().year}, Adam Charnock"
author = "Adam Charnock"

# The version info for the project you're documenting, acts as replacement for
Expand All @@ -73,7 +74,7 @@
#
# This is also used if you do content translation via gettext catalogs.
# Usually you set "language" from the command line for these cases.
language = None
language = "en"

# List of patterns, relative to source directory, that match files and
# directories to ignore when looking for source files.
Expand Down
43 changes: 24 additions & 19 deletions hordak/admin.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from django.contrib import admin
from django.db.models import Q, Sum
from django.db.models import Sum
from django.utils.html import escape
from django.utils.safestring import mark_safe
from mptt.admin import MPTTModelAdmin
Expand All @@ -16,10 +16,10 @@ class AccountAdmin(MPTTModelAdmin):
"code_",
"type_",
"currencies",
"balance_sum",
"income",
"balance",
"credits",
"debits",
)
readonly_fields = ("balance",)
raw_id_fields = ("parent",)
search_fields = (
"code",
Expand All @@ -28,31 +28,34 @@ class AccountAdmin(MPTTModelAdmin):
)
list_filter = ("type",)

@admin.display(ordering="balance_sum")
@admin.display(ordering="balance")
def balance(self, obj):
return obj.balance()
if obj.balance is None:
return "-"
return obj.balance

@admin.display(ordering="balance_sum")
def balance_sum(self, obj):
if obj.balance_sum:
return -obj.balance_sum
return "-"
balance.admin_order_field = "balance"

balance_sum.admin_order_field = "balance_sum"
@admin.display(ordering="credits")
def credits(self, obj):
if obj.credits is None:
return "-"
return obj.credits

@admin.display(ordering="income")
def income(self, obj):
if obj.income:
return -obj.income
return "-"
@admin.display(ordering="debits")
def debits(self, obj):
if obj.debits is None:
return "-"
return obj.debits

def get_queryset(self, *args, **kwargs):
return (
super()
.get_queryset(*args, **kwargs)
.with_balances_orm()
.annotate(
balance_sum=Sum("legs__amount"),
income=Sum("legs__amount", filter=Q(legs__amount__gt=0)),
credits=Sum("legs__credit"),
debits=Sum("legs__debit"),
)
)

Expand Down Expand Up @@ -186,6 +189,8 @@ def debit_(self, obj: models.LegView):


def _fmt_admin_decimal(v):
if hasattr(v, "amount"):
v = v.amount
if v is None:
v = "-"
else:
Expand Down
24 changes: 24 additions & 0 deletions hordak/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,3 +85,27 @@ class NoMoreAccountCodesAvailableInSequence(HordakError):
"""

pass


class InvalidOrMissingAccountTypeError(Exception):
"""When an unexpected account type is encountered"""

pass


class NeitherCreditNorDebitPresentError(Exception):
"""When neither credit nor debit present for a Leg"""

pass


class BothCreditAndDebitPresentError(Exception):
"""When both credit and debit present for a Leg"""

pass


class CreditOrDebitIsNegativeError(Exception):
"""When either the credit and debit field of a Leg is negative"""

pass
18 changes: 14 additions & 4 deletions hordak/forms/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,16 @@ def clean_amount(self):

return amount

def save(self, commit=True):
inst: Leg = super().save(commit=False)
if self.cleaned_data["amount"].amount > 0:
inst.credit = self.cleaned_data["amount"]
else:
inst.debit = abs(self.cleaned_data["amount"])

if commit:
inst.save()


class BaseLegFormSet(BaseInlineFormSet):
def __init__(self, **kwargs):
Expand Down Expand Up @@ -240,17 +250,17 @@ def save(self):
description=self.cleaned_data.get("description")
)
Leg.objects.create(
transaction=transaction, account=source_account, amount=source_amount
transaction=transaction, account=source_account, credit=source_amount
)
Leg.objects.create(
transaction=transaction, account=trading_account, amount=-source_amount
transaction=transaction, account=trading_account, debit=source_amount
)
Leg.objects.create(
transaction=transaction, account=trading_account, amount=destination_amount
transaction=transaction, account=trading_account, credit=destination_amount
)
Leg.objects.create(
transaction=transaction,
account=destination_account,
amount=-destination_amount,
debit=destination_amount,
)
return transaction
16 changes: 9 additions & 7 deletions hordak/management/commands/create_benchmark_transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,20 @@ def handle(self, *args, **options):
print("Done")
print("")

print(f"Bank: {str(bank.balance()):<15} {str(bank.legs.count()):<15}")
print(
f"Assets: {str(assets.balance()):<15} {str(assets.legs.count()):<15}"
f"Bank: {str(bank.get_balance()):<15} {str(bank.legs.count()):<15}"
)
print(
f"Expenses: {str(expenses.balance()):<15} {str(expenses.legs.count()):<15}"
f"Assets: {str(assets.get_balance()):<15} {str(assets.legs.count()):<15}"
)
print(
f"Liabilities: {str(liabilities.balance()):<15} {str(liabilities.legs.count()):<15}"
f"Expenses: {str(expenses.get_balance()):<15} {str(expenses.legs.count()):<15}"
)
print(
f"Capital: {str(capital.balance()):<15} {str(capital.legs.count()):<15}"
f"Liabilities: {str(liabilities.get_balance()):<15} {str(liabilities.legs.count()):<15}"
)
print(
f"Capital: {str(capital.get_balance()):<15} {str(capital.legs.count()):<15}"
)


Expand Down Expand Up @@ -101,6 +103,6 @@ def _transfer_no_commit(

transaction = Transaction()
legs = []
legs.append(Leg(transaction=transaction, account=debit, amount=-amount))
legs.append(Leg(transaction=transaction, account=credit, amount=amount))
legs.append(Leg(transaction=transaction, account=debit, debit=amount))
legs.append(Leg(transaction=transaction, account=credit, credit=amount))
return transaction, legs
Loading
Loading