django-hordak
django-hordak copied to clipboard
Change of architecture :)
Update from @adamcharnock: Implementation now in #133
I know it would be a huge re-factor but using a credit and debit field on each leg rather than a signed amount would ease your problems with signs. The accounting module of the Tryton project uses this principle. Instead of transaction/leg they use the terminology move/line. https://github.com/tryton/account/blob/1b182f260d52ea7767547ff688ee94e24f29092a/move.py#L562
Thanks for this module! I am using it in a payroll app.
Hi @lie-nielsen, sorry for the delay in replying!
You could well be right. From my point of view, I was coming to this from a strong engineering and software background, and it seemed very odd to me that the meaning of debit and credit would change depending on the account type. Using signs made the engineer in me much happier (see Double Entry Accounting for Developers in the docs).
That being said, I came to this with minimal accountancy experience. I'm very willing to accept that there are good reasons why Hordak's implementation is less than ideal, but I'd just need those reasons spelling out to me.
I'd be happy to see a change of architecture in Hordak 2.0, and I'd also be happy for that process to involve adding an additional maintainer to the project (as per #43).
Also, for the record, I'd like to see the 'account code' field ditched in Hordak 2 (#40, #41), it causes a lot of hassle and I don't think it gets much use :-)
adam im not sure if this project is still maintained or not but if you are looking for concrete implementation of double entry acounting for hordak 2.0 you should have look at this stackoferflow answer https://stackoverflow.com/questions/59432964/relational-data-model-for-double-entry-accounting/59465148#59465148
adam im not sure if this project is still maintained or not but if you are looking for concrete implementation of double entry acounting for hordak 2.0 you should have look at this stackoferflow answer https://stackoverflow.com/questions/59432964/relational-data-model-for-double-entry-accounting/59465148#59465148
I'm getting pretty deep into hordak and ledgers in general.
@rajeshr188 thanks for linking that stack post, I learned a lot from it. If you follow the comments, it seems like there was a pretty heated battle - lots of ego, but I think there were enough corrections to the answer that it makes a lot of sense in the end. In particular the Data Model diagram is a great starting place.
Benefits of Dr/Cr vs +/-
- T accounts are SUPER easy to build
- No specialized knowledge needed to use DB data to build external dashboards (i.e. metabase, powerbi).
Drawbacks
- Account Balance is as simple as
SUM(...)in +/-. Would need a more complex-function/sql-query. (I imagine why @adamcharnock built it like this initially)
I'm sure there's more to add, but I think the biggest reason to do a rewrite + change is the fact that we can hand any accountant/auditor that knows SQL and they can put together the audit. Same with anyone building a Metabase dashboard, otherwise you need to learn how Hordak inserted the data and your queries become specialized.
FWIW - Hordak's data model is quite good and assumes very little of the use case. It actually covers the StackOverflow post image quite well (arguably better).
I blurred out the sections that don't apply, but if you were to map the equivlent objects it would be:
Ledger==AccountLedgerTransaction==LegAccountTransaction==Transaction
Hordak can already do >2 Legged transactions.
In the StackOverflow's post about Account which I'll call SO_Account, I argue that Hordak shouldn't care and remain a general library that doesn't carry specific use cases.
@nitsujri can i have look at your implementation of @performancedba s stackoverflow answer please?
So I'm pretty new to Django and Hordak, accounting in general, so I'm definitely open to feedback on our data models.
For us, we use Hordak directly and almost as is. So to do the "External" part of @performancedba SO_Post and make it useful to our business case we:
- Have "org objects" have accounts,
PartnersMerchants -> various accounts - Have an
TransactionSourcejoin object/table.
class Partner(TimeStampedModel):
account_assets = models.ForeignKey(Account...)
def setupAccounts(self):
self.account_assets = Account.objects.create(type=Account.TYPES.asset, name..., currencies...)
...other accounts
Above here is pretty boring, I think everyone is doing this.
class TransactionSource(TimeStampedModel):
transaction = models.ForeignKey(Transaction...)
payment = models.ForeignKey(Payment..., null=True, blank=True)
some_other_money = models.ForeignKey(Payment..., null=True, blank=True)
SOURCE_NAMES = {
"payment": "Payment",
"some_other_money": "Some Other Money",
}
@property
def source(self):
for source_key in self.SOURCE_NAMES.keys():
source = getattr(self, source_key)
if source:
return source
@property
def source_display_name(self):
for source_key in self.SOURCE_NAMES.keys():
source = getattr(self, source_key)
if source:
return self.SOURCE_NAMES[source_key]
So the most interesting question is "Why did Money move?" and if you view @performancedba's model diagram it's very suited for a Bank. It's not great at things like Ecommerce where the "AccountTransaction" should point to say an Invoice/Order.
To answer "Why did Money move?" we are using a hard link via TransactionSource which is a form of Polymorphism but with hard links via multiple columns (makes the RMDBS gods happy). Now, building richer descriptions and auditing can be easier.
Every time we move money we have to do a bit more code:
invoice_payment = InvoicePayment...
hordak_trxn = partner.assets_cash.transfer_to(
partner.assets_expense,
invoice_payment.amount,
description=_("Payment for Widget"),
)
audit = TransactionSource(transaction=hordak_trxn, payment=invoice_payment)
audit.full_clean()
audit.save()
So now I can query Transaction.objects.all().first().transaction_source.source and get back what exactly caused this transaction to happen.
Again, I'm quite new to this, so would love feedback.
Monkey Patches
Nothing special here, just making things easier for us to view/debug.
from hordak.models import Account, Leg, Transaction
from hordak.utilities.currency import Balance
def trxn_new_str_rep(self):
return f"Transaction#{self.id}: {self.description[:15]} {self.timestamp.strftime('%d-%m-%Y')}"
Transaction.__str__ = trxn_new_str_rep
def in_currency(self, currency):
return self.normalise(currency).monies()[0]
Balance.in_currency = in_currency
def get_transactions(self):
account_ids = self.get_descendants(include_self=True).values_list("id", flat=True)
return Transaction.objects.filter(legs__account_id__in=account_ids).order_by(
"-timestamp"
)
Account.get_transactions = get_transactions
def leg_new_str_rep(self):
return f"Leg#{self.id}: {self.amount}; {self.account.name}; {self.transaction.description[:15]}"
Leg.__str__ = leg_new_str_rep
Thank you for all the well thought-through and constructive comments on this. As it stands right now – this project is currently maintained by @PetrDlouhy, which I appreciate greatly. I sometimes come by and leave high-level non-mandatory comments, like now.
Cr/Dr
I can see that being able to view Hordak's internels through the lens of Dr/Cr would be helpful in some cases. In particular when interacting specifically with the database. This is the traditional view and sometimes it would be great to be able to access that via SQL (either because it makes more sense to trained-humans, or because writing queries based on Dr/Cr can make more sense sometimes)
Changing architecture is a bad idea
My reasons are entirely due to the reality of the project:
- Many people have already built complex systems atop Hordak and this would certainly be a breaking change. I would expect these existing systems to require signifiant code changes. This would split our user base into Hordak v1 and the new Hordak v2 (think the hellish Python 2/3 transition). Except in our case I think (IMHO) some/many people wouldn't make the transition.
- As a result we would need to split resources between supporting Hordak v1/v2. We just don't have the resources for this. And even if we did, I don't think this would be the right way to spend them.
The best of both worlds
But! I really can see that there is a demand for this. So my suggestion is that we create one or more views in the database. These views allow users to access the underlying +/- tables in a Dr/Cr format.
This way data is available in the format that is being asked for, but with minimal actual development overhead and no breaking changes.
We already keep a lot of logic in the database (which I feel good about), so I don't think anything will be lost by adding whatever views are needed.
Thoughts welcome!
@nitsujri I'm oretty sure why money moved is or can be derived from xacttypecode_ext inperformancedba schema
@nitsujri i ll be happy to show my implementation if you would like to review and criticize , I'm planning to publish it as django-dea
@nitsujri i ll be happy to show my implementation if you would like to review and criticize , I'm planning to publish it as django-dea
Would love to take a look. Tag in your repo once you've released so we can move the discussion over there. This thread isn't really the best place for that.
@adamcharnock thanks for the feedback and definitely agree the full rewrite is not necessary.
Similar to the view, but one step closer, I propose adding 2 extra columns:
accounting_amount- moneyed; positive onlyaccounting_type- DR/CR
Method Changes
Leg#save- pre-fillsaccounting_amount,accounting_typecolumns if empty.Account#accounting_transfer_to- fills new accounting columns
Method Additions
Leg#is_accounting_debitLeg#is_accounting_creditTransaction.multi_leg_transfer- i.e.multi_leg_transfer(from=[(account1, amount), (account2, amount2)], to=[(account3, amount3), (account4, amount4), (account5, amount5)] # returns Transaction- Helper function to set DR/CR.
- Expand DB check_leg to check new colunn
amount+amount2 == amount3+amount4+amount5
To ease migration path, releases are incremental over years:
- 1 minor version - columns allowed empty, only get filled with data via
accounting_transfer_to - +1 minor version
- add data migration to backfill
Legs with empty accounting_amount/type. - migration to set null=False for 2 new columns
- add data migration to backfill
- 2.0 version
- Change balance() to use
account_columns - Update html templates to Accounting Debit/Credit
raiseerror ontransfer_to,is_credit,is_debit- Maintain backward compat by converting
Leg#saveto backfillLeg.amountif empty
- Change balance() to use
My Thoughts
While I understand it's not as easy/clean of a solution compared to a view, I'm motivated to help migrate this for 2 reasons:
- Closes the gap Hordak method vs accounting method.
- I needed to debug a problem with our accountant and while a view would help, I spent quite a bit of time translating our accountant's fix into the current data-model having to do multiple back-and-forths to ensure the correction and future code was applied correctly.
Thoughts/Corrections?