Remove HTML todolist from translation linter

All the existing strings in the app have had the HTML removed from them. This means we no longer need the HTML todolist functionality and all strings can go through the normal translation process. Remove this functionality from the app to make it less complicated. Follow up from: https://gitlab.com/gitlab-org/gitlab/-/issues/228846

Remove HTML todolist from translation linter
All the existing strings in the app have had the HTML removed from them. This means we no longer need the HTML todolist functionality and all strings can go through the normal translation process. Remove this functionality from the app to make it less complicated. Follow up from: https://gitlab.com/gitlab-org/gitlab/-/issues/228846
8533a7c5 · Josianne Hyson · ba1eb804 · ba1eb804 · 8533a7c5 · 8533a7c5
Commit 8533a7c5 authored Nov 18, 2020 by Josianne Hyson
6 changed files
--- a/lib/gitlab/i18n/html_todo.yml
+++ b/lib/gitlab/i18n/html_todo.yml
-#
-# PLEASE DO NOT ADD NEW STRINGS TO THIS FILE.
-#
-# See https://docs.gitlab.com/ee/development/i18n/externalization.html#html
-# for information on how to handle HTML in translations.
-#
-# This file contains strings that need to be fixed to use the
-# updated HTML guidelines. Any strings in this file will no
-# longer be able to be translated until they have been updated.
-#
-# This file (and the functionality around it) will be removed
-# once https://gitlab.com/gitlab-org/gitlab/-/issues/217933 is complete.
-#
-# See https://gitlab.com/gitlab-org/gitlab/-/issues/19485 for more details
-# why this change has been made.
-#
-#
-# Strings below are fixed in the source code but the translations are still present in CrowdIn so the
-# locale files will fail the linter. They can be deleted after next CrowdIn sync, likely in:
-# https://gitlab.com/gitlab-org/gitlab/-/issues/226008
-#
-"This commit was signed with an <strong>unverified</strong> signature.":
-  plural_id:
-  translations:
-    - "このコミットは<strong>検証されていない</strong> 署名でサインされています。"
-    - "Этот коммит был подписан <strong>непроверенной</strong> подписью."
-    - "此提交使用 <strong>未经验证的</strong> 签名进行签名。"
-    - "Цей коміт підписано <strong>неперевіреним</strong> підписом."
-    - "Esta commit fue firmado con una firma <strong>no verificada</strong>."
-"This commit was signed with a <strong>verified</strong> signature and the committer email is verified to belong to the same user.":
-  plural_id:
-  translations:
-    - "このコミットは <strong>検証済み</strong> の署名でサインされており、このコミッターのメールは同じユーザーのものであることが検証されています。"
-    - "Это коммит был подписан <strong>верифицированной</strong> подписью и коммитер подтвердил, что адрес почты принадлежит ему."
-    - "此提交使用 <strong>已验证</strong> 的签名进行签名，并且已验证提交者的电子邮件属于同一用户。"
-    - "Цей коміт підписано <strong>перевіреним</strong> підписом і адреса електронної пошти комітера гарантовано належить тому самому користувачу."
-    - "Este commit fue firmado con una firma verificada, y <strong>se ha verificado</strong> que la dirección de correo electrónico del committer y la firma pertenecen al mismo usuario."
-"Branch <strong>%{branch_name}</strong> was created. To set up auto deploy, choose a GitLab CI Yaml template and commit your changes. %{link_to_autodeploy_doc}":
-  plural_id:
-  translations:
-    - "分支 <strong>%{branch_name}</strong> 已創建。如需設置自動部署， 請選擇合適的 GitLab CI Yaml 模板併提交更改。%{link_to_autodeploy_doc}"
-    - "O branch <strong>%{branch_name}</strong> foi criado. Para configurar o deploy automático, selecione um modelo de Yaml do GitLab CI e commit suas mudanças. %{link_to_autodeploy_doc}"
-    - "<strong>%{branch_name}</strong> ブランチが作成されました。自動デプロイを設定するには、GitLab CI Yaml テンプレートを選択して、変更をコミットしてください。 %{link_to_autodeploy_doc}"
-    - "La branch <strong>%{branch_name}</strong> è stata creata. Per impostare un rilascio automatico scegli un template CI di Gitlab e committa le tue modifiche %{link_to_autodeploy_doc}"
-    - "O ramo <strong>%{branch_name}</strong> foi criado. Para configurar a implantação automática, seleciona um modelo de Yaml do GitLab CI e envia as tuas alterações. %{link_to_autodeploy_doc}"
-    - "Ветка <strong>%{branch_name}</strong> создана. Для настройки автоматического развертывания выберите YAML-шаблон для GitLab CI и зафиксируйте свои изменения. %{link_to_autodeploy_doc}"
-    - "已创建分支 <strong>%{branch_name}</strong> 。如需设置自动部署， 请选择合适的 GitLab CI Yaml 模板并提交更改。%{link_to_autodeploy_doc}"
-    - "Гілка <strong>%{branch_name}</strong> створена. Для настройки автоматичного розгортання виберіть GitLab CI Yaml-шаблон і закомітьте зміни. %{link_to_autodeploy_doc}"
-    - "Клонът <strong>%{branch_name}</strong> беше създаден. За да настроите автоматичното внедряване, изберете Yaml шаблон за GitLab CI и подайте промените си. %{link_to_autodeploy_doc}"
-    - "Branch <strong>%{branch_name}</strong> wurde erstellt. Um die automatische Bereitstellung einzurichten, wähle eine GitLab CI Yaml Vorlage und committe deine Änderungen. %{link_to_autodeploy_doc}"
-    - "<strong>%{branch_name}</strong> 브랜치가 생성되었습니다. 자동 배포를 설정하려면 GitLab CI Yaml 템플릿을 선택하고 변경 사항을 적용하십시오. %{link_to_autodeploy_doc}"
-    - "La branĉo <strong>%{branch_name}</strong> estis kreita. Por agordi aŭtomatan disponigadon, bonvolu elekti Yaml-ŝablonon por GitLab CI kaj enmeti viajn ŝanĝojn. %{link_to_autodeploy_doc}"
-    - "La branche <strong>%{branch_name}</strong> a été créée. Pour mettre en place le déploiement automatisé, sélectionnez un modèle de fichier YAML pour l’intégration continue (CI) de GitLab, et validez les modifications. %{link_to_autodeploy_doc}"
-    - "La rama <strong>%{branch_name}</strong> fue creada. Para configurar el auto despliegue, escoge una plantilla Yaml para GitLab CI y envía tus cambios. %{link_to_autodeploy_doc}"
--- a/lib/gitlab/i18n/po_linter.rb
+++ b/lib/gitlab/i18n/po_linter.rb
@@ -5,14 +5,13 @@ module Gitlab
    class PoLinter
      include Gitlab::Utils::StrongMemoize
