crowdin / github-action

A GitHub action to manage and synchronize localization resources with your Crowdin project
https://crowdin.com
MIT License
155 stars 54 forks source link

Issue with new pull_request_url output #207

Closed tinogo closed 10 months ago

tinogo commented 10 months ago

First of all: Thank you for adding the "pull_request_url"-output to the Crowdin-Github-Action. It made our Crowdin-Download-Workflow much easier to maintain, as we need to fetch the corresponding Pull Request for ourselves anymore. :)

Describe the bug But we still stumbled upon a small bug/annoyance:

  1. We are running a workflow which tries to download the updated translations from Crowdin every hour (scheduled workflow)
  2. If something has changed, at least for the projects default branch a PR will be created, the PR gets approved and the "auto-merge" functionality enabled, i.e. so that it will automatically be merged, as soon as the PR's pipeline is "green"

The bug now is:

  1. Change a translation within your Crowdin project
  2. Let the scheduled workflow run (or trigger the workflow manually)
  3. Let the workflow create the PR and merge it (and also delete the localisation branch within your GitHub repo)
  4. Let the scheduled workflow run again (or trigger the workflow manually)
  5. You will receive a notification, that the PR got approved again

image

To Reproduce Steps to reproduce the behavior:

  1. Crowdin GitHub Action configuration

Reusable Workflow:

name: Crowdin Workflow (Download)

on:
  workflow_call:
    inputs:
      repo_default_branch:
        description: 'Specifies the default branch for this repository. Usually something like main, master or develop.'
        type: string
        required: false
        default: 'main'
      crowdin_config_path:
        description: 'Specifies the path to the Crowdin config file.'
        type: string
        required: false
        default: 'crowdin.yml'
      auto_merge_check_name:
        description: 'Specifies the name of the job, for which the auto-merge-job will wait, until it finished successfully, so that it can merge the PR automatically.'
        type: string
        required: false
        default: ''
      export_only_approved:
        description: 'Include approved translations only in exported files. If not combined with "skip_untranslated_strings" option, strings without approval are fulfilled with the source language'
        type: boolean
        required: false
        default: false
      skip_untranslated_strings:
        description: 'Skip untranslated strings in exported files (does not work with .docx, .html, .md and other document files)'
        type: boolean
        required: false
        default: false
    secrets:
      token:
        description: 'The Github access token.'
        required: true
      crowdin_project_id:
        description: 'The Project-ID within Crowdin for this repository.'
        required: true
      crowdin_token:
        description: 'The Crowdin access token.'
        required: true

concurrency:
  group: crowdin-${{ github.ref }}

jobs:
  set-parameters:
    runs-on: ubuntu-latest
    steps:
      - id: branch-name
        run: echo "name=$(echo ${GITHUB_REF#refs/heads/})" >> $GITHUB_OUTPUT
      - id: branch-name-clean
        # Crowdin's internal branch names can't contain slashes, therefore we need to replace them.
        run: echo "name=$(echo ${GITHUB_REF#refs/heads/} | sed 's/\//-/g')" >> $GITHUB_OUTPUT
    outputs:
      branch-name: ${{ steps.branch-name.outputs.name }}
      branch-name-clean: ${{ steps.branch-name-clean.outputs.name }}

  fetch-translations-from-crowdin:
    runs-on: ubuntu-latest
    needs:
      - set-parameters
    steps:
      - name: Checkout
        uses: actions/checkout@v4

      - name: crowdin action
        uses: crowdin/github-action@v1
        id: crowdin-download
        with:
          upload_sources: false
          upload_translations: false
          download_sources: true
          download_translations: true
          auto_approve_imported: false
          # If we've got a feature-branch, allow Crowdin to directly commit and push to it.
          # If the current branch is the repository's default branch, a separate branch + pull-request needs to be created,
          # as the default branch is usually protected.
          localization_branch_name: ${{ needs.set-parameters.outputs.branch-name == inputs.repo_default_branch && format('{0}_{1}', needs.set-parameters.outputs.branch-name, 'l10n') || needs.set-parameters.outputs.branch-name }}
          create_pull_request: ${{ needs.set-parameters.outputs.branch-name == inputs.repo_default_branch }}
          pull_request_title: 'chore(i18n): New Crowdin Translations for branch: ${{ needs.set-parameters.outputs.branch-name }}'
          pull_request_body: 'New Crowdin translations by [Crowdin GH Action](https://github.com/crowdin/github-action)'
          pull_request_base_branch_name: ${{ needs.set-parameters.outputs.branch-name }}
          crowdin_branch_name: ${{ needs.set-parameters.outputs.branch-name-clean }}
          config: ${{ inputs.crowdin_config_path }}
          export_only_approved: ${{ inputs.export_only_approved }}
          skip_untranslated_strings: ${{ inputs.skip_untranslated_strings }}
        env:
          GITHUB_TOKEN: ${{ secrets.token }}
          CROWDIN_PROJECT_ID: ${{ secrets.crowdin_project_id }}
          CROWDIN_PERSONAL_TOKEN: ${{ secrets.crowdin_token }}
      # We only need to execute this for the Crowdin-PR which points to the repository's default branch,
      # as Crowdin directly commits and pushes changes to all the other branches.
      - name: Enable auto-merge for the PR
        if: ${{ steps.crowdin-download.outputs.pull_request_url && needs.set-parameters.outputs.branch-name == inputs.repo_default_branch }}
        run: gh pr --repo $GITHUB_REPOSITORY merge ${{ steps.crowdin-download.outputs.pull_request_url }} --auto --merge
        env:
          GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
      - name: Approve the PR
        if: ${{ steps.crowdin-download.outputs.pull_request_url && needs.set-parameters.outputs.branch-name == inputs.repo_default_branch }}
        run: gh pr --repo $GITHUB_REPOSITORY review ${{ steps.crowdin-download.outputs.pull_request_url }} --approve
        env:
          GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}

