diff --git a/lib/gitlab/github_import/importer/pull_request_review_importer.rb b/lib/gitlab/github_import/importer/pull_request_review_importer.rb index f476ee13392fbb61c44133b3ecae943a9908e691..dd5b7c93ced7154265ee792bb506a1c343d93e97 100644 --- a/lib/gitlab/github_import/importer/pull_request_review_importer.rb +++ b/lib/gitlab/github_import/importer/pull_request_review_importer.rb @@ -69,8 +69,8 @@ def note_attributes(author_id, note, extra = {}) author_id: author_id, note: note, system: false, - created_at: review.submitted_at, - updated_at: review.submitted_at + created_at: submitted_at, + updated_at: submitted_at }.merge(extra) end @@ -80,8 +80,8 @@ def add_approval!(user_id) approval_attribues = { merge_request_id: merge_request.id, user_id: user_id, - created_at: review.submitted_at, - updated_at: review.submitted_at + created_at: submitted_at, + updated_at: submitted_at } result = ::Approval.insert( @@ -105,6 +105,10 @@ def add_approval_system_note!(user_id) Note.create!(attributes) end + + def submitted_at + @submitted_at ||= (review.submitted_at || merge_request.updated_at) + end end end end diff --git a/lib/gitlab/github_import/representation/pull_request_review.rb b/lib/gitlab/github_import/representation/pull_request_review.rb index 3205259a1ededa75d82cbc8f7e07b4c1990fd562..08b3160fc4cc3ae4cdbe11542aadee3059085ba4 100644 --- a/lib/gitlab/github_import/representation/pull_request_review.rb +++ b/lib/gitlab/github_import/representation/pull_request_review.rb @@ -29,7 +29,7 @@ def self.from_json_hash(raw_hash) hash = Representation.symbolize_hash(raw_hash) hash[:author] &&= Representation::User.from_json_hash(hash[:author]) - hash[:submitted_at] = Time.parse(hash[:submitted_at]).in_time_zone + hash[:submitted_at] = Time.parse(hash[:submitted_at]).in_time_zone if hash[:submitted_at].present? new(hash) end diff --git a/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb b/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb index fa8b5e6ccf065b8641d8580b9df56b534df71847..a6da40f47f1c15d15273c403b5317127d99cdaea 100644 --- a/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb +++ b/spec/lib/gitlab/github_import/importer/pull_request_review_importer_spec.rb @@ -167,6 +167,19 @@ end end + context 'when the submitted_at is not provided' do + let(:review) { create_review(type: 'APPROVED', note: '', submitted_at: nil) } + + it 'creates a note for the review without the author information' do + expect { subject.execute }.to change(Note, :count).by(1) + + last_note = merge_request.notes.last + + expect(last_note.created_at) + .to be_within(1.second).of(merge_request.updated_at) + end + end + context 'when the review has a note text' do context 'when the review is "APPROVED"' do let(:review) { create_review(type: 'APPROVED') } @@ -215,13 +228,15 @@ end end - def create_review(type:, note: 'note', author: { id: 999, login: 'author' }) + def create_review(type:, **extra) Gitlab::GithubImport::Representation::PullRequestReview.from_json_hash( - merge_request_id: merge_request.id, - review_type: type, - note: note, - submitted_at: submitted_at.to_s, - author: author + extra.reverse_merge( + author: { id: 999, login: 'author' }, + merge_request_id: merge_request.id, + review_type: type, + note: 'note', + submitted_at: submitted_at.to_s + ) ) end end diff --git a/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb index f976345546846a75903c95d75cde3bcc0fe0ca39..cad9b13774ef120dc43a2604f48b6dd80bbc07cd 100644 --- a/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb +++ b/spec/lib/gitlab/github_import/representation/pull_request_review_spec.rb @@ -68,5 +68,11 @@ expect(review.author).to be_nil end + + it 'does not fail when submitted_at is blank' do + review = described_class.from_json_hash(hash.except('submitted_at')) + + expect(review.submitted_at).to be_nil + end end end