feat: show more relevant results for 'dependencies' dropdown (#8003)
Some checks are pending
/ release (push) Waiting to run
testing-integration / test-sqlite (push) Has been skipped
testing-integration / test-unit (push) Has been skipped
testing / backend-checks (push) Has been skipped
testing / test-e2e (push) Has been skipped
testing / test-pgsql (push) Has been skipped
testing / test-sqlite (push) Has been skipped
testing / test-remote-cacher (valkey) (push) Has been skipped
testing / frontend-checks (push) Has been skipped
testing / test-unit (push) Has been skipped
testing / test-mysql (push) Has been skipped
testing / test-remote-cacher (redis) (push) Has been skipped
testing / test-remote-cacher (garnet) (push) Has been skipped
testing / test-remote-cacher (redict) (push) Has been skipped
testing / security-check (push) Has been skipped

- Fix issue dropdown breaking when currently selected issue is included in results.
- Add `sort` parameter to `/issues/search` API.
- Sort dropdown by relevance.
- Make priority_repo_id work again.
- Added E2E test.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8003
Reviewed-by: Shiny Nematoda <snematoda@noreply.codeberg.org>
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Co-authored-by: Danko Aleksejevs <danko@very.lv>
Co-committed-by: Danko Aleksejevs <danko@very.lv>
This commit is contained in:
Danko Aleksejevs 2025-06-26 20:06:21 +02:00 committed by Gusted
parent 414199fc66
commit 184e068f37
17 changed files with 269 additions and 41 deletions

View file

@ -32,7 +32,7 @@
created_unix: 1731254961
updated_unix: 1731254961
topics: '[]'
-
id: 2
owner_id: 2

View file

@ -48,7 +48,9 @@ type IssuesOptions struct { //nolint
UpdatedBeforeUnix int64
// prioritize issues from this repo
PriorityRepoID int64
IsArchived optional.Option[bool]
// if this issue index (not ID) exists and matches the filters, *and* priorityrepo sort is used, show it first
PriorityIssueIndex int64
IsArchived optional.Option[bool]
// If combined with AllPublic, then private as well as public issues
// that matches the criteria will be returned, if AllPublic is false
@ -60,7 +62,7 @@ type IssuesOptions struct { //nolint
// applySorts sort an issues-related session based on the provided
// sortType string
func applySorts(sess *xorm.Session, sortType string, priorityRepoID int64) {
func applySorts(sess *xorm.Session, sortType string, priorityRepoID, priorityIssueIndex int64) {
switch sortType {
case "oldest":
sess.Asc("issue.created_unix").Asc("issue.id")
@ -97,8 +99,11 @@ func applySorts(sess *xorm.Session, sortType string, priorityRepoID int64) {
case "priorityrepo":
sess.OrderBy("CASE "+
"WHEN issue.repo_id = ? THEN 1 "+
"ELSE 2 END ASC", priorityRepoID).
Desc("issue.created_unix").
"ELSE 2 END ASC", priorityRepoID)
if priorityIssueIndex != 0 {
sess.OrderBy("issue.index = ? DESC", priorityIssueIndex)
}
sess.Desc("issue.created_unix").
Desc("issue.id")
case "project-column-sorting":
sess.Asc("project_issue.sorting").Desc("issue.created_unix").Desc("issue.id")
@ -470,7 +475,7 @@ func Issues(ctx context.Context, opts *IssuesOptions) (IssueList, error) {
Join("INNER", "repository", "`issue`.repo_id = `repository`.id")
applyLimit(sess, opts)
applyConditions(sess, opts)
applySorts(sess, opts.SortType, opts.PriorityRepoID)
applySorts(sess, opts.SortType, opts.PriorityRepoID, opts.PriorityIssueIndex)
issues := IssueList{}
if err := sess.Find(&issues); err != nil {
@ -494,7 +499,7 @@ func IssueIDs(ctx context.Context, opts *IssuesOptions, otherConds ...builder.Co
}
applyLimit(sess, opts)
applySorts(sess, opts.SortType, opts.PriorityRepoID)
applySorts(sess, opts.SortType, opts.PriorityRepoID, opts.PriorityIssueIndex)
var res []int64
total, err := sess.Select("`issue`.id").Table(&Issue{}).FindAndCount(&res)

View file

@ -149,7 +149,7 @@ func PullRequests(ctx context.Context, baseRepoID int64, opts *PullRequestsOptio
}
findSession := listPullRequestStatement(ctx, baseRepoID, opts)
applySorts(findSession, opts.SortType, 0)
applySorts(findSession, opts.SortType, 0, 0)
findSession = db.SetSessionPagination(findSession, opts)
prs := make([]*PullRequest, 0, opts.PageSize)
found := findSession.Find(&prs)

View file

@ -170,7 +170,7 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
if issueID, err := token.ParseIssueReference(); err == nil {
idQuery := inner_bleve.NumericEqualityQuery(issueID, "index")
idQuery.SetBoost(5.0)
idQuery.SetBoost(20.0)
innerQ.AddQuery(idQuery)
}
@ -197,6 +197,15 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
queries = append(queries, bleve.NewDisjunctionQuery(repoQueries...))
}
if options.PriorityRepoID.Has() {
eq := inner_bleve.NumericEqualityQuery(options.PriorityRepoID.Value(), "repo_id")
eq.SetBoost(10.0)
meh := bleve.NewMatchAllQuery()
meh.SetBoost(0)
should := bleve.NewDisjunctionQuery(eq, meh)
queries = append(queries, should)
}
if options.IsPull.Has() {
queries = append(queries, inner_bleve.BoolFieldQuery(options.IsPull.Value(), "is_pull"))
}

View file

@ -53,6 +53,7 @@ func (i *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
cond := builder.NewCond()
var priorityIssueIndex int64
if options.Keyword != "" {
repoCond := builder.In("repo_id", options.RepoIDs)
if len(options.RepoIDs) == 1 {
@ -82,6 +83,7 @@ func (i *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
builder.Eq{"`index`": issueID},
cond,
)
priorityIssueIndex = issueID
}
}
@ -89,6 +91,7 @@ func (i *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
if err != nil {
return nil, err
}
opt.PriorityIssueIndex = priorityIssueIndex
// If pagesize == 0, return total count only. It's a special case for search count.
if options.Paginator != nil && options.Paginator.PageSize == 0 {

View file

@ -78,6 +78,11 @@ func ToDBOptions(ctx context.Context, options *internal.SearchOptions) (*issue_m
User: nil,
}
if options.PriorityRepoID.Has() {
opts.SortType = "priorityrepo"
opts.PriorityRepoID = options.PriorityRepoID.Value()
}
if len(options.MilestoneIDs) == 1 && options.MilestoneIDs[0] == 0 {
opts.MilestoneIDs = []int64{db.NoConditionID}
} else {

View file

@ -165,7 +165,7 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
}
var eitherQ elastic.Query = innerQ
if issueID, err := token.ParseIssueReference(); err == nil {
indexQ := elastic.NewTermQuery("index", issueID).Boost(15.0)
indexQ := elastic.NewTermQuery("index", issueID).Boost(20)
eitherQ = elastic.NewDisMaxQuery().Query(indexQ).Query(innerQ).TieBreaker(0.5)
}
switch token.Kind {
@ -188,6 +188,10 @@ func (b *Indexer) Search(ctx context.Context, options *internal.SearchOptions) (
}
query.Must(q)
}
if options.PriorityRepoID.Has() {
q := elastic.NewTermQuery("repo_id", options.PriorityRepoID.Value()).Boost(10)
query.Should(q)
}
if options.IsPull.Has() {
query.Must(elastic.NewTermQuery("is_pull", options.IsPull.Value()))

View file

@ -75,8 +75,9 @@ type SearchResult struct {
type SearchOptions struct {
Keyword string // keyword to search
RepoIDs []int64 // repository IDs which the issues belong to
AllPublic bool // if include all public repositories
RepoIDs []int64 // repository IDs which the issues belong to
AllPublic bool // if include all public repositories
PriorityRepoID optional.Option[int64] // issues from this repository will be prioritized when SortByScore
IsPull optional.Option[bool] // if the issues is a pull request
IsClosed optional.Option[bool] // if the issues is closed

View file

@ -742,6 +742,25 @@ var cases = []*testIndexerCase{
}
},
},
{
Name: "PriorityRepoID",
SearchOptions: &internal.SearchOptions{
IsPull: optional.Some(false),
IsClosed: optional.Some(false),
PriorityRepoID: optional.Some(int64(3)),
Paginator: &db.ListOptionsAll,
SortBy: internal.SortByScore,
},
Expected: func(t *testing.T, data map[int64]*internal.IndexerData, result *internal.SearchResult) {
for i, v := range result.Hits {
if i < 7 {
assert.Equal(t, int64(3), data[v.ID].RepoID)
} else {
assert.NotEqual(t, int64(3), data[v.ID].RepoID)
}
}
},
},
}
type testIndexerCase struct {

View file

@ -121,6 +121,12 @@ func SearchIssues(ctx *context.APIContext) {
// description: Number of items per page
// type: integer
// minimum: 0
// - name: sort
// in: query
// description: Type of sort
// type: string
// enum: [relevance, latest, oldest, recentupdate, leastupdate, mostcomment, leastcomment, nearduedate, farduedate]
// default: latest
// responses:
// "200":
// "$ref": "#/responses/IssueList"
@ -276,7 +282,7 @@ func SearchIssues(ctx *context.APIContext) {
IsClosed: isClosed,
IncludedAnyLabelIDs: includedAnyLabels,
MilestoneIDs: includedMilestones,
SortBy: issue_indexer.SortByCreatedDesc,
SortBy: issue_indexer.ParseSortBy(ctx.FormString("sort"), issue_indexer.SortByCreatedDesc),
}
if since != 0 {
@ -305,9 +311,10 @@ func SearchIssues(ctx *context.APIContext) {
}
}
// FIXME: It's unsupported to sort by priority repo when searching by indexer,
// it's indeed an regression, but I think it is worth to support filtering by indexer first.
_ = ctx.FormInt64("priority_repo_id")
priorityRepoID := ctx.FormInt64("priority_repo_id")
if priorityRepoID > 0 {
searchOpt.PriorityRepoID = optional.Some(priorityRepoID)
}
ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt)
if err != nil {

View file

@ -2775,7 +2775,7 @@ func SearchIssues(ctx *context.Context) {
IncludedAnyLabelIDs: includedAnyLabels,
MilestoneIDs: includedMilestones,
ProjectID: projectID,
SortBy: issue_indexer.SortByCreatedDesc,
SortBy: issue_indexer.ParseSortBy(ctx.FormString("sort"), issue_indexer.SortByCreatedDesc),
}
if since != 0 {
@ -2804,9 +2804,10 @@ func SearchIssues(ctx *context.Context) {
}
}
// FIXME: It's unsupported to sort by priority repo when searching by indexer,
// it's indeed an regression, but I think it is worth to support filtering by indexer first.
_ = ctx.FormInt64("priority_repo_id")
priorityRepoID := ctx.FormInt64("priority_repo_id")
if priorityRepoID > 0 {
searchOpt.PriorityRepoID = optional.Some(priorityRepoID)
}
ids, total, err := issue_indexer.SearchIssues(ctx, searchOpt)
if err != nil {
@ -2944,7 +2945,7 @@ func ListIssues(ctx *context.Context) {
IsPull: isPull,
IsClosed: isClosed,
ProjectID: projectID,
SortBy: issue_indexer.SortByCreatedDesc,
SortBy: issue_indexer.ParseSortBy(ctx.FormString("sort"), issue_indexer.SortByCreatedDesc),
}
if since != 0 {
searchOpt.UpdatedAfterUnix = optional.Some(since)

View file

@ -4524,6 +4524,24 @@
"description": "Number of items per page",
"name": "limit",
"in": "query"
},
{
"enum": [
"relevance",
"latest",
"oldest",
"recentupdate",
"leastupdate",
"mostcomment",
"leastcomment",
"nearduedate",
"farduedate"
],
"type": "string",
"default": "latest",
"description": "Type of sort",
"name": "sort",
"in": "query"
}
],
"responses": {

View file

@ -9,16 +9,23 @@ import (
"testing"
"time"
"forgejo.org/models/db"
issues_model "forgejo.org/models/issues"
repo_model "forgejo.org/models/repo"
unit_model "forgejo.org/models/unit"
"forgejo.org/models/unittest"
user_model "forgejo.org/models/user"
"forgejo.org/modules/git"
"forgejo.org/modules/indexer/stats"
"forgejo.org/modules/optional"
"forgejo.org/modules/timeutil"
issue_service "forgejo.org/services/issue"
files_service "forgejo.org/services/repository/files"
"forgejo.org/tests"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"xorm.io/xorm/convert"
)
// first entry represents filename
@ -29,19 +36,34 @@ type FileChanges struct {
Versions []string
}
// performs additional repo setup as needed
type SetupRepo func(*user_model.User, *repo_model.Repository)
// put your Git repo declarations in here
// feel free to amend the helper function below or use the raw variant directly
func DeclareGitRepos(t *testing.T) func() {
now := timeutil.TimeStampNow()
postIssue := func(repo *repo_model.Repository, user *user_model.User, age int64, title, content string) {
issue := &issues_model.Issue{
RepoID: repo.ID,
PosterID: user.ID,
Title: title,
Content: content,
CreatedUnix: now.Add(-age),
}
require.NoError(t, issue_service.NewIssue(db.DefaultContext, repo, issue, nil, nil, nil))
}
cleanupFunctions := []func(){
newRepo(t, 2, "diff-test", []FileChanges{{
newRepo(t, 2, "diff-test", nil, []FileChanges{{
Filename: "testfile",
Versions: []string{"hello", "hallo", "hola", "native", "ubuntu-latest", "- runs-on: ubuntu-latest", "- runs-on: debian-latest"},
}}),
newRepo(t, 2, "language-stats-test", []FileChanges{{
}}, nil),
newRepo(t, 2, "language-stats-test", nil, []FileChanges{{
Filename: "main.rs",
Versions: []string{"fn main() {", "println!(\"Hello World!\");", "}"},
}}),
newRepo(t, 2, "mentions-highlighted", []FileChanges{
}}, nil),
newRepo(t, 2, "mentions-highlighted", nil, []FileChanges{
{
Filename: "history1.md",
Versions: []string{""},
@ -52,11 +74,34 @@ func DeclareGitRepos(t *testing.T) func() {
Versions: []string{""},
CommitMsg: "Another commit which mentions @user1 in the title\nand @user2 in the text",
},
}),
newRepo(t, 2, "unicode-escaping", []FileChanges{{
}, nil),
newRepo(t, 2, "unicode-escaping", nil, []FileChanges{{
Filename: "a-file",
Versions: []string{"{a}{а}"},
}}),
}}, nil),
newRepo(t, 11, "dependency-test", &tests.DeclarativeRepoOptions{
UnitConfig: optional.Some(map[unit_model.Type]convert.Conversion{
unit_model.TypeIssues: &repo_model.IssuesConfig{
EnableDependencies: true,
},
}),
}, []FileChanges{}, func(user *user_model.User, repo *repo_model.Repository) {
postIssue(repo, user, 500, "first issue here", "an issue created earlier")
postIssue(repo, user, 400, "second issue here (not 1)", "not the right issue, but in the right repo")
postIssue(repo, user, 300, "third issue here", "depends on things")
postIssue(repo, user, 200, "unrelated issue", "shrug emoji")
postIssue(repo, user, 100, "newest issue", "very new")
}),
newRepo(t, 11, "dependency-test-2", &tests.DeclarativeRepoOptions{
UnitConfig: optional.Some(map[unit_model.Type]convert.Conversion{
unit_model.TypeIssues: &repo_model.IssuesConfig{
EnableDependencies: true,
},
}),
}, []FileChanges{}, func(user *user_model.User, repo *repo_model.Repository) {
postIssue(repo, user, 450, "right issue", "an issue containing word right")
postIssue(repo, user, 150, "left issue", "an issue containing word left")
}),
// add your repo declarations here
}
@ -67,12 +112,18 @@ func DeclareGitRepos(t *testing.T) func() {
}
}
func newRepo(t *testing.T, userID int64, repoName string, fileChanges []FileChanges) func() {
func newRepo(t *testing.T, userID int64, repoName string, initOpts *tests.DeclarativeRepoOptions, fileChanges []FileChanges, setup SetupRepo) func() {
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID})
somerepo, _, cleanupFunc := tests.CreateDeclarativeRepo(t, user, repoName,
[]unit_model.Type{unit_model.TypeCode, unit_model.TypeIssues}, nil,
nil,
)
opts := tests.DeclarativeRepoOptions{}
if initOpts != nil {
opts = *initOpts
}
opts.Name = optional.Some(repoName)
if !opts.EnabledUnits.Has() {
opts.EnabledUnits = optional.Some([]unit_model.Type{unit_model.TypeCode, unit_model.TypeIssues})
}
somerepo, _, cleanupFunc := tests.CreateDeclarativeRepoWithOptions(t, user, opts)
var lastCommitID string
for _, file := range fileChanges {
@ -118,6 +169,10 @@ func newRepo(t *testing.T, userID int64, repoName string, fileChanges []FileChan
}
}
if setup != nil {
setup(user, somerepo)
}
err := stats.UpdateRepoIndexer(somerepo)
require.NoError(t, err)

View file

@ -262,3 +262,91 @@ test('New Issue: Milestone', async ({page}, workerInfo) => {
await expect(selectedMilestone).toContainText('No milestone');
await save_visual(page);
});
test.describe('Dependency dropdown', () => {
test.use({user: 'user11'});
test('Issue: Dependencies', async ({page}) => {
const response = await page.goto('/user11/dependency-test/issues/3');
expect(response?.status()).toBe(200);
const depsBlock = page.locator('.issue-content-right .depending');
const deleteDepBtn = page.locator('.issue-content-right .depending .delete-dependency-button');
const input = page.locator('#new-dependency-drop-list .search');
const current = page.locator('#new-dependency-drop-list .text').first();
const menu = page.locator('#new-dependency-drop-list .menu');
const items = page.locator('#new-dependency-drop-list .menu .item');
const confirmDelete = async () => {
const modal = page.locator('.modal.remove-dependency');
await expect(modal).toBeVisible();
await expect(modal).toContainText('This will remove the dependency from this issue');
await modal.locator('button.ok').click();
};
// A kludge to set the dropdown to the *wrong* value so it lets us select the correct one next.
const resetDropdown = async () => {
if (await current.textContent().then((s) => s.includes('#4'))) return;
await input.click();
await input.fill('unrelated');
await expect(items.first()).toContainText('unrelated');
await items.first().click();
await expect(current).toContainText('#4');
await input.click();
};
await expect(depsBlock).toBeVisible();
while (await deleteDepBtn.first().isVisible()) {
await deleteDepBtn.first().click(); // wipe added dependencies from any previously failed tests
await confirmDelete();
}
await expect(depsBlock).toContainText('No dependencies set');
await input.scrollIntoViewIfNeeded();
await input.click();
const first = 'first issue here';
const second = 'second issue here';
const newest = 'newest issue';
// Without query, it should show issues in the same repo, sorted by date, except current one.
await expect(menu).toBeVisible();
await expect(items).toHaveCount(4); // 5 issues in this repo, minus current one
await expect(items.first()).toContainText(newest);
await expect(items.last()).toContainText(first);
await resetDropdown();
// With query, it should search all repos, but show current repo issues first.
await input.fill('right');
await expect(items.first()).toContainText(second);
await expect.poll(() => items.count()).toBeGreaterThan(1); // there is an issue in user11/dependency-test-2 containing the word "right"
await resetDropdown();
// When entering an issue number, it should always show that one first, then all text matches.
await input.fill('1');
await expect(items.first()).toContainText(first);
await expect(items.nth(1)).toBeVisible();
await resetDropdown();
// Should behave the same with a prefix
await input.fill('#1');
await expect(items.first()).toContainText(first);
// Selecting an issue
await items.first().click();
await expect(current).toContainText(first);
// Add dependency
const link = page.locator('.issue-content-right .depending .dependency a.title');
await page.locator('.issue-content-right .depending button').click();
await expect(link).toHaveAttribute('href', '/user11/dependency-test/issues/1');
// Remove dependency
await expect(deleteDepBtn).toBeVisible();
await deleteDepBtn.click();
await confirmDelete();
await expect(depsBlock).toContainText('No dependencies set');
});
});

View file

@ -93,6 +93,7 @@ func createSessions(t testing.TB) {
users := []string{
"user1",
"user2",
"user11",
"user12",
"user18",
"user29",

View file

@ -42,6 +42,7 @@ import (
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"xorm.io/xorm/convert"
)
func exitf(format string, args ...any) {
@ -342,6 +343,7 @@ type DeclarativeRepoOptions struct {
Name optional.Option[string]
EnabledUnits optional.Option[[]unit_model.Type]
DisabledUnits optional.Option[[]unit_model.Type]
UnitConfig optional.Option[map[unit_model.Type]convert.Conversion]
Files optional.Option[[]*files_service.ChangeRepoFile]
WikiBranch optional.Option[string]
AutoInit optional.Option[bool]
@ -390,9 +392,14 @@ func CreateDeclarativeRepoWithOptions(t *testing.T, owner *user_model.User, opts
enabledUnits = make([]repo_model.RepoUnit, len(units))
for i, unitType := range units {
var config convert.Conversion
if cfg, ok := opts.UnitConfig.Value()[unitType]; ok {
config = cfg
}
enabledUnits[i] = repo_model.RepoUnit{
RepoID: repo.ID,
Type: unitType,
Config: config,
}
}
}

View file

@ -125,16 +125,21 @@ function excludeLabel(item) {
export function initRepoIssueSidebarList() {
const repolink = $('#repolink').val();
const repoId = $('#repoId').val();
const crossRepoSearch = $('#crossRepoSearch').val();
const crossRepoSearch = $('#crossRepoSearch').val() === 'true';
const tp = $('#type').val();
let issueSearchUrl = `${appSubUrl}/${repolink}/issues/search?q={query}&type=${tp}`;
if (crossRepoSearch === 'true') {
issueSearchUrl = `${appSubUrl}/issues/search?q={query}&priority_repo_id=${repoId}&type=${tp}`;
}
$('#new-dependency-drop-list')
.dropdown({
apiSettings: {
url: issueSearchUrl,
beforeSend(settings) {
if (!settings.urlData.query.trim()) {
settings.url = `${appSubUrl}/${repolink}/issues/search?q={query}&type=${tp}&sort=updated`;
} else if (crossRepoSearch) {
settings.url = `${appSubUrl}/issues/search?q={query}&priority_repo_id=${repoId}&type=${tp}&sort=relevance`;
} else {
settings.url = `${appSubUrl}/${repolink}/issues/search?q={query}&type=${tp}&sort=relevance`;
}
return settings;
},
onResponse(response) {
const filteredResponse = {success: true, results: []};
const currIssueId = $('#new-dependency-drop-list').data('issue-id');
@ -142,7 +147,7 @@ export function initRepoIssueSidebarList() {
for (const [_, issue] of Object.entries(response)) {
// Don't list current issue in the dependency list.
if (issue.id === currIssueId) {
return;
continue;
}
filteredResponse.results.push({
name: `#${issue.number} ${issueTitleHTML(htmlEscape(issue.title))