Merge "Fix NPE in SubmitRequirementsAdapter"
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index 3aeeccb..f619c99 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -705,14 +705,18 @@
- Uploader:
+
The user that uploaded the commit as a patch set to Gerrit, e.g. the
-user that executed the `git push` command.
+user that executed the `git push` command. For commits that are created through
+an action in the web UI the uploader is the user that triggered the action (e.g.
+if a commit is created by clicking on the `REBASE` button, the user clicking on
+the button becomes the uploader of the newly created commit).
+
The uploader of the first patch set is the change owner.
+
The uploader of the latest patch set, the user that uploaded the
-current patch set, is relevant when [self approvals on labels are
-ignored](config-labels.html#label_ignoreSelfApproval), as in this case
-approvals from the uploader of the latest patch set are ignored.
+current patch set, is relevant when
+link:config-labels.html#label_ignoreSelfApproval[self approvals on labels are
+ignored], as in this case approvals from the uploader of the latest patch set
+are ignored.
- Change Owner:
+
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 36b3473..0318cd7 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -79,27 +79,19 @@
* `performance/operations`: Latency of performing operations
** `operation_name`:
The operation that was performed.
-** `change_identifier`:
- The ID of the change for which the operation was performed (format =
- '<project>~<numeric-change-id>').
-** `trace_id`:
- The ID of the trace if tracing was done.
-* `performance/operations_count`: Number of performed operations
-** `operation_name`:
- The operation that was performed.
-** `trace_id`:
- The ID of the trace if tracing was done.
** `request`:
The request for which the operation was performed (format = '<request-type>
<redacted-request-uri>').
-* `performance/plugin_operations_count`: Number of performed operations by
- plugin
-** `operation_name`:
- The operation that was performed.
** `plugin`:
The name of the plugin that performed the operation.
-** `trace_id`:
- The ID of the trace if tracing was done.
+* `performance/operations_count`: Number of performed operations
+** `operation_name`:
+ The operation that was performed.
+** `request`:
+ The request for which the operation was performed (format = '<request-type>
+ <redacted-request-uri>').
+** `plugin`:
+ The name of the plugin that performed the operation.
=== Pushes
diff --git a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
index e7354ab..16dca66 100644
--- a/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImpl.java
@@ -23,7 +23,6 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.gerrit.acceptance.testsuite.project.TestProjectCreation.Builder;
import com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.TestCapability;
import com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.TestLabelPermission;
import com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.TestPermission;
@@ -81,7 +80,7 @@
}
@Override
- public Builder newProject() {
+ public TestProjectCreation.Builder newProject() {
return TestProjectCreation.builder(this::createNewProject);
}
diff --git a/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java b/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java
index 3c20ff7..8f5af76 100644
--- a/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java
+++ b/java/com/google/gerrit/extensions/client/ProjectWatchInfo.java
@@ -54,6 +54,7 @@
}
@Override
+ @SuppressWarnings("OrphanedFormatString")
public String toString() {
StringBuilder b = new StringBuilder();
b.append(project);
diff --git a/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index 0a54448..1605397 100644
--- a/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -21,8 +21,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.httpd.WebSessionManager.Key;
-import com.google.gerrit.httpd.WebSessionManager.Val;
import com.google.gerrit.httpd.restapi.ParameterParser;
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser;
@@ -56,8 +54,8 @@
private final AccountCache byIdCache;
private Cookie outCookie;
- private Key key;
- private Val val;
+ private WebSessionManager.Key key;
+ private WebSessionManager.Val val;
private CurrentUser user;
protected CacheBasedWebSession(
@@ -101,7 +99,7 @@
}
private void authFromCookie(String cookie) {
- key = new Key(cookie);
+ key = new WebSessionManager.Key(cookie);
val = manager.get(key);
String token = request.getHeader(XsrfConstants.XSRF_HEADER_NAME);
if (val != null && token != null && token.equals(val.getAuth())) {
@@ -110,7 +108,7 @@
}
private void authFromQueryParameter(String accessToken) {
- key = new Key(accessToken);
+ key = new WebSessionManager.Key(accessToken);
val = manager.get(key);
if (val != null) {
okPaths.add(AccessPath.REST_API);
@@ -204,8 +202,8 @@
/** Set the user account for this current request only. */
@Override
public void setUserAccountId(Account.Id id) {
- key = new Key("id:" + id);
- val = new Val(id, 0, false, null, 0, null, null);
+ key = new WebSessionManager.Key("id:" + id);
+ val = new WebSessionManager.Val(id, 0, false, null, 0, null, null);
user = identified.runAs(id, user, PropertyMap.EMPTY);
}
diff --git a/java/com/google/gerrit/pgm/DeleteZombieDrafts.java b/java/com/google/gerrit/pgm/DeleteZombieDrafts.java
index c08e999..57f8394 100644
--- a/java/com/google/gerrit/pgm/DeleteZombieDrafts.java
+++ b/java/com/google/gerrit/pgm/DeleteZombieDrafts.java
@@ -21,7 +21,6 @@
import com.google.gerrit.server.config.GerritServerConfigModule;
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs;
-import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs.Factory;
import com.google.gerrit.server.schema.SchemaModule;
import com.google.gerrit.server.securestore.SecureStoreClassName;
import com.google.inject.AbstractModule;
@@ -54,7 +53,7 @@
mustHaveValidSite();
Injector sysInjector = getSysInjector();
DeleteZombieCommentsRefs cleanup =
- sysInjector.getInstance(Factory.class).create(cleanupPercentage);
+ sysInjector.getInstance(DeleteZombieCommentsRefs.Factory.class).create(cleanupPercentage);
cleanup.execute();
return 0;
}
diff --git a/java/com/google/gerrit/pgm/SetPasswd.java b/java/com/google/gerrit/pgm/SetPasswd.java
index c6ece21..c3f6a7b 100644
--- a/java/com/google/gerrit/pgm/SetPasswd.java
+++ b/java/com/google/gerrit/pgm/SetPasswd.java
@@ -16,13 +16,12 @@
import com.google.gerrit.pgm.init.api.ConsoleUI;
import com.google.gerrit.pgm.init.api.Section;
-import com.google.gerrit.pgm.init.api.Section.Factory;
import com.google.inject.Inject;
public class SetPasswd {
private ConsoleUI ui;
- private Factory sections;
+ private Section.Factory sections;
@Inject
public SetPasswd(ConsoleUI ui, Section.Factory sections) {
diff --git a/java/com/google/gerrit/pgm/init/SitePathInitializer.java b/java/com/google/gerrit/pgm/init/SitePathInitializer.java
index ddc4f79..236d185 100644
--- a/java/com/google/gerrit/pgm/init/SitePathInitializer.java
+++ b/java/com/google/gerrit/pgm/init/SitePathInitializer.java
@@ -27,7 +27,6 @@
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.pgm.init.api.InitStep;
import com.google.gerrit.pgm.init.api.Section;
-import com.google.gerrit.pgm.init.api.Section.Factory;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.mail.EmailModule;
import com.google.inject.Binding;
@@ -46,7 +45,7 @@
private final InitFlags flags;
private final SitePaths site;
private final List<InitStep> steps;
- private final Factory sectionFactory;
+ private final Section.Factory sectionFactory;
private final SecureStoreInitData secureStoreInitData;
@Inject
diff --git a/java/com/google/gerrit/server/PerformanceMetrics.java b/java/com/google/gerrit/server/PerformanceMetrics.java
index 85452ce..327da4d 100644
--- a/java/com/google/gerrit/server/PerformanceMetrics.java
+++ b/java/com/google/gerrit/server/PerformanceMetrics.java
@@ -32,12 +32,9 @@
public class PerformanceMetrics implements PerformanceLogger {
private static final String OPERATION_LATENCY_METRIC_NAME = "performance/operations";
private static final String OPERATION_COUNT_METRIC_NAME = "performance/operations_count";
- private static final String PLUGIN_OPERATION_COUNT_METRIC_NAME =
- "performance/plugin_operations_count";
public final Timer3<String, String, String> operationsLatency;
public final Counter3<String, String, String> operationsCounter;
- public final Counter3<String, String, String> pluginOperationsCounter;
@Inject
PerformanceMetrics(MetricMaker metricMaker) {
@@ -47,16 +44,6 @@
(metadataBuilder, fieldValue) -> metadataBuilder.operationName(fieldValue))
.description("The operation that was performed.")
.build();
- Field<String> changeIdentifierField =
- Field.ofString("change_identifier", (metadataBuilder, fieldValue) -> {})
- .description(
- "The ID of the change for which the operation was performed"
- + " (format = '<project>~<numeric-change-id>').")
- .build();
- Field<String> traceIdField =
- Field.ofString("trace_id", (metadataBuilder, fieldValue) -> {})
- .description("The ID of the trace if tracing was done.")
- .build();
Field<String> requestField =
Field.ofString("request", (metadataBuilder, fieldValue) -> {})
.description(
@@ -76,22 +63,15 @@
.setCumulative()
.setUnit(Description.Units.MILLISECONDS),
operationNameField,
- changeIdentifierField,
- traceIdField);
+ requestField,
+ pluginField);
this.operationsCounter =
metricMaker.newCounter(
OPERATION_COUNT_METRIC_NAME,
new Description("Number of performed operations").setRate(),
operationNameField,
- traceIdField,
- requestField);
- this.pluginOperationsCounter =
- metricMaker.newCounter(
- PLUGIN_OPERATION_COUNT_METRIC_NAME,
- new Description("Number of performed operations by plugin").setRate(),
- operationNameField,
- pluginField,
- traceIdField);
+ requestField,
+ pluginField);
}
@Override
@@ -110,28 +90,9 @@
return;
}
- String traceId = TraceContext.getTraceId().orElse("");
-
- operationsLatency.record(
- operation, formatChangeIdentifier(metadata), traceId, durationMs, TimeUnit.MILLISECONDS);
-
String requestTag = TraceContext.getTag(TraceRequestListener.TAG_REQUEST).orElse("");
- operationsCounter.increment(operation, traceId, requestTag);
-
- TraceContext.getPluginTag()
- .ifPresent(pluginName -> pluginOperationsCounter.increment(operation, pluginName, traceId));
- }
-
- private String formatChangeIdentifier(@Nullable Metadata metadata) {
- if (metadata == null
- || (!metadata.projectName().isPresent() && !metadata.changeId().isPresent())) {
- return "";
- }
-
- StringBuilder sb = new StringBuilder();
- sb.append(metadata.projectName().orElse("n/a"));
- sb.append('~');
- sb.append(metadata.changeId().map(String::valueOf).orElse("n/a"));
- return sb.toString();
+ String pluginTag = TraceContext.getPluginTag().orElse("");
+ operationsLatency.record(operation, requestTag, pluginTag, durationMs, TimeUnit.MILLISECONDS);
+ operationsCounter.increment(operation, requestTag, pluginTag);
}
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java b/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java
index ee42d67..502bab9 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdFactory.java
@@ -23,7 +23,6 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.server.account.HashedPassword;
-import com.google.gerrit.server.account.externalids.ExternalId.Key;
import java.util.Set;
import javax.inject.Inject;
import javax.inject.Singleton;
@@ -81,7 +80,7 @@
* @param accountId the ID of the account to which the external ID belongs
* @return the created external ID
*/
- public ExternalId create(Key key, Account.Id accountId) {
+ public ExternalId create(ExternalId.Key key, Account.Id accountId) {
return create(key, accountId, null, null);
}
@@ -95,7 +94,10 @@
* @return the created external ID
*/
public ExternalId create(
- Key key, Account.Id accountId, @Nullable String email, @Nullable String hashedPassword) {
+ ExternalId.Key key,
+ Account.Id accountId,
+ @Nullable String email,
+ @Nullable String hashedPassword) {
return create(
key, accountId, Strings.emptyToNull(email), Strings.emptyToNull(hashedPassword), null);
}
@@ -110,7 +112,10 @@
* @return the created external ID
*/
public ExternalId createWithPassword(
- Key key, Account.Id accountId, @Nullable String email, @Nullable String plainPassword) {
+ ExternalId.Key key,
+ Account.Id accountId,
+ @Nullable String email,
+ @Nullable String plainPassword) {
plainPassword = Strings.emptyToNull(plainPassword);
String hashedPassword =
plainPassword != null ? HashedPassword.fromPassword(plainPassword).encode() : null;
@@ -157,7 +162,8 @@
* @param email the email of the external ID, may be {@code null}
* @return the created external ID
*/
- public ExternalId createWithEmail(Key key, Account.Id accountId, @Nullable String email) {
+ public ExternalId createWithEmail(
+ ExternalId.Key key, Account.Id accountId, @Nullable String email) {
return create(key, accountId, Strings.emptyToNull(email), null);
}
@@ -188,7 +194,7 @@
* @return the created external ID
*/
public ExternalId create(
- Key key,
+ ExternalId.Key key,
Account.Id accountId,
@Nullable String email,
@Nullable String hashedPassword,
@@ -238,7 +244,7 @@
}
String externalIdKeyStr = Iterables.getOnlyElement(externalIdKeys);
- Key externalIdKey = externalIdKeyFactory.parse(externalIdKeyStr);
+ ExternalId.Key externalIdKey = externalIdKeyFactory.parse(externalIdKeyStr);
if (externalIdKey == null) {
throw invalidConfig(noteId, String.format("External ID %s is invalid", externalIdKeyStr));
}
diff --git a/java/com/google/gerrit/server/approval/ApprovalInference.java b/java/com/google/gerrit/server/approval/ApprovalInference.java
index 4cb080a..695997a 100644
--- a/java/com/google/gerrit/server/approval/ApprovalInference.java
+++ b/java/com/google/gerrit/server/approval/ApprovalInference.java
@@ -54,6 +54,7 @@
import java.util.Map;
import java.util.Optional;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.revwalk.RevWalk;
/**
@@ -134,8 +135,9 @@
PatchSet.Id psId,
ChangeKind kind,
LabelType type,
- @Nullable Map<String, FileDiffOutput> modifiedFiles,
- @Nullable Map<String, FileDiffOutput> modifiedFilesLastPatchset) {
+ @Nullable Map<String, FileDiffOutput> baseVsCurrentDiff,
+ @Nullable Map<String, FileDiffOutput> baseVsPriorDiff,
+ @Nullable Map<String, FileDiffOutput> priorVsCurrentDiff) {
int n = psa.key().patchSetId().get();
checkArgument(n != psId.get());
@@ -185,7 +187,8 @@
project.getName());
return true;
} else if (type.isCopyAllScoresIfListOfFilesDidNotChange()
- && listOfFilesUnchangedPredicate.match(modifiedFiles, modifiedFilesLastPatchset)) {
+ && listOfFilesUnchangedPredicate.match(
+ baseVsCurrentDiff, baseVsPriorDiff, priorVsCurrentDiff)) {
logger.atFine().log(
"approval %d on label %s of patch set %d of change %d can be copied"
+ " to patch set %d because the label has set "
@@ -402,8 +405,9 @@
priorPatchSet.getValue().id().changeId(),
changeKind);
- Map<String, FileDiffOutput> modifiedFiles = null;
- Map<String, FileDiffOutput> modifiedFilesLastPatchSet = null;
+ Map<String, FileDiffOutput> baseVsCurrent = null;
+ Map<String, FileDiffOutput> baseVsPrior = null;
+ Map<String, FileDiffOutput> priorVsCurrent = null;
LabelTypes labelTypes = project.getLabelTypes();
for (PatchSetApproval psa : priorApprovals) {
if (resultByUser.contains(psa.label(), psa.accountId())) {
@@ -411,11 +415,13 @@
}
Optional<LabelType> type = labelTypes.byLabel(psa.labelId());
// Only compute modified files if there is a relevant label, since this is expensive.
- if (modifiedFiles == null
+ if (baseVsCurrent == null
&& type.isPresent()
&& type.get().isCopyAllScoresIfListOfFilesDidNotChange()) {
- modifiedFiles = listModifiedFiles(project, patchSet);
- modifiedFilesLastPatchSet = listModifiedFiles(project, priorPatchSet.getValue());
+ baseVsCurrent = listModifiedFiles(project, patchSet);
+ baseVsPrior = listModifiedFiles(project, priorPatchSet.getValue());
+ priorVsCurrent =
+ listModifiedFiles(project, priorPatchSet.getValue().commitId(), patchSet.commitId());
}
if (!type.isPresent()) {
logger.atFine().log(
@@ -435,8 +441,9 @@
patchSet.id(),
changeKind,
type.get(),
- modifiedFiles,
- modifiedFilesLastPatchSet)
+ baseVsCurrent,
+ baseVsPrior,
+ priorVsCurrent)
&& !canCopyBasedOnCopyCondition(notes, psa, patchSet, type.get(), changeKind)) {
continue;
}
@@ -465,4 +472,21 @@
ex);
}
}
+
+ /**
+ * Gets the modified files between two commits corresponding to different patchsets of the same
+ * change.
+ */
+ private Map<String, FileDiffOutput> listModifiedFiles(
+ ProjectState project, ObjectId sourceCommit, ObjectId targetCommit) {
+ try {
+ return diffOperations.listModifiedFiles(project.getNameKey(), sourceCommit, targetCommit);
+ } catch (DiffNotAvailableException ex) {
+ throw new StorageException(
+ "failed to compute difference in files, so won't copy"
+ + " votes on labels even if list of files is the same and "
+ + "copyAllIfListOfFilesDidNotChange",
+ ex);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/fixes/testing/GitEditSubject.java b/java/com/google/gerrit/server/fixes/testing/GitEditSubject.java
index 53b88b1..d90618c 100644
--- a/java/com/google/gerrit/server/fixes/testing/GitEditSubject.java
+++ b/java/com/google/gerrit/server/fixes/testing/GitEditSubject.java
@@ -22,7 +22,6 @@
import com.google.gerrit.jgit.diff.ReplaceEdit;
import com.google.gerrit.truth.ListSubject;
import org.eclipse.jgit.diff.Edit;
-import org.eclipse.jgit.diff.Edit.Type;
public class GitEditSubject extends Subject {
@@ -49,32 +48,32 @@
check("endB").that(edit.getEndB()).isEqualTo(endB);
}
- public void hasType(Type type) {
+ public void hasType(Edit.Type type) {
isNotNull();
check("getType").that(edit.getType()).isEqualTo(type);
}
public void isInsert(int insertPos, int beginB, int insertedLength) {
isNotNull();
- hasType(Type.INSERT);
+ hasType(Edit.Type.INSERT);
hasRegions(insertPos, insertPos, beginB, beginB + insertedLength);
}
public void isDelete(int deletePos, int deletedLength, int posB) {
isNotNull();
- hasType(Type.DELETE);
+ hasType(Edit.Type.DELETE);
hasRegions(deletePos, deletePos + deletedLength, posB, posB);
}
public void isReplace(int originalPos, int originalLength, int newPos, int newLength) {
isNotNull();
- hasType(Type.REPLACE);
+ hasType(Edit.Type.REPLACE);
hasRegions(originalPos, originalPos + originalLength, newPos, newPos + newLength);
}
public void isEmpty() {
isNotNull();
- hasType(Type.EMPTY);
+ hasType(Edit.Type.EMPTY);
}
public ListSubject<GitEditSubject, Edit> internalEdits() {
diff --git a/java/com/google/gerrit/server/git/CommitUtil.java b/java/com/google/gerrit/server/git/CommitUtil.java
index 2259741..73378f6 100644
--- a/java/com/google/gerrit/server/git/CommitUtil.java
+++ b/java/com/google/gerrit/server/git/CommitUtil.java
@@ -20,7 +20,6 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Account.Id;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -278,7 +277,7 @@
ReviewerSet reviewerSet = approvalsUtil.getReviewers(notes);
- Set<Id> reviewers = new HashSet<>();
+ Set<Account.Id> reviewers = new HashSet<>();
reviewers.add(changeToRevert.getOwner());
reviewers.addAll(reviewerSet.byState(ReviewerStateInternal.REVIEWER));
reviewers.remove(user.getAccountId());
diff --git a/java/com/google/gerrit/server/git/validators/CommitValidators.java b/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 1cb0bea..056407e 100644
--- a/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -45,7 +45,6 @@
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ValidationError;
-import com.google.gerrit.server.git.validators.ValidationMessage.Type;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
@@ -320,7 +319,7 @@
+ "Hint: run\n"
+ " git commit --amend\n"
+ "and move 'Change-Id: Ixxx..' to the bottom on a separate line\n",
- Type.ERROR));
+ ValidationMessage.Type.ERROR));
throw new CommitValidationException(CHANGE_ID_ABOVE_FOOTER_MSG, messages);
}
if (projectState.is(BooleanProjectConfig.REQUIRE_CHANGE_ID)) {
@@ -360,7 +359,7 @@
+ "and then amend the commit:\n"
+ " git commit --amend --no-edit\n"
+ "Finally, push your changes again\n",
- Type.ERROR);
+ ValidationMessage.Type.ERROR);
}
private String getCommitMessageHookInstallationHint() {
@@ -873,7 +872,7 @@
throw new CommitValidationException(
"invalid account configuration",
errorMessages.stream()
- .map(m -> new CommitValidationMessage(m, Type.ERROR))
+ .map(m -> new CommitValidationMessage(m, ValidationMessage.Type.ERROR))
.collect(toList()));
}
} catch (IOException e) {
@@ -956,7 +955,7 @@
.append(urlFormatter.getSettingsUrl("EmailAddresses").get())
.append("\n\n");
}
- return new CommitValidationMessage(sb.toString(), Type.ERROR);
+ return new CommitValidationMessage(sb.toString(), ValidationMessage.Type.ERROR);
}
/**
@@ -977,6 +976,6 @@
}
private static void addError(String error, List<CommitValidationMessage> messages) {
- messages.add(new CommitValidationMessage(error, Type.ERROR));
+ messages.add(new CommitValidationMessage(error, ValidationMessage.Type.ERROR));
}
}
diff --git a/java/com/google/gerrit/server/notedb/CommitRewriter.java b/java/com/google/gerrit/server/notedb/CommitRewriter.java
index 3cbe546..e940b1e 100644
--- a/java/com/google/gerrit/server/notedb/CommitRewriter.java
+++ b/java/com/google/gerrit/server/notedb/CommitRewriter.java
@@ -543,12 +543,16 @@
Matcher assigneeDeletedMatcher = ASSIGNEE_DELETED_PATTERN.matcher(originalChangeMessage);
if (assigneeDeletedMatcher.matches()) {
if (!NON_REPLACE_ACCOUNT_PATTERN.matcher(assigneeDeletedMatcher.group(1)).matches()) {
+ Optional<String> assigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ oldAssignee,
+ getAccountInfoFromNameEmail(assigneeDeletedMatcher.group(1)));
+
return Optional.of(
- "Assignee deleted: "
- + getPossibleAccountReplacement(
- changeFixProgress,
- oldAssignee,
- ParsedAccountInfo.create(assigneeDeletedMatcher.group(1))));
+ assigneeReplacement.isPresent()
+ ? "Assignee deleted: " + assigneeReplacement.get()
+ : "Assignee was deleted.");
}
return Optional.empty();
}
@@ -556,12 +560,15 @@
Matcher assigneeAddedMatcher = ASSIGNEE_ADDED_PATTERN.matcher(originalChangeMessage);
if (assigneeAddedMatcher.matches()) {
if (!NON_REPLACE_ACCOUNT_PATTERN.matcher(assigneeAddedMatcher.group(1)).matches()) {
+ Optional<String> assigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ newAssignee,
+ getAccountInfoFromNameEmail(assigneeAddedMatcher.group(1)));
return Optional.of(
- "Assignee added: "
- + getPossibleAccountReplacement(
- changeFixProgress,
- newAssignee,
- ParsedAccountInfo.create(assigneeAddedMatcher.group(1))));
+ assigneeReplacement.isPresent()
+ ? "Assignee added: " + assigneeReplacement.get()
+ : "Assignee was added.");
}
return Optional.empty();
}
@@ -569,17 +576,22 @@
Matcher assigneeChangedMatcher = ASSIGNEE_CHANGED_PATTERN.matcher(originalChangeMessage);
if (assigneeChangedMatcher.matches()) {
if (!NON_REPLACE_ACCOUNT_PATTERN.matcher(assigneeChangedMatcher.group(1)).matches()) {
+ Optional<String> oldAssigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ oldAssignee,
+ getAccountInfoFromNameEmail(assigneeChangedMatcher.group(1)));
+ Optional<String> newAssigneeReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress,
+ newAssignee,
+ getAccountInfoFromNameEmail(assigneeChangedMatcher.group(2)));
return Optional.of(
- String.format(
- "Assignee changed from: %s to: %s",
- getPossibleAccountReplacement(
- changeFixProgress,
- oldAssignee,
- ParsedAccountInfo.create(assigneeChangedMatcher.group(1))),
- getPossibleAccountReplacement(
- changeFixProgress,
- newAssignee,
- ParsedAccountInfo.create(assigneeChangedMatcher.group(2)))));
+ oldAssigneeReplacement.isPresent() && newAssigneeReplacement.isPresent()
+ ? String.format(
+ "Assignee changed from: %s to: %s",
+ oldAssigneeReplacement.get(), newAssigneeReplacement.get())
+ : "Assignee was changed.");
}
return Optional.empty();
}
@@ -610,12 +622,15 @@
Matcher matcher = REMOVED_VOTE_PATTERN.matcher(originalChangeMessage);
if (matcher.matches() && !NON_REPLACE_ACCOUNT_PATTERN.matcher(matcher.group(2)).matches()) {
- return Optional.of(
- String.format(
- "Removed %s by %s",
- matcher.group(1),
- getPossibleAccountReplacement(
- changeFixProgress, reviewer, getAccountInfoFromNameEmail(matcher.group(2)))));
+ Optional<String> reviewerReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress, reviewer, getAccountInfoFromNameEmail(matcher.group(2)));
+ StringBuilder replacement = new StringBuilder();
+ replacement.append("Removed ").append(matcher.group(1));
+ if (reviewerReplacement.isPresent()) {
+ replacement.append(" by ").append(reviewerReplacement.get());
+ }
+ return Optional.of(replacement.toString());
}
return Optional.empty();
}
@@ -637,14 +652,14 @@
String replacementLine = lines[i];
if (matcher.matches() && !NON_REPLACE_ACCOUNT_PATTERN.matcher(matcher.group(2)).matches()) {
anyFixed = true;
- replacementLine =
- String.format(
- "* %s by %s\n",
- matcher.group(1),
- getPossibleAccountReplacement(
- changeFixProgress,
- Optional.empty(),
- getAccountInfoFromNameEmail(matcher.group(2))));
+ Optional<String> reviewerReplacement =
+ getPossibleAccountReplacement(
+ changeFixProgress, Optional.empty(), getAccountInfoFromNameEmail(matcher.group(2)));
+ replacementLine = "* " + matcher.group(1);
+ if (reviewerReplacement.isPresent()) {
+ replacementLine += " by " + reviewerReplacement.get();
+ }
+ replacementLine += "\n";
}
fixedLines.append(replacementLine);
}
@@ -708,11 +723,14 @@
StringBuffer sb = new StringBuffer();
while (onAddReviewerMatcher.find()) {
String reviewerName = normalizeOnCodeOwnerAddReviewerMatch(onAddReviewerMatcher.group(1));
- String replacementName =
+ Optional<String> replacementName =
getPossibleAccountReplacement(
changeFixProgress, Optional.empty(), ParsedAccountInfo.create(reviewerName));
onAddReviewerMatcher.appendReplacement(
- sb, replacementName + ", who was added as reviewer owns the following files");
+ sb,
+ replacementName.isPresent()
+ ? replacementName.get() + ", who was added as reviewer owns the following files"
+ : "Added reviewer owns the following files");
}
onAddReviewerMatcher.appendTail(sb);
sb.append("\n");
@@ -1071,19 +1089,20 @@
* <p>If {@code account} is known, replace with {@link AccountTemplateUtil#getAccountTemplate}.
* Otherwise, try to guess the correct replacement account for {@code accountName} among {@link
* ChangeFixProgress#parsedAccounts} that appeared in the change. If this fails {@link
- * #DEFAULT_ACCOUNT_REPLACEMENT} is applied.
+ * Optional#empty} is returned.
*
* @param changeFixProgress see {@link ChangeFixProgress}
* @param account account that should be used for replacement, if known
* @param accountInfo {@link ParsedAccountInfo} to replace.
- * @return replacement for {@code accountName}
+ * @return replacement for {@code accountName} or {@link Optional#empty}, if the replacement could
+ * not be determined.
*/
- private String getPossibleAccountReplacement(
+ private Optional<String> getPossibleAccountReplacement(
ChangeFixProgress changeFixProgress,
Optional<Account.Id> account,
ParsedAccountInfo accountInfo) {
if (account.isPresent()) {
- return AccountTemplateUtil.getAccountTemplate(account.get());
+ return Optional.of(AccountTemplateUtil.getAccountTemplate(account.get()));
}
// Retrieve reviewer accounts from cache and try to match by their name.
Map<Account.Id, AccountState> missingAccountStateReviewers =
@@ -1129,7 +1148,7 @@
e.getValue().get().account().getName(), accountInfo.name()))
.collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, e -> e.getValue().get()));
}
- String replacementName = DEFAULT_ACCOUNT_REPLACEMENT;
+ Optional<String> replacementName = Optional.empty();
if (possibleReplacements.isEmpty()) {
logger.atWarning().log(
"Fixing ref %s, could not find reviewer account matching name %s",
@@ -1140,8 +1159,9 @@
changeFixProgress.changeMetaRef, accountInfo);
} else {
replacementName =
- AccountTemplateUtil.getAccountTemplate(
- Iterables.getOnlyElement(possibleReplacements.keySet()));
+ Optional.of(
+ AccountTemplateUtil.getAccountTemplate(
+ Iterables.getOnlyElement(possibleReplacements.keySet())));
}
return replacementName;
}
diff --git a/java/com/google/gerrit/server/project/RefValidationHelper.java b/java/com/google/gerrit/server/project/RefValidationHelper.java
index 1912660..a6020a3 100644
--- a/java/com/google/gerrit/server/project/RefValidationHelper.java
+++ b/java/com/google/gerrit/server/project/RefValidationHelper.java
@@ -22,19 +22,20 @@
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.transport.ReceiveCommand.Type;
+import org.eclipse.jgit.transport.ReceiveCommand;
public class RefValidationHelper {
public interface Factory {
- RefValidationHelper create(Type operationType);
+ RefValidationHelper create(ReceiveCommand.Type operationType);
}
private final RefOperationValidators.Factory refValidatorsFactory;
- private final Type operationType;
+ private final ReceiveCommand.Type operationType;
@Inject
RefValidationHelper(
- RefOperationValidators.Factory refValidatorsFactory, @Assisted Type operationType) {
+ RefOperationValidators.Factory refValidatorsFactory,
+ @Assisted ReceiveCommand.Type operationType) {
this.refValidatorsFactory = refValidatorsFactory;
this.operationType = operationType;
}
diff --git a/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java b/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
index 55c27be..de7dd0a 100644
--- a/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
+++ b/java/com/google/gerrit/server/query/approval/ListOfFilesUnchangedPredicate.java
@@ -58,13 +58,18 @@
Integer parentNum =
isInitialCommit(ctx.changeNotes().getProjectName(), targetPatchSet.commitId()) ? 0 : 1;
try {
- Map<String, FileDiffOutput> modifiedTargetPatchSet =
+ Map<String, FileDiffOutput> baseVsCurrent =
diffOperations.listModifiedFilesAgainstParent(
ctx.changeNotes().getProjectName(), targetPatchSet.commitId(), parentNum);
- Map<String, FileDiffOutput> modifiedSourcePatchSet =
+ Map<String, FileDiffOutput> baseVsPrior =
diffOperations.listModifiedFilesAgainstParent(
ctx.changeNotes().getProjectName(), sourcePatchSet.commitId(), parentNum);
- return match(modifiedTargetPatchSet, modifiedSourcePatchSet);
+ Map<String, FileDiffOutput> priorVsCurrent =
+ diffOperations.listModifiedFiles(
+ ctx.changeNotes().getProjectName(),
+ sourcePatchSet.commitId(),
+ targetPatchSet.commitId());
+ return match(baseVsCurrent, baseVsPrior, priorVsCurrent);
} catch (DiffNotAvailableException ex) {
throw new StorageException(
"failed to compute difference in files, so won't copy"
@@ -79,16 +84,23 @@
* {@link ChangeType} matches for each modified file.
*/
public boolean match(
- Map<String, FileDiffOutput> modifiedFiles1, Map<String, FileDiffOutput> modifiedFiles2) {
+ Map<String, FileDiffOutput> baseVsCurrent,
+ Map<String, FileDiffOutput> baseVsPrior,
+ Map<String, FileDiffOutput> priorVsCurrent) {
Set<String> allFiles = new HashSet<>();
- allFiles.addAll(modifiedFiles1.keySet());
- allFiles.addAll(modifiedFiles2.keySet());
+ allFiles.addAll(baseVsCurrent.keySet());
+ allFiles.addAll(baseVsPrior.keySet());
for (String file : allFiles) {
if (Patch.isMagic(file)) {
continue;
}
- FileDiffOutput fileDiffOutput1 = modifiedFiles1.get(file);
- FileDiffOutput fileDiffOutput2 = modifiedFiles2.get(file);
+ FileDiffOutput fileDiffOutput1 = baseVsCurrent.get(file);
+ FileDiffOutput fileDiffOutput2 = baseVsPrior.get(file);
+ if (!priorVsCurrent.containsKey(file)) {
+ // If the file is not modified between prior and current patchsets, then scan safely skip
+ // it. The file might has been modified due to rebase.
+ continue;
+ }
if (fileDiffOutput1 == null || fileDiffOutput2 == null) {
return false;
}
diff --git a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
index d225798..1485a6e56 100644
--- a/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
+++ b/java/com/google/gerrit/server/restapi/account/DeleteDraftComments.java
@@ -47,7 +47,6 @@
import com.google.gerrit.server.restapi.change.CommentJson;
import com.google.gerrit.server.restapi.change.CommentJson.HumanCommentFormatter;
import com.google.gerrit.server.update.BatchUpdate;
-import com.google.gerrit.server.update.BatchUpdate.Factory;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.UpdateException;
@@ -80,7 +79,7 @@
@Inject
DeleteDraftComments(
Provider<CurrentUser> userProvider,
- Factory batchUpdateFactory,
+ BatchUpdate.Factory batchUpdateFactory,
Provider<ChangeQueryBuilder> queryBuilderProvider,
Provider<InternalChangeQuery> queryProvider,
ChangeData.Factory changeDataFactory,
diff --git a/java/com/google/gerrit/server/restapi/change/CommentPorter.java b/java/com/google/gerrit/server/restapi/change/CommentPorter.java
index 35d512a..1a4eb18 100644
--- a/java/com/google/gerrit/server/restapi/change/CommentPorter.java
+++ b/java/com/google/gerrit/server/restapi/change/CommentPorter.java
@@ -53,7 +53,6 @@
import com.google.inject.Singleton;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Stream;
@@ -220,7 +219,7 @@
Map<Short, List<HumanComment>> commentsPerSide =
comments.stream().collect(groupingBy(comment -> comment.side));
ImmutableList.Builder<HumanComment> portedComments = ImmutableList.builder();
- for (Entry<Short, List<HumanComment>> sideAndComments : commentsPerSide.entrySet()) {
+ for (Map.Entry<Short, List<HumanComment>> sideAndComments : commentsPerSide.entrySet()) {
portedComments.addAll(
portSamePatchsetAndSide(
project,
diff --git a/java/com/google/gerrit/server/restapi/change/GetFixPreview.java b/java/com/google/gerrit/server/restapi/change/GetFixPreview.java
index 99c8a0a..95e26a23 100644
--- a/java/com/google/gerrit/server/restapi/change/GetFixPreview.java
+++ b/java/com/google/gerrit/server/restapi/change/GetFixPreview.java
@@ -35,7 +35,6 @@
import com.google.gerrit.server.change.FixResource;
import com.google.gerrit.server.diff.DiffInfoCreator;
import com.google.gerrit.server.diff.DiffSide;
-import com.google.gerrit.server.diff.DiffSide.Type;
import com.google.gerrit.server.diff.DiffWebLinksProvider;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LargeObjectException;
@@ -121,7 +120,7 @@
DiffSide.create(
ps.getFileInfoA(),
MoreObjects.firstNonNull(ps.getOldName(), ps.getNewName()),
- Type.SIDE_A);
+ DiffSide.Type.SIDE_A);
DiffSide sideB = DiffSide.create(ps.getFileInfoB(), ps.getNewName(), DiffSide.Type.SIDE_B);
DiffInfoCreator diffInfoCreator =
@@ -142,7 +141,7 @@
}
@Override
- public ImmutableList<WebLinkInfo> getFileWebLinks(Type fileInfoType) {
+ public ImmutableList<WebLinkInfo> getFileWebLinks(DiffSide.Type fileInfoType) {
return ImmutableList.of();
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java b/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
index 0f49e63..11d8b19 100644
--- a/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
+++ b/java/com/google/gerrit/server/restapi/project/LabelDefinitionInputParser.java
@@ -28,7 +28,6 @@
import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
@@ -43,7 +42,7 @@
throws BadRequestException {
List<LabelValue> valueList = new ArrayList<>();
Set<Short> allValues = new HashSet<>();
- for (Entry<String, String> e : values.entrySet()) {
+ for (Map.Entry<String, String> e : values.entrySet()) {
short value;
try {
value = Shorts.checkedCast(PermissionRule.parseInt(e.getKey().trim()));
diff --git a/java/com/google/gerrit/server/restapi/project/PostLabels.java b/java/com/google/gerrit/server/restapi/project/PostLabels.java
index 0c42ab2..a56cfe6 100644
--- a/java/com/google/gerrit/server/restapi/project/PostLabels.java
+++ b/java/com/google/gerrit/server/restapi/project/PostLabels.java
@@ -37,7 +37,7 @@
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Map.Entry;
+import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
/** REST endpoint that allows to add, update and delete label definitions in a batch. */
@@ -118,7 +118,7 @@
}
if (input.update != null && !input.update.isEmpty()) {
- for (Entry<String, LabelDefinitionInput> e : input.update.entrySet()) {
+ for (Map.Entry<String, LabelDefinitionInput> e : input.update.entrySet()) {
LabelType labelType = config.getLabelSections().get(e.getKey().trim());
if (labelType == null) {
throw new UnprocessableEntityException(String.format("label %s not found", e.getKey()));
diff --git a/java/com/google/gerrit/server/restapi/project/PutConfig.java b/java/com/google/gerrit/server/restapi/project/PutConfig.java
index afa08cd..fa8638e 100644
--- a/java/com/google/gerrit/server/restapi/project/PutConfig.java
+++ b/java/com/google/gerrit/server/restapi/project/PutConfig.java
@@ -53,7 +53,6 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.project.ProjectState;
-import com.google.gerrit.server.project.ProjectState.Factory;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -91,7 +90,7 @@
@EnableSignedPush boolean serverEnableSignedPush,
Provider<MetaDataUpdate.User> metaDataUpdateFactory,
ProjectCache projectCache,
- Factory projectStateFactory,
+ ProjectState.Factory projectStateFactory,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
PluginConfigFactory cfgFactory,
AllProjectsName allProjects,
diff --git a/java/com/google/gerrit/server/rules/StoredValues.java b/java/com/google/gerrit/server/rules/StoredValues.java
index 1d10c1f..fd66a3a 100644
--- a/java/com/google/gerrit/server/rules/StoredValues.java
+++ b/java/com/google/gerrit/server/rules/StoredValues.java
@@ -14,8 +14,6 @@
package com.google.gerrit.server.rules;
-import static com.google.gerrit.server.rules.StoredValue.create;
-
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
@@ -44,11 +42,13 @@
import org.eclipse.jgit.revwalk.RevCommit;
public final class StoredValues {
- public static final StoredValue<Accounts> ACCOUNTS = create(Accounts.class);
- public static final StoredValue<AccountCache> ACCOUNT_CACHE = create(AccountCache.class);
- public static final StoredValue<Emails> EMAILS = create(Emails.class);
- public static final StoredValue<ChangeData> CHANGE_DATA = create(ChangeData.class);
- public static final StoredValue<ProjectState> PROJECT_STATE = create(ProjectState.class);
+ public static final StoredValue<Accounts> ACCOUNTS = StoredValue.create(Accounts.class);
+ public static final StoredValue<AccountCache> ACCOUNT_CACHE =
+ StoredValue.create(AccountCache.class);
+ public static final StoredValue<Emails> EMAILS = StoredValue.create(Emails.class);
+ public static final StoredValue<ChangeData> CHANGE_DATA = StoredValue.create(ChangeData.class);
+ public static final StoredValue<ProjectState> PROJECT_STATE =
+ StoredValue.create(ProjectState.class);
public static Change getChange(Prolog engine) throws SystemException {
ChangeData cd = CHANGE_DATA.get(engine);
diff --git a/java/com/google/gerrit/server/schema/AllUsersCreator.java b/java/com/google/gerrit/server/schema/AllUsersCreator.java
index 3588860..f2fe7f6 100644
--- a/java/com/google/gerrit/server/schema/AllUsersCreator.java
+++ b/java/com/google/gerrit/server/schema/AllUsersCreator.java
@@ -34,7 +34,6 @@
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.ProjectConfig;
-import com.google.gerrit.server.project.ProjectConfig.Factory;
import com.google.gerrit.server.project.RefPattern;
import com.google.inject.Inject;
import java.io.IOException;
@@ -62,7 +61,7 @@
AllUsersName allUsersName,
SystemGroupBackend systemGroupBackend,
@GerritPersonIdent PersonIdent serverUser,
- Factory projectConfigFactory) {
+ ProjectConfig.Factory projectConfigFactory) {
this.mgr = mgr;
this.allUsersName = allUsersName;
this.serverUser = serverUser;
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index f558d30..917e967 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -37,7 +37,6 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
-import com.google.gerrit.entities.PatchSet.Id;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.config.FactoryModule;
@@ -75,7 +74,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import java.util.Objects;
import java.util.Optional;
import java.util.TimeZone;
@@ -301,7 +299,7 @@
* commit in NoteDb by re-using the same ChangeUpdate instance. Will still be one commit per
* patch set.
*/
- private final ListMultimap<Id, ChangeUpdate> distinctUpdates;
+ private final ListMultimap<PatchSet.Id, ChangeUpdate> distinctUpdates;
private boolean deleted;
@@ -554,7 +552,7 @@
try {
logDebug("Executing updateRepo on %d ops", ops.size());
RepoContextImpl ctx = new RepoContextImpl();
- for (Entry<Change.Id, BatchUpdateOp> op : ops.entries()) {
+ for (Map.Entry<Change.Id, BatchUpdateOp> op : ops.entries()) {
try (TraceContext.TraceTimer ignored =
TraceContext.newTimer(
op.getClass().getSimpleName() + "#updateRepo",
diff --git a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
index cd9e876..3888679 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java
@@ -43,6 +43,7 @@
import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.LabelId;
import com.google.gerrit.entities.LabelType;
@@ -557,6 +558,46 @@
}
@Test
+ public void
+ stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModifiedDueToRebase_withoutCopyCondition()
+ throws Exception {
+ try (ProjectConfigUpdate u = updateProject(project)) {
+ u.getConfig()
+ .updateLabelType(
+ LabelId.CODE_REVIEW, b -> b.setCopyAllScoresIfListOfFilesDidNotChange(true));
+ u.save();
+ }
+ // Create two changes both with the same parent
+ PushOneCommit.Result r = createChange();
+ testRepo.reset("HEAD~1");
+ PushOneCommit.Result r2 = createChange();
+
+ // Modify f.txt in change 1. Approve and submit the first change
+ gApi.changes().id(r.getChangeId()).edit().modifyFile("f.txt", RawInputUtil.create("content"));
+ gApi.changes().id(r.getChangeId()).edit().publish();
+ RevisionApi revision = gApi.changes().id(r.getChangeId()).current();
+ revision.review(ReviewInput.approve().label(LabelId.VERIFIED, 1));
+ revision.submit();
+
+ // Add an approval whose score should be copied on change 2.
+ gApi.changes().id(r2.getChangeId()).current().review(ReviewInput.recommend());
+
+ // Rebase the second change. The rebase adds f1.txt.
+ gApi.changes().id(r2.getChangeId()).rebase();
+
+ // The code-review approval is copied for the second change between PS1 and PS2 since the only
+ // modified file is due to rebase.
+ List<PatchSetApproval> patchSetApprovals =
+ r2.getChange().notes().getApprovalsWithCopied().values().stream()
+ .sorted(comparing(a -> a.patchSetId().get()))
+ .collect(toImmutableList());
+ PatchSetApproval nonCopied = patchSetApprovals.get(0);
+ PatchSetApproval copied = patchSetApprovals.get(1);
+ assertCopied(nonCopied, /* psId= */ 1, LabelId.CODE_REVIEW, (short) 1, false);
+ assertCopied(copied, /* psId= */ 2, LabelId.CODE_REVIEW, (short) 1, true);
+ }
+
+ @Test
public void stickyWithCopyAllScoresIfListOfFilesDidNotChangeWhenFileIsModified_withCopyCondition()
throws Exception {
try (ProjectConfigUpdate u = updateProject(project)) {
@@ -1072,17 +1113,9 @@
.sorted(comparing(a -> a.patchSetId().get()))
.collect(toImmutableList());
PatchSetApproval nonCopied = patchSetApprovals.get(0);
-
- assertThat(nonCopied.patchSetId().get()).isEqualTo(1);
- assertThat(nonCopied.label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(nonCopied.value()).isEqualTo((short) 1);
- assertThat(nonCopied.copied()).isFalse();
-
PatchSetApproval copied = patchSetApprovals.get(1);
- assertThat(copied.patchSetId().get()).isEqualTo(2);
- assertThat(copied.label()).isEqualTo(LabelId.CODE_REVIEW);
- assertThat(copied.value()).isEqualTo((short) 1);
- assertThat(copied.copied()).isTrue();
+ assertCopied(nonCopied, 1, LabelId.CODE_REVIEW, (short) 1, /* copied= */ false);
+ assertCopied(copied, 2, LabelId.CODE_REVIEW, (short) 1, /* copied= */ true);
}
@Test
@@ -1311,4 +1344,12 @@
}
assertWithMessage(name).that(vote).isEqualTo(expectedVote);
}
+
+ private void assertCopied(
+ PatchSetApproval approval, int psId, String label, short value, boolean copied) {
+ assertThat(approval.patchSetId().get()).isEqualTo(psId);
+ assertThat(approval.label()).isEqualTo(label);
+ assertThat(approval.value()).isEqualTo(value);
+ assertThat(approval.copied()).isEqualTo(copied);
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 81b9ba8..4cbc36b 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -72,7 +72,6 @@
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupAuditEventInfo;
import com.google.gerrit.extensions.common.GroupAuditEventInfo.GroupMemberAuditEventInfo;
-import com.google.gerrit.extensions.common.GroupAuditEventInfo.Type;
import com.google.gerrit.extensions.common.GroupAuditEventInfo.UserMemberAuditEventInfo;
import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.GroupOptionsInfo;
@@ -408,7 +407,8 @@
List<? extends GroupAuditEventInfo> auditEvents = gApi.groups().id(group.get()).auditLog();
assertThat(auditEvents).hasSize(1);
- assertSubgroupAuditEvent(auditEvents.get(0), Type.ADD_GROUP, admin.id(), "Registered Users");
+ assertSubgroupAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.ADD_GROUP, admin.id(), "Registered Users");
}
@Test
@@ -1046,41 +1046,48 @@
GroupApi g = gApi.groups().create(name("group"));
List<? extends GroupAuditEventInfo> auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(1);
- assertMemberAuditEvent(auditEvents.get(0), Type.ADD_USER, admin.id(), admin.id());
+ assertMemberAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.ADD_USER, admin.id(), admin.id());
g.addMembers(user.username());
auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(2);
- assertMemberAuditEvent(auditEvents.get(0), Type.ADD_USER, admin.id(), user.id());
+ assertMemberAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.ADD_USER, admin.id(), user.id());
g.removeMembers(user.username());
auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(3);
- assertMemberAuditEvent(auditEvents.get(0), Type.REMOVE_USER, admin.id(), user.id());
+ assertMemberAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.REMOVE_USER, admin.id(), user.id());
String otherGroup = name("otherGroup");
gApi.groups().create(otherGroup);
g.addGroups(otherGroup);
auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(4);
- assertSubgroupAuditEvent(auditEvents.get(0), Type.ADD_GROUP, admin.id(), otherGroup);
+ assertSubgroupAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.ADD_GROUP, admin.id(), otherGroup);
g.removeGroups(otherGroup);
auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(5);
- assertSubgroupAuditEvent(auditEvents.get(0), Type.REMOVE_GROUP, admin.id(), otherGroup);
+ assertSubgroupAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.REMOVE_GROUP, admin.id(), otherGroup);
// Add a removed member back again.
g.addMembers(user.username());
auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(6);
- assertMemberAuditEvent(auditEvents.get(0), Type.ADD_USER, admin.id(), user.id());
+ assertMemberAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.ADD_USER, admin.id(), user.id());
// Add a removed group back again.
g.addGroups(otherGroup);
auditEvents = g.auditLog();
assertThat(auditEvents).hasSize(7);
- assertSubgroupAuditEvent(auditEvents.get(0), Type.ADD_GROUP, admin.id(), otherGroup);
+ assertSubgroupAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.ADD_GROUP, admin.id(), otherGroup);
Timestamp lastDate = null;
for (GroupAuditEventInfo auditEvent : auditEvents) {
@@ -1112,7 +1119,8 @@
List<? extends GroupAuditEventInfo> auditEvents = gApi.groups().id(parentGroup.id).auditLog();
assertThat(auditEvents).hasSize(2);
// Verify the unavailable subgroup's name is null.
- assertSubgroupAuditEvent(auditEvents.get(0), Type.ADD_GROUP, admin.id(), null);
+ assertSubgroupAuditEvent(
+ auditEvents.get(0), GroupAuditEventInfo.Type.ADD_GROUP, admin.id(), null);
}
private void deleteGroupRef(String groupId) throws Exception {
@@ -1617,7 +1625,7 @@
private void assertMemberAuditEvent(
GroupAuditEventInfo info,
- Type expectedType,
+ GroupAuditEventInfo.Type expectedType,
Account.Id expectedUser,
Account.Id expectedMember) {
assertThat(info.user._accountId).isEqualTo(expectedUser.get());
@@ -1628,7 +1636,7 @@
private void assertSubgroupAuditEvent(
GroupAuditEventInfo info,
- Type expectedType,
+ GroupAuditEventInfo.Type expectedType,
Account.Id expectedUser,
String expectedMemberGroupName) {
assertThat(info.user._accountId).isEqualTo(expectedUser.get());
diff --git a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
index 32e8232..bf428f9 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
@@ -180,7 +180,7 @@
projectCache.evict(newProjectName);
ProjectAccessInfo actual = pApi().access();
// Permissions don't change
- assertThat(expected.local).isEqualTo(actual.local);
+ assertThat(actual.local).isEqualTo(expected.local);
}
@Test
diff --git a/javatests/com/google/gerrit/extensions/conditions/BooleanConditionTest.java b/javatests/com/google/gerrit/extensions/conditions/BooleanConditionTest.java
index f9f1fa85..f8945b5 100644
--- a/javatests/com/google/gerrit/extensions/conditions/BooleanConditionTest.java
+++ b/javatests/com/google/gerrit/extensions/conditions/BooleanConditionTest.java
@@ -14,10 +14,6 @@
package com.google.gerrit.extensions.conditions;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.not;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.valueOf;
import static org.junit.Assert.assertEquals;
import org.junit.Test;
@@ -49,78 +45,84 @@
@Test
public void reduceAnd_CutOffNonTrivialWhenPossible() throws Exception {
- BooleanCondition nonReduced = and(false, NO_TRIVIAL_EVALUATION);
- BooleanCondition reduced = valueOf(false);
+ BooleanCondition nonReduced = BooleanCondition.and(false, NO_TRIVIAL_EVALUATION);
+ BooleanCondition reduced = BooleanCondition.valueOf(false);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceAnd_CutOffNonTrivialWhenPossibleSwapped() throws Exception {
- BooleanCondition nonReduced = and(NO_TRIVIAL_EVALUATION, valueOf(false));
- BooleanCondition reduced = valueOf(false);
+ BooleanCondition nonReduced =
+ BooleanCondition.and(NO_TRIVIAL_EVALUATION, BooleanCondition.valueOf(false));
+ BooleanCondition reduced = BooleanCondition.valueOf(false);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceAnd_KeepNonTrivialWhenNoCutOffPossible() throws Exception {
- BooleanCondition nonReduced = and(true, NO_TRIVIAL_EVALUATION);
- BooleanCondition reduced = and(true, NO_TRIVIAL_EVALUATION);
+ BooleanCondition nonReduced = BooleanCondition.and(true, NO_TRIVIAL_EVALUATION);
+ BooleanCondition reduced = BooleanCondition.and(true, NO_TRIVIAL_EVALUATION);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceAnd_KeepNonTrivialWhenNoCutOffPossibleSwapped() throws Exception {
- BooleanCondition nonReduced = and(NO_TRIVIAL_EVALUATION, valueOf(true));
- BooleanCondition reduced = and(NO_TRIVIAL_EVALUATION, valueOf(true));
+ BooleanCondition nonReduced =
+ BooleanCondition.and(NO_TRIVIAL_EVALUATION, BooleanCondition.valueOf(true));
+ BooleanCondition reduced =
+ BooleanCondition.and(NO_TRIVIAL_EVALUATION, BooleanCondition.valueOf(true));
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceOr_CutOffNonTrivialWhenPossible() throws Exception {
- BooleanCondition nonReduced = or(true, NO_TRIVIAL_EVALUATION);
- BooleanCondition reduced = valueOf(true);
+ BooleanCondition nonReduced = BooleanCondition.or(true, NO_TRIVIAL_EVALUATION);
+ BooleanCondition reduced = BooleanCondition.valueOf(true);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceOr_CutOffNonTrivialWhenPossibleSwapped() throws Exception {
- BooleanCondition nonReduced = or(NO_TRIVIAL_EVALUATION, valueOf(true));
- BooleanCondition reduced = valueOf(true);
+ BooleanCondition nonReduced =
+ BooleanCondition.or(NO_TRIVIAL_EVALUATION, BooleanCondition.valueOf(true));
+ BooleanCondition reduced = BooleanCondition.valueOf(true);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceOr_KeepNonTrivialWhenNoCutOffPossible() throws Exception {
- BooleanCondition nonReduced = or(false, NO_TRIVIAL_EVALUATION);
- BooleanCondition reduced = or(false, NO_TRIVIAL_EVALUATION);
+ BooleanCondition nonReduced = BooleanCondition.or(false, NO_TRIVIAL_EVALUATION);
+ BooleanCondition reduced = BooleanCondition.or(false, NO_TRIVIAL_EVALUATION);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceOr_KeepNonTrivialWhenNoCutOffPossibleSwapped() throws Exception {
- BooleanCondition nonReduced = or(NO_TRIVIAL_EVALUATION, valueOf(false));
- BooleanCondition reduced = or(NO_TRIVIAL_EVALUATION, valueOf(false));
+ BooleanCondition nonReduced =
+ BooleanCondition.or(NO_TRIVIAL_EVALUATION, BooleanCondition.valueOf(false));
+ BooleanCondition reduced =
+ BooleanCondition.or(NO_TRIVIAL_EVALUATION, BooleanCondition.valueOf(false));
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceNot_ReduceIrrelevant() throws Exception {
- BooleanCondition nonReduced = not(valueOf(true));
- BooleanCondition reduced = valueOf(false);
+ BooleanCondition nonReduced = BooleanCondition.not(BooleanCondition.valueOf(true));
+ BooleanCondition reduced = BooleanCondition.valueOf(false);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceNot_ReduceIrrelevant2() throws Exception {
- BooleanCondition nonReduced = not(valueOf(false));
- BooleanCondition reduced = valueOf(true);
+ BooleanCondition nonReduced = BooleanCondition.not(BooleanCondition.valueOf(false));
+ BooleanCondition reduced = BooleanCondition.valueOf(true);
assertEquals(nonReduced.reduce(), reduced);
}
@Test
public void reduceNot_KeepNonTrivialWhenNoCutOffPossible() throws Exception {
- BooleanCondition nonReduced = not(NO_TRIVIAL_EVALUATION);
- BooleanCondition reduced = not(NO_TRIVIAL_EVALUATION);
+ BooleanCondition nonReduced = BooleanCondition.not(NO_TRIVIAL_EVALUATION);
+ BooleanCondition reduced = BooleanCondition.not(NO_TRIVIAL_EVALUATION);
assertEquals(nonReduced.reduce(), reduced);
}
@@ -132,8 +134,10 @@
// / \ \
// NTE NTE TRUE
BooleanCondition nonReduced =
- and(or(NO_TRIVIAL_EVALUATION, NO_TRIVIAL_EVALUATION), not(valueOf(true)));
- BooleanCondition reduced = valueOf(false);
+ BooleanCondition.and(
+ BooleanCondition.or(NO_TRIVIAL_EVALUATION, NO_TRIVIAL_EVALUATION),
+ BooleanCondition.not(BooleanCondition.valueOf(true)));
+ BooleanCondition reduced = BooleanCondition.valueOf(false);
assertEquals(nonReduced.reduce(), reduced);
}
@@ -145,8 +149,13 @@
// / \ / \
// NTE NTE T F
BooleanCondition nonReduced =
- and(or(NO_TRIVIAL_EVALUATION, NO_TRIVIAL_EVALUATION), or(valueOf(true), valueOf(false)));
- BooleanCondition reduced = and(or(NO_TRIVIAL_EVALUATION, NO_TRIVIAL_EVALUATION), valueOf(true));
+ BooleanCondition.and(
+ BooleanCondition.or(NO_TRIVIAL_EVALUATION, NO_TRIVIAL_EVALUATION),
+ BooleanCondition.or(BooleanCondition.valueOf(true), BooleanCondition.valueOf(false)));
+ BooleanCondition reduced =
+ BooleanCondition.and(
+ BooleanCondition.or(NO_TRIVIAL_EVALUATION, NO_TRIVIAL_EVALUATION),
+ BooleanCondition.valueOf(true));
assertEquals(nonReduced.reduce(), reduced);
}
}
diff --git a/javatests/com/google/gerrit/index/query/AndPredicateTest.java b/javatests/com/google/gerrit/index/query/AndPredicateTest.java
index 16828dd..01fa99b 100644
--- a/javatests/com/google/gerrit/index/query/AndPredicateTest.java
+++ b/javatests/com/google/gerrit/index/query/AndPredicateTest.java
@@ -14,7 +14,6 @@
package com.google.gerrit.index.query;
-import static com.google.common.collect.ImmutableList.of;
import static com.google.gerrit.index.query.Predicate.and;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.junit.Assert.assertEquals;
@@ -23,6 +22,7 @@
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
+import com.google.common.collect.ImmutableList;
import java.util.List;
import org.junit.Test;
@@ -44,13 +44,13 @@
final Predicate<String> n = and(a, b);
assertThrows(UnsupportedOperationException.class, () -> n.getChildren().clear());
- assertChildren("clear", n, of(a, b));
+ assertChildren("clear", n, ImmutableList.of(a, b));
assertThrows(UnsupportedOperationException.class, () -> n.getChildren().remove(0));
- assertChildren("remove(0)", n, of(a, b));
+ assertChildren("remove(0)", n, ImmutableList.of(a, b));
assertThrows(UnsupportedOperationException.class, () -> n.getChildren().iterator().remove());
- assertChildren("iterator().remove()", n, of(a, b));
+ assertChildren("iterator().remove()", n, ImmutableList.of(a, b));
}
private static void assertChildren(
@@ -98,8 +98,8 @@
final TestPredicate a = f("author", "alice");
final TestPredicate b = f("author", "bob");
final TestPredicate c = f("author", "charlie");
- final List<TestPredicate> s2 = of(a, b);
- final List<TestPredicate> s3 = of(a, b, c);
+ final List<TestPredicate> s2 = ImmutableList.of(a, b);
+ final List<TestPredicate> s3 = ImmutableList.of(a, b, c);
final Predicate<String> n2 = and(a, b);
assertNotSame(n2, n2.copy(s2));
diff --git a/javatests/com/google/gerrit/index/query/OrPredicateTest.java b/javatests/com/google/gerrit/index/query/OrPredicateTest.java
index 1cbcb75..4d6c6e1 100644
--- a/javatests/com/google/gerrit/index/query/OrPredicateTest.java
+++ b/javatests/com/google/gerrit/index/query/OrPredicateTest.java
@@ -14,7 +14,6 @@
package com.google.gerrit.index.query;
-import static com.google.common.collect.ImmutableList.of;
import static com.google.gerrit.index.query.Predicate.or;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.junit.Assert.assertEquals;
@@ -23,6 +22,7 @@
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
+import com.google.common.collect.ImmutableList;
import java.util.List;
import org.junit.Test;
@@ -44,13 +44,13 @@
final Predicate<String> n = or(a, b);
assertThrows(UnsupportedOperationException.class, () -> n.getChildren().clear());
- assertChildren("clear", n, of(a, b));
+ assertChildren("clear", n, ImmutableList.of(a, b));
assertThrows(UnsupportedOperationException.class, () -> n.getChildren().remove(0));
- assertChildren("remove(0)", n, of(a, b));
+ assertChildren("remove(0)", n, ImmutableList.of(a, b));
assertThrows(UnsupportedOperationException.class, () -> n.getChildren().iterator().remove());
- assertChildren("iterator().remove()", n, of(a, b));
+ assertChildren("iterator().remove()", n, ImmutableList.of(a, b));
}
private static void assertChildren(
@@ -98,8 +98,8 @@
final TestPredicate a = f("author", "alice");
final TestPredicate b = f("author", "bob");
final TestPredicate c = f("author", "charlie");
- final List<TestPredicate> s2 = of(a, b);
- final List<TestPredicate> s3 = of(a, b, c);
+ final List<TestPredicate> s2 = ImmutableList.of(a, b);
+ final List<TestPredicate> s3 = ImmutableList.of(a, b, c);
final Predicate<String> n2 = or(a, b);
assertNotSame(n2, n2.copy(s2));
diff --git a/javatests/com/google/gerrit/server/cache/serialize/CommentContextSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/CommentContextSerializerTest.java
index 8fe7662..8f5b215 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/CommentContextSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/CommentContextSerializerTest.java
@@ -1,12 +1,12 @@
package com.google.gerrit.server.cache.serialize;
import static com.google.common.truth.Truth.assertThat;
-import static com.google.gerrit.server.comment.CommentContextCacheImpl.CommentContextSerializer.INSTANCE;
import com.google.common.collect.ImmutableMap;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.CommentContext;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.comment.CommentContextCacheImpl.CommentContextSerializer;
import com.google.gerrit.server.comment.CommentContextKey;
import org.junit.Test;
@@ -16,8 +16,8 @@
CommentContext commentContext =
CommentContext.create(ImmutableMap.of(1, "line_1", 2, "line_2"), "text/x-java");
- byte[] serialized = INSTANCE.serialize(commentContext);
- CommentContext deserialized = INSTANCE.deserialize(serialized);
+ byte[] serialized = CommentContextSerializer.INSTANCE.serialize(commentContext);
+ CommentContext deserialized = CommentContextSerializer.INSTANCE.deserialize(serialized);
assertThat(commentContext).isEqualTo(deserialized);
}
diff --git a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
index 20813f6..01537e0 100644
--- a/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
+++ b/javatests/com/google/gerrit/server/change/ChangeKindCacheImplTest.java
@@ -23,7 +23,6 @@
import com.google.gerrit.proto.testing.SerializedClassSubject;
import com.google.gerrit.server.cache.proto.Cache.ChangeKindKeyProto;
import com.google.gerrit.server.cache.serialize.CacheSerializer;
-import com.google.gerrit.server.change.ChangeKindCacheImpl.Key;
import org.eclipse.jgit.lib.ObjectId;
import org.junit.Test;
@@ -31,7 +30,7 @@
@Test
public void keySerializer() throws Exception {
ChangeKindCacheImpl.Key key =
- Key.create(
+ ChangeKindCacheImpl.Key.create(
ObjectId.zeroId(),
ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
"aStrategy");
diff --git a/javatests/com/google/gerrit/server/change/CommentThreadTest.java b/javatests/com/google/gerrit/server/change/CommentThreadTest.java
index dc46e48..08485a4 100644
--- a/javatests/com/google/gerrit/server/change/CommentThreadTest.java
+++ b/javatests/com/google/gerrit/server/change/CommentThreadTest.java
@@ -19,7 +19,6 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Comment;
-import com.google.gerrit.entities.Comment.Key;
import com.google.gerrit.entities.HumanComment;
import java.sql.Timestamp;
import org.junit.Test;
@@ -59,7 +58,7 @@
private static HumanComment createComment(String commentUuid) {
return new HumanComment(
- new Key(commentUuid, "myFile", 1),
+ new Comment.Key(commentUuid, "myFile", 1),
Account.id(100),
new Timestamp(1234),
(short) 1,
diff --git a/javatests/com/google/gerrit/server/change/CommentThreadsTest.java b/javatests/com/google/gerrit/server/change/CommentThreadsTest.java
index 56566d3..0c61906 100644
--- a/javatests/com/google/gerrit/server/change/CommentThreadsTest.java
+++ b/javatests/com/google/gerrit/server/change/CommentThreadsTest.java
@@ -19,7 +19,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
-import com.google.gerrit.entities.Comment.Key;
+import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.HumanComment;
import java.sql.Timestamp;
import org.junit.Test;
@@ -260,7 +260,7 @@
private static HumanComment createComment(String commentUuid) {
return new HumanComment(
- new Key(commentUuid, "myFile", 1),
+ new Comment.Key(commentUuid, "myFile", 1),
Account.id(100),
new Timestamp(1234),
(short) 1,
diff --git a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
index 19c2bcf..056c7dc 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
@@ -785,7 +785,7 @@
.containsExactly(
"@@ -6 +6 @@\n"
+ "-Removed Verified+2 by Other Account <other@account.com>\n"
- + "+Removed Verified+2 by Gerrit Account\n");
+ + "+Removed Verified+2\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
@@ -1671,10 +1671,9 @@
+ " * file1.java\n"
+ "\n<GERRIT_ACCOUNT_2>, who was added as reviewer owns the following files:\n"
+ " * file3.js\n"
- + "\nGerrit Account, who was added as reviewer owns the following files:\n"
+ + "\nAdded reviewer owns the following files:\n"
+ " * file4.java\n",
- "Gerrit Account, who was added as reviewer owns the following files:\n"
- + " * file6.java\n",
+ "Added reviewer owns the following files:\n" + " * file6.java\n",
"Gerrit Account who was added as reviewer owns the following files:\n"
+ " * file1.java\n"
+ "\n<GERRIT_ACCOUNT_1> who was added as reviewer owns the following files:\n"
@@ -1701,10 +1700,10 @@
+ "+<GERRIT_ACCOUNT_2>, who was added as reviewer owns the following files:\n"
+ "@@ -12 +12 @@\n"
+ "-Missing Reviewer who was added as reviewer owns the following files:\n"
- + "+Gerrit Account, who was added as reviewer owns the following files:\n",
+ + "+Added reviewer owns the following files:\n",
"@@ -6 +6 @@\n"
+ "-Reviewer User who was added as reviewer owns the following files:\n"
- + "+Gerrit Account, who was added as reviewer owns the following files:\n");
+ + "+Added reviewer owns the following files:\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
@@ -2051,7 +2050,8 @@
getChangeUpdateBody(
c,
String.format(
- "Assignee changed from: %s to: %s", changeOwner.getName(), otherUser.getName())),
+ "Assignee changed from: %s to: %s",
+ changeOwner.getNameEmail(), otherUser.getNameEmail())),
getAuthorIdent(otherUser.getAccount()));
writeUpdate(
RefNames.changeMetaRef(c.getId()),
@@ -2086,14 +2086,12 @@
+ "-Assignee added: Change Owner\n"
+ "+Assignee added: <GERRIT_ACCOUNT_1>\n",
"@@ -6 +6 @@\n"
- + "-Assignee changed from: Change Owner to: Other Account\n"
+ + "-Assignee changed from: Change Owner <change@owner.com> to: Other Account <other@account.com>\n"
+ "+Assignee changed from: <GERRIT_ACCOUNT_1> to: <GERRIT_ACCOUNT_2>\n",
"@@ -6 +6 @@\n"
+ "-Assignee deleted: Other Account\n"
+ "+Assignee deleted: <GERRIT_ACCOUNT_2>\n",
- "@@ -6 +6 @@\n"
- + "-Assignee added: Reviewer User\n"
- + "+Assignee added: Gerrit Account\n");
+ "@@ -6 +6 @@\n" + "-Assignee added: Reviewer User\n" + "+Assignee was added.\n");
BackfillResult secondRunResult = rewriter.backfillProject(project, repo, options);
assertThat(secondRunResult.fixedRefDiff.keySet()).isEmpty();
assertThat(secondRunResult.refsFailedToFix).isEmpty();
@@ -2228,11 +2226,11 @@
private void assertFixedCommits(
ImmutableList<ObjectId> expectedFixedCommits, BackfillResult result, Change.Id changeId) {
- assertThat(expectedFixedCommits)
- .containsExactlyElementsIn(
+ assertThat(
result.fixedRefDiff.get(RefNames.changeMetaRef(changeId)).stream()
.map(CommitDiff::oldSha1)
- .collect(toImmutableList()));
+ .collect(toImmutableList()))
+ .containsExactlyElementsIn(expectedFixedCommits);
}
private String getAccountIdentToFix(Account account) {
diff --git a/plugins/replication b/plugins/replication
index cd17fe7..639ea4b 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit cd17fe7f90e5a36ab84b9b7ce0aab22e60e48a70
+Subproject commit 639ea4b3a3b3a67c80d29a6f83130499686543d3
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index fd7b5d1..fc2cbe5 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -1824,7 +1824,7 @@
changeIsOpen(change)
) {
fireAlert(this, 'Change edit not found. Please create a change edit.');
- GerritNav.navigateToChange(change);
+ fireReload(this, true);
return;
}
@@ -1837,7 +1837,7 @@
this,
'Change edits cannot be created if change is merged or abandoned. Redirected to non edit mode.'
);
- GerritNav.navigateToChange(change);
+ fireReload(this, true);
return;
}
diff --git a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts
index 0af2f36..e21584e 100644
--- a/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-score-row/gr-label-score-row_html.ts
@@ -56,7 +56,7 @@
gr-tooltip-content.iron-selected > gr-button[vote='max'] {
--button-background-color: var(--vote-color-approved);
}
- gr-tooltip-content.iron-selected > gr-buttonvote='positive'] {
+ gr-tooltip-content.iron-selected > gr-button[vote='positive'] {
--button-background-color: var(--vote-color-recommended);
}
gr-tooltip-content.iron-selected > gr-button[vote='min'] {
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
index 4b1dba6..3c9f54c 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard.ts
@@ -14,19 +14,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import '../../../styles/gr-font-styles';
-import '../../../styles/gr-hovercard-styles';
-import '../../../styles/shared-styles';
import '../../shared/gr-button/gr-button';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {customElement, property} from '@polymer/decorators';
-import {HovercardBehaviorMixin} from '../../shared/gr-hovercard/gr-hovercard-behavior';
-import {htmlTemplate} from './gr-submit-requirement-hovercard_html';
+import '../../shared/gr-label-info/gr-label-info';
+import '../../shared/gr-limited-text/gr-limited-text';
+import {customElement, property} from 'lit/decorators';
import {
AccountInfo,
SubmitRequirementExpressionInfo,
SubmitRequirementResultInfo,
- SubmitRequirementStatus,
} from '../../../api/rest-api';
import {
extractAssociatedLabels,
@@ -34,16 +29,15 @@
} from '../../../utils/label-util';
import {ParsedChangeInfo} from '../../../types/types';
import {Label} from '../gr-change-requirements/gr-change-requirements';
+import {css, html, LitElement} from 'lit';
+import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
+import {fontStyles} from '../../../styles/gr-font-styles';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = HovercardBehaviorMixin(PolymerElement);
+const base = HovercardMixin(LitElement);
@customElement('gr-submit-requirement-hovercard')
export class GrHovercardRun extends base {
- static get template() {
- return htmlTemplate;
- }
-
@property({type: Object})
requirement?: SubmitRequirementResultInfo;
@@ -59,16 +53,176 @@
@property({type: Boolean})
expanded = false;
- @property({type: Array, computed: 'computeLabels(change, requirement)'})
- _labels: Label[] = [];
+ static override get styles() {
+ return [
+ fontStyles,
+ base.styles || [],
+ css`
+ #container {
+ min-width: 356px;
+ max-width: 356px;
+ padding: var(--spacing-xl) 0 var(--spacing-m) 0;
+ }
+ section.label {
+ display: table-row;
+ }
+ .label-title {
+ min-width: 10em;
+ padding-top: var(--spacing-s);
+ }
+ .label-value {
+ padding-top: var(--spacing-s);
+ }
+ .label-title,
+ .label-value {
+ display: table-cell;
+ vertical-align: top;
+ }
+ .row {
+ display: flex;
+ }
+ .title {
+ color: var(--deemphasized-text-color);
+ margin-right: var(--spacing-m);
+ }
+ div.section {
+ margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
+ display: flex;
+ align-items: center;
+ }
+ div.sectionIcon {
+ flex: 0 0 30px;
+ }
+ div.sectionIcon iron-icon {
+ position: relative;
+ width: 20px;
+ height: 20px;
+ }
+ .condition {
+ background-color: var(--gray-background);
+ padding: var(--spacing-m);
+ flex-grow: 1;
+ }
+ .expression {
+ color: var(--gray-foreground);
+ }
+ iron-icon.check {
+ color: var(--success-foreground);
+ }
+ iron-icon.close {
+ color: var(--warning-foreground);
+ }
+ .showConditions iron-icon {
+ color: inherit;
+ }
+ div.showConditions {
+ border-top: 1px solid var(--border-color);
+ margin-top: var(--spacing-m);
+ padding: var(--spacing-m) var(--spacing-xl) 0;
+ }
+ `,
+ ];
+ }
- computeLabels(
- change?: ParsedChangeInfo,
- requirement?: SubmitRequirementResultInfo
- ) {
- if (!requirement) return [];
- const requirementLabels = extractAssociatedLabels(requirement);
- const labels = change?.labels ?? {};
+ override render() {
+ if (!this.requirement) return;
+ const icon = iconForStatus(this.requirement.status);
+ return html` <div id="container" role="tooltip" tabindex="-1">
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="${icon}" icon="gr-icons:${icon}"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <h3 class="name heading-3">
+ <span>${this.requirement.name}</span>
+ </h3>
+ </div>
+ </div>
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <div class="row">
+ <div class="title">Status</div>
+ <div>${this.requirement.status}</div>
+ </div>
+ </div>
+ </div>
+ ${this.renderLabelSection()} ${this.renderConditionSection()}
+ </div>`;
+ }
+
+ private renderLabelSection() {
+ const labels = this.computeLabels();
+ return html` <div class="section">
+ ${labels.map(l => this.renderLabel(l))}
+ </div>`;
+ }
+
+ private renderLabel(label: Label) {
+ return html`
+ <section class="label">
+ <div class="label-title">
+ <gr-limited-text
+ class="name"
+ limit="25"
+ text="${label.labelName}"
+ ></gr-limited-text>
+ </div>
+ <div class="label-value">
+ <gr-label-info
+ .change=${this.change}
+ .account=${this.account}
+ .mutable=${this.mutable}
+ .label="${label.labelName}"
+ .labelInfo="${label.labelInfo}"
+ ></gr-label-info>
+ </div>
+ </section>
+ `;
+ }
+
+ private renderConditionSection() {
+ if (!this.expanded) {
+ return html` <div class="showConditions">
+ <gr-button
+ link=""
+ class="showConditions"
+ @click="${(_: MouseEvent) => this.handleShowConditions()}"
+ >
+ View condition
+ <iron-icon icon="gr-icons:expand-more"></iron-icon
+ ></gr-button>
+ </div>`;
+ } else {
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon icon="gr-icons:description"></iron-icon>
+ </div>
+ <div class="sectionContent">${this.requirement?.description}</div>
+ </div>
+ ${this.renderCondition(
+ 'Blocking condition',
+ this.requirement?.submittability_expression_result
+ )}
+ ${this.renderCondition(
+ 'Application condition',
+ this.requirement?.applicability_expression_result
+ )}
+ ${this.renderCondition(
+ 'Override condition',
+ this.requirement?.override_expression_result
+ )}
+ `;
+ }
+ }
+
+ private computeLabels() {
+ if (!this.requirement) return [];
+ const requirementLabels = extractAssociatedLabels(this.requirement);
+ const labels = this.change?.labels ?? {};
const allLabels: Label[] = [];
@@ -85,17 +239,23 @@
return allLabels;
}
- computeIcon(status: SubmitRequirementStatus) {
- return iconForStatus(status);
- }
-
- renderCondition(expression?: SubmitRequirementExpressionInfo) {
+ private renderCondition(
+ name: string,
+ expression?: SubmitRequirementExpressionInfo
+ ) {
if (!expression) return '';
-
- return expression.expression;
+ return html`
+ <div class="section">
+ <div class="sectionIcon"></div>
+ <div class="sectionContent condition">
+ ${name}:<br />
+ <span class="expression"> ${expression.expression} </span>
+ </div>
+ </div>
+ `;
}
- _handleShowConditions() {
+ private handleShowConditions() {
this.expanded = true;
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts b/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts
deleted file mode 100644
index 5023895..0000000
--- a/polygerrit-ui/app/elements/change/gr-submit-requirement-hovercard/gr-submit-requirement-hovercard_html.ts
+++ /dev/null
@@ -1,192 +0,0 @@
-/**
- * @license
- * Copyright (C) 2021 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-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="shared-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-hovercard-styles">
- #container {
- min-width: 356px;
- max-width: 356px;
- padding: var(--spacing-xl) 0 var(--spacing-m) 0;
- }
- section.label {
- display: table-row;
- }
- .label-title {
- min-width: 10em;
- padding-top: var(--spacing-s);
- }
- .label-value {
- padding-top: var(--spacing-s);
- }
- .label-title,
- .label-value {
- display: table-cell;
- vertical-align: top;
- }
- .row {
- display: flex;
- }
- .title {
- color: var(--deemphasized-text-color);
- margin-right: var(--spacing-m);
- }
- div.section {
- margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
- display: flex;
- align-items: center;
- }
- div.sectionIcon {
- flex: 0 0 30px;
- }
- div.sectionIcon iron-icon {
- position: relative;
- width: 20px;
- height: 20px;
- }
- .condition {
- background-color: var(--gray-background);
- padding: var(--spacing-m);
- flex-grow: 1;
- }
- .expression {
- color: var(--gray-foreground);
- }
- iron-icon.check {
- color: var(--success-foreground);
- }
- iron-icon.close {
- color: var(--warning-foreground);
- }
- .showConditions iron-icon {
- color: inherit;
- }
- div.showConditions {
- border-top: 1px solid var(--border-color);
- margin-top: var(--spacing-m);
- padding: var(--spacing-m) var(--spacing-xl) 0;
- }
- </style>
- <div id="container" role="tooltip" tabindex="-1">
- <div class="section">
- <div class="sectionIcon">
- <iron-icon
- class$="[[computeIcon(requirement.status)]]"
- icon="gr-icons:[[computeIcon(requirement.status)]]"
- ></iron-icon>
- </div>
- <div class="sectionContent">
- <h3 class="name heading-3">
- <span>[[requirement.name]]</span>
- </h3>
- </div>
- </div>
- <div class="section">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
- </div>
- <div class="sectionContent">
- <div class="row">
- <div class="title">Status</div>
- <div>[[requirement.status]]</div>
- </div>
- </div>
- </div>
- <div class="section">
- <template is="dom-repeat" items="[[_labels]]">
- <section class="label">
- <div class="label-title">
- <gr-limited-text
- class="name"
- limit="25"
- text="[[item.labelName]]"
- ></gr-limited-text>
- </div>
- <div class="label-value">
- <gr-label-info
- change="{{change}}"
- account="[[account]]"
- mutable="[[mutable]]"
- label="[[item.labelName]]"
- label-info="[[item.labelInfo]]"
- ></gr-label-info>
- </div>
- </section>
- </template>
- </div>
- <template is="dom-if" if="[[!expanded]]">
- <div class="showConditions">
- <gr-button
- link=""
- class="showConditions"
- on-click="_handleShowConditions"
- >
- View condition
- <iron-icon icon="gr-icons:expand-more"></iron-icon
- ></gr-button>
- </div>
- </template>
- <template is="dom-if" if="[[expanded]]">
- <div class="section">
- <div class="sectionIcon">
- <iron-icon icon="gr-icons:description"></iron-icon>
- </div>
- <div class="sectionContent">[[requirement.description]]</div>
- </div>
- <div class="section">
- <div class="sectionIcon"></div>
- <div class="sectionContent condition">
- Blocking condition:<br />
- <span class="expression">
- [[renderCondition(requirement.submittability_expression_result)]]
- </span>
- </div>
- </div>
- <template
- is="dom-if"
- if="[[requirement.applicability_expression_result]]"
- >
- <div class="section">
- <div class="sectionIcon"></div>
- <div class="sectionContent condition">
- Application condition:<br />
- <span class="expression">
- [[renderCondition(requirement.applicability_expression_result)]]
- </span>
- </div>
- </div>
- </template>
- <template is="dom-if" if="[[requirement.override_expression_result]]">
- <div class="section">
- <div class="sectionIcon"></div>
- <div class="sectionContent condition">
- Override condition:<br />
- <span class="expression">
- [[renderCondition(requirement.override_expression_result)]]
- </span>
- </div>
- </div>
- </template>
- </template>
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
index 21e8093..004d594 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements.ts
@@ -21,6 +21,7 @@
import {
AccountInfo,
isDetailedLabelInfo,
+ isQuickLabelInfo,
LabelInfo,
LabelNameToInfoMap,
SubmitRequirementResultInfo,
@@ -30,6 +31,7 @@
import {
extractAssociatedLabels,
getAllUniqueApprovals,
+ hasNeutralStatus,
hasVotes,
iconForStatus,
} from '../../../utils/label-util';
@@ -79,20 +81,6 @@
iron-icon.close {
color: var(--warning-foreground);
}
- .testing {
- margin-top: var(--spacing-xxl);
- padding-left: var(--metadata-horizontal-padding);
- color: var(--deemphasized-text-color);
- }
- .testing gr-button {
- min-width: 25px;
- }
- .testing * {
- visibility: hidden;
- }
- .testing:hover * {
- visibility: visible;
- }
.requirements,
section.trigger-votes {
margin-left: var(--spacing-l);
@@ -191,9 +179,7 @@
></gr-submit-requirement-hovercard>
`
)}
- ${this.renderTriggerVotes(
- submit_requirements
- )}${this.renderFakeControls()}`;
+ ${this.renderTriggerVotes(submit_requirements)}`;
}
renderStatus(status: SubmitRequirementStatus) {
@@ -225,18 +211,25 @@
renderLabelVote(label: string, labels: LabelNameToInfoMap) {
const labelInfo = labels[label];
- if (!isDetailedLabelInfo(labelInfo)) return;
- const uniqueApprovals = getAllUniqueApprovals(labelInfo);
- return uniqueApprovals.map(
- approvalInfo =>
- html`<gr-vote-chip
- .vote="${approvalInfo}"
- .label="${labelInfo}"
- .more="${(labelInfo.all ?? []).filter(
- other => other.value === approvalInfo.value
- ).length > 1}"
- ></gr-vote-chip>`
- );
+ if (isDetailedLabelInfo(labelInfo)) {
+ const uniqueApprovals = getAllUniqueApprovals(labelInfo).filter(
+ approval => !hasNeutralStatus(labelInfo, approval)
+ );
+ return uniqueApprovals.map(
+ approvalInfo =>
+ html`<gr-vote-chip
+ .vote="${approvalInfo}"
+ .label="${labelInfo}"
+ .more="${(labelInfo.all ?? []).filter(
+ other => other.value === approvalInfo.value
+ ).length > 1}"
+ ></gr-vote-chip>`
+ );
+ } else if (isQuickLabelInfo(labelInfo)) {
+ return [html`<gr-vote-chip .label="${labelInfo}"></gr-vote-chip>`];
+ } else {
+ return html``;
+ }
}
renderChecks(requirement: SubmitRequirementResultInfo) {
@@ -282,49 +275,6 @@
)}
</section>`;
}
-
- renderFakeControls() {
- return html`
- <div class="testing">
- <div>Toggle fake data:</div>
- <gr-button link @click="${() => this.renderFakeSubmitRequirements()}"
- >G</gr-button
- >
- </div>
- `;
- }
-
- renderFakeSubmitRequirements() {
- if (!this.change) return;
- this.change = {
- ...this.change,
- submit_requirements: [
- {
- name: 'Code-Review',
- status: SubmitRequirementStatus.SATISFIED,
- description:
- "At least one maximum vote for label 'Code-Review' is required",
- submittability_expression_result: {
- expression: 'label:Code-Review=MAX -label:Code-Review=MIN',
- fulfilled: true,
- passing_atoms: [],
- failing_atoms: [],
- },
- },
- {
- name: 'Verified',
- status: SubmitRequirementStatus.UNSATISFIED,
- description: 'CI build and tests results are verified',
- submittability_expression_result: {
- expression: 'label:Verified=MAX -label:Verified=MIN',
- fulfilled: false,
- passing_atoms: [],
- failing_atoms: [],
- },
- },
- ],
- };
- }
}
@customElement('gr-trigger-vote')
@@ -364,19 +314,34 @@
}
override render() {
- const uniqueApprovals = getAllUniqueApprovals(this.labelInfo);
+ if (!this.labelInfo) return;
return html`
<div class="container">
<span class="label">${this.label}</span>
- ${uniqueApprovals.map(
- approvalInfo => html`<gr-vote-chip
- .vote="${approvalInfo}"
- .label="${this.labelInfo}"
- ></gr-vote-chip>`
- )}
+ ${this.renderVotes()}
</div>
`;
}
+
+ private renderVotes() {
+ const {labelInfo} = this;
+ if (!labelInfo) return;
+ if (isDetailedLabelInfo(labelInfo)) {
+ const approvals = getAllUniqueApprovals(labelInfo).filter(
+ approval => !hasNeutralStatus(labelInfo, approval)
+ );
+ return approvals.map(
+ approvalInfo => html`<gr-vote-chip
+ .vote="${approvalInfo}"
+ .label="${labelInfo}"
+ ></gr-vote-chip>`
+ );
+ } else if (isQuickLabelInfo(labelInfo)) {
+ return [html`<gr-vote-chip .label="${this.labelInfo}"></gr-vote-chip>`];
+ } else {
+ return html``;
+ }
+ }
}
declare global {
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
index 57eac3b..95b7157 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run.ts
@@ -14,14 +14,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import './gr-checks-styles';
-import '../../styles/gr-font-styles';
-import '../../styles/gr-hovercard-styles';
-import '../../styles/shared-styles';
-import {HovercardBehaviorMixin} from '../shared/gr-hovercard/gr-hovercard-behavior';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-hovercard-run_html';
-import {customElement, property} from '@polymer/decorators';
+import {fontStyles} from '../../styles/gr-font-styles';
+import {customElement, property} from 'lit/decorators';
import './gr-checks-action';
import {CheckRun} from '../../services/checks/checks-model';
import {
@@ -33,102 +27,346 @@
import {durationString, fromNow} from '../../utils/date-util';
import {RunStatus} from '../../api/checks';
import {ordinal} from '../../utils/string-util';
+import {HovercardMixin} from '../../mixins/hovercard-mixin/hovercard-mixin';
+import {css, html, LitElement} from 'lit';
+import {checksStyles} from './gr-checks-styles';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = HovercardBehaviorMixin(PolymerElement);
+const base = HovercardMixin(LitElement);
@customElement('gr-hovercard-run')
export class GrHovercardRun extends base {
- static get template() {
- return htmlTemplate;
- }
-
@property({type: Object})
run?: CheckRun;
- computeIcon(run?: CheckRun) {
- if (!run) return '';
- const category = worstCategory(run);
+ static override get styles() {
+ return [
+ fontStyles,
+ checksStyles,
+ base.styles || [],
+ css`
+ #container {
+ min-width: 356px;
+ max-width: 356px;
+ padding: var(--spacing-xl) 0 var(--spacing-m) 0;
+ }
+ .row {
+ display: flex;
+ margin-top: var(--spacing-s);
+ }
+ .attempts.row {
+ flex-wrap: wrap;
+ }
+ .chipRow {
+ display: flex;
+ margin-top: var(--spacing-s);
+ }
+ .chip {
+ background: var(--gray-background);
+ color: var(--gray-foreground);
+ border-radius: 20px;
+ padding: var(--spacing-xs) var(--spacing-m) var(--spacing-xs)
+ var(--spacing-s);
+ }
+ .title {
+ color: var(--deemphasized-text-color);
+ margin-right: var(--spacing-m);
+ }
+ div.section {
+ margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
+ display: flex;
+ }
+ div.sectionIcon {
+ flex: 0 0 30px;
+ }
+ div.chip iron-icon {
+ width: 16px;
+ height: 16px;
+ /* Positioning of a 16px icon in the middle of a 20px line. */
+ position: relative;
+ top: 2px;
+ }
+ div.sectionIcon iron-icon {
+ position: relative;
+ top: 2px;
+ width: 20px;
+ height: 20px;
+ }
+ div.sectionIcon iron-icon.small {
+ position: relative;
+ top: 6px;
+ width: 16px;
+ height: 16px;
+ }
+ div.sectionContent iron-icon.link {
+ color: var(--link-color);
+ }
+ div.sectionContent .attemptIcon iron-icon,
+ div.sectionContent iron-icon.small {
+ width: 16px;
+ height: 16px;
+ margin-right: var(--spacing-s);
+ /* Positioning of a 16px icon in the middle of a 20px line. */
+ position: relative;
+ top: 2px;
+ }
+ div.sectionContent .attemptIcon iron-icon {
+ margin-right: 0;
+ }
+ .attemptIcon,
+ .attemptNumber {
+ margin-right: var(--spacing-s);
+ color: var(--deemphasized-text-color);
+ text-align: center;
+ width: 24px;
+ font-size: var(--font-size-small);
+ }
+ div.action {
+ border-top: 1px solid var(--border-color);
+ margin-top: var(--spacing-m);
+ padding: var(--spacing-m) var(--spacing-xl) 0;
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ if (!this.run) return '';
+ const icon = this.computeIcon();
+ return html`
+ <div id="container" role="tooltip" tabindex="-1">
+ <div class="section">
+ <div
+ ?hidden="${!this.run || this.run.status === RunStatus.RUNNABLE}"
+ class="chipRow"
+ >
+ <div class="chip">
+ <iron-icon icon="gr-icons:${this.computeChipIcon()}"></iron-icon>
+ <span>${this.run.status}</span>
+ </div>
+ </div>
+ </div>
+ <div class="section">
+ <div class="sectionIcon" ?hidden="${icon.length === 0}">
+ <iron-icon class="${icon}" icon="gr-icons:${icon}"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <h3 class="name heading-3">
+ <span>${this.run.checkName}</span>
+ </h3>
+ </div>
+ </div>
+ ${this.renderStatusSection()} ${this.renderAttemptSection()}
+ ${this.renderTimestampSection()} ${this.renderDescriptionSection()}
+ ${this.renderActions()}
+ </div>
+ `;
+ }
+
+ private renderStatusSection() {
+ if (!this.run || (!this.run.statusLink && !this.run.statusDescription))
+ return;
+
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ ${this.run.statusLink
+ ? html` <div class="row">
+ <div class="title">Status</div>
+ <div>
+ <a href="${this.run.statusLink}" target="_blank"
+ ><iron-icon
+ aria-label="external link to check status"
+ class="small link"
+ icon="gr-icons:launch"
+ ></iron-icon
+ >${this.computeHostName(this.run.statusLink)}
+ </a>
+ </div>
+ </div>`
+ : ''}
+ ${this.run.statusDescription
+ ? html` <div class="row">
+ <div class="title">Message</div>
+ <div>${this.run.statusDescription}</div>
+ </div>`
+ : ''}
+ </div>
+ </div>
+ `;
+ }
+
+ private renderAttemptSection() {
+ if (this.hideAttempts()) return;
+ const attempts = this.computeAttempts();
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:arrow-forward"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <div class="attempts row">
+ <div class="title">Attempt</div>
+ ${attempts.map(a => this.renderAttempt(a))}
+ </div>
+ </div>
+ </div>
+ `;
+ }
+
+ private renderAttempt(attempt: AttemptDetail) {
+ return html`
+ <div>
+ <div class="attemptIcon">
+ <iron-icon
+ class="${attempt.icon}"
+ icon="gr-icons:${attempt.icon}"
+ ></iron-icon>
+ </div>
+ <div class="attemptNumber">${ordinal(attempt.attempt)}</div>
+ </div>
+ `;
+ }
+
+ private renderTimestampSection() {
+ if (
+ !this.run ||
+ (!this.run.startedTimestamp &&
+ !this.run.scheduledTimestamp &&
+ !this.run.finishedTimestamp)
+ )
+ return;
+
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:schedule"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ <div ?hidden="${this.hideScheduled()}" class="row">
+ <div class="title">Scheduled</div>
+ <div>${this.computeDuration(this.run.scheduledTimestamp)}</div>
+ </div>
+ <div ?hidden="${!this.run.startedTimestamp}" class="row">
+ <div class="title">Started</div>
+ <div>${this.computeDuration(this.run.startedTimestamp)}</div>
+ </div>
+ <div ?hidden="${!this.run.finishedTimestamp}" class="row">
+ <div class="title">Ended</div>
+ <div>${this.computeDuration(this.run.finishedTimestamp)}</div>
+ </div>
+ <div ?hidden="${this.hideCompletion()}" class="row">
+ <div class="title">Completion</div>
+ <div>${this.computeCompletionDuration()}</div>
+ </div>
+ </div>
+ </div>
+ `;
+ }
+
+ private renderDescriptionSection() {
+ if (!this.run || (!this.run.checkLink && !this.run.checkDescription))
+ return;
+ return html`
+ <div class="section">
+ <div class="sectionIcon">
+ <iron-icon class="small" icon="gr-icons:link"></iron-icon>
+ </div>
+ <div class="sectionContent">
+ ${this.run.checkDescription
+ ? html` <div class="row">
+ <div class="title">Description</div>
+ <div>${this.run.checkDescription}</div>
+ </div>`
+ : ''}
+ ${this.run.checkLink
+ ? html` <div class="row">
+ <div class="title">Documentation</div>
+ <div>
+ <a href="${this.run.checkLink}" target="_blank"
+ ><iron-icon
+ aria-label="external link to check documentation"
+ class="small link"
+ icon="gr-icons:launch"
+ ></iron-icon
+ >${this.computeHostName(this.run.checkLink)}
+ </a>
+ </div>
+ </div>`
+ : ''}
+ </div>
+ </div>
+ `;
+ }
+
+ private renderActions() {
+ const actions = runActions(this.run);
+ return actions.map(
+ action =>
+ html`
+ <div class="action">
+ <gr-checks-action
+ .eventTarget="${this._target}"
+ .action="${action}"
+ ></gr-checks-action>
+ </div>
+ `
+ );
+ }
+
+ computeIcon() {
+ if (!this.run) return '';
+ const category = worstCategory(this.run);
if (category) return iconFor(category);
- return run.status === RunStatus.COMPLETED
+ return this.run.status === RunStatus.COMPLETED
? iconFor(RunStatus.COMPLETED)
: '';
}
- computeActions(run?: CheckRun) {
- return runActions(run);
- }
-
- computeAttempt(attempt?: number) {
- return ordinal(attempt);
- }
-
- computeAttempts(run?: CheckRun): AttemptDetail[] {
- const details = run?.attemptDetails ?? [];
+ computeAttempts(): AttemptDetail[] {
+ const details = this.run?.attemptDetails ?? [];
const more =
details.length > 7 ? [{icon: 'more-horiz', attempt: undefined}] : [];
return [...more, ...details.slice(-7)];
}
- computeChipIcon(run?: CheckRun) {
- if (run?.status === RunStatus.COMPLETED) return 'check';
- if (run?.status === RunStatus.RUNNING) return 'timelapse';
+ private computeChipIcon() {
+ if (this.run?.status === RunStatus.COMPLETED) return 'check';
+ if (this.run?.status === RunStatus.RUNNING) return 'timelapse';
return '';
}
- computeCompletionDuration(run?: CheckRun) {
- if (!run?.finishedTimestamp || !run?.startedTimestamp) return '';
- return durationString(run.startedTimestamp, run.finishedTimestamp, true);
- }
-
- computeDuration(date?: Date) {
- return date ? fromNow(date) : '';
- }
-
- computeHostName(link?: string) {
- return link ? new URL(link).hostname : '';
- }
-
- hideChip(run?: CheckRun) {
- return !run || run.status === RunStatus.RUNNABLE;
- }
-
- hideHeaderSectionIcon(run?: CheckRun) {
- return this.computeIcon(run).length === 0;
- }
-
- hideStatusSection(run?: CheckRun) {
- if (!run) return true;
- return !run.statusLink && !run.statusDescription;
- }
-
- hideTimestampSection(run?: CheckRun) {
- if (!run) return true;
- return (
- !run.startedTimestamp && !run.scheduledTimestamp && !run.finishedTimestamp
+ private computeCompletionDuration() {
+ if (!this.run?.finishedTimestamp || !this.run?.startedTimestamp) return '';
+ return durationString(
+ this.run.startedTimestamp,
+ this.run.finishedTimestamp,
+ true
);
}
- hideAttempts(run?: CheckRun) {
- const attemptCount = run?.attemptDetails?.length;
+ private computeDuration(date?: Date) {
+ return date ? fromNow(date) : '';
+ }
+
+ private computeHostName(link?: string) {
+ return link ? new URL(link).hostname : '';
+ }
+
+ private hideAttempts() {
+ const attemptCount = this.run?.attemptDetails?.length;
return attemptCount === undefined || attemptCount < 2;
}
- hideScheduled(run?: CheckRun) {
- return !run?.scheduledTimestamp || !!run?.startedTimestamp;
+ private hideScheduled() {
+ return !this.run?.scheduledTimestamp || !!this.run?.startedTimestamp;
}
- hideCompletion(run?: CheckRun) {
- return !run?.startedTimestamp || !run?.finishedTimestamp;
- }
-
- hideDescriptionSection(run?: CheckRun) {
- if (!run) return true;
- return !run.checkLink && !run.checkDescription;
- }
-
- _convertUndefined(value?: string) {
- return value ?? '';
+ private hideCompletion() {
+ return !this.run?.startedTimestamp || !this.run?.finishedTimestamp;
}
}
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts
deleted file mode 100644
index 52dbb9c..0000000
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_html.ts
+++ /dev/null
@@ -1,235 +0,0 @@
-/**
- * @license
- * Copyright (C) 2021 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-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-checks-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="shared-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-hovercard-styles">
- #container {
- min-width: 356px;
- max-width: 356px;
- padding: var(--spacing-xl) 0 var(--spacing-m) 0;
- }
- .row {
- display: flex;
- margin-top: var(--spacing-s);
- }
- .attempts.row {
- flex-wrap: wrap;
- }
- .chipRow {
- display: flex;
- margin-top: var(--spacing-s);
- }
- .chip {
- background: var(--gray-background);
- color: var(--gray-foreground);
- border-radius: 20px;
- padding: var(--spacing-xs) var(--spacing-m) var(--spacing-xs)
- var(--spacing-s);
- }
- .title {
- color: var(--deemphasized-text-color);
- margin-right: var(--spacing-m);
- }
- div.section {
- margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl);
- display: flex;
- }
- div.sectionIcon {
- flex: 0 0 30px;
- }
- div.chip iron-icon {
- width: 16px;
- height: 16px;
- /* Positioning of a 16px icon in the middle of a 20px line. */
- position: relative;
- top: 2px;
- }
- div.sectionIcon iron-icon {
- position: relative;
- top: 2px;
- width: 20px;
- height: 20px;
- }
- div.sectionIcon iron-icon.small {
- position: relative;
- top: 6px;
- width: 16px;
- height: 16px;
- }
- div.sectionContent iron-icon.link {
- color: var(--link-color);
- }
- div.sectionContent .attemptIcon iron-icon,
- div.sectionContent iron-icon.small {
- width: 16px;
- height: 16px;
- margin-right: var(--spacing-s);
- /* Positioning of a 16px icon in the middle of a 20px line. */
- position: relative;
- top: 2px;
- }
- div.sectionContent .attemptIcon iron-icon {
- margin-right: 0;
- }
- .attemptIcon,
- .attemptNumber {
- margin-right: var(--spacing-s);
- color: var(--deemphasized-text-color);
- text-align: center;
- width: 24px;
- font-size: var(--font-size-small);
- }
- div.action {
- border-top: 1px solid var(--border-color);
- margin-top: var(--spacing-m);
- padding: var(--spacing-m) var(--spacing-xl) 0;
- }
- </style>
- <div id="container" role="tooltip" tabindex="-1">
- <div class="section">
- <div hidden$="[[hideChip(run)]]" class="chipRow">
- <div class="chip">
- <iron-icon icon="gr-icons:[[computeChipIcon(run)]]"></iron-icon>
- <span>[[run.status]]</span>
- </div>
- </div>
- </div>
- <div class="section">
- <div class="sectionIcon" hidden$="[[hideHeaderSectionIcon(run)]]">
- <iron-icon
- class$="[[computeIcon(run)]]"
- icon="gr-icons:[[computeIcon(run)]]"
- ></iron-icon>
- </div>
- <div class="sectionContent">
- <h3 class="name heading-3">
- <span>[[run.checkName]]</span>
- </h3>
- </div>
- </div>
- <div class="section" hidden$="[[hideStatusSection(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:info-outline"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[!run.statusLink]]" class="row">
- <div class="title">Status</div>
- <div>
- <a href="[[_convertUndefined(run.statusLink)]]" target="_blank"
- ><iron-icon
- aria-label="external link to check status"
- class="small link"
- icon="gr-icons:launch"
- ></iron-icon
- >[[computeHostName(run.statusLink)]]
- </a>
- </div>
- </div>
- <div hidden$="[[!run.statusDescription]]" class="row">
- <div class="title">Message</div>
- <div>[[run.statusDescription]]</div>
- </div>
- </div>
- </div>
- <div class="section" hidden$="[[hideAttempts(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:arrow-forward"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[hideAttempts(run)]]" class="attempts row">
- <div class="title">Attempt</div>
- <template is="dom-repeat" items="[[computeAttempts(run)]]">
- <div>
- <div class="attemptIcon">
- <iron-icon
- class$="[[item.icon]]"
- icon="gr-icons:[[item.icon]]"
- ></iron-icon>
- </div>
- <div class="attemptNumber">[[computeAttempt(item.attempt)]]</div>
- </div>
- </template>
- </div>
- </div>
- </div>
- <div class="section" hidden$="[[hideTimestampSection(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:schedule"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[hideScheduled(run)]]" class="row">
- <div class="title">Scheduled</div>
- <div>[[computeDuration(run.scheduledTimestamp)]]</div>
- </div>
- <div hidden$="[[!run.startedTimestamp]]" class="row">
- <div class="title">Started</div>
- <div>[[computeDuration(run.startedTimestamp)]]</div>
- </div>
- <div hidden$="[[!run.finishedTimestamp]]" class="row">
- <div class="title">Ended</div>
- <div>[[computeDuration(run.finishedTimestamp)]]</div>
- </div>
- <div hidden$="[[hideCompletion(run)]]" class="row">
- <div class="title">Completion</div>
- <div>[[computeCompletionDuration(run)]]</div>
- </div>
- </div>
- </div>
- <div class="section" hidden$="[[hideDescriptionSection(run)]]">
- <div class="sectionIcon">
- <iron-icon class="small" icon="gr-icons:link"></iron-icon>
- </div>
- <div class="sectionContent">
- <div hidden$="[[!run.checkDescription]]" class="row">
- <div class="title">Description</div>
- <div>[[run.checkDescription]]</div>
- </div>
- <div hidden$="[[!run.checkLink]]" class="row">
- <div class="title">Documentation</div>
- <div>
- <a href="[[_convertUndefined(run.checkLink)]]" target="_blank"
- ><iron-icon
- aria-label="external link to check documentation"
- class="small link"
- icon="gr-icons:launch"
- ></iron-icon
- >[[computeHostName(run.checkLink)]]
- </a>
- </div>
- </div>
- </div>
- </div>
- <template is="dom-repeat" items="[[computeActions(run)]]">
- <div class="action">
- <gr-checks-action
- event-target="[[_target]]"
- action="[[item]]"
- ></gr-checks-action>
- </div>
- </template>
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
index 67781f5..352219a 100644
--- a/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
+++ b/polygerrit-ui/app/elements/checks/gr-hovercard-run_test.ts
@@ -33,7 +33,7 @@
});
teardown(() => {
- element.hide();
+ element.hide(new MouseEvent('click'));
});
test('hovercard is shown', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index e2baec9..5b48345 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -1120,12 +1120,10 @@
return;
}
- this._change = undefined;
this._files = {sortedFileList: [], changeFilesByPath: {}};
this._path = undefined;
this._patchRange = undefined;
this._commitRange = undefined;
- this._changeComments = undefined;
this._focusLineNum = undefined;
if (value.changeNum && value.project) {
@@ -1156,8 +1154,9 @@
})
);
- promises.push(this._getChangeDetail(this._changeNum));
- this._loadComments(value.patchNum);
+ if (!this._change) promises.push(this._getChangeDetail(this._changeNum));
+
+ if (!this._changeComments) this._loadComments(value.patchNum);
promises.push(this._getChangeEdit());
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 735624a..2c4750a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -29,6 +29,7 @@
} from '../../../test/test-data-generators.js';
import {EditPatchSetNum} from '../../../types/common.js';
import {CursorMoveResult} from '../../../api/core.js';
+import {EventType} from '../../../types/events.js';
const basicFixture = fixtureFromElement('gr-diff-view');
@@ -323,6 +324,60 @@
assert.equal(element._isFileUnchanged(diff), true);
});
+ test('change detail is not rerequested if changeNum doesnt change',
+ async () => {
+ const dispatchEventStub = sinon.stub(element, 'dispatchEvent');
+ assert.isFalse(getDiffChangeDetailStub.called);
+ sinon.stub(element.reporting, 'diffViewDisplayed');
+ sinon.stub(element, '_loadBlame');
+ sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
+ sinon.spy(element, '_paramsChanged');
+ element._change = undefined;
+ getDiffChangeDetailStub.returns(
+ Promise.resolve({
+ ...createChange(),
+ revisions: createRevisions(11),
+ }));
+ element._patchRange = {
+ patchNum: 2,
+ basePatchNum: 1,
+ };
+ sinon.stub(element, '_isFileUnchanged').returns(false);
+
+ element.params = {
+ view: GerritNav.View.DIFF,
+ changeNum: '42',
+ project: 'p',
+ commentId: 'c1',
+ commentLink: true,
+ };
+ await element._paramsChanged.returnValues[0];
+
+ assert.equal(getDiffChangeDetailStub.callCount, 1);
+ element.params = {
+ view: GerritNav.View.DIFF,
+ changeNum: '42',
+ project: 'p',
+ commentId: 'c1',
+ commentLink: true,
+ };
+ await element._paramsChanged.returnValues[0];
+
+ assert.equal(getDiffChangeDetailStub.callCount, 1);
+ element.params = {
+ view: GerritNav.View.DIFF,
+ changeNum: '43',
+ project: 'p',
+ commentId: 'c1',
+ commentLink: true,
+ };
+ await element._paramsChanged.returnValues[0];
+
+ // change page is recreated now
+ assert.equal(dispatchEventStub.lastCall.args[0].type,
+ EventType.RECREATE_DIFF_VIEW);
+ });
+
test('diff toast to go to latest is shown and not base', async () => {
diffCommentsStub.returns(Promise.resolve({
'/COMMIT_MSG': [
@@ -344,6 +399,7 @@
sinon.stub(element, '_loadBlame');
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
sinon.spy(element, '_paramsChanged');
+ element._change = undefined;
getDiffChangeDetailStub.returns(
Promise.resolve({
...createChange(),
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
index d87b573..1615a23 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls.ts
@@ -36,7 +36,7 @@
} from '../../shared/gr-autocomplete/gr-autocomplete';
import {appContext} from '../../../services/app-context';
import {IronInputElement} from '@polymer/iron-input';
-import {fireAlert} from '../../../utils/event-util';
+import {fireAlert, fireReload} from '../../../utils/event-util';
export interface GrEditControls {
$: {
@@ -237,7 +237,7 @@
return;
}
this._closeDialog(this.$.openDialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this, true);
});
}
@@ -257,7 +257,7 @@
return;
}
this._closeDialog(dialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this);
});
}
@@ -275,7 +275,7 @@
return;
}
this._closeDialog(dialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this);
});
}
@@ -293,7 +293,7 @@
return;
}
this._closeDialog(dialog);
- GerritNav.navigateToChange(this.change);
+ fireReload(this, true);
});
}
diff --git a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
index 0ba68e2..6198f17 100644
--- a/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
+++ b/polygerrit-ui/app/elements/edit/gr-edit-controls/gr-edit-controls_test.ts
@@ -122,12 +122,12 @@
});
suite('delete button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let eventStub: sinon.SinonStub;
let deleteStub: sinon.SinonStub;
let deleteAutocomplete: GrAutocomplete;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToChange');
+ eventStub = sinon.stub(element, 'dispatchEvent');
deleteStub = stubRestApi('deleteFileInChangeEdit');
deleteAutocomplete =
element.$.deleteDialog!.querySelector('gr-autocomplete')!;
@@ -155,7 +155,7 @@
assert.isTrue(deleteStub.called);
await deleteStub.lastCall.returnValue;
assert.equal(element._path, '');
- assert.isTrue(navStub.called);
+ assert.equal(eventStub.firstCall.args[0].type, 'reload');
assert.isTrue(closeDialogSpy.called);
});
@@ -181,7 +181,7 @@
assert.isTrue(deleteStub.called);
await deleteStub.lastCall.returnValue;
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isFalse(closeDialogSpy.called);
});
@@ -195,7 +195,7 @@
MockInteractions.tap(
queryAndAssert(element.$.deleteDialog, 'gr-button')
);
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isTrue(closeDialogSpy.called);
assert.equal(element._path, '');
});
@@ -203,12 +203,12 @@
});
suite('rename button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let eventStub: sinon.SinonStub;
let renameStub: sinon.SinonStub;
let renameAutocomplete: GrAutocomplete;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToChange');
+ eventStub = sinon.stub(element, 'dispatchEvent');
renameStub = stubRestApi('renameFileInChangeEdit');
renameAutocomplete =
element.$.renameDialog!.querySelector('gr-autocomplete')!;
@@ -241,7 +241,7 @@
await renameStub.lastCall.returnValue;
assert.equal(element._path, '');
- assert.isTrue(navStub.called);
+ assert.equal(eventStub.firstCall.args[0].type, 'reload');
assert.isTrue(closeDialogSpy.called);
});
@@ -272,7 +272,7 @@
assert.isTrue(renameStub.called);
await renameStub.lastCall.returnValue;
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isFalse(closeDialogSpy.called);
});
@@ -287,7 +287,7 @@
MockInteractions.tap(
queryAndAssert(element.$.renameDialog, 'gr-button')
);
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isTrue(closeDialogSpy.called);
assert.equal(element._path, '');
assert.equal(element._newPath, '');
@@ -296,11 +296,11 @@
});
suite('restore button CUJ', () => {
- let navStub: sinon.SinonStub;
+ let eventStub: sinon.SinonStub;
let restoreStub: sinon.SinonStub;
setup(() => {
- navStub = sinon.stub(GerritNav, 'navigateToChange');
+ eventStub = sinon.stub(element, 'dispatchEvent');
restoreStub = stubRestApi('restoreFileInChangeEdit');
});
@@ -324,7 +324,7 @@
assert.equal(restoreStub.lastCall.args[1], 'src/test.cpp');
return restoreStub.lastCall.returnValue.then(() => {
assert.equal(element._path, '');
- assert.isTrue(navStub.called);
+ assert.equal(eventStub.firstCall.args[0].type, 'reload');
assert.isTrue(closeDialogSpy.called);
});
});
@@ -343,7 +343,7 @@
assert.isTrue(restoreStub.called);
assert.equal(restoreStub.lastCall.args[1], 'src/test.cpp');
return restoreStub.lastCall.returnValue.then(() => {
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isFalse(closeDialogSpy.called);
});
});
@@ -356,7 +356,7 @@
MockInteractions.tap(
queryAndAssert(element.$.restoreDialog, 'gr-button')
);
- assert.isFalse(navStub.called);
+ assert.isFalse(eventStub.called);
assert.isTrue(closeDialogSpy.called);
assert.equal(element._path, '');
});
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 3b93bea..38f55bd 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -242,7 +242,7 @@
this.addEventListener(EventType.DIALOG_CHANGE, e => {
this._handleDialogChange(e as CustomEvent<DialogChangeEventDetail>);
});
- this.addEventListener('location-change', e =>
+ this.addEventListener(EventType.LOCATION_CHANGE, e =>
this._handleLocationChange(e)
);
this.addEventListener(EventType.RECREATE_CHANGE_VIEW, () =>
@@ -251,7 +251,7 @@
this.addEventListener(EventType.RECREATE_DIFF_VIEW, () =>
this.handleRecreateView(GerritView.DIFF)
);
- document.addEventListener('gr-rpc-log', e => this._handleRpcLog(e));
+ document.addEventListener(EventType.GR_RPC_LOG, e => this._handleRpcLog(e));
}
override ready() {
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index 9897a9f..dabf761 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -199,9 +199,9 @@
${!this.hideHovercard
? html`<gr-hovercard-account
for="hovercardTarget"
- .account="${account}"
- .change="${change}"
- ?highlight-attention=${highlightAttention}
+ .account=${account}
+ .change=${change}
+ .highlightAttention=${highlightAttention}
.voteableText=${this.voteableText}
></gr-hovercard-account>`
: ''}
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
index 3988095..6a34fbb 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts
@@ -16,17 +16,11 @@
*/
import '@polymer/iron-icon/iron-icon';
-import '../../../styles/gr-font-styles';
-import '../../../styles/shared-styles';
-import '../../../styles/gr-hovercard-styles';
import '../gr-avatar/gr-avatar';
import '../gr-button/gr-button';
-import {HovercardBehaviorMixin} from '../gr-hovercard/gr-hovercard-behavior';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-hovercard-account_html';
import {appContext} from '../../../services/app-context';
import {accountKey, isSelf} from '../../../utils/account-util';
-import {customElement, property} from '@polymer/decorators';
+import {customElement, property} from 'lit/decorators';
import {
AccountInfo,
ChangeInfo,
@@ -46,16 +40,15 @@
import {CURRENT} from '../../../utils/patch-set-util';
import {isInvolved, isRemovableReviewer} from '../../../utils/change-util';
import {assertIsDefined} from '../../../utils/common-util';
+import {fontStyles} from '../../../styles/gr-font-styles';
+import {css, html, LitElement} from 'lit';
+import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
-const base = HovercardBehaviorMixin(PolymerElement);
+const base = HovercardMixin(LitElement);
@customElement('gr-hovercard-account')
export class GrHovercardAccount extends base {
- static get template() {
- return htmlTemplate;
- }
-
@property({type: Object})
account!: AccountInfo;
@@ -107,9 +100,214 @@
});
}
- _computeText(account?: AccountInfo, selfAccount?: AccountInfo) {
- if (!account || !selfAccount) return '';
- return isSelf(account, selfAccount) ? 'Your' : 'Their';
+ static override get styles() {
+ return [
+ fontStyles,
+ base.styles || [],
+ css`
+ .top,
+ .attention,
+ .status,
+ .voteable {
+ padding: var(--spacing-s) var(--spacing-l);
+ }
+ .top {
+ display: flex;
+ padding-top: var(--spacing-xl);
+ min-width: 300px;
+ }
+ gr-avatar {
+ height: 48px;
+ width: 48px;
+ margin-right: var(--spacing-l);
+ }
+ .title,
+ .email {
+ color: var(--deemphasized-text-color);
+ }
+ .action {
+ border-top: 1px solid var(--border-color);
+ padding: var(--spacing-s) var(--spacing-l);
+ --gr-button-padding: var(--spacing-s) var(--spacing-m);
+ }
+ .attention {
+ background-color: var(--emphasis-color);
+ }
+ .attention a {
+ text-decoration: none;
+ }
+ iron-icon {
+ vertical-align: top;
+ }
+ .status iron-icon {
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 2px;
+ }
+ iron-icon.attentionIcon {
+ width: 14px;
+ height: 14px;
+ position: relative;
+ top: 3px;
+ }
+ .reason {
+ padding-top: var(--spacing-s);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html`
+ <div id="container" role="tooltip" tabindex="-1">
+ ${this.renderContent()}
+ </div>
+ `;
+ }
+
+ private renderContent() {
+ if (!this._isShowing) return;
+ return html`
+ <div class="top">
+ <div class="avatar">
+ <gr-avautar .account=${this.account} imageSize="56"></gr-avatar>
+ </div>
+ <div class="account">
+ <h3 class="name heading-3">${this.account.name}</h3>
+ <div class="email">${this.account.email}</div>
+ </div>
+ </div>
+ ${this.renderAccountStatus()}
+ ${
+ this.voteableText
+ ? html`
+ <div class="voteable">
+ <span class="title">Voteable:</span>
+ <span class="value">${this.voteableText}</span>
+ </div>
+ `
+ : ''
+ }
+ ${this.renderNeedsAttention()} ${this.renderAddToAttention()}
+ ${this.renderRemoveFromAttention()} ${this.renderReviewerOrCcActions()}
+ `;
+ }
+
+ private renderReviewerOrCcActions() {
+ if (!this._selfAccount || !isRemovableReviewer(this.change, this.account))
+ return;
+ return html`
+ <div class="action">
+ <gr-button
+ class="removeReviewerOrCC"
+ link=""
+ no-uppercase
+ @click="${this.handleRemoveReviewerOrCC}"
+ >
+ Remove ${this.computeReviewerOrCCText()}
+ </gr-button>
+ </div>
+ <div class="action">
+ <gr-button
+ class="changeReviewerOrCC"
+ link=""
+ no-uppercase
+ @click="${this.handleChangeReviewerOrCCStatus}"
+ >
+ ${this.computeChangeReviewerOrCCText()}
+ </gr-button>
+ </div>
+ `;
+ }
+
+ private renderAccountStatus() {
+ if (!this.account.status) return;
+ return html`
+ <div class="status">
+ <span class="title">
+ <iron-icon icon="gr-icons:calendar"></iron-icon>
+ Status:
+ </span>
+ <span class="value">${this.account.status}</span>
+ </div>
+ `;
+ }
+
+ private renderNeedsAttention() {
+ if (!(this.isAttentionEnabled && this.hasUserAttention)) return;
+ const lastUpdate = getLastUpdate(this.account, this.change);
+ return html`
+ <div class="attention">
+ <div>
+ <iron-icon
+ class="attentionIcon"
+ icon="gr-icons:attention"
+ ></iron-icon>
+ <span> ${this.computePronoun()} turn to take this action. </span>
+ <a
+ href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html"
+ target="_blank"
+ >
+ <iron-icon
+ icon="gr-icons:help-outline"
+ title="read documentation"
+ ></iron-icon>
+ </a>
+ </div>
+ <div class="reason">
+ <span class="title">Reason:</span>
+ <span class="value">
+ ${getReason(this._config, this.account, this.change)}
+ </span>
+ ${lastUpdate
+ ? html` (<gr-date-formatter
+ withTooltip
+ .dateStr="${lastUpdate}"
+ ></gr-date-formatter
+ >)`
+ : ''}
+ </div>
+ </div>
+ `;
+ }
+
+ private renderAddToAttention() {
+ if (!this.computeShowActionAddToAttentionSet()) return;
+ return html`
+ <div class="action">
+ <gr-button
+ class="addToAttentionSet"
+ link=""
+ no-uppercase
+ @click="${this.handleClickAddToAttentionSet}"
+ >
+ Add to attention set
+ </gr-button>
+ </div>
+ `;
+ }
+
+ private renderRemoveFromAttention() {
+ if (!this.computeShowActionRemoveFromAttentionSet()) return;
+ return html`
+ <div class="action">
+ <gr-button
+ class="removeFromAttentionSet"
+ link=""
+ no-uppercase
+ @click="${this.handleClickRemoveFromAttentionSet}"
+ >
+ Remove from attention set
+ </gr-button>
+ </div>
+ `;
+ }
+
+ // private but used by tests
+ computePronoun() {
+ if (!this.account || !this._selfAccount) return '';
+ return isSelf(this.account, this._selfAccount) ? 'Your' : 'Their';
}
get isAttentionEnabled() {
@@ -124,27 +322,11 @@
return hasAttention(this.account, this.change);
}
- _computeReason(change?: ChangeInfo) {
- return getReason(this._config, this.account, change);
- }
-
- _computeLastUpdate(change?: ChangeInfo) {
- return getLastUpdate(this.account, change);
- }
-
- /** 3rd parameter is just for *triggering* re-computation. */
- _showReviewerOrCCActions(
- account?: AccountInfo,
- change?: ChangeInfo,
- _?: unknown
- ) {
- return !!this._selfAccount && isRemovableReviewer(change, account);
- }
-
- _getReviewerState(account: AccountInfo, change: ChangeInfo) {
+ private getReviewerState() {
if (
- change.reviewers[ReviewerState.REVIEWER]?.some(
- (reviewer: AccountInfo) => reviewer._account_id === account._account_id
+ this.change!.reviewers[ReviewerState.REVIEWER]?.some(
+ (reviewer: AccountInfo) =>
+ reviewer._account_id === this.account._account_id
)
) {
return ReviewerState.REVIEWER;
@@ -152,21 +334,21 @@
return ReviewerState.CC;
}
- _computeReviewerOrCCText(account?: AccountInfo, change?: ChangeInfo) {
- if (!change || !account) return '';
- return this._getReviewerState(account, change) === ReviewerState.REVIEWER
+ private computeReviewerOrCCText() {
+ if (!this.change || !this.account) return '';
+ return this.getReviewerState() === ReviewerState.REVIEWER
? 'Reviewer'
: 'CC';
}
- _computeChangeReviewerOrCCText(account?: AccountInfo, change?: ChangeInfo) {
- if (!change || !account) return '';
- return this._getReviewerState(account, change) === ReviewerState.REVIEWER
+ private computeChangeReviewerOrCCText() {
+ if (!this.change || !this.account) return '';
+ return this.getReviewerState() === ReviewerState.REVIEWER
? 'Move Reviewer to CC'
: 'Move CC to Reviewer';
}
- _handleChangeReviewerOrCCStatus() {
+ private handleChangeReviewerOrCCStatus() {
assertIsDefined(this.change, 'change');
// accountKey() throws an error if _account_id & email is not found, which
// we want to check before showing reloading toast
@@ -179,7 +361,7 @@
{
reviewer: _accountKey,
state:
- this._getReviewerState(this.account, this.change) === ReviewerState.CC
+ this.getReviewerState() === ReviewerState.CC
? ReviewerState.REVIEWER
: ReviewerState.CC,
},
@@ -190,15 +372,14 @@
.then(response => {
if (!response || !response.ok) {
throw new Error(
- 'something went wrong when toggling' +
- this._getReviewerState(this.account, this.change!)
+ 'something went wrong when toggling' + this.getReviewerState()
);
}
this.dispatchEventThroughTarget('reload', {clearPatchset: true});
});
}
- _handleRemoveReviewerOrCC() {
+ private handleRemoveReviewerOrCC() {
if (!this.change || !(this.account?._account_id || this.account?.email))
throw new Error('Missing change or account.');
this.dispatchEventThroughTarget('show-alert', {
@@ -218,45 +399,21 @@
});
}
- /** Parameters are just for *triggering* re-computation. */
- _computeShowLabelNeedsAttention(
- _1: unknown,
- _2: unknown,
- _3: unknown,
- _4: unknown
- ) {
- return this.isAttentionEnabled && this.hasUserAttention;
- }
-
- /** Parameters are just for *triggering* re-computation. */
- _computeShowActionAddToAttentionSet(
- _1: unknown,
- _2: unknown,
- _3: unknown,
- _4: unknown,
- _5: unknown
- ) {
+ private computeShowActionAddToAttentionSet() {
const involvedOrSelf =
isInvolved(this.change, this._selfAccount) ||
isSelf(this.account, this._selfAccount);
return involvedOrSelf && this.isAttentionEnabled && !this.hasUserAttention;
}
- /** Parameters are just for *triggering* re-computation. */
- _computeShowActionRemoveFromAttentionSet(
- _1: unknown,
- _2: unknown,
- _3: unknown,
- _4: unknown,
- _5: unknown
- ) {
+ private computeShowActionRemoveFromAttentionSet() {
const involvedOrSelf =
isInvolved(this.change, this._selfAccount) ||
isSelf(this.account, this._selfAccount);
return involvedOrSelf && this.isAttentionEnabled && this.hasUserAttention;
}
- _handleClickAddToAttentionSet() {
+ private handleClickAddToAttentionSet(e: MouseEvent) {
if (!this.change || !this.account._account_id) return;
this.dispatchEventThroughTarget('show-alert', {
message: 'Saving attention set update ...',
@@ -277,17 +434,17 @@
this.reporting.reportInteraction(
'attention-hovercard-add',
- this._reportingDetails()
+ this.reportingDetails()
);
this.restApiService
.addToAttentionSet(this.change._number, this.account._account_id, reason)
.then(() => {
this.dispatchEventThroughTarget('hide-alert');
});
- this.hide();
+ this.hide(e);
}
- _handleClickRemoveFromAttentionSet() {
+ private handleClickRemoveFromAttentionSet(e: MouseEvent) {
if (!this.change || !this.account._account_id) return;
this.dispatchEventThroughTarget('show-alert', {
message: 'Saving attention set update ...',
@@ -304,7 +461,7 @@
this.reporting.reportInteraction(
'attention-hovercard-remove',
- this._reportingDetails()
+ this.reportingDetails()
);
this.restApiService
.removeFromAttentionSet(
@@ -315,10 +472,10 @@
.then(() => {
this.dispatchEventThroughTarget('hide-alert');
});
- this.hide();
+ this.hide(e);
}
- _reportingDetails() {
+ private reportingDetails() {
const targetId = this.account._account_id;
const ownerId =
(this.change && this.change.owner && this.change.owner._account_id) || -1;
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
deleted file mode 100644
index cba7293..0000000
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts
+++ /dev/null
@@ -1,197 +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 '../../../styles/gr-hovercard-styles';
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="shared-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-hovercard-styles">
- .top,
- .attention,
- .status,
- .voteable {
- padding: var(--spacing-s) var(--spacing-l);
- }
- .top {
- display: flex;
- padding-top: var(--spacing-xl);
- min-width: 300px;
- }
- gr-avatar {
- height: 48px;
- width: 48px;
- margin-right: var(--spacing-l);
- }
- .title,
- .email {
- color: var(--deemphasized-text-color);
- }
- .action {
- border-top: 1px solid var(--border-color);
- padding: var(--spacing-s) var(--spacing-l);
- --gr-button-padding: var(--spacing-s) var(--spacing-m);
- }
- .attention {
- background-color: var(--emphasis-color);
- }
- .attention a {
- text-decoration: none;
- }
- iron-icon {
- vertical-align: top;
- }
- .status iron-icon {
- width: 14px;
- height: 14px;
- position: relative;
- top: 2px;
- }
- iron-icon.attentionIcon {
- width: 14px;
- height: 14px;
- position: relative;
- top: 3px;
- }
- .reason {
- padding-top: var(--spacing-s);
- }
- </style>
- <div id="container" role="tooltip" tabindex="-1">
- <template is="dom-if" if="[[_isShowing]]">
- <div class="top">
- <div class="avatar">
- <gr-avatar account="[[account]]" imageSize="56"></gr-avatar>
- </div>
- <div class="account">
- <h3 class="name heading-3">[[account.name]]</h3>
- <div class="email">[[account.email]]</div>
- </div>
- </div>
- <template is="dom-if" if="[[account.status]]">
- <div class="status">
- <span class="title">
- <iron-icon icon="gr-icons:calendar"></iron-icon>
- Status:
- </span>
- <span class="value">[[account.status]]</span>
- </div>
- </template>
- <template is="dom-if" if="[[voteableText]]">
- <div class="voteable">
- <span class="title">Voteable:</span>
- <span class="value">[[voteableText]]</span>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_computeShowLabelNeedsAttention(_config, highlightAttention, account, change)]]"
- >
- <div class="attention">
- <div>
- <iron-icon
- class="attentionIcon"
- icon="gr-icons:attention"
- ></iron-icon>
- <span>
- [[_computeText(account, _selfAccount)]] turn to take action.
- </span>
- <a
- href="https://gerrit-review.googlesource.com/Documentation/user-attention-set.html"
- target="_blank"
- >
- <iron-icon
- icon="gr-icons:help-outline"
- title="read documentation"
- ></iron-icon>
- </a>
- </div>
- <div class="reason">
- <span class="title">Reason:</span>
- <span class="value">[[_computeReason(change)]]</span>
- <template is="dom-if" if="[[_computeLastUpdate(change)]]">
- (<gr-date-formatter
- withTooltip
- date-str="[[_computeLastUpdate(change)]]"
- ></gr-date-formatter
- >)
- </template>
- </div>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_computeShowActionAddToAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
- >
- <div class="action">
- <gr-button
- class="addToAttentionSet"
- link=""
- no-uppercase=""
- on-click="_handleClickAddToAttentionSet"
- >
- Add to attention set
- </gr-button>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_computeShowActionRemoveFromAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
- >
- <div class="action">
- <gr-button
- class="removeFromAttentionSet"
- link=""
- no-uppercase=""
- on-click="_handleClickRemoveFromAttentionSet"
- >
- Remove from attention set
- </gr-button>
- </div>
- </template>
- <template
- is="dom-if"
- if="[[_showReviewerOrCCActions(account, change, _selfAccount)]]"
- >
- <div class="action">
- <gr-button
- class="removeReviewerOrCC"
- link=""
- no-uppercase=""
- on-click="_handleRemoveReviewerOrCC"
- >
- Remove [[_computeReviewerOrCCText(account, change)]]
- </gr-button>
- </div>
- <div class="action">
- <gr-button
- class="changeReviewerOrCC"
- link=""
- no-uppercase=""
- on-click="_handleChangeReviewerOrCCStatus"
- >
- [[_computeChangeReviewerOrCCText(account, change)]]
- </gr-button>
- </div>
- </template>
- </template>
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
index 82f64d0..5530d7c 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js
@@ -19,7 +19,7 @@
import './gr-hovercard-account.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {ReviewerState} from '../../../constants/constants.js';
-import {stubRestApi} from '../../../test/test-utils.js';
+import {mockPromise, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromTemplate(html`
<gr-hovercard-account class="hovered"></gr-hovercard-account>
@@ -57,33 +57,21 @@
'Kermit The Frog');
});
- test('_computeLastUpdate', () => {
- const last_update = '2019-07-17 19:39:02.000000000';
- const change = {
- attention_set: {
- 31415926535: {
- last_update,
- },
- },
- };
- assert.equal(element._computeLastUpdate(change), last_update);
- });
-
- test('_computeText', () => {
- let account = {_account_id: '1'};
- const selfAccount = {_account_id: '1'};
- assert.equal(element._computeText(account, selfAccount), 'Your');
- account = {_account_id: '2'};
- assert.equal(element._computeText(account, selfAccount), 'Their');
+ test('computePronoun', () => {
+ element.account = {_account_id: '1'};
+ element._selfAccount = {_account_id: '1'};
+ assert.equal(element.computePronoun(), 'Your');
+ element.account = {_account_id: '2'};
+ assert.equal(element.computePronoun(), 'Their');
});
test('account status is not shown if the property is not set', () => {
assert.isNull(element.shadowRoot.querySelector('.status'));
});
- test('account status is displayed', () => {
+ test('account status is displayed', async () => {
element.account = {status: 'OOO', ...ACCOUNT};
- flush();
+ await element.updateComplete;
assert.equal(element.shadowRoot.querySelector('.status .value').innerText,
'OOO');
});
@@ -92,9 +80,9 @@
assert.isNull(element.shadowRoot.querySelector('.voteable'));
});
- test('voteable div is displayed', () => {
+ test('voteable div is displayed', async () => {
element.voteableText = 'CodeReview: +2';
- flush();
+ await element.updateComplete;
assert.equal(element.shadowRoot.querySelector('.voteable .value').innerText,
element.voteableText);
});
@@ -106,15 +94,15 @@
[ReviewerState.REVIEWER]: [ACCOUNT],
},
};
+ await element.updateComplete;
stubRestApi('removeChangeReviewer').returns(Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.removeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Remove Reviewer');
MockInteractions.tap(button);
- await flush();
+ await element.updateComplete;
assert.isTrue(reloadListener.called);
});
@@ -125,6 +113,7 @@
[ReviewerState.REVIEWER]: [ACCOUNT],
},
};
+ await element.updateComplete;
const saveReviewStub = stubRestApi(
'saveChangeReview').returns(
Promise.resolve({ok: true}));
@@ -132,14 +121,12 @@
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.changeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Move Reviewer to CC');
MockInteractions.tap(button);
- await flush();
-
+ await element.updateComplete;
assert.isTrue(saveReviewStub.called);
assert.isTrue(reloadListener.called);
});
@@ -151,20 +138,19 @@
[ReviewerState.REVIEWER]: [],
},
};
+ await element.updateComplete;
const saveReviewStub = stubRestApi(
'saveChangeReview').returns(Promise.resolve({ok: true}));
stubRestApi('removeChangeReviewer').returns(Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.changeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Move CC to Reviewer');
MockInteractions.tap(button);
- await flush();
-
+ await element.updateComplete;
assert.isTrue(saveReviewStub.called);
assert.isTrue(reloadListener.called);
});
@@ -176,31 +162,26 @@
[ReviewerState.REVIEWER]: [],
},
};
+ await element.updateComplete;
stubRestApi('removeChangeReviewer').returns(Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
- await flush();
const button = element.shadowRoot.querySelector('.removeReviewerOrCC');
assert.equal(button.innerText, 'Remove CC');
assert.isOk(button);
MockInteractions.tap(button);
-
- await flush();
-
+ await element.updateComplete;
assert.isTrue(reloadListener.called);
});
test('add to attention set', async () => {
- let apiResolve;
- const apiPromise = new Promise(r => {
- apiResolve = r;
- });
+ const apiPromise = mockPromise();
const apiSpy = stubRestApi('addToAttentionSet').returns(apiPromise);
element.highlightAttention = true;
element._target = document.createElement('div');
- flush();
+ await element.updateComplete;
const showAlertListener = sinon.spy();
const hideAlertListener = sinon.spy();
const updatedListener = sinon.spy();
@@ -224,8 +205,8 @@
assert.isTrue(updatedListener.called, 'updatedListener was called');
assert.isFalse(element._isShowing, 'hovercard is hidden');
- apiResolve({});
- await flush();
+ apiPromise.resolve({});
+ await element.updateComplete;
assert.isTrue(apiSpy.calledOnce);
assert.equal(apiSpy.lastCall.args[2],
`Added by <GERRIT_ACCOUNT_${ACCOUNT._account_id}>`
@@ -234,10 +215,7 @@
});
test('remove from attention set', async () => {
- let apiResolve;
- const apiPromise = new Promise(r => {
- apiResolve = r;
- });
+ const apiPromise = mockPromise();
const apiSpy = stubRestApi('removeFromAttentionSet').returns(apiPromise);
element.highlightAttention = true;
element.change = {
@@ -246,7 +224,7 @@
owner: {...ACCOUNT},
};
element._target = document.createElement('div');
- flush();
+ await element.updateComplete;
const showAlertListener = sinon.spy();
const hideAlertListener = sinon.spy();
const updatedListener = sinon.spy();
@@ -264,8 +242,8 @@
assert.isTrue(updatedListener.called, 'updatedListener was called');
assert.isFalse(element._isShowing, 'hovercard is hidden');
- apiResolve({});
- await flush();
+ apiPromise.resolve({});
+ await element.updateComplete;
assert.isTrue(apiSpy.calledOnce);
assert.equal(apiSpy.lastCall.args[2],
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
deleted file mode 100644
index 3d8702b..0000000
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
+++ /dev/null
@@ -1,487 +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 '../../../styles/shared-styles';
-import {flush} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {getRootElement} from '../../../scripts/rootElement';
-import {Constructor} from '../../../utils/common-util';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {observe, property} from '@polymer/decorators';
-import {
- pushScrollLock,
- removeScrollLock,
-} from '@polymer/iron-overlay-behavior/iron-scroll-manager';
-import {ShowAlertEventDetail} from '../../../types/events';
-import {debounce, DelayedTask} from '../../../utils/async-util';
-interface ReloadEventDetail {
- clearPatchset?: boolean;
-}
-
-const HOVER_CLASS = 'hovered';
-const HIDE_CLASS = 'hide';
-
-/**
- * ID for the container element.
- */
-const containerId = 'gr-hovercard-container';
-
-export function getHovercardContainer(
- options: {createIfNotExists: boolean} = {createIfNotExists: false}
-): HTMLElement | null {
- let container = getRootElement().querySelector<HTMLElement>(
- `#${containerId}`
- );
- if (!container && options.createIfNotExists) {
- // If it does not exist, create and initialize the hovercard container.
- container = document.createElement('div');
- container.setAttribute('id', containerId);
- getRootElement().appendChild(container);
- }
- return container;
-}
-
-/**
- * How long should we wait before showing the hovercard when the user hovers
- * over the element?
- */
-const SHOW_DELAY_MS = 550;
-
-/**
- * How long should we wait before hiding the hovercard when the user moves from
- * target to the hovercard.
- *
- * Note: this should be lower than SHOW_DELAY_MS to avoid flickering.
- */
-const HIDE_DELAY_MS = 500;
-
-/**
- * The mixin for gr-hovercard-behavior.
- *
- * @example
- *
- * class YourComponent extends hovercardBehaviorMixin(
- * PolymerElement
- *
- * @see gr-hovercard.ts
- *
- * // following annotations are required for polylint
- * @polymer
- * @mixinFunction
- */
-export const HovercardBehaviorMixin = <T extends Constructor<PolymerElement>>(
- superClass: T
-) => {
- /**
- * @polymer
- * @mixinClass
- */
- class Mixin extends superClass {
- @property({type: Object})
- _target: HTMLElement | null = null;
-
- // Determines whether or not the hovercard is visible.
- @property({type: Boolean})
- _isShowing = false;
-
- // The `id` of the element that the hovercard is anchored to.
- @property({type: String})
- for?: string;
-
- /**
- * The spacing between the top of the hovercard and the element it is
- * anchored to.
- */
- @property({type: Number})
- offset = 14;
-
- /**
- * Positions the hovercard to the top, right, bottom, left, bottom-left,
- * bottom-right, top-left, or top-right of its content.
- */
- @property({type: String})
- position = 'right';
-
- @property({type: Object})
- container: HTMLElement | null = null;
-
- private hideTask?: DelayedTask;
-
- private showTask?: DelayedTask;
-
- private isScheduledToShow?: boolean;
-
- private isScheduledToHide?: boolean;
-
- override connectedCallback() {
- super.connectedCallback();
- if (!this._target) {
- this._target = this.target;
- this.addTargetEventListeners();
- }
-
- // show the hovercard if mouse moves to hovercard
- // this will cancel pending hide as well
- this.addEventListener('mouseenter', this.show);
- this.addEventListener('mouseenter', this.lock);
- // when leave hovercard, hide it immediately
- this.addEventListener('mouseleave', this.hide);
- this.addEventListener('mouseleave', this.unlock);
- }
-
- override disconnectedCallback() {
- this.cancelShowTask();
- this.cancelHideTask();
- this.unlock();
- super.disconnectedCallback();
- }
-
- addTargetEventListeners() {
- this._target?.addEventListener('mouseenter', this.debounceShow);
- this._target?.addEventListener('focus', this.debounceShow);
- this._target?.addEventListener('mouseleave', this.debounceHide);
- this._target?.addEventListener('blur', this.debounceHide);
- this._target?.addEventListener('click', this.hide);
- }
-
- removeTargetEventListeners() {
- this._target?.removeEventListener('mouseenter', this.debounceShow);
- this._target?.removeEventListener('focus', this.debounceShow);
- this._target?.removeEventListener('mouseleave', this.debounceHide);
- this._target?.removeEventListener('blur', this.debounceHide);
- this._target?.removeEventListener('click', this.hide);
- }
-
- override ready() {
- super.ready();
- // First, check to see if the container has already been created.
- this.container = getHovercardContainer({createIfNotExists: true});
- }
-
- readonly debounceHide = () => {
- this.cancelShowTask();
- if (!this._isShowing || this.isScheduledToHide) return;
- this.isScheduledToHide = true;
- this.hideTask = debounce(
- this.hideTask,
- () => {
- // This happens when hide immediately through click or mouse leave
- // on the hovercard
- if (!this.isScheduledToHide) return;
- this.hide();
- },
- HIDE_DELAY_MS
- );
- };
-
- cancelHideTask() {
- if (!this.hideTask) return;
- this.hideTask.cancel();
- this.isScheduledToHide = false;
- this.hideTask = undefined;
- }
-
- /**
- * Hovercard elements are created outside of <gr-app>, so if you want to fire
- * events, then you probably want to do that through the target element.
- */
-
- dispatchEventThroughTarget(eventName: string): void;
-
- dispatchEventThroughTarget(
- eventName: 'show-alert',
- detail: ShowAlertEventDetail
- ): void;
-
- dispatchEventThroughTarget(
- eventName: 'reload',
- detail: ReloadEventDetail
- ): void;
-
- dispatchEventThroughTarget(eventName: string, detail?: unknown) {
- if (!detail) detail = {};
- if (this._target)
- this._target.dispatchEvent(
- new CustomEvent(eventName, {
- detail,
- bubbles: true,
- composed: true,
- })
- );
- }
-
- /**
- * Returns the target element that the hovercard is anchored to (the `id` of
- * the `for` property).
- */
- get target(): HTMLElement {
- const parentNode = this.parentNode;
- // If the parentNode is a document fragment, then we need to use the host.
- const ownerRoot = this.getRootNode() as ShadowRoot;
- let target;
- if (this.for) {
- target = ownerRoot.querySelector('#' + this.for);
- } else {
- target =
- !parentNode || parentNode.nodeType === Node.DOCUMENT_FRAGMENT_NODE
- ? ownerRoot.host
- : parentNode;
- }
- return target as HTMLElement;
- }
-
- /**
- * unlock scroll, this will resume the scroll outside of the hovercard.
- */
- readonly unlock = () => {
- removeScrollLock(this);
- };
-
- /**
- * Hides/closes the hovercard. This occurs when the user triggers the
- * `mouseleave` event on the hovercard's `target` element (as long as the
- * user is not hovering over the hovercard).
- *
- */
- readonly hide = (e?: MouseEvent) => {
- this.cancelHideTask();
- this.cancelShowTask();
- if (!this._isShowing) {
- return;
- }
-
- // If the user is now hovering over the hovercard or the user is returning
- // from the hovercard but now hovering over the target (to stop an annoying
- // flicker effect), just return.
- if (e) {
- if (
- e.relatedTarget === this ||
- (e.target === this && e.relatedTarget === this._target)
- ) {
- return;
- }
- }
-
- // Mark that the hovercard is not visible and do not allow focusing
- this._isShowing = false;
-
- // Clear styles in preparation for the next time we need to show the card
- this.classList.remove(HOVER_CLASS);
-
- // Reset and remove the hovercard from the DOM
- this.style.cssText = '';
- this.$['container'].setAttribute('tabindex', '-1');
-
- // Remove the hovercard from the container, given that it is still a child
- // of the container.
- if (this.container?.contains(this)) {
- this.container.removeChild(this);
- }
- };
-
- /**
- * Shows/opens the hovercard with a fixed delay.
- */
- readonly debounceShow = () => {
- this.debounceShowBy(SHOW_DELAY_MS);
- };
-
- /**
- * Shows/opens the hovercard with the given delay.
- */
- debounceShowBy(delayMs: number) {
- this.cancelHideTask();
- if (this._isShowing || this.isScheduledToShow) return;
- this.isScheduledToShow = true;
- this.showTask = debounce(
- this.showTask,
- () => {
- // This happens when the mouse leaves the target before the delay is over.
- if (!this.isScheduledToShow) return;
- this.show();
- },
- delayMs
- );
- }
-
- cancelShowTask() {
- if (!this.showTask) return;
- this.showTask.cancel();
- this.isScheduledToShow = false;
- this.showTask = undefined;
- }
-
- /**
- * Lock background scroll but enable scroll inside of current hovercard.
- */
- readonly lock = () => {
- pushScrollLock(this);
- };
-
- /**
- * Shows/opens the hovercard. This occurs when the user triggers the
- * `mousenter` event on the hovercard's `target` element.
- */
- readonly show = async () => {
- this.cancelHideTask();
- this.cancelShowTask();
- if (this._isShowing || !this.container) {
- return;
- }
-
- // Mark that the hovercard is now visible
- this._isShowing = true;
- this.setAttribute('tabindex', '0');
-
- // Add it to the DOM and calculate its position
- this.container.appendChild(this);
- // We temporarily hide the hovercard until we have found the correct
- // position for it.
- this.classList.add(HIDE_CLASS);
- this.classList.add(HOVER_CLASS);
- // Make sure that the hovercard actually rendered and all dom-if
- // statements processed, so that we can measure the (invisible)
- // hovercard properly in updatePosition().
- await flush();
- this.updatePosition();
- this.classList.remove(HIDE_CLASS);
- };
-
- updatePosition() {
- const positionsToTry = new Set([
- this.position,
- 'right',
- 'bottom-right',
- 'top-right',
- 'bottom',
- 'top',
- 'bottom-left',
- 'top-left',
- 'left',
- ]);
- for (const position of positionsToTry) {
- this.updatePositionTo(position);
- if (this._isInsideViewport()) return;
- }
- console.warn('Could not find a visible position for the hovercard.');
- }
-
- _isInsideViewport() {
- const thisRect = this.getBoundingClientRect();
- if (thisRect.top < 0) return false;
- if (thisRect.left < 0) return false;
- const docuRect = document.documentElement.getBoundingClientRect();
- if (thisRect.bottom > docuRect.height) return false;
- if (thisRect.right > docuRect.width) return false;
- return true;
- }
-
- /**
- * Updates the hovercard's position based the current position of the `target`
- * element.
- *
- * The hovercard is supposed to stay open if the user hovers over it.
- * To keep it open when the user moves away from the target, the bounding
- * rects of the target and hovercard must touch or overlap.
- *
- * NOTE: You do not need to directly call this method unless you need to
- * update the position of the tooltip while it is already visible (the
- * target element has moved and the tooltip is still open).
- */
- updatePositionTo(position: string) {
- if (!this._target) {
- return;
- }
-
- // Make sure that thisRect will not get any paddings and such included
- // in the width and height of the bounding client rect.
- this.style.cssText = '';
-
- const docuRect = document.documentElement.getBoundingClientRect();
- const targetRect = this._target.getBoundingClientRect();
- const thisRect = this.getBoundingClientRect();
-
- const targetLeft = targetRect.left - docuRect.left;
- const targetTop = targetRect.top - docuRect.top;
-
- let hovercardLeft;
- let hovercardTop;
-
- switch (position) {
- case 'top':
- hovercardLeft = targetLeft + (targetRect.width - thisRect.width) / 2;
- hovercardTop = targetTop - thisRect.height - this.offset;
- break;
- case 'bottom':
- hovercardLeft = targetLeft + (targetRect.width - thisRect.width) / 2;
- hovercardTop = targetTop + targetRect.height + this.offset;
- break;
- case 'left':
- hovercardLeft = targetLeft - thisRect.width - this.offset;
- hovercardTop = targetTop + (targetRect.height - thisRect.height) / 2;
- break;
- case 'right':
- hovercardLeft = targetLeft + targetRect.width + this.offset;
- hovercardTop = targetTop + (targetRect.height - thisRect.height) / 2;
- break;
- case 'bottom-right':
- hovercardLeft = targetLeft + targetRect.width + this.offset;
- hovercardTop = targetTop;
- break;
- case 'bottom-left':
- hovercardLeft = targetLeft - thisRect.width - this.offset;
- hovercardTop = targetTop;
- break;
- case 'top-left':
- hovercardLeft = targetLeft - thisRect.width - this.offset;
- hovercardTop = targetTop + targetRect.height - thisRect.height;
- break;
- case 'top-right':
- hovercardLeft = targetLeft + targetRect.width + this.offset;
- hovercardTop = targetTop + targetRect.height - thisRect.height;
- break;
- }
-
- this.style.left = `${hovercardLeft}px`;
- this.style.top = `${hovercardTop}px`;
- }
-
- /**
- * Responds to a change in the `for` value and gets the updated `target`
- * element for the hovercard.
- */
- @observe('for')
- _forChanged() {
- this.removeTargetEventListeners();
- this._target = this.target;
- this.addTargetEventListeners();
- }
- }
-
- return Mixin as T & Constructor<GrHovercardBehaviorInterface>;
-};
-
-export interface GrHovercardBehaviorInterface {
- _target: HTMLElement | null;
- _isShowing: boolean;
- ready(): void;
- dispatchEventThroughTarget(eventName: string, detail?: unknown): void;
- hide(e?: MouseEvent): void;
- debounceShow(): void;
- debounceShowBy(delayMs: number): void;
- cancelShowTask(): void;
- show(): void;
- updatePosition(): void;
-}
diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts
index 5fc53e6..bf35c06 100644
--- a/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts
+++ b/polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard.ts
@@ -18,8 +18,6 @@
import {customElement} from 'lit/decorators';
import {HovercardMixin} from '../../../mixins/hovercard-mixin/hovercard-mixin';
import {css, html, LitElement} from 'lit';
-import {hovercardStyles} from '../../../styles/gr-hovercard-styles';
-import {sharedStyles} from '../../../styles/shared-styles';
// This avoids JSC_DYNAMIC_EXTENDS_WITHOUT_JSDOC closure compiler error.
const base = HovercardMixin(LitElement);
@@ -28,8 +26,7 @@
export class GrHovercard extends base {
static override get styles() {
return [
- sharedStyles,
- hovercardStyles,
+ base.styles ?? [],
css`
#container {
padding: var(--spacing-l);
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
index 5a6d821..2df2ccb 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info.ts
@@ -24,7 +24,6 @@
import '../gr-label/gr-label';
import '../gr-tooltip-content/gr-tooltip-content';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {
AccountInfo,
LabelInfo,
@@ -44,6 +43,7 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {votingStyles} from '../../../styles/gr-voting-styles';
import {ifDefined} from 'lit/directives/if-defined';
+import {fireReload} from '../../../utils/event-util';
declare global {
interface HTMLElementTagNameMap {
@@ -349,7 +349,7 @@
return;
}
if (this.change) {
- GerritNav.navigateToChange(this.change);
+ fireReload(this);
}
})
.catch(err => {
diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
index 1b84089..3d6b5ed 100644
--- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
+++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.ts
@@ -22,7 +22,7 @@
import {IronOverlayBehavior} from '@polymer/iron-overlay-behavior/iron-overlay-behavior';
import {findActiveElement} from '../../../utils/dom-util';
import {fireEvent} from '../../../utils/event-util';
-import {getHovercardContainer} from '../gr-hovercard/gr-hovercard-behavior';
+import {getHovercardContainer} from '../../../mixins/hovercard-mixin/hovercard-mixin';
const AWAIT_MAX_ITERS = 10;
const AWAIT_STEP = 5;
diff --git a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
index 3762d8d..9013088 100644
--- a/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
+++ b/polygerrit-ui/app/elements/shared/gr-vote-chip/gr-vote-chip.ts
@@ -16,7 +16,12 @@
*/
import {LitElement, css, html} from 'lit';
import {customElement, property} from 'lit/decorators';
-import {ApprovalInfo, LabelInfo} from '../../../api/rest-api';
+import {
+ ApprovalInfo,
+ isDetailedLabelInfo,
+ isQuickLabelInfo,
+ LabelInfo,
+} from '../../../api/rest-api';
import {appContext} from '../../../services/app-context';
import {KnownExperimentId} from '../../../services/flags/flags';
import {
@@ -109,22 +114,51 @@
override render() {
if (!this.flagsService.isEnabled(KnownExperimentId.SUBMIT_REQUIREMENTS_UI))
return;
- if (!this.vote?.value) return;
- const className = this.computeClass(this.vote.value, this.label);
+
+ const renderValue = this.renderValue();
+ if (!renderValue) return;
+
return html`<span class="container">
- <div class="vote-chip ${className} ${this.more ? 'more' : ''}">
- ${valueString(this.vote.value)}
+ <div class="vote-chip ${this.computeClass()} ${this.more ? 'more' : ''}">
+ ${renderValue}
</div>
${this.more
- ? html`<div class="chip-angle ${className}">
- ${valueString(this.vote.value)}
+ ? html`<div class="chip-angle ${this.computeClass()}">
+ ${renderValue}
</div>`
: ''}
</span>`;
}
- computeClass(vote: number, label?: LabelInfo) {
- const status = getLabelStatus(label, vote);
- return classForLabelStatus(status);
+ private renderValue() {
+ if (!this.label) {
+ return '';
+ } else if (isDetailedLabelInfo(this.label)) {
+ if (this.vote?.value) {
+ return valueString(this.vote.value);
+ }
+ } else if (isQuickLabelInfo(this.label)) {
+ if (this.label.approved) {
+ return '👍️';
+ } else if (this.label.rejected) {
+ return '👎️';
+ }
+ }
+ return '';
+ }
+
+ private computeClass() {
+ if (!this.label) {
+ return '';
+ } else if (isDetailedLabelInfo(this.label)) {
+ if (this.vote?.value) {
+ const status = getLabelStatus(this.label, this.vote.value);
+ return classForLabelStatus(status);
+ }
+ } else if (isQuickLabelInfo(this.label)) {
+ const status = getLabelStatus(this.label);
+ return classForLabelStatus(status);
+ }
+ return '';
}
}
diff --git a/polygerrit-ui/app/utils/label-util.ts b/polygerrit-ui/app/utils/label-util.ts
index 884afd7..78c4132 100644
--- a/polygerrit-ui/app/utils/label-util.ts
+++ b/polygerrit-ui/app/utils/label-util.ts
@@ -70,21 +70,33 @@
return max > -min ? max : min;
}
-export function getLabelStatus(
- label?: DetailedLabelInfo,
- vote?: number
-): LabelStatus {
- const value = vote ?? getRepresentativeValue(label);
- const range = getVotingRangeOrDefault(label);
- if (value < 0) {
- return value === range.min ? LabelStatus.REJECTED : LabelStatus.DISLIKED;
- }
- if (value > 0) {
- return value === range.max ? LabelStatus.APPROVED : LabelStatus.RECOMMENDED;
+export function getLabelStatus(label?: LabelInfo, vote?: number): LabelStatus {
+ if (!label) return LabelStatus.NEUTRAL;
+ if (isDetailedLabelInfo(label)) {
+ const value = vote ?? getRepresentativeValue(label);
+ const range = getVotingRangeOrDefault(label);
+ if (value < 0) {
+ return value === range.min ? LabelStatus.REJECTED : LabelStatus.DISLIKED;
+ }
+ if (value > 0) {
+ return value === range.max
+ ? LabelStatus.APPROVED
+ : LabelStatus.RECOMMENDED;
+ }
+ } else if (isQuickLabelInfo(label)) {
+ if (label.approved) return LabelStatus.RECOMMENDED;
+ if (label.rejected) return LabelStatus.DISLIKED;
}
return LabelStatus.NEUTRAL;
}
+export function hasNeutralStatus(
+ label: DetailedLabelInfo,
+ approvalInfo: ApprovalInfo
+) {
+ return getLabelStatus(label, approvalInfo.value) === LabelStatus.NEUTRAL;
+}
+
export function classForLabelStatus(status: LabelStatus) {
switch (status) {
case LabelStatus.APPROVED:
diff --git a/polygerrit-ui/app/utils/label-util_test.ts b/polygerrit-ui/app/utils/label-util_test.ts
index 196c5e9..1004aac 100644
--- a/polygerrit-ui/app/utils/label-util_test.ts
+++ b/polygerrit-ui/app/utils/label-util_test.ts
@@ -32,8 +32,11 @@
AccountInfo,
ApprovalInfo,
DetailedLabelInfo,
+ LabelInfo,
+ QuickLabelInfo,
} from '../types/common';
import {
+ createAccountWithEmail,
createSubmitRequirementExpressionInfo,
createSubmitRequirementResultInfo,
} from '../test/test-data-generators';
@@ -171,6 +174,25 @@
assert.equal(getLabelStatus(labelInfo), LabelStatus.REJECTED);
});
+ test('getLabelStatus - quicklabelinfo', () => {
+ let labelInfo: QuickLabelInfo = {};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.NEUTRAL);
+ labelInfo = {approved: createAccountWithEmail()};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.RECOMMENDED);
+ labelInfo = {rejected: createAccountWithEmail()};
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.DISLIKED);
+ });
+
+ test('getLabelStatus - detailed and quick info', () => {
+ let labelInfo: LabelInfo = {all: [], values: VALUES_2};
+ labelInfo = {
+ all: [{value: 0}],
+ values: VALUES_0,
+ rejected: createAccountWithEmail(),
+ };
+ assert.equal(getLabelStatus(labelInfo), LabelStatus.NEUTRAL);
+ });
+
test('getRepresentativeValue', () => {
let labelInfo: DetailedLabelInfo = {all: []};
assert.equal(getRepresentativeValue(labelInfo), 0);
diff --git a/tools/BUILD b/tools/BUILD
index 64b0665..46671ac 100644
--- a/tools/BUILD
+++ b/tools/BUILD
@@ -106,7 +106,7 @@
# "-Xep:AutoValueSubclassLeaked:WARN",
"-Xep:BadAnnotationImplementation:ERROR",
"-Xep:BadComparable:ERROR",
- # "-Xep:BadImport:WARN",
+ "-Xep:BadImport:ERROR",
"-Xep:BadInstanceof:ERROR",
"-Xep:BadShiftAmount:ERROR",
"-Xep:BanSerializableRead:ERROR",
@@ -418,7 +418,7 @@
"-Xep:TimeUnitConversionChecker:ERROR",
"-Xep:ToStringReturnsNull:ERROR",
"-Xep:TreeToString:ERROR",
- # "-Xep:TruthAssertExpected:WARN",
+ "-Xep:TruthAssertExpected:ERROR",
"-Xep:TruthConstantAsserts:ERROR",
"-Xep:TruthGetOrDefault:ERROR",
"-Xep:TruthIncompatibleType:ERROR",
