Update image diff comments when the file type change
Summary
In an MR, when there are comments on an image and, in a further push, the file's type change, those comments don't get updated and still reference an image diff. This bug raises the following error:
Completed 500 Internal Server Error in 169ms (ActiveRecord: 20.1ms | Elasticsearch: 0.0ms)
NoMethodError (undefined method `line_age' for #<Gitlab::Diff::Formatters::ImageFormatter:0x00007fc406f4ea58>):
lib/gitlab/diff/position.rb:79:in `type'
lib/gitlab/metrics/instrumentation.rb:161:in `block in type'
lib/gitlab/metrics/method_call.rb:36:in `measure'
lib/gitlab/metrics/instrumentation.rb:161:in `type'
lib/gitlab/diff/position.rb:83:in `unchanged?'
lib/gitlab/metrics/instrumentation.rb:161:in `block in unchanged?'
lib/gitlab/metrics/method_call.rb:36:in `measure'
lib/gitlab/metrics/instrumentation.rb:161:in `unchanged?'
lib/gitlab/diff/lines_unfolder.rb:57:in `block in unfold_required?'
lib/gitlab/utils/strong_memoize.rb:28:in `strong_memoize'
lib/gitlab/diff/lines_unfolder.rb:55:in `unfold_required?'
lib/gitlab/metrics/instrumentation.rb:161:in `block in unfold_required?'
lib/gitlab/metrics/method_call.rb:36:in `measure'
lib/gitlab/metrics/instrumentation.rb:161:in `unfold_required?'
lib/gitlab/diff/lines_unfolder.rb:157:in `calculate_from_blob_line!'
lib/gitlab/metrics/instrumentation.rb:161:in `block in calculate_from_blob_line!'
lib/gitlab/metrics/method_call.rb:36:in `measure'
lib/gitlab/metrics/instrumentation.rb:161:in `calculate_from_blob_line!'
lib/gitlab/diff/lines_unfolder.rb:21:in `initialize'
lib/gitlab/metrics/instrumentation.rb:161:in `block in initialize'
lib/gitlab/metrics/method_call.rb:36:in `measure'
lib/gitlab/metrics/instrumentation.rb:161:in `initialize'
lib/gitlab/diff/file.rb:175:in `new'
lib/gitlab/diff/file.rb:175:in `unfold_diff_lines'
lib/gitlab/metrics/instrumentation.rb:161:in `block in unfold_diff_lines'
lib/gitlab/metrics/method_call.rb:36:in `measure'
lib/gitlab/metrics/instrumentation.rb:161:in `unfold_diff_lines'
lib/gitlab/diff/file_collection/base.rb:43:in `block (2 levels) in unfold_diff_files'
lib/gitlab/diff/file_collection/base.rb:43:in `each'
lib/gitlab/diff/file_collection/base.rb:43:in `block in unfold_diff_files'
lib/gitlab/git/diff_collection.rb:43:in `each'
lib/gitlab/git/diff_collection.rb:43:in `each'
lib/gitlab/metrics/instrumentation.rb:161:in `block in each'
lib/gitlab/metrics/method_call.rb:36:in `measure'
lib/gitlab/metrics/instrumentation.rb:161:in `each'
lib/gitlab/diff/file_collection/base.rb:41:in `unfold_diff_files'
app/controllers/projects/merge_requests/diffs_controller.rb:29:in `render_diffs'
app/controllers/projects/merge_requests/diffs_controller.rb:16:in `show'
lib/gitlab/i18n.rb:55:in `with_locale'
lib/gitlab/i18n.rb:61:in `with_user_locale'
app/controllers/application_controller.rb:417:in `set_locale'
lib/gitlab/middleware/rails_queue_duration.rb:24:in `call'
lib/gitlab/metrics/rack_middleware.rb:17:in `block in call'
lib/gitlab/metrics/transaction.rb:55:in `run'
lib/gitlab/metrics/rack_middleware.rb:17:in `call'
lib/gitlab/middleware/multipart.rb:103:in `call'
lib/gitlab/request_profiler/middleware.rb:16:in `call'
ee/lib/gitlab/jira/middleware.rb:15:in `call'
lib/gitlab/middleware/go.rb:20:in `call'
lib/gitlab/etag_caching/middleware.rb:13:in `call'
lib/gitlab/middleware/correlation_id.rb:16:in `block in call'
lib/gitlab/correlation_id.rb:15:in `use_id'
lib/gitlab/middleware/correlation_id.rb:15:in `call'
lib/gitlab/middleware/read_only/controller.rb:42:in `call'
lib/gitlab/middleware/read_only.rb:18:in `call'
lib/gitlab/middleware/basic_health_check.rb:25:in `call'
lib/gitlab/request_context.rb:20:in `call'
lib/gitlab/metrics/requests_rack_middleware.rb:29:in `call'
lib/gitlab/middleware/release_env.rb:13:in `call'
In the error, we can see that the object that raises the error is Gitlab::Diff::Formatters::ImageFormatter
. This is because, when the Gitlab::Diff::Position
is initialized, the position_type
is still image
and the diff assume that the proper formatter is the image one.
Steps to reproduce
- Create an MR with images
- Make a comment on the image
- Replace the image file with, for example, a symlink with the same name.
- Reload the page.
What is the expected correct behavior?
If the file type has changed, we shouldn't try to display the old diff comments. Those comments should be outdated and let them be part of the old diff.
/cc @DouweM