Merge "Only allow involved users to change the attention set"
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
index a6c4201..a6e881f 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.ts
@@ -30,6 +30,7 @@
import {ChangeInfo, AccountInfo, ServerInfo} from '../../../types/common';
import {hasOwnProperty} from '../../../utils/common-util';
import {fireEvent} from '../../../utils/event-util';
+import {isInvolved} from '../../../utils/change-util';
@customElement('gr-account-label')
export class GrAccountLabel extends GestureEventListeners(
@@ -260,15 +261,39 @@
};
}
+ _computeAttentionButtonEnabled(
+ config: ServerInfo | undefined,
+ highlight: boolean,
+ account: AccountInfo,
+ change: ChangeInfo,
+ selfAccount: AccountInfo
+ ) {
+ return (
+ this._hasUnforcedAttention(config, highlight, account, change) &&
+ isInvolved(change, selfAccount)
+ );
+ }
+
_computeAttentionIconTitle(
config: ServerInfo | undefined,
highlight: boolean,
account: AccountInfo,
- change: ChangeInfo
+ change: ChangeInfo,
+ selfAccount: AccountInfo,
+ force: boolean
) {
- return this._hasUnforcedAttention(config, highlight, account, change)
+ const enabled = this._computeAttentionButtonEnabled(
+ config,
+ highlight,
+ account,
+ change,
+ selfAccount
+ );
+ return enabled
? 'Click to remove the user from the attention set'
- : 'Disabled. Use "Modify" to make changes.';
+ : force
+ ? 'Disabled. Use "Modify" to make changes.'
+ : 'Disabled. Only involved users can change.';
}
}
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
index b62df9e..39447ad 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.ts
@@ -109,9 +109,9 @@
link=""
aria-label="Remove user from attention set"
on-click="_handleRemoveAttentionClick"
- disabled="[[!_hasUnforcedAttention(_config, highlightAttention, account, change)]]"
- has-tooltip="[[_hasUnforcedAttention(_config, highlightAttention, account, change)]]"
- title="[[_computeAttentionIconTitle(_config, highlightAttention, account, change)]]"
+ disabled="[[!_computeAttentionButtonEnabled(_config, highlightAttention, account, change, _selfAccount)]]"
+ has-tooltip="[[_computeAttentionButtonEnabled(_config, highlightAttention, account, change, _selfAccount)]]"
+ title="[[_computeAttentionIconTitle(_config, highlightAttention, account, change, _selfAccount, forceAttention)]]"
><iron-icon class="attention" icon="gr-icons:attention"></iron-icon>
</gr-button>
</template>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js
index 42b1dd7..6912776 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.js
@@ -82,16 +82,21 @@
});
suite('attention set', () => {
- setup(() => {
+ setup(async () => {
+ const kermit = createAccount('kermit', 31);
element.highlightAttention = true;
element._config = {
change: {enable_attention_set: true},
user: {anonymous_coward_name: 'Anonymous Coward'},
};
- element._selfAccount = createAccount('kermit', 31);
+ element._selfAccount = kermit;
element.account = createAccount('ernie', 42);
- element.change = {attention_set: {42: {}}};
- flush();
+ element.change = {
+ attention_set: {42: {}},
+ owner: kermit,
+ reviewers: {},
+ };
+ await flush();
});
test('show attention button', () => {
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 761e670..f6c4c1c 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
@@ -43,8 +43,8 @@
isAttentionSetEnabled,
} from '../../../utils/attention-set-util';
import {ReviewerState} from '../../../constants/constants';
-import {isRemovableReviewer} from '../../../utils/change-util';
import {CURRENT} from '../../../utils/patch-set-util';
+import {isInvolved, isRemovableReviewer} from '../../../utils/change-util';
@customElement('gr-hovercard-account')
export class GrHovercardAccount extends GestureEventListeners(
@@ -219,15 +219,13 @@
}
_computeShowActionAddToAttentionSet() {
- return (
- this._selfAccount && this.isAttentionEnabled && !this.hasUserAttention
- );
+ const involved = isInvolved(this.change, this._selfAccount);
+ return involved && this.isAttentionEnabled && !this.hasUserAttention;
}
_computeShowActionRemoveFromAttentionSet() {
- return (
- this._selfAccount && this.isAttentionEnabled && this.hasUserAttention
- );
+ const involved = isInvolved(this.change, this._selfAccount);
+ return involved && this.isAttentionEnabled && this.hasUserAttention;
}
_handleClickAddToAttentionSet() {
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 99d3d6c..42bd76c 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
@@ -144,7 +144,7 @@
</template>
<template
is="dom-if"
- if="[[_computeShowActionAddToAttentionSet(_config, highlightAttention, account, change)]]"
+ if="[[_computeShowActionAddToAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
>
<div class="action">
<gr-button
@@ -159,7 +159,7 @@
</template>
<template
is="dom-if"
- if="[[_computeShowActionRemoveFromAttentionSet(_config, highlightAttention, account, change)]]"
+ if="[[_computeShowActionRemoveFromAttentionSet(_config, highlightAttention, account, change, _selfAccount)]]"
>
<div class="action">
<gr-button
@@ -172,7 +172,10 @@
</gr-button>
</div>
</template>
- <template is="dom-if" if="[[_showReviewerOrCCActions(account, change)]]">
+ <template
+ is="dom-if"
+ if="[[_showReviewerOrCCActions(account, change, _selfAccount)]]"
+ >
<div class="action">
<gr-button
class="removeReviewerOrCC"
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 c8b4b42..8d1f499 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
@@ -19,6 +19,7 @@
import './gr-hovercard-account.js';
import {html} from '@polymer/polymer/lib/utils/html-tag.js';
import {ReviewerState} from '../../../constants/constants.js';
+import {appContext} from '../../../services/app-context.js';
const basicFixture = fixtureFromTemplate(html`
<gr-hovercard-account class="hovered"></gr-hovercard-account>
@@ -34,22 +35,22 @@
_account_id: '31415926535',
};
- setup(() => {
- element = basicFixture.instantiate();
- sinon.stub(element.restApiService, 'getAccount').returns(
- new Promise(resolve => { '2'; })
+ setup(async () => {
+ sinon.stub(appContext.restApiService, 'getAccount').returns(
+ Promise.resolve({...ACCOUNT})
);
-
- element._selfAccount = {...ACCOUNT};
+ sinon.stub(appContext.restApiService, 'getConfig').returns(
+ Promise.resolve({change: {enable_attention_set: true}})
+ );
+ element = basicFixture.instantiate();
element.account = {...ACCOUNT};
- element._config = {
- change: {enable_attention_set: true},
- };
element.change = {
attention_set: {},
+ reviewers: {},
+ owner: {...ACCOUNT},
};
element.show({});
- flush();
+ await flush();
});
teardown(() => {
@@ -242,7 +243,11 @@
sinon.stub(element.restApiService, 'removeFromAttentionSet')
.callsFake(() => apiPromise);
element.highlightAttention = true;
- element.change = {attention_set: {31415926535: {}}};
+ element.change = {
+ attention_set: {31415926535: {}},
+ reviewers: {},
+ owner: {...ACCOUNT},
+ };
element._target = document.createElement('div');
flush();
const showAlertListener = sinon.spy();
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 47924e6..ff9fff8 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -177,6 +177,30 @@
return change.owner?._account_id === account._account_id;
}
+export function isReviewer(change?: ChangeInfo, account?: AccountInfo) {
+ if (!change || !account) return false;
+ const reviewers = change.reviewers.REVIEWER ?? [];
+ return reviewers.some(r => r._account_id === account._account_id);
+}
+
+export function isUploader(change?: ChangeInfo, account?: AccountInfo) {
+ if (!change || !account) return false;
+ const rev = getCurrentRevision(change);
+ return rev?.uploader._account_id === account._account_id;
+}
+
+export function isInvolved(change?: ChangeInfo, account?: AccountInfo) {
+ const owner = isOwner(change, account);
+ const uploader = isUploader(change, account);
+ const reviewer = isReviewer(change, account);
+ return owner || uploader || reviewer;
+}
+
+export function getCurrentRevision(change?: ChangeInfo) {
+ if (!change?.revisions || !change?.current_revision) return undefined;
+ return change.revisions[change.current_revision];
+}
+
export function changeStatusString(change: ChangeInfo) {
return changeStatuses(change).join(', ');
}
