[16.0][FIX] account_analytic_distribution_manual: partial fix for bug #844#845
Conversation
This is a partial fix for bug OCA#844 [16.0/17.0/18.0] account_analytic_distribution_manual: JS crash in account_reconcile_oca Backport of the save() method from 17.0 to 16.0.
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root Cause
The test failures are related to JavaScript unit tests in the web_editor module, specifically around tab handling and indentation logic. These failures occur because the expected tab widths (used for rendering indented content) differ from actual computed values, likely due to changes in how tab widths are calculated or rendered in the editor.
2. Suggested Fix
The issue stems from the web_editor JavaScript tests expecting fixed tab widths (e.g., 32.8906px) but receiving different computed values (e.g., 32px). This mismatch is not directly caused by the account_analytic_distribution_manual patch, but rather indicates a floating-point precision or rendering inconsistency in the editor's tab width calculation logic.
Files to inspect:
web_editor/static/src/js/editor/odoo_editor.jsweb_editor/static/src/js/editor/toolbar.jsweb_editor/static/src/js/editor/indentation.js
Action: Ensure tab width calculations are consistent and deterministic, possibly by using a fixed tab size or adjusting test expectations to tolerate small floating-point differences.
3. Additional Code Issues
No issues found in the provided diff (account_analytic_distribution_manual). The patch correctly adds a conditional check before updating the manual_distribution_id, which prevents unnecessary updates when id is falsy. This is a valid defensive programming practice.
4. Test Improvements
To better cover the changed code in account_analytic_distribution_manual:
- Add a test case in
account_analytic_distribution_manual/tests/test_ui.py(usingSavepointCase) to verify thatsave()does not update the record whenstate_manual_distribution.idis falsy. - Tag the test with
@tag('ui')and@tag('manual_distribution')to align with OCA conventions. - Test both branches of the conditional logic: one where
idis present and one where it is not.
Example test snippet:
def test_save_with_empty_id(self):
# Simulate state with id = 0 or None
self.component.state_manual_distribution = {'id': None, 'label': 'Test'}
with patch.object(self.component.props.record, 'update') as mock_update:
self.component.save()
self.assertFalse(mock_update.called)This ensures that the conditional logic in save() is properly tested and prevents regressions.
Environment: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA PR Reviewer + qwen3-coder:30b
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Passed
All tests for account_analytic_distribution_manual passed successfully on Odoo 16.0.
Environment: Minikube + K8s Job + oca-ci/py3.10-odoo16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b
This is a partial fix for bug #844
Backport of the save() method from 17.0 to 16.0.
It solves the JS crash when manually configuring an analytic distribution inside the "manual operation" tab of the OCA bank reconciliation interface (module account_reconcile_oca)
It still crashes when loading a pre-configured manual analytic distribution inside the "manual operation" tab of the OCA bank reconciliation interface (but this second problem is also present in 17.0 and 18.0).