flash-oss / medici

Double-entry accounting system for nodejs + mongoose
MIT License
308 stars 84 forks source link

After upgrading to 5.1.0 retrieving balance using custom properties as keys returns 0 #58

Closed cycleguy closed 2 years ago

cycleguy commented 2 years ago

@koresar Thank you for this module.

We have created a test program that demonstrates this. We have introduced two custom properties: _businessId and _assetId.

For the 3 test cases the correct balance should be

To run without custom schema defined (ensure line 15 in app.js is commented out) Tun run with custom schema defined (ensure line 15 in app.js is uncommented)

Possibly we are doing something wrong in our schema definition. Thanks in advance for your help.

medici-v5.zip

Uzlopak commented 2 years ago

The implicit registration of Schemas is obsolete with medici v5. you have to write a mongoose schema and use setTransactionSchema use as first parameter the Schema. Based on the Schema, medici v5 determines if it is a meta key or root key. As you did not assigned it, medici tries to search for _businessId and _assetId in the meta field and not on the root field

Also be aware: In medici v4 there was an implicit transformation of all vaues to ObjectIds if the key was starting with an _. Now based on the Schema, we determine if it is an ObjectId or not.

cycleguy commented 2 years ago

We have written a mongoose schema and we are using setTransactionSchema within the mediciTransaction.js file. You mentioned that we did not assign it, what exactly are you referring to? Thanks.

Uzlopak commented 2 years ago

In your zipfile you don't call setTransactionSchema.

cycleguy commented 2 years ago

I double checked the zip file that I sent you....

dolcalmi commented 2 years ago

We have the same issue, I think is a problem in parseBalanceQuery https://github.com/flash-oss/medici/blob/master/src/helper/parse/parseBalanceQuery.ts#L38 should use the same logic used in parseFilterQuery https://github.com/flash-oss/medici/blob/master/src/helper/parse/parseFilterQuery.ts#L52

koresar commented 2 years ago

@dolcalmi @cycleguy any chance you can fix the parseBalanceQuery for us in a PR? That would be awesome and the fastest solution.

cycleguy commented 2 years ago

Thank you in advance @dolcalmi for taking the time to fix this issue.

koresar commented 2 years ago

The difference exists because the balance query (not the balance, but the query JSON) is being serialised to a single string as a Quick Balance cache key: https://github.com/flash-oss/medici/blob/12dd0b3b778cd0341a1492020323da4b7d4bb971/src/models/balance.ts#L48

The key is being constructed wrongly when schema is not native and you query by our own keys.

Quick workarounds would be to disable Quick Balance feature. Pass balanceSnapshotSec as zero: new Book(name, { balanceSnapshotSec: 0 });

dolcalmi commented 2 years ago

this is not the only issue, the problem is how the query is parsed. I will work on it today

dolcalmi commented 2 years ago

@koresar this can be closed once the new version is released

koresar commented 2 years ago

Published in v5.2