Skip to content

Commit a4a121c

Browse files
wxiaoguangwxiaoguang
authored
Fix suggestions for issues (#32380)
1 parent f4d3aae commit a4a121c

File tree

11 files changed

+82
-88
lines changed

11 files changed

+82
-88
lines changed

‎routers/web/repo/issue_suggestions.go

+8-19
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,10 @@ import (
1111
"code.gitea.io/gitea/models/unit"
1212
issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
1313
"code.gitea.io/gitea/modules/optional"
14+
"code.gitea.io/gitea/modules/structs"
1415
"code.gitea.io/gitea/services/context"
1516
)
1617

17-
type issueSuggestion struct {
18-
ID int64 `json:"id"`
19-
Title string `json:"title"`
20-
State string `json:"state"`
21-
PullRequest *struct {
22-
Merged bool `json:"merged"`
23-
Draft bool `json:"draft"`
24-
} `json:"pull_request,omitempty"`
25-
}
26-
2718
// IssueSuggestions returns a list of issue suggestions
2819
func IssueSuggestions(ctx *context.Context) {
2920
keyword := ctx.Req.FormValue("q")
@@ -61,13 +52,14 @@ func IssueSuggestions(ctx *context.Context) {
6152
return
6253
}
6354

64-
suggestions := make([]*issueSuggestion, 0, len(issues))
55+
suggestions := make([]*structs.Issue, 0, len(issues))
6556

6657
for _, issue := range issues {
67-
suggestion := &issueSuggestion{
58+
suggestion := &structs.Issue{
6859
ID: issue.ID,
60+
Index: issue.Index,
6961
Title: issue.Title,
70-
State: string(issue.State()),
62+
State: issue.State(),
7163
}
7264

7365
if issue.IsPull {
@@ -76,12 +68,9 @@ func IssueSuggestions(ctx *context.Context) {
7668
return
7769
}
7870
if issue.PullRequest != nil {
79-
suggestion.PullRequest = &struct {
80-
Merged bool `json:"merged"`
81-
Draft bool `json:"draft"`
82-
}{
83-
Merged: issue.PullRequest.HasMerged,
84-
Draft: issue.PullRequest.IsWorkInProgress(ctx),
71+
suggestion.PullRequest = &structs.PullRequestMeta{
72+
HasMerged: issue.PullRequest.HasMerged,
73+
IsWorkInProgress: issue.PullRequest.IsWorkInProgress(ctx),
8574
}
8675
}
8776
}

‎web_src/js/components/ContextPopup.vue

+4-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {SvgIcon} from '../svg.ts';
33
import {GET} from '../modules/fetch.ts';
44
import {getIssueColor, getIssueIcon} from '../features/issue.ts';
55
import {computed, onMounted, ref} from 'vue';
6+
import type {IssuePathInfo} from '../types.ts';
67
78
const {appSubUrl, i18n} = window.config;
89
@@ -25,19 +26,19 @@ const root = ref<HTMLElement | null>(null);
2526
2627
onMounted(() => {
2728
root.value.addEventListener('ce-load-context-popup', (e: CustomEvent) => {
28-
const data = e.detail;
29+
const data: IssuePathInfo = e.detail;
2930
if (!loading.value && issue.value === null) {
3031
load(data);
3132
}
3233
});
3334
});
3435
35-
async function load(data) {
36+
async function load(issuePathInfo: IssuePathInfo) {
3637
loading.value = true;
3738
i18nErrorMessage.value = null;
3839
3940
try {
40-
const response = await GET(`${appSubUrl}/${data.owner}/${data.repo}/issues/${data.index}/info`); // backend: GetIssueInfo
41+
const response = await GET(`${appSubUrl}/${issuePathInfo.ownerName}/${issuePathInfo.repoName}/issues/${issuePathInfo.indexString}/info`); // backend: GetIssueInfo
4142
const respJson = await response.json();
4243
if (!response.ok) {
4344
i18nErrorMessage.value = respJson.message ?? i18n.network_error;

‎web_src/js/features/comp/TextExpander.ts

+13-23
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,29 @@
11
import {matchEmoji, matchMention, matchIssue} from '../../utils/match.ts';
22
import {emojiString} from '../emoji.ts';
33
import {svg} from '../../svg.ts';
4-
import {parseIssueHref} from '../../utils.ts';
4+
import {parseIssueHref, parseIssueNewHref} from '../../utils.ts';
55
import {createElementFromAttrs, createElementFromHTML} from '../../utils/dom.ts';
66
import {getIssueColor, getIssueIcon} from '../issue.ts';
77
import {debounce} from 'perfect-debounce';
88

99
const debouncedSuggestIssues = debounce((key: string, text: string) => new Promise<{matched:boolean; fragment?: HTMLElement}>(async (resolve) => {
10-
const {owner, repo, index} = parseIssueHref(window.location.href);
11-
const matches = await matchIssue(owner, repo, index, text);
10+
let issuePathInfo = parseIssueHref(window.location.href);
11+
if (!issuePathInfo.ownerName) issuePathInfo = parseIssueNewHref(window.location.href);
12+
if (!issuePathInfo.ownerName) return resolve({matched: false});
13+
14+
const matches = await matchIssue(issuePathInfo.ownerName, issuePathInfo.repoName, issuePathInfo.indexString, text);
1215
if (!matches.length) return resolve({matched: false});
1316

14-
const ul = document.createElement('ul');
15-
ul.classList.add('suggestions');
17+
const ul = createElementFromAttrs('ul', {class: 'suggestions'});
1618
for (const issue of matches) {
17-
const li = createElementFromAttrs('li', {
18-
role: 'option',
19-
'data-value': `${key}${issue.id}`,
20-
class: 'tw-flex tw-gap-2',
21-
});
22-
23-
const icon = svg(getIssueIcon(issue), 16, ['text', getIssueColor(issue)].join(' '));
24-
li.append(createElementFromHTML(icon));
25-
26-
const id = document.createElement('span');
27-
id.textContent = issue.id.toString();
28-
li.append(id);
29-
30-
const nameSpan = document.createElement('span');
31-
nameSpan.textContent = issue.title;
32-
li.append(nameSpan);
33-
19+
const li = createElementFromAttrs(
20+
'li', {role: 'option', class: 'tw-flex tw-gap-2', 'data-value': `${key}${issue.number}`},
21+
createElementFromHTML(svg(getIssueIcon(issue), 16, ['text', getIssueColor(issue)])),
22+
createElementFromAttrs('span', null, `#${issue.number}`),
23+
createElementFromAttrs('span', null, issue.title),
24+
);
3425
ul.append(li);
3526
}
36-
3727
resolve({matched: true, fragment: ul});
3828
}), 100);
3929

‎web_src/js/features/contextpopup.ts

+4-6
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@ export function initContextPopups() {
1010

1111
export function attachRefIssueContextPopup(refIssues) {
1212
for (const refIssue of refIssues) {
13-
if (refIssue.classList.contains('ref-external-issue')) {
14-
return;
15-
}
13+
if (refIssue.classList.contains('ref-external-issue')) continue;
1614

17-
const {owner, repo, index} = parseIssueHref(refIssue.getAttribute('href'));
18-
if (!owner) return;
15+
const issuePathInfo = parseIssueHref(refIssue.getAttribute('href'));
16+
if (!issuePathInfo.ownerName) continue;
1917

2018
const el = document.createElement('div');
2119
el.classList.add('tw-p-3');
@@ -38,7 +36,7 @@ export function attachRefIssueContextPopup(refIssues) {
3836
role: 'dialog',
3937
interactiveBorder: 5,
4038
onShow: () => {
41-
el.firstChild.dispatchEvent(new CustomEvent('ce-load-context-popup', {detail: {owner, repo, index}}));
39+
el.firstChild.dispatchEvent(new CustomEvent('ce-load-context-popup', {detail: issuePathInfo}));
4240
},
4341
});
4442
}

‎web_src/js/svg.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ export type SvgName = keyof typeof svgs;
153153
// most of the SVG icons in assets couldn't be used directly.
154154

155155
// retrieve an HTML string for given SVG icon name, size and additional classes
156-
export function svg(name: SvgName, size = 16, className = '') {
156+
export function svg(name: SvgName, size = 16, classNames: string|string[]): string {
157+
const className = Array.isArray(classNames) ? classNames.join(' ') : classNames;
157158
if (!(name in svgs)) throw new Error(`Unknown SVG icon: ${name}`);
158159
if (size === 16 && !className) return svgs[name];
159160

‎web_src/js/types.ts

+6-5
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,16 @@ export type RequestOpts = {
3030
data?: RequestData,
3131
} & RequestInit;
3232

33-
export type IssueData = {
34-
owner: string,
35-
repo: string,
36-
type: string,
37-
index: string,
33+
export type IssuePathInfo = {
34+
ownerName: string,
35+
repoName: string,
36+
pathType: string,
37+
indexString?: string,
3838
}
3939

4040
export type Issue = {
4141
id: number;
42+
number: number;
4243
title: string;
4344
state: 'open' | 'closed';
4445
pull_request?: {

‎web_src/js/utils.test.ts

+22-16
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {
22
basename, extname, isObject, stripTags, parseIssueHref,
33
parseUrl, translateMonth, translateDay, blobToDataURI,
4-
toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile,
4+
toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, parseIssueNewHref,
55
} from './utils.ts';
66

77
test('basename', () => {
@@ -28,21 +28,27 @@ test('stripTags', () => {
2828
});
2929

3030
test('parseIssueHref', () => {
31-
expect(parseIssueHref('/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
32-
expect(parseIssueHref('/owner/repo/pulls/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'});
33-
expect(parseIssueHref('/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
34-
expect(parseIssueHref('/sub/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
35-
expect(parseIssueHref('/sub/sub2/owner/repo/pulls/1')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'});
36-
expect(parseIssueHref('/sub/sub2/owner/repo/issues/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
37-
expect(parseIssueHref('/sub/sub2/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
38-
expect(parseIssueHref('https://example.com/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
39-
expect(parseIssueHref('https://example.com/owner/repo/pulls/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'});
40-
expect(parseIssueHref('https://example.com/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
41-
expect(parseIssueHref('https://example.com/sub/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
42-
expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/pulls/1')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'});
43-
expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
44-
expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
45-
expect(parseIssueHref('')).toEqual({owner: undefined, repo: undefined, type: undefined, index: undefined});
31+
expect(parseIssueHref('/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
32+
expect(parseIssueHref('/owner/repo/pulls/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'});
33+
expect(parseIssueHref('/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
34+
expect(parseIssueHref('/sub/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
35+
expect(parseIssueHref('/sub/sub2/owner/repo/pulls/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'});
36+
expect(parseIssueHref('/sub/sub2/owner/repo/issues/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
37+
expect(parseIssueHref('/sub/sub2/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
38+
expect(parseIssueHref('https://example.com/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
39+
expect(parseIssueHref('https://example.com/owner/repo/pulls/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'});
40+
expect(parseIssueHref('https://example.com/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
41+
expect(parseIssueHref('https://example.com/sub/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
42+
expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/pulls/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'});
43+
expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
44+
expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'});
45+
expect(parseIssueHref('')).toEqual({ownerName: undefined, repoName: undefined, type: undefined, index: undefined});
46+
});
47+
48+
test('parseIssueNewHref', () => {
49+
expect(parseIssueNewHref('/owner/repo/issues/new')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'});
50+
expect(parseIssueNewHref('/owner/repo/issues/new?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'});
51+
expect(parseIssueNewHref('/sub/owner/repo/issues/new#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'});
4652
});
4753

4854
test('parseUrl', () => {

‎web_src/js/utils.ts

+10-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {encode, decode} from 'uint8-to-base64';
2-
import type {IssueData} from './types.ts';
2+
import type {IssuePathInfo} from './types.ts';
33

44
// transform /path/to/file.ext to file.ext
55
export function basename(path: string): string {
@@ -31,10 +31,16 @@ export function stripTags(text: string): string {
3131
return text.replace(/<[^>]*>?/g, '');
3232
}
3333

34-
export function parseIssueHref(href: string): IssueData {
34+
export function parseIssueHref(href: string): IssuePathInfo {
3535
const path = (href || '').replace(/[#?].*$/, '');
36-
const [_, owner, repo, type, index] = /([^/]+)\/([^/]+)\/(issues|pulls)\/([0-9]+)/.exec(path) || [];
37-
return {owner, repo, type, index};
36+
const [_, ownerName, repoName, pathType, indexString] = /([^/]+)\/([^/]+)\/(issues|pulls)\/([0-9]+)/.exec(path) || [];
37+
return {ownerName, repoName, pathType, indexString};
38+
}
39+
40+
export function parseIssueNewHref(href: string): IssuePathInfo {
41+
const path = (href || '').replace(/[#?].*$/, '');
42+
const [_, ownerName, repoName, pathType, indexString] = /([^/]+)\/([^/]+)\/(issues|pulls)\/new/.exec(path) || [];
43+
return {ownerName, repoName, pathType, indexString};
3844
}
3945

4046
// parse a URL, either relative '/path' or absolute 'https://localhost/path'

‎web_src/js/utils/dom.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ test('createElementFromAttrs', () => {
88
const el = createElementFromAttrs('button', {
99
id: 'the-id',
1010
class: 'cls-1 cls-2',
11-
'data-foo': 'the-data',
1211
disabled: true,
1312
checked: false,
1413
required: null,
1514
tabindex: 0,
16-
});
17-
expect(el.outerHTML).toEqual('<button id="the-id" class="cls-1 cls-2" data-foo="the-data" disabled="" tabindex="0"></button>');
15+
'data-foo': 'the-data',
16+
}, 'txt', createElementFromHTML('<span>inner</span>'));
17+
expect(el.outerHTML).toEqual('<button id="the-id" class="cls-1 cls-2" disabled="" tabindex="0" data-foo="the-data">txt<span>inner</span></button>');
1818
});

‎web_src/js/utils/dom.ts

+7-5
Original file line numberDiff line numberDiff line change
@@ -298,22 +298,24 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st
298298
}
299299

300300
// Warning: Do not enter any unsanitized variables here
301-
export function createElementFromHTML(htmlString: string) {
301+
export function createElementFromHTML(htmlString: string): HTMLElement {
302302
const div = document.createElement('div');
303303
div.innerHTML = htmlString.trim();
304-
return div.firstChild as Element;
304+
return div.firstChild as HTMLElement;
305305
}
306306

307-
export function createElementFromAttrs(tagName: string, attrs: Record<string, any>) {
307+
export function createElementFromAttrs(tagName: string, attrs: Record<string, any>, ...children: (Node|string)[]): HTMLElement {
308308
const el = document.createElement(tagName);
309-
for (const [key, value] of Object.entries(attrs)) {
309+
for (const [key, value] of Object.entries(attrs || {})) {
310310
if (value === undefined || value === null) continue;
311311
if (typeof value === 'boolean') {
312312
el.toggleAttribute(key, value);
313313
} else {
314314
el.setAttribute(key, String(value));
315315
}
316-
// TODO: in the future we could make it also support "textContent" and "innerHTML" properties if needed
316+
}
317+
for (const child of children) {
318+
el.append(child instanceof Node ? child : document.createTextNode(child));
317319
}
318320
return el;
319321
}

‎web_src/js/utils/match.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import emojis from '../../../assets/emoji.json';
2-
import type {Issue} from '../features/issue.ts';
32
import {GET} from '../modules/fetch.ts';
3+
import type {Issue} from '../features/issue.ts';
44

55
const maxMatches = 6;
66

@@ -49,8 +49,8 @@ export async function matchIssue(owner: string, repo: string, issueIndexStr: str
4949
const res = await GET(`${window.config.appSubUrl}/${owner}/${repo}/issues/suggestions?q=${encodeURIComponent(query)}`);
5050

5151
const issues: Issue[] = await res.json();
52-
const issueIndex = parseInt(issueIndexStr);
52+
const issueNumber = parseInt(issueIndexStr);
5353

5454
// filter out issue with same id
55-
return issues.filter((i) => i.id !== issueIndex);
55+
return issues.filter((i) => i.number !== issueNumber);
5656
}

0 commit comments

Comments
 (0)