1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-10-10 19:32:02 +00:00

fix: use another method for suggestions

This commit is contained in:
Maxim Slipenko 2025-06-07 18:21:41 +03:00
parent 3d4372c8bf
commit 6e2c72cd70
9 changed files with 122 additions and 102 deletions

View file

@ -539,28 +539,6 @@ func FindLatestUpdatedIssues(ctx context.Context, repoID int64, isPull optional.
return issues, err return issues, err
} }
func FindIssuesSuggestionByKeyword(ctx context.Context, repoID int64, keyword string, isPull optional.Option[bool], excludedID int64, pageSize int) (IssueList, error) {
cond := builder.NewCond()
if excludedID > 0 {
cond = cond.And(builder.Neq{"`id`": excludedID})
}
// It seems that GitHub searches both title and content (maybe sorting by the search engine's ranking system?)
// The first PR (https://github.com/go-gitea/gitea/pull/32327) uses "search indexer" to search "name(title) + content"
// But it seems that searching "content" (especially LIKE by DB engine) generates worse (unusable) results.
// So now (https://github.com/go-gitea/gitea/pull/33538) it only searches "name(title)", leave the improvements to the future.
cond = cond.And(db.BuildCaseInsensitiveLike("`name`", keyword))
issues := make([]*Issue, 0, pageSize)
err := db.GetEngine(ctx).Where("repo_id = ?", repoID).
And(isPullToCond(isPull)).
And(cond).
OrderBy("updated_unix DESC, `index` DESC").
Limit(pageSize).
Find(&issues)
return issues, err
}
// GetIssueWithAttrsByIndex returns issue by index in a repository. // GetIssueWithAttrsByIndex returns issue by index in a repository.
func GetIssueWithAttrsByIndex(ctx context.Context, repoID, index int64) (*Issue, error) { func GetIssueWithAttrsByIndex(ctx context.Context, repoID, index int64) (*Issue, error) {
issue, err := GetIssueByIndex(ctx, repoID, index) issue, err := GetIssueByIndex(ctx, repoID, index)

View file

@ -14,8 +14,6 @@ import (
// IssueSuggestions returns a list of issue suggestions // IssueSuggestions returns a list of issue suggestions
func IssueSuggestions(ctx *context.Context) { func IssueSuggestions(ctx *context.Context) {
keyword := ctx.Req.FormValue("q")
canReadIssues := ctx.Repo.CanRead(unit.TypeIssues) canReadIssues := ctx.Repo.CanRead(unit.TypeIssues)
canReadPulls := ctx.Repo.CanRead(unit.TypePullRequests) canReadPulls := ctx.Repo.CanRead(unit.TypePullRequests)
@ -26,7 +24,7 @@ func IssueSuggestions(ctx *context.Context) {
isPull = optional.Some(false) isPull = optional.Some(false)
} }
suggestions, err := issue_service.GetSuggestion(ctx, ctx.Repo.Repository, isPull, keyword) suggestions, err := issue_service.GetSuggestion(ctx, ctx.Repo.Repository, isPull)
if err != nil { if err != nil {
ctx.ServerError("GetSuggestion", err) ctx.ServerError("GetSuggestion", err)
return return

View file

@ -5,7 +5,6 @@ package issue
import ( import (
"context" "context"
"strconv"
issues_model "forgejo.org/models/issues" issues_model "forgejo.org/models/issues"
repo_model "forgejo.org/models/repo" repo_model "forgejo.org/models/repo"
@ -13,38 +12,14 @@ import (
"forgejo.org/modules/structs" "forgejo.org/modules/structs"
) )
func GetSuggestion(ctx context.Context, repo *repo_model.Repository, isPull optional.Option[bool], keyword string) ([]*structs.Issue, error) { func GetSuggestion(ctx context.Context, repo *repo_model.Repository, isPull optional.Option[bool]) ([]*structs.Issue, error) {
var issues issues_model.IssueList var issues issues_model.IssueList
var err error var err error
pageSize := 5 pageSize := 1000
if keyword == "" {
issues, err = issues_model.FindLatestUpdatedIssues(ctx, repo.ID, isPull, pageSize)
if err != nil {
return nil, err
}
} else {
indexKeyword, _ := strconv.ParseInt(keyword, 10, 64)
var issueByIndex *issues_model.Issue
var excludedID int64
if indexKeyword > 0 {
issueByIndex, err = issues_model.GetIssueByIndex(ctx, repo.ID, indexKeyword)
if err != nil && !issues_model.IsErrIssueNotExist(err) {
return nil, err
}
if issueByIndex != nil {
excludedID = issueByIndex.ID
pageSize--
}
}
issues, err = issues_model.FindIssuesSuggestionByKeyword(ctx, repo.ID, keyword, isPull, excludedID, pageSize) issues, err = issues_model.FindLatestUpdatedIssues(ctx, repo.ID, isPull, pageSize)
if err != nil { if err != nil {
return nil, err return nil, err
}
if issueByIndex != nil {
issues = append([]*issues_model.Issue{issueByIndex}, issues...)
}
} }
if err := issues.LoadPullRequests(ctx); err != nil { if err := issues.LoadPullRequests(ctx); err != nil {

View file

@ -22,31 +22,19 @@ func Test_Suggestion(t *testing.T) {
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
testCases := []struct { testCases := []struct {
keyword string name string
isPull optional.Option[bool] isPull optional.Option[bool]
expectedIndexes []int64 expectedIndexes []int64
}{ }{
{ {
keyword: "", name: "All",
expectedIndexes: []int64{5, 1, 4, 2, 3}, expectedIndexes: []int64{5, 1, 4, 2, 3},
}, },
{
keyword: "1",
expectedIndexes: []int64{1},
},
{
keyword: "issue",
expectedIndexes: []int64{4, 1, 2, 3},
},
{
keyword: "pull",
expectedIndexes: []int64{5},
},
} }
for _, testCase := range testCases { for _, testCase := range testCases {
t.Run(testCase.keyword, func(t *testing.T) { t.Run(testCase.name, func(t *testing.T) {
issues, err := GetSuggestion(db.DefaultContext, repo1, testCase.isPull, testCase.keyword) issues, err := GetSuggestion(db.DefaultContext, repo1, testCase.isPull)
require.NoError(t, err) require.NoError(t, err)
issueIndexes := make([]int64, 0, len(issues)) issueIndexes := make([]int64, 0, len(issues))

View file

@ -1,30 +1,29 @@
import {matchEmoji, matchMention, matchIssue} from '../../utils/match.js'; import {matchEmoji, matchMention, matchIssue} from '../../utils/match.js';
import {emojiString} from '../emoji.js'; import {emojiString} from '../emoji.js';
import {getIssueIcon, getIssueColor} from '../issue.js' import {getIssueIcon, getIssueColor,isIssueSuggestionsLoaded, fetchIssueSuggestions} from '../issue.js'
import {parseIssueHref} from '../../utils.js'
import {svg} from '../../svg.js' import {svg} from '../../svg.js'
import {createElementFromHTML} from '../../utils/dom.js'; import {createElementFromHTML} from '../../utils/dom.js';
import {debounce} from 'perfect-debounce'; import { GET } from '../../modules/fetch.js';
const debouncedSuggestIssues = debounce((key, text) => new Promise( async function issueSuggestions(text) {
async (resolve, reject) => { const key = '#';
const {owner, repo, index} = parseIssueHref(window.location.href);
const matches = await matchIssue(owner, repo, index, text); const matches = matchIssue(text);
if (!matches.length) return resolve({matched: false}); if (!matches.length) return {matched: false};
const ul = document.createElement('ul'); const ul = document.createElement('ul');
ul.classList.add('suggestions'); ul.classList.add('suggestions');
for (const issue of matches) { for (const issue of matches) {
const li = document.createElement('li'); const li = document.createElement('li');
li.setAttribute('role', 'option'); li.setAttribute('role', 'option');
li.setAttribute('data-value', `${key}${issue.id}`); li.setAttribute('data-value', `${key}${issue.number}`);
li.classList.add('tw-flex', 'tw-gap-2') li.classList.add('tw-flex', 'tw-gap-2')
const icon = svg(getIssueIcon(issue), 16, ['text', getIssueColor(issue)].join(' ')); const icon = svg(getIssueIcon(issue), 16, ['text', getIssueColor(issue)].join(' '));
li.append(createElementFromHTML(icon)); li.append(createElementFromHTML(icon));
const id = document.createElement('span'); const id = document.createElement('span');
id.textContent = issue.id.toString(); id.textContent = issue.number.toString();
li.append(id); li.append(id);
const nameSpan = document.createElement('span'); const nameSpan = document.createElement('span');
@ -34,10 +33,14 @@ const debouncedSuggestIssues = debounce((key, text) => new Promise(
ul.append(li); ul.append(li);
} }
resolve({matched: true, fragment: ul}); return {matched: true, fragment: ul};
}), 100) }
export function initTextExpander(expander) { export function initTextExpander(expander) {
if (!expander) return;
const textarea = expander.querySelector('textarea');
expander?.addEventListener('text-expander-change', ({detail: {key, provide, text}}) => { expander?.addEventListener('text-expander-change', ({detail: {key, provide, text}}) => {
if (key === ':') { if (key === ':') {
const matches = matchEmoji(text); const matches = matchEmoji(text);
@ -86,7 +89,11 @@ export function initTextExpander(expander) {
provide({matched: true, fragment: ul}); provide({matched: true, fragment: ul});
} else if (key === '#') { } else if (key === '#') {
provide(debouncedSuggestIssues(key, text)); if (!isIssueSuggestionsLoaded()) {
provide(fetchIssueSuggestions().then(() => issueSuggestions(text)));
} else {
provide(issueSuggestions(text));
}
} }
}); });
expander?.addEventListener('text-expander-value', ({detail}) => { expander?.addEventListener('text-expander-value', ({detail}) => {

View file

@ -1,3 +1,6 @@
import { GET } from '../modules/fetch.js';
import {parseIssueHref, parseRepoOwnerPathInfo} from '../utils.js'
export function getIssueIcon(issue) { export function getIssueIcon(issue) {
if (issue.pull_request) { if (issue.pull_request) {
if (issue.state === 'open') { if (issue.state === 'open') {
@ -15,16 +18,37 @@ export function getIssueIcon(issue) {
return 'octicon-issue-closed'; // Closed Issue return 'octicon-issue-closed'; // Closed Issue
} }
export function getIssueColor(issue) { export function getIssueColor(issue) {
if (issue.pull_request) { if (issue.pull_request) {
if (issue.pull_request.draft === true) { if (issue.pull_request.draft === true) {
return 'grey'; // WIP PR return 'grey'; // WIP PR
} else if (issue.pull_request.merged === true) { } else if (issue.pull_request.merged === true) {
return 'purple'; // Merged PR return 'purple'; // Merged PR
}
} }
if (issue.state === 'open') {
return 'green'; // Open Issue
}
return 'red'; // Closed Issue
} }
if (issue.state === 'open') {
return 'green'; // Open Issue
}
return 'red'; // Closed Issue
}
export function isIssueSuggestionsLoaded() {
return !!window.config.issueValues
}
async function fetchIssueSuggestions() {
const issuePathInfo = parseIssueHref(window.location.href);
if (!issuePathInfo.ownerName) {
const repoOwnerPathInfo = parseRepoOwnerPathInfo(window.location.pathname);
issuePathInfo.ownerName = repoOwnerPathInfo.ownerName;
issuePathInfo.repoName = repoOwnerPathInfo.repoName;
// then no issuePathInfo.indexString here, it is only used to exclude the current issue when "matchIssue"
}
if (!issuePathInfo.ownerName) {
throw new Error('unexpected');
}
const res = await GET(`${window.config.appSubUrl}/${issuePathInfo.ownerName}/${issuePathInfo.repoName}/issues/suggestions`);
const issues = await res.json();
window.config.issueValues = issues;
}

View file

@ -34,6 +34,13 @@ export function parseIssueHref(href) {
return {owner, repo, type, index}; return {owner, repo, type, index};
} }
export function parseRepoOwnerPathInfo(pathname) {
const appSubUrl = window.config.appSubUrl;
if (appSubUrl && pathname.startsWith(appSubUrl)) pathname = pathname.substring(appSubUrl.length);
const [_, ownerName, repoName] = /([^/]+)\/([^/]+)/.exec(pathname) || [];
return {ownerName, repoName};
}
// parse a URL, either relative '/path' or absolute 'https://localhost/path' // parse a URL, either relative '/path' or absolute 'https://localhost/path'
export function parseUrl(str) { export function parseUrl(str) {
return new URL(str, str.startsWith('http') ? undefined : window.location.origin); return new URL(str, str.startsWith('http') ? undefined : window.location.origin);

View file

@ -76,6 +76,16 @@ test('parseIssueHref', () => {
expect(parseIssueHref('')).toEqual({owner: undefined, repo: undefined, type: undefined, index: undefined}); expect(parseIssueHref('')).toEqual({owner: undefined, repo: undefined, type: undefined, index: undefined});
}); });
test('parseRepoOwnerPathInfo', () => {
expect(parseRepoOwnerPathInfo('/owner/repo/issues/new')).toEqual({ownerName: 'owner', repoName: 'repo'});
expect(parseRepoOwnerPathInfo('/owner/repo/releases')).toEqual({ownerName: 'owner', repoName: 'repo'});
expect(parseRepoOwnerPathInfo('/other')).toEqual({});
window.config.appSubUrl = '/sub';
expect(parseRepoOwnerPathInfo('/sub/owner/repo/issues/new')).toEqual({ownerName: 'owner', repoName: 'repo'});
expect(parseRepoOwnerPathInfo('/sub/owner/repo/compare/feature/branch-1...fix/branch-2')).toEqual({ownerName: 'owner', repoName: 'repo'});
window.config.appSubUrl = '';
});
test('parseUrl', () => { test('parseUrl', () => {
expect(parseUrl('').pathname).toEqual('/'); expect(parseUrl('').pathname).toEqual('/');
expect(parseUrl('/path').pathname).toEqual('/path'); expect(parseUrl('/path').pathname).toEqual('/path');

View file

@ -44,12 +44,45 @@ export function matchMention(queryText) {
return sortAndReduce(results); return sortAndReduce(results);
} }
export async function matchIssue(owner, repo, issueIndexStr, query) { export function matchIssue(queryText) {
const res = await GET(`${window.config.appSubUrl}/${owner}/${repo}/issues/suggestions?q=${encodeURIComponent(query)}`); const issues = window.config.issueValues ?? [];
const query = queryText.toLowerCase().trim();
const issues = await res.json(); if (!query) {
const issueIndex = parseInt(issueIndexStr); // Return latest 5 issues/prs sorted by number descending
return [...issues]
.sort((a, b) => b.number - a.number)
.slice(0, 5);
}
// filter out issue with same id const isDigital = /^\d+$/.test(query);
return issues.filter((i) => i.id !== issueIndex); const results = [];
if (isDigital) {
// Find issues/prs with number starting with the query (prefix), sorted by number ascending
const prefixMatches = issues.filter(issue =>
String(issue.number).startsWith(query)
).sort((a, b) => a.number - b.number);
results.push(...prefixMatches);
}
if (!isDigital || results.length < 5) {
// Fallback: find by title match, sorted by number descending
const titleMatches = issues
.filter(issue =>
issue.title.toLowerCase().includes(query)
)
.sort((a, b) => b.number - a.number);
// Add only those not already in the result set
for (const match of titleMatches) {
if (!results.includes(match)) {
results.push(match);
if (results.length >= 5) break;
}
}
}
return results.slice(0, 5);
} }