[wasud]

Decisions to review

• D4, data (UoM):
  • data/uom_uom_data.xml is rewritten for the 19 UoM model: no categories, relative_uom_id + relative_factor, same xmlids. Each relative_factor is 1/factor of the 16.0 data, so every conversion gives the same result (1 kg/m³ = 100 mg/dl).
  • OpenUpgrade 19 converts only units without an xmlid ("left to the module providing them"), and Nirun's units are noupdate. So migrations/19.0.1.0.0/post-migration.py converts them from OpenUpgrade's legacy columns and drops the old category xmlids. It does nothing without an OpenUpgrade run.
• D4, OpenUpgrade bug:
  • Upstream uom/19.0.1.0/post-migration.py sets relative_factor = factor for "bigger" units without an xmlid. 18.0 stored factor < 1 for those (dozen = 1/12), so their ratio is inverted: a user-created "Box of 24" becomes 1/24 of a unit.
  • Reproduced on the rehearsal. upgrade_path/uom/19.0.1.0/post-nirun_uom_bigger.py (separate [FIX] commit) recomputes them as 1/factor and runs after OpenUpgrade's script.
  • Reported and fixed upstream: [19.0][OU-FIX] uom: relative_factor of bigger units is 1 / factor OCA/OpenUpgrade#6054. Drop the workaround once it is merged.
