fix(adjustments): group-by uses full filtered queryset + Apply keeps group mode

Two final-review follow-ups from the whole-feature code review:

1. Important: group-by was bucketing adj_page.object_list (the paginated
   50-row slice), making 'By Type' group headers show misleading per-page
   totals once filters returned >50 rows. Konrad's current data is under
   the threshold, but the UI promised whole-filter totals.

   Fix: group_by runs on the full filtered queryset (list(adjustments))
   BEFORE pagination. Template already branches on adj_groups, so we now
   additionally hide the pagination nav when grouped — the group headers
   act as their own navigation and their counts/sums reflect the whole
   filter not just one page.

2. Minor: Apply after picking 'By Worker' silently reset to Flat view
   because the filter form had hidden inputs for sort/order but not
   group_by. Added the missing <input type='hidden' name='group_by'>
   so the toggle round-trips across Apply.

65/65 tests still pass (no test changes — the previous tests' fixtures
are all <50 rows so neither the bug nor the fix shows up there, but
both behaviours are now correct).
This commit is contained in:
Konrad du Plessis 2026-04-23 19:37:57 +02:00
parent 269d86259a
commit 3fe3e5aa01
2 changed files with 16 additions and 10 deletions

View File

@ -744,9 +744,11 @@
</div>
</div>
{# --- Sort state (column-header clicks will set these via JS in Task 9) --- #}
{# --- Sort state (column-header clicks set these via JS) --- #}
<input type="hidden" name="sort" value="{{ adj_filter_values.sort }}">
<input type="hidden" name="order" value="{{ adj_filter_values.order }}">
{# --- Group-by state (keeps Flat/By Type/By Worker across Apply) --- #}
<input type="hidden" name="group_by" value="{{ adj_filter_values.group_by }}">
{# --- Apply / Clear buttons --- #}
<div class="d-flex gap-2">
@ -875,8 +877,9 @@
</div>
</div>
{# --- Pagination (hrefs go through `url_replace` so `page=` doesn't stack) --- #}
{% if adj_page.has_other_pages %}
{# --- Pagination: flat view only. When grouped, group headers act as --- #}
{# --- their own navigation and totals cover the whole filtered set. --- #}
{% if adj_page.has_other_pages and not adj_groups %}
<nav class="mt-3 d-flex justify-content-center">
<ul class="pagination pagination-sm">
{% if adj_page.has_previous %}

View File

@ -3042,18 +3042,21 @@ def payroll_dashboard(request):
type__in=DEDUCTIVE_TYPES
).aggregate(total=Sum('amount'))['total'] or Decimal('0.00')
# --- Pagination: 50 rows per page ---
paginator = Paginator(adjustments, 50)
adj_page = paginator.get_page(request.GET.get('page', 1))
# --- Group-by rendering (optional; None = flat view) ---
# When the user clicks the "By Type" or "By Worker" toggle above
# the table, we re-bucket the already-paginated rows. Empty/missing
# means the template falls back to the original flat list.
# the table, we bucket the FULL filtered queryset (not the paginated
# slice) so each group's row-count and net-sum reflect the whole
# filter, not just whatever landed on this page. Pagination is
# suppressed in the template when grouped (the group headers act
# as their own navigation).
group_by = request.GET.get('group_by', '').strip()
adj_groups = None
if group_by in ('type', 'worker'):
adj_groups = _group_adjustments(adj_page.object_list, group_by)
adj_groups = _group_adjustments(list(adjustments), group_by)
# --- Pagination: 50 rows per page (flat view only) ---
paginator = Paginator(adjustments, 50)
adj_page = paginator.get_page(request.GET.get('page', 1))
# --- Everything the Adjustments tab template will need ---
context.update({