-
Notifications
You must be signed in to change notification settings - Fork 270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Blueprints] setSiteLanguage step – download the latest RC translations for Nightly and Beta builds of WordPress #1987
Conversation
/** | ||
* Returns a WordPress build version string known to Playground | ||
* for a given WordPress version string. | ||
* | ||
* Each released version will be converted to the major.minor format. | ||
* For example 6.6.1 will be converted to 6.6. | ||
* | ||
* Release candidates (RC) and beta releases are converted to the "beta" | ||
* | ||
* Nightly releases are converted to "nightly". | ||
* | ||
* @param wpVersionString - A WordPress version string. | ||
* @returns A Playground WordPress build version. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I documented this function as it wasn't fully clear to me what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely, thank you! Let's explain what's "known to Playground" in "build version string known to Playground". Also, the downloads.wordpress.org aspect of this function is relevant – the returned identifier can be interpolated into a URL to download a WordPress release – let's cover that as well similarly to how the docstring inside getWordPressTranslationUrl
explains it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a change to explain "build version string known to Playground".
Also, the downloads.wordpress.org aspect of this function is relevant – the returned identifier can be interpolated into a URL to download a WordPress release – let's cover that as well similarly to how the docstring inside getWordPressTranslationUrl explains it.
I'm not sure I understand what you would like to document here.
If we are talking about WordPress releases from wordpress.org.
This function output can't be interpolated to a URL to download a WordPress release.
This is exactly why we had to implement getWordPressTranslationUrl
.
WordPress ZIP download URLs are even more different, for example:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I documented this function as it wasn't fully clear to me what it does.
That function definitely deserved documentation. I'm sorry for not providing it initially. Thank you for catching and fixing it, @bgrgicak!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related thought: I wonder if there would be value in exposing a WP version parsing lib from WP core.
The scheme may be well-documented someplace, but the way I wrote the initial code was simply by sampling version strings from different revisions of wp-includes/versions.php
at different points in the release cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea!
* for a given WordPress version string. | ||
* | ||
* Playground supports the last 4 major.minor versions of WordPress, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect, though. There's nothing in this function that limits the returned version to the last 4 major releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, I wanted to explain what supported means with that sentence, but it's actually not relevant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the mention of support in 0d062ea because it's not relevant.
* For example translations for WordPress 6.6-BETA1 or 6.6-RC1 are found under | ||
* https://downloads.wordpress.org/translation/core/6.6-RC/en_GB.zip | ||
*/ | ||
const wpVersionString = versionStringToLoadedWordPressVersion(wpVersion); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that if wpVersion
is "woah-this-is-cool"
then wpVersionString
will also be "woah-this-is-cool"
. What would be a sensible fallback here? Perhaps a dedicated version conversion logic would make more sense here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8cf6394 will default translations to the latest WP version to ensure we can always have a valid URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this already uses involved patterns, calling versionStringToLoadedWordPressVersion seem to add complexity. What would this look like without that call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would look something like this f1f5842
I like this approach more, relying on versionStringToLoadedWordPressVersion
could lead to issues if the function is changed, especially without tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lovely, looking good. Thank you!
…lations Uses a major WordPress version to fetch the translations, e.g. https://downloads.wordpress.org/translation/core/6.6-RC/en_US.zip instead of https://downloads.wordpress.org/translation/core/6.6.1-RC/en_US.zip This fixes an issue introduced in #1987 that caused the E2E tests to fail on trunk: ``` ✘ 77 [chromium] › blueprints.spec.ts:438:6 › should translate WP-admin to Spanish for the beta WordPress build (retry #3) (30.2s) ``` The setSiteLanguage step would try to download the translations from https://downloads.wordpress.org/translation/core/6.7.1-RC/es_ES.zip which responded with a 404, instead of https://downloads.wordpress.org/translation/core/6.7-RC/es_ES.zip where the translations are actually available. ## Testing instructions * Confirm the E2E tests pass * Go here and confirm you can see a Spanish version of wp admin: http://localhost:5400/website-server/?language=es_ES&url=%2Fwp-admin%2F&wp=beta&modal=error-report
…lations (#2017) Uses a major WordPress version to fetch the translations, e.g. `https://downloads.wordpress.org/translation/core/6.6-RC/en_US.zip` instead of `https://downloads.wordpress.org/translation/core/6.6.1-RC/en_US.zip` This fixes an issue introduced in #1987 that caused the E2E tests to fail on trunk: ``` ✘ 77 [chromium] › blueprints.spec.ts:438:6 › should translate WP-admin to Spanish for the beta WordPress build (retry #3) (30.2s) ``` The setSiteLanguage step would try to download the translations from https://downloads.wordpress.org/translation/core/6.7.1-RC/es_ES.zip which responded with a 404, instead of https://downloads.wordpress.org/translation/core/6.7-RC/es_ES.zip where the translations are actually available. ## Testing instructions * Confirm the E2E tests pass * Go here and confirm you can see a Spanish version of wp admin: http://localhost:5400/website-server/?language=es_ES&url=%2Fwp-admin%2F&wp=beta&modal=error-report
This PR created a dependency between the Blueprints package and the WordPress builds package. @bgrgicak let's pull the WordPress version details from api.wordpress.org/core/version-check/1.7?channel=beta instead of using the version of the latest minified web build the web as a proxy for the latest WordPress version. |
The resolveWPRelease CLI function might be ripe for reuse in the Blueprints library. That's one less Playground CLI-specific code path. |
… of assuming it's the same as the last minified build (#2027) #1987 created a dependency between the `@wp-playground/blueprints` package and the `@wp-playground/wordpress-builds` package breaking `@wp-playground/cli` – see #2026 This PR updates the setSiteLanguage step to pull the latest/best WordPress version details from api.wordpress.org/core/version-check/1.7?channel=beta instead of implicitly assuming the latest version is the same as that of the latest minified web build. It reuses the Playground CLI resolveWordPressRelease function, bringing us closer to having zero CLI-specific logic. ## Follow-up work Invent an ESLint rule to prevent further dependencies on `@wp-playground/wordpress-builds` ## Testing instructions * CI tests * Run Playground CLI via `bun packages/playground/cli/src/cli.ts server` and confirm WordPress is still being downloaded without errors. cc @swissspidy @bgrgicak
This PR enables using
setSiteLanguage
with beta and nightly WordPress versions:Before this PR, the above Blueprint would fetch translations from
https://downloads.wordpress.org/translation/core/6.7-RC2/es_ES.zip
and fail after a 404 API response from the WordPress.org translations API. The6.7-RC2
version string in the URL came directly from WordPress's version.php.The WordPress translations API only offers translations for full, official WordPress releases (e.g.
6.6
,6.6.1
)plus RC translations for each major WordPress version (e.g.
6.6-RC
,6.7-RC
). Note that beta and RC versions use the same translations bundle. Also, there is no translation bundle for the development versions so the best we can do is download the latest RC translations.This PR transforms the raw version string, such as
6.7-RC2
or6.8-alpha-59341
, into one recognized by the API, such as6.7-RC
or6.8-RC
. As a result, the above Blueprint now fetches the translations fromhttps://downloads.wordpress.org/translation/core/6.7-RC/es_ES.zip
.Testing Instructions (or ideally a Blueprint)
Manual testing instructions