diff options
author | Ben Rohlfs <brohlfs@google.com> | 2022-10-14 12:01:21 +0200 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2022-10-14 17:35:06 +0100 |
commit | 7072581203a08021d42aec4360c2c06b395483e9 (patch) | |
tree | c1e32478eb05a17bc700a60d4b21754dedbe5228 | |
parent | a178531be78b904bd646e6312f475c69d0affdeb (diff) |
Fix the base path of all URL creation methods
During the router refactoring we did not pay close enough attention to
the "base path" feature, which is not relevant for Google hosted Gerrit.
But it allows a Gerrit instance to be served from a different path than
the root path. Every URL that is created must prefix the URL with
`getBasePath()`.
It would be nice, if this could be handled in a more generic fashion,
because this is a bit fragile: Every developer has to know/remember that
they have to prefix URLs with the base path in view model factory
functions. The only way to fix this seems to be an additional layer,
which more indirections and dependencies, so we are not introducing this
here until we are sure that there is no better solution.
Release-Notes: skip
Bug: Issue 16337
Change-Id: I45cfcfe484d2a47d0953b31c2ecbaed8a802f895
-rw-r--r-- | polygerrit-ui/app/models/views/change.ts | 4 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/change_test.ts | 7 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/dashboard.ts | 8 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/dashboard_test.ts | 6 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/diff.ts | 10 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/diff_test.ts | 4 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/edit.ts | 10 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/edit_test.ts | 4 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/search.ts | 6 | ||||
-rw-r--r-- | polygerrit-ui/app/models/views/search_test.ts | 4 |
10 files changed, 48 insertions, 15 deletions
diff --git a/polygerrit-ui/app/models/views/change.ts b/polygerrit-ui/app/models/views/change.ts index 3790bb0521..100c46bac8 100644 --- a/polygerrit-ui/app/models/views/change.ts +++ b/polygerrit-ui/app/models/views/change.ts @@ -146,9 +146,9 @@ export function createChangeUrl( } if (state.project) { const encodedProject = encodeURL(state.project, true); - return getBaseUrl() + `/c/${encodedProject}/+/${state.changeNum}${suffix}`; + return `${getBaseUrl()}/c/${encodedProject}/+/${state.changeNum}${suffix}`; } else { - return getBaseUrl() + `/c/${state.changeNum}${suffix}`; + return `${getBaseUrl()}/c/${state.changeNum}${suffix}`; } } diff --git a/polygerrit-ui/app/models/views/change_test.ts b/polygerrit-ui/app/models/views/change_test.ts index 2cea325332..24ced82ac3 100644 --- a/polygerrit-ui/app/models/views/change_test.ts +++ b/polygerrit-ui/app/models/views/change_test.ts @@ -36,6 +36,13 @@ suite('change view state tests', () => { assert.equal(createChangeUrl(state), '/c/test/+/1234/5..10#123'); }); + test('createChangeUrl() baseUrl', () => { + window.CANONICAL_PATH = '/base'; + const state: ChangeViewState = {...STATE}; + assert.equal(createChangeUrl(state).substring(0, 5), '/base'); + window.CANONICAL_PATH = undefined; + }); + test('createChangeUrl() checksRunsSelected', () => { const state: ChangeViewState = { ...STATE, diff --git a/polygerrit-ui/app/models/views/dashboard.ts b/polygerrit-ui/app/models/views/dashboard.ts index 7141637c1f..d9ff2d28d8 100644 --- a/polygerrit-ui/app/models/views/dashboard.ts +++ b/polygerrit-ui/app/models/views/dashboard.ts @@ -7,7 +7,7 @@ import {RepoName} from '../../api/rest-api'; import {GerritView} from '../../services/router/router-model'; import {DashboardId} from '../../types/common'; import {DashboardSection} from '../../utils/dashboard-util'; -import {encodeURL} from '../../utils/url-util'; +import {encodeURL, getBaseUrl} from '../../utils/url-util'; import {define} from '../dependency'; import {Model} from '../model'; import {ViewState} from './base'; @@ -46,14 +46,14 @@ export function createDashboardUrl(state: Omit<DashboardViewState, 'view'>) { queryParams.push('title=' + encodeURIComponent(state.title)); } const user = state.user ? state.user : ''; - return `/dashboard/${user}?${queryParams.join('&')}`; + return `${getBaseUrl()}/dashboard/${user}?${queryParams.join('&')}`; } else if (repoName) { // Project dashboard. const encodedRepo = encodeURL(repoName, true); - return `/p/${encodedRepo}/+/dashboard/${state.dashboard}`; + return `${getBaseUrl()}/p/${encodedRepo}/+/dashboard/${state.dashboard}`; } else { // User dashboard. - return `/dashboard/${state.user || 'self'}`; + return `${getBaseUrl()}/dashboard/${state.user || 'self'}`; } } diff --git a/polygerrit-ui/app/models/views/dashboard_test.ts b/polygerrit-ui/app/models/views/dashboard_test.ts index 9deed72738..86bb5c052c 100644 --- a/polygerrit-ui/app/models/views/dashboard_test.ts +++ b/polygerrit-ui/app/models/views/dashboard_test.ts @@ -15,6 +15,12 @@ suite('dashboard view state tests', () => { assert.equal(createDashboardUrl({}), '/dashboard/self'); }); + test('baseUrl', () => { + window.CANONICAL_PATH = '/base'; + assert.equal(createDashboardUrl({}).substring(0, 5), '/base'); + window.CANONICAL_PATH = undefined; + }); + test('user dashboard', () => { assert.equal(createDashboardUrl({user: 'user'}), '/dashboard/user'); }); diff --git a/polygerrit-ui/app/models/views/diff.ts b/polygerrit-ui/app/models/views/diff.ts index 63df521939..3cc107abcf 100644 --- a/polygerrit-ui/app/models/views/diff.ts +++ b/polygerrit-ui/app/models/views/diff.ts @@ -12,7 +12,11 @@ import { } from '../../api/rest-api'; import {GerritView} from '../../services/router/router-model'; import {UrlEncodedCommentId} from '../../types/common'; -import {encodeURL, getPatchRangeExpression} from '../../utils/url-util'; +import { + encodeURL, + getBaseUrl, + getPatchRangeExpression, +} from '../../utils/url-util'; import {define} from '../dependency'; import {Model} from '../model'; import {ViewState} from './base'; @@ -85,9 +89,9 @@ export function createDiffUrl( if (state.project) { const encodedProject = encodeURL(state.project, true); - return `/c/${encodedProject}/+/${state.changeNum}${suffix}`; + return `${getBaseUrl()}/c/${encodedProject}/+/${state.changeNum}${suffix}`; } else { - return `/c/${state.changeNum}${suffix}`; + return `${getBaseUrl()}/c/${state.changeNum}${suffix}`; } } diff --git a/polygerrit-ui/app/models/views/diff_test.ts b/polygerrit-ui/app/models/views/diff_test.ts index ec20037746..b0f91bb316 100644 --- a/polygerrit-ui/app/models/views/diff_test.ts +++ b/polygerrit-ui/app/models/views/diff_test.ts @@ -25,6 +25,10 @@ suite('diff view state tests', () => { }; assert.equal(createDiffUrl(params), '/c/42/12/x%252By/path.cpp'); + window.CANONICAL_PATH = '/base'; + assert.equal(createDiffUrl(params).substring(0, 5), '/base'); + window.CANONICAL_PATH = undefined; + params.project = 'test' as RepoName; assert.equal(createDiffUrl(params), '/c/test/+/42/12/x%252By/path.cpp'); diff --git a/polygerrit-ui/app/models/views/edit.ts b/polygerrit-ui/app/models/views/edit.ts index d8f4770ee2..c63c8ce508 100644 --- a/polygerrit-ui/app/models/views/edit.ts +++ b/polygerrit-ui/app/models/views/edit.ts @@ -10,7 +10,11 @@ import { RevisionPatchSetNum, } from '../../api/rest-api'; import {GerritView} from '../../services/router/router-model'; -import {encodeURL, getPatchRangeExpression} from '../../utils/url-util'; +import { + encodeURL, + getBaseUrl, + getPatchRangeExpression, +} from '../../utils/url-util'; import {define} from '../dependency'; import {Model} from '../model'; import {ViewState} from './base'; @@ -41,9 +45,9 @@ export function createEditUrl(state: Omit<EditViewState, 'view'>): string { if (state.project) { const encodedProject = encodeURL(state.project, true); - return `/c/${encodedProject}/+/${state.changeNum}${suffix}`; + return `${getBaseUrl()}/c/${encodedProject}/+/${state.changeNum}${suffix}`; } else { - return `/c/${state.changeNum}${suffix}`; + return `${getBaseUrl()}/c/${state.changeNum}${suffix}`; } } diff --git a/polygerrit-ui/app/models/views/edit_test.ts b/polygerrit-ui/app/models/views/edit_test.ts index e1a05c75ac..291206334d 100644 --- a/polygerrit-ui/app/models/views/edit_test.ts +++ b/polygerrit-ui/app/models/views/edit_test.ts @@ -27,5 +27,9 @@ suite('edit view state tests', () => { createEditUrl(params), '/c/test-project/+/42/12/x%252By/path.cpp,edit#31' ); + + window.CANONICAL_PATH = '/base'; + assert.equal(createEditUrl(params).substring(0, 5), '/base'); + window.CANONICAL_PATH = undefined; }); }); diff --git a/polygerrit-ui/app/models/views/search.ts b/polygerrit-ui/app/models/views/search.ts index 58cb8f7ab5..13de8f39f2 100644 --- a/polygerrit-ui/app/models/views/search.ts +++ b/polygerrit-ui/app/models/views/search.ts @@ -6,7 +6,7 @@ import {RepoName, BranchName, TopicName} from '../../api/rest-api'; import {GerritView} from '../../services/router/router-model'; import {addQuotesWhen} from '../../utils/string-util'; -import {encodeURL} from '../../utils/url-util'; +import {encodeURL, getBaseUrl} from '../../utils/url-util'; import {define} from '../dependency'; import {Model} from '../model'; import {ViewState} from './base'; @@ -35,7 +35,7 @@ export function createSearchUrl(params: SearchUrlOptions): string { } if (params.query) { - return '/q/' + encodeURL(params.query, true) + offsetExpr; + return `${getBaseUrl()}/q/${encodeURL(params.query, true)}${offsetExpr}`; } const operators: string[] = []; @@ -80,7 +80,7 @@ export function createSearchUrl(params: SearchUrlOptions): string { } } - return '/q/' + operators.join('+') + offsetExpr; + return `${getBaseUrl()}/q/${operators.join('+')}${offsetExpr}`; } export const searchViewModelToken = diff --git a/polygerrit-ui/app/models/views/search_test.ts b/polygerrit-ui/app/models/views/search_test.ts index 138ce1ec09..d48667bfe5 100644 --- a/polygerrit-ui/app/models/views/search_test.ts +++ b/polygerrit-ui/app/models/views/search_test.ts @@ -23,6 +23,10 @@ suite('search view state tests', () => { 'topic:g%2525h+status:op%2525en' ); + window.CANONICAL_PATH = '/base'; + assert.equal(createSearchUrl(options).substring(0, 5), '/base'); + window.CANONICAL_PATH = undefined; + options.offset = 100; assert.equal( createSearchUrl(options), |