Commit 4bf1a144 authored by Andy Soiron's avatar Andy Soiron

Merge branch...

Merge branch 'kassio/remove-github_importer_use_diff_note_with_suggestions-feature-flag' into 'master'

Remove github_importer_use_diff_note_with_suggestions feature flag

See merge request gitlab-org/gitlab!79661
parents 8f7a5a74 0fce5dd2
---
name: github_importer_use_diff_note_with_suggestions
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/71765
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/344309
milestone: '14.5'
type: development
group: group::import
default_enabled: true
......@@ -26,7 +26,7 @@ The following aspects of a project are imported:
- Regular issue and pull request comments
- [Git Large File Storage (LFS) Objects](../../../topics/git/lfs/index.md)
- Pull request comments replies in discussions ([GitLab.com & 14.5+](https://gitlab.com/gitlab-org/gitlab/-/issues/336596))
- Diff Notes suggestions ([GitLab.com & 14.7+](https://gitlab.com/gitlab-org/gitlab/-/issues/340624)) [with a flag](../../../administration/feature_flags.md) named `github_importer_use_diff_note_with_suggestions`. Enabled by default.
- Diff Notes suggestions ([GitLab.com & 14.7+](https://gitlab.com/gitlab-org/gitlab/-/issues/340624))
References to pull requests and issues are preserved (GitLab.com & 8.7+), and
each imported repository maintains visibility level unless that [visibility
......
......@@ -26,7 +26,7 @@ module Gitlab
# because it cannot use the BulkImporting strategy, which skips
# callbacks and validations. For this reason, notes that don't have
# suggestions are still imported with LegacyDiffNote
if import_with_diff_note?
if note.contains_suggestion?
import_with_diff_note
else
import_with_legacy_diff_note
......@@ -48,17 +48,6 @@ module Gitlab
attr_reader :note, :project, :client, :author_id, :author_found
def import_with_diff_note?
note.contains_suggestion? && use_diff_note_with_suggestions_enabled?
end
def use_diff_note_with_suggestions_enabled?
Feature.enabled?(
:github_importer_use_diff_note_with_suggestions,
default_enabled: :yaml
)
end
def build_author_attributes
@author_id, @author_found = user_finder.author_id_for(note)
end
......
......@@ -119,123 +119,80 @@ RSpec.describe Gitlab::GithubImport::Importer::DiffNoteImporter, :aggregate_fail
.and_return(discussion_id)
end
context 'when github_importer_use_diff_note_with_suggestions is disabled' do
before do
stub_feature_flags(github_importer_use_diff_note_with_suggestions: false)
it_behaves_like 'diff notes without suggestion'
context 'when the note has suggestions' do
let(:note_body) do
<<~EOB
Suggestion:
```suggestion
what do you think to do it like this
```
EOB
end
it_behaves_like 'diff notes without suggestion'
before do
stub_user_finder(user.id, true)
end
context 'when the note has suggestions' do
let(:note_body) do
<<~EOB
it 'imports the note as diff note' do
expect { subject.execute }
.to change(DiffNote, :count)
.by(1)
note = project.notes.diff_notes.take
expect(note).to be_valid
expect(note.noteable_type).to eq('MergeRequest')
expect(note.noteable_id).to eq(merge_request.id)
expect(note.project_id).to eq(project.id)
expect(note.author_id).to eq(user.id)
expect(note.system).to eq(false)
expect(note.discussion_id).to eq(discussion_id)
expect(note.commit_id).to eq('original123abc')
expect(note.line_code).to eq(note_representation.line_code)
expect(note.type).to eq('DiffNote')
expect(note.created_at).to eq(created_at)
expect(note.updated_at).to eq(updated_at)
expect(note.position.to_h).to eq({
base_sha: merge_request.diffs.diff_refs.base_sha,
head_sha: merge_request.diffs.diff_refs.head_sha,
start_sha: merge_request.diffs.diff_refs.start_sha,
new_line: 15,
old_line: nil,
new_path: file_path,
old_path: file_path,
position_type: 'text',
line_range: nil
})
expect(note.note)
.to eq <<~NOTE
Suggestion:
```suggestion
```suggestion:-0+0
what do you think to do it like this
```
EOB
end
it 'imports the note' do
stub_user_finder(user.id, true)
expect { subject.execute }
.to change(LegacyDiffNote, :count)
.and not_change(DiffNote, :count)
note = project.notes.diff_notes.take
expect(note).to be_valid
expect(note.note)
.to eq <<~NOTE
Suggestion:
```suggestion:-0+0
what do you think to do it like this
```
NOTE
end
end
end
context 'when github_importer_use_diff_note_with_suggestions is enabled' do
before do
stub_feature_flags(github_importer_use_diff_note_with_suggestions: true)
NOTE
end
it_behaves_like 'diff notes without suggestion'
context 'when the note diff file creation fails' do
it 'falls back to the LegacyDiffNote' do
exception = ::DiffNote::NoteDiffFileCreationError.new('Failed to create diff note file')
context 'when the note has suggestions' do
let(:note_body) do
<<~EOB
Suggestion:
```suggestion
what do you think to do it like this
```
EOB
end
expect_next_instance_of(::Import::Github::Notes::CreateService) do |service|
expect(service)
.to receive(:execute)
.and_raise(exception)
end
before do
stub_user_finder(user.id, true)
end
expect(Gitlab::GithubImport::Logger)
.to receive(:warn)
.with(
message: 'Failed to create diff note file',
'error.class': 'DiffNote::NoteDiffFileCreationError'
)
it 'imports the note as diff note' do
expect { subject.execute }
.to change(DiffNote, :count)
.by(1)
note = project.notes.diff_notes.take
expect(note).to be_valid
expect(note.noteable_type).to eq('MergeRequest')
expect(note.noteable_id).to eq(merge_request.id)
expect(note.project_id).to eq(project.id)
expect(note.author_id).to eq(user.id)
expect(note.system).to eq(false)
expect(note.discussion_id).to eq(discussion_id)
expect(note.commit_id).to eq('original123abc')
expect(note.line_code).to eq(note_representation.line_code)
expect(note.type).to eq('DiffNote')
expect(note.created_at).to eq(created_at)
expect(note.updated_at).to eq(updated_at)
expect(note.position.to_h).to eq({
base_sha: merge_request.diffs.diff_refs.base_sha,
head_sha: merge_request.diffs.diff_refs.head_sha,
start_sha: merge_request.diffs.diff_refs.start_sha,
new_line: 15,
old_line: nil,
new_path: file_path,
old_path: file_path,
position_type: 'text',
line_range: nil
})
expect(note.note)
.to eq <<~NOTE
Suggestion:
```suggestion:-0+0
what do you think to do it like this
```
NOTE
end
context 'when the note diff file creation fails' do
it 'falls back to the LegacyDiffNote' do
exception = ::DiffNote::NoteDiffFileCreationError.new('Failed to create diff note file')
expect_next_instance_of(::Import::Github::Notes::CreateService) do |service|
expect(service)
.to receive(:execute)
.and_raise(exception)
end
expect(Gitlab::GithubImport::Logger)
.to receive(:warn)
.with(
message: 'Failed to create diff note file',
'error.class': 'DiffNote::NoteDiffFileCreationError'
)
expect { subject.execute }
.to change(LegacyDiffNote, :count)
.and not_change(DiffNote, :count)
end
.to change(LegacyDiffNote, :count)
.and not_change(DiffNote, :count)
end
end
end
......
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