Ensure review dismissal only dismisses the correct review (go-gitea#1…

…5477) Backport go-gitea#15477 Fix go-gitea#15472 Signed-off-by: Andrew Thornton art27@cantab.net
zeripath · Apr 15, 2021 · 45a30e0 · 45a30e0
1 parent 00e55dd
commit 45a30e0
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 8 deletions.
diff --git a/models/review.go b/models/review.go
@@ -566,7 +566,11 @@ func DismissReview(review *Review, isDismiss bool) (err error) {
 
 	review.Dismissed = isDismiss
 
-	_, err = x.Cols("dismissed").Update(review)
+	if review.ID == 0 {
+		return ErrReviewNotExist{}
+	}
+
+	_, err = x.ID(review.ID).Cols("dismissed").Update(review)
 
 	return
 }

diff --git a/models/review_test.go b/models/review_test.go
@@ -143,11 +143,57 @@ func TestGetReviewersByIssueID(t *testing.T) {
 }
 
 func TestDismissReview(t *testing.T) {
-	review1 := AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
-	review2 := AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
-	assert.NoError(t, DismissReview(review1, true))
-	assert.NoError(t, DismissReview(review2, true))
-	assert.NoError(t, DismissReview(review2, true))
-	assert.NoError(t, DismissReview(review2, false))
-	assert.NoError(t, DismissReview(review2, false))
+	assert.NoError(t, PrepareTestDatabase())
+
+	rejectReviewExample := AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
+	requestReviewExample := AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
+	approveReviewExample := AssertExistsAndLoadBean(t, &Review{ID: 8}).(*Review)
+	assert.False(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+	assert.False(t, approveReviewExample.Dismissed)
+
+	assert.NoError(t, DismissReview(rejectReviewExample, true))
+	rejectReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
+	requestReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
+	assert.True(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+
+	assert.NoError(t, DismissReview(requestReviewExample, true))
+	rejectReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
+	requestReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
+	assert.True(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+	assert.False(t, approveReviewExample.Dismissed)
+
+	assert.NoError(t, DismissReview(requestReviewExample, true))
+	rejectReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
+	requestReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
+	assert.True(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+	assert.False(t, approveReviewExample.Dismissed)
+
+	assert.NoError(t, DismissReview(requestReviewExample, false))
+	rejectReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
+	requestReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
+	assert.True(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+	assert.False(t, approveReviewExample.Dismissed)
+
+	assert.NoError(t, DismissReview(requestReviewExample, false))
+	rejectReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 9}).(*Review)
+	requestReviewExample = AssertExistsAndLoadBean(t, &Review{ID: 11}).(*Review)
+	assert.True(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+	assert.False(t, approveReviewExample.Dismissed)
+
+	assert.NoError(t, DismissReview(rejectReviewExample, false))
+	assert.False(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+	assert.False(t, approveReviewExample.Dismissed)
+
+	assert.NoError(t, DismissReview(approveReviewExample, true))
+	assert.False(t, rejectReviewExample.Dismissed)
+	assert.False(t, requestReviewExample.Dismissed)
+	assert.True(t, approveReviewExample.Dismissed)
+
 }