From a4a121c684fdbbad19971ff658166fd47932b192 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 31 Oct 2024 04:06:36 +0800 Subject: [PATCH] Fix suggestions for issues (#32380) --- routers/web/repo/issue_suggestions.go | 27 +++++------------ web_src/js/components/ContextPopup.vue | 7 +++-- web_src/js/features/comp/TextExpander.ts | 36 ++++++++-------------- web_src/js/features/contextpopup.ts | 10 +++---- web_src/js/svg.ts | 3 +- web_src/js/types.ts | 11 +++---- web_src/js/utils.test.ts | 38 ++++++++++++++---------- web_src/js/utils.ts | 14 ++++++--- web_src/js/utils/dom.test.ts | 6 ++-- web_src/js/utils/dom.ts | 12 ++++---- web_src/js/utils/match.ts | 6 ++-- 11 files changed, 82 insertions(+), 88 deletions(-) diff --git a/routers/web/repo/issue_suggestions.go b/routers/web/repo/issue_suggestions.go index 361da0ee60..46e9f339a5 100644 --- a/routers/web/repo/issue_suggestions.go +++ b/routers/web/repo/issue_suggestions.go @@ -11,19 +11,10 @@ import ( "code.gitea.io/gitea/models/unit" issue_indexer "code.gitea.io/gitea/modules/indexer/issues" "code.gitea.io/gitea/modules/optional" + "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/services/context" ) -type issueSuggestion struct { - ID int64 `json:"id"` - Title string `json:"title"` - State string `json:"state"` - PullRequest *struct { - Merged bool `json:"merged"` - Draft bool `json:"draft"` - } `json:"pull_request,omitempty"` -} - // IssueSuggestions returns a list of issue suggestions func IssueSuggestions(ctx *context.Context) { keyword := ctx.Req.FormValue("q") @@ -61,13 +52,14 @@ func IssueSuggestions(ctx *context.Context) { return } - suggestions := make([]*issueSuggestion, 0, len(issues)) + suggestions := make([]*structs.Issue, 0, len(issues)) for _, issue := range issues { - suggestion := &issueSuggestion{ + suggestion := &structs.Issue{ ID: issue.ID, + Index: issue.Index, Title: issue.Title, - State: string(issue.State()), + State: issue.State(), } if issue.IsPull { @@ -76,12 +68,9 @@ func IssueSuggestions(ctx *context.Context) { return } if issue.PullRequest != nil { - suggestion.PullRequest = &struct { - Merged bool `json:"merged"` - Draft bool `json:"draft"` - }{ - Merged: issue.PullRequest.HasMerged, - Draft: issue.PullRequest.IsWorkInProgress(ctx), + suggestion.PullRequest = &structs.PullRequestMeta{ + HasMerged: issue.PullRequest.HasMerged, + IsWorkInProgress: issue.PullRequest.IsWorkInProgress(ctx), } } } diff --git a/web_src/js/components/ContextPopup.vue b/web_src/js/components/ContextPopup.vue index 8c56af858e..b0e8447302 100644 --- a/web_src/js/components/ContextPopup.vue +++ b/web_src/js/components/ContextPopup.vue @@ -3,6 +3,7 @@ import {SvgIcon} from '../svg.ts'; import {GET} from '../modules/fetch.ts'; import {getIssueColor, getIssueIcon} from '../features/issue.ts'; import {computed, onMounted, ref} from 'vue'; +import type {IssuePathInfo} from '../types.ts'; const {appSubUrl, i18n} = window.config; @@ -25,19 +26,19 @@ const root = ref(null); onMounted(() => { root.value.addEventListener('ce-load-context-popup', (e: CustomEvent) => { - const data = e.detail; + const data: IssuePathInfo = e.detail; if (!loading.value && issue.value === null) { load(data); } }); }); -async function load(data) { +async function load(issuePathInfo: IssuePathInfo) { loading.value = true; i18nErrorMessage.value = null; try { - const response = await GET(`${appSubUrl}/${data.owner}/${data.repo}/issues/${data.index}/info`); // backend: GetIssueInfo + const response = await GET(`${appSubUrl}/${issuePathInfo.ownerName}/${issuePathInfo.repoName}/issues/${issuePathInfo.indexString}/info`); // backend: GetIssueInfo const respJson = await response.json(); if (!response.ok) { i18nErrorMessage.value = respJson.message ?? i18n.network_error; diff --git a/web_src/js/features/comp/TextExpander.ts b/web_src/js/features/comp/TextExpander.ts index 6ac4c4bf32..e0c4abed75 100644 --- a/web_src/js/features/comp/TextExpander.ts +++ b/web_src/js/features/comp/TextExpander.ts @@ -1,39 +1,29 @@ import {matchEmoji, matchMention, matchIssue} from '../../utils/match.ts'; import {emojiString} from '../emoji.ts'; import {svg} from '../../svg.ts'; -import {parseIssueHref} from '../../utils.ts'; +import {parseIssueHref, parseIssueNewHref} from '../../utils.ts'; import {createElementFromAttrs, createElementFromHTML} from '../../utils/dom.ts'; import {getIssueColor, getIssueIcon} from '../issue.ts'; import {debounce} from 'perfect-debounce'; const debouncedSuggestIssues = debounce((key: string, text: string) => new Promise<{matched:boolean; fragment?: HTMLElement}>(async (resolve) => { - const {owner, repo, index} = parseIssueHref(window.location.href); - const matches = await matchIssue(owner, repo, index, text); + let issuePathInfo = parseIssueHref(window.location.href); + if (!issuePathInfo.ownerName) issuePathInfo = parseIssueNewHref(window.location.href); + if (!issuePathInfo.ownerName) return resolve({matched: false}); + + const matches = await matchIssue(issuePathInfo.ownerName, issuePathInfo.repoName, issuePathInfo.indexString, text); if (!matches.length) return resolve({matched: false}); - const ul = document.createElement('ul'); - ul.classList.add('suggestions'); + const ul = createElementFromAttrs('ul', {class: 'suggestions'}); for (const issue of matches) { - const li = createElementFromAttrs('li', { - role: 'option', - 'data-value': `${key}${issue.id}`, - class: 'tw-flex tw-gap-2', - }); - - const icon = svg(getIssueIcon(issue), 16, ['text', getIssueColor(issue)].join(' ')); - li.append(createElementFromHTML(icon)); - - const id = document.createElement('span'); - id.textContent = issue.id.toString(); - li.append(id); - - const nameSpan = document.createElement('span'); - nameSpan.textContent = issue.title; - li.append(nameSpan); - + const li = createElementFromAttrs( + 'li', {role: 'option', class: 'tw-flex tw-gap-2', 'data-value': `${key}${issue.number}`}, + createElementFromHTML(svg(getIssueIcon(issue), 16, ['text', getIssueColor(issue)])), + createElementFromAttrs('span', null, `#${issue.number}`), + createElementFromAttrs('span', null, issue.title), + ); ul.append(li); } - resolve({matched: true, fragment: ul}); }), 100); diff --git a/web_src/js/features/contextpopup.ts b/web_src/js/features/contextpopup.ts index 5af7d176b6..33eead8431 100644 --- a/web_src/js/features/contextpopup.ts +++ b/web_src/js/features/contextpopup.ts @@ -10,12 +10,10 @@ export function initContextPopups() { export function attachRefIssueContextPopup(refIssues) { for (const refIssue of refIssues) { - if (refIssue.classList.contains('ref-external-issue')) { - return; - } + if (refIssue.classList.contains('ref-external-issue')) continue; - const {owner, repo, index} = parseIssueHref(refIssue.getAttribute('href')); - if (!owner) return; + const issuePathInfo = parseIssueHref(refIssue.getAttribute('href')); + if (!issuePathInfo.ownerName) continue; const el = document.createElement('div'); el.classList.add('tw-p-3'); @@ -38,7 +36,7 @@ export function attachRefIssueContextPopup(refIssues) { role: 'dialog', interactiveBorder: 5, onShow: () => { - el.firstChild.dispatchEvent(new CustomEvent('ce-load-context-popup', {detail: {owner, repo, index}})); + el.firstChild.dispatchEvent(new CustomEvent('ce-load-context-popup', {detail: issuePathInfo})); }, }); } diff --git a/web_src/js/svg.ts b/web_src/js/svg.ts index 6a4bfafc92..6227a85e33 100644 --- a/web_src/js/svg.ts +++ b/web_src/js/svg.ts @@ -153,7 +153,8 @@ export type SvgName = keyof typeof svgs; // most of the SVG icons in assets couldn't be used directly. // retrieve an HTML string for given SVG icon name, size and additional classes -export function svg(name: SvgName, size = 16, className = '') { +export function svg(name: SvgName, size = 16, classNames: string|string[]): string { + const className = Array.isArray(classNames) ? classNames.join(' ') : classNames; if (!(name in svgs)) throw new Error(`Unknown SVG icon: ${name}`); if (size === 16 && !className) return svgs[name]; diff --git a/web_src/js/types.ts b/web_src/js/types.ts index c38c8bda96..9c601456bd 100644 --- a/web_src/js/types.ts +++ b/web_src/js/types.ts @@ -30,15 +30,16 @@ export type RequestOpts = { data?: RequestData, } & RequestInit; -export type IssueData = { - owner: string, - repo: string, - type: string, - index: string, +export type IssuePathInfo = { + ownerName: string, + repoName: string, + pathType: string, + indexString?: string, } export type Issue = { id: number; + number: number; title: string; state: 'open' | 'closed'; pull_request?: { diff --git a/web_src/js/utils.test.ts b/web_src/js/utils.test.ts index 55896706ff..647676bf20 100644 --- a/web_src/js/utils.test.ts +++ b/web_src/js/utils.test.ts @@ -1,7 +1,7 @@ import { basename, extname, isObject, stripTags, parseIssueHref, parseUrl, translateMonth, translateDay, blobToDataURI, - toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, + toAbsoluteUrl, encodeURLEncodedBase64, decodeURLEncodedBase64, isImageFile, isVideoFile, parseIssueNewHref, } from './utils.ts'; test('basename', () => { @@ -28,21 +28,27 @@ test('stripTags', () => { }); test('parseIssueHref', () => { - expect(parseIssueHref('/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('/owner/repo/pulls/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'}); - expect(parseIssueHref('/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('/sub/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('/sub/sub2/owner/repo/pulls/1')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'}); - expect(parseIssueHref('/sub/sub2/owner/repo/issues/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('/sub/sub2/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('https://example.com/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('https://example.com/owner/repo/pulls/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'}); - expect(parseIssueHref('https://example.com/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('https://example.com/sub/owner/repo/issues/1')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/pulls/1')).toEqual({owner: 'owner', repo: 'repo', type: 'pulls', index: '1'}); - expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1?query')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'}); - expect(parseIssueHref('')).toEqual({owner: undefined, repo: undefined, type: undefined, index: undefined}); + expect(parseIssueHref('/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('/owner/repo/pulls/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'}); + expect(parseIssueHref('/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('/sub/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('/sub/sub2/owner/repo/pulls/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'}); + expect(parseIssueHref('/sub/sub2/owner/repo/issues/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('/sub/sub2/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('https://example.com/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('https://example.com/owner/repo/pulls/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'}); + expect(parseIssueHref('https://example.com/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('https://example.com/sub/owner/repo/issues/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/pulls/1')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'pulls', indexString: '1'}); + expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues', indexString: '1'}); + expect(parseIssueHref('')).toEqual({ownerName: undefined, repoName: undefined, type: undefined, index: undefined}); +}); + +test('parseIssueNewHref', () => { + expect(parseIssueNewHref('/owner/repo/issues/new')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'}); + expect(parseIssueNewHref('/owner/repo/issues/new?query')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'}); + expect(parseIssueNewHref('/sub/owner/repo/issues/new#hash')).toEqual({ownerName: 'owner', repoName: 'repo', pathType: 'issues'}); }); test('parseUrl', () => { diff --git a/web_src/js/utils.ts b/web_src/js/utils.ts index c52bf500d4..066a7c7b54 100644 --- a/web_src/js/utils.ts +++ b/web_src/js/utils.ts @@ -1,5 +1,5 @@ import {encode, decode} from 'uint8-to-base64'; -import type {IssueData} from './types.ts'; +import type {IssuePathInfo} from './types.ts'; // transform /path/to/file.ext to file.ext export function basename(path: string): string { @@ -31,10 +31,16 @@ export function stripTags(text: string): string { return text.replace(/<[^>]*>?/g, ''); } -export function parseIssueHref(href: string): IssueData { +export function parseIssueHref(href: string): IssuePathInfo { const path = (href || '').replace(/[#?].*$/, ''); - const [_, owner, repo, type, index] = /([^/]+)\/([^/]+)\/(issues|pulls)\/([0-9]+)/.exec(path) || []; - return {owner, repo, type, index}; + const [_, ownerName, repoName, pathType, indexString] = /([^/]+)\/([^/]+)\/(issues|pulls)\/([0-9]+)/.exec(path) || []; + return {ownerName, repoName, pathType, indexString}; +} + +export function parseIssueNewHref(href: string): IssuePathInfo { + const path = (href || '').replace(/[#?].*$/, ''); + const [_, ownerName, repoName, pathType, indexString] = /([^/]+)\/([^/]+)\/(issues|pulls)\/new/.exec(path) || []; + return {ownerName, repoName, pathType, indexString}; } // parse a URL, either relative '/path' or absolute 'https://localhost/path' diff --git a/web_src/js/utils/dom.test.ts b/web_src/js/utils/dom.test.ts index 13df82d9b4..5c235795fd 100644 --- a/web_src/js/utils/dom.test.ts +++ b/web_src/js/utils/dom.test.ts @@ -8,11 +8,11 @@ test('createElementFromAttrs', () => { const el = createElementFromAttrs('button', { id: 'the-id', class: 'cls-1 cls-2', - 'data-foo': 'the-data', disabled: true, checked: false, required: null, tabindex: 0, - }); - expect(el.outerHTML).toEqual(''); + 'data-foo': 'the-data', + }, 'txt', createElementFromHTML('inner')); + expect(el.outerHTML).toEqual(''); }); diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index 7dd63ecbbf..5b118b991d 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -298,22 +298,24 @@ export function replaceTextareaSelection(textarea: HTMLTextAreaElement, text: st } // Warning: Do not enter any unsanitized variables here -export function createElementFromHTML(htmlString: string) { +export function createElementFromHTML(htmlString: string): HTMLElement { const div = document.createElement('div'); div.innerHTML = htmlString.trim(); - return div.firstChild as Element; + return div.firstChild as HTMLElement; } -export function createElementFromAttrs(tagName: string, attrs: Record) { +export function createElementFromAttrs(tagName: string, attrs: Record, ...children: (Node|string)[]): HTMLElement { const el = document.createElement(tagName); - for (const [key, value] of Object.entries(attrs)) { + for (const [key, value] of Object.entries(attrs || {})) { if (value === undefined || value === null) continue; if (typeof value === 'boolean') { el.toggleAttribute(key, value); } else { el.setAttribute(key, String(value)); } - // TODO: in the future we could make it also support "textContent" and "innerHTML" properties if needed + } + for (const child of children) { + el.append(child instanceof Node ? child : document.createTextNode(child)); } return el; } diff --git a/web_src/js/utils/match.ts b/web_src/js/utils/match.ts index 2c7271f16e..1161ea6250 100644 --- a/web_src/js/utils/match.ts +++ b/web_src/js/utils/match.ts @@ -1,6 +1,6 @@ import emojis from '../../../assets/emoji.json'; -import type {Issue} from '../features/issue.ts'; import {GET} from '../modules/fetch.ts'; +import type {Issue} from '../features/issue.ts'; const maxMatches = 6; @@ -49,8 +49,8 @@ export async function matchIssue(owner: string, repo: string, issueIndexStr: str const res = await GET(`${window.config.appSubUrl}/${owner}/${repo}/issues/suggestions?q=${encodeURIComponent(query)}`); const issues: Issue[] = await res.json(); - const issueIndex = parseInt(issueIndexStr); + const issueNumber = parseInt(issueIndexStr); // filter out issue with same id - return issues.filter((i) => i.id !== issueIndex); + return issues.filter((i) => i.number !== issueNumber); }