Conversation
Thiritin
commented
Aug 1, 2025
- Introduced BadgeControllerFlowTest to cover various event states and badge creation scenarios.
- Updated PHPUnit configuration for in-memory SQLite database during tests.
- Enhanced Welcome page tests to verify user interface states based on event conditions.
- Added mass print detection tests to ensure correct messaging based on badge printing status.
- Refactored existing tests to improve clarity and maintainability.
- Adjusted Vue components for better responsiveness and layout consistency.
- Created a new log file for standard output during server operations.
- Introduced BadgeControllerFlowTest to cover various event states and badge creation scenarios. - Updated PHPUnit configuration for in-memory SQLite database during tests. - Enhanced Welcome page tests to verify user interface states based on event conditions. - Added mass print detection tests to ensure correct messaging based on badge printing status. - Refactored existing tests to improve clarity and maintainability. - Adjusted Vue components for better responsiveness and layout consistency. - Created a new log file for standard output during server operations.
…ase run composer update.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces comprehensive test coverage for badge creation and event handling functionality, along with improvements to the testing infrastructure and UI components. The changes focus on ensuring proper behavior across different event states and user scenarios.
- Enhanced test infrastructure with in-memory SQLite database and improved CI/CD pipeline
- Added comprehensive badge controller flow tests covering all event states (closed, open, order windows)
- Updated Welcome page tests to verify UI states based on event conditions and mass printing detection
- Improved Vue components for better layout consistency and responsiveness
Reviewed Changes
Copilot reviewed 15 out of 29 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Feature/WelcomePageFlowTest.php | New test file covering Welcome page states based on event conditions and mass print detection |
| tests/Feature/BadgeTest.php | Updated existing badge tests with improved storage handling and time adjustments |
| tests/Feature/BadgeControllerFlowTestFixed.php | New comprehensive test file for badge controller flows across different event states |
| tests/Feature/BadgeControllerFlowTest.php | Additional badge controller test file with detailed event state scenarios |
| resources/js/Pages/Welcome.vue | UI improvements for better button styling and layout consistency |
| resources/js/Pages/Badges/BadgesIndex.vue | Added responsive layout constraints |
| resources/js/Pages/Badges/BadgesEdit.vue | Added responsive layout constraints |
| phpunit.xml | Updated configuration for in-memory SQLite testing with additional environment variables |
| database/migrations/2024_09_01_027325_create_user_catch_rankings_table.php | Added SQLite compatibility for database constraints |
| app/Models/Event.php | Fixed getActiveEvent method to properly filter by start time |
| app/Http/Middleware/HandleInertiaRequests.php | Updated event sharing logic |
| app/Http/Middleware/EventEndedMiddleware.php | Enhanced middleware to handle badge routes when no active event exists |
| app/Http/Controllers/BadgeController.php | Added error handling for wallet operations and fixed query references |
| .github/workflows/laravel.yml | Updated CI workflow to use SQLite and improved caching |
| .env.testing | Simplified testing environment configuration |
Comments suppressed due to low confidence (2)
tests/Feature/BadgeControllerFlowTestFixed.php:95
- This test creates a badge but doesn't verify that the badge was not actually created in the database. Consider adding an assertion like
$this->assertDatabaseCount('badges', 0);after the forbidden response.
test('badge creation is forbidden when orders are closed', function () {
.github/workflows/laravel.yml:15
- PHP 8.4 does not exist yet. The latest stable version is PHP 8.3. Consider using PHP 8.3 instead.
php-version: ['8.4']
| ); | ||
| }); | ||
|
|
||
| test('shows closed state when event hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('shows closed state when event hasnt started', function () { | |
| test('shows closed state when event hasn\'t started', function () { |
| ); | ||
| }); | ||
|
|
||
| test('shows closed state when order window hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('shows closed state when order window hasnt started', function () { | |
| test('shows closed state when order window hasn\'t started', function () { |
| ]); | ||
| }); | ||
|
|
||
| test('event state is CLOSED when order window hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('event state is CLOSED when order window hasnt started', function () { | |
| test('event state is CLOSED when order window hasn\'t started', function () { |
| ]); | ||
| }); | ||
|
|
||
| test('getActiveEvent returns null when event exists but hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('getActiveEvent returns null when event exists but hasnt started', function () { | |
| test('getActiveEvent returns null when event exists but hasn't started', function () { |
| expect(Event::getActiveEvent())->toBeNull(); | ||
| }); | ||
|
|
||
| test('event state is CLOSED when event hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('event state is CLOSED when event hasnt started', function () { | |
| test('event state is CLOSED when event hasn\'t started', function () { |
| test('getActiveEvent returns null when event exists but hasnt started', function () { | ||
| expect(Event::getActiveEvent())->toBeNull(); | ||
| }); | ||
|
|
||
| test('event state is CLOSED when event hasnt started', function () { | ||
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | ||
| expect($this->event->allowsOrders())->toBeFalse(); | ||
| }); | ||
|
|
||
| test('badge creation is forbidden when event hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('getActiveEvent returns null when event exists but hasnt started', function () { | |
| expect(Event::getActiveEvent())->toBeNull(); | |
| }); | |
| test('event state is CLOSED when event hasnt started', function () { | |
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | |
| expect($this->event->allowsOrders())->toBeFalse(); | |
| }); | |
| test('badge creation is forbidden when event hasnt started', function () { | |
| test('getActiveEvent returns null when event exists but hasn\'t started', function () { | |
| expect(Event::getActiveEvent())->toBeNull(); | |
| }); | |
| test('event state is CLOSED when event hasn\'t started', function () { | |
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | |
| expect($this->event->allowsOrders())->toBeFalse(); | |
| }); | |
| test('badge creation is forbidden when event hasn\'t started', function () { |
| test('event state is CLOSED when order window hasnt started', function () { | ||
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | ||
| expect($this->event->allowsOrders())->toBeFalse(); | ||
| }); | ||
|
|
||
| test('badge creation is forbidden when order window hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('event state is CLOSED when order window hasnt started', function () { | |
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | |
| expect($this->event->allowsOrders())->toBeFalse(); | |
| }); | |
| test('badge creation is forbidden when order window hasnt started', function () { | |
| test('event state is CLOSED when order window hasn\'t started', function () { | |
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | |
| expect($this->event->allowsOrders())->toBeFalse(); | |
| }); | |
| test('badge creation is forbidden when order window hasn\'t started', function () { |
| test('event state is CLOSED when order window hasnt started', function () { | ||
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | ||
| expect($this->event->allowsOrders())->toBeFalse(); | ||
| }); | ||
|
|
||
| test('badge creation is forbidden when order window hasnt started', function () { |
There was a problem hiding this comment.
The contraction 'hasnt' is missing an apostrophe. It should be "hasn't".
| test('event state is CLOSED when order window hasnt started', function () { | |
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | |
| expect($this->event->allowsOrders())->toBeFalse(); | |
| }); | |
| test('badge creation is forbidden when order window hasnt started', function () { | |
| test('event state is CLOSED when order window hasn\'t started', function () { | |
| expect($this->event->state)->toBe(EventStateEnum::CLOSED); | |
| expect($this->event->allowsOrders())->toBeFalse(); | |
| }); | |
| test('badge creation is forbidden when order window hasn\'t started', function () { |
| $request->user()->balanceInt; | ||
| // Lock Badge | ||
| $badge->where('id', $badge->id)->orWhere('extra_copy_of', $badge->id)->lockForUpdate()->get(); | ||
| Badge::where('id', $badge->id)->orWhere('extra_copy_of', $badge->id)->lockForUpdate()->get(); |
There was a problem hiding this comment.
This query locks and retrieves all badge records but doesn't use the result. Consider using lockForUpdate() without get() if you only need to acquire locks, or store the result if it's needed.
| Badge::where('id', $badge->id)->orWhere('extra_copy_of', $badge->id)->lockForUpdate()->get(); | |
| // Acquire row-level locks for the badge and its copies (data not needed) | |
| $lockedBadges = Badge::where('id', $badge->id) | |
| ->orWhere('extra_copy_of', $badge->id) | |
| ->select('id') | |
| ->lockForUpdate() | |
| ->get(); |