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>
This commit is contained in:
Konrad du Plessis 2026-05-16 22:14:06 +02:00
parent 8f443faebc
commit 0c705129f6

View File

@ -0,0 +1,259 @@
# 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).