Repository navigation
api added - #8
Merged
Merged
Conversation
semkasanga
requested review from
jeanluckawel
and
a lite review from Copilot
September 15, 2026 12:13
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical payment and revenue defects, plus compatibility and validation issues, block approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds authenticated v1 APIs for students, revenue reporting, and payments, with validation, tests, and documentation.
Changes:
- Adds paginated student listings and approved-enrollment filtering.
- Adds revenue period filters and USD/CDF totals.
- Adds transactional payment processing and API validation.
File summaries
| File | Summary |
|---|---|
tests/Feature/ApiEndpointsTest.php |
Adds authentication and validation tests. |
routes/api.php |
Registers API routes; moderate (2 votes): reusing the index action changes the legacy endpoint contract. |
phpunit.xml |
Configures SQLite in-memory testing. |
app/Http/Resources/EleveResource.php |
Defines student responses; moderate (1 vote): nested relations may cause N+1 queries. |
app/Http/Requests/Api/V1/Finance/RevenueIndexRequest.php |
Validates revenue filters. |
app/Http/Requests/Api/V1/Finance/PaymentStoreRequest.php |
Critical (1 vote each): payment amount/currency are insufficiently constrained with perception_id, and paid_by is client-controlled. |
app/Http/Controllers/Api/V1/Scolarite/EleveController.php |
Moderate (1–2 votes): changes the legacy response, casts pagination values before validation, and omits relations needed by resources. |
app/Http/Controllers/Api/V1/Finance/RevenueController.php |
Critical (1 vote): explicit date ranges can throw a type error. Moderate (1 vote): responses bypass the established resource contract. |
app/Http/Controllers/Api/V1/Finance/PaymentController.php |
Critical (1 vote): CDF payments can incorrectly return 404. Critical (3 votes): selection can target an arbitrary unpaid perception. Moderate (2 votes): paid perceptions may return 404 instead of 409. |
API_ENDPOINTS.md |
Documents the new endpoints. |
.gitignore |
Ignores generated documentation and cache files. |
Review details
Suppressed comments (3)
app/Http/Controllers/Api/V1/Finance/RevenueController.php:45
- Returning model instances directly bypasses the existing finance
ReceiptResource/ReceiptServicecontract.PerceptionJSON now exposes internal foreign-key/audit fields and the eagerly loadedfraisrelation, making the public response depend on the database schema; map these items through a dedicated resource (or the established receipt resource) before returning them.
'revenues' => $revenues->items(),
app/Http/Controllers/Api/V1/Scolarite/EleveController.php:26
- This action is also still registered for the existing
/api/v1/scolarite/elevesroute. Applying the new current-year/approved filter here changes that endpoint's results, while the manual payload below also changes its response shape from the paginator object to a list plus custom metadata. Isolate the new/studentsbehavior in a separate action/resource or preserve the legacy contract.
$eleves = Eleve::query()
->with(['inscriptions' => fn ($query) => $query
->with('classe')
->when($annee, fn ($query) => $query->where('annee_id', $annee->id))
->where('status', InscriptionStatus::approved)])
app/Http/Resources/EleveResource.php:20
InscriptionResourcedereferences the inscription'seleveandannee, but the controller only eager-loadsinscriptions.classe. Adding this nested collection therefore causes lazy queries per inscription (and can make a paginated request N+1); eager-load the relations required by the resource or use a shallow inscription representation here.
'inscriptions' => InscriptionResource::collection($this->whenLoaded('inscriptions')),
- Files reviewed: 10/11 changed files
- Comments generated: 9
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+22
to
+24
| ->where('devise', $request->string('currency')->toString()) | ||
| ->where('frais_montant', (float) $request->input('amount')) | ||
| ->whereNull('paid_at') |
| ->where('frais_montant', (float) $request->input('amount')) | ||
| ->whereNull('paid_at') | ||
| ->when(Annee::encours(), fn ($query, $annee) => $query->where('annee_id', $annee->id)) | ||
| ->lockForUpdate()->firstOrFail(); |
Comment on lines
+23
to
+24
| Carbon::parse($request->string('startDate'))->startOfDay(), | ||
| Carbon::parse($request->string('endDate'))->endOfDay(), |
Comment on lines
+16
to
+17
| 'amount' => ['required_with:student_id', 'numeric', 'gt:0'], | ||
| 'currency' => ['required_with:student_id', Rule::enum(Devise::class)], |
| 'student_id' => ['nullable', 'string', 'exists:eleves,id', 'required_without:perception_id'], | ||
| 'amount' => ['required_with:student_id', 'numeric', 'gt:0'], | ||
| 'currency' => ['required_with:student_id', Rule::enum(Devise::class)], | ||
| 'paid_by' => ['nullable', 'string', 'max:255'], |
Comment on lines
+24
to
+26
| ->whereNull('paid_at') | ||
| ->when(Annee::encours(), fn ($query, $annee) => $query->where('annee_id', $annee->id)) | ||
| ->lockForUpdate()->firstOrFail(); |
Comment on lines
+17
to
+18
| $perPage = (int) $request->get('per_page', $request->get('limit', 20)); | ||
| abort_if($perPage < 1 || $perPage > 100, 422, 'Le nombre d\'éléments par page doit être compris entre 1 et 100.'); |
| ->paginate($request->get('per_page', 20)); | ||
| $eleves = Eleve::query() | ||
| ->with(['inscriptions' => fn ($query) => $query | ||
| ->with('classe') |
Comment on lines
+30
to
+31
| Route::get('/students', [EleveController::class, 'index']) | ||
| ->name('api.v1.students.index'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ajouts principaux :
GET /api/v1/studentsavec pagination et élèves inscrits approuvés.GET /api/v1/revenuesavec filtres jour/mois/année et totaux USD/CDF séparés.POST /api/v1/paymentsbasé sur les perceptions existantes.