• D4, pre-existing data issue: /sec and /hour are defined like time units (1 /min = 60 /sec), the inverse of a rate. Conversions are kept the same on purpose (no data change in a migration); noted in the README. Fixing it is a 16.0 data change (P0-4 [MIG 19.0][P0-4][FIX] Pre-existing bugs found during the 19.0 migration scan (16.0) #135?).
• D5, security: a res.groups.privilege "Observation" (res_groups_privilege_observation) holds Viewer/User/Manager. The groups get sequence and privilege_id, users → user_ids. Same pattern as P1-2 ([MIG 19.0][P1-2] ni_patient #137).
• D7, list widget: kept the 18 port. Core account 19 adds section menus, subsections and price totals, which observations do not use; the parts the widget relies on (getColumns, getSectionColumns, getCellClass, focusCell, this.columns) are unchanged.

What changed

• [MIG] ni_observation: Migration to 19.0 on top of the 18.0 PR, then the [FIX] upgrade_path commit.
• Migrator: _sql_constraints → models.Constraint, _cr/_context → env.cr/env.context.
• Manual work:
  • D4: data, post-migration, views/uom_uom_views.xml removed (uom_type is gone).
  • D5: groups.
  • self.env._(...) everywhere.
  • SQL in the migration with psycopg2.sql.
  • Inheritable lambda default.
  • f-string display name.
• Bug fixed: safe_eval(..., nocopy=True) raises TypeError on 19 (the argument is gone; safe_eval now updates the context dict by itself). The bare except hid it, so computed observations (BP, BMI) silently stopped; the existing tests caught it.
• New test tests/test_uom.py: reference units, relative units, conversions identical to 16.0, observation types keep their unit.
• th.po regenerated, 18.0 translations merged.

Why

Closes #139 (P1-4, epic #131).

Migration notes

• Effort: L (D4).
• What the migrator did: constraints, _cr/_context.
• Manual work:
  • UoM data, migration script, the OpenUpgrade workaround;
  • privilege;
  • self.env._;
  • safe_eval.
• Surprises:
  • OpenUpgrade leaves module units to the module.
  • OpenUpgrade inverts "bigger" units.
  • safe_eval lost nocopy, silently.
  • 19 _compute_quantity no longer checks categories.

How tested

• Unit/integration tests: 29 tests pass on 19 (25 existing + 4 UoM); fresh install and -u log have no WARNING lines.

• Real OpenUpgrade 18→19 (OpenUpgrade 19 @ 4783645, second --upgrade-path), on an 18 DB without demo, holding:

  • a patient and a sheet with BP and glucose;
  • two user-created units in a Nirun category (one smaller, one bigger).

  Results:

  • all 15 Nirun units converted, conversions equal to 18;
  • readings keep value and unit;
  • category xmlids gone;
  • privilege groups present;
  • with the workaround, the user-created bigger unit converts like on 18 (without it: 0.1 instead of 10).

• Manual check (browser, Odoo 19):

  • sheet with sections and value columns, BP computes "150 / 95";
  • sheet kanban card;
  • Units page lists the Nirun units with ratio and reference;
  • encounter Observation tab → wizard with the default category → code value → Save;
  • no JS console errors.

• pre-commit clean on ni_observation and upgrade_path.

Documentation

• Module README.md (19.0, units of measure, upgrade, privilege) and upgrade_path/README.md

Notes for reviewer

• Added after review started (P1-6 pilot, [MIG 19.0][P1-6] Data migration pilot: OpenUpgrade 16→17→18→19 on Phase 1 modules #141): [FIX] ni_observation: pick the latest observation deterministically (same as [17.0][MIG] ni_observation: Migration to 17.0 #156): tie-breaker id DESC in both latest-observation views, plus a test. 31 tests pass.

• Stacked on [18.0][MIG] ni_observation: Migration to 18.0 #157 (18.0) and [19.0][MIG] ni_patient: Migration to 19.0 #155 (P1-2 19.0). Merge those first.

• P1-6 ([MIG 19.0][P1-6] Data migration pilot: OpenUpgrade 16→17→18→19 on Phase 1 modules #141): pass upgrade_path/ as the second --upgrade-path. The snapshot should compare unit name and factor per observation.

[piruin]

Review

Blocking (mechanical): the branch conflicts with 19.0. It is stacked on #155, so rebase once that lands.

Fix: copyright headers were lost. The 18→19 step removed the <!-- Copyright … NSTDA --> header from 8 XML files:

• report/ni_encounter_observation_view.xml, report/ni_patient_observation_view.xml
• views/ni_observation_views.xml, views/ni_observation_sheet_views.xml, views/ni_observation_type_views.xml, views/ni_observation_interpretation_views.xml, views/ni_observation_reference_range_views.xml, views/ni_observation_value_code_views.xml

The cause is probably the migrator rewriting the search-view <group> tags with lxml, which drops the comment at the top of the file. Please restore the headers.

Recommended: log errors in the BP/BMI compute. ni_observation_sheet.py still wraps the safe_eval call in except Exception: continue. That is why the nocopy breakage silently stopped BP/BMI from computing. Log the exception (e.g. _logger.warning(..., exc_info=True)) so the next break is visible.

Minor: there is no [IMP] ni_observation: pre-commit auto fixes commit before [MIG] ni_observation: Migration to 19.0. The series goes straight from the 18.0 [MIG] to the 19.0 one.

Checked, OK: UoM. I checked this against the OpenUpgrade 19 source (uom/19.0.1.0/post-migration.py):

• OpenUpgrade converts only units without an xmlid (__export__ counts as none), and for bigger units it sets relative_factor = factor, which inverts the ratio. The bug is confirmed.
• post-nirun_uom_bigger.py selects the same set of units and skips the old reference units (relative_uom_id IS NOT NULL). The upstream fix, OCA/OpenUpgrade#6054, is still open, so the workaround is still needed.
• In migrations/19.0.1.0.0/post-migration.py and data/uom_uom_data.xml, relative_factor = 1/factor gives the same conversions as 16–18 (1 mg/dl = 0.01 kg/m³, 1 ×10⁶/µL = 1000 ×10³/µL, /hour = 60 /min as in the old data).

Checked, OK: other changes. The privilege, user_ids, models.Constraint, self.env._, the lambda default and the safe_eval nocopy removal are all right. CI is green.

[wasud]

Thanks for the review. Pushed (head adeb71d); the questionnaire stack (#161) is rebased on top.

• Conflict with 19.0: already gone after the last rebase onto 19.0; the branch contains the current 19.0 head and merges cleanly.
• Copyright headers: restored in the 8 XML files, inside [MIG] ni_observation: Migration to 19.0. You were right about the cause: odoo-module-migrate 18→19 drops the top comment when it rewrites the search-view <group>. data/ni_observation_reference_range_data.xml had no header on 18.0 either, so I left it as is.
• Missing pre-commit commit: added [IMP] ni_observation: pre-commit auto fixes (61bc296) before the 19.0 [MIG], with the migrator's automatic changes (version, models.Constraint, self.env.cr, self.env.context, <group>). The [MIG] commit now holds only the manual changes. The final tree is unchanged apart from the headers.
• Logging in the BP/BMI compute: new commit [IMP] ni_observation: log why a computed observation type fails (adeb71d). It logs _logger.warning(..., exc_info=True) with the type code and sheet id, and the existing exception test asserts the warning. The new warning turned up a broken test: test_code_not_assigning_result_does_not_create_obs used the type code test-noassign-inp, which is cut to 16 characters, so its compute code failed with a NameError instead of testing "no result assigned". I shortened the code, and the test now asserts that nothing is logged. The seeded BP/BMI code already guards against missing inputs, so normal entry doesn't log.

Tests on a fresh DB: ni_observation 31/31, and ni_questionnaire + ni_observation 64/64. pre-commit is clean on the changed files.