Use config model instead of calling rest api service Release-Notes: skip Change-Id: Ica6246fca6bab2e8028523121f7de5075a5bc808
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts index ab65b57..e614ebe 100644 --- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts +++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -10,9 +10,7 @@ AutocompleteSuggestion, GrAutocomplete, } from '../../shared/gr-autocomplete/gr-autocomplete'; -import {getDocsBaseUrl} from '../../../utils/url-util'; import {MergeabilityComputationBehavior} from '../../../constants/constants'; -import {getAppContext} from '../../../services/app-context'; import {sharedStyles} from '../../../styles/shared-styles'; import {LitElement, PropertyValues, html, css} from 'lit'; import { @@ -24,6 +22,9 @@ import {Shortcut, ShortcutController} from '../../lit/shortcut-controller'; import {query as queryUtil} from '../../../utils/common-util'; import {assertIsDefined} from '../../../utils/common-util'; +import {configModelToken} from '../../../models/config/config-model'; +import {resolve} from '../../../models/dependency'; +import {subscribe} from '../../lit/subscription-controller'; // Possible static search options for auto complete, without negations. const SEARCH_OPERATORS: ReadonlyArray<string> = [ @@ -152,9 +153,12 @@ @property({type: Object}) accountSuggestions: SuggestionProvider = () => Promise.resolve([]); - @property({type: Object}) + @state() serverConfig?: ServerInfo; + @state() + mergeabilityComputationBehavior?: MergeabilityComputationBehavior; + @property({type: String}) label = ''; @@ -162,22 +166,32 @@ @state() inputVal = ''; // private but used in test - @state() docBaseUrl: string | null = null; + @state() docsBaseUrl: string | null = null; @state() private query: AutocompleteQuery; @state() private threshold = 1; - private searchOperators = new Set(SEARCH_OPERATORS_WITH_NEGATIONS_SET); - - private readonly restApiService = getAppContext().restApiService; - private readonly shortcuts = new ShortcutController(this); + private readonly getConfigModel = resolve(this, configModelToken); + constructor() { super(); this.query = (input: string) => this.getSearchSuggestions(input); this.shortcuts.addAbstract(Shortcut.SEARCH, () => this.handleSearch()); + subscribe( + this, + () => this.getConfigModel().mergeabilityComputationBehavior$, + mergeabilityComputationBehavior => { + this.mergeabilityComputationBehavior = mergeabilityComputationBehavior; + } + ); + subscribe( + this, + () => this.getConfigModel().docsBaseUrl$, + docsBaseUrl => (this.docsBaseUrl = docsBaseUrl) + ); } static override get styles() { @@ -236,47 +250,34 @@ } override willUpdate(changedProperties: PropertyValues) { - if (changedProperties.has('serverConfig')) { - this.serverConfigChanged(); - } - if (changedProperties.has('value')) { this.valueChanged(); } } - private serverConfigChanged() { - const mergeability = - this.serverConfig?.change?.mergeability_computation_behavior; - if ( - mergeability === - MergeabilityComputationBehavior.API_REF_UPDATED_AND_CHANGE_REINDEX || - mergeability === - MergeabilityComputationBehavior.REF_UPDATED_AND_CHANGE_REINDEX - ) { - // add 'is:mergeable' to searchOperators - this.searchOperators.add('is:mergeable'); - this.searchOperators.add('-is:mergeable'); - } else { - this.searchOperators.delete('is:mergeable'); - this.searchOperators.delete('-is:mergeable'); - } - if (this.serverConfig) { - getDocsBaseUrl(this.serverConfig, this.restApiService).then(baseUrl => { - this.docBaseUrl = baseUrl; - }); - } - } - private valueChanged() { this.inputVal = this.value; } + private searchOperators() { + const set = new Set(SEARCH_OPERATORS_WITH_NEGATIONS_SET); + if ( + this.mergeabilityComputationBehavior === + MergeabilityComputationBehavior.API_REF_UPDATED_AND_CHANGE_REINDEX || + this.mergeabilityComputationBehavior === + MergeabilityComputationBehavior.REF_UPDATED_AND_CHANGE_REINDEX + ) { + set.add('is:mergeable'); + set.add('-is:mergeable'); + } + return set; + } + // private but used in test computeHelpDocLink() { // fallback to gerrit's official doc let baseUrl = - this.docBaseUrl || + this.docsBaseUrl || 'https://gerrit-review.googlesource.com/documentation/'; if (baseUrl.endsWith('/')) { baseUrl = baseUrl.substring(0, baseUrl.length - 1); @@ -307,7 +308,7 @@ if (!this.inputVal) return; const trimmedInput = this.inputVal.trim(); if (trimmedInput) { - const predefinedOpOnlyQuery = [...this.searchOperators].some( + const predefinedOpOnlyQuery = [...this.searchOperators()].some( op => op.endsWith(':') && op === trimmedInput ); if (predefinedOpOnlyQuery) { @@ -364,7 +365,7 @@ default: return Promise.resolve( - [...this.searchOperators] + [...this.searchOperators()] .filter(operator => operator.includes(input)) .map(operator => { return {text: operator};
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts index 90574a2..db46c22 100644 --- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts +++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.ts
@@ -8,11 +8,9 @@ import {GrSearchBar} from './gr-search-bar'; import '../../../scripts/util'; import {mockPromise, waitUntil} from '../../../test/test-utils'; -import {_testOnly_clearDocsBaseUrlCache} from '../../../utils/url-util'; import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions'; import { createChangeConfig, - createGerritInfo, createServerInfo, } from '../../../test/test-data-generators'; import {MergeabilityComputationBehavior} from '../../../constants/constants'; @@ -176,14 +174,8 @@ suite('getSearchSuggestions', () => { setup(async () => { element = basicFixture.instantiate(); - element.serverConfig = { - ...createServerInfo(), - change: { - ...createChangeConfig(), - mergeability_computation_behavior: - 'NEVER' as MergeabilityComputationBehavior, - }, - }; + element.mergeabilityComputationBehavior = + MergeabilityComputationBehavior.NEVER; await element.updateComplete; }); @@ -268,28 +260,21 @@ suite('doc url', () => { setup(async () => { - _testOnly_clearDocsBaseUrlCache(); element = basicFixture.instantiate(); - element.serverConfig = { - ...createServerInfo(), - gerrit: { - ...createGerritInfo(), - doc_url: 'https://doc.com/', - }, - }; - await element.updateComplete; }); - test('compute help doc url with correct path', () => { - assert.equal(element.docBaseUrl, 'https://doc.com/'); + test('compute help doc url with correct path', async () => { + element.docsBaseUrl = 'https://doc.com/'; + await element.updateComplete; assert.equal( element.computeHelpDocLink(), 'https://doc.com/user-search.html' ); }); - test('compute help doc url fallback to gerrit url', () => { - element.docBaseUrl = null; + test('compute help doc url fallback to gerrit url', async () => { + element.docsBaseUrl = null; + await element.updateComplete; assert.equal( element.computeHelpDocLink(), 'https://gerrit-review.googlesource.com/documentation/' +
diff --git a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts index edee3cb..bd3bd3e 100644 --- a/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts +++ b/polygerrit-ui/app/elements/core/gr-smart-search/gr-smart-search.ts
@@ -14,7 +14,10 @@ import {AutocompleteSuggestion} from '../../shared/gr-autocomplete/gr-autocomplete'; import {getAppContext} from '../../../services/app-context'; import {LitElement, html} from 'lit'; -import {customElement, property} from 'lit/decorators'; +import {customElement, property, state} from 'lit/decorators'; +import {subscribe} from '../../lit/subscription-controller'; +import {resolve} from '../../../models/dependency'; +import {configModelToken} from '../../../models/config/config-model'; const MAX_AUTOCOMPLETE_RESULTS = 10; const SELF_EXPRESSION = 'self'; @@ -34,7 +37,7 @@ @property({type: String}) searchQuery = ''; - @property({type: Object}) + @state() serverConfig?: ServerInfo; @property({type: String}) @@ -42,6 +45,19 @@ private readonly restApiService = getAppContext().restApiService; + private readonly getConfigModel = resolve(this, configModelToken); + + constructor() { + super(); + subscribe( + this, + () => this.getConfigModel().serverConfig$, + config => { + this.serverConfig = config; + } + ); + } + override render() { const accountSuggestions: SuggestionProvider = (predicate, expression) => this.fetchAccounts(predicate, expression); @@ -57,7 +73,6 @@ .projectSuggestions=${projectSuggestions} .groupSuggestions=${groupSuggestions} .accountSuggestions=${accountSuggestions} - .serverConfig=${this.serverConfig} @handle-search=${(e: CustomEvent<SearchBarHandleSearchDetail>) => { this.handleSearch(e); }}
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts index 381451f..ce2aff2 100644 --- a/polygerrit-ui/app/elements/gr-app-element.ts +++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -29,7 +29,7 @@ import {GerritNav} from './core/gr-navigation/gr-navigation'; import {getAppContext} from '../services/app-context'; import {GrRouter} from './core/gr-router/gr-router'; -import {AccountDetailInfo, ServerInfo} from '../types/common'; +import {AccountDetailInfo} from '../types/common'; import { constructServerErrorMsg, GrErrorManager, @@ -100,8 +100,6 @@ @state() private account?: AccountDetailInfo; - @state() private serverConfig?: ServerInfo; - @state() private version?: string; @state() private showChangeListView?: boolean; @@ -225,9 +223,6 @@ this.reporting.reportLifeCycle(LifeCycle.STARTED_AS_GUEST); } }); - this.restApiService.getConfig().then(config => { - this.serverConfig = config; - }); this.restApiService.getVersion().then(version => { this.version = version; this.logWelcome(); @@ -375,8 +370,7 @@ id="errorManager" .loginUrl=${this.loginUrl} ></gr-error-manager> - <gr-plugin-host id="plugins" .config=${this.serverConfig}> - </gr-plugin-host> + <gr-plugin-host id="plugins"></gr-plugin-host> <gr-external-style id="externalStyleForAll" name="app-theme" @@ -395,7 +389,6 @@ id="search" label="Search for changes" .searchQuery=${(this.params as AppElementSearchParam)?.query} - .serverConfig=${this.serverConfig} > </gr-smart-search> `;
diff --git a/polygerrit-ui/app/elements/gr-app_test.ts b/polygerrit-ui/app/elements/gr-app_test.ts index 06fce4df..2b84344 100644 --- a/polygerrit-ui/app/elements/gr-app_test.ts +++ b/polygerrit-ui/app/elements/gr-app_test.ts
@@ -16,7 +16,6 @@ createServerInfo, } from '../test/test-data-generators'; import {GrAppElement} from './gr-app-element'; -import {GrPluginHost} from './plugins/gr-plugin-host/gr-plugin-host'; import {GrRouter} from './core/gr-router/gr-router'; suite('gr-app tests', () => { @@ -48,12 +47,6 @@ sinon.assert.callOrder(appStartedStub, routerStartStub); }); - test('passes config to gr-plugin-host', () => { - const grAppElement = queryAndAssert<GrAppElement>(grApp, '#app-element'); - const pluginHost = queryAndAssert<GrPluginHost>(grAppElement, '#plugins'); - assert.deepEqual(pluginHost.config, config); - }); - test('_paramsChanged sets search page', () => { const grAppElement = queryAndAssert<GrAppElement>(grApp, '#app-element');
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts index 9b62743..dfdf12c 100644 --- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts +++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host.ts
@@ -3,27 +3,37 @@ * Copyright 2017 Google LLC * SPDX-License-Identifier: Apache-2.0 */ -import {LitElement, PropertyValues} from 'lit'; -import {customElement, property} from 'lit/decorators'; +import {LitElement} from 'lit'; +import {customElement, state} from 'lit/decorators'; import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader'; import {ServerInfo} from '../../../types/common'; +import {subscribe} from '../../lit/subscription-controller'; +import {resolve} from '../../../models/dependency'; +import {configModelToken} from '../../../models/config/config-model'; @customElement('gr-plugin-host') export class GrPluginHost extends LitElement { - @property({type: Object}) + @state() config?: ServerInfo; - _configChanged(config: ServerInfo) { - const jsPlugins = config.plugin?.js_resource_paths ?? []; - const themes: string[] = config.default_theme ? [config.default_theme] : []; - const instanceId = config.gerrit?.instance_id; - getPluginLoader().loadPlugins([...themes, ...jsPlugins], instanceId); - } + // visible for testing + readonly getConfigModel = resolve(this, configModelToken); - override updated(changedProperties: PropertyValues<GrPluginHost>) { - if (changedProperties.has('config') && this.config) { - this._configChanged(this.config); - } + constructor() { + super(); + subscribe( + this, + () => this.getConfigModel().serverConfig$, + config => { + if (!config) return; + const jsPlugins = config?.plugin?.js_resource_paths ?? []; + const themes: string[] = config?.default_theme + ? [config.default_theme] + : []; + const instanceId = config?.gerrit?.instance_id; + getPluginLoader().loadPlugins([...themes, ...jsPlugins], instanceId); + } + ); } }
diff --git a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts index 945af72..756221e 100644 --- a/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts +++ b/polygerrit-ui/app/elements/plugins/gr-plugin-host/gr-plugin-host_test.ts
@@ -8,28 +8,32 @@ import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader'; import {GrPluginHost} from './gr-plugin-host'; import {fixture, html} from '@open-wc/testing-helpers'; -import {ServerInfo} from '../../../api/rest-api'; +import {SinonStub} from 'sinon'; +import {createServerInfo} from '../../../test/test-data-generators'; suite('gr-plugin-host tests', () => { let element: GrPluginHost; + let loadPluginsStub: SinonStub; setup(async () => { + loadPluginsStub = sinon.stub(getPluginLoader(), 'loadPlugins'); element = await fixture<GrPluginHost>(html` <gr-plugin-host></gr-plugin-host> `); + await element.updateComplete; sinon.stub(document.body, 'appendChild'); }); test('load plugins should be called', async () => { - const loadPluginsStub = sinon.stub(getPluginLoader(), 'loadPlugins'); - element.config = { + loadPluginsStub.reset(); + element.getConfigModel().updateServerConfig({ + ...createServerInfo(), plugin: { has_avatars: false, js_resource_paths: ['plugins/42', 'plugins/foo/bar', 'plugins/baz'], }, - } as ServerInfo; - await flush(); + }); assert.isTrue(loadPluginsStub.calledOnce); assert.isTrue( loadPluginsStub.calledWith([ @@ -41,15 +45,15 @@ }); test('theme plugins should be loaded if enabled', async () => { - const loadPluginsStub = sinon.stub(getPluginLoader(), 'loadPlugins'); - element.config = { + loadPluginsStub.reset(); + element.getConfigModel().updateServerConfig({ + ...createServerInfo(), default_theme: 'gerrit-theme.js', plugin: { has_avatars: false, js_resource_paths: ['plugins/42', 'plugins/foo/bar', 'plugins/baz'], }, - } as ServerInfo; - await flush(); + }); assert.isTrue(loadPluginsStub.calledOnce); assert.isTrue( loadPluginsStub.calledWith([ @@ -60,4 +64,13 @@ ]) ); }); + + test('plugins loaded with instanceId ', async () => { + loadPluginsStub.reset(); + const config = createServerInfo(); + config.gerrit.instance_id = 'test-id'; + element.getConfigModel().updateServerConfig(config); + assert.isTrue(loadPluginsStub.calledOnce); + assert.isTrue(loadPluginsStub.calledWith([], 'test-id')); + }); });
diff --git a/polygerrit-ui/app/models/config/config-model.ts b/polygerrit-ui/app/models/config/config-model.ts index dd26182..6861348 100644 --- a/polygerrit-ui/app/models/config/config-model.ts +++ b/polygerrit-ui/app/models/config/config-model.ts
@@ -12,6 +12,7 @@ import {select} from '../../utils/observable-util'; import {Model} from '../model'; import {define} from '../dependency'; +import {getDocsBaseUrl} from '../../utils/url-util'; export interface ConfigState { repoConfig?: ConfigInfo; @@ -35,6 +36,20 @@ configState => configState.serverConfig ); + public mergeabilityComputationBehavior$ = select( + this.serverConfig$, + serverConfig => serverConfig?.change?.mergeability_computation_behavior + ); + + public docsBaseUrl$ = select( + this.serverConfig$.pipe( + switchMap(serverConfig => + from(getDocsBaseUrl(serverConfig, this.restApiService)) + ) + ), + url => url + ); + private subscriptions: Subscription[]; constructor( @@ -59,11 +74,12 @@ ]; } - updateRepoConfig(repoConfig?: ConfigInfo) { + private updateRepoConfig(repoConfig?: ConfigInfo) { const current = this.subject$.getValue(); this.subject$.next({...current, repoConfig}); } + // visible for testing updateServerConfig(serverConfig?: ServerInfo) { const current = this.subject$.getValue(); this.subject$.next({...current, serverConfig});