-
Notifications
You must be signed in to change notification settings - Fork 0
#41 - report system #57
base: main
Are you sure you want to change the base?
Conversation
…ware/interns2025b into 15-organization-model
…ssword mail, add dummmy route for password.reset
…ware/interns2025b into 15-organization-model
…and organization invitations; add related tests and translations.
…ze controllers with constants and improved response structures.
Co-authored-by: Dominikaninn <[email protected]>
… constants for request cooldowns in controllers.
… constants for request cooldowns in controllers.
… by replacing inline rate limiter usage.
… by replacing inline rate limiter usage.
… by replacing inline rate limiter usage.
…into 41-report-system
…into 41-report-system
Co-authored-by: Dominikaninn <[email protected]>
Co-authored-by: Dominikaninn <[email protected]>
…r` to use localization.
…e tests to remove `created_at` field usage.
…portController`.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a comprehensive reporting system that allows users to report other users, organizations, and events. The system includes rate limiting to prevent spam (one report per target per day) and admin endpoints to view reports by type.
- Adds a polymorphic Report model with database migration and factory
- Implements API endpoints for creating reports and viewing them by type
- Includes comprehensive test coverage for all reporting scenarios
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Feature/ReportTest.php | Comprehensive test suite covering report creation, validation, rate limiting, and admin viewing |
| routes/api.php | Adds report creation endpoint and admin routes for viewing reports by type |
| lang/pl/report.php | Polish translation strings for report messages |
| lang/en/report.php | English translation strings for report messages |
| database/migrations/2025_07_25_082312_create_reports_table.php | Creates reports table with polymorphic relationship and unique constraint |
| database/factories/ReportFactory.php | Factory for generating test report data |
| app/Models/User.php | Adds reports relationship to User model |
| app/Models/Report.php | Report model with polymorphic relationship and duplicate checking logic |
| app/Http/Resources/ReportResource.php | API resource for formatting report data |
| app/Http/Requests/StoreReportRequest.php | Validation rules for report creation requests |
| app/Http/Controllers/ReportController.php | Controller handling report creation and admin viewing endpoints |
|
|
||
| $table->unique([ | ||
| "reporter_id", | ||
| "reportable_type", | ||
| "reportable_id", | ||
| "created_at", |
Copilot
AI
Jul 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unique constraint includes 'created_at' which is a timestamp. This will not effectively prevent duplicate reports on the same day since 'created_at' includes time components. The constraint should use a date-based approach or the logic should rely on application-level checking only.
| $table->unique([ | |
| "reporter_id", | |
| "reportable_type", | |
| "reportable_id", | |
| "created_at", | |
| $table->date("created_date")->virtualAs("DATE(created_at)"); | |
| $table->unique([ | |
| "reporter_id", | |
| "reportable_type", | |
| "reportable_id", | |
| "created_date", |
| { | ||
| return response()->json([ | ||
| "data" => ReportResource::collection( | ||
| Report::query()->where("reportable_type", User::class)->latest()->get(), |
Copilot
AI
Jul 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using get() without pagination could cause performance issues when there are many reports. Consider implementing pagination for these admin endpoints.
| Report::query()->where("reportable_type", User::class)->latest()->get(), | |
| Report::query()->where("reportable_type", User::class)->latest()->paginate($request->query('per_page', 15)), |
| return response()->json([ | ||
| "data" => ReportResource::collection( | ||
| Report::query()->where("reportable_type", User::class)->latest()->get(), | ||
| ), | ||
| ], Status::HTTP_OK); | ||
| } | ||
|
|
||
| public function organizationReports(): JsonResponse | ||
| { | ||
| return response()->json([ | ||
| "data" => ReportResource::collection( | ||
| Report::query()->where("reportable_type", Organization::class)->latest()->get(), | ||
| ), | ||
| ], Status::HTTP_OK); | ||
| } | ||
|
|
||
| public function eventReports(): JsonResponse | ||
| { | ||
| return response()->json([ | ||
| "data" => ReportResource::collection( | ||
| Report::query()->where("reportable_type", Event::class)->latest()->get(), |
Copilot
AI
Jul 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using get() without pagination could cause performance issues when there are many reports. Consider implementing pagination for these admin endpoints.
| return response()->json([ | |
| "data" => ReportResource::collection( | |
| Report::query()->where("reportable_type", User::class)->latest()->get(), | |
| ), | |
| ], Status::HTTP_OK); | |
| } | |
| public function organizationReports(): JsonResponse | |
| { | |
| return response()->json([ | |
| "data" => ReportResource::collection( | |
| Report::query()->where("reportable_type", Organization::class)->latest()->get(), | |
| ), | |
| ], Status::HTTP_OK); | |
| } | |
| public function eventReports(): JsonResponse | |
| { | |
| return response()->json([ | |
| "data" => ReportResource::collection( | |
| Report::query()->where("reportable_type", Event::class)->latest()->get(), | |
| $perPage = request()->query('per_page', 15); | |
| return response()->json([ | |
| "data" => ReportResource::collection( | |
| Report::query()->where("reportable_type", User::class)->latest()->paginate($perPage), | |
| ), | |
| ], Status::HTTP_OK); | |
| } | |
| public function organizationReports(): JsonResponse | |
| { | |
| $perPage = request()->query('per_page', 15); | |
| return response()->json([ | |
| "data" => ReportResource::collection( | |
| Report::query()->where("reportable_type", Organization::class)->latest()->paginate($perPage), | |
| ), | |
| ], Status::HTTP_OK); | |
| } | |
| public function eventReports(): JsonResponse | |
| { | |
| $perPage = request()->query('per_page', 15); | |
| return response()->json([ | |
| "data" => ReportResource::collection( | |
| Report::query()->where("reportable_type", Event::class)->latest()->paginate($perPage), |
| { | ||
| return response()->json([ | ||
| "data" => ReportResource::collection( | ||
| Report::query()->where("reportable_type", Event::class)->latest()->get(), |
Copilot
AI
Jul 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using get() without pagination could cause performance issues when there are many reports. Consider implementing pagination for these admin endpoints.
| Report::query()->where("reportable_type", Event::class)->latest()->get(), | |
| Report::query()->where("reportable_type", Event::class)->latest()->paginate(15), |
| public function store(StoreReportRequest $request): JsonResponse | ||
| { | ||
| $user = $request->user(); | ||
|
|
||
| $type = match ($request->input("type")) { | ||
| "user" => User::class, | ||
| "organization" => Organization::class, | ||
| "event" => Event::class, | ||
| }; |
Copilot
AI
Jul 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type mapping logic is duplicated in three methods (store, userReports, organizationReports, eventReports). Consider extracting this into a private method or constant to reduce duplication.
| public function store(StoreReportRequest $request): JsonResponse | |
| { | |
| $user = $request->user(); | |
| $type = match ($request->input("type")) { | |
| "user" => User::class, | |
| "organization" => Organization::class, | |
| "event" => Event::class, | |
| }; | |
| private const TYPE_USER = User::class; | |
| private const TYPE_ORGANIZATION = Organization::class; | |
| private const TYPE_EVENT = Event::class; | |
| private function _getTypeMapping(string $type): string | |
| { | |
| return match ($type) { | |
| "user" => self::TYPE_USER, | |
| "organization" => self::TYPE_ORGANIZATION, | |
| "event" => self::TYPE_EVENT, | |
| default => throw new \InvalidArgumentException("Invalid type: $type"), | |
| }; | |
| } | |
| public function store(StoreReportRequest $request): JsonResponse | |
| { | |
| $user = $request->user(); | |
| $type = $this->_getTypeMapping($request->input("type")); |
| $alreadyReported = Report::alreadyReportedToday( | ||
| $user->id, | ||
| $type, | ||
| (int)$request->input("id"), |
Copilot
AI
Jul 30, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The controller doesn't validate that the reported entity exists before creating the report. This could lead to reports for non-existent entities.
|
Conflicts 👀 |
…into 41-report-system # Conflicts: # app/Models/User.php
This should close #41.