[16.0][OU-IMP] base: Review existent states and compare with installed states in order to update them properly

[etobella]

I think this code could go to all version just us a safety review 😉

@pedrobaeza

[pedrobaeza]

Add docstring to explain the purpose of this.

[pedrobaeza]

Is imd.name the best option?

[legalsylvain]

I think this code could go to all version just us a safety review 😉

If it should go in all versions, maybe better to create an openupgradelib function, and then call it in a single line, in each version. Don't you think ?

[marcos-mendez]

Automated Review -- Tests Failed

1. Root Cause

The test failure occurs because the pre-migration.py script attempts to access res.country.state.csv data using get_resource_path, but the file is not found or not correctly loaded in the test environment, leading to a database connection error during migration.

---

2. Suggested Fix

In _review_states_xmlids(cr), replace:

data_file = open(get_resource_path("base", "data", "res.country.state.csv"), "r")

with:

data_file = open(
    get_resource_path("base", "data", "res.country.state.csv"), "r", encoding="utf-8"
)

Ensure that the file path is correct and that res.country.state.csv exists in the base/data/ directory of the Odoo base module.

Additionally, wrap the file reading logic in a try...except block to gracefully handle missing files:

try:
    data_file = open(get_resource_path("base", "data", "res.country.state.csv"), "r", encoding="utf-8")
except FileNotFoundError:
    logging.warning("res.country.state.csv not found, skipping state XMLID review.")
    return

---

3. Additional Code Issues

• Missing encoding parameter: The file is opened without specifying encoding, which may cause issues in some environments.
• No fallback for missing XMLIDs: If cr.fetchone() returns None, the code silently skips the XMLID creation. Consider logging a warning or raising an exception if critical data is missing.
• No cleanup of data_file: The file handle should be closed after use, ideally using a with statement.

---

4. Test Improvements

Add a TransactionCase test in tests/test_pre_migration.py to validate:

• The _review_states_xmlids function does not crash when res.country.state.csv is present.
• The function gracefully handles missing or malformed data.
• XMLIDs are correctly added for existing res.country.state records.

Example test:

def test_review_states_xmlids(self):
    # Create a test country and state
    country = self.env['res.country'].create({'name': 'Test Country', 'code': 'TC'})
    state = self.env['res.country.state'].create({
        'name': 'Test State',
        'code': 'TS',
        'country_id': country.id,
    })
    # Simulate XMLID for country
    self.env['ir.model.data'].create({
        'name': 'country_test',
        'model': 'res.country',
        'res_id': country.id,
        'module': 'base',
    })
    # Run the function
    _review_states_xmlids(self.cr)
    # Check if XMLID was created
    xmlid = self.env['ir.model.data'].search([
        ('model', '=', 'res.country.state'),
        ('name', '=', 'state_ts'),
        ('module', '=', 'base')
    ])
    self.assertTrue(xmlid)

Use @tagged('post_install', 'standard') to ensure it runs in proper test environments.

---

⚠️ PR Aging Alert: CRITICAL

This PR by @etobella has been waiting for 612 days — that is over 20 months without being merged or closed.
💤 No activity for 612 days. Has this PR been forgotten?

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