commit 027446d46e9266b54a9a16f6ffcaf4b8009c16a2 Author: Ben Rohlfs Date: Fri Oct 2 11:03:46 2020 +0200 Add an attention-set-util Change-Id: I41c5574358ddeab200a356f8d45677773621ac35 diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts index 974238f..f486435 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts @@ -52,7 +52,10 @@ import { ServerInfo, PreferencesInput, } from '../../../types/common'; -import {hasAttention, isAttentionSetEnabled} from '../../../utils/account-util'; +import { + hasAttention, + isAttentionSetEnabled, +} from '../../../utils/attention-set-util'; const NUMBER_FIXED_COLUMNS = 3; const CLOSED_STATUS = ['MERGED', 'ABANDONED']; @@ -358,10 +361,8 @@ export class GrChangeList extends ChangeTableMixin( showReviewedState: boolean, config?: ServerInfo ) { - const isAttentionSetEnabled = - !!config && !!config.change && config.change.enable_attention_set; return ( - !isAttentionSetEnabled && + !isAttentionSetEnabled(config) && showReviewedState && !change.reviewed && !change.work_in_progress && diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts index d774607..6ce0ceb 100644 --- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts +++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts @@ -107,6 +107,7 @@ import {GrTextarea} from '../../shared/gr-textarea/gr-textarea'; import {GrAccountChip} from '../../shared/gr-account-chip/gr-account-chip'; import {GrOverlay} from '../../shared/gr-overlay/gr-overlay'; import {GrStorage, StorageLocation} from '../../shared/gr-storage/gr-storage'; +import {isAttentionSetEnabled} from '../../../utils/attention-set-util'; const STORAGE_DEBOUNCE_INTERVAL_MS = 400; @@ -633,7 +634,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin( reviewInput.ready = true; } - if (this._isAttentionSetEnabled(this.serverConfig)) { + if (isAttentionSetEnabled(this.serverConfig)) { const selfName = getDisplayName(this.serverConfig, this._account); const reason = `${selfName} replied on the change`; @@ -886,15 +887,11 @@ export class GrReplyDialog extends KeyboardShortcutMixin( } _showAttentionSummary(config?: ServerInfo, attentionExpanded?: boolean) { - return this._isAttentionSetEnabled(config) && !attentionExpanded; + return isAttentionSetEnabled(config) && !attentionExpanded; } _showAttentionDetails(config?: ServerInfo, attentionExpanded?: boolean) { - return this._isAttentionSetEnabled(config) && attentionExpanded; - } - - _isAttentionSetEnabled(config?: ServerInfo) { - return !!config && !!config.change && config.change.enable_attention_set; + return isAttentionSetEnabled(config) && attentionExpanded; } _handleAttentionClick(e: Event) { 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 70e7ba7..e8ceb56 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 @@ -23,7 +23,8 @@ import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-l import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin'; import {PolymerElement} from '@polymer/polymer/polymer-element'; import {htmlTemplate} from './gr-reviewer-list_html'; -import {hasAttention, isServiceUser} from '../../../utils/account-util'; +import {isServiceUser} from '../../../utils/account-util'; +import {hasAttention} from '../../../utils/attention-set-util'; import {customElement, property, computed, observe} from '@polymer/decorators'; import { ChangeInfo, 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 b3a53c5..35fa015 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,13 +26,18 @@ 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 {isServiceUser} 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 {ReportingService} from '../../../services/gr-reporting/gr-reporting'; -import {hasOwnProperty} from '../../../utils/common-util'; +import { + canHaveAttention, + getLastUpdate, + getReason, + hasAttention, + isAttentionSetEnabled, +} from '../../../utils/attention-set-util'; export interface GrHovercardAccount { $: { @@ -101,56 +106,37 @@ export class GrHovercardAccount extends GestureEventListeners( return account._account_id === selfAccount._account_id ? 'Your' : 'Their'; } - get isAttentionSetEnabled() { + get isAttentionEnabled() { return ( - !!this._config && - !!this._config.change && - !!this._config.change.enable_attention_set && + isAttentionSetEnabled(this._config) && !!this.highlightAttention && !!this.change && - !!this.account && - !isServiceUser(this.account) + canHaveAttention(this.account) ); } - get hasAttention() { - if ( - !this.isAttentionSetEnabled || - !this.change?.attention_set || - !this.account._account_id - ) - return false; - return hasOwnProperty(this.change.attention_set, this.account._account_id); + get hasUserAttention() { + return hasAttention(this._config, this.account, this.change); } _computeReason(change?: ChangeInfo) { - if (!change || !change.attention_set || !this.account._account_id) { - return ''; - } - const entry = change.attention_set[this.account._account_id]; - if (!entry || !entry.reason) return ''; - return entry.reason; + return getReason(this.account, change); } _computeLastUpdate(change?: ChangeInfo) { - if (!change || !change.attention_set || !this.account._account_id) { - return ''; - } - const entry = change.attention_set[this.account._account_id]; - if (!entry || !entry.last_update) return ''; - return entry.last_update; + return getLastUpdate(this.account, change); } _computeShowLabelNeedsAttention() { - return this.isAttentionSetEnabled && this.hasAttention; + return this.isAttentionEnabled && this.hasUserAttention; } _computeShowActionAddToAttentionSet() { - return this.isAttentionSetEnabled && !this.hasAttention; + return this.isAttentionEnabled && !this.hasUserAttention; } _computeShowActionRemoveFromAttentionSet() { - return this.isAttentionSetEnabled && this.hasAttention; + return this.isAttentionEnabled && this.hasUserAttention; } _handleClickAddToAttentionSet() { 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 f272507..6fd6c51 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 @@ -59,17 +59,6 @@ suite('gr-hovercard-account tests', () => { 'Kermit The Frog'); }); - test('_computeReason', () => { - const change = { - attention_set: { - 31415926535: { - reason: 'a good reason', - }, - }, - }; - assert.equal(element._computeReason(change), 'a good reason'); - }); - test('_computeLastUpdate', () => { const last_update = '2019-07-17 19:39:02.000000000'; const change = { diff --git a/polygerrit-ui/app/utils/account-util.ts b/polygerrit-ui/app/utils/account-util.ts index 9ad5648..7e425f8 100644 --- a/polygerrit-ui/app/utils/account-util.ts +++ b/polygerrit-ui/app/utils/account-util.ts @@ -15,13 +15,7 @@ * limitations under the License. */ -import { - AccountId, - AccountInfo, - ChangeInfo, - EmailAddress, - ServerInfo, -} from '../types/common'; +import {AccountId, AccountInfo, EmailAddress} from '../types/common'; import {AccountTag} from '../constants/constants'; export function accountKey(account: AccountInfo): AccountId | EmailAddress { @@ -37,23 +31,3 @@ export function isServiceUser(account?: AccountInfo): boolean { export function removeServiceUsers(accounts?: AccountInfo[]): AccountInfo[] { return accounts?.filter(a => !isServiceUser(a)) || []; } - -export function isAttentionSetEnabled(config?: ServerInfo): boolean { - return !!config?.change?.enable_attention_set; -} - -export function canHaveAttention(account?: AccountInfo): boolean { - return !!account && !!account._account_id && !isServiceUser(account); -} - -export function hasAttention( - config?: ServerInfo, - account?: AccountInfo, - change?: ChangeInfo -): boolean { - return ( - isAttentionSetEnabled(config) && - canHaveAttention(account) && - !!change?.attention_set?.hasOwnProperty(account?._account_id!) - ); -} diff --git a/polygerrit-ui/app/utils/attention-set-util.ts b/polygerrit-ui/app/utils/attention-set-util.ts new file mode 100644 index 0000000..b0aefcb --- /dev/null +++ b/polygerrit-ui/app/utils/attention-set-util.ts @@ -0,0 +1,63 @@ +/** + * @license + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import {AccountInfo, ChangeInfo} from '../types/common'; +import {isServiceUser} from './account-util'; + +// You would typically use a ServerInfo here, but this utility does not care +// about all the other parameters in that object. +interface SimpleServerInfo { + change?: { + enable_attention_set?: boolean; + }; +} + +const CONFIG_ENABLED: SimpleServerInfo = { + change: {enable_attention_set: true}, +}; + +export function isAttentionSetEnabled(config?: SimpleServerInfo): boolean { + return !!config?.change?.enable_attention_set; +} + +export function canHaveAttention(account?: AccountInfo): boolean { + return !!account?._account_id && !isServiceUser(account); +} + +export function hasAttention( + config?: SimpleServerInfo, + account?: AccountInfo, + change?: ChangeInfo +): boolean { + return ( + isAttentionSetEnabled(config) && + canHaveAttention(account) && + !!change?.attention_set?.hasOwnProperty(account!._account_id!) + ); +} + +export function getReason(account?: AccountInfo, change?: ChangeInfo) { + if (!hasAttention(CONFIG_ENABLED, account, change)) return ''; + const entry = change!.attention_set![account!._account_id!]; + return entry?.reason ? entry.reason : ''; +} + +export function getLastUpdate(account?: AccountInfo, change?: ChangeInfo) { + if (!hasAttention(CONFIG_ENABLED, account, change)) return ''; + const entry = change!.attention_set![account!._account_id!]; + return entry?.last_update ? entry.last_update : ''; +} diff --git a/polygerrit-ui/app/utils/attention-set-util_test.js b/polygerrit-ui/app/utils/attention-set-util_test.js new file mode 100644 index 0000000..0273a33 --- /dev/null +++ b/polygerrit-ui/app/utils/attention-set-util_test.js @@ -0,0 +1,62 @@ +/** + * @license + * Copyright (C) 2020 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import '../test/common-test-setup-karma.js'; +import { + hasAttention, getReason, +} from './attention-set-util.js'; + +suite('attention-set-util', () => { + test('hasAttention', () => { + const config = { + change: {enable_attention_set: true}, + }; + const change = { + attention_set: { + 31415926535: { + reason: 'a good reason', + }, + }, + }; + const account = { + email: 'kermit@gmail.com', + username: 'kermit', + name: 'Kermit The Frog', + _account_id: '31415926535', + }; + + assert.isTrue(hasAttention(config, account, change)); + }); + + test('getReason', () => { + const change = { + attention_set: { + 31415926535: { + reason: 'a good reason', + }, + }, + }; + const account = { + email: 'kermit@gmail.com', + username: 'kermit', + name: 'Kermit The Frog', + _account_id: '31415926535', + }; + + assert.equal(getReason(account, change), 'a good reason'); + }); +});