Fix suggestions for issues (#32380)

diff --git a/routers/web/repo/issue_suggestions.go b/routers/web/repo/issue_suggestions.go
index 361da0ee609532fe3b5ca7b30ae08c0fb1edffb1..46e9f339a5a539354fadfbd2bdeeab040ceedb61 100644 (file)
--- 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 8c56af858e33f9e21bd54392989d1c2222102651..b0e8447302ffe154018a1461202c651e3d4880f5 100644 (file)
--- 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<HTMLElement | null>(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 6ac4c4bf32c19a1c0ae14af198a48ba92df6d762..e0c4abed75b804c7d5c130dff33341f6b6b39c05 100644 (file)
--- 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 5af7d176b6fb040e64d51392577e7c8ab51adb5e..33eead8431c5f30e8c5e2cfd491d17d300f14a4d 100644 (file)
--- 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 6a4bfafc9244cd75f09d74dc64bc7e8ed5f69607..6227a85e33eeaf8ff665b8f37d1099343b926fae 100644 (file)
--- 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 c38c8bda96f0c1c2a940592015b4d72be0fccf4e..9c601456bd9f169160ed87fb3f8aba1b89300738 100644 (file)
--- 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 55896706fff469ebdc132c15b70314eb49ef5bdd..647676bf20562af3a4cc7c111fbf1a00438a47a2 100644 (file)
--- 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 c52bf500d439be862a4f655873a4fa75ee64253f..066a7c7b5460e8b376baa0ce1a2e6fe53a93969a 100644 (file)
--- 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 13df82d9b49f9b390a15abc3bc22c61c3704c129..5c235795fd45482a3eeb4edbf86fe615a73a0c4a 100644 (file)
--- 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('<button id="the-id" class="cls-1 cls-2" data-foo="the-data" disabled="" tabindex="0"></button>');
+    'data-foo': 'the-data',
+  }, 'txt', createElementFromHTML('<span>inner</span>'));
+  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>');
 });

diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts
index 7dd63ecbbf01a44da67bee150e7d2a17e56342d7..5b118b991d564bc2d4606e3633cb427cd51fc166 100644 (file)
--- 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<string, any>) {
+export function createElementFromAttrs(tagName: string, attrs: Record<string, any>, ...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 2c7271f16eee5a439d65c79fe689a95d8114b86a..1161ea6250b8f52f62d9ef4e49d8337f86f84c5a 100644 (file)
--- 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);
 }