Merge "Nearly convert <gr-rest-api-interface> element to a simple class"

This commit is contained in:
Ben Rohlfs
2021-01-26 10:06:08 +00:00
committed by Gerrit Code Review
32 changed files with 68 additions and 82 deletions

View File

@@ -75,7 +75,7 @@ export class GrGroupAuditLog extends ListViewMixin(
}
const errFn: ErrorCallback = response => {
firePageError(this, response);
firePageError(response);
};
return this.restApiService

View File

@@ -18,6 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-group-audit-log.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-group-audit-log');
@@ -84,7 +85,7 @@ suite('gr-group-audit-log tests', () => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -139,7 +139,7 @@ export class GrGroupMembers extends GestureEventListeners(
const promises: Promise<void>[] = [];
const errFn: ErrorCallback = response => {
firePageError(this, response);
firePageError(response);
};
return this.restApiService

View File

@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-group-members.js';
import {dom, flush} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {stubBaseUrl, stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubBaseUrl, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-group-members');
@@ -352,7 +352,7 @@ suite('gr-group-members tests', () => {
.callsFake((group, errFn) => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -148,7 +148,7 @@ export class GrGroup extends GestureEventListeners(
const promises: Promise<unknown>[] = [];
const errFn: ErrorCallback = response => {
firePageError(this, response);
firePageError(response);
};
return this.restApiService

View File

@@ -18,6 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-group.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-group');
@@ -216,7 +217,7 @@ suite('gr-group tests', () => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -95,7 +95,7 @@ export class GrPluginList extends ListViewMixin(
_getPlugins(filter: string, pluginsPerPage: number, offset?: number) {
const errFn: ErrorCallback = response => {
firePageError(this, response);
firePageError(response);
};
return this.restApiService
.getPlugins(filter, pluginsPerPage, offset, errFn)

View File

@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-plugin-list.js';
import 'lodash/lodash.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-plugin-list');
@@ -155,7 +155,7 @@ suite('gr-plugin-list tests', () => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -151,7 +151,7 @@ export class GrRepoAccess extends GestureEventListeners(
_reload(repo: RepoName) {
const errFn = (response?: Response | null) => {
firePageError(this, response);
firePageError(response);
};
this._editing = false;

View File

@@ -20,7 +20,7 @@ import './gr-repo-access.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {toSortedPermissionsArray} from '../../../utils/access-util.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-repo-access');
@@ -242,7 +242,7 @@ suite('gr-repo-access tests', () => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -106,7 +106,7 @@ export class GrRepoCommands extends GestureEventListeners(
// Do not process the error, if the component is not attached to the DOM
// anymore, which at least in tests can happen.
if (!this.isConnected) return;
firePageError(this, response);
firePageError(response);
};
this.restApiService.getProjectConfig(this.repo, errFn).then(config => {

View File

@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-repo-commands.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-repo-commands');
@@ -121,7 +121,7 @@ suite('gr-repo-commands tests', () => {
stubRestApi('getProjectConfig').callsFake((repo, errFn) => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -59,7 +59,7 @@ export class GrRepoDashboards extends GestureEventListeners(
}
const errFn: ErrorCallback = response => {
firePageError(this, response);
firePageError(response);
};
return this.restApiService

View File

@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-repo-dashboards.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-repo-dashboards');
@@ -128,7 +128,7 @@ suite('gr-repo-dashboards tests', () => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -181,7 +181,7 @@ export class GrRepoDetailList extends ListViewMixin(
this._items = [];
flush();
const errFn: ErrorCallback = response => {
firePageError(this, response);
firePageError(response);
};
if (detailType === RepoDetailView.BRANCHES) {

View File

@@ -20,7 +20,7 @@ import './gr-repo-detail-list.js';
import 'lodash/lodash.js';
import {page} from '../../../utils/page-wrapper-utils.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-repo-detail-list');
@@ -293,7 +293,7 @@ suite('gr-repo-detail-list', () => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});
@@ -488,7 +488,7 @@ suite('gr-repo-detail-list', () => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -171,7 +171,7 @@ export class GrRepo extends GestureEventListeners(
const promises = [];
const errFn: ErrorCallback = response => {
firePageError(this, response);
firePageError(response);
};
promises.push(

View File

@@ -18,7 +18,7 @@
import '../../../test/common-test-setup-karma.js';
import './gr-repo.js';
import {PolymerElement} from '@polymer/polymer/polymer-element.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-repo');
@@ -235,7 +235,7 @@ suite('gr-repo tests', () => {
stubRestApi('getProjectConfig').callsFake((repo, errFn) => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.deepEqual(e.detail.response, response);
done();
});

View File

@@ -172,7 +172,7 @@ export class GrDashboardView extends GestureEventListeners(
dashboard: DashboardId
): Promise<UserDashboard | undefined> {
const errFn = (response?: Response | null) => {
firePageError(this, response);
firePageError(response);
};
return this.restApiService
.getDashboard(project, dashboard, errFn)

View File

@@ -23,7 +23,7 @@ import {GerritView} from '../../../services/router/router-model.js';
import {changeIsOpen} from '../../../utils/change-util.js';
import {ChangeStatus} from '../../../constants/constants.js';
import {createAccountWithId} from '../../../test/test-data-generators.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-dashboard-view');
@@ -377,7 +377,7 @@ suite('gr-dashboard-view tests', () => {
async (project, dashboard, errFn) => {
errFn(response);
});
element.addEventListener('page-error', e => {
addListenerForTest(document, 'page-error', e => {
assert.strictEqual(e.detail.response, response);
paramsChangedPromise.then(done);
});

View File

@@ -1855,7 +1855,7 @@ export class GrChangeView extends KeyboardShortcutMixin(
}
_handleGetChangeDetailError(response?: Response | null) {
firePageError(this, response);
firePageError(response);
}
_getLoggedIn() {

View File

@@ -65,6 +65,7 @@ import {
} from '../../gr-app-types';
import {LocationChangeEventDetail} from '../../../types/events';
import {GerritView, updateState} from '../../../services/router/router-model';
import {firePageError} from '../../../utils/event-util';
const RoutePattern = {
ROOT: '/',
@@ -1777,9 +1778,7 @@ export class GrRouter extends GestureEventListeners(
// Note: the app's 404 display is tightly-coupled with catching 404
// network responses, so we simulate a 404 response status to display it.
// TODO: Decouple the gr-app error view from network responses.
this._appElement().dispatchEvent(
new CustomEvent('page-error', {detail: {response: {status: 404}}})
);
firePageError(new Response('', {status: 404}));
}
}

View File

@@ -22,6 +22,7 @@ import {GerritNav} from '../gr-navigation/gr-navigation.js';
import {stubBaseUrl} from '../../../test/test-utils.js';
import {_testOnly_RoutePattern} from './gr-router.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-router');
@@ -664,13 +665,11 @@ suite('gr-router tests', () => {
});
test('_handleDefaultRoute on first load', () => {
const appElementStub = {dispatchEvent: sinon.stub()};
element._appElement = () => appElementStub;
const spy = sinon.spy();
addListenerForTest(document, 'page-error', spy);
element._handleDefaultRoute();
assert.isTrue(appElementStub.dispatchEvent.calledOnce);
assert.equal(
appElementStub.dispatchEvent.lastCall.args[0].detail.response.status,
404);
assert.isTrue(spy.calledOnce);
assert.equal(spy.lastCall.args[0].detail.response.status, 404);
});
test('_handleDefaultRoute after internal navigation', () => {
@@ -684,8 +683,6 @@ suite('gr-router tests', () => {
sinon.stub(page, 'base');
element._startRouter();
const appElementStub = {dispatchEvent: sinon.stub()};
element._appElement = () => appElementStub;
element._handleDefaultRoute();
onExit('', () => {}); // we left page;

View File

@@ -609,7 +609,7 @@ export class GrDiffHost extends GestureEventListeners(
return;
}
firePageError(this, response);
firePageError(response);
}
/**

View File

@@ -25,7 +25,7 @@ import {Side, CommentSide} from '../../../constants/constants.js';
import {createChange} from '../../../test/test-data-generators.js';
import {FILE} from '../gr-diff/gr-diff-line.js';
import {CoverageType} from '../../../types/types.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
import {createDefaultDiffPrefs} from '../../../constants/constants.js';
const basicFixture = fixtureFromElement('gr-diff-host');
@@ -296,13 +296,9 @@ suite('gr-diff-host tests', () => {
setup(() => {
serverErrorStub = sinon.stub();
document.addEventListener('server-error', serverErrorStub);
addListenerForTest(document, 'server-error', serverErrorStub);
pageErrorStub = sinon.stub();
element.addEventListener('page-error', pageErrorStub);
});
teardown(() => {
document.removeEventListener('server-error', serverErrorStub);
addListenerForTest(document, 'page-error', pageErrorStub);
});
test('page error on HTTP-409', () => {
@@ -313,7 +309,10 @@ suite('gr-diff-host tests', () => {
});
test('server error on non-HTTP-409', () => {
element._handleGetDiffError({status: 500});
element._handleGetDiffError({
status: 500,
text: () => Promise.resolve(''),
});
assert.isFalse(serverErrorStub.called);
assert.isTrue(pageErrorStub.calledOnce);
assert.isNotOk(element._errorMessage);

View File

@@ -210,7 +210,7 @@ export class GrAppElement extends KeyboardShortcutMixin(
created() {
super.created();
this._bindKeyboardShortcuts();
this.addEventListener(EventType.PAGE_ERROR, e => {
document.addEventListener(EventType.PAGE_ERROR, e => {
this._handlePageError(e);
});
this.addEventListener(EventType.TITLE_CHANGE, e => {

View File

@@ -19,6 +19,7 @@ import '../../../test/common-test-setup-karma.js';
import './gr-js-api-interface.js';
import {GrPluginActionContext} from './gr-plugin-action-context.js';
import {_testOnly_initGerritPluginApi} from './gr-gerrit.js';
import {addListenerForTest} from '../../../test/test-utils.js';
const pluginApi = _testOnly_initGerritPluginApi();
@@ -137,7 +138,7 @@ suite('gr-plugin-action-context tests', () => {
send: sendStub,
});
const errorStub = sinon.stub();
document.addEventListener('show-alert', errorStub);
addListenerForTest(document, 'show-alert', errorStub);
instance.call();
await flush();
assert.isTrue(errorStub.calledOnce);

View File

@@ -22,7 +22,7 @@ import {_testOnly_resetPluginLoader} from './gr-plugin-loader.js';
import {resetPlugins, stubBaseUrl} from '../../../test/test-utils.js';
import {_testOnly_flushPreinstalls} from './gr-gerrit.js';
import {_testOnly_initGerritPluginApi} from './gr-gerrit.js';
import {stubRestApi} from '../../../test/test-utils.js';
import {addListenerForTest, stubRestApi} from '../../../test/test-utils.js';
const basicFixture = fixtureFromElement('gr-js-api-interface');
@@ -149,7 +149,7 @@ suite('gr-plugin-loader tests', () => {
];
const alertStub = sinon.stub();
document.addEventListener('show-alert', alertStub);
addListenerForTest(document, 'show-alert', alertStub);
sinon.stub(pluginLoader, '_loadJsPlugin').callsFake( url => {
pluginApi.install(() => {
@@ -177,7 +177,7 @@ suite('gr-plugin-loader tests', () => {
];
const alertStub = sinon.stub();
document.addEventListener('show-alert', alertStub);
addListenerForTest(document, 'show-alert', alertStub);
sinon.stub(pluginLoader, '_loadJsPlugin').callsFake( url => {
pluginApi.install(() => {
@@ -210,7 +210,7 @@ suite('gr-plugin-loader tests', () => {
];
const alertStub = sinon.stub();
document.addEventListener('show-alert', alertStub);
addListenerForTest(document, 'show-alert', alertStub);
sinon.stub(pluginLoader, '_loadJsPlugin').callsFake( url => {
pluginApi.install(() => {
@@ -236,7 +236,7 @@ suite('gr-plugin-loader tests', () => {
];
const alertStub = sinon.stub();
document.addEventListener('show-alert', alertStub);
addListenerForTest(document, 'show-alert', alertStub);
sinon.stub(pluginLoader, '_loadJsPlugin').callsFake( url => {
pluginApi.install(() => {

View File

@@ -16,8 +16,6 @@
*/
/* NB: Order is important, because of namespaced classes. */
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
import {LegacyElementMixin} from '@polymer/polymer/lib/legacy/legacy-element-mixin';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {GrEtagDecorator} from './gr-etag-decorator';
import {
@@ -41,7 +39,7 @@ import {
listChangesOptionsToHex,
} from '../../../utils/change-util';
import {assertNever, hasOwnProperty} from '../../../utils/common-util';
import {customElement, property} from '@polymer/decorators';
import {customElement} from '@polymer/decorators';
import {AuthRequestInit, AuthService} from '../../../services/gr-auth/gr-auth';
import {
AccountCapabilityInfo,
@@ -297,22 +295,16 @@ declare global {
}
@customElement('gr-rest-api-interface')
export class GrRestApiInterface
extends GestureEventListeners(LegacyElementMixin(PolymerElement))
export class GrRestApiInterface extends PolymerElement
implements RestApiService {
@property({type: Object})
readonly _cache = siteBasedCache; // Shared across instances.
@property({type: Object})
readonly _sharedFetchPromises = fetchPromisesCache; // Shared across instances.
@property({type: Object})
readonly _pendingRequests = pendingRequest; // Shared across instances.
@property({type: Object})
readonly _etags = grEtagDecorator; // Shared across instances.
@property({type: Object})
readonly _projectLookup = projectLookup; // Shared across instances.
// The value is set in created, before any other actions
@@ -3067,8 +3059,7 @@ export class GrRestApiInterface
return Promise.resolve(project);
}
const onError = (response?: Response | null) =>
firePageError(this, response);
const onError = (response?: Response | null) => firePageError(response);
return this.getChange(changeNum, onError).then(change => {
if (!change || !change.project) {

View File

@@ -16,8 +16,7 @@
*/
import '../../../test/common-test-setup-karma.js';
import './gr-rest-api-interface.js';
import {mockPromise} from '../../../test/test-utils.js';
import {addListenerForTest, mockPromise} from '../../../test/test-utils.js';
import {GrReviewerUpdatesParser} from './gr-reviewer-updates-parser.js';
import {ListChangesOption} from '../../../utils/change-util.js';
import {appContext} from '../../../services/app-context.js';
@@ -28,8 +27,7 @@ import {
readResponsePayload,
} from './gr-rest-apis/gr-rest-api-helper.js';
import {JSON_PREFIX} from './gr-rest-apis/gr-rest-api-helper.js';
const basicFixture = fixtureFromElement('gr-rest-api-interface');
import {GrRestApiInterface} from './gr-rest-api-interface.js';
suite('gr-rest-api-interface tests', () => {
let element;
@@ -53,7 +51,7 @@ suite('gr-rest-api-interface tests', () => {
// fake auth
sinon.stub(appContext.authService, 'authCheck')
.returns(Promise.resolve(true));
element = basicFixture.instantiate();
element = new GrRestApiInterface();
element._projectLookup = {};
});
@@ -267,7 +265,7 @@ suite('gr-rest-api-interface tests', () => {
const getResponseObjectStub = sinon.stub(element, 'getResponseObject');
window.fetch.returns(Promise.resolve({ok: false}));
const serverErrorEventPromise = new Promise(resolve => {
document.addEventListener('server-error', resolve);
addListenerForTest(document, 'server-error', resolve);
});
return Promise.all([element._restApiHelper.fetchJSON({}).then(response => {
@@ -1253,7 +1251,7 @@ suite('gr-rest-api-interface tests', () => {
test('getFileContent suppresses 404s', () => {
const res = {status: 404};
const spy = sinon.spy();
document.addEventListener('server-error', spy);
addListenerForTest(document, 'server-error', spy);
sinon.stub(appContext.authService, 'fetch').returns(Promise.resolve(res));
sinon.stub(element, '_changeBaseURL').returns(Promise.resolve(''));
return element.getFileContent('1', 'tst/path', '1')
@@ -1306,7 +1304,7 @@ suite('gr-rest-api-interface tests', () => {
test('_logCall only reports requests with anonymized URLss', () => {
sinon.stub(Date, 'now').returns(200);
const handler = sinon.stub();
document.addEventListener('gr-rpc-log', handler);
addListenerForTest(document, 'gr-rpc-log', handler);
element._restApiHelper._logCall({url: 'url'}, 100, 200);
assert.isFalse(handler.called);
@@ -1320,14 +1318,13 @@ suite('gr-rest-api-interface tests', () => {
test('ported comment errors do not trigger error dialog', () => {
const change = createChange();
const handler = sinon.stub();
document.addEventListener('server-error', handler);
addListenerForTest(document, 'server-error', handler);
sinon.stub(element._restApiHelper, 'fetchJSON').returns(Promise.resolve({
ok: false}));
element.getPortedComments(change._number, CURRENT);
assert.isFalse(handler.called);
document.removeEventListener('server-error', handler);
});
test('ported drafts are not requested user is not logged in', () => {

View File

@@ -40,7 +40,7 @@ export interface PageErrorEventDetail {
export type PageErrorEvent = CustomEvent<PageErrorEventDetail>;
declare global {
interface HTMLElementEventMap {
interface DocumentEventMap {
'page-error': PageErrorEvent;
}
}

View File

@@ -46,8 +46,8 @@ export function fireAlert(target: EventTarget, message: string) {
);
}
export function firePageError(target: EventTarget, response?: Response | null) {
target.dispatchEvent(
export function firePageError(response?: Response | null) {
document.dispatchEvent(
new CustomEvent(EventType.PAGE_ERROR, {
detail: {response},
composed: true,