fix(devops): key locale list by position instead of scraped priority - #760
fix(devops): key locale list by position instead of scraped priority#760Max Stegmeyer (mstegmeyer) wants to merge 1 commit into
Conversation
The locale scraper copied PayPal's "Language support priority" column verbatim into the array keys of `SupportedLocales::LOCALES`. PayPal renumbered that column from 0-based to 1-based, so the next scrape turned every region into a 1-based array without changing a single locale code. That silently breaks `LocaleCodeProvider::findMatchingSupportedLocale()`, which falls back to `LOCALES[$regionCode][0]`: for all 202 regions the lookup would miss and every unsupported-but-mappable locale would fall back to `en_GB` instead of the region's preferred locale. The priority is now only used to sort a region's locales, and the key is the resulting position. On top of that the scraper no longer trusts the page layout blindly: - column positions are resolved from the table header, so added, removed or reordered columns cannot shift the scraped values - region codes, locale codes and priorities are validated per row - a minimum region count guards against a truncated scrape All of these now fail the command loudly instead of writing a broken class and opening a pull request for it. `SupportedLocalesTest` pins the invariant on the generated file, so a regression fails CI on the automated locale update pull request.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fd1a82c3d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "Codex (@codex) review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".
| if ($cells->count() <= \max($columns)) { | ||
| return null; |
There was a problem hiding this comment.
If PayPal renders an individual locale row with too few <td> cells, this silently filters that row out; with 202 current regions, losing a few regions or locale variants still leaves the result above MINIMUM_REGIONS, so the weekly command can successfully rewrite SupportedLocales.php with incomplete data. A three-cell row previously reached the missing eq(3)->text() and failed rather than producing a truncated list, so throw for malformed data rows instead of returning null.
Useful? React with 👍 / 👎.
1 participant
Problem
#757 rewrote all 648 lines of
SupportedLocales.phpwithout changing a single locale code. All 202 regions, their order and their locale lists are byte-for-byte identical — only the array keys shifted from 0-based to 1-based:The scraper copied PayPal's "Language support priority" column verbatim into the array keys, and PayPal renumbered that column from 0-based to 1-based.
That is not cosmetic.
LocaleCodeProvider::findMatchingSupportedLocale()falls back toSupportedLocales::LOCALES[$regionCode][0], so with 1-based keys the lookup misses for all 202 regions:en_ZAen_USen_GB(default)de_ATde_DEen_GB(default)en_INen_INen_GB(default)fr_CAen_USen_GB(default)Every locale that PayPal does not support directly but that is mappable via its region would have silently fallen back to
en_GBinstead of the region's preferred locale.Fix
The scraped priority is now only used to sort a region's locales; the array key is the resulting position. The generated output no longer depends on how PayPal happens to number that column.
While in there, the scraper no longer trusts the page layout blindly — it previously read fixed column offsets (
eq(1),eq(2),eq(3)) and accepted whatever text it found:Each of these now fails
swag:paypal:scrape:localesloudly, so the weekly job cannot write a broken class and open a pull request for it.SupportedLocalesTestpins the invariant on the generated file, so a regression fails CI on the automated locale update PR rather than needing to be spotted by eye.Verification
SupportedLocales.phpis byte-for-byte identical to trunk, confirming chore: Update PayPal locales #757 carried no actual locale change.SupportedLocales.phpto chore: Update PayPal locales #757's version fails both the newSupportedLocalesTest::testRegionHasPrioritizedLocaleCodeList(Locale codes of region "AL" are not a 0-based list) and the pre-existingLocaleCodeProviderTest::testGetFormattedLocaleCodeWithFallbackLocale.phpunit tests/Util/SupportedLocalesTest.php tests/Util/LocaleCodeProviderTest.php→ 207 tests, 2175 assertions, green.phpstanclean on the touched files;php-cs-fixerreports no fixable files.Note on #757's CI
#757 was red, but on unrelated infrastructure flakiness (composer
HTTP/2 504while downloading dependencies), not on this bug — so the failure that mattered was never actually reported. The pre-existingLocaleCodeProviderTestwould have caught it had the suite run.