Chore/delete redundant command#382
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes several legacy console commands and shifts WorldHeritageQueryService to source the externally exposed region value from study_region (with seed/test updates to match).
Changes:
- Add
study_regionvalues to seededworld_heritage_sitesrows. - Update query service payloads to return
regionfromstudy_region(and trim thumbnail select columns). - Delete multiple now-redundant console commands related to splitting/import/backfilling UNESCO data.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/database/seeders/WorldHeritageSeeder.php | Populate study_region for seeded sites. |
| src/app/Packages/Domains/WorldHeritageQueryService.php | Use study_region as the source for returned region; adjust selected columns. |
| src/app/Packages/Domains/Test/QueryService/WorldHeritageQueryService_getByIdTest.php | Update expectations to align with study_region-backed region behavior. |
| src/app/Console/Commands/SplitWorldHeritageImageJson.php | Deleted redundant command. |
| src/app/Console/Commands/SplitCountryJson.php | Deleted redundant command. |
| src/app/Console/Commands/ImportWorldHeritageFromJson.php | Deleted redundant command. |
| src/app/Console/Commands/BuildWorldHeritageLocalDb.php | Deleted redundant command. |
| src/app/Console/Commands/BackfillThumbnailImageId.php | Deleted redundant command. |
| src/app/Console/Commands/BackfillStateParties.php | Deleted redundant command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $thumbnailQuery->select([ | ||
| 'images.id', | ||
| 'images.world_heritage_id', | ||
| 'disk', | ||
| 'path', | ||
| 'width', | ||
| 'height', | ||
| 'format', | ||
| 'checksum', | ||
| 'sort_order', | ||
| 'alt', | ||
| 'credit', | ||
| ]); |
There was a problem hiding this comment.
In the eager-loaded thumbnail relation select, the column names/table prefix appear to be for the old images table (images.id, images.world_heritage_id, path). The thumbnail() relation uses App\Models\Image which maps to world_heritage_site_images and its columns are world_heritage_site_id and url (no world_heritage_id/path). This will cause SQL errors when eager-loading the relation. Update the select to use the correct table/columns (or remove the select override entirely).
| @@ -199,6 +199,7 @@ public function getHeritagesByIds(array $ids, int $currentPage, int $perPage): P | |||
| 'name_jp', | |||
| 'country', | |||
| 'region', | |||
There was a problem hiding this comment.
getHeritagesByIds() selects both region and study_region, but the payload returned from the paginator uses only $heritage->study_region for the region field. Consider dropping region from the select list (or aliasing study_region as region) to avoid fetching an unused column and reduce confusion.
| 'region', |
| 'country' => $displayCountry, | ||
| 'country_name_jp' => $countryNameJp, | ||
| 'region' => $heritage->region, | ||
| 'region' => $heritage->study_region, |
There was a problem hiding this comment.
The PR title indicates only deleting a redundant command, but this change also alters the meaning of the API's region field to come from study_region (and updates seed/test data accordingly). Please update the PR title/description to reflect the behavior change, or split into separate PRs so reviewers/users can track the semantic change independently.
No description provided.