Skip to content

Commit 9116a72

Browse files
authored
fix(download): send explicit false export options (#1111)
1 parent 143d66b commit 9116a72

2 files changed

Lines changed: 115 additions & 14 deletions

File tree

src-next/cli/commands/download/DownloadCommand.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ interface SourcesOptions extends GlobalOptions {
6868
}
6969

7070
interface ExportCombo {
71-
skipUntranslatedStrings: boolean;
72-
skipUntranslatedFiles: boolean;
73-
exportApprovedOnly: boolean;
74-
exportStringsThatPassedWorkflow: boolean;
71+
skipUntranslatedStrings?: boolean;
72+
skipUntranslatedFiles?: boolean;
73+
exportApprovedOnly?: boolean;
74+
exportStringsThatPassedWorkflow?: boolean;
7575
}
7676

7777
export default class DownloadCommand {
@@ -519,6 +519,9 @@ export default class DownloadCommand {
519519
* `distinct()` over the four per-file flags) so each combo is built once and only its own file
520520
* groups are mapped from that archive. A CLI flag (when set) overrides the per-file config value
521521
* on every file, forcing `true` — picocli/commander boolean flags can only force true, never false.
522+
*
523+
* Combo values are tri-state: an unset option is left out of the build request so the project
524+
* export settings apply, while an explicit `false` is sent and overrides them.
522525
*/
523526
private buildExportGroups(
524527
config: Config,
@@ -558,10 +561,10 @@ export default class DownloadCommand {
558561
}
559562

560563
const effectiveCombo = (file: Config['files'][number]): ExportCombo => ({
561-
skipUntranslatedStrings: options.skipUntranslatedStrings ? true : (file.skip_untranslated_strings ?? false),
562-
skipUntranslatedFiles: options.skipUntranslatedFiles ? true : (file.skip_untranslated_files ?? false),
563-
exportApprovedOnly: options.exportOnlyApproved ? true : (file.export_only_approved ?? false),
564-
exportStringsThatPassedWorkflow: file.export_strings_that_passed_workflow ?? false,
564+
skipUntranslatedStrings: options.skipUntranslatedStrings ? true : file.skip_untranslated_strings,
565+
skipUntranslatedFiles: options.skipUntranslatedFiles ? true : file.skip_untranslated_files,
566+
exportApprovedOnly: options.exportOnlyApproved ? true : file.export_only_approved,
567+
exportStringsThatPassedWorkflow: file.export_strings_that_passed_workflow,
565568
});
566569

567570
// Group files by distinct combo, preserving first-seen order (mirrors Java distinct()).
@@ -590,15 +593,16 @@ export default class DownloadCommand {
590593
request.targetLanguageIds = resolvedLanguageIds;
591594
}
592595

593-
if (combo.skipUntranslatedStrings) {
594-
request.skipUntranslatedStrings = true;
596+
if (combo.skipUntranslatedStrings !== undefined) {
597+
request.skipUntranslatedStrings = combo.skipUntranslatedStrings;
595598
}
596599

597-
if (combo.skipUntranslatedFiles) {
598-
request.skipUntranslatedFiles = true;
600+
if (combo.skipUntranslatedFiles !== undefined) {
601+
request.skipUntranslatedFiles = combo.skipUntranslatedFiles;
599602
}
600603

601604
if (isOrganization) {
605+
// Enterprise has no way to force approvals off, so only `true` maps to a request field.
602606
if (combo.exportApprovedOnly) {
603607
request.exportWithMinApprovalsCount = 1;
604608
}
@@ -607,8 +611,8 @@ export default class DownloadCommand {
607611
request.exportStringsThatPassedWorkflow = true;
608612
}
609613
} else {
610-
if (combo.exportApprovedOnly) {
611-
request.exportApprovedOnly = true;
614+
if (combo.exportApprovedOnly !== undefined) {
615+
request.exportApprovedOnly = combo.exportApprovedOnly;
612616
}
613617

614618
if (combo.exportStringsThatPassedWorkflow) {

tests/cli/commands/download/DownloadCommand.test.ts

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,6 +1128,103 @@ describe('DownloadCommand', () => {
11281128
expect(apiClient.translationsApi.buildProject).toHaveBeenCalledWith(123, { skipUntranslatedFiles: true });
11291129
});
11301130

1131+
test('sends explicit false export options so project settings are overridden', async () => {
1132+
await Bun.write(join(tempDir, 'resources/en/messages.json'), '{}');
1133+
mockBuildAndDownload([language('fr', 'fr-FR')]);
1134+
mockZipEntries([{ entryName: 'resources/fr-FR/messages.json', content: 'translated' }]);
1135+
1136+
const downloadCommand = createCommandFor({
1137+
...config,
1138+
files: [
1139+
{
1140+
source: '/resources/en/*.json',
1141+
translation: '/resources/%locale%/%original_file_name%',
1142+
skip_untranslated_strings: false,
1143+
skip_untranslated_files: false,
1144+
export_only_approved: false,
1145+
},
1146+
],
1147+
});
1148+
1149+
await downloadCommand.translationsAction(commandContext);
1150+
1151+
expect(apiClient.translationsApi.buildProject).toHaveBeenCalledWith(123, {
1152+
skipUntranslatedStrings: false,
1153+
skipUntranslatedFiles: false,
1154+
exportApprovedOnly: false,
1155+
});
1156+
});
1157+
1158+
test('omits export options that are not set in the config', async () => {
1159+
await Bun.write(join(tempDir, 'resources/en/messages.json'), '{}');
1160+
mockBuildAndDownload([language('fr', 'fr-FR')]);
1161+
mockZipEntries([{ entryName: 'resources/fr-FR/messages.json', content: 'translated' }]);
1162+
1163+
const downloadCommand = createCommandFor({
1164+
...config,
1165+
files: [{ source: '/resources/en/*.json', translation: '/resources/%locale%/%original_file_name%' }],
1166+
});
1167+
1168+
await downloadCommand.translationsAction(commandContext);
1169+
1170+
expect(apiClient.translationsApi.buildProject).toHaveBeenCalledWith(123, {});
1171+
});
1172+
1173+
test('keeps an explicit false and an unset option as separate builds', async () => {
1174+
await Bun.write(join(tempDir, 'resources/en/a.json'), '{}');
1175+
await Bun.write(join(tempDir, 'resources/en/b.json'), '{}');
1176+
mockBuildAndDownload([language('fr', 'fr-FR')]);
1177+
mockZipEntries([
1178+
{ entryName: 'resources/fr-FR/a.json', content: 'a-fr' },
1179+
{ entryName: 'resources/fr-FR/b.json', content: 'b-fr' },
1180+
]);
1181+
1182+
commandContext = createCommandContext({ ...globalOptions, ignoreMatch: true });
1183+
1184+
const downloadCommand = createCommandFor({
1185+
...config,
1186+
files: [
1187+
{
1188+
source: '/resources/en/a.json',
1189+
translation: '/resources/%locale%/%original_file_name%',
1190+
skip_untranslated_files: false,
1191+
},
1192+
{
1193+
source: '/resources/en/b.json',
1194+
translation: '/resources/%locale%/%original_file_name%',
1195+
},
1196+
],
1197+
});
1198+
1199+
await downloadCommand.translationsAction(commandContext);
1200+
1201+
expect(apiClient.translationsApi.buildProject).toHaveBeenCalledTimes(2);
1202+
expect(apiClient.translationsApi.buildProject).toHaveBeenCalledWith(123, { skipUntranslatedFiles: false });
1203+
expect(apiClient.translationsApi.buildProject).toHaveBeenCalledWith(123, {});
1204+
});
1205+
1206+
test('ignores export_only_approved: false on enterprise, which cannot force approvals off', async () => {
1207+
spyOn(projectService, 'isEnterprise').mockReturnValue(true);
1208+
await Bun.write(join(tempDir, 'resources/en/messages.json'), '{}');
1209+
mockBuildAndDownload([language('fr', 'fr-FR')]);
1210+
mockZipEntries([{ entryName: 'resources/fr-FR/messages.json', content: 'translated' }]);
1211+
1212+
const downloadCommand = createCommandFor({
1213+
...config,
1214+
files: [
1215+
{
1216+
source: '/resources/en/*.json',
1217+
translation: '/resources/%locale%/%original_file_name%',
1218+
export_only_approved: false,
1219+
},
1220+
],
1221+
});
1222+
1223+
await downloadCommand.translationsAction(commandContext);
1224+
1225+
expect(apiClient.translationsApi.buildProject).toHaveBeenCalledWith(123, {});
1226+
});
1227+
11311228
test('maps export_only_approved to exportWithMinApprovalsCount on enterprise', async () => {
11321229
spyOn(projectService, 'isEnterprise').mockReturnValue(true);
11331230
await Bun.write(join(tempDir, 'resources/en/messages.json'), '{}');

0 commit comments

Comments
 (0)