Fix DB design, Entity, Repository#68
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements multi-country support for World Heritage sites by adding a pivot table relationship and updating the entity structure. The changes enable storing and retrieving transnational World Heritage properties that span multiple countries.
Key changes:
- Added a Country model with seeder containing all UNESCO state party codes and regional data
- Implemented a many-to-many relationship between World Heritage sites and countries through a pivot table
- Extended the WorldHeritageEntity to handle multiple state party codes with metadata (primary country, inscription years)
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| CountrySeeder.php | New seeder containing complete UNESCO state party data with country codes, names, and regional classifications |
| 2025_08_18_080644_site_state_parties.php | Migration fix changing state_party_code column size from 2 to 255 characters |
| 2025_08_18_080451_countries.php | Added proper down migration to drop countries table |
| 2025_07_28_195929_create_world_heritage_sites.php | Increased state_party column size to accommodate multiple country codes |
| WorldHeritageRepositoryInterface.php | Added commented update method signature for future implementation |
| WorldHeritageRepository.php | Major refactor to handle many-to-many country relationships with pivot data sync |
| WorldHeritageEntity.php | Extended entity with state party codes array, metadata, and transnational support methods |
| WorldHeritageEntityTest.php | Updated tests to cover both single and multi-country heritage site scenarios |
| WorldHeritageRepository_insertTest.php | Enhanced repository tests with country seeder setup and multi-country test cases |
| Country.php | Added timestamps = false to country model |
Comments suppressed due to low confidence (1)
src/app/Packages/Domains/WorldHeritageRepository.php:1
- Parameter order is incorrect. According to the entity constructor,
is_endangeredshould come afterlatitudeandlongitude, not before them.
<?php
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| self::arrayMultiData()['is_endangered'], | ||
| self::arrayMultiData()['latitude'], | ||
| self::arrayMultiData()['longitude'], |
There was a problem hiding this comment.
Parameter order is incorrect. According to the WorldHeritageEntity constructor, is_endangered should come after latitude and longitude, not before them.
| self::arrayMultiData()['is_endangered'], | |
| self::arrayMultiData()['latitude'], | |
| self::arrayMultiData()['longitude'], | |
| self::arrayMultiData()['latitude'], | |
| self::arrayMultiData()['longitude'], | |
| self::arrayMultiData()['is_endangered'], |
| self::arrayMultiData()['is_endangered'], | ||
| self::arrayMultiData()['latitude'], | ||
| self::arrayMultiData()['longitude'], |
There was a problem hiding this comment.
Parameter order is incorrect. According to the WorldHeritageEntity constructor, is_endangered should come after latitude and longitude, not before them.
| self::arrayMultiData()['is_endangered'], | |
| self::arrayMultiData()['latitude'], | |
| self::arrayMultiData()['longitude'], | |
| self::arrayMultiData()['latitude'], | |
| self::arrayMultiData()['longitude'], | |
| self::arrayMultiData()['is_endangered'], |
what i changed