From 229cbc48eeb55eca92de2afc4cde1766a586c1d8 Mon Sep 17 00:00:00 2001 From: edwh Date: Wed, 23 Sep 2026 16:05:30 +0100 Subject: [PATCH 1/2] Set group country from country_code on save, and backfill country was only filled in by the hourly groups:country job, and recent groups were found without it. It's kept for backwards compatibility (Metabase reports etc.), so set it (in English) whenever country_code is set, and backfill existing gaps. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/Group.php | 11 ++++++ ...ckfill_group_country_from_country_code.php | 37 +++++++++++++++++++ tests/Feature/Groups/GroupCountryTest.php | 12 ++++++ 3 files changed, 60 insertions(+) create mode 100644 database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php diff --git a/app/Group.php b/app/Group.php index 1a2dbebb32..9efe5ba8de 100644 --- a/app/Group.php +++ b/app/Group.php @@ -148,6 +148,17 @@ public function toArray() // Setters + /** + * Keep the legacy `country` column in step with `country_code`. `country` is still read by external + * consumers (e.g. ORA exports, direct DB reporting) so it must be populated as soon as the group is saved, + * rather than waiting for the hourly groups:country job. Countries are stored in English. + */ + public function setCountryCodeAttribute($value) + { + $this->attributes['country_code'] = $value; + $this->attributes['country'] = $value ? (\App\Helpers\Fixometer::getAllCountries('en')[$value] ?? null) : null; + } + //Getters public function findAll() { diff --git a/database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php b/database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php new file mode 100644 index 0000000000..92c7ce71f6 --- /dev/null +++ b/database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php @@ -0,0 +1,37 @@ +whereNotNull('country_code') + ->where(function ($q) { + $q->whereNull('country')->orWhere('country', ''); + }) + ->get(['idgroups', 'country_code']); + + foreach ($groups as $group) { + if (isset($countries[$group->country_code])) { + DB::table('groups') + ->where('idgroups', $group->idgroups) + ->update(['country' => $countries[$group->country_code]]); + } + } + } + + public function down(): void + { + // Data-only backfill; nothing to undo. + } +}; diff --git a/tests/Feature/Groups/GroupCountryTest.php b/tests/Feature/Groups/GroupCountryTest.php index 1394b6ee6a..5c9cf87991 100644 --- a/tests/Feature/Groups/GroupCountryTest.php +++ b/tests/Feature/Groups/GroupCountryTest.php @@ -29,4 +29,16 @@ public function testSync(): void { $group = Group::find($group->idgroups); $this->assertEquals('United Kingdom', $group->country); } + + public function testCountrySetOnSaveWithoutJob(): void { + // A French-speaking host creating a group must still get the English country name stored. + app()->setLocale('fr'); + + $group = Group::factory()->create(['country_code' => 'BE']); + $this->assertEquals('Belgium', Group::find($group->idgroups)->country); + + $group->country_code = 'GB'; + $group->save(); + $this->assertEquals('United Kingdom', Group::find($group->idgroups)->country); + } } From 98df9dba5c71fade06cd0d230ffc81e682ac4f72 Mon Sep 17 00:00:00 2001 From: edwh Date: Fri, 25 Sep 2026 16:30:30 +0100 Subject: [PATCH 2/2] Group country: one lookup for model, job and backfill; handle UK and unknown codes - Group::countryNameForCode() is used by the mutator, groups:country and the backfill migration, so they agree: '' for an empty or unknown code (the job's existing behaviour) instead of null from the mutator, which made the column flip on every save/job run. - 'UK' (used by group CSV imports) maps to GB. - The backfill logs codes it can't map. - Locale test uses fr-BE, which has its own country names; fr fell back to English, so it couldn't catch a regression. Co-Authored-By: Claude Opus 5.5 (1M context) --- app/Console/Commands/GroupCountryField.php | 3 +-- app/Group.php | 17 ++++++++++++++++- ...backfill_group_country_from_country_code.php | 15 ++++++++------- tests/Feature/Groups/GroupCountryTest.php | 16 ++++++++++++++-- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/app/Console/Commands/GroupCountryField.php b/app/Console/Commands/GroupCountryField.php index 97a044518d..7d0cffc931 100644 --- a/app/Console/Commands/GroupCountryField.php +++ b/app/Console/Commands/GroupCountryField.php @@ -3,7 +3,6 @@ namespace App\Console\Commands; use App\Group; -use App\Helpers\Fixometer; use Illuminate\Console\Command; class GroupCountryField extends Command @@ -30,7 +29,7 @@ public function handle(): void $groups = Group::all(); foreach ($groups as $group) { - $group->country = Fixometer::getCountryFromCountryCode($group->country_code); + $group->country = Group::countryNameForCode($group->country_code); $group->save(); } } diff --git a/app/Group.php b/app/Group.php index 9efe5ba8de..097de77cf5 100644 --- a/app/Group.php +++ b/app/Group.php @@ -156,7 +156,22 @@ public function toArray() public function setCountryCodeAttribute($value) { $this->attributes['country_code'] = $value; - $this->attributes['country'] = $value ? (\App\Helpers\Fixometer::getAllCountries('en')[$value] ?? null) : null; + $this->attributes['country'] = self::countryNameForCode($value); + } + + /** + * The English country name stored in the legacy `country` column; '' for an empty or unknown code. Group + * imports have used 'UK' for the United Kingdom, which isn't an ISO code. + */ + public static function countryNameForCode($code): string + { + if (! $code) { + return ''; + } + + $code = strtoupper($code) === 'UK' ? 'GB' : $code; + + return \App\Helpers\Fixometer::getAllCountries('en')[$code] ?? ''; } //Getters diff --git a/database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php b/database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php index 92c7ce71f6..10be9b9108 100644 --- a/database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php +++ b/database/migrations/2026_09_23_000000_backfill_group_country_from_country_code.php @@ -1,8 +1,9 @@ whereNotNull('country_code') ->where(function ($q) { @@ -22,10 +21,12 @@ public function up(): void ->get(['idgroups', 'country_code']); foreach ($groups as $group) { - if (isset($countries[$group->country_code])) { - DB::table('groups') - ->where('idgroups', $group->idgroups) - ->update(['country' => $countries[$group->country_code]]); + $country = Group::countryNameForCode($group->country_code); + + if ($country !== '') { + DB::table('groups')->where('idgroups', $group->idgroups)->update(['country' => $country]); + } else { + Log::warning("Group {$group->idgroups}: no country name for code '{$group->country_code}'"); } } } diff --git a/tests/Feature/Groups/GroupCountryTest.php b/tests/Feature/Groups/GroupCountryTest.php index 5c9cf87991..f607aa994b 100644 --- a/tests/Feature/Groups/GroupCountryTest.php +++ b/tests/Feature/Groups/GroupCountryTest.php @@ -31,8 +31,8 @@ public function testSync(): void { } public function testCountrySetOnSaveWithoutJob(): void { - // A French-speaking host creating a group must still get the English country name stored. - app()->setLocale('fr'); + // fr-BE has its own country names (e.g. 'Belgique'); the stored value must still be English. + app()->setLocale('fr-BE'); $group = Group::factory()->create(['country_code' => 'BE']); $this->assertEquals('Belgium', Group::find($group->idgroups)->country); @@ -41,4 +41,16 @@ public function testCountrySetOnSaveWithoutJob(): void { $group->save(); $this->assertEquals('United Kingdom', Group::find($group->idgroups)->country); } + + public function testUnknownAndLegacyCodes(): void { + $this->assertEquals('United Kingdom', Group::countryNameForCode('UK')); + $this->assertEquals('', Group::countryNameForCode('ZZ')); + $this->assertEquals('', Group::countryNameForCode(null)); + + // The model and the hourly job agree, so the column doesn't flip between values. + $group = Group::factory()->create(['country_code' => 'ZZ']); + $this->assertEquals('', Group::find($group->idgroups)->country); + $this->artisan('groups:country'); + $this->assertEquals('', Group::find($group->idgroups)->country); + } }