feat(npt): TETRA-1421 deprecate v0_4_0 child-table migration + fix npt_count short-circuit #61

Merged

tant merged 1 commit from feat/TETRA-1421-deprecate-v0_4_0-npt into develop

2026-04-29 19:45:29 +07:00

tant commented

2026-04-29 19:32:50 +07:00

Owner

Ticket

TETRA-1421 — [vhrms] Remove obsolete v0_4_0 child-table NPT migration + fix npt_count short-circuit

Context

Story 4.2 (v0_4_0) introduced an npt_dependents child table on Employee. v0_7_0 (migrate_npt_to_full_doctypes) replaced that model with the standalone NPT Dependent doctype. Two leftovers from v0_4_0 still caused silent payroll bugs:

_update_npt_count (override Employee on_update) had a short-circuit
```
if hasattr(doc, "npt_dependents") and doc.npt_dependents:
    return
```
Whenever an employee record kept legacy child rows lingering, npt_count was never refreshed from the standalone source → PIT deduction stale, payroll under/over-counts dependents.
The migration patch migrate_npt_dependent_to_child_table was still listed in patches.txt, running on every fresh install / re-migrate even though the destination model has been abandoned.

Changes

vhrms/overrides/employee.py::_update_npt_count — drop short-circuit; call get_active_npt_count(doc.name, today()) unconditionally; write back via db.set_value(update_modified=False) only when value differs (preserve infinite-loop guard); also update doc.npt_count so in-memory doc stays consistent.
vhrms/patches.txt — comment-out vhrms.vhrms.patches.v0_4_0.migrate_npt_dependent_to_child_table with TETRA-1421 note.
vhrms/patches/v0_4_0/migrate_npt_dependent_to_child_table.py → migrate_npt_dependent_to_child_table.py.deprecated (git mv preserves history).
vhrms/__init__.py — bump __version__ 0.9.5 → 0.9.6.
CHANGELOG.md — Unreleased / Fixed entry.
vhrms/tests/test_npt_count_standalone_priority.py — 5 unit tests (frappe-stub offline pattern).

Tests

cd /home/tan/work/vhrms && /home/tan/work/vhrms/.venv/bin/python -m unittest vhrms.vhrms.tests.test_npt_count_standalone_priority
.....
Ran 5 tests in 0.002s
OK

Cases:

test_standalone_used_when_child_table_empty — baseline: standalone count → set_value.
test_standalone_used_even_when_child_table_has_rows — regression guard for the bug: child rows present, standalone count still wins.
test_no_write_when_count_unchanged — infinite-loop guard preserved.
test_doc_attr_missing_does_not_crash — tolerates doc without npt_dependents attr.
test_exception_swallowed_logs_error — try/except contract preserved (save must not fail).

Full suite NOT run per feedback_vhrms_test_stub_fragility (stub collision when running multiple files together).

Out of scope (TODO)

