Merge changes from topic "show_all_owners"
* changes:
Do not place all owners on a new line if number of all owners is small
Pause/resume owners loading when switching between different owners
Add "Show all owners" option
diff --git a/ui/code-owners-api.js b/ui/code-owners-api.js
new file mode 100644
index 0000000..408428b
--- /dev/null
+++ b/ui/code-owners-api.js
@@ -0,0 +1,183 @@
+/**
+ * @license
+ * Copyright (C) 2021 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.
+ */
+
+
+// TODO: Try to remove it. The ResponseError and getErrorMessage duplicates
+// code from the gr-plugin-rest-api.ts. This code is required because
+// we want custom error processing in some functions. For details see
+// the original gr-plugin-rest-api.ts file/
+
+class ResponseError extends Error {
+ constructor(response) {
+ super();
+ this.response = response;
+ }
+}
+
+async function getErrorMessage(response) {
+ const text = await response.text();
+ return text ?
+ `${response.status}: ${text}` :
+ `${response.status}`;
+}
+
+/**
+ * Responsible for communicating with the rest-api
+ *
+ * @see resources/Documentation/rest-api.md
+ */
+export class CodeOwnersApi {
+ constructor(restApi) {
+ this.restApi = restApi;
+ }
+
+ /**
+ * Returns a promise fetching the owner statuses for all files within the change.
+ *
+ * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#change-endpoints
+ * @param {string} changeId
+ */
+ listOwnerStatus(changeId) {
+ return this.restApi.get(`/changes/${changeId}/code_owners.status`);
+ }
+
+ /**
+ * Returns a promise fetching the owners for a given path.
+ *
+ * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#list-code-owners-for-path-in-branch
+ * @param {string} changeId
+ * @param {string} path
+ */
+ listOwnersForPath(changeId, path, limit) {
+ return this.restApi.get(
+ `/changes/${changeId}/revisions/current/code_owners` +
+ `/${encodeURIComponent(path)}?limit=${limit}&o=DETAILS`
+ );
+ }
+
+ /**
+ * Returns a promise fetching the owners config for a given path.
+ *
+ * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#branch-endpoints
+ * @param {string} project
+ * @param {string} branch
+ * @param {string} path
+ */
+ getConfigForPath(project, branch, path) {
+ return this.restApi.get(
+ `/projects/${encodeURIComponent(project)}/` +
+ `branches/${encodeURIComponent(branch)}/` +
+ `code_owners.config/${encodeURIComponent(path)}`
+ );
+ }
+
+ /**
+ * Returns a promise fetching the owners config for a given branch.
+ *
+ * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#branch-endpoints
+ * @param {string} project
+ * @param {string} branch
+ */
+ async getBranchConfig(project, branch) {
+ const errFn = (response, error) => {
+ if (error) throw error;
+ if (response) throw new ResponseError(response);
+ throw new Error('Generic REST API error');
+ }
+ try {
+ const config = await this.restApi.send(
+ 'GET',
+ `/projects/${encodeURIComponent(project)}/` +
+ `branches/${encodeURIComponent(branch)}/` +
+ `code_owners.branch_config`,
+ undefined,
+ errFn
+ );
+ if (config.override_approval && !(config.override_approval
+ instanceof Array)) {
+ // In the upcoming backend changes, the override_approval will be changed
+ // to array with (possible) multiple items.
+ // While this transition is in progress, the frontend supports both API -
+ // the old one and the new one.
+ return {...config, override_approval: [config.override_approval]};
+ }
+ return config;
+ } catch(err) {
+ if (err instanceof ResponseError) {
+ if (err.response.status === 404) {
+ // The 404 error means that the branch doesn't exist and
+ // the plugin should be disabled.
+ return {disabled: true};
+ }
+ return getErrorMessage(err.response).then(msg => {
+ throw new Error(msg);
+ });
+ }
+ throw err;
+ }
+ }
+}
+
+/**
+ * Wrapper around codeOwnerApi, sends each requests only once and then cache
+ * the response. A new CodeOwnersCacheApi instance is created every time when a
+ * new change object is assigned.
+ * Gerrit never updates existing change object, but instead always assigns a new
+ * change object. Particularly, a new change object is assigned when a change
+ * is updated and user clicks reload toasts to see the updated change.
+ * As a result, the lifetime of a cache is the same as a lifetime of an assigned
+ * change object.
+ * Periodical cache invalidation can lead to inconsistency in UI, i.e.
+ * user can see the old reviewers list (reflects a state when a change was
+ * loaded) and code-owners status for the current reviewer list. To avoid
+ * this inconsistency, the cache doesn't invalidate.
+ */
+export class CodeOwnersCacheApi {
+ constructor(codeOwnerApi, change) {
+ this.codeOwnerApi = codeOwnerApi;
+ this.change = change;
+ this.promises = {};
+ }
+
+ _fetchOnce(cacheKey, asyncFn) {
+ if (!this.promises[cacheKey]) {
+ this.promises[cacheKey] = asyncFn();
+ }
+ return this.promises[cacheKey];
+ }
+
+ getAccount() {
+ return this._fetchOnce('getAccount', () => this._getAccount());
+ }
+
+ async _getAccount() {
+ const loggedIn = await this.codeOwnerApi.restApi.getLoggedIn();
+ if (!loggedIn) return undefined;
+ return await this.codeOwnerApi.restApi.getAccount();
+ }
+
+ listOwnerStatus() {
+ return this._fetchOnce('listOwnerStatus',
+ () => this.codeOwnerApi.listOwnerStatus(this.change._number));
+ }
+
+ getBranchConfig() {
+ return this._fetchOnce('getBranchConfig',
+ () => this.codeOwnerApi.getBranchConfig(this.change.project,
+ this.change.branch));
+ }
+}
diff --git a/ui/code-owners-fetcher.js b/ui/code-owners-fetcher.js
new file mode 100644
index 0000000..61e362b
--- /dev/null
+++ b/ui/code-owners-fetcher.js
@@ -0,0 +1,204 @@
+/**
+ * @license
+ * Copyright (C) 2021 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 {CodeOwnersApi} from './code-owners-api.js';
+
+/**
+ * All statuses returned for owner status.
+ *
+ * @enum
+ */
+export const OwnerStatus = {
+ INSUFFICIENT_REVIEWERS: 'INSUFFICIENT_REVIEWERS',
+ PENDING: 'PENDING',
+ APPROVED: 'APPROVED',
+};
+
+/**
+ * @enum
+ */
+export const FetchStatus = {
+ /** Fetch hasn't been started */
+ NOT_STARTED: 0,
+ /**
+ * Fetch has been started, but not all files has been finished.
+ * Pausing during fetching doesn't change state.
+ */
+ FETCHING: 1,
+ /**
+ * All owners has been loaded. resume/pause call doesn't change state.
+ */
+ FINISHED: 2,
+};
+
+/**
+ * Fetch owners for files. The class fetches owners in parallel and allows to
+ * pause/resume fetch.
+ */
+class OwnersFetcher {
+ /**
+ * Creates a fetcher in paused state. Actual fetching starts after resume()
+ * is called.
+ *
+ * @param {Array<string>} filesToFetch - Files paths for loading owners.
+ * @param {number} ownersLimit - number of requested owners per file.
+ * @param {number} maxConcurrentRequest - max number of concurrent requests to server.
+ */
+ constructor(codeOwnerApi, changeId, filesToFetch, ownersLimit,
+ maxConcurrentRequest) {
+ this._fetchedOwners = new Map();
+ this._ownersLimit = ownersLimit;
+ this._paused = true;
+ this._pausedFilesFetcher = [];
+ this._filesToFetch = filesToFetch;
+ this._fetchFilesPromises = [];
+ this._codeOwnerApi = codeOwnerApi;
+ this._changeId = changeId;
+
+ for (let i = 0; i < maxConcurrentRequest; i++) {
+ this._fetchFilesPromises.push(this._fetchFiles());
+ }
+ }
+
+ async _fetchFiles() {
+ for (;;) {
+ const filePath = await this._getNextFilePath();
+ if (!filePath) return;
+ try {
+ this._fetchedOwners.set(filePath, {
+ owners: await this._codeOwnerApi.listOwnersForPath(this._changeId,
+ filePath, this._ownersLimit),
+ });
+ } catch (error) {
+ this._fetchedOwners.set(filePath, {error});
+ }
+ }
+ }
+
+ async _getNextFilePath() {
+ if (this._paused) {
+ await new Promise(resolve => this._pausedFilesFetcher.push(resolve));
+ }
+ if (this._filesToFetch.length === 0) return null;
+ return this._filesToFetch.splice(0, 1)[0];
+ }
+
+ async waitFetchComplete() {
+ await Promise.allSettled(this._fetchFilesPromises);
+ }
+
+ resume() {
+ if (!this._paused) return;
+ this._paused = false;
+ for (const fetcher of this._pausedFilesFetcher.splice(0,
+ this._pausedFilesFetcher.length)) {
+ fetcher();
+ }
+ }
+
+ pause() {
+ this._paused = true;
+ }
+
+ getFetchedOwners() {
+ return this._fetchedOwners;
+ }
+
+ getFiles() {
+ const result = [];
+ for (const [path, info] of this._fetchedOwners.entries()) {
+ result.push({path, info});
+ }
+ return result;
+ }
+}
+
+export class OwnersProvider {
+ constructor(restApi, change, options) {
+ this.change = change;
+ this.options = options;
+ this._totalFetchCount = 0;
+ this._status = FetchStatus.NOT_STARTED;
+ this._codeOwnerApi = new CodeOwnersApi(restApi);
+ }
+
+ getStatus() {
+ return this._status;
+ }
+
+ getProgressString() {
+ return !this._ownersFetcher || this._totalFetchCount === 0 ?
+ `Loading suggested owners ...` :
+ `${this._ownersFetcher.getFetchedOwners().size} out of ` +
+ `${this._totalFetchCount} files have returned suggested owners.`;
+ }
+
+ getFiles() {
+ if (!this._ownersFetcher) return [];
+ return this._ownersFetcher.getFiles();
+ }
+
+ async fetchSuggestedOwners(codeOwnerStatusMap) {
+ if (this._status !== FetchStatus.NOT_STARTED) {
+ await this._ownersFetcher.waitFetchComplete();
+ return;
+ }
+ const filesToFetch = this._getFilesToFetch(codeOwnerStatusMap);
+ this._totalFetchCount = filesToFetch.length;
+ this._ownersFetcher = new OwnersFetcher(this._codeOwnerApi, this.change.id,
+ filesToFetch,
+ this.options.ownersLimit, this.options.maxConcurrentRequests);
+ this._status = FetchStatus.FETCHING;
+ this._ownersFetcher.resume();
+ await this._ownersFetcher.waitFetchComplete(filesToFetch);
+ this._status = FetchStatus.FINISHED;
+ }
+
+ _getFilesToFetch(codeOwnerStatusMap) {
+ // only fetch those not approved yet
+ const filesGroupByStatus = [...codeOwnerStatusMap.keys()].reduce(
+ (list, file) => {
+ const status = codeOwnerStatusMap
+ .get(file).status;
+ if (status === OwnerStatus.INSUFFICIENT_REVIEWERS) {
+ list.missing.push(file);
+ } else if (status === OwnerStatus.PENDING) {
+ list.pending.push(file);
+ }
+ return list;
+ }
+ , {pending: [], missing: []});
+ // always fetch INSUFFICIENT_REVIEWERS first and then pending
+ return filesGroupByStatus.missing.concat(filesGroupByStatus.pending);
+ }
+
+ pause() {
+ if (!this._ownersFetcher) return;
+ this._ownersFetcher.pause();
+ }
+
+ resume() {
+ if (!this._ownersFetcher) return;
+ this._ownersFetcher.resume();
+ }
+
+ reset() {
+ this._totalFetchCount = 0;
+ this.ownersFetcher = null;
+ this._status = FetchStatus.NOT_STARTED;
+ }
+}
diff --git a/ui/code-owners-model-loader.js b/ui/code-owners-model-loader.js
index 2cd9095..2fa18ac 100644
--- a/ui/code-owners-model-loader.js
+++ b/ui/code-owners-model-loader.js
@@ -39,6 +39,7 @@
try {
newValue = await propertyLoader();
} catch (e) {
+ console.error(e);
this.ownersModel.setPluginFailed(e.message);
return;
}
@@ -84,33 +85,62 @@
);
}
- async loadSuggestions() {
- // If a loading has been started already, do nothing
- if (this.ownersModel.suggestionsState
- !== SuggestionsState.NotLoaded) return;
-
- this.ownersModel.setSuggestionsState(SuggestionsState.Loading);
- let suggestedOwners;
- try {
- suggestedOwners = await this.ownersService.getSuggestedOwners();
- } catch (e) {
- this.ownersModel.setSuggestionsState(SuggestionsState.LoadFailed);
- this.ownersModel.setPluginFailed(e.message);
+ async loadSuggestions(suggestionsType) {
+ this.pauseActiveSuggestedOwnersLoading();
+ this.activeLoadSuggestionType = suggestionsType;
+ if (this.ownersModel.suggestionsByTypes[suggestionsType].state ===
+ SuggestionsState.Loading) {
+ this.ownersService.resumeSuggestedOwnersLoading(suggestionsType);
return;
}
- this.ownersModel.setSuggestions(suggestedOwners.suggestions);
- this.ownersModel.setSuggestionsState(SuggestionsState.Loaded);
- }
- async updateLoadSuggestionsProgress() {
+ // If a loading has been started already, do nothing
+ if (this.ownersModel.suggestionsByTypes[suggestionsType].state !==
+ SuggestionsState.NotLoaded) return;
+
+ this.ownersModel.setSuggestionsState(suggestionsType,
+ SuggestionsState.Loading);
let suggestedOwners;
try {
- suggestedOwners = await this.ownersService.getSuggestedOwnersProgress();
+ suggestedOwners =
+ await this.ownersService.getSuggestedOwners(suggestionsType);
+ } catch (e) {
+ console.error(e);
+ this.ownersModel.setSuggestionsState(suggestionsType,
+ SuggestionsState.LoadFailed);
+ // The selectedSuggestionsType can be changed while getSuggestedOwners
+ // is executed. The plugin should fail only if the selectedSuggestionsType
+ // is the same.
+ if (this.ownersModel.selectedSuggestionsType === suggestionsType) {
+ this.ownersModel.setPluginFailed(e.message);
+ }
+ return;
+ }
+ this.ownersModel.setSuggestionsFiles(suggestionsType,
+ suggestedOwners.files);
+ this.ownersModel.setSuggestionsState(suggestionsType,
+ SuggestionsState.Loaded);
+ }
+
+ pauseActiveSuggestedOwnersLoading() {
+ if (!this.activeLoadSuggestionType) return;
+ this.ownersService.pauseSuggestedOwnersLoading(
+ this.activeLoadSuggestionType);
+ }
+
+ async updateLoadSelectedSuggestionsProgress() {
+ const suggestionsType = this.ownersModel.selectedSuggestionsType;
+ let suggestedOwners;
+ try {
+ suggestedOwners =
+ await this.ownersService.getSuggestedOwnersProgress(suggestionsType);
} catch {
// Ignore any error, keep progress unchanged.
return;
}
- this.ownersModel.setSuggestionsLoadProgress(suggestedOwners.progress);
- this.ownersModel.setSuggestions(suggestedOwners.suggestions);
+ this.ownersModel.setSuggestionsLoadProgress(suggestionsType,
+ suggestedOwners.progress);
+ this.ownersModel.setSuggestionsFiles(suggestionsType,
+ suggestedOwners.files);
}
}
diff --git a/ui/code-owners-model-mixin.js b/ui/code-owners-model-mixin.js
index 722af6f..80c8ffb 100644
--- a/ui/code-owners-model-mixin.js
+++ b/ui/code-owners-model-mixin.js
@@ -37,6 +37,7 @@
*/
this.modelLoader = undefined;
}
+
static get properties() {
return {
/* The following 3 properties (change, reporting, restApi) have to be
diff --git a/ui/code-owners-model.js b/ui/code-owners-model.js
index cbd9bd2..079eb86 100644
--- a/ui/code-owners-model.js
+++ b/ui/code-owners-model.js
@@ -28,6 +28,14 @@
Failed: 'Failed',
};
+export const SuggestionsType = {
+ BEST_SUGGESTIONS: 'BEST_SUGGESTIONS',
+ ALL_SUGGESTIONS: 'ALL_SUGGESTIONS',
+};
+
+export const BestSuggestionsLimit = 5;
+export const AllSuggestionsLimit = 1000;
+
/**
* Maintain the state of code-owners.
* Raises 'model-property-changed' event when a property is changed.
@@ -54,13 +62,23 @@
this.userRole = undefined;
this.isCodeOwnerEnabled = undefined;
this.areAllFilesApproved = undefined;
- this.suggestions = undefined;
- this.suggestionsState = SuggestionsState.NotLoaded;
- this.suggestionsLoadProgress = undefined;
+ this.suggestionsByTypes = {};
+ for (const suggestionType of Object.values(SuggestionsType)) {
+ this.suggestionsByTypes[suggestionType] = {
+ files: undefined,
+ state: SuggestionsState.NotLoaded,
+ loadProgress: undefined,
+ };
+ }
+ this.selectedSuggestionsType = SuggestionsType.BEST_SUGGESTIONS;
this.showSuggestions = false;
this.pluginStatus = undefined;
}
+ get selectedSuggestions() {
+ return this.suggestionsByTypes[this.selectedSuggestionsType];
+ }
+
setBranchConfig(config) {
if (this.branchConfig === config) return;
this.branchConfig = config;
@@ -91,22 +109,32 @@
this._firePropertyChanged('areAllFilesApproved');
}
- setSuggestions(suggestions) {
- if (this.suggestions === suggestions) return;
- this.suggestions = suggestions;
- this._firePropertyChanged('suggestions');
+ setSuggestionsFiles(suggestionsType, files) {
+ const suggestions = this.suggestionsByTypes[suggestionsType];
+ if (suggestions.files === files) return;
+ suggestions.files = files;
+ this._fireSuggestionsChanged(suggestionsType, 'files');
}
- setSuggestionsState(state) {
- if (this.suggestionsState === state) return;
- this.suggestionsState = state;
- this._firePropertyChanged('suggestionsState');
+ setSuggestionsState(suggestionsType, state) {
+ const suggestions = this.suggestionsByTypes[suggestionsType];
+ if (suggestions.state === state) return;
+ suggestions.state = state;
+ this._fireSuggestionsChanged(suggestionsType, 'state');
}
- setSuggestionsLoadProgress(progress) {
- if (this.suggestionsLoadProgress === progress) return;
- this.suggestionsLoadProgress = progress;
- this._firePropertyChanged('suggestionsLoadProgress');
+ setSuggestionsLoadProgress(suggestionsType, progress) {
+ const suggestions = this.suggestionsByTypes[suggestionsType];
+ if (suggestions.loadProgress === progress) return;
+ suggestions.loadProgress = progress;
+ this._fireSuggestionsChanged(suggestionsType, 'loadProgress');
+ }
+
+ setSelectedSuggestionType(suggestionsType) {
+ if (this.selectedSuggestionsType === suggestionsType) return;
+ this.selectedSuggestionsType = suggestionsType;
+ this._firePropertyChanged('selectedSuggestionsType');
+ this._firePropertyChanged('selectedSuggestions');
}
setShowSuggestions(show) {
@@ -148,6 +176,14 @@
}));
}
+ _fireSuggestionsChanged(suggestionsType, propertyName) {
+ this._firePropertyChanged(
+ `suggestionsByTypes.${suggestionsType}.${propertyName}`);
+ if (suggestionsType === this.selectedSuggestionsType) {
+ this._firePropertyChanged(`selectedSuggestions.${propertyName}`);
+ }
+ }
+
static getModel(change) {
if (!this.model || this.model.change !== change) {
this.model = new CodeOwnersModel(change);
diff --git a/ui/code-owners-service.js b/ui/code-owners-service.js
index 633ffbc..0db6226 100644
--- a/ui/code-owners-service.js
+++ b/ui/code-owners-service.js
@@ -15,26 +15,9 @@
* limitations under the License.
*/
-/**
- * All statuses returned for owner status.
- *
- * @enum
- */
-export const OwnerStatus = {
- INSUFFICIENT_REVIEWERS: 'INSUFFICIENT_REVIEWERS',
- PENDING: 'PENDING',
- APPROVED: 'APPROVED',
-};
-
-/**
- * @enum
- */
-const FetchStatus = {
- NOT_STARTED: 0,
- FETCHING: 1,
- FINISHED: 2,
- ABORT: 3,
-};
+import {SuggestionsType, BestSuggestionsLimit, AllSuggestionsLimit} from './code-owners-model.js';
+import {OwnersProvider, OwnerStatus, FetchStatus} from './code-owners-fetcher.js';
+import {CodeOwnersApi, CodeOwnersCacheApi} from './code-owners-api.js';
/**
* Specifies status for a change. The same as ChangeStatus enum in gerrit
@@ -60,278 +43,6 @@
OTHER: 'OTHER',
};
-// TODO: Try to remove it. The ResponseError and getErrorMessage duplicates
-// code from the gr-plugin-rest-api.ts. This code is required because
-// we want custom error processing in some functions. For details see
-// the original gr-plugin-rest-api.ts file/
-
-class ResponseError extends Error {
- constructor(response) {
- super();
- this.response = response;
- }
-}
-
-async function getErrorMessage(response) {
- const text = await response.text();
- return text ?
- `${response.status}: ${text}` :
- `${response.status}`;
-}
-
-/**
- * Responsible for communicating with the rest-api
- *
- * @see resources/Documentation/rest-api.md
- */
-class CodeOwnerApi {
- constructor(restApi) {
- this.restApi = restApi;
- }
-
- /**
- * Returns a promise fetching the owner statuses for all files within the change.
- *
- * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#change-endpoints
- * @param {string} changeId
- */
- listOwnerStatus(changeId) {
- return this.restApi.get(`/changes/${changeId}/code_owners.status`);
- }
-
- /**
- * Returns a promise fetching the owners for a given path.
- *
- * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#list-code-owners-for-path-in-branch
- * @param {string} changeId
- * @param {string} path
- */
- listOwnersForPath(changeId, path) {
- return this.restApi.get(
- `/changes/${changeId}/revisions/current/code_owners` +
- `/${encodeURIComponent(path)}?limit=5&o=DETAILS`
- );
- }
-
- /**
- * Returns a promise fetching the owners config for a given path.
- *
- * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#branch-endpoints
- * @param {string} project
- * @param {string} branch
- * @param {string} path
- */
- getConfigForPath(project, branch, path) {
- return this.restApi.get(
- `/projects/${encodeURIComponent(project)}/` +
- `branches/${encodeURIComponent(branch)}/` +
- `code_owners.config/${encodeURIComponent(path)}`
- );
- }
-
- /**
- * Returns a promise fetching the owners config for a given branch.
- *
- * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#branch-endpoints
- * @param {string} project
- * @param {string} branch
- */
- async getBranchConfig(project, branch) {
- const errFn = (response, error) => {
- if (error) throw error;
- if (response) throw new ResponseError(response);
- throw new Error('Generic REST API error');
- }
- try {
- const config = await this.restApi.send(
- 'GET',
- `/projects/${encodeURIComponent(project)}/` +
- `branches/${encodeURIComponent(branch)}/` +
- `code_owners.branch_config`,
- undefined,
- errFn
- );
- if (config.override_approval && !(config.override_approval
- instanceof Array)) {
- // In the upcoming backend changes, the override_approval will be changed
- // to array with (possible) multiple items.
- // While this transition is in progress, the frontend supports both API -
- // the old one and the new one.
- return {...config, override_approval: [config.override_approval]};
- }
- return config;
- } catch(err) {
- if (err instanceof ResponseError) {
- if (err.response.status === 404) {
- // The 404 error means that the branch doesn't exist and
- // the plugin should be disabled.
- return {disabled: true};
- }
- return getErrorMessage(err.response).then(msg => {
- throw new Error(msg);
- });
- }
- throw err;
- }
- }
-}
-
-/**
- * Wrapper around codeOwnerApi, sends each requests only once and then cache
- * the response. A new CodeOwnersCacheApi instance is created every time when a
- * new change object is assigned.
- * Gerrit never updates existing change object, but instead always assigns a new
- * change object. Particularly, a new change object is assigned when a change
- * is updated and user clicks reload toasts to see the updated change.
- * As a result, the lifetime of a cache is the same as a lifetime of an assigned
- * change object.
- * Periodical cache invalidation can lead to inconsistency in UI, i.e.
- * user can see the old reviewers list (reflects a state when a change was
- * loaded) and code-owners status for the current reviewer list. To avoid
- * this inconsistency, the cache doesn't invalidate.
- */
-export class CodeOwnersCacheApi {
- constructor(codeOwnerApi, change) {
- this.codeOwnerApi = codeOwnerApi;
- this.change = change;
- this.promises = {};
- }
-
- _fetchOnce(cacheKey, asyncFn) {
- if (!this.promises[cacheKey]) {
- this.promises[cacheKey] = asyncFn();
- }
- return this.promises[cacheKey];
- }
-
- getAccount() {
- return this._fetchOnce('getAccount', () => this._getAccount());
- }
-
- async _getAccount() {
- const loggedIn = await this.codeOwnerApi.restApi.getLoggedIn();
- if (!loggedIn) return undefined;
- return await this.codeOwnerApi.restApi.getAccount();
- }
-
- listOwnerStatus() {
- return this._fetchOnce('listOwnerStatus',
- () => this.codeOwnerApi.listOwnerStatus(this.change._number));
- }
-
- getBranchConfig() {
- return this._fetchOnce('getBranchConfig',
- () => this.codeOwnerApi.getBranchConfig(this.change.project,
- this.change.branch));
- }
-
- listOwnersForPath(path) {
- return this._fetchOnce(`listOwnersForPath:${path}`,
- () => this.codeOwnerApi.listOwnersForPath(this.change.id, path));
- }
-}
-
-export class OwnersFetcher {
- constructor(restApi, change, options) {
- // fetched files and fetching status
- this._fetchedOwners = new Map();
- this._fetchStatus = FetchStatus.NOT_STARTED;
- this._totalFetchCount = 0;
- this.change = change;
- this.options = options;
- this.codeOwnerApi = new CodeOwnerApi(restApi);
- }
-
- getStatus() {
- return this._fetchStatus;
- }
-
- getProgressString() {
- return this._totalFetchCount === 0 ?
- `Loading suggested owners ...` :
- `${this._fetchedOwners.size} out of ${this._totalFetchCount} files have returned suggested owners.`;
- }
-
- getFiles() {
- const result = [];
- for (const [path, info] of this._fetchedOwners.entries()) {
- result.push({path, info});
- }
- return result;
- }
-
- async fetchSuggestedOwners(codeOwnerStatusMap) {
- // reset existing temporary storage
- this._fetchedOwners = new Map();
- this._fetchStatus = FetchStatus.FETCHING;
- this._totalFetchCount = 0;
-
- // only fetch those not approved yet
- const filesGroupByStatus = [...codeOwnerStatusMap.keys()].reduce(
- (list, file) => {
- const status = codeOwnerStatusMap
- .get(file).status;
- if (status === OwnerStatus.INSUFFICIENT_REVIEWERS) {
- list.missing.push(file);
- } else if (status === OwnerStatus.PENDING) {
- list.pending.push(file);
- }
- return list;
- }
- , {pending: [], missing: []});
- // always fetch INSUFFICIENT_REVIEWERS first and then pending
- const filesToFetch = filesGroupByStatus.missing
- .concat(filesGroupByStatus.pending);
- this._totalFetchCount = filesToFetch.length;
- await this._batchFetchCodeOwners(filesToFetch);
- this._fetchStatus = FetchStatus.FINISHED;
- }
-
- /**
- * Recursively fetches code owners for all files until finished.
- *
- * @param {!Array<string>} files
- */
- async _batchFetchCodeOwners(files) {
- if (this._fetchStatus === FetchStatus.ABORT) {
- return this._fetchedOwners;
- }
-
- const batchRequests = [];
- const maxConcurrentRequests = this.options.maxConcurrentRequests;
- for (let i = 0; i < maxConcurrentRequests; i++) {
- const filePath = files[i];
- if (filePath) {
- this._fetchedOwners.set(filePath, {});
- batchRequests.push(this._fetchOwnersForPath(this.change.id, filePath));
- }
- }
- const resPromise = Promise.all(batchRequests);
- await resPromise;
- if (files.length > maxConcurrentRequests) {
- return await this._batchFetchCodeOwners(
- files.slice(maxConcurrentRequests));
- }
- return this._fetchedOwners;
- }
-
- async _fetchOwnersForPath(changeId, filePath) {
- try {
- const owners = await this.codeOwnerApi.listOwnersForPath(changeId,
- filePath);
- this._fetchedOwners.get(filePath).owners = owners;
- } catch (e) {
- this._fetchedOwners.get(filePath).error = e;
- }
- }
-
- abort() {
- this._fetchStatus = FetchStatus.ABORT;
- this._fetchedOwners = new Map();
- this._totalFetchCount = 0;
- }
-}
-
/**
* Service for the data layer used in the plugin UI.
*/
@@ -339,13 +50,22 @@
constructor(restApi, change, options = {}) {
this.restApi = restApi;
this.change = change;
- const codeOwnerApi = new CodeOwnerApi(restApi);
- this.codeOwnerCacheApi = new CodeOwnersCacheApi(codeOwnerApi, change);
+ const codeOwnersApi = new CodeOwnersApi(restApi);
+ this.codeOwnersCacheApi = new CodeOwnersCacheApi(codeOwnersApi, change);
- const fetcherOptions = {
+ const providerOptions = {
maxConcurrentRequests: options.maxConcurrentRequests || 10,
};
- this.ownersFetcher = new OwnersFetcher(restApi, change, fetcherOptions);
+ this.ownersProviders = {
+ [SuggestionsType.BEST_SUGGESTIONS]: new OwnersProvider(restApi, change, {
+ ...providerOptions,
+ ownersLimit: BestSuggestionsLimit,
+ }),
+ [SuggestionsType.ALL_SUGGESTIONS]: new OwnersProvider(restApi, change, {
+ ...providerOptions,
+ ownersLimit: AllSuggestionsLimit,
+ }),
+ };
}
/**
@@ -354,7 +74,7 @@
async prefetch() {
try {
await Promise.all([
- this.codeOwnerCacheApi.getAccount(),
+ this.codeOwnersCacheApi.getAccount(),
this.getStatus(),
]);
} catch {
@@ -371,7 +91,7 @@
* role 'REVIEWER' remains unchanged until the change view is reloaded.
*/
async getLoggedInUserInitialRole() {
- const account = await this.codeOwnerCacheApi.getAccount();
+ const account = await this.codeOwnersCacheApi.getAccount();
if (!account) {
return UserRole.ANONYMOUS;
}
@@ -418,9 +138,9 @@
const status = await this._getStatus();
if (status.enabled && !this.isOnLatestPatchset(status.patchsetNumber)) {
// status is outdated, abort and re-init
- this.abort();
+ this.reset();
this.prefetch();
- return await this.codeOwnerCacheApi.getStatus();
+ return await this.codeOwnersCacheApi.getStatus();
}
return status;
}
@@ -436,7 +156,7 @@
};
}
- const onwerStatus = await this.codeOwnerCacheApi.listOwnerStatus();
+ const onwerStatus = await this.codeOwnersCacheApi.listOwnerStatus();
return {
enabled: true,
@@ -476,39 +196,42 @@
* }>
* }}
*/
- async getSuggestedOwners() {
+ async getSuggestedOwners(suggestionsType) {
const {codeOwnerStatusMap} = await this.getStatus();
+ const ownersProvider = this.ownersProviders[suggestionsType];
- // In case its aborted due to outdated patches
- // should kick start the fetching again
- // Note: we currently are not reusing the instance when switching changes,
- // so if its `abort` due to different changes, the whole instance will be
- // outdated and not used.
- if (this.ownersFetcher.getStatus() === FetchStatus.NOT_STARTED
- || this.ownersFetcher.getStatus() === FetchStatus.ABORT) {
- await this.ownersFetcher.fetchSuggestedOwners(codeOwnerStatusMap);
- }
+ await ownersProvider.fetchSuggestedOwners(codeOwnerStatusMap);
return {
- finished: this.ownersFetcher.getStatus() === FetchStatus.FINISHED,
- status: this.ownersFetcher.getStatus(),
- progress: this.ownersFetcher.getProgressString(),
- suggestions: this._groupFilesByOwners(codeOwnerStatusMap,
- this.ownersFetcher.getFiles()),
+ finished: ownersProvider.getStatus() === FetchStatus.FINISHED,
+ status: ownersProvider.getStatus(),
+ progress: ownersProvider.getProgressString(),
+ files: this._getFilesWithStatuses(codeOwnerStatusMap,
+ ownersProvider.getFiles()),
};
}
- async getSuggestedOwnersProgress() {
+ async getSuggestedOwnersProgress(suggestionsType) {
const {codeOwnerStatusMap} = await this.getStatus();
+ const ownersProvider = this.ownersProviders[suggestionsType];
return {
- finished: this.ownersFetcher.getStatus() === FetchStatus.FINISHED,
- status: this.ownersFetcher.getStatus(),
- progress: this.ownersFetcher.getProgressString(),
- suggestions: this._groupFilesByOwners(codeOwnerStatusMap,
- this.ownersFetcher.getFiles()),
+ finished: ownersProvider.getStatus() === FetchStatus.FINISHED,
+ status: ownersProvider.getStatus(),
+ progress: ownersProvider.getProgressString(),
+ files: this._getFilesWithStatuses(codeOwnerStatusMap,
+ ownersProvider.getFiles()),
};
}
+ pauseSuggestedOwnersLoading(suggestionsType) {
+ this.ownersProviders[suggestionsType].pause();
+ }
+
+ resumeSuggestedOwnersLoading(suggestionsType) {
+ this.ownersProviders[suggestionsType].resume();
+ }
+
+
_formatStatuses(statuses) {
// convert the array of statuses to map between file path -> status
return statuses.reduce((prev, cur) => {
@@ -542,74 +265,14 @@
return;
}
- _groupFilesByOwners(codeOwnerStatusMap, files) {
- // Note: for renamed or moved files, they will have two entries in the map
- // we will treat them as two entries when group as well
- const ownersFilesMap = new Map();
- const failedToFetchFiles = new Set();
- for (const file of files) {
- const fileInfo = {
+ _getFilesWithStatuses(codeOwnerStatusMap, files) {
+ return files.map(file => {
+ return {
path: file.path,
+ info: file.info,
status: this._computeFileStatus(codeOwnerStatusMap, file.path),
};
- // for files failed to fetch, add them to the special group
- if (file.info.error) {
- failedToFetchFiles.add(fileInfo);
- continue;
- }
-
- // do not include files still in fetching
- if (!file.info.owners) {
- continue;
- }
-
- const ownersKey = this._getOwnersGroupKey(file.info.owners);
- ownersFilesMap.set(
- ownersKey,
- ownersFilesMap.get(ownersKey) || {files: [], owners: file.info.owners}
- );
- ownersFilesMap.get(ownersKey).files.push(fileInfo);
- }
- const groupedItems = [];
- for (const ownersKey of ownersFilesMap.keys()) {
- const groupName = this.getGroupName(ownersFilesMap.get(ownersKey).files);
- groupedItems.push({
- groupName,
- files: ownersFilesMap.get(ownersKey).files,
- owners: ownersFilesMap.get(ownersKey).owners,
- });
- }
-
- if (failedToFetchFiles.size > 0) {
- const failedFiles = [...failedToFetchFiles];
- groupedItems.push({
- groupName: this.getGroupName(failedFiles),
- files: failedFiles,
- error: new Error(
- 'Failed to fetch code owner info. Try to refresh the page.'),
- });
- }
-
- return groupedItems;
- }
-
- _getOwnersGroupKey(owners) {
- if (owners.owned_by_all_users) {
- return '__owned_by_all_users__';
- }
- const code_owners = owners.code_owners;
- return code_owners
- .map(owner => owner.account._account_id)
- .sort()
- .join(',');
- }
-
- getGroupName(files) {
- const fileName = files[0].path.split('/').pop();
- return {
- name: fileName,
- prefix: files.length > 1 ? `+ ${files.length - 1} more` : '',
- };
+ });
}
isOnLatestPatchset(patchsetId) {
@@ -617,14 +280,16 @@
return `${latestRevision._number}` === `${patchsetId}`;
}
- abort() {
- this.ownersFetcher.abort();
- const codeOwnerApi = new CodeOwnerApi(this.restApi);
- this.codeOwnerCacheApi = new CodeOwnersCacheApi(codeOwnerApi, change);
+ reset() {
+ for (const provider of Object.values(this.ownersProviders)) {
+ provider.reset();
+ }
+ const codeOwnersApi = new CodeOwnersApi(this.restApi);
+ this.codeOwnersCacheApi = new CodeOwnersCacheApi(codeOwnersApi, change);
}
async getBranchConfig() {
- return this.codeOwnerCacheApi.getBranchConfig();
+ return this.codeOwnersCacheApi.getBranchConfig();
}
async isCodeOwnerEnabled() {
@@ -632,7 +297,7 @@
this.change.status === ChangeStatus.MERGED) {
return false;
}
- const config = await this.codeOwnerCacheApi.getBranchConfig();
+ const config = await this.codeOwnersCacheApi.getBranchConfig();
return config && !config.disabled;
}
diff --git a/ui/owner-requirement.js b/ui/owner-requirement.js
index 1ad5b4b..b506a8b 100644
--- a/ui/owner-requirement.js
+++ b/ui/owner-requirement.js
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-import {OwnerStatus} from './code-owners-service.js';
+import {OwnerStatus} from './code-owners-fetcher.js';
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
import {showPluginFailedMessage} from './code-owners-banner.js';
import {PluginState} from './code-owners-model.js';
diff --git a/ui/owner-status-column.js b/ui/owner-status-column.js
index b3d92fb..4e6e055 100644
--- a/ui/owner-status-column.js
+++ b/ui/owner-status-column.js
@@ -15,7 +15,7 @@
* limitations under the License.
*/
-import {OwnerStatus} from './code-owners-service.js';
+import {OwnerStatus} from './code-owners-fetcher.js';
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
const MAGIC_FILES = ['/COMMIT_MSG', '/MERGE_LIST', '/PATCHSET_LEVEL'];
diff --git a/ui/suggest-owners-util.js b/ui/suggest-owners-util.js
new file mode 100644
index 0000000..898a6e5
--- /dev/null
+++ b/ui/suggest-owners-util.js
@@ -0,0 +1,124 @@
+/**
+ * @license
+ * Copyright (C) 2021 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 {BestSuggestionsLimit} from './code-owners-model.js';
+
+/**
+ * For each file calculates owners to display and group all files by those
+ * owners. The function creates "fake" groups when one or more
+ * reviewers are included in all owners of a file, but none of reviewers is
+ * included in best reviewers for the file.
+ *
+ * Such situations are possible when user turns on "Show all owners", selects
+ * one of newly displayed owners and then turns off "Show all owners". Without
+ * "fake" groups a user can see inconsistent state in dialog.
+ */
+
+export function getDisplayOwnersGroups(files, allOwnersByPathMap,
+ reviewersIdSet, allowAllOwnersSubstition) {
+
+ const getDisplayOwnersFunc =
+ !allowAllOwnersSubstition || allOwnersByPathMap.size === 0 ||
+ reviewersIdSet.size === 0 ?
+ file => file.info.owners :
+ file => getDisplayOwners(file, allOwnersByPathMap, reviewersIdSet);
+ return groupFilesByOwners(files, getDisplayOwnersFunc);
+}
+
+function getDisplayOwners(file, allOwnersByPathMap, reviewersIdSet) {
+ const ownerSelected = owner => reviewersIdSet.has(owner.account._account_id);
+ const defaultOwners = file.info.owners;
+ if (!defaultOwners ||
+ defaultOwners.owned_by_all_users ||
+ defaultOwners.code_owners.some(ownerSelected)) {
+ return defaultOwners;
+ }
+ const allOwners = allOwnersByPathMap.get(file.path);
+ if (!allOwners) return defaultOwners;
+ if (allOwners.owned_by_all_users) return allOwners;
+ const selectedAllOwners = allOwners.code_owners.filter(ownerSelected);
+ if (selectedAllOwners.length === 0) return defaultOwners;
+ return {
+ code_owners: selectedAllOwners.slice(0, BestSuggestionsLimit),
+ };
+}
+
+function groupFilesByOwners(files, getDisplayOwnersFunc) {
+ // Note: for renamed or moved files, they will have two entries in the map
+ // we will treat them as two entries when group as well
+ const ownersFilesMap = new Map();
+ const failedToFetchFiles = new Set();
+ for (const file of files) {
+ // for files failed to fetch, add them to the special group
+ if (file.info.error) {
+ failedToFetchFiles.add(file);
+ continue;
+ }
+
+ // do not include files still in fetching
+ if (!file.info.owners) {
+ continue;
+ }
+ const displayOwners = getDisplayOwnersFunc(file);
+
+ const ownersKey = getOwnersGroupKey(displayOwners);
+ ownersFilesMap.set(
+ ownersKey,
+ ownersFilesMap.get(ownersKey) || {files: [], owners: displayOwners}
+ );
+ ownersFilesMap.get(ownersKey).files.push(file);
+ }
+ const groupedItems = [];
+ for (const ownersKey of ownersFilesMap.keys()) {
+ const groupName = getGroupName(ownersFilesMap.get(ownersKey).files);
+ groupedItems.push({
+ groupName,
+ files: ownersFilesMap.get(ownersKey).files,
+ owners: ownersFilesMap.get(ownersKey).owners,
+ });
+ }
+
+ if (failedToFetchFiles.size > 0) {
+ const failedFiles = [...failedToFetchFiles];
+ groupedItems.push({
+ groupName: getGroupName(failedFiles),
+ files: failedFiles,
+ error: new Error(
+ 'Failed to fetch code owner info. Try to refresh the page.'),
+ });
+ }
+ return groupedItems;
+}
+
+function getOwnersGroupKey(owners) {
+ if (owners.owned_by_all_users) {
+ return '__owned_by_all_users__';
+ }
+ const code_owners = owners.code_owners;
+ return code_owners
+ .map(owner => owner.account._account_id)
+ .sort()
+ .join(',');
+}
+
+function getGroupName(files) {
+ const fileName = files[0].path.split('/').pop();
+ return {
+ name: fileName,
+ prefix: files.length > 1 ? `+ ${files.length - 1} more` : '',
+ };
+}
diff --git a/ui/suggest-owners.js b/ui/suggest-owners.js
index 91c6f96..781f64b 100644
--- a/ui/suggest-owners.js
+++ b/ui/suggest-owners.js
@@ -15,7 +15,12 @@
* limitations under the License.
*/
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
-import {SuggestionsState} from './code-owners-model.js';
+import {
+ BestSuggestionsLimit,
+ SuggestionsState,
+ SuggestionsType,
+} from './code-owners-model.js';
+import {getDisplayOwnersGroups} from './suggest-owners-util.js';
const SUGGESTION_POLLING_INTERVAL = 1000;
@@ -115,15 +120,23 @@
max-height: 300px;
overflow-y: auto;
}
- .suggestion-row {
+ .flex-break {
+ height: 0;
+ flex-basis: 100%;
+ }
+ .suggestion-row, .show-all-owners-row {
display: flex;
flex-direction: row;
align-items: flex-start;
+ }
+ .suggestion-row {
+ flex-wrap: wrap;
border-bottom: 1px solid var(--border-color);
padding: var(--spacing-s) 0;
}
- .suggestion-row:last-of-type {
- border-bottom: none;
+ .show-all-owners-row {
+ padding: var(--spacing-m) var(--spacing-xl) var(--spacing-s);
+ justify-content: flex-end;
}
.suggestion-row-indicator {
margin-right: var(--spacing-m);
@@ -162,6 +175,9 @@
color: var(--deemphasized-text-color);
}
.suggested-owners {
+ --account-gap: var(--spacing-s);
+ --negative-account-gap: calc(-1*var(--account-gap));
+ margin: var(--negative-account-gap) 0 0 var(--negative-account-gap);
flex: 1;
}
.fetch-error-content,
@@ -171,7 +187,7 @@
flex: 1;
padding-left: var(--spacing-m);
}
-
+
.owned-by-all-users-content iron-icon {
width: 16px;
height: 16px;
@@ -200,6 +216,7 @@
are always fit in a single row */
--account-max-length: 60px;
border: 1px solid var(--border-color);
+ margin: var(--account-gap) 0 0 var(--account-gap)
}
gr-account-label:focus {
outline: none;
@@ -291,26 +308,39 @@
</div>
</template>
<template is="dom-if" if="[[!suggestion.owners.owned_by_all_users]]">
+ <template is="dom-if" if="[[_breakBeforeOwners(suggestion.owners.code_owners, _showAllOwners)]]">
+ <div class="flex-break"></div>
+ </template>
<ul class="suggested-owners">
<template
is="dom-repeat"
items="[[suggestion.owners.code_owners]]"
as="owner"
index-as="ownerIndex"
- >
- <gr-account-label
+ ><!--
+ --><gr-account-label
data-suggestion-index$="[[suggestionIndex]]"
data-owner-index$="[[ownerIndex]]"
account="[[owner.account]]"
selected$="[[isSelected(owner)]]"
on-click="toggleAccount">
- </gr-account-label>
- </template>
+ </gr-account-label><!--
+ --></template>
</ul>
</template>
</template>
</li>
</template>
+ <li class="show-all-owners-row">
+ <label>
+ <input
+ id="showAllOwnersCheckbox"
+ type="checkbox"
+ checked="{{_showAllOwners::change}}"
+ />
+ Show all owners
+ </label>
+ </li>
</ul>
`;
}
@@ -333,7 +363,21 @@
reviewers: {
type: Array,
},
+ _reviewersIdSet: {
+ type: Object,
+ computed: '_getReviewersIdSet(reviewers)',
+ },
pendingReviewers: Array,
+ _showAllOwners: {
+ type: Boolean,
+ value: false,
+ observer: '_showAllOwnersChanged',
+ },
+ _allOwnersByPathMap: {
+ type: Object,
+ computed:
+ `_getOwnersByPathMap(model.suggestionsByTypes.${SuggestionsType.ALL_SUGGESTIONS}.files)`,
+ },
};
}
@@ -341,15 +385,39 @@
return [
'_onReviewerChanged(reviewers)',
'_onShowSuggestionsChanged(model.showSuggestions)',
- '_onSuggestionsStateChanged(model.suggestionsState)',
- '_onSuggestionsChanged(model.suggestions, model.suggestionsState)',
- '_onSuggestionsLoadProgressChanged(model.suggestionsLoadProgress)',
+ '_onShowSuggestionsTypeChanged(model.showSuggestions,' +
+ 'model.selectedSuggestionsType)',
+ '_onSuggestionsStateChanged(model.selectedSuggestions.state)',
+ '_onSuggestionsFilesChanged(model.selectedSuggestions.files, ' +
+ '_allOwnersByPathMap, _reviewersIdSet, model.selectedSuggestionsType,' +
+ 'model.selectedSuggestions.state)',
+ '_onSuggestionsLoadProgressChanged(' +
+ 'model.selectedSuggestions.loadProgress)',
];
}
+ constructor() {
+ super();
+ // To prevent multiple reporting when switching back and forth showAllOwners
+ this.reportedEvents = {};
+ for (const suggestionType of Object.values(SuggestionsType)) {
+ this.reportedEvents[suggestionType] = {
+ fetchingStart: false,
+ fetchingFinished: false,
+ };
+ }
+ }
+
+ disconnectedCallback() {
+ super.disconnectedCallback();
+ this._stopUpdateProgressTimer();
+ if (this.modelLoader) {
+ this.modelLoader.pauseActiveSuggestedOwnersLoading();
+ }
+ }
+
_onShowSuggestionsChanged(showSuggestions) {
- if (!showSuggestions ||
- this.model.suggestionsLoadProgress === SuggestionsState.NotLoaded) {
+ if (!showSuggestions) {
return;
}
// this is more of a hack to let review input lose focus
@@ -360,20 +428,31 @@
// Can not use `this.async` as it's only available in
// legacy element mixin which not used in this plugin.
Polymer.Async.timeOut.run(() => this.click(), 100);
-
- this.modelLoader.loadSuggestions();
- this.reporting.reportLifeCycle('owners-suggestions-fetching-start');
}
- disconnectedCallback() {
- super.disconnectedCallback();
- this._stopUpdateProgressTimer();
+ _onShowSuggestionsTypeChanged(showSuggestion, selectedSuggestionsType) {
+ if (!showSuggestion) {
+ this.modelLoader.pauseActiveSuggestedOwnersLoading();
+ return;
+ }
+ this.modelLoader.loadSuggestions(selectedSuggestionsType);
+ // The progress is updated at the next _progressUpdateTimer tick.
+ // Without excplicit call to updateLoadSuggestionsProgress it looks like
+ // a slow reaction to checkbox.
+ this.modelLoader.updateLoadSelectedSuggestionsProgress();
+
+ if (!this.reportedEvents[selectedSuggestionsType].fetchingStart) {
+ this.reportedEvents[selectedSuggestionsType].fetchingStart = true;
+ this.reporting.reportLifeCycle('owners-suggestions-fetching-start', {
+ type: selectedSuggestionsType,
+ });
+ }
}
_startUpdateProgressTimer() {
if (this._progressUpdateTimer) return;
this._progressUpdateTimer = setInterval(() => {
- this.modelLoader.updateLoadSuggestionsProgress();
+ this.modelLoader.updateLoadSelectedSuggestionsProgress();
}, SUGGESTION_POLLING_INTERVAL);
}
@@ -403,20 +482,43 @@
this.modelLoader.loadAreAllFilesApproved();
}
- _onSuggestionsChanged(suggestions, suggestionsState) {
+ _getReviewersIdSet(reviewers) {
+ return new Set((reviewers || []).map(account => account._account_id));
+ }
+
+ _onSuggestionsFilesChanged(files, allOwnersByPathMap, reviewersIdSet,
+ selectedSuggestionsType, suggestionsState) {
+ if (files === undefined || allOwnersByPathMap === undefined ||
+ reviewersIdSet === undefined || selectedSuggestionsType === undefined ||
+ suggestionsState === undefined) return;
+
+ const groups = getDisplayOwnersGroups(
+ files, allOwnersByPathMap, reviewersIdSet,
+ selectedSuggestionsType !== SuggestionsType.ALL_SUGGESTIONS);
// The updateLoadSuggestionsProgress method also updates suggestions
- this._updateSuggestions(suggestions || []);
+ this._updateSuggestions(groups);
this._updateAllChips(this._currentReviewers);
- if (!suggestions || suggestionsState !== SuggestionsState.Loaded) return;
- const reportDetails = suggestions.reduce((details, cur) => {
- details.totalGroups++;
- details.stats.push([cur.files.length,
- cur.owners && cur.owners.code_owners ?
- cur.owners.code_owners.length : 0]);
- return details;
- }, {totalGroups: 0, stats: []});
- this.reporting.reportLifeCycle(
- 'owners-suggestions-fetching-finished', reportDetails);
+
+ if (suggestionsState !== SuggestionsState.Loaded) return;
+ if (!this.reportedEvents[selectedSuggestionsType].fetchingFinished) {
+ this.reportedEvents[selectedSuggestionsType].fetchingFinished = true;
+ const reportDetails = groups.reduce((details, cur) => {
+ details.totalGroups++;
+ details.stats.push([cur.files.length,
+ cur.owners && cur.owners.code_owners ?
+ cur.owners.code_owners.length : 0]);
+ return details;
+ }, {totalGroups: 0, stats: [], type: selectedSuggestionsType});
+ this.reporting.reportLifeCycle(
+ 'owners-suggestions-fetching-finished', reportDetails);
+ }
+ }
+
+ _getOwnersByPathMap(files) {
+ return new Map((files || [])
+ .filter(file => !file.info.error && file.info.owners)
+ .map(file => [file.path, file.info.owners])
+ );
}
_onSuggestionsLoadProgressChanged(progress) {
@@ -446,21 +548,28 @@
const res = {};
res.groupName = suggestion.groupName;
res.files = suggestion.files.slice();
- const codeOwners = (suggestion.owners.code_owners || []).map(owner => {
- const updatedOwner = {...owner};
- const reviewers = this.change.reviewers.REVIEWER;
- if (
- reviewers &&
- reviewers.find(reviewer => reviewer._account_id === owner._account_id)
- ) {
- updatedOwner.selected = true;
- }
- return updatedOwner;
- });
- res.owners = {
- owned_by_all_users: !!suggestion.owners.owned_by_all_users,
- code_owners: codeOwners,
- };
+ if (suggestion.owners) {
+ const codeOwners = (suggestion.owners.code_owners || []).map(owner => {
+ const updatedOwner = {...owner};
+ const reviewers = this.change.reviewers.REVIEWER;
+ if (reviewers &&
+ reviewers.find(
+ reviewer => reviewer._account_id === owner._account_id)
+ ) {
+ updatedOwner.selected = true;
+ }
+ return updatedOwner;
+ });
+ res.owners = {
+ owned_by_all_users: !!suggestion.owners.owned_by_all_users,
+ code_owners: codeOwners,
+ };
+ } else {
+ res.owners = {
+ owned_by_all_users: false,
+ code_owners: [],
+ };
+ }
res.error = suggestion.error;
return res;
@@ -511,10 +620,7 @@
this.suggestedOwners.forEach((suggestion, sId) => {
let hasSelected = false;
suggestion.owners.code_owners.forEach((owner, oId) => {
- if (
- accounts.some(account => account._account_id
- === owner.account._account_id)
- ) {
+ if (accounts.some(account => account._account_id === owner.account._account_id)) {
this.set(
['suggestedOwners', sId, 'owners', 'code_owners', oId],
{...owner,
@@ -559,6 +665,18 @@
'Any user can approve. Please select a user manually' :
'Any user from the other files can approve';
}
+
+ _showAllOwnersChanged(showAll) {
+ // The first call to this method happens before model is set.
+ if (!this.model) return;
+ this.model.setSelectedSuggestionType(showAll ?
+ SuggestionsType.ALL_SUGGESTIONS : SuggestionsType.BEST_SUGGESTIONS);
+ }
+
+ _breakBeforeOwners(codeOwners, showAllOwners) {
+ if (!codeOwners || !showAllOwners) return false;
+ return codeOwners.length > BestSuggestionsLimit;
+ }
}
customElements.define(SuggestOwners.is, SuggestOwners);
