docs: design for Salary auto-scope picker (filter + auto-untick)
Konrad-approved: when Add-Adjustment type=Salary, auto-set the pay-type filter to Managers-only, hide daily rows, and untick any selected daily worker so a Salary can never silently target a daily worker. Pure JS, hooks the toggleProjectField() chokepoint. Rides the same HARD STOP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Konrad du Plessis
parent
0d77d7228d
commit
8f443faebc
163
docs/plans/2026-05-16-salary-autoscope-picker-design.md
Normal file
163
docs/plans/2026-05-16-salary-autoscope-picker-design.md
Normal file
@ -0,0 +1,163 @@
|
||||
# Salary Auto-Scope Picker — Design
|
||||
|
||||
**Date:** 16 May 2026
|
||||
**Status:** Approved by Konrad on 16 May 2026; ready for implementation plan.
|
||||
**Branch:** `ai-dev`, on top of the **paused, un-pushed** Manager/Salaried
|
||||
+ pay-type-filter commits (HEAD `0d77d72`). Same logical feature line.
|
||||
**NOT to be pushed/deployed until Konrad confirms it works locally** —
|
||||
rides the same HARD STOP as the rest of the paused work; ships in one
|
||||
bundled push, Konrad's call.
|
||||
|
||||
## Goal (one sentence)
|
||||
|
||||
When the Add-Adjustment modal's adjustment **type** is set to
|
||||
**Salary**, automatically scope the worker picker to managers only —
|
||||
set the pay-type filter to "Managers only", hide daily-worker rows,
|
||||
**and untick any daily worker that was already selected** — so a
|
||||
`Salary` adjustment can never silently be created for a daily worker.
|
||||
|
||||
## Why
|
||||
|
||||
Salary is the manager / fixed-pay adjustment type. CLAUDE.md is
|
||||
explicit: `Salary` is **only** for `Worker(pay_type='fixed')`; a daily
|
||||
worker must never receive a `Salary` adjustment, and "accuracy is
|
||||
critical" for this payroll system. The pay-type filter shipped in the
|
||||
previous task only *hides* rows — a daily worker ticked *before*
|
||||
switching the type to Salary would stay checked-but-hidden and POST a
|
||||
bad `Salary` adjustment. Auto-unticking non-managers on entry to Salary
|
||||
closes that hole at the UI.
|
||||
|
||||
## Decision (from the brainstorm)
|
||||
|
||||
Konrad chose **"Filter + auto-untick"** over plain display-only filter
|
||||
(B) and over a hard lock (C). Rationale: it removes the *silent*
|
||||
mis-payment footgun without locking the UI; a deliberate manual
|
||||
override (switch filter back to "All", re-tick a daily worker) is still
|
||||
possible — a conscious act, consistent with the reversible,
|
||||
display-only philosophy of the rest of the pay-type filter.
|
||||
|
||||
## Behaviour (single rule, both directions)
|
||||
|
||||
- **Type → `Salary`:**
|
||||
1. `addAdjPayTypeFilter.value = 'fixed'`.
|
||||
2. For every `.add-adj-worker` checkbox: read its `.form-check`
|
||||
wrapper's `data-pay-type`. If it is **not** `'fixed'` → hide the
|
||||
row (`style.display = 'none'`) **and** uncheck it
|
||||
(`cb.checked = false`). If it **is** `'fixed'` → show the row
|
||||
(`style.display = ''`); leave its checked state untouched (a
|
||||
pre-ticked manager stays ticked).
|
||||
3. Call the existing `updateWorkerCount()` so the "X worker(s)
|
||||
selected" counter reflects any auto-untick.
|
||||
- **Type → anything else** (Bonus / Deduction / New Loan / …):
|
||||
1. `addAdjPayTypeFilter.value = ''`.
|
||||
2. Show every row (`style.display = ''`).
|
||||
3. Do **not** alter any checkbox state — daily workers that Salary
|
||||
auto-unticked are **not** auto-re-ticked. The user re-selects
|
||||
deliberately. (No hidden "remember what I unticked" state — keeps
|
||||
the behaviour predictable.)
|
||||
|
||||
## Where it hooks (DRY — one chokepoint)
|
||||
|
||||
`toggleProjectField()` in `core/templates/core/payroll_dashboard.html`
|
||||
is the single function that already runs on:
|
||||
- every manual `#addAdjType` change (bound listener),
|
||||
- init (called once at setup),
|
||||
- the **Pay Salary** button (`paySalaryBtn` sets
|
||||
`addAdjType.value='Salary'` then calls `toggleProjectField()`),
|
||||
- the header-open reset (sets `addAdjType.value='Bonus'` then calls
|
||||
`toggleProjectField()`).
|
||||
|
||||
Add the filter+untick sync logic at the **end** of
|
||||
`toggleProjectField()` (or as a small helper invoked there). This makes
|
||||
all four paths correct with zero new event wiring.
|
||||
|
||||
### One extra line for the Pay Salary path
|
||||
|
||||
The `show.bs.modal` reset (commit `18ec393`, lines ~2089–2100) runs
|
||||
*after* the Pay-Salary button's pre-show `toggleProjectField()` call
|
||||
and resets the pay-type filter to `''` + shows all rows, then
|
||||
**early-returns** on `_paySalaryOpen` (lines ~2105–2108) before any
|
||||
re-apply. So the managers-only scope set by the button would be wiped.
|
||||
|
||||
Fix: in the `_paySalaryOpen` branch, call `toggleProjectField()` once
|
||||
**before** `return`. `addAdjType.value` is already `'Salary'` there, so
|
||||
this idempotently re-applies the managers-only filter+untick *and*
|
||||
re-affirms the project / Pay-Immediately visibility that
|
||||
`toggleProjectField()` already manages. No behavioural change for that
|
||||
branch beyond restoring the intended scope.
|
||||
|
||||
The `_quickAdjustOpen` branch is **left untouched**: quick-adjust never
|
||||
selects `Salary`, and the reset block already leaves it all-visible —
|
||||
adding a call there is unnecessary and risks regressing the `18ec393`
|
||||
quick-adjust-visibility fix. YAGNI.
|
||||
|
||||
## Known, accepted boundary
|
||||
|
||||
While type = `Salary`, the user can still manually switch the pay-type
|
||||
`<select>` back to "All": the existing `#addAdjPayTypeFilter` change
|
||||
handler reveals the daily rows again (unticked, because Salary cleared
|
||||
them). Manually re-ticking a daily worker and submitting a `Salary` for
|
||||
them is then a **deliberate** act, not an accident. This is the
|
||||
explicit consequence of choosing "filter + auto-untick" over "lock" —
|
||||
the silent footgun is removed; a conscious override is not hard-blocked.
|
||||
Documented here so it is a known design boundary, not a latent bug.
|
||||
|
||||
## Edge cases
|
||||
|
||||
| Scenario | Result |
|
||||
|---|---|
|
||||
| Open (header) → Bonus, tick a daily worker, switch to Salary | Daily rows hide, that tick clears, counter decrements, filter shows "Managers only". |
|
||||
| Pre-ticked manager, switch to Salary | Manager row stays visible & ticked. |
|
||||
| Salary → Bonus | All rows reappear; nothing re-ticked; filter back to "All". |
|
||||
| Pay Salary button | Modal opens type=Salary, filter="Managers only", only managers visible (re-applied after the show.bs.modal reset). |
|
||||
| Quick-adjust button | Unchanged — pre-checked worker visible (type is non-Salary; `18ec393` behaviour preserved). |
|
||||
| Manual filter→"All" while type=Salary | Daily rows reappear **unticked** (accepted boundary above). |
|
||||
|
||||
## Testing
|
||||
|
||||
This is **pure client-side JS** with no view / server / template-data
|
||||
surface, so — exactly like the Task 3 toggle — it is verified by
|
||||
Konrad's manual local checklist, not a Django unit test (the Task 3
|
||||
plan established and documented this precedent). The automated
|
||||
regression gate is the full suite staying **207/207 OK** (the JS
|
||||
addition must not break template rendering).
|
||||
|
||||
Manual checklist additions (append to verification):
|
||||
1. Header-open → Bonus, all workers shown. Tick a daily worker.
|
||||
Switch type → **Salary**: daily rows hide, the daily tick clears,
|
||||
counter updates, pay-type select reads "Managers only".
|
||||
2. Tick a manager, switch **Salary → Bonus**: all rows reappear,
|
||||
the manager stays ticked, pay-type select back to "All pay types".
|
||||
3. Click **Pay Salary**: modal opens with type=Salary, only managers
|
||||
listed, pay-type select = "Managers only".
|
||||
4. While type=Salary, manually set the pay-type select to "All pay
|
||||
types": daily rows reappear and are **unticked**.
|
||||
|
||||
## Files touched
|
||||
|
||||
| File | Change |
|
||||
|---|---|
|
||||
| `core/templates/core/payroll_dashboard.html` | Extend `toggleProjectField()` with the Salary filter+untick sync (~25–40 LOC); add 1 `toggleProjectField()` call in the `_paySalaryOpen` branch before `return` |
|
||||
| `docs/plans/parked-work.md` | One line: the paused entry now also covers the Salary auto-scope behaviour |
|
||||
| `CLAUDE.md` | One line under "Manager / Salaried pay": type=Salary auto-applies Managers-only filter + unticks non-managers (UI guard for the Salary=`fixed` invariant) |
|
||||
|
||||
**No model / migration / view / URL / dependency changes.**
|
||||
|
||||
## Out of scope (deliberately)
|
||||
|
||||
- No hard lock / disabling of the pay-type `<select>` (option C — not
|
||||
chosen).
|
||||
- No server-side enforcement that `Salary` targets only
|
||||
`pay_type='fixed'` (that is a separate, larger hardening; this design
|
||||
is a UI safety improvement only — the existing server behaviour is
|
||||
unchanged).
|
||||
- No "remember and restore the daily workers I auto-unticked" feature
|
||||
(rejected as hidden state; YAGNI).
|
||||
- No change to the `_quickAdjustOpen` path.
|
||||
|
||||
## Branch / deploy
|
||||
|
||||
Build on `ai-dev` on top of `0d77d72`. **Do NOT push or deploy** until
|
||||
Konrad has run the local verification and explicitly approves. Ships
|
||||
bundled with the rest of the paused Manager/Salaried + pay-type-filter
|
||||
work in a single push, Konrad's call.
|
||||
Loading…
x
Reference in New Issue
Block a user