Re: [pbs-devel] [PATCH proxmox-backup] close #3103: default lang

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Looks as it'd work out in general, which is nice, a few style and other comments still inline
though.

The commit subject is not really telling, maybe use something like:

"fix #3103: node config: allow to configure default language for ui"

(don't care much for close vs. fix, latter is just shorter and a bit more
commonly used)

On 20.01.22 10:33, Matthias Heiserer wrote:
> A default language can now be set in node.cfg.
> This language is only used if none is set in the cookies.
> 
> Signed-off-by: Matthias Heiserer <m.heiserer@proxmox.com>
> ---
>  src/bin/proxmox-backup-proxy.rs |  8 ++++++
>  src/config/node.rs              | 45 ++++++++++++++++++++++++++++++---
>  2 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
> index 523966cf..c197b512 100644
> --- a/src/bin/proxmox-backup-proxy.rs
> +++ b/src/bin/proxmox-backup-proxy.rs
> @@ -168,6 +168,14 @@ async fn get_index_future(
>              lang = language;
>          }
>      }
> +    if lang.is_empty() {
> +        if let Ok((config, _)) = proxmox_backup::config::node::config() {
> +            if let Some(default_lang) = config.default_lang {
> +                // don't need to check for existance for language file because node::config() only returns valid results 

trailing whitespace and the line is to long in any way, use 100 character columns
as maximum in our rust code base.

> +                lang = default_lang;
> +            }
> +        }
> +    }
>  
>      let data = json!({
>          "NodeName": nodename,
> diff --git a/src/config/node.rs b/src/config/node.rs
> index 40d7b220..d78dfec6 100644
> --- a/src/config/node.rs
> +++ b/src/config/node.rs
> @@ -4,7 +4,7 @@ use openssl::ssl::{SslAcceptor, SslMethod};
>  use anyhow::{bail, Error};
>  use serde::{Deserialize, Serialize};
>  
> -use proxmox_schema::{api, ApiStringFormat, ApiType, Updater};
> +use proxmox_schema::{api, ApiStringFormat, ApiType, EnumEntry, Schema, StringSchema, Updater};
>  
>  use proxmox_http::ProxyConfig;
>  
> @@ -57,6 +57,37 @@ pub struct AcmeConfig {
>      account: AcmeAccountName,
>  }
>  
> +/// All available languages in Proxmox. Taken from proxmox-i18n repository

In the long run we probably want to (and/or):
a) auto-generate this from a central source of truth
b) add it as API type in the schema and so that the gui can get it from there,
   avoiding the hardcoded list also in the widget-toolkit and allows adding a
   new language without touching less repos.

But both are more involved and it's fine for me to get this now in with a hardcoded list,
maybe throw in some `// TODO: auto-generate` comment though as better reminder.

> +const LANG_SCHEMA: Schema = StringSchema::new("Available languages")
> +    .format(&ApiStringFormat::Enum(&[
> +        EnumEntry::new("ar", "ar"),

IMO the enum description should be the actual language, similar to how we display
it in the GUI. With the api macro you should be able to actually use a normal enum
which use the doc-comments for the description.


#[api]
#[derive(Serialize, Deserialize)]
#[serde(rename_all="lowercase")]
/// All available languages in Proxmox. Taken from proxmox-i18n repository
pub enum UILanguages {
    /// Arabic
    Ar,
    /// Catalan
    Ca,
    /// Danish
    Da,
}

...

       "default-lang" : {
           type: UILanguages,
           optional: true,
       }

...

     /// Default language used in the GUI
     #[serde(skip_serializing_if = "Option::is_none")]
     pub default_lang: Option<UILanguages>,


Note that I used UILanguage just as placeholder, there's hopefully a better name for
that type ;)

> +        EnumEntry::new("ca", "ca"),
> +        EnumEntry::new("da", "da"),
> +        EnumEntry::new("de", "de"),
> +        EnumEntry::new("es", "es"),
> +        EnumEntry::new("eu", "eu"),
> +        EnumEntry::new("fa", "fa"),
> +        EnumEntry::new("fr", "fr"),
> +        EnumEntry::new("gl", "gl"),
> +        EnumEntry::new("he", "he"),
> +        EnumEntry::new("hu", "hu"),
> +        EnumEntry::new("it", "it"),
> +        EnumEntry::new("ja", "ja"),
> +        EnumEntry::new("kr", "kr"),
> +        EnumEntry::new("nb", "nb"),
> +        EnumEntry::new("nl", "nl"),
> +        EnumEntry::new("nn", "nn"),
> +        EnumEntry::new("pl", "pl"),
> +        EnumEntry::new("pt_BR", "pt_BR"),
> +        EnumEntry::new("ru", "ru"),
> +        EnumEntry::new("sl", "sl"),
> +        EnumEntry::new("sv", "sv"),
> +        EnumEntry::new("tr", "tr"),
> +        EnumEntry::new("zh_CN", "zh_CN"),
> +        EnumEntry::new("zh_TW", "zh_TW"),
> +    ]))
> +    .schema();
> +
>  #[api(
>      properties: {
>          acme: {
> @@ -100,6 +131,10 @@ pub struct AcmeConfig {
>              schema: OPENSSL_CIPHERS_TLS_1_2_SCHEMA,
>              optional: true,
>          },
> +        "default-lang" : {
> +            schema: LANG_SCHEMA,
> +            optional: true,
> +        }
>      },
>  )]
>  #[derive(Deserialize, Serialize, Updater)]
> @@ -127,17 +162,21 @@ pub struct NodeConfig {
>  
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub http_proxy: Option<String>,
> -    
> +
>      #[serde(skip_serializing_if = "Option::is_none")]
>      pub email_from: Option<String>,
>  
>      /// List of TLS ciphers for TLS 1.3 that will be used by the proxy. (Proxy has to be restarted for changes to take effect)
>      #[serde(skip_serializing_if = "Option::is_none", rename="ciphers-tls-1.3")]
>      pub ciphers_tls_1_3: Option<String>,
> -    
> +
>      /// List of TLS ciphers for TLS <= 1.2 that will be used by the proxy. (Proxy has to be restarted for changes to take effect)
>      #[serde(skip_serializing_if = "Option::is_none", rename="ciphers-tls-1.2")]
>      pub ciphers_tls_1_2: Option<String>,
> +
> +    /// Default language used in the GUI
> +    #[serde(skip_serializing_if = "Option::is_none")]
> +    pub default_lang: Option<String>,
>  }
>  
>  impl NodeConfig {