Commit 041f190f authored by Igor Drozdov's avatar Igor Drozdov

Merge branch '33867-impossible-to-comment-on-a-diff-with-a-former-symlink-2' into 'master'

Add file_identifier_hash to diff file

Closes #33867

See merge request gitlab-org/gitlab!30680
parents 8d605594 ab03cd6e
......@@ -67,10 +67,8 @@ class DiffFileBaseEntity < Grape::Entity
end
end
expose :file_hash do |diff_file|
Digest::SHA1.hexdigest(diff_file.file_path)
end
expose :file_identifier_hash
expose :file_hash
expose :file_path
expose :old_path
expose :new_path
......
......@@ -7,7 +7,6 @@ class DiffFileMetadataEntity < Grape::Entity
expose :old_path
expose :new_file?, as: :new_file
expose :deleted_file?, as: :deleted_file
expose :file_hash do |diff_file|
Digest::SHA1.hexdigest(diff_file.file_path)
end
expose :file_identifier_hash
expose :file_hash
end
......@@ -28,6 +28,8 @@ class DraftNote < ApplicationRecord
scope :authored_by, ->(u) { where(author_id: u.id) }
delegate :file_path, :file_hash, :file_identifier_hash, to: :diff_file, allow_nil: true
def self.positions
where.not(position: nil)
.select(:position)
......@@ -90,18 +92,6 @@ class DraftNote < ApplicationRecord
@line_code ||= diff_file&.line_code_for_position(original_position)
end
def file_hash
return unless diff_file
Digest::SHA1.hexdigest(diff_file.file_path)
end
def file_path
return unless diff_file
diff_file.file_path
end
def publish_params
attrs = PUBLISH_ATTRS.dup
attrs.concat(DIFF_ATTRS) if on_diff?
......
......@@ -7,6 +7,7 @@ class DraftNoteEntity < Grape::Entity
expose :merge_request_id
expose :position, if: -> (note, _) { note.on_diff? }
expose :line_code
expose :file_identifier_hash
expose :file_hash
expose :file_path
expose :note
......
......@@ -79,6 +79,7 @@ describe Projects::MergeRequests::DraftsController do
expect(response).to have_gitlab_http_status(:ok)
expect(json_response['position']).to be_present
expect(json_response['file_hash']).to be_present
expect(json_response['file_identifier_hash']).to be_present
expect(json_response['line_code']).to match(/\w+_\d+_\d+/)
expect(json_response['note_html']).to eq('<p dir="auto">This is a unpublished comment</p>')
end
......
......@@ -5,7 +5,38 @@ require 'spec_helper'
describe DraftNote do
include RepoHelpers
let(:project) { create(:project, :repository) }
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
describe 'validations' do
it_behaves_like 'a valid diff positionable note', :draft_note
end
describe 'delegations' do
it { is_expected.to delegate_method(:file_path).to(:diff_file).allow_nil }
it { is_expected.to delegate_method(:file_hash).to(:diff_file).allow_nil }
it { is_expected.to delegate_method(:file_identifier_hash).to(:diff_file).allow_nil }
end
describe '#diff_file' do
let(:draft_note) { build(:draft_note, merge_request: merge_request) }
context 'when diff_file exists' do
it "returns an unfolded diff_file" do
diff_file = instance_double(Gitlab::Diff::File)
expect(draft_note.original_position).to receive(:diff_file).with(project.repository).and_return(diff_file)
expect(diff_file).to receive(:unfold_diff_lines).with(draft_note.original_position)
expect(draft_note.diff_file).to be diff_file
end
end
context 'when diff_file does not exist' do
it 'returns nil' do
expect(draft_note.original_position).to receive(:diff_file).with(project.repository).and_return(nil)
expect(draft_note.diff_file).to be_nil
end
end
end
end
......@@ -225,6 +225,10 @@ module Gitlab
new_path.presence || old_path
end
def file_hash
Digest::SHA1.hexdigest(file_path)
end
def added_lines
@stats&.additions || diff_lines.count(&:added?)
end
......@@ -237,6 +241,10 @@ module Gitlab
"#{file_path}-#{new_file?}-#{deleted_file?}-#{renamed_file?}"
end
def file_identifier_hash
Digest::SHA1.hexdigest(file_identifier)
end
def diffable?
repository.attributes(file_path).fetch('diff') { true }
end
......
......@@ -282,6 +282,18 @@ describe Gitlab::Diff::File do
end
end
describe '#file_hash' do
it 'returns a hash of file_path' do
expect(diff_file.file_hash).to eq(Digest::SHA1.hexdigest(diff_file.file_path))
end
end
describe '#file_identifier_hash' do
it 'returns a hash of file_identifier' do
expect(diff_file.file_identifier_hash).to eq(Digest::SHA1.hexdigest(diff_file.file_identifier))
end
end
context 'diff file stats' do
let(:diff_file) do
described_class.new(diff,
......
......@@ -8,7 +8,7 @@ RSpec.shared_examples 'diff file base entity' do
:file_hash, :file_path, :old_path, :new_path,
:viewer, :diff_refs, :stored_externally,
:external_storage, :renamed_file, :deleted_file,
:a_mode, :b_mode, :new_file)
:a_mode, :b_mode, :new_file, :file_identifier_hash)
end
# Converted diff files from GitHub import does not contain blob file
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment