Fixed review comments
* Rename "apiBaseUrl" to the more suitable "baseUrl". * Removed the Gerrit location doc (will be added later together with the "urlReader" implementation. * Fixed the line number handling in Gerrit/resolveUrl. Signed-off-by: Niklas Aronsson <niklasar@axis.com>
This commit is contained in:
@@ -136,7 +136,7 @@ export class GerritIntegration implements ScmIntegration {
|
||||
// @public
|
||||
export type GerritIntegrationConfig = {
|
||||
host: string;
|
||||
apiBaseUrl?: string;
|
||||
baseUrl?: string;
|
||||
username?: string;
|
||||
password?: string;
|
||||
};
|
||||
|
||||
Vendored
+1
-2
@@ -71,7 +71,7 @@ export interface Config {
|
||||
* The base url for the Gerrit instance.
|
||||
* @visibility frontend
|
||||
*/
|
||||
apiBaseUrl?: string;
|
||||
baseUrl?: string;
|
||||
/**
|
||||
* The username to use for authenticated requests.
|
||||
* @visibility secret
|
||||
@@ -80,7 +80,6 @@ export interface Config {
|
||||
/**
|
||||
* Gerrit password used to authenticate requests. This can be either a password
|
||||
* or a generated access token.
|
||||
* .
|
||||
* @visibility secret
|
||||
*/
|
||||
password?: string;
|
||||
|
||||
@@ -26,7 +26,7 @@ describe('GerritIntegration', () => {
|
||||
{
|
||||
host: 'gerrit-review.example.com',
|
||||
username: 'gerrituser',
|
||||
apiBaseUrl: 'https://gerrit-review.example.com/gerrit',
|
||||
baseUrl: 'https://gerrit-review.example.com/gerrit',
|
||||
password: '1234',
|
||||
},
|
||||
],
|
||||
@@ -37,12 +37,14 @@ describe('GerritIntegration', () => {
|
||||
expect(integrations.list()[0].config.host).toBe(
|
||||
'gerrit-review.example.com',
|
||||
);
|
||||
expect(integrations.list()[0].config.baseUrl).toBe(
|
||||
'https://gerrit-review.example.com/gerrit',
|
||||
);
|
||||
});
|
||||
|
||||
it('returns the basics', () => {
|
||||
const integration = new GerritIntegration({
|
||||
host: 'gerrit-review.example.com',
|
||||
apiBaseUrl: 'https://gerrit-review.example.com/gerrit',
|
||||
} as any);
|
||||
expect(integration.type).toBe('gerrit');
|
||||
expect(integration.title).toBe('gerrit-review.example.com');
|
||||
@@ -52,7 +54,6 @@ describe('GerritIntegration', () => {
|
||||
it('works for valid urls', () => {
|
||||
const integration = new GerritIntegration({
|
||||
host: 'gerrit-review.example.com',
|
||||
apiBaseUrl: 'https://gerrit-review.example.com/gerrit',
|
||||
} as any);
|
||||
|
||||
expect(
|
||||
@@ -63,13 +64,26 @@ describe('GerritIntegration', () => {
|
||||
}),
|
||||
).toBe('https://gerrit-review.example.com/catalog-info.yaml#9');
|
||||
});
|
||||
|
||||
it('handles line numbers', () => {
|
||||
const integration = new GerritIntegration({
|
||||
host: 'gerrit-review.example.com',
|
||||
} as any);
|
||||
|
||||
expect(
|
||||
integration.resolveUrl({
|
||||
url: '',
|
||||
base: 'https://gerrit-review.example.com/catalog-info.yaml#4',
|
||||
lineNumber: 9,
|
||||
}),
|
||||
).toBe('https://gerrit-review.example.com/catalog-info.yaml#9');
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolves with a relative url', () => {
|
||||
it('works for valid urls', () => {
|
||||
const integration = new GerritIntegration({
|
||||
host: 'gerrit-review.example.com',
|
||||
apiBaseUrl: 'https://gerrit-review.example.com/gerrit',
|
||||
} as any);
|
||||
|
||||
expect(
|
||||
@@ -86,7 +100,6 @@ describe('GerritIntegration', () => {
|
||||
it('resolve edit URL', () => {
|
||||
const integration = new GerritIntegration({
|
||||
host: 'gerrit-review.example.com',
|
||||
apiBaseUrl: 'https://gerrit-review.example.com/gerrit',
|
||||
} as any);
|
||||
|
||||
// Resolve edit URLs is not applicable for gerrit. Return the input
|
||||
|
||||
@@ -33,7 +33,7 @@ export class GerritIntegration implements ScmIntegration {
|
||||
);
|
||||
return basicIntegrations(
|
||||
configs.map(c => new GerritIntegration(c)),
|
||||
i => i.config.host ?? '',
|
||||
i => i.config.host,
|
||||
);
|
||||
};
|
||||
|
||||
@@ -59,14 +59,14 @@ export class GerritIntegration implements ScmIntegration {
|
||||
const { url, base, lineNumber } = options;
|
||||
let updated;
|
||||
if (url) {
|
||||
updated = new URL(url, base).toString();
|
||||
updated = new URL(url, base);
|
||||
} else {
|
||||
updated = base;
|
||||
updated = new URL(base);
|
||||
}
|
||||
if (lineNumber) {
|
||||
return `${updated}#${lineNumber}`;
|
||||
updated.hash = lineNumber.toString();
|
||||
}
|
||||
return updated;
|
||||
return updated.toString();
|
||||
}
|
||||
|
||||
resolveEditUrl(url: string): string {
|
||||
|
||||
@@ -55,14 +55,14 @@ describe('readGerritIntegrationConfig', () => {
|
||||
const output = readGerritIntegrationConfig(
|
||||
buildConfig({
|
||||
host: 'a.com',
|
||||
apiBaseUrl: 'https://a.com/api',
|
||||
baseUrl: 'https://a.com/api',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
}),
|
||||
);
|
||||
expect(output).toEqual({
|
||||
host: 'a.com',
|
||||
apiBaseUrl: 'https://a.com/api',
|
||||
baseUrl: 'https://a.com/api',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
});
|
||||
@@ -76,7 +76,7 @@ describe('readGerritIntegrationConfig', () => {
|
||||
);
|
||||
expect(output).toEqual({
|
||||
host: 'a.com',
|
||||
apiBaseUrl: 'https://a.com',
|
||||
baseUrl: 'https://a.com',
|
||||
username: undefined,
|
||||
password: undefined,
|
||||
});
|
||||
@@ -90,8 +90,8 @@ describe('readGerritIntegrationConfig', () => {
|
||||
readGerritIntegrationConfig(buildConfig({ ...valid, host: 2 })),
|
||||
).toThrow(/host/);
|
||||
expect(() =>
|
||||
readGerritIntegrationConfig(buildConfig({ ...valid, apiBaseUrl: 2 })),
|
||||
).toThrow(/apiBaseUrl/);
|
||||
readGerritIntegrationConfig(buildConfig({ ...valid, baseUrl: 2 })),
|
||||
).toThrow(/baseUrl/);
|
||||
});
|
||||
|
||||
it('works on the frontend', async () => {
|
||||
@@ -99,14 +99,14 @@ describe('readGerritIntegrationConfig', () => {
|
||||
readGerritIntegrationConfig(
|
||||
await buildFrontendConfig({
|
||||
host: 'a.com',
|
||||
apiBaseUrl: 'https://a.com/gerrit',
|
||||
baseUrl: 'https://a.com/gerrit',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
}),
|
||||
),
|
||||
).toEqual({
|
||||
host: 'a.com',
|
||||
apiBaseUrl: 'https://a.com/gerrit',
|
||||
baseUrl: 'https://a.com/gerrit',
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -121,26 +121,26 @@ describe('readGerritIntegrationConfigs', () => {
|
||||
buildConfig([
|
||||
{
|
||||
host: 'a.com',
|
||||
apiBaseUrl: 'https://a.com/api',
|
||||
baseUrl: 'https://a.com/api',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
},
|
||||
{
|
||||
host: 'b.com',
|
||||
apiBaseUrl: 'https://b.com/api',
|
||||
baseUrl: 'https://b.com/api',
|
||||
},
|
||||
]),
|
||||
);
|
||||
expect(output).toEqual([
|
||||
{
|
||||
host: 'a.com',
|
||||
apiBaseUrl: 'https://a.com/api',
|
||||
baseUrl: 'https://a.com/api',
|
||||
username: 'u',
|
||||
password: 'p',
|
||||
},
|
||||
{
|
||||
host: 'b.com',
|
||||
apiBaseUrl: 'https://b.com/api',
|
||||
baseUrl: 'https://b.com/api',
|
||||
username: undefined,
|
||||
password: undefined,
|
||||
},
|
||||
|
||||
@@ -36,7 +36,7 @@ export type GerritIntegrationConfig = {
|
||||
* "https://gerrit-review.com/gerrit". This is the url that you would open
|
||||
* in a browser.
|
||||
*/
|
||||
apiBaseUrl?: string;
|
||||
baseUrl?: string;
|
||||
|
||||
/**
|
||||
* The username to use for requests to gerrit.
|
||||
@@ -60,7 +60,7 @@ export function readGerritIntegrationConfig(
|
||||
config: Config,
|
||||
): GerritIntegrationConfig {
|
||||
const host = config.getString('host');
|
||||
let apiBaseUrl = config.getOptionalString('apiBaseUrl');
|
||||
let baseUrl = config.getOptionalString('baseUrl');
|
||||
const username = config.getOptionalString('username');
|
||||
const password = config.getOptionalString('password');
|
||||
|
||||
@@ -68,20 +68,20 @@ export function readGerritIntegrationConfig(
|
||||
throw new Error(
|
||||
`Invalid Gerrit integration config, '${host}' is not a valid host`,
|
||||
);
|
||||
} else if (apiBaseUrl && !isValidUrl(apiBaseUrl)) {
|
||||
} else if (baseUrl && !isValidUrl(baseUrl)) {
|
||||
throw new Error(
|
||||
`Invalid Gerrit integration config, '${apiBaseUrl}' is not a valid apiBaseUrl`,
|
||||
`Invalid Gerrit integration config, '${baseUrl}' is not a valid baseUrl`,
|
||||
);
|
||||
}
|
||||
if (apiBaseUrl) {
|
||||
apiBaseUrl = trimEnd(apiBaseUrl, '/');
|
||||
if (baseUrl) {
|
||||
baseUrl = trimEnd(baseUrl, '/');
|
||||
} else {
|
||||
apiBaseUrl = `https://${host}`;
|
||||
baseUrl = `https://${host}`;
|
||||
}
|
||||
|
||||
return {
|
||||
host,
|
||||
apiBaseUrl,
|
||||
baseUrl,
|
||||
username,
|
||||
password,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user