Move Merged as info from change header to change summary
Merged As info on change header points to the same change viewed
via a weblink(such as Gitiles).
The button has low usage(https://imgur.com/a/bHUg8ZU) and is taking
up valuable real estate, hence move to Change summary hidden by
default, similar to the parent metadata field.
Screenshot: https://imgur.com/a/MgZNe7G
Change-Id: Ie2a5715dc65a20447f296f5246962b6445836490
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 c19519d..d5a917f 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
@@ -567,6 +567,22 @@
return 'hideDisplay';
}
+ _computeMergedCommitInfo(
+ current_revision: CommitId,
+ revisions: {[revisionId: string]: RevisionInfo}
+ ) {
+ const rev = revisions[current_revision];
+ if (!rev || !rev.commit) {
+ return {};
+ }
+ // CommitInfo.commit is optional. Set commit in all cases to avoid error
+ // in <gr-commit-info>. @see Issue 5337
+ if (!rev.commit.commit) {
+ rev.commit.commit = current_revision;
+ }
+ return rev.commit;
+ }
+
_computeShowAllLabelText(showAllSections: boolean) {
if (showAllSections) {
return 'Show less';
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 4f5e6a6..2864578 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
@@ -340,6 +340,18 @@
</span>
</section>
<section
+ class$="[[_computeDisplayState(_showAllSections, change, _SECTION.MERGED_AS)]]"
+ >
+ <span class="title">Merged As</span>
+ <span class="value">
+ <gr-commit-info
+ change="[[change]]"
+ commit-info="[[_computeMergedCommitInfo(change.current_revision, change.revisions)]]"
+ server-config="[[serverConfig]]"
+ ></gr-commit-info>
+ </span>
+ </section>
+ <section
class$="topic [[_computeDisplayState(_showAllSections, change, _SECTION.TOPIC)]]"
>
<span class="title">Topic</span>
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
index 6bfa240..ab8e404 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
@@ -87,6 +87,26 @@
element = basicFixture.instantiate();
});
+ test('_computeMergedCommitInfo', () => {
+ const dummyRevs: {[revisionId: string]: RevisionInfo} = {
+ 1: createRevision(1),
+ 2: createRevision(2),
+ };
+ assert.deepEqual(
+ element._computeMergedCommitInfo('0' as CommitId, dummyRevs),
+ {}
+ );
+ assert.deepEqual(
+ element._computeMergedCommitInfo('1' as CommitId, dummyRevs),
+ dummyRevs[1].commit
+ );
+
+ // Regression test for issue 5337.
+ const commit = element._computeMergedCommitInfo('2' as CommitId, dummyRevs);
+ assert.notDeepEqual(commit, dummyRevs[2]);
+ assert.deepEqual(commit, dummyRevs[2].commit);
+ });
+
test('computed fields', () => {
assert.isFalse(
element._computeHideStrategy({
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 7a5bc6f..5936634 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
@@ -1573,22 +1573,6 @@
);
}
- _computeMergedCommitInfo(
- current_revision: CommitId,
- revisions: {[revisionId: string]: RevisionInfo}
- ) {
- const rev = revisions[current_revision];
- if (!rev || !rev.commit) {
- return {};
- }
- // CommitInfo.commit is optional. Set commit in all cases to avoid error
- // in <gr-commit-info>. @see Issue 5337
- if (!rev.commit.commit) {
- rev.commit.commit = current_revision;
- }
- return rev.commit;
- }
-
_computeChangeIdClass(displayChangeId: string) {
return displayChangeId === CHANGE_ID_ERROR.MISMATCH ? 'warning' : '';
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index 993970a..d7dcbf4 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -373,19 +373,6 @@
></gr-change-status>
</template>
</div>
- <div class="statusText">
- <template
- is="dom-if"
- if="[[_computeShowCommitInfo(_changeStatuses, _change.current_revision)]]"
- >
- <span class="text"> as </span>
- <gr-commit-info
- change="[[_change]]"
- commit-info="[[_computeMergedCommitInfo(_change.current_revision, _change.revisions)]]"
- server-config="[[_serverConfig]]"
- ></gr-commit-info>
- </template>
- </div>
<gr-change-star
id="changeStar"
change="{{_change}}"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 927d391..95aa20e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -60,6 +60,7 @@
getCurrentRevision,
createEditRevision,
createAccountWithIdNameAndEmail,
+ createChangeViewChange,
} from '../../../test/test-data-generators';
import {ChangeViewPatchRange, GrChangeView} from './gr-change-view';
import {
@@ -410,7 +411,7 @@
basePatchNum: ParentPatchSetNum,
patchNum: 1 as RevisionPatchSetNum,
};
- element._change = createChange();
+ element._change = createChangeViewChange();
const getUrlStub = sinon.stub(GerritNav, 'getUrlForChange');
const replaceStateStub = sinon.stub(history, 'replaceState');
element._handleMessageAnchorTap(
@@ -423,7 +424,7 @@
test('_handleDiffAgainstBase', () => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(10),
};
element._patchRange = {
@@ -440,7 +441,7 @@
test('_handleDiffAgainstLatest', () => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(10),
};
element._patchRange = {
@@ -460,7 +461,7 @@
test('_handleDiffBaseAgainstLeft', () => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(10),
};
element._patchRange = {
@@ -479,7 +480,7 @@
test('_handleDiffRightAgainstLatest', () => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(10),
};
element._patchRange = {
@@ -498,7 +499,7 @@
test('_handleDiffBaseAgainstLatest', () => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(10),
};
element._patchRange = {
@@ -686,14 +687,14 @@
test('A toggles overlay when logged in', done => {
sinon.stub(element, '_getLoggedIn').returns(Promise.resolve(true));
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(1),
messages: createChangeMessages(1),
};
element._change.labels = {};
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
// element has latest info
revisions: createRevisions(1),
messages: createChangeMessages(1),
@@ -723,7 +724,7 @@
element._loggedIn = true;
element._loading = false;
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
labels: {},
actions: {
abandon: {
@@ -750,7 +751,7 @@
element._loggedIn = true;
element._loading = false;
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
labels: {},
actions: {
abandon: {
@@ -837,7 +838,7 @@
patchNum: 1 as RevisionPatchSetNum,
};
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
rev1: createRevision(),
},
@@ -958,7 +959,7 @@
suite('_recomputeComments', () => {
setup(() => {
element._changeNum = TEST_NUMERIC_CHANGE_ID;
- element._change = createChange();
+ element._change = createChangeViewChange();
flush();
// Fake computeDraftCount as its required for ChangeComments,
// see gr-comment-api#reloadDrafts.
@@ -969,7 +970,7 @@
computeDraftCount: () => 0,
} as ChangeComments)
);
- element._change = createChange();
+ element._change = createChangeViewChange();
element._changeNum = element._change._number;
});
@@ -1080,7 +1081,7 @@
patchNum: 1 as RevisionPatchSetNum,
};
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
rev2: createRevision(2),
rev1: createRevision(1),
@@ -1112,7 +1113,7 @@
setup(done => {
element._changeNum = TEST_NUMERIC_CHANGE_ID;
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
rev2: createRevision(2),
rev1: createRevision(1),
@@ -1236,7 +1237,7 @@
test('_changeStatuses', () => {
element._loading = false;
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
rev2: createRevision(2),
rev1: createRevision(1),
@@ -1309,7 +1310,7 @@
patchNum: 1 as RevisionPatchSetNum,
};
const change = {
- ...createChange(),
+ ...createChangeViewChange(),
owner: createAccountWithIdNameAndEmail(),
revisions: {
rev2: createRevision(2),
@@ -1409,7 +1410,7 @@
test('change num change', () => {
const change = {
- ...createChange(),
+ ...createChangeViewChange(),
labels: {},
} as ParsedChangeInfo;
stubRestApi('getChangeDetail').returns(Promise.resolve(change));
@@ -1592,29 +1593,9 @@
});
});
- test('_computeMergedCommitInfo', () => {
- const dummyRevs: {[revisionId: string]: RevisionInfo} = {
- 1: createRevision(1),
- 2: createRevision(2),
- };
- assert.deepEqual(
- element._computeMergedCommitInfo('0' as CommitId, dummyRevs),
- {}
- );
- assert.deepEqual(
- element._computeMergedCommitInfo('1' as CommitId, dummyRevs),
- dummyRevs[1].commit
- );
-
- // Regression test for issue 5337.
- const commit = element._computeMergedCommitInfo('2' as CommitId, dummyRevs);
- assert.notDeepEqual(commit, dummyRevs[2]);
- assert.deepEqual(commit, dummyRevs[2].commit);
- });
-
test('_computeCopyTextForTitle', () => {
const change: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
_number: 123 as NumericChangeId,
subject: 'test subject',
revisions: {
@@ -1645,6 +1626,7 @@
revisions: {
rev1: createRevision(1),
},
+ current_revision: undefined,
};
assert.equal(element._getLatestRevisionSHA(change), 'rev1');
});
@@ -1652,7 +1634,7 @@
test('show commit message edit button', () => {
const change = createChange();
const mergedChanged: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
status: ChangeStatus.MERGED,
};
assert.isTrue(element._computeHideEditCommitMessage(false, false, change));
@@ -1671,7 +1653,7 @@
});
test('_handleCommitMessageSave trims trailing whitespace', () => {
- element._change = createChange();
+ element._change = createChangeViewChange();
// Response code is 500, because we want to avoid window reloading
const putStub = stubRestApi('putChangeCommitMessage').returns(
Promise.resolve(new Response(null, {status: 500}))
@@ -1694,7 +1676,7 @@
test('_computeChangeIdCommitMessageError', () => {
let commitMessage = 'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483';
let change: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
};
assert.equal(
@@ -1703,7 +1685,7 @@
);
change = {
- ...createChange(),
+ ...createChangeViewChange(),
change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
};
assert.equal(
@@ -1724,7 +1706,7 @@
'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483',
].join('\n');
let change: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
};
assert.equal(
@@ -1732,7 +1714,7 @@
null
);
change = {
- ...createChange(),
+ ...createChangeViewChange(),
change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
};
assert.equal(
@@ -1747,7 +1729,7 @@
'Change-Id: I4ce18b2395bca69d7a9aa48bf4554faa56282483',
].join(' and ');
let change: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282484' as ChangeId,
};
assert.equal(
@@ -1755,7 +1737,7 @@
null
);
change = {
- ...createChange(),
+ ...createChangeViewChange(),
change_id: 'I4ce18b2395bca69d7a9aa48bf4554faa56282483' as ChangeId,
};
assert.equal(
@@ -1793,7 +1775,7 @@
sinon.stub(element, '_changeChanged');
stubRestApi('getChangeDetail').returns(
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
labels: {},
current_revision: 'foo' as CommitId,
revisions: {foo: createRevision()},
@@ -1810,7 +1792,7 @@
sinon.stub(element, '_changeChanged');
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
labels: {},
current_revision: 'foo' as CommitId,
revisions: {foo: createRevision()},
@@ -1828,7 +1810,7 @@
const changeRevision = createRevision();
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
labels: {},
current_revision: 'foo' as CommitId,
revisions: {foo: {...changeRevision}},
@@ -1862,7 +1844,7 @@
test('_getBasePatchNum', () => {
const _change: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
'98da160735fb81604b4c40e93c368f380539dd0e': createRevision(),
},
@@ -1878,7 +1860,7 @@
};
const _change2: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
'98da160735fb81604b4c40e93c368f380539dd0e': {
...createRevision(1),
@@ -1992,7 +1974,7 @@
patchNum: 2 as RevisionPatchSetNum,
};
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
rev1: createRevision(1),
rev2: createRevision(2),
@@ -2053,14 +2035,14 @@
setup(() => {
sinon.stub(element.$.replyDialog, '_draftChanged');
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(1),
messages: createChangeMessages(1),
};
element._change.labels = {};
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
// element has latest info
revisions: {rev1: createRevision()},
messages: createChangeMessages(1),
@@ -2145,14 +2127,14 @@
suite('commit message expand/collapse', () => {
setup(() => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(1),
messages: createChangeMessages(1),
};
element._change.labels = {};
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
// new patchset was uploaded
revisions: createRevisions(2),
current_revision: getCurrentRevision(2),
@@ -2331,7 +2313,7 @@
clock = sinon.useFakeTimers();
startUpdateCheckTimerSpy = sinon.spy(element, '_startUpdateCheckTimer');
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: createRevisions(1),
messages: createChangeMessages(1),
};
@@ -2340,7 +2322,7 @@
test('_startUpdateCheckTimer negative delay', () => {
const getChangeDetailStub = stubRestApi('getChangeDetail').returns(
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
// element has latest info
revisions: {rev1: createRevision()},
messages: createChangeMessages(1),
@@ -2361,7 +2343,7 @@
const getChangeDetailStub = stubRestApi('getChangeDetail').callsFake(
() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
// element has latest info
revisions: {rev1: createRevision()},
messages: createChangeMessages(1),
@@ -2409,7 +2391,7 @@
test('_startUpdateCheckTimer respects _loading', async () => {
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
// new patchset was uploaded
revisions: createRevisions(2),
current_revision: getCurrentRevision(2),
@@ -2432,7 +2414,7 @@
test('_startUpdateCheckTimer new status shows an alert', async () => {
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
// element has latest info
revisions: {rev1: createRevision()},
messages: createChangeMessages(1),
@@ -2458,7 +2440,7 @@
test('_startUpdateCheckTimer new messages shows an alert', async () => {
stubRestApi('getChangeDetail').callsFake(() =>
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {rev1: createRevision()},
// element has new message
messages: createChangeMessages(2),
@@ -2484,7 +2466,7 @@
test('canStartReview computation', () => {
const change1: ChangeInfo = createChange();
const change2: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
actions: {
ready: {
enabled: true,
@@ -2492,7 +2474,7 @@
},
};
const change3: ChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
actions: {
ready: {
label: 'Ready for Review',
@@ -2567,7 +2549,7 @@
test('_processEdit', () => {
element._patchRange = {};
const change: ParsedChangeInfo = {
- ...createChange(),
+ ...createChangeViewChange(),
current_revision: 'foo' as CommitId,
revisions: {
foo: {...createRevision(), actions: {cherrypick: {enabled: true}}},
@@ -2622,7 +2604,7 @@
patchNum: 1 as RevisionPatchSetNum,
};
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
};
const fileList = element.$.fileList;
const Actions = GrEditConstants.Actions;
@@ -2700,7 +2682,7 @@
const revision2: RevisionInfo = createRevision(2);
stubRestApi('getChangeDetail').returns(
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
aaa: revision1,
bbb: revision2,
@@ -2729,7 +2711,7 @@
const revision3 = createEditRevision();
stubRestApi('getChangeDetail').returns(
Promise.resolve({
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {
aaa: revision1,
bbb: revision2,
@@ -2751,7 +2733,7 @@
});
test('_sendShowChangeEvent', () => {
- const change = {...createChange(), labels: {}};
+ const change = {...createChangeViewChange(), labels: {}};
element._change = {...change};
element._patchRange = {patchNum: 4 as RevisionPatchSetNum};
element._mergeable = true;
@@ -2776,7 +2758,7 @@
navigateToChangeStub.restore();
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
revisions: {rev1: createRevision()},
};
});
@@ -2830,7 +2812,7 @@
test('_handleStopEditTap', done => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
};
sinon.stub(element.$.metadata, '_computeLabelNames');
navigateToChangeStub.restore();
@@ -2848,7 +2830,7 @@
suite('plugin endpoints', () => {
test('endpoint params', done => {
- element._change = {...createChange(), labels: {}};
+ element._change = {...createChangeViewChange(), labels: {}};
element._selectedRevision = createRevision();
let hookEl: HTMLElement;
let plugin: PluginApi;
@@ -2875,7 +2857,7 @@
suite('_getMergeability', () => {
let getMergeableStub: SinonStubbedMember<RestApiService['getMergeable']>;
setup(() => {
- element._change = {...createChange(), labels: {}};
+ element._change = {...createChangeViewChange(), labels: {}};
getMergeableStub = stubRestApi('getMergeable').returns(
Promise.resolve({...createMergeable(), mergeable: true})
);
@@ -2927,7 +2909,7 @@
test('_handleToggleStar called when star is tapped', () => {
element._change = {
- ...createChange(),
+ ...createChangeViewChange(),
owner: {_account_id: 1 as AccountId},
starred: false,
};
diff --git a/polygerrit-ui/app/utils/change-metadata-util.ts b/polygerrit-ui/app/utils/change-metadata-util.ts
index c3eb846..05afe38 100644
--- a/polygerrit-ui/app/utils/change-metadata-util.ts
+++ b/polygerrit-ui/app/utils/change-metadata-util.ts
@@ -23,6 +23,7 @@
REPO_BRANCH = 'Repo | Branch',
SUBMITTED = 'Submitted',
PARENT = 'Parent',
+ MERGED_AS = 'Merged as',
STRATEGY = 'Strategy',
UPDATED = 'Updated',
CC = 'CC',
@@ -52,7 +53,12 @@
Metadata.ASSIGNEE,
Metadata.CHERRY_PICK_OF,
],
- ALWAYS_HIDE: [Metadata.PARENT, Metadata.STRATEGY, Metadata.UPDATED],
+ ALWAYS_HIDE: [
+ Metadata.PARENT,
+ Metadata.MERGED_AS,
+ Metadata.STRATEGY,
+ Metadata.UPDATED,
+ ],
};
export function isSectionSet(section: Metadata, change?: ParsedChangeInfo) {