Fix incorrect viewed files counter if file has changed (#36009)

File changes since last review didn't decrease the viewed files counter

---
<img width="440" height="178" alt="image"
src="https://github.com/user-attachments/assets/da34fcf4-452f-4f71-8da2-97edbfc31fdd"
/>

Also reported here ->
https://github.com/go-gitea/gitea/issues/35803#issuecomment-3567850285

---------

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>

models/pull/review_state.go

@@ -73,18 +73,18 @@ func GetReviewState(ctx context.Context, userID, pullID int64, commitSHA string)
 
 // UpdateReviewState updates the given review inside the database, regardless of whether it existed before or not
 // The given map of files with their viewed state will be merged with the previous review, if present
-func UpdateReviewState(ctx context.Context, userID, pullID int64, commitSHA string, updatedFiles map[string]ViewedState) error {
+func UpdateReviewState(ctx context.Context, userID, pullID int64, commitSHA string, updatedFiles map[string]ViewedState) (*ReviewState, error) {
 	log.Trace("Updating review for user %d, repo %d, commit %s with the updated files %v.", userID, pullID, commitSHA, updatedFiles)
 
 	review, exists, err := GetReviewState(ctx, userID, pullID, commitSHA)
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	if exists {
 		review.UpdatedFiles = mergeFiles(review.UpdatedFiles, updatedFiles)
 	} else if previousReview, err := getNewestReviewStateApartFrom(ctx, userID, pullID, commitSHA); err != nil {
-		return err
+		return nil, err
 
 		// Overwrite the viewed files of the previous review if present
 	} else if previousReview != nil {
@@ -98,11 +98,11 @@ func UpdateReviewState(ctx context.Context, userID, pullID int64, commitSHA stri
 	if !exists {
 		log.Trace("Inserting new review for user %d, repo %d, commit %s with the updated files %v.", userID, pullID, commitSHA, review.UpdatedFiles)
 		_, err := engine.Insert(review)
-		return err
+		return nil, err
 	}
 	log.Trace("Updating already existing review with ID %d (user %d, repo %d, commit %s) with the updated files %v.", review.ID, userID, pullID, commitSHA, review.UpdatedFiles)
-	_, err = engine.ID(review.ID).Update(&ReviewState{UpdatedFiles: review.UpdatedFiles})
-	return err
+	_, err = engine.ID(review.ID).Cols("updated_files").Update(review)
+	return review, err
 }
 
 // mergeFiles merges the given maps of files with their viewing state into one map.

routers/web/repo/pull_review.go

@@ -331,7 +331,7 @@ func UpdateViewedFiles(ctx *context.Context) {
 		updatedFiles[file] = state
 	}
 
-	if err := pull_model.UpdateReviewState(ctx, ctx.Doer.ID, pull.ID, data.HeadCommitSHA, updatedFiles); err != nil {
+	if _, err := pull_model.UpdateReviewState(ctx, ctx.Doer.ID, pull.ID, data.HeadCommitSHA, updatedFiles); err != nil {
 		ctx.ServerError("UpdateReview", err)
 	}
 }

services/gitdiff/gitdiff.go

@@ -1434,15 +1434,17 @@ outer:
 		}
 	}
 
-	// Explicitly store files that have changed in the database, if any is present at all.
-	// This has the benefit that the "Has Changed" attribute will be present as long as the user does not explicitly mark this file as viewed, so it will even survive a page reload after marking another file as viewed.
-	// On the other hand, this means that even if a commit reverting an unseen change is committed, the file will still be seen as changed.
 	if len(filesChangedSinceLastDiff) > 0 {
-		err := pull_model.UpdateReviewState(ctx, review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff)
+		// Explicitly store files that have changed in the database, if any is present at all.
+		// This has the benefit that the "Has Changed" attribute will be present as long as the user does not explicitly mark this file as viewed, so it will even survive a page reload after marking another file as viewed.
+		// On the other hand, this means that even if a commit reverting an unseen change is committed, the file will still be seen as changed.
+		updatedReview, err := pull_model.UpdateReviewState(ctx, review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff)
 		if err != nil {
 			log.Warn("Could not update review for user %d, pull %d, commit %s and the changed files %v: %v", review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff, err)
 			return nil, err
 		}
+		// Update the local review to reflect the changes immediately
+		review = updatedReview
 	}
 
 	return review, nil