[16.0] [FIX] maintenance_request_purchase: fix problems when currency_id field is not set#537
[16.0] [FIX] maintenance_request_purchase: fix problems when currency_id field is not set#537luisDIXMIT wants to merge 1 commit intoOCA:16.0from
Conversation
63b96d0 to
9ab1fc8
Compare
marcos-mendez
left a comment
There was a problem hiding this comment.
Automated Review -- Tests Failed
1. Root Cause
The test failure is due to a database connection error during the Odoo startup, not directly caused by the code changes in the PR. This typically occurs in CI environments like Runboat when the test database is not properly initialized or accessible. However, the code change introduces a potential circular dependency or incorrect field access in _compute_currency_id, which could cause runtime issues if not handled carefully.
2. Suggested Fix
In maintenance_request_purchase/models/maintenance_request.py, line 37 in _compute_currency_id, the method accesses record.company_id.currency_id.id without ensuring that record.company_id exists.
Fix:
@api.depends("company_id.currency_id")
def _compute_currency_id(self):
for record in self:
record.currency_id = (
record.company_id.currency_id.id
if record.company_id
else self.env.company.currency_id.id
)This ensures that company_id is checked before accessing its currency_id.
3. Additional Code Issues
- Potential runtime error: If
company_idisFalse(not set), accessingrecord.company_id.currency_idwill raise an error unless explicitly guarded. - Missing
@api.model: The use ofself.env.companyis correct, but the logic should be robust to avoid accessing fields on non-existent records.
4. Test Improvements
Add test cases in tests/test_maintenance_request.py or similar using SavepointCase to cover:
- Test with no company set: Ensure
currency_iddefaults toenv.company.currency_id. - Test with company set: Ensure
currency_idis correctly computed fromcompany_id.currency_id. - Test with purchase orders: Ensure
_compute_total_purchase_amountworks correctly with computedcurrency_id.
Example test snippet:
def test_compute_currency_id_without_company(self):
request = self.env['maintenance.request'].create({
'name': 'Test Request',
# No company_id set
})
self.assertEqual(request.currency_id, self.env.company.currency_id)
def test_compute_currency_id_with_company(self):
company = self.env['res.company'].create({'name': 'Test Co'})
request = self.env['maintenance.request'].create({
'name': 'Test Request',
'company_id': company.id,
})
self.assertEqual(request.currency_id, company.currency_id)Use SavepointCase for robust database isolation and @tag('post_install') for tests that require module installation.
⏰ This PR has been open for 86 days.
🔍 No human reviews yet after 86 days. PSC members: a quick review would help keep this contributor engaged with OCA.
💤 Last activity was 86 days ago.
Every ignored PR is a contributor who might not come back. Review time matters. (OCA Aging Report)
Reciprocal Review Request
Hi everyone! I found some test failures on this PR and left detailed feedback above. I am happy to discuss or help debug. In the meantime, if any of you get a chance, I would appreciate a look at my open PR(s):
My open PRs across OCA:
- server-tools#3554 [MIG] datetime_formatter: Migration to 18.0
- server-tools#3548 [18.0][MIG] base_kanban_stage: Migration to 18.0
- hr-attendance#262 [16.0][ADD] Hr_attendance_idsecure: iDSecure (ControliD) attendance integration
- stock-logistics-workflow#2276 [16.0][ADD] stock_move_line_devaluation
- stock-logistics-workflow#2275 [16.0][ADD] Stock move line analytic account
- stock-logistics-workflow#2268 [16.0][ADD] stock_move_line_picking_partner
- purchase-workflow#2694 [16.0][IMP]Purchase workflow added to review state & exception fix
Reviewing each other's work helps the whole community move forward. Thank you!
Environment via OCA Neural Reviewer: Minikube + K8s Job + oca-ci/py3.10-odoo16.0 | Odoo 16.0
Automated review by OCA Neural Reviewer + qwen3-coder:30b
We have detected problems related to company_id not being set because it is not required and it can be forced to not have a default value. @etobella