Skip to content

[REF] account_payment_pro_receiptbook: batch made_sequence_gap writes - #1187

Closed
ica-adhoc wants to merge 1 commit into
ingadhoc:19.0from
adhoc-dev:19.0-h-126129-ica-1
Closed

[REF] account_payment_pro_receiptbook: batch made_sequence_gap writes#1187
ica-adhoc wants to merge 1 commit into
ingadhoc:19.0from
adhoc-dev:19.0-h-126129-ica-1

Conversation

@ica-adhoc

Copy link
Copy Markdown
Contributor

Problema

_update_receiptbook_made_sequence_gap() asigna made_sequence_gap registro por registro, y escribe siempre, cambie o no el valor guardado:

for move in moves:
    move.made_sequence_gap = move.sequence_number > 1 and (move.sequence_number - 1) not in existing_numbers

Como el campo es stored, cada asignación es un write() sobre account.move y arrastra los overrides de todos los módulos instalados. El más caro es el de Purchase, que hace move.mapped('line_ids.purchase_line_id.order_id') por asiento. Sobre recordsets grandes eso escala mal — en el caso extremo (barrido de toda la tabla) termina en MemoryError, que es lo que corrige #1186.

El core evita exactamente esto en _update_sequence_made_gap():

Since the value is stored, we prevent unnecessary writes to made_sequence_gap by only assigning the value if it differs from the checks

Solución

Acumular los ids que hacen hueco y escribir al final, como mucho dos veces (True / False), solo sobre los asientos cuyo valor guardado difiere. En la práctica lo habitual es 0 writes, porque el valor ya es el correcto.

Sin cambio funcional: el made_sequence_gap resultante es idéntico.

Test plan

Simulación de ambas versiones del método sobre datos sintéticos, comparando estado final y contando writes. 25 escenarios: el barrido completo más 24 llamadas puntuales de 1 a 3 asientos, que es la forma en que el método se llama en runtime desde _update_sequence_made_gap().

escenarios: 25 | asientos en el universo: 43
estado final distinto: 0
writes acumulados  ->  antes: 66 | ahora: 28

Este PR es independiente de #1186 y se puede mergear en cualquier orden: aquel toca solo migrations/19.0.2.4.0/post-migration.py, este solo models/account_move.py.

Ticket: https://www.adhoc.inc/odoo/helpdesk.ticket/126129

Copilot AI lite review requested due to automatic review settings August 24, 2026 17:01
@roboadhoc

Copy link
Copy Markdown
Contributor

Pull request status dashboard

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Este PR refactoriza _update_receiptbook_made_sequence_gap() en el módulo account_payment_pro_receiptbook para evitar writes registro-por-registro sobre account.move (y el costo de overrides aguas abajo), manteniendo el mismo resultado funcional del flag made_sequence_gap.

Changes:

  • Acumula los account.move que deben quedar con made_sequence_gap=True y difiere la escritura.
  • Reduce los writes a como mucho 2 operaciones (set/unset) y solo cuando el valor almacenado realmente difiere.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +123 to +126
if to_set := moves_to_check.filtered(lambda m: m.id in gap_ids and not m.made_sequence_gap):
to_set.made_sequence_gap = True
if to_unset := moves_to_check.filtered(lambda m: m.id not in gap_ids and m.made_sequence_gap):
to_unset.made_sequence_gap = False
- Collect the ids that make a gap instead of assigning move by move in
  _update_receiptbook_made_sequence_gap
- Write at most twice (True / False) and only on the moves whose stored
  value actually changes, same criterion as the core
  _update_sequence_made_gap
- Each assignment is a write on account.move and drags the overrides of
  every installed module (purchase maps
  line_ids.purchase_line_id.order_id per move), so the previous
  per-record loop scaled badly on large recordsets
- No functional change: the resulting made_sequence_gap is identical
- Add tests: gap detection inside a receiptbook, scoping to the
  receiptbook instead of the journal, and no write when the stored value
  is already correct

Change note: Mejora interna de rendimiento. Al registrar o postear recibos en tandas grandes, el sistema ya no reescribe uno por uno el control de numeración de los talonarios: actualiza solamente los recibos cuyo valor cambia. No hay cambios visibles en pantalla ni en el comportamiento de la numeración.
@ica-adhoc
ica-adhoc force-pushed the 19.0-h-126129-ica-1 branch from 323c014 to 77377c5 Compare August 24, 2026 17:23
@ica-adhoc

Copy link
Copy Markdown
Contributor Author

Sumé tests al PR (amend sobre el mismo commit).

test_made_sequence_gap_writes_only_on_change — es el que guarda el cambio de este PR. Postea 5 recibos, deja el estado consistente y después:

  1. recomputa sin que nada haya cambiado → no debe haber ningún write con made_sequence_gap;
  2. invierte los valores guardados por SQL (bypass del ORM) y recomputa → el estado vuelve a ser el mismo de antes, en como mucho dos writes agrupados, no uno por asiento.

El conteo se hace envolviendo account.move.write con un spy que llama al original — los registros son reales, creados con .create(); no se mockea el ORM.

test_made_sequence_gap_within_receiptbook y test_made_sequence_gap_scoped_to_receiptbook — guardan la semántica que el refactor tiene que preservar: que el hueco se detecte dentro del talonario, y que un hueco en un talonario no marque los asientos de otro que numera en el mismo diario.

Los números de secuencia se asertan de forma relativa, no absoluta: ir.sequence se apoya en una secuencia de postgres, que no se rollbackea con la transacción del test, así que el talonario no arranca en 1 en cada corrida.

Corridas

Los 9 tests del módulo, sobre una base 19.0 limpia con el módulo instalado:

account_payment_pro_receiptbook: 11 tests 4.83s 6842 queries
0 failed, 0 error(s) of 9 tests

Y contra el código anterior al PR, revirtiendo solo models/account_move.py y dejando los tests nuevos:

### test_made_sequence_gap_within_receiptbook (codigo VIEJO)     -> 0 failed
### test_made_sequence_gap_scoped_to_receiptbook (codigo VIEJO)  -> 0 failed
### test_made_sequence_gap_writes_only_on_change (codigo VIEJO)  -> 1 failed
AssertionError: [({126}, False), ({127}, False), ({128}, False), ({129}, False), ({130}, False)]
  is not false : recomputing a consistent set of moves should not write

Los cinco writes separados —uno por asiento, todos escribiendo el valor que ya estaba guardado— son exactamente el comportamiento que el PR elimina. Los otros dos tests pasan en ambas versiones, que es lo esperado: son guardas de regresión, no del cambio.

@rov-adhoc

Copy link
Copy Markdown
Contributor

@roboadhoc r+ nobump

@roboadhoc roboadhoc closed this in d1315f0 Aug 25, 2026
@roboadhoc
roboadhoc deleted the 19.0-h-126129-ica-1 branch August 25, 2026 13:49
@roboadhoc roboadhoc added the 18.1 label Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants