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 21 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
14 changes: 12 additions & 2 deletions CHANGES.txt
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
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).
**Important:** It is strongly recommended that you
**upgrade to Hordak 1.16 before upgrading to Hordak 2.0**. Update your code
to address any depreciation warnings in 1.16 before upgrading to Hordak 2.0.

* **Breaking**: ``to_account()`` was deprecated in 1.16. This old implementation has now been removed
adamcharnock marked this conversation as resolved.
Show resolved Hide resolved
and the newer ``to_account_accounting()`` has been renamed to take its place.
Existing uses of ``to_account_accounting()`` should be updated to
point to ``to_account()``. This change standardises the behaviour of this method on industry norms.
* **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**: 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``
Expand Down
13 changes: 10 additions & 3 deletions hordak/admin.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from django.contrib import admin
from django.db.models import Q, Sum
from django.db.models import DecimalField, Q, Sum
from django.db.models.functions import Coalesce
from django.utils.html import escape
from django.utils.safestring import mark_safe
from mptt.admin import MPTTModelAdmin
Expand Down Expand Up @@ -51,8 +52,12 @@ def get_queryset(self, *args, **kwargs):
super()
.get_queryset(*args, **kwargs)
.annotate(
balance_sum=Sum("legs__amount"),
income=Sum("legs__amount", filter=Q(legs__amount__gt=0)),
# TODO: I think this needs some sign-changing work
adamcharnock marked this conversation as resolved.
Show resolved Hide resolved
balance_sum=Sum(
Coalesce("legs__credit", 0, output_field=DecimalField())
- Coalesce("legs__debit", 0, output_field=DecimalField())
),
income=Sum("legs__credit", filter=Q(legs__credit__isnull=False)),
)
)

Expand Down Expand Up @@ -186,6 +191,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
4 changes: 2 additions & 2 deletions hordak/management/commands/create_benchmark_transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,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
10 changes: 6 additions & 4 deletions hordak/migrations/0044_legview.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
import djmoney.models.fields
from django.db import migrations, models

from hordak.defaults import DECIMAL_PLACES


class Migration(migrations.Migration):
dependencies = [
Expand Down Expand Up @@ -350,7 +352,7 @@ class Migration(migrations.Migration):
(
"amount",
djmoney.models.fields.MoneyField(
decimal_places=6,
decimal_places=DECIMAL_PLACES,
default_currency="EUR",
max_digits=20,
verbose_name="amount",
Expand All @@ -368,7 +370,7 @@ class Migration(migrations.Migration):
(
"credit",
models.DecimalField(
decimal_places=6,
decimal_places=DECIMAL_PLACES,
help_text="Amount of this credit, or NULL if not a credit",
max_digits=20,
adamcharnock marked this conversation as resolved.
Show resolved Hide resolved
verbose_name="credit amount",
Expand All @@ -377,7 +379,7 @@ class Migration(migrations.Migration):
(
"debit",
models.DecimalField(
decimal_places=6,
decimal_places=DECIMAL_PLACES,
help_text="Amount of this debit, or NULL if not a debit",
max_digits=20,
verbose_name="debit amount",
Expand All @@ -386,7 +388,7 @@ class Migration(migrations.Migration):
(
"account_balance",
models.DecimalField(
decimal_places=6,
decimal_places=DECIMAL_PLACES,
help_text="Account balance following this transaction. For multiple-currency accounts this will be the balance of the same currency as the leg amount.",
max_digits=20,
verbose_name="account balance",
Expand Down
3 changes: 2 additions & 1 deletion hordak/migrations/0048_hordak_transaction_view.pg.sql
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,5 @@ INNER JOIN LATERAL (
GROUP BY T.id, T.uuid, T.timestamp, T.date, T.description
ORDER BY T.id DESC);
--- reverse:
drop view hordak_transaction_view;
-- can be implicitly dropped by migration 0050
drop view if exists hordak_transaction_view;
Loading
Loading