Merge "Migrate gr-diff-host_test from js to ts"
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 2e203e4..a61d86d 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3393,6 +3393,133 @@
HTTP/1.1 200 OK
----
+[[submit-requirement-endpoints]]
+== Submit Requirement Endpoints
+
+[[create-submit-requirement]]
+=== Create Submit Requirement
+
+--
+'PUT /projects/link:#project-name[\{project-name\}]/submit_requirements/link:#submit-requirement-name[\{submit-requirement-name\}]'
+--
+
+Creates a new submit requirement definition in this project.
+
+The calling user must have write access to the `refs/meta/config` branch of the
+project.
+
+If a submit requirement with this name is already defined in this project, this
+submit requirement definition is updated
+(see link:#update-submit-requirement[Update Submit Requirement]).
+
+The submit requirement data must be provided in the request body as
+link:#submit-requirement-input[SubmitRequirementInput].
+
+.Request
+----
+ PUT /projects/My-Project/submit_requirements/Code-Review HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "name": "Code-Review",
+ "description": "At least one maximum vote for the Code-Review label is required",
+ "submittability_expression": "label:Code-Review=+2"
+ }
+----
+
+As response a link:#submit-requirement-info[SubmitRequirementInfo] entity is
+returned that describes the created submit requirement.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "name": "Code-Review",
+ "description": "At least one maximum vote for the Code-Review label is required",
+ "submittability_expression": "label:Code-Review=+2",
+ "allow_override_in_child_projects": false
+ }
+----
+
+[[update-submit-requirement]]
+=== Update Submit Requirement
+
+--
+'PUT /projects/link:#project-name[\{project-name\}]/submit_requirements/link:#submit-requirement-name[\{submit-requirement-name\}]'
+--
+
+Updates the definition of a submit requirement that is defined in this project.
+
+The calling user must have write access to the `refs/meta/config` branch of the
+project.
+
+The new submit requirement will overwrite the existing submit requirement.
+That is, unspecified attributes will be set to their defaults.
+
+.Request
+----
+ PUT /projects/My-Project/submit_requirements/Code-Review HTTP/1.0
+ Content-Type: application/json; charset=UTF-8
+
+ {
+ "name": "Code-Review",
+ "description": "At least one maximum vote for the Code-Review label is required",
+ "submittability_expression": "label:Code-Review=+2"
+ }
+----
+
+As response a link:#submit-requirement-info[SubmitRequirementInfo] entity is
+returned that describes the created submit requirement.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "name": "Code-Review",
+ "description": "At least one maximum vote for the Code-Review label is required",
+ "submittability_expression": "label:Code-Review=+2",
+ "allow_override_in_child_projects": false
+ }
+----
+
+[[get-submit-requirement]]
+=== Get Submit Requirement
+--
+'GET /projects/link:#project-name[\{project-name\}]/submit_requirements/link:#submit-requirement-name[\{submit-requirement-name\}]'
+--
+
+Retrieves the definition of a submit requirement that is defined in this project.
+
+The calling user must have read access to the `refs/meta/config` branch of the
+project.
+
+.Request
+----
+ GET /projects/All-Projects/submit-requirement/Code-Review HTTP/1.0
+----
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "name": "Code-Review",
+ "description": "At least one maximum vote for the Code-Review label is required",
+ "submittability_expression": "label:Code-Review=+2",
+ "allow_override_in_child_projects": false
+ }
+----
[[ids]]
== IDs
@@ -3421,6 +3548,11 @@
=== \{label-name\}
The name of a review label.
+[[submit-requirement-name]]
+=== \{submit-requirement-name\}
+The name of a submit requirement.
+
+
[[project-name]]
=== \{project-name\}
The name of the project.
@@ -4366,6 +4498,57 @@
|`size_of_packed_objects` |Size of packed objects in bytes.
|======================================
+[[submit-requirement-info]]
+=== SubmitRequirementInfo
+The `SubmitRequirementInfo` entity describes a submit requirement.
+
+[options="header",cols="1,^1,5"]
+|===========================
+|Field Name ||Description
+|`name`||
+The submit requirement name.
+|`description`|optional|
+Description of the submit requirement.
+|`applicability_expression`|optional|
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is then applicable for this change.
+If not specified, the submit requirement is applicable for all changes.
+|`submittability_expression`||
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is fulfilled and not blocking change submission.
+|`override_expression`|optional|
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is overridden and not blocking change submission.
+|`allow_override_in_child_projects`||
+Whether this submit requirement can be overridden in child projects.
+|===========================
+
+[[submit-requirement-input]]
+=== SubmitRequirementInput
+The `SubmitRequirementInput` entity describes a submit requirement.
+
+[options="header",cols="1,^1,5"]
+|===========================
+|Field Name ||Description
+|`name`||
+The submit requirement name.
+|`description`|optional|
+Description of the submit requirement.
+|`applicability_expression`|optional|
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is then applicable for this change.
+If not specified, the submit requirement is applicable for all changes.
+|`submittability_expression`||
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is fulfilled and not blocking change submission.
+|`override_expression`|optional|
+Query expression that can be evaluated on any change. If evaluated to true on a
+change, the submit requirement is overridden and not blocking change submission.
+|`allow_override_in_child_projects`|optional|
+Whether this submit requirement can be overridden in child projects. Default is
+`false`.
+|===========================
+
[[submit-type-info]]
=== SubmitTypeInfo
Information about the link:config-project-config.html#submit-type[default submit
diff --git a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
index 59475a4..587c2c7 100644
--- a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
+++ b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
@@ -227,6 +227,8 @@
LabelApi label(String labelName) throws RestApiException;
+ SubmitRequirementApi submitRequirement(String name) throws RestApiException;
+
/**
* Adds, updates and deletes label definitions in a batch.
*
@@ -426,6 +428,11 @@
}
@Override
+ public SubmitRequirementApi submitRequirement(String name) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public void labels(BatchLabelInput input) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/extensions/api/projects/SubmitRequirementApi.java b/java/com/google/gerrit/extensions/api/projects/SubmitRequirementApi.java
new file mode 100644
index 0000000..a6e79db
--- /dev/null
+++ b/java/com/google/gerrit/extensions/api/projects/SubmitRequirementApi.java
@@ -0,0 +1,60 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.api.projects;
+
+import com.google.gerrit.extensions.common.SubmitRequirementInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.restapi.NotImplementedException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+
+public interface SubmitRequirementApi {
+ /** Create a new submit requirement. */
+ SubmitRequirementApi create(SubmitRequirementInput input) throws RestApiException;
+
+ /** Get existing submit requirement. */
+ SubmitRequirementInfo get() throws RestApiException;
+
+ /** Update existing submit requirement. */
+ SubmitRequirementInfo update(SubmitRequirementInput input) throws RestApiException;
+
+ /** Delete existing submit requirement. */
+ void delete() throws RestApiException;
+
+ /**
+ * A default implementation which allows source compatibility when adding new methods to the
+ * interface.
+ */
+ class NotImplemented implements SubmitRequirementApi {
+ @Override
+ public SubmitRequirementApi create(SubmitRequirementInput input) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public SubmitRequirementInfo get() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public SubmitRequirementInfo update(SubmitRequirementInput input) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public void delete() throws RestApiException {
+ throw new NotImplementedException();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementInfo.java b/java/com/google/gerrit/extensions/common/SubmitRequirementInfo.java
new file mode 100644
index 0000000..9347e7e
--- /dev/null
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementInfo.java
@@ -0,0 +1,45 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.extensions.common;
+
+public class SubmitRequirementInfo {
+ /** Name of the submit requirement. */
+ public String name;
+
+ /** Description of the submit requirement. */
+ public String description;
+
+ /**
+ * Expression string to be evaluated on a change. Decides if this submit requirement is applicable
+ * on the given change.
+ */
+ public String applicabilityExpression;
+
+ /**
+ * Expression string to be evaluated on a change. When evaluated to true, this submit requirement
+ * becomes fulfilled for this change.
+ */
+ public String submittabilityExpression;
+
+ /**
+ * Expression string to be evaluated on a change. When evaluated to true, this submit requirement
+ * becomes fulfilled for this change regardless of the evaluation of the {@link
+ * #submittabilityExpression}.
+ */
+ public String overrideExpression;
+
+ /** Boolean indicating if this submit requirement can be overridden in child projects. */
+ public boolean allowOverrideInChildProjects;
+}
diff --git a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
index 5baed86..f1620cc 100644
--- a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
+++ b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
@@ -41,6 +41,7 @@
import com.google.gerrit.extensions.api.projects.ParentInput;
import com.google.gerrit.extensions.api.projects.ProjectApi;
import com.google.gerrit.extensions.api.projects.ProjectInput;
+import com.google.gerrit.extensions.api.projects.SubmitRequirementApi;
import com.google.gerrit.extensions.api.projects.TagApi;
import com.google.gerrit.extensions.api.projects.TagInfo;
import com.google.gerrit.extensions.common.BatchLabelInput;
@@ -140,6 +141,7 @@
private final Provider<ListLabels> listLabels;
private final PostLabels postLabels;
private final LabelApiImpl.Factory labelApi;
+ private final SubmitRequirementApiImpl.Factory submitRequirementApi;
@AssistedInject
ProjectApiImpl(
@@ -179,6 +181,7 @@
Provider<ListLabels> listLabels,
PostLabels postLabels,
LabelApiImpl.Factory labelApi,
+ SubmitRequirementApiImpl.Factory submitRequirementApi,
@Assisted ProjectResource project) {
this(
permissionBackend,
@@ -218,6 +221,7 @@
listLabels,
postLabels,
labelApi,
+ submitRequirementApi,
null);
}
@@ -259,6 +263,7 @@
Provider<ListLabels> listLabels,
PostLabels postLabels,
LabelApiImpl.Factory labelApi,
+ SubmitRequirementApiImpl.Factory submitRequirementApi,
@Assisted String name) {
this(
permissionBackend,
@@ -298,6 +303,7 @@
listLabels,
postLabels,
labelApi,
+ submitRequirementApi,
name);
}
@@ -339,6 +345,7 @@
Provider<ListLabels> listLabels,
PostLabels postLabels,
LabelApiImpl.Factory labelApi,
+ SubmitRequirementApiImpl.Factory submitRequirementApi,
String name) {
this.permissionBackend = permissionBackend;
this.createProject = createProject;
@@ -378,6 +385,7 @@
this.listLabels = listLabels;
this.postLabels = postLabels;
this.labelApi = labelApi;
+ this.submitRequirementApi = submitRequirementApi;
}
@Override
@@ -746,6 +754,15 @@
}
@Override
+ public SubmitRequirementApi submitRequirement(String name) throws RestApiException {
+ try {
+ return submitRequirementApi.create(checkExists(), name);
+ } catch (Exception e) {
+ throw asRestApiException("Cannot parse submit requirement", e);
+ }
+ }
+
+ @Override
public void labels(BatchLabelInput input) throws RestApiException {
try {
postLabels.apply(checkExists(), input);
diff --git a/java/com/google/gerrit/server/api/projects/ProjectsModule.java b/java/com/google/gerrit/server/api/projects/ProjectsModule.java
index 987c71f..9f7e1b4 100644
--- a/java/com/google/gerrit/server/api/projects/ProjectsModule.java
+++ b/java/com/google/gerrit/server/api/projects/ProjectsModule.java
@@ -29,5 +29,6 @@
factory(CommitApiImpl.Factory.class);
factory(DashboardApiImpl.Factory.class);
factory(LabelApiImpl.Factory.class);
+ factory(SubmitRequirementApiImpl.Factory.class);
}
}
diff --git a/java/com/google/gerrit/server/api/projects/SubmitRequirementApiImpl.java b/java/com/google/gerrit/server/api/projects/SubmitRequirementApiImpl.java
new file mode 100644
index 0000000..96bdc82
--- /dev/null
+++ b/java/com/google/gerrit/server/api/projects/SubmitRequirementApiImpl.java
@@ -0,0 +1,113 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.api.projects;
+
+import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
+import static com.google.gerrit.server.project.ProjectCache.illegalState;
+
+import com.google.gerrit.extensions.api.projects.SubmitRequirementApi;
+import com.google.gerrit.extensions.common.SubmitRequirementInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.project.SubmitRequirementResource;
+import com.google.gerrit.server.restapi.project.CreateSubmitRequirement;
+import com.google.gerrit.server.restapi.project.GetSubmitRequirement;
+import com.google.gerrit.server.restapi.project.SubmitRequirementsCollection;
+import com.google.gerrit.server.restapi.project.UpdateSubmitRequirement;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+public class SubmitRequirementApiImpl implements SubmitRequirementApi {
+ interface Factory {
+ SubmitRequirementApiImpl create(ProjectResource project, String name);
+ }
+
+ private final SubmitRequirementsCollection submitRequirements;
+ private final CreateSubmitRequirement createSubmitRequirement;
+ private final UpdateSubmitRequirement updateSubmitRequirement;
+ private final GetSubmitRequirement getSubmitRequirement;
+ private final String name;
+ private final ProjectCache projectCache;
+
+ private ProjectResource project;
+
+ @Inject
+ SubmitRequirementApiImpl(
+ SubmitRequirementsCollection submitRequirements,
+ CreateSubmitRequirement createSubmitRequirement,
+ UpdateSubmitRequirement updateSubmitRequirement,
+ GetSubmitRequirement getSubmitRequirement,
+ ProjectCache projectCache,
+ @Assisted ProjectResource project,
+ @Assisted String name) {
+ this.submitRequirements = submitRequirements;
+ this.createSubmitRequirement = createSubmitRequirement;
+ this.updateSubmitRequirement = updateSubmitRequirement;
+ this.getSubmitRequirement = getSubmitRequirement;
+ this.projectCache = projectCache;
+ this.project = project;
+ this.name = name;
+ }
+
+ @Override
+ public SubmitRequirementApi create(SubmitRequirementInput input) throws RestApiException {
+ try {
+ createSubmitRequirement.apply(project, IdString.fromDecoded(name), input);
+
+ // recreate project resource because project state was updated
+ project =
+ new ProjectResource(
+ projectCache
+ .get(project.getNameKey())
+ .orElseThrow(illegalState(project.getNameKey())),
+ project.getUser());
+
+ return this;
+ } catch (Exception e) {
+ throw asRestApiException("Cannot create submit requirement", e);
+ }
+ }
+
+ @Override
+ public SubmitRequirementInfo get() throws RestApiException {
+ try {
+ return getSubmitRequirement.apply(resource()).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot get submit requirement", e);
+ }
+ }
+
+ @Override
+ public SubmitRequirementInfo update(SubmitRequirementInput input) throws RestApiException {
+ try {
+ return updateSubmitRequirement.apply(resource(), input).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot update submit requirement", e);
+ }
+ }
+
+ @Override
+ public void delete() throws RestApiException {
+ /** TODO(ghareeb): implement */
+ }
+
+ private SubmitRequirementResource resource() throws RestApiException, PermissionBackendException {
+ return submitRequirements.parse(project, IdString.fromDecoded(name));
+ }
+}
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 75bb550..c6263e2 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -190,7 +190,7 @@
import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.ProjectNameLockManager;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.SubmitRequirementExpressionsValidator;
+import com.google.gerrit.server.project.SubmitRequirementConfigValidator;
import com.google.gerrit.server.project.SubmitRequirementsEvaluatorImpl;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.FileEditsPredicate;
@@ -400,7 +400,7 @@
DynamicSet.setOf(binder(), UserScopedEventListener.class);
DynamicSet.setOf(binder(), CommitValidationListener.class);
DynamicSet.bind(binder(), CommitValidationListener.class)
- .to(SubmitRequirementExpressionsValidator.class);
+ .to(SubmitRequirementConfigValidator.class);
DynamicSet.setOf(binder(), CommentValidator.class);
DynamicSet.setOf(binder(), ChangeMessageModifier.class);
DynamicSet.setOf(binder(), RefOperationValidationListener.class);
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index d816d84..7ace1c8 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -550,6 +550,7 @@
return submitRequirementSections;
}
+ /** Adds or replaces the given {@link SubmitRequirement} in this config. */
public void upsertSubmitRequirement(SubmitRequirement requirement) {
submitRequirementSections.put(requirement.name(), requirement);
}
@@ -1018,7 +1019,7 @@
continue;
}
- // The expressions are validated in SubmitRequirementExpressionsValidator.
+ // The expressions are validated in SubmitRequirementConfigValidator.
SubmitRequirement submitRequirement =
SubmitRequirement.builder()
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementConfigValidator.java b/java/com/google/gerrit/server/project/SubmitRequirementConfigValidator.java
new file mode 100644
index 0000000..faca446
--- /dev/null
+++ b/java/com/google/gerrit/server/project/SubmitRequirementConfigValidator.java
@@ -0,0 +1,135 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.project;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.server.events.CommitReceivedEvent;
+import com.google.gerrit.server.git.validators.CommitValidationException;
+import com.google.gerrit.server.git.validators.CommitValidationListener;
+import com.google.gerrit.server.git.validators.CommitValidationMessage;
+import com.google.gerrit.server.git.validators.ValidationMessage;
+import com.google.gerrit.server.patch.DiffNotAvailableException;
+import com.google.gerrit.server.patch.DiffOperations;
+import com.google.gerrit.server.patch.DiffOptions;
+import com.google.inject.Inject;
+import java.io.IOException;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+/**
+ * Validates the expressions of submit requirements in {@code project.config}.
+ *
+ * <p>Other validation of submit requirements is done in {@link ProjectConfig}, see {@code
+ * ProjectConfig#loadSubmitRequirementSections(Config)}.
+ *
+ * <p>The validation of the expressions cannot be in {@link ProjectConfig} as it requires injecting
+ * {@link SubmitRequirementsEvaluator} and we cannot do injections into {@link ProjectConfig} (since
+ * {@link ProjectConfig} is cached in the project cache).
+ */
+public class SubmitRequirementConfigValidator implements CommitValidationListener {
+ private final DiffOperations diffOperations;
+ private final ProjectConfig.Factory projectConfigFactory;
+ private final SubmitRequirementExpressionsValidator submitRequirementExpressionsValidator;
+
+ @Inject
+ SubmitRequirementConfigValidator(
+ DiffOperations diffOperations,
+ ProjectConfig.Factory projectConfigFactory,
+ SubmitRequirementExpressionsValidator submitRequirementExpressionsValidator) {
+ this.diffOperations = diffOperations;
+ this.projectConfigFactory = projectConfigFactory;
+ this.submitRequirementExpressionsValidator = submitRequirementExpressionsValidator;
+ }
+
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent event)
+ throws CommitValidationException {
+ try {
+ if (!event.refName.equals(RefNames.REFS_CONFIG)
+ || !isFileChanged(event, ProjectConfig.PROJECT_CONFIG)) {
+ // the project.config file in refs/meta/config was not modified, hence we do not need to
+ // validate the submit requirements in it
+ return ImmutableList.of();
+ }
+
+ ProjectConfig projectConfig = getProjectConfig(event);
+ ImmutableList.Builder<String> validationMsgsBuilder = ImmutableList.builder();
+ for (SubmitRequirement submitRequirement :
+ projectConfig.getSubmitRequirementSections().values()) {
+ validationMsgsBuilder.addAll(
+ submitRequirementExpressionsValidator.validateExpressions(submitRequirement));
+ }
+ ImmutableList<String> validationMsgs = validationMsgsBuilder.build();
+ if (!validationMsgs.isEmpty()) {
+ throw new CommitValidationException(
+ String.format(
+ "invalid submit requirement expressions in %s (revision = %s)",
+ ProjectConfig.PROJECT_CONFIG, projectConfig.getRevision()),
+ new ImmutableList.Builder<CommitValidationMessage>()
+ .add(
+ new CommitValidationMessage(
+ "Invalid project configuration", ValidationMessage.Type.ERROR))
+ .addAll(
+ validationMsgs.stream()
+ .map(m -> toCommitValidationMessage(m))
+ .collect(Collectors.toList()))
+ .build());
+ }
+ return ImmutableList.of();
+ } catch (IOException | DiffNotAvailableException | ConfigInvalidException e) {
+ throw new CommitValidationException(
+ String.format(
+ "failed to validate submit requirement expressions in %s for revision %s in ref %s"
+ + " of project %s",
+ ProjectConfig.PROJECT_CONFIG,
+ event.commit.getName(),
+ RefNames.REFS_CONFIG,
+ event.project.getNameKey()),
+ e);
+ }
+ }
+
+ private static CommitValidationMessage toCommitValidationMessage(String message) {
+ return new CommitValidationMessage(message, ValidationMessage.Type.ERROR);
+ }
+
+ /**
+ * Whether the given file was changed in the given revision.
+ *
+ * @param receiveEvent the receive event
+ * @param fileName the name of the file
+ */
+ private boolean isFileChanged(CommitReceivedEvent receiveEvent, String fileName)
+ throws DiffNotAvailableException {
+ return diffOperations
+ .listModifiedFilesAgainstParent(
+ receiveEvent.project.getNameKey(),
+ receiveEvent.commit,
+ /* parentNum=*/ 0,
+ DiffOptions.DEFAULTS)
+ .keySet().stream()
+ .anyMatch(fileName::equals);
+ }
+
+ private ProjectConfig getProjectConfig(CommitReceivedEvent receiveEvent)
+ throws IOException, ConfigInvalidException {
+ ProjectConfig projectConfig = projectConfigFactory.create(receiveEvent.project.getNameKey());
+ projectConfig.load(receiveEvent.revWalk, receiveEvent.commit);
+ return projectConfig;
+ }
+}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementExpressionsValidator.java b/java/com/google/gerrit/server/project/SubmitRequirementExpressionsValidator.java
index 8717581..f2e4ff8 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementExpressionsValidator.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementExpressionsValidator.java
@@ -15,144 +15,59 @@
package com.google.gerrit.server.project;
import com.google.common.collect.ImmutableList;
-import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.index.query.QueryParseException;
-import com.google.gerrit.server.events.CommitReceivedEvent;
-import com.google.gerrit.server.git.validators.CommitValidationException;
-import com.google.gerrit.server.git.validators.CommitValidationListener;
-import com.google.gerrit.server.git.validators.CommitValidationMessage;
-import com.google.gerrit.server.git.validators.ValidationMessage;
-import com.google.gerrit.server.patch.DiffNotAvailableException;
-import com.google.gerrit.server.patch.DiffOperations;
-import com.google.gerrit.server.patch.DiffOptions;
import com.google.inject.Inject;
-import java.io.IOException;
+import com.google.inject.Singleton;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.List;
-import org.eclipse.jgit.errors.ConfigInvalidException;
-/**
- * Validates the expressions of submit requirements in {@code project.config}.
- *
- * <p>Other validation of submit requirements is done in {@link ProjectConfig}, see {@code
- * ProjectConfig#loadSubmitRequirementSections(Config)}.
- *
- * <p>The validation of the expressions cannot be in {@link ProjectConfig} as it requires injecting
- * {@link SubmitRequirementsEvaluator} and we cannot do injections into {@link ProjectConfig} (since
- * {@link ProjectConfig} is cached in the project cache).
- */
-public class SubmitRequirementExpressionsValidator implements CommitValidationListener {
- private final DiffOperations diffOperations;
- private final ProjectConfig.Factory projectConfigFactory;
+@Singleton
+public class SubmitRequirementExpressionsValidator {
private final SubmitRequirementsEvaluator submitRequirementsEvaluator;
@Inject
- SubmitRequirementExpressionsValidator(
- DiffOperations diffOperations,
- ProjectConfig.Factory projectConfigFactory,
- SubmitRequirementsEvaluator submitRequirementsEvaluator) {
- this.diffOperations = diffOperations;
- this.projectConfigFactory = projectConfigFactory;
+ SubmitRequirementExpressionsValidator(SubmitRequirementsEvaluator submitRequirementsEvaluator) {
this.submitRequirementsEvaluator = submitRequirementsEvaluator;
}
- @Override
- public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent event)
- throws CommitValidationException {
- try {
- if (!event.refName.equals(RefNames.REFS_CONFIG)
- || !isFileChanged(event, ProjectConfig.PROJECT_CONFIG)) {
- // the project.config file in refs/meta/config was not modified, hence we do not need to
- // validate the submit requirements in it
- return ImmutableList.of();
- }
-
- ProjectConfig projectConfig = getProjectConfig(event);
- ImmutableList<CommitValidationMessage> validationMessages =
- validateSubmitRequirementExpressions(
- projectConfig.getSubmitRequirementSections().values());
- if (!validationMessages.isEmpty()) {
- throw new CommitValidationException(
- String.format(
- "invalid submit requirement expressions in %s (revision = %s)",
- ProjectConfig.PROJECT_CONFIG, projectConfig.getRevision()),
- validationMessages);
- }
- return ImmutableList.of();
- } catch (IOException | DiffNotAvailableException | ConfigInvalidException e) {
- throw new CommitValidationException(
- String.format(
- "failed to validate submit requirement expressions in %s for revision %s in ref %s"
- + " of project %s",
- ProjectConfig.PROJECT_CONFIG,
- event.commit.getName(),
- RefNames.REFS_CONFIG,
- event.project.getNameKey()),
- e);
- }
- }
-
/**
- * Whether the given file was changed in the given revision.
+ * Validates the query expressions on the input {@code submitRequirement}.
*
- * @param receiveEvent the receive event
- * @param fileName the name of the file
+ * @return list of string containing the error messages resulting from the validation. The list is
+ * empty if the "submit requirement" is valid.
*/
- private boolean isFileChanged(CommitReceivedEvent receiveEvent, String fileName)
- throws DiffNotAvailableException {
- return diffOperations
- .listModifiedFilesAgainstParent(
- receiveEvent.project.getNameKey(),
- receiveEvent.commit,
- /* parentNum=*/ 0,
- DiffOptions.DEFAULTS)
- .keySet().stream()
- .anyMatch(fileName::equals);
- }
-
- private ProjectConfig getProjectConfig(CommitReceivedEvent receiveEvent)
- throws IOException, ConfigInvalidException {
- ProjectConfig projectConfig = projectConfigFactory.create(receiveEvent.project.getNameKey());
- projectConfig.load(receiveEvent.revWalk, receiveEvent.commit);
- return projectConfig;
- }
-
- private ImmutableList<CommitValidationMessage> validateSubmitRequirementExpressions(
- Collection<SubmitRequirement> submitRequirements) {
- List<CommitValidationMessage> validationMessages = new ArrayList<>();
- for (SubmitRequirement submitRequirement : submitRequirements) {
- validateSubmitRequirementExpression(
- validationMessages,
- submitRequirement,
- submitRequirement.submittabilityExpression(),
- ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION);
- submitRequirement
- .applicabilityExpression()
- .ifPresent(
- expression ->
- validateSubmitRequirementExpression(
- validationMessages,
- submitRequirement,
- expression,
- ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION));
- submitRequirement
- .overrideExpression()
- .ifPresent(
- expression ->
- validateSubmitRequirementExpression(
- validationMessages,
- submitRequirement,
- expression,
- ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION));
- }
+ public ImmutableList<String> validateExpressions(SubmitRequirement submitRequirement) {
+ List<String> validationMessages = new ArrayList<>();
+ validateSubmitRequirementExpression(
+ validationMessages,
+ submitRequirement,
+ submitRequirement.submittabilityExpression(),
+ ProjectConfig.KEY_SR_SUBMITTABILITY_EXPRESSION);
+ submitRequirement
+ .applicabilityExpression()
+ .ifPresent(
+ expression ->
+ validateSubmitRequirementExpression(
+ validationMessages,
+ submitRequirement,
+ expression,
+ ProjectConfig.KEY_SR_APPLICABILITY_EXPRESSION));
+ submitRequirement
+ .overrideExpression()
+ .ifPresent(
+ expression ->
+ validateSubmitRequirementExpression(
+ validationMessages,
+ submitRequirement,
+ expression,
+ ProjectConfig.KEY_SR_OVERRIDE_EXPRESSION));
return ImmutableList.copyOf(validationMessages);
}
private void validateSubmitRequirementExpression(
- List<CommitValidationMessage> validationMessages,
+ List<String> validationMessages,
SubmitRequirement submitRequirement,
SubmitRequirementExpression expression,
String configKey) {
@@ -160,23 +75,19 @@
submitRequirementsEvaluator.validateExpression(expression);
} catch (QueryParseException e) {
if (validationMessages.isEmpty()) {
- validationMessages.add(
- new CommitValidationMessage(
- "Invalid project configuration", ValidationMessage.Type.ERROR));
+ validationMessages.add("Invalid project configuration");
}
validationMessages.add(
- new CommitValidationMessage(
- String.format(
- " %s: Expression '%s' of submit requirement '%s' (parameter %s.%s.%s) is"
- + " invalid: %s",
- ProjectConfig.PROJECT_CONFIG,
- expression.expressionString(),
- submitRequirement.name(),
- ProjectConfig.SUBMIT_REQUIREMENT,
- submitRequirement.name(),
- configKey,
- e.getMessage()),
- ValidationMessage.Type.ERROR));
+ String.format(
+ " %s: Expression '%s' of submit requirement '%s' (parameter %s.%s.%s) is"
+ + " invalid: %s",
+ ProjectConfig.PROJECT_CONFIG,
+ expression.expressionString(),
+ submitRequirement.name(),
+ ProjectConfig.SUBMIT_REQUIREMENT,
+ submitRequirement.name(),
+ configKey,
+ e.getMessage()));
}
}
}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementJson.java b/java/com/google/gerrit/server/project/SubmitRequirementJson.java
new file mode 100644
index 0000000..5593ff4
--- /dev/null
+++ b/java/com/google/gerrit/server/project/SubmitRequirementJson.java
@@ -0,0 +1,38 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.project;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.extensions.common.SubmitRequirementInfo;
+import com.google.inject.Singleton;
+
+/** Converts a {@link SubmitRequirement} to a {@link SubmitRequirementInfo}. */
+@Singleton
+public class SubmitRequirementJson {
+ public static SubmitRequirementInfo format(SubmitRequirement sr) {
+ SubmitRequirementInfo info = new SubmitRequirementInfo();
+ info.name = sr.name();
+ info.description = sr.description().orElse(null);
+ if (sr.applicabilityExpression().isPresent()) {
+ info.applicabilityExpression = sr.applicabilityExpression().get().expressionString();
+ }
+ if (sr.overrideExpression().isPresent()) {
+ info.overrideExpression = sr.overrideExpression().get().expressionString();
+ }
+ info.submittabilityExpression = sr.submittabilityExpression().expressionString();
+ info.allowOverrideInChildProjects = sr.allowOverrideInChildProjects();
+ return info;
+ }
+}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementResource.java b/java/com/google/gerrit/server/project/SubmitRequirementResource.java
new file mode 100644
index 0000000..d075cd7
--- /dev/null
+++ b/java/com/google/gerrit/server/project/SubmitRequirementResource.java
@@ -0,0 +1,41 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.project;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.extensions.restapi.RestResource;
+import com.google.gerrit.extensions.restapi.RestView;
+import com.google.inject.TypeLiteral;
+
+public class SubmitRequirementResource implements RestResource {
+ public static final TypeLiteral<RestView<SubmitRequirementResource>> SUBMIT_REQUIREMENT_KIND =
+ new TypeLiteral<>() {};
+
+ private final ProjectResource project;
+ private final SubmitRequirement submitRequirement;
+
+ public SubmitRequirementResource(ProjectResource project, SubmitRequirement submitRequirement) {
+ this.project = project;
+ this.submitRequirement = submitRequirement;
+ }
+
+ public ProjectResource getProject() {
+ return project;
+ }
+
+ public SubmitRequirement getSubmitRequirement() {
+ return submitRequirement;
+ }
+}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
index c234c8c..3789c42 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsUtil.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.project;
import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.metrics.Counter2;
@@ -29,6 +30,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Set;
+import java.util.regex.Pattern;
import java.util.stream.Collectors;
/**
@@ -38,6 +40,12 @@
@Singleton
public class SubmitRequirementsUtil {
+ /**
+ * Submit requirement name can only contain alphanumeric characters or hyphen. Name cannot start
+ * with a hyphen or number.
+ */
+ private static final Pattern SUBMIT_REQ_NAME_PATTERN = Pattern.compile("[a-zA-Z][a-zA-Z0-9\\-]*");
+
@Singleton
static class Metrics {
final Counter2<String, String> submitRequirementsMatchingWithLegacy;
@@ -179,6 +187,20 @@
return ImmutableMap.copyOf(result);
}
+ /** Validates the name of submit requirements. */
+ public static void validateName(@Nullable String name) throws IllegalArgumentException {
+ if (name == null || name.isEmpty()) {
+ throw new IllegalArgumentException("Empty submit requirement name");
+ }
+ if (!SUBMIT_REQ_NAME_PATTERN.matcher(name).matches()) {
+ throw new IllegalArgumentException(
+ String.format(
+ "Illegal submit requirement name \"%s\". Name can only consist of "
+ + "alphanumeric characters and '-'. Name cannot start with '-' or number.",
+ name));
+ }
+ }
+
private static boolean shouldReportMetric(ChangeData cd) {
// We only care about recording differences in old and new requirements for open changes
// that did not have their data retrieved from the (potentially stale) change index.
diff --git a/java/com/google/gerrit/server/restapi/project/CreateSubmitRequirement.java b/java/com/google/gerrit/server/restapi/project/CreateSubmitRequirement.java
new file mode 100644
index 0000000..2aeba89
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/CreateSubmitRequirement.java
@@ -0,0 +1,168 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.project;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.extensions.common.SubmitRequirementInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.project.SubmitRequirementExpressionsValidator;
+import com.google.gerrit.server.project.SubmitRequirementJson;
+import com.google.gerrit.server.project.SubmitRequirementResource;
+import com.google.gerrit.server.project.SubmitRequirementsUtil;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+/** A rest create view that creates a "submit requirement" for a project. */
+@Singleton
+public class CreateSubmitRequirement
+ implements RestCollectionCreateView<
+ ProjectResource, SubmitRequirementResource, SubmitRequirementInput> {
+ private final Provider<CurrentUser> user;
+ private final PermissionBackend permissionBackend;
+ private final MetaDataUpdate.User updateFactory;
+ private final ProjectConfig.Factory projectConfigFactory;
+ private final ProjectCache projectCache;
+ private final SubmitRequirementExpressionsValidator submitRequirementExpressionsValidator;
+
+ @Inject
+ public CreateSubmitRequirement(
+ Provider<CurrentUser> user,
+ PermissionBackend permissionBackend,
+ MetaDataUpdate.User updateFactory,
+ ProjectConfig.Factory projectConfigFactory,
+ ProjectCache projectCache,
+ SubmitRequirementExpressionsValidator submitRequirementExpressionsValidator) {
+ this.user = user;
+ this.permissionBackend = permissionBackend;
+ this.updateFactory = updateFactory;
+ this.projectConfigFactory = projectConfigFactory;
+ this.projectCache = projectCache;
+ this.submitRequirementExpressionsValidator = submitRequirementExpressionsValidator;
+ }
+
+ @Override
+ public Response<SubmitRequirementInfo> apply(
+ ProjectResource rsrc, IdString id, SubmitRequirementInput input)
+ throws AuthException, BadRequestException, IOException, PermissionBackendException {
+ if (!user.get().isIdentifiedUser()) {
+ throw new AuthException("Authentication required");
+ }
+
+ permissionBackend
+ .currentUser()
+ .project(rsrc.getNameKey())
+ .check(ProjectPermission.WRITE_CONFIG);
+
+ if (input == null) {
+ input = new SubmitRequirementInput();
+ }
+
+ if (input.name != null && !input.name.equals(id.get())) {
+ throw new BadRequestException("name in input must match name in URL");
+ }
+
+ try (MetaDataUpdate md = updateFactory.create(rsrc.getNameKey())) {
+ ProjectConfig config = projectConfigFactory.read(md);
+
+ SubmitRequirement submitRequirement = createSubmitRequirement(config, id.get(), input);
+
+ md.setMessage(String.format("Create Submit Requirement %s", submitRequirement.name()));
+ config.commit(md);
+
+ projectCache.evict(rsrc.getProjectState().getProject().getNameKey());
+
+ return Response.created(SubmitRequirementJson.format(submitRequirement));
+ } catch (ConfigInvalidException e) {
+ throw new IOException("Failed to read project config", e);
+ } catch (ResourceConflictException e) {
+ throw new BadRequestException("Failed to create submit requirement", e);
+ }
+ }
+
+ public SubmitRequirement createSubmitRequirement(
+ ProjectConfig config, String name, SubmitRequirementInput input)
+ throws BadRequestException, ResourceConflictException {
+ validateSRName(name);
+ ensureSRUnique(name, config);
+ if (Strings.isNullOrEmpty(input.submittabilityExpression)) {
+ throw new BadRequestException("submittability_expression is required");
+ }
+ if (input.allowOverrideInChildProjects == null) {
+ // default is false
+ input.allowOverrideInChildProjects = false;
+ }
+ SubmitRequirement submitRequirement =
+ SubmitRequirement.builder()
+ .setName(name)
+ .setDescription(Optional.ofNullable(input.description))
+ .setApplicabilityExpression(
+ SubmitRequirementExpression.of(input.applicabilityExpression))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create(input.submittabilityExpression))
+ .setOverrideExpression(SubmitRequirementExpression.of(input.overrideExpression))
+ .setAllowOverrideInChildProjects(input.allowOverrideInChildProjects)
+ .build();
+
+ List<String> validationMessages =
+ submitRequirementExpressionsValidator.validateExpressions(submitRequirement);
+ if (!validationMessages.isEmpty()) {
+ throw new BadRequestException(
+ String.format("Invalid submit requirement input: %s", validationMessages));
+ }
+
+ config.upsertSubmitRequirement(submitRequirement);
+ return submitRequirement;
+ }
+
+ private void validateSRName(String name) throws BadRequestException {
+ try {
+ SubmitRequirementsUtil.validateName(name);
+ } catch (IllegalArgumentException e) {
+ throw new BadRequestException(e.getMessage(), e);
+ }
+ }
+
+ private void ensureSRUnique(String name, ProjectConfig config) throws ResourceConflictException {
+ for (String srName : config.getSubmitRequirementSections().keySet()) {
+ if (srName.equalsIgnoreCase(name)) {
+ throw new ResourceConflictException(
+ String.format(
+ "submit requirement \"%s\" conflicts with existing submit requirement \"%s\"",
+ name, srName));
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/project/GetSubmitRequirement.java b/java/com/google/gerrit/server/restapi/project/GetSubmitRequirement.java
new file mode 100644
index 0000000..ce482e3
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/GetSubmitRequirement.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.project;
+
+import com.google.gerrit.extensions.common.SubmitRequirementInfo;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.project.SubmitRequirementJson;
+import com.google.gerrit.server.project.SubmitRequirementResource;
+import com.google.inject.Singleton;
+
+/** A rest read view that retrieves a "submit requirement" for a project by its name. */
+@Singleton
+public class GetSubmitRequirement implements RestReadView<SubmitRequirementResource> {
+ @Override
+ public Response<SubmitRequirementInfo> apply(SubmitRequirementResource rsrc) {
+ return Response.ok(SubmitRequirementJson.format(rsrc.getSubmitRequirement()));
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
index e50a494..1752b4ec 100644
--- a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
@@ -21,6 +21,7 @@
import static com.google.gerrit.server.project.FileResource.FILE_KIND;
import static com.google.gerrit.server.project.LabelResource.LABEL_KIND;
import static com.google.gerrit.server.project.ProjectResource.PROJECT_KIND;
+import static com.google.gerrit.server.project.SubmitRequirementResource.SUBMIT_REQUIREMENT_KIND;
import static com.google.gerrit.server.project.TagResource.TAG_KIND;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -46,6 +47,7 @@
DynamicMap.mapOf(binder(), COMMIT_KIND);
DynamicMap.mapOf(binder(), TAG_KIND);
DynamicMap.mapOf(binder(), LABEL_KIND);
+ DynamicMap.mapOf(binder(), SUBMIT_REQUIREMENT_KIND);
DynamicSet.bind(binder(), GerritConfigListener.class).to(SetParent.class);
DynamicSet.bind(binder(), ProjectCreationValidationListener.class)
@@ -78,6 +80,11 @@
delete(LABEL_KIND).to(DeleteLabel.class);
postOnCollection(LABEL_KIND).to(PostLabels.class);
+ child(PROJECT_KIND, "submit_requirements").to(SubmitRequirementsCollection.class);
+ create(SUBMIT_REQUIREMENT_KIND).to(CreateSubmitRequirement.class);
+ put(SUBMIT_REQUIREMENT_KIND).to(UpdateSubmitRequirement.class);
+ get(SUBMIT_REQUIREMENT_KIND).to(GetSubmitRequirement.class);
+
get(PROJECT_KIND, "HEAD").to(GetHead.class);
put(PROJECT_KIND, "HEAD").to(SetHead.class);
diff --git a/java/com/google/gerrit/server/restapi/project/SubmitRequirementsCollection.java b/java/com/google/gerrit/server/restapi/project/SubmitRequirementsCollection.java
new file mode 100644
index 0000000..cd98bec
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/SubmitRequirementsCollection.java
@@ -0,0 +1,85 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.project;
+
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.extensions.registration.DynamicMap;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.ChildCollection;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.extensions.restapi.NotImplementedException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestView;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.project.SubmitRequirementResource;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+@Singleton
+public class SubmitRequirementsCollection
+ implements ChildCollection<ProjectResource, SubmitRequirementResource> {
+ private final Provider<CurrentUser> user;
+ private final PermissionBackend permissionBackend;
+ private final DynamicMap<RestView<SubmitRequirementResource>> views;
+
+ @Inject
+ SubmitRequirementsCollection(
+ Provider<CurrentUser> user,
+ PermissionBackend permissionBackend,
+ DynamicMap<RestView<SubmitRequirementResource>> views) {
+ this.user = user;
+ this.permissionBackend = permissionBackend;
+ this.views = views;
+ }
+
+ @Override
+ public RestView<ProjectResource> list() throws RestApiException {
+ /** TODO(ghareeb): implement. */
+ throw new NotImplementedException();
+ }
+
+ @Override
+ public SubmitRequirementResource parse(ProjectResource parent, IdString id)
+ throws AuthException, ResourceNotFoundException, PermissionBackendException {
+ if (!user.get().isIdentifiedUser()) {
+ throw new AuthException("Authentication required");
+ }
+
+ permissionBackend
+ .currentUser()
+ .project(parent.getNameKey())
+ .check(ProjectPermission.READ_CONFIG);
+
+ SubmitRequirement submitRequirement =
+ parent.getProjectState().getConfig().getSubmitRequirementSections().get(id.get());
+
+ if (submitRequirement == null) {
+ throw new ResourceNotFoundException(
+ String.format("Submit requirement '%s' does not exist", id));
+ }
+ return new SubmitRequirementResource(parent, submitRequirement);
+ }
+
+ @Override
+ public DynamicMap<RestView<SubmitRequirementResource>> views() {
+ return views;
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java b/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
new file mode 100644
index 0000000..a176bc4
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/project/UpdateSubmitRequirement.java
@@ -0,0 +1,156 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.restapi.project;
+
+import com.google.common.base.Strings;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.extensions.common.SubmitRequirementInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.SubmitRequirementExpressionsValidator;
+import com.google.gerrit.server.project.SubmitRequirementJson;
+import com.google.gerrit.server.project.SubmitRequirementResource;
+import com.google.gerrit.server.project.SubmitRequirementsUtil;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import java.io.IOException;
+import java.util.List;
+import java.util.Optional;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+
+/**
+ * A rest modify view that updates the definition of an existing submit requirement for a project.
+ */
+@Singleton
+public class UpdateSubmitRequirement
+ implements RestModifyView<SubmitRequirementResource, SubmitRequirementInput> {
+ private final Provider<CurrentUser> user;
+ private final PermissionBackend permissionBackend;
+ private final MetaDataUpdate.User updateFactory;
+ private final ProjectConfig.Factory projectConfigFactory;
+ private final ProjectCache projectCache;
+ private final SubmitRequirementExpressionsValidator submitRequirementExpressionsValidator;
+
+ @Inject
+ public UpdateSubmitRequirement(
+ Provider<CurrentUser> user,
+ PermissionBackend permissionBackend,
+ MetaDataUpdate.User updateFactory,
+ ProjectConfig.Factory projectConfigFactory,
+ ProjectCache projectCache,
+ SubmitRequirementExpressionsValidator submitRequirementExpressionsValidator) {
+ this.user = user;
+ this.permissionBackend = permissionBackend;
+ this.updateFactory = updateFactory;
+ this.projectConfigFactory = projectConfigFactory;
+ this.projectCache = projectCache;
+ this.submitRequirementExpressionsValidator = submitRequirementExpressionsValidator;
+ }
+
+ @Override
+ public Response<SubmitRequirementInfo> apply(
+ SubmitRequirementResource rsrc, SubmitRequirementInput input)
+ throws AuthException, BadRequestException, PermissionBackendException, IOException {
+ if (!user.get().isIdentifiedUser()) {
+ throw new AuthException("Authentication required");
+ }
+
+ permissionBackend
+ .currentUser()
+ .project(rsrc.getProject().getNameKey())
+ .check(ProjectPermission.WRITE_CONFIG);
+
+ if (input == null) {
+ input = new SubmitRequirementInput();
+ }
+
+ if (input.name != null && !input.name.equals(rsrc.getSubmitRequirement().name())) {
+ throw new BadRequestException("name in input must match name in URL");
+ }
+
+ try (MetaDataUpdate md = updateFactory.create(rsrc.getProject().getNameKey())) {
+ ProjectConfig config = projectConfigFactory.read(md);
+
+ SubmitRequirement submitRequirement =
+ createSubmitRequirement(config, rsrc.getSubmitRequirement().name(), input);
+
+ md.setMessage(String.format("Update Submit Requirement %s", submitRequirement.name()));
+ config.commit(md);
+
+ projectCache.evict(rsrc.getProject().getNameKey());
+
+ return Response.created(SubmitRequirementJson.format(submitRequirement));
+ } catch (ConfigInvalidException e) {
+ throw new IOException("Failed to read project config", e);
+ } catch (ResourceConflictException e) {
+ throw new BadRequestException("Failed to create submit requirement", e);
+ }
+ }
+
+ public SubmitRequirement createSubmitRequirement(
+ ProjectConfig config, String name, SubmitRequirementInput input)
+ throws BadRequestException, ResourceConflictException {
+ validateSRName(name);
+ if (Strings.isNullOrEmpty(input.submittabilityExpression)) {
+ throw new BadRequestException("submittability_expression is required");
+ }
+ if (input.allowOverrideInChildProjects == null) {
+ // default is false
+ input.allowOverrideInChildProjects = false;
+ }
+ SubmitRequirement submitRequirement =
+ SubmitRequirement.builder()
+ .setName(name)
+ .setDescription(Optional.ofNullable(input.description))
+ .setApplicabilityExpression(
+ SubmitRequirementExpression.of(input.applicabilityExpression))
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create(input.submittabilityExpression))
+ .setOverrideExpression(SubmitRequirementExpression.of(input.overrideExpression))
+ .setAllowOverrideInChildProjects(input.allowOverrideInChildProjects)
+ .build();
+
+ List<String> validationMessages =
+ submitRequirementExpressionsValidator.validateExpressions(submitRequirement);
+ if (!validationMessages.isEmpty()) {
+ throw new BadRequestException(
+ String.format("Invalid submit requirement input: %s", validationMessages));
+ }
+
+ config.upsertSubmitRequirement(submitRequirement);
+ return submitRequirement;
+ }
+
+ private void validateSRName(String name) throws BadRequestException {
+ try {
+ SubmitRequirementsUtil.validateName(name);
+ } catch (IllegalArgumentException e) {
+ throw new BadRequestException(e.getMessage(), e);
+ }
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/SubmitRequirementsAPIIT.java b/javatests/com/google/gerrit/acceptance/api/project/SubmitRequirementsAPIIT.java
new file mode 100644
index 0000000..b8f4f42
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/api/project/SubmitRequirementsAPIIT.java
@@ -0,0 +1,471 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.acceptance.api.project;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.extensions.common.SubmitRequirementInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementInput;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
+import com.google.inject.Inject;
+import org.junit.Test;
+
+@NoHttpd
+public class SubmitRequirementsAPIIT extends AbstractDaemonTest {
+ @Inject private RequestScopeOperations requestScopeOperations;
+
+ @Test
+ public void cannotGetANonExistingSR() throws Exception {
+ ResourceNotFoundException thrown =
+ assertThrows(
+ ResourceNotFoundException.class,
+ () -> gApi.projects().name(project.get()).submitRequirement("code-review").get());
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("Submit requirement 'code-review' does not exist");
+ }
+
+ @Test
+ public void getExistingSR() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.applicabilityExpression = "topic:foo";
+ input.submittabilityExpression = "label:code-review=+2";
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").get();
+ assertThat(info.name).isEqualTo("code-review");
+ assertThat(info.applicabilityExpression).isEqualTo("topic:foo");
+ assertThat(info.submittabilityExpression).isEqualTo("label:code-review=+2");
+ assertThat(info.allowOverrideInChildProjects).isEqualTo(false);
+ }
+
+ @Test
+ public void updateSubmitRequirement() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.applicabilityExpression = "topic:foo";
+ input.submittabilityExpression = "label:code-review=+2";
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ input.submittabilityExpression = "label:code-review=+1";
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input);
+ assertThat(info.submittabilityExpression).isEqualTo("label:code-review=+1");
+ }
+
+ @Test
+ public void updateSRWithEmptyApplicabilityExpression_isAllowed() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.applicabilityExpression = "topic:foo";
+ input.submittabilityExpression = "label:code-review=+2";
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ input.applicabilityExpression = null;
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input);
+ assertThat(info.applicabilityExpression).isNull();
+ }
+
+ @Test
+ public void updateSRWithEmptyOverrideExpression_isAllowed() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.overrideExpression = "topic:foo";
+ input.submittabilityExpression = "label:code-review=+2";
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ input.overrideExpression = null;
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input);
+ assertThat(info.overrideExpression).isNull();
+ }
+
+ @Test
+ public void allowOverrideInChildProjectsDefaultsToFalse_updateSR() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.submittabilityExpression = "label:code-review=+2";
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ input.overrideExpression = "topic:foo";
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input);
+ assertThat(info.allowOverrideInChildProjects).isFalse();
+ }
+
+ @Test
+ public void cannotUpdateSRAsAnonymousUser() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.submittabilityExpression = "label:code-review=+2";
+
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+ input.submittabilityExpression = "label:code-review=+1";
+ requestScopeOperations.setApiUserAnonymous();
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("code-review")
+ .update(new SubmitRequirementInput()));
+ assertThat(thrown).hasMessageThat().contains("Authentication required");
+ }
+
+ @Test
+ public void cannotUpdateSRtIfSRDoesNotExist() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.submittabilityExpression = "label:code-review=+2";
+
+ ResourceNotFoundException thrown =
+ assertThrows(
+ ResourceNotFoundException.class,
+ () ->
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo("Submit requirement 'code-review' does not exist");
+ }
+
+ @Test
+ public void cannotUpdateSRWithEmptySubmittableIf() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.submittabilityExpression = "project:foo AND branch:refs/heads/main";
+
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+ input.submittabilityExpression = null;
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input));
+
+ assertThat(thrown).hasMessageThat().isEqualTo("submittability_expression is required");
+ }
+
+ @Test
+ public void cannotUpdateSRWithInvalidSubmittableIfExpression() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.submittabilityExpression = "project:foo AND branch:refs/heads/main";
+
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+ input.submittabilityExpression = "invalid_field:invalid_value";
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "Invalid submit requirement input: "
+ + "[Invalid project configuration, "
+ + "project.config: Expression 'invalid_field:invalid_value' of "
+ + "submit requirement 'code-review' "
+ + "(parameter submit-requirement.code-review.submittableIf) is invalid: "
+ + "Unsupported operator invalid_field:invalid_value]");
+ }
+
+ @Test
+ public void cannotUpdateSRWithInvalidOverrideIfExpression() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.submittabilityExpression = "project:foo AND branch:refs/heads/main";
+
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+ input.overrideExpression = "invalid_field:invalid_value";
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "Invalid submit requirement input: "
+ + "[Invalid project configuration, "
+ + "project.config: Expression 'invalid_field:invalid_value' of "
+ + "submit requirement 'code-review' "
+ + "(parameter submit-requirement.code-review.overrideIf) is invalid: "
+ + "Unsupported operator invalid_field:invalid_value]");
+ }
+
+ @Test
+ public void cannotUpdateSRWithInvalidApplicableIfExpression() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.submittabilityExpression = "project:foo AND branch:refs/heads/main";
+
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+ input.applicabilityExpression = "invalid_field:invalid_value";
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects().name(project.get()).submitRequirement("code-review").update(input));
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "Invalid submit requirement input: "
+ + "[Invalid project configuration, "
+ + "project.config: Expression 'invalid_field:invalid_value' of "
+ + "submit requirement 'code-review' "
+ + "(parameter submit-requirement.code-review.applicableIf) is invalid: "
+ + "Unsupported operator invalid_field:invalid_value]");
+ }
+
+ @Test
+ public void createSubmitRequirement() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.applicabilityExpression = "project:foo AND branch:refs/heads/main";
+ input.submittabilityExpression = "label:code-review=+2";
+ input.overrideExpression = "label:build-cop-override=+1";
+ input.allowOverrideInChildProjects = true;
+
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ assertThat(info.name).isEqualTo("code-review");
+ assertThat(info.description).isEqualTo(input.description);
+ assertThat(info.applicabilityExpression).isEqualTo(input.applicabilityExpression);
+ assertThat(info.applicabilityExpression).isEqualTo(input.applicabilityExpression);
+ assertThat(info.submittabilityExpression).isEqualTo(input.submittabilityExpression);
+ assertThat(info.overrideExpression).isEqualTo(input.overrideExpression);
+ assertThat(info.allowOverrideInChildProjects).isEqualTo(true);
+ }
+
+ @Test
+ public void createSRWithEmptyApplicabilityExpression_isAllowed() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.submittabilityExpression = "label:code-review=+2";
+ input.overrideExpression = "label:build-cop-override=+1";
+
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ assertThat(info.name).isEqualTo("code-review");
+ assertThat(info.applicabilityExpression).isNull();
+ }
+
+ @Test
+ public void createSRWithEmptyOverrideExpression_isAllowed() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.applicabilityExpression = "project:foo AND branch:refs/heads/main";
+ input.submittabilityExpression = "label:code-review=+2";
+
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ assertThat(info.name).isEqualTo("code-review");
+ assertThat(info.overrideExpression).isNull();
+ }
+
+ @Test
+ public void allowOverrideInChildProjectsDefaultsToFalse_createSR() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.submittabilityExpression = "label:code-review=+2";
+
+ SubmitRequirementInfo info =
+ gApi.projects().name(project.get()).submitRequirement("code-review").create(input).get();
+
+ assertThat(info.allowOverrideInChildProjects).isEqualTo(false);
+ }
+
+ @Test
+ public void cannotCreateSRAsAnonymousUser() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.applicabilityExpression = "project:foo AND branch:refs/heads/main";
+ input.submittabilityExpression = "label:code-review=+2";
+ input.overrideExpression = "label:build-cop-override=+1";
+
+ requestScopeOperations.setApiUserAnonymous();
+ AuthException thrown =
+ assertThrows(
+ AuthException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("code-review")
+ .create(new SubmitRequirementInput()));
+ assertThat(thrown).hasMessageThat().contains("Authentication required");
+ }
+
+ @Test
+ public void cannotCreateSRtIfNameInInputDoesNotMatchResource() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.submittabilityExpression = "label:code-review=+2";
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("other-requirement")
+ .create(input)
+ .get());
+ assertThat(thrown).hasMessageThat().isEqualTo("name in input must match name in URL");
+ }
+
+ @Test
+ public void cannotCreateSRWithInvalidName() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "wrong$%";
+ input.submittabilityExpression = "label:code-review=+2";
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("wrong$%")
+ .create(input)
+ .get());
+ assertThat(thrown)
+ .hasMessageThat()
+ .isEqualTo(
+ "Illegal submit requirement name \"wrong$%\". "
+ + "Name can only consist of alphanumeric characters and '-'."
+ + " Name cannot start with '-' or number.");
+ }
+
+ @Test
+ public void cannotCreateSRWithEmptySubmittableIf() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.applicabilityExpression = "project:foo AND branch:refs/heads/main";
+ input.overrideExpression = "label:build-cop-override=+1";
+
+ BadRequestException thrown =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("code-review")
+ .create(input)
+ .get());
+
+ assertThat(thrown).hasMessageThat().isEqualTo("submittability_expression is required");
+ }
+
+ @Test
+ public void cannotCreateSRWithInvalidSubmittableIfExpression() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.submittabilityExpression = "invalid_field:invalid_value";
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("code-review")
+ .create(input)
+ .get());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ "Invalid submit requirement input: "
+ + "[Invalid project configuration, "
+ + "project.config: Expression 'invalid_field:invalid_value' of "
+ + "submit requirement 'code-review' "
+ + "(parameter submit-requirement.code-review.submittableIf) is invalid: "
+ + "Unsupported operator invalid_field:invalid_value]");
+ }
+
+ @Test
+ public void cannotCreateSRWithInvalidOverrideIfExpression() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.submittabilityExpression = "label:Code-Review=+2";
+ input.overrideExpression = "invalid_field:invalid_value";
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("code-review")
+ .create(input)
+ .get());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ "Invalid submit requirement input: "
+ + "[Invalid project configuration, "
+ + "project.config: Expression 'invalid_field:invalid_value' of "
+ + "submit requirement 'code-review' "
+ + "(parameter submit-requirement.code-review.overrideIf) is invalid: "
+ + "Unsupported operator invalid_field:invalid_value]");
+ }
+
+ @Test
+ public void cannotCreateSRWithInvalidApplicableIfExpression() throws Exception {
+ SubmitRequirementInput input = new SubmitRequirementInput();
+ input.name = "code-review";
+ input.description = "At least one +2 vote to the code-review label";
+ input.applicabilityExpression = "invalid_field:invalid_value";
+ input.submittabilityExpression = "label:Code-Review=+2";
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () ->
+ gApi.projects()
+ .name(project.get())
+ .submitRequirement("code-review")
+ .create(input)
+ .get());
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ "Invalid submit requirement input: "
+ + "[Invalid project configuration, "
+ + "project.config: Expression 'invalid_field:invalid_value' of "
+ + "submit requirement 'code-review' "
+ + "(parameter submit-requirement.code-review.applicableIf) is invalid: "
+ + "Unsupported operator invalid_field:invalid_value]");
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
index f1c0110..8879d53 100644
--- a/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/binding/ProjectsRestApiBindingsIT.java
@@ -32,6 +32,8 @@
import com.google.gerrit.entities.LabelFunction;
import com.google.gerrit.entities.Permission;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.projects.BranchInput;
import com.google.gerrit.extensions.api.projects.TagInput;
@@ -85,7 +87,8 @@
.build(),
RestCall.get("/projects/%s/dashboards"),
RestCall.put("/projects/%s/labels/new-label"),
- RestCall.post("/projects/%s/labels/"));
+ RestCall.post("/projects/%s/labels/"),
+ RestCall.put("/projects/%s/submit_requirements/new-sr"));
/**
* Child project REST endpoints to be tested, each URL contains placeholders for the parent
@@ -175,6 +178,15 @@
// Label deletion must be tested last
RestCall.delete("/projects/%s/labels/%s"));
+ /**
+ * Submit requirement REST endpoints to be tested, each URL contains placeholders for the project
+ * identifier and the submit requirement name.
+ */
+ private static final ImmutableList<RestCall> SUBMIT_REQUIREMENT_ENDPOINTS =
+ ImmutableList.of(
+ RestCall.get("/projects/%s/submit_requirements/%s"),
+ RestCall.put("/projects/%s/submit_requirements/%s"));
+
private static final String FILENAME = "test.txt";
@Inject private ProjectOperations projectOperations;
@@ -236,6 +248,20 @@
RestApiCallHelper.execute(adminRestSession, LABEL_ENDPOINTS, project.get(), label);
}
+ @Test
+ public void submitRequirementsEndpoints() throws Exception {
+ // Create the SR, so that the GET endpoint succeeds
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setAllowOverrideInChildProjects(false)
+ .build());
+ RestApiCallHelper.execute(
+ adminRestSession, SUBMIT_REQUIREMENT_ENDPOINTS, project.get(), "code-review");
+ }
+
private String createAndSubmitChange(String filename) throws Exception {
RevCommit c =
testRepo
diff --git a/javatests/com/google/gerrit/server/project/SubmitRequirementNameValidatorTest.java b/javatests/com/google/gerrit/server/project/SubmitRequirementNameValidatorTest.java
new file mode 100644
index 0000000..98ee71d
--- /dev/null
+++ b/javatests/com/google/gerrit/server/project/SubmitRequirementNameValidatorTest.java
@@ -0,0 +1,57 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.project;
+
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Test for {@link SubmitRequirementsUtil#validateName(String)}. */
+@RunWith(JUnit4.class)
+public class SubmitRequirementNameValidatorTest {
+ @Test
+ public void canStartWithSmallLetter() throws Exception {
+ SubmitRequirementsUtil.validateName("abc");
+ }
+
+ @Test
+ public void canStartWithCapitalLetter() throws Exception {
+ SubmitRequirementsUtil.validateName("Abc");
+ }
+
+ @Test
+ public void canBeEqualToOneLetter() throws Exception {
+ SubmitRequirementsUtil.validateName("a");
+ }
+
+ @Test
+ public void cannotStartWithNumber() throws Exception {
+ assertThrows(
+ IllegalArgumentException.class, () -> SubmitRequirementsUtil.validateName("98abc"));
+ }
+
+ @Test
+ public void cannotStartWithHyphen() throws Exception {
+ assertThrows(IllegalArgumentException.class, () -> SubmitRequirementsUtil.validateName("-abc"));
+ }
+
+ @Test
+ public void cannotContainNonAlphanumericOrHyphen() throws Exception {
+ assertThrows(
+ IllegalArgumentException.class, () -> SubmitRequirementsUtil.validateName("a&^bc"));
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index 6ec815c..0d52e79 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -821,30 +821,6 @@
return html`
<div>
<table>
- <tr ?hidden=${!this.showChecksSummary}>
- <td class="key">Checks</td>
- <td class="value">
- <div class="checksSummary">
- ${this.renderChecksZeroState()}${this.renderChecksChipForCategory(
- Category.ERROR
- )}${this.renderChecksChipForCategory(
- Category.WARNING
- )}${this.renderChecksChipForCategory(
- Category.INFO
- )}${this.renderChecksChipForCategory(
- Category.SUCCESS
- )}${hasNonRunningChip && hasRunningChip
- ? html`<br />`
- : ''}${this.renderChecksChipRunning()}
- <span
- class="loadingSpin"
- ?hidden=${!this.someProvidersAreLoading}
- ></span>
- ${this.renderErrorMessages()} ${this.renderChecksLogin()}
- ${this.renderSummaryMessage()} ${this.renderActions()}
- </div>
- </td>
- </tr>
<tr>
<td class="key">Comments</td>
<td class="value">
@@ -884,6 +860,30 @@
>
</td>
</tr>
+ <tr ?hidden=${!this.showChecksSummary}>
+ <td class="key">Checks</td>
+ <td class="value">
+ <div class="checksSummary">
+ ${this.renderChecksZeroState()}${this.renderChecksChipForCategory(
+ Category.ERROR
+ )}${this.renderChecksChipForCategory(
+ Category.WARNING
+ )}${this.renderChecksChipForCategory(
+ Category.INFO
+ )}${this.renderChecksChipForCategory(
+ Category.SUCCESS
+ )}${hasNonRunningChip && hasRunningChip
+ ? html`<br />`
+ : ''}${this.renderChecksChipRunning()}
+ <span
+ class="loadingSpin"
+ ?hidden=${!this.someProvidersAreLoading}
+ ></span>
+ ${this.renderErrorMessages()} ${this.renderChecksLogin()}
+ ${this.renderSummaryMessage()} ${this.renderActions()}
+ </div>
+ </td>
+ </tr>
<tr hidden>
<td class="key">Findings</td>
<td class="value"></td>
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 5d4db56..0eac350 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -546,13 +546,13 @@
diff-prefs="{{_diffPrefs}}"
change="[[_change]]"
change-num="[[_changeNum]]"
- patch-range="{{_patchRange}}"
+ patch-range="[[_patchRange]]"
selected-index="{{viewState.selectedFileIndex}}"
diff-view-mode="[[viewState.diffMode]]"
edit-mode="[[_editMode]]"
num-files-shown="{{_numFilesShown}}"
files-expanded="{{_filesExpanded}}"
- file-list-increment="{{_numFilesShown}}"
+ file-list-increment="[[_numFilesShown]]"
on-files-shown-changed="_setShownFiles"
on-file-action-tap="_handleFileActionTap"
observer-target="[[_computeObserverTarget()]]"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 45a6530..ce45424 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -103,6 +103,7 @@
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
import {GrChangeStar} from '../../shared/gr-change-star/gr-change-star';
import {GrThreadList} from '../gr-thread-list/gr-thread-list';
+import {assertIsDefined} from '../../../utils/common-util';
const fixture = fixtureFromElement('gr-change-view');
@@ -782,11 +783,10 @@
assert.isTrue(stub.called);
});
- test(', should open diff preferences', () => {
- const stub = sinon.stub(
- element.$.fileList.$.diffPreferencesDialog,
- 'open'
- );
+ test(', should open diff preferences', async () => {
+ await element.$.fileList.updateComplete;
+ assertIsDefined(element.$.fileList.diffPreferencesDialog);
+ const stub = sinon.stub(element.$.fileList.diffPreferencesDialog, 'open');
element._loggedIn = false;
pressAndReleaseKeyOn(element, 188, null, ',');
assert.isFalse(stub.called);
@@ -1183,7 +1183,8 @@
});
});
- test('diff preferences open when open-diff-prefs is fired', () => {
+ test('diff preferences open when open-diff-prefs is fired', async () => {
+ await element.$.fileList.updateComplete;
const overlayOpenStub = sinon.stub(element.$.fileList, 'openDiffPrefs');
element.$.fileListHeader.dispatchEvent(
new CustomEvent('open-diff-prefs', {
@@ -1326,6 +1327,7 @@
.stub(element, '_reloadPatchNumDependentResources')
.callsFake(() => Promise.resolve([undefined, undefined, undefined]));
flush();
+ await element.$.fileList.updateComplete;
const collapseStub = sinon.stub(element.$.fileList, 'collapseAllDiffs');
const value: AppElementChangeViewParams = {
...createAppElementChangeViewParams(),
@@ -1359,7 +1361,7 @@
sinon.stub(element, 'loadData').callsFake(() => Promise.resolve());
sinon.stub(element, 'loadAndSetCommitInfo');
sinon.stub(element.$.fileList, 'reload');
- flush();
+ await flush();
const reloadPortedCommentsStub = sinon.stub(
element.getCommentsModel(),
'reloadPortedComments'
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 7625756..20d625c 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -14,7 +14,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {Subscription} from 'rxjs';
import '../../../styles/gr-a11y-styles';
import '../../../styles/shared-styles';
import '../../../embed/diff/gr-diff-cursor/gr-diff-cursor';
@@ -29,14 +28,7 @@
import '../../shared/gr-tooltip-content/gr-tooltip-content';
import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
import '../../shared/gr-file-status-chip/gr-file-status-chip';
-import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {htmlTemplate} from './gr-file-list_html';
import {asyncForeach, debounce, DelayedTask} from '../../../utils/async-util';
-import {
- KeyboardShortcutMixin,
- Shortcut,
- ShortcutListener,
-} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {FilesExpandedState} from '../gr-file-list-constants';
import {pluralize} from '../../../utils/string-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
@@ -62,33 +54,40 @@
isMagicPath,
specialFilePathCompare,
} from '../../../utils/path-list-util';
-import {customElement, observe, property} from '@polymer/decorators';
+import {customElement, property, query, state} from 'lit/decorators';
import {
BasePatchSetNum,
EditPatchSetNum,
- ElementPropertyDeepChange,
FileInfo,
FileNameToFileInfoMap,
NumericChangeId,
PatchRange,
- RevisionPatchSetNum,
} from '../../../types/common';
import {DiffPreferencesInfo} from '../../../types/diff';
import {GrDiffHost} from '../../diff/gr-diff-host/gr-diff-host';
import {GrDiffPreferencesDialog} from '../../diff/gr-diff-preferences-dialog/gr-diff-preferences-dialog';
import {GrDiffCursor} from '../../../embed/diff/gr-diff-cursor/gr-diff-cursor';
import {GrCursorManager} from '../../shared/gr-cursor-manager/gr-cursor-manager';
-import {PolymerSpliceChange} from '@polymer/polymer/interfaces';
import {ChangeComments} from '../../diff/gr-comment-api/gr-comment-api';
import {ParsedChangeInfo, PatchSetFile} from '../../../types/types';
import {Timing} from '../../../constants/reporting';
import {RevisionInfo} from '../../shared/revision-info/revision-info';
-import {listen} from '../../../services/shortcuts/shortcuts-service';
import {select} from '../../../utils/observable-util';
-import {resolve, DIPolymerElement} from '../../../models/dependency';
+import {resolve} from '../../../models/dependency';
import {browserModelToken} from '../../../models/browser/browser-model';
import {commentsModelToken} from '../../../models/comments/comments-model';
import {changeModelToken} from '../../../models/change/change-model';
+import {ShortcutController} from '../../lit/shortcut-controller';
+import {css, html, LitElement, nothing, PropertyValues} from 'lit';
+import {Shortcut} from '../../../services/shortcuts/shortcuts-config';
+import {fire} from '../../../utils/event-util';
+import {a11yStyles} from '../../../styles/gr-a11y-styles';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {ValueChangedEvent} from '../../../types/events';
+import {subscribe} from '../../lit/subscription-controller';
+import {when} from 'lit/directives/when';
+import {incrementalRepeat} from '../../lit/incremental-repeat';
+import {ifDefined} from 'lit/directives/if-defined';
export const DEFAULT_NUM_FILES_SHOWN = 200;
@@ -101,12 +100,6 @@
const FILE_ROW_CLASS = 'file-row';
-export interface GrFileList {
- $: {
- diffPreferencesDialog: GrDiffPreferencesDialog;
- };
-}
-
interface ReviewedFileInfo extends FileInfo {
isReviewed?: boolean;
}
@@ -177,14 +170,26 @@
* @property {number} lines_inserted - fallback to 0 if not present in api
*/
-// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = KeyboardShortcutMixin(DIPolymerElement);
-
-@customElement('gr-file-list')
-export class GrFileList extends base {
- static get template() {
- return htmlTemplate;
+declare global {
+ interface HTMLElementEventMap {
+ 'num-files-shown-changed': ValueChangedEvent<number>;
+ 'files-expanded-changed': ValueChangedEvent<FilesExpandedState>;
+ 'diff-prefs-changed': ValueChangedEvent<DiffPreferencesInfo>;
}
+ interface HTMLElementTagNameMap {
+ 'gr-file-list': GrFileList;
+ }
+}
+@customElement('gr-file-list')
+export class GrFileList extends LitElement {
+ /**
+ * @event selected-index-changed
+ * @event files-expanded-changed
+ * @event num-files-shown-changed
+ * @event diff-prefs-changed
+ */
+ @query('#diffPreferencesDialog')
+ diffPreferencesDialog?: GrDiffPreferencesDialog;
@property({type: Object})
patchRange?: PatchRange;
@@ -198,121 +203,94 @@
@property({type: Object})
changeComments?: ChangeComments;
- @property({type: Number, notify: true})
+ @property({type: Number, attribute: 'selected-index'})
selectedIndex = -1;
@property({type: Object})
change?: ParsedChangeInfo;
- @property({type: String, notify: true, observer: '_updateDiffPreferences'})
+ @property({type: String})
diffViewMode?: DiffViewMode;
- @property({type: Boolean, observer: '_editModeChanged'})
+ @property({type: Boolean})
editMode?: boolean;
- @property({type: String, notify: true})
+ @property({type: String, attribute: 'files-expanded'})
filesExpanded = FilesExpandedState.NONE;
- @property({type: Object})
- _filesByPath?: FileNameToFileInfoMap;
+ // Private but used in tests.
+ @state()
+ filesByPath?: FileNameToFileInfoMap;
- @property({type: Array, observer: '_filesChanged'})
- _files: NormalizedFileInfo[] = [];
+ // Private but used in tests.
+ @state()
+ files: NormalizedFileInfo[] = [];
- @property({type: Boolean})
- _loggedIn = false;
+ // Private but used in tests.
+ @state()
+ loggedIn = false;
@property({type: Array})
reviewed?: string[] = [];
- @property({type: Object, notify: true, observer: '_updateDiffPreferences'})
+ @property({type: Object, attribute: 'diff-prefs'})
diffPrefs?: DiffPreferencesInfo;
- @property({type: Number, notify: true})
+ @property({type: Number, attribute: 'num-files-shown'})
numFilesShown: number = DEFAULT_NUM_FILES_SHOWN;
- @property({type: Object, computed: '_calculatePatchChange(_files)'})
- _patchChange: PatchChange = createDefaultPatchChange();
-
- @property({type: Number})
+ @property({type: Number, attribute: 'file-list-increment'})
fileListIncrement: number = DEFAULT_NUM_FILES_SHOWN;
- @property({type: Boolean, computed: '_shouldHideChangeTotals(_patchChange)'})
- _hideChangeTotals = true;
+ // Private but used in tests.
+ shownFiles: NormalizedFileInfo[] = [];
- @property({
- type: Boolean,
- computed: '_shouldHideBinaryChangeTotals(_patchChange)',
- })
- _hideBinaryChangeTotals = true;
+ @state()
+ private reportinShownFilesIncrement = 0;
- @property({
- type: Array,
- computed: '_computeFilesShown(numFilesShown, _files)',
- })
- _shownFiles: NormalizedFileInfo[] = [];
+ // Private but used in tests.
+ @state()
+ expandedFiles: PatchSetFile[] = [];
- @property({type: Number})
- _reportinShownFilesIncrement = 0;
+ // Private but used in tests.
+ @state()
+ displayLine?: boolean;
- @property({type: Array})
- _expandedFiles: PatchSetFile[] = [];
+ @state()
+ loading?: boolean;
- @property({type: Boolean})
- _displayLine?: boolean;
-
- @property({type: Boolean, observer: '_loadingChanged'})
- _loading?: boolean;
-
- @property({type: Object, computed: '_computeSizeBarLayout(_shownFiles.*)'})
- _sizeBarLayout: SizeBarLayout = createDefaultSizeBarLayout();
-
- @property({type: Boolean})
- _showSizeBars = true;
+ // Private but used in tests.
+ @state()
+ showSizeBars = true;
// For merge commits vs Auto Merge, an extra file row is shown detailing the
// files that were merged without conflict. These files are also passed to any
// plugins.
- @property({type: Array})
- _cleanlyMergedPaths: string[] = [];
+ @state()
+ private cleanlyMergedPaths: string[] = [];
- @property({type: Array})
- _cleanlyMergedOldPaths: string[] = [];
+ // Private but used in tests.
+ @state()
+ cleanlyMergedOldPaths: string[] = [];
- private _cancelForEachDiff?: () => void;
+ private cancelForEachDiff?: () => void;
loadingTask?: DelayedTask;
- @property({
- type: Boolean,
- computed:
- '_computeShowDynamicColumns(_dynamicHeaderEndpoints, ' +
- '_dynamicContentEndpoints, _dynamicSummaryEndpoints)',
- })
- _showDynamicColumns = false;
+ @state()
+ private dynamicHeaderEndpoints?: string[];
- @property({
- type: Boolean,
- computed:
- '_computeShowPrependedDynamicColumns(' +
- '_dynamicPrependedHeaderEndpoints, _dynamicPrependedContentEndpoints)',
- })
- _showPrependedDynamicColumns = false;
+ @state()
+ private dynamicContentEndpoints?: string[];
- @property({type: Array})
- _dynamicHeaderEndpoints?: string[];
+ @state()
+ private dynamicSummaryEndpoints?: string[];
- @property({type: Array})
- _dynamicContentEndpoints?: string[];
+ @state()
+ private dynamicPrependedHeaderEndpoints?: string[];
- @property({type: Array})
- _dynamicSummaryEndpoints?: string[];
-
- @property({type: Array})
- _dynamicPrependedHeaderEndpoints?: string[];
-
- @property({type: Array})
- _dynamicPrependedContentEndpoints?: string[];
+ @state()
+ private dynamicPrependedContentEndpoints?: string[];
private readonly reporting = getAppContext().reportingService;
@@ -326,48 +304,10 @@
private readonly getBrowserModel = resolve(this, browserModelToken);
- private subscriptions: Subscription[] = [];
-
/** Called in disconnectedCallback. */
private cleanups: (() => void)[] = [];
- override keyboardShortcuts(): ShortcutListener[] {
- return [
- listen(Shortcut.LEFT_PANE, _ => this._handleLeftPane()),
- listen(Shortcut.RIGHT_PANE, _ => this._handleRightPane()),
- listen(Shortcut.TOGGLE_INLINE_DIFF, _ => this._handleToggleInlineDiff()),
- listen(Shortcut.TOGGLE_ALL_INLINE_DIFFS, _ => this._toggleInlineDiffs()),
- listen(Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS, _ =>
- toggleClass(this, 'hideComments')
- ),
- listen(Shortcut.CURSOR_NEXT_FILE, e => this._handleCursorNext(e)),
- listen(Shortcut.CURSOR_PREV_FILE, e => this._handleCursorPrev(e)),
- // This is already been taken care of by CURSOR_NEXT_FILE above. The two
- // shortcuts share the same bindings. It depends on whether all files
- // are expanded whether the cursor moves to the next file or line.
- listen(Shortcut.NEXT_LINE, _ => {}), // docOnly
- // This is already been taken care of by CURSOR_PREV_FILE above. The two
- // shortcuts share the same bindings. It depends on whether all files
- // are expanded whether the cursor moves to the previous file or line.
- listen(Shortcut.PREV_LINE, _ => {}), // docOnly
- listen(Shortcut.NEW_COMMENT, _ => this._handleNewComment()),
- listen(Shortcut.OPEN_LAST_FILE, _ =>
- this._openSelectedFile(this._files.length - 1)
- ),
- listen(Shortcut.OPEN_FIRST_FILE, _ => this._openSelectedFile(0)),
- listen(Shortcut.OPEN_FILE, _ => this.handleOpenFile()),
- listen(Shortcut.NEXT_CHUNK, _ => this._handleNextChunk()),
- listen(Shortcut.PREV_CHUNK, _ => this._handlePrevChunk()),
- listen(Shortcut.NEXT_COMMENT_THREAD, _ => this._handleNextComment()),
- listen(Shortcut.PREV_COMMENT_THREAD, _ => this._handlePrevComment()),
- listen(Shortcut.TOGGLE_FILE_REVIEWED, _ =>
- this._handleToggleFileReviewed()
- ),
- listen(Shortcut.TOGGLE_LEFT_PANE, _ => this._handleToggleLeftPane()),
- listen(Shortcut.EXPAND_ALL_COMMENT_THREADS, _ => {}), // docOnly
- listen(Shortcut.COLLAPSE_ALL_COMMENT_THREADS, _ => {}), // docOnly
- ];
- }
+ shortcutsController = new ShortcutController(this);
// private but used in test
fileCursor = new GrCursorManager();
@@ -375,80 +315,506 @@
// private but used in test
diffCursor = new GrDiffCursor();
+ static override get styles() {
+ return [
+ a11yStyles,
+ sharedStyles,
+ css`
+ :host {
+ display: block;
+ }
+ .row {
+ align-items: center;
+ border-top: 1px solid var(--border-color);
+ display: flex;
+ min-height: calc(var(--line-height-normal) + 2 * var(--spacing-s));
+ padding: var(--spacing-xs) var(--spacing-l);
+ }
+ /* The class defines a content visible only to screen readers */
+ .noCommentsScreenReaderText {
+ opacity: 0;
+ max-width: 1px;
+ overflow: hidden;
+ display: none;
+ vertical-align: top;
+ }
+ div[role='gridcell']
+ > div.comments
+ > span:empty
+ + span:empty
+ + span.noCommentsScreenReaderText {
+ /* inline-block instead of block, such that it can control width */
+ display: inline-block;
+ }
+ :host(.loading) .row {
+ opacity: 0.5;
+ }
+ :host(.editMode) .hideOnEdit {
+ display: none;
+ }
+ .showOnEdit {
+ display: none;
+ }
+ :host(.editMode) .showOnEdit {
+ display: initial;
+ }
+ .invisible {
+ visibility: hidden;
+ }
+ .header-row {
+ background-color: var(--background-color-secondary);
+ }
+ .controlRow {
+ align-items: center;
+ display: flex;
+ height: 2.25em;
+ justify-content: center;
+ }
+ .controlRow.invisible,
+ .show-hide.invisible {
+ display: none;
+ }
+ .reviewed,
+ .status {
+ align-items: center;
+ display: inline-flex;
+ }
+ .reviewed {
+ display: inline-block;
+ text-align: left;
+ width: 1.5em;
+ }
+ .file-row {
+ cursor: pointer;
+ }
+ .file-row.expanded {
+ border-bottom: 1px solid var(--border-color);
+ position: -webkit-sticky;
+ position: sticky;
+ top: 0;
+ /* Has to visible above the diff view, and by default has a lower
+ z-index. setting to 1 places it directly above. */
+ z-index: 1;
+ }
+ .file-row:hover {
+ background-color: var(--hover-background-color);
+ }
+ .file-row.selected {
+ background-color: var(--selection-background-color);
+ }
+ .file-row.expanded,
+ .file-row.expanded:hover {
+ background-color: var(--expanded-background-color);
+ }
+ .path {
+ cursor: pointer;
+ flex: 1;
+ /* Wrap it into multiple lines if too long. */
+ white-space: normal;
+ word-break: break-word;
+ }
+ .oldPath {
+ color: var(--deemphasized-text-color);
+ }
+ .header-stats {
+ text-align: center;
+ min-width: 7.5em;
+ }
+ .stats {
+ text-align: right;
+ min-width: 7.5em;
+ }
+ .comments {
+ padding-left: var(--spacing-l);
+ min-width: 7.5em;
+ white-space: nowrap;
+ }
+ .row:not(.header-row) .stats,
+ .total-stats {
+ font-family: var(--monospace-font-family);
+ font-size: var(--font-size-mono);
+ line-height: var(--line-height-mono);
+ display: flex;
+ }
+ .sizeBars {
+ margin-left: var(--spacing-m);
+ min-width: 7em;
+ text-align: center;
+ }
+ .sizeBars.hide {
+ display: none;
+ }
+ .added,
+ .removed {
+ display: inline-block;
+ min-width: 3.5em;
+ }
+ .added {
+ color: var(--positive-green-text-color);
+ }
+ .removed {
+ color: var(--negative-red-text-color);
+ text-align: left;
+ min-width: 4em;
+ padding-left: var(--spacing-s);
+ }
+ .drafts {
+ color: var(--error-foreground);
+ font-weight: var(--font-weight-bold);
+ }
+ .show-hide-icon:focus {
+ outline: none;
+ }
+ .show-hide {
+ margin-left: var(--spacing-s);
+ width: 1.9em;
+ }
+ .fileListButton {
+ margin: var(--spacing-m);
+ }
+ .totalChanges {
+ justify-content: flex-end;
+ text-align: right;
+ }
+ .warning {
+ color: var(--deemphasized-text-color);
+ }
+ input.show-hide {
+ display: none;
+ }
+ label.show-hide {
+ cursor: pointer;
+ display: block;
+ min-width: 2em;
+ }
+ gr-diff {
+ display: block;
+ overflow-x: auto;
+ }
+ .truncatedFileName {
+ display: none;
+ }
+ .mobile {
+ display: none;
+ }
+ .reviewed {
+ margin-left: var(--spacing-xxl);
+ width: 15em;
+ }
+ .reviewedSwitch {
+ color: var(--link-color);
+ opacity: 0;
+ justify-content: flex-end;
+ width: 100%;
+ }
+ .reviewedSwitch:hover {
+ cursor: pointer;
+ opacity: 100;
+ }
+ .showParentButton {
+ line-height: var(--line-height-normal);
+ margin-bottom: calc(var(--spacing-s) * -1);
+ margin-left: var(--spacing-m);
+ margin-top: calc(var(--spacing-s) * -1);
+ }
+ .row:focus {
+ outline: none;
+ }
+ .row:hover .reviewedSwitch,
+ .row:focus-within .reviewedSwitch,
+ .row.expanded .reviewedSwitch {
+ opacity: 100;
+ }
+ .reviewedLabel {
+ color: var(--deemphasized-text-color);
+ margin-right: var(--spacing-l);
+ opacity: 0;
+ }
+ .reviewedLabel.isReviewed {
+ display: initial;
+ opacity: 100;
+ }
+ .editFileControls {
+ width: 7em;
+ }
+ .markReviewed:focus {
+ outline: none;
+ }
+ .markReviewed,
+ .pathLink {
+ display: inline-block;
+ margin: -2px 0;
+ padding: var(--spacing-s) 0;
+ text-decoration: none;
+ }
+ .pathLink:hover span.fullFileName,
+ .pathLink:hover span.truncatedFileName {
+ text-decoration: underline;
+ }
+
+ /** copy on file path **/
+ .pathLink gr-copy-clipboard,
+ .oldPath gr-copy-clipboard {
+ display: inline-block;
+ visibility: hidden;
+ vertical-align: bottom;
+ --gr-button-padding: 0px;
+ }
+ .row:focus-within gr-copy-clipboard,
+ .row:hover gr-copy-clipboard {
+ visibility: visible;
+ }
+
+ @media screen and (max-width: 1200px) {
+ gr-endpoint-decorator.extra-col {
+ display: none;
+ }
+ }
+
+ @media screen and (max-width: 1000px) {
+ .reviewed {
+ display: none;
+ }
+ }
+
+ @media screen and (max-width: 800px) {
+ .desktop {
+ display: none;
+ }
+ .mobile {
+ display: block;
+ }
+ .row.selected {
+ background-color: var(--view-background-color);
+ }
+ .stats {
+ display: none;
+ }
+ .reviewed,
+ .status {
+ justify-content: flex-start;
+ }
+ .comments {
+ min-width: initial;
+ }
+ .expanded .fullFileName,
+ .truncatedFileName {
+ display: inline;
+ }
+ .expanded .truncatedFileName,
+ .fullFileName {
+ display: none;
+ }
+ }
+ :host(.hideComments) {
+ --gr-comment-thread-display: none;
+ }
+ `,
+ ];
+ }
+
constructor() {
super();
this.fileCursor.scrollMode = ScrollMode.KEEP_VISIBLE;
this.fileCursor.cursorTargetClass = 'selected';
this.fileCursor.focusOnMove = true;
+ this.shortcutsController.addAbstract(Shortcut.LEFT_PANE, _ =>
+ this.handleLeftPane()
+ );
+ this.shortcutsController.addAbstract(Shortcut.RIGHT_PANE, _ =>
+ this.handleRightPane()
+ );
+ this.shortcutsController.addAbstract(Shortcut.TOGGLE_INLINE_DIFF, _ =>
+ this.handleToggleInlineDiff()
+ );
+ this.shortcutsController.addAbstract(Shortcut.TOGGLE_ALL_INLINE_DIFFS, _ =>
+ this.toggleInlineDiffs()
+ );
+ this.shortcutsController.addAbstract(
+ Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS,
+ _ => toggleClass(this, 'hideComments')
+ );
+ this.shortcutsController.addAbstract(Shortcut.CURSOR_NEXT_FILE, e =>
+ this.handleCursorNext(e)
+ );
+ this.shortcutsController.addAbstract(Shortcut.CURSOR_PREV_FILE, e =>
+ this.handleCursorPrev(e)
+ );
+ // This is already been taken care of by CURSOR_NEXT_FILE above. The two
+ // shortcuts share the same bindings. It depends on whether all files
+ // are expanded whether the cursor moves to the next file or line.
+ this.shortcutsController.addAbstract(Shortcut.NEXT_LINE, _ => {}); // docOnly
+ // This is already been taken care of by CURSOR_PREV_FILE above. The two
+ // shortcuts share the same bindings. It depends on whether all files
+ // are expanded whether the cursor moves to the previous file or line.
+ this.shortcutsController.addAbstract(Shortcut.PREV_LINE, _ => {}); // docOnly
+ this.shortcutsController.addAbstract(Shortcut.NEW_COMMENT, _ =>
+ this.handleNewComment()
+ );
+ this.shortcutsController.addAbstract(Shortcut.OPEN_LAST_FILE, _ =>
+ this.openSelectedFile(this.files.length - 1)
+ );
+ this.shortcutsController.addAbstract(Shortcut.OPEN_FIRST_FILE, _ =>
+ this.openSelectedFile(0)
+ );
+ this.shortcutsController.addAbstract(Shortcut.OPEN_FILE, _ =>
+ this.handleOpenFile()
+ );
+ this.shortcutsController.addAbstract(Shortcut.NEXT_CHUNK, _ =>
+ this.handleNextChunk()
+ );
+ this.shortcutsController.addAbstract(Shortcut.PREV_CHUNK, _ =>
+ this.handlePrevChunk()
+ );
+ this.shortcutsController.addAbstract(Shortcut.NEXT_COMMENT_THREAD, _ =>
+ this.handleNextComment()
+ );
+ this.shortcutsController.addAbstract(Shortcut.PREV_COMMENT_THREAD, _ =>
+ this.handlePrevComment()
+ );
+ this.shortcutsController.addAbstract(Shortcut.TOGGLE_FILE_REVIEWED, _ =>
+ this.handleToggleFileReviewed()
+ );
+ this.shortcutsController.addAbstract(Shortcut.TOGGLE_LEFT_PANE, _ =>
+ this.handleToggleLeftPane()
+ );
+ this.shortcutsController.addAbstract(
+ Shortcut.EXPAND_ALL_COMMENT_THREADS,
+ _ => {}
+ ); // docOnly
+ this.shortcutsController.addAbstract(
+ Shortcut.COLLAPSE_ALL_COMMENT_THREADS,
+ _ => {}
+ ); // docOnly
+ subscribe(
+ this,
+ () => this.getCommentsModel().changeComments$,
+ changeComments => {
+ this.changeComments = changeComments;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getBrowserModel().diffViewMode$,
+ diffView => {
+ this.diffViewMode = diffView;
+ }
+ );
+ subscribe(
+ this,
+ () => this.userModel.diffPreferences$,
+ diffPreferences => {
+ this.diffPrefs = diffPreferences;
+ fire(this, 'diff-prefs-changed', {value: this.diffPrefs});
+ }
+ );
+ subscribe(
+ this,
+ () =>
+ select(
+ this.userModel.preferences$,
+ prefs => !!prefs?.size_bar_in_change_table
+ ),
+ sizeBarInChangeTable => {
+ this.showSizeBars = sizeBarInChangeTable;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().reviewedFiles$,
+ reviewedFiles => {
+ this.reviewed = reviewedFiles ?? [];
+ }
+ );
+ }
+
+ override willUpdate(changedProperties: PropertyValues): void {
+ if (changedProperties.has('filesByPath')) {
+ this.updateCleanlyMergedPaths();
+ }
+ if (
+ changedProperties.has('diffPrefs') ||
+ changedProperties.has('diffViewMode')
+ ) {
+ this.updateDiffPreferences();
+ }
+ if (
+ changedProperties.has('filesByPath') ||
+ changedProperties.has('changeComments') ||
+ changedProperties.has('patchRange') ||
+ changedProperties.has('reviewed') ||
+ changedProperties.has('loading')
+ ) {
+ changedProperties.set('files', this.files);
+ this.computeFiles();
+ }
+ if (changedProperties.has('loading')) {
+ // Should run after files has been updated.
+ this.loadingChanged();
+ }
+ if (changedProperties.has('files')) {
+ this.filesChanged();
+ }
+ if (
+ changedProperties.has('files') ||
+ changedProperties.has('numFilesShown')
+ ) {
+ this.shownFiles = this.computeFilesShown();
+ }
+ if (changedProperties.has('expandedFiles')) {
+ changedProperties.set('filesExpanded', this.filesExpanded);
+ this.expandedFilesChanged(changedProperties.get('expandedFiles'));
+ }
+ if (changedProperties.has('filesExpanded')) {
+ fire(this, 'files-expanded-changed', {value: this.filesExpanded});
+ }
}
override connectedCallback() {
super.connectedCallback();
- this.subscriptions = [
- this.getCommentsModel().changeComments$.subscribe(changeComments => {
- this.changeComments = changeComments;
- }),
- this.getBrowserModel().diffViewMode$.subscribe(
- diffView => (this.diffViewMode = diffView)
- ),
- this.userModel.diffPreferences$.subscribe(diffPreferences => {
- this.diffPrefs = diffPreferences;
- }),
- select(
- this.userModel.preferences$,
- prefs => !!prefs?.size_bar_in_change_table
- ).subscribe(sizeBarInChangeTable => {
- this._showSizeBars = sizeBarInChangeTable;
- }),
- this.getChangeModel().reviewedFiles$.subscribe(reviewedFiles => {
- this.reviewed = reviewedFiles ?? [];
- }),
- ];
getPluginLoader()
.awaitPluginsLoaded()
.then(() => {
- this._dynamicHeaderEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ this.dynamicHeaderEndpoints = getPluginEndpoints().getDynamicEndpoints(
'change-view-file-list-header'
);
- this._dynamicContentEndpoints =
- getPluginEndpoints().getDynamicEndpoints(
- 'change-view-file-list-content'
- );
- this._dynamicPrependedHeaderEndpoints =
+ this.dynamicContentEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ 'change-view-file-list-content'
+ );
+ this.dynamicPrependedHeaderEndpoints =
getPluginEndpoints().getDynamicEndpoints(
'change-view-file-list-header-prepend'
);
- this._dynamicPrependedContentEndpoints =
+ this.dynamicPrependedContentEndpoints =
getPluginEndpoints().getDynamicEndpoints(
'change-view-file-list-content-prepend'
);
- this._dynamicSummaryEndpoints =
- getPluginEndpoints().getDynamicEndpoints(
- 'change-view-file-list-summary'
- );
+ this.dynamicSummaryEndpoints = getPluginEndpoints().getDynamicEndpoints(
+ 'change-view-file-list-summary'
+ );
if (
- this._dynamicHeaderEndpoints.length !==
- this._dynamicContentEndpoints.length
+ this.dynamicHeaderEndpoints.length !==
+ this.dynamicContentEndpoints.length
) {
this.reporting.error(new Error('dynamic header/content mismatch'));
}
if (
- this._dynamicPrependedHeaderEndpoints.length !==
- this._dynamicPrependedContentEndpoints.length
+ this.dynamicPrependedHeaderEndpoints.length !==
+ this.dynamicPrependedContentEndpoints.length
) {
this.reporting.error(new Error('dynamic header/content mismatch'));
}
if (
- this._dynamicHeaderEndpoints.length !==
- this._dynamicSummaryEndpoints.length
+ this.dynamicHeaderEndpoints.length !==
+ this.dynamicSummaryEndpoints.length
) {
this.reporting.error(new Error('dynamic header/content mismatch'));
}
});
this.cleanups.push(
- addGlobalShortcut({key: Key.ESC}, _ => this._handleEscKey()),
+ addGlobalShortcut({key: Key.ESC}, _ => this.handleEscKey()),
addShortcut(this, {key: Key.ENTER}, _ => this.handleOpenFile(), {
shouldSuppress: true,
})
@@ -456,19 +822,589 @@
}
override disconnectedCallback() {
- for (const s of this.subscriptions) {
- s.unsubscribe();
- }
- this.subscriptions = [];
this.diffCursor.dispose();
this.fileCursor.unsetCursor();
- this._cancelDiffs();
+ this.cancelDiffs();
this.loadingTask?.cancel();
for (const cleanup of this.cleanups) cleanup();
this.cleanups = [];
super.disconnectedCallback();
}
+ override render() {
+ this.classList.toggle('editMode', this.editMode);
+ const patchChange = this.calculatePatchChange();
+ return html`
+ <h3 class="assistive-tech-only">File list</h3>
+ ${this.renderContainer()} ${this.renderChangeTotals(patchChange)}
+ ${this.renderBinaryTotals(patchChange)} ${this.renderControlRow()}
+ <gr-diff-preferences-dialog
+ id="diffPreferencesDialog"
+ @reload-diff-preference=${this.handleReloadingDiffPreference}
+ >
+ </gr-diff-preferences-dialog>
+ `;
+ }
+
+ private renderContainer() {
+ return html`
+ <div
+ id="container"
+ @click=${(e: MouseEvent) => this.handleFileListClick(e)}
+ role="grid"
+ aria-label="Files list"
+ >
+ ${this.renderHeaderRow()} ${this.renderShownFiles()}
+ ${when(this.computeShowNumCleanlyMerged(), () =>
+ this.renderCleanlyMerged()
+ )}
+ </div>
+ `;
+ }
+
+ private renderHeaderRow() {
+ const showPrependedDynamicColumns =
+ this.computeShowPrependedDynamicColumns();
+ const showDynamicColumns = this.computeShowDynamicColumns();
+ return html` <div class="header-row row" role="row">
+ <!-- endpoint: change-view-file-list-header-prepend -->
+ ${when(showPrependedDynamicColumns, () =>
+ this.renderPrependedHeaderEndpoints()
+ )}
+
+ <div class="path" role="columnheader">File</div>
+ <div class="comments desktop" role="columnheader">Comments</div>
+ <div class="comments mobile" role="columnheader" title="Comments">C</div>
+ <div class="sizeBars desktop" role="columnheader">Size</div>
+ <div class="header-stats" role="columnheader">Delta</div>
+ <!-- endpoint: change-view-file-list-header -->
+ ${when(showDynamicColumns, () => this.renderDynamicHeaderEndpoints())}
+ <!-- Empty div here exists to keep spacing in sync with file rows. -->
+ <div
+ class="reviewed hideOnEdit"
+ ?hidden=${!this.loggedIn}
+ aria-hidden="true"
+ ></div>
+ <div class="editFileControls showOnEdit" aria-hidden="true"></div>
+ <div class="show-hide" aria-hidden="true"></div>
+ </div>`;
+ }
+
+ private renderPrependedHeaderEndpoints() {
+ return this.dynamicPrependedHeaderEndpoints?.map(
+ headerEndpoint => html`
+ <gr-endpoint-decorator
+ class="prepended-col"
+ .name=${headerEndpoint}
+ role="columnheader"
+ >
+ <gr-endpoint-param name="change" .value=${this.change}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="patchRange" .value=${this.patchRange}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="files" .value=${this.files}>
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ `
+ );
+ }
+
+ private renderDynamicHeaderEndpoints() {
+ return this.dynamicHeaderEndpoints?.map(
+ headerEndpoint => html`
+ <gr-endpoint-decorator
+ class="extra-col"
+ .name=${headerEndpoint}
+ role="columnheader"
+ ></gr-endpoint-decorator>
+ `
+ );
+ }
+
+ private renderShownFiles() {
+ const showDynamicColumns = this.computeShowDynamicColumns();
+ const showPrependedDynamicColumns =
+ this.computeShowPrependedDynamicColumns();
+ const sizeBarLayout = this.computeSizeBarLayout();
+
+ return incrementalRepeat({
+ values: this.shownFiles,
+ mapFn: (f, i) =>
+ this.renderFileRow(
+ f as NormalizedFileInfo,
+ i,
+ sizeBarLayout,
+ showDynamicColumns,
+ showPrependedDynamicColumns
+ ),
+ initialCount: this.fileListIncrement,
+ targetFrameRate: 30,
+ });
+ }
+
+ private renderFileRow(
+ file: NormalizedFileInfo,
+ index: number,
+ sizeBarLayout: SizeBarLayout,
+ showDynamicColumns: boolean,
+ showPrependedDynamicColumns: boolean
+ ) {
+ this.reportRenderedRow(index);
+ const patchSetFile = this.computePatchSetFile(file);
+ return html` <div class="stickyArea">
+ <div
+ class=${`file-row row ${this.computePathClass(file.__path)}`}
+ data-file=${JSON.stringify(patchSetFile)}
+ tabindex="-1"
+ role="row"
+ >
+ <!-- endpoint: change-view-file-list-content-prepend -->
+ ${when(showPrependedDynamicColumns, () =>
+ this.renderPrependedContentEndpointsForFile(file)
+ )}
+ ${this.renderFilePath(file)} ${this.renderFileComments(file)}
+ ${this.renderSizeBar(file, sizeBarLayout)} ${this.renderFileStats(file)}
+ ${when(showDynamicColumns, () =>
+ this.renderDynamicContentEndpointsForFile(file)
+ )}
+ <!-- endpoint: change-view-file-list-content -->
+ ${this.renderReviewed(file)} ${this.renderFileControls(file)}
+ ${this.renderShowHide(file)}
+ </div>
+ ${when(
+ this.isFileExpanded(file.__path),
+ () => html`
+ <gr-diff-host
+ ?noAutoRender=${true}
+ ?showLoadFailure=${true}
+ .displayLine=${this.displayLine}
+ .changeNum=${this.changeNum}
+ .change=${this.change}
+ .patchRange=${this.patchRange}
+ .file=${patchSetFile}
+ .path=${file.__path}
+ .prefs=${this.diffPrefs}
+ .projectName=${this.change?.project}
+ ?noRenderOnPrefsChange=${true}
+ ></gr-diff-host>
+ `
+ )}
+ </div>`;
+ }
+
+ private renderPrependedContentEndpointsForFile(file: NormalizedFileInfo) {
+ return this.dynamicPrependedContentEndpoints?.map(
+ contentEndpoint => html`
+ <gr-endpoint-decorator
+ class="prepended-col"
+ .name=${contentEndpoint}
+ role="gridcell"
+ >
+ <gr-endpoint-param name="change" .value=${this.change}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="changeNum" .value=${this.changeNum}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="patchRange" .value=${this.patchRange}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="path" .value=${file.__path}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="oldPath" .value=${this.getOldPath(file)}>
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ `
+ );
+ }
+
+ private renderFilePath(file: NormalizedFileInfo) {
+ return html` <span class="path" role="gridcell">
+ <a class="pathLink" href=${ifDefined(this.computeDiffURL(file.__path))}>
+ <span title=${computeDisplayPath(file.__path)} class="fullFileName">
+ ${computeDisplayPath(file.__path)}
+ </span>
+ <span
+ title=${computeDisplayPath(file.__path)}
+ class="truncatedFileName"
+ >
+ ${computeTruncatedPath(file.__path)}
+ </span>
+ <gr-file-status-chip .file=${file}></gr-file-status-chip>
+ <gr-copy-clipboard
+ ?hideInput=${true}
+ .text=${file.__path}
+ ></gr-copy-clipboard>
+ </a>
+ ${when(
+ file.old_path,
+ () => html`
+ <div class="oldPath" title=${ifDefined(file.old_path)}>
+ [[file.old_path]]
+ <gr-copy-clipboard
+ ?hideInput=${true}
+ .text=${file.old_path}
+ ></gr-copy-clipboard>
+ </div>
+ `
+ )}
+ </span>`;
+ }
+
+ private renderFileComments(file: NormalizedFileInfo) {
+ return html` <div role="gridcell">
+ <div class="comments desktop">
+ <span class="drafts">${this.computeDraftsString(file)}</span>
+ <span>${this.computeCommentsString(file)}</span>
+ <span class="noCommentsScreenReaderText">
+ <!-- Screen readers read the following content only if 2 other
+ spans in the parent div is empty. The content is not visible on
+ the page.
+ Without this span, screen readers don't navigate correctly inside
+ table, because empty div doesn't rendered. For example, VoiceOver
+ jumps back to the whole table.
+ We can use   instead, but it sounds worse.
+ -->
+ No comments
+ </span>
+ </div>
+ <div class="comments mobile">
+ <span class="drafts">${this.computeDraftsStringMobile(file)}</span>
+ <span>${this.computeCommentsStringMobile(file)}</span>
+ <span class="noCommentsScreenReaderText">
+ <!-- The same as for desktop comments -->
+ No comments
+ </span>
+ </div>
+ </div>`;
+ }
+
+ private renderSizeBar(
+ file: NormalizedFileInfo,
+ sizeBarLayout: SizeBarLayout
+ ) {
+ return html` <div class="desktop" role="gridcell">
+ <!-- The content must be in a separate div. It guarantees, that
+ gridcell always visible for screen readers.
+ For example, without a nested div screen readers pronounce the
+ "Commit message" row content with incorrect column headers.
+ -->
+ <div
+ class=${this.computeSizeBarsClass(file.__path)}
+ aria-label="A bar that represents the addition and deletion ratio for the current file"
+ >
+ <svg width="61" height="8">
+ <rect
+ x=${this.computeBarAdditionX(file, sizeBarLayout)}
+ y="0"
+ height="8"
+ fill="var(--positive-green-text-color)"
+ width=${this.computeBarAdditionWidth(file, sizeBarLayout)}
+ ></rect>
+ <rect
+ x=${this.computeBarDeletionX(sizeBarLayout)}
+ y="0"
+ height="8"
+ fill="var(--negative-red-text-color)"
+ width=${this.computeBarDeletionWidth(file, sizeBarLayout)}
+ ></rect>
+ </svg>
+ </div>
+ </div>`;
+ }
+
+ private renderFileStats(file: NormalizedFileInfo) {
+ return html` <div class="stats" role="gridcell">
+ <!-- The content must be in a separate div. It guarantees, that
+ gridcell always visible for screen readers.
+ For example, without a nested div screen readers pronounce the
+ "Commit message" row content with incorrect column headers.
+ -->
+ <div class=${this.computeClass('', file.__path)}>
+ <span
+ class="added"
+ tabindex="0"
+ aria-label=${`${file.lines_inserted} lines added`}
+ ?hidden=${file.binary}
+ >
+ +${file.lines_inserted}
+ </span>
+ <span
+ class="removed"
+ tabindex="0"
+ aria-label=${`${file.lines_deleted} lines removed`}
+ ?hidden=${file.binary}
+ >
+ -${file.lines_deleted}
+ </span>
+ <span
+ class=${ifDefined(this.computeBinaryClass(file.size_delta))}
+ ?hidden=${!file.binary}
+ >
+ ${this.formatBytes(file.size_delta)}
+ ${this.formatPercentage(file.size, file.size_delta)}
+ </span>
+ </div>
+ </div>`;
+ }
+
+ private renderDynamicContentEndpointsForFile(file: NormalizedFileInfo) {
+ this.dynamicContentEndpoints?.map(
+ contentEndpoint => html` <div
+ class=${this.computeClass('', file.__path)}
+ role="gridcell"
+ >
+ <gr-endpoint-decorator class="extra-col" .name=${contentEndpoint}>
+ <gr-endpoint-param name="change" .value=${this.change}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="changeNum" .value=${this.changeNum}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="patchRange" .value=${this.patchRange}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="path" .value=${file.__path}>
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ </div>`
+ );
+ }
+
+ private renderReviewed(file: NormalizedFileInfo) {
+ if (!this.loggedIn) return nothing;
+ return html` <div class="reviewed hideOnEdit" role="gridcell">
+ <span
+ class=${`reviewedLabel ${this.computeReviewedClass(file.isReviewed)}`}
+ aria-hidden=${this.booleanToString(!file.isReviewed)}
+ >Reviewed</span
+ >
+ <!-- Do not use input type="checkbox" with hidden input and
+ visible label here. Screen readers don't read/interract
+ correctly with such input.
+ -->
+ <span
+ class="reviewedSwitch"
+ role="switch"
+ tabindex="0"
+ @click=${(e: MouseEvent) => this.reviewedClick(e)}
+ @keydown=${(e: KeyboardEvent) => this.reviewedClick(e)}
+ aria-label="Reviewed"
+ aria-checked=${this.booleanToString(file.isReviewed)}
+ >
+ <!-- Trick with tabindex to avoid outline on mouse focus, but
+ preserve focus outline for keyboard navigation -->
+ <span
+ tabindex="-1"
+ class="markReviewed"
+ title=${this.reviewedTitle(file.isReviewed)}
+ >${this.computeReviewedText(file.isReviewed)}</span
+ >
+ </span>
+ </div>`;
+ }
+
+ private renderFileControls(file: NormalizedFileInfo) {
+ return html` <div
+ class="editFileControls showOnEdit"
+ role="gridcell"
+ aria-hidden=${this.booleanToString(!this.editMode)}
+ >
+ ${when(
+ this.editMode,
+ () => html`
+ <gr-edit-file-controls
+ class=${this.computeClass('', file.__path)}
+ .filePath=${file.__path}
+ ></gr-edit-file-controls>
+ `
+ )}
+ </div>`;
+ }
+
+ private renderShowHide(file: NormalizedFileInfo) {
+ return html` <div class="show-hide" role="gridcell">
+ <!-- Do not use input type="checkbox" with hidden input and
+ visible label here. Screen readers don't read/interract
+ correctly with such input.
+ -->
+ <span
+ class="show-hide"
+ data-path=${file.__path}
+ data-expand="true"
+ role="switch"
+ tabindex="0"
+ aria-checked=${this.isFileExpandedStr(file.__path)}
+ aria-label="Expand file"
+ @click=${this.expandedClick}
+ @keydown=${this.expandedClick}
+ >
+ <!-- Trick with tabindex to avoid outline on mouse focus, but
+ preserve focus outline for keyboard navigation -->
+ <iron-icon
+ class="show-hide-icon"
+ tabindex="-1"
+ id="icon"
+ icon=${this.computeShowHideIcon(file.__path)}
+ >
+ </iron-icon>
+ </span>
+ </div>`;
+ }
+
+ private renderCleanlyMerged() {
+ const showPrependedDynamicColumns =
+ this.computeShowPrependedDynamicColumns();
+ return html` <div class="row">
+ <!-- endpoint: change-view-file-list-content-prepend -->
+ ${when(showPrependedDynamicColumns, () =>
+ this.renderPrependedContentEndpoints()
+ )}
+ <div role="gridcell">
+ <div>
+ <span class="cleanlyMergedText">
+ ${this.computeCleanlyMergedText()}
+ </span>
+ <gr-button
+ link
+ class="showParentButton"
+ @click=${this.handleShowParent1}
+ >
+ Show Parent 1
+ </gr-button>
+ </div>
+ </div>
+ </div>`;
+ }
+
+ private renderPrependedContentEndpoints() {
+ return this.dynamicPrependedContentEndpoints?.map(
+ contentEndpoint => html`
+ <gr-endpoint-decorator
+ class="prepended-col"
+ .name=${contentEndpoint}
+ role="gridcell"
+ >
+ <gr-endpoint-param name="change" .value=${this.change}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="changeNum" .value=${this.changeNum}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="patchRange" .value=${this.patchRange}>
+ </gr-endpoint-param>
+ <gr-endpoint-param
+ name="cleanlyMergedPaths"
+ .value=${this.cleanlyMergedPaths}
+ >
+ </gr-endpoint-param>
+ <gr-endpoint-param
+ name="cleanlyMergedOldPaths"
+ .value=${this.cleanlyMergedOldPaths}
+ >
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ `
+ );
+ }
+
+ private renderChangeTotals(patchChange: PatchChange) {
+ const showDynamicColumns = this.computeShowDynamicColumns();
+ if (this.shouldHideChangeTotals(patchChange)) return nothing;
+ return html`
+ <div class="row totalChanges">
+ <div class="total-stats">
+ <div>
+ <span
+ class="added"
+ tabindex="0"
+ aria-label="Total ${patchChange.inserted} lines added"
+ >
+ +${patchChange.inserted}
+ </span>
+ <span
+ class="removed"
+ tabindex="0"
+ aria-label="Total ${patchChange.deleted} lines removed"
+ >
+ -${patchChange.deleted}
+ </span>
+ </div>
+ </div>
+ ${when(showDynamicColumns, () =>
+ this.dynamicSummaryEndpoints?.map(
+ summaryEndpoint => html`
+ <gr-endpoint-decorator class="extra-col" name=${summaryEndpoint}>
+ <gr-endpoint-param name="change" .value=${this.change}>
+ </gr-endpoint-param>
+ <gr-endpoint-param name="patchRange" .value=${this.patchRange}>
+ </gr-endpoint-param>
+ </gr-endpoint-decorator>
+ `
+ )
+ )}
+
+ <!-- Empty div here exists to keep spacing in sync with file rows. -->
+ <div class="reviewed hideOnEdit" ?hidden=${!this.loggedIn}></div>
+ <div class="editFileControls showOnEdit"></div>
+ <div class="show-hide"></div>
+ </div>
+ `;
+ }
+
+ private renderBinaryTotals(patchChange: PatchChange) {
+ if (this.shouldHideBinaryChangeTotals(patchChange)) return nothing;
+ const deltaInserted = this.formatBytes(patchChange.size_delta_inserted);
+ const deltaDeleted = this.formatBytes(patchChange.size_delta_deleted);
+ return html`
+ <div class="row totalChanges">
+ <div class="total-stats">
+ <span
+ class="added"
+ aria-label="Total bytes inserted: ${deltaInserted}"
+ >
+ ${deltaInserted}
+ ${this.formatPercentage(
+ patchChange.total_size,
+ patchChange.size_delta_inserted
+ )}
+ </span>
+ <span
+ class="removed"
+ aria-label="Total bytes removed: ${deltaDeleted}"
+ >
+ ${deltaDeleted}
+ ${this.formatPercentage(
+ patchChange.total_size,
+ patchChange.size_delta_deleted
+ )}
+ </span>
+ </div>
+ </div>
+ `;
+ }
+
+ private renderControlRow() {
+ return html`<div
+ class=${`row controlRow ${this.computeFileListControlClass()}`}
+ >
+ <gr-button
+ class="fileListButton"
+ id="incrementButton"
+ link=""
+ @click=${this.incrementNumFilesShown}
+ >
+ ${this.computeIncrementText()}
+ </gr-button>
+ <gr-tooltip-content
+ ?has-tooltip=${this.computeWarnShowAll()}
+ ?show-icon=${this.computeWarnShowAll()}
+ .title=${this.computeShowAllWarning()}
+ >
+ <gr-button
+ class="fileListButton"
+ id="showAllButton"
+ link=""
+ @click=${this.showAllFiles}
+ >
+ ${this.computeShowAllText()}
+ </gr-button>
+ </gr-tooltip-content>
+ </div>`;
+ }
+
reload() {
if (!this.changeNum || !this.patchRange?.patchNum) {
return Promise.resolve();
@@ -476,7 +1412,7 @@
const changeNum = this.changeNum;
const patchRange = this.patchRange;
- this._loading = true;
+ this.loading = true;
this.collapseAllDiffs();
const promises: Promise<boolean | void>[] = [];
@@ -485,23 +1421,22 @@
this.restApiService
.getChangeOrEditFiles(changeNum, patchRange)
.then(filesByPath => {
- this._filesByPath = filesByPath;
+ this.filesByPath = filesByPath;
})
);
promises.push(
- this._getLoggedIn().then(loggedIn => (this._loggedIn = loggedIn))
+ this.getLoggedIn().then(loggedIn => (this.loggedIn = loggedIn))
);
return Promise.all(promises).then(() => {
- this._loading = false;
- this._detectChromiteButler();
+ this.loading = false;
+ this.detectChromiteButler();
this.reporting.fileListDisplayed();
});
}
- @observe('_filesByPath')
- async _updateCleanlyMergedPaths(filesByPath?: FileNameToFileInfoMap) {
+ private async updateCleanlyMergedPaths() {
// When viewing Auto Merge base vs a patchset, add an additional row that
// knows how many files were cleanly merged. This requires an additional RPC
// for the diffs between target parent and the patch set. The cleanly merged
@@ -522,21 +1457,21 @@
patchNum: this.patchRange.patchNum,
}
);
- if (!allFilesByPath || !filesByPath) return;
- const conflictingPaths = Object.keys(filesByPath);
- this._cleanlyMergedPaths = Object.keys(allFilesByPath).filter(
+ if (!allFilesByPath || !this.filesByPath) return;
+ const conflictingPaths = Object.keys(this.filesByPath);
+ this.cleanlyMergedPaths = Object.keys(allFilesByPath).filter(
path => !conflictingPaths.includes(path)
);
- this._cleanlyMergedOldPaths = this._cleanlyMergedPaths
+ this.cleanlyMergedOldPaths = this.cleanlyMergedPaths
.map(path => allFilesByPath[path].old_path)
.filter((oldPath): oldPath is string => !!oldPath);
} else {
- this._cleanlyMergedPaths = [];
- this._cleanlyMergedOldPaths = [];
+ this.cleanlyMergedPaths = [];
+ this.cleanlyMergedOldPaths = [];
}
}
- _detectChromiteButler() {
+ private detectChromiteButler() {
const hasButler = !!document.getElementById('butler-suggested-owners');
if (hasButler) {
this.reporting.reportExtension('butler');
@@ -544,7 +1479,7 @@
}
get diffs(): GrDiffHost[] {
- const diffs = this.root!.querySelectorAll('gr-diff-host');
+ const diffs = this.shadowRoot!.querySelectorAll('gr-diff-host');
// It is possible that a bogus diff element is hanging around invisibly
// from earlier with a different patch set choice and associated with a
// different entry in the files array. So filter on visible items only.
@@ -554,12 +1489,13 @@
}
openDiffPrefs() {
- this.$.diffPreferencesDialog.open();
+ this.diffPreferencesDialog?.open();
}
- _calculatePatchChange(files: NormalizedFileInfo[]): PatchChange {
- const magicFilesExcluded = files.filter(
- files => !isMagicPath(files.__path)
+ // Private but used in tests.
+ calculatePatchChange(): PatchChange {
+ const magicFilesExcluded = this.files.filter(
+ file => !isMagicPath(file.__path)
);
return magicFilesExcluded.reduce((acc, obj) => {
@@ -582,40 +1518,39 @@
}
// private but used in test
- _toggleFileExpanded(file: PatchSetFile) {
+ toggleFileExpanded(file: PatchSetFile) {
// Is the path in the list of expanded diffs? If so, remove it, otherwise
// add it to the list.
- const indexInExpanded = this._expandedFiles.findIndex(
+ const indexInExpanded = this.expandedFiles.findIndex(
f => f.path === file.path
);
if (indexInExpanded === -1) {
- this.push('_expandedFiles', file);
+ this.expandedFiles = this.expandedFiles.concat([file]);
} else {
- this.splice('_expandedFiles', indexInExpanded, 1);
+ this.expandedFiles = this.expandedFiles.filter(
+ (_val, idx) => idx !== indexInExpanded
+ );
}
- const indexInAll = this._files.findIndex(f => f.__path === file.path);
- this.root!.querySelectorAll(`.${FILE_ROW_CLASS}`)[
+ const indexInAll = this.files.findIndex(f => f.__path === file.path);
+ this.shadowRoot!.querySelectorAll(`.${FILE_ROW_CLASS}`)[
indexInAll
].scrollIntoView({block: 'nearest'});
}
- _toggleFileExpandedByIndex(index: number) {
- this._toggleFileExpanded(this._computePatchSetFile(this._files[index]));
+ private toggleFileExpandedByIndex(index: number) {
+ this.toggleFileExpanded(this.computePatchSetFile(this.files[index]));
}
- _updateDiffPreferences() {
+ // Private but used in tests.
+ updateDiffPreferences() {
if (!this.diffs.length) {
return;
}
// Re-render all expanded diffs sequentially.
- this._renderInOrder(
- this._expandedFiles,
- this.diffs,
- this._expandedFiles.length
- );
+ this.renderInOrder(this.expandedFiles, this.diffs);
}
- _forEachDiff(fn: (host: GrDiffHost) => void) {
+ private forEachDiff(fn: (host: GrDiffHost) => void) {
const diffs = this.diffs;
for (let i = 0; i < diffs.length; i++) {
fn(diffs[i]);
@@ -627,54 +1562,45 @@
// expanded list.
const newFiles: PatchSetFile[] = [];
let path: string;
- for (let i = 0; i < this._shownFiles.length; i++) {
- path = this._shownFiles[i].__path;
- if (!this._expandedFiles.some(f => f.path === path)) {
- newFiles.push(this._computePatchSetFile(this._shownFiles[i]));
+ for (let i = 0; i < this.shownFiles.length; i++) {
+ path = this.shownFiles[i].__path;
+ if (!this.expandedFiles.some(f => f.path === path)) {
+ newFiles.push(this.computePatchSetFile(this.shownFiles[i]));
}
}
- this.splice('_expandedFiles', 0, 0, ...newFiles);
+ this.expandedFiles = newFiles.concat(this.expandedFiles);
}
collapseAllDiffs() {
- this._expandedFiles = [];
- this.filesExpanded = this._computeExpandedFiles(
- this._expandedFiles.length,
- this._files.length
- );
- this.diffCursor.handleDiffUpdate();
+ this.expandedFiles = [];
}
/**
* Computes a string with the number of comments and unresolved comments.
*/
- _computeCommentsString(
- changeComments?: ChangeComments,
- patchRange?: PatchRange,
- file?: NormalizedFileInfo
- ) {
+ computeCommentsString(file?: NormalizedFileInfo) {
if (
- changeComments === undefined ||
- patchRange === undefined ||
+ this.changeComments === undefined ||
+ this.patchRange === undefined ||
file?.__path === undefined
) {
return '';
}
- return changeComments.computeCommentsString(patchRange, file.__path, file);
+ return this.changeComments.computeCommentsString(
+ this.patchRange,
+ file.__path,
+ file
+ );
}
/**
* Computes a string with the number of drafts.
*/
- _computeDraftsString(
- changeComments?: ChangeComments,
- patchRange?: PatchRange,
- file?: NormalizedFileInfo
- ) {
- if (changeComments === undefined) return '';
- const draftCount = changeComments.computeDraftCountForFile(
- patchRange,
+ computeDraftsString(file?: NormalizedFileInfo) {
+ if (this.changeComments === undefined) return '';
+ const draftCount = this.changeComments.computeDraftCountForFile(
+ this.patchRange,
file
);
if (draftCount === 0) return '';
@@ -683,15 +1609,12 @@
/**
* Computes a shortened string with the number of drafts.
+ * Private but used in tests.
*/
- _computeDraftsStringMobile(
- changeComments?: ChangeComments,
- patchRange?: PatchRange,
- file?: NormalizedFileInfo
- ) {
- if (changeComments === undefined) return '';
- const draftCount = changeComments.computeDraftCountForFile(
- patchRange,
+ computeDraftsStringMobile(file?: NormalizedFileInfo) {
+ if (this.changeComments === undefined) return '';
+ const draftCount = this.changeComments.computeDraftCountForFile(
+ this.patchRange,
file
);
return draftCount === 0 ? '' : `${draftCount}d`;
@@ -700,43 +1623,38 @@
/**
* Computes a shortened string with the number of comments.
*/
- _computeCommentsStringMobile(
- changeComments?: ChangeComments,
- patchRange?: PatchRange,
- file?: NormalizedFileInfo
- ) {
+ computeCommentsStringMobile(file?: NormalizedFileInfo) {
if (
- changeComments === undefined ||
- patchRange === undefined ||
+ this.changeComments === undefined ||
+ this.patchRange === undefined ||
file === undefined
) {
return '';
}
const commentThreadCount =
- changeComments.computeCommentThreadCount({
- patchNum: patchRange.basePatchNum,
+ this.changeComments.computeCommentThreadCount({
+ patchNum: this.patchRange.basePatchNum,
path: file.__path,
}) +
- changeComments.computeCommentThreadCount({
- patchNum: patchRange.patchNum,
+ this.changeComments.computeCommentThreadCount({
+ patchNum: this.patchRange.patchNum,
path: file.__path,
});
return commentThreadCount === 0 ? '' : `${commentThreadCount}c`;
}
- // private but used in test
- _reviewFile(path: string, reviewed?: boolean) {
+ // Private but used in tests.
+ reviewFile(path: string, reviewed?: boolean) {
if (this.editMode) {
return Promise.resolve();
}
- const index = this._files.findIndex(file => file.__path === path);
- reviewed = reviewed || !this._files[index].isReviewed;
-
- this.set(['_files', index, 'isReviewed'], reviewed);
- if (index < this._shownFiles.length) {
- this.notifyPath(`_shownFiles.${index}.isReviewed`);
+ const index = this.files.findIndex(file => file.__path === path);
+ reviewed = reviewed || !this.files[index].isReviewed;
+ this.files[index].isReviewed = reviewed;
+ if (index < this.shownFiles.length) {
+ this.requestUpdate('shownFiles');
}
-
+ this.requestUpdate('files');
return this._saveReviewedState(path, reviewed);
}
@@ -753,18 +1671,11 @@
);
}
- _getLoggedIn() {
+ private getLoggedIn() {
return this.restApiService.getLoggedIn();
}
- _getReviewedFiles(changeNum: NumericChangeId, patchRange: PatchRange) {
- if (this.editMode) {
- return Promise.resolve([]);
- }
- return this.restApiService.getReviewedFiles(changeNum, patchRange.patchNum);
- }
-
- _normalizeChangeFilesResponse(
+ private normalizeChangeFilesResponse(
response: FileNameToReviewedFileInfoMap
): NormalizedFileInfo[] {
const paths = Object.keys(response).sort(specialFilePathCompare);
@@ -786,7 +1697,7 @@
* The click is: mouse click or pressing Enter or Space key
* P.S> Screen readers sends click event as well
*/
- _isClickEvent(e: MouseEvent | KeyboardEvent) {
+ private isClickEvent(e: MouseEvent | KeyboardEvent) {
if (e.type === 'click') {
return true;
}
@@ -795,41 +1706,43 @@
return ke.type === 'keydown' && isSpaceOrEnter;
}
- _fileActionClick(
+ private fileActionClick(
e: MouseEvent | KeyboardEvent,
fileAction: (file: PatchSetFile) => void
) {
- if (this._isClickEvent(e)) {
- const fileRow = this._getFileRowFromEvent(e);
+ if (this.isClickEvent(e)) {
+ const fileRow = this.getFileRowFromEvent(e);
if (!fileRow) {
return;
}
// Prevent default actions (e.g. scrolling for space key)
e.preventDefault();
- // Prevent _handleFileListClick handler call
+ // Prevent handleFileListClick handler call
e.stopPropagation();
this.fileCursor.setCursor(fileRow.element);
fileAction(fileRow.file);
}
}
- _reviewedClick(e: MouseEvent | KeyboardEvent) {
- this._fileActionClick(e, file => this._reviewFile(file.path));
+ // Private but used in tests.
+ reviewedClick(e: MouseEvent | KeyboardEvent) {
+ this.fileActionClick(e, file => this.reviewFile(file.path));
}
- _expandedClick(e: MouseEvent | KeyboardEvent) {
- this._fileActionClick(e, file => this._toggleFileExpanded(file));
+ private expandedClick(e: MouseEvent | KeyboardEvent) {
+ this.fileActionClick(e, file => this.toggleFileExpanded(file));
}
/**
* Handle all events from the file list dom-repeat so event handlers don't
* have to get registered for potentially very long lists.
+ * Private but used in tests.
*/
- _handleFileListClick(e: MouseEvent) {
+ handleFileListClick(e: MouseEvent) {
if (!e.target) {
return;
}
- const fileRow = this._getFileRowFromEvent(e);
+ const fileRow = this.getFileRowFromEvent(e);
if (!fileRow) {
return;
}
@@ -849,10 +1762,10 @@
e.preventDefault();
this.fileCursor.setCursor(fileRow.element);
- this._toggleFileExpanded(file);
+ this.toggleFileExpanded(file);
}
- _getFileRowFromEvent(e: Event): FileRow | null {
+ private getFileRowFromEvent(e: Event): FileRow | null {
// Traverse upwards to find the row element if the target is not the row.
let row = e.target as HTMLElement;
while (!row.classList.contains(FILE_ROW_CLASS) && row.parentElement) {
@@ -873,7 +1786,7 @@
/**
* Generates file range from file info object.
*/
- _computePatchSetFile(file: NormalizedFileInfo): PatchSetFile {
+ private computePatchSetFile(file: NormalizedFileInfo): PatchSetFile {
const fileData: PatchSetFile = {
path: file.__path,
};
@@ -883,90 +1796,95 @@
return fileData;
}
- _handleLeftPane() {
- if (this._noDiffsExpanded()) return;
+ private handleLeftPane() {
+ if (this.noDiffsExpanded()) return;
this.diffCursor.moveLeft();
}
- _handleRightPane() {
- if (this._noDiffsExpanded()) return;
+ private handleRightPane() {
+ if (this.noDiffsExpanded()) return;
this.diffCursor.moveRight();
}
- _handleToggleInlineDiff() {
+ private handleToggleInlineDiff() {
if (this.fileCursor.index === -1) return;
- this._toggleFileExpandedByIndex(this.fileCursor.index);
+ this.toggleFileExpandedByIndex(this.fileCursor.index);
}
- _handleCursorNext(e: KeyboardEvent) {
+ // Private but used in tests.
+ handleCursorNext(e: KeyboardEvent) {
if (this.filesExpanded === FilesExpandedState.ALL) {
this.diffCursor.moveDown();
- this._displayLine = true;
+ this.displayLine = true;
} else {
if (e.key === Key.DOWN) return;
this.fileCursor.next({circular: true});
this.selectedIndex = this.fileCursor.index;
+ fire(this, 'selected-index-changed', {value: this.fileCursor.index});
}
}
- _handleCursorPrev(e: KeyboardEvent) {
+ // Private but used in tests.
+ handleCursorPrev(e: KeyboardEvent) {
if (this.filesExpanded === FilesExpandedState.ALL) {
this.diffCursor.moveUp();
- this._displayLine = true;
+ this.displayLine = true;
} else {
if (e.key === Key.UP) return;
this.fileCursor.previous({circular: true});
this.selectedIndex = this.fileCursor.index;
+ fire(this, 'selected-index-changed', {value: this.fileCursor.index});
}
}
- _handleNewComment() {
+ private handleNewComment() {
this.classList.remove('hideComments');
this.diffCursor.createCommentInPlace();
}
+ // Private but used in tests.
handleOpenFile() {
if (this.filesExpanded === FilesExpandedState.ALL) {
- this._openCursorFile();
+ this.openCursorFile();
return;
}
- this._openSelectedFile();
+ this.openSelectedFile();
}
- _handleNextChunk() {
- if (this._noDiffsExpanded()) return;
+ private handleNextChunk() {
+ if (this.noDiffsExpanded()) return;
this.diffCursor.moveToNextChunk();
}
- _handleNextComment() {
- if (this._noDiffsExpanded()) return;
+ private handleNextComment() {
+ if (this.noDiffsExpanded()) return;
this.diffCursor.moveToNextCommentThread();
}
- _handlePrevChunk() {
- if (this._noDiffsExpanded()) return;
+ private handlePrevChunk() {
+ if (this.noDiffsExpanded()) return;
this.diffCursor.moveToPreviousChunk();
}
- _handlePrevComment() {
- if (this._noDiffsExpanded()) return;
+ private handlePrevComment() {
+ if (this.noDiffsExpanded()) return;
this.diffCursor.moveToPreviousCommentThread();
}
- _handleToggleFileReviewed() {
- if (!this._files[this.fileCursor.index]) {
+ private handleToggleFileReviewed() {
+ if (!this.files[this.fileCursor.index]) {
return;
}
- this._reviewFile(this._files[this.fileCursor.index].__path);
+ this.reviewFile(this.files[this.fileCursor.index].__path);
}
- _handleToggleLeftPane() {
- this._forEachDiff(diff => {
+ private handleToggleLeftPane() {
+ this.forEachDiff(diff => {
diff.toggleLeftDiff();
});
}
- _toggleInlineDiffs() {
+ private toggleInlineDiffs() {
if (this.filesExpanded === FilesExpandedState.ALL) {
this.collapseAllDiffs();
} else {
@@ -974,7 +1892,8 @@
}
}
- _openCursorFile() {
+ // Private but used in tests.
+ openCursorFile() {
const diff = this.diffCursor.getTargetDiffElement();
if (!this.change || !diff || !this.patchRange || !diff.path) {
throw new Error('change, diff and patchRange must be all set and valid');
@@ -987,11 +1906,12 @@
);
}
- _openSelectedFile(index?: number) {
+ // Private but used in tests.
+ openSelectedFile(index?: number) {
if (index !== undefined) {
this.fileCursor.setCursorAtIndex(index);
}
- if (!this._files[this.fileCursor.index]) {
+ if (!this.files[this.fileCursor.index]) {
return;
}
if (!this.change || !this.patchRange) {
@@ -999,45 +1919,52 @@
}
GerritNav.navigateToDiff(
this.change,
- this._files[this.fileCursor.index].__path,
+ this.files[this.fileCursor.index].__path,
this.patchRange.patchNum,
this.patchRange.basePatchNum
);
}
- _shouldHideChangeTotals(_patchChange: PatchChange): boolean {
- return _patchChange.inserted === 0 && _patchChange.deleted === 0;
+ // Private but used in tests.
+ shouldHideChangeTotals(patchChange: PatchChange): boolean {
+ return patchChange.inserted === 0 && patchChange.deleted === 0;
}
- _shouldHideBinaryChangeTotals(_patchChange: PatchChange) {
+ // Private but used in tests.
+ shouldHideBinaryChangeTotals(patchChange: PatchChange) {
return (
- _patchChange.size_delta_inserted === 0 &&
- _patchChange.size_delta_deleted === 0
+ patchChange.size_delta_inserted === 0 &&
+ patchChange.size_delta_deleted === 0
);
}
- _computeDiffURL(
- change?: ParsedChangeInfo,
- basePatchNum?: BasePatchSetNum,
- patchNum?: RevisionPatchSetNum,
- path?: string,
- editMode?: boolean
- ) {
+ // Private but used in tests
+ computeDiffURL(path?: string) {
if (
- change === undefined ||
- patchNum === undefined ||
+ this.change === undefined ||
+ this.patchRange?.patchNum === undefined ||
path === undefined ||
- editMode === undefined
+ this.editMode === undefined
) {
return;
}
- if (editMode && path !== SpecialFilePath.MERGE_LIST) {
- return GerritNav.getEditUrlForDiff(change, path, patchNum);
+ if (this.editMode && path !== SpecialFilePath.MERGE_LIST) {
+ return GerritNav.getEditUrlForDiff(
+ this.change,
+ path,
+ this.patchRange.patchNum
+ );
}
- return GerritNav.getUrlForDiff(change, path, patchNum, basePatchNum);
+ return GerritNav.getUrlForDiff(
+ this.change,
+ path,
+ this.patchRange.patchNum,
+ this.patchRange.basePatchNum
+ );
}
- _formatBytes(bytes?: number) {
+ // Private but used in tests.
+ formatBytes(bytes?: number) {
if (!bytes) return '+/-0 B';
const bits = 1024;
const decimals = 1;
@@ -1050,7 +1977,8 @@
return `${prepend}${value} ${sizes[exponent]}`;
}
- _formatPercentage(size?: number, delta?: number) {
+ // Private but used in tests.
+ formatPercentage(size?: number, delta?: number) {
if (size === undefined || delta === undefined) {
return '';
}
@@ -1064,14 +1992,14 @@
return `(${delta > 0 ? '+' : '-'}${percentage}%)`;
}
- _computeBinaryClass(delta?: number) {
+ private computeBinaryClass(delta?: number) {
if (!delta) {
return;
}
return delta > 0 ? 'added' : 'removed';
}
- _computeClass(baseClass?: string, path?: string) {
+ private computeClass(baseClass?: string, path?: string) {
const classes = [];
if (baseClass) {
classes.push(baseClass);
@@ -1085,32 +2013,26 @@
return classes.join(' ');
}
- _computePathClass(
- path: string | undefined,
- expandedFilesRecord: ElementPropertyDeepChange<GrFileList, '_expandedFiles'>
- ) {
- return this._isFileExpanded(path, expandedFilesRecord) ? 'expanded' : '';
+ private computePathClass(path: string | undefined) {
+ return this.isFileExpanded(path) ? 'expanded' : '';
}
- _computeShowHideIcon(
- path: string | undefined,
- expandedFilesRecord: ElementPropertyDeepChange<GrFileList, '_expandedFiles'>
- ) {
- return this._isFileExpanded(path, expandedFilesRecord)
+ private computeShowHideIcon(path: string | undefined) {
+ return this.isFileExpanded(path)
? 'gr-icons:expand-less'
: 'gr-icons:expand-more';
}
- _computeShowNumCleanlyMerged(cleanlyMergedPaths: string[]): boolean {
- return cleanlyMergedPaths.length > 0;
+ private computeShowNumCleanlyMerged(): boolean {
+ return this.cleanlyMergedPaths.length > 0;
}
- _computeCleanlyMergedText(cleanlyMergedPaths: string[]): string {
- const fileCount = pluralize(cleanlyMergedPaths.length, 'file');
+ private computeCleanlyMergedText(): string {
+ const fileCount = pluralize(this.cleanlyMergedPaths.length, 'file');
return `${fileCount} merged cleanly in Parent 1`;
}
- _handleShowParent1(): void {
+ private handleShowParent1(): void {
if (!this.change || !this.patchRange) return;
GerritNav.navigateToChange(this.change, {
patchNum: this.patchRange.patchNum,
@@ -1118,56 +2040,34 @@
});
}
- @observe(
- '_filesByPath',
- 'changeComments',
- 'patchRange',
- 'reviewed',
- '_loading'
- )
- _computeFiles(
- filesByPath?: FileNameToFileInfoMap,
- changeComments?: ChangeComments,
- patchRange?: PatchRange,
- reviewed?: string[],
- loading?: boolean
- ) {
- // Polymer 2: check for undefined
+ private computeFiles() {
if (
- filesByPath === undefined ||
- changeComments === undefined ||
- patchRange === undefined ||
- reviewed === undefined ||
- loading === undefined
+ this.filesByPath === undefined ||
+ this.changeComments === undefined ||
+ this.patchRange === undefined ||
+ this.reviewed === undefined ||
+ this.loading === undefined
) {
return;
}
// Await all promises resolving from reload. @See Issue 9057
- if (loading || !changeComments) {
+ if (this.loading || !this.changeComments) {
return;
}
- const commentedPaths = changeComments.getPaths(patchRange);
- const files: FileNameToReviewedFileInfoMap = {...filesByPath};
+ const commentedPaths = this.changeComments.getPaths(this.patchRange);
+ const files: FileNameToReviewedFileInfoMap = {...this.filesByPath};
addUnmodifiedFiles(files, commentedPaths);
- const reviewedSet = new Set(reviewed || []);
+ const reviewedSet = new Set(this.reviewed || []);
for (const [filePath, reviewedFileInfo] of Object.entries(files)) {
reviewedFileInfo.isReviewed = reviewedSet.has(filePath);
}
- this._files = this._normalizeChangeFilesResponse(files);
+ this.files = this.normalizeChangeFilesResponse(files);
}
- _computeFilesShown(
- numFilesShown: number,
- files: NormalizedFileInfo[]
- ): NormalizedFileInfo[] | undefined {
- // Polymer 2: check for undefined
- if (numFilesShown === undefined || files === undefined) return undefined;
+ private computeFilesShown(): NormalizedFileInfo[] {
+ const previousNumFilesShown = this.shownFiles ? this.shownFiles.length : 0;
- const previousNumFilesShown = this._shownFiles
- ? this._shownFiles.length
- : 0;
-
- const filesShown = files.slice(0, numFilesShown);
+ const filesShown = this.files.slice(0, this.numFilesShown);
this.dispatchEvent(
new CustomEvent('files-shown-changed', {
detail: {length: filesShown.length},
@@ -1177,12 +2077,12 @@
);
// Start the timer for the rendering work hwere because this is where the
- // _shownFiles property is being set, and _shownFiles is used in the
+ // shownFiles property is being set, and shownFiles is used in the
// dom-repeat binding.
this.reporting.time(Timing.FILE_RENDER);
// How many more files are being shown (if it's an increase).
- this._reportinShownFilesIncrement = Math.max(
+ this.reportinShownFilesIncrement = Math.max(
0,
filesShown.length - previousNumFilesShown
);
@@ -1190,59 +2090,56 @@
return filesShown;
}
- _updateDiffCursor() {
+ // Private but used in tests.
+ updateDiffCursor() {
// Overwrite the cursor's list of diffs:
this.diffCursor.replaceDiffs(this.diffs);
}
- _filesChanged() {
- if (this._files && this._files.length > 0) {
- flush();
- this.fileCursor.stops = Array.from(
- this.root!.querySelectorAll(`.${FILE_ROW_CLASS}`)
- );
- this.fileCursor.setCursorAtIndex(this.selectedIndex, true);
- }
+ async filesChanged() {
+ if (!this.files || this.files.length === 0) return;
+ await this.updateComplete;
+ this.fileCursor.stops = Array.from(
+ this.shadowRoot?.querySelectorAll(`.${FILE_ROW_CLASS}`) ?? []
+ );
+ this.fileCursor.setCursorAtIndex(this.selectedIndex, true);
}
- _incrementNumFilesShown() {
+ private incrementNumFilesShown() {
this.numFilesShown += this.fileListIncrement;
+ fire(this, 'num-files-shown-changed', {value: this.numFilesShown});
}
- _computeFileListControlClass(
- numFilesShown?: number,
- files?: NormalizedFileInfo[]
- ) {
- if (numFilesShown === undefined || files === undefined) return 'invisible';
- return numFilesShown >= files.length ? 'invisible' : '';
+ private computeFileListControlClass() {
+ return this.numFilesShown >= this.files.length ? 'invisible' : '';
}
- _computeIncrementText(numFilesShown?: number, files?: NormalizedFileInfo[]) {
- if (numFilesShown === undefined || files === undefined) return '';
- const text = Math.min(this.fileListIncrement, files.length - numFilesShown);
+ private computeIncrementText() {
+ const text = Math.min(
+ this.fileListIncrement,
+ this.files.length - this.numFilesShown
+ );
return `Show ${text} more`;
}
- _computeShowAllText(files: NormalizedFileInfo[]) {
- if (!files) {
+ private computeShowAllText() {
+ return `Show all ${this.files.length} files`;
+ }
+
+ private computeWarnShowAll() {
+ return this.files.length > WARN_SHOW_ALL_THRESHOLD;
+ }
+
+ private computeShowAllWarning() {
+ if (!this.computeWarnShowAll()) {
return '';
}
- return `Show all ${files.length} files`;
+ return `Warning: showing all ${this.files.length} files may take several seconds.`;
}
- _computeWarnShowAll(files: NormalizedFileInfo[]) {
- return files.length > WARN_SHOW_ALL_THRESHOLD;
- }
-
- _computeShowAllWarning(files: NormalizedFileInfo[]) {
- if (!this._computeWarnShowAll(files)) {
- return '';
- }
- return `Warning: showing all ${files.length} files may take several seconds.`;
- }
-
- _showAllFiles() {
- this.numFilesShown = this._files.length;
+ private showAllFiles() {
+ this.numFilesShown = this.files.length;
+ fire(this, 'num-files-shown-changed', {value: this.numFilesShown});
}
/**
@@ -1254,33 +2151,22 @@
*
* @return 'true' if val is true-like, otherwise false
*/
- _booleanToString(val?: unknown) {
+ private booleanToString(val?: unknown) {
return val ? 'true' : 'false';
}
- _isFileExpanded(
- path: string | undefined,
- expandedFilesRecord: ElementPropertyDeepChange<GrFileList, '_expandedFiles'>
- ) {
- return expandedFilesRecord.base.some(f => f.path === path);
+ private isFileExpanded(path: string | undefined) {
+ return this.expandedFiles.some(f => f.path === path);
}
- _isFileExpandedStr(
- path: string | undefined,
- expandedFilesRecord: ElementPropertyDeepChange<GrFileList, '_expandedFiles'>
- ) {
- return this._booleanToString(
- this._isFileExpanded(path, expandedFilesRecord)
- );
+ private isFileExpandedStr(path: string | undefined) {
+ return this.booleanToString(this.isFileExpanded(path));
}
- private _computeExpandedFiles(
- expandedCount: number,
- totalCount: number
- ): FilesExpandedState {
- if (expandedCount === 0) {
+ private computeExpandedFiles(): FilesExpandedState {
+ if (this.expandedFiles.length === 0) {
return FilesExpandedState.NONE;
- } else if (expandedCount === totalCount) {
+ } else if (this.expandedFiles.length === this.files.length) {
return FilesExpandedState.ALL;
}
return FilesExpandedState.SOME;
@@ -1292,44 +2178,35 @@
* order by waiting for the previous diff to finish before starting the next
* one.
*
- * @param record The splice record in the expanded paths list.
+ * @param newFiles The new files that have been added.
+ * Private but used in tests.
*/
- @observe('_expandedFiles.splices')
- _expandedFilesChanged(record?: PolymerSpliceChange<PatchSetFile[]>) {
+ async expandedFilesChanged(oldFiles: Array<PatchSetFile>) {
// Clear content for any diffs that are not open so if they get re-opened
// the stale content does not flash before it is cleared and reloaded.
const collapsedDiffs = this.diffs.filter(
- diff => this._expandedFiles.findIndex(f => f.path === diff.path) === -1
+ diff => this.expandedFiles.findIndex(f => f.path === diff.path) === -1
);
- this._clearCollapsedDiffs(collapsedDiffs);
+ this.clearCollapsedDiffs(collapsedDiffs);
- if (!record) {
- return;
- } // Happens after "Collapse all" clicked.
+ this.filesExpanded = this.computeExpandedFiles();
- this.filesExpanded = this._computeExpandedFiles(
- this._expandedFiles.length,
- this._files.length
- );
-
- // Find the paths introduced by the new index splices:
- const newFiles = record.indexSplices.flatMap(splice =>
- splice.object.slice(splice.index, splice.index + splice.addedCount)
+ const newFiles = this.expandedFiles.filter(
+ file => (oldFiles ?? []).findIndex(f => f.path === file.path) === -1
);
// Required so that the newly created diff view is included in this.diffs.
- flush();
+ await this.updateComplete;
if (newFiles.length) {
- this._renderInOrder(newFiles, this.diffs, newFiles.length);
+ await this.renderInOrder(newFiles, this.diffs);
}
-
- this._updateDiffCursor();
+ this.updateDiffCursor();
this.diffCursor.reInitAndUpdateStops();
}
// private but used in test
- _clearCollapsedDiffs(collapsedDiffs: GrDiffHost[]) {
+ clearCollapsedDiffs(collapsedDiffs: GrDiffHost[]) {
for (const diff of collapsedDiffs) {
diff.cancel();
diff.clearDiffContent();
@@ -1345,31 +2222,31 @@
*
* @param initialCount The total number of paths in the pass.
*/
- async _renderInOrder(
- files: PatchSetFile[],
- diffElements: GrDiffHost[],
- initialCount: number
- ) {
+ async renderInOrder(files: PatchSetFile[], diffElements: GrDiffHost[]) {
this.reporting.time(Timing.FILE_EXPAND_ALL);
for (const file of files) {
const path = file.path;
- const diffElem = this._findDiffByPath(path, diffElements);
- if (diffElem) {
- diffElem.prefetchDiff();
- }
- }
-
- await asyncForeach(files, (file, cancel) => {
- const path = file.path;
- this._cancelForEachDiff = cancel;
-
- const diffElem = this._findDiffByPath(path, diffElements);
+ const diffElem = this.findDiffByPath(path, diffElements);
if (!diffElem) {
this.reporting.error(
new Error(`Did not find <gr-diff-host> element for ${path}`)
);
- return Promise.resolve();
+ return;
+ }
+ diffElem.prefetchDiff();
+ }
+
+ await asyncForeach(files, async (file, cancel) => {
+ const path = file.path;
+ this.cancelForEachDiff = cancel;
+
+ const diffElem = this.findDiffByPath(path, diffElements);
+ if (!diffElem) {
+ this.reporting.error(
+ new Error(`Did not find <gr-diff-host> element for ${path}`)
+ );
+ return;
}
if (!this.diffPrefs) {
throw new Error('diffPrefs must be set');
@@ -1381,18 +2258,18 @@
// control over which diffs were actually seen. And for lots of diffs
// that would even be a problem for write QPS quota.
if (
- this._loggedIn &&
+ this.loggedIn &&
!this.diffPrefs.manual_review &&
- initialCount === 1
+ files.length === 1
) {
- this._reviewFile(path, true);
+ this.reviewFile(path, true);
}
- return diffElem.reload();
+ await diffElem.reload();
});
- this._cancelForEachDiff = undefined;
+ this.cancelForEachDiff = undefined;
this.reporting.timeEnd(Timing.FILE_EXPAND_ALL, {
- count: initialCount,
+ count: files.length,
height: this.clientHeight,
});
/* Block diff cursor from auto scrolling after files are done rendering.
@@ -1411,17 +2288,17 @@
}
/** Cancel the rendering work of every diff in the list */
- _cancelDiffs() {
- if (this._cancelForEachDiff) {
- this._cancelForEachDiff();
+ private cancelDiffs() {
+ if (this.cancelForEachDiff) {
+ this.cancelForEachDiff();
}
- this._forEachDiff(d => d.cancel());
+ this.forEachDiff(d => d.cancel());
}
/**
* In the given NodeList of diff elements, find the diff for the given path.
*/
- private _findDiffByPath(path: string, diffElements: GrDiffHost[]) {
+ private findDiffByPath(path: string, diffElements: GrDiffHost[]) {
for (let i = 0; i < diffElements.length; i++) {
if (diffElements[i].path === path) {
return diffElements[i];
@@ -1430,8 +2307,9 @@
return undefined;
}
- _handleEscKey() {
- this._displayLine = false;
+ // Private but used in tests.
+ handleEscKey() {
+ this.displayLine = false;
}
/**
@@ -1439,27 +2317,24 @@
* debouncer so that the file list doesn't flash gray when the API requests
* are reasonably fast.
*/
- _loadingChanged(loading?: boolean) {
+ private loadingChanged() {
+ const loading = this.loading;
this.loadingTask = debounce(
this.loadingTask,
() => {
// Only show set the loading if there have been files loaded to show. In
// this way, the gray loading style is not shown on initial loads.
- this.classList.toggle('loading', loading && !!this._files.length);
+ this.classList.toggle('loading', loading && !!this.files.length);
},
LOADING_DEBOUNCE_INTERVAL
);
}
- _editModeChanged(editMode?: boolean) {
- this.classList.toggle('editMode', editMode);
- }
-
- _computeReviewedClass(isReviewed?: boolean) {
+ private computeReviewedClass(isReviewed?: boolean) {
return isReviewed ? 'isReviewed' : '';
}
- _computeReviewedText(isReviewed?: boolean) {
+ private computeReviewedText(isReviewed?: boolean) {
return isReviewed ? 'MARK UNREVIEWED' : 'MARK REVIEWED';
}
@@ -1467,7 +2342,7 @@
* Given a file path, return whether that path should have visible size bars
* and be included in the size bars calculation.
*/
- _showBarsForPath(path?: string) {
+ private showBarsForPath(path?: string) {
return (
path !== SpecialFilePath.COMMIT_MESSAGE &&
path !== SpecialFilePath.MERGE_LIST
@@ -1476,16 +2351,12 @@
/**
* Compute size bar layout values from the file list.
+ * Private but used in tests.
*/
- _computeSizeBarLayout(
- shownFilesRecord?: ElementPropertyDeepChange<GrFileList, '_shownFiles'>
- ) {
+ computeSizeBarLayout() {
const stats: SizeBarLayout = createDefaultSizeBarLayout();
- if (!shownFilesRecord || !shownFilesRecord.base) {
- return stats;
- }
- shownFilesRecord.base
- .filter(f => this._showBarsForPath(f.__path))
+ this.shownFiles
+ .filter(f => this.showBarsForPath(f.__path))
.forEach(f => {
if (f.lines_inserted) {
stats.maxInserted = Math.max(stats.maxInserted, f.lines_inserted);
@@ -1507,14 +2378,15 @@
/**
* Get the width of the addition bar for a file.
+ * Private but used in tests.
*/
- _computeBarAdditionWidth(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
+ computeBarAdditionWidth(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
if (
!file ||
!stats ||
stats.maxInserted === 0 ||
!file.lines_inserted ||
- !this._showBarsForPath(file.__path)
+ !this.showBarsForPath(file.__path)
) {
return 0;
}
@@ -1525,22 +2397,24 @@
/**
* Get the x-offset of the addition bar for a file.
+ * Private but used in tests.
*/
- _computeBarAdditionX(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
+ computeBarAdditionX(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
if (!file || !stats) return;
- return stats.maxAdditionWidth - this._computeBarAdditionWidth(file, stats);
+ return stats.maxAdditionWidth - this.computeBarAdditionWidth(file, stats);
}
/**
* Get the width of the deletion bar for a file.
+ * Private but used in tests.
*/
- _computeBarDeletionWidth(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
+ computeBarDeletionWidth(file?: NormalizedFileInfo, stats?: SizeBarLayout) {
if (
!file ||
!stats ||
stats.maxDeleted === 0 ||
!file.lines_deleted ||
- !this._showBarsForPath(file.__path)
+ !this.showBarsForPath(file.__path)
) {
return 0;
}
@@ -1552,15 +2426,16 @@
/**
* Get the x-offset of the deletion bar for a file.
*/
- _computeBarDeletionX(stats: SizeBarLayout) {
+ private computeBarDeletionX(stats: SizeBarLayout) {
return stats.deletionOffset;
}
- _computeSizeBarsClass(showSizeBars?: boolean, path?: string) {
+ // Private but used in tests.
+ computeSizeBarsClass(path?: string) {
let hideClass = '';
- if (!showSizeBars) {
+ if (!this.showSizeBars) {
hideClass = 'hide';
- } else if (!this._showBarsForPath(path)) {
+ } else if (!this.showBarsForPath(path)) {
hideClass = 'invisible';
}
return `sizeBars ${hideClass}`;
@@ -1572,18 +2447,15 @@
* Ideally, there should be a better way to enforce the expectation of the
* dependencies between dynamic endpoints.
*/
- _computeShowDynamicColumns(
- headerEndpoints?: string,
- contentEndpoints?: string,
- summaryEndpoints?: string
- ) {
- return (
- headerEndpoints &&
- contentEndpoints &&
- summaryEndpoints &&
- headerEndpoints.length &&
- headerEndpoints.length === contentEndpoints.length &&
- headerEndpoints.length === summaryEndpoints.length
+ private computeShowDynamicColumns() {
+ return !!(
+ this.dynamicHeaderEndpoints &&
+ this.dynamicContentEndpoints &&
+ this.dynamicSummaryEndpoints &&
+ this.dynamicHeaderEndpoints.length &&
+ this.dynamicHeaderEndpoints.length ===
+ this.dynamicContentEndpoints.length &&
+ this.dynamicHeaderEndpoints.length === this.dynamicSummaryEndpoints.length
);
}
@@ -1591,22 +2463,21 @@
* Shows registered dynamic prepended columns iff the 'header', 'content'
* endpoints are registered the exact same number of times.
*/
- _computeShowPrependedDynamicColumns(
- headerEndpoints?: string,
- contentEndpoints?: string
- ) {
- return (
- headerEndpoints &&
- contentEndpoints &&
- headerEndpoints.length &&
- headerEndpoints.length === contentEndpoints.length
+ private computeShowPrependedDynamicColumns() {
+ return !!(
+ this.dynamicPrependedHeaderEndpoints &&
+ this.dynamicPrependedContentEndpoints &&
+ this.dynamicPrependedHeaderEndpoints.length &&
+ this.dynamicPrependedHeaderEndpoints.length ===
+ this.dynamicPrependedContentEndpoints.length
);
}
/**
* Returns true if none of the inline diffs have been expanded.
+ * Private but used in tests.
*/
- _noDiffsExpanded() {
+ noDiffsExpanded() {
return this.filesExpanded === FilesExpandedState.NONE;
}
@@ -1616,19 +2487,20 @@
* rendering.
*
* @param index The index of the row being rendered.
+ * Private but used in tests.
*/
- _reportRenderedRow(index: number) {
- if (index === this._shownFiles.length - 1) {
+ reportRenderedRow(index: number) {
+ if (index === this.shownFiles.length - 1) {
setTimeout(() => {
this.reporting.timeEnd(Timing.FILE_RENDER, {
- count: this._reportinShownFilesIncrement,
+ count: this.reportinShownFilesIncrement,
});
}, 1);
}
- return '';
}
- _reviewedTitle(reviewed?: boolean) {
+ // Private but used in tests.
+ reviewedTitle(reviewed?: boolean) {
if (reviewed) {
return 'Mark as not reviewed (shortcut: r)';
}
@@ -1636,25 +2508,11 @@
return 'Mark as reviewed (shortcut: r)';
}
- _handleReloadingDiffPreference() {
+ private handleReloadingDiffPreference() {
this.userModel.getDiffPreferences();
}
- /**
- * Wrapper for using in the element template and computed properties
- */
- _computeDisplayPath(path: string) {
- return computeDisplayPath(path);
- }
-
- /**
- * Wrapper for using in the element template and computed properties
- */
- _computeTruncatedPath(path: string) {
- return computeTruncatedPath(path);
- }
-
- _getOldPath(file: NormalizedFileInfo) {
+ private getOldPath(file: NormalizedFileInfo) {
// The gr-endpoint-decorator is waiting until all gr-endpoint-param
// values are updated.
// The old_path property is undefined for added files, and the
@@ -1664,9 +2522,3 @@
return file.old_path ?? null;
}
}
-
-declare global {
- interface HTMLElementTagNameMap {
- 'gr-file-list': GrFileList;
- }
-}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
deleted file mode 100644
index 67ca9be..0000000
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
+++ /dev/null
@@ -1,823 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-a11y-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="shared-styles">
- :host {
- display: block;
- }
- .row {
- align-items: center;
- border-top: 1px solid var(--border-color);
- display: flex;
- min-height: calc(var(--line-height-normal) + 2 * var(--spacing-s));
- padding: var(--spacing-xs) var(--spacing-l);
- }
- /* The class defines a content visible only to screen readers */
- .noCommentsScreenReaderText {
- opacity: 0;
- max-width: 1px;
- overflow: hidden;
- display: none;
- vertical-align: top;
- }
- div[role='gridcell']
- > div.comments
- > span:empty
- + span:empty
- + span.noCommentsScreenReaderText {
- /* inline-block instead of block, such that it can control width */
- display: inline-block;
- }
- :host(.loading) .row {
- opacity: 0.5;
- }
- :host(.editMode) .hideOnEdit {
- display: none;
- }
- .showOnEdit {
- display: none;
- }
- :host(.editMode) .showOnEdit {
- display: initial;
- }
- .invisible {
- visibility: hidden;
- }
- .header-row {
- background-color: var(--background-color-secondary);
- }
- .controlRow {
- align-items: center;
- display: flex;
- height: 2.25em;
- justify-content: center;
- }
- .controlRow.invisible,
- .show-hide.invisible {
- display: none;
- }
- .reviewed,
- .status {
- align-items: center;
- display: inline-flex;
- }
- .reviewed {
- display: inline-block;
- text-align: left;
- width: 1.5em;
- }
- .file-row {
- cursor: pointer;
- }
- .file-row.expanded {
- border-bottom: 1px solid var(--border-color);
- position: -webkit-sticky;
- position: sticky;
- top: 0;
- /* Has to visible above the diff view, and by default has a lower
- z-index. setting to 1 places it directly above. */
- z-index: 1;
- }
- .file-row:hover {
- background-color: var(--hover-background-color);
- }
- .file-row.selected {
- background-color: var(--selection-background-color);
- }
- .file-row.expanded,
- .file-row.expanded:hover {
- background-color: var(--expanded-background-color);
- }
- .path {
- cursor: pointer;
- flex: 1;
- /* Wrap it into multiple lines if too long. */
- white-space: normal;
- word-break: break-word;
- }
- .oldPath {
- color: var(--deemphasized-text-color);
- }
- .header-stats {
- text-align: center;
- min-width: 7.5em;
- }
- .stats {
- text-align: right;
- min-width: 7.5em;
- }
- .comments {
- padding-left: var(--spacing-l);
- min-width: 7.5em;
- white-space: nowrap;
- }
- .row:not(.header-row) .stats,
- .total-stats {
- font-family: var(--monospace-font-family);
- font-size: var(--font-size-mono);
- line-height: var(--line-height-mono);
- display: flex;
- }
- .sizeBars {
- margin-left: var(--spacing-m);
- min-width: 7em;
- text-align: center;
- }
- .sizeBars.hide {
- display: none;
- }
- .added,
- .removed {
- display: inline-block;
- min-width: 3.5em;
- }
- .added {
- color: var(--positive-green-text-color);
- }
- .removed {
- color: var(--negative-red-text-color);
- text-align: left;
- min-width: 4em;
- padding-left: var(--spacing-s);
- }
- .drafts {
- color: var(--error-foreground);
- font-weight: var(--font-weight-bold);
- }
- .show-hide-icon:focus {
- outline: none;
- }
- .show-hide {
- margin-left: var(--spacing-s);
- width: 1.9em;
- }
- .fileListButton {
- margin: var(--spacing-m);
- }
- .totalChanges {
- justify-content: flex-end;
- text-align: right;
- }
- .warning {
- color: var(--deemphasized-text-color);
- }
- input.show-hide {
- display: none;
- }
- label.show-hide {
- cursor: pointer;
- display: block;
- min-width: 2em;
- }
- gr-diff {
- display: block;
- overflow-x: auto;
- }
- .truncatedFileName {
- display: none;
- }
- .mobile {
- display: none;
- }
- .reviewed {
- margin-left: var(--spacing-xxl);
- width: 15em;
- }
- .reviewedSwitch {
- color: var(--link-color);
- opacity: 0;
- justify-content: flex-end;
- width: 100%;
- }
- .reviewedSwitch:hover {
- cursor: pointer;
- opacity: 100;
- }
- .showParentButton {
- line-height: var(--line-height-normal);
- margin-bottom: calc(var(--spacing-s) * -1);
- margin-left: var(--spacing-m);
- margin-top: calc(var(--spacing-s) * -1);
- }
- .row:focus {
- outline: none;
- }
- .row:hover .reviewedSwitch,
- .row:focus-within .reviewedSwitch,
- .row.expanded .reviewedSwitch {
- opacity: 100;
- }
- .reviewedLabel {
- color: var(--deemphasized-text-color);
- margin-right: var(--spacing-l);
- opacity: 0;
- }
- .reviewedLabel.isReviewed {
- display: initial;
- opacity: 100;
- }
- .editFileControls {
- width: 7em;
- }
- .markReviewed:focus {
- outline: none;
- }
- .markReviewed,
- .pathLink {
- display: inline-block;
- margin: -2px 0;
- padding: var(--spacing-s) 0;
- text-decoration: none;
- }
- .pathLink:hover span.fullFileName,
- .pathLink:hover span.truncatedFileName {
- text-decoration: underline;
- }
-
- /** copy on file path **/
- .pathLink gr-copy-clipboard,
- .oldPath gr-copy-clipboard {
- display: inline-block;
- visibility: hidden;
- vertical-align: bottom;
- --gr-button-padding: 0px;
- }
- .row:focus-within gr-copy-clipboard,
- .row:hover gr-copy-clipboard {
- visibility: visible;
- }
-
- @media screen and (max-width: 1200px) {
- gr-endpoint-decorator.extra-col {
- display: none;
- }
- }
-
- @media screen and (max-width: 1000px) {
- .reviewed {
- display: none;
- }
- }
-
- @media screen and (max-width: 800px) {
- .desktop {
- display: none;
- }
- .mobile {
- display: block;
- }
- .row.selected {
- background-color: var(--view-background-color);
- }
- .stats {
- display: none;
- }
- .reviewed,
- .status {
- justify-content: flex-start;
- }
- .comments {
- min-width: initial;
- }
- .expanded .fullFileName,
- .truncatedFileName {
- display: inline;
- }
- .expanded .truncatedFileName,
- .fullFileName {
- display: none;
- }
- }
- :host(.hideComments) {
- --gr-comment-thread-display: none;
- }
- </style>
- <h3 class="assistive-tech-only">File list</h3>
- <div
- id="container"
- on-click="_handleFileListClick"
- role="grid"
- aria-label="Files list"
- >
- <div class="header-row row" role="row">
- <!-- endpoint: change-view-file-list-header-prepend -->
- <template is="dom-if" if="[[_showPrependedDynamicColumns]]">
- <template
- is="dom-repeat"
- items="[[_dynamicPrependedHeaderEndpoints]]"
- as="headerEndpoint"
- >
- <gr-endpoint-decorator
- class="prepended-col"
- name$="[[headerEndpoint]]"
- role="columnheader"
- >
- <gr-endpoint-param name="change" value="[[change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="patchRange" value="[[patchRange]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="files" value="[[_files]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </template>
- </template>
- <div class="path" role="columnheader">File</div>
- <div class="comments desktop" role="columnheader">Comments</div>
- <div class="comments mobile" role="columnheader" title="Comments">C</div>
- <div class="sizeBars desktop" role="columnheader">Size</div>
- <div class="header-stats" role="columnheader">Delta</div>
- <!-- endpoint: change-view-file-list-header -->
- <template is="dom-if" if="[[_showDynamicColumns]]">
- <template
- is="dom-repeat"
- items="[[_dynamicHeaderEndpoints]]"
- as="headerEndpoint"
- >
- <gr-endpoint-decorator
- class="extra-col"
- name$="[[headerEndpoint]]"
- role="columnheader"
- >
- </gr-endpoint-decorator>
- </template>
- </template>
- <!-- Empty div here exists to keep spacing in sync with file rows. -->
- <div
- class="reviewed hideOnEdit"
- hidden$="[[!_loggedIn]]"
- aria-hidden="true"
- ></div>
- <div class="editFileControls showOnEdit" aria-hidden="true"></div>
- <div class="show-hide" aria-hidden="true"></div>
- </div>
-
- <template
- is="dom-repeat"
- items="[[_shownFiles]]"
- id="files"
- as="file"
- initial-count="[[fileListIncrement]]"
- target-framerate="1"
- >
- [[_reportRenderedRow(index)]]
- <div class="stickyArea">
- <div
- class$="file-row row [[_computePathClass(file.__path, _expandedFiles.*)]]"
- data-file$="[[_computePatchSetFile(file)]]"
- tabindex="-1"
- role="row"
- >
- <!-- endpoint: change-view-file-list-content-prepend -->
- <template is="dom-if" if="[[_showPrependedDynamicColumns]]">
- <template
- is="dom-repeat"
- items="[[_dynamicPrependedContentEndpoints]]"
- as="contentEndpoint"
- >
- <gr-endpoint-decorator
- class="prepended-col"
- name="[[contentEndpoint]]"
- role="gridcell"
- >
- <gr-endpoint-param name="change" value="[[change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="changeNum" value="[[changeNum]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="patchRange" value="[[patchRange]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="path" value="[[file.__path]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="oldPath" value="[[_getOldPath(file)]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </template>
- </template>
- <!-- TODO: Remove data-url as it appears its not used -->
- <span
- data-url="[[_computeDiffURL(change, patchRange.basePatchNum, patchRange.patchNum, file.__path, editMode)]]"
- class="path"
- role="gridcell"
- >
- <a
- class="pathLink"
- href$="[[_computeDiffURL(change, patchRange.basePatchNum, patchRange.patchNum, file.__path, editMode)]]"
- >
- <span
- title$="[[_computeDisplayPath(file.__path)]]"
- class="fullFileName"
- >
- [[_computeDisplayPath(file.__path)]]
- </span>
- <span
- title$="[[_computeDisplayPath(file.__path)]]"
- class="truncatedFileName"
- >
- [[_computeTruncatedPath(file.__path)]]
- </span>
- <gr-file-status-chip file="[[file]]"></gr-file-status-chip>
- <gr-copy-clipboard
- hideInput=""
- text="[[file.__path]]"
- ></gr-copy-clipboard>
- </a>
- <template is="dom-if" if="[[file.old_path]]">
- <div class="oldPath" title$="[[file.old_path]]">
- [[file.old_path]]
- <gr-copy-clipboard
- hideInput=""
- text="[[file.old_path]]"
- ></gr-copy-clipboard>
- </div>
- </template>
- </span>
- <div role="gridcell">
- <div class="comments desktop">
- <span class="drafts"
- ><!-- This comments ensure that span is empty when the function
- returns empty string.
- -->[[_computeDraftsString(changeComments, patchRange, file)]]<!-- This comments ensure that span is empty when
- the function returns empty string.
- --></span
- >
- <span
- ><!--
- -->[[_computeCommentsString(changeComments, patchRange, file)]]<!--
- --></span
- >
- <span class="noCommentsScreenReaderText">
- <!-- Screen readers read the following content only if 2 other
- spans in the parent div is empty. The content is not visible on
- the page.
- Without this span, screen readers don't navigate correctly inside
- table, because empty div doesn't rendered. For example, VoiceOver
- jumps back to the whole table.
- We can use   instead, but it sounds worse.
- -->
- No comments
- </span>
- </div>
- <div class="comments mobile">
- <span class="drafts"
- ><!-- This comments ensure that span is empty when the function
- returns empty string.
- -->[[_computeDraftsStringMobile(changeComments, patchRange,
- file)]]<!-- This comments ensure that span is empty when
- the function returns empty string.
- --></span
- >
- <span
- ><!--
- -->[[_computeCommentsStringMobile(changeComments, patchRange,
- file)]]<!--
- --></span
- >
- <span class="noCommentsScreenReaderText">
- <!-- The same as for desktop comments -->
- No comments
- </span>
- </div>
- </div>
- <div class="desktop" role="gridcell">
- <!-- The content must be in a separate div. It guarantees, that
- gridcell always visible for screen readers.
- For example, without a nested div screen readers pronounce the
- "Commit message" row content with incorrect column headers.
- -->
- <div
- class$="[[_computeSizeBarsClass(_showSizeBars, file.__path)]]"
- aria-label="A bar that represents the addition and deletion ratio for the current file"
- >
- <svg width="61" height="8">
- <rect
- x$="[[_computeBarAdditionX(file, _sizeBarLayout)]]"
- y="0"
- height="8"
- fill="var(--positive-green-text-color)"
- width$="[[_computeBarAdditionWidth(file, _sizeBarLayout)]]"
- ></rect>
- <rect
- x$="[[_computeBarDeletionX(_sizeBarLayout)]]"
- y="0"
- height="8"
- fill="var(--negative-red-text-color)"
- width$="[[_computeBarDeletionWidth(file, _sizeBarLayout)]]"
- ></rect>
- </svg>
- </div>
- </div>
- <div class="stats" role="gridcell">
- <!-- The content must be in a separate div. It guarantees, that
- gridcell always visible for screen readers.
- For example, without a nested div screen readers pronounce the
- "Commit message" row content with incorrect column headers.
- -->
- <div class$="[[_computeClass('', file.__path)]]">
- <span
- class="added"
- tabindex="0"
- aria-label$="[[file.lines_inserted]] lines added"
- hidden$="[[file.binary]]"
- >
- +[[file.lines_inserted]]
- </span>
- <span
- class="removed"
- tabindex="0"
- aria-label$="[[file.lines_deleted]] lines removed"
- hidden$="[[file.binary]]"
- >
- -[[file.lines_deleted]]
- </span>
- <span
- class$="[[_computeBinaryClass(file.size_delta)]]"
- hidden$="[[!file.binary]]"
- >
- [[_formatBytes(file.size_delta)]] [[_formatPercentage(file.size,
- file.size_delta)]]
- </span>
- </div>
- </div>
- <!-- endpoint: change-view-file-list-content -->
- <template is="dom-if" if="[[_showDynamicColumns]]">
- <template
- is="dom-repeat"
- items="[[_dynamicContentEndpoints]]"
- as="contentEndpoint"
- >
- <div class$="[[_computeClass('', file.__path)]]" role="gridcell">
- <gr-endpoint-decorator
- class="extra-col"
- name="[[contentEndpoint]]"
- >
- <gr-endpoint-param name="change" value="[[change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="changeNum" value="[[changeNum]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="patchRange" value="[[patchRange]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="path" value="[[file.__path]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </div>
- </template>
- </template>
- <div
- class="reviewed hideOnEdit"
- role="gridcell"
- hidden$="[[!_loggedIn]]"
- >
- <span
- class$="reviewedLabel [[_computeReviewedClass(file.isReviewed)]]"
- aria-hidden$="[[!file.isReviewed]]"
- >Reviewed</span
- >
- <!-- Do not use input type="checkbox" with hidden input and
- visible label here. Screen readers don't read/interract
- correctly with such input.
- -->
- <span
- class="reviewedSwitch"
- role="switch"
- tabindex="0"
- on-click="_reviewedClick"
- on-keydown="_reviewedClick"
- aria-label="Reviewed"
- aria-checked$="[[_booleanToString(file.isReviewed)]]"
- >
- <!-- Trick with tabindex to avoid outline on mouse focus, but
- preserve focus outline for keyboard navigation -->
- <span
- tabindex="-1"
- class="markReviewed"
- title$="[[_reviewedTitle(file.isReviewed)]]"
- >[[_computeReviewedText(file.isReviewed)]]</span
- >
- </span>
- </div>
- <div
- class="editFileControls showOnEdit"
- role="gridcell"
- aria-hidden$="[[!editMode]]"
- >
- <template is="dom-if" if="[[editMode]]">
- <gr-edit-file-controls
- class$="[[_computeClass('', file.__path)]]"
- file-path="[[file.__path]]"
- ></gr-edit-file-controls>
- </template>
- </div>
- <div class="show-hide" role="gridcell">
- <!-- Do not use input type="checkbox" with hidden input and
- visible label here. Screen readers don't read/interract
- correctly with such input.
- -->
- <span
- class="show-hide"
- data-path$="[[file.__path]]"
- data-expand="true"
- role="switch"
- tabindex="0"
- aria-checked$="[[_isFileExpandedStr(file.__path, _expandedFiles.*)]]"
- aria-label="Expand file"
- on-click="_expandedClick"
- on-keydown="_expandedClick"
- >
- <!-- Trick with tabindex to avoid outline on mouse focus, but
- preserve focus outline for keyboard navigation -->
- <iron-icon
- class="show-hide-icon"
- tabindex="-1"
- id="icon"
- icon="[[_computeShowHideIcon(file.__path, _expandedFiles.*)]]"
- >
- </iron-icon>
- </span>
- </div>
- </div>
- <template
- is="dom-if"
- if="[[_isFileExpanded(file.__path, _expandedFiles.*)]]"
- >
- <gr-diff-host
- no-auto-render=""
- show-load-failure=""
- display-line="[[_displayLine]]"
- hidden="[[!_isFileExpanded(file.__path, _expandedFiles.*)]]"
- change-num="[[changeNum]]"
- change="[[change]]"
- patch-range="[[patchRange]]"
- file="[[_computePatchSetFile(file)]]"
- path="[[file.__path]]"
- prefs="[[diffPrefs]]"
- project-name="[[change.project]]"
- no-render-on-prefs-change=""
- ></gr-diff-host>
- </template>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_computeShowNumCleanlyMerged(_cleanlyMergedPaths)]]"
- >
- <div class="row">
- <!-- endpoint: change-view-file-list-content-prepend -->
- <template is="dom-if" if="[[_showPrependedDynamicColumns]]">
- <template
- is="dom-repeat"
- items="[[_dynamicPrependedContentEndpoints]]"
- as="contentEndpoint"
- >
- <gr-endpoint-decorator
- class="prepended-col"
- name="[[contentEndpoint]]"
- role="gridcell"
- >
- <gr-endpoint-param name="change" value="[[change]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="changeNum" value="[[changeNum]]">
- </gr-endpoint-param>
- <gr-endpoint-param name="patchRange" value="[[patchRange]]">
- </gr-endpoint-param>
- <gr-endpoint-param
- name="cleanlyMergedPaths"
- value="[[_cleanlyMergedPaths]]"
- >
- </gr-endpoint-param>
- <gr-endpoint-param
- name="cleanlyMergedOldPaths"
- value="[[_cleanlyMergedOldPaths]]"
- >
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </template>
- </template>
- <div role="gridcell">
- <div>
- <span class="cleanlyMergedText">
- [[_computeCleanlyMergedText(_cleanlyMergedPaths)]]
- </span>
- <gr-button
- link
- class="showParentButton"
- on-click="_handleShowParent1"
- >
- Show Parent 1
- </gr-button>
- </div>
- </div>
- </div>
- </template>
- </div>
- <div class="row totalChanges" hidden$="[[_hideChangeTotals]]">
- <div class="total-stats">
- <div>
- <span
- class="added"
- tabindex="0"
- aria-label$="Total [[_patchChange.inserted]] lines added"
- >
- +[[_patchChange.inserted]]
- </span>
- <span
- class="removed"
- tabindex="0"
- aria-label$="Total [[_patchChange.deleted]] lines removed"
- >
- -[[_patchChange.deleted]]
- </span>
- </div>
- </div>
- <!-- endpoint: change-view-file-list-summary -->
- <template is="dom-if" if="[[_showDynamicColumns]]">
- <template
- is="dom-repeat"
- items="[[_dynamicSummaryEndpoints]]"
- as="summaryEndpoint"
- >
- <gr-endpoint-decorator class="extra-col" name="[[summaryEndpoint]]">
- <gr-endpoint-param
- name="change"
- value="[[change]]"
- ></gr-endpoint-param>
- <gr-endpoint-param name="patchRange" value="[[patchRange]]">
- </gr-endpoint-param>
- </gr-endpoint-decorator>
- </template>
- </template>
- <!-- Empty div here exists to keep spacing in sync with file rows. -->
- <div class="reviewed hideOnEdit" hidden$="[[!_loggedIn]]"></div>
- <div class="editFileControls showOnEdit"></div>
- <div class="show-hide"></div>
- </div>
- <div class="row totalChanges" hidden$="[[_hideBinaryChangeTotals]]">
- <div class="total-stats">
- <span
- class="added"
- aria-label$="Total bytes inserted: [[_formatBytes(_patchChange.size_delta_inserted)]] "
- >
- [[_formatBytes(_patchChange.size_delta_inserted)]]
- [[_formatPercentage(_patchChange.total_size,
- _patchChange.size_delta_inserted)]]
- </span>
- <span
- class="removed"
- aria-label$="Total bytes removed: [[_formatBytes(_patchChange.size_delta_deleted)]]"
- >
- [[_formatBytes(_patchChange.size_delta_deleted)]]
- [[_formatPercentage(_patchChange.total_size,
- _patchChange.size_delta_deleted)]]
- </span>
- </div>
- </div>
- <div
- class$="row controlRow [[_computeFileListControlClass(numFilesShown, _files)]]"
- >
- <gr-button
- class="fileListButton"
- id="incrementButton"
- link=""
- on-click="_incrementNumFilesShown"
- >
- [[_computeIncrementText(numFilesShown, _files)]]
- </gr-button>
- <gr-tooltip-content
- has-tooltip="[[_computeWarnShowAll(_files)]]"
- show-icon="[[_computeWarnShowAll(_files)]]"
- title$="[[_computeShowAllWarning(_files)]]"
- >
- <gr-button
- class="fileListButton"
- id="showAllButton"
- link=""
- on-click="_showAllFiles"
- >
- [[_computeShowAllText(_files)]] </gr-button
- ><!--
- --></gr-tooltip-content>
- </div>
- <gr-diff-preferences-dialog
- id="diffPreferencesDialog"
- diff-prefs="{{diffPrefs}}"
- on-reload-diff-preference="_handleReloadingDiffPreference"
- >
- </gr-diff-preferences-dialog>
-`;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
index 66df866..d39d648 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.ts
@@ -24,7 +24,6 @@
listenOnce,
mockPromise,
query,
- spyRestApi,
stubRestApi,
} from '../../../test/test-utils';
import {
@@ -32,6 +31,7 @@
CommitId,
EditPatchSetNum,
NumericChangeId,
+ ParentPatchSetNum,
PatchRange,
PatchSetNum,
RepoName,
@@ -47,16 +47,17 @@
createParsedChange,
createRevision,
} from '../../../test/test-data-generators';
-import {createDefaultDiffPrefs} from '../../../constants/constants';
+import {
+ createDefaultDiffPrefs,
+ DiffViewMode,
+} from '../../../constants/constants';
import {queryAll, queryAndAssert} from '../../../utils/common-util';
import {GrFileList, NormalizedFileInfo} from './gr-file-list';
-import {DiffPreferencesInfo} from '../../../types/diff';
import {GrButton} from '../../shared/gr-button/gr-button';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {ParsedChangeInfo} from '../../../types/types';
import {GrDiffHost} from '../../diff/gr-diff-host/gr-diff-host';
import {IronIconElement} from '@polymer/iron-icon';
-import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {GrEditFileControls} from '../../edit/gr-edit-file-controls/gr-edit-file-controls';
const basicFixture = fixtureFromElement('gr-file-list');
@@ -70,9 +71,9 @@
function createFilesByPath(count: number) {
return Array(count)
.fill(0)
- .reduce((_filesByPath, _, idx) => {
- _filesByPath[`'/file${idx}`] = {lines_inserted: 9};
- return _filesByPath;
+ .reduce((filesByPath, _, idx) => {
+ filesByPath[`'/file${idx}`] = {lines_inserted: 9};
+ return filesByPath;
}, {});
}
@@ -81,7 +82,7 @@
let saveStub: sinon.SinonStub;
- suite('basic tests', () => {
+ suite('basic tests', async () => {
setup(async () => {
stubRestApi('getDiffComments').returns(Promise.resolve({}));
stubRestApi('getDiffRobotComments').returns(Promise.resolve({}));
@@ -93,12 +94,22 @@
stub('gr-diff-host', 'reload').callsFake(() => Promise.resolve());
stub('gr-diff-host', 'prefetchDiff').callsFake(() => {});
- // Element must be wrapped in an element with direct access to the
- // comment API.
element = basicFixture.instantiate();
- element._loading = false;
- element.diffPrefs = {} as DiffPreferencesInfo;
+ element.loading = false;
+ element.diffPrefs = {
+ context: 10,
+ tab_size: 8,
+ font_size: 12,
+ line_length: 100,
+ cursor_blink_rate: 0,
+ line_wrapping: false,
+ show_line_endings: true,
+ show_tabs: true,
+ show_whitespace_errors: true,
+ syntax_highlighting: true,
+ ignore_whitespace: 'IGNORE_NONE',
+ };
element.numFilesShown = 200;
element.patchRange = {
basePatchNum: 'PARENT' as BasePatchSetNum,
@@ -107,6 +118,9 @@
saveStub = sinon
.stub(element, '_saveReviewedState')
.callsFake(() => Promise.resolve());
+ await element.updateComplete;
+ // Wait for expandedFilesChanged to complete.
+ await flush();
});
test('renders', () => {
@@ -117,9 +131,6 @@
</h3>
<div aria-label="Files list" id="container" role="grid">
<div class="header-row row" role="row">
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
<div class="path" role="columnheader">File</div>
<div class="comments desktop" role="columnheader">Comments</div>
<div class="comments mobile" role="columnheader" title="Comments">
@@ -127,60 +138,10 @@
</div>
<div class="desktop sizeBars" role="columnheader">Size</div>
<div class="header-stats" role="columnheader">Delta</div>
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
- <div
- aria-hidden="true"
- class="hideOnEdit reviewed"
- hidden="true"
- ></div>
+ <div aria-hidden="true" class="hideOnEdit reviewed" hidden=""></div>
<div aria-hidden="true" class="editFileControls showOnEdit"></div>
<div aria-hidden="true" class="show-hide"></div>
</div>
- <dom-repeat
- as="file"
- id="files"
- style="display: none;"
- target-framerate="1"
- >
- <template is="dom-repeat"> </template>
- </dom-repeat>
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
- </div>
- <div class="row totalChanges" hidden="true">
- <div class="total-stats">
- <div>
- <span aria-label="Total 0 lines added" class="added" tabindex="0">
- +0
- </span>
- <span
- aria-label="Total 0 lines removed"
- class="removed"
- tabindex="0"
- >
- -0
- </span>
- </div>
- </div>
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
- <div class="hideOnEdit reviewed" hidden="true"></div>
- <div class="editFileControls showOnEdit"></div>
- <div class="show-hide"></div>
- </div>
- <div class="row totalChanges" hidden="true">
- <div class="total-stats">
- <span aria-label="Total bytes inserted: +/-0 B " class="added">
- +/-0 B
- </span>
- <span aria-label="Total bytes removed: +/-0 B" class="removed">
- +/-0 B
- </span>
- </div>
</div>
<div class="controlRow invisible row">
<gr-button
@@ -211,9 +172,9 @@
></gr-diff-preferences-dialog>`);
});
- test('renders file row', () => {
- element._filesByPath = createFilesByPath(1);
- flush();
+ test('renders file row', async () => {
+ element.filesByPath = createFilesByPath(1);
+ await element.updateComplete;
const fileRows = queryAll<HTMLDivElement>(element, '.file-row');
expect(fileRows?.[0]).dom.equal(/* HTML */ `<div
class="file-row row"
@@ -221,9 +182,6 @@
role="row"
tabindex="-1"
>
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
<span class="path" role="gridcell">
<a class="pathLink">
<span class="fullFileName" title="'/file0"> '/file0 </span>
@@ -231,9 +189,6 @@
<gr-file-status-chip> </gr-file-status-chip>
<gr-copy-clipboard hideinput=""> </gr-copy-clipboard>
</a>
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
</span>
<div role="gridcell">
<div class="comments desktop">
@@ -248,7 +203,7 @@
<div class="desktop" role="gridcell">
<div
aria-label="A bar that represents the addition and deletion ratio for the current file"
- class="sizeBars"
+ class="hide sizeBars"
></div>
</div>
<div class="stats" role="gridcell">
@@ -259,35 +214,14 @@
<span aria-label="0 lines removed" class="removed" tabindex="0">
-0
</span>
- <span hidden="true"> +/-0 B </span>
+ <span hidden=""> +/-0 B </span>
</div>
</div>
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
- <div class="hideOnEdit reviewed" hidden="true" role="gridcell">
- <span aria-hidden="true" class="reviewedLabel"> Reviewed </span>
- <span
- aria-checked="false"
- aria-label="Reviewed"
- class="reviewedSwitch"
- role="switch"
- tabindex="0"
- >
- <span
- class="markReviewed"
- tabindex="-1"
- title="Mark as reviewed (shortcut: r)"
- >
- MARK REVIEWED
- </span>
- </span>
- </div>
- <div class="editFileControls showOnEdit" role="gridcell">
- <dom-if style="display: none;">
- <template is="dom-if"> </template>
- </dom-if>
- </div>
+ <div
+ aria-hidden="true"
+ class="editFileControls showOnEdit"
+ role="gridcell"
+ ></div>
<div class="show-hide" role="gridcell">
<span
aria-checked="false"
@@ -298,18 +232,24 @@
role="switch"
tabindex="0"
>
- <iron-icon class="show-hide-icon" id="icon" tabindex="-1">
+ <iron-icon
+ class="show-hide-icon"
+ id="icon"
+ tabindex="-1"
+ icon="gr-icons:expand-more"
+ >
</iron-icon>
</span>
</div>
</div>`);
});
- test('correct number of files are shown', () => {
+ test('correct number of files are shown', async () => {
element.fileListIncrement = 300;
- element._filesByPath = createFilesByPath(500);
+ element.filesByPath = createFilesByPath(500);
+ await element.updateComplete;
+ await flush();
- flush();
assert.equal(
queryAll<HTMLDivElement>(element, '.file-row').length,
element.numFilesShown
@@ -329,22 +269,25 @@
);
MockInteractions.tap(queryAndAssert<GrButton>(element, '#showAllButton'));
- flush();
+ await element.updateComplete;
+ await flush();
assert.equal(element.numFilesShown, 500);
- assert.equal(element._shownFiles.length, 500);
+ assert.equal(element.shownFiles.length, 500);
assert.isTrue(controlRow.classList.contains('invisible'));
});
- test('rendering each row calls the _reportRenderedRow method', () => {
- const renderedStub = sinon.stub(element, '_reportRenderedRow');
- element._filesByPath = createFilesByPath(10);
+ test('rendering each row calls the reportRenderedRow method', async () => {
+ const renderedStub = sinon.stub(element, 'reportRenderedRow');
+ element.filesByPath = createFilesByPath(10);
+ await element.updateComplete;
+
assert.equal(queryAll<HTMLDivElement>(element, '.file-row').length, 10);
assert.equal(renderedStub.callCount, 10);
});
- test('calculate totals for patch number', () => {
- element._filesByPath = {
+ test('calculate totals for patch number', async () => {
+ element.filesByPath = {
'/COMMIT_MSG': {
lines_inserted: 9,
size: 0,
@@ -368,19 +311,21 @@
size: 100,
},
};
+ await element.updateComplete;
- assert.deepEqual(element._patchChange, {
+ let patchChange = element.calculatePatchChange();
+ assert.deepEqual(patchChange, {
inserted: 2,
deleted: 2,
size_delta_inserted: 0,
size_delta_deleted: 0,
total_size: 0,
});
- assert.isTrue(element._hideBinaryChangeTotals);
- assert.isFalse(element._hideChangeTotals);
+ assert.isTrue(element.shouldHideBinaryChangeTotals(patchChange));
+ assert.isFalse(element.shouldHideChangeTotals(patchChange));
// Test with a commit message that isn't the first file.
- element._filesByPath = {
+ element.filesByPath = {
'file_added_in_rev2.txt': {
lines_inserted: 1,
lines_deleted: 1,
@@ -404,19 +349,21 @@
size_delta: 0,
},
};
+ await element.updateComplete;
- assert.deepEqual(element._patchChange, {
+ patchChange = element.calculatePatchChange();
+ assert.deepEqual(patchChange, {
inserted: 2,
deleted: 2,
size_delta_inserted: 0,
size_delta_deleted: 0,
total_size: 0,
});
- assert.isTrue(element._hideBinaryChangeTotals);
- assert.isFalse(element._hideChangeTotals);
+ assert.isTrue(element.shouldHideBinaryChangeTotals(patchChange));
+ assert.isFalse(element.shouldHideChangeTotals(patchChange));
// Test with no commit message.
- element._filesByPath = {
+ element.filesByPath = {
'file_added_in_rev2.txt': {
lines_inserted: 1,
lines_deleted: 1,
@@ -430,19 +377,21 @@
size_delta: 0,
},
};
+ await element.updateComplete;
- assert.deepEqual(element._patchChange, {
+ patchChange = element.calculatePatchChange();
+ assert.deepEqual(patchChange, {
inserted: 2,
deleted: 2,
size_delta_inserted: 0,
size_delta_deleted: 0,
total_size: 0,
});
- assert.isTrue(element._hideBinaryChangeTotals);
- assert.isFalse(element._hideChangeTotals);
+ assert.isTrue(element.shouldHideBinaryChangeTotals(patchChange));
+ assert.isFalse(element.shouldHideChangeTotals(patchChange));
// Test with files missing either lines_inserted or lines_deleted.
- element._filesByPath = {
+ element.filesByPath = {
'file_added_in_rev2.txt': {
lines_inserted: 1,
size: 0,
@@ -454,19 +403,22 @@
size_delta: 0,
},
};
- assert.deepEqual(element._patchChange, {
+ await element.updateComplete;
+
+ patchChange = element.calculatePatchChange();
+ assert.deepEqual(patchChange, {
inserted: 1,
deleted: 1,
size_delta_inserted: 0,
size_delta_deleted: 0,
total_size: 0,
});
- assert.isTrue(element._hideBinaryChangeTotals);
- assert.isFalse(element._hideChangeTotals);
+ assert.isTrue(element.shouldHideBinaryChangeTotals(patchChange));
+ assert.isFalse(element.shouldHideChangeTotals(patchChange));
});
- test('binary only files', () => {
- element._filesByPath = {
+ test('binary only files', async () => {
+ element.filesByPath = {
'/COMMIT_MSG': {
lines_inserted: 9,
size: 0,
@@ -475,19 +427,22 @@
file_binary_1: {binary: true, size_delta: 10, size: 100},
file_binary_2: {binary: true, size_delta: -5, size: 120},
};
- assert.deepEqual(element._patchChange, {
+ await element.updateComplete;
+
+ const patchChange = element.calculatePatchChange();
+ assert.deepEqual(patchChange, {
inserted: 0,
deleted: 0,
size_delta_inserted: 10,
size_delta_deleted: -5,
total_size: 220,
});
- assert.isFalse(element._hideBinaryChangeTotals);
- assert.isTrue(element._hideChangeTotals);
+ assert.isFalse(element.shouldHideBinaryChangeTotals(patchChange));
+ assert.isTrue(element.shouldHideChangeTotals(patchChange));
});
- test('binary and regular files', () => {
- element._filesByPath = {
+ test('binary and regular files', async () => {
+ element.filesByPath = {
'/COMMIT_MSG': {
lines_inserted: 9,
size: 0,
@@ -502,18 +457,21 @@
size_delta: 0,
},
};
- assert.deepEqual(element._patchChange, {
+ await element.updateComplete;
+
+ const patchChange = element.calculatePatchChange();
+ assert.deepEqual(patchChange, {
inserted: 10,
deleted: 5,
size_delta_inserted: 10,
size_delta_deleted: -5,
total_size: 220,
});
- assert.isFalse(element._hideBinaryChangeTotals);
- assert.isFalse(element._hideChangeTotals);
+ assert.isFalse(element.shouldHideBinaryChangeTotals(patchChange));
+ assert.isFalse(element.shouldHideChangeTotals(patchChange));
});
- test('_formatBytes function', () => {
+ test('formatBytes function', () => {
const table = {
'64': '+64 B',
'1023': '+1023 B',
@@ -528,11 +486,11 @@
'0': '+/-0 B',
};
for (const [bytes, expected] of Object.entries(table)) {
- assert.equal(element._formatBytes(Number(bytes)), expected);
+ assert.equal(element.formatBytes(Number(bytes)), expected);
}
});
- test('_formatPercentage function', () => {
+ test('formatPercentage function', () => {
const table = [
{size: 100, delta: 100, display: ''},
{size: 195060, delta: 64, display: '(+0%)'},
@@ -544,7 +502,7 @@
for (const item of table) {
assert.equal(
- element._formatPercentage(item.size, item.delta),
+ element.formatPercentage(item.size, item.delta),
item.display
);
}
@@ -567,224 +525,253 @@
patchNum: 2 as RevisionPatchSetNum,
};
+ element.patchRange = parentTo1;
assert.equal(
- element._computeCommentsStringMobile(
- element.changeComments,
- parentTo1,
- {__path: '/COMMIT_MSG', size: 0, size_delta: 0}
- ),
+ element.computeCommentsStringMobile({
+ __path: '/COMMIT_MSG',
+ size: 0,
+ size_delta: 0,
+ }),
'2c'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeCommentsStringMobile(element.changeComments, _1To2, {
+ element.computeCommentsStringMobile({
__path: '/COMMIT_MSG',
size: 0,
size_delta: 0,
}),
'3c'
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsString(element.changeComments, parentTo1, {
+ element.computeDraftsString({
__path: 'unresolved.file',
size: 0,
size_delta: 0,
}),
'1 draft'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsString(element.changeComments, _1To2, {
+ element.computeDraftsString({
__path: 'unresolved.file',
size: 0,
size_delta: 0,
}),
'1 draft'
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, parentTo1, {
+ element.computeDraftsStringMobile({
__path: 'unresolved.file',
size: 0,
size_delta: 0,
}),
'1d'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, _1To2, {
+ element.computeDraftsStringMobile({
__path: 'unresolved.file',
size: 0,
size_delta: 0,
}),
'1d'
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeCommentsStringMobile(
- element.changeComments,
- parentTo1,
- {__path: 'myfile.txt', size: 0, size_delta: 0}
- ),
+ element.computeCommentsStringMobile({
+ __path: 'myfile.txt',
+ size: 0,
+ size_delta: 0,
+ }),
'1c'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeCommentsStringMobile(element.changeComments, _1To2, {
+ element.computeCommentsStringMobile({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
}),
'3c'
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsString(element.changeComments, parentTo1, {
+ element.computeDraftsString({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsString(element.changeComments, _1To2, {
+ element.computeDraftsString({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
}),
''
);
+
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, parentTo1, {
+ element.computeDraftsStringMobile({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, _1To2, {
+ element.computeDraftsStringMobile({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeCommentsStringMobile(
- element.changeComments,
- parentTo1,
- {__path: 'file_added_in_rev2.txt', size: 0, size_delta: 0}
- ),
- ''
- );
- assert.equal(
- element._computeCommentsStringMobile(element.changeComments, _1To2, {
+ element.computeCommentsStringMobile({
__path: 'file_added_in_rev2.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsString(element.changeComments, parentTo1, {
+ element.computeCommentsStringMobile({
__path: 'file_added_in_rev2.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsString(element.changeComments, _1To2, {
+ element.computeDraftsString({
__path: 'file_added_in_rev2.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, parentTo1, {
+ element.computeDraftsString({
__path: 'file_added_in_rev2.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, _1To2, {
+ element.computeDraftsStringMobile({
__path: 'file_added_in_rev2.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeCommentsStringMobile(
- element.changeComments,
- parentTo2,
- {__path: '/COMMIT_MSG', size: 0, size_delta: 0}
- ),
+ element.computeDraftsStringMobile({
+ __path: 'file_added_in_rev2.txt',
+ size: 0,
+ size_delta: 0,
+ }),
+ ''
+ );
+ element.patchRange = parentTo2;
+ assert.equal(
+ element.computeCommentsStringMobile({
+ __path: '/COMMIT_MSG',
+ size: 0,
+ size_delta: 0,
+ }),
'1c'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeCommentsStringMobile(element.changeComments, _1To2, {
+ element.computeCommentsStringMobile({
__path: '/COMMIT_MSG',
size: 0,
size_delta: 0,
}),
'3c'
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsString(element.changeComments, parentTo1, {
+ element.computeDraftsString({
__path: '/COMMIT_MSG',
size: 0,
size_delta: 0,
}),
'2 drafts'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsString(element.changeComments, _1To2, {
+ element.computeDraftsString({
__path: '/COMMIT_MSG',
size: 0,
size_delta: 0,
}),
'2 drafts'
);
+ element.patchRange = parentTo1;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, parentTo1, {
+ element.computeDraftsStringMobile({
__path: '/COMMIT_MSG',
size: 0,
size_delta: 0,
}),
'2d'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, _1To2, {
+ element.computeDraftsStringMobile({
__path: '/COMMIT_MSG',
size: 0,
size_delta: 0,
}),
'2d'
);
+ element.patchRange = parentTo2;
assert.equal(
- element._computeCommentsStringMobile(
- element.changeComments,
- parentTo2,
- {__path: 'myfile.txt', size: 0, size_delta: 0}
- ),
+ element.computeCommentsStringMobile({
+ __path: 'myfile.txt',
+ size: 0,
+ size_delta: 0,
+ }),
'2c'
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeCommentsStringMobile(element.changeComments, _1To2, {
+ element.computeCommentsStringMobile({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
}),
'3c'
);
+ element.patchRange = parentTo2;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, parentTo2, {
+ element.computeDraftsStringMobile({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
}),
''
);
+ element.patchRange = _1To2;
assert.equal(
- element._computeDraftsStringMobile(element.changeComments, _1To2, {
+ element.computeDraftsStringMobile({
__path: 'myfile.txt',
size: 0,
size_delta: 0,
@@ -793,21 +780,21 @@
);
});
- test('_reviewedTitle', () => {
+ test('reviewedTitle', () => {
assert.equal(
- element._reviewedTitle(true),
+ element.reviewedTitle(true),
'Mark as not reviewed (shortcut: r)'
);
assert.equal(
- element._reviewedTitle(false),
+ element.reviewedTitle(false),
'Mark as reviewed (shortcut: r)'
);
});
suite('keyboard shortcuts', () => {
- setup(() => {
- element._filesByPath = {
+ setup(async () => {
+ element.filesByPath = {
'/COMMIT_MSG': {size: 0, size_delta: 0},
'file_added_in_rev2.txt': {size: 0, size_delta: 0},
'myfile.txt': {size: 0, size_delta: 0},
@@ -819,6 +806,8 @@
};
element.change = {_number: 42 as NumericChangeId} as ParsedChangeInfo;
element.fileCursor.setCursorAtIndex(0);
+ await element.updateComplete;
+ await flush();
});
test('toggle left diff via shortcut', () => {
@@ -833,9 +822,7 @@
diffsStub.restore();
});
- test('keyboard shortcuts', () => {
- flush();
-
+ test('keyboard shortcuts', async () => {
const items = [...queryAll<HTMLDivElement>(element, '.file-row')];
element.fileCursor.stops = items;
element.fileCursor.setCursorAtIndex(0);
@@ -895,67 +882,69 @@
assert.isTrue(createCommentInPlaceStub.called);
});
- test('i key shows/hides selected inline diff', () => {
- const paths = Object.keys(element._filesByPath!);
- sinon.stub(element, '_expandedFilesChanged');
- flush();
+ test('i key shows/hides selected inline diff', async () => {
+ const paths = Object.keys(element.filesByPath!);
+ sinon.stub(element, 'expandedFilesChanged');
const files = [...queryAll<HTMLDivElement>(element, '.file-row')];
element.fileCursor.stops = files;
element.fileCursor.setCursorAtIndex(0);
+ await element.updateComplete;
assert.equal(element.diffs.length, 0);
- assert.equal(element._expandedFiles.length, 0);
+ assert.equal(element.expandedFiles.length, 0);
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
- flush();
+ await element.updateComplete;
assert.equal(element.diffs.length, 1);
assert.equal(element.diffs[0].path, paths[0]);
- assert.equal(element._expandedFiles.length, 1);
- assert.equal(element._expandedFiles[0].path, paths[0]);
+ assert.equal(element.expandedFiles.length, 1);
+ assert.equal(element.expandedFiles[0].path, paths[0]);
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
- flush();
+ await element.updateComplete;
assert.equal(element.diffs.length, 0);
- assert.equal(element._expandedFiles.length, 0);
+ assert.equal(element.expandedFiles.length, 0);
element.fileCursor.setCursorAtIndex(1);
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
- flush();
+ await element.updateComplete;
assert.equal(element.diffs.length, 1);
assert.equal(element.diffs[0].path, paths[1]);
- assert.equal(element._expandedFiles.length, 1);
- assert.equal(element._expandedFiles[0].path, paths[1]);
+ assert.equal(element.expandedFiles.length, 1);
+ assert.equal(element.expandedFiles[0].path, paths[1]);
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'I');
- flush();
+ await element.updateComplete;
assert.equal(element.diffs.length, paths.length);
- assert.equal(element._expandedFiles.length, paths.length);
+ assert.equal(element.expandedFiles.length, paths.length);
for (const diff of element.diffs) {
- assert.isTrue(element._expandedFiles.some(f => f.path === diff.path));
+ assert.isTrue(element.expandedFiles.some(f => f.path === diff.path));
}
// since _expandedFilesChanged is stubbed
element.filesExpanded = FilesExpandedState.ALL;
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'I');
- flush();
+ await element.updateComplete;
assert.equal(element.diffs.length, 0);
- assert.equal(element._expandedFiles.length, 0);
+ assert.equal(element.expandedFiles.length, 0);
});
- test('r key toggles reviewed flag', () => {
+ test('r key toggles reviewed flag', async () => {
const reducer = (accum: number, file: NormalizedFileInfo) =>
file.isReviewed ? ++accum : accum;
- const getNumReviewed = () => element._files.reduce(reducer, 0);
- flush();
+ const getNumReviewed = () => element.files.reduce(reducer, 0);
+ await element.updateComplete;
// Default state should be unreviewed.
assert.equal(getNumReviewed(), 0);
// Press the review key to toggle it (set the flag).
+ element.handleCursorNext(new KeyboardEvent('keydown'));
MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
- flush();
+ await element.updateComplete;
assert.equal(getNumReviewed(), 1);
// Press the review key to toggle it (clear the flag).
MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
+ await element.updateComplete;
assert.equal(getNumReviewed(), 0);
});
@@ -963,9 +952,9 @@
let interact: Function;
setup(() => {
- const openCursorStub = sinon.stub(element, '_openCursorFile');
- const openSelectedStub = sinon.stub(element, '_openSelectedFile');
- const expandStub = sinon.stub(element, '_toggleFileExpanded');
+ const openCursorStub = sinon.stub(element, 'openCursorFile');
+ const openSelectedStub = sinon.stub(element, 'openSelectedFile');
+ const expandStub = sinon.stub(element, 'toggleFileExpanded');
interact = function () {
openCursorStub.reset();
@@ -1007,9 +996,7 @@
const moveRightStub = sinon.stub(element.diffCursor, 'moveRight');
let noDiffsExpanded = true;
- sinon
- .stub(element, '_noDiffsExpanded')
- .callsFake(() => noDiffsExpanded);
+ sinon.stub(element, 'noDiffsExpanded').callsFake(() => noDiffsExpanded);
MockInteractions.pressAndReleaseKeyOn(
element,
@@ -1045,25 +1032,26 @@
});
});
- test('file review status', () => {
+ test('file review status', async () => {
element.reviewed = ['/COMMIT_MSG', 'myfile.txt'];
- element._filesByPath = {
+ element.filesByPath = {
'/COMMIT_MSG': {size: 0, size_delta: 0},
'file_added_in_rev2.txt': {size: 0, size_delta: 0},
'myfile.txt': {size: 0, size_delta: 0},
};
- element._loggedIn = true;
+ element.loggedIn = true;
element.changeNum = 42 as NumericChangeId;
element.patchRange = {
basePatchNum: 'PARENT' as BasePatchSetNum,
patchNum: 2 as RevisionPatchSetNum,
};
element.fileCursor.setCursorAtIndex(0);
- const reviewSpy = sinon.spy(element, '_reviewFile');
- const toggleExpandSpy = sinon.spy(element, '_toggleFileExpanded');
- flush();
- queryAll(element, '.row:not(.header-row)');
+ const reviewSpy = sinon.spy(element, 'reviewFile');
+ const toggleExpandSpy = sinon.spy(element, 'toggleFileExpanded');
+
+ await element.updateComplete;
+
const fileRows = queryAll(element, '.row:not(.header-row)');
const checkSelector = 'span.reviewedSwitch[role="switch"]';
const commitMsg = fileRows[0].querySelector(checkSelector);
@@ -1075,19 +1063,26 @@
assert.equal(myFile!.getAttribute('aria-checked'), 'true');
const commitReviewLabel = fileRows[0].querySelector('.reviewedLabel');
+ assert.isOk(commitReviewLabel);
const markReviewLabel = fileRows[0].querySelector('.markReviewed');
+ assert.isOk(markReviewLabel);
assert.isTrue(commitReviewLabel!.classList.contains('isReviewed'));
assert.equal(markReviewLabel!.textContent, 'MARK UNREVIEWED');
- const clickSpy = sinon.spy(element, '_reviewedClick');
+ const clickSpy = sinon.spy(element, 'reviewedClick');
MockInteractions.tap(markReviewLabel!);
+ await element.updateComplete;
+
// assert.isTrue(saveStub.lastCall.calledWithExactly('/COMMIT_MSG', false));
// assert.isFalse(commitReviewLabel.classList.contains('isReviewed'));
assert.equal(markReviewLabel!.textContent, 'MARK REVIEWED');
+ assert.isTrue(clickSpy.calledOnce);
assert.isTrue(clickSpy.lastCall.args[0].defaultPrevented);
assert.isTrue(reviewSpy.calledOnce);
MockInteractions.tap(markReviewLabel!);
+ await element.updateComplete;
+
assert.isTrue(saveStub.lastCall.calledWithExactly('/COMMIT_MSG', true));
assert.isTrue(commitReviewLabel!.classList.contains('isReviewed'));
assert.equal(markReviewLabel!.textContent, 'MARK UNREVIEWED');
@@ -1097,8 +1092,8 @@
assert.isFalse(toggleExpandSpy.called);
});
- test('_handleFileListClick', () => {
- element._filesByPath = {
+ test('handleFileListClick', async () => {
+ element.filesByPath = {
'/COMMIT_MSG': {size: 0, size_delta: 0},
'f1.txt': {size: 0, size_delta: 0},
'f2.txt': {size: 0, size_delta: 0},
@@ -1108,33 +1103,36 @@
basePatchNum: 'PARENT' as BasePatchSetNum,
patchNum: 2 as RevisionPatchSetNum,
};
+ await element.updateComplete;
- const clickSpy = sinon.spy(element, '_handleFileListClick');
- const reviewStub = sinon.stub(element, '_reviewFile');
- const toggleExpandSpy = sinon.spy(element, '_toggleFileExpanded');
+ const clickSpy = sinon.spy(element, 'handleFileListClick');
+ const reviewStub = sinon.stub(element, 'reviewFile');
+ const toggleExpandSpy = sinon.spy(element, 'toggleFileExpanded');
const row = queryAndAssert(
element,
'.row[data-file=\'{"path":"f1.txt"}\']'
);
- // Click on the expand button, resulting in _toggleFileExpanded being
- // called and not resulting in a call to _reviewFile.
+ // Click on the expand button, resulting in toggleFileExpanded being
+ // called and not resulting in a call to reviewFile.
queryAndAssert<HTMLDivElement>(row, 'div.show-hide').click();
+ await element.updateComplete;
assert.isTrue(clickSpy.calledOnce);
assert.isTrue(toggleExpandSpy.calledOnce);
assert.isFalse(reviewStub.called);
// Click inside the diff. This should result in no additional calls to
- // _toggleFileExpanded or _reviewFile.
+ // toggleFileExpanded or reviewFile.
queryAndAssert<GrDiffHost>(element, 'gr-diff-host').click();
+ await element.updateComplete;
assert.isTrue(clickSpy.calledTwice);
assert.isTrue(toggleExpandSpy.calledOnce);
assert.isFalse(reviewStub.called);
});
- test('_handleFileListClick editMode', () => {
- element._filesByPath = {
+ test('handleFileListClick editMode', async () => {
+ element.filesByPath = {
'/COMMIT_MSG': {size: 0, size_delta: 0},
'f1.txt': {size: 0, size_delta: 0},
'f2.txt': {size: 0, size_delta: 0},
@@ -1145,18 +1143,21 @@
patchNum: 2 as RevisionPatchSetNum,
};
element.editMode = true;
- flush();
- const clickSpy = sinon.spy(element, '_handleFileListClick');
- const toggleExpandSpy = sinon.spy(element, '_toggleFileExpanded');
+ await element.updateComplete;
- // Tap the edit controls. Should be ignored by _handleFileListClick.
+ const clickSpy = sinon.spy(element, 'handleFileListClick');
+ const toggleExpandSpy = sinon.spy(element, 'toggleFileExpanded');
+
+ // Tap the edit controls. Should be ignored by handleFileListClick.
MockInteractions.tap(queryAndAssert(element, '.editFileControls'));
+ await element.updateComplete;
+
assert.isTrue(clickSpy.calledOnce);
assert.isFalse(toggleExpandSpy.called);
});
- test('checkbox shows/hides diff inline', () => {
- element._filesByPath = {
+ test('checkbox shows/hides diff inline', async () => {
+ element.filesByPath = {
'myfile.txt': {size: 0, size_delta: 0},
};
element.changeNum = 42 as NumericChangeId;
@@ -1165,8 +1166,8 @@
patchNum: 2 as RevisionPatchSetNum,
};
element.fileCursor.setCursorAtIndex(0);
- sinon.stub(element, '_expandedFilesChanged');
- flush();
+ sinon.stub(element, 'expandedFilesChanged');
+ await element.updateComplete;
const fileRows = queryAll(element, '.row:not(.header-row)');
// Because the label surrounds the input, the tap event is triggered
// there first.
@@ -1176,15 +1177,17 @@
const showHideLabel = showHideCheck!.querySelector('.show-hide-icon');
assert.equal(showHideCheck!.getAttribute('aria-checked'), 'false');
MockInteractions.tap(showHideLabel!);
+ await element.updateComplete;
+
assert.equal(showHideCheck!.getAttribute('aria-checked'), 'true');
assert.notEqual(
- element._expandedFiles.findIndex(f => f.path === 'myfile.txt'),
+ element.expandedFiles.findIndex(f => f.path === 'myfile.txt'),
-1
);
});
- test('diff mode correctly toggles the diffs', () => {
- element._filesByPath = {
+ test('diff mode correctly toggles the diffs', async () => {
+ element.filesByPath = {
'myfile.txt': {size: 0, size_delta: 0},
};
element.changeNum = 42 as NumericChangeId;
@@ -1192,28 +1195,30 @@
basePatchNum: 'PARENT' as BasePatchSetNum,
patchNum: 2 as RevisionPatchSetNum,
};
- const updateDiffPrefSpy = sinon.spy(element, '_updateDiffPreferences');
+ const updateDiffPrefSpy = sinon.spy(element, 'updateDiffPreferences');
element.fileCursor.setCursorAtIndex(0);
- flush();
+ await element.updateComplete;
// Tap on a file to generate the diff.
const row = queryAll(element, '.row:not(.header-row) span.show-hide')[0];
MockInteractions.tap(row);
- flush();
- element.set('diffViewMode', 'UNIFIED_DIFF');
+
+ element.diffViewMode = DiffViewMode.UNIFIED;
+ await element.updateComplete;
+
assert.isTrue(updateDiffPrefSpy.called);
});
test('expanded attribute not set on path when not expanded', () => {
- element._filesByPath = {
+ element.filesByPath = {
'/COMMIT_MSG': {size: 0, size_delta: 0},
};
assert.isNotOk(query(element, 'expanded'));
});
- test('tapping row ignores links', () => {
- element._filesByPath = {
+ test('tapping row ignores links', async () => {
+ element.filesByPath = {
'/COMMIT_MSG': {size: 0, size_delta: 0},
};
element.changeNum = 42 as NumericChangeId;
@@ -1221,8 +1226,8 @@
basePatchNum: 'PARENT' as BasePatchSetNum,
patchNum: 2 as RevisionPatchSetNum,
};
- sinon.stub(element, '_expandedFilesChanged');
- flush();
+ sinon.stub(element, 'expandedFilesChanged');
+ await element.updateComplete;
const commitMsgFile = queryAll(
element,
'.row:not(.header-row) a.pathLink'
@@ -1230,10 +1235,10 @@
// Remove href attribute so the app doesn't route to a diff view
commitMsgFile.removeAttribute('href');
- const togglePathSpy = sinon.spy(element, '_toggleFileExpanded');
+ const togglePathSpy = sinon.spy(element, 'toggleFileExpanded');
MockInteractions.tap(commitMsgFile);
- flush();
+ await element.updateComplete;
assert(togglePathSpy.notCalled, 'file is opened as diff view');
assert.isNotOk(query(element, '.expanded'));
assert.notEqual(
@@ -1242,19 +1247,26 @@
);
});
- test('_toggleFileExpanded', () => {
+ test('toggleFileExpanded', async () => {
const path = 'path/to/my/file.txt';
- element._filesByPath = {[path]: {size: 0, size_delta: 0}};
- const renderSpy = sinon.spy(element, '_renderInOrder');
- const collapseStub = sinon.stub(element, '_clearCollapsedDiffs');
+ element.filesByPath = {[path]: {size: 0, size_delta: 0}};
+ await element.updateComplete;
+ // Wait for expandedFilesChanged to finish.
+ await flush();
+
+ const renderSpy = sinon.spy(element, 'renderInOrder');
+ const collapseStub = sinon.stub(element, 'clearCollapsedDiffs');
assert.equal(
queryAndAssert<IronIconElement>(element, 'iron-icon').icon,
'gr-icons:expand-more'
);
- assert.equal(element._expandedFiles.length, 0);
- element._toggleFileExpanded({path});
- flush();
+ assert.equal(element.expandedFiles.length, 0);
+ element.toggleFileExpanded({path});
+ await element.updateComplete;
+ // Wait for expandedFilesChanged to finish.
+ await flush();
+
assert.equal(collapseStub.lastCall.args[0].length, 0);
assert.equal(
queryAndAssert<IronIconElement>(element, 'iron-icon').icon,
@@ -1262,45 +1274,49 @@
);
assert.equal(renderSpy.callCount, 1);
- assert.isTrue(element._expandedFiles.some(f => f.path === path));
- element._toggleFileExpanded({path});
- flush();
+ assert.isTrue(element.expandedFiles.some(f => f.path === path));
+ element.toggleFileExpanded({path});
+ await element.updateComplete;
+ // Wait for expandedFilesChanged to finish.
+ await flush();
assert.equal(
queryAndAssert<IronIconElement>(element, 'iron-icon').icon,
'gr-icons:expand-more'
);
assert.equal(renderSpy.callCount, 1);
- assert.isFalse(element._expandedFiles.some(f => f.path === path));
+ assert.isFalse(element.expandedFiles.some(f => f.path === path));
assert.equal(collapseStub.lastCall.args[0].length, 1);
});
- test('expandAllDiffs and collapseAllDiffs', () => {
- const collapseStub = sinon.stub(element, '_clearCollapsedDiffs');
- const cursorUpdateStub = sinon.stub(
- element.diffCursor,
- 'handleDiffUpdate'
- );
+ test('expandAllDiffs and collapseAllDiffs', async () => {
+ const collapseStub = sinon.stub(element, 'clearCollapsedDiffs');
const reInitStub = sinon.stub(element.diffCursor, 'reInitAndUpdateStops');
const path = 'path/to/my/file.txt';
- element._filesByPath = {[path]: {size: 0, size_delta: 0}};
+ element.filesByPath = {[path]: {size: 0, size_delta: 0}};
+ // Wait for diffs to be computed.
+ await element.updateComplete;
+ await flush();
element.expandAllDiffs();
- flush();
+ await element.updateComplete;
+ // Wait for expandedFilesChanged to finish.
+ await flush();
assert.equal(element.filesExpanded, FilesExpandedState.ALL);
- assert.isTrue(reInitStub.calledOnce);
+ assert.isTrue(reInitStub.calledTwice);
assert.equal(collapseStub.lastCall.args[0].length, 0);
element.collapseAllDiffs();
- flush();
- assert.equal(element._expandedFiles.length, 0);
+ await element.updateComplete;
+ // Wait for expandedFilesChanged to finish.
+ await flush();
+ assert.equal(element.expandedFiles.length, 0);
assert.equal(element.filesExpanded, FilesExpandedState.NONE);
- assert.isTrue(cursorUpdateStub.calledOnce);
assert.equal(collapseStub.lastCall.args[0].length, 1);
});
- test('_expandedFilesChanged', async () => {
- sinon.stub(element, '_reviewFile');
+ test('expandedFilesChanged', async () => {
+ sinon.stub(element, 'reviewFile');
const path = 'path/to/my/file.txt';
const promise = mockPromise();
const diffs = [
@@ -1326,11 +1342,13 @@
},
];
sinon.stub(element, 'diffs').get(() => diffs);
- element.push('_expandedFiles', {path});
+ element.expandedFiles = element.expandedFiles.concat([{path}]);
+ await element.updateComplete;
+ await flush();
await promise;
});
- test('_clearCollapsedDiffs', () => {
+ test('clearCollapsedDiffs', () => {
// Have to type as any because the type is 'GrDiffHost'
// which would require stubbing so many different
// methods / properties that it isn't worth it.
@@ -1338,34 +1356,36 @@
cancel: sinon.stub(),
clearDiffContent: sinon.stub(),
} as any;
- element._clearCollapsedDiffs([diff]);
+ element.clearCollapsedDiffs([diff]);
assert.isTrue(diff.cancel.calledOnce);
assert.isTrue(diff.clearDiffContent.calledOnce);
});
- test('filesExpanded value updates to correct enum', () => {
- element._filesByPath = {
+ test('filesExpanded value updates to correct enum', async () => {
+ element.filesByPath = {
'foo.bar': {size: 0, size_delta: 0},
'baz.bar': {size: 0, size_delta: 0},
};
- flush();
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.NONE);
- element.push('_expandedFiles', {path: 'baz.bar'});
- flush();
+ element.expandedFiles.push({path: 'baz.bar'});
+ element.expandedFilesChanged([{path: 'baz.bar'}]);
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.SOME);
- element.push('_expandedFiles', {path: 'foo.bar'});
- flush();
+ element.expandedFiles.push({path: 'foo.bar'});
+ element.expandedFilesChanged([{path: 'foo.bar'}]);
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.ALL);
element.collapseAllDiffs();
- flush();
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.NONE);
element.expandAllDiffs();
- flush();
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.ALL);
});
- test('_renderInOrder', async () => {
- const reviewStub = sinon.stub(element, '_reviewFile');
+ test('renderInOrder', async () => {
+ const reviewStub = sinon.stub(element, 'reviewFile');
let callCount = 0;
// Have to type as any because the type is 'GrDiffHost'
// which would require stubbing so many different
@@ -1399,18 +1419,14 @@
},
},
] as any;
- element._renderInOrder(
- [{path: 'p2'}, {path: 'p1'}, {path: 'p0'}],
- diffs,
- 3
- );
- await flush();
+ element.renderInOrder([{path: 'p2'}, {path: 'p1'}, {path: 'p0'}], diffs);
+ await element.updateComplete;
assert.isFalse(reviewStub.called);
});
- test('_renderInOrder logged in', async () => {
- element._loggedIn = true;
- const reviewStub = sinon.stub(element, '_reviewFile');
+ test('renderInOrder logged in', async () => {
+ element.loggedIn = true;
+ const reviewStub = sinon.stub(element, 'reviewFile');
let callCount = 0;
// Have to type as any because the type is 'GrDiffHost'
// which would require stubbing so many different
@@ -1427,15 +1443,28 @@
},
},
] as any;
- element._renderInOrder([{path: 'p2'}], diffs, 1);
- await flush();
+ element.renderInOrder([{path: 'p2'}], diffs);
+ await element.updateComplete;
assert.equal(reviewStub.callCount, 1);
});
- test('_renderInOrder respects diffPrefs.manual_review', async () => {
- element._loggedIn = true;
- element.diffPrefs = {manual_review: true} as DiffPreferencesInfo;
- const reviewStub = sinon.stub(element, '_reviewFile');
+ test('renderInOrder respects diffPrefs.manual_review', async () => {
+ element.loggedIn = true;
+ element.diffPrefs = {
+ context: 10,
+ tab_size: 8,
+ font_size: 12,
+ line_length: 100,
+ cursor_blink_rate: 0,
+ line_wrapping: false,
+ show_line_endings: true,
+ show_tabs: true,
+ show_whitespace_errors: true,
+ syntax_highlighting: true,
+ ignore_whitespace: 'IGNORE_NONE',
+ manual_review: true,
+ };
+ const reviewStub = sinon.stub(element, 'reviewFile');
// Have to type as any because the type is 'GrDiffHost'
// which would require stubbing so many different
// methods / properties that it isn't worth it.
@@ -1450,17 +1479,19 @@
},
] as any;
- element._renderInOrder([{path: 'p'}], diffs, 1);
- await flush();
+ element.renderInOrder([{path: 'p'}], diffs);
+ await element.updateComplete;
assert.isFalse(reviewStub.called);
delete element.diffPrefs.manual_review;
- element._renderInOrder([{path: 'p'}], diffs, 1);
+ element.renderInOrder([{path: 'p'}], diffs);
+ await element.updateComplete;
+ // Wait for renderInOrder to finish
await flush();
assert.isTrue(reviewStub.called);
assert.isTrue(reviewStub.calledWithExactly('p', true));
});
- test('_loadingChanged fired from reload in debouncer', async () => {
+ test('loadingChanged fired from reload in debouncer', async () => {
const reloadBlocker = mockPromise();
stubRestApi('getChangeOrEditFiles').resolves({
'foo.bar': {size: 0, size_delta: 0},
@@ -1471,14 +1502,17 @@
element.changeNum = 123 as NumericChangeId;
element.patchRange = {patchNum: 12 as RevisionPatchSetNum} as PatchRange;
- element._filesByPath = {'foo.bar': {size: 0, size_delta: 0}};
+ element.filesByPath = {'foo.bar': {size: 0, size_delta: 0}};
element.change = {
...createParsedChange(),
_number: 123 as NumericChangeId,
};
+ await element.updateComplete;
+ await flush();
const reloaded = element.reload();
- assert.isTrue(element._loading);
+ await element.updateComplete;
+ assert.isTrue(element.loading);
assert.isFalse(element.classList.contains('loading'));
element.loadingTask!.flush();
assert.isTrue(element.classList.contains('loading'));
@@ -1486,15 +1520,14 @@
reloadBlocker.resolve();
await reloaded;
- assert.isFalse(element._loading);
+ assert.isFalse(element.loading);
element.loadingTask!.flush();
assert.isFalse(element.classList.contains('loading'));
});
- test('_loadingChanged does not set class when there are no files', () => {
+ test('loadingChanged does not set class when there are no files', () => {
const reloadBlocker = mockPromise();
stubRestApi('getLoggedIn').returns(reloadBlocker.then(() => false));
- sinon.stub(element, '_getReviewedFiles').resolves([]);
element.changeNum = 123 as NumericChangeId;
element.patchRange = {patchNum: 12 as RevisionPatchSetNum} as PatchRange;
element.change = {
@@ -1503,7 +1536,7 @@
};
element.reload();
- assert.isTrue(element._loading);
+ assert.isTrue(element.loading);
element.loadingTask!.flush();
@@ -1545,12 +1578,13 @@
basePatchNum: 'PARENT' as BasePatchSetNum,
patchNum: 1 as RevisionPatchSetNum,
};
+ await element.updateComplete;
await flush();
});
test('displays cleanly merged file count', async () => {
await element.reload();
- await flush();
+ await element.updateComplete;
const message = queryAndAssert<HTMLSpanElement>(
element,
@@ -1571,7 +1605,7 @@
'anotherCleanlyMergedFile.js': {size: 0, size_delta: 0},
});
await element.reload();
- await flush();
+ await element.updateComplete;
const message = queryAndAssert(
element,
@@ -1582,7 +1616,7 @@
test('displays button for navigating to parent 1 base', async () => {
await element.reload();
- await flush();
+ await element.updateComplete;
queryAndAssert(element, '.showParentButton');
});
@@ -1602,9 +1636,9 @@
},
});
await element.reload();
- await flush();
+ await element.updateComplete;
- assert.deepEqual(element._cleanlyMergedOldPaths, [
+ assert.deepEqual(element.cleanlyMergedOldPaths, [
'cleanlyMergedFileOldName.js',
]);
});
@@ -1615,7 +1649,7 @@
patchNum: 2 as RevisionPatchSetNum,
};
await element.reload();
- await flush();
+ await element.updateComplete;
assert.notOk(query(element, '.cleanlyMergedText'));
assert.notOk(query(element, '.showParentButton'));
@@ -1627,7 +1661,7 @@
patchNum: EditPatchSetNum,
};
await element.reload();
- await flush();
+ await element.updateComplete;
assert.notOk(query(element, '.cleanlyMergedText'));
assert.notOk(query(element, '.showParentButton'));
@@ -1640,22 +1674,18 @@
const diffStub = sinon
.stub(GerritNav, 'getUrlForDiff')
.returns('/c/gerrit/+/1/1/index.php');
- const change = {
+ element.change = {
...createParsedChange(),
_number: 1 as NumericChangeId,
project: 'gerrit' as RepoName,
};
+ element.patchRange = {
+ basePatchNum: ParentPatchSetNum,
+ patchNum: 1 as RevisionPatchSetNum,
+ };
const path = 'index.php';
- assert.equal(
- element._computeDiffURL(
- change,
- undefined,
- 1 as RevisionPatchSetNum,
- path,
- false
- ),
- '/c/gerrit/+/1/1/index.php'
- );
+ element.editMode = false;
+ assert.equal(element.computeDiffURL(path), '/c/gerrit/+/1/1/index.php');
diffStub.restore();
});
@@ -1663,22 +1693,18 @@
const diffStub = sinon
.stub(GerritNav, 'getUrlForDiff')
.returns('/c/gerrit/+/1/1//COMMIT_MSG');
- const change = {
+ element.change = {
...createParsedChange(),
_number: 1 as NumericChangeId,
project: 'gerrit' as RepoName,
};
+ element.patchRange = {
+ basePatchNum: ParentPatchSetNum,
+ patchNum: 1 as RevisionPatchSetNum,
+ };
+ element.editMode = false;
const path = '/COMMIT_MSG';
- assert.equal(
- element._computeDiffURL(
- change,
- undefined,
- 1 as RevisionPatchSetNum,
- path,
- false
- ),
- '/c/gerrit/+/1/1//COMMIT_MSG'
- );
+ assert.equal(element.computeDiffURL(path), '/c/gerrit/+/1/1//COMMIT_MSG');
diffStub.restore();
});
@@ -1686,20 +1712,19 @@
const editStub = sinon
.stub(GerritNav, 'getEditUrlForDiff')
.returns('/c/gerrit/+/1/edit/index.php,edit');
- const change = {
+ element.change = {
...createParsedChange(),
_number: 1 as NumericChangeId,
project: 'gerrit' as RepoName,
};
+ element.patchRange = {
+ basePatchNum: ParentPatchSetNum,
+ patchNum: 1 as RevisionPatchSetNum,
+ };
+ element.editMode = true;
const path = 'index.php';
assert.equal(
- element._computeDiffURL(
- change,
- undefined,
- 1 as RevisionPatchSetNum,
- path,
- true
- ),
+ element.computeDiffURL(path),
'/c/gerrit/+/1/edit/index.php,edit'
);
editStub.restore();
@@ -1709,20 +1734,19 @@
const editStub = sinon
.stub(GerritNav, 'getEditUrlForDiff')
.returns('/c/gerrit/+/1/edit//COMMIT_MSG,edit');
- const change = {
+ element.change = {
...createParsedChange(),
_number: 1 as NumericChangeId,
project: 'gerrit' as RepoName,
};
+ element.patchRange = {
+ basePatchNum: ParentPatchSetNum,
+ patchNum: 1 as RevisionPatchSetNum,
+ };
+ element.editMode = true;
const path = '/COMMIT_MSG';
assert.equal(
- element._computeDiffURL(
- change,
- undefined,
- 1 as RevisionPatchSetNum,
- path,
- true
- ),
+ element.computeDiffURL(path),
'/c/gerrit/+/1/edit//COMMIT_MSG,edit'
);
editStub.restore();
@@ -1730,7 +1754,7 @@
});
suite('size bars', () => {
- test('_computeSizeBarLayout', () => {
+ test('computeSizeBarLayout', async () => {
const defaultSizeBarLayout = {
maxInserted: 0,
maxDeleted: 0,
@@ -1739,37 +1763,39 @@
deletionOffset: 0,
};
- assert.deepEqual(
- element._computeSizeBarLayout(undefined),
- defaultSizeBarLayout
- );
- assert.deepEqual(
- element._computeSizeBarLayout(
- {} as PolymerDeepPropertyChange<
- NormalizedFileInfo[],
- NormalizedFileInfo[]
- >
- ),
- defaultSizeBarLayout
- );
- assert.deepEqual(
- element._computeSizeBarLayout({base: []} as any),
- defaultSizeBarLayout
- );
+ element.files = [];
+ await element.updateComplete;
+ assert.deepEqual(element.computeSizeBarLayout(), defaultSizeBarLayout);
- const files = [
- {__path: '/COMMIT_MSG', lines_inserted: 10000},
- {__path: 'foo', lines_inserted: 4, lines_deleted: 10},
- {__path: 'bar', lines_inserted: 5, lines_deleted: 8},
+ element.files = [
+ {
+ __path: '/COMMIT_MSG',
+ lines_inserted: 10000,
+ size_delta: 10000,
+ size: 10000,
+ },
+ {
+ __path: 'foo',
+ lines_inserted: 4,
+ lines_deleted: 10,
+ size_delta: 14,
+ size: 20,
+ },
+ {
+ __path: 'bar',
+ lines_inserted: 5,
+ lines_deleted: 8,
+ size_delta: 13,
+ size: 21,
+ },
];
- const layout = element._computeSizeBarLayout({
- base: files,
- } as PolymerDeepPropertyChange<NormalizedFileInfo[], NormalizedFileInfo[]>);
+ await element.updateComplete;
+ const layout = element.computeSizeBarLayout();
assert.equal(layout.maxInserted, 5);
assert.equal(layout.maxDeleted, 10);
});
- test('_computeBarAdditionWidth', () => {
+ test('computeBarAdditionWidth', () => {
const file = {
__path: 'foo/bar.baz',
lines_inserted: 5,
@@ -1787,27 +1813,27 @@
// Uses half the space when file is half the largest addition and there
// are no deletions.
- assert.equal(element._computeBarAdditionWidth(file, stats), 30);
+ assert.equal(element.computeBarAdditionWidth(file, stats), 30);
// If there are no insertions, there is no width.
stats.maxInserted = 0;
- assert.equal(element._computeBarAdditionWidth(file, stats), 0);
+ assert.equal(element.computeBarAdditionWidth(file, stats), 0);
// If the insertions is not present on the file, there is no width.
stats.maxInserted = 10;
file.lines_inserted = 0;
- assert.equal(element._computeBarAdditionWidth(file, stats), 0);
+ assert.equal(element.computeBarAdditionWidth(file, stats), 0);
// If the file is a commit message, returns zero.
file.lines_inserted = 5;
file.__path = '/COMMIT_MSG';
- assert.equal(element._computeBarAdditionWidth(file, stats), 0);
+ assert.equal(element.computeBarAdditionWidth(file, stats), 0);
// Width bottoms-out at the minimum width.
file.__path = 'stuff.txt';
file.lines_inserted = 1;
stats.maxInserted = 1000000;
- assert.equal(element._computeBarAdditionWidth(file, stats), 1.5);
+ assert.equal(element.computeBarAdditionWidth(file, stats), 1.5);
});
test('_computeBarAdditionX', () => {
@@ -1825,10 +1851,10 @@
maxDeletionWidth: 0,
deletionOffset: 60,
};
- assert.equal(element._computeBarAdditionX(file, stats), 30);
+ assert.equal(element.computeBarAdditionX(file, stats), 30);
});
- test('_computeBarDeletionWidth', () => {
+ test('computeBarDeletionWidth', () => {
const file = {
__path: 'foo/bar.baz',
lines_inserted: 0,
@@ -1846,42 +1872,41 @@
// Uses a quarter the space when file is half the largest deletions and
// there are equal additions.
- assert.equal(element._computeBarDeletionWidth(file, stats), 15);
+ assert.equal(element.computeBarDeletionWidth(file, stats), 15);
// If there are no deletions, there is no width.
stats.maxDeleted = 0;
- assert.equal(element._computeBarDeletionWidth(file, stats), 0);
+ assert.equal(element.computeBarDeletionWidth(file, stats), 0);
// If the deletions is not present on the file, there is no width.
stats.maxDeleted = 10;
file.lines_deleted = 0;
- assert.equal(element._computeBarDeletionWidth(file, stats), 0);
+ assert.equal(element.computeBarDeletionWidth(file, stats), 0);
// If the file is a commit message, returns zero.
file.lines_deleted = 5;
file.__path = '/COMMIT_MSG';
- assert.equal(element._computeBarDeletionWidth(file, stats), 0);
+ assert.equal(element.computeBarDeletionWidth(file, stats), 0);
// Width bottoms-out at the minimum width.
file.__path = 'stuff.txt';
file.lines_deleted = 1;
stats.maxDeleted = 1000000;
- assert.equal(element._computeBarDeletionWidth(file, stats), 1.5);
+ assert.equal(element.computeBarDeletionWidth(file, stats), 1.5);
});
test('_computeSizeBarsClass', () => {
+ element.showSizeBars = false;
assert.equal(
- element._computeSizeBarsClass(false, 'foo/bar.baz'),
+ element.computeSizeBarsClass('foo/bar.baz'),
'sizeBars hide'
);
+ element.showSizeBars = true;
assert.equal(
- element._computeSizeBarsClass(true, '/COMMIT_MSG'),
+ element.computeSizeBarsClass('/COMMIT_MSG'),
'sizeBars invisible'
);
- assert.equal(
- element._computeSizeBarsClass(true, 'foo/bar.baz'),
- 'sizeBars '
- );
+ assert.equal(element.computeSizeBarsClass('foo/bar.baz'), 'sizeBars ');
});
});
@@ -1949,7 +1974,7 @@
await setupDiff(diffs[i]);
}
- element._updateDiffCursor();
+ element.updateDiffCursor();
element.diffCursor.handleDiffUpdate();
return diffs;
}
@@ -1965,21 +1990,31 @@
stub('gr-diff-host', 'reload').callsFake(() => Promise.resolve());
stub('gr-diff-host', 'prefetchDiff').callsFake(() => {});
- // Element must be wrapped in an element with direct access to the
- // comment API.
element = basicFixture.instantiate();
- element.diffPrefs = {} as DiffPreferencesInfo;
+ element.diffPrefs = {
+ context: 10,
+ tab_size: 8,
+ font_size: 12,
+ line_length: 100,
+ cursor_blink_rate: 0,
+ line_wrapping: false,
+ show_line_endings: true,
+ show_tabs: true,
+ show_whitespace_errors: true,
+ syntax_highlighting: true,
+ ignore_whitespace: 'IGNORE_NONE',
+ };
element.change = {
...createParsedChange(),
_number: 42 as NumericChangeId,
project: 'testRepo' as RepoName,
};
- reviewFileStub = sinon.stub(element, '_reviewFile');
+ reviewFileStub = sinon.stub(element, 'reviewFile');
- element._loading = false;
+ element.loading = false;
element.numFilesShown = 75;
element.selectedIndex = 0;
- element._filesByPath = {
+ element.filesByPath = {
'/COMMIT_MSG': {lines_inserted: 9, size: 0, size_delta: 0},
'file_added_in_rev2.txt': {
lines_inserted: 1,
@@ -1995,7 +2030,7 @@
},
};
element.reviewed = ['/COMMIT_MSG', 'myfile.txt'];
- element._loggedIn = true;
+ element.loggedIn = true;
element.changeNum = 42 as NumericChangeId;
element.patchRange = {
basePatchNum: 'PARENT' as BasePatchSetNum,
@@ -2004,12 +2039,13 @@
sinon
.stub(window, 'fetch')
.callsFake(() => Promise.resolve(new Response()));
- await flush();
+ await element.updateComplete;
});
test('cursor with individually opened files', async () => {
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
- await flush();
+ await element.updateComplete;
+
let diffs = await renderAndGetNewDiffs(0);
const diffStops = diffs[0].getCursorStops();
@@ -2025,7 +2061,7 @@
MockInteractions.tap(
queryAll(diffStops[10] as HTMLElement, '.contentText')[0]
);
- await flush();
+ await element.updateComplete;
assert.isTrue(
(diffStops[10] as HTMLElement).classList.contains('target-row')
);
@@ -2033,7 +2069,7 @@
// Keyboard shortcuts are still moving the file cursor, not the diff
// cursor.
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
- await flush();
+ await element.updateComplete;
assert.isTrue(
(diffStops[10] as HTMLElement).classList.contains('target-row')
);
@@ -2045,7 +2081,7 @@
assert.equal(element.fileCursor.index, 1);
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i');
- await flush();
+ await element.updateComplete;
diffs = await renderAndGetNewDiffs(1);
// Two diffs should be rendered.
@@ -2064,7 +2100,7 @@
test('cursor with toggle all files', async () => {
MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'I');
- await flush();
+ await element.updateComplete;
const diffs = await renderAndGetNewDiffs(0);
const diffStops = diffs[0].getCursorStops();
@@ -2081,7 +2117,7 @@
MockInteractions.tap(
queryAll(diffStops[10] as HTMLElement, '.contentText')[0]
);
- await flush();
+ await element.updateComplete;
assert.isTrue(
(diffStops[10] as HTMLElement).classList.contains('target-row')
);
@@ -2089,7 +2125,7 @@
// Keyboard shortcuts are still moving the file cursor, not the diff
// cursor.
MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j');
- await flush();
+ await element.updateComplete;
assert.isFalse(
(diffStops[10] as HTMLElement).classList.contains('target-row')
);
@@ -2107,7 +2143,7 @@
let fileRows: NodeListOf<HTMLDivElement>;
setup(() => {
- sinon.stub(element, '_renderInOrder').returns(Promise.resolve());
+ sinon.stub(element, 'renderInOrder').returns(Promise.resolve());
nextCommentStub = sinon.stub(
element.diffCursor,
'moveToNextCommentThread'
@@ -2116,72 +2152,77 @@
fileRows = queryAll<HTMLDivElement>(element, '.row:not(.header-row)');
});
- test('n key with some files expanded', async () => {
+ test('correct number of files expanded', async () => {
MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'i');
- await flush();
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.SOME);
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+ await element.updateComplete;
assert.isTrue(nextChunkStub.calledOnce);
});
test('N key with some files expanded', async () => {
MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'i');
- await flush();
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.SOME);
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'N');
+ await element.updateComplete;
assert.isTrue(nextCommentStub.calledOnce);
});
test('n key with all files expanded', async () => {
MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'I');
- await flush();
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.ALL);
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
+ await element.updateComplete;
assert.isTrue(nextChunkStub.calledOnce);
});
test('N key with all files expanded', async () => {
MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'I');
- await flush();
+ await element.updateComplete;
assert.equal(element.filesExpanded, FilesExpandedState.ALL);
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'N');
+ await element.updateComplete;
assert.isTrue(nextCommentStub.called);
});
});
- test('_openSelectedFile behavior', async () => {
- const _filesByPath = element._filesByPath;
- element.set('_filesByPath', {});
+ test('openSelectedFile behavior', async () => {
+ const filesByPath = element.filesByPath;
+ element.filesByPath = {};
+ await element.updateComplete;
const navStub = sinon.stub(GerritNav, 'navigateToDiff');
// Noop when there are no files.
- element._openSelectedFile();
+ element.openSelectedFile();
assert.isFalse(navStub.called);
- element.set('_filesByPath', _filesByPath);
- await flush();
+ element.filesByPath = filesByPath;
+ await element.updateComplete;
// Navigates when a file is selected.
- element._openSelectedFile();
+ element.openSelectedFile();
assert.isTrue(navStub.called);
});
- test('_displayLine', () => {
+ test('displayLine', () => {
element.filesExpanded = FilesExpandedState.ALL;
- element._displayLine = false;
- element._handleCursorNext(new KeyboardEvent('keydown'));
- assert.isTrue(element._displayLine);
+ element.displayLine = false;
+ element.handleCursorNext(new KeyboardEvent('keydown'));
+ assert.isTrue(element.displayLine);
- element._displayLine = false;
- element._handleCursorPrev(new KeyboardEvent('keydown'));
- assert.isTrue(element._displayLine);
+ element.displayLine = false;
+ element.handleCursorPrev(new KeyboardEvent('keydown'));
+ assert.isTrue(element.displayLine);
- element._displayLine = true;
- element._handleEscKey();
- assert.isFalse(element._displayLine);
+ element.displayLine = true;
+ element.handleEscKey();
+ assert.isFalse(element.displayLine);
});
suite('editMode behavior', () => {
@@ -2194,22 +2235,11 @@
assert.isTrue(saveReviewStub.calledOnce);
element.editMode = true;
- await flush();
+ await element.updateComplete;
MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
assert.isTrue(saveReviewStub.calledOnce);
});
-
- test('_getReviewedFiles does not call API', () => {
- const apiSpy = spyRestApi('getReviewedFiles');
- element.editMode = true;
- return element
- ._getReviewedFiles(0 as NumericChangeId, {patchNum: 0} as PatchRange)
- .then(files => {
- assert.equal(files!.length, 0);
- assert.isFalse(apiSpy.called);
- });
- });
});
test('editing actions', async () => {
@@ -2219,7 +2249,7 @@
);
element.editMode = true;
- await flush();
+ await element.updateComplete;
// Commit message should not have edit controls.
const editControls = Array.from(
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index df2ba28..9e4bf3f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -319,13 +319,6 @@
);
}
- override ready() {
- super.ready();
- if (this._canReload()) {
- this.reload();
- }
- }
-
override connectedCallback() {
super.connectedCallback();
this.subscriptions.push(
@@ -394,7 +387,6 @@
// assets in parallel.
const layerPromise = this.initLayers();
const diff = await this._getDiff();
- this.subscribeToChecks();
this._loadedWhitespaceLevel = whitespaceLevel;
this._reportDiff(diff);
@@ -412,8 +404,10 @@
this.reporting.timeEnd(Timing.DIFF_LOAD, this.timingDetails());
this.reporting.time(Timing.DIFF_CONTENT);
+
const syntaxLayerPromise = this.syntaxLayer.process(diff);
await waitForEventOnce(this, 'render');
+ this.subscribeToChecks();
this.reporting.timeEnd(Timing.DIFF_CONTENT, this.timingDetails());
if (shouldReportMetric) {
@@ -756,12 +750,6 @@
return this.restApiService.getLoggedIn();
}
- _canReload() {
- return (
- !!this.changeNum && !!this.patchRange && !!this.path && !this.noAutoRender
- );
- }
-
// TODO(milutin): Use rest-api with fetchCacheURL instead of this.
prefetchDiff() {
if (
@@ -1166,7 +1154,9 @@
if (prefsChangeRecord === undefined) return;
if (prefsChangeRecord.path !== 'prefs.syntax_highlighting') return;
- if (!noRenderOnPrefsChange) this.reload();
+ if (!noRenderOnPrefsChange) {
+ this.reload();
+ }
}
_computeParentIndex(
diff --git a/polygerrit-ui/app/elements/lit/incremental-repeat.ts b/polygerrit-ui/app/elements/lit/incremental-repeat.ts
new file mode 100644
index 0000000..cf14cc1
--- /dev/null
+++ b/polygerrit-ui/app/elements/lit/incremental-repeat.ts
@@ -0,0 +1,109 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {directive, AsyncDirective} from 'lit/async-directive.js';
+import {DirectiveParameters, ChildPart} from 'lit/directive.js';
+import {
+ insertPart,
+ setChildPartValue,
+ removePart,
+} from 'lit/directive-helpers.js';
+
+interface RepeatOptions<T> {
+ values: T[];
+ mapFn?: (val: T, idx: number) => unknown;
+ initialCount: number;
+ targetFrameRate?: number;
+ startAt?: number;
+ // TODO: targetFramerate
+}
+
+interface RepeatState<T> {
+ values: T[];
+ mapFn?: (val: T, idx: number) => unknown;
+ startAt: number;
+ incrementAmount: number;
+ lastRenderedAt: number;
+ targetFrameRate: number;
+}
+
+class IncrementalRepeat<T> extends AsyncDirective {
+ private parts: ChildPart[] = [];
+
+ private part!: ChildPart;
+
+ private state!: RepeatState<T>;
+
+ render(options: RepeatOptions<T>) {
+ const values = options.values.slice(
+ options.startAt ?? 0,
+ (options.startAt ?? 0) + options.initialCount
+ );
+ if (options.mapFn) {
+ return values.map(options.mapFn);
+ }
+ return values;
+ }
+
+ override update(part: ChildPart, [options]: DirectiveParameters<this>) {
+ if (
+ options.values !== this.state?.values ||
+ options.mapFn !== this.state?.mapFn
+ ) {
+ if (this.nextScheduledFrameWork !== undefined)
+ cancelAnimationFrame(this.nextScheduledFrameWork);
+ this.nextScheduledFrameWork = requestAnimationFrame(
+ this.animationFrameHandler
+ );
+ this.part = part;
+ for (let i = 0; i < this.parts.length; i++) {
+ removePart(this.parts[i]);
+ }
+ this.parts = [];
+ this.state = {
+ values: options.values,
+ mapFn: options.mapFn,
+ startAt: options.initialCount,
+ incrementAmount: options.initialCount,
+ lastRenderedAt: performance.now(),
+ targetFrameRate: options.targetFrameRate ?? 30,
+ };
+ }
+ return this.render(options);
+ }
+
+ private nextScheduledFrameWork: number | undefined;
+
+ private animationFrameHandler = () => {
+ const now = performance.now();
+ const frameRate = 1000 / (now - this.state.lastRenderedAt);
+ if (frameRate < this.state.targetFrameRate) {
+ // https://en.wikipedia.org/wiki/Additive_increase/multiplicative_decrease
+ this.state.incrementAmount = Math.max(1, this.state.incrementAmount / 2);
+ } else {
+ this.state.incrementAmount++;
+ }
+ this.state.lastRenderedAt = now;
+ const part = insertPart(this.part);
+ this.parts.push(part);
+ setChildPartValue(
+ part,
+ this.render({
+ mapFn: this.state.mapFn,
+ values: this.state.values,
+ initialCount: this.state.incrementAmount,
+ startAt: this.state.startAt,
+ })
+ );
+ this.state.startAt += this.state.incrementAmount;
+ if (this.state.startAt < this.state.values.length) {
+ this.nextScheduledFrameWork = requestAnimationFrame(
+ this.animationFrameHandler
+ );
+ }
+ };
+}
+
+export const incrementalRepeat = directive(IncrementalRepeat);
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_html.ts
index 40d4e7f..a1082f9 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_html.ts
@@ -349,7 +349,7 @@
background-color: var(--dark-rebased-remove-highlight-color);
}
.dueToRebase .content.remove .contentText {
- background-color: var(--light-remove-add-highlight-color);
+ background-color: var(--light-rebased-remove-highlight-color);
}
/* dueToMove */
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index dc17ee6..97e0c01 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -372,9 +372,20 @@
/* diff colors */
--dark-add-highlight-color: #aaf2aa;
- --dark-rebased-add-highlight-color: #d7d7f9;
- --dark-rebased-remove-highlight-color: #f7e8b7;
+ --light-add-highlight-color: #d8fed8;
--dark-remove-highlight-color: #ffcdd2;
+ --light-remove-highlight-color: #ffebee;
+
+ --dark-rebased-add-highlight-color: #d7d7f9;
+ --light-rebased-add-highlight-color: #eef;
+ --dark-rebased-remove-highlight-color: #f7e8b7;
+ --light-rebased-remove-highlight-color: #fff8dc;
+
+ --diff-moved-in-background: var(--cyan-50);
+ --diff-moved-in-label-color: var(--cyan-900);
+ --diff-moved-out-background: var(--purple-50);
+ --diff-moved-out-label-color: var(--purple-900);
+
--diff-blank-background-color: var(--background-color-secondary);
--diff-context-control-background-color: #fff7d4;
--diff-context-control-border-color: #f6e6a5;
@@ -385,14 +396,6 @@
--diff-tab-indicator-color: var(--deemphasized-text-color);
--diff-trailing-whitespace-indicator: #ff9ad2;
--focused-line-outline-color: var(--blue-700);
- --light-add-highlight-color: #d8fed8;
- --light-rebased-add-highlight-color: #eef;
- --diff-moved-in-background: var(--cyan-50);
- --diff-moved-out-background: var(--purple-50);
- --diff-moved-in-label-color: var(--cyan-900);
- --diff-moved-out-label-color: var(--purple-900);
- --light-remove-add-highlight-color: #fff8dc;
- --light-remove-highlight-color: #ffebee;
--coverage-covered: #e0f2f1;
--coverage-not-covered: #ffd1a4;
--ranged-comment-hint-text-color: var(--orange-900);
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index 6f19924..17c967d 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -207,9 +207,20 @@
/* diff colors */
--dark-add-highlight-color: var(--green-tonal);
- --dark-rebased-add-highlight-color: rgba(11, 255, 155, 0.15);
- --dark-rebased-remove-highlight-color: rgba(255, 139, 6, 0.15);
+ --light-add-highlight-color: #182b1f;
--dark-remove-highlight-color: #62110f;
+ --light-remove-highlight-color: #320404;
+
+ --dark-rebased-add-highlight-color: rgba(11, 255, 155, 0.15);
+ --light-rebased-add-highlight-color: #487165;
+ --dark-rebased-remove-highlight-color: rgba(255, 139, 6, 0.15);
+ --light-rebased-remove-highlight-color: #2f3f2f;
+
+ --diff-moved-in-background: #1d4042;
+ --diff-moved-in-label-color: var(--cyan-50);
+ --diff-moved-out-background: #230e34;
+ --diff-moved-out-label-color: var(--purple-50);
+
--diff-blank-background-color: var(--background-color-secondary);
--diff-context-control-background-color: #333311;
--diff-context-control-border-color: var(--border-color);
@@ -220,14 +231,6 @@
--diff-tab-indicator-color: var(--deemphasized-text-color);
--diff-trailing-whitespace-indicator: #ff9ad2;
--focused-line-outline-color: var(--blue-200);
- --light-add-highlight-color: #182b1f;
- --light-rebased-add-highlight-color: #487165;
- --diff-moved-in-background: #1d4042;
- --diff-moved-out-background: #230e34;
- --diff-moved-in-label-color: var(--cyan-50);
- --diff-moved-out-label-color: var(--purple-50);
- --light-remove-add-highlight-color: #2f3f2f;
- --light-remove-highlight-color: #320404;
--coverage-covered: #112826;
--coverage-not-covered: #6b3600;
--ranged-comment-hint-text-color: var(--blue-50);
