Use CodeOwnersModel in suggest-owners
Change-Id: I1dbfbc00013478a54dd6216b62fd5b17953c399b
diff --git a/ui/code-owners-model-loader.js b/ui/code-owners-model-loader.js
index ea10813..2074fcb 100644
--- a/ui/code-owners-model-loader.js
+++ b/ui/code-owners-model-loader.js
@@ -15,6 +15,8 @@
* 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.
@@ -75,4 +77,22 @@
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);
+ const suggestedOwners = await this.ownersService.getSuggestedOwners();
+ this.ownersModel.setSuggestions(suggestedOwners.suggestions);
+ this.ownersModel.setSuggestionsState(SuggestionsState.Loaded);
+ }
+
+ async updateLoadSuggestionsProgress() {
+ const suggestedOwners =
+ await this.ownersService.getSuggestedOwnersProgress();
+ this.ownersModel.setSuggestionsLoadProgress(suggestedOwners.progress);
+ this.ownersModel.setSuggestions(suggestedOwners.suggestions);
+ }
}
diff --git a/ui/code-owners-model.js b/ui/code-owners-model.js
index 5600773..4c63419 100644
--- a/ui/code-owners-model.js
+++ b/ui/code-owners-model.js
@@ -15,11 +15,17 @@
* limitations under the License.
*/
+export const SuggestionsState = {
+ NotLoaded: 'NotLoaded',
+ Loaded: 'Loaded',
+ Loading: 'Loading',
+};
+
/**
* 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 (thought it is not
- * required).
+ * 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
@@ -41,6 +47,10 @@
this.userRole = undefined;
this.isCodeOwnerEnabled = undefined;
this.areAllFilesApproved = undefined;
+ this.suggestions = undefined;
+ this.suggestionsState = SuggestionsState.NotLoaded;
+ this.suggestionsLoadProgress = undefined;
+ this.showSuggestions = false;
}
setBranchConfig(config) {
@@ -73,6 +83,30 @@
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');
+ }
+
_firePropertyChanged(propertyName) {
this.dispatchEvent(new CustomEvent('model-property-changed', {
detail: {
diff --git a/ui/code-owners-service.js b/ui/code-owners-service.js
index 52638ac..9b670b9 100644
--- a/ui/code-owners-service.js
+++ b/ui/code-owners-service.js
@@ -440,6 +440,17 @@
};
}
+ 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) {
// convert the array of statuses to map between file path -> status
return statuses.reduce((prev, cur) => {
diff --git a/ui/owner-requirement.js b/ui/owner-requirement.js
index b7b9dbb..c9d9cb8 100644
--- a/ui/owner-requirement.js
+++ b/ui/owner-requirement.js
@@ -16,7 +16,6 @@
*/
import {OwnerStatus} from './code-owners-service.js';
-import {ownerState} from './owner-ui-state.js';
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
/**
@@ -215,6 +214,7 @@
}
_openReplyDialog() {
+ this.model.setShowSuggestions(true);
this.dispatchEvent(
new CustomEvent('open-reply-dialog', {
detail: {},
@@ -222,7 +222,6 @@
bubbles: true,
})
);
- ownerState.expandSuggestion = true;
this.reporting.reportInteraction('suggest-owners-from-submit-requirement',
{user_role: this.model.userRole});
}
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/suggest-owners-trigger.js b/ui/suggest-owners-trigger.js
index c6fd55d..8e0e293 100644
--- a/ui/suggest-owners-trigger.js
+++ b/ui/suggest-owners-trigger.js
@@ -14,7 +14,6 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {ownerState} from './owner-ui-state.js';
import {CodeOwnersModelMixin} from './code-owners-model-mixin.js';
export class SuggestOwnersTrigger extends
@@ -23,17 +22,8 @@
return 'suggest-owners-trigger';
}
- constructor(props) {
- super(props);
- this.expandSuggestionStateUnsubscriber = undefined;
- }
-
static get properties() {
return {
- expanded: {
- type: Boolean,
- value: false,
- },
hidden: {
type: Boolean,
computed: '_computeHidden(model.isCodeOwnerEnabled,' +
@@ -62,7 +52,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">
@@ -82,22 +72,6 @@
this.modelLoader.loadAreAllFilesApproved();
}
- connectedCallback() {
- super.connectedCallback();
- this.expandSuggestionStateUnsubscriber = ownerState
- .onExpandSuggestionChange(expanded => {
- this.expanded = expanded;
- });
- }
-
- disconnnectedCallback() {
- super.disconnectedCallback();
- if (this.expandSuggestionStateUnsubscriber) {
- this.expandSuggestionStateUnsubscriber();
- this.expandSuggestionStateUnsubscriber = undefined;
- }
- }
-
_computeHidden(enabled, allFilesApproved, userRole) {
if (enabled === undefined ||
allFilesApproved === undefined ||
@@ -112,8 +86,7 @@
}
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.model.userRole ?
diff --git a/ui/suggest-owners.js b/ui/suggest-owners.js
index 29e989a..068b933 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';
}
@@ -299,18 +294,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,95 +316,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) {
@@ -423,7 +406,7 @@
});
}
- onReviewerChange(reviewers) {
+ _onReviewerChanged(reviewers) {
this._currentReviewers = reviewers;
this._updateAllChips(reviewers);
}