-      attr_reader :po_path, :translation_entries, :metadata_entry, :locale, :html_todolist
+      attr_reader :po_path, :translation_entries, :metadata_entry, :locale
      VARIABLE_REGEX = /%{\w*}|%[a-z]/.freeze
-      def initialize(po_path:, html_todolist:, locale: I18n.locale.to_s)
+      def initialize(po_path:, locale: I18n.locale.to_s)
        @po_path = po_path
        @locale = locale
-        @html_todolist = html_todolist
      end
      def errors
@@ -43,8 +42,7 @@ module Gitlab
        @translation_entries = entries.map do |entry_data|
          Gitlab::I18n::TranslationEntry.new(
            entry_data: entry_data,
-            nplurals: metadata_entry.expected_forms,
+            nplurals: metadata_entry.expected_forms
-            html_allowed: html_todolist.fetch(entry_data[:msgid], false)
          )
        end
@@ -97,15 +95,15 @@ module Gitlab
        common_message = 'contains < or >. Use variables to include HTML in the string, or the &lt; and &gt; codes ' \
          'for the symbols. For more info see: https://docs.gitlab.com/ee/development/i18n/externalization.html#html'
-        if entry.msgid_contains_potential_html? && !entry.msgid_html_allowed?
+        if entry.msgid_contains_potential_html?
          errors << common_message
        end
-        if entry.plural_id_contains_potential_html? && !entry.plural_id_html_allowed?
+        if entry.plural_id_contains_potential_html?
          errors << 'plural id ' + common_message
        end
-        if entry.translations_contain_potential_html? && !entry.translations_html_allowed?
+        if entry.translations_contain_potential_html?
          errors << 'translation ' + common_message
        end
      end

