Fix bug disallowing seeing changed files in PRs from forks

Author: delvh

routers/web/repo/pull.go

@@ -702,7 +702,7 @@ func ViewPullFiles(ctx *context.Context) {
 		methodWithError = "GetDiff"
 	} else {
 		diff, err = gitdiff.SyncAndGetUserSpecificDiff(ctx, ctx.Doer.ID, pull, gitRepo, diffOptions, files...)
-		methodWithError = "GetUserSpecificDiff"
+		methodWithError = "SyncAndGetUserSpecificDiff"
 	}
 	if err != nil {
 		ctx.ServerError(methodWithError, err)

services/gitdiff/gitdiff.go

@@ -1526,8 +1526,12 @@ func SyncAndGetUserSpecificDiff(ctx context.Context, userID int64, pull *models.
 		return diff, err
 	}
 
-	// Prepation to check for each file whether it was changed since the last review
-	changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, pull.HeadBranch)
+	latestCommit := opts.AfterCommitID
+	if latestCommit == "" {
+		latestCommit = pull.HeadBranch // opts.AfterCommitID is preferred because it handles PRs from forks correctly and the branch name doesn't
+	}
+
+	changedFiles, err := gitRepo.GetFilesChangedBetween(review.CommitSHA, latestCommit)
 	if err != nil {
 		return diff, err
 	}
@@ -1563,14 +1567,12 @@ 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.
-	// The update is performed in the background because it only affects future calls to this function and not the present one, and because it is quite an expensive operation
 	if len(filesChangedSinceLastDiff) > 0 {
-		go func() {
-			err := pulls.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)
-			}
-		}()
+		err := pulls.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
+		}
 	}
 
 	return diff, err