VPN-6749: Fix shared addon ids

[mcleinman]

Description

The “shared addons strings” for VPN (VPN PR, translations PR) had a bug - the string IDs in pontoon are of the form CommonStringsSubtitle, and the intention was vpn.commonStrings.subtitle. While this is non-standard (per the rest of our string IDs), it also provides an unlikely-but-real technical issue of a future clash - update.soonMessage and updateSoon.message from different parts of the YAML file would end up with the same ID of CommonStringsUpdateSoonMessage.

This PR does 3 things:

• Fixes the bug in generate_shared_addons_xliff.py
• Adds a new test in check_l10n_issues.py that probably should have been there from the start - it would have caught this issue. This confirms that all shared strings from addons are present in the processed translation file.
• Adds a note to the documentation. (I tried to automatically do this via cmake, but belatedly learnend that cmake cannot modify source directory files.)

There is another PR on the l10n repo to update the current translation strings to this id format: mozilla-l10n/mozilla-vpn-client-l10n#494

The two PRs must be merged at the same time, so that the subsequent ingestion script run doesn't re-introduce the bad IDs and doesn't introduce the good IDs (without translations).

Reference

VPN-6749

Checklist

• My code follows the style guidelines for this project
• I have not added any packages that contain high risk or unknown licenses (GPL, LGPL, MPL, etc. consult with DevOps if in question)
• I have performed a self review of my own code
• I have commented my code PARTICULARLY in hard to understand areas
• I have added thorough tests where needed

[flodolo]

Instead of regenerating, have you considered changing write_en_language(). It looks like a cleaner approach to me. This would be the diff compared to your current patch.

diff --git a/scripts/shared.py b/scripts/shared.py
index 46ea19700..5b6222561 100644
--- a/scripts/shared.py
+++ b/scripts/shared.py
@@ -2,7 +2,7 @@ import os
 import sys
 import xml.etree.ElementTree as ET
 
-def write_en_language(filename, strings):
+def write_en_language(filename, strings, key_as_id=True):
     ts = ET.Element("TS")
     ts.set("version", "2.1")
     ts.set("language", "en")
@@ -11,8 +11,9 @@ def write_en_language(filename, strings):
     ET.SubElement(context, "name")
 
     for key, value in strings.items():
+        id = key if key_as_id else value["string_id"]
         message = ET.SubElement(context, "message")
-        message.set("id", key)
+        message.set("id", id)
 
         location = ET.SubElement(message, "location")
         location.set("filename", "addon.qml")
diff --git a/scripts/utils/generate_shared_addon_xliff.py b/scripts/utils/generate_shared_addon_xliff.py
index 56efdc486..39b2f91e2 100644
--- a/scripts/utils/generate_shared_addon_xliff.py
+++ b/scripts/utils/generate_shared_addon_xliff.py
@@ -28,16 +28,6 @@ def prune_lists_to_strings(data):
             sys.exit("Unexpected input")
     return data
 
-def use_proper_id(data):
-    return_data = {}
-    for value in data.values():
-        string_id = value["string_id"]
-        return_data[string_id] = {
-            "value": value["value"],
-            "comments": value["comments"]
-        }
-    return return_data
-
 # Make sure we have all the required things
 # Lookup our required tools for addon generation.
 
@@ -69,13 +59,11 @@ print("Preparing the addons shared string file")
 print("First, pull in the strings from the YAML file")
 translation_strings = parseYAMLTranslationStrings(args.infile)
 translation_strings = prune_lists_to_strings(translation_strings)
-# parseYAMLTranslationStrings gives a different ID than we want to use
-translation_strings = use_proper_id(translation_strings)
 
 print("Then, write the strings to a .ts file")
 tmp_path = tempfile.mkdtemp()
 ts_file = os.path.join(tmp_path, "sharedAddonsStrings.ts")
-write_en_language(ts_file, translation_strings)
+write_en_language(ts_file, translation_strings, key_as_id=False)
 
 print("Finally, convert the ts file into an xliff file")
 os.system(f"{lconvert} -if ts -i {ts_file} -of xlf -o {args.outfile}")

[mcleinman]

Good call, thanks. I've updated this.

[flodolo]

Suggested change

	print(f"No content found in: {filepath}")
	sys.exit(f"No content found in: {filepath}")

You can exit directly with the message (across the file).

[mcleinman]

I had it that way initially, but moved to print combined with sys.exit(1) to match the style in the rest of this file.