[ADD] account_reconcile_bg: background processing for large bank reconciliations#400
[ADD] account_reconcile_bg: background processing for large bank reconciliations#400feg-adhoc wants to merge 1 commit intoingadhoc:18.0from
Conversation
There was a problem hiding this comment.
Pull request overview
Este PR agrega el módulo account_reconcile_bg para enviar conciliaciones bancarias “grandes” a procesamiento en background mediante base_bg, evitando timeouts y bloqueos del UI durante la validación en el widget de conciliación.
Changes:
- Override de
bank.rec.widget._js_action_validate()para decidir sync vs background según un umbral configurable. - Encolado de un job
base_bgy agregado de flagreconciliation_in_backgroundenaccount.bank.statement.line. - Inclusión de tests, documentación (README + guía de testing) y parámetro de sistema por defecto.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| account_reconcile_bg/models/bank_rec_widget.py | Override del validate del widget y encolado del job en base_bg |
| account_reconcile_bg/models/account_bank_statement_line.py | Método ejecutado por el job y flag de “en proceso” en la línea de extracto |
| account_reconcile_bg/data/ir_config_parameter_data.xml | Default del sysparam del umbral |
| account_reconcile_bg/tests/test_account_reconcile_bg.py | Suite de tests para el comportamiento sync/async |
| account_reconcile_bg/TESTING.md | Guía manual de pruebas |
| account_reconcile_bg/README.rst | Documentación funcional/técnica del módulo |
| account_reconcile_bg/security/ir.model.access.csv | Archivo de ACL incluido (actualmente vacío) |
| account_reconcile_bg/manifest.py | Manifest del nuevo módulo y dependencias |
| account_reconcile_bg/init.py, models/init.py, tests/init.py | Inicialización de paquete y carga de modelos/tests |
| # Configurar el return_todo_command para mostrar la notificación | ||
| self.return_todo_command = { | ||
| "display_notification": { | ||
| "type": "success", | ||
| "title": _("Reconciliation sent to background"), | ||
| "message": _( | ||
| "This reconciliation is being processed in background. You will be notified when it's done." | ||
| ), | ||
| "sticky": False, | ||
| }, | ||
| "done": True, | ||
| } | ||
|
|
There was a problem hiding this comment.
return_todo_command se asigna, pero el método termina devolviendo action (el return de base.bg.bg_enqueue_records). Con el código actual, ese return_todo_command no tiene efecto y puede confundir (parece código muerto). Sugerencia: o bien devolver una acción construida a partir de return_todo_command, o eliminar esta asignación y confiar en la notificación estándar de base_bg.
| # Configurar el return_todo_command para mostrar la notificación | |
| self.return_todo_command = { | |
| "display_notification": { | |
| "type": "success", | |
| "title": _("Reconciliation sent to background"), | |
| "message": _( | |
| "This reconciliation is being processed in background. You will be notified when it's done." | |
| ), | |
| "sticky": False, | |
| }, | |
| "done": True, | |
| } |
| @@ -0,0 +1 @@ | |||
| id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink | |||
There was a problem hiding this comment.
El módulo incluye security/ir.model.access.csv pero el archivo solo tiene el header y no define ningún ACL. Si no se crean modelos nuevos, esto es innecesario y puede confundir (además de ser una carga extra en el manifest). Sugerencia: eliminar el archivo y su referencia en __manifest__.py, o agregar las reglas de acceso reales si se pretendía exponer algo nuevo.
| id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink |
| job = self.env["bg.job"].search( | ||
| [ | ||
| ("model", "=", "bank.rec.widget"), | ||
| ("method", "=", "_do_validate"), | ||
| ("state", "=", "enqueued"), | ||
| ], | ||
| limit=1, | ||
| ) |
There was a problem hiding this comment.
Este test vuelve a buscar el job por model='bank.rec.widget'/method='_do_validate', pero el job real se encola sobre account.bank.statement.line con _bg_validate_reconciliation. Con el código actual, este search puede no encontrar nada o estar validando el objetivo equivocado. Ajustar el search y, idealmente, también verificar que el método en background concilia respetando la selección enviada.
| jobs = self.env["bg.job"].search( | ||
| [ | ||
| ("model", "=", "bank.rec.widget"), | ||
| ("method", "=", "_do_validate"), | ||
| ] | ||
| ) | ||
| self.assertGreaterEqual(len(jobs), 1, "Debe crearse job porque 60 > threshold default (50)") |
There was a problem hiding this comment.
Mismo problema que en otros tests: se busca bg.job por model='bank.rec.widget' y method='_do_validate', pero el código encola account.bank.statement.line._bg_validate_reconciliation. Este assert debería ajustarse al modelo/método real del job para que el test valide el comportamiento correcto.
| # Verificar job creado | ||
| jobs = self.env["bg.job"].search( | ||
| [ | ||
| ("model", "=", "bank.rec.widget"), | ||
| ("method", "=", "_do_validate"), | ||
| ] | ||
| ) | ||
| self.assertEqual(len(jobs), 1, "Con 10 líneas y threshold 10, debe ir a background (>=)") |
There was a problem hiding this comment.
Mismo problema que en otros tests: el job se está buscando por model='bank.rec.widget'/method='_do_validate', pero el enqueue real se hace sobre account.bank.statement.line con _bg_validate_reconciliation. Corregir este search para cubrir el caso de borde contra el threshold.
| # Mock del método padre para verificar que se llama | ||
| with patch.object( | ||
| type(wizard).with_context(wizard._context), | ||
| "_js_action_validate", | ||
| autospec=True, | ||
| ) as mock_super: | ||
| mock_super.return_value = {"type": "ir.actions.act_window_close"} | ||
|
|
||
| # Ejecutar validación | ||
| result = wizard._js_action_validate() | ||
|
|
||
| # Verificar que se llamó al método padre (sync) | ||
| self.assertTrue(mock_super.called) | ||
| # Verificar que NO se creó un job en background | ||
| jobs = self.env["bg.job"].search( | ||
| [ | ||
| ("model", "=", "bank.rec.widget"), | ||
| ("method", "=", "_do_validate"), | ||
| ] | ||
| ) | ||
| self.assertEqual(len(jobs), 0, "No debería crearse job en background") |
There was a problem hiding this comment.
En test_reconcile_sync_when_below_threshold se está parcheando _js_action_validate sobre el propio modelo/recordset; al llamar wizard._js_action_validate() termina ejecutándose el mock, por lo que el test no ejerce la lógica de este módulo (ni verifica que se llame a super()). Para validar el comportamiento sync, conviene ejecutar el método real y comprobar que no se crea bg.job y/o que la línea queda conciliada; si se quiere espiar super(), hay que parchear el método original del módulo heredado, no el override.
| # Mock del método padre para verificar que se llama | |
| with patch.object( | |
| type(wizard).with_context(wizard._context), | |
| "_js_action_validate", | |
| autospec=True, | |
| ) as mock_super: | |
| mock_super.return_value = {"type": "ir.actions.act_window_close"} | |
| # Ejecutar validación | |
| result = wizard._js_action_validate() | |
| # Verificar que se llamó al método padre (sync) | |
| self.assertTrue(mock_super.called) | |
| # Verificar que NO se creó un job en background | |
| jobs = self.env["bg.job"].search( | |
| [ | |
| ("model", "=", "bank.rec.widget"), | |
| ("method", "=", "_do_validate"), | |
| ] | |
| ) | |
| self.assertEqual(len(jobs), 0, "No debería crearse job en background") | |
| # Ejecutar la validación real | |
| result = wizard._js_action_validate() | |
| # Verificar que NO se devolvió la notificación de background | |
| self.assertNotEqual(result.get("tag"), "display_notification") | |
| # Verificar que NO se creó un job en background | |
| jobs = self.env["bg.job"].search( | |
| [ | |
| ("model", "=", "bank.rec.widget"), | |
| ("method", "=", "_do_validate"), | |
| ] | |
| ) | |
| self.assertEqual(len(jobs), 0, "No debería crearse job en background") |
|
|
||
| <!-- Parámetro de configuración para el umbral de líneas --> | ||
| <record id="config_lines_threshold" model="ir.config_parameter"> | ||
| <field name="key">account_reconcile_bg.lines_threshold</field> | ||
| <field name="value">50</field> | ||
| </record> | ||
|
|
There was a problem hiding this comment.
Este ir.config_parameter se carga como dato normal (sin noupdate="1"). En cada actualización del módulo, Odoo volverá a escribir el valor 50 y puede pisar la configuración que haya hecho un administrador. Si la intención es solo proveer un default inicial, conviene marcar el record como noupdate="1" o setear el valor solo si el parámetro no existe (post-init hook / lógica en código).
| <!-- Parámetro de configuración para el umbral de líneas --> | |
| <record id="config_lines_threshold" model="ir.config_parameter"> | |
| <field name="key">account_reconcile_bg.lines_threshold</field> | |
| <field name="value">50</field> | |
| </record> | |
| <data noupdate="1"> | |
| <!-- Parámetro de configuración para el umbral de líneas --> | |
| <record id="config_lines_threshold" model="ir.config_parameter"> | |
| <field name="key">account_reconcile_bg.lines_threshold</field> | |
| <field name="value">50</field> | |
| </record> | |
| </data> |
| # Obtener el umbral de líneas desde parámetros del sistema (default: 50) | ||
| threshold = int(self.env["ir.config_parameter"].sudo().get_param("account_reconcile_bg.lines_threshold", "50")) | ||
|
|
||
| # Contar las líneas seleccionadas para conciliar | ||
| lines_count = len(self.selected_aml_ids) |
There was a problem hiding this comment.
El parámetro account_reconcile_bg.lines_threshold se convierte con int(...) sin validar. Si el sysparam queda con un valor no numérico (o vacío), esto va a lanzar ValueError y romper la validación del widget. Sugerencia: capturar la excepción y usar fallback seguro (p.ej. 50) o levantar UserError indicando que el parámetro debe ser un entero.
| jobs = self.env["bg.job"].search( | ||
| [ | ||
| ("model", "=", "bank.rec.widget"), | ||
| ("method", "=", "_do_validate"), | ||
| ] | ||
| ) | ||
| self.assertEqual(len(jobs), 0, "No debería crearse job en background") | ||
|
|
||
| def test_reconcile_background_when_above_threshold(self): | ||
| """ | ||
| Valida que con muchas líneas (>= threshold) la conciliación | ||
| se envía a background usando base_bg. | ||
| """ | ||
| # Configurar umbral bajo para forzar background | ||
| self.env["ir.config_parameter"].sudo().set_param("account_reconcile_bg.lines_threshold", "5") | ||
|
|
||
| # Crear widget con 10 pagos (>= 5) | ||
| wizard = self._create_bank_rec_widget_with_payments(payment_count=10) | ||
|
|
||
| # Ejecutar validación | ||
| result = wizard._js_action_validate() | ||
|
|
||
| # Verificar que se retorna una notificación de background | ||
| self.assertEqual(result.get("type"), "ir.actions.client") | ||
| self.assertEqual(result.get("tag"), "display_notification") | ||
| self.assertIn("success", result.get("params", {}).get("type", "")) | ||
|
|
||
| # Verificar que se creó un job en background | ||
| jobs = self.env["bg.job"].search( | ||
| [ | ||
| ("model", "=", "bank.rec.widget"), | ||
| ("method", "=", "_do_validate"), | ||
| ] | ||
| ) | ||
| self.assertEqual(len(jobs), 1, "Debe crearse exactamente 1 job") |
There was a problem hiding this comment.
Los tests están buscando jobs con model = 'bank.rec.widget' y method = '_do_validate', pero el código encola self.st_line_id con el método '_bg_validate_reconciliation' (modelo account.bank.statement.line). Así como está, estos asserts no validan el comportamiento real (y probablemente fallen o pasen por motivos incorrectos). Ajustar los criterios de búsqueda del job a lo que realmente se encola.
|
|
||
| * ``_js_action_validate()``: Detects large reconciliations and routes to background | ||
| * ``_validate_in_background()``: Enqueues the job using base_bg | ||
| * ``_do_validate()``: Executes the actual validation in background |
There was a problem hiding this comment.
La documentación técnica indica que el job ejecuta bank.rec.widget._do_validate(), pero el código en realidad encola account.bank.statement.line._bg_validate_reconciliation() y no usa _do_validate. Esto hace que el README quede inconsistente con la implementación (y también con la guía de tests). Sugerencia: actualizar el README para reflejar el método/modelo real del job, o cambiar el código para que el job invoque efectivamente _do_validate si ese era el diseño.
| * ``_do_validate()``: Executes the actual validation in background | |
| * ``account.bank.statement.line._bg_validate_reconciliation()``: Executes the actual validation in background |
74cb6c9 to
5e53a34
Compare
…nciliations Implements background job processing for bank reconciliations when dealing with large payment batches to prevent timeouts and improve user experience. Main features: - Automatic background processing for large reconciliations (configurable threshold) - User notifications when background reconciliation completes - Seamless integration with existing bank reconciliation workflow - No UI blocking: users can continue working while large batches process Technical implementation: - Override bank.rec.widget._js_action_validate() to detect large reconciliations - Use base_bg to enqueue reconciliation jobs - Add reconciliation_in_background flag to account.bank.statement.line - Configurable via system parameter: account_reconcile_bg.lines_threshold Includes comprehensive testing guide and unit tests.
5e53a34 to
5a9cb70
Compare

Implements background job processing for bank reconciliations when dealing with large payment batches to prevent timeouts and improve user experience.
Main features:
Technical implementation:
Includes comprehensive testing guide and unit tests.