fix(fusion_accounting_bank_rec): MV correctness for V19 schema + Odoo test harness
Three issues surfaced when running the MV smoke tests against westin-v19: 1. account_bank_statement_line has no `date` column in V19 — `date` is a related field flowing through move_id -> account_move.date. The MV now JOINs account_move and selects am.date. 2. is_reconciled is nullable; replace `= FALSE` with `IS NOT TRUE` so nulls (genuinely unreconciled lines that haven't had the compute run yet) are still included. 3. _refresh() now flushes the ORM cache (env.flush_all()) before the REFRESH so computed-stored fields like is_reconciled are written to the DB before the materialization snapshot reads them. Previously the reconcile-then-refresh path saw the pre-reconcile column value. 4. _trigger_mv_refresh() (suggestion create/write hook) now uses concurrently=False because Postgres forbids REFRESH MATERIALIZED VIEW CONCURRENTLY inside a transaction block, and Odoo's per-request cursor is always inside one. The cron path (Task 25) will open an autocommit cursor for CONCURRENTLY refreshes. 5. Tests dropped the env.cr.commit() pattern: Postgres always shows a transaction its own writes, so a non-CONCURRENTLY refresh in the same txn picks up freshly-inserted rows. Cleaner + works inside TransactionCase, which forbids cr.commit(). Verified: 4 new MV tests pass, 0 failures across 118 logical tests (178 with parametrized property-based runs) of fusion_accounting_bank_rec on westin-v19. Made-with: Cursor
This commit is contained in:
@@ -2,12 +2,15 @@
|
||||
-- Refreshed on cron (Task 25) and on suggestion writes.
|
||||
-- Indexed on (company_id, journal_id, date) for fast UI queries.
|
||||
|
||||
-- NOTE: account_bank_statement_line does not store `date` directly in V19;
|
||||
-- it is a related field through move_id -> account_move.date. We JOIN on
|
||||
-- account_move to get it.
|
||||
CREATE MATERIALIZED VIEW IF NOT EXISTS fusion_unreconciled_bank_line_mv AS
|
||||
SELECT
|
||||
bsl.id AS id,
|
||||
bsl.company_id AS company_id,
|
||||
bsl.journal_id AS journal_id,
|
||||
bsl.date AS date,
|
||||
am.date AS date,
|
||||
bsl.amount AS amount,
|
||||
bsl.payment_ref AS payment_ref,
|
||||
bsl.currency_id AS currency_id,
|
||||
@@ -41,7 +44,8 @@ SELECT
|
||||
WHERE p.partner_id = bsl.partner_id AND p.company_id = bsl.company_id LIMIT 1), 0)
|
||||
AS partner_reconcile_count
|
||||
FROM account_bank_statement_line bsl
|
||||
WHERE bsl.is_reconciled = FALSE;
|
||||
JOIN account_move am ON am.id = bsl.move_id
|
||||
WHERE bsl.is_reconciled IS NOT TRUE;
|
||||
|
||||
-- Indexes for the common UI queries: filter by company + journal, sort by date desc.
|
||||
CREATE INDEX IF NOT EXISTS fusion_mv_unrec_company_journal_date_idx
|
||||
|
||||
@@ -120,10 +120,18 @@ class FusionReconcileSuggestion(models.Model):
|
||||
return res
|
||||
|
||||
def _trigger_mv_refresh(self):
|
||||
"""Best-effort MV refresh; never poison the originating transaction."""
|
||||
"""Best-effort MV refresh; never poison the originating transaction.
|
||||
|
||||
Uses concurrently=False because Postgres forbids
|
||||
REFRESH MATERIALIZED VIEW CONCURRENTLY inside a transaction block,
|
||||
and Odoo's per-request cursor is always in a transaction. The cron
|
||||
job (Task 25) opens a dedicated autocommit cursor for CONCURRENTLY
|
||||
refreshes when the MV grows large enough that a brief blocking
|
||||
refresh becomes objectionable.
|
||||
"""
|
||||
try:
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=True)
|
||||
concurrently=False)
|
||||
except Exception as e: # noqa: BLE001
|
||||
_logger.warning(
|
||||
"MV refresh after suggestion write failed: %s", e)
|
||||
|
||||
@@ -63,7 +63,12 @@ class FusionUnreconciledBankLineMV(models.Model):
|
||||
REFRESH MATERIALIZED VIEW CONCURRENTLY (requires the unique index).
|
||||
Falls back to a blocking refresh on the first refresh after creation
|
||||
(when CONCURRENTLY is not yet allowed because the MV has never been
|
||||
populated)."""
|
||||
populated).
|
||||
|
||||
Flushes the ORM cache first so the materialization sees the latest
|
||||
committed-to-DB values for fields like ``is_reconciled`` (computed,
|
||||
stored — sometimes still buffered in the cache mid-request)."""
|
||||
self.env.flush_all()
|
||||
keyword = "CONCURRENTLY" if concurrently else ""
|
||||
try:
|
||||
self.env.cr.execute(
|
||||
|
||||
@@ -1,17 +1,17 @@
|
||||
"""Smoke tests for the fusion_unreconciled_bank_line_mv materialized view.
|
||||
|
||||
NOTE on the explicit ``self.env.cr.commit()`` calls below:
|
||||
PostgreSQL's ``REFRESH MATERIALIZED VIEW`` only sees data from
|
||||
*committed* transactions. ``TransactionCase`` rolls back at the end
|
||||
of each test, so without an explicit commit the freshly-inserted
|
||||
bank line would not be visible to the refresh and the assertions
|
||||
would fail. The trade-off is that the records we create *are*
|
||||
persisted; we therefore unlink them in a ``finally`` block to keep
|
||||
test isolation.
|
||||
|
||||
If this proves too brittle later we can convert this case to extend
|
||||
``BaseCase`` (no rollback) and clean up explicitly. For Phase 1 the
|
||||
commit-+-finally pattern is the simpler choice.
|
||||
Notes on transactional semantics:
|
||||
- REFRESH MATERIALIZED VIEW (non-CONCURRENTLY) IS transactional and runs
|
||||
inside the current transaction. Postgres always shows a transaction
|
||||
its own (uncommitted) writes, so an INSERT followed by a REFRESH in
|
||||
the same transaction picks up the new row — no `cr.commit()` needed.
|
||||
- Odoo's TransactionCase forbids cr.commit() anyway (it would break the
|
||||
per-test savepoint rollback). We rely on rollback to clean up both
|
||||
the test fixtures and the MV-table mutations from the refresh.
|
||||
- REFRESH MATERIALIZED VIEW CONCURRENTLY must run OUTSIDE a transaction
|
||||
block; we always pass concurrently=False from tests. The production
|
||||
cron path (Task 25) will open a dedicated autocommit cursor for the
|
||||
concurrent refresh.
|
||||
"""
|
||||
|
||||
from odoo.tests.common import TransactionCase, tagged
|
||||
@@ -26,9 +26,8 @@ class TestUnreconciledBankLineMV(TransactionCase):
|
||||
self.partner = self.env['res.partner'].create({
|
||||
'name': 'MV Test Partner',
|
||||
})
|
||||
# Force a refresh so we see freshly-inserted lines from prior tests.
|
||||
# First refresh after creation may need to be blocking; the
|
||||
# _refresh helper handles fallback automatically.
|
||||
# Refresh once at the start so the MV reflects the current snapshot
|
||||
# (including any rows inserted earlier in this savepoint chain).
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=False)
|
||||
|
||||
@@ -41,75 +40,43 @@ class TestUnreconciledBankLineMV(TransactionCase):
|
||||
def test_mv_includes_unreconciled_line(self):
|
||||
bank_line = f.make_bank_line(
|
||||
self.env, amount=999.99, partner=self.partner)
|
||||
# MV refresh sees committed data only; commit, refresh, assert,
|
||||
# then unlink to keep test isolation.
|
||||
self.env.cr.commit()
|
||||
try:
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=False)
|
||||
mv_row = self.env['fusion.unreconciled.bank.line.mv'].search([
|
||||
('id', '=', bank_line.id),
|
||||
])
|
||||
self.assertTrue(
|
||||
mv_row,
|
||||
"MV should contain freshly-inserted unreconciled line")
|
||||
self.assertAlmostEqual(mv_row.amount, 999.99, places=2)
|
||||
# No suggestion yet -> band 'none', confidence 0.
|
||||
self.assertEqual(mv_row.confidence_band, 'none')
|
||||
self.assertEqual(mv_row.attachment_count, 0)
|
||||
finally:
|
||||
bank_line.unlink()
|
||||
self.env.cr.commit()
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=False)
|
||||
mv_row = self.env['fusion.unreconciled.bank.line.mv'].search([
|
||||
('id', '=', bank_line.id),
|
||||
])
|
||||
self.assertTrue(
|
||||
mv_row,
|
||||
"MV should contain freshly-inserted unreconciled line")
|
||||
self.assertAlmostEqual(mv_row.amount, 999.99, places=2)
|
||||
# No suggestion yet -> band 'none', confidence 0.
|
||||
self.assertEqual(mv_row.confidence_band, 'none')
|
||||
self.assertEqual(mv_row.attachment_count, 0)
|
||||
|
||||
def test_mv_excludes_reconciled_line(self):
|
||||
bank_line, recv_lines = f.make_reconcileable_pair(
|
||||
self.env, amount=100.00, partner=self.partner)
|
||||
self.env['fusion.reconcile.engine'].reconcile_one(
|
||||
bank_line, against_lines=recv_lines)
|
||||
self.env.cr.commit()
|
||||
try:
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=False)
|
||||
mv_row = self.env['fusion.unreconciled.bank.line.mv'].search([
|
||||
('id', '=', bank_line.id),
|
||||
])
|
||||
self.assertFalse(
|
||||
mv_row, "Reconciled line should be excluded from MV")
|
||||
finally:
|
||||
# Best-effort unwind for test isolation. Use the engine's
|
||||
# standard undo path since reconcile_one rewrites the bank
|
||||
# move's line_ids.
|
||||
try:
|
||||
bank_line.action_undo_reconciliation()
|
||||
except Exception:
|
||||
pass
|
||||
try:
|
||||
bank_line.unlink()
|
||||
except Exception:
|
||||
pass
|
||||
self.env.cr.commit()
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=False)
|
||||
mv_row = self.env['fusion.unreconciled.bank.line.mv'].search([
|
||||
('id', '=', bank_line.id),
|
||||
])
|
||||
self.assertFalse(
|
||||
mv_row, "Reconciled line should be excluded from MV")
|
||||
|
||||
def test_mv_confidence_band_high_for_high_conf_suggestion(self):
|
||||
bank_line = f.make_bank_line(
|
||||
self.env, amount=500.00, partner=self.partner)
|
||||
f.make_suggestion(
|
||||
self.env, statement_line=bank_line, confidence=0.92)
|
||||
self.env.cr.commit()
|
||||
try:
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=False)
|
||||
mv_row = self.env['fusion.unreconciled.bank.line.mv'].search([
|
||||
('id', '=', bank_line.id),
|
||||
])
|
||||
self.assertTrue(mv_row, "MV row should exist for suggestion line")
|
||||
# 0.92 falls in the 'high' band per the SQL CASE (>= 0.85).
|
||||
self.assertEqual(mv_row.confidence_band, 'high')
|
||||
self.assertAlmostEqual(mv_row.top_confidence, 0.92, places=2)
|
||||
finally:
|
||||
# Unlink suggestion first (cascade would handle it but explicit
|
||||
# is safer if the test order reuses partner records).
|
||||
self.env['fusion.reconcile.suggestion'].search([
|
||||
('statement_line_id', '=', bank_line.id),
|
||||
]).unlink()
|
||||
bank_line.unlink()
|
||||
self.env.cr.commit()
|
||||
self.env['fusion.unreconciled.bank.line.mv']._refresh(
|
||||
concurrently=False)
|
||||
mv_row = self.env['fusion.unreconciled.bank.line.mv'].search([
|
||||
('id', '=', bank_line.id),
|
||||
])
|
||||
self.assertTrue(mv_row, "MV row should exist for suggestion line")
|
||||
# 0.92 falls in the 'high' band per the SQL CASE (>= 0.85).
|
||||
self.assertEqual(mv_row.confidence_band, 'high')
|
||||
self.assertAlmostEqual(mv_row.top_confidence, 0.92, places=2)
|
||||
|
||||
Reference in New Issue
Block a user