Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add task command: remove skip untranslated strings option #874

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/com/crowdin/cli/commands/Actions.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ NewAction<BaseProperties, ClientTm> tmDownload(

NewAction<ProjectProperties, ClientTask> taskAdd(
boolean noProgress, String title, Integer type, String language, List<String> files, String branch, Long workflowStep,
String description, boolean skipAssignedStrings, boolean skipUntranslatedStrings, boolean includePreTranslatedStringsOnly,
String description, Boolean skipAssignedStrings, Boolean includePreTranslatedStringsOnly,
List<Long> labels, ProjectClient projectClient, boolean plainView);

NewAction<ProjectProperties, ClientDistribution> distributionList(boolean plainView);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,10 @@ public NewAction<ProjectProperties, ClientTask> taskList(boolean plainView, bool

@Override
public NewAction<ProjectProperties, ClientTask> taskAdd(boolean noProgress, String title, Integer type, String language,
List<String> files, String branch, Long workflowStep, String description, boolean skipAssignedStrings,
boolean skipUntranslatedStrings, boolean includePreTranslatedStringsOnly, List<Long> labels, ProjectClient projectClient, boolean plainView
List<String> files, String branch, Long workflowStep, String description, Boolean skipAssignedStrings,
Boolean includePreTranslatedStringsOnly, List<Long> labels, ProjectClient projectClient, boolean plainView
) {
return new TaskAddAction(noProgress, title, type, language, files, branch, workflowStep, description, skipAssignedStrings,
skipUntranslatedStrings, includePreTranslatedStringsOnly, labels, projectClient, plainView);
return new TaskAddAction(noProgress, title, type, language, files, branch, workflowStep, description, skipAssignedStrings, includePreTranslatedStringsOnly, labels, projectClient, plainView);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,8 @@ class TaskAddAction implements NewAction<ProjectProperties, ClientTask> {
private String branch;
private Long workflowStep;
private String description;
private boolean skipAssignedStrings;
private boolean skipUntranslatedStrings;
private boolean includePreTranslatedStringsOnly;
private Boolean skipAssignedStrings;
private Boolean includePreTranslatedStringsOnly;
private List<Long> labels;
private ProjectClient projectClient;
private boolean plainView;
Expand Down Expand Up @@ -82,7 +81,6 @@ public void act(Outputter out, ProjectProperties pb, ClientTask client) {
Optional.ofNullable(fileIds).ifPresent(value -> ((CreateTaskRequest) addTaskRequest).setFileIds(value));
Optional.ofNullable(description).ifPresent(value -> ((CreateTaskRequest) addTaskRequest).setDescription(value));
Optional.ofNullable(skipAssignedStrings).ifPresent(value -> ((CreateTaskRequest) addTaskRequest).setSkipAssignedStrings(value));
Optional.ofNullable(skipUntranslatedStrings).ifPresent(value -> ((CreateTaskRequest) addTaskRequest).setSkipUntranslatedStrings(value));
Optional.ofNullable(includePreTranslatedStringsOnly).ifPresent(value -> ((CreateTaskRequest) addTaskRequest).setIncludePreTranslatedStringsOnly(value));
Optional.ofNullable(labels).ifPresent(value -> ((CreateTaskRequest) addTaskRequest).setLabelIds(value));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,14 @@ public static AddDistributionRequest addDistribution(String name, ExportMode exp
return request;
}

public static CreateTaskRequest addCrowdinTask(String title, com.crowdin.client.tasks.model.Type type, String languageId, List<Long> fileId, String description, boolean skipAssignedStrings, boolean skipUntranslatedStrings, boolean includePreTranslatedStringsOnly, List<Long> labelIds) {
public static CreateTaskRequest addCrowdinTask(String title, com.crowdin.client.tasks.model.Type type, String languageId, List<Long> fileId, String description, boolean skipAssignedStrings, boolean includePreTranslatedStringsOnly, List<Long> labelIds) {
CreateTaskRequest request = new CreateTaskRequest();
request.setTitle(title);
request.setType(type);
request.setLanguageId(languageId);
request.setFileIds(fileId);
request.setDescription(description);
request.setSkipAssignedStrings(skipAssignedStrings);
request.setSkipUntranslatedStrings(skipUntranslatedStrings);
request.setIncludePreTranslatedStringsOnly(includePreTranslatedStringsOnly);
request.setLabelIds(labelIds);
return request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

import java.util.*;

import static com.crowdin.cli.utils.console.ExecutionStatus.WARNING;
import static java.lang.System.out;

@CommandLine.Command(
Expand Down Expand Up @@ -47,13 +46,10 @@ class TaskAddSubcommand extends ActCommandTask {
protected String description;

@CommandLine.Option(names = {"--skip-assigned-strings"}, paramLabel = "...", negatable = true, descriptionKey = "crowdin.task.add.skip-assigned-strings", order = -2)
protected boolean skipAssignedStrings;

@CommandLine.Option(names = {"--skip-untranslated-strings"}, hidden = true, paramLabel = "...", negatable = true, descriptionKey = "crowdin.task.add.skip-untranslated-strings", order = -2)
protected boolean skipUntranslatedStrings;
protected Boolean skipAssignedStrings;

@CommandLine.Option(names = {"--include-pre-translated-strings-only"}, paramLabel = "...", negatable = true, descriptionKey = "crowdin.task.add.include-pre-translated-strings-only", order = -2)
protected boolean includePreTranslatedStringsOnly;
protected Boolean includePreTranslatedStringsOnly;

@CommandLine.Option(names = {"--label"}, paramLabel = "...", descriptionKey = "crowdin.task.add.label", order = -2)
protected List<Long> labels;
Expand All @@ -77,7 +73,6 @@ protected NewAction<ProjectProperties, ClientTask> getAction(Actions actions) {
workflowStep,
description,
skipAssignedStrings,
skipUntranslatedStrings,
includePreTranslatedStringsOnly,
labels,
projectClient,
Expand All @@ -87,9 +82,6 @@ protected NewAction<ProjectProperties, ClientTask> getAction(Actions actions) {

@Override
protected List<String> checkOptions() {
if (skipUntranslatedStrings) {
out.println(WARNING.withIcon(RESOURCE_BUNDLE.getString("message.skip-untranslated-strings_deprecated")));
}
String url = this.getProperties(propertiesBuilders, new PicocliOutputter(out, isAnsi())).getBaseUrl();
boolean isEnterprise = PropertiesBeanUtils.isOrganization(url);
return checkOptions(isEnterprise);
Expand All @@ -106,13 +98,6 @@ protected List<String> checkOptions(boolean isEnterprise) {
errors.add(RESOURCE_BUNDLE.getString("error.task.unsupported.type"));
}

if (TRANSLATE_TASK_TYPE.equalsIgnoreCase(type) && skipUntranslatedStrings) {
errors.add(RESOURCE_BUNDLE.getString("error.task.translate_type_skip_untranslated_strings"));
}

if (includePreTranslatedStringsOnly && !skipUntranslatedStrings) {
errors.add(RESOURCE_BUNDLE.getString("error.task.skip_untranslated_strings_include_pre_translated"));
}
} else {
if (workflowStep == null) {
errors.add(RESOURCE_BUNDLE.getString("error.task.empty_workflow_step"));
Expand All @@ -131,7 +116,7 @@ protected List<String> checkOptions(boolean isEnterprise) {
errors.add(RESOURCE_BUNDLE.getString("error.task.empty_file"));
}

if (TRANSLATE_TASK_TYPE.equalsIgnoreCase(type) && includePreTranslatedStringsOnly) {
if (TRANSLATE_TASK_TYPE.equalsIgnoreCase(type) && (includePreTranslatedStringsOnly != null && includePreTranslatedStringsOnly)) {
errors.add(RESOURCE_BUNDLE.getString("error.task.translate_type_include_pre_translated_strings"));
}

Expand Down
4 changes: 0 additions & 4 deletions src/main/resources/messages/messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,6 @@ crowdin.task.add.file=File path in the Crowdin project. Can be specified multipl
crowdin.task.add.workflow-step=Task workflow step (Crowdin Enterprise only)
crowdin.task.add.description=Task description
crowdin.task.add.skip-assigned-strings=Skip strings already included in other tasks
crowdin.task.add.skip-untranslated-strings=Defines whether to export only translated strings (crowdin.com only)
crowdin.task.add.include-pre-translated-strings-only=Defines whether to export only pre-translated strings
crowdin.task.add.label=Label identifier. Could be specified multiple times

Expand Down Expand Up @@ -585,9 +584,7 @@ error.task.empty_title=Task title can not be empty
error.task.empty_language=Language can not be empty. (e.g. es-ES, en-US)
error.task.empty_file=The '--file' value can not be empty
error.task.empty_workflow_step=Workflow step id can not be empty
error.task.translate_type_skip_untranslated_strings=The '--skip-untranslated-strings' option can't be used with the 'translate' task type
error.task.translate_type_include_pre_translated_strings=The '--include-pre-translated-strings-only' option can't be used with the 'translate' task type
error.task.skip_untranslated_strings_include_pre_translated=The '--include-pre-translated-strings-only' option should be used with the '--skip-untranslated-strings' option
error.task.no_valid_file=No valid file specified for the task. At least one valid file is required

error.bundle_is_not_added=Bundle was not added: '%s'
Expand Down Expand Up @@ -729,7 +726,6 @@ message.already_uploaded=Skipping file '%s' because it is already uploading/uplo
message.extracted_organization_name=Extracted organization name from provided url: %s
message.file_deleted=@|green File '%s' deleted|@
message.no_file_string_project=File management is not available for string-based projects
message.skip-untranslated-strings_deprecated=The '--skip-untranslated-strings' option is deprecated and will be removed in the future

message.download_sources.preserve_hierarchy_warning=Because the 'preserve_hierarchy' parameter is set to 'false':\n\t- CLI might download some unexpected files that match the pattern;\n\t- Source file hierarchy may not be preserved and will be the same as in Crowdin.
message.download_translations.preserve_hierarchy_warning=Because the 'preserve_hierarchy' parameter is set to 'false' CLI might download some unexpected files that match the pattern
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,23 @@
import com.crowdin.cli.client.ClientTask;
import com.crowdin.cli.client.ProjectBuilder;
import com.crowdin.cli.client.ProjectClient;
import com.crowdin.cli.client.ResponseException;
import com.crowdin.cli.commands.NewAction;
import com.crowdin.cli.commands.Outputter;
import com.crowdin.cli.commands.functionality.FilesInterface;
import com.crowdin.cli.commands.functionality.RequestBuilder;
import com.crowdin.cli.properties.NewPropertiesWithFilesUtilBuilder;
import com.crowdin.cli.properties.ProjectProperties;
import com.crowdin.cli.properties.PropertiesWithFiles;
import com.crowdin.cli.utils.Utils;
import com.crowdin.client.labels.model.Label;
import com.crowdin.client.sourcestrings.model.AddSourceStringRequest;
import com.crowdin.client.tasks.model.*;
import com.crowdin.client.tasks.model.CreateTaskEnterpriseRequest;
import com.crowdin.client.tasks.model.CreateTaskRequest;
import com.crowdin.client.tasks.model.Task;
import com.crowdin.client.tasks.model.Type;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.assertThrows;
Expand All @@ -39,7 +33,7 @@ public class TaskAddActionTest {
@ParameterizedTest
@MethodSource
public void testTaskAdd(String title, Integer type, String languageId, Map<String, Long> filesMap, List<String> files, String description,
boolean skipAssignedStrings, boolean skipUntranslatedStrings, boolean includePreTranslatedStringsOnly, List<Long> labelIds) {
boolean skipAssignedStrings, boolean includePreTranslatedStringsOnly, List<Long> labelIds) {
NewPropertiesWithFilesUtilBuilder pbBuilder = NewPropertiesWithFilesUtilBuilder
.minimalBuiltPropertiesBean("*", Utils.PATH_SEPARATOR + "%original_file_name%-CR-%locale%")
.setBasePath(Utils.PATH_SEPARATOR);
Expand All @@ -55,7 +49,7 @@ public void testTaskAdd(String title, Integer type, String languageId, Map<Strin
.thenReturn(projectBuilder.build());

CreateTaskRequest request = RequestBuilder.addCrowdinTask(title, Type.from(String.valueOf(type)), languageId, new ArrayList<>(filesMap.values()),
description, skipAssignedStrings, skipUntranslatedStrings, includePreTranslatedStringsOnly, labelIds);
description, skipAssignedStrings, includePreTranslatedStringsOnly, labelIds);

ClientTask client = mock(ClientTask.class);
when(client.addTask(request))
Expand All @@ -66,7 +60,7 @@ public void testTaskAdd(String title, Integer type, String languageId, Map<Strin
setTitle(request.getTitle());
}});

action = new TaskAddAction(true, title, type, languageId, files, null, null, description, skipAssignedStrings, skipUntranslatedStrings, includePreTranslatedStringsOnly, labelIds, projectClient, false);
action = new TaskAddAction(true, title, type, languageId, files, null, null, description, skipAssignedStrings, includePreTranslatedStringsOnly, labelIds, projectClient, false);
action.act(Outputter.getDefault(), pb, client);

verify(client).addTask(request);
Expand All @@ -83,7 +77,7 @@ public static Stream<Arguments> testTaskAdd() {
add("second.txt");
add("first.txt");
}};
return Stream.of(arguments("My title", 1, "es", filesMap, files, "It's description", false, false, false, null));
return Stream.of(arguments("My title", 1, "es", filesMap, files, "It's description", false, false, null));
}

@ParameterizedTest
Expand Down Expand Up @@ -118,7 +112,7 @@ public void testEnterpriseTaskAdd(String title, String languageId, Map<String, L
setTitle(request.getTitle());
}});

action = new TaskAddAction(false, title, null, languageId, files, null, workflowStepId, description, skipAssignedStrings, false, false, labelIds, projectClient, true);
action = new TaskAddAction(false, title, null, languageId, files, null, workflowStepId, description, skipAssignedStrings, false, labelIds, projectClient, true);
action.act(Outputter.getDefault(), pb, client);

verify(client).addTask(request);
Expand Down Expand Up @@ -148,12 +142,12 @@ public void testAddTaskThrows() {
.thenReturn(projectBuilder.build());

CreateTaskRequest request = RequestBuilder.addCrowdinTask(null, null, null,
Arrays.asList(1L), null, false, false, false, null);
Arrays.asList(1L), null, false, false, null);

when(client.addTask(request))
.thenThrow(new RuntimeException("Whoops"));

action = new TaskAddAction(false, null, null, null, Arrays.asList("file.txt"), null, null, null, false, false, false, null, projectClient, false);
action = new TaskAddAction(false, null, null, null, Arrays.asList("file.txt"), null, null, null, false, false, null, projectClient, false);

assertThrows(RuntimeException.class, () -> action.act(Outputter.getDefault(), pb, client));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ void mockActions() {
.thenReturn(actionMock);
when(actionsMock.taskList(anyBoolean(), anyBoolean(), any(), any()))
.thenReturn(actionMock);
when(actionsMock.taskAdd(anyBoolean(), any(), any(), any(), any(), any(), anyLong(), any(), anyBoolean(), anyBoolean(), anyBoolean(), any(), any(), anyBoolean()))
when(actionsMock.taskAdd(anyBoolean(), any(), any(), any(), any(), any(), anyLong(), any(), anyBoolean(), anyBoolean(), any(), any(), anyBoolean()))
.thenReturn(actionMock);
when(actionsMock.distributionList(anyBoolean()))
.thenReturn(actionMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public void testTaskAddInvalidOptions() {

@ParameterizedTest
@MethodSource
public void testSubCommandCheckValidOptions(boolean isEnterprise, String title, String type, Long workflowStep, String languageId, List<String> files) {
public void testSubCommandCheckValidOptions(boolean isEnterprise, String title, String type, Long workflowStep, String languageId, List<String> files) {
TaskAddSubcommand taskAddSubcommand = new TaskAddSubcommand();
taskAddSubcommand.title = title;
taskAddSubcommand.type = type;
Expand All @@ -34,7 +34,6 @@ public void testSubCommandCheckValidOptions(boolean isEnterprise, String title,
taskAddSubcommand.description = "";
taskAddSubcommand.labels = Arrays.asList(10L);
taskAddSubcommand.skipAssignedStrings = false;
taskAddSubcommand.skipUntranslatedStrings = false;

List<String> errors = taskAddSubcommand.checkOptions(isEnterprise);
assertTrue(errors.isEmpty());
Expand All @@ -61,7 +60,6 @@ public void testSubCommandCheckInvalidOptions(boolean isEnterprise, String title
taskAddSubcommand.description = "";
taskAddSubcommand.labels = Arrays.asList(10L);
taskAddSubcommand.skipAssignedStrings = false;
taskAddSubcommand.skipUntranslatedStrings = false;
List<String> errors = taskAddSubcommand.checkOptions(isEnterprise);
assertThat(errors, Matchers.equalTo(expErrors));
}
Expand Down
Loading