--- a/lib/gitlab/i18n/translation_entry.rb
+++ b/lib/gitlab/i18n/translation_entry.rb
@@ -6,12 +6,11 @@ module Gitlab
      PERCENT_REGEX = /(?:^|[^%])%(?!{\w*}|[a-z%])/.freeze
      ANGLE_BRACKET_REGEX = /[<>]/.freeze
-      attr_reader :nplurals, :entry_data, :html_allowed
+      attr_reader :nplurals, :entry_data
-      def initialize(entry_data:, nplurals:, html_allowed:)
+      def initialize(entry_data:, nplurals:)
        @entry_data = entry_data
        @nplurals = nplurals
-        @html_allowed = html_allowed
      end
      def msgid
@@ -97,20 +96,6 @@ module Gitlab
        all_translations.any? { |translation| contains_angle_brackets?(translation) }
      end
-      def msgid_html_allowed?
-        html_allowed.present?
-      end
-      def plural_id_html_allowed?
-        html_allowed.present? && html_allowed['plural_id'] == plural_id
-      end
-      def translations_html_allowed?
-        msgid_html_allowed? && html_allowed['translations'].present? && all_translations.all? do |translation|
-          html_allowed['translations'].include?(translation)
-        end
-      end
      private
      def contains_angle_brackets?(string)

--- a/lib/tasks/gettext.rake
+++ b/lib/tasks/gettext.rake
@@ -65,10 +65,10 @@ namespace :gettext do
    linters = files.map do |file|
      locale = File.basename(File.dirname(file))
-      Gitlab::I18n::PoLinter.new(po_path: file, html_todolist: html_todolist, locale: locale)
+      Gitlab::I18n::PoLinter.new(po_path: file, locale: locale)
    end
-    linters.unshift(Gitlab::I18n::PoLinter.new(po_path: pot_file_path, html_todolist: html_todolist))
+    linters.unshift(Gitlab::I18n::PoLinter.new(po_path: pot_file_path))
    failed_linters = linters.select { |linter| linter.errors.any? }

--- a/spec/lib/gitlab/i18n/po_linter_spec.rb
+++ b/spec/lib/gitlab/i18n/po_linter_spec.rb
@@ -6,7 +6,7 @@ require 'simple_po_parser'
 # Disabling this cop to allow for multi-language examples in comments
 # rubocop:disable Style/AsciiComments
 RSpec.describe Gitlab::I18n::PoLinter do
-  let(:linter) { described_class.new(po_path: po_path, html_todolist: {}) }
+  let(:linter) { described_class.new(po_path: po_path) }
  let(:po_path) { 'spec/fixtures/valid.po' }
  def fake_translation(msgid:, translation:, plural_id: nil, plurals: [])
@@ -24,8 +24,7 @@ RSpec.describe Gitlab::I18n::PoLinter do
    Gitlab::I18n::TranslationEntry.new(
      entry_data: data,
-      nplurals: plurals.size + 1,
+      nplurals: plurals.size + 1
-      html_allowed: nil
    )
  end
@@ -160,53 +159,6 @@ RSpec.describe Gitlab::I18n::PoLinter do
        ]
      end
    end
-    context 'when an entry contains html on the todolist' do
-      subject(:linter) { described_class.new(po_path: po_path, html_todolist: todolist) }
-      let(:po_path) { 'spec/fixtures/potential_html.po' }
-      let(:todolist) do
-        {
-          'String with a legitimate < use' => {
-            'plural_id' => 'String with lots of < > uses',
-            'translations' => [
-              'Translated string with a legitimate < use',
-              'Translated string with lots of < > uses'
-            ]
-          }
-        }
-      end
-      it 'does not present an error' do
-        message_id = 'String with a legitimate < use'
-        expect(errors[message_id]).to be_nil
-      end
-    end
-    context 'when an entry on the html todolist has changed' do
-      subject(:linter) { described_class.new(po_path: po_path, html_todolist: todolist) }
-      let(:po_path) { 'spec/fixtures/potential_html.po' }
-      let(:todolist) do
-        {
-          'String with a legitimate < use' => {
-            'plural_id' => 'String with lots of < > uses',
-            'translations' => [
-              'Translated string with a different legitimate < use',
-              'Translated string with lots of < > uses'
-            ]
-          }
-        }
-      end
-      it 'presents an error for the changed component' do
-        message_id = 'String with a legitimate < use'
-        expect(errors[message_id])
-          .to include a_string_starting_with('translation contains < or >.')
-      end
-    end
  end
  describe '#parse_po' do
