Add more functionalities to hovercard

Remove reviewer
Remove CC
Move CC to Reviewer
Move Reviewer to CC

Screenshot: https://imgur.com/a/HzmgvSc
Change-Id: I312bb99f55cae70a2003c7081f1561c4d65b7633
This commit is contained in:
Dhruv Srivastava
2020-09-29 22:18:21 +02:00
parent ebd77675cc
commit 7973c7e59d
6 changed files with 252 additions and 22 deletions

View File

@@ -40,6 +40,7 @@ import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {hasOwnProperty} from '../../../utils/common-util';
import {isRemovableReviewer} from '../../../utils/change-util';
export interface GrReviewerList {
$: {
@@ -252,25 +253,7 @@ export class GrReviewerList extends GestureEventListeners(
}
_computeCanRemoveReviewer(reviewer: AccountInfo, mutable: boolean) {
if (
!mutable ||
this.change === undefined ||
this.change.removable_reviewers === undefined
) {
return false;
}
let current;
for (let i = 0; i < this.change.removable_reviewers.length; i++) {
current = this.change.removable_reviewers[i];
if (
current._account_id === reviewer._account_id ||
(!reviewer._account_id && current.email === reviewer.email)
) {
return true;
}
}
return false;
return mutable && isRemovableReviewer(this.change, reviewer);
}
_handleRemove(e: Event) {

View File

@@ -26,10 +26,16 @@ import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mix
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-hovercard-account_html';
import {appContext} from '../../../services/app-context';
import {accountKey} from '../../../utils/account-util';
import {getDisplayName} from '../../../utils/display-name-util';
import {customElement, property} from '@polymer/decorators';
import {RestApiService} from '../../../services/services/gr-rest-api/gr-rest-api';
import {AccountInfo, ChangeInfo, ServerInfo} from '../../../types/common';
import {
AccountInfo,
ChangeInfo,
ServerInfo,
ReviewInput,
} from '../../../types/common';
import {ReportingService} from '../../../services/gr-reporting/gr-reporting';
import {
canHaveAttention,
@@ -38,6 +44,8 @@ import {
hasAttention,
isAttentionSetEnabled,
} from '../../../utils/attention-set-util';
import {ReviewerState} from '../../../constants/constants';
import {isRemovableReviewer} from '../../../utils/change-util';
export interface GrHovercardAccount {
$: {
@@ -127,6 +135,93 @@ export class GrHovercardAccount extends GestureEventListeners(
return getLastUpdate(this.account, change);
}
_showReviewerOrCCActions(account?: AccountInfo, change?: ChangeInfo) {
return !!this._selfAccount && isRemovableReviewer(change, account);
}
_getReviewerState(account: AccountInfo, change: ChangeInfo) {
if (
change.reviewers[ReviewerState.REVIEWER]?.some(
(reviewer: AccountInfo) => {
return reviewer._account_id === account._account_id;
}
)
) {
return ReviewerState.REVIEWER;
}
return ReviewerState.CC;
}
_computeReviewerOrCCText(account?: AccountInfo, change?: ChangeInfo) {
if (!change || !account) return '';
return this._getReviewerState(account, change) === ReviewerState.REVIEWER
? 'Reviewer'
: 'CC';
}
_computeChangeReviewerOrCCText(account?: AccountInfo, change?: ChangeInfo) {
if (!change || !account) return '';
return this._getReviewerState(account, change) === ReviewerState.REVIEWER
? 'Move Reviewer to CC'
: 'Move CC to Reviewer';
}
_handleChangeReviewerOrCCStatus() {
if (!this.change) throw new Error('expected change object to be present');
// accountKey() throws an error if _account_id & email is not found, which
// we want to check before showing reloading toast
const _accountKey = accountKey(this.account);
this.dispatchEventThroughTarget('show-alert', {
detail: {
message: 'Reloading page...',
},
});
const reviewInput: Partial<ReviewInput> = {};
reviewInput.reviewers = [
{
reviewer: _accountKey,
state:
this._getReviewerState(this.account, this.change) === ReviewerState.CC
? ReviewerState.REVIEWER
: ReviewerState.CC,
},
];
this.$.restAPI
.saveChangeReview(this.change._number, 'current', reviewInput)
.then(response => {
if (!response || !response.ok) {
throw new Error(
'something went wrong when toggling' +
this._getReviewerState(this.account, this.change!)
);
}
this.dispatchEventThroughTarget('reload', {clearPatchset: true});
});
}
_handleRemoveReviewerOrCC() {
if (!this.change || !(this.account?._account_id || this.account?.email))
throw new Error('Missing change or account.');
this.dispatchEventThroughTarget('show-alert', {
detail: {
message: 'Reloading page...',
},
});
this.$.restAPI
.removeChangeReviewer(
this.change._number,
(this.account?._account_id || this.account?.email)!
)
.then((response: Response | undefined) => {
if (!response || !response.ok) {
throw new Error('something went wrong when removing user');
}
this.dispatchEventThroughTarget('reload', {clearPatchset: true});
return response;
});
}
_computeShowLabelNeedsAttention() {
return this.isAttentionEnabled && this.hasUserAttention;
}

View File

@@ -172,6 +172,28 @@ export const htmlTemplate = html`
</gr-button>
</div>
</template>
<template is="dom-if" if="[[_showReviewerOrCCActions(account, change)]]">
<div class="action">
<gr-button
class="removeReviewerOrCC"
link=""
no-uppercase=""
on-click="_handleRemoveReviewerOrCC"
>
Remove [[_computeReviewerOrCCText(account, change)]]
</gr-button>
</div>
<div class="action">
<gr-button
class="changeReviewerOrCC"
link=""
no-uppercase=""
on-click="_handleChangeReviewerOrCCStatus"
>
[[_computeChangeReviewerOrCCText(account, change)]]
</gr-button>
</div>
</template>
</template>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>

View File

@@ -18,6 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-hovercard-account.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {ReviewerState} from '../../../constants/constants.js';
const basicFixture = fixtureFromTemplate(html`
<gr-hovercard-account class="hovered"></gr-hovercard-account>
@@ -102,6 +103,104 @@ suite('gr-hovercard-account tests', () => {
element.voteableText);
});
test('remove reviewer', async () => {
element.change = {
removable_reviewers: [ACCOUNT],
reviewers: {
[ReviewerState.REVIEWER]: [ACCOUNT],
},
};
sinon.stub(element.$.restAPI, 'removeChangeReviewer').returns(
Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
flush();
const button = element.shadowRoot.querySelector('.removeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Remove Reviewer');
MockInteractions.tap(button);
await flush();
assert.isTrue(reloadListener.called);
});
test('move reviewer to cc', async () => {
element.change = {
removable_reviewers: [ACCOUNT],
reviewers: {
[ReviewerState.REVIEWER]: [ACCOUNT],
},
};
const saveReviewStub = sinon.stub(element.$.restAPI,
'saveChangeReview').returns(
Promise.resolve({ok: true}));
sinon.stub(element.$.restAPI, 'removeChangeReviewer').returns(
Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
flush();
const button = element.shadowRoot.querySelector('.changeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Move Reviewer to CC');
MockInteractions.tap(button);
await flush();
assert.isTrue(saveReviewStub.called);
assert.isTrue(reloadListener.called);
});
test('move reviewer to cc', async () => {
element.change = {
removable_reviewers: [ACCOUNT],
reviewers: {
[ReviewerState.REVIEWER]: [],
},
};
const saveReviewStub = sinon.stub(element.$.restAPI,
'saveChangeReview').returns(
Promise.resolve({ok: true}));
sinon.stub(element.$.restAPI, 'removeChangeReviewer').returns(
Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
flush();
const button = element.shadowRoot.querySelector('.changeReviewerOrCC');
assert.isOk(button);
assert.equal(button.innerText, 'Move CC to Reviewer');
MockInteractions.tap(button);
await flush();
assert.isTrue(saveReviewStub.called);
assert.isTrue(reloadListener.called);
});
test('remove cc', async () => {
element.change = {
removable_reviewers: [ACCOUNT],
reviewers: {
[ReviewerState.REVIEWER]: [],
},
};
sinon.stub(element.$.restAPI, 'removeChangeReviewer').returns(
Promise.resolve({ok: true}));
const reloadListener = sinon.spy();
element._target.addEventListener('reload', reloadListener);
flush();
const button = element.shadowRoot.querySelector('.removeReviewerOrCC');
assert.equal(button.innerText, 'Remove CC');
assert.isOk(button);
MockInteractions.tap(button);
await flush();
assert.isTrue(reloadListener.called);
});
test('add to attention set', async () => {
let apiResolve;
const apiPromise = new Promise(r => {

View File

@@ -16,9 +16,13 @@
*/
import {getBaseUrl} from './url-util';
import {ChangeStatus} from '../constants/constants';
import {NumericChangeId, PatchSetNum, ChangeInfo} from '../types/common';
import {
NumericChangeId,
PatchSetNum,
ChangeInfo,
AccountInfo,
} from '../types/common';
import {ParsedChangeInfo} from '../elements/shared/gr-rest-api-interface/gr-reviewer-updates-parser';
import {AccountInfo} from '../types/common';
// This can be wrong! See WARNING above
interface ChangeStatusesOptions {
@@ -176,3 +180,15 @@ export function isOwner(change?: ChangeInfo, account?: AccountInfo) {
export function changeStatusString(change: ChangeInfo) {
return changeStatuses(change).join(', ');
}
export function isRemovableReviewer(
change?: ChangeInfo,
reviewer?: AccountInfo
): boolean {
if (!change?.removable_reviewers || !reviewer) return false;
return change.removable_reviewers.some(
account =>
account._account_id === reviewer._account_id ||
(!reviewer._account_id && account.email === reviewer.email)
);
}

View File

@@ -21,6 +21,7 @@ import {
changePath,
changeStatuses,
changeStatusString,
isRemovableReviewer,
} from './change-util.js';
suite('change-util tests', () => {
@@ -198,5 +199,19 @@ suite('change-util tests', () => {
assert.deepEqual(statuses, ['Merge Conflict', 'WIP', 'Private']);
assert.equal(statusString, 'Merge Conflict, WIP, Private');
});
test('isRemovableReviewer', () => {
let change = {
removable_reviewers: [{_account_id: 1}],
};
const reviewer = {_account_id: 1};
assert.equal(isRemovableReviewer(change, reviewer), true);
change = {
removable_reviewers: [{_account_id: 2}],
};
assert.equal(isRemovableReviewer(change, reviewer), false);
});
});