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

Allows repo search to match against "owner/repo" pattern strings #19754

Merged
merged 7 commits into from
May 21, 2022
Merged

Allows repo search to match against "owner/repo" pattern strings #19754

merged 7 commits into from
May 21, 2022

Conversation

Eekle
Copy link
Contributor

@Eekle Eekle commented May 19, 2022

Closes #19617

If the teamID is 0 so we're not searching in a specific team, and the keyword has a single slash in - try and match each part of "owner/repo" against the owner_name and lower_name fields.

Architecturally I don't know if this messes with API endpoints or something - I'm not that familiar. But I thought better to make the PR so there's something to discuss around at least.

delvh
delvh approved these changes May 19, 2022
View reviewed changes
Copy link
Member

@delvh delvh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Yep, I thought for about two minutes if you missed any edge cases but couldn't find any, so LGTM.
(However, I would recommend running make fmt.)

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label May 19, 2022
@Eekle
Copy link
Contributor Author

Eekle commented May 19, 2022

Yep, I thought for about two minutes if you missed any edge cases but couldn't find any, so LGTM. (However, I would recommend running make fmt.)

Yeah my bad!

I used gofumpt based on the output from drone - is that equivalent?

@delvh
Copy link
Member

delvh commented May 19, 2022

I think so. make fmt basically downloads gofumpt if not present and runs it then.

@delvh delvh added type/enhancement An improvement of existing functionality kind/usability labels May 19, 2022
@delvh delvh added this to the 1.17.0 milestone May 19, 2022
@Eekle
Copy link
Contributor Author

Eekle commented May 19, 2022

@delvh Is this assigned to me because there's more for me to do?

@6543
Copy link
Member

6543 commented May 19, 2022

a unit test to add this case would be nice ... if you need help just ask

@delvh
Copy link
Member

delvh commented May 19, 2022 •
edited
Loading

@Eekle my definition of assignee is the person working on this PR, so I simply assigned you because you opened it.
What else can assignee mean?

@Eekle
Copy link
Contributor Author

Eekle commented May 19, 2022

@delvh Oh cool np. I'm used to issue workflows where the assignee is whoever next has to do a thing - so if this were assigned to anyone I would expect it to be a second reviewer.

But this makes just as much sense :) Thanks

@6543 Sounds good. I will try and work that out tomorrow but will holler if I need help!

@lunny
Copy link
Member

lunny commented May 19, 2022

So then we cannot search / in the text?

@Eekle
Copy link
Contributor Author

Eekle commented May 19, 2022

So then we cannot search / in the text?

This change does not remove any results - it only adds them. IE anything that matched before will still match - and some new stuff might too.

@lunny
Copy link
Member

lunny commented May 19, 2022

So then we cannot search / in the text?

This change does not remove any results - it only adds them. IE anything that matched before will still match - and some new stuff might too.

Maybe we need have a sort so that repository matched owner/repo will be on the top.

@Eekle
Copy link
Contributor Author

Eekle commented May 19, 2022

So then we cannot search / in the text?

This change does not remove any results - it only adds them. IE anything that matched before will still match - and some new stuff might too.

Maybe we need have a sort so that repository matched owner/repo will be on the top.

That sounds sensible - I'm not sure I know how to do that but I'll give it a try.

Out of curiosity how could a string with a forward slash match otherwise? I didn't think it was an allowed character?

@Eekle
Copy link
Contributor Author

Eekle commented May 20, 2022 •
edited
Loading

@6543 Unit tests added. Required a bit of change to the code that runs the cases just because of the way it checks pass/fail on matches.

@Eekle
Copy link
Contributor Author

Eekle commented May 20, 2022 •
edited
Loading

@lunny Adds search ordering such that owner matched repositories are at the top, only when the owner priority ID field isn't already filled.

It's hard to think how to validate this - because I don't think a search string with a slash in could match any other way.

@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 May 20, 2022
@6543
Copy link
Member

6543 commented May 20, 2022

please "update branch"

@lunny
Copy link
Member

lunny commented May 21, 2022

make L-G-T-M work

@lunny lunny merged commit 876cad0 into go-gitea:main May 21, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 21, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
788c3c5 
* giteaofficial/main:
  Prevent NPE when cache service is disabled (go-gitea#19703)
  Detect truncated utf-8 characters at the end of content as still representing utf-8 (go-gitea#19773)
  Add silentcodeg to MAINTAINERS (go-gitea#19771)
  Allows repo search to match against "owner/repo" pattern strings (go-gitea#19754)
  Update JS dependencies (go-gitea#19767)
  Nuke the incorrect permission report on /api/v1/notifications (go-gitea#19761)
AbdulrhmnGhanem pushed a commit to kitspace/gitea that referenced this pull request Aug 24, 2022
@Eekle @AbdulrhmnGhanem
Allows repo search to match against "owner/repo" pattern strings (go-…
e7ae5b4 
…gitea#19754)

* Allows repo search to match against "owner/repo" pattern strings

* Gofumpt

* Adds test case for "owner/repo" style repo search

* With "owner/repo" search terms, prioritise results which match the owner field

* Fixes unquoted SQL string in repo search
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
@delvh delvh added topic/ui-interaction Change the process how users use Gitea instead of the visual appearance and removed kind/usability labels Oct 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@delvh delvh delvh approved these changes

@6543 6543 6543 approved these changes

Assignees

@Eekle Eekle

Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui-interaction Change the process how users use Gitea instead of the visual appearance type/enhancement An improvement of existing functionality
Projects
None yet
Milestone
1.17.0
Development

Successfully merging this pull request may close these issues.

Repository search ignores org/username
5 participants
@Eekle @delvh @6543 @lunny @GiteaBot