Only allow involved users to change the attention set
We want to prevent spammers from interfering with the attention set.
Thus we only allow the owner, uploader and reviewers to change the
attention set state of users on a change.
Change-Id: I4c306140f27e9a221f3051ea649e06ba5684b0bc
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(', ');
}
