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

Pull request check list is limited to 30 entries #25990

Closed
diorcety opened this issue Jul 19, 2023 · 4 comments · Fixed by #26179 · May be fixed by #26247
Closed

Pull request check list is limited to 30 entries #25990

diorcety opened this issue Jul 19, 2023 · 4 comments · Fixed by #26179 · May be fixed by #26247
Labels
type/bug

Comments

@diorcety
Copy link

Description

This is linked to the DEFAULT_PAGING_NUM value.
The computation of the check summary status is also wrong due to this limit: A status can be in progress and the summary value having the value "All checks were successful"

Gitea Version

1.19

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

Docker

Database

PostgreSQL
@diorcety
Copy link
Author

Increasing DEFAULT_PAGING_NUM fix the issue: all checks are displayed and the summary status is correct

@CaiCandong
Copy link
Member

CaiCandong commented Jul 27, 2023 •
edited
Loading

I've understood the error, I made a PR to fix this.

@CaiCandong CaiCandong mentioned this issue Jul 27, 2023
Merged
@yp05327
Copy link
Member

yp05327 commented Jul 27, 2023 •
edited
Loading

Is it possible to fix this by:

  1. Using Int type to save CommitStatus in DB
  2. Using min in sql to get the CommitStatus instead of CalcCommitStatus
  3. Adding a pager bar in web ui

The logic now is loading all CommitStatus (limited 30) into memory from db, and get the minimum value of the status defined in CommitStatusPriorities in CalcCommitStatus.
If we use ListAll, all CommitStatus objects will be loaded into memory.
I think this can be done by db side, then we can avoid loading them into memory and calculating them.

@CaiCandong
Copy link
Member

CaiCandong commented Jul 27, 2023 •
edited
Loading

Is it possible to fix this by:

  1. Using Int type to save CommitStatus in DB
  2. Using min in sql to get the CommitStatus instead of CalcCommitStatus
  3. Adding a pager bar in web ui

The logic now is loading all CommitStatus (limited 30) into memory from db, and get the minimum value of the status defined in CommitStatusPriorities in CalcCommitStatus. If we use ListAll, all CommitStatus objects will be loaded into memory. I think this can be done by db side, then we can avoid loading them into memory and calculating them.

I agree with you. But this requires more changes to be made and also involves changes to the database structure.

silverwind pushed a commit that referenced this issue Jul 31, 2023
@CaiCandong
Fix pull request check list is limited (#26179)
983167c 
In the original implementation, we can only get the first 30 records of
the commit status (the default paging size), if the commit status is
more than 30, it will lead to the bug #25990. I made the following two
changes.
- On the page, use the ` db.ListOptions{ListAll: true}` parameter
instead of `db.ListOptions{}`
- The `GetLatestCommitStatus` function makes a determination as to
whether or not a pager is being used.

fixed #25990
@GiteaBot GiteaBot mentioned this issue Jul 31, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Jul 31, 2023
@CaiCandong @GiteaBot
Fix pull request check list is limited (go-gitea#26179)
7185815 
In the original implementation, we can only get the first 30 records of
the commit status (the default paging size), if the commit status is
more than 30, it will lead to the bug go-gitea#25990. I made the following two
changes.
- On the page, use the ` db.ListOptions{ListAll: true}` parameter
instead of `db.ListOptions{}`
- The `GetLatestCommitStatus` function makes a determination as to
whether or not a pager is being used.

fixed go-gitea#25990
@yp05327 yp05327 mentioned this issue Jul 31, 2023
Draft
silverwind pushed a commit that referenced this issue Jul 31, 2023
@GiteaBot @CaiCandong
Fix pull request check list is limited (#26179) (#26245)
0600269 
Backport #26179 by @CaiCandong

In the original implementation, we can only get the first 30 records of
the commit status (the default paging size), if the commit status is
more than 30, it will lead to the bug #25990. I made the following two
changes.
- On the page, use the ` db.ListOptions{ListAll: true}` parameter
instead of `db.ListOptions{}`
- The `GetLatestCommitStatus` function makes a determination as to
whether or not a pager is being used.

fixed #25990

Co-authored-by: caicandong <50507092+CaiCandong@users.noreply.github.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
No milestone
3 participants
@diorcety @yp05327 @CaiCandong