Merge changes Ibcc71ffc,Ia09cc5f4,I0701b269,I1dbfbc00,I9d4c4fda, ...
* changes:
Do not show "Suggest owners" if code owners is disabled for a branch
Let users know if no code owners are defined yet
Handle restAPI errors
Use CodeOwnersModel in suggest-owners
Introduce CodeOwnersModel and ModelLoader and use them in UI elements
Extract code-owners fetcher into a separate class
Add cache for api calls
Use async/await instead of promises
Remove unused methods
Fix eslint problems
diff --git a/.eslintrc.json b/.eslintrc.json
index b9a6517..2c0d24a 100644
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -90,7 +90,8 @@
2,
{
"ignoreComments": true,
- "ignorePattern": "^import .*;$"
+ "ignorePattern": "^import .*;$",
+ "ignoreTemplateLiterals": true
}
],
"new-cap": [
@@ -100,7 +101,8 @@
"Polymer",
"LegacyElementMixin",
"GestureEventListeners",
- "LegacyDataMixin"
+ "LegacyDataMixin",
+ "CodeOwnersModelMixin"
]
}
],
@@ -235,4 +237,4 @@
}
}
]
-}
\ No newline at end of file
+}
diff --git a/ui/code-owners-banner.js b/ui/code-owners-banner.js
new file mode 100644
index 0000000..a44e258
--- /dev/null
+++ b/ui/code-owners-banner.js
@@ -0,0 +1,176 @@
+/**
+ * @license
+ * Copyright (C) 2020 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 {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
+import {PluginState} from './code-owners-model.js';
+
+// There are 2 elements in this file:
+// CodeOwnersBanner - visual elements. This element is shown at the top
+// of the page when pluginStatus are changed to failed.
+// The CodeOwnersBanner is added to the 'banner' endpoint. The endpoint
+// is placed at the top level of the gerrit and doesn't provide a current
+// change, so it is impossible to get the pluginStatus directly in the
+// CodeOwnersBanner.
+// To solve this problem, this files provides non-visual
+// CodeOwnersPluginStatusNotifier element. This element is added to the
+// change-view-integration endpoint, so it can track the plugin state.
+// When a plugin state is changed, the CodeOwnersPluginStatusNotifier updates
+// the pluginStatus property of the banner.
+
+// CodeOwnersBanner and CodeOwnersPluginStatusNotifier can be attached and
+// detached in any order. The following piece of code ensures that these
+// banner and notifier are always connected correctly. The code assumes,
+// that max one banner and one notifier can be connected at any time.
+let activeBanner = undefined;
+let activeStatusNotifier = undefined;
+
+function setActiveBanner(banner) {
+ // banner is null when CodeOwnersBanner has been disconnected
+ activeBanner = banner;
+ if (activeStatusNotifier) {
+ activeStatusNotifier.banner = banner;
+ }
+}
+
+function setActiveStatusNotifier(notifier) {
+ // notifier is null when CodeOwnersBanner has been disconnected
+ if (activeStatusNotifier) {
+ if (activeStatusNotifier.banner) {
+ activeStatusNotifier.banner.pluginStatus = undefined;
+ }
+ activeStatusNotifier.banner = undefined;
+ }
+ activeStatusNotifier = notifier;
+ if (activeStatusNotifier) {
+ activeStatusNotifier.banner = activeBanner;
+ }
+}
+
+export class CodeOwnersBanner extends Polymer.Element {
+ static get is() { return 'gr-code-owners-banner'; }
+
+ static get template() {
+ return Polymer.html`
+ <style include="shared-styles">
+ :host {
+ display: block;
+ overflow: hidden;
+ background: red;
+ }
+ .text {
+ color: white;
+ font-family: var(--header-font-family);
+ font-size: var(--font-size-h3);
+ font-weight: var(--font-weight-h3);
+ line-height: var(--line-height-h3);
+ margin-left: var(--spacing-l);
+ }
+ </style>
+ <span class="text">Error: Code-owners plugin has failed</span>
+ <gr-button link on-click="_showFailDetails">
+ Details
+ </gr-button>
+ `;
+ }
+
+ static get properties() {
+ return {
+ // @internal attributes
+ hidden: {
+ type: Boolean,
+ value: true,
+ reflectToAttribute: true,
+ computed: '_computeHidden(pluginStatus)',
+ },
+ pluginStatus: {
+ type: Object,
+ },
+ };
+ }
+
+ connectedCallback() {
+ super.connectedCallback();
+ setActiveBanner(this);
+ }
+
+ disconnectedCallback() {
+ super.disconnectedCallback();
+ setActiveBanner(undefined);
+ }
+
+ _computeHidden(pluginStatus) {
+ return !pluginStatus || pluginStatus.state !== PluginState.Failed;
+ }
+
+ _showFailDetails() {
+ showPluginFailedMessage(this, this.pluginStatus);
+ }
+}
+
+customElements.define(CodeOwnersBanner.is, CodeOwnersBanner);
+
+export class CodeOwnersPluginStatusNotifier extends
+ CodeOwnersModelMixin(Polymer.Element) {
+ static get is() {
+ return 'owners-plugin-status-notifier';
+ }
+
+ static get properties() {
+ return {
+ banner: {
+ type: Object,
+ },
+ };
+ }
+
+ static get observers() {
+ return [
+ '_pluginStatusOrBannerChanged(model.pluginStatus, banner)',
+ ];
+ }
+
+ connectedCallback() {
+ super.connectedCallback();
+ setActiveStatusNotifier(this);
+ }
+
+ disconnectedCallback() {
+ super.disconnectedCallback();
+ setActiveStatusNotifier(undefined);
+ }
+
+ _pluginStatusOrBannerChanged(status, banner) {
+ if (!banner) return;
+ banner.pluginStatus = status;
+ }
+
+ _loadDataAfterStateChanged() {
+ this.modelLoader.loadPluginStatus();
+ }
+}
+
+customElements.define(CodeOwnersPluginStatusNotifier.is,
+ CodeOwnersPluginStatusNotifier);
+
+export function showPluginFailedMessage(sourceEl, pluginStatus) {
+ sourceEl.dispatchEvent(
+ new CustomEvent('show-error', {
+ detail: {message: pluginStatus.failedMessage},
+ composed: true,
+ bubbles: true,
+ })
+ );
+}
diff --git a/ui/code-owners-model-loader.js b/ui/code-owners-model-loader.js
new file mode 100644
index 0000000..2cd9095
--- /dev/null
+++ b/ui/code-owners-model-loader.js
@@ -0,0 +1,116 @@
+/**
+ * @license
+ * Copyright (C) 2020 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 {SuggestionsState} from './code-owners-model.js';
+
+/**
+ * ModelLoader provides a method for loading data into the model.
+ * It is a bridge between an ownersModel and an ownersService.
+ * When UI elements depends on a model property XXX, the element should
+ * observe (or bind to) the property and call modelLoader.loadXXX method
+ * to load the value.
+ * It is recommended to use CodeOwnersModelMixin and call all load... methods
+ * in the loadPropertiesAfterModelChanged method
+ */
+export class ModelLoader {
+ constructor(ownersService, ownersModel) {
+ this.ownersService = ownersService;
+ this.ownersModel = ownersModel;
+ }
+
+ async _loadProperty(propertyName, propertyLoader, propertySetter) {
+ // Load property only if it was not already loaded
+ if (this.ownersModel[propertyName] !== undefined) return;
+ let newValue;
+ try {
+ newValue = await propertyLoader();
+ } catch (e) {
+ this.ownersModel.setPluginFailed(e.message);
+ return;
+ }
+ // It is possible, that several requests is made in parallel.
+ // Store only the first result and discard all other results.
+ // (also, due to the CodeOwnersCacheApi all result must be identical)
+ if (this.ownersModel[propertyName] !== undefined) return;
+ propertySetter(newValue);
+ }
+
+ async loadBranchConfig() {
+ await this._loadProperty('branchConfig',
+ () => this.ownersService.getBranchConfig(),
+ value => this.ownersModel.setBranchConfig(value)
+ );
+ }
+
+ async loadStatus() {
+ await this._loadProperty('status',
+ () => this.ownersService.getStatus(),
+ value => this.ownersModel.setStatus(value)
+ );
+ }
+
+ async loadUserRole() {
+ await this._loadProperty('userRole',
+ () => this.ownersService.getLoggedInUserInitialRole(),
+ value => this.ownersModel.setUserRole(value)
+ );
+ }
+
+ async loadPluginStatus() {
+ await this._loadProperty('pluginStatus',
+ () => this.ownersService.isCodeOwnerEnabled(),
+ value => this.ownersModel.setPluginEnabled(value)
+ );
+ }
+
+ async loadAreAllFilesApproved() {
+ await this._loadProperty('areAllFilesApproved',
+ () => this.ownersService.areAllFilesApproved(),
+ value => this.ownersModel.setAreAllFilesApproved(value)
+ );
+ }
+
+ 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);
+ return;
+ }
+ this.ownersModel.setSuggestions(suggestedOwners.suggestions);
+ this.ownersModel.setSuggestionsState(SuggestionsState.Loaded);
+ }
+
+ async updateLoadSuggestionsProgress() {
+ let suggestedOwners;
+ try {
+ suggestedOwners = await this.ownersService.getSuggestedOwnersProgress();
+ } catch {
+ // Ignore any error, keep progress unchanged.
+ return;
+ }
+ this.ownersModel.setSuggestionsLoadProgress(suggestedOwners.progress);
+ this.ownersModel.setSuggestions(suggestedOwners.suggestions);
+ }
+}
diff --git a/ui/code-owners-model-mixin.js b/ui/code-owners-model-mixin.js
new file mode 100644
index 0000000..722af6f
--- /dev/null
+++ b/ui/code-owners-model-mixin.js
@@ -0,0 +1,100 @@
+/**
+ * @license
+ * Copyright (C) 2020 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 {CodeOwnerService} from './code-owners-service.js';
+import {ModelLoader} from './code-owners-model-loader.js';
+import {CodeOwnersModel} from './code-owners-model.js';
+
+/**
+ * The CodeOwnersMixin adds several properties to a class and translates
+ * 'model-property-changed' events from the model to the notifyPath calls.
+ * This allows to use properties of the model in observers, calculated
+ * properties and bindings.
+ * It is guaranteed, that model and modelLoader are set only if change,
+ * reporting and restApi properties are set.
+ */
+export const CodeOwnersModelMixin = Polymer.dedupingMixin(base => {
+ return class extends base {
+ constructor(...args) {
+ super(...args);
+ /**
+ * The modelLoader allows an element to request a property
+ * Typically should be used in loadPropertiesAfterModelChanged
+ * to ensure that all required model properties are loaded
+ */
+ this.modelLoader = undefined;
+ }
+ static get properties() {
+ return {
+ /* The following 3 properties (change, reporting, restApi) have to be
+ * set from the outside for the mixin to work.
+ */
+ change: Object,
+ reporting: Object,
+ restApi: Object,
+ model: {
+ type: Object,
+ observer: '_modelChanged',
+ },
+ };
+ }
+
+ static get observers() {
+ return ['onInputChanged(restApi, change, reporting)'];
+ }
+
+ onInputChanged(restApi, change, reporting) {
+ if ([restApi, change, reporting].includes(undefined)) {
+ this.model = undefined;
+ this.modelLoader = undefined;
+ return;
+ }
+ const ownerService = CodeOwnerService.getOwnerService(
+ this.restApi,
+ this.change
+ );
+ const model = CodeOwnersModel.getModel(change);
+ this.modelLoader = new ModelLoader(ownerService, model);
+ // Assign model after modelLoader, so modelLoader can be used in
+ // the _requirePropertiesAfterModelChanged method
+ this.model = model;
+ }
+
+ _modelChanged(newModel) {
+ if (this.modelPropertyChangedUnsubscriber) {
+ this.modelPropertyChangedUnsubscriber();
+ this.modelPropertyChangedUnsubscriber = undefined;
+ }
+ if (!newModel) return;
+ const propertyChangedListener = e => {
+ this.notifyPath(`model.${e.detail.propertyName}`);
+ };
+ newModel.addEventListener('model-property-changed',
+ propertyChangedListener);
+ this.modelPropertyChangedUnsubscriber = () => {
+ newModel.removeEventListener('model-property-changed',
+ propertyChangedListener);
+ };
+ this.loadPropertiesAfterModelChanged();
+ }
+
+ loadPropertiesAfterModelChanged() {
+ // The class should override this method and calls appropriate methods
+ // from this.modelLoader to ensure that all required properties are
+ // set in model.
+ }
+ };
+});
diff --git a/ui/code-owners-model.js b/ui/code-owners-model.js
new file mode 100644
index 0000000..cbd9bd2
--- /dev/null
+++ b/ui/code-owners-model.js
@@ -0,0 +1,157 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+export const SuggestionsState = {
+ NotLoaded: 'NotLoaded',
+ Loaded: 'Loaded',
+ Loading: 'Loading',
+ LoadFailed: 'LoadFailed',
+};
+
+export const PluginState = {
+ Enabled: 'Enabled',
+ Disabled: 'Disabled',
+ Failed: 'Failed',
+};
+
+/**
+ * Maintain the state of code-owners.
+ * Raises 'model-property-changed' event when a property is changed.
+ * The plugin shares the same model between all UI elements (if it is not,
+ * the plugin can't maintain showSuggestions state across different UI elements).
+ * UI elements use values from this model to display information
+ * and listens for the model-property-changed event. To do so, UI elements
+ * add CodeOwnersModelMixin, which is doing the listening and the translation
+ * from model-property-changed event to Polymer property-changed-event. The
+ * translation allows to use model properties in observables, bindings,
+ * computed properties, etc...
+ * The CodeOwnersModelLoader updates the model.
+ *
+ * It would be good to use RxJs Observable for implementing model properties.
+ * However, RxJs library is imported by Gerrit and there is no
+ * good way to reuse the same library in the plugin.
+ */
+export class CodeOwnersModel extends EventTarget {
+ constructor(change) {
+ super();
+ this.change = change;
+ this.branchConfig = undefined;
+ this.status = undefined;
+ this.userRole = undefined;
+ this.isCodeOwnerEnabled = undefined;
+ this.areAllFilesApproved = undefined;
+ this.suggestions = undefined;
+ this.suggestionsState = SuggestionsState.NotLoaded;
+ this.suggestionsLoadProgress = undefined;
+ this.showSuggestions = false;
+ this.pluginStatus = undefined;
+ }
+
+ setBranchConfig(config) {
+ if (this.branchConfig === config) return;
+ this.branchConfig = config;
+ this._firePropertyChanged('branchConfig');
+ }
+
+ setStatus(status) {
+ if (this.status === status) return;
+ this.status = status;
+ this._firePropertyChanged('status');
+ }
+
+ setUserRole(userRole) {
+ if (this.userRole === userRole) return;
+ this.userRole = userRole;
+ this._firePropertyChanged('userRole');
+ }
+
+ setIsCodeOwnerEnabled(enabled) {
+ if (this.isCodeOwnerEnabled === enabled) return;
+ this.isCodeOwnerEnabled = enabled;
+ this._firePropertyChanged('isCodeOwnerEnabled');
+ }
+
+ setAreAllFilesApproved(approved) {
+ if (this.areAllFilesApproved === approved) return;
+ this.areAllFilesApproved = approved;
+ this._firePropertyChanged('areAllFilesApproved');
+ }
+
+ setSuggestions(suggestions) {
+ if (this.suggestions === suggestions) return;
+ this.suggestions = suggestions;
+ this._firePropertyChanged('suggestions');
+ }
+
+ setSuggestionsState(state) {
+ if (this.suggestionsState === state) return;
+ this.suggestionsState = state;
+ this._firePropertyChanged('suggestionsState');
+ }
+
+ setSuggestionsLoadProgress(progress) {
+ if (this.suggestionsLoadProgress === progress) return;
+ this.suggestionsLoadProgress = progress;
+ this._firePropertyChanged('suggestionsLoadProgress');
+ }
+
+ setShowSuggestions(show) {
+ if (this.showSuggestions === show) return;
+ this.showSuggestions = show;
+ this._firePropertyChanged('showSuggestions');
+ }
+
+ setPluginEnabled(enabled) {
+ this._setPluginStatus({state: enabled ?
+ PluginState.Enabled : PluginState.Disabled});
+ }
+
+ setPluginFailed(failedMessage) {
+ this._setPluginStatus({state: PluginState.Failed, failedMessage});
+ }
+
+ _setPluginStatus(status) {
+ if (this._arePluginStatusesEqual(this.pluginStatus, status)) return;
+ this.pluginStatus = status;
+ this._firePropertyChanged('pluginStatus');
+ }
+
+ _arePluginStatusesEqual(status1, status2) {
+ if (status1 === undefined || status2 === undefined) {
+ return status1 === status2;
+ }
+ if (status1.state !== status2.state) return false;
+ return status1.state === PluginState.Failed ?
+ status1.failedMessage === status2.failedMessage :
+ true;
+ }
+
+ _firePropertyChanged(propertyName) {
+ this.dispatchEvent(new CustomEvent('model-property-changed', {
+ detail: {
+ propertyName,
+ },
+ }));
+ }
+
+ static getModel(change) {
+ if (!this.model || this.model.change !== change) {
+ this.model = new CodeOwnersModel(change);
+ }
+ return this.model;
+ }
+}
diff --git a/ui/code-owners-service.js b/ui/code-owners-service.js
index 416b3ec..0e07411 100644
--- a/ui/code-owners-service.js
+++ b/ui/code-owners-service.js
@@ -111,18 +111,6 @@
}
/**
- * Returns a promise fetching project_config for code owners.
- *
- * @doc https://gerrit.googlesource.com/plugins/code-owners/+/refs/heads/master/resources/Documentation/rest-api.md#get-code-owner-project-config
- * @param {string} project
- */
- getProjectConfig(project) {
- return this.restApi.get(
- `/projects/${encodeURIComponent(project)}/code_owners.project_config`
- );
- }
-
- /**
* 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
@@ -139,56 +127,191 @@
}
/**
+ * 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 = new Set(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.
*/
export class CodeOwnerService {
constructor(restApi, change, options = {}) {
this.restApi = restApi;
this.change = change;
- this.options = {maxConcurrentRequests: 10, ...options};
- this.codeOwnerApi = new CodeOwnerApi(restApi);
+ const codeOwnerApi = new CodeOwnerApi(restApi);
+ this.codeOwnerCacheApi = new CodeOwnersCacheApi(codeOwnerApi, change);
- // fetched files and fetching status
- this._fetchedOwners = new Map();
- this._fetchStatus = FetchStatus.NOT_STARTED;
- this._totalFetchCount = 0;
-
- this.init();
+ const fetcherOptions = {
+ maxConcurrentRequests: options.maxConcurrentRequests || 10,
+ };
+ this.ownersFetcher = new OwnersFetcher(restApi, change, fetcherOptions);
}
/**
- * Initial fetches.
+ * Prefetch data
*/
- init() {
- this.accountPromise = this.restApi.getLoggedIn().then(loggedIn => {
- if (!loggedIn) {
- return undefined;
- }
- return this.restApi.getAccount();
- });
-
- this.statusPromise = this.isCodeOwnerEnabled().then(enabled => {
- if (!enabled) {
- return {
- patchsetNumber: 0,
- enabled: false,
- codeOwnerStatusMap: new Map(),
- rawStatuses: [],
- };
- }
- return this.codeOwnerApi
- .listOwnerStatus(this.change._number)
- .then(res => {
- return {
- enabled: true,
- patchsetNumber: res.patch_set_number,
- codeOwnerStatusMap: this._formatStatuses(
- res.file_code_owner_statuses
- ),
- rawStatuses: res.file_code_owner_statuses,
- };
- });
- });
+ async prefetch() {
+ try {
+ await Promise.all([
+ this.codeOwnerCacheApi.getAccount(),
+ this.getStatus(),
+ ]);
+ } catch {
+ // Ignore any errors during prefetch.
+ // The same call from a different place throws the same exception
+ // again. The CodeOwnerService is not responsible for error processing.
+ }
}
/**
@@ -197,71 +320,92 @@
* For example, if a user removes themselves as a reviewer, the returned
* role 'REVIEWER' remains unchanged until the change view is reloaded.
*/
- getLoggedInUserInitialRole() {
- return this.accountPromise.then(account => {
- if (!account) {
- return UserRole.ANONYMOUS;
- }
- const change = this.change;
+ async getLoggedInUserInitialRole() {
+ const account = await this.codeOwnerCacheApi.getAccount();
+ if (!account) {
+ return UserRole.ANONYMOUS;
+ }
+ const change = this.change;
+ if (
+ change.revisions &&
+ change.current_revision &&
+ change.revisions[change.current_revision]
+ ) {
+ const commit = change.revisions[change.current_revision].commit;
if (
- change.revisions &&
- change.current_revision &&
- change.revisions[change.current_revision]
+ commit &&
+ commit.author &&
+ account.email &&
+ commit.author.email === account.email
) {
- const commit = change.revisions[change.current_revision].commit;
- if (
- commit &&
- commit.author &&
- account.email &&
- commit.author.email === account.email
- ) {
- return UserRole.AUTHOR;
- }
+ return UserRole.AUTHOR;
}
- if (change.owner._account_id === account._account_id) {
- return UserRole.CHANGE_OWNER;
+ }
+ if (change.owner._account_id === account._account_id) {
+ return UserRole.CHANGE_OWNER;
+ }
+ if (change.reviewers) {
+ if (this._accountInReviewers(change.reviewers.REVIEWER, account)) {
+ return UserRole.REVIEWER;
+ } else if (this._accountInReviewers(change.reviewers.CC, account)) {
+ return UserRole.CC;
+ } else if (this._accountInReviewers(change.reviewers.REMOVED, account)) {
+ return UserRole.REMOVED_REVIEWER;
}
- if (change.reviewers) {
- if (this._accountInReviewers(change.reviewers.REVIEWER, account)) {
- return UserRole.REVIEWER;
- } else if (this._accountInReviewers(change.reviewers.CC, account)) {
- return UserRole.CC;
- } else if (this._accountInReviewers(change.reviewers.REMOVED, account)) {
- return UserRole.REMOVED_REVIEWER;
- }
- }
- return UserRole.OTHER;
- })
+ }
+ return UserRole.OTHER;
}
_accountInReviewers(reviewers, account) {
if (!reviewers) {
return false;
}
- return reviewers.some(reviewer => reviewer._account_id === account._account_id);
+ return reviewers.some(reviewer =>
+ reviewer._account_id === account._account_id);
}
- getStatus() {
- return this.statusPromise.then(res => {
- if (res.enabled && !this.isOnLatestPatchset(res.patchsetNumber)) {
- // status is outdated, abort and re-init
- this.abort();
- this.init();
- return this.statusPromise;
- }
- return res;
- });
+ async getStatus() {
+ const status = await this._getStatus();
+ if (status.enabled && !this.isOnLatestPatchset(status.patchsetNumber)) {
+ // status is outdated, abort and re-init
+ this.abort();
+ this.prefetch();
+ return await this.codeOwnerCacheApi.getStatus();
+ }
+ return status;
}
- areAllFilesApproved() {
- return this.getStatus().then(({rawStatuses}) => {
- return !rawStatuses.some(status => {
- const oldPathStatus = status.old_path_status;
- const newPathStatus = status.new_path_status;
- // For deleted files, no new_path_status exists
- return (newPathStatus && newPathStatus.status !== OwnerStatus.APPROVED)
- || (oldPathStatus && oldPathStatus.status !== OwnerStatus.APPROVED);
- });
+ async _getStatus() {
+ const enabled = await this.isCodeOwnerEnabled();
+ if (!enabled) {
+ return {
+ patchsetNumber: 0,
+ enabled: false,
+ codeOwnerStatusMap: new Map(),
+ rawStatuses: [],
+ };
+ }
+
+ const onwerStatus = await this.codeOwnerCacheApi.listOwnerStatus();
+
+ return {
+ enabled: true,
+ patchsetNumber: onwerStatus.patch_set_number,
+ codeOwnerStatusMap: this._formatStatuses(
+ onwerStatus.file_code_owner_statuses
+ ),
+ rawStatuses: onwerStatus.file_code_owner_statuses,
+ };
+ }
+
+ async areAllFilesApproved() {
+ const {rawStatuses} = await this.getStatus();
+ return !rawStatuses.some(status => {
+ const oldPathStatus = status.old_path_status;
+ const newPathStatus = status.new_path_status;
+ // For deleted files, no new_path_status exists
+ return (newPathStatus && newPathStatus.status !== OwnerStatus.APPROVED)
+ || (oldPathStatus && oldPathStatus.status !== OwnerStatus.APPROVED);
});
}
@@ -282,57 +426,37 @@
* }>
* }}
*/
- getSuggestedOwners() {
+ async getSuggestedOwners() {
+ const {codeOwnerStatusMap} = await this.getStatus();
+
// 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._fetchStatus === FetchStatus.NOT_STARTED
- || this._fetchStatus === FetchStatus.ABORT) {
- this._fetchSuggestedOwners().then(() => {
- this._fetchStatus = FetchStatus.FINISHED;
- });
+ if (this.ownersFetcher.getStatus() === FetchStatus.NOT_STARTED
+ || this.ownersFetcher.getStatus() === FetchStatus.ABORT) {
+ await this.ownersFetcher.fetchSuggestedOwners(codeOwnerStatusMap);
}
- return this.getStatus().then(({codeOwnerStatusMap}) => {
- return {
- finished: this._fetchStatus === FetchStatus.FINISHED,
- status: this._fetchStatus,
- progress: this._totalFetchCount === 0 ?
- `Loading suggested owners ...` :
- `${this._fetchedOwners.size} out of ${this._totalFetchCount} files have returned suggested owners.`,
- suggestions: this._groupFilesByOwners(codeOwnerStatusMap),
- };
- });
+ return {
+ finished: this.ownersFetcher.getStatus() === FetchStatus.FINISHED,
+ status: this.ownersFetcher.getStatus(),
+ progress: this.ownersFetcher.getProgressString(),
+ suggestions: this._groupFilesByOwners(codeOwnerStatusMap,
+ this.ownersFetcher.getFiles()),
+ };
}
- _fetchSuggestedOwners() {
- // reset existing temporary storage
- this._fetchedOwners = new Map();
- this._fetchStatus = FetchStatus.FETCHING;
- this._totalFetchCount = 0;
-
- return this.getStatus()
- .then(({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
- const filesToFetch = filesGroupByStatus.missing.concat(filesGroupByStatus.pending);
- this._totalFetchCount = filesToFetch.length;
- return this._batchFetchCodeOwners(filesToFetch);
- });
+ async getSuggestedOwnersProgress() {
+ const {codeOwnerStatusMap} = await this.getStatus();
+ return {
+ finished: this.ownersFetcher.getStatus() === FetchStatus.FINISHED,
+ status: this.ownersFetcher.getStatus(),
+ progress: this.ownersFetcher.getProgressString(),
+ suggestions: this._groupFilesByOwners(codeOwnerStatusMap,
+ this.ownersFetcher.getFiles()),
+ };
}
_formatStatuses(statuses) {
@@ -368,28 +492,27 @@
return;
}
- _groupFilesByOwners(codeOwnerStatusMap) {
+ _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 allFiles = [...this._fetchedOwners.keys()];
const ownersFilesMap = new Map();
const failedToFetchFiles = new Set();
- for (let i = 0; i < allFiles.length; i++) {
+ for (const file of files) {
const fileInfo = {
- path: allFiles[i],
- status: this._computeFileStatus(codeOwnerStatusMap, allFiles[i]),
+ path: file.path,
+ status: this._computeFileStatus(codeOwnerStatusMap, file.path),
};
// for files failed to fetch, add them to the special group
- if (this._fetchedOwners.get(fileInfo.path).error) {
+ if (file.info.error) {
failedToFetchFiles.add(fileInfo);
continue;
}
// do not include files still in fetching
- if (!this._fetchedOwners.get(fileInfo.path).owners) {
+ if (!file.info.owners) {
continue;
}
- const owners = [...this._fetchedOwners.get(fileInfo.path).owners];
+ const owners = [...file.info.owners];
const ownersKey = owners
.map(owner => owner.account._account_id)
.sort()
@@ -415,7 +538,8 @@
groupedItems.push({
groupName: this.getGroupName(failedFiles),
files: failedFiles,
- error: new Error('Failed to fetch code owner info. Try to refresh the page.'),
+ error: new Error(
+ 'Failed to fetch code owner info. Try to refresh the page.'),
});
}
@@ -435,78 +559,23 @@
return `${latestRevision._number}` === `${patchsetId}`;
}
- /**
- * Recursively fetches code owners for all files until finished.
- *
- * @param {!Array<string>} files
- */
- _batchFetchCodeOwners(files) {
- if (this._fetchStatus === FetchStatus.ABORT) {
- return Promise.resolve(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.codeOwnerApi
- .listOwnersForPath(
- this.change.id,
- filePath
- )
- .then(owners => {
- // use Set to de-dup
- this._fetchedOwners.get(filePath).owners = new Set(owners);
- })
- .catch(e => {
- this._fetchedOwners.get(filePath).error = e;
- })
- );
- }
- }
- const resPromise = Promise.all(batchRequests);
- if (files.length > maxConcurrentRequests) {
- return resPromise.then(() => {
- return this._batchFetchCodeOwners(files.slice(maxConcurrentRequests));
- });
- }
- return resPromise.then(() => this._fetchedOwners);
- }
-
abort() {
- this._fetchStatus = FetchStatus.ABORT;
- this._fetchedOwners = new Map();
- this._totalFetchCount = 0;
+ this.ownersFetcher.abort();
+ const codeOwnerApi = new CodeOwnerApi(this.restApi);
+ this.codeOwnerCacheApi = new CodeOwnersCacheApi(codeOwnerApi, change);
}
- getProjectConfig() {
- if (!this.getProjectConfigPromise) {
- this.getProjectConfigPromise =
- this.codeOwnerApi.getProjectConfig(this.change.project);
- }
- return this.getProjectConfigPromise;
+ async getBranchConfig() {
+ return this.codeOwnerCacheApi.getBranchConfig();
}
- getBranchConfig() {
- if (!this.getBranchConfigPromise) {
- this.getBranchConfigPromise =
- this.codeOwnerApi.getBranchConfig(this.change.project,
- this.change.branch);
- }
- return this.getBranchConfigPromise;
- }
-
- isCodeOwnerEnabled() {
+ async isCodeOwnerEnabled() {
if (this.change.status === ChangeStatus.ABANDONED ||
this.change.status === ChangeStatus.MERGED) {
- return Promise.resolve(false);
+ return false;
}
- return this.getBranchConfig().then(config => {
- return !(config.status && config.status.disabled);
- });
+ const config = await this.codeOwnerCacheApi.getBranchConfig();
+ return config && !config.disabled;
}
static getOwnerService(restApi, change) {
@@ -515,7 +584,9 @@
// Chrome has a limit of 6 connections per host name, and a max of 10 connections.
maxConcurrentRequests: 6,
});
+ this.ownerService.prefetch();
}
return this.ownerService;
}
}
+
diff --git a/ui/code-owners-service_test.html b/ui/code-owners-service_test.html
index 6986ea9..884b97f 100644
--- a/ui/code-owners-service_test.html
+++ b/ui/code-owners-service_test.html
@@ -98,12 +98,14 @@
suite('basic api request tests', () => {
setup(done => {
- getApiStub.withArgs(`/changes/${fakeChange._number}/code_owners.status`).returns(Promise.resolve(fakeStatus));
- codeOwnersService = CodeOwnerService.getOwnerService(fakeRestApi, {...fakeChange});
+ getApiStub.withArgs(`/changes/${fakeChange._number}/code_owners.status`)
+ .returns(Promise.resolve(fakeStatus));
+ codeOwnersService = CodeOwnerService.getOwnerService(fakeRestApi,
+ {...fakeChange});
flush(done);
});
- test('same change should return the same instance through getOwnerService', () => {
+ test('getOwnerService - same change returns the same instance', () => {
assert.equal(
CodeOwnerService.getOwnerService(fakeRestApi, fakeChange),
CodeOwnerService.getOwnerService(fakeRestApi, fakeChange)
@@ -119,7 +121,8 @@
test('should fetch status after init', () => {
assert.isTrue(getApiStub.calledOnce);
- assert.equal(getApiStub.lastCall.args[0], `/changes/${fakeChange._number}/code_owners.status`);
+ assert.equal(getApiStub.lastCall.args[0],
+ `/changes/${fakeChange._number}/code_owners.status`);
});
test('getSuggestion should kickoff the fetch', done => {
@@ -144,33 +147,35 @@
suite('all approved case', () => {
setup(done => {
- getApiStub.withArgs(`/changes/${fakeChange._number}/code_owners.status`).returns(Promise.resolve({
- // fake data with fake files
- patch_set_number: 1,
- file_code_owner_statuses: [
- {
- new_path_status: {
- path: 'a.js',
- status: 'APPROVED',
- },
- },
- {
- new_path_status: {
- path: 'b.js',
- status: 'APPROVED',
- },
- },
- {
- old_path_status: {
- path: 'd.js',
- status: 'APPROVED',
- },
- change_type: 'DELETED',
- },
- ],
- }));
+ getApiStub.withArgs(`/changes/${fakeChange._number}/code_owners.status`)
+ .returns(Promise.resolve({
+ // fake data with fake files
+ patch_set_number: 1,
+ file_code_owner_statuses: [
+ {
+ new_path_status: {
+ path: 'a.js',
+ status: 'APPROVED',
+ },
+ },
+ {
+ new_path_status: {
+ path: 'b.js',
+ status: 'APPROVED',
+ },
+ },
+ {
+ old_path_status: {
+ path: 'd.js',
+ status: 'APPROVED',
+ },
+ change_type: 'DELETED',
+ },
+ ],
+ }));
- codeOwnersService = CodeOwnerService.getOwnerService(fakeRestApi, {...fakeChange});
+ codeOwnersService = CodeOwnerService.getOwnerService(fakeRestApi,
+ {...fakeChange});
flush(done);
});
@@ -196,11 +201,12 @@
});
suite('abort', () => {
- let resolver;
setup(done => {
- getApiStub.withArgs(`/changes/${fakeChange._number}/code_owners.status`).returns(Promise.resolve(fakeStatus));
+ getApiStub.withArgs(`/changes/${fakeChange._number}/code_owners.status`)
+ .returns(Promise.resolve(fakeStatus));
- codeOwnersService = CodeOwnerService.getOwnerService(fakeRestApi, {...fakeChange});
+ codeOwnersService = CodeOwnerService.getOwnerService(fakeRestApi,
+ {...fakeChange});
flush(done);
});
diff --git a/ui/owner-requirement.js b/ui/owner-requirement.js
index 7bd74ee..d08bacc 100644
--- a/ui/owner-requirement.js
+++ b/ui/owner-requirement.js
@@ -15,8 +15,10 @@
* limitations under the License.
*/
-import {CodeOwnerService, OwnerStatus} from './code-owners-service.js';
-import {ownerState} from './owner-ui-state.js';
+import {OwnerStatus} from './code-owners-service.js';
+import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
+import {showPluginFailedMessage} from './code-owners-banner.js';
+import {PluginState} from './code-owners-model.js';
/**
* Owner requirement control for `submit-requirement-item-code-owners` endpoint.
@@ -24,7 +26,8 @@
* This will show the status and suggest owners button next to
* the code-owners submit requirement.
*/
-export class OwnerRequirementValue extends Polymer.Element {
+export class OwnerRequirementValue extends
+ CodeOwnersModelMixin(Polymer.Element) {
static get is() {
return 'owner-requirement-value';
}
@@ -61,16 +64,35 @@
Loading status ...
</p>
<template is="dom-if" if="[[!_isLoading]]">
- <span>[[_computeStatusText(_statusCount, _isOverriden)]]</span>
- <template is="dom-if" if="[[_overrideInfoUrl]]">
- <a on-click="_reportDocClick" href="[[_overrideInfoUrl]]" target="_blank">
- <iron-icon icon="gr-icons:help-outline" title="Documentation for overriding code owners"></iron-icon>
- </a>
+ <template is="dom-if" if="[[!_pluginFailed(model.pluginStatus)]]">
+ <template is="dom-if" if="[[!model.branchConfig.no_code_owners_defined]]">
+ <span>[[_computeStatusText(_statusCount, _isOverriden)]]</span>
+ <template is="dom-if" if="[[_overrideInfoUrl]]">
+ <a on-click="_reportDocClick" href="[[_overrideInfoUrl]]"
+ target="_blank">
+ <iron-icon icon="gr-icons:help-outline"
+ title="Documentation for overriding code owners"></iron-icon>
+ </a>
+ </template>
+ <template is="dom-if" if="[[!_allApproved]]">
+ <gr-button link on-click="_openReplyDialog">
+ Suggest owners
+ </gr-button>
+ </template>
+ </template>
+ <template is="dom-if" if="[[model.branchConfig.no_code_owners_defined]]">
+ <span>No code-owners file</span>
+ <a href="https://gerrit.googlesource.com/plugins/code-owners/+/master/resources/Documentation/user-guide.md#how-to-submit-changes-with-files-that-have-no-code-owners" target="_blank">
+ <iron-icon icon="gr-icons:help-outline"
+ title="Documentation about submitting changes with files that have no code owners?"></iron-icon>
+ </a>
+ </template>
</template>
- <template is="dom-if" if="[[!_allApproved]]">
- <gr-button link on-click="_openReplyDialog">
- Suggest owners
- </gr-button>
+ <template is="dom-if" if="[[_pluginFailed(model.pluginStatus)]]">
+ <span>Code-owners plugin has failed</span>
+ <gr-button link on-click="_showFailDetails">
+ Details
+ </gr-button>
</template>
</template>
`;
@@ -78,87 +100,93 @@
static get properties() {
return {
- change: Object,
- reporting: Object,
- restApi: Object,
-
- ownerService: Object,
-
_statusCount: Object,
_isLoading: {
type: Boolean,
- value: true,
+ computed: '_computeIsLoading(model.branchConfig, model.status, '
+ + 'model.userRole, model.pluginStatus)',
},
_allApproved: {
type: Boolean,
computed: '_computeAllApproved(_statusCount)',
},
- _isOverriden: Boolean,
- _overrideInfoUrl: String,
+ _isOverriden: {
+ type: Boolean,
+ computed: '_computeIsOverriden(model.branchConfig)',
+ },
+ _overrideInfoUrl: {
+ type: String,
+ computed: '_computeOverrideInfoUrl(model.branchConfig)',
+ },
};
}
static get observers() {
return [
- 'onInputChanged(restApi, change, reporting)',
+ '_onStatusChanged(model.status, model.userRole)',
];
}
- _checkIfOverriden() {
- this.ownerService.getBranchConfig().then(res => {
- if (!res['override_approval']) {
- // no override label configured
- this._isOverriden = false;
- return;
- }
-
- const overridenLabel = res['override_approval'].label;
- const overridenValue = res['override_approval'].value;
-
- if (this.change.labels[overridenLabel]) {
- const votes = this.change.labels[overridenLabel].all || [];
- if (votes.find(v => `${v.value}` === `${overridenValue}`)) {
- this._isOverriden = true;
- return;
- }
- }
-
- // otherwise always reset it to false
- this._isOverriden = false;
- });
- }
-
- _updateStatus() {
- this._isLoading = true;
+ loadPropertiesAfterModelChanged() {
+ super.loadPropertiesAfterModelChanged();
this.reporting.reportLifeCycle('owners-submit-requirement-summary-start');
-
- return this.ownerService.getStatus()
- .then(({rawStatuses}) => {
- this._statusCount = this._getStatusCount(rawStatuses);
- this.ownerService.getLoggedInUserInitialRole().then(role => {
- // Send a metric with overall summary when code owners submit
- // requirement shown and finished fetching status
- this.reporting.reportLifeCycle(
- 'owners-submit-requirement-summary-shown',
- {...this._statusCount, user_role: role}
- );
- });
- })
- .finally(() => {
- this._isLoading = false;
- });
+ this.modelLoader.loadBranchConfig();
+ this.modelLoader.loadStatus();
+ this.modelLoader.loadUserRole();
}
- _updateOverrideInfoUrl() {
- this.ownerService.getBranchConfig().then(config => {
- this._overrideInfoUrl = config.general && config.general.override_info_url
- ?
- config.general.override_info_url : '';
- });
+ _computeIsLoading(branchConfig, status, userRole, pluginStatus) {
+ if (this._pluginFailed(pluginStatus)) {
+ return false;
+ }
+ return !branchConfig || !status || !userRole;
+ }
+
+ _pluginFailed(pluginStatus) {
+ return pluginStatus && pluginStatus.state === PluginState.Failed;
+ }
+
+ _onStatusChanged(status, userRole) {
+ if (!status || !userRole) {
+ this._statusCount = undefined;
+ return;
+ }
+ const rawStatuses = status.rawStatuses;
+ this._statusCount = this._getStatusCount(rawStatuses);
+ this.reporting.reportLifeCycle('owners-submit-requirement-summary-shown',
+ {...this._statusCount, user_role: userRole});
+ }
+
+ _computeOverrideInfoUrl(branchConfig) {
+ if (!branchConfig) {
+ return '';
+ }
+ return branchConfig.general && branchConfig.general.override_info_url
+ ? branchConfig.general.override_info_url : '';
+ }
+
+ _computeIsOverriden(branchConfig) {
+ if (!branchConfig || !branchConfig['override_approval']) {
+ // no override label configured
+ return false;
+ }
+
+ const overridenLabel = branchConfig['override_approval'].label;
+ const overridenValue = branchConfig['override_approval'].value;
+
+ if (this.change.labels[overridenLabel]) {
+ const votes = this.change.labels[overridenLabel].all || [];
+ if (votes.find(v => `${v.value}` === `${overridenValue}`)) {
+ return true;
+ }
+ }
+
+ // otherwise always reset it to false
+ return false;
}
_computeAllApproved(statusCount) {
- return statusCount.missing === 0
+ return statusCount && statusCount.missing === 0
&& statusCount.pending === 0;
}
@@ -186,6 +214,7 @@
}
_computeStatusText(statusCount, isOverriden) {
+ if (statusCount === undefined || isOverriden === undefined) return '';
const statusText = [];
if (statusCount.missing) {
statusText.push(`${statusCount.missing} missing`);
@@ -210,15 +239,8 @@
return status === OwnerStatus.PENDING;
}
- onInputChanged(restApi, change, reporting) {
- if ([restApi, change, reporting].includes(undefined)) return;
- this.ownerService = CodeOwnerService.getOwnerService(this.restApi, change);
- this._updateStatus();
- this._checkIfOverriden();
- this._updateOverrideInfoUrl();
- }
-
_openReplyDialog() {
+ this.model.setShowSuggestions(true);
this.dispatchEvent(
new CustomEvent('open-reply-dialog', {
detail: {},
@@ -226,11 +248,12 @@
bubbles: true,
})
);
- ownerState.expandSuggestion = true;
- this.ownerService.getLoggedInUserInitialRole().then(role => {
- this.reporting.reportInteraction(
- 'suggest-owners-from-submit-requirement', {user_role: role});
- });
+ this.reporting.reportInteraction('suggest-owners-from-submit-requirement',
+ {user_role: this.model.userRole});
+ }
+
+ _showFailDetails() {
+ showPluginFailedMessage(this, this.model.pluginStatus);
}
}
diff --git a/ui/owner-status-column.js b/ui/owner-status-column.js
index 3fdcd50..31ad654 100644
--- a/ui/owner-status-column.js
+++ b/ui/owner-status-column.js
@@ -15,7 +15,8 @@
* limitations under the License.
*/
-import {CodeOwnerService, OwnerStatus} from './code-owners-service.js';
+import {OwnerStatus} from './code-owners-service.js';
+import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
const MAGIC_FILES = ['/COMMIT_MSG', '/MERGE_LIST', '/PATCHSET_LEVEL'];
const STATUS_CODE = {
@@ -38,13 +39,15 @@
const STATUS_TOOLTIP = {
[STATUS_CODE.PENDING]: 'Pending code owner approval',
[STATUS_CODE.MISSING]: 'Missing code owner approval',
- [STATUS_CODE.PENDING_OLD_PATH]: 'Pending code owner approval on pre-renamed file',
- [STATUS_CODE.MISSING_OLD_PATH]: 'Missing code owner approval on pre-renamed file',
+ [STATUS_CODE.PENDING_OLD_PATH]:
+ 'Pending code owner approval on pre-renamed file',
+ [STATUS_CODE.MISSING_OLD_PATH]:
+ 'Missing code owner approval on pre-renamed file',
[STATUS_CODE.APPROVED]: 'Approved by code owner',
[STATUS_CODE.ERROR]: 'Failed to fetch code owner status',
};
-class BaseEl extends Polymer.Element {
+class BaseEl extends CodeOwnersModelMixin(Polymer.Element) {
computeHidden(change, patchRange) {
if ([change, patchRange].includes(undefined)) return true;
// if code-owners is not a submit requirement, don't show status column
@@ -64,11 +67,6 @@
if (`${patchRange.patchNum}` !== `${latestPatchset._number}`) return true;
return false;
}
-
- onInputChanged(restApi, change) {
- if ([restApi, change].includes(undefined)) return;
- this.ownerService = CodeOwnerService.getOwnerService(this.restApi, change);
- }
}
/**
@@ -94,26 +92,15 @@
static get properties() {
return {
- change: Object,
- reporting: Object,
patchRange: Object,
- restApi: Object,
hidden: {
type: Boolean,
reflectToAttribute: true,
computed: 'computeHidden(change, patchRange)',
},
- ownerService: Object,
};
}
-
- static get observers() {
- return [
- 'onInputChanged(restApi, change)',
- 'onOwnerServiceChanged(ownerServive)',
- ];
- }
}
customElements.define(OwnerStatusColumnHeader.is, OwnerStatusColumnHeader);
@@ -128,10 +115,6 @@
static get properties() {
return {
- change: Object,
- reporting: Object,
- restApi: Object,
-
path: String,
patchRange: Object,
hidden: {
@@ -186,47 +169,45 @@
static get observers() {
return [
- 'onInputChanged(restApi, change)',
- 'computeStatusIcon(ownerService, path)',
+ 'computeStatusIcon(model.status, path)',
];
}
- computeStatusIcon(ownerService, path) {
- if ([ownerService, path].includes(undefined)) return;
+ loadPropertiesAfterModelChanged() {
+ super.loadPropertiesAfterModelChanged();
+ this.modelLoader.loadStatus();
+ }
+
+ computeStatusIcon(modelStatus, path) {
+ if ([modelStatus, path].includes(undefined)) return;
if (MAGIC_FILES.includes(path)) return;
- ownerService.getStatus()
- .then(({codeOwnerStatusMap}) => {
- const statusItem = codeOwnerStatusMap.get(path);
- if (!statusItem) {
- this.status = STATUS_CODE.ERROR;
- return;
- }
+ const codeOwnerStatusMap = modelStatus.codeOwnerStatusMap;
+ const statusItem = codeOwnerStatusMap.get(path);
+ if (!statusItem) {
+ this.status = STATUS_CODE.ERROR;
+ return;
+ }
- const status = statusItem.status;
- let oldPathStatus = null;
- if (statusItem.oldPath) {
- const oldStatusItem = codeOwnerStatusMap.get(statusItem.oldPath);
- if (!oldStatusItem) {
- // should not happen
- } else {
- oldPathStatus = oldStatusItem.status;
- }
- }
+ const status = statusItem.status;
+ let oldPathStatus = null;
+ if (statusItem.oldPath) {
+ const oldStatusItem = codeOwnerStatusMap.get(statusItem.oldPath);
+ if (!oldStatusItem) {
+ // should not happen
+ } else {
+ oldPathStatus = oldStatusItem.status;
+ }
+ }
- const newPathStatus = this._computeStatus(status);
- if (!oldPathStatus) {
- this.status = newPathStatus;
- } else {
- this.status = newPathStatus === STATUS_CODE.APPROVED
- ? this._computeStatus(oldPathStatus, /* oldPath= */ true)
- : newPathStatus;
- }
- })
- .catch(e => {
- this.status = STATUS_CODE.ERROR;
- throw e;
- });
+ const newPathStatus = this._computeStatus(status);
+ if (!oldPathStatus) {
+ this.status = newPathStatus;
+ } else {
+ this.status = newPathStatus === STATUS_CODE.APPROVED
+ ? this._computeStatus(oldPathStatus, /* oldPath= */ true)
+ : newPathStatus;
+ }
}
_computeIcon(status) {
@@ -248,4 +229,4 @@
}
}
-customElements.define(OwnerStatusColumnContent.is, OwnerStatusColumnContent);
\ No newline at end of file
+customElements.define(OwnerStatusColumnContent.is, OwnerStatusColumnContent);
diff --git a/ui/owner-ui-state.js b/ui/owner-ui-state.js
deleted file mode 100644
index 3d47ef9..0000000
--- a/ui/owner-ui-state.js
+++ /dev/null
@@ -1,69 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 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.
- */
-
-/**
- * @enum
- */
-const OwnerUIStateEventType = {
- EXPAND_SUGGESTION: 'expandSuggestion',
-};
-
-/**
- * For states used in code owners plugin across multiple components.
- */
-class OwnerUIState {
- constructor() {
- this._expandSuggestion = false;
- this._listeners = new Map();
- this._listeners.set(OwnerUIStateEventType.EXPAND_SUGGESTION, []);
- }
-
- get expandSuggestion() {
- return this._expandSuggestion;
- }
-
- set expandSuggestion(value) {
- this._expandSuggestion = value;
- this._listeners.get(OwnerUIStateEventType.EXPAND_SUGGESTION).forEach(cb => {
- try {
- cb(value);
- } catch (e) {
- console.warn(e);
- }
- });
- }
-
- _subscribeEvent(eventType, cb) {
- this._listeners.get(eventType).push(cb);
- return () => {
- this._unsubscribeEvent(eventType, cb);
- };
- }
-
- _unsubscribeEvent(eventType, cb) {
- this._listeners.set(
- eventType,
- this._listeners.get(eventType).filter(handler => handler !== cb)
- );
- }
-
- onExpandSuggestionChange(cb) {
- return this._subscribeEvent(OwnerUIStateEventType.EXPAND_SUGGESTION, cb);
- }
-}
-
-export const ownerState = new OwnerUIState();
diff --git a/ui/plugin.js b/ui/plugin.js
index 1a278dd..40bc4e5 100644
--- a/ui/plugin.js
+++ b/ui/plugin.js
@@ -19,11 +19,21 @@
import {OwnerStatusColumnContent, OwnerStatusColumnHeader} from './owner-status-column.js';
import {OwnerRequirementValue} from './owner-requirement.js';
import {SuggestOwnersTrigger} from './suggest-owners-trigger.js';
+import {CodeOwnersBanner, CodeOwnersPluginStatusNotifier} from './code-owners-banner.js';
Gerrit.install(plugin => {
const restApi = plugin.restApi();
const reporting = plugin.reporting();
+ plugin.registerCustomComponent('banner', CodeOwnersBanner.is);
+
+ plugin.registerCustomComponent(
+ 'change-view-integration', CodeOwnersPluginStatusNotifier.is)
+ .onAttached(view => {
+ view.restApi = restApi;
+ view.reporting = reporting;
+ });
+
// owner status column / rows for file list
plugin.registerDynamicCustomComponent(
'change-view-file-list-header-prepend', OwnerStatusColumnHeader.is)
@@ -61,4 +71,4 @@
view.restApi = restApi;
view.reporting = reporting;
});
-});
\ No newline at end of file
+});
diff --git a/ui/suggest-owners-trigger.js b/ui/suggest-owners-trigger.js
index 2c0ccc0..97f616d 100644
--- a/ui/suggest-owners-trigger.js
+++ b/ui/suggest-owners-trigger.js
@@ -14,41 +14,26 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {CodeOwnerService} from './code-owners-service.js';
-import {ownerState} from './owner-ui-state.js';
+import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
+import {PluginState} from './code-owners-model.js';
-export class SuggestOwnersTrigger extends Polymer.Element {
+export class SuggestOwnersTrigger extends
+ CodeOwnersModelMixin(Polymer.Element) {
static get is() {
return 'suggest-owners-trigger';
}
- constructor(props) {
- super(props);
- this.expandSuggestionStateUnsubscriber = undefined;
- }
-
static get properties() {
return {
- change: Object,
- reporting: Object,
- restApi: Object,
-
- expanded: {
- type: Boolean,
- value: false,
- },
hidden: {
type: Boolean,
- value: true,
+ computed: '_computeHidden(model.pluginStatus,' +
+ 'model.areAllFilesApproved, model.userRole, model.branchConfig)',
reflectToAttribute: true,
},
};
}
- static get observers() {
- return ['onInputChanged(restApi, change)'];
- }
-
static get template() {
return Polymer.html`
<style include="shared-styles">
@@ -68,7 +53,7 @@
has-tooltip
title="Suggest owners for your change"
>
- [[computeButtonText(expanded)]]
+ [[computeButtonText(model.showSuggestions)]]
</gr-button>
<span>
<a on-click="_reportBugClick" href="https://bugs.chromium.org/p/gerrit/issues/entry?template=code-owners-plugin" target="_blank">
@@ -81,50 +66,35 @@
`;
}
- connectedCallback() {
- super.connectedCallback();
- this.expandSuggestionStateUnsubscriber = ownerState
- .onExpandSuggestionChange(expanded => {
- this.expanded = expanded;
- });
+ loadPropertiesAfterModelChanged() {
+ super.loadPropertiesAfterModelChanged();
+ this.modelLoader.loadUserRole();
+ this.modelLoader.loadPluginStatus();
+ this.modelLoader.loadAreAllFilesApproved();
+ this.modelLoader.loadBranchConfig();
}
- disconnnectedCallback() {
- super.disconnectedCallback();
- if (this.expandSuggestionStateUnsubscriber) {
- this.expandSuggestionStateUnsubscriber();
- this.expandSuggestionStateUnsubscriber = undefined;
+ _computeHidden(pluginStatus, allFilesApproved, userRole, branchConfig) {
+ if (pluginStatus === undefined ||
+ allFilesApproved === undefined ||
+ userRole === undefined ||
+ branchConfig === undefined) {
+ return true;
+ }
+ if (branchConfig.no_code_owners_defined) return true;
+ if (pluginStatus.state === PluginState.Enabled) {
+ return allFilesApproved;
+ } else {
+ return true;
}
}
- onInputChanged(restApi, change) {
- if ([restApi, change].includes(undefined)) return;
- this.ownerService = CodeOwnerService.getOwnerService(
- this.restApi,
- this.change
- );
-
- Promise.all([
- this.ownerService.isCodeOwnerEnabled(),
- this.ownerService.areAllFilesApproved(),
- this.ownerService.getLoggedInUserInitialRole()
- ])
- .then(([enabled, approved, userRole]) => {
- if (enabled) {
- this.hidden = approved;
- } else {
- this.hidden = true;
- }
- this._userRole = userRole;
- });
- }
-
toggleControlContent() {
- this.expanded = !this.expanded;
- ownerState.expandSuggestion = this.expanded;
+ this.model.setShowSuggestions(!this.model.showSuggestions);
this.reporting.reportInteraction('toggle-suggest-owners', {
expanded: this.expanded,
- user_role: this._userRole ? this._userRole : 'UNKNOWN',
+ user_role: this.model.userRole ?
+ this.model.userRole : 'UNKNOWN',
});
}
diff --git a/ui/suggest-owners.js b/ui/suggest-owners.js
index 2e1a487..e0bf8a8 100644
--- a/ui/suggest-owners.js
+++ b/ui/suggest-owners.js
@@ -14,8 +14,8 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {CodeOwnerService} from './code-owners-service.js';
-import {ownerState} from './owner-ui-state.js';
+import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
+import {SuggestionsState} from './code-owners-model.js';
const SUGGESTION_POLLING_INTERVAL = 1000;
@@ -24,11 +24,6 @@
return 'owner-group-file-list';
}
- constructor() {
- super();
- this.expandSuggestionStateUnsubscriber = undefined;
- }
-
static get properties() {
return {
files: Array,
@@ -89,7 +84,7 @@
customElements.define(OwnerGroupFileList.is, OwnerGroupFileList);
-export class SuggestOwners extends Polymer.Element {
+export class SuggestOwners extends CodeOwnersModelMixin(Polymer.Element) {
static get is() {
return 'suggest-owners';
}
@@ -262,6 +257,7 @@
<template is="dom-if" if="[[suggestion.error]]">
<div class="fetch-error-content">
[[suggestion.error]]
+ <a on-click="_showErrorDetails"
</div>
</template>
<template is="dom-if" if="[[!suggestion.error]]">
@@ -299,18 +295,14 @@
static get properties() {
return {
- // @input
- change: Object,
- restApi: Object,
- reporting: Object,
-
// @internal attributes
hidden: {
type: Boolean,
value: true,
reflectToAttribute: true,
+ computed: '_isHidden(model.areAllFilesApproved, ' +
+ 'model.showSuggestions)',
},
- ownerService: Object,
suggestedOwners: Array,
isLoading: {
type: Boolean,
@@ -325,93 +317,87 @@
static get observers() {
return [
- 'onInputChanged(restApi, change)',
- 'onReviewerChange(reviewers)',
+ '_onReviewerChanged(reviewers)',
+ '_onShowSuggestionsChanged(model.showSuggestions)',
+ '_onSuggestionsStateChanged(model.suggestionsState)',
+ '_onSuggestionsChanged(model.suggestions, model.suggestionsState)',
+ '_onSuggestionsLoadProgressChanged(model.suggestionsLoadProgress)',
];
}
- connectedCallback() {
- super.connectedCallback();
- this.expandSuggestionStateUnsubscriber = ownerState
- .onExpandSuggestionChange(expanded => {
- this.hidden = !expanded;
- if (expanded) {
- // this is more of a hack to let reivew input lose focus
- // to avoid suggestion dropdown
- // gr-autocomplete has a internal state for tracking focus
- // that will be canceled if any click happens outside of
- // it's target
- // 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);
+ _onShowSuggestionsChanged(showSuggestions) {
+ if (!showSuggestions ||
+ this.model.suggestionsLoadProgress === SuggestionsState.NotLoaded) {
+ return;
+ }
+ // this is more of a hack to let review input lose focus
+ // to avoid suggestion dropdown
+ // gr-autocomplete has a internal state for tracking focus
+ // that will be canceled if any click happens outside of
+ // it's target
+ // 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);
- // start fetching suggestions
- if (!this._suggestionsTimer) {
- this._suggestionsTimer = setInterval(() => {
- this._pollingSuggestions();
- }, SUGGESTION_POLLING_INTERVAL);
-
- // poll immediately to kick start the fetching
- this.reporting
- .reportLifeCycle('owners-suggestions-fetching-start');
- this._pollingSuggestions();
- }
- }
- });
+ this.modelLoader.loadSuggestions();
+ this.reporting.reportLifeCycle('owners-suggestions-fetching-start');
}
disconnectedCallback() {
super.disconnectedCallback();
- if (this.expandSuggestionStateUnsubscriber) {
- this.expandSuggestionStateUnsubscriber();
- this.expandSuggestionStateUnsubscriber = undefined;
- }
+ this._stopUpdateProgressTimer();
}
- onInputChanged(restApi, change) {
- ownerState.expandSuggestion = false;
- if ([restApi, change].includes(undefined)) return;
- this.isLoading = true;
- const ownerService = CodeOwnerService.getOwnerService(this.restApi, change);
- if (this.ownerService && this.ownerService !== ownerService) {
- // abort all pending requests
- this.ownerService.abort();
- clearInterval(this._suggestionsTimer);
- this._suggestionsTimer = null;
- }
- this.ownerService = ownerService;
+ _startUpdateProgressTimer() {
+ if (this._progressUpdateTimer) return;
+ this._progressUpdateTimer = setInterval(() => {
+ this.modelLoader.updateLoadSuggestionsProgress();
+ }, SUGGESTION_POLLING_INTERVAL);
+ }
+ _stopUpdateProgressTimer() {
+ if (!this._progressUpdateTimer) return;
+ clearInterval(this._progressUpdateTimer);
+ this._progressUpdateTimer = undefined;
+ }
+
+ _onSuggestionsStateChanged(state) {
+ this._stopUpdateProgressTimer();
+ if (state === SuggestionsState.Loading) {
+ this._startUpdateProgressTimer();
+ }
+ this.isLoading = state === SuggestionsState.Loading;
+ }
+
+ _isHidden(allFilesApproved, showSuggestions) {
+ if (!showSuggestions) return true;
// if all approved, no need to show the container
- this.ownerService.areAllFilesApproved().then(approved => {
- if (approved) {
- this.hidden = approved;
- }
- });
+ return allFilesApproved === undefined || !!allFilesApproved;
}
- _pollingSuggestions() {
- this.ownerService
- .getSuggestedOwners()
- .then(res => {
- if (res.finished) {
- clearInterval(this._suggestionsTimer);
- this._suggestionsTimer = null;
- const reportDetails = res.suggestions.reduce((details, cur) => {
- details.totalGroups++;
- details.stats.push([cur.files.length, cur.owners ? cur.owners.length : 0]);
- return details;
- }, {totalGroups: 0, stats: []});
- this.reporting.reportLifeCycle('owners-suggestions-fetching-finished', reportDetails);
- }
- this.progressText = res.progress;
- this.isLoading = !res.finished;
+ loadPropertiesAfterModelChanged() {
+ super.loadPropertiesAfterModelChanged();
+ this._stopUpdateProgressTimer();
+ this.modelLoader.loadAreAllFilesApproved();
+ }
- this._updateSuggestions(res.suggestions);
+ _onSuggestionsChanged(suggestions, suggestionsState) {
+ // The updateLoadSuggestionsProgress method also updates suggestions
+ this._updateSuggestions(suggestions || []);
+ 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.length : 0]);
+ return details;
+ }, {totalGroups: 0, stats: []});
+ this.reporting.reportLifeCycle(
+ 'owners-suggestions-fetching-finished', reportDetails);
+ }
- // in case `_updateAllChips` called before suggestedOwners ready
- // from onReviewerChange
- this._updateAllChips(this._currentReviewers);
- });
+ _onSuggestionsLoadProgressChanged(progress) {
+ this.progressText = progress;
}
_updateSuggestions(suggestions) {
@@ -421,7 +407,7 @@
});
}
- onReviewerChange(reviewers) {
+ _onReviewerChanged(reviewers) {
this._currentReviewers = reviewers;
this._updateAllChips(reviewers);
}
@@ -517,7 +503,8 @@
}
_reportDocClick() {
- this.reporting.reportInteraction('code-owners-doc-click', {section: 'no owners found'});
+ this.reporting.reportInteraction('code-owners-doc-click',
+ {section: 'no owners found'});
}
}
