Skip to content

E
El-Captain Sports Management
Commit 28fc2e02 authored by Mahmoud Aglan's avatar Mahmoud Aglan
Browse files
Options

feat(reports): what a player owes and has paid, by birth cohort 

Every dues report was per invoice, so answering 'how much is on this boy'
meant adding rows up by eye, and there was no way to ask it of a cohort at
all — no report anywhere could filter on a date of birth. Two new ones:
participant_dues totals each player, participant_invoice_history is the
same money invoice by invoice. Both take a birth-year box, because that is
how a club talks about its players: مواليد 2015 is a cohort, not a search.

What is owed sums each invoice's stored due_amount floored at zero rather
than subtracting paid from billed. An overpaid invoice carries a negative
due_amount, and summing the subtraction would let it cancel another
invoice's real debt — the report would quietly forgive money.

The birth-year filter reaches invoices → participants → people through
whereHas rather than a join, so Participant's BranchScope survives the
subquery and another branch's player cannot be reached by typing their
membership number into the search box.

Filters are declared in the report config instead of special-cased, and
the CSV export builds its arguments from the same list. That closes an
existing divergence: the minimum-absences filter was wired into the page
and not the export, so filtering the absentees report and downloading it
gave you the unfiltered one.
Co-Authored-By: 's avatarClaude Opus 5 (1M context) <noreply@anthropic.com>
parent cc9d0d65
Hide whitespace changes
Inline Side-by-side
Showing with 430 additions and 10 deletions
app/Domain/Shared/Services/ReportService.php
View file @ 28fc2e02
...@@ -69,6 +69,127 @@ public function outstandingBalances(string $from, string $to, ?int $branchId = n ...@@ -69,6 +69,127 @@ public function outstandingBalances(string $from, string $to, ?int $branchId = n
]); ]);
} }
/**
* What each player has been billed, has paid, and still owes.
*
* The one figure the desk is asked for constantly and had nowhere to read:
* every existing dues report is per invoice, so answering "how much is on
* this boy" meant adding up rows by eye.
*
* Filterable by birth year because that is how a club talks about its
* players — "مواليد 2015" is a cohort, not a search.
*/
public function participantDues(
string $from,
string $to,
?int $branchId = null,
?string $birthYear = null,
?string $search = null,
): Collection {
return $this->duesInvoices($from, $to, $branchId, $birthYear, $search)
->groupBy(fn ($inv) => $inv->billable_id)
->map(function ($invoices) {
$participant = $invoices->first()->billable;
$billed = (int) $invoices->sum('total_amount');
$paid = (int) $invoices->sum('paid_amount');
// From the stored columns, not total − paid: an overpaid
// invoice carries a negative due_amount, and summing the
// subtraction would let it cancel out another invoice's real
// debt. Floor at zero per invoice, then add.
$remaining = (int) $invoices->sum(fn ($inv) => max(0, (int) $inv->due_amount));
return [
'participant' => $participant?->person?->name_ar ?? '',
'membership_id' => $participant?->membership_id ?? '',
'phone' => $participant?->person?->phone ?? '',
'birth_year' => $participant?->person?->date_of_birth?->format('Y') ?? '',
'invoices' => $invoices->count(),
'billed' => $billed,
'paid' => $paid,
'remaining' => $remaining,
'status' => $remaining > 0 ? 'مستحق' : 'مدفوع بالكامل',
];
})
->sortByDesc('remaining')
->values();
}
/**
* The same money, invoice by invoice — one player's history rather than
* their total. Same filters, so a cohort can be read either way.
*/
public function participantInvoiceHistory(
string $from,
string $to,
?int $branchId = null,
?string $birthYear = null,
?string $search = null,
): Collection {
return $this->duesInvoices($from, $to, $branchId, $birthYear, $search)
->sortByDesc(fn ($inv) => $inv->issue_date?->timestamp ?? 0)
->values()
->map(function ($inv) {
$remaining = max(0, (int) $inv->due_amount);
return [
'participant' => $inv->billable?->person?->name_ar ?? '',
'membership_id' => $inv->billable?->membership_id ?? '',
'birth_year' => $inv->billable?->person?->date_of_birth?->format('Y') ?? '',
'invoice_number' => $inv->number,
'issue_date' => $inv->issue_date?->format('Y-m-d') ?? '',
'due_date' => $inv->due_date?->format('Y-m-d') ?? '',
'total' => (int) $inv->total_amount,
'paid' => (int) $inv->paid_amount,
'remaining' => $remaining,
'status' => $inv->status instanceof \App\Domain\Financial\Enums\InvoiceStatus
? $inv->status->label()
: (string) $inv->status,
];
});
}
/**
* The invoices both dues reports read: a participant's, in the window, at
* this branch, excluding the cancelled ones.
*
* The birth-year filter reaches through participants to people, which is
* the only place a date of birth is recorded.
*/
private function duesInvoices(
string $from,
string $to,
?int $branchId,
?string $birthYear,
?string $search,
): Collection {
$birthYear = ($birthYear !== null && $birthYear !== '' && ctype_digit((string) $birthYear))
? (int) $birthYear
: null;
$search = is_string($search) ? trim($search) : '';
return Invoice::with(['billable.person'])
->where('billable_type', Participant::class)
->whereNotIn('status', ['cancelled', 'draft'])
->when($branchId, fn ($q) => $q->where('invoices.branch_id', $branchId))
->when($from, fn ($q) => $q->whereDate('issue_date', '>=', $from))
->when($to, fn ($q) => $q->whereDate('issue_date', '<=', $to))
// whereHas, not a join: Participant carries BranchScope and the
// subquery keeps it, so another branch's player cannot be reached
// by typing their membership number into the search box.
->when($birthYear, fn ($q) => $q->whereHas('billable', fn ($p) => $p
->whereHas('person', fn ($person) => $person
->whereBetween('date_of_birth', ["{$birthYear}-01-01", "{$birthYear}-12-31"]))))
->when($search !== '', fn ($q) => $q->whereHas('billable', fn ($p) => $p
->where(fn ($w) => $w
->where('membership_id', 'ilike', "%{$search}%")
->orWhereHas('person', fn ($person) => $person
->where('name_ar', 'ilike', "%{$search}%")
->orWhere('name', 'ilike', "%{$search}%")))))
->get();
}
public function paymentMethodBreakdown(string $from, string $to, ?int $branchId = null): Collection public function paymentMethodBreakdown(string $from, string $to, ?int $branchId = null): Collection
{ {
return Payment::where('direction', 'inbound')->where('status', 'confirmed') return Payment::where('direction', 'inbound')->where('status', 'confirmed')
......
app/Http/Controllers/ExportController.php
View file @ 28fc2e02
...@@ -200,10 +200,30 @@ public function report(Request $request, ReportService $reportService): Streamed ...@@ -200,10 +200,30 @@ public function report(Request $request, ReportService $reportService): Streamed
$from = $request->get('from', now()->startOfMonth()->toDateString()); $from = $request->get('from', now()->startOfMonth()->toDateString());
$to = $request->get('to', now()->toDateString()); $to = $request->get('to', now()->toDateString());
// The report's own filters, read from the query string the viewer put
// them in and passed positionally in the order the report declares
// them. Built from the same list the page uses, so a CSV cannot quietly
// be of something other than what is on screen — which is exactly what
// happened while the minimum-absences filter was a special case here
// and not there.
$extra = [];
foreach ($config['extra_filters'] ?? [] as $filter) {
$extra[] = $request->get($filter);
}
// A filter the request never carried is dropped rather than passed as
// null, so the service method's own default applies. Only from the tail
// — the arguments are positional, so an interior gap would shift every
// filter after it onto the wrong parameter.
while ($extra !== [] && end($extra) === null) {
array_pop($extra);
}
if ($usesDates) { if ($usesDates) {
$data = $reportService->$method($from, $to, $branchId); $data = $reportService->$method($from, $to, $branchId, ...$extra);
} else { } else {
$data = $reportService->$method($branchId); $data = $reportService->$method($branchId, ...$extra);
} }
$moneyCols = $config['money_cols'] ?? []; $moneyCols = $config['money_cols'] ?? [];
......
app/Livewire/Reports/ReportViewer.php
View file @ 28fc2e02
...@@ -37,6 +37,32 @@ class ReportViewer extends Component ...@@ -37,6 +37,32 @@ class ReportViewer extends Component
#[Url] #[Url]
public int $minAbsences = 1; public int $minAbsences = 1;
/**
* A birth cohort — "مواليد 2015". Kept as a string because it is a filter
* box that is usually empty, and an int property would read a blank as 0
* and filter on year zero.
*/
#[Url]
public string $birthYear = '';
#[Url]
public string $search = '';
/**
* Extra filters a report may declare, in `extra_filters`, and the property
* each one reads. Every value is passed to the service method positionally,
* after $branchId, in the order the report lists them.
*
* Declared rather than special-cased so a new filter cannot be wired into
* the page and forgotten in the CSV — the export builds its arguments from
* this same list.
*/
public const FILTER_PROPERTIES = [
'min_absences' => 'minAbsences',
'birth_year' => 'birthYear',
'search' => 'search',
];
public function mount(string $report = ''): void public function mount(string $report = ''): void
{ {
$this->authorize('reports.view'); $this->authorize('reports.view');
...@@ -50,6 +76,27 @@ public function mount(string $report = ''): void ...@@ -50,6 +76,27 @@ public function mount(string $report = ''): void
public function getReportConfig(): array public function getReportConfig(): array
{ {
return [ return [
// What each player owes and has paid, and the same money invoice by
// invoice. Both take a birth-year box because that is how a club
// asks the question — "مواليد 2015", a cohort rather than a search.
'participant_dues' => [
'name' => 'المستحق والمدفوع لكل مشترك',
'method' => 'participantDues',
'headers' => ['المشترك', 'كود ' . term('membership'), 'الهاتف', 'سنة الميلاد', 'عدد الفواتير', 'إجمالي الفواتير', 'المدفوع', 'المتبقي', 'الحالة'],
'columns' => ['participant', 'membership_id', 'phone', 'birth_year', 'invoices', 'billed', 'paid', 'remaining', 'status'],
'money_cols' => ['billed', 'paid', 'remaining'],
'uses_dates' => true,
'extra_filters' => ['birth_year', 'search'],
],
'participant_invoice_history' => [
'name' => 'سجل فواتير المشتركين',
'method' => 'participantInvoiceHistory',
'headers' => ['المشترك', 'كود ' . term('membership'), 'سنة الميلاد', 'رقم الفاتورة', 'تاريخ الإصدار', 'تاريخ الاستحقاق', 'الإجمالي', 'المدفوع', 'المتبقي', 'الحالة'],
'columns' => ['participant', 'membership_id', 'birth_year', 'invoice_number', 'issue_date', 'due_date', 'total', 'paid', 'remaining', 'status'],
'money_cols' => ['total', 'paid', 'remaining'],
'uses_dates' => true,
'extra_filters' => ['birth_year', 'search'],
],
'daily_revenue' => [ 'daily_revenue' => [
'name' => 'الإيرادات اليومية', 'name' => 'الإيرادات اليومية',
'method' => 'dailyRevenue', 'method' => 'dailyRevenue',
...@@ -194,6 +241,7 @@ public function getReportConfig(): array ...@@ -194,6 +241,7 @@ public function getReportConfig(): array
'money_cols' => [], 'money_cols' => [],
'uses_dates' => true, 'uses_dates' => true,
'has_min_absences_filter' => true, 'has_min_absences_filter' => true,
'extra_filters' => ['min_absences'],
], ],
'trainer_attendance' => [ 'trainer_attendance' => [
'name' => 'حضور المدربين', 'name' => 'حضور المدربين',
...@@ -354,6 +402,49 @@ protected function resolveConfig(): array ...@@ -354,6 +402,49 @@ protected function resolveConfig(): array
return $config; return $config;
} }
/**
* The values of the filters this report declares, in declaration order,
* ready to splat after $branchId.
*
* @return array<int, mixed>
*/
protected function extraArguments(array $config): array
{
$out = [];
foreach ($config['extra_filters'] ?? [] as $filter) {
$property = self::FILTER_PROPERTIES[$filter] ?? null;
if ($property !== null) {
$out[] = $this->{$property};
}
}
return $out;
}
/**
* The same filters as query-string parameters, so the CSV is of what is on
* screen. Before this the export dropped them and a filtered report
* downloaded unfiltered.
*
* @return array<string, mixed>
*/
protected function exportFilters(array $config): array
{
$out = [];
foreach ($config['extra_filters'] ?? [] as $filter) {
$property = self::FILTER_PROPERTIES[$filter] ?? null;
if ($property !== null) {
$out[$filter] = $this->{$property};
}
}
return $out;
}
public function render(ReportService $reportService) public function render(ReportService $reportService)
{ {
$config = $this->resolveConfig(); $config = $this->resolveConfig();
...@@ -375,19 +466,19 @@ public function render(ReportService $reportService) ...@@ -375,19 +466,19 @@ public function render(ReportService $reportService)
} elseif ($usesDates) { } elseif ($usesDates) {
$from = $this->from ?: now()->startOfMonth()->format('Y-m-d'); $from = $this->from ?: now()->startOfMonth()->format('Y-m-d');
$to = $this->to ?: now()->format('Y-m-d'); $to = $this->to ?: now()->format('Y-m-d');
if ($method === 'absentParticipantsByTrainer') {
$data = $reportService->$method($from, $to, $branchId, $this->minAbsences); $data = $reportService->$method($from, $to, $branchId, ...$this->extraArguments($config));
} else {
$data = $reportService->$method($from, $to, $branchId);
}
} else { } else {
$data = $reportService->$method($branchId); $data = $reportService->$method($branchId, ...$this->extraArguments($config));
} }
return view('livewire.reports.report-viewer', [ return view('livewire.reports.report-viewer', [
'config' => $config, 'config' => $config,
'data' => $data, 'data' => $data,
'exportUrl' => route('export.report', ['report' => $this->report, 'from' => $this->from, 'to' => $this->to]), 'exportUrl' => route('export.report', array_merge(
['report' => $this->report, 'from' => $this->from, 'to' => $this->to],
$this->exportFilters($config),
)),
]); ]);
} }
} }
app/Livewire/Reports/ReportsHub.php
View file @ 28fc2e02
...@@ -63,6 +63,8 @@ private function allReports(): array ...@@ -63,6 +63,8 @@ private function allReports(): array
'icon' => 'banknotes', 'icon' => 'banknotes',
'color' => 'emerald', 'color' => 'emerald',
'reports' => [ 'reports' => [
['key' => 'participant_dues', 'name' => 'المستحق والمدفوع لكل مشترك', 'desc' => 'كل لاعب: اتحاسب كام، دفع كام، وباقي عليه كام — بفلتر بالمواليد'],
['key' => 'participant_invoice_history', 'name' => 'سجل فواتير المشتركين', 'desc' => 'فاتورة فاتورة: المستحق والمدفوع والمتبقي — بفلتر بالمواليد'],
['key' => 'daily_revenue', 'name' => 'الإيرادات اليومية', 'desc' => 'المبالغ المحصلة يومياً مع طريقة الدفع'], ['key' => 'daily_revenue', 'name' => 'الإيرادات اليومية', 'desc' => 'المبالغ المحصلة يومياً مع طريقة الدفع'],
['key' => 'outstanding_balances', 'name' => 'الأرصدة المعلقة', 'desc' => 'فواتير غير مسددة بالكامل مع بيانات التواصل'], ['key' => 'outstanding_balances', 'name' => 'الأرصدة المعلقة', 'desc' => 'فواتير غير مسددة بالكامل مع بيانات التواصل'],
['key' => 'payment_methods', 'name' => 'طرق الدفع', 'desc' => 'توزيع المدفوعات حسب الطريقة (نقدي، بطاقة، محفظة)'], ['key' => 'payment_methods', 'name' => 'طرق الدفع', 'desc' => 'توزيع المدفوعات حسب الطريقة (نقدي، بطاقة، محفظة)'],
......
resources/views/livewire/reports/report-viewer.blade.php
View file @ 28fc2e02
...@@ -17,7 +17,7 @@ class="inline-flex items-center gap-2 px-4 py-2 bg-green-600 text-white rounded- ...@@ -17,7 +17,7 @@ class="inline-flex items-center gap-2 px-4 py-2 bg-green-600 text-white rounded-
</div> </div>
<!-- Filters --> <!-- Filters -->
@if($config['uses_dates'] || ($config['has_min_absences_filter'] ?? false)) @if($config['uses_dates'] || ($config['has_min_absences_filter'] ?? false) || ($config['extra_filters'] ?? []))
<div class="bg-white rounded-xl shadow-sm border border-gray-200 p-4 mb-4"> <div class="bg-white rounded-xl shadow-sm border border-gray-200 p-4 mb-4">
<div class="grid grid-cols-2 sm:grid-cols-4 gap-3"> <div class="grid grid-cols-2 sm:grid-cols-4 gap-3">
@if($config['uses_dates']) @if($config['uses_dates'])
...@@ -44,6 +44,24 @@ class="w-full rounded-lg border-gray-300 shadow-sm focus:border-blue-500 focus:r ...@@ -44,6 +44,24 @@ class="w-full rounded-lg border-gray-300 shadow-sm focus:border-blue-500 focus:r
</select> </select>
</div> </div>
@endif @endif
@if(in_array('birth_year', $config['extra_filters'] ?? [], true))
<div>
<label class="block text-xs font-medium text-gray-600 mb-1">{{ __('المواليد (سنة الميلاد)') }}</label>
<input type="number" wire:model.live.debounce.400ms="birthYear" dir="ltr"
min="1900" max="{{ now()->year }}" placeholder="{{ __('كل المواليد') }}"
class="w-full rounded-lg border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 text-sm">
</div>
@endif
@if(in_array('search', $config['extra_filters'] ?? [], true))
<div>
<label class="block text-xs font-medium text-gray-600 mb-1">{{ __('الاسم أو الكود') }}</label>
<input type="text" wire:model.live.debounce.400ms="search"
placeholder="{{ __('بحث...') }}"
class="w-full rounded-lg border-gray-300 shadow-sm focus:border-blue-500 focus:ring-blue-500 text-sm">
</div>
@endif
</div> </div>
</div> </div>
@endif @endif
......
tests/Feature/DuesReportRenderTest.php 0 → 100644
View file @ 28fc2e02
<?php
namespace Tests\Feature;
use App\Domain\Financial\Models\Invoice;
use App\Domain\Participant\Models\Participant;
use App\Livewire\Reports\ReportViewer;
use App\Models\User;
use Tests\TestCase;
/**
* The two dues reports, rendered against real books.
*
* A report is a config array, a dynamically-called service method and a Blade
* table; nothing type-checks the join between them, so a renamed column or a
* filter the method does not accept is a 500 at request time and nothing
* earlier catches it. The birth-year filter reaches invoices → participants →
* people, three tables deep, which is exactly the kind of query that is fine in
* isolation and wrong under the branch scope.
*
* Runs against a restored Postgres tenant; skips on the default SQLite suite:
*
* DB_CONNECTION=pgsql DB_DATABASE=oc_sport_test ./vendor/bin/phpunit --filter DuesReportRenderTest
*/
class DuesReportRenderTest extends TestCase
{
protected function setUp(): void
{
parent::setUp();
if (config('database.default') !== 'pgsql') {
$this->markTestSkipped('Needs a restored Postgres tenant; see the class comment.');
}
}
/**
* The academy owner, who holds reports.view. Picked by role slug rather
* than by taking the first user in the table — most users in a real tenant
* are receptionists and coaches, and the pages under test are gated.
*/
private function anAdmin(): User
{
$user = User::withoutGlobalScopes()
->whereHas('primaryRole', fn ($q) => $q->where('slug', 'academy_owner'))
->first();
if (! $user) {
$this->markTestSkipped('No academy_owner in the restored tenant.');
}
return $user;
}
/** A window wide enough to contain the tenant's whole history. */
private function wholeHistory(): array
{
return ['from' => '2000-01-01', 'to' => now()->addYear()->toDateString()];
}
public function test_both_dues_reports_are_registered_and_declare_their_filters(): void
{
$configs = (new ReportViewer())->getReportConfig();
foreach (['participant_dues', 'participant_invoice_history'] as $key) {
$this->assertArrayHasKey($key, $configs);
$this->assertSame(
['birth_year', 'search'],
$configs[$key]['extra_filters'],
'The export builds its arguments from this list, so it is what keeps the CSV and the page in step.'
);
$this->assertSameSize(
$configs[$key]['headers'],
$configs[$key]['columns'],
'A header with no column prints an empty cell and shifts the whole row.'
);
}
}
public function test_the_dues_report_renders(): void
{
$response = $this->actingAs($this->anAdmin())
->get(route('reports.viewer', ['report' => 'participant_dues'] + $this->wholeHistory()));
$response->assertOk();
$response->assertSee('المتبقي', escape: false);
}
public function test_the_invoice_history_report_renders(): void
{
$response = $this->actingAs($this->anAdmin())
->get(route('reports.viewer', ['report' => 'participant_invoice_history'] + $this->wholeHistory()));
$response->assertOk();
$response->assertSee('رقم الفاتورة', escape: false);
}
public function test_the_birth_year_filter_narrows_to_that_cohort(): void
{
$participant = Participant::query()
->whereHas('person', fn ($q) => $q->whereNotNull('date_of_birth'))
->whereHas('invoices')
->with('person')
->first();
if (! $participant) {
$this->markTestSkipped('No invoiced participant with a date of birth in the restored tenant.');
}
$year = (int) $participant->person->date_of_birth->format('Y');
$rows = app(\App\Domain\Shared\Services\ReportService::class)
->participantDues('2000-01-01', now()->addYear()->toDateString(), null, (string) $year);
$this->assertNotEmpty($rows, "No rows for the cohort of {$year}, whose own row should be there.");
foreach ($rows as $row) {
$this->assertSame((string) $year, $row['birth_year'], 'A player from another cohort came back.');
}
}
public function test_an_empty_birth_year_is_not_a_filter_on_year_zero(): void
{
$service = app(\App\Domain\Shared\Services\ReportService::class);
[$from, $to] = ['2000-01-01', now()->addYear()->toDateString()];
// The property behind this box is a string and is usually blank; an int
// property would read '' as 0 and quietly return nothing.
$this->assertEquals(
$service->participantDues($from, $to, null)->count(),
$service->participantDues($from, $to, null, '')->count(),
);
}
public function test_what_a_player_owes_never_goes_negative_on_an_overpayment(): void
{
$rows = app(\App\Domain\Shared\Services\ReportService::class)
->participantDues('2000-01-01', now()->addYear()->toDateString(), null);
foreach ($rows as $row) {
$this->assertGreaterThanOrEqual(
0,
$row['remaining'],
'An overpaid invoice carries a negative due_amount; summed raw it would cancel a real debt.'
);
}
}
public function test_the_totals_are_the_invoices_own_stored_figures(): void
{
$service = app(\App\Domain\Shared\Services\ReportService::class);
[$from, $to] = ['2000-01-01', now()->addYear()->toDateString()];
$rows = $service->participantDues($from, $to, null);
$this->assertNotEmpty($rows, 'The restored tenant has invoices; the report returned none.');
$reportBilled = $rows->sum('billed');
$booksBilled = (int) Invoice::query()
->where('billable_type', Participant::class)
->whereNotIn('status', ['cancelled', 'draft'])
->whereDate('issue_date', '>=', $from)
->whereDate('issue_date', '<=', $to)
->sum('total_amount');
$this->assertSame($booksBilled, (int) $reportBilled, 'The report is billing a different total from the books.');
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment