diff --git a/app/concepts/project/operation/create_remix.rb b/app/concepts/project/operation/create_remix.rb index 22a7c3818..ac4cfdeb5 100644 --- a/app/concepts/project/operation/create_remix.rb +++ b/app/concepts/project/operation/create_remix.rb @@ -5,39 +5,41 @@ module Operation class CreateRemix require 'operation_response' - def self.call(params, user_id) - response = OperationResponse.new - - validate_params(response, params, user_id) - return response if response.failure? + class << self + def call(params:, user_id:, original_project:) + response = OperationResponse.new - remix_project(response, params, user_id) - response - end + validate_params(response, params, user_id, original_project) + remix_project(response, params, user_id, original_project) + response + end - class << self private - def validate_params(response, params, user_id) - valid = params[:project_id].present? && user_id.present? - response[:error] = 'Invalid parameters' unless valid + def validate_params(response, params, user_id, original_project) + valid = params[:identifier].present? && user_id.present? && original_project.present? + response[:error] = I18n.t('errors.project.remixing.invalid_params') unless valid end - def remix_project(response, params, user_id) - original_project = Project.find_by!(identifier: params[:project_id]) + def remix_project(response, params, user_id, original_project) + return if response[:error] - response[:project] = create_remix(original_project, user_id) + response[:project] = create_remix(original_project, params, user_id) - response[:error] = 'Unable to create project' unless response[:project].save + response[:error] = I18n.t('errors.project.remixing.cannot_save') unless response[:project].save response end - def create_remix(original_project, user_id) - original_project.dup.tap do |proj| + def create_remix(original_project, params, user_id) + remix = original_project.dup.tap do |proj| proj.user_id = user_id - proj.components = original_project.components.map(&:dup) proj.remixed_from_id = original_project.id end + + params[:components].each do |x| + remix.components.build(x.slice(:name, :extension, :content, :index)) + end + remix end end end diff --git a/app/concepts/project/operation/update.rb b/app/concepts/project/operation/update.rb index d210d321f..cfde67f70 100644 --- a/app/concepts/project/operation/update.rb +++ b/app/concepts/project/operation/update.rb @@ -5,7 +5,7 @@ module Operation class Update require 'operation_response' - def self.call(params, project) + def self.call(params:, project:) response = setup_response(project) setup_deletions(response, params) diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb index f24a87963..cc57d69cf 100644 --- a/app/controllers/api/projects/remixes_controller.rb +++ b/app/controllers/api/projects/remixes_controller.rb @@ -6,7 +6,9 @@ class RemixesController < ApiController before_action :require_oauth_user def create - result = Project::Operation::CreateRemix.call(remix_params, oauth_user_id) + result = Project::Operation::CreateRemix.call(params: remix_params, + user_id: oauth_user_id, + original_project: project) if result.success? @project = result[:project] @@ -18,8 +20,15 @@ def create private + def project + @project ||= Project.find_by!(identifier: params[:project_id]) + end + def remix_params - params.permit(:project_id) + params.require(:project) + .permit(:name, + :identifier, + components: %i[id name extension content index]) end end end diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index 0de3c02c5..e3f15ee1f 100644 --- a/app/controllers/api/projects_controller.rb +++ b/app/controllers/api/projects_controller.rb @@ -12,7 +12,7 @@ def show end def update - result = Project::Operation::Update.call(project_params, @project) + result = Project::Operation::Update.call(params: project_params, project: @project) if result.success? render :show, formats: [:json] diff --git a/config/locales/en.yml b/config/locales/en.yml index d5379a51c..70e5074e6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -5,3 +5,6 @@ en: delete_default_component: 'Cannot delete default file' change_default_name: 'Cannot amend default file name' change_default_extension: 'Cannot amend default file extension' + remixing: + invalid_params: 'Invalid parameters' + cannot_save: 'Cannot create project remix' diff --git a/spec/concepts/project/operation/create_remix_spec.rb b/spec/concepts/project/operation/create_remix_spec.rb index c2c58e221..4bc05f711 100644 --- a/spec/concepts/project/operation/create_remix_spec.rb +++ b/spec/concepts/project/operation/create_remix_spec.rb @@ -3,78 +3,148 @@ require 'rails_helper' RSpec.describe Project::Operation::CreateRemix, type: :unit do - subject(:create_remix) { described_class.call(params, user_id) } + subject(:create_remix) { described_class.call(params: remix_params, user_id: user_id, original_project: original_project) } let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } let!(:original_project) { create(:project, :with_components) } + let(:remix_params) do + component = original_project.components.first + { + name: original_project.name, + identifier: original_project.identifier, + components: [ + { + id: component.id, + name: component.name, + extension: component.extension, + content: 'some updated component content', + index: component.index + } + ] + } + end before do mock_phrase_generation end describe '.call' do - context 'when all params valid' do - let(:params) { { project_id: original_project.identifier } } + let(:params) { { project_id: original_project.identifier } } - it 'returns success' do - result = create_remix - expect(result.success?).to eq(true) - end + it 'returns success' do + result = create_remix + expect(result.success?).to eq(true) + end + + it 'creates new project' do + expect { create_remix }.to change(Project, :count).by(1) + end + + it 'assigns a new identifer to new project' do + result = create_remix + remixed_project = result[:project] + expect(remixed_project.identifier).not_to eq(original_project.identifier) + end + + it 'assigns user_id to new project' do + remixed_project = create_remix[:project] + expect(remixed_project.user_id).to eq(user_id) + end + + it 'duplicates properties on new project' do + remixed_project = create_remix[:project] - it 'creates new project' do - expect { create_remix }.to change(Project, :count).by(1) + remixed_attrs = remixed_project.attributes.symbolize_keys.slice(:name, :project_type) + original_attrs = original_project.attributes.symbolize_keys.slice(:name, :project_type) + expect(remixed_attrs).to eq(original_attrs) + end + + it 'creates new components' do + expect { create_remix }.to change(Component, :count).by(1) + end + + it 'persists changes made to submitted components' do + remixed_project = create_remix[:project] + expect(remixed_project.components.first.content).to eq('some updated component content') + end + + context 'when a new component has been added before remixing' do + let(:new_component_params) { { name: 'added_component', extension: 'py', content: 'some added component content', index: 9999 } } + + before do + remix_params[:components] << new_component_params end - it 'assigns a new identifer to new project' do - result = create_remix - remixed_project = result[:project] - expect(remixed_project.identifier).not_to eq(original_project.identifier) + it 'creates all components' do + expect { create_remix }.to change(Component, :count).by(2) end - it 'assigns user_id to new project' do + it 'persists the new component' do remixed_project = create_remix[:project] - expect(remixed_project.user_id).to eq(user_id) + expect(remixed_project.components.last.attributes.symbolize_keys).to include(new_component_params) end + end - it 'duplicates properties on new project' do - remixed_project = create_remix[:project] + context 'when user_id is not present' do + let(:user_id) { nil } + let(:params) { { project_id: original_project.identifier } } - remixed_attrs = remixed_project.attributes.symbolize_keys.slice(:name, :project_type) - original_attrs = original_project.attributes.symbolize_keys.slice(:name, :project_type) - expect(remixed_attrs).to eq(original_attrs) + it 'returns failure' do + result = create_remix + expect(result.failure?).to eq(true) end - it 'duplicates project components' do - remixed_props_array = component_array_props(create_remix[:project].components) - original_props_array = component_array_props(original_project.components) + it 'returns error message' do + result = create_remix + expect(result[:error]).to eq(I18n.t('errors.project.remixing.invalid_params')) + end - expect(remixed_props_array).to match_array(original_props_array) + it 'does not create new project' do + expect { create_remix }.not_to change(Project, :count) end end - context 'when user_id is not present' do - let(:user_id) { nil } - let(:params) { { project_id: original_project.identifier } } + context 'when original project is not present' do + subject(:create_remix) { described_class.call(params: remix_params, user_id: user_id, original_project: nil) } it 'returns failure' do result = create_remix expect(result.failure?).to eq(true) end + it 'returns error message' do + result = create_remix + expect(result[:error]).to eq(I18n.t('errors.project.remixing.invalid_params')) + end + it 'does not create new project' do expect { create_remix }.not_to change(Project, :count) end end - end - def component_array_props(components) - components.map do |x| - { - name: x.name, - content: x.content, - extension: x.extension, - index: x.index - } + context 'when project components are invalid' do + let(:invalid_component_params) { { name: 'added_component', extension: 'py', content: '' } } + + before do + remix_params[:components] << invalid_component_params + end + + it 'returns failure' do + expect(create_remix.failure?).to eq(true) + end + + it 'sets error message' do + expect(create_remix[:error]).to eq(I18n.t('errors.project.remixing.cannot_save')) + end end end + + def component_props(component) + { + name: component.name, + content: component.content, + extension: component.extension, + index: component.index + } + end end diff --git a/spec/concepts/project/operation/update_default_component_spec.rb b/spec/concepts/project/operation/update_default_component_spec.rb index 7d888c323..4c55207bc 100644 --- a/spec/concepts/project/operation/update_default_component_spec.rb +++ b/spec/concepts/project/operation/update_default_component_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe Project::Operation::Update, type: :unit do - subject(:update) { described_class.call(project_params, project) } + subject(:update) { described_class.call(params: project_params, project: project) } let!(:project) { create(:project, :with_default_component) } let(:default_component) { project.components.first } diff --git a/spec/concepts/project/operation/update_delete_components_spec.rb b/spec/concepts/project/operation/update_delete_components_spec.rb index b33ec43d7..ad08160f3 100644 --- a/spec/concepts/project/operation/update_delete_components_spec.rb +++ b/spec/concepts/project/operation/update_delete_components_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe Project::Operation::Update, type: :unit do - subject(:update) { described_class.call(project_params, project) } + subject(:update) { described_class.call(params: project_params, project: project) } let!(:project) { create(:project, :with_default_component, :with_components) } let(:component_to_delete) { project.components.last } diff --git a/spec/concepts/project/operation/update_invalid_spec.rb b/spec/concepts/project/operation/update_invalid_spec.rb index 2725c1fac..449faa2fd 100644 --- a/spec/concepts/project/operation/update_invalid_spec.rb +++ b/spec/concepts/project/operation/update_invalid_spec.rb @@ -8,7 +8,7 @@ name: 'updated project name', components: [default_component_params, edited_component_params, new_component_params] } - described_class.call(params, project) + described_class.call(params: params, project: project) end let!(:project) { create(:project, :with_default_component, :with_components, component_count: 2) } diff --git a/spec/concepts/project/operation/update_spec.rb b/spec/concepts/project/operation/update_spec.rb index e815ccdd2..a0ea8a655 100644 --- a/spec/concepts/project/operation/update_spec.rb +++ b/spec/concepts/project/operation/update_spec.rb @@ -8,7 +8,7 @@ name: 'updated project name', components: component_params } - described_class.call(params, project) + described_class.call(params: params, project: project) end let!(:project) { create(:project, :with_default_component, :with_components) } diff --git a/spec/factories/component.rb b/spec/factories/component.rb index 20b3b7841..debe18f42 100644 --- a/spec/factories/component.rb +++ b/spec/factories/component.rb @@ -6,6 +6,7 @@ extension { 'py' } sequence(:index) { |n| n } default { false } + content { Faker::Lorem.paragraph } project factory :default_python_component do diff --git a/spec/request/projects/remix_spec.rb b/spec/request/projects/remix_spec.rb index bb8a28fc7..430d43e93 100644 --- a/spec/request/projects/remix_spec.rb +++ b/spec/request/projects/remix_spec.rb @@ -1,10 +1,18 @@ # frozen_string_literal: true require 'rails_helper' +require_relative '../../../lib/operation_response' RSpec.describe 'Remix requests', type: :request do let!(:original_project) { create(:project) } let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } + let(:project_params) do + { + name: original_project.name, + identifier: original_project.identifier, + components: [] + } + end describe 'create' do before do @@ -13,22 +21,44 @@ context 'when auth is correct' do before do - mock_oauth_user + mock_oauth_user(user_id) end it 'returns success response' do - post "/api/projects/#{original_project.identifier}/remix" + post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params } expect(response.status).to eq(200) end it 'returns 404 response if invalid project' do - post '/api/projects/no-such-project/remix' + project_params[:identifier] = 'no-such-project' + post '/api/projects/no-such-project/remix', params: { project: project_params } expect(response.status).to eq(404) end end + context 'when project can not be saved' do + before do + mock_oauth_user(user_id) + error_response = OperationResponse.new + error_response[:error] = 'Something went wrong' + allow(Project::Operation::CreateRemix).to receive(:call).and_return(error_response) + end + + it 'returns 400' do + post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params } + + expect(response.status).to eq(400) + end + + it 'returns error message' do + post "/api/projects/#{original_project.identifier}/remix", params: { project: project_params } + + expect(response.body).to eq({ error: 'Something went wrong' }.to_json) + end + end + context 'when auth is invalid' do it 'returns unauthorized' do post "/api/projects/#{original_project.identifier}/remix" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5d227ba75..7db67711e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -86,7 +86,7 @@ # # order dependency and want to debug it, you can fix the order by providing # # the seed, which is printed after each run. # # --seed 1234 - # config.order = :random + config.order = :random # # # Seed global randomization in this process using the `--seed` CLI option. # # Setting this allows you to use `--seed` to deterministically reproduce