Commit 7c6429a6 authored by Kassio Borges's avatar Kassio Borges

BulkImports: Remove code duplication on Epic dependent pipelines

To avoid the current duplication on the Epic Dependent pipelines, the
duplicated logic was moved to the parent class. This way, the navigation
among epics and data pages is all done in just one place.
parent 3bd11feb
# frozen_string_literal: true
module EE
module BulkImports
module Groups
module Loaders
class EpicAwardEmojiLoader
NotAllowedError = Class.new(StandardError)
# rubocop: disable CodeReuse/ActiveRecord
def load(context, data)
return unless data
epic = context.group.epics.find_by(iid: context.extra[:epic_iid])
return if award_emoji_exists?(epic, data)
raise NotAllowedError unless Ability.allowed?(context.current_user, :award_emoji, epic)
epic.award_emoji.create!(data)
end
private
def award_emoji_exists?(epic, data)
epic.award_emoji.exists?(user_id: data['user_id'], name: data['name'])
end
# rubocop: enable CodeReuse/ActiveRecord
end
end
end
end
end
......@@ -4,38 +4,30 @@ module EE
module BulkImports
module Groups
module Pipelines
class EpicAwardEmojiPipeline
include ::BulkImports::Pipeline
class EpicAwardEmojiPipeline < EE::BulkImports::Pipeline::EpicBase
extractor ::BulkImports::Common::Extractors::GraphqlExtractor,
query: EE::BulkImports::Groups::Graphql::GetEpicAwardEmojiQuery
transformer ::BulkImports::Common::Transformers::ProhibitedAttributesTransformer
transformer ::BulkImports::Common::Transformers::UserReferenceTransformer
loader EE::BulkImports::Groups::Loaders::EpicAwardEmojiLoader
# rubocop: disable CodeReuse/ActiveRecord
def initialize(context)
@context = context
@group = context.group
@epic_iids = @group.epics.order(iid: :desc).pluck(:iid)
def load(context, data)
return unless data
set_next_epic
end
epic = context.group.epics.find_by(iid: context.extra[:epic_iid])
private
return if award_emoji_exists?(epic, data)
def after_run(extracted_data)
set_next_epic unless extracted_data.has_next_page?
raise NotAllowedError unless Ability.allowed?(context.current_user, :award_emoji, epic)
if extracted_data.has_next_page? || context.extra[:epic_iid]
run
end
epic.award_emoji.create!(data)
end
def set_next_epic
context.extra[:epic_iid] = @epic_iids.pop
private
def award_emoji_exists?(epic, data)
epic.award_emoji.exists?(user_id: data['user_id'], name: data['name'])
end
# rubocop: enable CodeReuse/ActiveRecord
end
......
......@@ -4,23 +4,13 @@ module EE
module BulkImports
module Groups
module Pipelines
class EpicEventsPipeline
include ::BulkImports::Pipeline
class EpicEventsPipeline < EE::BulkImports::Pipeline::EpicBase
extractor ::BulkImports::Common::Extractors::GraphqlExtractor,
query: EE::BulkImports::Groups::Graphql::GetEpicEventsQuery
transformer ::BulkImports::Common::Transformers::ProhibitedAttributesTransformer
transformer ::BulkImports::Common::Transformers::UserReferenceTransformer, reference: 'author'
def initialize(context)
@context = context
@group = context.group
@epic_iids = @group.epics.order(iid: :desc).pluck(:iid) # rubocop: disable CodeReuse/ActiveRecord
set_next_epic
end
def transform(context, data)
# Only create 'reopened' & 'closed' events.
# 'created' event get created when epic is persisted.
......@@ -49,18 +39,6 @@ module EE
private
def after_run(extracted_data)
set_next_epic unless extracted_data.has_next_page?
if extracted_data.has_next_page? || context.extra[:epic_iid]
run
end
end
def set_next_epic
context.extra[:epic_iid] = @epic_iids.pop
end
def create_event!(epic, data)
epic.events.create!(data)
end
......
# frozen_string_literal: true
module EE
module BulkImports
module Pipeline
class EpicBase
include ::BulkImports::Pipeline
def initialize(...)
super
@epic_iids = context.group.epics.order(iid: :desc).pluck(:iid) # rubocop: disable CodeReuse/ActiveRecord
set_next_epic
end
def run
return skip!('Skipping because group has no epics') if current_epic_iid.blank?
super
end
private
attr_reader :epic_iids
def after_run(extracted_data)
set_next_epic unless extracted_data.has_next_page?
if has_next_page_or_next_epic?(extracted_data)
run
end
end
def set_next_epic
context.extra[:epic_iid] = epic_iids.pop
end
def has_next_page_or_next_epic?(extracted_data)
extracted_data.has_next_page? || current_epic_iid
end
def current_epic_iid
context.extra[:epic_iid]
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe EE::BulkImports::Groups::Loaders::EpicAwardEmojiLoader do
describe '#load' do
let_it_be(:user) { create(:user) }
let_it_be(:group) { create(:group) }
let_it_be(:epic) { create(:epic, group: group, iid: 1) }
let_it_be(:bulk_import) { create(:bulk_import, user: user) }
let_it_be(:entity) { create(:bulk_import_entity, bulk_import: bulk_import, group: group) }
let_it_be(:tracker) { create(:bulk_import_tracker, entity: entity) }
let_it_be(:context) { BulkImports::Pipeline::Context.new(tracker) }
let_it_be(:data) do
{
'name' => 'banana',
'user_id' => user.id
}
end
before do
stub_licensed_features(epics: true)
context.extra[:epic_iid] = epic.iid
group.add_developer(user)
end
context 'when emoji does not exist' do
it 'creates new emoji' do
expect { subject.load(context, data) }.to change(::AwardEmoji, :count).by(1)
epic = group.epics.last
emoji = epic.award_emoji.first
expect(emoji.name).to eq(data['name'])
expect(emoji.user).to eq(user)
end
end
context 'when same emoji exists' do
it 'does not create a new emoji' do
epic.award_emoji.create!(data)
expect { subject.load(context, data) }.not_to change(::AwardEmoji, :count)
end
end
context 'when user is not allowed to award emoji' do
before do
allow(Ability).to receive(:allowed?).with(user, :award_emoji, epic).and_return(false)
end
it 'raises NotAllowedError exception' do
expect { subject.load(context, data) }.to raise_error(described_class::NotAllowedError)
end
end
end
end
......@@ -48,48 +48,80 @@ RSpec.describe EE::BulkImports::Groups::Pipelines::EpicAwardEmojiPipeline do
expect { subject.run }.to change(::AwardEmoji, :count).by(1)
expect(epic.award_emoji.first.name).to eq('thumbsup')
end
end
context 'when extracted data many pages' do
it 'runs pipeline for the second page' do
first_page = extracted_data(has_next_page: true)
last_page = extracted_data
context 'when extracted data many pages' do
it 'runs pipeline for the second page' do
first_page = extracted_data(has_next_page: true)
last_page = extracted_data
allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor|
allow(extractor)
.to receive(:extract)
.and_return(first_page, last_page)
allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor|
allow(extractor)
.to receive(:extract)
.and_return(first_page, last_page)
end
subject.run
end
end
subject.run
context 'when there is many epics to import' do
let_it_be(:second_epic) { create(:epic, group: group) }
it 'runs the pipeline for the next epic' do
allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor|
allow(extractor)
.to receive(:extract)
.twice # for each epic
.and_return(extracted_data)
end
expect(context.extra)
.to receive(:[]=)
.with(:epic_iid, epic.iid)
.and_call_original
expect(context.extra)
.to receive(:[]=)
.with(:epic_iid, second_epic.iid)
.and_call_original
expect(context.extra)
.to receive(:[]=)
.with(:epic_iid, nil)
.and_call_original
subject.run
end
end
end
context 'when there is many epics to import' do
let_it_be(:second_epic) { create(:epic, group: group) }
describe '#load' do
let(:data) { { 'name' => 'thumbsup', 'user_id' => user.id } }
it 'runs the pipeline for the next epic' do
allow_next_instance_of(BulkImports::Common::Extractors::GraphqlExtractor) do |extractor|
allow(extractor)
.to receive(:extract)
.twice # for each epic
.and_return(extracted_data)
context 'when emoji does not exist' do
it 'creates new emoji' do
expect { subject.load(context, data) }.to change(::AwardEmoji, :count).by(1)
epic = group.epics.last
emoji = epic.award_emoji.first
expect(emoji.name).to eq(data['name'])
expect(emoji.user).to eq(user)
end
end
expect(context.extra)
.to receive(:[]=)
.with(:epic_iid, epic.iid)
.and_call_original
expect(context.extra)
.to receive(:[]=)
.with(:epic_iid, second_epic.iid)
.and_call_original
expect(context.extra)
.to receive(:[]=)
.with(:epic_iid, nil)
.and_call_original
subject.run
context 'when same emoji exists' do
it 'does not create a new emoji' do
epic.award_emoji.create!(data)
expect { subject.load(context, data) }.not_to change(::AwardEmoji, :count)
end
end
context 'when user is not allowed to award emoji' do
it 'raises NotAllowedError exception' do
allow(Ability).to receive(:allowed?).with(user, :award_emoji, epic).and_return(false)
expect { subject.load(context, data) }.to raise_error(described_class::NotAllowedError)
end
end
end
......@@ -114,10 +146,6 @@ RSpec.describe EE::BulkImports::Groups::Pipelines::EpicAwardEmojiPipeline do
{ klass: BulkImports::Common::Transformers::UserReferenceTransformer, options: nil }
)
end
it 'has loaders' do
expect(described_class.get_loader).to eq(klass: EE::BulkImports::Groups::Loaders::EpicAwardEmojiLoader, options: nil)
end
end
def extracted_data(has_next_page: false)
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe EE::BulkImports::Pipeline::EpicBase do
let_it_be(:group) { create(:group) }
let_it_be(:entity) { create(:bulk_import_entity, group: group) }
let_it_be(:pipeline_tracker) { create(:bulk_import_tracker, entity: entity) }
let_it_be(:context) { BulkImports::Pipeline::Context.new(pipeline_tracker) }
let(:pipeline_class) do
Class.new(EE::BulkImports::Pipeline::EpicBase) do
def extract(_)
::BulkImports::Pipeline::ExtractedData.new
end
def transform(_, data)
data
end
def load(_, data)
data
end
end
end
subject { MyPipeline.new(context) }
before do
stub_const('MyPipeline', pipeline_class)
end
context 'when the group has no epics' do
it 'skips the pipeline' do
expect_next_instance_of(Gitlab::Import::Logger) do |logger|
expect(logger).to receive(:warn)
.with(
bulk_import_id: entity.bulk_import.id,
bulk_import_entity_id: entity.id,
bulk_import_entity_type: entity.source_type,
context_extra: { epic_iid: nil },
message: 'Skipping because group has no epics',
pipeline_class: 'MyPipeline'
)
end
expect { subject.run }
.to change(pipeline_tracker, :status_name).to(:skipped)
end
end
context 'when the group has one epic with one page of data' do
it 'runs the pipeline for each epic' do
create(:epic, group: group)
expect(subject)
.to receive(:run)
.once
.and_call_original
subject.run
end
end
context 'when the group has one epic with multiple page of data' do
it 'runs the pipeline for page' do
create(:epic, group: group)
first_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
'has_next_page' => true,
'next_page' => 'page'
})
last_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
'has_next_page' => false
})
expect(subject)
.to receive(:extract)
.and_return(first_page, last_page)
expect(subject)
.to receive(:run)
.twice
.and_call_original
subject.run
end
end
context 'when the group has multiple epics with one page of data' do
it 'runs the pipeline once for each epic' do
create(:epic, group: group)
create(:epic, group: group)
expect(subject)
.to receive(:run)
.twice
.and_call_original
subject.run
end
end
context 'when the group has multiple epics with multiple pages of data' do
it 'runs the pipeline once for each page of each epic' do
create(:epic, group: group)
create(:epic, group: group)
first_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
'has_next_page' => true,
'next_page' => 'page'
})
last_page = ::BulkImports::Pipeline::ExtractedData.new(page_info: {
'has_next_page' => false
})
expect(subject)
.to receive(:extract)
.and_return(
first_page, # first epic
last_page, # first epic
first_page, # last epic
last_page # last epic
)
expect(subject)
.to receive(:run)
.exactly(4).times
.and_call_original
subject.run
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