@@ -276,8 +228,7 @@ RSpec.describe Gitlab::I18n::PoLinter do
      fake_entry = Gitlab::I18n::TranslationEntry.new(
        entry_data: { msgid: 'the singular', msgid_plural: 'the plural', 'msgstr[0]' => 'the singular' },
-        nplurals: 2,
+        nplurals: 2
-        html_allowed: nil
      )
      errors = []

--- a/spec/lib/gitlab/i18n/translation_entry_spec.rb
+++ b/spec/lib/gitlab/i18n/translation_entry_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#singular_translation' do
    it 'returns the normal `msgstr` for translations without plural' do
      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry.singular_translation).to eq('Bonjour monde')
    end
@@ -18,7 +18,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
        'msgstr[0]' => 'Bonjour monde',
        'msgstr[1]' => 'Bonjour mondes'
      }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry.singular_translation).to eq('Bonjour monde')
    end
@@ -27,7 +27,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#all_translations' do
    it 'returns all translations for singular translations' do
      data = { msgid: 'Hello world', msgstr: 'Bonjour monde' }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry.all_translations).to eq(['Bonjour monde'])
    end
@@ -39,7 +39,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
        'msgstr[0]' => 'Bonjour monde',
        'msgstr[1]' => 'Bonjour mondes'
      }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry.all_translations).to eq(['Bonjour monde', 'Bonjour mondes'])
    end
@@ -52,7 +52,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
        msgid_plural: 'Hello worlds',
        'msgstr[0]' => 'Bonjour monde'
      }
-      entry = described_class.new(entry_data: data, nplurals: 1, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 1)
      expect(entry.plural_translations).to eq(['Bonjour monde'])
    end
@@ -65,7 +65,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
        'msgstr[1]' => 'Bonjour mondes',
        'msgstr[2]' => 'Bonjour tous les mondes'
      }
-      entry = described_class.new(entry_data: data, nplurals: 3, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 3)
      expect(entry.plural_translations).to eq(['Bonjour mondes', 'Bonjour tous les mondes'])
    end
@@ -77,7 +77,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
        msgid: 'hello world',
        msgstr: 'hello'
      }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry).to have_singular_translation
    end
@@ -89,7 +89,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
        "msgstr[0]" => 'hello world',
        "msgstr[1]" => 'hello worlds'
      }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry).to have_singular_translation
    end
@@ -100,7 +100,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
        msgid_plural: 'hello worlds',
        "msgstr[0]" => 'hello worlds'
      }
-      entry = described_class.new(entry_data: data, nplurals: 1, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 1)
      expect(entry).not_to have_singular_translation
    end
@@ -109,7 +109,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#msgid_contains_newlines' do
    it 'is true when the msgid is an array' do
      data = { msgid: %w(hello world) }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry.msgid_has_multiple_lines?).to be_truthy
    end
@@ -118,7 +118,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#plural_id_contains_newlines' do
    it 'is true when the msgid is an array' do
      data = { msgid_plural: %w(hello world) }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry.plural_id_has_multiple_lines?).to be_truthy
    end
@@ -127,7 +127,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#translations_contain_newlines' do
    it 'is true when the msgid is an array' do
      data = { msgstr: %w(hello world) }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry.translations_have_multiple_lines?).to be_truthy
    end
@@ -135,7 +135,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#contains_unescaped_chars' do
    let(:data) { { msgid: '' } }
-    let(:entry) { described_class.new(entry_data: data, nplurals: 2, html_allowed: nil) }
+    let(:entry) { described_class.new(entry_data: data, nplurals: 2) }
    it 'is true when the msgid is an array' do
      string = '「100%確定」'
@@ -177,7 +177,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#msgid_contains_unescaped_chars' do
    it 'is true when the msgid contains a `%`' do
      data = { msgid: '「100%確定」' }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry).to receive(:contains_unescaped_chars?).and_call_original
      expect(entry.msgid_contains_unescaped_chars?).to be_truthy
@@ -187,7 +187,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#plural_id_contains_unescaped_chars' do
    it 'is true when the plural msgid contains a `%`' do
      data = { msgid_plural: '「100%確定」' }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry).to receive(:contains_unescaped_chars?).and_call_original
      expect(entry.plural_id_contains_unescaped_chars?).to be_truthy
