Merge branch 'stable-3.9' into stable-3.10 * stable-3.9: Fix IT tests for `GetFilesOwners` Expose files that are approved by owners in REST API Display owned files as links to the diff Introduce `Owned Files` tab to the change screen Reorganize UI components to introduce owned files tab Reuse account details that are part of the change Display `Copy owner's email` button for owner Display file owner's vote value in the File Owners summary Display file owners when a owner's status icon is hovered over Indicate file that needs file owner's review Introduce `allFilesApproved` and `filesOwners` states to plugin UI Add `getAllFilesApproved` and `getFileOwners` functions to service Introduce the skeleton of UI for file owners status display Fix cache invalidation of path_owners_entries Change-Id: Ie035c37061e472c62588b3bd83522fca6c63d5a1
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java index 3af7a58..c882414 100644 --- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java +++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntriesCacheImpl.java
@@ -20,6 +20,7 @@ import com.google.common.cache.LoadingCache; import com.google.common.collect.HashMultimap; import com.google.common.collect.Multimap; +import com.google.gerrit.entities.RefNames; import com.google.inject.Inject; import com.google.inject.Singleton; import com.google.inject.name.Named; @@ -88,6 +89,6 @@ } private String indexKey(String project, String branch) { - return new StringBuilder(project).append('@').append(branch).toString(); + return new StringBuilder(project).append('@').append(RefNames.fullName(branch)).toString(); } }
diff --git a/owners/BUILD b/owners/BUILD index f08ec84..c1dac46 100644 --- a/owners/BUILD +++ b/owners/BUILD
@@ -38,7 +38,9 @@ "Implementation-URL: https://gerrit.googlesource.com/plugins/owners", "Gerrit-PluginName: owners", "Gerrit-Module: com.googlesource.gerrit.owners.OwnersModule", + "Gerrit-HttpModule: com.googlesource.gerrit.owners.HttpModule", ], + resource_jars = ["//plugins/owners/web:gr-owners"], resources = glob(["src/main/resources/**/*"]), deps = [ ":gerrit-owners-predicates",
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/HttpModule.java b/owners/src/main/java/com/googlesource/gerrit/owners/HttpModule.java new file mode 100644 index 0000000..10759c0 --- /dev/null +++ b/owners/src/main/java/com/googlesource/gerrit/owners/HttpModule.java
@@ -0,0 +1,26 @@ +// Copyright (C) 2024 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. +package com.googlesource.gerrit.owners; + +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.extensions.webui.JavaScriptPlugin; +import com.google.gerrit.extensions.webui.WebUiPlugin; +import com.google.inject.servlet.ServletModule; + +public class HttpModule extends ServletModule { + @Override + protected void configureServlets() { + DynamicSet.bind(binder(), WebUiPlugin.class).toInstance(new JavaScriptPlugin("gr-owners.js")); + } +}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java index 30c9c73..3f6c937 100644 --- a/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java +++ b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java
@@ -24,11 +24,15 @@ public final Map<String, Set<GroupOwner>> files; public final Map<Integer, Map<String, Integer>> ownersLabels; + public final Map<String, Set<GroupOwner>> filesApproved; public FilesOwnersResponse( - Map<Integer, Map<String, Integer>> ownersLabels, Map<String, Set<GroupOwner>> files) { + Map<Integer, Map<String, Integer>> ownersLabels, + Map<String, Set<GroupOwner>> files, + Map<String, Set<GroupOwner>> filesApproved) { this.ownersLabels = ownersLabels; this.files = files; + this.filesApproved = filesApproved; } @Override @@ -36,16 +40,25 @@ if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; FilesOwnersResponse that = (FilesOwnersResponse) o; - return Objects.equal(files, that.files) && Objects.equal(ownersLabels, that.ownersLabels); + return Objects.equal(files, that.files) + && Objects.equal(ownersLabels, that.ownersLabels) + && Objects.equal(filesApproved, that.filesApproved); } @Override public int hashCode() { - return Objects.hashCode(files, ownersLabels); + return Objects.hashCode(files, ownersLabels, filesApproved); } @Override public String toString() { - return "FilesOwnersResponse{" + "files=" + files + ", ownersLabels=" + ownersLabels + '}'; + return "FilesOwnersResponse{" + + "files=" + + files + + ", ownersLabels=" + + ownersLabels + + ", filesApproved=" + + filesApproved + + '}'; } }
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java index bbe3007..005e83f 100644 --- a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java +++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -145,7 +145,14 @@ !isApprovedByOwner( fileExpandedOwners.get(fileOwnerEntry.getKey()), ownersLabels, label)); - return Response.ok(new FilesOwnersResponse(ownersLabels, filesWithPendingOwners)); + Map<String, Set<GroupOwner>> filesApprovedByOwners = + Maps.filterEntries( + fileToOwners, + (fileOwnerEntry) -> + isApprovedByOwner( + fileExpandedOwners.get(fileOwnerEntry.getKey()), ownersLabels, label)); + + return Response.ok(new FilesOwnersResponse(ownersLabels, filesWithPendingOwners, filesApprovedByOwners)); } catch (InvalidOwnersFileException e) { logger.atSevere().withCause(e).log("Reading/parsing OWNERS file error."); throw new ResourceConflictException(e.getMessage(), e);
diff --git a/owners/src/main/resources/Documentation/rest-api.md b/owners/src/main/resources/Documentation/rest-api.md index 68fb463..3505d6c 100644 --- a/owners/src/main/resources/Documentation/rest-api.md +++ b/owners/src/main/resources/Documentation/rest-api.md
@@ -1,7 +1,8 @@ # Rest API -The @PLUGIN@ exposes a Rest API endpoint to list the owners associated to each file that -needs a review, and, for each owner, its current labels and votes: +The @PLUGIN@ exposes a Rest API endpoint to list the owners associated with each file that +needs approval (`file` field), is approved (`files_approved`) and, for each owner, +its current labels and votes (`owners_labels`): ```bash GET /changes/{change-id}/revisions/{revision-id}/owners~files-owners @@ -18,6 +19,12 @@ { "name":"Jack", "id": 1000003 } ] }, + "files_approved": { + "NewBuild.build":[ + { "name":"John", "id": 1000004 }, + { "name":"Release Engineer", "id": 1000001 } + ] + }, "owners_labels" : { "1000002": { "Verified": 1,
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java index eba9a68..4593e33 100644 --- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java +++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersITAbstract.java
@@ -41,6 +41,7 @@ import com.googlesource.gerrit.owners.entities.GroupOwner; import com.googlesource.gerrit.owners.entities.Owner; import com.googlesource.gerrit.owners.restapi.GetFilesOwners.LabelNotFoundException; +import java.util.Map; import javax.servlet.http.HttpServletResponse; import org.apache.commons.compress.utils.Sets; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; @@ -98,7 +99,7 @@ } @Test - public void shouldReturnOwnersLabelsWhenNotApprovedByOwners() throws Exception { + public void shouldReturnEmptyOwnersLabelsWhenNotApprovedByOwners() throws Exception { addOwnerFileToRoot(true); String changeId = createChange().getChangeId(); @@ -112,7 +113,21 @@ } @Test - public void shouldReturnEmptyResponseWhenApprovedByOwners() throws Exception { + public void shouldReturnNonEmptyOwnersLabelsWhenApprovedByOwners() throws Exception { + addOwnerFileToRoot(true); + String changeId = createChange().getChangeId(); + approve(changeId); + + Response<FilesOwnersResponse> resp = + assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); + + assertThat(resp.value().ownersLabels) + .containsExactly(admin.id().get(), Map.of(LabelId.CODE_REVIEW, 2)); + } + + @Test + public void shouldReturnEmptyFilesAndNonEmptyFilesApprovedResponseWhenApprovedByOwners() + throws Exception { addOwnerFileToRoot(true); String changeId = createChange().getChangeId(); approve(changeId); @@ -121,6 +136,8 @@ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).isEmpty(); + assertThat(resp.value().filesApproved) + .containsExactly("a.txt", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); } @Test @@ -134,12 +151,14 @@ assertThat(resp.value().files) .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username()))); + assertThat(resp.value().filesApproved).isEmpty(); } @Test @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false") - public void shouldReturnEmptyResponseWhenApprovedByOwnersWithUnexpandedFileOwners() - throws Exception { + public void + shouldReturnEmptyFilesAndNonEmptyFilesApprovedResponseWhenApprovedByOwnersWithUnexpandedFileOwners() + throws Exception { addOwnerFileToRoot(true); String changeId = createChange().getChangeId(); approve(changeId); @@ -148,6 +167,8 @@ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).isEmpty(); + assertThat(resp.value().filesApproved) + .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username()))); } @Test @@ -161,6 +182,7 @@ assertThat(resp.value().files) .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username()))); + assertThat(resp.value().filesApproved).isEmpty(); } @Test @@ -188,6 +210,7 @@ addOwnerFileToProjectConfig(allProjects, true, user); resp = assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(projectOwner)); + assertThat(resp.value().filesApproved).isEmpty(); } @Test @@ -261,6 +284,7 @@ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner)); + assertThat(resp.value().filesApproved).isEmpty(); } private void assertInheritFromProject(Project.NameKey projectNameKey) throws Exception { @@ -273,6 +297,7 @@ assertThat(resp.value().files) .containsExactly("a.txt", Sets.newHashSet(rootOwner, projectOwner)); + assertThat(resp.value().filesApproved).isEmpty(); } private void addBrokenOwnersFileToRoot() throws Exception {
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java index bc57ab2..d45deb5 100644 --- a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java +++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersSubmitRequirementsIT.java
@@ -74,6 +74,7 @@ assertThat(resp.value().files) .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); assertThat(resp.value().ownersLabels).isEmpty(); + assertThat(resp.value().filesApproved).isEmpty(); // give CR+1 as requested recommend(changeId); @@ -82,6 +83,8 @@ assertThat(resp.value().files).isEmpty(); assertThat(resp.value().ownersLabels) .containsExactly(admin.id().get(), Map.of(LabelId.CODE_REVIEW, 1)); + assertThat(resp.value().filesApproved) + .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); } @Test @@ -98,6 +101,7 @@ assertThat(resp.value().files) .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); assertThat(resp.value().ownersLabels).isEmpty(); + assertThat(resp.value().filesApproved).isEmpty(); // give LabelFoo+1 as requested gApi.changes().id(changeId).current().review(new ReviewInput().label(label, 1)); @@ -105,6 +109,8 @@ resp = assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId))); assertThat(resp.value().files).isEmpty(); assertThat(resp.value().ownersLabels).containsEntry(admin.id().get(), Map.of(label, 1)); + assertThat(resp.value().filesApproved) + .containsExactly("foo", Sets.newHashSet(new Owner(admin.fullName(), admin.id().get()))); } private void addOwnerFileToRoot(LabelDefinition label, TestAccount u) throws Exception {
diff --git a/owners/web/.eslintrc.js b/owners/web/.eslintrc.js new file mode 100644 index 0000000..776d84e --- /dev/null +++ b/owners/web/.eslintrc.js
@@ -0,0 +1,20 @@ +/** + * @license + * Copyright (C) 2024 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. + */ +__plugindir = 'owners/web'; +module.exports = { + extends: '../../.eslintrc.js', +}; \ No newline at end of file
diff --git a/owners/web/BUILD b/owners/web/BUILD new file mode 100644 index 0000000..56ab697 --- /dev/null +++ b/owners/web/BUILD
@@ -0,0 +1,68 @@ +load("@npm//@bazel/typescript:index.bzl", "ts_config", "ts_project") +load("//tools/bzl:js.bzl", "gerrit_js_bundle", "karma_test") +load("//tools/js:eslint.bzl", "plugin_eslint") + +package_group( + name = "visibility", + packages = ["//plugins/owners/..."], +) + +package(default_visibility = [":visibility"]) + +ts_config( + name = "tsconfig", + src = "tsconfig.json", + deps = [ + "//plugins:tsconfig-plugins-base.json", + ], +) + +ts_project( + name = "owners-ts", + srcs = glob( + ["**/*.ts"], + exclude = [ + "**/*test*", + "**/test-utils.ts", + ], + ), + incremental = True, + out_dir = "_bazel_ts_out", + tsc = "//tools/node_tools:tsc-bin", + tsconfig = ":tsconfig", + deps = [ + "@plugins_npm//@gerritcodereview/typescript-api", + "@plugins_npm//lit", + "@plugins_npm//rxjs", + ], +) + +ts_project( + name = "owners-ts-tests", + srcs = glob([ + "**/*.ts", + "**/test-utils.ts", + ]), + incremental = True, + out_dir = "_bazel_ts_out_tests", + tsc = "//tools/node_tools:tsc-bin", + tsconfig = ":tsconfig", + deps = [ + "@plugins_npm//:node_modules", + "@ui_dev_npm//:node_modules", + ], +) + +gerrit_js_bundle( + name = "gr-owners", + srcs = [":owners-ts"], + entry_point = "_bazel_ts_out/plugin.js", +) + +karma_test( + name = "karma_test", + srcs = ["karma_test.sh"], + data = [":owners-ts-tests"], +) + +plugin_eslint()
diff --git a/owners/web/gerrit-model.ts b/owners/web/gerrit-model.ts new file mode 100644 index 0000000..da0d42c --- /dev/null +++ b/owners/web/gerrit-model.ts
@@ -0,0 +1,54 @@ +/** + * @license + * Copyright (C) 2024 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} from '@gerritcodereview/typescript-api/rest-api'; + +/** + * A provider for a value. Copied from Gerrit core. + */ +export type Provider<T> = () => T; + +/** + * Partial Gerrit's `AccountsModel` interface that defines functions needed in the owners plugin + */ +export interface AccountsModel { + fillDetails(account: AccountInfo): AccountInfo; +} + +/** + * Parital Gerrit's `GrAccountLabel` interface that provides access to the `AccountsModel` + */ +export interface GrAccountLabel extends Element { + getAccountsModel: Provider<AccountsModel>; +} + +/** + * Partial Gerrit's `SpecialFilePath` enum + */ +export enum SpecialFilePath { + COMMIT_MESSAGE = '/COMMIT_MSG', + MERGE_LIST = '/MERGE_LIST', +} + +/** + * Partial Gerrit's `Window` interface defintion so that `getBaseUrl` function can work. + */ +declare global { + interface Window { + CANONICAL_PATH?: string; + } +}
diff --git a/owners/web/gr-files.ts b/owners/web/gr-files.ts new file mode 100644 index 0000000..1f0c7ef --- /dev/null +++ b/owners/web/gr-files.ts
@@ -0,0 +1,484 @@ +/** + * @license + * Copyright (C) 2024 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 {css, html, LitElement, nothing, PropertyValues, CSSResult} from 'lit'; +import {customElement, property} from 'lit/decorators'; +import { + AccountInfo, + ApprovalInfo, + ChangeInfo, + ChangeStatus, + LabelInfo, + isDetailedLabelInfo, + EmailAddress, + ReviewerState, +} from '@gerritcodereview/typescript-api/rest-api'; +import { + FilesOwners, + isOwner, + OwnersLabels, + OWNERS_SUBMIT_REQUIREMENT, +} from './owners-service'; +import {FileOwnership, FileStatus, PatchRange, UserRole} from './owners-model'; +import {query} from './utils'; +import {GrAccountLabel} from './gerrit-model'; +import {OwnersMixin} from './owners-mixin'; + +const STATUS_CODE = { + MISSING: 'missing', +}; + +const STATUS_ICON = { + [STATUS_CODE.MISSING]: 'schedule', +}; + +const FILE_STATUS = { + [FileStatus.NEEDS_APPROVAL]: STATUS_CODE.MISSING, +}; + +const DISPLAY_OWNERS_FOR_FILE_LIMIT = 5; +const LIMITED_FILES_OWNERS_TOOLTIP = `File owners limited to first ${DISPLAY_OWNERS_FOR_FILE_LIMIT} accounts.`; + +const common = OwnersMixin(LitElement); + +class FilesCommon extends common { + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + + this.hidden = shouldHide( + this.change, + this.patchRange, + this.allFilesApproved, + this.user?.role + ); + } + + protected static commonStyles(): CSSResult[] { + return [window?.Gerrit?.styles.font as CSSResult]; + } +} + +export const FILES_OWNERS_COLUMN_HEADER = 'files-owners-column-header'; +@customElement(FILES_OWNERS_COLUMN_HEADER) +export class FilesColumnHeader extends FilesCommon { + static override get styles() { + return [ + ...FilesCommon.commonStyles(), + css` + :host() { + display: block; + padding-right: var(--spacing-m); + width: 4em; + } + :host[hidden] { + display: none; + } + `, + ]; + } + + override render() { + if (this.hidden) return nothing; + return html`<div>Status</div>`; + } +} + +/** + * It has to be part of this file as components defined in dedicated files are not visible + */ +@customElement('gr-owner') +export class GrOwner extends LitElement { + @property({type: Object}) + owner?: AccountInfo; + + @property({type: Object}) + approval?: ApprovalInfo; + + @property({type: Object}) + info?: LabelInfo; + + @property({type: String}) + email?: EmailAddress; + + static override get styles() { + return [ + css` + .container { + display: flex; + } + gr-vote-chip { + margin-left: 5px; + --gr-vote-chip-width: 14px; + --gr-vote-chip-height: 14px; + } + gr-tooltip-content { + display: inline-block; + } + `, + ]; + } + + override render() { + if (!this.owner) { + return nothing; + } + + const voteChip = this.approval + ? html` <gr-vote-chip + slot="vote-chip" + .vote=${this.approval} + .label=${this.info} + circle-shape + ></gr-vote-chip>` + : nothing; + + const copyEmail = this.email + ? html` <gr-copy-clipboard + .text=${this.email} + hasTooltip + hideinput + buttonTitle=${"Copy owner's email to clipboard"} + ></gr-copy-clipboard>` + : nothing; + + return html` + <div class="container"> + <gr-account-label .account=${this.owner}></gr-account-label> + ${voteChip} ${copyEmail} + </div> + `; + } + + override async updated(changedProperties: PropertyValues) { + super.updated(changedProperties); + + if (changedProperties.has('owner')) { + if (this.owner && !this.email) { + const accountLabel = query<GrAccountLabel>(this, 'gr-account-label'); + if (!accountLabel) { + return; + } + + const updateOwner = await accountLabel + ?.getAccountsModel() + ?.fillDetails(this.owner); + this.email = updateOwner?.email; + } + } + } +} + +export const FILES_OWNERS_COLUMN_CONTENT = 'files-owners-column-content'; +@customElement(FILES_OWNERS_COLUMN_CONTENT) +export class FilesColumnContent extends FilesCommon { + @property({type: String}) + path?: string; + + @property({type: String}) + oldPath?: string; + + @property({type: String, reflect: true, attribute: 'file-status'}) + fileStatus?: string; + + private owners?: AccountInfo[]; + + // taken from Gerrit's common-util.ts + private uniqueId = Math.random().toString(36).substring(2); + + static override get styles() { + return [ + ...FilesCommon.commonStyles(), + css` + :host { + display: flex; + padding-right: var(--spacing-m); + width: 4em; + text-align: center; + } + :host[hidden] { + display: none; + } + gr-icon { + padding: var(--spacing-xs) 0px; + } + :host([file-status='missing']) gr-icon.status { + color: #ffa62f; + } + `, + ]; + } + + override render() { + if (this.hidden || !this.fileStatus) { + return nothing; + } + + const icon = STATUS_ICON[this.fileStatus]; + return html` + <gr-icon + id="${this.pathId()}" + class="status" + icon=${icon} + aria-hidden="true" + ></gr-icon> + ${this.renderFileOwners()} + `; + } + + private pathId(): string { + return `path-${this.uniqueId}`; + } + + private renderFileOwners() { + const owners = this.owners ?? []; + const splicedOwners = owners.splice(0, DISPLAY_OWNERS_FOR_FILE_LIMIT); + const showEllipsis = owners.length > DISPLAY_OWNERS_FOR_FILE_LIMIT; + // inlining <style> here is ugly but an alternative would be to copy the `HovercardMixin` from Gerrit and implement hoover from scratch + return html`<gr-hovercard for="${this.pathId()}"> + <style> + #file-owners-hoovercard { + min-width: 256px; + max-width: 256px; + margin: -10px; + padding: var(--spacing-xl) 0 var(--spacing-m) 0; + } + h3 { + font: inherit; + } + .heading-3 { + margin-left: -2px; + font-family: var(--header-font-family); + font-size: var(--font-size-h3); + font-weight: var(--font-weight-h3); + line-height: var(--line-height-h3); + } + .row { + display: flex; + } + div.section { + margin: 0 var(--spacing-xl) var(--spacing-m) var(--spacing-xl); + display: flex; + align-items: center; + } + div.sectionIcon { + flex: 0 0 30px; + } + div.sectionIcon gr-icon { + position: relative; + } + div.sectionContent .row { + margin-left: 2px; + } + div.sectionContent .notLast { + margin-bottom: 2px; + } + div.sectionContent .ellipsis { + margin-left: 5px; + } + </style> + <div id="file-owners-hoovercard"> + <div class="section"> + <div class="sectionIcon"> + <gr-icon class="status" icon="info" aria-hidden="true"></gr-icon> + </div> + <div class="sectionContent"> + <h3 class="name heading-3"> + <span>Needs Owners' Approval</span> + </h3> + </div> + </div> + <div class="section"> + <div class="sectionContent"> + ${splicedOwners.map((owner, idx) => { + const [approval, info] = + computeApprovalAndInfo( + owner, + this.filesOwners?.owners_labels ?? {}, + this.change + ) ?? []; + return html` + <div + class="row ${showEllipsis || idx + 1 < splicedOwners.length + ? 'notLast' + : ''}" + > + <gr-owner + .owner=${owner} + .approval=${approval} + .info=${info} + .email=${owner.email} + ></gr-owner> + </div> + `; + })} + ${showEllipsis + ? html` + <gr-tooltip-content + title=${LIMITED_FILES_OWNERS_TOOLTIP} + aria-label="limited-file-onwers" + aria-description=${LIMITED_FILES_OWNERS_TOOLTIP} + has-tooltip + > + <div class="row ellipsis">...</div> + </gt-tooltip-content>` + : nothing} + </div> + </div> + </div> + </gr-hovercard>`; + } + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + this.computeFileState(); + } + + private computeFileState(): void { + const fileOwnership = getFileOwnership( + this.path, + this.allFilesApproved, + this.filesOwners + ); + if ( + !fileOwnership || + fileOwnership.fileStatus === FileStatus.NOT_OWNED_OR_APPROVED + ) { + this.fileStatus = undefined; + this.owners = undefined; + return; + } + + this.fileStatus = FILE_STATUS[fileOwnership.fileStatus]; + const accounts = getChangeAccounts(this.change); + + // TODO for the time being filter out or group owners - to be decided what/how to display them + this.owners = (fileOwnership.owners ?? []) + .filter(isOwner) + .map( + o => + accounts.get(o.id) ?? ({_account_id: o.id} as unknown as AccountInfo) + ); + } +} + +export function shouldHide( + change?: ChangeInfo, + patchRange?: PatchRange, + allFilesApproved?: boolean, + userRole?: UserRole +): boolean { + // don't show owners when no change or change is merged + if (change === undefined || patchRange === undefined) { + return true; + } + if ( + change.status === ChangeStatus.ABANDONED || + change.status === ChangeStatus.MERGED + ) { + return true; + } + + // Note: in some special cases, patchNum is undefined on latest patchset + // like after publishing the edit, still show for them + // TODO: this should be fixed in Gerrit + if (patchRange?.patchNum === undefined) return false; + + // only show if its latest patchset + const latestPatchset = change.revisions![change.current_revision!]; + if (`${patchRange.patchNum}` !== `${latestPatchset._number}`) { + return true; + } + + // show owners when they apply to the change and for logged in user + if ( + !allFilesApproved && + change.submit_requirements && + change.submit_requirements.find(r => r.name === OWNERS_SUBMIT_REQUIREMENT) + ) { + return !userRole || userRole === UserRole.ANONYMOUS; + } + return true; +} + +export function getFileOwnership( + path?: string, + allFilesApproved?: boolean, + filesOwners?: FilesOwners +): FileOwnership | undefined { + if (path === undefined || filesOwners === undefined) { + return undefined; + } + + const fileOwners = (filesOwners.files ?? {})[path]; + return (allFilesApproved || !fileOwners + ? {fileStatus: FileStatus.NOT_OWNED_OR_APPROVED} + : { + fileStatus: FileStatus.NEEDS_APPROVAL, + owners: fileOwners, + }) as unknown as FileOwnership; +} + +export function computeApprovalAndInfo( + fileOwner: AccountInfo, + labels: OwnersLabels, + change?: ChangeInfo +): [ApprovalInfo, LabelInfo] | undefined { + if (!change?.labels) { + return; + } + const ownersLabel = labels[`${fileOwner._account_id}`]; + if (!ownersLabel) { + return; + } + + for (const label of Object.keys(ownersLabel)) { + const info = change.labels[label]; + if (!info || !isDetailedLabelInfo(info)) { + return; + } + + const vote = ownersLabel[label]; + if ((info.default_value && info.default_value === vote) || vote === 0) { + // ignore default value + return; + } + + const approval = info.all?.filter( + x => x._account_id === fileOwner._account_id + )[0]; + return approval ? [approval, info] : undefined; + } + + return; +} + +export function getChangeAccounts( + change?: ChangeInfo +): Map<number, AccountInfo> { + const accounts = new Map(); + if (!change) { + return accounts; + } + + [ + change.owner, + ...(change.submitter ? [change.submitter] : []), + ...(change.reviewers[ReviewerState.REVIEWER] ?? []), + ...(change.reviewers[ReviewerState.CC] ?? []), + ].forEach(account => accounts.set(account._account_id, account)); + return accounts; +}
diff --git a/owners/web/gr-files_test.ts b/owners/web/gr-files_test.ts new file mode 100644 index 0000000..0da0a35 --- /dev/null +++ b/owners/web/gr-files_test.ts
@@ -0,0 +1,410 @@ +/** + * @license + * Copyright (C) 2024 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 {assert} from '@open-wc/testing'; + +import { + computeApprovalAndInfo, + getChangeAccounts, + getFileOwnership, + shouldHide, +} from './gr-files'; +import {FileOwnership, FileStatus, PatchRange, UserRole} from './owners-model'; +import { + AccountInfo, + ApprovalInfo, + ChangeInfo, + ChangeStatus, + DetailedLabelInfo, + SubmitRequirementResultInfo, + ReviewerState, +} from '@gerritcodereview/typescript-api/rest-api'; +import {FilesOwners, OwnersLabels} from './owners-service'; +import {deepEqual} from './utils'; +import {getRandom} from './test-utils'; + +suite('owners status tests', () => { + const allFilesApproved = true; + + suite('shouldHide tests', () => { + const loggedIn = getRandom(UserRole.CHANGE_OWNER, UserRole.OTHER); + + test('shouldHide - should be `true` when change is not defined', () => { + const undefinedChange = undefined; + const definedPatchRange = {} as unknown as PatchRange; + assert.equal( + shouldHide( + undefinedChange, + definedPatchRange, + allFilesApproved, + loggedIn + ), + true + ); + }); + + test('shouldHide - should be `true` when patch range is not defined', () => { + const definedChange = {} as unknown as ChangeInfo; + const undefinedPatchRange = undefined; + assert.equal( + shouldHide( + definedChange, + undefinedPatchRange, + allFilesApproved, + loggedIn + ), + true + ); + }); + + test('shouldHide - should be `true` when change is abandoned', () => { + const abandonedChange = { + status: ChangeStatus.ABANDONED, + } as unknown as ChangeInfo; + const definedPatchRange = {} as unknown as PatchRange; + assert.equal( + shouldHide( + abandonedChange, + definedPatchRange, + allFilesApproved, + loggedIn + ), + true + ); + }); + + test('shouldHide - should be `true` when change is merged', () => { + const mergedChange = { + status: ChangeStatus.MERGED, + } as unknown as ChangeInfo; + const definedPatchRange = {} as unknown as PatchRange; + assert.equal( + shouldHide(mergedChange, definedPatchRange, allFilesApproved, loggedIn), + true + ); + }); + + test('shouldHide - should be `true` if not on the latest PS', () => { + const changeWithPs2 = { + status: ChangeStatus.NEW, + revisions: { + current_rev: {_number: 2}, + }, + current_revision: 'current_rev', + } as unknown as ChangeInfo; + const patchRangeOnPs1 = {patchNum: 1} as unknown as PatchRange; + assert.equal( + shouldHide(changeWithPs2, patchRangeOnPs1, allFilesApproved, loggedIn), + true + ); + }); + + const change = { + status: ChangeStatus.NEW, + revisions: { + current_rev: {_number: 1}, + }, + current_revision: 'current_rev', + } as unknown as ChangeInfo; + const patchRange = {patchNum: 1} as unknown as PatchRange; + + test('shouldHide - should be `true` when change has no submit requirements', () => { + assert.equal( + shouldHide(change, patchRange, !allFilesApproved, loggedIn), + true + ); + }); + + test('shouldHide - should be `true` when change has no `Owner-Approval` submit requirements', () => { + const changeWithDifferentSubmitReqs = { + ...change, + submit_requirements: [ + {name: 'other'}, + ] as unknown as SubmitRequirementResultInfo[], + }; + assert.equal( + shouldHide( + changeWithDifferentSubmitReqs, + patchRange, + !allFilesApproved, + loggedIn + ), + true + ); + }); + + const changeWithSubmitRequirements = { + ...change, + submit_requirements: [ + {name: 'Owner-Approval'}, + ] as unknown as SubmitRequirementResultInfo[], + }; + + test('shouldHide - should be `true` when user is not change owner', () => { + const anonymous = UserRole.ANONYMOUS; + assert.equal( + shouldHide( + changeWithSubmitRequirements, + patchRange, + !allFilesApproved, + anonymous + ), + true + ); + }); + + test('shouldHide - should be `true` when change has submit requirements and has all files approved even if user is logged in', () => { + assert.equal( + shouldHide( + changeWithSubmitRequirements, + patchRange, + allFilesApproved, + loggedIn + ), + true + ); + }); + + test('shouldHide - should be `false` when change has submit requirements, has no all files approved and user is logged in', () => { + assert.equal( + shouldHide( + changeWithSubmitRequirements, + patchRange, + !allFilesApproved, + loggedIn + ), + false + ); + }); + + test('shouldHide - should be `false` when in edit mode', () => { + const patchRangeWithoutPatchNum = {} as unknown as PatchRange; + assert.equal( + shouldHide( + change, + patchRangeWithoutPatchNum, + allFilesApproved, + loggedIn + ), + false + ); + }); + }); + + suite('getFileOwnership tests', () => { + const path = 'readme.md'; + const emptyFilesOwners = {} as unknown as FilesOwners; + const fileOwnersWithPath = { + files: {[path]: [{name: 'John', id: 1}]}, + } as unknown as FilesOwners; + + test('getFileOwnership - should be `undefined` when path is `undefined', () => { + const undefinedPath = undefined; + assert.equal( + getFileOwnership(undefinedPath, allFilesApproved, emptyFilesOwners), + undefined + ); + }); + + test('getFileOwnership - should be `undefined` when file owners are `undefined', () => { + const undefinedFileOwners = undefined; + assert.equal( + getFileOwnership(path, allFilesApproved, undefinedFileOwners), + undefined + ); + }); + + test('getFileOwnership - should return `FileOwnership` with `NOT_OWNED_OR_APPROVED` fileStatus when `allFilesApproved`', () => { + assert.equal( + deepEqual( + getFileOwnership(path, allFilesApproved, fileOwnersWithPath), + {fileStatus: FileStatus.NOT_OWNED_OR_APPROVED} as FileOwnership + ), + true + ); + }); + + test('getFileOwnership - should return `FileOwnership` with `NOT_OWNED_OR_APPROVED` fileStatus when file has no owner', () => { + assert.equal( + deepEqual(getFileOwnership(path, !allFilesApproved, emptyFilesOwners), { + fileStatus: FileStatus.NOT_OWNED_OR_APPROVED, + } as FileOwnership), + true + ); + }); + + test('getFileOwnership - should return `FileOwnership` with `NEEDS_APPROVAL` fileStatus when file has owner', () => { + assert.equal( + deepEqual( + getFileOwnership(path, !allFilesApproved, fileOwnersWithPath), + { + fileStatus: FileStatus.NEEDS_APPROVAL, + owners: [{name: 'John', id: 1}], + } as FileOwnership + ), + true + ); + }); + }); + + suite('computeApprovalAndInfo tests', () => { + const account = 1; + const fileOwner = {_account_id: account} as unknown as AccountInfo; + const label = 'Code-Review'; + const crPlus1OwnersVote = { + [`${account}`]: {[label]: 1}, + } as unknown as OwnersLabels; + const changeWithLabels = { + labels: { + [label]: { + all: [ + { + value: 1, + date: '2024-10-22 17:26:21.000000000', + permitted_voting_range: { + min: -2, + max: 2, + }, + _account_id: account, + }, + ], + values: { + '-2': 'This shall not be submitted', + '-1': 'I would prefer this is not submitted as is', + ' 0': 'No score', + '+1': 'Looks good to me, but someone else must approve', + '+2': 'Looks good to me, approved', + }, + description: '', + default_value: 0, + }, + }, + } as unknown as ChangeInfo; + + test('computeApprovalAndInfo - should be `undefined` when change is `undefined', () => { + const undefinedChange = undefined; + assert.equal( + computeApprovalAndInfo(fileOwner, crPlus1OwnersVote, undefinedChange), + undefined + ); + }); + + test('computeApprovalAndInfo - should be `undefined` when there is no owners vote', () => { + const emptyOwnersVote = {}; + assert.equal( + computeApprovalAndInfo(fileOwner, emptyOwnersVote, changeWithLabels), + undefined + ); + }); + + test('computeApprovalAndInfo - should be `undefined` for default owners vote', () => { + const defaultOwnersVote = {[label]: 0} as unknown as OwnersLabels; + assert.equal( + computeApprovalAndInfo(fileOwner, defaultOwnersVote, changeWithLabels), + undefined + ); + }); + + test('computeApprovalAndInfo - should be computed for CR+1 owners vote', () => { + const expectedApproval = { + value: 1, + date: '2024-10-22 17:26:21.000000000', + permitted_voting_range: { + min: -2, + max: 2, + }, + _account_id: account, + } as unknown as ApprovalInfo; + const expectedInfo = { + all: [expectedApproval], + values: { + '-2': 'This shall not be submitted', + '-1': 'I would prefer this is not submitted as is', + ' 0': 'No score', + '+1': 'Looks good to me, but someone else must approve', + '+2': 'Looks good to me, approved', + }, + description: '', + default_value: 0, + } as unknown as DetailedLabelInfo; + + assert.equal( + deepEqual( + computeApprovalAndInfo( + fileOwner, + crPlus1OwnersVote, + changeWithLabels + ), + [expectedApproval, expectedInfo] + ), + true + ); + }); + }); + + suite('getChangeAccounts tests', () => { + test('getChangeAccounts - should return empty map when change is `undefined', () => { + const undefinedChange = undefined; + assert.equal(getChangeAccounts(undefinedChange).size, 0); + }); + + test('getChangeAccounts - should return map with owner when change has only owner and empty reviewers defined', () => { + const owner = account(1); + const changeWithOwner = { + owner, + reviewers: {}, + } as unknown as ChangeInfo; + assert.equal( + deepEqual(getChangeAccounts(changeWithOwner), new Map([[1, owner]])), + true + ); + }); + + test('getChangeAccounts - should return map with owner, submitter and reviewers', () => { + const owner = account(1); + const submitter = account(2); + const reviewer = account(3); + const ccReviewer = account(4); + const change = { + owner, + submitter, + reviewers: { + [ReviewerState.REVIEWER]: [reviewer], + [ReviewerState.CC]: [ccReviewer], + }, + } as unknown as ChangeInfo; + assert.equal( + deepEqual( + getChangeAccounts(change), + new Map([ + [1, owner], + [2, submitter], + [3, reviewer], + [4, ccReviewer], + ]) + ), + true + ); + }); + }); +}); + +function account(id: number) { + return { + _account_id: id, + } as unknown as AccountInfo; +}
diff --git a/owners/web/gr-owned-files.ts b/owners/web/gr-owned-files.ts new file mode 100644 index 0000000..969d91c --- /dev/null +++ b/owners/web/gr-owned-files.ts
@@ -0,0 +1,331 @@ +/** + * @license + * Copyright (C) 2024 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 {css, html, LitElement, PropertyValues, nothing, CSSResult} from 'lit'; +import {ifDefined} from 'lit/directives/if-defined.js'; +import {OwnersMixin} from './owners-mixin'; +import {customElement, property} from 'lit/decorators'; +import { + AccountInfo, + ChangeInfo, + ChangeStatus, + RevisionInfo, + EDIT, +} from '@gerritcodereview/typescript-api/rest-api'; +import {User, UserRole} from './owners-model'; +import {isOwner, OwnedFiles, OWNERS_SUBMIT_REQUIREMENT} from './owners-service'; +import { + computeDisplayPath, + diffFilePaths, + encodeURL, + getBaseUrl, + truncatePath, +} from './utils'; + +const common = OwnersMixin(LitElement); + +class OwnedFilesCommon extends common { + @property({type: Object}) + revision?: RevisionInfo; + + protected ownedFiles?: string[]; + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + this.computeOwnedFiles(); + + this.hidden = shouldHide( + this.change, + this.revision, + this.allFilesApproved, + this.user, + this.ownedFiles + ); + } + + static commonStyles(): CSSResult[] { + return [window?.Gerrit?.styles.font as CSSResult]; + } + + private computeOwnedFiles() { + this.ownedFiles = ownedFiles(this.user?.account, this.filesOwners?.files); + } +} + +export const OWNED_FILES_TAB_HEADER = 'owned-files-tab-header'; +@customElement(OWNED_FILES_TAB_HEADER) +export class OwnedFilesTabHeader extends OwnedFilesCommon { + static override get styles() { + return [...OwnedFilesCommon.commonStyles()]; + } + + override render() { + if (this.hidden) return nothing; + return html`<div>Owned Files</div>`; + } +} + +@customElement('gr-owned-files-list') +export class GrOwnedFilesList extends LitElement { + @property({type: Object}) + change?: ChangeInfo; + + @property({type: Object}) + revision?: RevisionInfo; + + @property({type: Array}) + ownedFiles?: string[]; + + static override get styles() { + return [ + ...OwnedFilesCommon.commonStyles(), + css` + :host { + display: block; + } + .row { + align-items: center; + border-top: 1px solid var(--border-color); + display: flex; + min-height: calc(var(--line-height-normal) + 2 * var(--spacing-s)); + padding: var(--spacing-xs) var(--spacing-l); + } + .header-row { + background-color: var(--background-color-secondary); + } + .file-row { + cursor: pointer; + } + .file-row:hover { + background-color: var(--hover-background-color); + } + .file-row.selected { + background-color: var(--selection-background-color); + } + .path { + cursor: pointer; + flex: 1; + /* Wrap it into multiple lines if too long. */ + white-space: normal; + word-break: break-word; + } + .matchingFilePath { + color: var(--deemphasized-text-color); + } + .newFilePath { + color: var(--primary-text-color); + } + .fileName { + color: var(--link-color); + } + .truncatedFileName { + display: none; + } + .row:focus { + outline: none; + } + .row:hover { + opacity: 100; + } + .pathLink { + display: inline-block; + margin: -2px 0; + padding: var(--spacing-s) 0; + text-decoration: none; + } + .pathLink:hover span.fullFileName, + .pathLink:hover span.truncatedFileName { + text-decoration: underline; + } + `, + ]; + } + + override render() { + if (!this.change || !this.revision || !this.ownedFiles) return nothing; + + return html` + <div id="container" role="grid" aria-label="Owned files list"> + ${this.renderOwnedFilesHeaderRow()} + ${this.ownedFiles?.map((ownedFile, index) => + this.renderOwnedFileRow(ownedFile, index) + )} + </div> + `; + } + + private renderOwnedFilesHeaderRow() { + return html` + <div class="header-row row" role="row"> + <div class="path" role="columnheader">File</div> + </div> + `; + } + + private renderOwnedFileRow(ownedFile: string, index: number) { + return html` + <div + class="file-row row" + tabindex="-1" + role="row" + aria-label=${ownedFile} + > + ${this.renderFilePath(ownedFile, index)} + </div> + `; + } + + private renderFilePath(file: string, index: number) { + const displayPath = computeDisplayPath(file); + const previousFile = (this.ownedFiles ?? [])[index - 1]; + return html` + <span class="path" role="gridcell"> + <a + class="pathLink" + href=${ifDefined(computeDiffUrl(file, this.change, this.revision))} + > + <span title=${displayPath} class="fullFileName"> + ${this.renderStyledPath(file, previousFile)} + </span> + <span title=${displayPath} class="truncatedFileName"> + ${truncatePath(displayPath)} + </span> + </a> + </span> + `; + } + + private renderStyledPath(filePath: string, previousFilePath?: string) { + const {matchingFolders, newFolders, fileName} = diffFilePaths( + filePath, + previousFilePath + ); + return [ + matchingFolders.length > 0 + ? html`<span class="matchingFilePath">${matchingFolders}</span>` + : nothing, + newFolders.length > 0 + ? html`<span class="newFilePath">${newFolders}</span>` + : nothing, + html`<span class="fileName">${fileName}</span>`, + ]; + } +} + +export const OWNED_FILES_TAB_CONTENT = 'owned-files-tab-content'; +@customElement(OWNED_FILES_TAB_CONTENT) +export class OwnedFilesTabContent extends OwnedFilesCommon { + static override get styles() { + return [ + ...OwnedFilesCommon.commonStyles(), + css` + :host { + display: block; + } + `, + ]; + } + + override render() { + if (this.hidden) return nothing; + + return html` + <gr-owned-files-list + id="ownedFilesList" + .change=${this.change} + .revision=${this.revision} + .ownedFiles=${this.ownedFiles} + > + </gr-owned-files-list> + `; + } +} + +export function shouldHide( + change?: ChangeInfo, + revision?: RevisionInfo, + allFilesApproved?: boolean, + user?: User, + ownedFiles?: string[] +) { + // don't show owned files when no change or change is abandoned/merged or being edited + if ( + change === undefined || + change.status === ChangeStatus.ABANDONED || + change.status === ChangeStatus.MERGED || + revision === undefined || + revision._number === EDIT + ) { + return true; + } + + // show owned files if user owns anything + if ( + !allFilesApproved && + change.submit_requirements && + change.submit_requirements.find(r => r.name === OWNERS_SUBMIT_REQUIREMENT) + ) { + return ( + !user || + user.role === UserRole.ANONYMOUS || + (ownedFiles ?? []).length === 0 + ); + } + return true; +} + +export function ownedFiles( + owner?: AccountInfo, + files?: OwnedFiles +): string[] | undefined { + if (!owner || !files) { + return; + } + + const ownedFiles = []; + for (const file of Object.keys(files)) { + if ( + files[file].find( + fileOwner => isOwner(fileOwner) && fileOwner.id === owner._account_id + ) + ) { + ownedFiles.push(file); + } + } + + return ownedFiles; +} + +export function computeDiffUrl( + file: string, + change?: ChangeInfo, + revision?: RevisionInfo +) { + if (change === undefined || revision?._number === undefined) { + return; + } + + let repo = ''; + if (change.project) repo = `${encodeURL(change.project)}/+/`; + + // TODO it can be a range of patchsets but `PatchRange` is not passed to the `change-view-tab-content` :/ fix in Gerrit? + const range = `/${revision._number}`; + + const path = `/${encodeURL(file)}`; + + return `${getBaseUrl()}/c/${repo}${change._number}${range}${path}`; +}
diff --git a/owners/web/gr-owned-files_test.ts b/owners/web/gr-owned-files_test.ts new file mode 100644 index 0000000..0437ab9 --- /dev/null +++ b/owners/web/gr-owned-files_test.ts
@@ -0,0 +1,185 @@ +/** + * @license + * Copyright (C) 2024 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 {assert} from '@open-wc/testing'; +import { + AccountInfo, + ChangeInfo, + ChangeStatus, + RevisionInfo, + EDIT, + SubmitRequirementResultInfo, +} from '@gerritcodereview/typescript-api/rest-api'; +import {ownedFiles, shouldHide} from './gr-owned-files'; +import {OwnedFiles, Owner} from './owners-service'; +import {deepEqual} from './utils'; +import {User, UserRole} from './owners-model'; +import {getRandom} from './test-utils'; + +suite('owned files tests', () => { + suite('ownedFiles tests', () => { + const ownerAccountId = 1; + const owner = account(ownerAccountId); + + const ownedFile = 'README.md'; + const files = { + [ownedFile]: [fileOwner(1)], + 'some.text': [fileOwner(6)], + } as unknown as OwnedFiles; + + test('ownedFiles - should be `undefined` when owner is `undefined`', () => { + const undefinedOwner = undefined; + assert.equal(ownedFiles(undefinedOwner, files), undefined); + }); + + test('ownedFiles - should be `undefined` when files are `undefined`', () => { + const undefinedFiles = undefined; + assert.equal(ownedFiles(owner, undefinedFiles), undefined); + }); + + test('ownedFiles - should return empty owned file when no files are owned by user', () => { + const user = account(2); + assert.equal(deepEqual(ownedFiles(user, files), []), true); + }); + + test('ownedFiles - should return owned files', () => { + assert.equal(deepEqual(ownedFiles(owner, files), [ownedFile]), true); + }); + }); + + suite('shouldHide tests', () => { + const change = { + status: ChangeStatus.NEW, + submit_requirements: [ + {name: 'Owner-Approval'}, + ] as unknown as SubmitRequirementResultInfo[], + } as unknown as ChangeInfo; + const revisionInfo = {_number: 1} as unknown as RevisionInfo; + const allFilesApproved = true; + const user = {account: account(1), role: UserRole.OTHER}; + const ownedFiles = ['README.md']; + + test('shouldHide - should be `true` when change is `undefined`', () => { + const undefinedChange = undefined; + assert.equal( + shouldHide( + undefinedChange, + revisionInfo, + !allFilesApproved, + user, + ownedFiles + ), + true + ); + }); + + test('shouldHide - should be `true` when change is `ABANDONED` or `MERGED`', () => { + const abandonedOrMergedChange = { + ...change, + status: getRandom(ChangeStatus.ABANDONED, ChangeStatus.MERGED), + }; + assert.equal( + shouldHide( + abandonedOrMergedChange, + revisionInfo, + !allFilesApproved, + user, + ownedFiles + ), + true + ); + }); + + test('shouldHide - should be `true` when revisionInfo is `undefined` or in `EDIT` mode', () => { + const undefinedOrEditRevisionInfo = getRandom(undefined, { + _number: EDIT, + } as unknown as RevisionInfo); + assert.equal( + shouldHide( + change, + undefinedOrEditRevisionInfo, + !allFilesApproved, + user, + ownedFiles + ), + true + ); + }); + + test('shouldHide - should be `true` when change has different submit requirements', () => { + const changeWithOtherSubmitRequirements = { + ...change, + submit_requirements: [ + {name: 'Other'}, + ] as unknown as SubmitRequirementResultInfo[], + }; + assert.equal( + shouldHide( + changeWithOtherSubmitRequirements, + revisionInfo, + !allFilesApproved, + user, + ownedFiles + ), + true + ); + }); + + test('shouldHide - should be `true` when all files are approved', () => { + assert.equal( + shouldHide(change, revisionInfo, allFilesApproved, user, ownedFiles), + true + ); + }); + + test('shouldHide - should be `true` when user is `undefined` or `ANONYMOUS`', () => { + const undefinedOrAnonymousUser = getRandom(undefined, { + role: UserRole.ANONYMOUS, + } as unknown as User); + assert.equal( + shouldHide( + change, + revisionInfo, + !allFilesApproved, + undefinedOrAnonymousUser, + ownedFiles + ), + true + ); + }); + + test('shouldHide - should be `false` when user owns files', () => { + assert.equal( + shouldHide(change, revisionInfo, !allFilesApproved, user, ownedFiles), + false + ); + }); + }); +}); + +function account(id: number): AccountInfo { + return { + _account_id: id, + } as unknown as AccountInfo; +} + +function fileOwner(id: number): Owner { + return { + id, + name: `name for account: ${id}`, + } as unknown as Owner; +}
diff --git a/owners/web/karma_test.sh b/owners/web/karma_test.sh new file mode 100755 index 0000000..7cecdae --- /dev/null +++ b/owners/web/karma_test.sh
@@ -0,0 +1,6 @@ +#!/bin/bash +set -euo pipefail +./$1 start $2 --single-run \ + --root 'plugins/owners/web/_bazel_ts_out_tests/' \ + --test-files '*_test.js' \ + --browsers ${3:-ChromeHeadless}
diff --git a/owners/web/owners-mixin.ts b/owners/web/owners-mixin.ts new file mode 100644 index 0000000..0bc813a --- /dev/null +++ b/owners/web/owners-mixin.ts
@@ -0,0 +1,135 @@ +/** + * @license + * Copyright (C) 2024 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 {Subscription} from 'rxjs'; +import {LitElement, PropertyValues} from 'lit'; +import {property, state} from 'lit/decorators'; +import {ChangeInfo} from '@gerritcodereview/typescript-api/rest-api'; +import {FilesOwners, OwnersService} from './owners-service'; +import {RestPluginApi} from '@gerritcodereview/typescript-api/rest'; +import {ModelLoader, OwnersModel, PatchRange, User} from './owners-model'; + +// Lit mixin definition as described in https://lit.dev/docs/composition/mixins/ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type Constructor<T> = new (...args: any[]) => T; + +export interface OwnersInterface extends LitElement { + change?: ChangeInfo; + patchRange?: PatchRange; + restApi?: RestPluginApi; + user?: User; + allFilesApproved?: boolean; + filesOwners?: FilesOwners; + + onModelUpdate(): void; +} + +export const OwnersMixin = <T extends Constructor<LitElement>>( + superClass: T +) => { + class Mixin extends superClass { + @property({type: Object}) + change?: ChangeInfo; + + @property({type: Object}) + patchRange?: PatchRange; + + @property({type: Object}) + restApi?: RestPluginApi; + + @state() + user?: User; + + @state() + allFilesApproved?: boolean; + + @state() + filesOwners?: FilesOwners; + + private _model?: OwnersModel; + + modelLoader?: ModelLoader; + + private subscriptions: Array<Subscription> = []; + + get model() { + return this._model; + } + + set model(model: OwnersModel | undefined) { + if (this._model === model) return; + for (const s of this.subscriptions) { + s.unsubscribe(); + } + this.subscriptions = []; + this._model = model; + if (!model) return; + + this.subscriptions.push( + model.state$.subscribe(s => { + this.user = s.user; + }) + ); + + this.subscriptions.push( + model.state$.subscribe(s => { + this.allFilesApproved = s.allFilesApproved; + }) + ); + + this.subscriptions.push( + model.state$.subscribe(s => { + this.filesOwners = s.filesOwners; + }) + ); + + this.onModelUpdate(); + } + + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + + if (changedProperties.has('change') || changedProperties.has('restApi')) { + if (!this.restApi || !this.change) { + this.model = undefined; + this.modelLoader = undefined; + return; + } + const service = OwnersService.getOwnersService( + this.restApi, + this.change + ); + const model = OwnersModel.getModel(this.change); + this.modelLoader = new ModelLoader(service, model); + this.model = model; + } + } + + protected onModelUpdate() { + this.modelLoader?.loadUser(); + this.modelLoader?.loadAllFilesApproved(); + this.modelLoader?.loadFilesOwners(); + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + constructor(...args: any[]) { + super(...args); + } + } + + return Mixin as unknown as T & Constructor<OwnersInterface>; +};
diff --git a/owners/web/owners-model.ts b/owners/web/owners-model.ts new file mode 100644 index 0000000..6d048e0 --- /dev/null +++ b/owners/web/owners-model.ts
@@ -0,0 +1,164 @@ +/** + * @license + * Copyright (C) 2024 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 {BehaviorSubject, Observable} from 'rxjs'; +import { + AccountInfo, + BasePatchSetNum, + ChangeInfo, + RevisionPatchSetNum, +} from '@gerritcodereview/typescript-api/rest-api'; +import {FileOwner, FilesOwners, OwnersService} from './owners-service'; +import {deepEqual} from './utils'; + +export interface PatchRange { + patchNum: RevisionPatchSetNum; + basePatchNum: BasePatchSetNum; +} + +export enum UserRole { + ANONYMOUS = 'ANONYMOUS', + CHANGE_OWNER = 'CHANGE_OWNER', + OTHER = 'OTHER', +} + +export interface User { + account?: AccountInfo; + role: UserRole; +} + +export interface OwnersState { + user?: User; + allFilesApproved?: boolean; + filesOwners?: FilesOwners; +} + +/** + * TODO: The plugin's REST endpoint returns only files that still need to be reviewed which means that file can only have two states: + * * needs approval + * * was not subject of OWNERS file or is already approved + */ +export enum FileStatus { + NEEDS_APPROVAL = 'NEEDS_APPROVAL', + NOT_OWNED_OR_APPROVED = 'NOT_OWNED_OR_APPROVED', +} + +/** + * TODO: extend FileOwnership with owners when it will be used by UI elements + */ +export interface FileOwnership { + fileStatus: FileStatus; + owners?: FileOwner[]; +} + +let ownersModel: OwnersModel | undefined; + +export class OwnersModel extends EventTarget { + private subject$: BehaviorSubject<OwnersState> = new BehaviorSubject( + {} as OwnersState + ); + + public state$: Observable<OwnersState> = this.subject$.asObservable(); + + constructor(readonly change: ChangeInfo) { + super(); + } + + get state() { + return this.subject$.getValue(); + } + + private setState(state: OwnersState) { + this.subject$.next(Object.freeze(state)); + } + + setUser(user: User) { + const current = this.subject$.getValue(); + if (current.user === user) return; + this.setState({...current, user}); + } + + setAllFilesApproved(allFilesApproved: boolean | undefined) { + const current = this.subject$.getValue(); + if (current.allFilesApproved === allFilesApproved) return; + this.setState({...current, allFilesApproved}); + } + + setFilesOwners(filesOwners: FilesOwners | undefined) { + const current = this.subject$.getValue(); + if (deepEqual(current.filesOwners, filesOwners)) return; + this.setState({...current, filesOwners}); + } + + static getModel(change: ChangeInfo) { + if (!ownersModel || ownersModel.change !== change) { + ownersModel = new OwnersModel(change); + } + return ownersModel; + } +} + +export class ModelLoader { + constructor( + private readonly service: OwnersService, + private readonly model: OwnersModel + ) {} + + async loadUser() { + await this._loadProperty( + 'user', + () => this.service.getLoggedInUser(), + value => this.model.setUser(value) + ); + } + + async loadAllFilesApproved() { + await this._loadProperty( + 'allFilesApproved', + () => this.service.getAllFilesApproved(), + value => this.model.setAllFilesApproved(value) + ); + } + + async loadFilesOwners() { + await this._loadProperty( + 'filesOwners', + () => this.service.getFilesOwners(), + value => this.model.setFilesOwners(value) + ); + } + + private async _loadProperty<K extends keyof OwnersState, T>( + propertyName: K, + propertyLoader: () => Promise<T>, + propertySetter: (value: T) => void + ) { + if (this.model.state[propertyName] !== undefined) return; + let newValue: T; + try { + newValue = await propertyLoader(); + } catch (e) { + console.error(e); + return; + } + // It is possible, that several requests is made in parallel. + // Store only the first result and discard all other results. + // (also, due to the CodeOwnersCacheApi all result must be identical) + if (this.model.state[propertyName] !== undefined) return; + propertySetter(newValue); + } +}
diff --git a/owners/web/owners-service.ts b/owners/web/owners-service.ts new file mode 100644 index 0000000..f1bb4fd --- /dev/null +++ b/owners/web/owners-service.ts
@@ -0,0 +1,190 @@ +/** + * @license + * Copyright (C) 2024 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 {HttpMethod, RestPluginApi} from '@gerritcodereview/typescript-api/rest'; +import { + AccountDetailInfo, + ChangeInfo, + ChangeStatus, + NumericChangeId, + RepoName, + SubmitRequirementStatus, +} from '@gerritcodereview/typescript-api/rest-api'; +import {User, UserRole} from './owners-model'; + +export interface GroupOwner { + name: string; +} + +export interface Owner extends GroupOwner { + id: number; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export function isOwner(o: any): o is Owner { + return o && typeof o.id === 'number' && typeof o.name === 'string'; +} + +export type FileOwner = Owner | GroupOwner; + +export interface OwnedFiles { + [fileName: string]: FileOwner[]; +} + +export interface OwnersLabels { + [id: string]: { + [label: string]: number; + }; +} + +export interface FilesOwners { + files: OwnedFiles; + owners_labels: OwnersLabels; +} + +export const OWNERS_SUBMIT_REQUIREMENT = 'Owner-Approval'; + +class ResponseError extends Error { + constructor(readonly response: Response) { + super(); + } +} + +async function getErrorMessage(response: Response) { + const text = await response.text(); + return text ? `${response.status}: ${text}` : `${response.status}`; +} + +class OwnersApi { + constructor(readonly restApi: RestPluginApi) {} + + async getAccount(): Promise<AccountDetailInfo | undefined> { + const loggedIn = await this.restApi.getLoggedIn(); + if (!loggedIn) return undefined; + return await this.restApi.getAccount(); + } + + /** + * Returns the list of owners associated to each file that needs a review, + * and, for each owner, its current labels and votes. + * + * @doc + * https://gerrit.googlesource.com/plugins/owners/+/refs/heads/master/owners/src/main/resources/Documentation/rest-api.md + */ + getFilesOwners( + repoName: RepoName, + changeId: NumericChangeId, + revision: String + ): Promise<FilesOwners> { + return this.get( + `/changes/${encodeURIComponent( + repoName + )}~${changeId}/revisions/${revision}/owners~files-owners` + ) as Promise<FilesOwners>; + } + + private async get(url: string): Promise<unknown> { + const errFn = (response?: Response | null, error?: Error) => { + if (error) throw error; + if (response) throw new ResponseError(response); + throw new Error('Generic REST API error'); + }; + try { + return await this.restApi.send(HttpMethod.GET, url, undefined, errFn); + } catch (err) { + if (err instanceof ResponseError && err.response.status === 409) { + getErrorMessage(err.response).then(msg => { + // TODO handle 409 in UI - show banner with error + console.error(`Plugin configuration errot: ${msg}`); + }); + } + throw err; + } + } +} + +let service: OwnersService | undefined; + +export class OwnersService { + private api: OwnersApi; + + constructor(readonly restApi: RestPluginApi, readonly change: ChangeInfo) { + this.api = new OwnersApi(restApi); + } + + async getLoggedInUser(): Promise<User> { + const account = await this.api.getAccount(); + if (!account) { + return {role: UserRole.ANONYMOUS} as unknown as User; + } + const role = + this.change.owner._account_id === account._account_id + ? UserRole.CHANGE_OWNER + : UserRole.OTHER; + return {account, role} as unknown as User; + } + + async getAllFilesApproved(): Promise<boolean | undefined> { + if (!(await this.isLoggedIn())) { + return Promise.resolve(undefined); + } + + if ( + this.change?.status === ChangeStatus.ABANDONED || + this.change?.status === ChangeStatus.MERGED + ) { + return Promise.resolve(undefined); + } + + const ownersSr = this.change?.submit_requirements?.find( + r => r.name === OWNERS_SUBMIT_REQUIREMENT + ); + if (!ownersSr) { + return Promise.resolve(undefined); + } + + return Promise.resolve( + ownersSr.status === SubmitRequirementStatus.SATISFIED + ); + } + + async getFilesOwners(): Promise<FilesOwners | undefined> { + const allFilesApproved = await this.getAllFilesApproved(); + + if (allFilesApproved === undefined || allFilesApproved) { + return Promise.resolve(undefined); + } + + return this.api.getFilesOwners( + this.change.project, + this.change._number, + this.change.current_revision ?? 'current' + ); + } + + private async isLoggedIn(): Promise<boolean> { + const user = await this.getLoggedInUser(); + return user && user.role !== UserRole.ANONYMOUS; + } + + static getOwnersService(restApi: RestPluginApi, change: ChangeInfo) { + if (!service || service.change !== change) { + service = new OwnersService(restApi, change); + } + return service; + } +}
diff --git a/owners/web/owners-service_test.ts b/owners/web/owners-service_test.ts new file mode 100644 index 0000000..0b77f66 --- /dev/null +++ b/owners/web/owners-service_test.ts
@@ -0,0 +1,308 @@ +/** + * @license + * Copyright (C) 2024 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 {FilesOwners, OwnersService} from './owners-service'; +import { + RequestPayload, + RestPluginApi, +} from '@gerritcodereview/typescript-api/rest'; +import { + AccountInfo, + ChangeInfo, + ChangeStatus, + HttpMethod, + SubmitRequirementResultInfo, +} from '@gerritcodereview/typescript-api/rest-api'; +import {assert} from '@open-wc/testing'; +import {UserRole} from './owners-model'; +import {deepEqual} from './utils'; + +suite('owners service tests', () => { + const fakeRestApi = {} as unknown as RestPluginApi; + const fakeChange = {} as unknown as ChangeInfo; + + suite('basic api request tests', () => { + test('getOwnersService - same change returns the same instance', () => { + assert.equal( + OwnersService.getOwnersService(fakeRestApi, fakeChange), + OwnersService.getOwnersService(fakeRestApi, fakeChange) + ); + }); + + test('getOwnersService - modified change returns new instance', () => { + assert.notEqual( + OwnersService.getOwnersService(fakeRestApi, {...fakeChange}), + OwnersService.getOwnersService(fakeRestApi, {...fakeChange}) + ); + }); + }); + + suite('user role tests', () => { + test('getLoggedInUser - returns ANONYMOUS when user not logged in', async () => { + const notLoggedInApi = { + getLoggedIn() { + return Promise.resolve(false); + }, + } as unknown as RestPluginApi; + + const service = OwnersService.getOwnersService( + notLoggedInApi, + fakeChange + ); + const user = await service.getLoggedInUser(); + assert.equal(user.role, UserRole.ANONYMOUS); + assert.equal(user.account, undefined); + }); + + test('getLoggedInUser - returns OTHER for logged in user that is NOT change owner', async () => { + const loggedUser = account(2); + const userLoggedInApi = { + getLoggedIn() { + return Promise.resolve(true); + }, + getAccount() { + return Promise.resolve(loggedUser); + }, + } as unknown as RestPluginApi; + const change = {owner: account(1)} as unknown as ChangeInfo; + + const service = OwnersService.getOwnersService(userLoggedInApi, change); + const user = await service.getLoggedInUser(); + assert.equal(user.role, UserRole.OTHER); + assert.equal(user.account, loggedUser); + }); + + test('getLoggedInUser - returns CHANGE_OWNER for logged in user that is a change owner', async () => { + const owner = account(1); + const changeOwnerLoggedInApi = { + getLoggedIn() { + return Promise.resolve(true); + }, + getAccount() { + return Promise.resolve(owner); + }, + } as unknown as RestPluginApi; + const change = {owner} as unknown as ChangeInfo; + + const service = OwnersService.getOwnersService( + changeOwnerLoggedInApi, + change + ); + const user = await service.getLoggedInUser(); + assert.equal(user.role, UserRole.CHANGE_OWNER); + assert.equal(user.account, owner); + }); + }); + + suite('files owners tests', () => { + teardown(() => { + sinon.restore(); + }); + + function setupRestApiForLoggedIn(loggedIn: boolean): RestPluginApi { + return { + getLoggedIn() { + return Promise.resolve(loggedIn); + }, + send( + _method: HttpMethod, + _url: string, + _payload?: RequestPayload, + _errFn?: ErrorCallback, + _contentType?: string + ) { + return Promise.resolve({}); + }, + } as unknown as RestPluginApi; + } + + const notLoggedInRestApiService = setupRestApiForLoggedIn(false); + + function loggedInRestApiService(acc: number): RestPluginApi { + return { + ...setupRestApiForLoggedIn(true), + getAccount() { + return Promise.resolve(account(acc)); + }, + } as unknown as RestPluginApi; + } + + let service: OwnersService; + let getApiStub: sinon.SinonStub; + + function setup( + loggedIn: boolean, + changeStats: ChangeStatus = ChangeStatus.NEW, + submitRequirementsSatisfied?: boolean, + response = {} + ) { + const acc = account(1); + const base_change = { + ...fakeChange, + _number: 1, + status: changeStats, + project: 'test_repo', + owner: acc, + } as unknown as ChangeInfo; + const change = + submitRequirementsSatisfied === undefined + ? base_change + : { + ...base_change, + submit_requirements: [ + { + name: 'Owner-Approval', + status: submitRequirementsSatisfied + ? 'SATISFIED' + : 'UNSATISFIED', + } as unknown as SubmitRequirementResultInfo, + ], + }; + const restApi = loggedIn + ? loggedInRestApiService(1) + : notLoggedInRestApiService; + + getApiStub = sinon.stub(restApi, 'send'); + getApiStub + .withArgs( + sinon.match.any, + `/changes/${change.project}~${change._number}/revisions/current/owners~files-owners`, + sinon.match.any, + sinon.match.any + ) + .returns(Promise.resolve(response)); + service = OwnersService.getOwnersService(restApi, change); + } + + const isLoggedIn = true; + const changeMerged = ChangeStatus.MERGED; + const changeNew = ChangeStatus.NEW; + const ownersSubmitRequirementsSatisfied = true; + + function setupGetAllFilesApproved_undefined() { + setup(!isLoggedIn); + } + + test('should have getAllFilesApproved `undefined` for no change owner', async () => { + setupGetAllFilesApproved_undefined(); + + const response = await service.getAllFilesApproved(); + assert.equal(response, undefined); + }); + + test('should have getAllFilesApproved `undefined` for `MERGED` change', async () => { + setup(isLoggedIn, changeMerged); + + const response = await service.getAllFilesApproved(); + assert.equal(response, undefined); + }); + + test('should have getAllFilesApproved `undefined` when no submit requirements', async () => { + setup(isLoggedIn, changeNew); + + const response = await service.getAllFilesApproved(); + assert.equal(response, undefined); + }); + + function setupGetAllFilesApproved_false(response = {}) { + setup( + isLoggedIn, + changeNew, + !ownersSubmitRequirementsSatisfied, + response + ); + } + + test('should have getAllFilesApproved `false` when no submit requirements are not satisfied', async () => { + setupGetAllFilesApproved_false(); + + const response = await service.getAllFilesApproved(); + assert.equal(response, false); + }); + + function setupGetAllFilesApproved_true() { + setup(isLoggedIn, changeNew, ownersSubmitRequirementsSatisfied); + } + test('should have getAllFilesApproved `true` when no submit requirements are satisfied', async () => { + setupGetAllFilesApproved_true(); + + const response = await service.getAllFilesApproved(); + assert.equal(response, true); + }); + + test('should not call getFilesOwners when getAllFilesApproved is `undefined`', async () => { + setupGetAllFilesApproved_undefined(); + + const response = await service.getFilesOwners(); + await flush(); + assert.equal(getApiStub.callCount, 0); + assert.equal(response, undefined); + }); + + test('should not call getFilesOwners when getAllFilesApproved is `true`', async () => { + setupGetAllFilesApproved_true(); + + const response = await service.getFilesOwners(); + await flush(); + assert.equal(getApiStub.callCount, 0); + assert.equal(response, undefined); + }); + + test('should call getFilesOwners when getAllFilesApproved is `false`', async () => { + const expected = { + files: { + 'AJavaFile.java': [{name: 'Bob', id: 1000001}], + 'Aptyhonfileroot.py': [ + {name: 'John', id: 1000002}, + {name: 'Bob', id: 1000001}, + {name: 'Jack', id: 1000003}, + ], + }, + owners_labels: { + '1000002': { + Verified: 1, + 'Code-Review': 0, + }, + '1000001': { + 'Code-Review': 2, + }, + }, + }; + setupGetAllFilesApproved_false(expected); + + const response = await service.getFilesOwners(); + await flush(); + assert.equal(getApiStub.callCount, 1); + assert.equal( + deepEqual(response, {...expected} as unknown as FilesOwners), + true + ); + }); + }); +}); + +function account(id: number): AccountInfo { + return { + _account_id: id, + } as unknown as AccountInfo; +} + +function flush() { + return new Promise((resolve, _reject) => { + setTimeout(resolve, 0); + }); +}
diff --git a/owners/web/plugin.ts b/owners/web/plugin.ts new file mode 100644 index 0000000..596c8ab --- /dev/null +++ b/owners/web/plugin.ts
@@ -0,0 +1,66 @@ +/** + * @license + * Copyright (C) 2024 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 '@gerritcodereview/typescript-api/gerrit'; +import { + FILES_OWNERS_COLUMN_CONTENT, + FILES_OWNERS_COLUMN_HEADER, + FilesColumnContent, + FilesColumnHeader, +} from './gr-files'; +import { + OWNED_FILES_TAB_CONTENT, + OWNED_FILES_TAB_HEADER, + OwnedFilesTabContent, + OwnedFilesTabHeader, +} from './gr-owned-files'; + +window.Gerrit.install(plugin => { + const restApi = plugin.restApi(); + + plugin + .registerDynamicCustomComponent( + 'change-view-file-list-header-prepend', + FILES_OWNERS_COLUMN_HEADER + ) + .onAttached(view => { + (view as unknown as FilesColumnHeader).restApi = restApi; + }); + plugin + .registerDynamicCustomComponent( + 'change-view-file-list-content-prepend', + FILES_OWNERS_COLUMN_CONTENT + ) + .onAttached(view => { + (view as unknown as FilesColumnContent).restApi = restApi; + }); + plugin + .registerDynamicCustomComponent( + 'change-view-tab-header', + OWNED_FILES_TAB_HEADER + ) + .onAttached(view => { + (view as unknown as OwnedFilesTabHeader).restApi = restApi; + }); + plugin + .registerDynamicCustomComponent( + 'change-view-tab-content', + OWNED_FILES_TAB_CONTENT + ) + .onAttached(view => { + (view as unknown as OwnedFilesTabContent).restApi = restApi; + }); +});
diff --git a/owners/web/test-utils.ts b/owners/web/test-utils.ts new file mode 100644 index 0000000..8f0c8a6 --- /dev/null +++ b/owners/web/test-utils.ts
@@ -0,0 +1,21 @@ +/** + * @license + * Copyright (C) 2024 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. + */ + +export function getRandom<T>(...values: T[]): T { + const idx = Math.floor(Math.random() * values.length); + return values[idx]; +}
diff --git a/owners/web/tsconfig.json b/owners/web/tsconfig.json new file mode 100644 index 0000000..0a15ca9 --- /dev/null +++ b/owners/web/tsconfig.json
@@ -0,0 +1,15 @@ +{ + "extends": "../../tsconfig-plugins-base.json", + "compilerOptions": { + "outDir": "../../../.ts-out/plugins/owners", /* overridden by bazel */ + "lib": [ + "dom", + "dom.iterable", + "es2021", + "webworker" + ], + }, + "include": [ + "**/*.ts" + ], +}
diff --git a/owners/web/utils.ts b/owners/web/utils.ts new file mode 100644 index 0000000..81e0adb --- /dev/null +++ b/owners/web/utils.ts
@@ -0,0 +1,222 @@ +/** + * @license + * Copyright (C) 2024 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 {SpecialFilePath} from './gerrit-model'; + +export function deepEqual<T>(a: T, b: T): boolean { + if (a === b) return true; + if (a === undefined || b === undefined) return false; + if (a === null || b === null) return false; + if (a instanceof Date || b instanceof Date) { + if (!(a instanceof Date && b instanceof Date)) return false; + return a.getTime() === b.getTime(); + } + + if (a instanceof Set || b instanceof Set) { + if (!(a instanceof Set && b instanceof Set)) return false; + if (a.size !== b.size) return false; + for (const ai of a) if (!b.has(ai)) return false; + return true; + } + if (a instanceof Map || b instanceof Map) { + if (!(a instanceof Map && b instanceof Map)) return false; + if (a.size !== b.size) return false; + for (const [aKey, aValue] of a.entries()) { + if (!b.has(aKey) || !deepEqual(aValue, b.get(aKey))) return false; + } + return true; + } + + if (typeof a === 'object') { + if (typeof b !== 'object') return false; + const aObj = a as Record<string, unknown>; + const bObj = b as Record<string, unknown>; + const aKeys = Object.keys(aObj); + const bKeys = Object.keys(bObj); + if (aKeys.length !== bKeys.length) return false; + for (const key of aKeys) { + if (!deepEqual(aObj[key], bObj[key])) return false; + } + return true; + } + + return false; +} + +/** + * Queries for child element specified with a selector. Copied from Gerrit's common-util.ts. + */ +export function query<E extends Element = Element>( + el: Element | null | undefined, + selector: string +): E | undefined { + if (!el) return undefined; + if (el.shadowRoot) { + const r = el.shadowRoot.querySelector<E>(selector); + if (r) return r; + } + return el.querySelector<E>(selector) ?? undefined; +} + +/** + * Copied from Gerrit's path-list-util.ts. + */ +export function computeDisplayPath(path?: string) { + if (path === SpecialFilePath.COMMIT_MESSAGE) { + return 'Commit message'; + } else if (path === SpecialFilePath.MERGE_LIST) { + return 'Merge list'; + } + return path ?? ''; +} + +/** + * Separates a path into: + * - The part that matches another path, + * - The part that does not match the other path, + * - The file name + * + * For example: + * diffFilePaths('same/part/new/part/foo.js', 'same/part/different/foo.js'); + * yields: { + * matchingFolders: 'same/part/', + * newFolders: 'new/part/', + * fileName: 'foo.js', + * } + * + * Copied from Gerrit's string-util.ts. + */ +export function diffFilePaths(filePath: string, otherFilePath?: string) { + // Separate each string into an array of folder names + file name. + const displayPath = computeDisplayPath(filePath); + const previousFileDisplayPath = computeDisplayPath(otherFilePath); + const displayPathParts = displayPath.split('/'); + const previousFileDisplayPathParts = previousFileDisplayPath.split('/'); + + // Construct separate strings for matching folders, new folders, and file + // name. + const firstDifferencePartIndex = displayPathParts.findIndex( + (part, index) => previousFileDisplayPathParts[index] !== part + ); + const matchingSection = displayPathParts + .slice(0, firstDifferencePartIndex) + .join('/'); + const newFolderSection = displayPathParts + .slice(firstDifferencePartIndex, -1) + .join('/'); + const fileNameSection = displayPathParts[displayPathParts.length - 1]; + + // Note: folder sections need '/' appended back. + return { + matchingFolders: matchingSection.length > 0 ? `${matchingSection}/` : '', + newFolders: newFolderSection.length > 0 ? `${newFolderSection}/` : '', + fileName: fileNameSection, + }; +} + +/** + * Truncates URLs to display filename only + * Example + * // returns '.../text.html' + * util.truncatePath.('dir/text.html'); + * Example + * // returns 'text.html' + * util.truncatePath.('text.html'); + * + * Copied from Gerrit's path-list-util.ts. + */ +export function truncatePath(path: string, threshold = 1) { + const pathPieces = path.split('/'); + + if (pathPieces.length <= threshold) { + return path; + } + + const index = pathPieces.length - threshold; + // Character is an ellipsis. + return `\u2026/${pathPieces.slice(index).join('/')}`; +} + +/** + * Encodes *parts* of a URL. See inline comments below for the details. + * Note specifically that ? & = # are encoded. So this is very close to + * encodeURIComponent() with some tweaks. + * + * Copied from Gerrit's url-util.ts. + */ +export function encodeURL(url: string): string { + // gr-page decodes the entire URL, and then decodes once more the + // individual regex matching groups. It uses `decodeURIComponent()`, which + // will choke on singular `%` chars without two trailing digits. We prefer + // to not double encode *everything* (just for readaiblity and simplicity), + // but `%` *must* be double encoded. + let output = url.replaceAll('%', '%25'); // `replaceAll` function requires `es2021` hence the `lib` section was added to `tsconfig.json` + // `+` also requires double encoding, because `%2B` would be decoded to `+` + // and then replaced by ` `. + output = output.replaceAll('+', '%2B'); + + // This escapes ALL characters EXCEPT: + // A–Z a–z 0–9 - _ . ! ~ * ' ( ) + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent + output = encodeURIComponent(output); + + // If we would use `encodeURI()` instead of `encodeURIComponent()`, then we + // would also NOT encode: + // ; / ? : @ & = + $ , # + // + // That would be more readable, but for example ? and & have special meaning + // in the URL, so they must be encoded. Let's discuss all these chars and + // decide whether we have to encode them or not. + // + // ? & = # have to be encoded. Otherwise we might mess up the URL. + // + // : @ do not have to be encoded, because we are only dealing with path, + // query and fragment of the URL, not with scheme, user, host, port. + // For search queries it is much nicer to not encode those chars, think of + // searching for `owner:spearce@spearce.org`. + // + // / does not have to be encoded, because we don't care about individual path + // components. File path and repo names are so much nicer to read without / + // being encoded! + // + // + must be encoded, because we want to use it instead of %20 for spaces, see + // below. + // + // ; $ , probably don't have to be encoded, but we don't bother about them + // much, so we don't reverse the encoding here, but we don't think it would + // cause any harm, if we did. + output = output.replace(/%3A/g, ':'); + output = output.replace(/%40/g, '@'); + output = output.replace(/%2F/g, '/'); + + // gr-page replaces `+` by ` ` in addition to calling `decodeURIComponent()`. + // So we can use `+` to increase readability. + output = output.replace(/%20/g, '+'); + + return output; +} + +/** + * Function to obtain the base URL. + * + * Copied from Gerrit's url-util.ts. + */ +export function getBaseUrl(): string { + // window is not defined in service worker, therefore no CANONICAL_PATH + if (typeof window === 'undefined') return ''; + return self.CANONICAL_PATH || ''; +}