Scheduled workflow:

name: Crowdin Workflow (Download)

on:
  schedule:
    - cron: '0 6-20 * * 1-5'
  workflow_dispatch:

jobs:
  fetch-translations-from-crowdin:
    uses: path/to/reusable-workflow/.github/workflows/crowdin-download.yml@v1
    with:
      repo_default_branch: 'develop'
    secrets:
      token: ${{ secrets.BOT_ACTIONS_TOKEN }}
      crowdin_project_id: ${{ secrets.CROWDIN_PROJECT_ID }}
      crowdin_token: ${{ secrets.CROWDIN_PERSONAL_TOKEN }}
  1. crowdin.yml file content
base_url: 'https://bike24.api.crowdin.com'
project_id_env: CROWDIN_PROJECT_ID
api_token_env: CROWDIN_PERSONAL_TOKEN
preserve_hierarchy: true
files:
  - source: /theme/src/assets/locales/en.json
    translation: /theme/src/assets/locales/%two_letters_code%.%file_extension%
  - source: /theme/src/assets/locales/storybook/en.json
    translation: /theme/src/assets/locales/storybook/%two_letters_code%.%file_extension%
  - source: /phpinclude/languages/en_US/*.php
    translation: /phpinclude/languages/%locale_with_underscore%/%original_file_name%
  - source: /phpinclude/languages/en_US/*.json
    translation: /phpinclude/languages/%locale_with_underscore%/%original_file_name%
    ignore:
      - /phpinclude/languages/en_US/svg
  - source: /phpinclude/languages/en_US/**/*.json
    translation: /phpinclude/languages/%locale_with_underscore%/**/%original_file_name%
    ignore:
      - /phpinclude/languages/en_US/svg
  1. Information about workflow (OS, steps, etc.)

Expected behavior When the Crowdin Action detects, that there are no translation changes: image ...it either should leave the "pull_request_url"-output empty or provide another output, which states, that a PR for this localization branch already exists.

Screenshots If applicable, add screenshots to help explain your problem.

Additional context Add any other context about the problem here.

andrii-bodnar commented 10 months ago

Hi @tinogo,

The Action will fill the pull_request_url only in the case when it actually creates a new PR (https://github.com/crowdin/github-action/blob/master/entrypoint.sh#L139)

In other cases, the value of this output will remain empty (https://github.com/crowdin/github-action/blob/master/entrypoint.sh#L4)

tinogo commented 10 months ago

Hi @andrii-bodnar,

thank you for your reply - really appreciate that! :)

While debugging this a little more, I think that I've found the error: In the cases, where the pull_request_url is supposed to be empty, it actually seems to contain two single-quotes, i.e. ''.

See the screenshot of my workflow run: image

The '' is the value of the pull_request_url. If it would be really empty, the added if-expressions of some of my workflow-steps wouldn't execute (like if: ${{ steps.crowdin-download.outputs.pull_request_url && needs.set-parameters.outputs.branch-name == inputs.repo_default_branch }}, as an empty string/value for a step-output should evaluate to false).

Edit: I've just supplied a pull-request, which should fix the issue (but haven't tested it).

andrii-bodnar commented 10 months ago

@tinogo thanks a lot for your investigation! 🚀

I've already merged your PR, could you please try if it works before the release by specifying the commit hash as a version:

uses: crowdin/github-action@195e70c077744813cd72a7c439c7acac98131239

Thank you!

tinogo commented 10 months ago

@andrii-bodnar Awesome - looks good now:

image

andrii-bodnar commented 10 months ago

@tinogo great, thank you for the quick feedback! 🙌

Releasing a new version

andrii-bodnar commented 10 months ago

@tinogo Would you mind if I used some of your workflow to enhance the usage examples? https://github.com/crowdin/github-action/blob/master/EXAMPLES.md

tinogo commented 10 months ago

@andrii-bodnar Of course you can use them. 🙂