commit 7973c7e59d52a688035a7958307953928f37ede4 Author: Dhruv Srivastava Date: Tue Sep 29 22:18:21 2020 +0200 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 diff --git a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts index e8ceb56..d7a1f17 100644 --- a/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts +++ b/polygerrit-ui/app/elements/change/gr-reviewer-list/gr-reviewer-list.ts @@ -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) { diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts index d487b6c..0eddbc0 100644 --- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts +++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account.ts @@ -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.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; } diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts index 6556a94..1d437fb 100644 --- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts +++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_html.ts @@ -172,6 +172,28 @@ export const htmlTemplate = html` + diff --git a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js index e972b91..b09f0ce 100644 --- a/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js +++ b/polygerrit-ui/app/elements/shared/gr-hovercard-account/gr-hovercard-account_test.js @@ -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` @@ -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 => { diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts index 11c861e..47924e6 100644 --- a/polygerrit-ui/app/utils/change-util.ts +++ b/polygerrit-ui/app/utils/change-util.ts @@ -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) + ); +} diff --git a/polygerrit-ui/app/utils/change-util_test.js b/polygerrit-ui/app/utils/change-util_test.js index 20b9578..fd181fe 100644 --- a/polygerrit-ui/app/utils/change-util_test.js +++ b/polygerrit-ui/app/utils/change-util_test.js @@ -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); + }); });