38686-vm/docs/plans/2026-05-16-salary-autoscope-picker-plan.md
Konrad du Plessis 0c705129f6 docs: TDD plan for Salary auto-scope picker (2 tasks, HARD STOP)
2 small tasks: (1) toggleProjectField() Salary sync + _paySalaryOpen
re-apply, (2) docs. JS-only — manual-checklist verified, suite stays
207/207. Nothing pushed until Konrad's local verification.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-16 22:14:06 +02:00

260 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Salary Auto-Scope Picker — Implementation Plan
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development to implement this plan task-by-task (in-session, fresh subagent + 2-stage review per task).
**Goal:** When the Add-Adjustment modal's type is set to `Salary`, auto-set the pay-type filter to "Managers only", hide daily-worker rows, and untick any selected daily worker — so a `Salary` adjustment can never silently target a daily worker.
**Architecture:** Pure client-side JS. One sync block appended to the existing `toggleProjectField()` chokepoint (already fires on type change / init / Pay-Salary button / header reset) + one re-apply line in the `_paySalaryOpen` modal-open branch (because the `show.bs.modal` reset wipes the filter after the Pay-Salary button sets it). No view/model/template-data/URL change.
**Tech Stack:** Django 5.2.7 template with vanilla JS; SQLite local (`USE_SQLITE=true`); Bootstrap 5 modal.
**Design doc:** `docs/plans/2026-05-16-salary-autoscope-picker-design.md` (commit `8f443fa`).
**Branch / baseline:** `ai-dev`, HEAD `8f443fa`, **207/207 tests passing**. On top of the paused, un-pushed Manager/Salaried + pay-type-filter commits.
**Test command (Git Bash, per CLAUDE.md):**
```bash
USE_SQLITE=true DJANGO_DEBUG=true python manage.py test core.tests -v 2
```
> **TDD note — read this.** This change is **pure browser JS** with no
> Django view / server / template-context surface, so there is no
> meaningful failing Django unit test to write first (asserting raw JS
> source text would be brittle and low-value). This matches the
> explicit, Konrad-approved precedent from the Task 3 modal toggle: the
> behaviour is verified by Konrad's **manual local checklist** (in the
> design doc), and the **automated regression gate is the full suite
> staying 207/207 OK** (the JS edit must not break template rendering).
> Do NOT add a source-text-sniffing test to satisfy a TDD ritual — the
> design doc documents this decision; the spec/quality reviewers will
> be told this is expected.
> ⛔ **HARD STOP after Task 2.** Do NOT `git push`, do NOT deploy. Hand
> back to Konrad to run the manual verification checklist. Ships bundled
> with the rest of the paused Manager/Salaried + pay-type-filter work in
> ONE push, on Konrad's explicit say-so only.
---
### Task 1: Salary auto-scope sync in `toggleProjectField()` + Pay-Salary re-apply
**Files:**
- Modify: `core/templates/core/payroll_dashboard.html``toggleProjectField()` (currently lines 19281951) and the `_paySalaryOpen` branch (currently lines 21052108)
**Step 1: Confirm the green baseline (regression anchor)**
Run:
```bash
USE_SQLITE=true DJANGO_DEBUG=true python manage.py test core.tests -v 2
```
Expected: **207 tests, OK**. If not, STOP and report — do not proceed.
**Step 2: Add the Salary sync block to `toggleProjectField()`**
`toggleProjectField()` currently ends like this (lines 19431951):
```javascript
// "Pay Immediately" checkbox — shown for New Loan AND Salary.
// Salary mirrors New Loan: ticked = pay the manager now + email
// payslip; unticked = leave it pending for the next pay cycle.
if (addAdjPayImmediatelyGroup) {
var payNowTypes = ['New Loan', 'Salary'];
addAdjPayImmediatelyGroup.style.display =
(payNowTypes.indexOf(addAdjType.value) !== -1) ? '' : 'none';
}
}
```
Insert the following block **immediately before the closing `}` of
`toggleProjectField()`** (i.e. after the `if (addAdjPayImmediatelyGroup) { … }`
block, before the line ` }` that closes the function):
```javascript
// === Salary → managers-only picker scope ===
// Salary is a managers-only adjustment (Worker.pay_type='fixed').
// To make it IMPOSSIBLE to accidentally pay a Salary to a daily
// worker, when the type is Salary we: (1) set the pay-type
// filter to "Managers only", (2) hide every non-manager row,
// and (3) UNTICK any non-manager that was already selected so a
// stray daily-worker tick can't ride along hidden into a bad
// Salary adjustment. For any other type we reset the filter to
// "All" and re-show every row (we do NOT re-tick anything — the
// user re-selects deliberately). Re-grab the filter <select> by
// id here rather than relying on the outer `addAdjPayTypeFilter`
// var, because this function also runs at init time (before that
// var is assigned).
var _ptFilter = document.getElementById('addAdjPayTypeFilter');
if (_ptFilter) {
var _isSalary = (addAdjType.value === 'Salary');
_ptFilter.value = _isSalary ? 'fixed' : '';
addAdjWorkerCheckboxes.forEach(function(cb) {
var row = cb.closest('.form-check');
if (!row) return;
var rowType = row.getAttribute('data-pay-type') || '';
if (_isSalary && rowType !== 'fixed') {
// Non-manager while Salary: hide AND untick it.
row.style.display = 'none';
cb.checked = false;
} else {
// Manager (any type) or any row when not Salary:
// ensure visible; leave its checked state alone.
row.style.display = '';
}
});
// Reflect any auto-untick in the "X worker(s) selected"
// counter (function-hoisted, safe to call here).
updateWorkerCount();
}
```
Notes for the implementer:
- `addAdjWorkerCheckboxes` is the `const` NodeList declared at line 1920 (before this function) — use it directly.
- `updateWorkerCount` is a function declaration (hoisted) — safe to call even though defined later in the file.
- Do NOT re-tick anything in the non-Salary branch. Do NOT add "remember what I unticked" state. (Design § Out of scope.)
- Match the file's existing indentation (8 spaces at this nesting level inside the function).
**Step 3: Re-apply on the Pay-Salary open path**
The `_paySalaryOpen` branch currently is (lines 21052108):
```javascript
if (_paySalaryOpen) {
_paySalaryOpen = false;
return; // Pay-Salary already set type=Salary
}
```
Change it to (add the `toggleProjectField()` call + comment before `return`):
```javascript
if (_paySalaryOpen) {
_paySalaryOpen = false;
// The reset block above just cleared the pay-type
// filter + re-showed all rows. Type is already 'Salary'
// (set by the Pay-Salary button), so re-run the
// chokepoint to restore Managers-only scope + untick
// any non-manager.
toggleProjectField();
return; // Pay-Salary already set type=Salary
}
```
Do NOT touch the `_quickAdjustOpen` branch (design § Out of scope —
quick-adjust never selects Salary; the `18ec393` visibility behaviour
must be preserved).
**Step 4: Guard greps**
```bash
grep -rn "^\s*{#" core/templates/core/payroll_dashboard.html | awk -F: '$0 !~ /#}/ {print}'
grep -c 'id="addAdjPayTypeFilter"' core/templates/core/payroll_dashboard.html
grep -c 'function toggleProjectField' core/templates/core/payroll_dashboard.html
```
Expected: first → no output; second → `1`; third → `1` (no accidental duplicate function).
**Step 5: Regression gate**
```bash
USE_SQLITE=true DJANGO_DEBUG=true python manage.py test core.tests -v 2
```
Expected: **207 tests, OK** (UNCHANGED — JS-only edit, no new/changed test by design). If the count changed or anything FAILS, STOP and report.
**Step 6: Commit (local only, NO push, NEW commit)**
```bash
git add core/templates/core/payroll_dashboard.html
git commit -m "feat: type=Salary auto-scopes Add-Adjustment picker to managers
When type=Salary: set 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. Re-applied on the Pay-Salary open
path (the show.bs.modal reset clears it first). Pure JS; verified by
manual checklist; suite stays 207/207.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
```
**Self-review:** `git show HEAD` — only `payroll_dashboard.html` changed; the sync block is inside `toggleProjectField()` before its closing `}`; the `_paySalaryOpen` branch gained exactly the one `toggleProjectField()` call; `_quickAdjustOpen` untouched; indentation consistent; guard greps clean.
---
### Task 2: Docs + final regression + HARD STOP
**Files:**
- Modify: `docs/plans/parked-work.md`
- Modify: `CLAUDE.md`
**Step 1: Full regression (anchor)**
```bash
USE_SQLITE=true DJANGO_DEBUG=true python manage.py test core.tests -v 2
```
Expected: **207 OK**. If not, STOP.
**Step 2: Update `docs/plans/parked-work.md`**
Read the file, find the paused Manager/Salaried entry (the one already
updated with the pay-type filter note — under the "⏸ Paused …
awaiting Konrad's verification" section). Append, matching the file's
prose style:
> Also now: setting the Add-Adjustment **type = Salary** auto-scopes
> the picker — pay-type filter → "Managers only", daily rows hidden,
> and any selected daily worker auto-unticked (UI guard so a `Salary`
> can never silently target a daily worker). Design
> `docs/plans/2026-05-16-salary-autoscope-picker-design.md`, plan
> `docs/plans/2026-05-16-salary-autoscope-picker-plan.md`. Same HARD
> STOP — bundled into the one push on Konrad's say-so.
**Step 3: Update `CLAUDE.md`**
In the "## Manager / Salaried pay (May 2026)" section, immediately after
the existing **"Finding managers:"** line (added by the prior task),
append one line in the same terse style:
> **Salary picker safety:** in the Add-Adjustment modal, choosing
> type=`Salary` auto-sets the pay-type filter to Managers-only, hides
> daily rows, and **unticks** any already-selected daily worker (so a
> `Salary` can never silently be created for a `pay_type='daily'`
> worker). Switching to any other type resets the filter to "All" and
> re-shows everyone (no auto-re-tick). Pure JS in
> `toggleProjectField()` (`payroll_dashboard.html`); not a hard lock —
> manually switching the filter back to "All" is still allowed
> (deliberate override, not the silent footgun).
**Step 4: Verify docs**
```bash
grep -n "auto-scope\|auto-untick\|Salary picker safety\|autoscope" docs/plans/parked-work.md CLAUDE.md
```
Expected: the new lines appear in both files. Sanity-check no Markdown
broken (plain prose only).
**Step 5: Commit (local only, NO push, NEW commit)**
```bash
git add docs/plans/parked-work.md CLAUDE.md
git commit -m "docs: note Salary auto-scope picker (rides paused bundle)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>"
```
**Self-review:** `git show --stat HEAD` — only the 2 doc files changed, additions only.
---
## ⛔ HARD STOP — hand back to Konrad
After Task 2's commit:
1. `git status` clean; `git log --oneline -3` shows the 2 new commits on `ai-dev` on top of `8f443fa`.
2. Full suite once more → **207/207 OK**.
3. **Do NOT `git push`. Do NOT deploy.** Report the commit list + test count and point Konrad at the **Testing / manual checklist** section of `docs/plans/2026-05-16-salary-autoscope-picker-design.md` (the 4 Salary-scope steps) plus the existing pay-type-filter checklist.
4. Push/deploy only on Konrad's explicit approval, bundled with the rest of the paused Manager/Salaried + pay-type-filter work in ONE push (github + gitea; deploy order: pull → migrate → collectstatic → restart last).
## Notes
- **DRY:** the sync reuses the single `toggleProjectField()` chokepoint and the existing `data-pay-type` attribute + `updateWorkerCount()` helper. No new event listeners.
- **YAGNI:** no hard lock (option C declined), no server-side `Salary`-target enforcement, no "restore auto-unticked workers" memory.
- **Why re-grab `addAdjPayTypeFilter` by id inside the function:** `toggleProjectField()` runs once at init (line ~1954) *before* the outer `var addAdjPayTypeFilter` is assigned (~line 2011); the local `document.getElementById(...)` + `if (_ptFilter)` guard makes it order-independent and a harmless no-op at init (modal not open yet).
- **No new migration / view / test by design** — JS-only; manual-checklist verified (documented precedent from Task 3, Konrad-approved in the design doc).