Merge "Feedback for triggered actions"
diff --git a/java/com/google/gerrit/server/ApprovalInference.java b/java/com/google/gerrit/server/ApprovalInference.java
index 572ae7a..40bf249 100644
--- a/java/com/google/gerrit/server/ApprovalInference.java
+++ b/java/com/google/gerrit/server/ApprovalInference.java
@@ -109,10 +109,10 @@
PatchSetApproval psa,
PatchSet.Id psId,
ChangeKind kind,
- PatchList patchList) {
+ LabelType type,
+ @Nullable PatchList patchList) {
int n = psa.key().patchSetId().get();
checkArgument(n != psId.get());
- LabelType type = project.getLabelTypes().byLabel(psa.labelId());
if (type == null) {
logger.atFine().log(
@@ -367,12 +367,17 @@
logger.atFine().log(
"change kind for patch set %d of change %d against prior patch set %s is %s",
ps.id().get(), ps.id().changeId().get(), priorPatchSet.getValue().id().changeId(), kind);
- PatchList patchList = getPatchList(project, ps, priorPatchSet);
+ PatchList patchList = null;
for (PatchSetApproval psa : priorApprovals) {
if (resultByUser.contains(psa.label(), psa.accountId())) {
continue;
}
- if (!canCopy(project, psa, ps.id(), kind, patchList)) {
+ LabelType type = project.getLabelTypes().byLabel(psa.labelId());
+ // Only compute patchList if there is a relevant label, since this is expensive.
+ if (patchList == null && type != null && type.isCopyAllScoresIfListOfFilesDidNotChange()) {
+ patchList = getPatchList(project, ps, priorPatchSet);
+ }
+ if (!canCopy(project, psa, ps.id(), kind, type, patchList)) {
continue;
}
resultByUser.put(psa.label(), psa.accountId(), psa.copyWithPatchSet(ps.id()));
diff --git a/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java b/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java
index 9f58aaf..63e0b7a 100644
--- a/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java
+++ b/java/com/google/gerrit/server/patch/SubmitWithStickyApprovalDiff.java
@@ -88,8 +88,10 @@
// If the latest approved patchset is the current patchset, no need to return anything.
return "";
}
- String diff =
- String.format("\n\n%d is the latest approved patch-set.\n", latestApprovedPatchsetId.get());
+ StringBuilder diff =
+ new StringBuilder(
+ String.format(
+ "\n\n%d is the latest approved patch-set.\n", latestApprovedPatchsetId.get()));
PatchList patchList =
getPatchList(
notes.getProjectName(),
@@ -103,19 +105,19 @@
.collect(Collectors.toList());
if (patchListEntryList.isEmpty()) {
- diff +=
- "No files were changed between the latest approved patch-set and the submitted one.\n";
- return diff;
+ diff.append(
+ "No files were changed between the latest approved patch-set and the submitted one.\n");
+ return diff.toString();
}
- diff += "The change was submitted with unreviewed changes in the following files:\n\n";
+ diff.append("The change was submitted with unreviewed changes in the following files:\n\n");
for (PatchListEntry patchListEntry : patchListEntryList) {
- diff +=
+ diff.append(
getDiffForFile(
- notes, currentPatchset.id(), latestApprovedPatchsetId, patchListEntry, currentUser);
+ notes, currentPatchset.id(), latestApprovedPatchsetId, patchListEntry, currentUser));
}
- return diff;
+ return diff.toString();
}
private String getDiffForFile(
@@ -126,12 +128,13 @@
CurrentUser currentUser)
throws AuthException, InvalidChangeOperationException, IOException,
PermissionBackendException {
- String diff =
- String.format(
- "The name of the file: %s\nInsertions: %d, Deletions: %d.\n\n",
- patchListEntry.getNewName(),
- patchListEntry.getInsertions(),
- patchListEntry.getDeletions());
+ StringBuilder diff =
+ new StringBuilder(
+ String.format(
+ "The name of the file: %s\nInsertions: %d, Deletions: %d.\n\n",
+ patchListEntry.getNewName(),
+ patchListEntry.getInsertions(),
+ patchListEntry.getDeletions()));
DiffPreferencesInfo diffPreferencesInfo = createDefaultDiffPreferencesInfo();
PatchScriptFactory patchScriptFactory =
patchScriptFactoryFactory.create(
@@ -145,66 +148,66 @@
try {
patchScript = patchScriptFactory.call();
} catch (LargeObjectException exception) {
- diff += "The file content is too large for showing the full diff. \n\n";
- return diff;
+ diff.append("The file content is too large for showing the full diff. \n\n");
+ return diff.toString();
}
if (patchScript.getChangeType() == ChangeType.RENAMED) {
- diff +=
+ diff.append(
String.format(
"The file %s was renamed to %s\n",
- patchListEntry.getOldName(), patchListEntry.getNewName());
+ patchListEntry.getOldName(), patchListEntry.getNewName()));
}
SparseFileContent.Accessor fileA = patchScript.getA().createAccessor();
SparseFileContent.Accessor fileB = patchScript.getB().createAccessor();
boolean editsExist = false;
if (patchScript.getEdits().stream().anyMatch(e -> e.getType() != Edit.Type.EMPTY)) {
- diff += "```\n";
+ diff.append("```\n");
editsExist = true;
}
for (Edit edit : patchScript.getEdits()) {
- diff += getDiffForEdit(fileA, fileB, edit);
+ diff.append(getDiffForEdit(fileA, fileB, edit));
}
if (editsExist) {
- diff += "```\n";
+ diff.append("```\n");
}
- return diff;
+ return diff.toString();
}
private String getDiffForEdit(Accessor fileA, Accessor fileB, Edit edit) {
- String diff = "";
+ StringBuilder diff = new StringBuilder();
Edit.Type type = edit.getType();
switch (type) {
case INSERT:
- diff += String.format("@@ +%d:%d @@\n", edit.getBeginB(), edit.getEndB());
- diff += getModifiedLines(fileB, edit.getBeginB(), edit.getEndB(), '+');
- diff += "\n";
+ diff.append(String.format("@@ +%d:%d @@\n", edit.getBeginB(), edit.getEndB()));
+ diff.append(getModifiedLines(fileB, edit.getBeginB(), edit.getEndB(), '+'));
+ diff.append("\n");
break;
case DELETE:
- diff += String.format("@@ -%d:%d @@\n", edit.getBeginA(), edit.getEndA());
- diff += getModifiedLines(fileA, edit.getBeginA(), edit.getEndA(), '-');
- diff += "\n";
+ diff.append(String.format("@@ -%d:%d @@\n", edit.getBeginA(), edit.getEndA()));
+ diff.append(getModifiedLines(fileA, edit.getBeginA(), edit.getEndA(), '-'));
+ diff.append("\n");
break;
case REPLACE:
- diff +=
+ diff.append(
String.format(
"@@ -%d:%d, +%d:%d @@\n",
- edit.getBeginA(), edit.getEndA(), edit.getBeginB(), edit.getEndB());
- diff += getModifiedLines(fileA, edit.getBeginA(), edit.getEndA(), '-');
- diff += getModifiedLines(fileB, edit.getBeginB(), edit.getEndB(), '+');
- diff += "\n";
+ edit.getBeginA(), edit.getEndA(), edit.getBeginB(), edit.getEndB()));
+ diff.append(getModifiedLines(fileA, edit.getBeginA(), edit.getEndA(), '-'));
+ diff.append(getModifiedLines(fileB, edit.getBeginB(), edit.getEndB(), '+'));
+ diff.append("\n");
break;
case EMPTY:
// do nothing since there is no change here.
}
- return diff;
+ return diff.toString();
}
private String getModifiedLines(Accessor file, int begin, int end, char modificationType) {
- String diff = "";
+ StringBuilder diff = new StringBuilder();
for (int i = begin; i < end; i++) {
- diff += String.format("%c %s\n", modificationType, file.get(i));
+ diff.append(String.format("%c %s\n", modificationType, file.get(i)));
}
- return diff;
+ return diff.toString();
}
private DiffPreferencesInfo createDefaultDiffPreferencesInfo() {
diff --git a/plugins/replication b/plugins/replication
index a379adc..14766e7 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit a379adcd2f4f1a41731818e74b2a214de3fcf5d8
+Subproject commit 14766e75f91886ab48951035d59a78c8c3f07471
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
index 816c8ef..a34bd63 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
@@ -380,6 +380,19 @@
e.detail.change._number,
e.detail.starred
);
+ // When a change is updated the same change may appear elsewhere in the
+ // dashboard (but is not the same object), so we must update other
+ // occurrences of the same change.
+ this._results?.forEach((dashboardChange, dashboardIndex) =>
+ dashboardChange.results.forEach((change, changeIndex) => {
+ if (change.id === e.detail.change.id) {
+ this.set(
+ `_results.${dashboardIndex}.results.${changeIndex}.starred`,
+ e.detail.starred
+ );
+ }
+ })
+ );
}
_handleToggleReviewed(e: CustomEvent<ChangeListToggleReviewedDetail>) {
@@ -387,6 +400,19 @@
e.detail.change._number,
e.detail.reviewed
);
+ // When a change is updated the same change may appear elsewhere in the
+ // dashboard (but is not the same object), so we must update other
+ // occurrences of the same change.
+ this._results?.forEach((dashboardChange, dashboardIndex) =>
+ dashboardChange.results.forEach((change, changeIndex) => {
+ if (change.id === e.detail.change.id) {
+ this.set(
+ `_results.${dashboardIndex}.results.${changeIndex}.reviewed`,
+ e.detail.reviewed
+ );
+ }
+ })
+ );
}
/**
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js
index 47f885b..a5de72b 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.js
@@ -332,6 +332,56 @@
});
});
+ test('toggling star will update change everywhere', () => {
+ // It is important that the same change is represented by multiple objects
+ // and all are updated.
+ const change = {id: '5', starred: false};
+ const sameChange = {id: '5', starred: false};
+ const differentChange = {id: '4', starred: false};
+ element._results = [
+ {query: 'has:draft', results: [change]},
+ {query: 'is:open', results: [sameChange, differentChange]},
+ ];
+
+ element._handleToggleStar(
+ new CustomEvent('toggle-star', {
+ detail: {
+ change,
+ starred: true,
+ },
+ })
+ );
+
+ assert.isTrue(change.starred);
+ assert.isTrue(sameChange.starred);
+ assert.isFalse(differentChange.starred);
+ });
+
+ test('toggling reviewed will update change everywhere', () => {
+ // It is important that the same change is represented by multiple objects
+ // and all are updated.
+ const change = {id: '5', reviewed: false};
+ const sameChange = {id: '5', reviewed: false};
+ const differentChange = {id: '4', reviewed: false};
+ element._results = [
+ {query: 'has:draft', results: [change]},
+ {query: 'is:open', results: [sameChange, differentChange]},
+ ];
+
+ element._handleToggleReviewed(
+ new CustomEvent('toggle-reviewed', {
+ detail: {
+ change,
+ reviewed: true,
+ },
+ })
+ );
+
+ assert.isTrue(change.reviewed);
+ assert.isTrue(sameChange.reviewed);
+ assert.isFalse(differentChange.reviewed);
+ });
+
test('_showNewUserHelp', () => {
element._loading = false;
element._showNewUserHelp = false;
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index 538fa4969..8322f3c 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -111,13 +111,6 @@
.summaryChip.check iron-icon {
color: var(--gray-foreground);
}
- .summaryChip.info {
- border-color: var(--info-deemphasized-foreground;
- background-color: var(--info-deemphasized-background);
- }
- .summaryChip.info iron-icon {
- color: var(--info-deemphasized-foreground);
- }
`,
];
}
@@ -445,7 +438,6 @@
><gr-summary-chip
styleType=${SummaryChipStyles.WARNING}
category=${CommentTabState.UNRESOLVED}
- icon="message"
?hidden=${!countUnresolvedComments}
>
${unresolvedAuthors.map(
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
index b93040b..8d03e32 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_html.ts
@@ -305,7 +305,7 @@
</gr-button>
</template>
<template is="dom-if" if="[[message._revision_number]]">
- <span class="patchset">[[message._revision_number]]</span>
+ <span class="patchset">[[message._revision_number]] |</span>
</template>
<template is="dom-if" if="[[!message.id]]">
<span class="date">
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index 1b49f8a..31f17724 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {html} from 'lit-html';
+import {html, nothing} from 'lit-html';
import {classMap} from 'lit-html/directives/class-map';
import {
css,
@@ -29,8 +29,10 @@
import {
compareByWorstCategory,
fireActionTriggered,
+ iconForCategory,
iconForRun,
primaryRunAction,
+ worstCategory,
} from '../../services/checks/checks-util';
import {
allRuns$,
@@ -114,16 +116,16 @@
.chip.placeholder iron-icon {
display: none;
}
- .chip.error iron-icon {
+ iron-icon.error {
color: var(--error-foreground);
}
- .chip.warning iron-icon {
+ iron-icon.warning {
color: var(--warning-foreground);
}
- .chip.info-outline iron-icon {
+ iron-icon.info-outline {
color: var(--info-foreground);
}
- .chip.check-circle-outline iron-icon {
+ iron-icon.check-circle-outline {
color: var(--success-foreground);
}
/* Additional 'div' for increased specificity. */
@@ -198,7 +200,8 @@
return html`
<div @click="${this.handleChipClick}" class="${classMap(classes)}">
<div class="left">
- <iron-icon icon="gr-icons:${icon}"></iron-icon>
+ <iron-icon class="${icon}" icon="gr-icons:${icon}"></iron-icon>
+ ${this.renderAdditionalIcon()}
<span class="name">${this.run.checkName}</span>
</div>
<div class="right">
@@ -215,6 +218,20 @@
`;
}
+ /**
+ * For RUNNING we also want to render an icon representing the worst result
+ * that has been reported until now - if there are any results already.
+ */
+ renderAdditionalIcon() {
+ if (this.run.status !== RunStatus.RUNNING) return nothing;
+ const category = worstCategory(this.run);
+ if (!category) return nothing;
+ const icon = iconForCategory(category);
+ return html`
+ <iron-icon class="${icon}" icon="gr-icons:${icon}"></iron-icon>
+ `;
+ }
+
private handleChipClick(e: MouseEvent) {
e.stopPropagation();
e.preventDefault();
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
index 374cc62..0f530bf 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-content/gr-editable-content_html.ts
@@ -68,6 +68,11 @@
border-radius: 0 0 4px 4px;
border-color: var(--border-color);
box-shadow: var(--elevation-level-1);
+ /* slightly up to cover rounded corner of the commit msg */
+ margin-top: calc(-1 * var(--spacing-xs));
+ /* To make this bar pop over editor, since editor has relative position.
+ */
+ position: relative;
}
.show-all-container .show-all-button {
margin-right: auto;
diff --git a/polygerrit-ui/app/services/checks/checks-util.ts b/polygerrit-ui/app/services/checks/checks-util.ts
index 1613359..ea532ea 100644
--- a/polygerrit-ui/app/services/checks/checks-util.ts
+++ b/polygerrit-ui/app/services/checks/checks-util.ts
@@ -76,8 +76,12 @@
}
export function iconForRun(run: CheckRun) {
- const category = worstCategory(run);
- return category ? iconForCategory(category) : iconForStatus(run.status);
+ if (run.status !== RunStatus.COMPLETED) {
+ return iconForStatus(run.status);
+ } else {
+ const category = worstCategory(run);
+ return category ? iconForCategory(category) : iconForStatus(run.status);
+ }
}
export function iconForStatus(status: RunStatus) {
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts
index 2d83cf5..18c12b0 100644
--- a/polygerrit-ui/app/styles/themes/app-theme.ts
+++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -76,8 +76,6 @@
--info-background: var(--blue-50);
--selected-foreground: var(--blue-700);
--selected-background: var(--blue-50);
- --info-deemphasized-foreground: var(--gray-300);
- --info-deemphasized-background: var(--gray-50);
--success-foreground: var(--green-700);
--success-background: var(--green-50);
--gray-foreground: var(--gray-700);
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts
index 072ca8f..5455a24 100644
--- a/polygerrit-ui/app/styles/themes/dark-theme.ts
+++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -46,8 +46,6 @@
--info-background: var(--blue-900);
--selected-foreground: var(--blue-200);
--selected-background: var(--blue-900);
- --info-deemphasized-foreground: var(--gray-700);
- --info-deemphasized-background: var(--primary-text-color);
--success-foreground: var(--green-200);
--success-background: var(--green-900);
--gray-foreground: var(--gray-100);
