Skip to content

Commit d36f88c

Browse files
authored
Merge branch 'trunk' into add/buildkite-build-trigger-action
2 parents db5ed2c + 08602aa commit d36f88c

File tree

7 files changed

+407
-9
lines changed

7 files changed

+407
-9
lines changed

.rubocop_todo.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ RSpec/FilePath:
146146
- 'spec/ios_l10n_helper_spec.rb'
147147
- 'spec/ios_merge_strings_files_spec.rb'
148148
- 'spec/gp_update_metadata_source_spec.rb'
149+
- 'spec/ios_download_strings_files_from_glotpress_spec.rb'
149150

150151
# Offense count: 8
151152
# Cop supports --auto-correct.

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
* Introduce new `ios_merge_strings_files` action. [#329]
1414
* Introduce new `buildkite_trigger_build` action. [#333]
15+
* Introduce new `ios_download_strings_files_from_glotpress` action. [#331]
1516

1617
### Bug Fixes
1718

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
module Fastlane
2+
module Actions
3+
class IosDownloadStringsFilesFromGlotpressAction < Action
4+
def self.run(params)
5+
# TODO: Once we introduce the `Locale` POD via #296, check if the param is an array of locales and if so convert it to Hash{glotpress=>lproj}
6+
locales = params[:locales]
7+
download_dir = params[:download_dir]
8+
9+
UI.user_error!("The parent directory `#{download_dir}` (which contains all the `*.lproj` subdirectories) must already exist") unless Dir.exist?(download_dir)
10+
11+
locales.each do |glotpress_locale, lproj_name|
12+
# Download the export in the proper `.lproj` directory
13+
lproj_dir = File.join(download_dir, "#{lproj_name}.lproj")
14+
destination = File.join(lproj_dir, "#{params[:table_basename]}.strings")
15+
FileUtils.mkdir(lproj_dir) unless Dir.exist?(lproj_dir)
16+
17+
Fastlane::Helper::Ios::L10nHelper.download_glotpress_export_file(
18+
project_url: params[:project_url],
19+
locale: glotpress_locale,
20+
filters: params[:filters],
21+
destination: destination
22+
)
23+
# Do a quick check of the downloaded `.strings` file to ensure it looks valid
24+
validate_strings_file(destination) unless params[:skip_file_validation]
25+
end
26+
end
27+
28+
# Validate that a `.strings` file downloaded from GlotPress seems valid and does not contain empty translations
29+
def self.validate_strings_file(destination)
30+
return unless File.exist?(destination) # If the file failed to download, don't try to validate an non-existing file. We'd already have a separate error for the download failure anyway.
31+
32+
translations = Fastlane::Helper::Ios::L10nHelper.read_strings_file_as_hash(path: destination)
33+
empty_keys = translations.select { |_, value| value.nil? || value.empty? }.keys.sort
34+
unless empty_keys.empty?
35+
UI.error(
36+
"Found empty translations in `#{destination}` for the following keys: #{empty_keys.inspect}.\n" \
37+
+ "This is likely a GlotPress bug, and will lead to copies replaced by empty text in the UI.\n" \
38+
+ 'Please report this to the GlotPress team, and fix the file locally before continuing.'
39+
)
40+
end
41+
rescue StandardError => e
42+
UI.error("Error while validating the file exported from GlotPress (`#{destination}`) - #{e.message.chomp}")
43+
end
44+
45+
#####################################################
46+
# @!group Documentation
47+
#####################################################
48+
49+
def self.description
50+
'Downloads the `.strings` files from GlotPress for the various locales'
51+
end
52+
53+
def self.details
54+
<<~DETAILS
55+
Downloads the `.strings` files from GlotPress for the various locales,
56+
validates them, and saves them in the relevant `*.lproj` directories for each locale
57+
DETAILS
58+
end
59+
60+
def self.available_options
61+
[
62+
FastlaneCore::ConfigItem.new(key: :project_url,
63+
env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_PROJECT_URL',
64+
description: 'URL to the GlotPress project',
65+
type: String),
66+
FastlaneCore::ConfigItem.new(key: :locales,
67+
env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_LOCALES',
68+
description: 'The map of locales to download, each entry of the Hash corresponding to a { glotpress-locale-code => lproj-folder-basename } pair',
69+
type: Hash), # TODO: also support an Array of `Locale` POD/struct type when we introduce it later (see #296)
70+
FastlaneCore::ConfigItem.new(key: :download_dir,
71+
env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_DOWNLOAD_DIR',
72+
description: 'The parent directory containing all the `*.lproj` subdirectories in which the downloaded files will be saved',
73+
type: String),
74+
FastlaneCore::ConfigItem.new(key: :table_basename,
75+
env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_TABLE_BASENAME',
76+
description: 'The basename to save the `.strings` files under',
77+
type: String,
78+
optional: true,
79+
default_value: 'Localizable'),
80+
FastlaneCore::ConfigItem.new(key: :filters,
81+
env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_FILTERS',
82+
description: 'The GlotPress filters to use when requesting the translations export',
83+
type: Hash,
84+
optional: true,
85+
default_value: { status: 'current' }),
86+
FastlaneCore::ConfigItem.new(key: :skip_file_validation,
87+
env_name: 'FL_IOS_DOWNLOAD_STRINGS_FILES_FROM_GLOTPRESS_SKIP_FILE_VALIDATION',
88+
description: 'If true, skips the validation of `.strings` files after download',
89+
type: Fastlane::Boolean,
90+
optional: true,
91+
default_value: false),
92+
]
93+
end
94+
95+
def self.return_type
96+
# Describes what type of data is expected to be returned
97+
# see RETURN_TYPES in https://github.com/fastlane/fastlane/blob/master/fastlane/lib/fastlane/action.rb
98+
end
99+
100+
def self.return_value
101+
# Textual description of what the return value is
102+
end
103+
104+
def self.authors
105+
['Automattic']
106+
end
107+
108+
def self.is_supported?(platform)
109+
[:ios, :mac].include?(platform)
110+
end
111+
end
112+
end
113+
end

lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
require 'open3'
2-
require 'tempfile'
31
require 'fileutils'
4-
require 'nokogiri'
52
require 'json'
3+
require 'nokogiri'
4+
require 'open3'
5+
require 'open-uri'
6+
require 'tempfile'
67

78
module Fastlane
89
module Helper
@@ -116,6 +117,25 @@ def self.generate_strings_file_from_hash(translations:, output_path:)
116117
end
117118
File.write(output_path, builder.to_xml)
118119
end
120+
121+
# Downloads the export from GlotPress for a given locale and given filters.
122+
#
123+
# @param [String] project_url The URL to the GlotPress project to export from, e.g. `"https://translate.wordpress.org/projects/apps/ios/dev"`
124+
# @param [String] locale The GlotPress locale code to download strings for.
125+
# @param [Hash{Symbol=>String}] filters The hash of filters to apply when exporting from GlotPress.
126+
# Typical examples include `{ status: 'current' }` or `{ status: 'review' }`.
127+
# @param [String, IO] destination The path or `IO`-like instance, where to write the downloaded file on disk.
128+
#
129+
def self.download_glotpress_export_file(project_url:, locale:, filters:, destination:)
130+
query_params = (filters || {}).transform_keys { |k| "filters[#{k}]" }.merge(format: 'strings')
131+
uri = URI.parse("#{project_url.chomp('/')}/#{locale}/default/export-translations?#{URI.encode_www_form(query_params)}")
132+
begin
133+
IO.copy_stream(uri.open, destination)
134+
rescue StandardError => e
135+
UI.error "Error downloading locale `#{locale}` — #{e.message}"
136+
return nil
137+
end
138+
end
119139
end
120140
end
121141
end

spec/an_localize_libs_action_spec.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,10 @@
2626
]
2727
)
2828

29-
# Notice the extra indentation in the library strings. The action doesn't
30-
# modify the app's strings content indentation, but it applies its own
31-
# standard to the values read from the given library strings
3229
expected = <<~XML
3330
<string name="a_string">test from app</string>
34-
<string name="a_lib1_string">test from lib 1</string>
35-
<string name="a_lib2_string">test from lib 2</string>
31+
<string name="a_lib1_string">test from lib 1</string>
32+
<string name="a_lib2_string">test from lib 2</string>
3633
XML
3734
expect(File.read(app_strings_path)).to eq(android_xml_with_content(expected))
3835
end
@@ -43,7 +40,10 @@ def android_xml_with_content(content)
4340
# I couldn't find a way to interpolate a multiline string preserving its
4441
# indentation in the heredoc below, so I hacked the following transformation
4542
# of the input that adds the desired indentation to all lines.
46-
indented_content = content.chomp.lines.map { |l| " #{l}" }.join
43+
#
44+
# The desired indentation is 4 spaces to stay aligned with the production
45+
# code applies when merging the XMLs.
46+
indented_content = content.chomp.lines.map { |l| " #{l}" }.join
4747

4848
return <<~XML
4949
<?xml version="1.0" encoding="UTF-8"?>
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
require 'spec_helper'
2+
require 'tmpdir'
3+
4+
describe Fastlane::Actions::IosDownloadStringsFilesFromGlotpressAction do
5+
let(:test_data_dir) { File.join(File.dirname(__FILE__), 'test-data', 'translations', 'ios_generate_strings_file_from_code') }
6+
let(:gp_fake_url) { 'https://stub.glotpress.com/rspec-fake-project' }
7+
let(:locales_subset) { { 'fr-FR': 'fr', 'zh-cn': 'zh-Hans' } }
8+
9+
def gp_stub(locale:, query:)
10+
stub_request(:get, "#{gp_fake_url}/#{locale}/default/export-translations").with(query: query)
11+
end
12+
13+
describe 'downloading export files from GlotPress' do
14+
def test_gp_download(filters:, tablename:, expected_gp_params:)
15+
Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
16+
# Arrange
17+
stub_fr = gp_stub(locale: 'fr-FR', query: expected_gp_params).to_return(body: '"key" = "fr-copy";')
18+
stub_zh = gp_stub(locale: 'zh-cn', query: expected_gp_params).to_return(body: '"key" = "zh-copy";')
19+
20+
# Act
21+
# Note: The use of `.compact` here allows us to remove the keys (like `table_basename` and `filters`) from the `Hash` whose values are `nil`,
22+
# so that we don't include those parameters at all in the action's call site -- making them use the default value from their respective
23+
# `ConfigItem` (as opposed to passing a value of `nil` explicitly and overwrite the default value, which is not what we want to test).
24+
run_described_fastlane_action({
25+
project_url: gp_fake_url,
26+
locales: locales_subset,
27+
download_dir: tmp_dir,
28+
table_basename: tablename,
29+
filters: filters
30+
}.compact)
31+
32+
# Assert
33+
expect(stub_fr).to have_been_made.once
34+
file_fr = File.join(tmp_dir, 'fr.lproj', "#{tablename || 'Localizable'}.strings")
35+
expect(File).to exist(file_fr)
36+
expect(File.read(file_fr)).to eq('"key" = "fr-copy";')
37+
38+
expect(stub_zh).to have_been_made.once
39+
file_zh = File.join(tmp_dir, 'zh-Hans.lproj', "#{tablename || 'Localizable'}.strings")
40+
expect(File).to exist(file_zh)
41+
expect(File.read(file_zh)).to eq('"key" = "zh-copy";')
42+
end
43+
end
44+
45+
it 'downloads all the locales into the expected directories' do
46+
test_gp_download(filters: nil, tablename: nil, expected_gp_params: { 'filters[status]': 'current', format: 'strings' })
47+
end
48+
49+
it 'uses the proper filters when exporting the files from GlotPress' do
50+
test_gp_download(
51+
filters: { term: 'foo', status: 'review' },
52+
tablename: nil,
53+
expected_gp_params: { 'filters[term]': 'foo', 'filters[status]': 'review', format: 'strings' }
54+
)
55+
end
56+
57+
it 'uses a custom table name for the `.strings` files if provided' do
58+
test_gp_download(
59+
filters: nil,
60+
tablename: 'MyApp',
61+
expected_gp_params: { 'filters[status]': 'current', format: 'strings' }
62+
)
63+
end
64+
end
65+
66+
describe 'error handling' do
67+
it 'shows an error if an invalid locale is provided (404)' do
68+
Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
69+
# Arrange
70+
stub = gp_stub(locale: 'unknown-locale', query: { 'filters[status]': 'current', format: 'strings' }).to_return(status: [404, 'Not Found'])
71+
error_messages = []
72+
allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
73+
74+
# Act
75+
run_described_fastlane_action(
76+
project_url: gp_fake_url,
77+
locales: { 'unknown-locale': 'Base' },
78+
download_dir: tmp_dir
79+
)
80+
81+
# Assert
82+
expect(stub).to have_been_made.once
83+
expect(File).not_to exist(File.join(tmp_dir, 'Base.lproj', 'Localizable.strings'))
84+
expect(error_messages).to eq(['Error downloading locale `unknown-locale` — 404 Not Found'])
85+
end
86+
end
87+
88+
it 'shows an error if the file cannot be written in the destination' do
89+
# Arrange
90+
download_dir = '/these/are/not/the/dirs/you/are/looking/for/'
91+
92+
# Act
93+
act = lambda do
94+
run_described_fastlane_action(
95+
project_url: gp_fake_url,
96+
locales: { 'fr-FR': 'fr' },
97+
download_dir: download_dir
98+
)
99+
end
100+
101+
# Assert
102+
# Note: FastlaneError is the exception raised by UI.user_error!
103+
expect { act.call }.to raise_error(FastlaneCore::Interface::FastlaneError, "The parent directory `#{download_dir}` (which contains all the `*.lproj` subdirectories) must already exist")
104+
end
105+
106+
it 'reports if a downloaded file is not a valid `.strings` file' do
107+
Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
108+
# Arrange
109+
stub = gp_stub(locale: 'fr-FR', query: { 'filters[status]': 'current', format: 'strings' }).to_return(body: 'some invalid strings file content')
110+
error_messages = []
111+
allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
112+
113+
# Act
114+
run_described_fastlane_action(
115+
project_url: gp_fake_url,
116+
locales: { 'fr-FR': 'fr' },
117+
download_dir: tmp_dir
118+
)
119+
120+
# Assert
121+
expect(stub).to have_been_made.once
122+
file = File.join(tmp_dir, 'fr.lproj', 'Localizable.strings')
123+
expect(File).to exist(file)
124+
expected_error = 'Property List error: Unexpected character s at line 1 / JSON error: JSON text did not start with array or object and option to allow fragments not set.'
125+
expect(error_messages.count).to eq(1)
126+
expect(error_messages.first).to start_with("Error while validating the file exported from GlotPress (`#{file}`) - #{file}: #{expected_error}") # Different versions of `plutil` might append the line/column as well, but not all.
127+
end
128+
end
129+
130+
it 'reports if a downloaded file has empty translations' do
131+
Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
132+
# Arrange
133+
stub = gp_stub(locale: 'fr-FR', query: { 'filters[status]': 'current', format: 'strings' })
134+
.to_return(body: ['"key1" = "value1";', '"key2" = "";', '"key3" = "";', '/* translators: use "" quotes please */', '"key4" = "value4";'].join("\n"))
135+
error_messages = []
136+
allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
137+
138+
# Act
139+
run_described_fastlane_action(
140+
project_url: gp_fake_url,
141+
locales: { 'fr-FR': 'fr' },
142+
download_dir: tmp_dir
143+
)
144+
145+
# Assert
146+
expect(stub).to have_been_made.once
147+
file = File.join(tmp_dir, 'fr.lproj', 'Localizable.strings')
148+
expect(File).to exist(file)
149+
expected_error = <<~MSG.chomp
150+
Found empty translations in `#{file}` for the following keys: ["key2", "key3"].
151+
This is likely a GlotPress bug, and will lead to copies replaced by empty text in the UI.
152+
Please report this to the GlotPress team, and fix the file locally before continuing.
153+
MSG
154+
expect(error_messages).to eq([expected_error])
155+
end
156+
end
157+
158+
it 'does not report invalid downloaded files if `skip_file_validation:true`' do
159+
Dir.mktmpdir('a8c-release-toolkit-tests-') do |tmp_dir|
160+
# Arrange
161+
stub = gp_stub(locale: 'fr-FR', query: { 'filters[status]': 'current', format: 'strings' }).to_return(body: 'some invalid strings file content')
162+
error_messages = []
163+
allow(FastlaneCore::UI).to receive(:error) { |message| error_messages.append(message) }
164+
165+
# Act
166+
act = lambda do
167+
run_described_fastlane_action(
168+
project_url: gp_fake_url,
169+
locales: { 'fr-FR': 'fr' },
170+
download_dir: tmp_dir,
171+
skip_file_validation: true
172+
)
173+
end
174+
175+
# Assert
176+
expect { act.call }.not_to raise_error
177+
expect(stub).to have_been_made.once
178+
file = File.join(tmp_dir, 'fr.lproj', 'Localizable.strings')
179+
expect(File).to exist(file)
180+
expect(error_messages).to eq([])
181+
end
182+
end
183+
end
184+
end

0 commit comments

Comments
 (0)