Show on whose behalf rebase will be done
When a user is not the latest uploader, we show a message that rebase
will be done on behalf of the uploader. It may encourage reviewers to
rebase changes since they will be assured that they will not become a
new uploader - which often means losing voting privileges.
Screenshot: https://imgur.com/a/BESgYBp
Release-Notes: skip
Google-Bug-Id: b/269731839
Change-Id: I966b73ff93f22e4457fc24dbca1fbf7438b08b2a
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
index 27fde97..dbfe55b 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog.ts
@@ -3,6 +3,7 @@
* Copyright 2016 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
+import '../../shared/gr-account-chip/gr-account-chip';
import {css, html, LitElement, PropertyValues} from 'lit';
import {customElement, property, query, state} from 'lit/decorators.js';
import {when} from 'lit/directives/when.js';
@@ -10,6 +11,8 @@
NumericChangeId,
BranchName,
ChangeActionDialog,
+ AccountDetailInfo,
+ AccountInfo,
} from '../../../types/common';
import '../../shared/gr-dialog/gr-dialog';
import '../../shared/gr-autocomplete/gr-autocomplete';
@@ -26,6 +29,7 @@
import {resolve} from '../../../models/dependency';
import {changeModelToken} from '../../../models/change/change-model';
import {subscribe} from '../../lit/subscription-controller';
+import {userModelToken} from '../../../models/user/user-model';
export interface RebaseChange {
name: string;
@@ -86,9 +90,6 @@
@state()
allowConflicts = false;
- @state()
- isOwner = false;
-
@query('#rebaseOnParentInput')
private rebaseOnParentInput!: HTMLInputElement;
@@ -107,17 +108,30 @@
@query('#parentInput')
parentInput!: GrAutocomplete;
+ @state()
+ account?: AccountDetailInfo;
+
+ @state()
+ uploader?: AccountInfo;
+
private readonly restApiService = getAppContext().restApiService;
private readonly getChangeModel = resolve(this, changeModelToken);
+ private readonly getUserModel = resolve(this, userModelToken);
+
constructor() {
super();
this.query = input => this.getChangeSuggestions(input);
subscribe(
this,
- () => this.getChangeModel().isOwner$,
- x => (this.isOwner = x)
+ () => this.getUserModel().account$,
+ x => (this.account = x)
+ );
+ subscribe(
+ this,
+ () => this.getChangeModel().latestUploader$,
+ x => (this.uploader = x)
);
}
@@ -158,6 +172,9 @@
.rebaseOption {
margin: var(--spacing-m) 0;
}
+ .rebaseOnBehalfMsg {
+ margin-top: var(--spacing-m);
+ }
`,
];
@@ -255,7 +272,7 @@
>
</div>
${when(
- !this.isOwner && this.allowConflicts,
+ !this.isCurrentUserEqualToLatestUploader() && this.allowConflicts,
() =>
html`<span class="message"
>Rebase cannot be done on behalf of the uploader when allowing
@@ -276,6 +293,16 @@
<label for="rebaseChain">Rebase all ancestors</label>
</div>`
)}
+ ${when(
+ !this.isCurrentUserEqualToLatestUploader(),
+ () => html`<div class="rebaseOnBehalfMsg">Rebase will be done on behalf of${
+ !this.allowConflicts ? ' the uploader:' : ''
+ } <gr-account-chip
+ .account=${this.allowConflicts ? this.account : this.uploader}
+ .hideHovercard=${true}
+ ></gr-account-chip
+ ><span></div>`
+ )}
</div>
</gr-dialog>
`;
@@ -310,6 +337,11 @@
});
}
+ isCurrentUserEqualToLatestUploader() {
+ if (!this.account || !this.uploader) return true;
+ return this.account._account_id === this.uploader._account_id;
+ }
+
getRecentChanges() {
if (this.recentChanges) {
return Promise.resolve(this.recentChanges);
diff --git a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts
index 3064014..8d907c9 100644
--- a/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-confirm-rebase-dialog/gr-confirm-rebase-dialog_test.ts
@@ -9,21 +9,34 @@
import {
pressKey,
queryAndAssert,
- stubFlags,
stubRestApi,
waitUntil,
} from '../../../test/test-utils';
-import {NumericChangeId, BranchName} from '../../../types/common';
-import {createChangeViewChange} from '../../../test/test-data-generators';
+import {NumericChangeId, BranchName, Timestamp} from '../../../types/common';
+import {
+ createAccountWithEmail,
+ createChangeViewChange,
+} from '../../../test/test-data-generators';
import {fixture, html, assert} from '@open-wc/testing';
import {Key} from '../../../utils/dom-util';
import {GrDialog} from '../../shared/gr-dialog/gr-dialog';
+import {testResolver} from '../../../test/common-test-setup';
+import {userModelToken} from '../../../models/user/user-model';
+import {
+ changeModelToken,
+ LoadingStatus,
+} from '../../../models/change/change-model';
+import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip';
suite('gr-confirm-rebase-dialog tests', () => {
let element: GrConfirmRebaseDialog;
setup(async () => {
- stubFlags('isEnabled').returns(true);
+ const userModel = testResolver(userModelToken);
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
element = await fixture(
html`<gr-confirm-rebase-dialog></gr-confirm-rebase-dialog>`
);
@@ -91,6 +104,57 @@
);
});
+ suite('on behalf of uploader', () => {
+ let changeModel;
+ const change = {
+ ...createChangeViewChange(),
+ };
+ setup(async () => {
+ element.branch = 'test' as BranchName;
+ await element.updateComplete;
+ changeModel = testResolver(changeModelToken);
+ changeModel.setState({
+ loadingStatus: LoadingStatus.LOADED,
+ change,
+ });
+ });
+ test('for reviewer it shows message about on behalf', () => {
+ const rebaseOnBehalfMsg = queryAndAssert(element, '.rebaseOnBehalfMsg');
+ assert.dom.equal(
+ rebaseOnBehalfMsg,
+ /* HTML */ `<div class="rebaseOnBehalfMsg">
+ Rebase will be done on behalf of the uploader:
+ <gr-account-chip> </gr-account-chip> <span> </span>
+ </div>`
+ );
+ const accountChip: GrAccountChip = queryAndAssert(
+ rebaseOnBehalfMsg,
+ 'gr-account-chip'
+ );
+ assert.equal(
+ accountChip.account!,
+ change?.revisions[change.current_revision]?.uploader
+ );
+ });
+ test('allowConflicts', async () => {
+ element.allowConflicts = true;
+ await element.updateComplete;
+ const rebaseOnBehalfMsg = queryAndAssert(element, '.rebaseOnBehalfMsg');
+ assert.dom.equal(
+ rebaseOnBehalfMsg,
+ /* HTML */ `<div class="rebaseOnBehalfMsg">
+ Rebase will be done on behalf of
+ <gr-account-chip> </gr-account-chip> <span> </span>
+ </div>`
+ );
+ const accountChip: GrAccountChip = queryAndAssert(
+ rebaseOnBehalfMsg,
+ 'gr-account-chip'
+ );
+ assert.equal(accountChip.account, element.account);
+ });
+ });
+
test('disableActions property disables dialog confirm', async () => {
element.disableActions = false;
await element.updateComplete;
diff --git a/polygerrit-ui/app/models/change/change-model.ts b/polygerrit-ui/app/models/change/change-model.ts
index ad5b217..b6b9de7 100644
--- a/polygerrit-ui/app/models/change/change-model.ts
+++ b/polygerrit-ui/app/models/change/change-model.ts
@@ -196,6 +196,11 @@
computeLatestPatchNumWithEdit(patchsets)
);
+ public readonly latestUploader$ = select(
+ this.change$,
+ change => change?.revisions[change.current_revision]?.uploader
+ );
+
/**
* Emits the current patchset number. If the route does not define the current
* patchset num, then this selector waits for the change to be defined and
