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

Fix wrong review requested number #26784

Merged
merged 8 commits into from
Sep 3, 2023
Merged

Fix wrong review requested number #26784

merged 8 commits into from
Sep 3, 2023

Conversation

lng2020
Copy link
Member

@lng2020 lng2020 commented Aug 29, 2023 •
edited
Loading

Fix the wrong review requested number mentioned by #18808 .
Fix #18808
Before:
ksnip_20230829-140750
After:
ksnip_20230829-141817

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 29, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 29, 2023
@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 Aug 29, 2023
@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 Aug 29, 2023
@lunny lunny added type/bug backport/v1.20 This PR should be backported to Gitea 1.20 outdated/backport/v1.19 This PR should be backported to Gitea 1.19 labels Aug 29, 2023
@lunny lunny added this to the 1.21.0 milestone Aug 29, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 29, 2023
@GiteaBot
Copy link
Contributor

@lng2020 please fix the merge conflicts. 🍵

@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 29, 2023
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 29, 2023
@silverwind silverwind enabled auto-merge (squash) August 29, 2023 20:40
@lng2020 lng2020 marked this pull request as draft August 30, 2023 03:26
auto-merge was automatically disabled August 30, 2023 03:26

Pull request was converted to draft

@yp05327
Copy link
Contributor

yp05327 commented Aug 30, 2023 •
edited
Loading

image
https://stackoverflow.com/questions/36829911/how-to-resolve-order-by-clause-is-not-in-select-list-caused-mysql-5-7-with-sel

