Skip to content

Commit

Permalink
Refactoring the web translations (#1777)
Browse files Browse the repository at this point in the history
## Problem

- The web translations still use the original Cockpit mechanism for
creating and loading the translations
- That uses a server based logic which has quite some drawbacks
- We have to reimplement the very same logic in a Webpack plugin to make
it work also in the local development server (`npm run server`). And
unfortunately we cannot share the code because it is Rust vs.
Javascript...
- This will not work in the Agama online demo site with completely
static files (without any server side logic, everything needs to be on
the client side).
- For me the original logic was quite strange, why the server should
decide which translations should the client use? It should be completely
handled on the client side, there is no reason for the special server
logic. The server should just report the available translations and let
the client to pick and use the right one.
- The old implementation has also some ugly parts like requiring a
global Javascript variable...

## Solution

- Move the translation logic from the server to the client (browser)
- Use dynamic imports instead of HTTP redirection

## Implementation details

- Removed the `po.js` handler from the Agama HTTP server
- Removed the `po.js` handler from the Webpack development server
- Removed the original Gettext PO files from Git, replaced by already
converted JS files
- Removed the Cockpit Webpack plugin for converting PO files to JS,
replaced by a simple script with similar functionality. (As a bonus the
code was fixed to avoid adding unnecessary `null` values into the
output.)
- Added dynamic import for downloading the requested translations
- Some code cleanup (removing the global JS object)
- Moved the `jed` and `gettext-parser` NPM dependencies to the PO->JS
converter, that means those NPM packages are installed only when
converting the PO files to JS (basically only in the GitHub action which
merges them), they are not installed when building the Agama web
frontend

## Testing

- Tested manually, loading the translations works fine 
- I manually triggered the GitHub Action which merges the translations
from Weblate. It works fine, the test run opened this [pull
request](#1780) with updated
translations (there are more changes as it was executed in the same
branch as this pull request).
  • Loading branch information
lslezak authored Nov 28, 2024
1 parent e53ae19 commit f83484b
Show file tree
Hide file tree
Showing 50 changed files with 21,001 additions and 57,754 deletions.
58 changes: 32 additions & 26 deletions .github/workflows/weblate-merge-po.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
zypper --non-interactive ref
- name: Install tools
run: zypper --non-interactive install --no-recommends git gettext-tools python3-langtable
run: zypper --non-interactive install --no-recommends nodejs npm git gettext-tools python3-langtable

- name: Configure Git
run: |
Expand All @@ -53,61 +53,67 @@ jobs:
path: agama-weblate
repository: ${{ github.repository_owner }}/agama-weblate

- name: Update PO files
- name: Install NPM packages
working-directory: ./agama/web/share/po
run: npm ci

- name: Validate the PO files
working-directory: ./agama-weblate
run: ls web/*.po | xargs -n1 msgfmt --check-format -o /dev/null

- name: Update JS translations
working-directory: ./agama
run: |
mkdir -p web/po
mkdir -p web/src/po
# delete the current translations
find web/po -name '*.po' -exec git rm '{}' ';'
find web/src/po -name '*.js' -exec git rm '{}' ';'
# copy the new ones
mkdir -p web/po
cp -a ../agama-weblate/web/*.po web/po
git add web/po/*.po
# update the list of supported languages, it is used by the PO->JS converter in the next step
web/share/update-languages.py --po-directory ../agama-weblate/web > web/src/languages.json
- name: Validate the PO files
working-directory: ./agama
run: ls web/po/*.po | xargs -n1 msgfmt --check-format -o /dev/null
# rebuild the JS files
(cd web/src/po && SRCDIR=../../../../agama-weblate/web ../../share/po/po-converter.js)
# stage the changes
git add web/src/po/*.js web/src/languages.json
# any changes besides the timestamps in the PO files?
# any changes detected?
- name: Check changes
id: check_changes
working-directory: ./agama
run: |
git diff --staged --ignore-matching-lines="POT-Creation-Date:" \
--ignore-matching-lines="PO-Revision-Date:" web/po > po.diff
git diff --staged web | tee tr.diff
if [ -s po.diff ]; then
echo "PO files updated"
if [ -s tr.diff ]; then
echo "Translations updated"
# this is an Output Parameter
# https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-an-output-parameter
echo "po_updated=true" >> $GITHUB_OUTPUT
echo "updated=true" >> $GITHUB_OUTPUT
else
echo "PO files unchanged"
echo "po_updated=false" >> $GITHUB_OUTPUT
echo "Translations not changed"
echo "updated=false" >> $GITHUB_OUTPUT
fi
rm po.diff
rm tr.diff
- name: Push updated PO files
# run only when a PO file has been updated
if: steps.check_changes.outputs.po_updated == 'true'
if: steps.check_changes.outputs.updated == 'true'
working-directory: ./agama
run: |
web/share/update-languages.py > web/src/languages.json
# use a unique branch to avoid possible conflicts with already existing branches
# use an unique branch to avoid possible conflicts with already existing branches
git checkout -b "po_merge_${GITHUB_RUN_ID}"
git commit -a -m "Update web PO files"$'\n\n'"Agama-weblate commit: `git -C ../agama-weblate rev-parse HEAD`"
git commit -a -m "Update web translation files"$'\n\n'"Agama-weblate commit: `git -C ../agama-weblate rev-parse HEAD`"
git push origin "po_merge_${GITHUB_RUN_ID}"
- name: Create pull request
# run only when a PO file has been updated
if: steps.check_changes.outputs.po_updated == 'true'
if: steps.check_changes.outputs.updated == 'true'
working-directory: ./agama
run: |
gh pr create -B master -H "po_merge_${GITHUB_RUN_ID}" \
--label translations --label bot \
--title "Update web PO files" \
--title "Update web translation files" \
--body "Updating the web translation files from the agama-weblate repository"
env:
GH_TOKEN: ${{ github.token }}
60 changes: 0 additions & 60 deletions rust/agama-server/src/web/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,11 @@
use super::{auth::AuthError, state::ServiceState};
use agama_lib::auth::{AuthToken, TokenClaims};
use axum::{
body::Body,
extract::{Query, State},
http::{header, HeaderMap, HeaderValue, StatusCode},
response::IntoResponse,
Json,
};
use axum_extra::extract::cookie::CookieJar;
use pam::Client;
use serde::{Deserialize, Serialize};
use utoipa::ToSchema;
Expand Down Expand Up @@ -160,61 +158,3 @@ pub async fn session(_claims: TokenClaims) -> Result<(), AuthError> {
fn auth_cookie_from_token(token: &AuthToken) -> String {
format!("agamaToken={}; HttpOnly", &token.to_string())
}

// builds a response tuple for translation redirection
fn redirect_to_file(file: &str) -> (StatusCode, HeaderMap, Body) {
tracing::info!("Redirecting to translation file {}", file);

let mut response_headers = HeaderMap::new();
// translation found, redirect to the real file
response_headers.insert(
header::LOCATION,
// if the file exists then the name is a valid value and unwrapping is safe
HeaderValue::from_str(file).unwrap(),
);

(
StatusCode::TEMPORARY_REDIRECT,
response_headers,
Body::empty(),
)
}

// handle the /po.js request
// the requested language (locale) is sent in the "agamaLang" HTTP cookie
// this reimplements the Cockpit translation support
pub async fn po(State(state): State<ServiceState>, jar: CookieJar) -> impl IntoResponse {
if let Some(cookie) = jar.get("agamaLang") {
tracing::info!("Language cookie: {}", cookie.value());
// try parsing the cookie
if let Some((lang, region)) = cookie.value().split_once('-') {
// first try language + country
let target_file = format!("po.{}_{}.js", lang, region.to_uppercase());
if state.public_dir.join(&target_file).exists() {
return redirect_to_file(&target_file);
} else {
// then try the language only
let target_file = format!("po.{}.js", lang);
if state.public_dir.join(&target_file).exists() {
return redirect_to_file(&target_file);
};
}
} else {
// use the cookie as is
let target_file = format!("po.{}.js", cookie.value());
if state.public_dir.join(&target_file).exists() {
return redirect_to_file(&target_file);
}
}
}

tracing::info!("Translation not found");
// fallback, return empty javascript translations if the language is not supported
let mut response_headers = HeaderMap::new();
response_headers.insert(
header::CONTENT_TYPE,
HeaderValue::from_static("text/javascript"),
);

(StatusCode::OK, response_headers, Body::empty())
}
1 change: 0 additions & 1 deletion rust/agama-server/src/web/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ impl MainServiceBuilder {
Router::new()
.nest_service("/", serve)
.route("/login", get(login_from_query))
.route("/po.js", get(super::http::po))
.nest("/api", api_router)
.layer(
TraceLayer::new_for_http()
Expand Down
7 changes: 7 additions & 0 deletions rust/package/agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Thu Nov 28 15:58:59 UTC 2024 - Ladislav Slezák <[email protected]>

- Drop the handler for the "po.js" path in the HTTP server,
the web frontend now uses dynamic imports for loading the
translation files (gh#agama-project/agama#1777)

-------------------------------------------------------------------
Thu Nov 28 11:07:58 UTC 2024 - Imobach Gonzalez Sosa <[email protected]>

Expand Down
1 change: 0 additions & 1 deletion web/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ coverage/
dist/
/*.spec
/.vagrant
package-lock.json
Test*FAIL*
/bots
test/common/
Expand Down
7 changes: 7 additions & 0 deletions web/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ export default [
files: ["src/i18n.test.js"],
rules: { "agama-i18n/string-literals": "off" },
},
{
// the translation JS files generated from the PO files use some code in the plural form rule,
// ignore the possible "problems" there (using "==" operator instead of more strict "===" or not
// using the "n" variable in languages which do not have plural form)
files: ["src/po/*.js"],
rules: { eqeqeq: "off", "@typescript-eslint/no-unused-vars": "off" },
},
{
ignores: ["node_modules/*", "src/lib/*", "src/**/test-data/*"],
},
Expand Down
Loading

0 comments on commit f83484b

Please sign in to comment.