From a6cf766394ec88c4690374c05526bedd56ad9377 Mon Sep 17 00:00:00 2001 From: Konrad du Plessis Date: Thu, 14 May 2026 23:04:12 +0200 Subject: [PATCH] =?UTF-8?q?fix(absences):=20pre-push=20polish=20=E2=80=94?= =?UTF-8?q?=20admin=20sync=20+=20bulk-delete=20cascade=20+=20supervisor=20?= =?UTF-8?q?menu?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small fixes from the final review: - AbsenceAdmin.save_model() now runs _sync_absence_payroll_adjustment so toggling is_paid via /admin/ updates the linked Bonus consistently with the friendly UI. - _delete_adjustment_with_cascade clears absence.is_paid when deleting a Bonus linked to an Absence — closes the state-drift window after bulk-delete from /payroll/?status=adjustments. - base.html — Resources dropdown 'Absences' entry now shows for supervisors as well as staff (was staff-only). View-layer permission helpers (_absence_user_queryset, _user_can_log_absences) already enforce the real access boundary; this just makes the menu honest. 2 regression tests. --- core/admin.py | 22 +++++++++++ core/templates/base.html | 10 ++++- core/tests.py | 81 ++++++++++++++++++++++++++++++++++++++++ core/views.py | 16 ++++++++ 4 files changed, 127 insertions(+), 2 deletions(-) diff --git a/core/admin.py b/core/admin.py index 66e717a..75c5553 100644 --- a/core/admin.py +++ b/core/admin.py @@ -101,6 +101,28 @@ class AbsenceAdmin(admin.ModelAdmin): readonly_fields = ('created_at', 'updated_at') date_hierarchy = 'date' + # === KEEP PAYROLL ADJUSTMENT IN SYNC WITH is_paid === + # Without this, toggling `is_paid` via /admin/core/absence//change/ + # saves the row but does NOT create (or delete) the linked Bonus + # PayrollAdjustment — the friendly UI (`absence_edit`) does this + # correctly. Mirror that behavior here so the two paths are + # consistent. + def save_model(self, request, obj, form, change): + super().save_model(request, obj, form, change) + # Import the sync helper inside the method so admin.py doesn't + # have to import from views at module load (avoids any chance + # of circular-import surprises during Django startup). + from .views import _sync_absence_payroll_adjustment + try: + _sync_absence_payroll_adjustment(obj) + except ValueError as e: + # The sync helper raises ValueError when the user tries to + # un-pay an Absence whose linked adjustment has already been + # paid (i.e. a PayrollRecord exists). Surface that to the + # admin via the messages framework rather than blowing up. + from django.contrib import messages + messages.error(request, str(e)) + @admin.register(Team) class TeamAdmin(admin.ModelAdmin): list_display = ('name', 'supervisor', 'pay_frequency', 'pay_start_date', 'active') diff --git a/core/templates/base.html b/core/templates/base.html index f11e9ea..d5b6c1c 100644 --- a/core/templates/base.html +++ b/core/templates/base.html @@ -69,17 +69,20 @@ History - {% if user.is_staff %} - + {% if user.is_staff or user.supervised_teams.exists %} + {# Resources dropdown — Workers/Teams/Projects are staff-only; Absences also shows for supervisors. #}
    + {% if user.is_staff %}
  • Workers
  • Teams
  • Projects
    • + {% endif %} + {# Absences is broader — supervisors see it too (view-layer enforces real access). #}
  • Absences
@@ -149,6 +152,9 @@ Projects + {% endif %} + {% if user.is_staff or user.supervised_teams.exists %} + {# Absences shows for supervisors too — Workers/Teams/Projects stay staff-only. #} Absences diff --git a/core/tests.py b/core/tests.py index 20c1267..881421f 100644 --- a/core/tests.py +++ b/core/tests.py @@ -2786,3 +2786,84 @@ class AbsenceDashboardCardTests(TestCase): resp = self.client.get('/') self.assertEqual(resp.context['absences_recent_count'], 0) self.assertNotContains(resp, 'absent in last 7 days') + + +# ============================================================================= +# === FINAL PRE-PUSH POLISH FIXES === +# Three small fixes from the final code review of the Absences feature: +# 1) AbsenceAdmin.save_model() runs the sync helper. +# 2) _delete_adjustment_with_cascade clears Absence.is_paid when +# deleting a Bonus that was created from an Absence. +# 3) (template only — no test needed) Resources menu shows Absences +# to supervisors as well as staff. +# ============================================================================= + + +class AbsenceAdminAndCascadeTests(TestCase): + """Final-pre-push fixes: Django admin sync + bulk-delete cascade + + supervisor menu visibility.""" + + @classmethod + def setUpTestData(cls): + cls.admin = User.objects.create_user( + username='admin_polish', password='pw', is_staff=True, is_superuser=True, + ) + cls.worker = Worker.objects.create( + name='W', id_number='1', monthly_salary=Decimal('6000'), + ) + + def test_admin_save_model_syncs_adjustment(self): + """Django admin edit toggling is_paid -> True creates the Bonus.""" + from django.contrib.admin.sites import AdminSite + from core.admin import AbsenceAdmin + from django.http import HttpRequest + + absence = Absence.objects.create( + worker=self.worker, date=_date(2026, 6, 1), + reason='sick', is_paid=False, + ) + site = AdminSite() + admin_cls = AbsenceAdmin(Absence, site) + # Simulate admin form submission: flip is_paid to True. + absence.is_paid = True + req = HttpRequest() + req.user = self.admin + req.session = {} + # The messages framework needs middleware that isn't wired up in + # a bare HttpRequest. We only care that save_model triggers the + # sync helper — if a message tries to render and the framework + # objects, swallow it. + try: + admin_cls.save_model(req, absence, None, change=True) + except Exception: + pass + absence.refresh_from_db() + self.assertTrue(absence.is_paid) + self.assertIsNotNone(absence.payroll_adjustment) + self.assertEqual(absence.payroll_adjustment.type, 'Bonus') + + def test_delete_adjustment_cascade_clears_absence_is_paid(self): + """Deleting a Bonus via _delete_adjustment_with_cascade clears the + is_paid flag on the linked Absence (preventing state drift).""" + from core.views import ( + _sync_absence_payroll_adjustment, + _delete_adjustment_with_cascade, + ) + absence = Absence.objects.create( + worker=self.worker, date=_date(2026, 6, 5), + reason='sick', is_paid=True, + ) + _sync_absence_payroll_adjustment(absence) + absence.refresh_from_db() + self.assertIsNotNone(absence.payroll_adjustment) + adj = absence.payroll_adjustment + + # Delete the adjustment via the cascade helper — this is what + # /payroll/?status=adjustments bulk-delete goes through. + ok, reason = _delete_adjustment_with_cascade(adj) + self.assertTrue(ok) + self.assertIsNone(reason) + + absence.refresh_from_db() + self.assertFalse(absence.is_paid) + self.assertIsNone(absence.payroll_adjustment) diff --git a/core/views.py b/core/views.py index 0e4df5c..ea332bd 100644 --- a/core/views.py +++ b/core/views.py @@ -4340,6 +4340,22 @@ def _delete_adjustment_with_cascade(adj): adj_type = adj.type + # === ABSENCE LINK CLEANUP === + # If this Bonus was auto-created from an Absence (linked via the + # Absence.payroll_adjustment OneToOneField, `related_name='absence'`), + # clear the Absence's `is_paid` flag before deleting. The + # `on_delete=SET_NULL` on the FK nulls `Absence.payroll_adjustment` + # automatically — but `is_paid` wouldn't change without this, so + # the two would drift out of sync (Absence shows "Paid" with no + # linked adjustment). Common case (adjustment NOT from an absence) + # raises DoesNotExist and we just move on. + try: + linked_absence = adj.absence # reverse OneToOne accessor + linked_absence.is_paid = False + linked_absence.save(update_fields=['is_paid']) + except Absence.DoesNotExist: + pass + if adj_type in ('New Loan', 'Advance Payment') and adj.loan: repayment_type = 'Advance Repayment' if adj_type == 'Advance Payment' else 'Loan Repayment' paid_repayments = PayrollAdjustment.objects.filter(