Cleanup residual Employee.npt_dependents Custom Field + Employee NPT Item rows on staging DB (PR #3 in original ticket plan). Requires baseline query on staging DB; staging is offline (project_staging_offline_2026_04). Defer until staging back.

Risk

Low. Change isolated to _update_npt_count. Standalone get_active_npt_count was already the canonical payroll source; this aligns the override hook with the rest of the engine.
Migration patch deprecation is safe: previously-migrated child rows remain in DB (we do not drop the Custom Field here); they are simply no longer read by the hook, and not re-migrated on fresh installs.

## Ticket [TETRA-1421](https://tetravn.atlassian.net/browse/TETRA-1421) — [vhrms] Remove obsolete v0_4_0 child-table NPT migration + fix npt_count short-circuit ## Context Story 4.2 (v0_4_0) introduced an `npt_dependents` child table on Employee. v0_7_0 (`migrate_npt_to_full_doctypes`) replaced that model with the standalone `NPT Dependent` doctype. Two leftovers from v0_4_0 still caused silent payroll bugs: 1. `_update_npt_count` (override Employee `on_update`) had a short-circuit ```python if hasattr(doc, "npt_dependents") and doc.npt_dependents: return ``` Whenever an employee record kept legacy child rows lingering, `npt_count` was never refreshed from the standalone source → PIT deduction stale, payroll under/over-counts dependents. 2. The migration patch `migrate_npt_dependent_to_child_table` was still listed in `patches.txt`, running on every fresh install / re-migrate even though the destination model has been abandoned. ## Changes - `vhrms/overrides/employee.py::_update_npt_count` — drop short-circuit; call `get_active_npt_count(doc.name, today())` unconditionally; write back via `db.set_value(update_modified=False)` only when value differs (preserve infinite-loop guard); also update `doc.npt_count` so in-memory doc stays consistent. - `vhrms/patches.txt` — comment-out `vhrms.vhrms.patches.v0_4_0.migrate_npt_dependent_to_child_table` with TETRA-1421 note. - `vhrms/patches/v0_4_0/migrate_npt_dependent_to_child_table.py` → `migrate_npt_dependent_to_child_table.py.deprecated` (`git mv` preserves history). - `vhrms/__init__.py` — bump `__version__` 0.9.5 → 0.9.6. - `CHANGELOG.md` — Unreleased / Fixed entry. - `vhrms/tests/test_npt_count_standalone_priority.py` — 5 unit tests (frappe-stub offline pattern). ## Tests ``` cd /home/tan/work/vhrms && /home/tan/work/vhrms/.venv/bin/python -m unittest vhrms.vhrms.tests.test_npt_count_standalone_priority ..... Ran 5 tests in 0.002s OK ``` Cases: 1. `test_standalone_used_when_child_table_empty` — baseline: standalone count → set_value. 2. `test_standalone_used_even_when_child_table_has_rows` — **regression guard** for the bug: child rows present, standalone count still wins. 3. `test_no_write_when_count_unchanged` — infinite-loop guard preserved. 4. `test_doc_attr_missing_does_not_crash` — tolerates doc without `npt_dependents` attr. 5. `test_exception_swallowed_logs_error` — try/except contract preserved (save must not fail). Full suite NOT run per `feedback_vhrms_test_stub_fragility` (stub collision when running multiple files together). ## Out of scope (TODO) - Cleanup residual `Employee.npt_dependents` Custom Field + `Employee NPT Item` rows on staging DB (PR #3 in original ticket plan). Requires baseline query on staging DB; staging is offline (`project_staging_offline_2026_04`). Defer until staging back. ## Risk - Low. Change isolated to `_update_npt_count`. Standalone `get_active_npt_count` was already the canonical payroll source; this aligns the override hook with the rest of the engine. - Migration patch deprecation is safe: previously-migrated child rows remain in DB (we do not drop the Custom Field here); they are simply no longer read by the hook, and not re-migrated on fresh installs.

tant added 1 commit

2026-04-29 19:32:50 +07:00

feat(npt): TETRA-1421 deprecate v0_4_0 child-table migration + fix npt_count short-circuit 8b369462d0

Story 4.2 (v0_4_0) introduced an `npt_dependents` child table on Employee.
v0_7_0 (`migrate_npt_to_full_doctypes`) replaced that model with the
standalone `NPT Dependent` doctype. Two leftovers from v0_4_0 still caused
silent payroll bugs:

1. `_update_npt_count` (override Employee on_update) had a short-circuit
   `if hasattr(doc, "npt_dependents") and doc.npt_dependents: return`.
   Whenever an employee record kept legacy child rows lingering, `npt_count`
   was never refreshed from the standalone source → PIT deduction stale.
2. The migration patch `migrate_npt_dependent_to_child_table` was still
   listed in `patches.txt`, running on every fresh install / re-migrate
   even though the destination model has been abandoned.

Fix: drop the short-circuit, call `get_active_npt_count(doc.name, today())`
unconditionally, and write back via `db.set_value(update_modified=False)`
only when the value differs (preserve infinite-loop guard). Comment-out the
v0_4_0 migration patch and rename `migrate_npt_dependent_to_child_table.py`
→ `.py.deprecated` (preserve git history via `git mv`).

Tests `test_npt_count_standalone_priority.py` (5 cases): standalone count
when child-table empty, regression — standalone wins when child rows
present, no-op when count unchanged, tolerate doc missing attr, exception
swallowed + logged.

TODO (out of scope): cleanup `Employee.npt_dependents` Custom Field +
`Employee NPT Item` rows residual on staging DB — deferred until staging
is back online (current `project_staging_offline_2026_04`).

Refs: TETRA-1421