Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rework suggestion backend #33538

Merged
merged 12 commits into from
Feb 10, 2025
Merged

Rework suggestion backend #33538

merged 12 commits into from
Feb 10, 2025
+173 −49

Conversation

lunny
Copy link
Member

@lunny lunny commented Feb 9, 2025 •
edited
Loading

Fix #33522

The suggestion backend logic now is

  • If the keyword is empty, returned the latest 5 issues/prs with index desc order
  • If the keyword is digital, find all issues/prs which index has a prefix with that, with index asc order
  • If the keyword is non-digital or if the queried records less than 5, searching issues/prs title with a like, with index desc order

Empty keyword

image

Digital

image

Digital and title contains the digital

image

non-Digital

image image

@lunny lunny added type/bug backport/v1.23 This PR should be backported to Gitea 1.23 labels Feb 9, 2025
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 9, 2025
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 9, 2025
@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Feb 9, 2025
@lunny lunny mentioned this pull request Feb 9, 2025
Closed
wxiaoguang
wxiaoguang reviewed Feb 9, 2025
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't be right

@lunny lunny changed the title Make index matched issues in the front of other keyword matched issues Rework suggestion backend Feb 9, 2025
@lunny
Copy link
Member Author

lunny commented Feb 9, 2025

It can't be right

I did some improvements, please review again.

wxiaoguang
wxiaoguang reviewed Feb 10, 2025
View reviewed changes
wxiaoguang
wxiaoguang reviewed Feb 10, 2025
View reviewed changes
wxiaoguang
wxiaoguang reviewed Feb 10, 2025
View reviewed changes
wxiaoguang
wxiaoguang reviewed Feb 10, 2025
View reviewed changes
wxiaoguang
wxiaoguang reviewed Feb 10, 2025
View reviewed changes
routers/web/repo/issue_suggestions.go
SortBy: issue_indexer.SortByUpdatedDesc,
}

ids, _, err := issue_indexer.SearchIssues(ctx, searchOpt)
Copy link
Contributor

@wxiaoguang wxiaoguang Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue_indexer.SearchIssues does full-text search including issue content?

I am not sure whether it is good enough to only search title(name) by this PR's new approach.

image

Copy link
Member Author

@lunny lunny Feb 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue_indexer.SearchIssues does full-text search including issue content?

I am not sure whether it is good enough to only search title(name) by this PR's new approach.

Looks like github did what I did but with a updated order.

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Github will search both title and content but the title-matchted issue will be listed first.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Github will search both title and content but the title-matchted issue will be listed first.

Maybe GitHub uses search engine's ranking, not simply put something first.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can merge this first. Searching content is not usable from my side.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 10, 2025
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 10, 2025
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Feb 10, 2025
@lunny lunny enabled auto-merge (squash) February 10, 2025 15:58
@lunny lunny merged commit 72518a8 into go-gitea:main Feb 10, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.24.0 milestone Feb 10, 2025
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Feb 10, 2025
@lunny @wxiaoguang @GiteaBot
Rework suggestion backend (go-gitea#33538)
728a7cb 
Fix go-gitea#33522 

The suggestion backend logic now is

- If the keyword is empty, returned the latest 5 issues/prs with index
desc order
- If the keyword is digital, find all issues/prs which `index` has a
prefix with that, with index asc order
- If the keyword is non-digital or if the queried records less than 5,
searching issues/prs title with a `like`, with index desc order

## Empty keyword
<img width="310" alt="image"
src="https://github.com/user-attachments/assets/1912c634-0d98-4eeb-8542-d54240901f77"
/>

## Digital
<img width="479" alt="image"
src="https://github.com/user-attachments/assets/0356a936-7110-4a24-b21e-7400201bf9b8"
/>

## Digital and title contains the digital
<img width="363" alt="image"
src="https://github.com/user-attachments/assets/6e12f908-28fe-48de-8ccc-09cbeab024d4"
/>

## non-Digital
<img width="435" alt="image"
src="https://github.com/user-attachments/assets/2722bb53-baa2-4d67-a224-522a65f73856"
/>
<img width="477" alt="image"
src="https://github.com/user-attachments/assets/06708dd9-80d1-4a88-b32b-d29072dd1ba6"
/>

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot mentioned this pull request Feb 10, 2025
Merged
@GiteaBot GiteaBot added backport/done All backports for this PR have been created and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Feb 10, 2025
@wxiaoguang wxiaoguang deleted the lunny/fix_issue_suggestion branch February 10, 2025 16:26
lunny added a commit that referenced this pull request Feb 10, 2025
@GiteaBot @lunny @wxiaoguang
Rework suggestion backend (#33538) (#33546)
a014d07 
Backport #33538 by @lunny

Fix #33522 

The suggestion backend logic now is

- If the keyword is empty, returned the latest 5 issues/prs with index
desc order
- If the keyword is digital, find all issues/prs which `index` has a
prefix with that, with index asc order
- If the keyword is non-digital or if the queried records less than 5,
searching issues/prs title with a `like`, with index desc order

## Empty keyword
<img width="310" alt="image"
src="https://github.com/user-attachments/assets/1912c634-0d98-4eeb-8542-d54240901f77"
/>

## Digital
<img width="479" alt="image"
src="https://github.com/user-attachments/assets/0356a936-7110-4a24-b21e-7400201bf9b8"
/>

## Digital and title contains the digital
<img width="363" alt="image"
src="https://github.com/user-attachments/assets/6e12f908-28fe-48de-8ccc-09cbeab024d4"
/>

## non-Digital
<img width="435" alt="image"
src="https://github.com/user-attachments/assets/2722bb53-baa2-4d67-a224-522a65f73856"
/>
<img width="477" alt="image"
src="https://github.com/user-attachments/assets/06708dd9-80d1-4a88-b32b-d29072dd1ba6"
/>

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 11, 2025
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
2383151 
* giteaofficial/main:
  Enhance routers for the Actions runner operations (go-gitea#33549)
  [skip ci] Updated translations via Crowdin
  Run yamllint with strict mode, fix issue (go-gitea#33551)
  Enhance routers for the Actions variable operations (go-gitea#33547)
  enhancement: add additional command hints for PowerShell & CMD (go-gitea#33548)
  Feature: Support workflow event dispatch via API (go-gitea#33545)
  Optimize the dashboard (go-gitea#32990)
  Rework suggestion backend (go-gitea#33538)
  Revert "Feature: Support workflow event dispatch via API (go-gitea#32059)" (go-gitea#33541)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@hiifong hiifong hiifong approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.23 This PR should be backported to Gitea 1.23 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/go Pull requests that update Go code size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
1.24.0
Development

Successfully merging this pull request may close these issues.

Wrong issue suggested and selected when copy-pasting ID
4 participants
@lunny @wxiaoguang @hiifong @GiteaBot