fix(absences): pre-push polish — admin sync + bulk-delete cascade + supervisor menu
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.
This commit is contained in:
Konrad du Plessis
parent
9345dacfbf
commit
a6cf766394
@ -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/<id>/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')
|
||||
|
||||
@ -69,17 +69,20 @@
|
||||
<a href="{% url 'work_history' %}" class="topbar-nav__link {% if request.resolver_match.url_name == 'work_history' %}active{% endif %}">
|
||||
<i class="fas fa-clock"></i><span>History</span>
|
||||
</a>
|
||||
{% if user.is_staff %}
|
||||
<!-- Resources dropdown: Workers, Teams, Projects, Absences -->
|
||||
{% if user.is_staff or user.supervised_teams.exists %}
|
||||
{# Resources dropdown — Workers/Teams/Projects are staff-only; Absences also shows for supervisors. #}
|
||||
<div class="dropdown">
|
||||
<a href="#" class="topbar-nav__link dropdown-toggle {% if 'worker' in request.resolver_match.url_name|default:'' or 'team' in request.resolver_match.url_name|default:'' or 'project' in request.resolver_match.url_name|default:'' or 'absence' in request.resolver_match.url_name|default:'' %}active{% endif %}"
|
||||
data-bs-toggle="dropdown" aria-expanded="false" role="button">
|
||||
<i class="fas fa-hard-hat"></i><span>Resources</span>
|
||||
</a>
|
||||
<ul class="dropdown-menu">
|
||||
{% if user.is_staff %}
|
||||
<li><a class="dropdown-item" href="{% url 'worker_list' %}"><i class="fas fa-hard-hat me-2" style="color: var(--accent);"></i>Workers</a></li>
|
||||
<li><a class="dropdown-item" href="{% url 'team_list' %}"><i class="fas fa-users me-2" style="color: var(--accent);"></i>Teams</a></li>
|
||||
<li><a class="dropdown-item" href="{% url 'project_list' %}"><i class="fas fa-project-diagram me-2" style="color: var(--accent);"></i>Projects</a></li>
|
||||
{% endif %}
|
||||
{# Absences is broader — supervisors see it too (view-layer enforces real access). #}
|
||||
<li><a class="dropdown-item" href="{% url 'absence_list' %}"><i class="fas fa-user-clock me-2" style="color: var(--accent);"></i>Absences</a></li>
|
||||
</ul>
|
||||
</div>
|
||||
@ -149,6 +152,9 @@
|
||||
<a href="{% url 'project_list' %}" class="mobile-menu__link">
|
||||
<i class="fas fa-project-diagram"></i><span>Projects</span>
|
||||
</a>
|
||||
{% endif %}
|
||||
{% if user.is_staff or user.supervised_teams.exists %}
|
||||
{# Absences shows for supervisors too — Workers/Teams/Projects stay staff-only. #}
|
||||
<a href="{% url 'absence_list' %}" class="mobile-menu__link">
|
||||
<i class="fas fa-user-clock"></i><span>Absences</span>
|
||||
</a>
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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(
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user