@@ -197,7 +197,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  describe '#translations_contain_unescaped_chars' do
    it 'is true when the translation contains a `%`' do
      data = { msgstr: '「100%確定」' }
-      entry = described_class.new(entry_data: data, nplurals: 2, html_allowed: nil)
+      entry = described_class.new(entry_data: data, nplurals: 2)
      expect(entry).to receive(:contains_unescaped_chars?).and_call_original
      expect(entry.translations_contain_unescaped_chars?).to be_truthy
@@ -205,7 +205,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  end
  describe '#msgid_contains_potential_html?' do
-    subject(:entry) { described_class.new(entry_data: data, nplurals: 2, html_allowed: nil) }
+    subject(:entry) { described_class.new(entry_data: data, nplurals: 2) }
    context 'when there are no angle brackets in the msgid' do
      let(:data) { { msgid: 'String with no brackets' } }
@@ -225,7 +225,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  end
  describe '#plural_id_contains_potential_html?' do
-    subject(:entry) { described_class.new(entry_data: data, nplurals: 2, html_allowed: nil) }
+    subject(:entry) { described_class.new(entry_data: data, nplurals: 2) }
    context 'when there are no angle brackets in the plural_id' do
      let(:data) { { msgid_plural: 'String with no brackets' } }
@@ -245,7 +245,7 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
  end
  describe '#translations_contain_potential_html?' do
-    subject(:entry) { described_class.new(entry_data: data, nplurals: 2, html_allowed: nil) }
+    subject(:entry) { described_class.new(entry_data: data, nplurals: 2) }
    context 'when there are no angle brackets in the translations' do
      let(:data) { { msgstr: 'This string has no angle brackets' } }
@@ -263,78 +263,4 @@ RSpec.describe Gitlab::I18n::TranslationEntry do
      end
    end
  end
-  describe '#msgid_html_allowed?' do
-    subject(:entry) do
-      described_class.new(entry_data: { msgid: 'String with a <strong>' }, nplurals: 2, html_allowed: html_todo)
-    end
-    context 'when the html in the string is in the todolist' do
-      let(:html_todo) { { 'plural_id' => nil, 'translations' => [] } }
-      it 'returns true' do
-        expect(entry.msgid_html_allowed?).to be true
-      end
-    end
-    context 'when the html in the string is not in the todolist' do
-      let(:html_todo) { nil }
-      it 'returns false' do
-        expect(entry.msgid_html_allowed?).to be false
-      end
-    end
-  end
-  describe '#plural_id_html_allowed?' do
-    subject(:entry) do
-      described_class.new(entry_data: { msgid_plural: 'String with many <strong>' }, nplurals: 2, html_allowed: html_todo)
-    end
-    context 'when the html in the string is in the todolist' do
-      let(:html_todo) { { 'plural_id' => 'String with many <strong>', 'translations' => [] } }
-      it 'returns true' do
-        expect(entry.plural_id_html_allowed?).to be true
-      end
-    end
-    context 'when the html in the string is not in the todolist' do
-      let(:html_todo) { { 'plural_id' => 'String with some <strong>', 'translations' => [] } }
-      it 'returns false' do
-        expect(entry.plural_id_html_allowed?).to be false
-      end
-    end
-  end
-  describe '#translations_html_allowed?' do
-    subject(:entry) do
-      described_class.new(entry_data: { msgstr: 'String with a <strong>' }, nplurals: 2, html_allowed: html_todo)
-    end
-    context 'when the html in the string is in the todolist' do
-      let(:html_todo) { { 'plural_id' => nil, 'translations' => ['String with a <strong>'] } }
-      it 'returns true' do
-        expect(entry.translations_html_allowed?).to be true
-      end
-    end
-    context 'when the html in the string is not in the todolist' do
-      let(:html_todo) { { 'plural_id' => nil, 'translations' => ['String with a different <strong>'] } }
-      it 'returns false' do
-        expect(entry.translations_html_allowed?).to be false
-      end
-    end
-    context 'when the todolist only has the msgid' do
-      let(:html_todo) { { 'plural_id' => nil, 'translations' => nil } }
-      it 'returns false' do
-        expect(entry.translations_html_allowed?).to be false
-      end
-    end
-  end
 end