[wasud]

Decisions to review

• License (D3, changed after review): ni_patient depends on the new Nirun module ni_partner_mobile (LGPL-3, adapted from Odoo 18.0 base and phone_validation), not on OCA partner_mobile (AGPL-3), for the same reason [19.0][MIG] ni_period, ni_identifier, ni_coding, partner_age, partner_gender, partner_religion, uom_alias: Migration to 19.0 #149 replaced partner_title with ni_partner_title. It keeps the patient mobile number as its own field (FHIR ContactPoint.use = mobile) instead of Odoo 19's merged phone.
• Data (D3): upgrade_path/base/19.0.1.3/post-nirun_partner_mobile.py puts back the original phone that OpenUpgrade 19 merged with the mobile ("phone // mobile"), using OpenUpgrade's legacy copy. It runs only when ni_patient is installed, and needs upgrade_path/ as a second --upgrade-path (see [19.0][MIG] ni_period, ni_identifier, ni_coding, partner_age, partner_gender, partner_religion, uom_alias: Migration to 19.0 #149).
• Security (D5): groups now hang off a new res.groups.privilege "Patients" (res_groups_privilege_patient) in the existing Medical > Patients category; users → user_ids. OpenUpgrade leaves privileges to modules; memberships survive (checked).

What changed

• ni_patient replayed from the 18.0 PR branch, then [MIG] ni_patient: Migration to 19.0.
• Migrator: models.Constraint, bypass_search_access, env.cr/env.context, group_ids.
• D3:
  • title → title_id (onchange, demo, test);
  • ni_partner_mobile dependency (new LGPL module, commit before the [MIG]);
  • phone restore script.
• D5: privilege and group sequence.
• Other 19 changes:
  • self.env._(...) with arguments.
  • Tests: name_search(domain=), fields.Date/fields.Datetime; they don't rely on demo data.
  • Inheritable image default.
  • Redundant labels removed (19 pylint).
  • Single-line CHECK constraints.
• README (titles/mobile, privilege, dependencies) and th.po.

Why

Closes #137 (P1-2, epic #131).

Migration notes

• Effort: M.
• What the migrator did: most of the model API changes.
• Manual work:
  • D3/D5;
  • translations;
  • a larger pylint backlog (19 flags more redundant labels).
• Surprise: name_search(args=) is gone in 19.

How tested

• Unit/integration tests: 32 tests pass on 19 without demo data, and the log has no WARNING lines.
• Real OpenUpgrade 18→19 (OpenUpgrade 19 + upgrade_path/) on an 18 DB with ni_patient and no demo:
  • patients keep their titles (Thai custom and base), phone and mobile, each separate;
  • a nurse stays in ni_patient.group_user;
  • migration_snapshot.py patient/encounter hashes are identical before and after.
• Manual check: browser smoke on 19 (see Notes).
• pre-commit run -a clean.

Documentation

• Module README.md updated

Notes for reviewer

