Add status icon to the `Owned Files` tab header
When files owner opens a change screen status icon immediately indicates
the following status of owned files:
* they need a review with orange clock icon and the following tooltip
when hovered
Missing approval for X file[s].
* they are already approved (also by someone else) with green checked
icon and the following tooltip when hovered
X file[s] already approved.
New functionality was covered in the unit tests.
Bug: Issue 383596444
Change-Id: I1cf4fccf692d966e3658b42aa153a465242a7b09
diff --git a/owners/web/gr-owned-files.ts b/owners/web/gr-owned-files.ts
index 4d27e03..13398d0 100644
--- a/owners/web/gr-owned-files.ts
+++ b/owners/web/gr-owned-files.ts
@@ -42,13 +42,29 @@
truncatePath,
} from './utils';
+const STATUS_CODE = {
+ MISSING: 'missing',
+ APPROVED: 'approved',
+};
+
+const STATUS_ICON = {
+ [STATUS_CODE.MISSING]: 'schedule',
+ [STATUS_CODE.APPROVED]: 'check',
+};
+
+export interface OwnedFilesInfo {
+ ownedFiles: string[];
+ numberOfPending: number;
+ numberOfApproved: number;
+}
+
const common = OwnersMixin(LitElement);
class OwnedFilesCommon extends common {
@property({type: Object})
revision?: RevisionInfo;
- protected ownedFiles?: string[];
+ protected ownedFilesInfo?: OwnedFilesInfo;
protected override willUpdate(changedProperties: PropertyValues): void {
super.willUpdate(changedProperties);
@@ -58,7 +74,7 @@
this.change,
this.revision,
this.user,
- this.ownedFiles
+ this.ownedFilesInfo?.ownedFiles
);
}
@@ -67,13 +83,18 @@
}
private computeOwnedFiles() {
- this.ownedFiles = ownedFiles(this.user?.account, this.filesOwners);
+ this.ownedFilesInfo = ownedFiles(this.user?.account, this.filesOwners);
}
}
export const OWNED_FILES_TAB_HEADER = 'owned-files-tab-header';
@customElement(OWNED_FILES_TAB_HEADER)
export class OwnedFilesTabHeader extends OwnedFilesCommon {
+ @property({type: String, reflect: true, attribute: 'files-status'})
+ filesStatus?: string;
+
+ private ownedFiles: number | undefined;
+
static override get styles() {
return [
...OwnedFilesCommon.commonStyles(),
@@ -81,10 +102,34 @@
[hidden] {
display: none;
}
+ gr-icon {
+ padding: var(--spacing-xs) 0px;
+ margin-left: 3px;
+ }
+ :host([files-status='approved']) gr-icon.status {
+ color: var(--positive-green-text-color);
+ }
+ :host([files-status='missing']) gr-icon.status {
+ color: #ffa62f;
+ }
`,
];
}
+ protected override willUpdate(changedProperties: PropertyValues): void {
+ super.willUpdate(changedProperties);
+
+ if (this.hidden || this.ownedFilesInfo === undefined) {
+ this.filesStatus = undefined;
+ this.ownedFiles = undefined;
+ } else {
+ [this.filesStatus, this.ownedFiles] =
+ this.ownedFilesInfo.numberOfApproved > 0
+ ? [STATUS_CODE.APPROVED, this.ownedFilesInfo.numberOfApproved]
+ : [STATUS_CODE.MISSING, this.ownedFilesInfo.numberOfPending];
+ }
+ }
+
override render() {
// even if `nothing` is returned Gerrit still shows the pointer and allows
// clicking at it, redirecting to the empty tab when done; traverse through
@@ -105,7 +150,55 @@
if (tabParent && tabParent.getAttribute('disabled')) {
tabParent.removeAttribute('disabled');
}
- return html`<div>Owned Files</div>`;
+
+ if (
+ this.ownedFilesInfo === undefined ||
+ this.filesStatus === undefined ||
+ this.ownedFiles === undefined
+ ) {
+ return html`<div>Owned Files</div>`;
+ }
+
+ const icon = STATUS_ICON[this.filesStatus];
+ const [info, summary] = this.buildInfoAndSummary(
+ this.filesStatus,
+ this.ownedFilesInfo.numberOfApproved,
+ this.ownedFilesInfo.numberOfPending
+ );
+ return html` <div>
+ Owned Files
+ <gr-tooltip-content
+ title=${info}
+ aria-label=${summary}
+ aria-description=${info}
+ has-tooltip
+ >
+ <gr-icon
+ id="owned-files-status"
+ class="status"
+ icon=${icon}
+ aria-hidden="true"
+ ></gr-icon>
+ </gr-tooltip-content>
+ </div>`;
+ }
+
+ private buildInfoAndSummary(
+ filesStatus: string,
+ filesApproved: number,
+ filesPending: number
+ ): [string, string] {
+ const count = filesApproved + filesPending;
+ const plural = count === 1 ? '' : 's';
+ const info = `${
+ STATUS_CODE.APPROVED === filesStatus
+ ? `${count} file${plural} already approved.`
+ : `Missing approval for ${count} file${plural}.`
+ }`;
+ const summary = `${
+ STATUS_CODE.APPROVED === filesStatus ? 'Approved' : 'Missing'
+ }`;
+ return [info, summary];
}
}
@@ -278,7 +371,7 @@
id="ownedFilesList"
.change=${this.change}
.revision=${this.revision}
- .ownedFiles=${this.ownedFiles}
+ .ownedFiles=${this.ownedFilesInfo?.ownedFiles}
>
</gr-owned-files-list>
`;
@@ -319,7 +412,7 @@
groupPrefix: string,
files: OwnedFiles,
emailWithoutDomain?: string
-) {
+): string[] {
const ownedFiles = [];
for (const file of Object.keys(files ?? [])) {
if (
@@ -345,7 +438,7 @@
export function ownedFiles(
owner?: AccountInfo,
filesOwners?: FilesOwners
-): string[] | undefined {
+): OwnedFilesInfo | undefined {
if (
!owner ||
!filesOwners ||
@@ -356,20 +449,23 @@
const groupPrefix = 'group/';
const emailWithoutDomain = toEmailWithoutDomain(owner.email);
- return [
- ...collectOwnedFiles(
- owner,
- groupPrefix,
- filesOwners.files,
- emailWithoutDomain
- ),
- ...collectOwnedFiles(
- owner,
- groupPrefix,
- filesOwners.files_approved,
- emailWithoutDomain
- ),
- ];
+ const pendingFiles = collectOwnedFiles(
+ owner,
+ groupPrefix,
+ filesOwners.files,
+ emailWithoutDomain
+ );
+ const approvedFiles = collectOwnedFiles(
+ owner,
+ groupPrefix,
+ filesOwners.files_approved,
+ emailWithoutDomain
+ );
+ return {
+ ownedFiles: [...pendingFiles, ...approvedFiles],
+ numberOfPending: pendingFiles.length,
+ numberOfApproved: approvedFiles.length,
+ } as OwnedFilesInfo;
}
export function computeDiffUrl(
diff --git a/owners/web/gr-owned-files_test.ts b/owners/web/gr-owned-files_test.ts
index af9f814..d7bef24 100644
--- a/owners/web/gr-owned-files_test.ts
+++ b/owners/web/gr-owned-files_test.ts
@@ -26,7 +26,7 @@
EDIT,
SubmitRequirementResultInfo,
} from '@gerritcodereview/typescript-api/rest-api';
-import {ownedFiles, shouldHide} from './gr-owned-files';
+import {ownedFiles, OwnedFilesInfo, shouldHide} from './gr-owned-files';
import {FilesOwners, GroupOwner, Owner} from './owners-service';
import {deepEqual} from './utils';
import {User, UserRole} from './owners-model';
@@ -50,6 +50,11 @@
files,
files_approved,
} as unknown as FilesOwners;
+ const emptyOwnerFilesInfo = {
+ ownedFiles: [],
+ numberOfApproved: 0,
+ numberOfPending: 0,
+ } as unknown as OwnedFilesInfo;
test('ownedFiles - should be `undefined` when owner is `undefined`', () => {
const undefinedOwner = undefined;
@@ -63,15 +68,19 @@
test('ownedFiles - should return empty owned file when no files are owned by user', () => {
const user = account(2);
- assert.equal(deepEqual(ownedFiles(user, filesOwners), []), true);
+ assert.equal(
+ deepEqual(ownedFiles(user, filesOwners), emptyOwnerFilesInfo),
+ true
+ );
});
test('ownedFiles - should return owned files', () => {
assert.equal(
- deepEqual(ownedFiles(owner, filesOwners), [
- ownedFile,
- ownedApprovedFile,
- ]),
+ deepEqual(ownedFiles(owner, filesOwners), {
+ ownedFiles: [ownedFile, ownedApprovedFile],
+ numberOfApproved: 1,
+ numberOfPending: 1,
+ }),
true
);
});
@@ -84,7 +93,11 @@
},
} as unknown as FilesOwners;
assert.equal(
- deepEqual(ownedFiles(owner, filesOwners), [ownedFile]),
+ deepEqual(ownedFiles(owner, filesOwners), {
+ ownedFiles: [ownedFile],
+ numberOfApproved: 0,
+ numberOfPending: 1,
+ }),
true
);
});
@@ -97,14 +110,21 @@
},
} as unknown as FilesOwners;
assert.equal(
- deepEqual(ownedFiles(owner, filesOwners), [ownedFile]),
+ deepEqual(ownedFiles(owner, filesOwners), {
+ ownedFiles: [ownedFile],
+ numberOfApproved: 1,
+ numberOfPending: 0,
+ }),
true
);
});
test('ownedFiles - should NOT match file owner over email without domain or full name when account id is different', () => {
const notFileOwner = {...owner, _account_id: 2 as unknown as AccountId};
- assert.equal(deepEqual(ownedFiles(notFileOwner, filesOwners), []), true);
+ assert.equal(
+ deepEqual(ownedFiles(notFileOwner, filesOwners), emptyOwnerFilesInfo),
+ true
+ );
});
});
