Feature/reconciliation - #12
Open
akeenkarkare wants to merge 14 commits into
Open
Conversation
feature/reconciliation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…ent-mode rules
Easebuzz settlement logs were stored but never turned into accounting entries,
so the Easebuzz PG suspense account never cleared. Build step 3 of the flow --
Dr Bank / Cr Easebuzz suspense -- as an automatic, idempotent posting pipeline.
Parse, preflight, segregate, validate, post, reconcile:
* Payload parsing. `settlement_hook` stores a Python repr of
{'status', 'data', 'cmd'} and the settlement body lives in the *inner* JSON
string. The old code parsed the outer wrapper, so split_payouts,
total_amount and service_charge_amount were never reachable.
`parse_settlement_payload` unwraps the envelope and also accepts bare JSON
and already-parsed dicts.
* One Journal Entry per company, not per split payout. 364 of 400 production
payloads span two companies (UESF and RESPL) and a Journal Entry belongs to
exactly one company, so a single JE for the whole log is not representable
in ERPNext. Each JE carries one debit row per split payout, one charges row,
and one suspense credit; the credit is the sum of the rounded debits, so it
balances exactly rather than to within a rounding error.
* Charge attribution. split_transactions[] carries no account_number -- only
split_payout_id -- which is why the old charges loop resolved a Bank Account
from None and never posted a single charge JE. Charges now join back to a
company through split_payout_id, which resolves cleanly in all 400 payloads.
* Payment-mode rules. New `Easebuzz Payment Mode Rule` child table on Easebuzz
Settings replaces the hardcoded transaction_type filter. A mode with
Debit Charges unticked has its service charge and tax excluded from the
charges line. Bank debits are never filtered -- that money physically
reached the bank, so removing it would produce a JE that disagrees with the
statement. Matching is on the raw Easebuzz string rather than the Mode of
Payment link name, so renaming the ERPNext master cannot silently break a
rule. Duplicate rows are rejected on save.
* Idempotency. Easebuzz redelivers settlements (payout PTOBWKJAUF arrives
twice in the production log set), so an already-posted (payout_id, company)
pair is recorded as Already Posted instead of posting again. A redelivery
arrives as a new log with its own job_id, so `deduplicate` does not apply --
posting is serialised on the payout with a filelock.
* Trigger moved from before_save to an enqueued after_insert. before_save
re-posted the whole settlement on every subsequent save of the log.
* Failures are visible. Status (Pending / Processing / Processed /
Needs Review / Failed / Skipped), the error text, and a per-company
reconciliation row recording the JE, bank total, charges debited, charges
skipped and the modes responsible. The bare except that logged to
`logs/ease.log` -- a file no other easebuzz component writes to -- is gone.
* Preflight fails loudly with the company named when
default_easebuzz_account or custom_easebuzz_charges is unset, rather than
posting `account = None` and swallowing the validation error.
* First JE per company is held as Draft with the log at Needs Review so the
mapping can be confirmed before anything reaches the ledger; auto-submit
unlocks per company once one has been submitted. Settlements older than
Reconcile Settlements On or After are marked Skipped and never posted.
Verified against 400 production payloads: 764 JEs, all balanced, bank totals
identical with rules on and off, included + skipped reconciles to the payload
header, and JEs whose charges are entirely skipped get no charges line rather
than a 0.00 one. Verified end to end against real ERPNext for payout
PT1J0YM50V: RESPL 1,018,437.87 + UESF 3,275,405.49 = 4,293,843.36 =
total_amount, difference 0.0, both submitted, then rolled back.
Note: split-level charge sums drift from the payload header by up to 0.13, so
the reconciliation tolerance defaults to 0.50 rather than 0.05.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reconciliation branch never picked up the packaging metadata that prod carries, so the app had only the legacy setup.py and no ruff configuration. Restore prod's file verbatim so the two branches converge cleanly on merge rather than diverging on build backend or lint rules. Declares flit as the build backend with the version read from easebuzz.__version__, requires Python >=3.10, pins frappe >=15.40.4,<16.0.0 for bench, and configures ruff (line length 110, target py310). setup.py, requirements.txt and MANIFEST.in are left in place, matching prod. Linting the app under this config reports 86 pre-existing findings across 10 legacy files -- mostly true-false comparisons, trailing whitespace and unsorted imports. None are in the reconciliation code and none are fixed here; that backlog is left for a separate pass so this commit stays packaging-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ccount on reconcilation
…e insert Saving an Easebuzz Settlement Log did nothing, and with reconciliation switched on it failed outright. Two causes, both fixed here. Queueing could abort ingestion. `after_insert` called `frappe.enqueue` unguarded, and enqueue raises ConnectionError when Redis is unreachable (it only falls back to running synchronously during a migration). The exception escaped `after_insert` and rolled back the insert, so on a bench with no queue the settlement log was never saved at all -- the payload was lost, and the gateway posts it exactly once. Reproduced on unity.local: with Auto Create Journal Entry on and redis down, insert failed with ConnectionError; with no settings record it saved but silently did nothing. Queueing is now wrapped so nothing in it can raise. When the queue is unreachable the work is registered on `frappe.db.after_commit` instead, which runs it outside the save cycle, so a bench without workers still reconciles. Trigger moved from `after_insert` to `on_update`, which fires on insert and on every later save. Saving a log now starts reconciliation, and re-saving a Failed one retries it -- fix the bank account mapping, hit save, it posts. This is safe now in a way the original `before_save` hook was not: the pipeline is idempotent, so a re-run records Already Posted rather than posting twice. Only Pending and Failed logs auto-fire; Processed, Processing, Needs Review and Skipped are left alone so re-saving cannot refire them. `_finish` marks its own save with a flag so writing the outcome does not start another round. Silent no-ops are now explained on the record. Previously a missing Easebuzz Settings record or an unticked Auto Create Journal Entry returned without a trace, which is indistinguishable from a broken hook. The reason is written to the log's Error Message instead, and cleared once reconciliation starts. Verified on unity.local across all four paths: no settings record -> saves, Pending, reason recorded; Auto Create off -> saves, reason names the settings record; Auto Create on with redis down -> saves (previously aborted) and the after-commit fallback runs and records the real failure reason; re-saving a Failed log retries it and terminates. 19 unit tests pass, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.