• Fix (2026-10-14): odoo-module-migrate 17 rewrote Thai text in XML (comments, string, placeholder) as &#NNNN; entities, and prettier then split one of them with spaces. The Thai text is back as UTF-8.
• Fix (2026-10-14): from 17.0, invisible="1" on a list field only hides the cells, so the column header still shows. Hidden list fields now use column_invisible="1". Found in the browser check of ni_observation ([MIG 19.0][P1-4] ni_observation #139).
• Stacked on [19.0][MIG] ni_period, ni_identifier, ni_coding, partner_age, partner_gender, partner_religion, uom_alias: Migration to 19.0 #149 (P1-1 19.0) and [18.0][MIG] ni_patient: Migration to 18.0 #154 (ni_patient 18.0).
• Rebased 2026-10-15 on the new [19.0][MIG] ni_period, ni_identifier, ni_coding, partner_age, partner_gender, partner_religion, uom_alias: Migration to 19.0 #149 (ni_partner_title): demo and tests use ni_partner_title.res_partner_title_*.
• Added ni_partner_mobile (2026-10-15). Without OCA partner-contact on the addons path: 144 tests pass on the top of the stack ([19.0][MIG] ni_questionnaire: Migration to 19.0 #161). OpenUpgrade 18→19 of the [MIG 19.0][P1-6] Data migration pilot: OpenUpgrade 16→17→18→19 on Phase 1 modules #141 pilot DB: 0 errors, and its snapshot is identical to the pilot's 19 snapshot made with the OCA modules (7 mobiles, 7 titles, no merged phones).
• Browser smoke on 19 (Odoo 19 with demo data):
  • patient list, kanban and form;
  • Archive from the list and the form opens the archive wizard;
  • encounter kanban (patient image) and form (chatter);
  • portal /my/home and /my/patients.
• Bug found and fixed by the smoke: 19 dropped ListController.getSelectedResIds(), so the list now reads the selection with model.root.getResIds(true).
• The Thai "Mobile" label comes from ni_partner_mobile's own th.po (มือถือ), so the earlier gap with OCA partner_mobile (no Thai translation) is gone.

[piruin]

Review

Blocking (mechanical): the branch conflicts with 19.0.

• upgrade_path/README.md is added on both sides.
• The branch still carries #149's commits from before that PR was merged, so GitHub shows about 226 changed files instead of about 40.
• A rebase onto origin/19.0 fixes both.

Non-blocking: a bug carried over from 16.0 in ni_encounter.check_origin_date.

• raise self.env._("...") raises a plain string, which fails with a TypeError.
• rec.origin_date > rec.period_start compares a Date with a Datetime, which also raises a TypeError.
• Per the epic, fix it on 16.0 under P0-4 (#135) and forward-port it with oca-port, rather than inside the [MIG] commit.

Checked, OK

• post-nirun_partner_mobile.py against OpenUpgrade 19's _migrate_partner_mobile:
  • OpenUpgrade copies phone to the legacy column, then sets phone to phone // mobile, or to just the mobile number when the phone is empty.
  • The WHERE COALESCE(mobile, '') != '' AND phone IS DISTINCT FROM legacy condition undoes exactly those rows. An originally empty phone ends up empty again.
  • Rows where phone equalled mobile are skipped by OpenUpgrade, and the script leaves them alone too.
  • The mobile column is never dropped, so ni_partner_mobile picks it up.
• The privilege setup (res.groups.privilege plus group sequence), user_ids, bypass_search_access, models.Constraint and self.env._(...) with arguments are all used correctly.
• The list view reads the selection with model.root.getResIds(true).
• CI is green.

[piruin]

Follow-up review

Two more items, found while reviewing #158:

• Fix: copyright headers were lost. The 18→19 step removed the <!-- Copyright … NSTDA --> header from views/ni_encounter_location_views.xml, views/ni_encounter_participant_views.xml, views/ni_location_views.xml and views/ni_patient_views.xml. The cause is probably the migrator rewriting the search-view <group> tags with lxml. Please restore the headers.
• Minor: there is no [IMP] ni_patient: pre-commit auto fixes commit before [MIG] ni_patient: Migration to 19.0, which the epic's commit order asks for.

[wasud]

Thanks. All fixed:

• Conflict: rebased onto origin/19.0. [19.0][MIG] ni_period, ni_identifier, ni_coding, partner_age, partner_gender, partner_religion, uom_alias: Migration to 19.0 #149's commits are gone, and so are the already-merged survey_subject/survey_grading commits in the questionnaire stack. upgrade_path/README.md merged cleanly and lists both scripts. The PR now touches only ni_patient, ni_partner_mobile and upgrade_path.
• Copyright headers: restored in the 4 views. You were right about the cause: odoo-module-migrate's lxml rewrite of the search-view <group> drops the leading comment. 17.0 and 18.0 lost no headers.
• Commit order: added [IMP] ni_patient: pre-commit auto fixes (odoo-module-migrate + pre-commit output on the 18.0 code) before [MIG] ni_patient: Migration to 19.0. The [MIG] diff now holds only the manual changes.
• Replayed from [17.0][MIG] ni_patient: Migration to 17.0 #153/[18.0][MIG] ni_patient: Migration to 18.0 #154: the label and Thai fixes, and the move of the default-views test to ni_coding. The PR body no longer lists "Location Moves" as a 19.0 change.
• check_origin_date: fixed on 16.0 as you asked, with forward-ports for 17/18/19 prepared. That PR is not opened yet.

A fresh 19.0 install with ni_coding and ni_patient tests: 32/32 pass. #158 and #161 were rebased on top and only gain the 4 headers.