Refactor weblinks
`gr-navigation` and `gr-router` had 5 methods for generating weblinks.
There was no obvious reason why these methods were part of those
classes, so instead we have moved them to a weblink-util.ts file.
Of the 5 types of weblinks 3 could actually be replaced by simply
inlining something at the callsite. `file`, `edit` and `resolve
conflict` weblinks do not even need a utility function. We have only
kept a utility function for `change` and `patchset` weblinks.
`gr-commit-info` offered so much easy refactoring opportunity for
making it simpler without changing the outcome that we just to go for
it along the way.
Release-Notes: skip
Google-Bug-Id: b/244279450
Change-Id: I374b14500b0005b53160b92822f44f64fb7301f3
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
index 8b28500..228e7ce 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.ts
@@ -20,7 +20,6 @@
import '../gr-submit-requirements/gr-submit-requirements';
import '../gr-commit-info/gr-commit-info';
import '../gr-reviewer-list/gr-reviewer-list';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {
ChangeStatus,
GpgKeyInfoStatus,
@@ -81,6 +80,7 @@
import {ifDefined} from 'lit/directives/if-defined.js';
import {createSearchUrl} from '../../../models/views/search';
import {createChangeUrl} from '../../../models/views/change';
+import {GeneratedWebLink, getChangeWeblinks} from '../../../utils/weblink-util';
const HASHTAG_ADD_MESSAGE = 'Add Hashtag';
@@ -510,10 +510,7 @@
<ol class=${this.computeParentListClass()}>
${this.currentParents.map(
parent => html` <li>
- <gr-commit-info
- .change=${this.change}
- .commitInfo=${parent}
- ></gr-commit-info>
+ <gr-commit-info .commitInfo=${parent}></gr-commit-info>
<gr-tooltip-content
id="parentNotCurrentMessage"
has-tooltip
@@ -534,7 +531,6 @@
<span class="title">Merged As</span>
<span class="value">
<gr-commit-info
- .change=${this.change}
.commitInfo=${this.computeMergedCommitInfo(
this.change?.current_revision,
this.change?.revisions
@@ -553,7 +549,6 @@
<span class="title">${this.getRevertSectionTitle()}</span>
<span class="value">
<gr-commit-info
- .change=${this.change}
.commitInfo=${this.computeRevertCommit()}
></gr-commit-info>
</span>
@@ -724,23 +719,9 @@
}
}
- /**
- * @return If array is empty, returns undefined instead so
- * an existential check can be used to hide or show the webLinks
- * section.
- * private but used in test
- */
- computeWebLinks() {
- if (!this.commitInfo) return [];
- const weblinks = GerritNav.getChangeWeblinks(
- this.change ? this.change.project : ('' as RepoName),
- this.commitInfo.commit,
- {
- weblinks: this.commitInfo.web_links,
- config: this.serverConfig,
- }
- );
- return weblinks.length ? weblinks : [];
+ // private but used in test
+ computeWebLinks(): GeneratedWebLink[] {
+ return getChangeWeblinks(this.commitInfo?.web_links, this.serverConfig);
}
private computeStrategy() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
index 6fde9ca..038c34a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.ts
@@ -5,7 +5,6 @@
*/
import '../../../test/common-test-setup';
import './gr-change-metadata';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader';
import {ChangeRole, GrChangeMetadata} from './gr-change-metadata';
import {
@@ -246,19 +245,6 @@
assert.isNull(element.shadowRoot?.querySelector('.strategy'));
});
- test('weblinks use GerritNav interface', async () => {
- const weblinksStub = sinon
- .stub(GerritNav, '_generateWeblinks')
- .returns([{name: 'stubb', url: '#s'}]);
- element.commitInfo = createCommitInfoWithRequiredCommit();
- element.serverConfig = createServerInfo();
- await element.updateComplete;
- const webLinks = element.webLinks!;
- assert.isTrue(weblinksStub.called);
- assert.isNotNull(webLinks);
- assert.equal(element.computeWebLinks().length, 1);
- });
-
test('weblinks hidden when no weblinks', async () => {
element.commitInfo = createCommitInfoWithRequiredCommit();
element.serverConfig = createServerInfo();
@@ -295,10 +281,6 @@
});
test('weblinks are visible when other weblinks', async () => {
- sinon
- .stub(GerritNav, '_generateWeblinks')
- .callsFake(() => [{name: 'test-name'}]);
-
element.commitInfo = {
...createCommitInfoWithRequiredCommit(),
web_links: [{...createWebLinkInfo(), name: 'test', url: '#'}],
@@ -310,10 +292,6 @@
});
test('weblinks are visible when gitiles and other weblinks', async () => {
- sinon
- .stub(GerritNav, '_generateWeblinks')
- .callsFake(() => [{name: 'test-name'}]);
-
element.commitInfo = {
...createCommitInfoWithRequiredCommit(),
web_links: [
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 4c2119e..b482b82 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -1258,7 +1258,7 @@
}
private renderHeaderTitle() {
- const resolveWeblinks = this.computeResolveWeblinks();
+ const resolveWeblinks = this.commitInfo?.resolve_conflicts_web_links ?? [];
return html` <div class="headerTitle">
<div class="changeStatuses">
${this.changeStatuses.map(
@@ -3068,20 +3068,6 @@
});
}
- private computeResolveWeblinks() {
- if (!this.change || !this.commitInfo || !this.serverConfig) {
- return [];
- }
- return GerritNav.getResolveConflictsWeblinks(
- this.change.project,
- this.commitInfo.commit,
- {
- weblinks: this.commitInfo.resolve_conflicts_web_links,
- config: this.serverConfig,
- }
- );
- }
-
/**
* Returns the text to be copied when
* click the copy icon next to change subject
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.ts b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.ts
index c6019d0..d6a5327 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.ts
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info.ts
@@ -4,15 +4,15 @@
* SPDX-License-Identifier: Apache-2.0
*/
import '../../shared/gr-copy-clipboard/gr-copy-clipboard';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
-import {ChangeInfo, CommitInfo, ServerInfo} from '../../../types/common';
+import {CommitInfo, ServerInfo} from '../../../types/common';
import {sharedStyles} from '../../../styles/shared-styles';
-import {LitElement, css, html} from 'lit';
+import {LitElement, css, html, nothing} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
import {subscribe} from '../../lit/subscription-controller';
import {resolve} from '../../../models/dependency';
import {configModelToken} from '../../../models/config/config-model';
import {createSearchUrl} from '../../../models/views/search';
+import {getPatchSetWeblink} from '../../../utils/weblink-util';
declare global {
interface HTMLElementTagNameMap {
@@ -22,17 +22,11 @@
@customElement('gr-commit-info')
export class GrCommitInfo extends LitElement {
- // TODO(TS): can not use `?` here as @computed require dependencies as
- // not optional
+ // TODO(TS): Maybe limit to StandaloneCommitInfo.
@property({type: Object})
- change: ChangeInfo | undefined;
+ commitInfo?: CommitInfo;
- // TODO(TS): maybe limit to StandaloneCommitInfo if never pass in
- // with commit inside RevisionInfo
- @property({type: Object})
- commitInfo: CommitInfo | undefined;
-
- @state() serverConfig: ServerInfo | undefined;
+ @state() serverConfig?: ServerInfo;
private readonly getConfigModel = resolve(this, configModelToken);
@@ -53,96 +47,40 @@
subscribe(
this,
() => this.getConfigModel().serverConfig$,
- config => {
- this.serverConfig = config;
- }
+ config => (this.serverConfig = config)
);
}
override render() {
+ const commit = this.commitInfo?.commit;
+ if (!commit) return nothing;
return html` <div class="container">
- <a
- target="_blank"
- rel="noopener"
- href=${this.computeCommitLink(
- this._webLink,
- this.change,
- this.commitInfo,
- this.serverConfig
- )}
- >${this._computeShortHash(
- this.change,
- this.commitInfo,
- this.serverConfig
- )}</a
+ <a target="_blank" rel="noopener" href=${this.computeCommitLink()}
+ >${this.getWeblink()?.name ?? ''}</a
>
<gr-copy-clipboard
hastooltip
.buttonTitle=${'Copy full SHA to clipboard'}
hideinput
- .text=${this.commitInfo?.commit}
+ .text=${commit}
>
</gr-copy-clipboard>
</div>`;
}
- /**
- * Used only within the tests.
- */
- get _showWebLink(): boolean {
- if (!this.change || !this.commitInfo || !this.serverConfig) {
- return false;
- }
-
- const weblink = this._getWeblink(
- this.change,
- this.commitInfo,
+ getWeblink() {
+ return getPatchSetWeblink(
+ this.commitInfo?.commit,
+ this.commitInfo?.web_links,
this.serverConfig
);
- return !!weblink && !!weblink.url;
}
- get _webLink(): string | undefined {
- if (!this.change || !this.commitInfo || !this.serverConfig) {
- return '';
- }
+ computeCommitLink() {
+ const weblink = this.getWeblink();
+ if (weblink?.url) return weblink.url;
- // TODO(TS): if getPatchSetWeblink always return a valid WebLink,
- // can remove the fallback here
- const {url} =
- this._getWeblink(this.change, this.commitInfo, this.serverConfig) || {};
- return url;
- }
-
- _getWeblink(change: ChangeInfo, commitInfo: CommitInfo, config: ServerInfo) {
- return GerritNav.getPatchSetWeblink(change.project, commitInfo.commit, {
- weblinks: commitInfo.web_links,
- config,
- });
- }
-
- computeCommitLink(
- webLink?: string,
- change?: ChangeInfo,
- commitInfo?: CommitInfo,
- serverConfig?: ServerInfo
- ) {
- if (webLink) return webLink;
- const hash = this._computeShortHash(change, commitInfo, serverConfig);
- if (hash === undefined) return '';
- return createSearchUrl({query: hash});
- }
-
- _computeShortHash(
- change?: ChangeInfo,
- commitInfo?: CommitInfo,
- serverConfig?: ServerInfo
- ) {
- if (!change || !commitInfo || !serverConfig) {
- return '';
- }
-
- const weblink = this._getWeblink(change, commitInfo, serverConfig);
- return weblink?.name ?? '';
+ const hash = weblink?.name;
+ return hash ? createSearchUrl({query: hash}) : '';
}
}
diff --git a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.ts b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.ts
index 92fc2bb..d992ffe 100644
--- a/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-commit-info/gr-commit-info_test.ts
@@ -6,73 +6,62 @@
import '../../../test/common-test-setup';
import './gr-commit-info';
import {GrCommitInfo} from './gr-commit-info';
-import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {
- createChange,
createCommit,
createServerInfo,
} from '../../../test/test-data-generators';
-import {CommitId, RepoName} from '../../../types/common';
+import {CommitId} from '../../../types/common';
import {fixture, html, assert} from '@open-wc/testing';
-import {waitEventLoop} from '../../../test/test-utils';
suite('gr-commit-info tests', () => {
let element: GrCommitInfo;
setup(async () => {
element = await fixture(html`<gr-commit-info></gr-commit-info>`);
+ element.serverConfig = createServerInfo();
});
- test('render', async () => {
- element.change = createChange();
+ test('render nothing', async () => {
element.commitInfo = createCommit();
- element.serverConfig = createServerInfo();
+ await element.updateComplete;
+
+ assert.shadowDom.equal(element, '');
+ });
+
+ test('web link from commit info', async () => {
+ element.commitInfo = {
+ ...createCommit(),
+ commit: 'sha45678901234567890' as CommitId,
+ web_links: [{name: 'gitweb', url: 'link-url'}],
+ };
await element.updateComplete;
assert.shadowDom.equal(
element,
/* HTML */ `
<div class="container">
- <a href="/q/" rel="noopener" target="_blank"> </a>
+ <a href="link-url" rel="noopener" target="_blank">sha4567</a>
<gr-copy-clipboard hastooltip="" hideinput=""> </gr-copy-clipboard>
</div>
`
);
});
- test('weblinks use GerritNav interface', async () => {
- const weblinksStub = sinon
- .stub(GerritNav, '_generateWeblinks')
- .returns([{name: 'stubb', url: '#s'}]);
- element.change = createChange();
- element.commitInfo = createCommit();
- element.serverConfig = createServerInfo();
- await waitEventLoop();
- assert.isTrue(weblinksStub.called);
- });
-
- test('no web link when unavailable', () => {
- element.commitInfo = createCommit();
- element.serverConfig = createServerInfo();
- element.change = {...createChange(), labels: {}, project: '' as RepoName};
-
- assert.isNotOk(element._showWebLink);
- });
-
- test('use web link when available', () => {
- sinon.stub(GerritNav, 'getPatchSetWeblink').callsFake(() => {
- return {name: 'test-name', url: 'test-url'};
- });
-
- element.change = {...createChange(), labels: {}, project: '' as RepoName};
+ test('web link fall back to search query', async () => {
element.commitInfo = {
...createCommit(),
- commit: 'commitsha' as CommitId,
- web_links: [{name: 'gitweb', url: 'link-url'}],
+ commit: 'sha45678901234567890' as CommitId,
};
- element.serverConfig = createServerInfo();
+ await element.updateComplete;
- assert.isOk(element._showWebLink);
- assert.equal(element._webLink, 'test-url');
+ assert.shadowDom.equal(
+ element,
+ /* HTML */ `
+ <div class="container">
+ <a href="/q/sha4567" rel="noopener" target="_blank">sha4567</a>
+ <gr-copy-clipboard hastooltip="" hideinput=""> </gr-copy-clipboard>
+ </div>
+ `
+ );
});
});
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index cd31ce3..bbebf10 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -285,11 +285,7 @@
>
</gr-patch-range-select>
<span class="separator"></span>
- <gr-commit-info
- .change=${this.change}
- .serverConfig=${this.serverConfig}
- .commitInfo=${this.commitInfo}
- ></gr-commit-info>
+ <gr-commit-info .commitInfo=${this.commitInfo}></gr-commit-info>
<span class="container latestPatchContainer">
<span class="separator"></span>
<a href=${ifDefined(this.changeUrl)}>Go to latest patch set</a>
diff --git a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
index 4780a53..ed2b8af 100644
--- a/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
+++ b/polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.ts
@@ -8,11 +8,9 @@
ChangeConfigInfo,
ChangeInfo,
CommentLinks,
- CommitId,
PatchSetNum,
RepoName,
RevisionPatchSetNum,
- ServerInfo,
} from '../../../types/common';
import {ParsedChangeInfo} from '../../../types/types';
import {createRepoUrl} from '../../../models/views/repo';
@@ -33,11 +31,6 @@
return '';
};
-const uninitializedGenerateWebLinks: GenerateWebLinksCallback = () => {
- uninitialized();
- return [];
-};
-
const uninitializedMapCommentLinks: MapCommentLinksCallback = () => {
uninitialized();
return {};
@@ -121,79 +114,10 @@
CLOSED,
];
-export interface GenerateWebLinksOptions {
- weblinks?: GeneratedWebLink[];
- config?: ServerInfo;
-}
-
-export interface GenerateWebLinksPatchsetParameters {
- type: WeblinkType.PATCHSET;
- repo: RepoName;
- commit?: CommitId;
- options?: GenerateWebLinksOptions;
-}
-export interface GenerateWebLinksResolveConflictsParameters {
- type: WeblinkType.RESOLVE_CONFLICTS;
- repo: RepoName;
- commit?: CommitId;
- options?: GenerateWebLinksOptions;
-}
-export interface GenerateWebLinksEditParameters {
- type: WeblinkType.EDIT;
- repo: RepoName;
- commit: CommitId;
- file: string;
- options?: GenerateWebLinksOptions;
-}
-export interface GenerateWebLinksFileParameters {
- type: WeblinkType.FILE;
- repo: RepoName;
- commit: CommitId;
- file: string;
- options?: GenerateWebLinksOptions;
-}
-export interface GenerateWebLinksChangeParameters {
- type: WeblinkType.CHANGE;
- repo: RepoName;
- commit: CommitId;
- options?: GenerateWebLinksOptions;
-}
-
-export type GenerateWebLinksParameters =
- | GenerateWebLinksPatchsetParameters
- | GenerateWebLinksResolveConflictsParameters
- | GenerateWebLinksEditParameters
- | GenerateWebLinksFileParameters
- | GenerateWebLinksChangeParameters;
-
export type NavigateCallback = (target: string, redirect?: boolean) => void;
-// TODO: Refactor to return only GeneratedWebLink[]
-export type GenerateWebLinksCallback = (
- params: GenerateWebLinksParameters
-) => GeneratedWebLink[] | GeneratedWebLink;
export type MapCommentLinksCallback = (patterns: CommentLinks) => CommentLinks;
-export interface WebLink {
- name?: string;
- label: string;
- url: string;
-}
-
-export interface GeneratedWebLink {
- name?: string;
- label?: string;
- url?: string;
-}
-
-export enum WeblinkType {
- CHANGE = 'change',
- EDIT = 'edit',
- FILE = 'file',
- PATCHSET = 'patchset',
- RESOLVE_CONFLICTS = 'resolve-conflicts',
-}
-
interface NavigateToChangeParams {
patchNum?: RevisionPatchSetNum;
basePatchNum?: BasePatchSetNum;
@@ -208,8 +132,6 @@
export const GerritNav = {
_navigate: uninitializedNavigate,
- _generateWeblinks: uninitializedGenerateWebLinks,
-
mapCommentlinks: uninitializedMapCommentLinks,
_checkPatchRange(patchNum?: PatchSetNum, basePatchNum?: BasePatchSetNum) {
@@ -225,37 +147,17 @@
* `window.location.href = ...` or window.location.replace(...). The
* string is a new location and boolean defines is it redirect or not
* (true means redirect, i.e. equivalent of window.location.replace).
- * @param generateUrl generates a URL given
- * navigation parameters, detailed in the file header.
- * @param generateWeblinks weblinks generator
- * function takes single payload parameter with type property that
- * determines which
- * part of the UI is the consumer of the weblinks. type property can
- * be one of file, change, or patchset.
- * - For file type, payload will also contain string properties: repo,
- * commit, file.
- * - For patchset type, payload will also contain string properties:
- * repo, commit.
- * - For change type, payload will also contain string properties:
- * repo, commit. If server provides weblinks, those will be passed
- * as options.weblinks property on the main payload object.
* @param mapCommentlinks provides an escape
* hatch to modify the commentlinks object, e.g. if it contains any
* relative URLs.
*/
- setup(
- navigate: NavigateCallback,
- generateWeblinks: GenerateWebLinksCallback,
- mapCommentlinks: MapCommentLinksCallback
- ) {
+ setup(navigate: NavigateCallback, mapCommentlinks: MapCommentLinksCallback) {
this._navigate = navigate;
- this._generateWeblinks = generateWeblinks;
this.mapCommentlinks = mapCommentlinks;
},
destroy() {
this._navigate = uninitializedNavigate;
- this._generateWeblinks = uninitializedGenerateWebLinks;
this.mapCommentlinks = uninitializedMapCommentLinks;
},
@@ -354,98 +256,6 @@
this._navigate(createRepoUrl({repo}));
},
- getEditWebLinks(
- repo: RepoName,
- commit: CommitId,
- file: string,
- options?: GenerateWebLinksOptions
- ): GeneratedWebLink[] {
- const params: GenerateWebLinksEditParameters = {
- type: WeblinkType.EDIT,
- repo,
- commit,
- file,
- };
- if (options) {
- params.options = options;
- }
- return ([] as GeneratedWebLink[]).concat(this._generateWeblinks(params));
- },
-
- getFileWebLinks(
- repo: RepoName,
- commit: CommitId,
- file: string,
- options?: GenerateWebLinksOptions
- ): GeneratedWebLink[] {
- const params: GenerateWebLinksFileParameters = {
- type: WeblinkType.FILE,
- repo,
- commit,
- file,
- };
- if (options) {
- params.options = options;
- }
- return ([] as GeneratedWebLink[]).concat(this._generateWeblinks(params));
- },
-
- getPatchSetWeblink(
- repo: RepoName,
- commit?: CommitId,
- options?: GenerateWebLinksOptions
- ): GeneratedWebLink {
- const params: GenerateWebLinksPatchsetParameters = {
- type: WeblinkType.PATCHSET,
- repo,
- commit,
- };
- if (options) {
- params.options = options;
- }
- const result = this._generateWeblinks(params);
- if (Array.isArray(result)) {
- // TODO(TS): Unclear what to do with empty array.
- // Either write a comment why result can't be empty or change the return
- // type or add a check.
- return result.pop()!;
- } else {
- return result;
- }
- },
-
- getResolveConflictsWeblinks(
- repo: RepoName,
- commit?: CommitId,
- options?: GenerateWebLinksOptions
- ): GeneratedWebLink[] {
- const params: GenerateWebLinksResolveConflictsParameters = {
- type: WeblinkType.RESOLVE_CONFLICTS,
- repo,
- commit,
- };
- if (options) {
- params.options = options;
- }
- return ([] as GeneratedWebLink[]).concat(this._generateWeblinks(params));
- },
-
- getChangeWeblinks(
- repo: RepoName,
- commit: CommitId,
- options?: GenerateWebLinksOptions
- ): GeneratedWebLink[] {
- const params: GenerateWebLinksChangeParameters = {
- type: WeblinkType.CHANGE,
- repo,
- commit,
- };
- if (options) {
- params.options = options;
- }
- return ([] as GeneratedWebLink[]).concat(this._generateWeblinks(params));
- },
-
getUserDashboard(
user = 'self',
sections = DEFAULT_SECTIONS,
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 8e46786..705f2cc 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -8,20 +8,10 @@
PageContext,
PageNextCallback,
} from '../../../utils/page-wrapper-utils';
-import {
- GeneratedWebLink,
- GenerateWebLinksChangeParameters,
- GenerateWebLinksEditParameters,
- GenerateWebLinksFileParameters,
- GenerateWebLinksParameters,
- GenerateWebLinksPatchsetParameters,
- GenerateWebLinksResolveConflictsParameters,
- GerritNav,
- WeblinkType,
-} from '../gr-navigation/gr-navigation';
+import {GerritNav} from '../gr-navigation/gr-navigation';
import {getAppContext} from '../../../services/app-context';
import {convertToPatchSetNum} from '../../../utils/patch-set-util';
-import {assertIsDefined, assertNever} from '../../../utils/common-util';
+import {assertIsDefined} from '../../../utils/common-util';
import {
BasePatchSetNum,
DashboardId,
@@ -29,7 +19,6 @@
NumericChangeId,
RevisionPatchSetNum,
RepoName,
- ServerInfo,
UrlEncodedCommentId,
PARENT,
} from '../../../types/common';
@@ -363,107 +352,6 @@
page.redirect(url);
}
- generateWeblinks(
- params: GenerateWebLinksParameters
- ): GeneratedWebLink[] | GeneratedWebLink {
- switch (params.type) {
- case WeblinkType.EDIT:
- return this.getEditWebLinks(params);
- case WeblinkType.FILE:
- return this.getFileWebLinks(params);
- case WeblinkType.CHANGE:
- return this.getChangeWeblinks(params);
- case WeblinkType.PATCHSET:
- return this.getPatchSetWeblink(params);
- case WeblinkType.RESOLVE_CONFLICTS:
- return this.getResolveConflictsWeblinks(params);
- default:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- assertNever(params, `Unsupported weblink ${(params as any).type}!`);
- }
- }
-
- private getPatchSetWeblink(
- params: GenerateWebLinksPatchsetParameters
- ): GeneratedWebLink {
- const {commit, options} = params;
- const {weblinks, config} = options || {};
- const name = commit && commit.slice(0, 7);
- const weblink = this.getBrowseCommitWeblink(weblinks, config);
- if (!weblink || !weblink.url) {
- return {name};
- } else {
- return {name, url: weblink.url};
- }
- }
-
- private getResolveConflictsWeblinks(
- params: GenerateWebLinksResolveConflictsParameters
- ): GeneratedWebLink[] {
- return params.options?.weblinks ?? [];
- }
-
- firstCodeBrowserWeblink(weblinks: GeneratedWebLink[]) {
- // This is an ordered allowed list of web link types that provide direct
- // links to the commit in the url property.
- const codeBrowserLinks = ['gitiles', 'browse', 'gitweb'];
- for (let i = 0; i < codeBrowserLinks.length; i++) {
- const weblink = weblinks.find(
- weblink => weblink.name === codeBrowserLinks[i]
- );
- if (weblink) {
- return weblink;
- }
- }
- return null;
- }
-
- getBrowseCommitWeblink(weblinks?: GeneratedWebLink[], config?: ServerInfo) {
- if (!weblinks) {
- return null;
- }
- let weblink;
- // Use primary weblink if configured and exists.
- if (config?.gerrit?.primary_weblink_name) {
- const primaryWeblinkName = config.gerrit.primary_weblink_name;
- weblink = weblinks.find(weblink => weblink.name === primaryWeblinkName);
- }
- if (!weblink) {
- weblink = this.firstCodeBrowserWeblink(weblinks);
- }
- if (!weblink) {
- return null;
- }
- return weblink;
- }
-
- getChangeWeblinks(
- params: GenerateWebLinksChangeParameters
- ): GeneratedWebLink[] {
- const weblinks = params.options?.weblinks;
- const config = params.options?.config;
- if (!weblinks || !weblinks.length) return [];
- const commitWeblink = this.getBrowseCommitWeblink(weblinks, config);
- return weblinks.filter(
- weblink =>
- !commitWeblink ||
- !commitWeblink.name ||
- weblink.name !== commitWeblink.name
- );
- }
-
- private getEditWebLinks(
- params: GenerateWebLinksEditParameters
- ): GeneratedWebLink[] {
- return params.options?.weblinks ?? [];
- }
-
- private getFileWebLinks(
- params: GenerateWebLinksFileParameters
- ): GeneratedWebLink[] {
- return params.options?.weblinks ?? [];
- }
-
/**
* Normalizes the patchset numbers of the params object.
*/
@@ -611,7 +499,6 @@
page.show(url);
}
},
- params => this.generateWeblinks(params),
x => x
);
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index 42659ca..fd87828 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -6,7 +6,7 @@
import '../../../test/common-test-setup';
import './gr-router';
import {page} from '../../../utils/page-wrapper-utils';
-import {GerritNav, WeblinkType} from '../gr-navigation/gr-navigation';
+import {GerritNav} from '../gr-navigation/gr-navigation';
import {
stubBaseUrl,
stubRestApi,
@@ -22,19 +22,13 @@
import {GerritView} from '../../../services/router/router-model';
import {
BasePatchSetNum,
- CommitId,
GroupId,
NumericChangeId,
PARENT,
RepoName,
RevisionPatchSetNum,
UrlEncodedCommentId,
- WebLinkInfo,
} from '../../../types/common';
-import {
- createGerritInfo,
- createServerInfo,
-} from '../../../test/test-data-generators';
import {AppElementParams} from '../../gr-app-types';
import {assert} from '@open-wc/testing';
import {AdminChildView} from '../../../models/views/admin';
@@ -54,71 +48,6 @@
);
});
- test('firstCodeBrowserWeblink', () => {
- assert.deepEqual(
- router.firstCodeBrowserWeblink([
- {name: 'gitweb'},
- {name: 'gitiles'},
- {name: 'browse'},
- {name: 'test'},
- ]),
- {name: 'gitiles'}
- );
-
- assert.deepEqual(
- router.firstCodeBrowserWeblink([{name: 'gitweb'}, {name: 'test'}]),
- {name: 'gitweb'}
- );
- });
-
- test('getBrowseCommitWeblink', () => {
- const browserLink = {name: 'browser', url: 'browser/url'};
- const link = {name: 'test', url: 'test/url'};
- const weblinks = [browserLink, link];
- const config = {
- ...createServerInfo(),
- gerrit: {...createGerritInfo(), primary_weblink_name: browserLink.name},
- };
- sinon.stub(router, 'firstCodeBrowserWeblink').returns(link);
-
- assert.deepEqual(
- router.getBrowseCommitWeblink(weblinks, config),
- browserLink
- );
-
- assert.deepEqual(router.getBrowseCommitWeblink(weblinks), link);
- });
-
- test('getChangeWeblinks', () => {
- const link = {name: 'test', url: 'test/url'};
- const browserLink = {name: 'browser', url: 'browser/url'};
- const mapLinksToConfig = (weblinks: WebLinkInfo[]) => {
- return {
- type: 'change' as WeblinkType.CHANGE,
- repo: 'test' as RepoName,
- commit: '111' as CommitId,
- options: {weblinks},
- };
- };
- sinon.stub(router, 'getBrowseCommitWeblink').returns(browserLink);
-
- assert.deepEqual(
- router.getChangeWeblinks(mapLinksToConfig([link, browserLink]))[0],
- {name: 'test', url: 'test/url'}
- );
-
- assert.deepEqual(router.getChangeWeblinks(mapLinksToConfig([link]))[0], {
- name: 'test',
- url: 'test/url',
- });
-
- link.url = `https://${link.url}`;
- assert.deepEqual(router.getChangeWeblinks(mapLinksToConfig([link]))[0], {
- name: 'test',
- url: 'https://test/url',
- });
- });
-
test('getHashFromCanonicalPath', () => {
let url = '/foo/bar';
let hash = router.getHashFromCanonicalPath(url);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index 7554bf0..91c9dbcb 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -7,10 +7,6 @@
import '../../checks/gr-diff-check-result';
import '../../../embed/diff/gr-diff/gr-diff';
import {
- GerritNav,
- GeneratedWebLink,
-} from '../../core/gr-navigation/gr-navigation';
-import {
anyLineTooLong,
getDiffLength,
getLine,
@@ -98,6 +94,7 @@
DELAYED_CANCELLATION,
} from '../../../utils/async-util';
import {subscribe} from '../../lit/subscription-controller';
+import {GeneratedWebLink} from '../../../utils/weblink-util';
const EMPTY_BLAME = 'No blame information for this diff.';
@@ -906,30 +903,13 @@
}
private getEditWeblinks(diff: DiffInfo) {
- if (!this.projectName || !this.commitRange || !this.path) return undefined;
- return GerritNav.getEditWebLinks(
- this.projectName,
- this.commitRange.baseCommit,
- this.path,
- {weblinks: diff?.edit_web_links}
- );
+ return diff?.edit_web_links ?? [];
}
private getFilesWeblinks(diff: DiffInfo) {
- if (!this.projectName || !this.commitRange || !this.path) return undefined;
return {
- meta_a: GerritNav.getFileWebLinks(
- this.projectName,
- this.commitRange.baseCommit,
- this.path,
- {weblinks: diff?.meta_a?.web_links}
- ),
- meta_b: GerritNav.getFileWebLinks(
- this.projectName,
- this.commitRange.commit,
- this.path,
- {weblinks: diff?.meta_b?.web_links}
- ),
+ meta_a: diff?.meta_a?.web_links ?? [],
+ meta_b: diff?.meta_b?.web_links ?? [],
};
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
index ffee004..598819b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
@@ -33,18 +33,15 @@
BasePatchSetNum,
BlameInfo,
CommentRange,
- CommitId,
EDIT,
ImageInfo,
NumericChangeId,
PARENT,
PatchSetNum,
- RepoName,
RevisionPatchSetNum,
UrlEncodedCommentId,
} from '../../../types/common';
import {CoverageType} from '../../../types/types';
-import {GerritNav, WeblinkType} from '../../core/gr-navigation/gr-navigation';
import {GrDiffBuilderImage} from '../../../embed/diff/gr-diff-builder/gr-diff-builder-image';
import {GrDiffHost, LineInfo} from './gr-diff-host';
import {DiffInfo, DiffViewMode, IgnoreWhitespaceType} from '../../../api/diff';
@@ -185,62 +182,6 @@
assert.isTrue(cancelStub.called);
});
- test('reload() loads files weblinks', async () => {
- element.change = createChange();
- const weblinksStub = sinon
- .stub(GerritNav, '_generateWeblinks')
- .returns({name: 'stubb', url: '#s'});
- getDiffRestApiStub.returns(Promise.resolve(createDiff()));
- element.projectName = 'test-project' as RepoName;
- element.path = 'test-path';
- element.commitRange = {
- baseCommit: 'test-base' as CommitId,
- commit: 'test-commit' as CommitId,
- };
- element.patchRange = createPatchRange();
-
- await element.reload();
-
- assert.equal(weblinksStub.callCount, 3);
- assert.deepEqual(weblinksStub.firstCall.args[0], {
- commit: 'test-base' as CommitId,
- file: 'test-path',
- options: {
- weblinks: undefined,
- },
- repo: 'test-project' as RepoName,
- type: WeblinkType.EDIT,
- });
- assert.deepEqual(element.editWeblinks, [
- {
- name: 'stubb',
- url: '#s',
- },
- ]);
- assert.deepEqual(weblinksStub.secondCall.args[0], {
- commit: 'test-base' as CommitId,
- file: 'test-path',
- options: {
- weblinks: undefined,
- },
- repo: 'test-project' as RepoName,
- type: WeblinkType.FILE,
- });
- assert.deepEqual(weblinksStub.thirdCall.args[0], {
- commit: 'test-commit' as CommitId,
- file: 'test-path',
- options: {
- weblinks: undefined,
- },
- repo: 'test-project' as RepoName,
- type: WeblinkType.FILE,
- });
- assert.deepEqual(element.filesWeblinks, {
- meta_a: [{name: 'stubb', url: '#s'}],
- meta_b: [{name: 'stubb', url: '#s'}],
- });
- });
-
test('prefetch getDiff', async () => {
getDiffRestApiStub.returns(Promise.resolve(createDiff()));
element.changeNum = 123 as NumericChangeId;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 464102a..5134d46 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -21,10 +21,7 @@
import '../gr-patch-range-select/gr-patch-range-select';
import '../../change/gr-download-dialog/gr-download-dialog';
import '../../shared/gr-overlay/gr-overlay';
-import {
- GeneratedWebLink,
- GerritNav,
-} from '../../core/gr-navigation/gr-navigation';
+import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {getAppContext} from '../../../services/app-context';
import {
computeAllPatchSets,
@@ -124,6 +121,7 @@
} from '../../../models/views/diff';
import {createChangeUrl} from '../../../models/views/change';
import {createEditUrl} from '../../../models/views/edit';
+import {GeneratedWebLink} from '../../../utils/weblink-util';
const LOADING_BLAME = 'Loading blame...';
const LOADED_BLAME = 'Blame loaded';
diff --git a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
index 6f301a6..d9f88f7 100644
--- a/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
+++ b/polygerrit-ui/app/elements/diff/gr-patch-range-select/gr-patch-range-select.ts
@@ -33,7 +33,6 @@
DropdownItem,
GrDropdownList,
} from '../../shared/gr-dropdown-list/gr-dropdown-list';
-import {GeneratedWebLink} from '../../core/gr-navigation/gr-navigation';
import {EditRevisionInfo} from '../../../types/types';
import {a11yStyles} from '../../../styles/gr-a11y-styles';
import {sharedStyles} from '../../../styles/shared-styles';
@@ -44,6 +43,7 @@
import {resolve} from '../../../models/dependency';
import {ifDefined} from 'lit/directives/if-defined.js';
import {ValueChangedEvent} from '../../../types/events';
+import {GeneratedWebLink} from '../../../utils/weblink-util';
// Maximum length for patch set descriptions.
const PATCH_DESC_MAX_LENGTH = 500;
diff --git a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
index eab6638..d2b9e2d 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-status/gr-change-status.ts
@@ -6,13 +6,13 @@
import '../gr-icon/gr-icon';
import '../gr-tooltip-content/gr-tooltip-content';
import '../../../styles/shared-styles';
-import {GeneratedWebLink} from '../../core/gr-navigation/gr-navigation';
import {ChangeInfo} from '../../../types/common';
import {ParsedChangeInfo} from '../../../types/types';
import {sharedStyles} from '../../../styles/shared-styles';
import {LitElement, PropertyValues, html, css} from 'lit';
import {customElement, property, state} from 'lit/decorators.js';
import {createSearchUrl} from '../../../models/views/search';
+import {GeneratedWebLink} from '../../../utils/weblink-util';
export enum ChangeStates {
ABANDONED = 'Abandoned',
diff --git a/polygerrit-ui/app/test/test-router.ts b/polygerrit-ui/app/test/test-router.ts
index 942a41a..4a6d390 100644
--- a/polygerrit-ui/app/test/test-router.ts
+++ b/polygerrit-ui/app/test/test-router.ts
@@ -9,7 +9,6 @@
() => {
/* noop */
},
- () => [],
() => {
return {};
}
diff --git a/polygerrit-ui/app/utils/weblink-util.ts b/polygerrit-ui/app/utils/weblink-util.ts
new file mode 100644
index 0000000..1e9315c
--- /dev/null
+++ b/polygerrit-ui/app/utils/weblink-util.ts
@@ -0,0 +1,72 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {CommitId, ServerInfo} from '../api/rest-api';
+
+export interface WebLink {
+ name?: string;
+ label: string;
+ url: string;
+}
+
+export interface GeneratedWebLink {
+ name?: string;
+ label?: string;
+ url?: string;
+}
+
+export function getPatchSetWeblink(
+ commit?: CommitId,
+ weblinks?: GeneratedWebLink[],
+ config?: ServerInfo
+): GeneratedWebLink | undefined {
+ if (!commit) return undefined;
+ const name = commit.slice(0, 7);
+ const weblink = getBrowseCommitWeblink(weblinks, config);
+ if (!weblink?.url) return {name};
+ return {name, url: weblink.url};
+}
+
+// visible for testing
+export function getCodeBrowserWeblink(weblinks: GeneratedWebLink[]) {
+ // is an ordered allowed list of web link types that provide direct
+ // links to the commit in the url property.
+ const codeBrowserLinks = ['gitiles', 'browse', 'gitweb'];
+ for (let i = 0; i < codeBrowserLinks.length; i++) {
+ const weblink = weblinks.find(
+ weblink => weblink.name === codeBrowserLinks[i]
+ );
+ if (weblink) return weblink;
+ }
+ return undefined;
+}
+
+// visible for testing
+export function getBrowseCommitWeblink(
+ weblinks?: GeneratedWebLink[],
+ config?: ServerInfo
+): GeneratedWebLink | undefined {
+ if (!weblinks) return undefined;
+
+ // Use primary weblink if configured and exists.
+ const primaryWeblinkName = config?.gerrit?.primary_weblink_name;
+ if (primaryWeblinkName) {
+ const weblink = weblinks.find(link => link.name === primaryWeblinkName);
+ if (weblink) return weblink;
+ }
+
+ return getCodeBrowserWeblink(weblinks);
+}
+
+export function getChangeWeblinks(
+ weblinks?: GeneratedWebLink[],
+ config?: ServerInfo
+): GeneratedWebLink[] {
+ if (!weblinks?.length) return [];
+ const commitWeblink = getBrowseCommitWeblink(weblinks, config);
+ return weblinks.filter(
+ weblink => !commitWeblink?.name || weblink.name !== commitWeblink.name
+ );
+}
diff --git a/polygerrit-ui/app/utils/weblink-util_test.ts b/polygerrit-ui/app/utils/weblink-util_test.ts
new file mode 100644
index 0000000..be97cfd
--- /dev/null
+++ b/polygerrit-ui/app/utils/weblink-util_test.ts
@@ -0,0 +1,66 @@
+/**
+ * @license
+ * Copyright 2022 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {assert} from '@open-wc/testing';
+import '../test/common-test-setup';
+import {createServerInfo, createGerritInfo} from '../test/test-data-generators';
+import {
+ getCodeBrowserWeblink,
+ getBrowseCommitWeblink,
+ getChangeWeblinks,
+} from './weblink-util';
+
+suite('weblink util tests', () => {
+ test('getCodeBrowserWeblink', () => {
+ assert.deepEqual(
+ getCodeBrowserWeblink([
+ {name: 'gitweb'},
+ {name: 'gitiles'},
+ {name: 'browse'},
+ {name: 'test'},
+ ]),
+ {name: 'gitiles'}
+ );
+
+ assert.deepEqual(
+ getCodeBrowserWeblink([{name: 'gitweb'}, {name: 'test'}]),
+ {name: 'gitweb'}
+ );
+ });
+
+ test('getBrowseCommitWeblink', () => {
+ const browserLink = {name: 'browser', url: 'browser/url'};
+ const link = {name: 'gitiles', url: 'test/url'};
+ const weblinks = [browserLink, link];
+ const config = {
+ ...createServerInfo(),
+ gerrit: {...createGerritInfo(), primary_weblink_name: browserLink.name},
+ };
+
+ assert.deepEqual(getBrowseCommitWeblink(weblinks, config), browserLink);
+ assert.deepEqual(getBrowseCommitWeblink(weblinks), link);
+ });
+
+ test('getChangeWeblinks', () => {
+ const link = {name: 'test', url: 'test/url'};
+ const browserLink = {name: 'browser', url: 'browser/url'};
+
+ assert.deepEqual(getChangeWeblinks([link, browserLink])[0], {
+ name: 'test',
+ url: 'test/url',
+ });
+
+ assert.deepEqual(getChangeWeblinks([link])[0], {
+ name: 'test',
+ url: 'test/url',
+ });
+
+ link.url = `https://${link.url}`;
+ assert.deepEqual(getChangeWeblinks([link])[0], {
+ name: 'test',
+ url: 'https://test/url',
+ });
+ });
+});
