Ensure review dismissal only dismisses the correct review (#15477)

Fix #15472

Signed-off-by: Andrew Thornton art27@cantab.net

diff --git a/models/review.go b/models/review.go
index 702e9634ef799a6e867dcdaa85eb0ba19b8e7d62..343621c0fa5ab323cd4900da30806813039e419e 100644 (file)
--- 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
index 4f049b45e39d79317ea5527e60fdaf805c158e47..accc1841933eccd1d3a3671edf7bab1f9944561e 100644 (file)
--- 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)
+
 }