Fix gr-change-list-action-bar showing to logged out users
Release-Notes: none
Change-Id: I6e5a9fcd91e19ba161f985ba3606a40d20889f72
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
index 342b876..19207bc 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
@@ -42,6 +42,7 @@
import {classMap} from 'lit/directives/class-map.js';
import {createSearchUrl} from '../../../models/views/search';
import {createChangeUrl} from '../../../models/views/change';
+import {userModelToken} from '../../../models/user/user-model';
import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
enum ChangeSize {
@@ -94,9 +95,6 @@
sectionName?: string;
@property({type: Boolean})
- showStar = false;
-
- @property({type: Boolean})
showNumber = false;
@property({type: String})
@@ -125,6 +123,10 @@
private readonly getNavigation = resolve(this, navigationToken);
+ private readonly getUserModel = resolve(this, userModelToken);
+
+ @state() private isLoggedIn = false;
+
constructor() {
super();
subscribe(
@@ -134,6 +136,11 @@
this.updateCheckedState(selectedChangeNums);
}
);
+ subscribe(
+ this,
+ () => this.getUserModel().loggedIn$,
+ isLoggedIn => (this.isLoggedIn = isLoggedIn)
+ );
}
override connectedCallback() {
@@ -332,6 +339,8 @@
}
private renderCellSelectionBox() {
+ if (!this.isLoggedIn) return;
+
return html`
<td class="cell selection">
<!--
@@ -352,7 +361,7 @@
}
private renderCellStar() {
- if (!this.showStar) return;
+ if (!this.isLoggedIn) return;
return html`
<td class="cell star">
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
index c7cb5b8..5e31cc8 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.ts
@@ -8,9 +8,11 @@
import {
SubmitRequirementResultInfo,
NumericChangeId,
+ Timestamp,
} from '../../../api/rest-api';
import '../../../test/common-test-setup';
import {
+ createAccountWithEmail,
createAccountWithId,
createChange,
createSubmitRequirementExpressionInfo,
@@ -21,7 +23,6 @@
import {
query,
queryAndAssert,
- stubRestApi,
waitUntilObserved,
} from '../../../test/test-utils';
import {
@@ -43,6 +44,7 @@
bulkActionsModelToken,
BulkActionsModel,
} from '../../../models/bulk-actions/bulk-actions-model';
+import {UserModel, userModelToken} from '../../../models/user/user-model';
import {createTestAppContext} from '../../../test/test-app-context-init';
import {ColumnNames} from '../../../constants/constants';
import {testResolver} from '../../../test/common-test-setup';
@@ -58,13 +60,13 @@
let element: GrChangeListItem;
let bulkActionsModel: BulkActionsModel;
+ let userModel: UserModel;
setup(async () => {
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
-
bulkActionsModel = new BulkActionsModel(
createTestAppContext().restApiService
);
+ userModel = testResolver(userModelToken);
element = (
await fixture<DIProviderElement>(
wrapInProvider(
@@ -105,6 +107,10 @@
test('bulk actions checkboxes', async () => {
element.change = {...createChange(), _number: 1 as NumericChangeId};
bulkActionsModel.sync([element.change]);
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
await element.updateComplete;
const checkbox = queryAndAssert<HTMLInputElement>(
@@ -134,6 +140,10 @@
element.globalIndex = 5;
element.change = {...createChange(), _number: 1 as NumericChangeId};
bulkActionsModel.sync([element.change]);
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
await element.updateComplete;
const checkbox = queryAndAssert<HTMLInputElement>(
@@ -147,6 +157,10 @@
});
test('checkbox state updates with model updates', async () => {
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
element.requestUpdate();
await element.updateComplete;
@@ -168,6 +182,10 @@
});
test('checkbox state updates with change id update', async () => {
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
element.requestUpdate();
await element.updateComplete;
@@ -361,7 +379,10 @@
const change = createChange();
bulkActionsModel.sync([change]);
bulkActionsModel.addSelectedChangeNum(change._number);
- element.showStar = true;
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
element.showNumber = true;
element.account = createAccountWithId(1);
element.config = createServerInfo();
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts
index 8227e11..61b276e 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section.ts
@@ -15,14 +15,15 @@
import {sharedStyles} from '../../../styles/shared-styles';
import {Metadata} from '../../../utils/change-metadata-util';
import {WAITING} from '../../../constants/constants';
-import {provide} from '../../../models/dependency';
+import {provide, resolve} from '../../../models/dependency';
import {
bulkActionsModelToken,
BulkActionsModel,
} from '../../../models/bulk-actions/bulk-actions-model';
+import {createSearchUrl} from '../../../models/views/search';
+import {userModelToken} from '../../../models/user/user-model';
import {subscribe} from '../../lit/subscription-controller';
import {classMap} from 'lit/directives/class-map.js';
-import {createSearchUrl} from '../../../models/views/search';
const NUMBER_FIXED_COLUMNS = 4;
const LABEL_PREFIX_INVALID_PROLOG = 'Invalid-Prolog-Rules-Label-Name--';
@@ -52,9 +53,6 @@
visibleChangeTableColumns?: string[];
@property({type: Boolean})
- showStar = false;
-
- @property({type: Boolean})
showNumber?: boolean; // No default value to prevent flickering.
@property({type: Number})
@@ -104,6 +102,10 @@
getAppContext().restApiService
);
+ private readonly getUserModel = resolve(this, userModelToken);
+
+ private isLoggedIn = false;
+
static override get styles() {
return [
changeListStyles,
@@ -156,6 +158,11 @@
() => this.bulkActionsModel.totalChangeCount$,
totalChangeCount => (this.totalChangeCount = totalChangeCount)
);
+ subscribe(
+ this,
+ () => this.getUserModel().loggedIn$,
+ isLoggedIn => (this.isLoggedIn = isLoggedIn)
+ );
}
override willUpdate(changedProperties: PropertyValues) {
@@ -189,8 +196,8 @@
<td class="leftPadding" aria-hidden="true"></td>
<td
class="star"
- ?aria-hidden=${!this.showStar}
- ?hidden=${!this.showStar}
+ ?aria-hidden=${!this.isLoggedIn}
+ ?hidden=${!this.isLoggedIn}
></td>
<td class="cell" colspan=${colSpan}>
${this.changeSection.emptyStateSlotName
@@ -213,7 +220,7 @@
<tbody>
<tr class="groupHeader">
<td aria-hidden="true" class="leftPadding"></td>
- <td aria-hidden="true" class="star" ?hidden=${!this.showStar}></td>
+ <td aria-hidden="true" class="star" ?hidden=${!this.isLoggedIn}></td>
<td class="cell" colspan=${colSpan}>
<h2 class="heading-3">
<a
@@ -248,7 +255,7 @@
: html` <td
class="star"
aria-label="Star status column"
- ?hidden=${!this.showStar}
+ ?hidden=${!this.isLoggedIn}
></td>
<td class="number" ?hidden=${!this.showNumber}>#</td>
${columns.map(item => this.renderHeaderCell(item))}
@@ -267,7 +274,7 @@
const indeterminate =
this.numSelected > 0 && this.numSelected !== this.totalChangeCount;
return html`
- <td class="selection">
+ <td class="selection" ?hidden=${!this.isLoggedIn}>
<!--
The .checked property must be used rather than the attribute because
the attribute only controls the default checked state and does not
@@ -322,7 +329,6 @@
.sectionName=${this.changeSection.name}
.visibleChangeTableColumns=${columns}
.showNumber=${this.showNumber}
- ?showStar=${this.showStar}
.usp=${this.usp}
.labelNames=${this.labelNames}
.globalIndex=${this.startIndex + index}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts
index 8dfecdc..63552c7 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-section/gr-change-list-section_test.ts
@@ -13,9 +13,10 @@
import {
createChange,
createAccountDetailWithId,
+ createAccountWithEmail,
createServerInfo,
} from '../../../test/test-data-generators';
-import {NumericChangeId, ChangeInfoId} from '../../../api/rest-api';
+import {ChangeInfoId, NumericChangeId, Timestamp} from '../../../api/rest-api';
import {
queryAll,
query,
@@ -27,11 +28,15 @@
import {ChangeListSection} from '../gr-change-list/gr-change-list';
import {fixture, html, assert} from '@open-wc/testing';
import {ColumnNames} from '../../../constants/constants';
+import {testResolver} from '../../../test/common-test-setup';
+import {UserModel, userModelToken} from '../../../models/user/user-model';
suite('gr-change-list section', () => {
let element: GrChangeListSection;
+ let userModel: UserModel;
setup(async () => {
+ userModel = testResolver(userModelToken);
const changeSection: ChangeListSection = {
name: 'test',
query: 'test',
@@ -193,6 +198,10 @@
],
emptyStateSlotName: 'test',
};
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
await element.updateComplete;
let rows = queryAll(element, 'gr-change-list-item');
assert.lengthOf(rows, 2);
@@ -235,6 +244,10 @@
],
emptyStateSlotName: 'test',
};
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
await element.updateComplete;
const rows = queryAll(element, 'gr-change-list-item');
@@ -273,6 +286,31 @@
});
});
+ test('no checkbox when logged out', async () => {
+ element.changeSection = {
+ name: 'test',
+ query: 'test',
+ results: [
+ {
+ ...createChange(),
+ _number: 1 as NumericChangeId,
+ id: '1' as ChangeInfoId,
+ },
+ {
+ ...createChange(),
+ _number: 2 as NumericChangeId,
+ id: '2' as ChangeInfoId,
+ },
+ ],
+ emptyStateSlotName: 'test',
+ };
+ userModel.setAccount(undefined);
+ await element.updateComplete;
+ const rows = queryAll(element, 'gr-change-list-item');
+ assert.lengthOf(rows, 2);
+ assert.isUndefined(query<HTMLInputElement>(rows[0], 'input'));
+ });
+
test('colspans', async () => {
element.visibleChangeTableColumns = [];
element.changeSection = {results: [{...createChange()}]};
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
index d2ba2c9..1c86354 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-view/gr-change-list-view.ts
@@ -195,7 +195,6 @@
.account=${this.account}
.changes=${this.changes}
.preferences=${this.preferences}
- .showStar=${this.loggedIn}
@toggle-star=${(e: CustomEvent<ChangeStarToggleStarDetail>) => {
this.handleToggleStar(e);
}}
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 4c43da5..748c2b8 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
@@ -113,9 +113,6 @@
showNumber?: boolean; // No default value to prevent flickering.
@property({type: Boolean})
- showStar = false;
-
- @property({type: Boolean})
showReviewedState = false;
@property({type: Array})
@@ -270,7 +267,6 @@
sectionIndex,
this.sections
)}
- ?showStar=${this.showStar}
.showNumber=${this.showNumber}
.visibleChangeTableColumns=${this.visibleChangeTableColumns}
.usp=${this.usp}
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
index bef3166..7e9735f 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.ts
@@ -23,6 +23,7 @@
} from '../../../constants/constants';
import {AccountId, NumericChangeId} from '../../../types/common';
import {
+ createAccountWithEmail,
createChange,
createServerInfo,
createSubmitRequirementResultInfo,
@@ -32,12 +33,16 @@
import {fixture, assert} from '@open-wc/testing';
import {html} from 'lit';
import {testResolver} from '../../../test/common-test-setup';
+import {Timestamp} from '../../../api/rest-api';
+import {UserModel, userModelToken} from '../../../models/user/user-model';
suite('gr-change-list basic tests', () => {
let element: GrChangeList;
+ let userModel: UserModel;
setup(async () => {
element = await fixture(html`<gr-change-list></gr-change-list>`);
+ userModel = testResolver(userModelToken);
});
test('renders', async () => {
@@ -285,6 +290,12 @@
});
test('toggle checkbox keyboard shortcut', async () => {
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
+ await element.updateComplete;
+
const getCheckbox = (item: GrChangeListItem) =>
queryAndAssert<HTMLInputElement>(query(item, '.selection'), 'input');
@@ -515,7 +526,7 @@
assert.isTrue(element.isColumnEnabled('Subject'));
});
- test('showStar and showNumber', async () => {
+ test('loggedIn and showNumber', async () => {
element.sections = [{results: [{...createChange()}], name: 'a'}];
element.account = {_account_id: 1001 as AccountId};
element.preferences = {
@@ -534,6 +545,7 @@
],
};
element.config = createServerInfo();
+ userModel.setAccount(undefined);
await element.updateComplete;
const section = query<GrChangeListSection>(
element,
@@ -547,7 +559,10 @@
assert.isNotOk(query(query(section, 'gr-change-list-item'), '.star'));
assert.isNotOk(query(query(section, 'gr-change-list-item'), '.number'));
- element.showStar = true;
+ userModel.setAccount({
+ ...createAccountWithEmail('abc@def.com'),
+ registered_on: '2015-03-12 18:32:08.000000000' as Timestamp,
+ });
await element.updateComplete;
await section.updateComplete;
assert.isOk(query(query(section, 'gr-change-list-item'), '.star'));
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
index d013654..cd30440 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view.ts
@@ -284,7 +284,6 @@
${this.renderUserHeader()}
<h1 class="assistive-tech-only">Dashboard</h1>
<gr-change-list
- ?showStar=${true}
.account=${this.account}
.preferences=${this.preferences}
.sections=${this.results}
diff --git a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.ts b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.ts
index 17d7e95..84a3139 100644
--- a/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-dashboard-view/gr-dashboard-view_test.ts
@@ -82,7 +82,7 @@
<div class="loading" hidden="">Loading...</div>
<div>
<h1 class="assistive-tech-only">Dashboard</h1>
- <gr-change-list showstar="">
+ <gr-change-list>
<div id="emptyOutgoing" slot="outgoing-slot">No changes</div>
<div id="emptyYourTurn" slot="your-turn-slot">
<span> No changes need your attention  ðŸŽ‰ </span>