Fix: change request data for applying new method#77
Fix: change request data for applying new method#77zigzagdev merged 2 commits intofeature/central-asiafrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates test data across multiple World Heritage site test files to change from Japanese heritage sites to European/Asian transnational heritage sites, likely to support testing of a new method that handles multi-country heritage sites with state party relationships.
Key changes:
- Updated test fixture data from Japanese heritage sites to transnational sites (Ancient Beech Forests and Silk Roads)
- Added support for
state_partiesandstate_parties_metafields in DTOs and factories - Enhanced test setup with Country seeder and additional database table cleanup
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| WorldHeritageDtoCollectionTest.php | Updated test data and added complex assertion logic for state party handling |
| CreateWorldHeritagesUseCaseTest.php | Updated test fixtures and enhanced database cleanup |
| CreateWorldHeritageUseCaseTest.php | Removed hardcoded ID from test data array |
| WorldHeritageDtoCollectionFactory.php | Added state party fields to DTO construction |
| WorldHeritageDtoCollection.php | Added state party fields to array output |
| WorldHeritageDto.php | Fixed field name from 'state_parties' to 'state_party_codes' |
| WorldHeritageEntityCollectionTest.php | Updated test data and removed comments |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| $actual = collect($dtoCollection->toArray())->map(function($item) { | ||
| if (is_array($item['statePartyCodes'])) { | ||
| $item['state_parties'] = collect($item['statePartyCodes'])->map(function($code) { | ||
| return Str::upper($code); | ||
| })->toArray(); | ||
| } | ||
| return collect($item)->keyBy(function($value, $key) { | ||
| return Str::snake($key); | ||
| })->toArray(); | ||
| })->toArray(); | ||
|
|
||
| $expected = collect($data)->map(function($item) { | ||
| if (is_array($item['state_parties'])) { |
There was a problem hiding this comment.
The test assertion logic is overly complex with nested transformations and conditionals. Consider extracting this transformation logic into a separate helper method to improve readability and maintainability.
| $actual = collect($dtoCollection->toArray())->map(function($item) { | |
| if (is_array($item['statePartyCodes'])) { | |
| $item['state_parties'] = collect($item['statePartyCodes'])->map(function($code) { | |
| return Str::upper($code); | |
| })->toArray(); | |
| } | |
| return collect($item)->keyBy(function($value, $key) { | |
| return Str::snake($key); | |
| })->toArray(); | |
| })->toArray(); | |
| $expected = collect($data)->map(function($item) { | |
| if (is_array($item['state_parties'])) { | |
| $actual = $this->transformCollectionForComparison($dtoCollection->toArray(), 'dto'); | |
| $expected = $this->transformCollectionForComparison($data, 'array'); | |
| $this->assertEquals($expected, $actual); | |
| } | |
| /** | |
| * Helper to transform collection for comparison in tests. | |
| * | |
| * @param array $collection | |
| * @param string $mode 'dto' or 'array' | |
| * @return array | |
| */ | |
| private function transformCollectionForComparison(array $collection, string $mode): array | |
| { | |
| return collect($collection)->map(function($item) use ($mode) { | |
| if ($mode === 'dto' && isset($item['statePartyCodes']) && is_array($item['statePartyCodes'])) { | |
| $item['state_parties'] = collect($item['statePartyCodes'])->map(function($code) { | |
| return Str::upper($code); | |
| })->toArray(); | |
| } | |
| if ($mode === 'array' && isset($item['state_parties']) && is_array($item['state_parties'])) { |
| 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.
Database connection is hardcoded to 'mysql'. Consider using the default connection or making it configurable to support different database types in testing environments.
| 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;'); |
| 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.
Database connection is hardcoded to 'mysql'. Consider using the default connection or making it configurable to support different database types in testing environments.
| 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;'); |
No description provided.