From 6803c8bcd1154ab5aea85f094dc5533b716a773c Mon Sep 17 00:00:00 2001
From: Omar Al-Ithawi <i@omardo.com>
Date: Wed, 18 Oct 2023 15:11:30 +0300
Subject: [PATCH] feat: post translation validation errors as a GitHub comment

---
 .../workflows/validate-translation-files.yml  | 37 +++++++++-
 Makefile                                      |  7 +-
 scripts/validate_translation_files.py         | 70 +++++++++++++++++++
 3 files changed, 107 insertions(+), 7 deletions(-)
 create mode 100644 scripts/validate_translation_files.py

diff --git a/.github/workflows/validate-translation-files.yml b/.github/workflows/validate-translation-files.yml
index 65c8065b0cc..5ba1dda1c8e 100644
--- a/.github/workflows/validate-translation-files.yml
+++ b/.github/workflows/validate-translation-files.yml
@@ -8,6 +8,9 @@ on:
 jobs:
   validate-po-files:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: write
     steps:
       # Clones the openedx-translations repo
       - name: clone openedx/openedx-translations
@@ -17,7 +20,39 @@ jobs:
         run: |
           sudo apt install -y gettext
 
+      - name: Hide previous bot comments
+        uses: int128/hide-comment-action@721ce4b6c64d2b4bc909702341e94355b0f94cd5
+        with:
+          contains: |
+           <!-- generator:validate-translation-files.yml -->
+
       - name: Validate translation files
         id: validate_translation_files
         run: |
-          make validate_translation_files
+          has_validation_errors=0
+          python scripts/validate_translation_files.py 2>validation_errors.txt || has_validation_errors=1
+          cat validation_errors.txt
+
+          {
+            echo 'VALIDATION_ERRORS<<EOF'
+            fold -w 100 -s validation_errors.txt
+            echo EOF
+          } >> "$GITHUB_OUTPUT"
+
+          exit $has_validation_errors
+
+      - name: Post comment if validation fails
+        if: failure()
+        uses: peter-evans/create-or-update-comment@411d7f9b4092af4736447c5420752e3b2be55ec1
+        with:
+          issue-number: ${{ github.event.pull_request.number }}
+          body: |
+            :warning: There are errors in the translation files:
+
+            ```
+            ${{ steps.validate_translation_files.outputs.VALIDATION_ERRORS }}
+            ```
+
+            This comment has been posted by the `validate-translation-files.yml` GitHub Actions workflow.
+
+            <!-- generator:validate-translation-files.yml -->
diff --git a/Makefile b/Makefile
index 861d21c8036..d2a8663ad99 100644
--- a/Makefile
+++ b/Makefile
@@ -38,12 +38,7 @@ test:  ## Run scripts tests
 	 pytest -v -s --cov=. --cov-report=term-missing --cov-report=html scripts/tests
 
 validate_translation_files:  ## Run basic validation to ensure files are compilable
-	find translations/ -name '*.po' \
-	    | grep -v '/en/LC_MESSAGES/' \
-	    | xargs -I{} msgfmt -v --strict --check {}
-	@echo '-----------------------------------------'
-	@echo 'Congratulations! Translation files are valid.'
-	@echo '-----------------------------------------'
+	python scripts/validate_translation_files.py
 
 sync_translations:  ## Syncs from the old projects to the new openedx-translations project
 	python scripts/sync_translations.py $(SYNC_ARGS)
diff --git a/scripts/validate_translation_files.py b/scripts/validate_translation_files.py
new file mode 100644
index 00000000000..75785fa6710
--- /dev/null
+++ b/scripts/validate_translation_files.py
@@ -0,0 +1,70 @@
+import sys
+import os
+import os.path
+import subprocess
+
+
+def get_translation_files(translation_directory):
+    """
+    List all translations '*.po' files in the specified directory.
+    """
+    po_files = []
+    for root, _dirs, files in os.walk(translation_directory):
+        for file_name in files:
+            pofile_path = os.path.join(root, file_name)
+            if file_name.endswith('.po') and '/en/LC_MESSAGES/' not in pofile_path:
+                po_files.append(pofile_path)
+    return po_files
+
+
+def validate_translation_file(po_file):
+    """
+    Validate a translation file and return errors if any.
+
+    This function combines both stderr and stdout output of the `msgfmt` in a
+    single variable.
+    """
+    completed_process = subprocess.run(
+        ['msgfmt', '-v', '--strict', '--check', po_file],
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        text=True,
+    )
+    return {
+        'valid': completed_process.returncode == 0,
+        'output': completed_process.stdout + '\n' + completed_process.stderr
+    }
+
+
+def main():
+    """
+    Run msgfmt and print errors to stderr.
+    """
+    translations_valid = True
+
+    po_files = get_translation_files('translations')
+    for po_file in po_files:
+        result = validate_translation_file(po_file)
+
+        if result['valid']:
+            print('VALID: ' + po_file)
+            print(result['output'], '\n' * 2)
+        else:
+            print('INVALID: ' + po_file, file=sys.stderr)
+            print(result['output'], '\n' * 2, file=sys.stderr)
+            translations_valid = False
+
+    print('-----------------------------------------')
+    if translations_valid:
+        print('SUCCESS: All translation files are valid.')
+        exit_code = 0
+    else:
+        print('FAILURE: Some translations are invalid. Check the stderr for error messages.')
+        exit_code = 1
+    print('-----------------------------------------')
+
+    sys.exit(exit_code)
+
+
+if __name__ == '__main__':
+    main()