Skip to content

Allow searching by Issue Index #17929

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
Closed
wants to merge 5 commits into from
Closed

Allow searching by Issue Index #17929

wants to merge 5 commits into from

Conversation

aceArt-GmbH
Copy link
Contributor

@aceArt-GmbH aceArt-GmbH commented Dec 7, 2021 •
edited
Loading

Hey,
first of all thanks for creating and maintaining this repo :D

A user of our instance was trying to search for an issue via ID. Currently this does not work.
The alternative we have been using is opening any issue and changing the URL to the desired ID, but this workflow is not good for non technical users. (i.e. /go-gitea/gitea/issues/17927 -> replace 17927 with whatever ticket ID I want to search)

I came up with this simple solution, but I am not sure if it is good enough.
For example, it does not allow searching for issues starting with the keyword (search "1" and find issue 12 and 13)
Please let me know about your opinion and an alternative implementation :)

image
issue #3 is found although neither title nor content contains a 3

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 7, 2021
@lunny
Copy link
Member

lunny commented Dec 7, 2021

I would like to input #3 and enter so that we can just open the issue directly.

@aceArt-GmbH
Copy link
Contributor Author

we had that idea too, but is it intuitive?
For you probably as you know #3 references issues, but less technical users maybe not

@aceArt-GmbH aceArt-GmbH changed the title Allow searching by issue ID Draft: Allow searching by issue ID Dec 8, 2021
@aceArt-GmbH aceArt-GmbH force-pushed the issueSearch branch 2 times, most recently from 7cd5906 to cc4b3c5 Compare December 8, 2021 13:01
@aceArt-GmbH
Copy link
Contributor Author

It is working now :D
(although I still don't know how to refreshed the search indexer)

I didn't test the elastic search part.

Please test and review as I am not a Go dev

@aceArt-GmbH
Copy link
Contributor Author

is it normal for testing-amd64 test-mysql to fail?
What is the problem?

@aceArt-GmbH aceArt-GmbH changed the title Draft: Allow searching by issue ID Allow searching by issue ID Dec 8, 2021
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 8, 2021 •
edited
Loading

It is working now :D (although I still don't know how to refreshed the search indexer)

I didn't test the elastic search part.

Please test and review as I am not a Go dev

You can make some edits to an issue, then the search indexer will update the content.

ps: I think it's better to have some unit test code to guarantee the code works as expected. If you have any question, Gitea developers will help.

is it normal for testing-amd64 test-mysql to fail?

Some failures are caused by some bugs in Gitea test code, it doesn't matter. Unrelated failures can be ignored.

@aceArt-GmbH
Copy link
Contributor Author

I think it's better to have some unit test code to guarantee the code works as expected.

I added a case to bleve_test.go, is that enough?

@wxiaoguang
Copy link
Contributor

That's fine. I just realized that there was no unit test for elasticsearch ...

lunny
lunny reviewed Dec 9, 2021
View reviewed changes
modules/indexer/issues/bleve.go
@@ -236,6 +239,7 @@ func (b *BleveIndexer) Search(keyword string, repoIDs []int64, limit, start int)
for _, repoID := range repoIDs {
repoQueriesP = append(repoQueriesP, numericEqualityQuery(repoID, "RepoID"))
}
index, _ := strconv.ParseInt(keyword, 10, 64)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe keyword is a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

does it matter?
I didn't notice a difference in behavior.
How would I dynamically add the query if keyword is a string and the parser errors?

@lunny
Copy link
Member

lunny commented Dec 9, 2021

To keep the compatible, we need to upgrade indexer version so that all index will be rebuilt.

@aceArt-GmbH
Copy link
Contributor Author

To keep the compatible, we need to upgrade indexer version so that all index will be rebuilt.

How do I do that? Increase issueIndexerLatestVersion?

@lunny
Copy link
Member

lunny commented Dec 9, 2021

To keep the compatible, we need to upgrade indexer version so that all index will be rebuilt.

How do I do that? Increase issueIndexerLatestVersion?

Yes

Allow searching by issue ID
95f5017 
Add issue ID to found issues if it is a string
@aceArt-GmbH
Copy link
Contributor Author

I increased the issueIndexerLatestVersion.
Anything else left here? :)

@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 Dec 22, 2021
@wxiaoguang wxiaoguang added this to the 1.17.0 milestone Jan 16, 2022
@zeripath
Copy link
Contributor

Why did you close this?

@zeripath zeripath reopened this Feb 23, 2022
@zeripath zeripath added the type/enhancement An improvement of existing functionality label Feb 23, 2022
@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 23, 2022
@aceArt-GmbH
Copy link
Contributor Author

Why did you close this?

Because this is taking too long

@aceArt-GmbH aceArt-GmbH deleted the issueSearch branch March 17, 2022 07:42
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
@go-gitea go-gitea unlocked this conversation May 7, 2022
@6543
Copy link
Member

6543 commented May 7, 2022

Because this is taking too long

consider that maintainers maintain this project in there free time, so yes it can take long - sometimes ~2years++ for big features.

@6543 6543 changed the title Allow searching by issue ID Allow searching by Issue Index May 7, 2022
@6543
Copy link
Member

6543 commented May 7, 2022

so in your case it just fall out of the "manual merge queue" i guess

@6543 6543 mentioned this pull request May 7, 2022
Closed
@6543
Copy link
Member

6543 commented May 7, 2022

@zeripath @wxiaoguang @aceArt-GmbH -> #19646

@6543
Copy link
Member

6543 commented May 7, 2022 •
edited
Loading

so in your case it just fall out of the "manual merge queue" i guess

I think this is because github does not allow maintainers to "update pulls" because you created a pull from an repo owned by an organisation ...

@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny left review comments

@zeripath zeripath zeripath approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.
6 participants
@aceArt-GmbH @lunny @wxiaoguang @zeripath @6543 @GiteaBot