Merge "Improve Documentation for User Suggested Edits"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 8aa5c7f..eb6080f 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -7374,9 +7374,10 @@
|`message` ||
The text left by the user or Gerrit system. Accounts are served as account IDs
inlined in the text as `<GERRIT_ACCOUNT_18419>`.
-All accounts, used in message, can be found in `accountsInMessage`
+All accounts, used in message, can be found in `accounts_in_message`
field.
-|`accountsInMessage` ||Accounts, used in `message`.
+|`accounts_in_message` ||
+link:rest-api-accounts.html#account-info[AccountInfo] list, used in `message`.
|`tag` |optional|
Value of the `tag` field from link:#review-input[ReviewInput] set
while posting the review. Votes/comments that contain `tag` with
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 0991f20..39e12c4 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -77,7 +77,9 @@
@Override
public void validateExpression(SubmitRequirementExpression expression)
throws QueryParseException {
- queryBuilder.get().parse(expression.expressionString());
+ try (ManualRequestContext ignored = requestContext.open()) {
+ queryBuilder.get().parse(expression.expressionString());
+ }
}
@Override
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
index a643d56..63068c6 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsValidationIT.java
@@ -19,13 +19,32 @@
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
+import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
+import com.google.gerrit.index.query.Matchable;
+import com.google.gerrit.index.query.OperatorPredicate;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.util.List;
import java.util.Locale;
import java.util.function.Consumer;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.transport.PushResult;
@@ -447,6 +466,98 @@
r.assertOkStatus();
}
+ protected static class IsOperatorModule extends AbstractModule {
+ @Override
+ public void configure() {
+ bind(ChangeQueryBuilder.ChangeIsOperandFactory.class)
+ .annotatedWith(Exports.named("changeNumberEven"))
+ .to(SampleIsOperand.class);
+ }
+ }
+
+ private static class SampleIsOperand implements ChangeQueryBuilder.ChangeIsOperandFactory {
+ final Provider<CurrentUser> currentUserProvider;
+
+ @Inject
+ SampleIsOperand(Provider<CurrentUser> currentUserProvider) {
+ this.currentUserProvider = currentUserProvider;
+ }
+
+ @Override
+ public Predicate<ChangeData> create(ChangeQueryBuilder builder) throws QueryParseException {
+ return new IsSamplePredicate(currentUserProvider.get());
+ }
+ }
+
+ private static class IsSamplePredicate extends OperatorPredicate<ChangeData>
+ implements Matchable<ChangeData> {
+
+ CurrentUser currentUser;
+
+ public IsSamplePredicate(CurrentUser currentUser) {
+ super("is", "changeNumberEven");
+ this.currentUser = currentUser;
+ assertServerUser();
+ }
+
+ private void assertServerUser() {
+ try {
+ currentUser.asIdentifiedUser();
+ throw new IllegalStateException("is an identified user");
+ } catch (UnsupportedOperationException e) {
+ // as expected.
+ }
+ }
+
+ @Override
+ public boolean match(ChangeData changeData) {
+ assertServerUser();
+ return true;
+ }
+
+ @Override
+ public int getCost() {
+ return 0;
+ }
+ }
+
+ @Test
+ public void submitRequirementValidationRunsAsServer() throws Exception {
+ try (TestRepository<Repository> testRepo =
+ new TestRepository<>(repoManager.openRepository(project))) {
+ testRepo.delete(RefNames.REFS_CONFIG);
+ }
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ try (AutoCloseable ignored = installPlugin("myplugin", IsOperatorModule.class)) {
+ PushOneCommit push =
+ pushFactory
+ .create(
+ admin.newIdent(),
+ testRepo,
+ "Test Change",
+ ProjectConfig.PROJECT_CONFIG,
+ "[submit-requirement \"SAMPLE\"]\n"
+ + " submittableIf = is:changeNumberEven_myplugin\n")
+ .setParents(ImmutableList.of());
+ PushOneCommit.Result cfgPush = push.to(RefNames.REFS_CONFIG);
+ cfgPush.assertOkStatus();
+
+ ChangeInfo info = gApi.changes().id(changeId).get(ListChangesOption.SUBMIT_REQUIREMENTS);
+ List<SubmitRequirementResultInfo> results =
+ info.submitRequirements.stream()
+ .filter(x -> x.name.equals("SAMPLE"))
+ .collect(Collectors.toList());
+ assertThat(results).hasSize(1);
+ assertThat(results.get(0).status).isNotEqualTo(SubmitRequirementResultInfo.Status.ERROR);
+ }
+
+ // TODO(hanwen): should return 500 ISE for
+ // gApi.changes().id(changeId).get(ListChangesOption.SUBMIT_REQUIREMENTS);
+ }
+
@Test
public void invalidSubmitRequirementIsRejectedWhenPushingForReview() throws Exception {
fetchRefsMetaConfig();
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
index 1a70f2b..ca7a78f 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog.ts
@@ -112,12 +112,15 @@
</section>
<section>
<span class="title">Default Branch</span>
- <iron-input
- .bindValue=${convertToString(this.defaultBranch)}
- @bind-value-changed=${this.handleBranchNameBindValueChanged}
- >
- <input id="defaultBranchNameInput" autocomplete="off" />
- </iron-input>
+ <span class="value">
+ <gr-autocomplete
+ id="defaultBranchNameInput"
+ .text=${convertToString(this.defaultBranch)}
+ .placeholder=${"Optional, defaults to 'master'"}
+ @text-changed=${this.handleBranchNameBindValueChanged}
+ >
+ </gr-autocomplete>
+ </span>
</section>
<section>
<span class="title">Rights inherit from</span>
diff --git a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.ts b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.ts
index 63b8c06..f8476e6 100644
--- a/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.ts
+++ b/polygerrit-ui/app/elements/admin/gr-create-repo-dialog/gr-create-repo-dialog_test.ts
@@ -39,9 +39,9 @@
</section>
<section>
<span class="title"> Default Branch </span>
- <iron-input>
- <input autocomplete="off" id="defaultBranchNameInput" />
- </iron-input>
+ <span class="value">
+ <gr-autocomplete id="defaultBranchNameInput"> </gr-autocomplete>
+ </span>
</section>
<section>
<span class="title"> Rights inherit from </span>
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 640b9d0..29a22a3 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
@@ -42,7 +42,6 @@
import {submitRequirementsStyles} from '../../../styles/gr-submit-requirements-styles';
import {resolve} from '../../../models/dependency';
import {checksModelToken} from '../../../models/checks/checks-model';
-import {join} from 'lit/directives/join.js';
import {map} from 'lit/directives/map.js';
/**
@@ -111,14 +110,15 @@
white-space: nowrap;
vertical-align: top;
}
- .votes-cell {
+ .votes {
+ display: flex;
+ flex-flow: column;
+ row-gap: var(--spacing-s);
+ }
+ .votes-line {
display: flex;
flex-flow: wrap;
}
- .votes-cell .separator {
- width: 100%;
- margin-top: var(--spacing-s);
- }
gr-vote-chip {
margin-right: var(--spacing-s);
}
@@ -280,16 +280,21 @@
hasVotes(allLabels[label])
);
- return html`${join(
- map(
+ return html`<div class="votes">
+ ${map(
associatedLabelsWithVotes,
label =>
- html`${this.renderLabelVote(label, allLabels)}
- ${this.renderOverrideLabels(requirement, label)}`
- ),
- html`<span class="separator"></span>`
- )}
- ${this.renderChecks(requirement)}`;
+ html`<div class="votes-line">
+ ${this.renderLabelVote(label, allLabels)}
+ ${this.renderOverrideLabels(
+ requirement,
+ label,
+ associatedLabelsWithVotes.length > 1
+ )}
+ ${this.renderChecks(requirement, label)}
+ </div>`
+ )}
+ </div> `;
}
renderLabelVote(label: string, labels: LabelNameToInfoMap) {
@@ -317,12 +322,17 @@
renderOverrideLabels(
requirement: SubmitRequirementResultInfo,
- forLabel: string
+ forLabel: string,
+ showForAllLabel: boolean
) {
- if (requirement.status !== SubmitRequirementStatus.OVERRIDDEN) return;
+ if (
+ !showForAllLabel &&
+ requirement.status !== SubmitRequirementStatus.OVERRIDDEN
+ )
+ return;
const requirementLabels = extractAssociatedLabels(
requirement,
- 'onlyOverride'
+ showForAllLabel ? 'all' : 'onlyOverride'
)
.filter(label => label === forLabel)
.filter(label => {
@@ -334,13 +344,17 @@
);
}
- renderChecks(requirement: SubmitRequirementResultInfo) {
+ renderChecks(requirement: SubmitRequirementResultInfo, labelName?: string) {
const requirementLabels = extractAssociatedLabels(requirement);
const errorRuns = this.runs
.filter(run => hasResultsOf(run, Category.ERROR))
- .filter(
- run => run.labelName && requirementLabels.includes(run.labelName)
- );
+ .filter(run => {
+ if (labelName) {
+ return labelName === run.labelName;
+ } else {
+ return run.labelName && requirementLabels.includes(run.labelName);
+ }
+ });
const errorRunsCount = errorRuns.reduce(
(sum, run) => sum + getResultsOf(run, Category.ERROR).length,
0
@@ -357,9 +371,13 @@
.filter(
r => r.status === RunStatus.RUNNING || r.status === RunStatus.SCHEDULED
)
- .filter(
- run => run.labelName && requirementLabels.includes(run.labelName)
- );
+ .filter(run => {
+ if (labelName) {
+ return labelName === run.labelName;
+ } else {
+ return run.labelName && requirementLabels.includes(run.labelName);
+ }
+ });
const runningRunsCount = runningRuns.length;
if (runningRunsCount > 0) {
diff --git a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
index 212394c..a5a336c 100644
--- a/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-submit-requirements/gr-submit-requirements_test.ts
@@ -102,7 +102,11 @@
>
<gr-endpoint-param name="change"></gr-endpoint-param>
<gr-endpoint-param name="requirement"></gr-endpoint-param>
- <gr-vote-chip></gr-vote-chip>
+ <div class="votes">
+ <div class="votes-line">
+ <gr-vote-chip> </gr-vote-chip>
+ </div>
+ </div>
</gr-endpoint-decorator>
</td>
</tr>
@@ -125,7 +129,11 @@
votesCell?.[0],
/* HTML */ `
<div class="votes-cell">
- <gr-vote-chip> </gr-vote-chip>
+ <div class="votes">
+ <div class="votes-line">
+ <gr-vote-chip> </gr-vote-chip>
+ </div>
+ </div>
</div>
`
);
@@ -174,8 +182,12 @@
votesCell?.[0],
/* HTML */ `
<div class="votes-cell">
- <gr-vote-chip></gr-vote-chip>
- <gr-checks-chip></gr-checks-chip>
+ <div class="votes">
+ <div class="votes-line">
+ <gr-vote-chip> </gr-vote-chip>
+ <gr-checks-chip> </gr-checks-chip>
+ </div>
+ </div>
</div>
`
);
@@ -196,8 +208,12 @@
votesCell?.[0],
/* HTML */ `
<div class="votes-cell">
- <gr-vote-chip></gr-vote-chip>
- <gr-checks-chip></gr-checks-chip>
+ <div class="votes">
+ <div class="votes-line">
+ <gr-vote-chip> </gr-vote-chip>
+ <gr-checks-chip> </gr-checks-chip>
+ </div>
+ </div>
</div>
`
);
@@ -231,8 +247,12 @@
assert.dom.equal(
votesCell?.[0],
/* HTML */ `<div class="votes-cell">
- <gr-vote-chip> </gr-vote-chip>
- <span class="overrideLabel"> Override </span>
+ <div class="votes">
+ <div class="votes-line">
+ <gr-vote-chip> </gr-vote-chip>
+ <span class="overrideLabel"> Override </span>
+ </div>
+ </div>
</div>`
);
});
@@ -274,11 +294,16 @@
assert.dom.equal(
votesCell?.[0],
/* HTML */ `<div class="votes-cell">
- <gr-vote-chip> </gr-vote-chip>
- <span class="overrideLabel"> Override </span>
- <span class="separator"></span>
- <gr-vote-chip> </gr-vote-chip>
- <span class="overrideLabel"> Override2 </span>
+ <div class="votes">
+ <div class="votes-line">
+ <gr-vote-chip> </gr-vote-chip>
+ <span class="overrideLabel"> Override </span>
+ </div>
+ <div class="votes-line">
+ <gr-vote-chip> </gr-vote-chip>
+ <span class="overrideLabel"> Override2 </span>
+ </div>
+ </div>
</div>`
);
});
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts
index 681378d..1cda32b 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.ts
@@ -70,6 +70,9 @@
var(--tooltip-background-color);
top: calc(-1 * var(--gr-tooltip-arrow-size));
}
+ .text {
+ white-space: pre;
+ }
`,
];
}
@@ -82,7 +85,7 @@
class="arrowPositionBelow arrow"
style=${styleMap({marginLeft: this.arrowCenterOffset})}
></i>
- ${this.text}
+ <div class="text">${this.text}</div>
<i
class="arrowPositionAbove arrow"
style=${styleMap({marginLeft: this.arrowCenterOffset})}
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts
index 63ed1ff..bc0cfba 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip_test.ts
@@ -26,7 +26,7 @@
/* HTML */ `
<div class="tooltip">
<i class="arrow arrowPositionBelow" style="margin-left:0;"> </i>
- tooltipText
+ <div class="text">tooltipText</div>
<i class="arrow arrowPositionAbove" style="margin-left:0;"> </i>
</div>
`
