From 92036f7e4cacfb88a48cc88b012811d8406f2eae Mon Sep 17 00:00:00 2001 From: Konrad du Plessis Date: Wed, 22 Apr 2026 21:36:35 +0200 Subject: [PATCH] Docs: update CLAUDE.md with session learnings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five focused updates from the Apr 22-23 bug-fix + gitignore session: 1. Fix stale supervisor-picker queryset doc: it was showing the pre-fix Q(is_staff)|Q(is_superuser)|Q(groups__name='Work Logger') filter. Since commit 0ceceeb the queryset is just User.objects.filter(is_active=True). 2. Update "How to add a new supervisor" step 2: Work Logger group membership is no longer required for picker visibility — optional now. 3. Add "Schema name-drifts to remember" block near Key Models. Three recurring gotchas that burned four subagent tasks across two sessions: - PayrollAdjustment.description (not reason) - log.adjustments_by_work_log (not payrolladjustment_set) - log.overtime_amount (not log.overtime) 4. Add canonical test-command one-liner to the Commands section: USE_SQLITE=true DJANGO_DEBUG=true python manage.py test core.tests -v 2 5. Add "Django ORM gotcha" subsection documenting the M2M filter + values().annotate(Sum()) inflation bug and the id__in subquery fix pattern (refs commit f1e246c, ReportContextFilterInflationTests). No code changes; no test impact. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 42 +++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 76cc19e..e27a773 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,6 +59,14 @@ staticfiles/ — Collected static assets (Bootstrap, admin) — NOT in git ( - **WorkerCertificate** — per-worker certifications (Skills, PDP, First Aid, Medical, Work at Height) with `valid_until` expiry and optional document upload. Unique per (worker, cert_type). Has `is_expired` and `expires_soon` (≤30 days) properties. - **WorkerWarning** — disciplinary records per worker with severity (verbal/written/final), reason, optional document. Ordered -date. +### Schema name-drifts to remember +Fields / accessors that differ from what you'd guess. Each has bitten multiple +sessions; grep `core/models.py` before using any field you haven't used before: + +- `PayrollAdjustment.description` — NOT `reason` +- `log.adjustments_by_work_log` (reverse accessor for PayrollAdjustment.work_log FK) — NOT `payrolladjustment_set` (the FK has `related_name` set) +- `log.overtime_amount` (DecimalField, default 0.00) — NOT `log.overtime` + ## Key Business Rules - All business logic lives in the `core/` app — do not create additional Django apps - Workers have a `daily_rate` property: `monthly_salary / Decimal('20.00')` @@ -75,6 +83,17 @@ Defined at top of views.py — used in dashboard calculations and payment proces - **ADDITIVE_TYPES** = `['Bonus', 'Overtime', 'New Loan', 'Advance Payment']` — increase worker's net pay - **DEDUCTIVE_TYPES** = `['Deduction', 'Loan Repayment', 'Advance Repayment']` — decrease net pay +## Django ORM gotcha — M2M filter + aggregate inflation +Chained `.filter(m2m__field=X).filter(m2m__other=Y)` creates **separate JOIN +aliases**, producing a cartesian product of rows. `.aggregate(Sum(...))` dedupes +via subquery when `distinct()` is present; `.values().annotate(Sum(...))` does +NOT — it `GROUP BY`s the inflated rows and multiplies sums by N×M (where N and +M are the counts of matching related rows). Fix pattern: use +`.filter(id__in=Model.objects.filter(m2m__field=X).values('id'))` to keep the +outer queryset JOIN-free. See `_build_report_context` in `core/views.py` and +`ReportContextFilterInflationTests` in `core/tests.py` for the reference +implementation (commit f1e246c, Apr 2026). + ## PayrollAdjustment Type Handling - **Bonus / Deduction** — standalone, require a linked Project - **New Loan** — creates a `Loan` record (`loan_type='loan'`); has a "Pay Immediately" checkbox (checked by default) that auto-processes the loan (creates PayrollRecord, sends payslip to Spark, marks as paid). When unchecked, the loan sits in Pending Payments for the next pay cycle. Editing syncs loan amount/balance/reason; deleting cascades to Loan + unpaid repayments @@ -102,6 +121,9 @@ python manage.py setup_test_data # Populate sample workers, projects, python manage.py import_production_data # Import real production data (14 workers) python manage.py collectstatic # Collect static files for production python manage.py check # System check + +# Run the test suite (sets env vars inline — works in Git Bash; on cmd.exe use `set` first) +USE_SQLITE=true DJANGO_DEBUG=true python manage.py test core.tests -v 2 ``` ## Development Workflow @@ -420,13 +442,16 @@ When editing a Team or Project via the friendly UI (`/teams//edit/` or `_supervisor_user_queryset()` in `core/forms.py`: ```python -User.objects.filter(is_active=True).filter( - Q(is_staff=True) | Q(is_superuser=True) | Q(groups__name='Work Logger') -).distinct() +User.objects.filter(is_active=True).order_by('username') ``` -So anyone who's either an admin OR a Work Logger shows up as an eligible -supervisor. Deactivated accounts (`is_active=False`) are hidden. +Any active user can be picked. The picker is deliberately NOT pre-filtered +by group/staff flags because `is_supervisor(user)` (views.py) grants +supervisor powers to anyone assigned to a team/project FK/M2M — so the +picker shouldn't be stricter than the permission model. Pre-Apr 2026 the +picker required Work Logger group membership, which hid valid supervisors +(see commit 0ceceeb for the fix + regression tests). Deactivated accounts +are still hidden. ### Typical user setups @@ -449,10 +474,13 @@ we'd have to add a separate flag or group check — not currently supported. 1. Go to `/admin/auth/user/add/` and create the user with a username and password. **Uncheck "Staff status"** on the initial form (they don't need Django admin access). -2. On the user's change page, add them to the **Work Logger** group. +2. (Optional) Add them to the **Work Logger** group if you want + `is_supervisor(user)` to return True even without a team/project + assignment. Not required for the picker to show them — the picker + shows any active user (see commit 0ceceeb, Apr 2026). 3. (Optional) Assign them as the supervisor of one or more teams via `/teams//edit/` (Supervisor dropdown — they'll appear in the list - because of their Work Logger group membership). + because they're active). 4. (Optional) Add them to one or more projects via `/projects//edit/` (Supervisors M2M checklist). 5. They can now log in at `/accounts/login/` and will land on the Dashboard