Merge "Fix DynamicOptions to support a custom bean"
diff --git a/Documentation/config-accounts.txt b/Documentation/config-accounts.txt
index b4a5cef..c6d9fb4 100644
--- a/Documentation/config-accounts.txt
+++ b/Documentation/config-accounts.txt
@@ -343,6 +343,12 @@
The `accountId` field is mandatory. The `email` and `password` fields
are optional.
+Note that git will automatically nest these notes at varying levels. If
+refs/meta/external-ids:7c/2a55657d911109dbc930836e7a770fb946e8ef is not
+found then check
+refs/meta/external-ids:7c/2a/55657d911109dbc930836e7a770fb946e8ef and
+so on.
+
The external IDs are maintained by Gerrit. This means users are not
allowed to manually edit their external IDs. Only users with the
link:access-control.html#capability_accessDatabase[Access Database]
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index b2e1589..1db27d5 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2729,8 +2729,8 @@
[source, java]
----
import java.util.Optional;
-import com.google.gerrit.common.data.SubmitRecord;
-import com.google.gerrit.common.data.SubmitRecord.Status;
+import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.entities.SubmitRecord.Status;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.SubmitRule;
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index d34ccb4..6889de3 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -4196,7 +4196,9 @@
repository.<name>.defaultSubmitType] is set to a different value.
|`branches` |optional|
A list of branches that should be initially created. +
-For the branch names the `refs/heads/` prefix can be omitted.
+For the branch names the `refs/heads/` prefix can be omitted. +
+The first entry of the list will be the default branch (ie. the target +
+of the `HEAD` symbolic ref).
|`owners` |optional|
A list of groups that should be assigned as project owner. +
Each group in the list must be specified as
diff --git a/WORKSPACE b/WORKSPACE
index 3c30026..fff9235 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -51,10 +51,10 @@
http_archive(
name = "com_google_protobuf",
- sha256 = "71030a04aedf9f612d2991c1c552317038c3c5a2b578ac4745267a45e7037c29",
- strip_prefix = "protobuf-3.12.3",
+ sha256 = "d0f5f605d0d656007ce6c8b5a82df3037e1d8fe8b121ed42e536f569dec16113",
+ strip_prefix = "protobuf-3.14.0",
urls = [
- "https://github.com/protocolbuffers/protobuf/archive/v3.12.3.tar.gz",
+ "https://github.com/protocolbuffers/protobuf/archive/v3.14.0.tar.gz",
],
)
diff --git a/java/com/google/gerrit/entities/CoreDownloadSchemes.java b/java/com/google/gerrit/entities/CoreDownloadSchemes.java
index 37c10f1..9bcd365 100644
--- a/java/com/google/gerrit/entities/CoreDownloadSchemes.java
+++ b/java/com/google/gerrit/entities/CoreDownloadSchemes.java
@@ -21,6 +21,7 @@
public static final String HTTP = "http";
public static final String SSH = "ssh";
public static final String REPO_DOWNLOAD = "repo";
+ public static final String REPO = "repo";
private CoreDownloadSchemes() {}
}
diff --git a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
index 681d0bd..4cb52b7 100644
--- a/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
+++ b/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
@@ -27,7 +27,6 @@
/** Preferred method to download a change. */
public enum DownloadCommand {
- REPO_DOWNLOAD,
PULL,
CHECKOUT,
CHERRY_PICK,
diff --git a/java/com/google/gerrit/server/change/DeleteReviewerOp.java b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
index 07cb04f..bf00d27 100644
--- a/java/com/google/gerrit/server/change/DeleteReviewerOp.java
+++ b/java/com/google/gerrit/server/change/DeleteReviewerOp.java
@@ -40,6 +40,7 @@
import com.google.gerrit.server.mail.send.DeleteReviewerSender;
import com.google.gerrit.server.mail.send.MessageIdGenerator;
import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.notedb.ReviewerStateInternal;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.RemoveReviewerControl;
@@ -132,9 +133,15 @@
for (LabelType lt : labelTypes.getLabelTypes()) {
newApprovals.put(lt.getName(), (short) 0);
}
-
+ String ccOrReviewer =
+ approvalsUtil
+ .getReviewers(ctx.getNotes())
+ .byState(ReviewerStateInternal.CC)
+ .contains(reviewerId)
+ ? "cc"
+ : "reviewer";
StringBuilder msg = new StringBuilder();
- msg.append("Removed reviewer " + reviewer.account().fullName());
+ msg.append(String.format("Removed %s %s", ccOrReviewer, reviewer.account().fullName()));
StringBuilder removedVotesMsg = new StringBuilder();
removedVotesMsg.append(" with the following votes:\n\n");
boolean votesRemoved = false;
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
index 1650421..220e683 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesCache.java
@@ -121,12 +121,18 @@
// Single Timestamp overhead.
private static final int T = O + 8;
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Take all columns and all collection sizes into account, but use estimated average element
+ * sizes rather than iterating over collections. Numbers are largely hand-wavy based on
+ * http://stackoverflow.com/questions/258120/what-is-the-memory-consumption-of-an-object-in-java
+ *
+ * <p>Should be kept up to date with {@link ChangeNotesState}. Please, keep weights listed in
+ * the same order as fields.
+ */
@Override
public int weigh(Key key, ChangeNotesState state) {
- // Take all columns and all collection sizes into account, but use
- // estimated average element sizes rather than iterating over collections.
- // Numbers are largely hand-wavy based on
- // http://stackoverflow.com/questions/258120/what-is-the-memory-consumption-of-an-object-in-java
return P
+ O
+ 20 // metaId
@@ -138,6 +144,7 @@
+ K // owner
+ P
+ str(state.columns().branch())
+ + P // status
+ P
+ patchSetId() // currentPatchSetId
+ P
@@ -148,9 +155,16 @@
+ str(state.columns().originalSubject())
+ P
+ str(state.columns().submissionId())
- + P // status
+ + 1 // isPrivate
+ + 1 // workInProgress
+ + 1 // reviewStarted
+ + P
+ + K // revertOf
+ + P
+ + patchSetId() // cherryPickOf
+ P
+ set(state.hashtags(), str(10))
+ + str(state.serverId()) // serverId
+ P
+ list(state.patchSets(), patchSet())
+ P
@@ -177,9 +191,6 @@
+ list(state.changeMessages(), changeMessage())
+ P
+ map(state.publishedComments().asMap(), comment())
- + 1 // isPrivate
- + 1 // workInProgress
- + 1 // reviewStarted
+ I; // updateCount
}
diff --git a/java/com/google/gerrit/server/notedb/ChangeNotesState.java b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
index fa32686..27cfb70 100644
--- a/java/com/google/gerrit/server/notedb/ChangeNotesState.java
+++ b/java/com/google/gerrit/server/notedb/ChangeNotesState.java
@@ -81,6 +81,9 @@
* <p>One instance is the output of a single {@link ChangeNotesParser}, and contains types required
* to support public methods on {@link ChangeNotes}. It is intended to be cached in-process.
*
+ * <p>When new fields are added to the {@link ChangeNotesState}, {@link
+ * ChangeNotesCache.Weigher#weigh} should be updated.
+ *
* <p>Note that {@link ChangeNotes} contains more than just a single {@code ChangeNoteState}, such
* as per-draft information, so that class is not cached directly.
*/
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index f59fba0..b7f1ef0 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -2258,6 +2258,10 @@
assertThat(message.body()).contains("Removed reviewer " + user.fullName() + ".");
assertThat(message.body()).doesNotContain("with the following votes");
+ // Make sure the change message for removing a reviewer is correct.
+ assertThat(Iterables.getLast(gApi.changes().id(changeId).messages()).message)
+ .contains("Removed reviewer " + user.fullName());
+
// Make sure the reviewer can still be added again.
gApi.changes().id(changeId).addReviewer(user.id().toString());
c = gApi.changes().id(changeId).get();
@@ -2273,6 +2277,31 @@
}
@Test
+ public void removeCC() throws Exception {
+ PushOneCommit.Result result = createChange();
+ String changeId = result.getChangeId();
+ // Add a cc
+ AddReviewerInput addReviewerInput = new AddReviewerInput();
+ addReviewerInput.state = CC;
+ addReviewerInput.reviewer = user.id().toString();
+ gApi.changes().id(changeId).addReviewer(addReviewerInput);
+
+ // Remove a cc
+ sender.clear();
+ gApi.changes().id(changeId).reviewer(user.id().toString()).remove();
+ assertThat(gApi.changes().id(changeId).get().reviewers).isEmpty();
+
+ // Make sure the email for removing a cc is correct.
+ assertThat(sender.getMessages()).hasSize(1);
+ Message message = sender.getMessages().get(0);
+ assertThat(message.body()).contains("Removed cc " + user.fullName() + ".");
+
+ // Make sure the change message for removing a reviewer is correct.
+ assertThat(Iterables.getLast(gApi.changes().id(changeId).messages()).message)
+ .contains("Removed cc " + user.fullName());
+ }
+
+ @Test
public void removeReviewer() throws Exception {
testRemoveReviewer(true);
}
diff --git a/plugins/download-commands b/plugins/download-commands
index 87e3930..cfa03bc 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit 87e3930cea7c06aea454998abdddf6515a9f103b
+Subproject commit cfa03bc5e7a7e1e27b83f2dba60e9a9eb7c8f4aa
diff --git a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
index 2582170..e5a1586 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-audit-log/gr-group-audit-log.ts
@@ -80,7 +80,9 @@
return '';
}
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getGroupAuditLog(this.groupId, errFn)
diff --git a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
index 1cd55ca..01571a2 100644
--- a/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group-members/gr-group-members.ts
@@ -140,7 +140,9 @@
const promises: Promise<void>[] = [];
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI.getGroupConfig(this.groupId, errFn).then(config => {
if (!config || !config.name) {
diff --git a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
index 95a6e5d..4525543 100644
--- a/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
+++ b/polygerrit-ui/app/elements/admin/gr-group/gr-group.ts
@@ -145,7 +145,9 @@
const promises: Promise<unknown>[] = [];
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI.getGroupConfig(this.groupId, errFn).then(config => {
if (!config || !config.name) {
diff --git a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
index 922f592..9337042 100644
--- a/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-plugin-list/gr-plugin-list.ts
@@ -98,7 +98,9 @@
}
_getPlugins(filter: string, pluginsPerPage: number, offset?: number) {
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getPlugins(filter, pluginsPerPage, offset, errFn)
.then(plugins => {
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
index fde7234..31e1f65 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-access/gr-repo-access.ts
@@ -155,7 +155,9 @@
}
_reload(repo: RepoName) {
- const errFn = (response?: Response | null) => firePageError(this, response);
+ const errFn = (response?: Response | null) => {
+ firePageError(this, response);
+ };
this._editing = false;
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
index f2735fb..99f85fa 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-dashboards/gr-repo-dashboards.ts
@@ -62,7 +62,9 @@
return Promise.resolve();
}
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getRepoDashboards(repo, errFn)
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
index e4e9d34..17414c0 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-detail-list/gr-repo-detail-list.ts
@@ -182,7 +182,9 @@
this._loading = true;
this._items = [];
flush();
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
if (detailType === RepoDetailView.BRANCHES) {
return this.$.restAPI
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
index 3acf8fa..426e512 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.ts
@@ -176,7 +176,9 @@
const promises = [];
- const errFn: ErrorCallback = response => firePageError(this, response);
+ const errFn: ErrorCallback = response => {
+ firePageError(this, response);
+ };
promises.push(
this._getLoggedIn().then(loggedIn => {
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 dbaed07..ce3a811 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
@@ -150,7 +150,9 @@
project: RepoName,
dashboard: DashboardId
): Promise<UserDashboard | undefined> {
- const errFn = (response?: Response | null) => firePageError(this, response);
+ const errFn = (response?: Response | null) => {
+ firePageError(this, response);
+ };
return this.$.restAPI
.getDashboard(project, dashboard, errFn)
.then(response => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
index 8989440..20fbc32 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts
@@ -42,6 +42,7 @@
import {
fetchChangeUpdates,
patchNumEquals,
+ CURRENT,
} from '../../../utils/patch-set-util';
import {
changeIsOpen,
@@ -114,6 +115,7 @@
UIActionInfo,
} from '../../shared/gr-js-api-interface/gr-change-actions-js-api';
import {fireAlert} from '../../../utils/event-util';
+import {CODE_REVIEW} from '../../../utils/label-util';
const ERR_BRANCH_EMPTY = 'The destination branch can’t be empty.';
const ERR_COMMIT_EMPTY = 'The commit message can’t be empty.';
@@ -947,6 +949,16 @@
return null;
}
}
+ // Allow the user to use quick approve to vote the max score on code review
+ // even if it is already granted.
+ if (
+ !result &&
+ this.change.labels[CODE_REVIEW] &&
+ this._getLabelStatus(this.change.labels[CODE_REVIEW]) === LabelStatus.OK
+ ) {
+ result = CODE_REVIEW;
+ }
+
if (result) {
const score = this.change.permitted_labels[result].slice(-1)[0];
const labelInfo = this.change.labels[result];
@@ -1580,7 +1592,7 @@
if (!labels) {
return Promise.resolve(undefined);
}
- return this.$.restAPI.saveChangeReview(newChangeId, 'current', {labels});
+ return this.$.restAPI.saveChangeReview(newChangeId, CURRENT, {labels});
}
_handleResponse(action: UIActionInfo, response?: Response) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
index ae49a57..1098760 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.js
@@ -1717,6 +1717,31 @@
.querySelector('gr-button[data-action-key=\'review\']');
assert.equal(approveButton.getAttribute('data-label'), 'bar+2');
});
+
+ test('added when can approve an already-approved code review label',
+ () => {
+ element.change = {
+ current_revision: 'abc1234',
+ labels: {
+ 'Code-Review': {
+ approved: {},
+ values: {
+ ' 0': '',
+ '+1': '',
+ '+2': '',
+ },
+ },
+ },
+ permitted_labels: {
+ 'Code-Review': [' 0', '+1', '+2'],
+ },
+ };
+ flush();
+ const approveButton =
+ element.shadowRoot
+ .querySelector('gr-button[data-action-key=\'review\']');
+ assert.isNotNull(approveButton);
+ });
});
test('adds download revision action', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index f8a5940..6b04153 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -76,6 +76,13 @@
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {GrEditableLabel} from '../../shared/gr-editable-label/gr-editable-label';
import {GrLinkedChip} from '../../shared/gr-linked-chip/gr-linked-chip';
+import {appContext} from '../../../services/app-context';
+import {KnownExperimentId} from '../../../services/flags/flags';
+import {
+ Metadata,
+ isSectionSet,
+ DisplayRules,
+} from '../../../utils/change-metadata-util';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -194,6 +201,25 @@
@property({type: Object})
_CHANGE_ROLE = ChangeRole;
+ @property({type: Object})
+ _SECTION = Metadata;
+
+ @property({type: Boolean})
+ _showAllSections = false;
+
+ @property({type: Boolean})
+ _isNewChangeSummaryUiEnabled = false;
+
+ flagsService = appContext.flagsService;
+
+ /** @override */
+ ready() {
+ super.ready();
+ this._isNewChangeSummaryUiEnabled = this.flagsService.isEnabled(
+ KnownExperimentId.NEW_CHANGE_SUMMARY_UI
+ );
+ }
+
@observe('change.labels')
_labelsChanged(labels?: LabelNameToInfoMap) {
this.labels = {...labels} || null;
@@ -566,6 +592,35 @@
return this._getNonOwnerRole(change, role) ? '' : 'hideDisplay';
}
+ _computeDisplayState(
+ showAllSections: boolean,
+ change: ParsedChangeInfo | undefined,
+ section: Metadata
+ ) {
+ if (
+ !this._isNewChangeSummaryUiEnabled ||
+ showAllSections ||
+ DisplayRules.ALWAYS_SHOW.includes(section) ||
+ (DisplayRules.SHOW_IF_SET.includes(section) &&
+ isSectionSet(section, change))
+ ) {
+ return '';
+ }
+ return 'hideDisplay';
+ }
+
+ _computeShowAllLabelText(showAllSections: boolean) {
+ if (showAllSections) {
+ return 'Show less';
+ } else {
+ return 'Show all';
+ }
+ }
+
+ _onShowAllClick() {
+ this._showAllSections = !this._showAllSections;
+ }
+
/**
* Get the user with the specified role on the change. Returns null if the
* user with that role is the same as the owner.
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
index f1d1127..88f7351 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_html.ts
@@ -94,9 +94,31 @@
--account-max-length: 120px;
max-width: 285px;
}
+ .metadata-title {
+ font-weight: var(--font-weight-bold);
+ color: var(--deemphasized-text-color);
+ padding-left: var(--metadata-horizontal-padding);
+ }
+ .metadata-header {
+ display: flex;
+ justify-content: space-between;
+ }
</style>
<gr-external-style id="externalStyle" name="change-metadata">
- <section>
+ <template is="dom-if" if="[[_isNewChangeSummaryUiEnabled]]">
+ <div class="metadata-header">
+ <h3 class="metadata-title">Change Info</h3>
+ <gr-button
+ class="show-all-button"
+ on-click="_onShowAllClick"
+ no-uppercase=""
+ >[[_computeShowAllLabelText(_showAllSections)]]</gr-button
+ >
+ </div>
+ </template>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.UPDATED)]]"
+ >
<span class="title">Updated</span>
<span class="value">
<gr-date-formatter
@@ -105,7 +127,9 @@
></gr-date-formatter>
</span>
</section>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.OWNER)]]"
+ >
<span class="title">Owner</span>
<span class="value">
<gr-account-chip
@@ -156,7 +180,9 @@
</span>
</section>
<template is="dom-if" if="[[_isAssigneeEnabled(serverConfig)]]">
- <section class="assignee">
+ <section
+ class$="assignee [[_computeDisplayState(_showAllSections, change, _SECTION.ASSIGNEE)]]"
+ >
<span class="title">Assignee</span>
<span class="value">
<gr-account-list
@@ -172,7 +198,9 @@
</span>
</section>
</template>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REVIEWERS)]]"
+ >
<span class="title">Reviewers</span>
<span class="value">
<gr-reviewer-list
@@ -183,7 +211,9 @@
></gr-reviewer-list>
</span>
</section>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.CC)]]"
+ >
<span class="title">CC</span>
<span class="value">
<gr-reviewer-list
@@ -198,7 +228,9 @@
is="dom-if"
if="[[_computeShowRepoBranchTogether(change.project, change.branch)]]"
>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REPO_BRANCH)]]"
+ >
<span class="title">Repo | Branch</span>
<span class="value">
<a href$="[[_computeProjectUrl(change.project)]]"
@@ -215,7 +247,9 @@
is="dom-if"
if="[[!_computeShowRepoBranchTogether(change.project, change.branch)]]"
>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REPO_BRANCH)]]"
+ >
<span class="title">Repo</span>
<span class="value">
<a href$="[[_computeProjectUrl(change.project)]]">
@@ -226,7 +260,9 @@
</a>
</span>
</section>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.REPO_BRANCH)]]"
+ >
<span class="title">Branch</span>
<span class="value">
<a href$="[[_computeBranchUrl(change.project, change.branch)]]">
@@ -238,7 +274,9 @@
</span>
</section>
</template>
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.PARENT)]]"
+ >
<span class="title">[[_computeParentsLabel(_currentParents)]]</span>
<span class="value">
<ol
@@ -262,7 +300,9 @@
</ol>
</span>
</section>
- <section class="topic">
+ <section
+ class$="topic [[_computeDisplayState(_showAllSections, change, _SECTION.TOPIC)]]"
+ >
<span class="title">Topic</span>
<span class="value">
<template is="dom-if" if="[[_showTopicChip(change.*, _settingTopic)]]">
@@ -288,7 +328,9 @@
</span>
</section>
<template is="dom-if" if="[[_showCherryPickOf(change.*)]]">
- <section>
+ <section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.CHERRY_PICK_OF)]]"
+ >
<span class="title">Cherry pick of</span>
<span class="value">
<a
@@ -304,14 +346,16 @@
</section>
</template>
<section
- class="strategy"
+ class$="strategy [[_computeDisplayState(_showAllSections, change, _SECTION.STRATEGY)]]"
hidden$="[[_computeHideStrategy(change)]]"
hidden=""
>
<span class="title">Strategy</span>
<span class="value">[[_computeStrategy(change)]]</span>
</section>
- <section class="hashtag">
+ <section
+ class$="hashtag [[_computeDisplayState(_showAllSections, change, _SECTION.HASHTAGS)]]"
+ >
<span class="title">Hashtags</span>
<span class="value">
<template is="dom-repeat" items="[[change.hashtags]]">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 108f9ed..7759b7b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -1558,7 +1558,7 @@
'changeComments, patchRange and diffPrefs must be set'
);
}
- diffElem.comments = this.changeComments.getCommentsBySideForFile(
+ diffElem.threads = this.changeComments.getThreadsBySideForFile(
file,
this.patchRange,
this.projectConfig
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
index 4f3d7ce6..e4e4056 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.ts
@@ -336,7 +336,7 @@
const el = this._createToastAlert();
el.show(text, actionText, actionCallback);
this._alertElement = el;
- this.fire('iron-announce', {text}, {bubbles: true});
+ this.fire('iron-announce', {text: `Alert: ${text}`}, {bubbles: true});
this.reporting.reportInteraction('show-alert', {text});
}
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index b01a110..ef5be9fd 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -23,6 +23,7 @@
getParentIndex,
isMergeParent,
patchNumEquals,
+ CURRENT,
} from '../../../utils/patch-set-util';
import {customElement, property} from '@polymer/decorators';
import {
@@ -38,7 +39,7 @@
RevisionId,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
-import {CommentSide} from '../../../constants/constants';
+import {CommentSide, Side} from '../../../constants/constants';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {
Comment,
@@ -313,6 +314,30 @@
return allDrafts;
}
+ _addCommentSide(comments: TwoSidesComments) {
+ const allComments = [];
+ for (const side of [Side.LEFT, Side.RIGHT]) {
+ // This is needed by the threading.
+ for (const comment of comments[side]) {
+ comment.__commentSide = side;
+ }
+ allComments.push(...comments[side]);
+ }
+ return allComments;
+ }
+
+ getThreadsBySideForPath(
+ path: string,
+ patchRange: PatchRange,
+ projectConfig?: ConfigInfo
+ ): CommentThread[] {
+ return createCommentThreads(
+ this._addCommentSide(
+ this.getCommentsBySideForPath(path, patchRange, projectConfig)
+ )
+ );
+ }
+
/**
* Get the comments (with drafts and robot comments) for a path and
* patch-range. Returns an object with left and right properties mapping to
@@ -371,6 +396,18 @@
};
}
+ getThreadsBySideForFile(
+ file: PatchSetFile,
+ patchRange: PatchRange,
+ projectConfig?: ConfigInfo
+ ): CommentThread[] {
+ return createCommentThreads(
+ this._addCommentSide(
+ this.getCommentsBySideForFile(file, patchRange, projectConfig)
+ )
+ );
+ }
+
/**
* Get the comments (with drafts and robot comments) for a file and
* patch-range. Returns an object with left and right properties mapping to
@@ -576,7 +613,7 @@
}
getPortedComments(changeNum: NumericChangeId, revision?: RevisionId) {
- if (!revision) revision = 'current';
+ if (!revision) revision = CURRENT;
return Promise.all([
this.$.restAPI.getPortedComments(changeNum, revision),
this.$.restAPI.getPortedDrafts(changeNum, revision),
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
index 1ae302a..ecedb28 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.ts
@@ -105,6 +105,8 @@
row.classList.add('diff-row', 'side-by-side');
row.setAttribute('left-type', leftLine.type);
row.setAttribute('right-type', rightLine.type);
+ // TabIndex makes screen reader read a row when navigating with j/k
+ row.tabIndex = -1;
row.appendChild(this._createBlameCell(leftLine.beforeNumber));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
index 04ac472..2011a59 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.ts
@@ -104,6 +104,8 @@
_createRow(line: GrDiffLine) {
const row = this._createElement('tr', line.type);
row.classList.add('diff-row', 'unified');
+ // TabIndex makes screen reader read a row when navigating with j/k
+ row.tabIndex = -1;
row.appendChild(this._createBlameCell(line.beforeNumber));
let lineNumberEl = this._createLineEl(
line,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
index 684a58a..43ed77f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.ts
@@ -226,9 +226,10 @@
bubbles: true,
})
);
+ } else {
+ this.lastDisplayedNavigateToNextFileToast = Date.now();
+ fireAlert(this, 'Press n again to navigate to next unreviewed file');
}
- this.lastDisplayedNavigateToNextFileToast = Date.now();
- fireAlert(this, 'Press n again to navigate to next unreviewed file');
}
this._fixSide();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 8c08084..4b4f429 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -31,14 +31,7 @@
isMergeParent,
isNumber,
} from '../../../utils/patch-set-util';
-import {
- Comment,
- isDraft,
- UIComment,
- CommentThread,
- createCommentThreads,
-} from '../../../utils/comment-util';
-import {TwoSidesComments} from '../gr-comment-api/gr-comment-api';
+import {CommentThread} from '../../../utils/comment-util';
import {customElement, observe, property} from '@polymer/decorators';
import {
CommitRange,
@@ -198,8 +191,8 @@
@property({type: Boolean})
noRenderOnPrefsChange = false;
- @property({type: Object, observer: '_commentsChanged'})
- comments?: TwoSidesComments;
+ @property({type: Object, observer: '_threadsChanged'})
+ threads?: CommentThread[];
@property({type: Boolean})
lineWrapping = false;
@@ -276,11 +269,12 @@
'create-comment',
e => this._handleCreateComment(e)
);
- this.addEventListener('comment-discard', e =>
- this._handleCommentDiscard(e)
+ this.addEventListener('comment-discard', () =>
+ this._handleCommentSaveOrDiscard()
);
- this.addEventListener('comment-update', e => this._handleCommentUpdate(e));
- this.addEventListener('comment-save', e => this._handleCommentSave(e));
+ this.addEventListener('comment-save', () =>
+ this._handleCommentSaveOrDiscard()
+ );
this.addEventListener('render-start', () => this._handleRenderStart());
this.addEventListener('render-content', () => this._handleRenderContent());
this.addEventListener('normalize-range', event =>
@@ -679,21 +673,12 @@
return isImageDiff(diff);
}
- _commentsChanged(newComments: TwoSidesComments) {
- const allComments = [];
- for (const side of [Side.LEFT, Side.RIGHT]) {
- // This is needed by the threading.
- for (const comment of newComments[side]) {
- comment.__commentSide = side;
- }
- allComments.push(...newComments[side]);
- }
+ _threadsChanged(threads: CommentThread[]) {
// Currently, the only way this is ever changed here is when the initial
- // comments are loaded, so it's okay performance wise to clear the threads
+ // threads are loaded, so it's okay performance wise to clear the threads
// and recreate them. If this changes in future, we might want to reuse
// some DOM nodes here.
this._clearThreads();
- const threads = createCommentThreads(allComments);
for (const thread of threads) {
const threadEl = this._createThreadElement(thread);
this._attachThreadElement(threadEl);
@@ -932,79 +917,12 @@
: null;
}
- _handleCommentSave(e: CustomEvent) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- const idx = this._findDraftIndex(comment, side);
- this.set(['comments', side, idx], comment);
- this._handleCommentSaveOrDiscard();
- }
-
- _handleCommentDiscard(e: CustomEvent) {
- const comment = e.detail.comment;
- this._removeComment(comment);
- this._handleCommentSaveOrDiscard();
- }
-
- _handleCommentUpdate(e: CustomEvent) {
- const comment = e.detail.comment;
- const side = e.detail.comment.__commentSide;
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) {
- // Update draft or comment.
- this.set(['comments', side, idx], comment);
- } else {
- // Create new draft.
- this.push(['comments', side], comment);
- }
- }
-
_handleCommentSaveOrDiscard() {
this.dispatchEvent(
new CustomEvent('diff-comments-modified', {bubbles: true, composed: true})
);
}
- _removeComment(comment: UIComment) {
- const side = comment.__commentSide;
- if (!side) throw new Error('Missing required "side" in comment.');
- this._removeCommentFromSide(comment, side);
- }
-
- _removeCommentFromSide(comment: Comment, side: Side) {
- let idx = this._findCommentIndex(comment, side);
- if (idx === -1) {
- idx = this._findDraftIndex(comment, side);
- }
- if (idx !== -1) {
- this.splice('comments.' + side, idx, 1);
- }
- }
-
- _findCommentIndex(comment: Comment, side: Side) {
- if (!comment.id || !this.comments || !this.comments[side]) {
- return -1;
- }
- return this.comments[side].findIndex(item => item.id === comment.id);
- }
-
- _findDraftIndex(comment: Comment, side: Side) {
- if (
- !isDraft(comment) ||
- !comment.__draftID ||
- !this.comments ||
- !this.comments[side]
- ) {
- return -1;
- }
- return this.comments[side].findIndex(
- item => isDraft(item) && item.__draftID === comment.__draftID
- );
- }
-
_isSyntaxHighlightingEnabled(
preferenceChangeRecord?: PolymerDeepPropertyChange<
DiffPreferencesInfo,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index bf4d912..402cb52 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -62,198 +62,6 @@
});
});
- suite('handle comment-update', () => {
- setup(() => {
- sinon.stub(element, '_commentsChanged');
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
- });
-
- test('creating a draft', () => {
- const comment = {__draft: true, __draftID: 'tempID', side: 'PARENT',
- __commentSide: 'left'};
- element.dispatchEvent(
- new CustomEvent('comment-update', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- assert.include(element.comments.left, comment);
- });
-
- test('discarding a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sinon.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.dispatchEvent(
- new CustomEvent('comment-discard', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- const drafts = element.comments.left
- .filter(item => item.__draftID === draftID);
- assert.equal(drafts.length, 0);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
-
- test('saving a draft', () => {
- const draftID = 'tempID';
- const id = 'savedID';
- const comment = {
- __draft: true,
- __draftID: draftID,
- side: 'PARENT',
- __commentSide: 'left',
- };
- const diffCommentsModifiedStub = sinon.stub();
- element.addEventListener('diff-comments-modified',
- diffCommentsModifiedStub);
- element.comments.left.push(comment);
- comment.id = id;
- element.dispatchEvent(
- new CustomEvent('comment-save', {
- detail: {comment},
- composed: true, bubbles: true,
- }));
- const drafts = element.comments.left
- .filter(item => item.__draftID === draftID);
- assert.equal(drafts.length, 1);
- assert.equal(drafts[0].id, id);
- assert.isTrue(diffCommentsModifiedStub.called);
- });
- });
-
- test('remove comment', () => {
- sinon.stub(element, '_commentsChanged');
- element.comments = {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- };
-
- // Using JSON.stringify because Safari 9.1 (11601.5.17.1) doesn’t seem
- // to believe that one object deepEquals another even when they do :-/.
- assert.equal(JSON.stringify(element.comments), JSON.stringify({
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- }));
-
- element._removeComment({id: 'bc2', side: 'PARENT',
- __commentSide: 'left'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- {id: 'd2', __draft: true, __commentSide: 'right'},
- ],
- });
-
- element._removeComment({id: 'd2', __commentSide: 'right'});
- assert.deepEqual(element.comments, {
- meta: {
- changeNum: '42',
- patchRange: {
- basePatchNum: 'PARENT',
- patchNum: 3,
- },
- path: '/path/to/foo',
- projectConfig: {foo: 'bar'},
- },
- left: [
- {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
- {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
- {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
- ],
- right: [
- {id: 'c1', __commentSide: 'right'},
- {id: 'c2', __commentSide: 'right'},
- {id: 'd1', __draft: true, __commentSide: 'right'},
- ],
- });
- });
-
test('thread-discard handling', () => {
const threads = createCommentThreads([
{
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 c169bf4..5a6113c 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
@@ -48,6 +48,7 @@
computeLatestPatchNum,
patchNumEquals,
PatchSet,
+ CURRENT,
} from '../../../utils/patch-set-util';
import {
addUnmodifiedFiles,
@@ -65,11 +66,7 @@
GrDropdownList,
} from '../../shared/gr-dropdown-list/gr-dropdown-list';
import {GrOverlay} from '../../shared/gr-overlay/gr-overlay';
-import {
- ChangeComments,
- GrCommentApi,
- TwoSidesComments,
-} from '../gr-comment-api/gr-comment-api';
+import {ChangeComments, GrCommentApi} from '../gr-comment-api/gr-comment-api';
import {GrDiffModeSelector} from '../gr-diff-mode-selector/gr-diff-mode-selector';
import {
ChangeInfo,
@@ -240,9 +237,6 @@
@property({type: Object})
_commentMap?: CommentMap;
- @property({type: Object})
- _commentsForDiff?: TwoSidesComments;
-
@property({
type: Object,
computed: '_computeCommentSkips(_commentMap, _fileList, _path)',
@@ -1013,12 +1007,6 @@
}
this._commentMap = this._getPaths(this._patchRange);
-
- this._commentsForDiff = this._getCommentsForPath(
- this._path,
- this._patchRange,
- this._projectConfig
- );
}
_isFileUnchanged(diff: DiffInfo) {
@@ -1060,7 +1048,7 @@
const portedCommentsPromise = this.$.commentAPI.getPortedComments(
value.changeNum,
- value.patchNum || 'current'
+ value.patchNum || CURRENT
);
const promises: Promise<unknown>[] = [];
@@ -1087,7 +1075,13 @@
this._loading = false;
this._initPatchRange();
this._initCommitRange();
- this.$.diffHost.comments = this._commentsForDiff;
+ if (this._changeComments && this._path && this._patchRange) {
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForPath(
+ this._path,
+ this._patchRange,
+ this._projectConfig
+ );
+ }
portedCommentsPromise.then(() => {
this.reporting.timeEnd(PORTING_COMMENTS_DIFF_LATENCY_LABEL);
});
@@ -1575,13 +1569,11 @@
const file = files[path];
if (file && file.old_path) {
- this._commentsForDiff = this._changeComments.getCommentsBySideForFile(
+ this.$.diffHost.threads = this._changeComments.getThreadsBySideForFile(
{path, basePath: file.old_path},
patchRange,
projectConfig
);
-
- this.$.diffHost.comments = this._commentsForDiff;
}
}
@@ -1590,22 +1582,6 @@
return this._changeComments.getPaths(patchRange);
}
- _getCommentsForPath(
- path?: string,
- patchRange?: PatchRange,
- projectConfig?: ConfigInfo
- ) {
- if (!path) return undefined;
- if (!patchRange) return undefined;
- if (!this._changeComments) return undefined;
-
- return this._changeComments.getCommentsBySideForPath(
- path,
- patchRange,
- projectConfig
- );
- }
-
_getDiffDrafts() {
if (!this._changeNum) throw new Error('Missing this._changeNum');
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 7ef75ed..8e1e5c1 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
@@ -151,8 +151,10 @@
computeCommentThreadCount: () => {},
computeUnresolvedNum: () => {},
getPaths: () => {},
- getCommentsBySideForPath: () => {},
+ getThreadsBySideForPath: () => {},
+ getThreadsBySideForFile: () => {},
findCommentById: _testOnly_findCommentById,
+
}));
await element._loadComments();
await flush();
@@ -200,11 +202,6 @@
commentLink: true,
commentId: 'c1',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
- sinon.stub(element, '_getCommentsForPath').returns({
- left: [{id: 'c1', __commentSide: 'left', line: 10}],
- right: [{id: 'c2', __commentSide: 'right', line: 11}],
- });
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -263,7 +260,6 @@
commentLink: true,
commentId: 'c1',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -293,7 +289,6 @@
commentLink: true,
commentId: 'c3',
};
- sinon.stub(element.$.diffHost, '_commentsChanged');
element._change = {
...createChange(),
revisions: createRevisions(11),
@@ -1455,7 +1450,6 @@
await flush();
});
test('empty', () => {
- sinon.stub(element, '_getCommentsForPath');
sinon.stub(element, '_getPaths').returns(new Map());
element._initPatchRange();
assert.equal(Object.keys(element._commentMap).length, 0);
@@ -1467,7 +1461,6 @@
'path/to/file/one.cpp': [{patch_set: 3, message: 'lorem'}],
'path-to/file/two.py': [{patch_set: 5, message: 'ipsum'}],
});
- sinon.stub(element, '_getCommentsForPath').returns({meta: {}});
element._changeNum = '42';
element._patchRange = {
basePatchNum: 3,
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index c15634b..617034b 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -210,10 +210,12 @@
created() {
super.created();
this._bindKeyboardShortcuts();
- this.addEventListener(EventType.PAGE_ERROR, e => this._handlePageError(e));
- this.addEventListener(EventType.TITLE_CHANGE, e =>
- this._handleTitleChange(e)
- );
+ this.addEventListener(EventType.PAGE_ERROR, e => {
+ this._handlePageError(e);
+ });
+ this.addEventListener(EventType.TITLE_CHANGE, e => {
+ this._handleTitleChange(e);
+ });
this.addEventListener('location-change', e =>
this._handleLocationChange(e)
);
@@ -490,6 +492,9 @@
registrationOverlay.refit();
});
}
+ // To fix bug announce read after each new view, we reset announce with
+ // empty space
+ this.fire('iron-announce', {text: ' '}, {bubbles: true});
}
_handleShortcutTriggered(event: ShortcutTriggeredEvent) {
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 da2881e..0b4e577 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
@@ -46,6 +46,7 @@
} from '../../../utils/attention-set-util';
import {ReviewerState} from '../../../constants/constants';
import {isRemovableReviewer} from '../../../utils/change-util';
+import {CURRENT} from '../../../utils/patch-set-util';
export interface GrHovercardAccount {
$: {
@@ -186,7 +187,7 @@
];
this.$.restAPI
- .saveChangeReview(this.change._number, 'current', reviewInput)
+ .saveChangeReview(this.change._number, CURRENT, reviewInput)
.then(response => {
if (!response || !response.ok) {
throw new Error(
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
index d75c186..b86fcff 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.js
@@ -22,6 +22,7 @@
import {ListChangesOption} from '../../../utils/change-util.js';
import {appContext} from '../../../services/app-context.js';
import {createChange} from '../../../test/test-data-generators.js';
+import {CURRENT} from '../../../utils/patch-set-util.js';
const basicFixture = fixtureFromElement('gr-rest-api-interface');
@@ -1350,7 +1351,7 @@
sinon.stub(element._restApiHelper, 'fetchJSON').returns(Promise.resolve({
ok: false}));
- element.getPortedComments(change._number, 'current');
+ element.getPortedComments(change._number, CURRENT);
assert.isFalse(dispatchStub.called);
});
@@ -1361,7 +1362,7 @@
const getChangeURLAndFetchStub = sinon.stub(element,
'_getChangeURLAndFetch');
- element.getPortedDrafts(change._number, 'current');
+ element.getPortedDrafts(change._number, CURRENT);
assert.isFalse(getChangeURLAndFetchStub.called);
});
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index e40412d..ba33954 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -27,4 +27,5 @@
PATCHSET_COMMENTS = 'UiFeature__patchset_comments',
NEW_CONTEXT_CONTROLS = 'UiFeature__new_context_controls',
CI_REBOOT_CHECKS = 'UiFeature__ci_reboot_checks',
+ NEW_CHANGE_SUMMARY_UI = 'UiFeature__new_change_summary_ui',
}
diff --git a/polygerrit-ui/app/utils/change-metadata-util.ts b/polygerrit-ui/app/utils/change-metadata-util.ts
new file mode 100644
index 0000000..6ce1483
--- /dev/null
+++ b/polygerrit-ui/app/utils/change-metadata-util.ts
@@ -0,0 +1,75 @@
+/**
+ * @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 {ParsedChangeInfo} from '../elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser';
+
+export enum Metadata {
+ OWNER = 'Owner',
+ REVIEWERS = 'Reviewers',
+ REPO_BRANCH = 'Repo | Branch',
+ SUBMITTED = 'Submitted',
+ PARENT = 'Parent',
+ STRATEGY = 'Strategy',
+ UPDATED = 'Updated',
+ CC = 'CC',
+ HASHTAGS = 'Hashtags',
+ TOPIC = 'Topic',
+ UPLOADER = 'Uploader',
+ AUTHOR = 'Author',
+ COMMITTER = 'Committer',
+ ASSIGNEE = 'Assignee',
+ CHERRY_PICK_OF = 'Cherry pick of',
+}
+
+export const DisplayRules = {
+ ALWAYS_SHOW: [
+ Metadata.OWNER,
+ Metadata.REVIEWERS,
+ Metadata.REPO_BRANCH,
+ Metadata.SUBMITTED,
+ ],
+ SHOW_IF_SET: [
+ Metadata.CC,
+ Metadata.HASHTAGS,
+ Metadata.TOPIC,
+ Metadata.UPLOADER,
+ Metadata.AUTHOR,
+ Metadata.COMMITTER,
+ Metadata.ASSIGNEE,
+ Metadata.CHERRY_PICK_OF,
+ ],
+ ALWAYS_HIDE: [Metadata.PARENT, Metadata.STRATEGY, Metadata.UPDATED],
+};
+
+export function isSectionSet(section: Metadata, change?: ParsedChangeInfo) {
+ switch (section) {
+ case Metadata.CC:
+ return !!change?.reviewers?.CC?.length;
+ case Metadata.HASHTAGS:
+ return !!change?.hashtags?.length;
+ case Metadata.TOPIC:
+ return !!change?.topic;
+ case Metadata.UPLOADER:
+ case Metadata.AUTHOR:
+ case Metadata.COMMITTER:
+ case Metadata.ASSIGNEE:
+ return false;
+ case Metadata.CHERRY_PICK_OF:
+ return !!change?.cherry_pick_of_change;
+ }
+ return true;
+}
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index 8974af8..d063168 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -45,6 +45,8 @@
PARENT: 'PARENT',
};
+export const CURRENT = 'current';
+
export interface PatchSet {
num: PatchSetNum;
desc: string | undefined;
