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.

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

WIP: Make it possible to delete stale review requests that incorrectly remain in DB #33356

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions models/issues/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
Expand Down Expand Up @@ -401,6 +402,7 @@ func (pr *PullRequest) getReviewedByLines(ctx context.Context, writer io.Writer)
Types: []ReviewType{ReviewTypeApprove},
IssueID: pr.IssueID,
OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
Dismissed: optional.Some(true),
})
if err != nil {
log.Error("Unable to FindReviews for PR ID %d: %v", pr.ID, err)
Expand Down
18 changes: 13 additions & 5 deletions models/issues/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
access_model "code.gitea.io/gitea/models/perm/access"
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/optional"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util"
Expand Down Expand Up @@ -392,6 +393,7 @@ func GetCurrentReview(ctx context.Context, reviewer *user_model.User, issue *Iss
Types: []ReviewType{ReviewTypePending},
IssueID: issue.ID,
ReviewerID: reviewer.ID,
Dismissed: optional.Some(true),
})
if err != nil {
return nil, err
Expand Down Expand Up @@ -534,12 +536,18 @@ func SubmitReview(ctx context.Context, doer *user_model.User, issue *Issue, revi
}

// GetReviewByIssueIDAndUserID get the latest review of reviewer for a pull request
func GetReviewByIssueIDAndUserID(ctx context.Context, issueID, userID int64) (*Review, error) {
func GetReviewByIssueIDAndUserID(ctx context.Context, issueID, userID int64, dismissed ...bool) (*Review, error) {
review := new(Review)

has, err := db.GetEngine(ctx).Where(
builder.In("type", ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest).
And(builder.Eq{"issue_id": issueID, "reviewer_id": userID, "original_author_id": 0})).
cond := builder.In("type", ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest).
And(builder.Eq{"issue_id": issueID, "reviewer_id": userID, "original_author_id": 0})

// apply optional filter for dismissed
if len(dismissed) != 0 {
cond = cond.And(builder.Eq{"dismissed": dismissed[0]})
}

has, err := db.GetEngine(ctx).Where(cond).
Desc("id").
Get(review)
if err != nil {
Expand Down Expand Up @@ -731,7 +739,7 @@ func RemoveReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user
}
defer committer.Close()

review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID)
review, err := GetReviewByIssueIDAndUserID(ctx, issue.ID, reviewer.ID, false)
if err != nil && !IsErrReviewNotExist(err) {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions models/issues/review_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ func GetReviewsByIssueID(ctx context.Context, issueID int64) (latestReviews, mig
reviews := make([]*Review, 0, 10)

// Get all reviews for the issue id
if err := db.GetEngine(ctx).Where("issue_id=?", issueID).OrderBy("updated_unix ASC").Find(&reviews); err != nil {
if err := db.GetEngine(ctx).Where("issue_id=? AND dismissed=?", issueID, false).OrderBy("updated_unix ASC").Find(&reviews); err != nil {
return nil, nil, err
}

Expand All @@ -175,7 +175,7 @@ func GetReviewsByIssueID(ctx context.Context, issueID int64) (latestReviews, mig
reviewTeamsMap := make(map[int64][]*Review) // key is reviewer team id
countedReivewTypes := []ReviewType{ReviewTypeApprove, ReviewTypeReject, ReviewTypeRequest}
for _, review := range reviews {
if review.ReviewerTeamID == 0 && slices.Contains(countedReivewTypes, review.Type) && !review.Dismissed {
if review.ReviewerTeamID == 0 && slices.Contains(countedReivewTypes, review.Type) {
if review.OriginalAuthorID != 0 {
originalReviewersMap[review.OriginalAuthorID] = append(originalReviewersMap[review.OriginalAuthorID], review)
} else {
Expand Down
2 changes: 2 additions & 0 deletions services/pull/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"code.gitea.io/gitea/modules/graceful"
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/optional"
repo_module "code.gitea.io/gitea/modules/repository"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"
Expand Down Expand Up @@ -1102,6 +1103,7 @@ func GetPullCommits(ctx *gitea_context.Context, issue *issues_model.Issue) ([]Co
issues_model.ReviewTypeComment,
issues_model.ReviewTypeReject,
},
Dismissed: optional.Some(false),
})

if err != nil && !issues_model.IsErrReviewNotExist(err) {
Expand Down
Loading