Add a first experimental version of <gr-revision-parents>
This is flag protected by `UiFeature__revision_parents_data` and only
visible to developers.
This change only sets up the new component and the code structure. It
surfaces the data for developer purposes only and looks like this:
https://imgur.com/a/v6dhdwn. The actual UI design will happen in
subsequent changes.
We are requesting the `PARENTS` change detail option, which is not even
flag protected. It will just return more information that we will
definitely need for this feature. And this is not expected to add any
substantial latency.
We have also refined the type model of `RevisionInfo` by removing
`basePatchNum` from it, which should only be a property of
`EditRevisionInfo`.
Release-Notes: skip
Google-Bug-Id: b/289177174
Change-Id: I271d8cdcc88de68d2f7738a06b23909ef3e8f1c0
diff --git a/polygerrit-ui/app/api/rest-api.ts b/polygerrit-ui/app/api/rest-api.ts
index 125471e..2125123 100644
--- a/polygerrit-ui/app/api/rest-api.ts
+++ b/polygerrit-ui/app/api/rest-api.ts
@@ -1025,7 +1025,6 @@
* fields are returned by default. Additional fields can be obtained by
* adding o parameters as described in Query Changes.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#revision-info
- * basePatchNum is present in case RevisionInfo is of type 'edit'
*/
export declare interface RevisionInfo {
kind: RevisionKind;
@@ -1040,7 +1039,22 @@
commit_with_footers?: boolean;
push_certificate?: PushCertificateInfo;
description?: string;
- basePatchNum?: BasePatchSetNum;
+ parents_data?: ParentInfo[];
+}
+
+/**
+ * The ParentInfo entity contains detailed information the parent commit of a revision.
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#parent-info
+ * basePatchNum is present in case RevisionInfo is of type 'edit'
+ */
+export declare interface ParentInfo {
+ branch_name?: string;
+ commit_id?: string;
+ is_merged_in_target_branch?: boolean;
+ change_id?: ChangeId;
+ change_number?: NumericChangeId;
+ patch_set_number?: PatchSetNumber;
+ change_status?: ChangeStatus;
}
export type SchemesInfoMap = {[name: string]: DownloadSchemeInfo};
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 f450ec0..c6675b3 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
@@ -24,6 +24,7 @@
import '../gr-download-dialog/gr-download-dialog';
import '../gr-file-list-header/gr-file-list-header';
import '../gr-file-list/gr-file-list';
+import '../gr-revision-parents/gr-revision-parents';
import '../gr-included-in-dialog/gr-included-in-dialog';
import '../gr-messages-list/gr-messages-list';
import '../gr-related-changes-list/gr-related-changes-list';
@@ -148,6 +149,7 @@
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {modalStyles} from '../../../styles/gr-modal-styles';
import {relatedChangesModelToken} from '../../../models/change/related-changes-model';
+import {KnownExperimentId} from '../../../services/flags/flags';
const MIN_LINES_FOR_COMMIT_COLLAPSE = 18;
@@ -408,6 +410,8 @@
private readonly getChecksModel = resolve(this, checksModelToken);
+ readonly flagService = getAppContext().flagsService;
+
readonly restApiService = getAppContext().restApiService;
private readonly getPluginLoader = resolve(this, pluginLoaderToken);
@@ -1416,6 +1420,10 @@
@collapse-diffs=${this.collapseAllDiffs}
>
</gr-file-list-header>
+ ${when(
+ this.flagService.isEnabled(KnownExperimentId.REVISION_PARENTS_DATA),
+ () => html`<gr-revision-parents></gr-revision-parents>`
+ )}
<gr-file-list
id="fileList"
.change=${this.change}
diff --git a/polygerrit-ui/app/elements/change/gr-revision-parents/gr-revision-parents.ts b/polygerrit-ui/app/elements/change/gr-revision-parents/gr-revision-parents.ts
new file mode 100644
index 0000000..9f6f322
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-revision-parents/gr-revision-parents.ts
@@ -0,0 +1,102 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {customElement, state} from 'lit/decorators.js';
+import {css, html, LitElement} from 'lit';
+import {resolve} from '../../../models/dependency';
+import {subscribe} from '../../lit/subscription-controller';
+import {changeModelToken} from '../../../models/change/change-model';
+import {EDIT, RevisionInfo} from '../../../api/rest-api';
+import {fontStyles} from '../../../styles/gr-font-styles';
+
+@customElement('gr-revision-parents')
+export class GrRevisionParents extends LitElement {
+ @state() revision?: RevisionInfo;
+
+ @state() baseRevision?: RevisionInfo;
+
+ private readonly getChangeModel = resolve(this, changeModelToken);
+
+ constructor() {
+ super();
+ subscribe(
+ this,
+ () => this.getChangeModel().revision$,
+ x => {
+ if (x?._number === EDIT) x = undefined;
+ this.revision = x as RevisionInfo | undefined;
+ }
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().baseRevision$,
+ x => (this.baseRevision = x)
+ );
+ }
+
+ static override get styles() {
+ return [
+ fontStyles,
+ css`
+ :host {
+ display: block;
+ }
+ div.container {
+ padding: var(--spacing-m) var(--spacing-l);
+ border-top: 1px solid var(--border-color);
+ background-color: var(--yellow-50);
+ }
+ .section {
+ margin-top: var(--spacing-m);
+ }
+ .section > div {
+ margin-left: var(--spacing-xl);
+ }
+ .title {
+ font-weight: var(--font-weight-bold);
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ // TODO(revision-parents): Figure out what to do about multiple parents.
+ const baseParent = this.baseRevision?.parents_data?.[0];
+ const parent = this.revision?.parents_data?.[0];
+ if (!parent || !baseParent) return;
+ // TODO(revision-parents): Design something nicer for the various cases.
+ return html`
+ <div class="container">
+ <h3 class="heading-3">Parent Information</h3>
+ <div class="section">
+ <h4 class="heading-4">Left Revision</h4>
+ <div>Branch: ${baseParent.branch_name}</div>
+ <div>Commit ID: ${baseParent.commit_id}</div>
+ <div>Is Merged: ${baseParent.is_merged_in_target_branch}</div>
+ <div>Change ID: ${baseParent.change_id}</div>
+ <div>Change Number: ${baseParent.change_number}</div>
+ <div>Patchset Number: ${baseParent.patch_set_number}</div>
+ <div>Change Status: ${baseParent.change_status}</div>
+ </div>
+ <div class="section">
+ <h4 class="heading-4">Right Revision</h4>
+ <div>Branch: ${parent.branch_name}</div>
+ <div>Commit ID: ${parent.commit_id}</div>
+ <div>Is Merged: ${parent.is_merged_in_target_branch}</div>
+ <div>Change ID: ${parent.change_id}</div>
+ <div>Change Number: ${parent.change_number}</div>
+ <div>Patchset Number: ${parent.patch_set_number}</div>
+ <div>Change Status: ${parent.change_status}</div>
+ </div>
+ </div>
+ `;
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-revision-parents': GrRevisionParents;
+ }
+}
diff --git a/polygerrit-ui/app/elements/change/gr-revision-parents/gr-revision-parents_test.ts b/polygerrit-ui/app/elements/change/gr-revision-parents/gr-revision-parents_test.ts
new file mode 100644
index 0000000..03627ca
--- /dev/null
+++ b/polygerrit-ui/app/elements/change/gr-revision-parents/gr-revision-parents_test.ts
@@ -0,0 +1,124 @@
+/**
+ * @license
+ * Copyright 2023 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+import './gr-revision-parents';
+import {fixture, html, assert} from '@open-wc/testing';
+import {GrRevisionParents} from './gr-revision-parents';
+import {createRevision} from '../../../test/test-data-generators';
+import {
+ ChangeId,
+ ChangeStatus,
+ NumericChangeId,
+ PatchSetNumber,
+} from '../../../api/rest-api';
+
+suite('gr-revision-parents tests', () => {
+ let element: GrRevisionParents;
+
+ setup(async () => {
+ element = await fixture(html`<gr-revision-parents></gr-revision-parents>`);
+ await element.updateComplete;
+ });
+
+ test('render empty', () => {
+ assert.shadowDom.equal(element, '');
+ });
+
+ test('render', async () => {
+ element.baseRevision = {
+ ...createRevision(1),
+ parents_data: [
+ {
+ branch_name: 'refs/heads/master',
+ commit_id: '78e52ce873b1c08396422f51ad6aacf77ed95541',
+ is_merged_in_target_branch: false,
+ change_id: 'Idc69e6d7bba0ce0a9a0bdcd22adb506c0b76e628' as ChangeId,
+ change_number: 1500 as NumericChangeId,
+ patch_set_number: 1 as PatchSetNumber,
+ change_status: ChangeStatus.NEW,
+ },
+ ],
+ };
+ element.revision = {
+ ...createRevision(2),
+ parents_data: [
+ {
+ branch_name: 'refs/heads/master',
+ commit_id: '78e52ce873b1c08396422f51ad6aacf77ed95541',
+ is_merged_in_target_branch: false,
+ change_id: 'Idc69e6d7bba0ce0a9a0bdcd22adb506c0b76e628' as ChangeId,
+ change_number: 1500 as NumericChangeId,
+ patch_set_number: 2 as PatchSetNumber,
+ change_status: ChangeStatus.NEW,
+ },
+ ],
+ };
+ await element.updateComplete;
+
+ assert.shadowDom.equal(
+ element,
+ /* HTML*/ `
+ <div class="container">
+ <h3 class="heading-3">
+ Parent Information
+ </h3>
+ <div class="section">
+ <h4 class="heading-4">
+ Left Revision
+ </h4>
+ <div>
+ Branch: refs/heads/master
+ </div>
+ <div>
+ Commit ID: 78e52ce873b1c08396422f51ad6aacf77ed95541
+ </div>
+ <div>
+ Is Merged: false
+ </div>
+ <div>
+ Change ID: Idc69e6d7bba0ce0a9a0bdcd22adb506c0b76e628
+ </div>
+ <div>
+ Change Number: 1500
+ </div>
+ <div>
+ Patchset Number: 1
+ </div>
+ <div>
+ Change Status: NEW
+ </div>
+ </div>
+ <div class="section">
+ <h4 class="heading-4">
+ Right Revision
+ </h4>
+ <div>
+ Branch: refs/heads/master
+ </div>
+ <div>
+ Commit ID: 78e52ce873b1c08396422f51ad6aacf77ed95541
+ </div>
+ <div>
+ Is Merged: false
+ </div>
+ <div>
+ Change ID: Idc69e6d7bba0ce0a9a0bdcd22adb506c0b76e628
+ </div>
+ <div>
+ Change Number: 1500
+ </div>
+ <div>
+ Patchset Number: 2
+ </div>
+ <div>
+ Change Status: NEW
+ </div>
+ </div>
+ </div>
+ `
+ );
+ });
+});
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index acae8ad..e6f8512 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -15,6 +15,7 @@
RevisionPatchSetNum,
PatchSetNumber,
CommitId,
+ RevisionInfo,
} from '../../types/common';
import {ChangeStatus, DefaultBase} from '../../constants/constants';
import {combineLatest, from, Observable, forkJoin, of} from 'rxjs';
@@ -324,12 +325,12 @@
);
private selectRevision(
- revisionNum$: Observable<RevisionPatchSetNum | undefined>
+ revisionNum$: Observable<RevisionPatchSetNum | BasePatchSetNum | undefined>
) {
return select(
combineLatest([this.revisions$, revisionNum$]),
([revisions, patchNum]) => {
- if (!revisions || !patchNum) return undefined;
+ if (!revisions || !patchNum || patchNum === PARENT) return undefined;
return Object.values(revisions).find(
revision => revision._number === patchNum
);
@@ -339,6 +340,10 @@
public readonly revision$ = this.selectRevision(this.patchNum$);
+ public readonly baseRevision$ = this.selectRevision(
+ this.basePatchNum$
+ ) as Observable<RevisionInfo | undefined>;
+
public readonly latestRevision$ = this.selectRevision(this.latestPatchNum$);
public readonly isOwner$: Observable<boolean> = select(
diff --git a/polygerrit-ui/app/models/change/change-model_test.ts b/polygerrit-ui/app/models/change/change-model_test.ts
index db9187a..00a03fe 100644
--- a/polygerrit-ui/app/models/change/change-model_test.ts
+++ b/polygerrit-ui/app/models/change/change-model_test.ts
@@ -30,7 +30,7 @@
PatchSetNum,
PatchSetNumber,
} from '../../types/common';
-import {ParsedChangeInfo} from '../../types/types';
+import {EditRevisionInfo, ParsedChangeInfo} from '../../types/types';
import {getAppContext} from '../../services/app-context';
import {
ChangeState,
@@ -75,7 +75,9 @@
let change: ParsedChangeInfo | undefined = createParsedChange();
const edit = createEditInfo();
change = updateChangeWithEdit(change, edit);
- const editRev = change?.revisions[`${edit.commit.commit}`];
+ const editRev = change?.revisions[
+ `${edit.commit.commit}`
+ ] as EditRevisionInfo;
assert.isDefined(editRev);
assert.equal(editRev?._number, EDIT);
assert.equal(editRev?.basePatchNum, edit.base_patch_set_number);
diff --git a/polygerrit-ui/app/services/flags/flags.ts b/polygerrit-ui/app/services/flags/flags.ts
index a8d4a3b..a59f3da 100644
--- a/polygerrit-ui/app/services/flags/flags.ts
+++ b/polygerrit-ui/app/services/flags/flags.ts
@@ -20,4 +20,5 @@
PUSH_NOTIFICATIONS = 'UiFeature__push_notifications',
ML_SUGGESTED_EDIT = 'UiFeature__ml_suggested_edit',
DIFF_FOR_USER_SUGGESTED_EDIT = 'UiFeature__diff_for_user_suggested_edit',
+ REVISION_PARENTS_DATA = 'UiFeature__revision_parents_data',
}
diff --git a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
index 37e61b2..d7d7135 100644
--- a/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
+++ b/polygerrit-ui/app/services/gr-rest-api/gr-rest-api-impl.ts
@@ -1240,6 +1240,7 @@
ListChangesOption.WEB_LINKS,
ListChangesOption.SKIP_DIFFSTAT,
ListChangesOption.SUBMIT_REQUIREMENTS,
+ ListChangesOption.PARENTS,
];
if (config?.receive?.enable_signed_push) {
options.push(ListChangesOption.PUSH_CERTIFICATES);
@@ -1265,6 +1266,7 @@
'WEB_LINKS',
'SKIP_DIFFSTAT',
'SUBMIT_REQUIREMENTS',
+ 'PARENTS',
];
if (config?.receive?.enable_signed_push) {
options.push('PUSH_CERTIFICATES');
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 88bb6b9..7f197d6 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -326,6 +326,10 @@
// Include the 'starred' field, that is if the change is starred by the
// current user.
STAR: 26,
+
+ // Include the `parents_data` field in each revision, e.g. if it's merged in the target branch and
+ // whether it points to a patch-set of another change.
+ PARENTS: 27,
};
// https://gerrit-review.googlesource.com/Documentation/rest-api-accounts.html#contributor-agreement-input
diff --git a/polygerrit-ui/app/types/types.ts b/polygerrit-ui/app/types/types.ts
index 9f2374a..c9f3414 100644
--- a/polygerrit-ui/app/types/types.ts
+++ b/polygerrit-ui/app/types/types.ts
@@ -116,8 +116,12 @@
updates: {message: string; reviewers: AccountInfo[]}[];
}
+/**
+ * https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#edit-info
+ */
export interface EditRevisionInfo extends Partial<RevisionInfo> {
// EditRevisionInfo has less required properties then RevisionInfo
+ // TODO: Explicitly list which props are required and optional here.
_number: EditPatchSet;
basePatchNum: BasePatchSetNum;
commit: CommitInfo;
diff --git a/polygerrit-ui/app/utils/patch-set-util_test.ts b/polygerrit-ui/app/utils/patch-set-util_test.ts
index b67db9b..3214c76 100644
--- a/polygerrit-ui/app/utils/patch-set-util_test.ts
+++ b/polygerrit-ui/app/utils/patch-set-util_test.ts
@@ -18,6 +18,7 @@
PatchSetNumber,
ReviewInputTag,
PARENT,
+ RevisionInfo,
} from '../types/common';
import {
_testOnly_computeWipForPatchSets,
@@ -29,6 +30,7 @@
isMergeParent,
sortRevisions,
} from './patch-set-util';
+import {EditRevisionInfo} from '../types/types';
suite('gr-patch-set-util tests', () => {
test('getRevisionByPatchNum', () => {
@@ -159,7 +161,11 @@
});
test('findEditParentRevision', () => {
- const revisions = [createRevision(0), createRevision(1), createRevision(2)];
+ const revisions: Array<RevisionInfo | EditRevisionInfo> = [
+ createRevision(0),
+ createRevision(1),
+ createRevision(2),
+ ];
assert.strictEqual(findEditParentRevision(revisions), null);
revisions.push({
@@ -173,7 +179,11 @@
});
test('findEditParentPatchNum', () => {
- const revisions = [createRevision(0), createRevision(1), createRevision(2)];
+ const revisions: Array<RevisionInfo | EditRevisionInfo> = [
+ createRevision(0),
+ createRevision(1),
+ createRevision(2),
+ ];
assert.equal(findEditParentPatchNum(revisions), -1);
revisions.push(
@@ -187,8 +197,16 @@
});
test('sortRevisions', () => {
- const revisions = [createRevision(0), createRevision(2), createRevision(1)];
- const sorted = [createRevision(2), createRevision(1), createRevision(0)];
+ const revisions: Array<RevisionInfo | EditRevisionInfo> = [
+ createRevision(0),
+ createRevision(2),
+ createRevision(1),
+ ];
+ const sorted: Array<RevisionInfo | EditRevisionInfo> = [
+ createRevision(2),
+ createRevision(1),
+ createRevision(0),
+ ];
assert.deepEqual(sortRevisions(revisions), sorted);
@@ -203,8 +221,8 @@
});
assert.deepEqual(sortRevisions(revisions), sorted);
- revisions[0].basePatchNum = 0 as BasePatchSetNum;
- const edit = sorted.shift()!;
+ (revisions[0] as EditRevisionInfo).basePatchNum = 0 as BasePatchSetNum;
+ const edit = sorted.shift() as EditRevisionInfo;
edit.basePatchNum = 0 as BasePatchSetNum;
// Edit patchset should be at index 2.
sorted.splice(2, 0, edit);
