Merge "Remove the Deprecated ChangeAttributeFactory Interface"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index d897210..3f0c751 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -2052,7 +2052,7 @@
comments for each path are sorted by patch set number. Each comment has
the `patch_set` and `author` fields set.
-If the `enable_context` request parameter is set to true, the comment entries
+If the `enable-context` request parameter is set to true, the comment entries
will contain a list of link:#context-line[ContextLine] containing the lines of
the source file where the comment was written.
@@ -6699,7 +6699,7 @@
this comment applies.
|`context_lines` |optional|
A list of link:#context-line[ContextLine] containing the lines of the source
-file where the comment was written. Available only if the "enable_context"
+file where the comment was written. Available only if the "enable-context"
parameter (see link:#list-change-comments[List Change Comments]) is set.
|===========================
diff --git a/java/com/google/gerrit/acceptance/PushOneCommit.java b/java/com/google/gerrit/acceptance/PushOneCommit.java
index afd451a..4215255 100644
--- a/java/com/google/gerrit/acceptance/PushOneCommit.java
+++ b/java/com/google/gerrit/acceptance/PushOneCommit.java
@@ -43,8 +43,12 @@
import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import org.eclipse.jgit.api.TagCommand;
+import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
+import org.eclipse.jgit.dircache.DirCacheEntry;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevBlob;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate;
@@ -277,6 +281,19 @@
return this;
}
+ public PushOneCommit addSymlink(String path, String target) throws Exception {
+ RevBlob blobId = testRepo.blob(target);
+ commitBuilder.edit(
+ new PathEdit(path) {
+ @Override
+ public void apply(DirCacheEntry ent) {
+ ent.setFileMode(FileMode.SYMLINK);
+ ent.setObjectId(blobId);
+ }
+ });
+ return this;
+ }
+
public Result to(String ref) throws Exception {
for (Map.Entry<String, String> e : files.entrySet()) {
commitBuilder.add(e.getKey(), e.getValue());
diff --git a/java/com/google/gerrit/server/restapi/change/CommentJson.java b/java/com/google/gerrit/server/restapi/change/CommentJson.java
index cc8ad47..02f39ab 100644
--- a/java/com/google/gerrit/server/restapi/change/CommentJson.java
+++ b/java/com/google/gerrit/server/restapi/change/CommentJson.java
@@ -123,7 +123,6 @@
list = new ArrayList<>();
out.put(o.path, list);
}
- o.path = null;
list.add(o);
}
@@ -133,10 +132,11 @@
loader.fill();
}
+ List<T> allComments = out.values().stream().flatMap(Collection::stream).collect(toList());
if (fillCommentContext) {
- List<T> allComments = out.values().stream().flatMap(Collection::stream).collect(toList());
addCommentContext(allComments);
}
+ allComments.forEach(c -> c.path = null); // we don't need path since it exists in the map keys
return out;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
index 39366bd..ff785a2 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionDiffIT.java
@@ -2745,6 +2745,55 @@
}
@Test
+ public void symlinkConveredToRegularFileIsIdentifiedAsDeleted() throws Exception {
+ // TODO(ghareeb): See https://bugs.chromium.org/p/gerrit/issues/detail?id=13914.
+ // This test creates a corner scenario of replacing a symlink with a regular file
+ // of the same name. When both patchsets are diffed, the List Files endpoint identifies the
+ // file as a 'REWRITE', however the diff endpoint for the symlink file identifies the file as
+ // deleted. This case is a bit risky since it hides from the user the new content that was added
+ // in the new regular file. Ideally, the diff endpoint should show two entries for the deleted
+ // symlink and the added file, or only one entry "REWRITE" with the content that was added to
+ // the new file.
+ String target = "file.txt";
+ String symlink = "link.lnk";
+
+ // Create a change adding file "FileName" and a symlink "symLink" pointing to the file
+ PushOneCommit push =
+ pushFactory
+ .create(admin.newIdent(), testRepo, "Commit Subject", target, "content")
+ .addSymlink(symlink, target);
+
+ PushOneCommit.Result result = push.to("refs/for/master");
+ String initialRev = gApi.changes().id(result.getChangeId()).get().currentRevision;
+
+ // Delete the symlink with patchset 2
+ gApi.changes().id(result.getChangeId()).edit().deleteFile(symlink);
+ gApi.changes().id(result.getChangeId()).edit().publish();
+
+ // Re-add the symlink as a regular file with patchset 3
+ gApi.changes()
+ .id(result.getChangeId())
+ .edit()
+ .modifyFile(symlink, RawInputUtil.create("Content of the new file named 'symlink'"));
+ gApi.changes().id(result.getChangeId()).edit().publish();
+
+ Map<String, FileInfo> changedFiles =
+ gApi.changes().id(result.getChangeId()).current().files(initialRev);
+
+ assertThat(changedFiles.keySet()).containsExactly("/COMMIT_MSG", symlink);
+ assertThat(changedFiles.get(symlink).status).isEqualTo('W'); // Rewrite
+
+ DiffInfo diffInfo =
+ gApi.changes().id(result.getChangeId()).current().file(symlink).diff(initialRev);
+
+ // TODO(ghareeb): This is not a desired behaviour. The diff endpoint treats the file as
+ // 'DELETED', hence hiding any new content that was added to the new regular file. It is better
+ // to show the file as 'ADDED'. See https://bugs.chromium.org/p/gerrit/issues/detail?id=13914
+ // for more details.
+ assertThat(diffInfo.changeType).isEqualTo(ChangeType.DELETED);
+ }
+
+ @Test
public void diffOfNonExistentFileIsAnEmptyDiffResult() throws Exception {
addModifiedPatchSet(changeId, FILE_NAME, content -> content.replace("Line 2\n", "Line two\n"));
diff --git a/plugins/codemirror-editor b/plugins/codemirror-editor
index 7357ab4..740c35a 160000
--- a/plugins/codemirror-editor
+++ b/plugins/codemirror-editor
@@ -1 +1 @@
-Subproject commit 7357ab473599d16ae33cc982bbd65472f08c2dd6
+Subproject commit 740c35ae36f44748b3c91e60ee7dcb2fb6e99549
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 febf18a..fdf2811 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
@@ -38,11 +38,7 @@
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {appContext} from '../../../services/app-context';
-import {
- fetchChangeUpdates,
- patchNumEquals,
- CURRENT,
-} from '../../../utils/patch-set-util';
+import {fetchChangeUpdates, CURRENT} from '../../../utils/patch-set-util';
import {
changeIsOpen,
isOwner,
@@ -1153,7 +1149,7 @@
_getRevision(change: ChangeViewChangeInfo, patchNum?: PatchSetNum) {
for (const rev of Object.values(change.revisions)) {
- if (patchNumEquals(rev._number, patchNum)) {
+ if (rev._number === patchNum) {
return rev;
}
}
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 0279b7e..f7bca82 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
@@ -258,7 +258,7 @@
rev2: revObj,
},
};
- assert.deepEqual(element._getRevision(change, '2'), revObj);
+ assert.deepEqual(element._getRevision(change, 2), revObj);
});
test('_actionComparator sort order', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 684891f..126d017 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -69,7 +69,6 @@
fetchChangeUpdates,
hasEditBasedOnCurrentPatchSet,
hasEditPatchsetLoaded,
- patchNumEquals,
PatchSet,
} from '../../../utils/patch-set-util';
import {changeStatuses, changeStatusString} from '../../../utils/change-util';
@@ -1656,7 +1655,7 @@
if (!this._change) throw new Error('missing required change property');
if (!this._patchRange)
throw new Error('missing required _patchRange property');
- if (patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) {
+ if (this._patchRange.basePatchNum === ParentPatchSetNum) {
fireAlert(this, 'Base is already selected.');
return;
}
@@ -1670,7 +1669,7 @@
if (!this._change) throw new Error('missing required change property');
if (!this._patchRange)
throw new Error('missing required _patchRange property');
- if (patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) {
+ if (this._patchRange.basePatchNum === ParentPatchSetNum) {
fireAlert(this, 'Left is already base.');
return;
}
@@ -1685,7 +1684,7 @@
if (!this._patchRange)
throw new Error('missing required _patchRange property');
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (patchNumEquals(this._patchRange.patchNum, latestPatchNum)) {
+ if (this._patchRange.patchNum === latestPatchNum) {
fireAlert(this, 'Latest is already selected.');
return;
}
@@ -1704,7 +1703,7 @@
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
if (!this._patchRange)
throw new Error('missing required _patchRange property');
- if (patchNumEquals(this._patchRange.patchNum, latestPatchNum)) {
+ if (this._patchRange.patchNum === latestPatchNum) {
fireAlert(this, 'Right is already latest.');
return;
}
@@ -1724,8 +1723,8 @@
throw new Error('missing required _patchRange property');
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
if (
- patchNumEquals(this._patchRange.patchNum, latestPatchNum) &&
- patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)
+ this._patchRange.patchNum === latestPatchNum &&
+ this._patchRange.basePatchNum === ParentPatchSetNum
) {
fireAlert(this, 'Already diffing base against latest.');
return;
@@ -1967,7 +1966,7 @@
if (
!this._patchRange ||
!this._patchRange.patchNum ||
- patchNumEquals(this._patchRange.patchNum, currentRevision._number)
+ this._patchRange.patchNum === currentRevision._number
) {
// CommitInfo.commit is optional, and may need patching.
if (currentRevision.commit && !currentRevision.commit.commit) {
@@ -2567,7 +2566,7 @@
}
const patchRange = patchRangeRecord.base || {};
- return patchNumEquals(patchRange.patchNum, EditPatchSetNum);
+ return patchRange.patchNum === EditPatchSetNum;
}
_handleFileActionTap(e: CustomEvent<{path: string; action: string}>) {
@@ -2651,10 +2650,7 @@
throw new Error('missing required _patchRange property');
let patchNum;
if (
- !patchNumEquals(
- this._patchRange.patchNum,
- computeLatestPatchNum(this._allPatchSets)
- )
+ !(this._patchRange.patchNum === computeLatestPatchNum(this._allPatchSets))
) {
patchNum = this._patchRange.patchNum;
}
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
index 1195ea6..c4526bb 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog.ts
@@ -246,8 +246,10 @@
_computeDisableCherryPick(
cherryPickType: CherryPickType,
duplicateProjectChanges: boolean,
- statuses: Statuses
+ statuses: Statuses,
+ branch?: BranchName
) {
+ if (!branch) return true;
const duplicateProject =
cherryPickType === CherryPickType.TOPIC && duplicateProjectChanges;
if (duplicateProject) return true;
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_html.ts b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_html.ts
index 16262c6..4de395c 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_html.ts
@@ -85,7 +85,7 @@
<gr-dialog
confirm-label="Cherry Pick"
cancel-label="[[_computeCancelLabel(_statuses)]]"
- disabled$="[[_computeDisableCherryPick(_cherryPickType, _duplicateProjectChanges, _statuses)]]"
+ disabled$="[[_computeDisableCherryPick(_cherryPickType, _duplicateProjectChanges, _statuses, branch)]]"
on-confirm="_handleConfirmTap"
on-cancel="_handleCancelTap"
>
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
index ae34b70..5564fcf 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-confirm-cherrypick-dialog/gr-confirm-cherrypick-dialog_test.js
@@ -174,6 +174,9 @@
test('submit button is blocked while cherry picks is running', done => {
const confirmButton = element.shadowRoot.querySelector('gr-dialog').$
.confirm;
+ assert.isTrue(confirmButton.hasAttribute('disabled'));
+ element.branch = 'b';
+ flush();
assert.isFalse(confirmButton.hasAttribute('disabled'));
element.updateStatus(changes[0], {status: 'RUNNING'});
flush(() => {
diff --git a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
index 27d9756..5949513 100644
--- a/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-download-dialog/gr-download-dialog.ts
@@ -20,7 +20,6 @@
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-download-dialog_html';
-import {patchNumEquals} from '../../../utils/patch-set-util';
import {changeBaseURL} from '../../../utils/change-util';
import {customElement, property, computed, observe} from '@polymer/decorators';
import {ChangeInfo, ServerInfo, PatchSetNum} from '../../../types/common';
@@ -72,7 +71,7 @@
}
for (const rev of Object.values(this.change.revisions || {})) {
- if (patchNumEquals(rev._number, this.patchNum)) {
+ if (rev._number === this.patchNum) {
const fetch = rev.fetch;
if (fetch) {
return Object.keys(fetch).sort();
@@ -113,7 +112,7 @@
if (!change || !selectedScheme) return [];
for (const rev of Object.values(change.revisions || {})) {
if (
- patchNumEquals(rev._number, patchNum) &&
+ rev._number === patchNum &&
rev &&
rev.fetch &&
hasOwnProperty(rev.fetch, selectedScheme)
@@ -171,7 +170,7 @@
let shortRev = '';
for (const rev in change.revisions) {
- if (patchNumEquals(change.revisions[rev]._number, patchNum)) {
+ if (change.revisions[rev]._number === patchNum) {
shortRev = rev.substr(0, 7);
break;
}
@@ -185,7 +184,7 @@
return false;
}
for (const rev of Object.values(change.revisions || {})) {
- if (patchNumEquals(rev._number, patchNum)) {
+ if (rev._number === patchNum) {
const parentLength =
rev.commit && rev.commit.parents ? rev.commit.parents.length : 0;
return parentLength === 0 || parentLength > 1;
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index 3c99648..146b3e2 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -35,7 +35,6 @@
import {
computeLatestPatchNum,
getRevisionByPatchNum,
- patchNumEquals,
PatchSet,
} from '../../../utils/patch-set-util';
import {property, computed, observe, customElement} from '@polymer/decorators';
@@ -321,8 +320,7 @@
_handlePatchChange(e: CustomEvent) {
const {basePatchNum, patchNum} = e.detail;
if (
- (patchNumEquals(basePatchNum, this.basePatchNum) &&
- patchNumEquals(patchNum, this.patchNum)) ||
+ (basePatchNum === this.basePatchNum && patchNum === this.patchNum) ||
!this.change
) {
return;
@@ -354,7 +352,7 @@
_computePatchInfoClass(patchNum?: PatchSetNum, allPatchSets?: PatchSet[]) {
const latestNum = computeLatestPatchNum(allPatchSets);
- if (patchNumEquals(patchNum, latestNum)) {
+ if (patchNum === latestNum) {
return '';
}
return 'patchInfoOldPatchSet';
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
index cee0262..b8670c0 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header_test.js
@@ -249,11 +249,11 @@
test('class is applied to file list on old patch set', () => {
const allPatchSets = [{num: 4}, {num: 2}, {num: 1}];
- assert.equal(element._computePatchInfoClass('1', allPatchSets),
+ assert.equal(element._computePatchInfoClass(1, allPatchSets),
'patchInfoOldPatchSet');
- assert.equal(element._computePatchInfoClass('2', allPatchSets),
+ assert.equal(element._computePatchInfoClass(2, allPatchSets),
'patchInfoOldPatchSet');
- assert.equal(element._computePatchInfoClass('4', allPatchSets), '');
+ assert.equal(element._computePatchInfoClass(4, allPatchSets), '');
});
suite('editMode behavior', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
index fb20f0c..bc61dad 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -46,6 +46,11 @@
import {appContext} from '../../../services/app-context';
import {pluralize} from '../../../utils/string-util';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
+import {
+ computeAllPatchSets,
+ computeLatestPatchNum,
+ computePredecessor,
+} from '../../../utils/patch-set-util';
const PATCH_SET_PREFIX_PATTERN = /^(?:Uploaded\s*)?(?:P|p)atch (?:S|s)et \d+:\s*(.*)/;
const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?[.]?$/;
@@ -270,13 +275,20 @@
_handleViewPatchsetDiff(e: Event) {
if (!this.message || !this.change) return;
const match = this.message.message.match(/Uploaded patch set (\d+)./);
- if (!match || match.length < 1) return;
- const patchNum = Number(match[1]);
- if (isNaN(patchNum)) throw new Error('invalid patchnum in message');
+ let patchNum: PatchSetNum;
+ // Message is of the form "Commit Message was updated" or "Patchset X
+ // was rebased"
+ if (!match || match.length < 1) {
+ patchNum = computeLatestPatchNum(computeAllPatchSets(this.change))!;
+ } else {
+ if (isNaN(Number(match[1])))
+ throw new Error('invalid patchnum in message');
+ patchNum = Number(match[1]) as PatchSetNum;
+ }
GerritNav.navigateToChange(
this.change,
- patchNum as PatchSetNum,
- (patchNum === 1 ? 'PARENT' : patchNum - 1) as PatchSetNum
+ patchNum,
+ computePredecessor(patchNum)
);
// stop propagation to stop message expansion
e.stopPropagation();
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js
index 94507e6..facde01 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message_test.js
@@ -18,6 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-message.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
+import {createChange, createRevisions} from '../../../test/test-data-generators.js';
const basicFixture = fixtureFromElement('gr-message');
@@ -232,7 +233,7 @@
suite('uploaded patchset X message navigates to X - 1 vs X', () => {
let navStub;
setup(() => {
- element.change = {changeNum: 12345};
+ element.change = {...createChange(), revisions: createRevisions(4)};
navStub = sinon.stub(GerritNav, 'navigateToChange');
});
@@ -241,7 +242,7 @@
message: 'Uploaded patch set 1.',
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(navStub.calledWithExactly({changeNum: 12345}, 1,
+ assert.isTrue(navStub.calledWithExactly(element.change, 1,
'PARENT'));
});
@@ -250,21 +251,21 @@
message: 'Uploaded patch set 2.',
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(navStub.calledWithExactly({changeNum: 12345}, 2, 1));
+ assert.isTrue(navStub.calledWithExactly(element.change, 2, 1));
element.message = {
message: 'Uploaded patch set 200.',
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isTrue(navStub.calledWithExactly({changeNum: 12345}, 200, 199));
+ assert.isTrue(navStub.calledWithExactly(element.change, 200, 199));
});
- test('invalid patchset does not cause navigation', () => {
+ test('Commit message updated', () => {
element.message = {
- message: 'Uploaded patch set XYZ.',
+ message: 'Commit message updated.',
};
element._handleViewPatchsetDiff(new MouseEvent('click'));
- assert.isFalse(navStub.called);
+ assert.isTrue(navStub.calledWithExactly(element.change, 4, 3));
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index fc6b5ba..e6c9f42 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -25,7 +25,7 @@
import {htmlTemplate} from './gr-related-changes-list_html';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {ChangeStatus} from '../../../constants/constants';
-import {patchNumEquals} from '../../../utils/patch-set-util';
+
import {changeIsOpen} from '../../../utils/change-util';
import {getPluginEndpoints} from '../../shared/gr-js-api-interface/gr-plugin-endpoints';
import {customElement, observe, property} from '@polymer/decorators';
@@ -410,7 +410,7 @@
return [];
}
for (const rev in change.revisions) {
- if (patchNumEquals(change.revisions[rev]._number, patchNum)) {
+ if (change.revisions[rev]._number === patchNum) {
changeRevision = rev;
}
}
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 07045cd..2f3d03c 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -46,10 +46,7 @@
WeblinkType,
} from '../gr-navigation/gr-navigation';
import {appContext} from '../../../services/app-context';
-import {
- convertToPatchSetNum,
- patchNumEquals,
-} from '../../../utils/patch-set-util';
+import {convertToPatchSetNum} from '../../../utils/patch-set-util';
import {customElement, property} from '@polymer/decorators';
import {assertNever} from '../../../utils/common-util';
import {
@@ -693,10 +690,7 @@
// Diffing a patch against itself is invalid, so if the base and revision
// patches are equal clear the base.
- if (
- params.patchNum &&
- patchNumEquals(params.basePatchNum, params.patchNum)
- ) {
+ if (params.patchNum && params.basePatchNum === params.patchNum) {
needsRedirect = true;
params.basePatchNum = null;
} else if (!hasPatchNum) {
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 9988116..5210616 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
@@ -18,7 +18,7 @@
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-comment-api_html';
-import {patchNumEquals, CURRENT} from '../../../utils/patch-set-util';
+import {CURRENT} from '../../../utils/patch-set-util';
import {customElement, property} from '@polymer/decorators';
import {
CommentBasics,
@@ -30,6 +30,7 @@
NumericChangeId,
PathToCommentsInfoMap,
FileInfo,
+ ParentPatchSetNum,
} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {
@@ -235,9 +236,7 @@
allComments = allComments.concat(drafts);
}
if (patchNum) {
- allComments = allComments.filter(c =>
- patchNumEquals(c.patch_set, patchNum)
- );
+ allComments = allComments.filter(c => c.patch_set === patchNum);
}
return allComments.map(c => {
return {...c};
@@ -284,7 +283,7 @@
getAllDraftsForPath(path: string, patchNum?: PatchSetNum): Comment[] {
let comments = this._drafts[path] || [];
if (patchNum) {
- comments = comments.filter(c => patchNumEquals(c.patch_set, patchNum));
+ comments = comments.filter(c => c.patch_set === patchNum);
}
return comments.map(c => {
return {...c, __draft: true};
@@ -369,6 +368,10 @@
* does not return the range of the ported thread and it becomes a file level
* thread.
*
+ * If a comment was created with Side=PARENT, then we only show this ported
+ * comment if Base is part of the patch range, always on the left side of
+ * the diff.
+ *
* @return only the ported threads for the specified file and patch range
*/
_getPortedCommentThreads(
@@ -407,15 +410,22 @@
return false;
}
- // TODO(dhruvsri): Add handling for thread.commentSide = PARENT
- if (thread.commentSide === CommentSide.PARENT) return false;
+ thread.diffSide = Side.RIGHT;
+ if (thread.commentSide === CommentSide.PARENT) {
+ // TODO(dhruvsri): Add handling for merge parents
+ if (
+ patchRange.basePatchNum !== ParentPatchSetNum ||
+ !!thread.mergeParentNum
+ )
+ return false;
+ thread.diffSide = Side.LEFT;
+ }
if (!isUnresolved(thread) && !isDraftThread(thread)) return false;
thread.range = portedComment.range;
thread.line = portedComment.line;
thread.ported = true;
- thread.diffSide = Side.RIGHT;
return true;
});
}
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
index dd36613..e107c16 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api_test.js
@@ -20,6 +20,7 @@
import {ChangeComments} from './gr-comment-api.js';
import {isInRevisionOfPatchRange, isInBaseOfPatchRange, isDraftThread, isUnresolved, createCommentThreads} from '../../../utils/comment-util.js';
import {createDraft, createComment, createChangeComments, createCommentThread} from '../../../test/test-data-generators.js';
+import {CommentSide, Side} from '../../../constants/constants.js';
const basicFixture = fixtureFromElement('gr-comment-api');
@@ -153,7 +154,7 @@
const comment1 = {
...createComment(),
unresolved: true,
- id: 'db977012_e1f13818',
+ id: '1',
line: 136,
patch_set: 2,
range: {
@@ -167,10 +168,22 @@
const comment2 = {
...createComment(),
patch_set: 2,
- id: 'ecf0b9fa_fe1a5f62',
+ id: '2',
line: 5,
};
+ const comment3 = {
+ ...createComment(),
+ side: CommentSide.PARENT,
+ line: 10,
+ unresolved: true,
+ };
+
+ const comment4 = {
+ ...comment3,
+ parent: -2,
+ };
+
const draft1 = {
...createDraft(),
id: 'db977012_e1f13828',
@@ -277,6 +290,74 @@
).length, 0);
});
+ test('comments with side=PARENT are ported over', () => {
+ changeComments = new ChangeComments(
+ {/* comments */
+ // comment left on Base
+ 'karma.conf.js': [comment3],
+ },
+ {}/* robot comments */,
+ {/* drafts */
+ 'karma.conf.js': [draft2],
+ },
+ {/* ported comments */
+ 'karma.conf.js': [{
+ ...comment3,
+ line: 31,
+ patch_set: 4,
+ }],
+ },
+ {}/* ported drafts */
+ );
+
+ const portedThreads = changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'});
+ assert.equal(portedThreads.length, 1);
+ assert.equal(portedThreads[0].line, 31);
+ assert.equal(portedThreads[0].diffSide, Side.LEFT);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: -2}
+ ).length, 0);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 2}
+ ).length, 0);
+ });
+
+ test('comments left on merge parent is not ported over', () => {
+ changeComments = new ChangeComments(
+ {/* comments */
+ // comment left on Base
+ 'karma.conf.js': [comment4],
+ },
+ {}/* robot comments */,
+ {/* drafts */
+ 'karma.conf.js': [draft2],
+ },
+ {/* ported comments */
+ 'karma.conf.js': [{
+ ...comment4,
+ line: 31,
+ patch_set: 4,
+ }],
+ },
+ {}/* ported drafts */
+ );
+
+ const portedThreads = changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 'PARENT'});
+ assert.equal(portedThreads.length, 0);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: -2}
+ ).length, 0);
+
+ assert.equal(changeComments._getPortedCommentThreads(
+ {path: 'karma.conf.js'}, {patchNum: 4, basePatchNum: 2}
+ ).length, 0);
+ });
+
test('ported comments contribute to comment count', () => {
assert.equal(changeComments.computeCommentsString(
{basePatchNum: 'PARENT', patchNum: 2}, 'karma.conf.js',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index 601ea80..9574c1d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -716,10 +716,19 @@
}
if (lineNumberEl) {
- for (const layer of this.layers) {
- if (typeof layer.annotate === 'function') {
- layer.annotate(contentText, lineNumberEl, line);
+ const ANNOTATE_MAX_LINE_LENGTH = 1000;
+ // For performance reason, we skip annotating long lines.
+ if (line.text.length < ANNOTATE_MAX_LINE_LENGTH) {
+ for (const layer of this.layers) {
+ if (typeof layer.annotate === 'function') {
+ layer.annotate(contentText, lineNumberEl, line);
+ }
}
+ } else {
+ const msg =
+ `A line is longer than ${ANNOTATE_MAX_LINE_LENGTH}.` +
+ ' Line annotation was skipped.';
+ console.warn(msg);
}
} else {
console.error('The lineNumberEl is null, skipping layer annotations.');
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 2c050f9..d09c811 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
@@ -1040,6 +1040,7 @@
commentSide: CommentSide.REVISION,
path: '/p',
rootId: 'betsys_confession',
+ mergeParentNum: undefined,
comments: [{
id: 'betsys_confession',
path: '/p',
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 828c53d..84b227b 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
@@ -44,7 +44,6 @@
import {
computeAllPatchSets,
computeLatestPatchNum,
- patchNumEquals,
PatchSet,
} from '../../../utils/patch-set-util';
import {
@@ -880,12 +879,10 @@
_displayDiffAgainstLatestToast(latestPatchNum?: PatchSetNum) {
if (!this._patchRange) return;
- const leftPatchset = patchNumEquals(
- this._patchRange.basePatchNum,
- ParentPatchSetNum
- )
- ? 'Base'
- : `Patchset ${this._patchRange.basePatchNum}`;
+ const leftPatchset =
+ this._patchRange.basePatchNum === ParentPatchSetNum
+ ? 'Base'
+ : `Patchset ${this._patchRange.basePatchNum}`;
fireAlert(
this,
`${leftPatchset} vs
@@ -896,12 +893,12 @@
_displayToasts() {
if (!this._patchRange) return;
- if (!patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) {
+ if (this._patchRange.basePatchNum !== ParentPatchSetNum) {
this._displayDiffBaseAgainstLeftToast();
return;
}
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (!patchNumEquals(this._patchRange.patchNum, latestPatchNum)) {
+ if (this._patchRange.patchNum !== latestPatchNum) {
this._displayDiffAgainstLatestToast(latestPatchNum);
return;
}
@@ -916,17 +913,17 @@
if (!hasOwnProperty(this._change.revisions, commitSha)) continue;
const revision = this._change.revisions[commitSha];
const patchNum = revision._number;
- if (patchNumEquals(patchNum, this._patchRange.patchNum)) {
+ if (patchNum === this._patchRange.patchNum) {
commit = commitSha as CommitId;
const commitObj = revision.commit;
const parents = commitObj?.parents || [];
if (
- patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum) &&
+ this._patchRange.basePatchNum === ParentPatchSetNum &&
parents.length
) {
baseCommit = parents[parents.length - 1].commit;
}
- } else if (patchNumEquals(patchNum, this._patchRange.basePatchNum)) {
+ } else if (patchNum === this._patchRange.basePatchNum) {
baseCommit = commitSha as CommitId;
}
}
@@ -1250,7 +1247,7 @@
if (!patchRange) return {patchNum, basePatchNum};
if (
patchRange.basePatchNum !== ParentPatchSetNum ||
- !patchNumEquals(patchRange.patchNum, latestPatchNum as PatchSetNum)
+ patchRange.patchNum !== latestPatchNum
) {
patchNum = patchRange.patchNum;
basePatchNum = patchRange.basePatchNum;
@@ -1352,8 +1349,8 @@
const {basePatchNum, patchNum} = e.detail;
if (
- patchNumEquals(basePatchNum, this._patchRange.basePatchNum) &&
- patchNumEquals(patchNum, this._patchRange.patchNum)
+ basePatchNum === this._patchRange.basePatchNum &&
+ patchNum === this._patchRange.patchNum
) {
return;
}
@@ -1582,7 +1579,7 @@
patchRangeRecord: PolymerDeepPropertyChange<PatchRange, PatchRange>
) {
const patchRange = patchRangeRecord.base || {};
- return patchNumEquals(patchRange.patchNum, EditPatchSetNum);
+ return patchRange.patchNum === EditPatchSetNum;
}
_computeBlameToggleLabel(loaded?: boolean, loading?: boolean) {
@@ -1641,7 +1638,7 @@
if (!this._path) return;
if (!this._patchRange) return;
- if (patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) {
+ if (this._patchRange.basePatchNum === ParentPatchSetNum) {
fireAlert(this, 'Base is already selected.');
return;
}
@@ -1658,7 +1655,7 @@
if (!this._path) return;
if (!this._patchRange) return;
- if (patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)) {
+ if (this._patchRange.basePatchNum === ParentPatchSetNum) {
fireAlert(this, 'Left is already base.');
return;
}
@@ -1680,7 +1677,7 @@
if (!this._patchRange) return;
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (patchNumEquals(this._patchRange.patchNum, latestPatchNum)) {
+ if (this._patchRange.patchNum === latestPatchNum) {
fireAlert(this, 'Latest is already selected.');
return;
}
@@ -1700,7 +1697,7 @@
if (!this._patchRange) return;
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
- if (patchNumEquals(this._patchRange.patchNum, latestPatchNum)) {
+ if (this._patchRange.patchNum === latestPatchNum) {
fireAlert(this, 'Right is already latest.');
return;
}
@@ -1720,8 +1717,8 @@
const latestPatchNum = computeLatestPatchNum(this._allPatchSets);
if (
- patchNumEquals(this._patchRange.patchNum, latestPatchNum) &&
- patchNumEquals(this._patchRange.basePatchNum, ParentPatchSetNum)
+ this._patchRange.patchNum === latestPatchNum &&
+ this._patchRange.basePatchNum === ParentPatchSetNum
) {
fireAlert(this, 'Already diffing base against latest.');
return;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 04714bb..39d645e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -38,7 +38,7 @@
rangesEqual,
} from './gr-diff-utils';
import {getHiddenScroll} from '../../../scripts/hiddenscroll';
-import {isMergeParent, patchNumEquals} from '../../../utils/patch-set-util';
+import {isMergeParent} from '../../../utils/patch-set-util';
import {customElement, observe, property} from '@polymer/decorators';
import {
BlameInfo,
@@ -605,10 +605,10 @@
? this.patchRange.basePatchNum
: this.patchRange.patchNum;
- const isEdit = patchNumEquals(patchNum, EditPatchSetNum);
+ const isEdit = patchNum === EditPatchSetNum;
const isEditBase =
- patchNumEquals(patchNum, ParentPatchSetNum) &&
- patchNumEquals(this.patchRange.patchNum, EditPatchSetNum);
+ patchNum === ParentPatchSetNum &&
+ this.patchRange.patchNum === EditPatchSetNum;
if (isEdit) {
fireAlert(this, 'You cannot comment on an edit.');
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index ec39da2..671bd41 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -30,7 +30,6 @@
getParentIndex,
getRevisionByPatchNum,
isMergeParent,
- patchNumEquals,
sortRevisions,
PatchSet,
} from '../../../utils/patch-set-util';
@@ -346,7 +345,7 @@
patchNum: PatchSetNum,
sortedRevisions: RevisionInfo[]
): boolean {
- if (patchNumEquals(basePatchNum, ParentPatchSetNum)) {
+ if (basePatchNum === ParentPatchSetNum) {
return false;
}
@@ -441,7 +440,7 @@
});
detail.patchNum = patchSetValue;
} else {
- if (patchNumEquals(detail.basePatchNum, patchSetValue)) return;
+ if (detail.basePatchNum === patchSetValue) return;
this.reporting.reportInteraction('left-patchset-changed', {
previous: detail.basePatchNum,
current: e.detail.value,
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
index d9a3187..fbf7f47 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_html.ts
@@ -264,10 +264,16 @@
</gr-account-label>
</template>
<template is="dom-if" if="[[showPortedComment]]">
- <a href="[[_getUrlForComment(comment)]]">
- <span class="portedMessage">
- Ported from patchset [[comment.patch_set]]
- </span>
+ <a href="[[_getUrlForComment(comment)]]"
+ ><span class="portedMessage"
+ >Ported from patchset [[comment.patch_set]]</span
+ ></a
+ >
+ <a
+ href="https://bugs.chromium.org/p/gerrit/issues/entry?template=Porting+Comments"
+ target="_blank"
+ >
+ <iron-icon icon="gr-icons:bug" title="report a problem"></iron-icon>
</a>
</template>
<gr-tooltip-content
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts
index f313174..c163924 100644
--- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list_html.ts
@@ -145,7 +145,6 @@
slot="dropdown-content"
attr-for-selected="data-value"
selected="{{value}}"
- on-tap="_handleDropdownTap"
>
<template
is="dom-repeat"
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
index ae6e155..00f9a40 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface-element.ts
@@ -18,7 +18,7 @@
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {getPluginLoader} from './gr-plugin-loader';
-import {patchNumEquals} from '../../../utils/patch-set-util';
+
import {customElement} from '@polymer/decorators';
import {
ChangeInfo,
@@ -154,7 +154,7 @@
let revision;
for (const rev of Object.values(change.revisions || {})) {
- if (patchNumEquals(rev._number, patchNum)) {
+ if (rev._number === patchNum) {
revision = rev;
break;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
index 078e07f..9e77784 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.ts
@@ -36,11 +36,7 @@
import {parseDate} from '../../../utils/date-util';
import {getBaseUrl} from '../../../utils/url-util';
import {appContext} from '../../../services/app-context';
-import {
- getParentIndex,
- isMergeParent,
- patchNumEquals,
-} from '../../../utils/patch-set-util';
+import {getParentIndex, isMergeParent} from '../../../utils/patch-set-util';
import {
ListChangesOption,
listChangesOptionsToHex,
@@ -1354,7 +1350,7 @@
let params = undefined;
if (isMergeParent(patchRange.basePatchNum)) {
params = {parent: getParentIndex(patchRange.basePatchNum)};
- } else if (!patchNumEquals(patchRange.basePatchNum, ParentPatchSetNum)) {
+ } else if (patchRange.basePatchNum !== ParentPatchSetNum) {
params = {base: patchRange.basePatchNum};
}
return this._getChangeURLAndFetch({
@@ -1401,7 +1397,7 @@
changeNum: NumericChangeId,
patchRange: PatchRange
): Promise<FileNameToFileInfoMap | undefined> {
- if (patchNumEquals(patchRange.patchNum, EditPatchSetNum)) {
+ if (patchRange.patchNum === EditPatchSetNum) {
return this.getChangeEditFiles(changeNum, patchRange).then(
res => res && res.files
);
@@ -1983,9 +1979,10 @@
}
return res;
};
- const promise = patchNumEquals(patchNum, EditPatchSetNum)
- ? this._getFileInChangeEdit(changeNum, path)
- : this._getFileInRevision(changeNum, path, patchNum, suppress404s);
+ const promise =
+ patchNum === EditPatchSetNum
+ ? this._getFileInChangeEdit(changeNum, path)
+ : this._getFileInRevision(changeNum, path, patchNum, suppress404s);
return promise.then(res => {
if (!res || !res.ok) {
@@ -2277,8 +2274,7 @@
};
if (isMergeParent(basePatchNum)) {
params.parent = getParentIndex(basePatchNum);
- } else if (!patchNumEquals(basePatchNum, ParentPatchSetNum)) {
- // TODO (TS): fix as PatchSetNum in the condition above
+ } else if (basePatchNum !== ParentPatchSetNum) {
params.base = basePatchNum;
}
const endpoint = `/files/${encodeURIComponent(path)}/diff`;
diff --git a/polygerrit-ui/app/elements/shared/revision-info/revision-info.ts b/polygerrit-ui/app/elements/shared/revision-info/revision-info.ts
index fadbfa7..d4bbe16 100644
--- a/polygerrit-ui/app/elements/shared/revision-info/revision-info.ts
+++ b/polygerrit-ui/app/elements/shared/revision-info/revision-info.ts
@@ -15,7 +15,6 @@
* limitations under the License.
*/
-import {patchNumEquals} from '../../../utils/patch-set-util';
import {ChangeInfo, PatchSetNum} from '../../../types/common';
import {ParsedChangeInfo} from '../gr-rest-api-interface/gr-reviewer-updates-parser';
@@ -71,8 +70,8 @@
getParentId(patchNum: PatchSetNum, parentIndex: number) {
if (!this.change.revisions) return;
- const rev = Object.values(this.change.revisions).find(rev =>
- patchNumEquals(rev._number, patchNum)
+ const rev = Object.values(this.change.revisions).find(
+ rev => rev._number === patchNum
);
if (!rev || !rev.commit) return;
return rev.commit.parents[parentIndex].commit;
diff --git a/polygerrit-ui/app/utils/comment-util.ts b/polygerrit-ui/app/utils/comment-util.ts
index 5d8ea37..f4c0692 100644
--- a/polygerrit-ui/app/utils/comment-util.ts
+++ b/polygerrit-ui/app/utils/comment-util.ts
@@ -29,7 +29,7 @@
import {parseDate} from './date-util';
import {LineNumber} from '../elements/diff/gr-diff/gr-diff-line';
import {CommentIdToCommentThreadMap} from '../elements/diff/gr-comment-api/gr-comment-api';
-import {isMergeParent, getParentIndex, patchNumEquals} from './patch-set-util';
+import {isMergeParent, getParentIndex} from './patch-set-util';
export interface DraftCommentProps {
__draft?: boolean;
@@ -126,6 +126,7 @@
comments: [comment],
patchNum: comment.patch_set,
commentSide: comment.side ?? CommentSide.REVISION,
+ mergeParentNum: comment.parent,
path: comment.path,
line: comment.line,
range: comment.range,
@@ -151,6 +152,11 @@
comments: UIComment[];
path: string;
commentSide: CommentSide;
+ /* mergeParentNum is the merge parent number only valid for merge commits
+ when commentSide is PARENT.
+ mergeParentNum is undefined for auto merge commits
+ */
+ mergeParentNum?: number;
patchNum?: PatchSetNum;
line?: LineNumber;
/* rootId is optional since we create a empty comment thread element for
@@ -196,7 +202,7 @@
if (
range.basePatchNum === ParentPatchSetNum &&
comment.side === CommentSide.PARENT &&
- patchNumEquals(comment.patch_set, range.patchNum)
+ comment.patch_set === range.patchNum
) {
return true;
}
@@ -204,7 +210,7 @@
return (
range.basePatchNum !== ParentPatchSetNum &&
comment.side !== CommentSide.PARENT &&
- patchNumEquals(comment.patch_set, range.basePatchNum)
+ comment.patch_set === range.basePatchNum
);
}
@@ -217,8 +223,7 @@
range: PatchRange
) {
return (
- comment.side !== CommentSide.PARENT &&
- patchNumEquals(comment.patch_set, range.patchNum)
+ comment.side !== CommentSide.PARENT && comment.patch_set === range.patchNum
);
}
@@ -247,7 +252,7 @@
patchNum: comment.patch_set,
basePatchNum: ParentPatchSetNum,
};
- } else if (patchNumEquals(latestPatchNum, comment.patch_set)) {
+ } else if (latestPatchNum === comment.patch_set) {
return {
patchNum: latestPatchNum,
basePatchNum: ParentPatchSetNum,
diff --git a/polygerrit-ui/app/utils/patch-set-util.ts b/polygerrit-ui/app/utils/patch-set-util.ts
index fda302b..cb0bdec 100644
--- a/polygerrit-ui/app/utils/patch-set-util.ts
+++ b/polygerrit-ui/app/utils/patch-set-util.ts
@@ -60,19 +60,6 @@
}
/**
- * As patchNum can be either a string (e.g. 'edit', 'PARENT') OR a number,
- * this function checks for patchNum equality.
- *
- */
-export function patchNumEquals(a?: PatchSetNum, b?: PatchSetNum) {
- if (a === undefined) {
- return a === b;
- }
- // TODO(TS): replace with a===b when the whole code is converted to ts
- return `${a}` === `${b}`;
-}
-
-/**
* Whether the given patch is a numbered parent of a merge (i.e. a negative
* number).
*/
@@ -114,7 +101,7 @@
patchNum: PatchSetNum
) {
for (const rev of revisions) {
- if (patchNumEquals(rev._number, patchNum)) {
+ if (rev._number === patchNum) {
return rev;
}
}
@@ -275,6 +262,20 @@
return allPatchSets[0].num;
}
+export function computePredecessor(
+ patchset?: PatchSetNum
+): PatchSetNum | undefined {
+ if (
+ !patchset ||
+ patchset === ParentPatchSetNum ||
+ patchset === EditPatchSetNum
+ ) {
+ return undefined;
+ }
+ if (patchset === 1) return ParentPatchSetNum;
+ return (Number(patchset) - 1) as PatchSetNum;
+}
+
export function hasEditBasedOnCurrentPatchSet(allPatchSets: PatchSet[]) {
if (!allPatchSets || allPatchSets.length < 2) {
return false;
diff --git a/polygerrit-ui/app/utils/patch-set-util_test.js b/polygerrit-ui/app/utils/patch-set-util_test.js
index 29cc370..12f5ed4 100644
--- a/polygerrit-ui/app/utils/patch-set-util_test.js
+++ b/polygerrit-ui/app/utils/patch-set-util_test.js
@@ -21,7 +21,7 @@
fetchChangeUpdates, findEditParentPatchNum, findEditParentRevision,
getParentIndex, getRevisionByPatchNum,
isMergeParent,
- patchNumEquals, sortRevisions,
+ sortRevisions,
} from './patch-set-util.js';
suite('gr-patch-set-util tests', () => {
@@ -31,9 +31,9 @@
{_number: 1},
{_number: 2},
];
- assert.deepEqual(getRevisionByPatchNum(revisions, '1'), revisions[1]);
+ assert.deepEqual(getRevisionByPatchNum(revisions, 1), revisions[1]);
assert.deepEqual(getRevisionByPatchNum(revisions, 2), revisions[2]);
- assert.equal(getRevisionByPatchNum(revisions, '3'), undefined);
+ assert.equal(getRevisionByPatchNum(revisions, 3), undefined);
});
test('fetchChangeUpdates on latest', done => {
@@ -231,17 +231,6 @@
.assertWip(6, true); // PS6 was uploaded with WIP option
});
- test('patchNumEquals', () => {
- assert.isFalse(patchNumEquals('edit', 'PARENT'));
- assert.isFalse(patchNumEquals('edit', NaN));
- assert.isFalse(patchNumEquals(1, '2'));
-
- assert.isTrue(patchNumEquals(1, '1'));
- assert.isTrue(patchNumEquals(1, 1));
- assert.isTrue(patchNumEquals('edit', 'edit'));
- assert.isTrue(patchNumEquals('PARENT', 'PARENT'));
- });
-
test('isMergeParent', () => {
assert.isFalse(isMergeParent(1));
assert.isFalse(isMergeParent(4321));
