Fix Presentation Layer Tests#99
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes presentation layer tests for the WorldHeritageController by updating test data structures and adding database management functionality. The main purpose is to align the test data with changes to the WorldHeritage data model that now includes state party information and removes UNESCO ID references.
- Updated test data from Japanese heritage sites to European/Asian transnational sites with state party metadata
- Added database seeding and cleanup functionality with proper foreign key constraint handling
- Removed UNESCO ID field references and added state party fields to match updated data model
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| WorldHeritageController_getByIdsTest.php | Updated test data structure, added database refresh logic, and modified assertions to match new data model |
| WorldHeritageController_getByIdTest.php | Updated test data, added database cleanup methods, and updated mock DTO to handle new state party fields |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| private function refresh(): void | ||
| { | ||
| if (env('APP_ENV') === 'testing') { |
There was a problem hiding this comment.
Using env() directly in application code is discouraged. Use config('app.env') instead, as env() should only be used in configuration files.
| if (env('APP_ENV') === 'testing') { | |
| if (config('app.env') === 'testing') { |
|
|
||
| private function refresh(): void | ||
| { | ||
| if (env('APP_ENV') === 'testing') { |
There was a problem hiding this comment.
Using env() directly in application code is discouraged. Use config('app.env') instead, as env() should only be used in configuration files.
| if (env('APP_ENV') === 'testing') { | |
| if (config('app.env') === 'testing') { |
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=0;'); | ||
| WorldHeritage::truncate(); | ||
| Country::truncate(); | ||
| DB::table('site_state_parties')->truncate(); | ||
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=1;'); |
There was a problem hiding this comment.
Hard-coding the MySQL connection makes the tests less portable. Consider using the default connection or making it configurable for different database drivers.
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=0;'); | |
| WorldHeritage::truncate(); | |
| Country::truncate(); | |
| DB::table('site_state_parties')->truncate(); | |
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=1;'); | |
| $connection = DB::connection(); | |
| $driver = $connection->getDriverName(); | |
| if ($driver === 'mysql') { | |
| $connection->statement('SET FOREIGN_KEY_CHECKS=0;'); | |
| } | |
| WorldHeritage::truncate(); | |
| Country::truncate(); | |
| DB::table('site_state_parties')->truncate(); | |
| if ($driver === 'mysql') { | |
| $connection->statement('SET FOREIGN_KEY_CHECKS=1;'); | |
| } |
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=0;'); | ||
| WorldHeritage::truncate(); | ||
| Country::truncate(); | ||
| DB::table('site_state_parties')->truncate(); | ||
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=1;'); |
There was a problem hiding this comment.
Hard-coding the MySQL connection makes the tests less portable. Consider using the default connection or making it configurable for different database drivers.
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=0;'); | |
| WorldHeritage::truncate(); | |
| Country::truncate(); | |
| DB::table('site_state_parties')->truncate(); | |
| DB::connection('mysql')->statement('SET FOREIGN_KEY_CHECKS=1;'); | |
| DB::statement('SET FOREIGN_KEY_CHECKS=0;'); | |
| WorldHeritage::truncate(); | |
| Country::truncate(); | |
| DB::table('site_state_parties')->truncate(); | |
| DB::statement('SET FOREIGN_KEY_CHECKS=1;'); |
what i have done