Support rendering all files for file-by-file reviews

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

• Close this issue

It was discussed at https://gitlab.com/gitlab-org/gitlab-ce/issues/40844#note_199603517 the limitation factor for MR diffs. Meaning that when hitting a diff collection limit and having a overflow we'll stop persisting merge_request_diff_files at some point through the collection, which will lead to a limited amount of diff files, even for a file-by-file review workflow.

From https://gitlab.com/gitlab-org/gitlab-ce/issues/40844#note_199603517:

Today these limits are mostly applied at the source (Gitaly), so what the client (Rails app) has today is a limited view of the files. In that sense, we might want to reconsider if keep limiting at the source (enforce_limits: true to CommitDiff RPC) upon merge_request_diff_files persistence still makes sense.

Possible solutions:

1. Use a enforce_limits=false upon MR diff files persistence, which will stop applying limits at the source (Gitaly), so we'll be capable of persisting all data at merge_request_diff_files. Making it possible to request any of those at /diff_for_path endpoint. The downside here is that if the object storage persistence is not enabled, we'll put more pressure at the DB size for this table, which doesn't seem ideal. The upside is that we'll be able to support file-by-file reviews for multiple MR versions (old merge_request_diffs).

2. Persist merge_request_diff_files with an empty diff column if it reaches a overflow. That solves the problem for falling back to Gitaly at /diff_for_path to fetch the diff data, though if we're looking forward to allow a file-by-file diff for multiple MR versions (old merge_request_diffs), it'll be a limited approach, given Gitaly won't keep track of all old refs forever.

So in general it depends on how we're looking forward to use this feature and iterate at it in the future. Today we delete old revisions merge_request_diff_files automatically when sending a new MR update (e.g. push). So we'd need to change that too, if we opt into the 1st solution for instance.