yp05327
yp05327 reviewed Aug 30, 2023
View reviewed changes
models/issues/issue_search.go Outdated
@@ -480,7 +480,7 @@ func IssueIDs(ctx context.Context, opts *IssuesOptions, otherConds ...builder.Co
applySorts(sess, opts.SortType, opts.PriorityRepoID)
Copy link
Contributor

@yp05327 yp05327 Aug 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
applySorts(sess, opts.SortType, opts.PriorityRepoID)

As a simple fix method, we only care about issue ids here, so no need to add sort options.
It seems that using both orderby and distinct will cause sql error.

ps: only tested in mysql8, not sure this will fix all ci fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

But maybe in some cases, we need a sorted issue ids list? Not sure about this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your advice! I also agree that removing applySorts will solve the problem, but manually removing the duplicate key looks safer. I doubt if the function Issues() will have the same problem as duplicate records too and we just haven't discovered the case yet.

@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Aug 30, 2023
@lng2020 lng2020 marked this pull request as ready for review August 30, 2023 07:50
@techknowlogick techknowlogick added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 1, 2023
auto-merge was automatically disabled September 2, 2023 18:01

Head branch was pushed to by a user without write access

@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 2, 2023
@pull-request-size pull-request-size bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 2, 2023
@lunny lunny enabled auto-merge (squash) September 3, 2023 01:45
@lunny lunny merged commit f1fe102 into go-gitea:main Sep 3, 2023
24 checks passed
@GiteaBot GiteaBot mentioned this pull request Sep 3, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Sep 3, 2023
@lng2020 @GiteaBot
Fix wrong review requested number (go-gitea#26784)
9982596 
Fix the wrong review requested number mentioned by go-gitea#18808 .
Fix go-gitea#18808 
Before:

![ksnip_20230829-140750](https://github.com/go-gitea/gitea/assets/70063547/0af2055b-6f16-4699-a944-c7186831d7f9)
After:

![ksnip_20230829-141817](https://github.com/go-gitea/gitea/assets/70063547/16633264-20ba-45e3-bfbb-a495ed76a45b)
@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 Sep 3, 2023
KN4CK3R pushed a commit that referenced this pull request Sep 3, 2023
@GiteaBot @lng2020
Fix wrong review requested number (#26784) (#26880)
2a18479 
Backport #26784 by @lng2020

Fix the wrong review requested number mentioned by #18808 .
Fix #18808 
Before:

![ksnip_20230829-140750](https://github.com/go-gitea/gitea/assets/70063547/0af2055b-6f16-4699-a944-c7186831d7f9)
After:

![ksnip_20230829-141817](https://github.com/go-gitea/gitea/assets/70063547/16633264-20ba-45e3-bfbb-a495ed76a45b)

Co-authored-by: Nanguan Lin <70063547+lng2020@users.noreply.github.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Sep 4, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
258382e 
* giteaofficial/main:
  [skip ci] Updated licenses and gitignores
  Update documents to fix some links (go-gitea#26885)
  clarify aspects of the dump command (go-gitea#26887)
  Relocate the `RSS user feed` button (go-gitea#26882)
  Use Go 1.21 and update dependencies (go-gitea#26878)
  Update docs about attachment path (go-gitea#26883)
  Refactor "shortsha" (go-gitea#26877)
  Fix wrong review requested number (go-gitea#26784)
  Refactor `og:description` to limit the max length (go-gitea#26876)
  Reorder blocks in vue SFCs (go-gitea#26874)
  Make it posible to customize nav text color via css var (go-gitea#26807)
  Enable djlint H008 and fix issues (go-gitea#26869)
  Improve opengraph previews (go-gitea#26851)
  Add more descriptive error on forgot password page (go-gitea#26848)
  Allow users with write permissions for issues to add attachments with API (go-gitea#26837)
  Move licenses.txt to /assets directory (go-gitea#26866)

# Conflicts:
#	templates/base/footer_content.tmpl
nrdufour added a commit to nrdufour/home-ops that referenced this pull request Sep 8, 2023
@nrdufour
chore(deps): update docker.io/gitea/gitea docker tag to v1.20.4 (#79)
7ac6200 
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker.io/gitea/gitea](https://github.com/go-gitea/gitea) | patch | `1.20.3` -> `1.20.4` |

---

### Release Notes

<details>
<summary>go-gitea/gitea (docker.io/gitea/gitea)</summary>

### [`v1.20.4`](https://github.com/go-gitea/gitea/blob/HEAD/CHANGELOG.md#1204---2023-09-08)

[Compare Source](go-gitea/gitea@v1.20.3...v1.20.4)

-   SECURITY
    -   Check blocklist for emails when adding them to account ([#&#8203;26812](go-gitea/gitea#26812)) ([#&#8203;26831](go-gitea/gitea#26831))
-   ENHANCEMENTS
    -   Add `branch_filter` to hooks API endpoints ([#&#8203;26599](go-gitea/gitea#26599)) ([#&#8203;26632](go-gitea/gitea#26632))
    -   Fix incorrect "tabindex" attributes ([#&#8203;26733](go-gitea/gitea#26733)) ([#&#8203;26734](go-gitea/gitea#26734))
    -   Use line-height: normal by default ([#&#8203;26635](go-gitea/gitea#26635)) ([#&#8203;26708](go-gitea/gitea#26708))
    -   Fix unable to display individual-level project ([#&#8203;26198](go-gitea/gitea#26198)) ([#&#8203;26636](go-gitea/gitea#26636))
-   BUGFIXES
    -   Fix wrong review requested number ([#&#8203;26784](go-gitea/gitea#26784)) ([#&#8203;26880](go-gitea/gitea#26880))
    -   Avoid double-unescaping of form value ([#&#8203;26853](go-gitea/gitea#26853)) ([#&#8203;26863](go-gitea/gitea#26863))
    -   Redirect from `{repo}/issues/new` to `{repo}/issues/new/choose` when blank issues are disabled ([#&#8203;26813](go-gitea/gitea#26813)) ([#&#8203;26847](go-gitea/gitea#26847))
    -   Sync tags when adopting repos ([#&#8203;26816](go-gitea/gitea#26816)) ([#&#8203;26834](go-gitea/gitea#26834))
    -   Fix verifyCommits error when push a new branch ([#&#8203;26664](go-gitea/gitea#26664)) ([#&#8203;26810](go-gitea/gitea#26810))
    -   Include the GITHUB_TOKEN/GITEA_TOKEN secret for fork pull requests ([#&#8203;26759](go-gitea/gitea#26759)) ([#&#8203;26806](go-gitea/gitea#26806))
    -   Fix some slice append usages ([#&#8203;26778](go-gitea/gitea#26778)) ([#&#8203;26798](go-gitea/gitea#26798))
    -   Add fix incorrect can_create_org_repo for org owner team ([#&#8203;26683](go-gitea/gitea#26683)) ([#&#8203;26791](go-gitea/gitea#26791))
    -   Fix bug for ctx usage ([#&#8203;26763](go-gitea/gitea#26763))
    -   Make issue template field template access correct template data ([#&#8203;26698](go-gitea/gitea#26698)) ([#&#8203;26709](go-gitea/gitea#26709))
    -   Use correct minio error ([#&#8203;26634](go-gitea/gitea#26634)) ([#&#8203;26639](go-gitea/gitea#26639))
    -   Ignore the trailing slashes when comparing oauth2 redirect_uri ([#&#8203;26597](go-gitea/gitea#26597)) ([#&#8203;26618](go-gitea/gitea#26618))
    -   Set errwriter for urfave/cli v1 ([#&#8203;26616](go-gitea/gitea#26616))
    -   Fix reopen logic for agit flow pull request ([#&#8203;26399](go-gitea/gitea#26399)) ([#&#8203;26613](go-gitea/gitea#26613))
    -   Fix context filter has no effect in dashboard ([#&#8203;26695](go-gitea/gitea#26695)) ([#&#8203;26811](go-gitea/gitea#26811))
    -   Fix being unable to use a repo that prohibits accepting PRs as a PR source. ([#&#8203;26785](go-gitea/gitea#26785)) ([#&#8203;26790](go-gitea/gitea#26790))
    -   Fix Page Not Found error ([#&#8203;26768](go-gitea/gitea#26768))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4yMy4yIiwidXBkYXRlZEluVmVyIjoiMzYuMjMuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Reviewed-on: https://git.home/nrdufour/home-ops/pulls/79
Co-authored-by: Renovate <renovate@ptinem.io>
Co-committed-by: Renovate <renovate@ptinem.io>
This was referenced Sep 11, 2023
Closed
Merged
silverwind pushed a commit that referenced this pull request Sep 21, 2023
@lng2020 @CaiCandong @GiteaBot
Fix review request number and add more tests (#27104)
2f8e160 
fix #27019 
## testfixture yml
1. add issue20(a pr issue) in repo 23, org 17
2. add user15 to team 9
3. add four reviews about issue20
## test case
add two tests that are described with code comments
the code before pr #26784 failed the first test
<img width="479" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/1d9b5787-11b4-4c4d-931f-6a9869547f35">
current code failed the second test(as mentioned in #27019)
<img width="484" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/05608055-7587-43d1-bae1-92c688270819">
Any advice is appreciated.

---------

Co-authored-by: CaiCandong <50507092+CaiCandong@users.noreply.github.com>
Co-authored-by: Giteabot <teabot@gitea.io>
@GiteaBot GiteaBot mentioned this pull request Sep 21, 2023
Merged
GiteaBot added a commit to GiteaBot/gitea that referenced this pull request Sep 21, 2023
@lng2020 @CaiCandong @GiteaBot
Fix review request number and add more tests (go-gitea#27104)
996fe3a 
fix go-gitea#27019 
## testfixture yml
1. add issue20(a pr issue) in repo 23, org 17
2. add user15 to team 9
3. add four reviews about issue20
## test case
add two tests that are described with code comments
the code before pr go-gitea#26784 failed the first test
<img width="479" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/1d9b5787-11b4-4c4d-931f-6a9869547f35">
current code failed the second test(as mentioned in go-gitea#27019)
<img width="484" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/05608055-7587-43d1-bae1-92c688270819">
Any advice is appreciated.

---------

Co-authored-by: CaiCandong <50507092+CaiCandong@users.noreply.github.com>
Co-authored-by: Giteabot <teabot@gitea.io>
silverwind pushed a commit that referenced this pull request Sep 21, 2023
@GiteaBot @lng2020 @CaiCandong
Fix review request number and add more tests (#27104) (#27168)
81ae35a 
Backport #27104 by @lng2020

fix #27019 
## testfixture yml
1. add issue20(a pr issue) in repo 23, org 17
2. add user15 to team 9
3. add four reviews about issue20
## test case
add two tests that are described with code comments
the code before pr #26784 failed the first test
<img width="479" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/1d9b5787-11b4-4c4d-931f-6a9869547f35">
current code failed the second test(as mentioned in #27019)
<img width="484" alt="image"
src="https://github.com/go-gitea/gitea/assets/70063547/05608055-7587-43d1-bae1-92c688270819">
Any advice is appreciated.

Co-authored-by: Nanguan Lin <70063547+lng2020@users.noreply.github.com>
Co-authored-by: CaiCandong <50507092+CaiCandong@users.noreply.github.com>
@lng2020 lng2020 deleted the fix/review-request-number branch November 7, 2023 07:50
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@yp05327 yp05327 yp05327 approved these changes

@CaiCandong CaiCandong CaiCandong approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
backport/done All backports for this PR have been created backport/v1.20 This PR should be backported to Gitea 1.20 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. type/bug
Projects
None yet
Milestone
1.21.0
Development

Successfully merging this pull request may close these issues.

Member of Two Teams Shows Up Twice on Review Request
9 participants
@lng2020 @GiteaBot @yp05327 @techknowlogick @CaiCandong @delvh @lunny @silverwind @jolheiser