From 2bba3bbeeb24f180a1c56a44dc008c4b09437966 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 20 Dec 2023 11:56:27 +0100 Subject: [PATCH 1/7] RequestReview get deleted on review. So we dont have to try to load them on comments. --- models/issues/comment_list.go | 3 +++ models/issues/review.go | 3 +-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 93af45870ed69..a98e66ed5fdea 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -428,6 +428,9 @@ func (comments CommentList) loadReviews(ctx context.Context) error { } for _, comment := range comments { + if comment.Type == CommentTypeReviewRequest { + continue + } comment.Review = reviews[comment.ReviewID] if comment.Review == nil { if comment.ReviewID > 0 { diff --git a/models/issues/review.go b/models/issues/review.go index 3db73a09ebcb7..4437d16295189 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -595,7 +595,7 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo } } - review, err = CreateReview(ctx, CreateReviewOptions{ + _, err = CreateReview(ctx, CreateReviewOptions{ Type: ReviewTypeRequest, Issue: issue, Reviewer: reviewer, @@ -613,7 +613,6 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo Issue: issue, RemovedAssignee: false, // Use RemovedAssignee as !isRequest AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID - ReviewID: review.ID, }) if err != nil { return nil, err From 3c21bd769cc2acef073715ee38259f079e8874a4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Wed, 20 Dec 2023 12:49:32 +0100 Subject: [PATCH 2/7] func caller use the created comment to retrieve created review too --- models/issues/comment.go | 3 +++ models/issues/review.go | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/models/issues/comment.go b/models/issues/comment.go index ba5aed9c652e9..97255dffeea42 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -696,6 +696,9 @@ func (c *Comment) LoadReactions(ctx context.Context, repo *repo_model.Repository } func (c *Comment) loadReview(ctx context.Context) (err error) { + if c.ReviewID == 0 { + return nil + } if c.Review == nil { if c.Review, err = GetReviewByID(ctx, c.ReviewID); err != nil { return err diff --git a/models/issues/review.go b/models/issues/review.go index 4437d16295189..7b47682b2b3e1 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -595,7 +595,7 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo } } - _, err = CreateReview(ctx, CreateReviewOptions{ + review, err = CreateReview(ctx, CreateReviewOptions{ Type: ReviewTypeRequest, Issue: issue, Reviewer: reviewer, @@ -618,6 +618,9 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo return nil, err } + // func caller use the created comment to retrieve created review too. + comment.Review = review + return comment, committer.Commit() } From 372922183a93ba0dd3ee0f2dfd1505084a7d02b4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 30 Dec 2023 22:40:57 +0100 Subject: [PATCH 3/7] add back --- models/issues/review.go | 1 + 1 file changed, 1 insertion(+) diff --git a/models/issues/review.go b/models/issues/review.go index 3737311f6ba4d..32d61f36b4380 100644 --- a/models/issues/review.go +++ b/models/issues/review.go @@ -615,6 +615,7 @@ func AddReviewRequest(ctx context.Context, issue *Issue, reviewer, doer *user_mo Issue: issue, RemovedAssignee: false, // Use RemovedAssignee as !isRequest AssigneeID: reviewer.ID, // Use AssigneeID as reviewer ID + ReviewID: review.ID, }) if err != nil { return nil, err From ec62290945d03e697c0947623110c780cac75109 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 13 Jan 2024 02:44:31 +0100 Subject: [PATCH 4/7] add note why we do skip it --- models/issues/comment_list.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index a98e66ed5fdea..d664259687dec 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -428,6 +428,9 @@ func (comments CommentList) loadReviews(ctx context.Context) error { } for _, comment := range comments { + // skip review request as the comment struct already has all the info needed to display the infos. + // also it would throw errors for all review requests who has been replaced by actual reviews + // as they do not exist in database anymore. if comment.Type == CommentTypeReviewRequest { continue } From 34ea361e66e3ec963b3e29fdd3739b916a4c8461 Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 12 Feb 2024 22:15:21 +0100 Subject: [PATCH 5/7] ignore error --- models/issues/comment.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/models/issues/comment.go b/models/issues/comment.go index 2cdb5d3e25780..a586caf1b57e0 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -700,6 +700,10 @@ func (c *Comment) loadReview(ctx context.Context) (err error) { } if c.Review == nil { if c.Review, err = GetReviewByID(ctx, c.ReviewID); err != nil { + // review request which has been replaced by actual reviews doesn't exist in database anymore, so ignorem them. + if c.Type == CommentTypeReviewRequest { + return nil + } return err } } From ba24e2699ab658ce3b161365cb5b1094cf551fbe Mon Sep 17 00:00:00 2001 From: "m.huber" Date: Mon, 12 Feb 2024 22:40:20 +0100 Subject: [PATCH 6/7] ignore error msg --- models/issues/comment_list.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index d664259687dec..9f62fa150f889 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -436,7 +436,8 @@ func (comments CommentList) loadReviews(ctx context.Context) error { } comment.Review = reviews[comment.ReviewID] if comment.Review == nil { - if comment.ReviewID > 0 { + // review request which has been replaced by actual reviews doesn't exist in database anymore, so don't log errors for them. + if comment.ReviewID > 0 && comment.Type != CommentTypeReviewRequest { log.Error("comment with review id [%d] but has no review record", comment.ReviewID) } continue From 2d0d3b43d43a0921a4f2ca276888f127599a2730 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Mon, 12 Feb 2024 22:42:09 +0100 Subject: [PATCH 7/7] Update models/issues/comment_list.go --- models/issues/comment_list.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/models/issues/comment_list.go b/models/issues/comment_list.go index 9f62fa150f889..cb7df3270d6ad 100644 --- a/models/issues/comment_list.go +++ b/models/issues/comment_list.go @@ -428,12 +428,6 @@ func (comments CommentList) loadReviews(ctx context.Context) error { } for _, comment := range comments { - // skip review request as the comment struct already has all the info needed to display the infos. - // also it would throw errors for all review requests who has been replaced by actual reviews - // as they do not exist in database anymore. - if comment.Type == CommentTypeReviewRequest { - continue - } comment.Review = reviews[comment.ReviewID] if comment.Review == nil { // review request which has been replaced by actual reviews doesn't exist in database anymore, so don't log errors for them.