Skip to content

Commit

Permalink
Do not use Cockpit manifest for storing supported languages (#1121)
Browse files Browse the repository at this point in the history
## Problem

- Continue with dropping the Cockpit dependency

## Solution

- Do not use the Cockpit manifest for storing the list of supported
languages
- Use a separate `languages.json` file instead of the `manifest.json`
- Import the JSON file directly into the JS code
- The workflow is not changed, the update script just generates a
different file
- The change is backward compatible, works with both Cockpit and the new
server

## Testing

- Updated unit tests
- Tested manually, the language selection still works fine
  • Loading branch information
lslezak committed Mar 27, 2024
1 parent f143990 commit 8cf36f0
Show file tree
Hide file tree
Showing 13 changed files with 66 additions and 130 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/weblate-merge-po.yml
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ jobs:
if: steps.check_changes.outputs.po_updated == 'true'
working-directory: ./agama
run: |
web/share/update-manifest.py web/src/manifest.json
web/share/update-languages.py > web/src/languages.json
# use a 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`"
Expand Down
1 change: 1 addition & 0 deletions web/cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"language": "en",
"allowCompoundWords": false,
"ignorePaths": [
"src/languages.json",
"src/lib/cockpit.js",
"src/lib/cockpit-po-plugin.js",
"src/manifest.json",
Expand Down
9 changes: 9 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
-------------------------------------------------------------------
Wed Mar 27 12:41:11 UTC 2024 - Ladislav Slezák <lslezak@suse.com>

- Dropping Cockpit dependency:
- Do not use Cockpit gettext functionality
(gh#openSUSE/agama#1118)
- Do not store the list of supported languages to the Cockpit
manifest file (gh#openSUSE/agama#1121)

-------------------------------------------------------------------
Tue Mar 19 14:15:30 UTC 2024 - José Iván López González <jlopez@suse.com>

Expand Down
55 changes: 25 additions & 30 deletions web/share/update-manifest.py → web/share/update-languages.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@
# To contact SUSE LLC about this file by physical or electronic mail, you may
# find current contact information at www.suse.com.

#
# This script generates the list of supported languages in JSON format.
#

from argparse import ArgumentParser
from langtable import language_name
from pathlib import Path
import json
import re
import subprocess

import sys

class Locale:
language: str
Expand Down Expand Up @@ -76,20 +79,15 @@ def language(self):
return self.path.stem


class Manifest:
""" This class takes care of updating the manifest file"""
class Languages:
""" This class takes care of generating the supported languages file"""

def __init__(self, path: Path):
self.path = path
self.__read__()

def __read__(self):
with open(self.path) as file:
self.content = json.load(file)
def __init__(self):
self.content = dict()

def update(self, po_files, lang2territory: str, threshold: int):
"""
Updates the list of locales in the manifest file
Generate the list of supported locales
It does not write the changes to file system. Use the write() function
for that.
Expand All @@ -111,46 +109,43 @@ def update(self, po_files, lang2territory: str, threshold: int):
if locale.territory is None:
print(
"could not find a territory for '{language}'"
.format(language=locale.language)
.format(language=locale.language),
file=sys.stderr
)
elif po_file.coverage() < threshold:
print(
"not enough coverage for '{language}' ({coverage}%)"
.format(
language=locale.code(),
coverage=po_file.coverage())
coverage=po_file.coverage()),
file=sys.stderr
)
else:
supported.append(locale)

languages = [loc.language for loc in supported]
self.content["locales"] = dict()
for locale in supported:
include_territory = languages.count(locale.language) > 1
self.content["locales"][locale.code()] = locale.name(
include_territory)
self.content[locale.code()] = locale.name(include_territory)

def write(self):
with open(self.path, "w+") as file:
json.dump(self.content, file, indent=4, ensure_ascii=False)
def dump(self):
json.dump(self.content, sys.stdout, indent=4, ensure_ascii=False,
sort_keys=True)


def update_manifest(args):
"""Command to update the manifest.json file"""
manifest = Manifest(Path(args.manifest))
def update_languages(args):
"""Print the supported languages in JSON format"""
languages = Languages()
paths = [path for path in Path(args.po_directory).glob("*.po")]
with open(args.territories) as file:
lang2territory = json.load(file)
manifest.update(paths, lang2territory, args.threshold)
manifest.write()
languages.update(paths, lang2territory, args.threshold)
languages.dump()


if __name__ == "__main__":
parser = ArgumentParser(prog="locales.py")
parser.set_defaults(func=update_manifest)
parser.add_argument(
"manifest", type=str, help="Path to the manifest file",
)
parser = ArgumentParser(prog="update-languages.py")
parser.set_defaults(func=update_languages)
parser.add_argument(
"--po-directory", type=str, help="Directory containing the po files",
default="web/po"
Expand Down
7 changes: 3 additions & 4 deletions web/src/components/l10n/InstallerLocaleSwitcher.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,20 @@ import { FormSelect, FormSelectOption, Popover } from "@patternfly/react-core";
import { Icon } from "../layout";
import { _ } from "~/i18n";
import { useInstallerL10n } from "~/context/installerL10n";
import cockpit from "~/lib/cockpit";
import supportedLanguages from "~/languages.json";

export default function InstallerLocaleSwitcher() {
const { language, changeLanguage } = useInstallerL10n();
const [selected, setSelected] = useState(null);
const languages = cockpit.manifests.agama?.locales || [];

const onChange = useCallback((_event, value) => {
setSelected(value);
changeLanguage(value);
}, [setSelected, changeLanguage]);

// sort by the language code to maintain consistent order
const options = Object.keys(languages).sort()
.map(id => <FormSelectOption key={id} value={id} label={languages[id]} />);
const options = Object.keys(supportedLanguages).sort()
.map(id => <FormSelectOption key={id} value={id} label={supportedLanguages[id]} />);

// TRANSLATORS: help text for the language selector in the sidebar,
// %s will be replaced by the "Localization" page link
Expand Down
15 changes: 4 additions & 11 deletions web/src/components/l10n/InstallerLocaleSwitcher.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,10 @@ import InstallerLocaleSwitcher from "./InstallerLocaleSwitcher";
const mockLanguage = "es-es";
let mockChangeLanguageFn;

jest.mock("~/lib/cockpit", () => ({
gettext: term => term,
manifests: {
agama: {
locales: {
"de-de": "Deutsch",
"en-us": "English (US)",
"es-es": "Español"
}
}
}
jest.mock("~/languages.json", () => ({
"de-de": "Deutsch",
"en-us": "English (US)",
"es-es": "Español"
}));

jest.mock("~/context/installerL10n", () => ({
Expand Down
3 changes: 2 additions & 1 deletion web/src/context/installerL10n.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { useCancellablePromise, locationReload, setLocationSearch } from "~/util
import cockpit from "../lib/cockpit";
import { useInstallerClient } from "./installer";
import agama from "~/agama";
import supportedLanguages from "~/languages.json";

const L10nContext = React.createContext(null);

Expand Down Expand Up @@ -154,7 +155,7 @@ function navigatorLanguages() {
* @return {string|undefined} Undefined if none of the given languages is supported.
*/
function findSupportedLanguage(languages) {
const supported = Object.keys(cockpit.manifests.agama?.locales || {});
const supported = Object.keys(supportedLanguages);

for (const candidate of languages) {
const [language, country] = candidate.split("-");
Expand Down
18 changes: 7 additions & 11 deletions web/src/context/installerL10n.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,14 @@ const client = {
onDisconnect: jest.fn()
};

jest.mock("~/languages.json", () => ({
"es-ar": "Español (Argentina)",
"cs-cz": "čeština",
"en-us": "English (US)",
"es-es": "Español"
}));

jest.mock("~/lib/cockpit", () => ({
gettext: term => term,
manifests: {
agama: {
locales: {
"es-ar": "Español (Argentina)",
"cs-cz": "čeština",
"en-us": "English (US)",
"es-es": "Español"
}
}
},
spawn: jest.fn().mockResolvedValue()
}));

Expand Down
12 changes: 12 additions & 0 deletions web/src/languages.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"ca-es": "Català",
"de-de": "Deutsch",
"en-us": "English",
"es-es": "Español",
"fr-fr": "Français",
"id-id": "Indonesia",
"ja-jp": "日本語",
"nl-nl": "Nederlands",
"sv-se": "Svenska",
"zh-Hans": "中文"
}
48 changes: 0 additions & 48 deletions web/src/lib/webpack-manifests-handler.js

This file was deleted.

14 changes: 1 addition & 13 deletions web/src/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,5 @@
"index": {
"label": "Agama"
}
},
"locales": {
"en-us": "English",
"es-es": "Español",
"id-id": "Indonesia",
"fr-fr": "Français",
"sv-se": "Svenska",
"ca-es": "Català",
"ja-jp": "日本語",
"nl-nl": "Nederlands",
"zh-Hans": "中文",
"de-de": "Deutsch"
}
}
}
1 change: 1 addition & 0 deletions web/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"noEmit": true,
"target": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"allowJs": true,
"jsx": "react",
"allowSyntheticDefaultImports": true,
Expand Down
11 changes: 0 additions & 11 deletions web/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ const TsconfigPathsPlugin = require('tsconfig-paths-webpack-plugin');
const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin');
const webpack = require('webpack');
const po_handler = require("./src/lib/webpack-po-handler");
const manifests_handler = require("./src/lib/webpack-manifests-handler");

/* A standard nodejs and webpack pattern */
const production = process.env.NODE_ENV === 'production';
Expand Down Expand Up @@ -112,16 +111,6 @@ module.exports = {
// ignore SSL problems (self-signed certificate)
secure: false,
},
// forward the manifests.js request and patch the response with the
// current Agama manifest from the ./src/manifest.json file
"/manifests.js": {
target: cockpitTarget + "/cockpit/@localhost/",
// ignore SSL problems (self-signed certificate)
secure: false,
// the response is modified by the onProxyRes handler
selfHandleResponse : true,
onProxyRes: manifests_handler,
},
},
// use https so Cockpit uses wss:// when connecting to the backend
server: "https",
Expand Down

0 comments on commit 8cf36f0

Please sign in to comment.