Add "Show all owners" option
The checkbox allows to request all owners for files in a change.
The implementation has an issue: if a user clicks checkbox before
owners has been loaded, the plugin starts loading all owners
without pausing best owners loading. This issue is fixed in the
next change.
Change-Id: Id664476c47d6bcb170095da524877895c9484dfa
diff --git a/ui/code-owners-model-loader.js b/ui/code-owners-model-loader.js
index 2cd9095..0e5ace7 100644
--- a/ui/code-owners-model-loader.js
+++ b/ui/code-owners-model-loader.js
@@ -84,33 +84,47 @@
);
}
- async loadSuggestions() {
+ async loadSuggestions(suggestionsType) {
// If a loading has been started already, do nothing
- if (this.ownersModel.suggestionsState
- !== SuggestionsState.NotLoaded) return;
+ if (this.ownersModel.suggestionsByTypes[suggestionsType].state !==
+ SuggestionsState.NotLoaded) return;
- this.ownersModel.setSuggestionsState(SuggestionsState.Loading);
+ this.ownersModel.setSuggestionsState(suggestionsType,
+ SuggestionsState.Loading);
let suggestedOwners;
try {
- suggestedOwners = await this.ownersService.getSuggestedOwners();
+ suggestedOwners =
+ await this.ownersService.getSuggestedOwners(suggestionsType);
} catch (e) {
- this.ownersModel.setSuggestionsState(SuggestionsState.LoadFailed);
- this.ownersModel.setPluginFailed(e.message);
+ 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.setSuggestions(suggestedOwners.suggestions);
- this.ownersModel.setSuggestionsState(SuggestionsState.Loaded);
+ this.ownersModel.setSuggestionsFiles(suggestionsType,
+ suggestedOwners.files);
+ this.ownersModel.setSuggestionsState(suggestionsType,
+ SuggestionsState.Loaded);
}
- async updateLoadSuggestionsProgress() {
+ async updateLoadSelectedSuggestionsProgress() {
+ const suggestionsType = this.ownersModel.selectedSuggestionsType;
let suggestedOwners;
try {
- suggestedOwners = await this.ownersService.getSuggestedOwnersProgress();
+ 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.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..55ad38a 100644
--- a/ui/code-owners-service.js
+++ b/ui/code-owners-service.js
@@ -15,6 +15,8 @@
* limitations under the License.
*/
+import {SuggestionsType, BestSuggestionsLimit, AllSuggestionsLimit} from './code-owners-model.js';
+
/**
* All statuses returned for owner status.
*
@@ -106,10 +108,10 @@
* @param {string} changeId
* @param {string} path
*/
- listOwnersForPath(changeId, path) {
+ listOwnersForPath(changeId, path, limit) {
return this.restApi.get(
`/changes/${changeId}/revisions/current/code_owners` +
- `/${encodeURIComponent(path)}?limit=5&o=DETAILS`
+ `/${encodeURIComponent(path)}?limit=${limit}&o=DETAILS`
);
}
@@ -224,11 +226,6 @@
() => 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 {
@@ -318,7 +315,7 @@
async _fetchOwnersForPath(changeId, filePath) {
try {
const owners = await this.codeOwnerApi.listOwnersForPath(changeId,
- filePath);
+ filePath, this.options.ownersLimit);
this._fetchedOwners.get(filePath).owners = owners;
} catch (e) {
this._fetchedOwners.get(filePath).error = e;
@@ -345,7 +342,16 @@
const fetcherOptions = {
maxConcurrentRequests: options.maxConcurrentRequests || 10,
};
- this.ownersFetcher = new OwnersFetcher(restApi, change, fetcherOptions);
+ this.ownersFetchers = {
+ [SuggestionsType.BEST_SUGGESTIONS]: new OwnersFetcher(restApi, change, {
+ ...fetcherOptions,
+ ownersLimit: BestSuggestionsLimit,
+ }),
+ [SuggestionsType.ALL_SUGGESTIONS]: new OwnersFetcher(restApi, change, {
+ ...fetcherOptions,
+ ownersLimit: AllSuggestionsLimit,
+ }),
+ };
}
/**
@@ -476,36 +482,38 @@
* }>
* }}
*/
- async getSuggestedOwners() {
+ async getSuggestedOwners(suggestionsType) {
const {codeOwnerStatusMap} = await this.getStatus();
+ const ownersFetcher = this.ownersFetchers[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);
+ if (ownersFetcher.getStatus() === FetchStatus.NOT_STARTED
+ || ownersFetcher.getStatus() === FetchStatus.ABORT) {
+ await ownersFetcher.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: ownersFetcher.getStatus() === FetchStatus.FINISHED,
+ status: ownersFetcher.getStatus(),
+ progress: ownersFetcher.getProgressString(),
+ files: this._getFilesWithStatuses(codeOwnerStatusMap,
+ ownersFetcher.getFiles()),
};
}
- async getSuggestedOwnersProgress() {
+ async getSuggestedOwnersProgress(suggestionsType) {
const {codeOwnerStatusMap} = await this.getStatus();
+ const ownersFetcher = this.ownersFetchers[suggestionsType];
return {
- finished: this.ownersFetcher.getStatus() === FetchStatus.FINISHED,
- status: this.ownersFetcher.getStatus(),
- progress: this.ownersFetcher.getProgressString(),
- suggestions: this._groupFilesByOwners(codeOwnerStatusMap,
- this.ownersFetcher.getFiles()),
+ finished: ownersFetcher.getStatus() === FetchStatus.FINISHED,
+ status: ownersFetcher.getStatus(),
+ progress: ownersFetcher.getProgressString(),
+ files: this._getFilesWithStatuses(codeOwnerStatusMap,
+ ownersFetcher.getFiles()),
};
}
@@ -542,74 +550,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) {
@@ -618,7 +566,9 @@
}
abort() {
- this.ownersFetcher.abort();
+ for (const fetcher of Object.values(this.ownersFetchers)) {
+ fetcher.abort();
+ }
const codeOwnerApi = new CodeOwnerApi(this.restApi);
this.codeOwnerCacheApi = new CodeOwnersCacheApi(codeOwnerApi, change);
}
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..4f9553a 100644
--- a/ui/suggest-owners.js
+++ b/ui/suggest-owners.js
@@ -15,7 +15,8 @@
* limitations under the License.
*/
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
-import {SuggestionsState} from './code-owners-model.js';
+import {SuggestionsState, SuggestionsType} from './code-owners-model.js';
+import {getDisplayOwnersGroups} from './suggest-owners-util.js';
const SUGGESTION_POLLING_INTERVAL = 1000;
@@ -115,15 +116,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 +171,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 +183,7 @@
flex: 1;
padding-left: var(--spacing-m);
}
-
+
.owned-by-all-users-content iron-icon {
width: 16px;
height: 16px;
@@ -200,6 +212,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 +304,39 @@
</div>
</template>
<template is="dom-if" if="[[!suggestion.owners.owned_by_all_users]]">
+ <template is="dom-if" if="[[_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 +359,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 +381,31 @@
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,
+ };
+ }
+ }
+
_onShowSuggestionsChanged(showSuggestions) {
- if (!showSuggestions ||
- this.model.suggestionsLoadProgress === SuggestionsState.NotLoaded) {
+ if (!showSuggestions) {
return;
}
// this is more of a hack to let review input lose focus
@@ -361,8 +417,24 @@
// 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');
+ }
+
+ _onShowSuggestionsTypeChanged(showSuggestion, selectedSuggestionsType) {
+ if (!showSuggestion) {
+ 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,
+ });
+ }
}
disconnectedCallback() {
@@ -373,7 +445,7 @@
_startUpdateProgressTimer() {
if (this._progressUpdateTimer) return;
this._progressUpdateTimer = setInterval(() => {
- this.modelLoader.updateLoadSuggestionsProgress();
+ this.modelLoader.updateLoadSelectedSuggestionsProgress();
}, SUGGESTION_POLLING_INTERVAL);
}
@@ -403,20 +475,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) {
@@ -511,10 +606,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 +651,13 @@
'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);
+ }
}
customElements.define(SuggestOwners.is, SuggestOwners);
