Audit and fix i18n markup (#1662)

Also upgrade dependencies and make necessary changes to pass pre-commit checks. Three problems remain as of this commit:

1. `pybabel extract` is no longer processing the plural form in `ngettext` (unknown since when), so plurals are entirely missing from the message catalog.
2. There are strings and string templates in JS that are not wrapped in gettext calls.
3. `format_date` and other Babel functions are missing in JS, so the project schedule view (rendered in JS) does not have localized dates.

.pre-commit-config.yaml

@@ -124,7 +124,7 @@ repos:
         language_version: python3
         args: ['-c', 'pyproject.toml']
         additional_dependencies:
-          - toml
+          - 'bandit[toml]'
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.4.0
     hooks:

Makefile

@@ -23,7 +23,7 @@ all:
 
 babelpy:
 	ZXCVBN_DIR=`python -c "import zxcvbn; import pathlib; print(pathlib.Path(zxcvbn.__file__).parent, end='')"`
-	pybabel extract -F babel.cfg -k _ -k __ -k ngettext -o funnel/translations/messages.pot . ${ZXCVBN_DIR}
+	pybabel extract -F babel.cfg -k _ -k __ -k _n -k __n -k gettext -k ngettext -o funnel/translations/messages.pot funnel ${ZXCVBN_DIR}
 	pybabel update -N -i funnel/translations/messages.pot -d funnel/translations
 	pybabel compile -f -d funnel/translations
 
@@ -33,8 +33,8 @@ babeljs: baseframe_dir = $(flask baseframe_translations_path)
 
 babeljs:
 	@mkdir -p $(target_dir)
-	ls $(source_dir) | grep -E '[[:lower:]]{2}_[[:upper:]]{2}' | xargs -I % sh -c 'mkdir -p $(target_dir)/% && ./node_modules/.bin/po2json --format=jed --pretty $(source_dir)/%/LC_MESSAGES/messages.po $(target_dir)/%/messages.json'
-	ls $(baseframe_dir) | grep -E '[[:lower:]]{2}_[[:upper:]]{2}' | xargs -I % sh -c './node_modules/.bin/po2json --format=jed --pretty $(baseframe_dir)/%/LC_MESSAGES/baseframe.po $(target_dir)/%/baseframe.json'
+	ls $(source_dir) | grep -E '[[:lower:]]{2}_[[:upper:]]{2}' | xargs -I % sh -c 'mkdir -p $(target_dir)/% && ./node_modules/.bin/po2json --format=jed --pretty --domain=messages $(source_dir)/%/LC_MESSAGES/messages.po $(target_dir)/%/messages.json'
+	ls $(baseframe_dir) | grep -E '[[:lower:]]{2}_[[:upper:]]{2}' | xargs -I % sh -c './node_modules/.bin/po2json --format=jed --pretty --domain=baseframe $(baseframe_dir)/%/LC_MESSAGES/baseframe.po $(target_dir)/%/baseframe.json'
 	./node_modules/.bin/prettier --write $(target_dir)/**/**.json
 
 babel: babelpy babeljs

babel.cfg

@@ -7,4 +7,4 @@ extensions=jinja2.ext.do,webassets.ext.jinja2.AssetsExtension
 [javascript: **/assets/js/**.js]
 [javascript: **/static/js/**.js]
 encoding = utf-8
-extract_messages=gettext,ngettext
+extract_messages=_,__,_n,__n,gettext,ngettext

funnel/assets/js/utils/gettext.js

@@ -99,15 +99,9 @@ class Gettext {
       if (msgid in this.catalog) {
         const msgidCatalog = this.catalog[msgid];
 
-        if (msgidCatalog.length < 2) {
-          // eslint-disable-next-line no-console
-          console.error(
-            'Invalid format for translated messages, at least 2 values expected'
-          );
+        if (msgidCatalog[0] !== '') {
+          return vsprintf(msgidCatalog[0], args);
         }
-        // in case of gettext() first element is empty because it's the msgid_plural,
-        // and the second element is the translated msgstr
-        return vsprintf(msgidCatalog[1], args);
       }
       return vsprintf(msgid, args);
     };
@@ -116,27 +110,27 @@ class Gettext {
       if (msgid in this.catalog) {
         const msgidCatalog = this.catalog[msgid];
 
-        if (msgidCatalog.length < 3) {
+        if (msgidCatalog.length < 2) {
           // eslint-disable-next-line no-console
           console.error(
-            'Invalid format for translated messages, at least 3 values expected for plural translations'
+            'Invalid format for translated messages, at least 2 values expected for plural translations'
           );
-        }
-
-        if (msgidPlural !== msgidCatalog[0]) {
-          // eslint-disable-next-line no-console
-          console.error("Plural forms don't match");
-        }
-
-        let msgstr = '';
-        if (num <= 1) {
-          msgstr = sprintf(msgidCatalog[1], { num });
         } else {
-          msgstr = sprintf(msgidCatalog[2], { num });
+          let msgstr = '';
+          if (num === 1) {
+            msgstr = sprintf(msgidCatalog[0], { num });
+          } else {
+            msgstr = sprintf(msgidCatalog[1], { num });
+          }
+          if (msgstr !== '') {
+            return vsprintf(msgstr, args);
+          }
         }
-        return vsprintf(msgstr, args);
       }
-      return vsprintf(msgid, args);
+      if (num === 1) {
+        return vsprintf(sprintf(msgid, { num }), args);
+      }
+      return vsprintf(sprintf(msgidPlural, { num }), args);
     };
   }
 }

funnel/forms/comment.py

@@ -15,7 +15,7 @@ class CommentForm(forms.Form):
 
     message = forms.MarkdownField(
         "",
-        id="comment_message",
+        id='comment_message',
         validators=[forms.validators.DataRequired()],
     )
 

funnel/forms/sync_ticket.py

@@ -64,7 +64,7 @@ class TicketEventForm(forms.Form):
     )
     badge_template = forms.URLField(
         __("Badge template URL"),
-        description="URL of background image for the badge",
+        description=__("URL of background image for the badge"),
         validators=[forms.validators.Optional(), forms.validators.ValidUrl()],
     )
 

funnel/models/auth_client.py

@@ -468,18 +468,14 @@ class AuthToken(ScopeMixin, BaseMixin, db.Model):  # type: ignore[name-defined]
     )
     #: The token
     token = sa.Column(sa.String(22), default=make_buid, nullable=False, unique=True)
-    #: The token's type
-    token_type = sa.Column(
-        sa.String(250), default='bearer', nullable=False
-    )  # 'bearer', 'mac' or a URL
+    #: The token's type, 'bearer', 'mac' or a URL
+    token_type = sa.Column(sa.String(250), default='bearer', nullable=False)
     #: Token secret for 'mac' type
     secret = sa.Column(sa.String(44), nullable=True)
     #: Secret's algorithm (for 'mac' type)
     algorithm = sa.Column(sa.String(20), nullable=True)
-    #: Token's validity, 0 = unlimited
-    validity = sa.Column(
-        sa.Integer, nullable=False, default=0
-    )  # Validity period in seconds
+    #: Token's validity period in seconds, 0 = unlimited
+    validity = sa.Column(sa.Integer, nullable=False, default=0)
     #: Refresh token, to obtain a new token
     refresh_token = sa.Column(sa.String(22), nullable=True, unique=True)
 

funnel/models/helpers.py

@@ -461,7 +461,7 @@ class MyModel(db.Model):  # type: ignore[name-defined]
     )
 
     update_statement = (
-        f'UPDATE {pgquote(model.__tablename__)}'
+        f'UPDATE {pgquote(model.__tablename__)}'  # nosec
         f' SET {pgquote(column_name)} = {update_expr};'
     )
 

funnel/models/label.py

@@ -82,7 +82,7 @@ class Label(
     seq = sa.Column(sa.Integer, nullable=False)
 
     # A single-line description of this label, shown when picking labels (optional)
-    description = sa.Column(sa.UnicodeText, nullable=False, default="")
+    description = sa.Column(sa.UnicodeText, nullable=False, default='')
 
     #: Icon for displaying in space-constrained UI. Contains one emoji symbol.
     #: Since emoji can be composed from multiple symbols, there is no length

funnel/models/utils.py

@@ -167,10 +167,8 @@ def do_migrate_instances(
     if old_instance == new_instance:
         raise ValueError("Old and new are the same")
 
-    # User id column (for foreign keys)
-    id_column = (
-        old_instance.__class__.__table__.c.id
-    )  # 'id' is from IdMixin via BaseMixin
+    # User id column (for foreign keys); 'id' is from IdMixin via BaseMixin
+    id_column = old_instance.__class__.__table__.c.id
 
     # Session (for queries)
     session = old_instance.query.session

funnel/registry.py

@@ -87,9 +87,8 @@ def wrapper() -> ReturnResponse:
                 if not authtoken.is_valid():
                     return resource_auth_error(_("Access token has expired"))
 
-                tokenscope = set(
-                    authtoken.effective_scope
-                )  # Read once to avoid reparsing below
+                # Read once to avoid reparsing below
+                tokenscope = set(authtoken.effective_scope)
                 wildcardscope = usescope.split('/', 1)[0] + '/*'
                 if not (authtoken.auth_client.trusted and '*' in tokenscope):
                     # If a trusted client has '*' in token scope, all good,

funnel/static/translations/en_IN/baseframe.json

@@ -0,0 +1,4 @@
+{
+  "domain": "messages",
+  "locale_data": {}
+}

funnel/static/translations/en_IN/messages.json

@@ -0,0 +1,4 @@
+{
+  "domain": "messages",
+  "locale_data": {}
+}