Merge "Change heading-3 to be bolder"
diff --git a/java/com/google/gerrit/acceptance/ExtensionRegistry.java b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
index 85c4c13..1e5598e 100644
--- a/java/com/google/gerrit/acceptance/ExtensionRegistry.java
+++ b/java/com/google/gerrit/acceptance/ExtensionRegistry.java
@@ -25,6 +25,8 @@
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.events.GroupIndexedListener;
import com.google.gerrit.extensions.events.ProjectIndexedListener;
+import com.google.gerrit.extensions.events.ReviewerAddedListener;
+import com.google.gerrit.extensions.events.ReviewerDeletedListener;
import com.google.gerrit.extensions.events.RevisionCreatedListener;
import com.google.gerrit.extensions.events.TopicEditedListener;
import com.google.gerrit.extensions.events.WorkInProgressStateChangedListener;
@@ -91,6 +93,8 @@
private final DynamicMap<ProjectConfigEntry> pluginConfigEntries;
private final DynamicSet<PluginPushOption> pluginPushOptions;
private final DynamicSet<OnPostReview> onPostReviews;
+ private final DynamicSet<ReviewerAddedListener> reviewerAddedListeners;
+ private final DynamicSet<ReviewerDeletedListener> reviewerDeletedListeners;
@Inject
ExtensionRegistry(
@@ -125,7 +129,9 @@
DynamicMap<PluginProjectPermissionDefinition> pluginProjectPermissionDefinitions,
DynamicMap<ProjectConfigEntry> pluginConfigEntries,
DynamicSet<PluginPushOption> pluginPushOption,
- DynamicSet<OnPostReview> onPostReviews) {
+ DynamicSet<OnPostReview> onPostReviews,
+ DynamicSet<ReviewerAddedListener> reviewerAddedListeners,
+ DynamicSet<ReviewerDeletedListener> reviewerDeletedListeners) {
this.accountIndexedListeners = accountIndexedListeners;
this.changeIndexedListeners = changeIndexedListeners;
this.groupIndexedListeners = groupIndexedListeners;
@@ -158,6 +164,8 @@
this.pluginConfigEntries = pluginConfigEntries;
this.pluginPushOptions = pluginPushOption;
this.onPostReviews = onPostReviews;
+ this.reviewerAddedListeners = reviewerAddedListeners;
+ this.reviewerDeletedListeners = reviewerDeletedListeners;
}
public Registration newRegistration() {
@@ -302,6 +310,14 @@
return add(onPostReviews, onPostReview);
}
+ public Registration add(ReviewerAddedListener reviewerAddedListener) {
+ return add(reviewerAddedListeners, reviewerAddedListener);
+ }
+
+ public Registration add(ReviewerDeletedListener reviewerDeletedListener) {
+ return add(reviewerDeletedListeners, reviewerDeletedListener);
+ }
+
private <T> Registration add(DynamicSet<T> dynamicSet, T extension) {
return add(dynamicSet, extension, "gerrit");
}
diff --git a/java/com/google/gerrit/server/change/AddReviewersOp.java b/java/com/google/gerrit/server/change/AddReviewersOp.java
index a333ce5..cbbd01a 100644
--- a/java/com/google/gerrit/server/change/AddReviewersOp.java
+++ b/java/com/google/gerrit/server/change/AddReviewersOp.java
@@ -219,8 +219,13 @@
.map(r -> accountCache.get(r.accountId()))
.flatMap(Streams::stream)
.collect(toList());
- reviewerAdded.fire(
- ctx.getChangeData(change), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
+ eventSender =
+ () ->
+ reviewerAdded.fire(
+ ctx.getChangeData(change), patchSet, reviewers, ctx.getAccount(), ctx.getWhen());
+ if (sendEvent) {
+ sendEvent();
+ }
}
}
}
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index a26f107..1e40429 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -202,16 +202,23 @@
"Cannot email update for change %s", currChange.getId());
}
}
- reviewerDeleted.fire(
- ctx.getChangeData(currChange),
- patchSet,
- accountCache.get(reviewer.id()).orElse(AccountState.forAccount(reviewer)),
- ctx.getAccount(),
- mailMessage,
- newApprovals,
- oldApprovals,
- notify.handling(),
- ctx.getWhen());
+
+ NotifyHandling notifyHandling = notify.handling();
+ eventSender =
+ () ->
+ reviewerDeleted.fire(
+ ctx.getChangeData(currChange),
+ patchSet,
+ accountCache.get(reviewer.id()).orElse(AccountState.forAccount(reviewer)),
+ ctx.getAccount(),
+ mailMessage,
+ newApprovals,
+ oldApprovals,
+ notifyHandling,
+ ctx.getWhen());
+ if (sendEvent) {
+ sendEvent();
+ }
}
private Iterable<PatchSetApproval> approvals(ChangeContext ctx, Account.Id accountId) {
diff --git a/java/com/google/gerrit/server/change/ReviewerOp.java b/java/com/google/gerrit/server/change/ReviewerOp.java
index 716ac5e..12227c2 100644
--- a/java/com/google/gerrit/server/change/ReviewerOp.java
+++ b/java/com/google/gerrit/server/change/ReviewerOp.java
@@ -32,6 +32,8 @@
public class ReviewerOp implements BatchUpdateOp {
protected boolean sendEmail = true;
+ protected boolean sendEvent = true;
+ protected Runnable eventSender = () -> {};
protected PatchSet patchSet;
protected Result opResult;
@@ -42,6 +44,14 @@
this.sendEmail = false;
}
+ public void suppressEvent() {
+ this.sendEvent = false;
+ }
+
+ public void sendEvent() {
+ eventSender.run();
+ }
+
void setPatchSet(PatchSet patchSet) {
this.patchSet = requireNonNull(patchSet);
}
diff --git a/java/com/google/gerrit/server/notedb/CommitRewriter.java b/java/com/google/gerrit/server/notedb/CommitRewriter.java
index 7f13731..3cbe546 100644
--- a/java/com/google/gerrit/server/notedb/CommitRewriter.java
+++ b/java/com/google/gerrit/server/notedb/CommitRewriter.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_LABEL;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_REAL_USER;
import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_SUBMITTED_WITH;
+import static com.google.gerrit.server.notedb.ChangeNoteUtil.FOOTER_TAG;
import static com.google.gerrit.server.util.AccountTemplateUtil.ACCOUNT_TEMPLATE_PATTERN;
import static com.google.gerrit.server.util.AccountTemplateUtil.ACCOUNT_TEMPLATE_REGEX;
@@ -39,6 +40,7 @@
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.git.RefUpdateUtil;
import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.server.ChangeMessagesUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -169,7 +171,8 @@
Pattern.compile("Assignee changed from: (.*) to: (.*)");
private static final Pattern REMOVED_REVIEWER_PATTERN =
- Pattern.compile("Removed (cc|reviewer) (.*)(\\.| with the following votes)");
+ Pattern.compile(
+ "Removed (cc|reviewer) (.*)(\\.| with the following votes:\n.*)", Pattern.DOTALL);
private static final Pattern REMOVED_VOTE_PATTERN = Pattern.compile("Removed (.*) by (.*)");
@@ -185,12 +188,18 @@
private static final Pattern ON_CODE_OWNER_ADD_REVIEWER_PATTERN =
Pattern.compile("(.*) who was added as reviewer owns the following files");
+
+ private static final String CODE_OWNER_ADD_REVIEWER_TAG =
+ ChangeMessagesUtil.AUTOGENERATED_BY_GERRIT_TAG_PREFIX + "code-owners:addReviewer";
+
private static final String ON_CODE_OWNER_APPROVAL_REGEX = "code-owner approved by (.*):";
private static final String ON_CODE_OWNER_OVERRIDE_REGEX =
"code-owners submit requirement .* overridden by (.*)";
private static final Pattern ON_CODE_OWNER_REVIEW_PATTERN =
Pattern.compile(ON_CODE_OWNER_APPROVAL_REGEX + "|" + ON_CODE_OWNER_OVERRIDE_REGEX);
+ private static final Pattern ON_CODE_OWNER_POST_REVIEW_PATTERN =
+ Pattern.compile("Patch Set [0-9]+:[\\s\\S]*By (voting|removing)[\\s\\S]*");
private static final Pattern REPLY_BY_REASON_PATTERN =
Pattern.compile("(.*) replied on the change");
@@ -583,7 +592,7 @@
}
Matcher matcher = REMOVED_REVIEWER_PATTERN.matcher(originalChangeMessage);
- if (matcher.find() && !ACCOUNT_TEMPLATE_PATTERN.matcher(matcher.group(2)).matches()) {
+ if (matcher.matches() && !ACCOUNT_TEMPLATE_PATTERN.matcher(matcher.group(2)).matches()) {
// Since we do not use change messages for reviewer updates on UI, it does not matter what we
// rewrite it to.
return Optional.of(originalChangeMessage.substring(0, matcher.end(1)));
@@ -731,7 +740,11 @@
if (Strings.isNullOrEmpty(originalMessage)) {
return Optional.empty();
}
-
+ Matcher onCodeOwnerPostReviewMatcher =
+ ON_CODE_OWNER_POST_REVIEW_PATTERN.matcher(originalMessage);
+ if (!onCodeOwnerPostReviewMatcher.matches()) {
+ return Optional.empty();
+ }
Matcher onCodeOwnerReviewMatcher = ON_CODE_OWNER_REVIEW_PATTERN.matcher(originalMessage);
while (onCodeOwnerReviewMatcher.find()) {
String accountName =
@@ -823,7 +836,9 @@
for (FooterLine fl : footerLines) {
String footerKey = fl.getKey();
String footerValue = fl.getValue();
- if (footerKey.equalsIgnoreCase(FOOTER_ASSIGNEE.getName())) {
+ if (footerKey.equalsIgnoreCase(FOOTER_TAG.getName())) {
+ fixProgress.tag = footerValue;
+ } else if (footerKey.equalsIgnoreCase(FOOTER_ASSIGNEE.getName())) {
Account.Id oldAssignee = fixProgress.assigneeId;
FixIdentResult fixedAssignee = null;
if (footerValue.equals("")) {
@@ -954,7 +969,8 @@
fixedChangeMessage =
fixCodeOwnersOnReviewChangeMessage(fixProgress.updateAuthorId, originalChangeMessage);
}
- if (!fixedChangeMessage.isPresent()) {
+ if (!fixedChangeMessage.isPresent()
+ && Objects.equals(fixProgress.tag, CODE_OWNER_ADD_REVIEWER_TAG)) {
fixedChangeMessage =
fixCodeOwnersOnAddReviewerChangeMessage(fixProgress, originalChangeMessage);
}
@@ -1194,6 +1210,9 @@
/** {@link RefNames#changeMetaRef} of the change that is being fixed. */
final String changeMetaRef;
+ /** Tag at current commit update. */
+ String tag = null;
+
/** Assignee at current commit update. */
Account.Id assigneeId = null;
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 5002a82..5c252f4 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -92,7 +92,9 @@
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.PublishCommentUtil;
import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountResolver;
+import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.approval.ApprovalsUtil;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.EmailReviewComments;
@@ -105,6 +107,7 @@
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.extensions.events.CommentAdded;
+import com.google.gerrit.server.extensions.events.ReviewerAdded;
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -187,6 +190,7 @@
private final BatchUpdate.Factory updateFactory;
private final ChangeResource.Factory changeResourceFactory;
private final ChangeData.Factory changeDataFactory;
+ private final AccountCache accountCache;
private final ApprovalsUtil approvalsUtil;
private final ChangeMessagesUtil cmUtil;
private final CommentsUtil commentsUtil;
@@ -206,6 +210,7 @@
private final PluginSetContext<CommentValidator> commentValidators;
private final PluginSetContext<OnPostReview> onPostReviews;
private final ReplyAttentionSetUpdates replyAttentionSetUpdates;
+ private final ReviewerAdded reviewerAdded;
private final boolean strictLabels;
private final boolean publishPatchSetLevelComment;
@@ -214,6 +219,7 @@
BatchUpdate.Factory updateFactory,
ChangeResource.Factory changeResourceFactory,
ChangeData.Factory changeDataFactory,
+ AccountCache accountCache,
ApprovalsUtil approvalsUtil,
ChangeMessagesUtil cmUtil,
CommentsUtil commentsUtil,
@@ -233,10 +239,12 @@
PermissionBackend permissionBackend,
PluginSetContext<CommentValidator> commentValidators,
PluginSetContext<OnPostReview> onPostReviews,
- ReplyAttentionSetUpdates replyAttentionSetUpdates) {
+ ReplyAttentionSetUpdates replyAttentionSetUpdates,
+ ReviewerAdded reviewerAdded) {
this.updateFactory = updateFactory;
this.changeResourceFactory = changeResourceFactory;
this.changeDataFactory = changeDataFactory;
+ this.accountCache = accountCache;
this.commentsUtil = commentsUtil;
this.publishCommentUtil = publishCommentUtil;
this.psUtil = psUtil;
@@ -256,6 +264,7 @@
this.commentValidators = commentValidators;
this.onPostReviews = onPostReviews;
this.replyAttentionSetUpdates = replyAttentionSetUpdates;
+ this.reviewerAdded = reviewerAdded;
this.strictLabels = gerritConfig.getBoolean("change", "strictLabels", false);
this.publishPatchSetLevelComment =
gerritConfig.getBoolean("event", "comment-added", "publishPatchSetLevelComment", true);
@@ -371,6 +380,7 @@
logger.atFine().log("adding reviewer additions");
for (ReviewerModification reviewerResult : reviewerResults) {
reviewerResult.op.suppressEmail(); // Send a single batch email below.
+ reviewerResult.op.suppressEvent(); // Send events below, if possible as batch.
bu.addOp(revision.getChange().getId(), reviewerResult.op);
if (!ccOrReviewer && reviewerResult.reviewers.contains(account)) {
logger.atFine().log("calling user is explicitly added as reviewer or CC");
@@ -386,6 +396,7 @@
ReviewerModification selfAddition =
reviewerModifier.ccCurrentUser(revision.getUser(), revision);
selfAddition.op.suppressEmail();
+ selfAddition.op.suppressEvent();
bu.addOp(revision.getChange().getId(), selfAddition.op);
}
@@ -433,8 +444,10 @@
reviewerResult.gatherResults(cd);
}
- // Sending from AddReviewersOp was suppressed so we can send a single batch email here.
+ // Sending emails and events from ReviewersOps was suppressed so we can send a single batch
+ // email/event here.
batchEmailReviewers(revision.getUser(), revision.getChange(), reviewerResults, notify);
+ batchReviewerEvents(revision.getUser(), cd, revision.getPatchSet(), reviewerResults, ts);
}
return Response.ok(output);
@@ -512,6 +525,35 @@
}
}
+ private void batchReviewerEvents(
+ CurrentUser user,
+ ChangeData cd,
+ PatchSet patchSet,
+ List<ReviewerModification> reviewerModifications,
+ Timestamp when) {
+ List<AccountState> newlyAddedReviewers = new ArrayList<>();
+
+ // There are no events for CCs and reviewers added/deleted by email.
+ for (ReviewerModification modification : reviewerModifications) {
+ Result reviewerAdditionResult = modification.op.getResult();
+ if (modification.state() == ReviewerState.REVIEWER) {
+ newlyAddedReviewers.addAll(
+ reviewerAdditionResult.addedReviewers().stream()
+ .map(psa -> psa.accountId())
+ .map(accountId -> accountCache.get(accountId))
+ .flatMap(Streams::stream)
+ .collect(toList()));
+ } else if (modification.state() == ReviewerState.REMOVED) {
+ // There is no batch event for reviewer removals, hence fire the event for each
+ // modification that deleted a reviewer immediately.
+ modification.op.sendEvent();
+ }
+ }
+
+ // Fire a batch event for all newly added reviewers.
+ reviewerAdded.fire(cd, patchSet, newlyAddedReviewers, user.asIdentifiedUser().state(), when);
+ }
+
private RevisionResource onBehalfOf(RevisionResource rev, LabelTypes labelTypes, ReviewInput in)
throws BadRequestException, AuthException, UnprocessableEntityException,
PermissionBackendException, IOException, ConfigInvalidException {
diff --git a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
index b79be80..96bc65d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/PostReviewIT.java
@@ -27,6 +27,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.truth.Correspondence;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -56,6 +57,8 @@
import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.events.CommentAddedListener;
+import com.google.gerrit.extensions.events.ReviewerAddedListener;
+import com.google.gerrit.extensions.events.ReviewerDeletedListener;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -74,6 +77,7 @@
import com.google.inject.Inject;
import com.google.inject.Module;
import java.sql.Timestamp;
+import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map;
@@ -699,6 +703,61 @@
}
@Test
+ public void addingReviewers() throws Exception {
+ PushOneCommit.Result r = createChange();
+
+ TestAccount user2 = accountCreator.user2();
+
+ TestReviewerAddedListener testReviewerAddedListener = new TestReviewerAddedListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(testReviewerAddedListener)) {
+ // add user and user2
+ ReviewResult reviewResult =
+ gApi.changes()
+ .id(r.getChangeId())
+ .current()
+ .review(ReviewInput.create().reviewer(user.email()).reviewer(user2.email()));
+
+ assertThat(
+ reviewResult.reviewers.values().stream()
+ .filter(a -> a.reviewers != null)
+ .map(a -> Iterables.getOnlyElement(a.reviewers).name)
+ .collect(toImmutableSet()))
+ .containsExactly(user.fullName(), user2.fullName());
+ }
+
+ assertThat(
+ gApi.changes().id(r.getChangeId()).reviewers().stream()
+ .map(a -> a.name)
+ .collect(toImmutableSet()))
+ .containsExactly(user.fullName(), user2.fullName());
+
+ // Ensure only one batch email was sent for this operation
+ FakeEmailSender.Message message = Iterables.getOnlyElement(sender.getMessages());
+ assertThat(message.body())
+ .containsMatch(
+ Pattern.quote("Hello ")
+ + "("
+ + Pattern.quote(String.format("%s, %s", user.fullName(), user2.fullName()))
+ + "|"
+ + Pattern.quote(String.format("%s, %s", user2.fullName(), user.fullName()))
+ + ")");
+ assertThat(message.htmlBody())
+ .containsMatch(
+ "("
+ + Pattern.quote(String.format("%s and %s", user.fullName(), user2.fullName()))
+ + "|"
+ + Pattern.quote(String.format("%s and %s", user2.fullName(), user.fullName()))
+ + ")"
+ + Pattern.quote(" to <strong>review</strong> this change"));
+
+ // Ensure that a batch event has been sent:
+ // * 1 batch event for adding user and user2 as reviewers
+ assertThat(testReviewerAddedListener.receivedEvents).hasSize(1);
+ assertThat(testReviewerAddedListener.getReviewerIds()).containsExactly(user.id(), user2.id());
+ }
+
+ @Test
public void deletingReviewers() throws Exception {
PushOneCommit.Result r = createChange();
@@ -712,21 +771,25 @@
sender.clear();
- // remove user and user2
- ReviewResult reviewResult =
- gApi.changes()
- .id(r.getChangeId())
- .current()
- .review(
- ReviewInput.create()
- .reviewer(user.email(), ReviewerState.REMOVED, /* confirmed= */ true)
- .reviewer(user2.email(), ReviewerState.REMOVED, /* confirmed= */ true));
+ TestReviewerDeletedListener testReviewerDeletedListener = new TestReviewerDeletedListener();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(testReviewerDeletedListener)) {
+ // remove user and user2
+ ReviewResult reviewResult =
+ gApi.changes()
+ .id(r.getChangeId())
+ .current()
+ .review(
+ ReviewInput.create()
+ .reviewer(user.email(), ReviewerState.REMOVED, /* confirmed= */ true)
+ .reviewer(user2.email(), ReviewerState.REMOVED, /* confirmed= */ true));
- assertThat(
- reviewResult.reviewers.values().stream()
- .map(a -> a.removed.name)
- .collect(toImmutableSet()))
- .containsExactly(user.fullName(), user2.fullName());
+ assertThat(
+ reviewResult.reviewers.values().stream()
+ .map(a -> a.removed.name)
+ .collect(toImmutableSet()))
+ .containsExactly(user.fullName(), user2.fullName());
+ }
assertThat(gApi.changes().id(r.getChangeId()).reviewers()).isEmpty();
@@ -748,6 +811,12 @@
+ "|"
+ Pattern.quote(String.format("%s and %s", user2.fullName(), user.fullName()))
+ ")");
+
+ // Ensure that events have been sent:
+ // * 2 events for removing user and user2 as reviewers (one event per removed reviewer, batch
+ // event not available for reviewer removal)
+ assertThat(testReviewerDeletedListener.receivedEvents).hasSize(2);
+ assertThat(testReviewerDeletedListener.getReviewerIds()).containsExactly(user.id(), user2.id());
}
@Test
@@ -766,30 +835,38 @@
sender.clear();
- // remove user and user2 while adding user3 and user4
- ReviewResult reviewResult =
- gApi.changes()
- .id(r.getChangeId())
- .current()
- .review(
- ReviewInput.create()
- .reviewer(user.email(), ReviewerState.REMOVED, /* confirmed= */ true)
- .reviewer(user2.email(), ReviewerState.REMOVED, /* confirmed= */ true)
- .reviewer(user3.email())
- .reviewer(user4.email()));
+ TestReviewerAddedListener testReviewerAddedListener = new TestReviewerAddedListener();
+ TestReviewerDeletedListener testReviewerDeletedListener = new TestReviewerDeletedListener();
+ try (Registration registration =
+ extensionRegistry
+ .newRegistration()
+ .add(testReviewerAddedListener)
+ .add(testReviewerDeletedListener)) {
+ // remove user and user2 while adding user3 and user4
+ ReviewResult reviewResult =
+ gApi.changes()
+ .id(r.getChangeId())
+ .current()
+ .review(
+ ReviewInput.create()
+ .reviewer(user.email(), ReviewerState.REMOVED, /* confirmed= */ true)
+ .reviewer(user2.email(), ReviewerState.REMOVED, /* confirmed= */ true)
+ .reviewer(user3.email())
+ .reviewer(user4.email()));
- assertThat(
- reviewResult.reviewers.values().stream()
- .filter(a -> a.removed != null)
- .map(a -> a.removed.name)
- .collect(toImmutableSet()))
- .containsExactly(user.fullName(), user2.fullName());
- assertThat(
- reviewResult.reviewers.values().stream()
- .filter(a -> a.reviewers != null)
- .map(a -> Iterables.getOnlyElement(a.reviewers).name)
- .collect(toImmutableSet()))
- .containsExactly(user3.fullName(), user4.fullName());
+ assertThat(
+ reviewResult.reviewers.values().stream()
+ .filter(a -> a.removed != null)
+ .map(a -> a.removed.name)
+ .collect(toImmutableSet()))
+ .containsExactly(user.fullName(), user2.fullName());
+ assertThat(
+ reviewResult.reviewers.values().stream()
+ .filter(a -> a.reviewers != null)
+ .map(a -> Iterables.getOnlyElement(a.reviewers).name)
+ .collect(toImmutableSet()))
+ .containsExactly(user3.fullName(), user4.fullName());
+ }
assertThat(
gApi.changes().id(r.getChangeId()).reviewers().stream()
@@ -832,6 +909,15 @@
+ "|"
+ Pattern.quote(String.format("%s and %s", user2.fullName(), user.fullName()))
+ ")");
+
+ // Ensure that events have been sent:
+ // * 1 batch event for adding user3 and user4 as reviewers
+ // * 2 events for removing user and user2 as reviewers (one event per removed reviewer, batch
+ // event not available for reviewer removal)
+ assertThat(testReviewerAddedListener.receivedEvents).hasSize(1);
+ assertThat(testReviewerAddedListener.getReviewerIds()).containsExactly(user3.id(), user4.id());
+ assertThat(testReviewerDeletedListener.receivedEvents).hasSize(2);
+ assertThat(testReviewerDeletedListener.getReviewerIds()).containsExactly(user.id(), user2.id());
}
@Test
@@ -964,4 +1050,36 @@
return Optional.empty();
}
}
+
+ private static class TestReviewerAddedListener implements ReviewerAddedListener {
+ List<ReviewerAddedListener.Event> receivedEvents = new ArrayList<>();
+
+ @Override
+ public void onReviewersAdded(ReviewerAddedListener.Event event) {
+ receivedEvents.add(event);
+ }
+
+ public ImmutableSet<Account.Id> getReviewerIds() {
+ return receivedEvents.stream()
+ .flatMap(e -> e.getReviewers().stream())
+ .map(accountInfo -> Account.id(accountInfo._accountId))
+ .collect(toImmutableSet());
+ }
+ }
+
+ private static class TestReviewerDeletedListener implements ReviewerDeletedListener {
+ List<ReviewerDeletedListener.Event> receivedEvents = new ArrayList<>();
+
+ @Override
+ public void onReviewerDeleted(ReviewerDeletedListener.Event event) {
+ receivedEvents.add(event);
+ }
+
+ public ImmutableSet<Account.Id> getReviewerIds() {
+ return receivedEvents.stream()
+ .map(ReviewerDeletedListener.Event::getReviewer)
+ .map(accountInfo -> Account.id(accountInfo._accountId))
+ .collect(toImmutableSet());
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
index f316660..19c2bcf 100644
--- a/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
+++ b/javatests/com/google/gerrit/server/notedb/CommitRewriterTest.java
@@ -34,6 +34,7 @@
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.entities.SubmitRecord;
import com.google.gerrit.json.OutputFormat;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.notedb.ChangeNoteUtil.AttentionStatusInNoteDb;
@@ -1603,19 +1604,20 @@
accountCache.put(duplicateReviewer);
Change c = newChange();
ImmutableList.Builder<ObjectId> commitsToFix = new ImmutableList.Builder<>();
- ChangeUpdate addReviewerUpdate = newUpdate(c, changeOwner);
+ ChangeUpdate addReviewerUpdate = newCodeOwnerAddReviewerUpdate(c, changeOwner);
addReviewerUpdate.putReviewer(reviewer.id(), REVIEWER);
addReviewerUpdate.commit();
- ChangeUpdate invalidOnAddReviewerUpdate = newUpdate(c, changeOwner);
+ ChangeUpdate invalidOnAddReviewerUpdate = newCodeOwnerAddReviewerUpdate(c, changeOwner);
invalidOnAddReviewerUpdate.setChangeMessage(
"Reviewer User who was added as reviewer owns the following files:\n"
+ " * file1.java\n"
+ " * file2.ts\n");
commitsToFix.add(invalidOnAddReviewerUpdate.commit());
- ChangeUpdate addOtherReviewerUpdate = newUpdate(c, changeOwner);
+ ChangeUpdate addOtherReviewerUpdate = newCodeOwnerAddReviewerUpdate(c, changeOwner);
addOtherReviewerUpdate.putReviewer(otherUserId, REVIEWER);
addOtherReviewerUpdate.commit();
- ChangeUpdate invalidOnAddReviewerMultipleReviewerUpdate = newUpdate(c, changeOwner);
+ ChangeUpdate invalidOnAddReviewerMultipleReviewerUpdate =
+ newCodeOwnerAddReviewerUpdate(c, changeOwner);
invalidOnAddReviewerMultipleReviewerUpdate.setChangeMessage(
"Reviewer User who was added as reviewer owns the following files:\n"
+ " * file1.java\n"
@@ -1624,17 +1626,17 @@
+ "\nMissing Reviewer who was added as reviewer owns the following files:\n"
+ " * file4.java\n");
commitsToFix.add(invalidOnAddReviewerMultipleReviewerUpdate.commit());
- ChangeUpdate addDuplicateReviewerUpdate = newUpdate(c, changeOwner);
+ ChangeUpdate addDuplicateReviewerUpdate = newCodeOwnerAddReviewerUpdate(c, changeOwner);
addDuplicateReviewerUpdate.putReviewer(duplicateReviewer.id(), REVIEWER);
addDuplicateReviewerUpdate.commit();
// Reviewer name resolves to multiple accounts in the same change
- ChangeUpdate onAddReviewerUpdateWithDuplicate = newUpdate(c, changeOwner);
+ ChangeUpdate onAddReviewerUpdateWithDuplicate = newCodeOwnerAddReviewerUpdate(c, changeOwner);
onAddReviewerUpdateWithDuplicate.setChangeMessage(
"Reviewer User who was added as reviewer owns the following files:\n"
+ " * file6.java\n");
commitsToFix.add(onAddReviewerUpdateWithDuplicate.commit());
- ChangeUpdate validOnAddReviewerUpdate = newUpdate(c, changeOwner);
+ ChangeUpdate validOnAddReviewerUpdate = newCodeOwnerAddReviewerUpdate(c, changeOwner);
validOnAddReviewerUpdate.setChangeMessage(
"Gerrit Account who was added as reviewer owns the following files:\n"
+ " * file1.java\n"
@@ -2249,6 +2251,13 @@
.collect(toImmutableList());
}
+ protected ChangeUpdate newCodeOwnerAddReviewerUpdate(Change c, CurrentUser user)
+ throws Exception {
+ ChangeUpdate update = newUpdate(c, user, true);
+ update.setTag("autogenerated:gerrit:code-owners:addReviewer");
+ return update;
+ }
+
private ImmutableList<String> commitHistoryDiff(BackfillResult result, Change.Id changeId) {
return result.fixedRefDiff.get(RefNames.changeMetaRef(changeId)).stream()
.map(CommitDiff::diff)
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
index 192a812..b7b4d9c 100644
--- 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
@@ -51,13 +51,13 @@
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;
- top: 2px;
width: 20px;
height: 20px;
}
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 8e70eb9..f0cff85 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -178,7 +178,7 @@
CHANGE_ID_QUERY: /^\/id\/(I[0-9a-f]{40})$/,
// Matches /c/<changeNum>/[*][/].
- CHANGE_LEGACY: /^\/c\/(\d+)\/(.*)$/,
+ CHANGE_LEGACY: /^\/c\/(\d+)\/?(.*)$/,
CHANGE_NUMBER_LEGACY: /^\/(\d+)\/?/,
// Matches
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 dba36a4..67f84a5 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,23 +24,26 @@
import '../gr-label/gr-label';
import '../gr-tooltip-content/gr-tooltip-content';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-label-info_html';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
-import {customElement, property} from '@polymer/decorators';
import {
- ChangeInfo,
AccountInfo,
LabelInfo,
ApprovalInfo,
AccountId,
isQuickLabelInfo,
isDetailedLabelInfo,
+ LabelNameToInfoMap,
} from '../../../types/common';
+import {LitElement, css, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
import {GrButton} from '../gr-button/gr-button';
import {getVotingRangeOrDefault} from '../../../utils/label-util';
import {appContext} from '../../../services/app-context';
import {ParsedChangeInfo} from '../../../types/types';
+import {fontStyles} from '../../../styles/gr-font-styles';
+import {sharedStyles} from '../../../styles/shared-styles';
+import {votingStyles} from '../../../styles/gr-voting-styles';
+import {ifDefined} from 'lit/directives/if-defined';
declare global {
interface HTMLElementTagNameMap {
@@ -57,16 +60,12 @@
interface FormattedLabel {
className?: LabelClassName;
- account: ApprovalInfo;
+ account: ApprovalInfo | AccountInfo;
value: string;
}
@customElement('gr-label-info')
-export class GrLabelInfo extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
+export class GrLabelInfo extends LitElement {
@property({type: Object})
labelInfo?: LabelInfo;
@@ -89,11 +88,148 @@
// TODO(TS): not used, remove later
_xhrPromise?: Promise<void>;
+ static override get styles() {
+ return [
+ sharedStyles,
+ fontStyles,
+ votingStyles,
+ css`
+ .placeholder {
+ color: var(--deemphasized-text-color);
+ }
+ .hidden {
+ display: none;
+ }
+ /* Note that most of the .voteChip styles are coming from the
+ gr-voting-styles include. */
+ .voteChip {
+ display: flex;
+ justify-content: center;
+ margin-right: var(--spacing-s);
+ padding: 1px;
+ }
+ .max {
+ background-color: var(--vote-color-approved);
+ }
+ .min {
+ background-color: var(--vote-color-rejected);
+ }
+ .positive {
+ background-color: var(--vote-color-recommended);
+ border-radius: 12px;
+ border: 1px solid var(--vote-outline-recommended);
+ color: var(--chip-color);
+ }
+ .negative {
+ background-color: var(--vote-color-disliked);
+ border-radius: 12px;
+ border: 1px solid var(--vote-outline-disliked);
+ color: var(--chip-color);
+ }
+ .hidden {
+ display: none;
+ }
+ td {
+ vertical-align: top;
+ }
+ tr {
+ min-height: var(--line-height-normal);
+ }
+ gr-tooltip-content {
+ display: block;
+ }
+ gr-button {
+ vertical-align: top;
+ }
+ gr-button::part(paper-button) {
+ height: var(--line-height-normal);
+ width: var(--line-height-normal);
+ padding: 0;
+ }
+ gr-button[disabled] iron-icon {
+ color: var(--border-color);
+ }
+ gr-account-link {
+ --account-max-length: 100px;
+ margin-right: var(--spacing-xs);
+ }
+ iron-icon {
+ height: calc(var(--line-height-normal) - 2px);
+ width: calc(var(--line-height-normal) - 2px);
+ }
+ .labelValueContainer:not(:first-of-type) td {
+ padding-top: var(--spacing-s);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ return html` <p
+ class="placeholder ${this.computeShowPlaceholder(
+ this.labelInfo,
+ this.change?.labels
+ )}"
+ >
+ No votes
+ </p>
+ <table>
+ ${this.mapLabelInfo(
+ this.labelInfo,
+ this.account,
+ this.change?.labels
+ ).map(mappedLabel => this.renderLabel(mappedLabel))}
+ </table>`;
+ }
+
+ renderLabel(mappedLabel: FormattedLabel) {
+ const {labelInfo, change} = this;
+ return html` <tr class="labelValueContainer">
+ <td>
+ <gr-tooltip-content
+ has-tooltip
+ title="${this._computeValueTooltip(labelInfo, mappedLabel.value)}"
+ >
+ <gr-label class="${mappedLabel.className} voteChip font-small">
+ ${mappedLabel.value}
+ </gr-label>
+ </gr-tooltip-content>
+ </td>
+ <td>
+ <gr-account-link
+ .account="${mappedLabel.account}"
+ .change="${change}"
+ ></gr-account-link>
+ </td>
+ <td>
+ <gr-tooltip-content has-tooltip title="Remove vote">
+ <gr-button
+ link
+ aria-label="Remove vote"
+ @click="${this.onDeleteVote}"
+ data-account-id="${ifDefined(mappedLabel.account._account_id)}"
+ class="deleteBtn ${this.computeDeleteClass(
+ mappedLabel.account,
+ this.mutable,
+ change
+ )}"
+ >
+ <iron-icon icon="gr-icons:delete"></iron-icon>
+ </gr-button>
+ </gr-tooltip-content>
+ </td>
+ </tr>`;
+ }
+
/**
* This method also listens on change.labels.*,
* to trigger computation when a label is removed from the change.
*/
- _mapLabelInfo(labelInfo?: LabelInfo, account?: AccountInfo) {
+ private mapLabelInfo(
+ labelInfo?: LabelInfo,
+ account?: AccountInfo,
+ _?: LabelNameToInfoMap
+ ): FormattedLabel[] {
const result: FormattedLabel[] = [];
if (!labelInfo) {
return result;
@@ -108,7 +244,8 @@
{
value: ok ? '👍️' : '👎️',
className: ok ? LabelClassName.POSITIVE : LabelClassName.NEGATIVE,
- account: ok ? labelInfo.approved : labelInfo.rejected,
+ // executed only if approved or rejected is not undefined
+ account: ok ? labelInfo.approved! : labelInfo.rejected!,
},
];
}
@@ -143,7 +280,7 @@
labelClassName = LabelClassName.NEGATIVE;
}
}
- const formattedLabel = {
+ const formattedLabel: FormattedLabel = {
value: `${labelValPrefix}${label.value}`,
className: labelClassName,
account: label,
@@ -167,10 +304,10 @@
* @param reviewer An object describing the reviewer that left the
* vote.
*/
- _computeDeleteClass(
+ private computeDeleteClass(
reviewer: ApprovalInfo,
mutable: boolean,
- change: ChangeInfo
+ change?: ParsedChangeInfo
) {
if (!mutable || !change || !change.removable_reviewers) {
return 'hidden';
@@ -186,7 +323,7 @@
* Closure annotation for Polymer.prototype.splice is off.
* For now, suppressing annotations.
*/
- _onDeleteVote(e: MouseEvent) {
+ private onDeleteVote(e: MouseEvent) {
if (!this.change) return;
e.preventDefault();
@@ -220,7 +357,7 @@
});
}
- _computeValueTooltip(labelInfo: LabelInfo, score: string) {
+ _computeValueTooltip(labelInfo: LabelInfo | undefined, score: string) {
if (
!labelInfo ||
!isDetailedLabelInfo(labelInfo) ||
@@ -235,7 +372,10 @@
* This method also listens change.labels.* in
* order to trigger computation when a label is removed from the change.
*/
- _computeShowPlaceholder(labelInfo?: LabelInfo) {
+ private computeShowPlaceholder(
+ labelInfo?: LabelInfo,
+ _?: LabelNameToInfoMap
+ ) {
if (!labelInfo) {
return '';
}
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
deleted file mode 100644
index 1186ee1..0000000
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_html.ts
+++ /dev/null
@@ -1,135 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="gr-font-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="gr-voting-styles">
- /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
- </style>
- <style include="shared-styles">
- .placeholder {
- color: var(--deemphasized-text-color);
- }
- .hidden {
- display: none;
- }
- /* Note that most of the .voteChip styles are coming from the
- gr-voting-styles include. */
- .voteChip {
- display: flex;
- justify-content: center;
- margin-right: var(--spacing-s);
- padding: 1px;
- }
- .max {
- background-color: var(--vote-color-approved);
- }
- .min {
- background-color: var(--vote-color-rejected);
- }
- .positive {
- background-color: var(--vote-color-recommended);
- border-radius: 12px;
- border: 1px solid var(--vote-outline-recommended);
- color: var(--chip-color);
- }
- .negative {
- background-color: var(--vote-color-disliked);
- border-radius: 12px;
- border: 1px solid var(--vote-outline-disliked);
- color: var(--chip-color);
- }
- .hidden {
- display: none;
- }
- td {
- vertical-align: top;
- }
- tr {
- min-height: var(--line-height-normal);
- }
- gr-tooltip-content {
- display: block;
- }
- gr-button {
- display: block;
- vertical-align: top;
- --gr-button-padding: 1px;
- }
- gr-button[disabled] iron-icon {
- color: var(--border-color);
- }
- gr-account-link {
- --account-max-length: 100px;
- margin-right: var(--spacing-xs);
- }
- iron-icon {
- height: calc(var(--line-height-normal) - 2px);
- width: calc(var(--line-height-normal) - 2px);
- }
- .labelValueContainer:not(:first-of-type) td {
- padding-top: var(--spacing-s);
- }
- </style>
- <p
- class$="placeholder [[_computeShowPlaceholder(labelInfo, change.labels.*)]]"
- >
- No votes
- </p>
- <table>
- <template
- is="dom-repeat"
- items="[[_mapLabelInfo(labelInfo, account, change.labels.*)]]"
- as="mappedLabel"
- >
- <tr class="labelValueContainer">
- <td>
- <gr-tooltip-content
- has-tooltip
- title="[[_computeValueTooltip(labelInfo, mappedLabel.value)]]"
- >
- <gr-label class$="[[mappedLabel.className]] voteChip font-small">
- [[mappedLabel.value]]
- </gr-label>
- </gr-tooltip-content>
- </td>
- <td>
- <gr-account-link
- account="[[mappedLabel.account]]"
- change="[[change]]"
- ></gr-account-link>
- </td>
- <td>
- <gr-tooltip-content has-tooltip title="Remove vote">
- <gr-button
- link=""
- aria-label="Remove vote"
- on-click="_onDeleteVote"
- data-account-id$="[[mappedLabel.account._account_id]]"
- class$="deleteBtn [[_computeDeleteClass(mappedLabel.account, mutable, change)]]"
- >
- <iron-icon icon="gr-icons:delete"></iron-icon>
- </gr-button>
- </gr-tooltip-content>
- </td>
- </tr>
- </template>
- </table>
-`;
diff --git a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.ts b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.ts
index b1bd6fa..cad1f69 100644
--- a/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-label-info/gr-label-info_test.ts
@@ -66,15 +66,17 @@
element.labelInfo = label;
element.label = 'Code-Review';
- await flush();
+ await element.updateComplete;
});
- test('_computeCanDeleteVote', () => {
+ test('_computeCanDeleteVote', async () => {
element.mutable = false;
+ await element.updateComplete;
const removeButton = queryAndAssert<GrButton>(element, 'gr-button');
assert.isTrue(isHidden(removeButton));
element.change!.removable_reviewers = [account];
element.mutable = true;
+ await element.updateComplete;
assert.isFalse(isHidden(removeButton));
});
@@ -109,14 +111,14 @@
suite('label color and order', () => {
test('valueless label rejected', async () => {
element.labelInfo = {rejected: {name: 'someone'}};
- await flush();
+ await element.updateComplete;
const labels = queryAll<GrLabel>(element, 'gr-label');
assert.isTrue(labels[0].classList.contains('negative'));
});
test('valueless label approved', async () => {
element.labelInfo = {approved: {name: 'someone'}};
- await flush();
+ await element.updateComplete;
const labels = queryAll<GrLabel>(element, 'gr-label');
assert.isTrue(labels[0].classList.contains('positive'));
});
@@ -137,7 +139,7 @@
'+2': 'Ready to submit',
},
};
- await flush();
+ await element.updateComplete;
const labels = queryAll<GrLabel>(element, 'gr-label');
assert.isTrue(labels[0].classList.contains('max'));
assert.isTrue(labels[1].classList.contains('positive'));
@@ -157,7 +159,7 @@
'+1': 'Looks good to me',
},
};
- await flush();
+ await element.updateComplete;
const labels = queryAll<GrLabel>(element, 'gr-label');
assert.isTrue(labels[0].classList.contains('max'));
assert.isTrue(labels[1].classList.contains('min'));
@@ -175,7 +177,7 @@
'+2': 'Looks good to me',
},
};
- await flush();
+ await element.updateComplete;
const labels = queryAll<GrLabel>(element, 'gr-label');
assert.isTrue(labels[0].classList.contains('max'));
assert.isTrue(labels[1].classList.contains('positive'));
@@ -195,7 +197,7 @@
'+1': 'Looks good to me',
},
};
- await flush();
+ await element.updateComplete;
const chips = queryAll<GrAccountLink>(element, 'gr-account-link');
assert.equal(chips[0].account!._account_id, element.account._account_id);
});
@@ -217,7 +219,7 @@
assert.equal(element._computeValueTooltip(labelInfo, score), '');
});
- test('placeholder', () => {
+ test('placeholder', async () => {
const values = {
'0': 'No score',
'+1': 'good',
@@ -226,30 +228,37 @@
'-2': 'terrible',
};
element.labelInfo = {};
+ await element.updateComplete;
assert.isFalse(
isHidden(queryAndAssert<HTMLParagraphElement>(element, '.placeholder'))
);
element.labelInfo = {all: [], values};
+ await element.updateComplete;
assert.isFalse(
isHidden(queryAndAssert<HTMLParagraphElement>(element, '.placeholder'))
);
element.labelInfo = {all: [{value: 1}], values};
+ await element.updateComplete;
assert.isTrue(
isHidden(queryAndAssert<HTMLParagraphElement>(element, '.placeholder'))
);
element.labelInfo = {rejected: account};
+ await element.updateComplete;
assert.isTrue(
isHidden(queryAndAssert<HTMLParagraphElement>(element, '.placeholder'))
);
element.labelInfo = {rejected: account, all: [{value: 1}], values};
+ await element.updateComplete;
assert.isTrue(
isHidden(queryAndAssert<HTMLParagraphElement>(element, '.placeholder'))
);
element.labelInfo = {approved: account};
+ await element.updateComplete;
assert.isTrue(
isHidden(queryAndAssert<HTMLParagraphElement>(element, '.placeholder'))
);
element.labelInfo = {approved: account, all: [{value: 1}], values};
+ await element.updateComplete;
assert.isTrue(
isHidden(queryAndAssert<HTMLParagraphElement>(element, '.placeholder'))
);
