Show which files are owned by the current user
Also move to @open-wc/testing. This change depends on
https://gerrit-review.googlesource.com/c/gerrit/+/345554
Google-Bug-Id: b/178474499
Screenshot: https://imgur.com/a/GcyNQpA
Release-Notes: skip
Change-Id: Ic06f4834e6959eea4dadfb5af2dedd550d1f2f10
diff --git a/web/code-owners-api.ts b/web/code-owners-api.ts
index 70573c1..117fe13 100644
--- a/web/code-owners-api.ts
+++ b/web/code-owners-api.ts
@@ -133,6 +133,21 @@
info: FetchedOwner;
status?: string;
}
+
+export interface OwnedPathInfo {
+ path: string;
+ owned?: boolean;
+}
+
+export interface OwnedChangedFileInfo {
+ new_path?: OwnedPathInfo;
+ old_path?: OwnedPathInfo;
+}
+
+export interface OwnedPathsInfo {
+ owned_changed_files?: Array<OwnedChangedFileInfo>;
+}
+
/**
* Responsible for communicating with the rest-api
*
@@ -145,7 +160,7 @@
* Send a get request and provides custom response-code handling
*/
private async get(url: string): Promise<unknown> {
- const errFn = (response?: Response|null, error?: Error) => {
+ const errFn = (response?: Response | null, error?: Error) => {
if (error) throw error;
if (response) throw new ResponseError(response);
throw new Error('Generic REST API error');
@@ -170,8 +185,18 @@
* https://gerrit.googlesource.com/plugins/code-owners/+/HEAD/resources/Documentation/rest-api.md#change-endpoints
*/
listOwnerStatus(changeId: NumericChangeId): Promise<CodeOwnerStatusInfo> {
- return this.get(`/changes/${changeId}/code_owners.status?limit=100000`) as
- Promise<CodeOwnerStatusInfo>;
+ return this.get(
+ `/changes/${changeId}/code_owners.status?limit=100000`
+ ) as Promise<CodeOwnerStatusInfo>;
+ }
+
+ /**
+ * Returns a promise fetching which files are owned by a given user.
+ */
+ listOwnedPaths(changeId: NumericChangeId, account: AccountInfo) {
+ return this.get(
+ `/changes/${changeId}/revisions/current/owned_paths?user=${account.email}&limit=10000`
+ ) as Promise<OwnedPathsInfo>;
}
/**
@@ -180,12 +205,15 @@
* @doc
* https://gerrit.googlesource.com/plugins/code-owners/+/HEAD/resources/Documentation/rest-api.md#list-code-owners-for-path-in-branch
*/
- listOwnersForPath(changeId: NumericChangeId, path: string, limit: number):
- Promise<CodeOwnersInfo> {
+ listOwnersForPath(
+ changeId: NumericChangeId,
+ path: string,
+ limit: number
+ ): Promise<CodeOwnersInfo> {
return this.get(
- `/changes/${changeId}/revisions/current/code_owners` +
- `/${encodeURIComponent(path)}?limit=${limit}&o=DETAILS`) as
- Promise<CodeOwnersInfo>;
+ `/changes/${changeId}/revisions/current/code_owners` +
+ `/${encodeURIComponent(path)}?limit=${limit}&o=DETAILS`
+ ) as Promise<CodeOwnersInfo>;
}
/**
@@ -196,9 +224,10 @@
*/
getConfigForPath(project: string, branch: string, path: string) {
return this.get(
- `/projects/${encodeURIComponent(project)}/` +
+ `/projects/${encodeURIComponent(project)}/` +
`branches/${encodeURIComponent(branch)}/` +
- `code_owners.config/${encodeURIComponent(path)}`);
+ `code_owners.config/${encodeURIComponent(path)}`
+ );
}
/**
@@ -209,11 +238,11 @@
*/
async getBranchConfig(project: RepoName, branch: BranchName) {
try {
- const config =
- (await this.get(
- `/projects/${encodeURIComponent(project)}/` +
- `branches/${encodeURIComponent(branch)}/` +
- `code_owners.branch_config`)) as CodeOwnerBranchConfigInfo;
+ const config = (await this.get(
+ `/projects/${encodeURIComponent(project)}/` +
+ `branches/${encodeURIComponent(branch)}/` +
+ `code_owners.branch_config`
+ )) as CodeOwnerBranchConfigInfo;
return config;
} catch (err) {
if (err instanceof ResponseError) {
@@ -249,11 +278,14 @@
private promises = new Map<string, Promise<unknown>>();
constructor(
- private readonly codeOwnerApi: CodeOwnersApi,
- private readonly change: ChangeInfo) {}
+ private readonly codeOwnerApi: CodeOwnersApi,
+ private readonly change: ChangeInfo
+ ) {}
- private fetchOnce(cacheKey: string, asyncFn: () => Promise<unknown>):
- Promise<unknown> {
+ private fetchOnce(
+ cacheKey: string,
+ asyncFn: () => Promise<unknown>
+ ): Promise<unknown> {
let promise = this.promises.get(cacheKey);
if (promise) return promise;
promise = asyncFn();
@@ -261,9 +293,10 @@
return promise;
}
- getAccount(): Promise<AccountDetailInfo|undefined> {
- return this.fetchOnce('getAccount', () => this.getAccountImpl()) as
- Promise<AccountDetailInfo|undefined>;
+ getAccount(): Promise<AccountDetailInfo | undefined> {
+ return this.fetchOnce('getAccount', () => this.getAccountImpl()) as Promise<
+ AccountDetailInfo | undefined
+ >;
}
private async getAccountImpl() {
@@ -273,17 +306,22 @@
}
listOwnerStatus(): Promise<CodeOwnerStatusInfo> {
- return this.fetchOnce(
- 'listOwnerStatus',
- () => this.codeOwnerApi.listOwnerStatus(this.change._number)) as
- Promise<CodeOwnerStatusInfo>;
+ return this.fetchOnce('listOwnerStatus', () =>
+ this.codeOwnerApi.listOwnerStatus(this.change._number)
+ ) as Promise<CodeOwnerStatusInfo>;
+ }
+
+ async listOwnedPaths(): Promise<OwnedPathsInfo | undefined> {
+ const account = await this.getAccount();
+ if (!account) return undefined;
+ return this.fetchOnce('listOwnedPaths', () =>
+ this.codeOwnerApi.listOwnedPaths(this.change._number, account)
+ ) as Promise<OwnedPathsInfo>;
}
getBranchConfig(): Promise<CodeOwnerBranchConfigInfo> {
- return this.fetchOnce(
- 'getBranchConfig',
- () => this.codeOwnerApi.getBranchConfig(
- this.change.project, this.change.branch)) as
- Promise<CodeOwnerBranchConfigInfo>;
+ return this.fetchOnce('getBranchConfig', () =>
+ this.codeOwnerApi.getBranchConfig(this.change.project, this.change.branch)
+ ) as Promise<CodeOwnerBranchConfigInfo>;
}
}
diff --git a/web/code-owners-model-loader.ts b/web/code-owners-model-loader.ts
index cecb57b..c9a79e9 100644
--- a/web/code-owners-model-loader.ts
+++ b/web/code-owners-model-loader.ts
@@ -83,6 +83,14 @@
);
}
+ async loadOwnedPaths() {
+ await this._loadProperty(
+ 'ownedPaths',
+ () => this.ownersService.getOwnedPaths(),
+ paths => this.ownersModel.setOwnedPaths(paths)
+ );
+ }
+
async loadUserRole() {
await this._loadProperty(
'userRole',
diff --git a/web/code-owners-model-mixin.ts b/web/code-owners-model-mixin.ts
index dbbb2f6..18f5e51 100644
--- a/web/code-owners-model-mixin.ts
+++ b/web/code-owners-model-mixin.ts
@@ -19,6 +19,7 @@
import {ModelLoader} from './code-owners-model-loader';
import {
CodeOwnersModel,
+ OwnedPathsInfoOpt,
PluginStatus,
Status,
SuggestionsState,
@@ -53,6 +54,7 @@
selectedSuggestionsFiles?: Array<FetchedFile>;
selectedSuggestionsState?: SuggestionsState;
selectedSuggestionsLoadProgress?: string;
+ ownedPaths?: OwnedPathsInfoOpt;
loadPropertiesAfterModelChanged(): void;
}
@@ -112,6 +114,9 @@
@state()
selectedSuggestionsLoadProgress?: string;
+ @state()
+ ownedPaths?: OwnedPathsInfoOpt;
+
private model_?: CodeOwnersModel;
private subscriptions: Array<Subscription> = [];
@@ -142,12 +147,12 @@
);
this.subscriptions.push(
model.state$.subscribe(s => {
- this.userRole = s.userRole;
+ this.status = s.status;
})
);
this.subscriptions.push(
model.state$.subscribe(s => {
- this.status = s.status;
+ this.userRole = s.userRole;
})
);
this.subscriptions.push(
@@ -180,6 +185,11 @@
this.selectedSuggestionsState = s;
})
);
+ this.subscriptions.push(
+ model.ownedPaths$.subscribe(s => {
+ this.ownedPaths = s;
+ })
+ );
this.loadPropertiesAfterModelChanged();
}
diff --git a/web/code-owners-model.ts b/web/code-owners-model.ts
index 3e8d8ff..324c3e2 100644
--- a/web/code-owners-model.ts
+++ b/web/code-owners-model.ts
@@ -23,6 +23,7 @@
FetchedFile,
FileCodeOwnerStatusInfo,
OwnerStatus,
+ OwnedPathsInfo,
} from './code-owners-api';
import {deepEqual} from './deep-util';
import {select} from './observable-util';
@@ -48,8 +49,9 @@
export function isPluginErrorState(state: PluginState) {
return (
- state === PluginState.ServerConfigurationError ||
- state === PluginState.Failed);
+ state === PluginState.ServerConfigurationError ||
+ state === PluginState.Failed
+ );
}
export enum SuggestionsType {
@@ -87,14 +89,19 @@
export interface FileStatus {
changeType: ChangeType;
status: OwnerStatus;
- newPath?: string|null;
- oldPath?: string|null;
+ newPath?: string | null;
+ oldPath?: string | null;
}
export const BestSuggestionsLimit = 5;
export const AllSuggestionsLimit = 1000;
-let codeOwnersModel: CodeOwnersModel|undefined;
+let codeOwnersModel: CodeOwnersModel | undefined;
+
+export interface OwnedPathsInfoOpt {
+ oldPaths: Set<string>;
+ newPaths: Set<string>;
+}
export interface CodeOwnersState {
showSuggestions: boolean;
@@ -102,6 +109,7 @@
suggestionsByTypes: Map<SuggestionsType, Suggestion>;
pluginStatus?: PluginStatus;
branchConfig?: CodeOwnerBranchConfigInfo;
+ ownedPaths?: OwnedPathsInfoOpt;
userRole?: UserRole;
areAllFilesApproved?: boolean;
status?: Status;
@@ -135,23 +143,28 @@
public showSuggestions$ = select(this.state$, state => state.showSuggestions);
- public selectedSuggestionsType$ =
- select(this.state$, state => state.selectedSuggestionsType);
+ public selectedSuggestionsType$ = select(
+ this.state$,
+ state => state.selectedSuggestionsType
+ );
public selectedSuggestionsFiles$ = select(
- this.state$,
- state =>
- state.suggestionsByTypes.get(state.selectedSuggestionsType)?.files);
+ this.state$,
+ state => state.suggestionsByTypes.get(state.selectedSuggestionsType)?.files
+ );
public selectedSuggestionsState$ = select(
- this.state$,
- state =>
- state.suggestionsByTypes.get(state.selectedSuggestionsType)!.state);
+ this.state$,
+ state => state.suggestionsByTypes.get(state.selectedSuggestionsType)!.state
+ );
public selectedSuggestionsLoadProgress$ = select(
- this.state$,
- state => state.suggestionsByTypes.get(state.selectedSuggestionsType)!
- .loadProgress);
+ this.state$,
+ state =>
+ state.suggestionsByTypes.get(state.selectedSuggestionsType)!.loadProgress
+ );
+
+ public ownedPaths$ = select(this.state$, state => state.ownedPaths);
constructor(readonly change: ChangeInfo) {
super();
@@ -229,8 +242,9 @@
}
private updateSuggestion(
- suggestionsType: SuggestionsType,
- suggestionsUpdate: Partial<Suggestion>) {
+ suggestionsType: SuggestionsType,
+ suggestionsUpdate: Partial<Suggestion>
+ ) {
const current = this.subject$.getValue();
const nextState = {
...current,
@@ -244,7 +258,9 @@
}
setSuggestionsFiles(
- suggestionsType: SuggestionsType, files: Array<FetchedFile>) {
+ suggestionsType: SuggestionsType,
+ files: Array<FetchedFile>
+ ) {
const current = this.subject$.getValue();
const suggestions = current.suggestionsByTypes.get(suggestionsType);
if (!suggestions) return;
@@ -254,7 +270,9 @@
}
setSuggestionsState(
- suggestionsType: SuggestionsType, state: SuggestionsState) {
+ suggestionsType: SuggestionsType,
+ state: SuggestionsState
+ ) {
const current = this.subject$.getValue();
const suggestions = current.suggestionsByTypes.get(suggestionsType);
if (!suggestions) return;
@@ -263,7 +281,9 @@
}
setSuggestionsLoadProgress(
- suggestionsType: SuggestionsType, loadProgress: string) {
+ suggestionsType: SuggestionsType,
+ loadProgress: string
+ ) {
const current = this.subject$.getValue();
const suggestions = current.suggestionsByTypes.get(suggestionsType);
if (!suggestions) return;
@@ -271,6 +291,25 @@
this.updateSuggestion(suggestionsType, {loadProgress});
}
+ setOwnedPaths(ownedPathsInfo: OwnedPathsInfo | undefined) {
+ const current = this.subject$.getValue();
+ const ownedPaths = {
+ oldPaths: new Set<string>(),
+ newPaths: new Set<string>(),
+ };
+ for (const changed_file of ownedPathsInfo?.owned_changed_files ?? []) {
+ if (changed_file.old_path?.owned)
+ ownedPaths.oldPaths.add(changed_file.old_path.path);
+ if (changed_file.new_path?.owned)
+ ownedPaths.newPaths.add(changed_file.new_path.path);
+ }
+ const nextState = {
+ ...current,
+ ownedPaths,
+ };
+ this.setState(nextState);
+ }
+
setPluginEnabled(enabled: boolean) {
this._setPluginStatus({
state: enabled ? PluginState.Enabled : PluginState.Disabled,
@@ -289,14 +328,16 @@
}
_arePluginStatusesEqual(
- status1: PluginStatus|undefined, status2: PluginStatus|undefined) {
+ status1: PluginStatus | undefined,
+ status2: PluginStatus | undefined
+ ) {
if (status1 === undefined || status2 === undefined) {
return status1 === status2;
}
if (status1.state !== status2.state) return false;
- return isPluginErrorState(status1.state) ?
- status1.failedMessage === status2.failedMessage :
- true;
+ return isPluginErrorState(status1.state)
+ ? status1.failedMessage === status2.failedMessage
+ : true;
}
static getModel(change: ChangeInfo) {
diff --git a/web/code-owners-service.ts b/web/code-owners-service.ts
index a7206f0..599c49b 100644
--- a/web/code-owners-service.ts
+++ b/web/code-owners-service.ts
@@ -95,14 +95,18 @@
}
/**
+ * Fetch the account.
+ */
+ getAccount(): Promise<AccountDetailInfo | undefined> {
+ return this.codeOwnersCacheApi.getAccount();
+ }
+
+ /**
* Prefetch data
*/
async prefetch() {
try {
- await Promise.all([
- this.codeOwnersCacheApi.getAccount(),
- this.getStatus(),
- ]);
+ await Promise.all([this.getAccount(), this.getStatus()]);
} catch {
// Ignore any errors during prefetch.
// The same call from a different place throws the same exception
@@ -117,7 +121,7 @@
* role 'REVIEWER' remains unchanged until the change view is reloaded.
*/
async getLoggedInUserInitialRole(): Promise<UserRole> {
- const account = await this.codeOwnersCacheApi.getAccount();
+ const account = await this.getAccount();
if (!account) {
return UserRole.ANONYMOUS;
}
@@ -141,18 +145,18 @@
return UserRole.CHANGE_OWNER;
}
if (change.reviewers) {
- if (this._accountInReviewers(change.reviewers.REVIEWER, account)) {
+ if (this.accountInReviewers(change.reviewers.REVIEWER, account)) {
return UserRole.REVIEWER;
- } else if (this._accountInReviewers(change.reviewers.CC, account)) {
+ } else if (this.accountInReviewers(change.reviewers.CC, account)) {
return UserRole.CC;
- } else if (this._accountInReviewers(change.reviewers.REMOVED, account)) {
+ } else if (this.accountInReviewers(change.reviewers.REMOVED, account)) {
return UserRole.REMOVED_REVIEWER;
}
}
return UserRole.OTHER;
}
- _accountInReviewers(
+ private accountInReviewers(
reviewers: AccountInfo[] | undefined,
account: AccountDetailInfo
) {
@@ -222,6 +226,13 @@
}
/**
+ * Gets which files are owned by the given user.
+ */
+ async getOwnedPaths() {
+ return this.codeOwnersCacheApi.listOwnedPaths();
+ }
+
+ /**
* Gets owner suggestions.
*/
async getSuggestedOwners(suggestionsType: SuggestionsType) {
diff --git a/web/code-owners-service_test.ts b/web/code-owners-service_test.ts
index e764de5..b1648a3 100644
--- a/web/code-owners-service_test.ts
+++ b/web/code-owners-service_test.ts
@@ -1,4 +1,3 @@
-import './test-setup.js';
import {CodeOwnerService} from './code-owners-service.js';
import {
RequestPayload,
@@ -9,6 +8,7 @@
HttpMethod,
} from '@gerritcodereview/typescript-api/rest-api.js';
import {SuggestionsType} from './code-owners-model.js';
+import {assert} from '@open-wc/testing';
function flush() {
return new Promise((resolve, _reject) => {
diff --git a/web/owner-status-column.ts b/web/owner-status-column.ts
index e426ce0..2ec88ef 100644
--- a/web/owner-status-column.ts
+++ b/web/owner-status-column.ts
@@ -56,7 +56,7 @@
[STATUS_CODE.PENDING]: 'schedule',
[STATUS_CODE.MISSING]: 'close',
[STATUS_CODE.PENDING_OLD_PATH]: 'schedule',
- [STATUS_CODE.MISSING_OLD_PATH]: ':close',
+ [STATUS_CODE.MISSING_OLD_PATH]: 'close',
[STATUS_CODE.APPROVED]: 'check',
[STATUS_CODE.ERROR]: 'info',
};
@@ -73,6 +73,17 @@
[STATUS_CODE.ERROR_OLD_PATH]: 'Failed to fetch code owner status',
};
+export function hasPath(ownedPaths: Set<string>, path: string | undefined) {
+ if (!path) return false;
+ if (path.charAt(0) === '/') {
+ if (ownedPaths.has(path)) return true;
+ } else {
+ // NOTE: The backend returns absolute paths.
+ if (ownedPaths.has('/' + path)) return true;
+ }
+ return false;
+}
+
const base = CodeOwnersModelMixin(LitElement);
class BaseEl extends base {
@@ -160,7 +171,7 @@
return [
css`
:host {
- display: block;
+ display: flex;
padding-right: var(--spacing-m);
width: 3em;
text-align: center;
@@ -168,14 +179,14 @@
gr-icon {
padding: var(--spacing-xs) 0px;
}
- :host([owner-status='approved']) gr-icon {
+ :host([owner-status='approved']) gr-icon.status {
color: var(--positive-green-text-color);
}
- :host([owner-status='pending']) gr-icon {
+ :host([owner-status='pending']) gr-icon.status {
color: #ffa62f;
}
- :host([owner-status='missing']) gr-icon,
- :host([owner-status='error']) gr-icon {
+ :host([owner-status='missing']) gr-icon.status,
+ :host([owner-status='error']) gr-icon.status {
color: var(--negative-red-text-color);
}
`,
@@ -189,19 +200,42 @@
override render() {
if (this.computeHidden() || this.status === undefined) return nothing;
+ return html`${this.renderStatus()}${this.renderOwnership()}`;
+ }
+ private renderStatus() {
const statusInfo = this.computeTooltip();
- const statusIcon = this.computeIcon();
+ const statusIcon = this.computeStatusIcon();
return html`
<gr-tooltip-content title=${statusInfo} has-tooltip>
- <gr-icon icon=${statusIcon}></gr-icon>
+ <gr-icon class="status" icon=${statusIcon}></gr-icon>
</gr-tooltip-content>
`;
}
+ private renderOwnership() {
+ if (!this.isOwned()) return nothing;
+ return html`
+ <gr-tooltip-content title="You own this file" has-tooltip>
+ <gr-icon filled icon="policy"></gr-icon>
+ </gr-tooltip-content>
+ `;
+ }
+
+ private isOwned() {
+ if (!this.ownedPaths) return false;
+ if (
+ hasPath(this.ownedPaths.newPaths, this.path) ||
+ hasPath(this.ownedPaths.oldPaths, this.oldPath)
+ )
+ return true;
+ return false;
+ }
+
override loadPropertiesAfterModelChanged() {
super.loadPropertiesAfterModelChanged();
this.modelLoader?.loadStatus();
+ this.modelLoader?.loadOwnedPaths();
}
private computeStatus() {
@@ -237,7 +271,7 @@
);
}
- private computeIcon() {
+ private computeStatusIcon() {
return STATUS_ICON[this.ownerStatus!];
}
diff --git a/web/owner-status-column_test.ts b/web/owner-status-column_test.ts
new file mode 100644
index 0000000..2fd223a
--- /dev/null
+++ b/web/owner-status-column_test.ts
@@ -0,0 +1,10 @@
+import {assert} from '@open-wc/testing';
+
+import {hasPath} from './owner-status-column';
+
+suite('owner-status-column', () => {
+ test('hasPath', () => {
+ assert(hasPath(new Set(['/COMMIT']), '/COMMIT'));
+ assert(hasPath(new Set(['/some/file']), 'some/file'));
+ });
+});
diff --git a/web/test-setup.ts b/web/test-setup.ts
deleted file mode 100644
index 2a08ab1..0000000
--- a/web/test-setup.ts
+++ /dev/null
@@ -1,14 +0,0 @@
-import 'chai/chai';
-declare global {
- interface Window {
- assert: typeof chai.assert;
- expect: typeof chai.expect;
- sinon: typeof sinon;
- }
- let assert: typeof chai.assert;
- let expect: typeof chai.expect;
- let sinon: typeof sinon;
-}
-window.assert = chai.assert;
-window.expect = chai.expect;
-window.sinon = sinon;
