From 5ee0d1b29c84375d1b4e2e1dd8b041210c1b1d8c Mon Sep 17 00:00:00 2001 From: ArayB Date: Thu, 24 Feb 2022 10:06:36 +0000 Subject: [PATCH 01/10] Create draft PR for #16 From 66a242183ad40d1ae2d3adee2c3d356d6293a017 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Thu, 24 Feb 2022 16:07:48 +0000 Subject: [PATCH 02/10] Add user_id to remixed projects Move logic into operation class to allow easier testing and keep controllers thin --- .../project/operation/create_remix.rb | 44 +++++++++++ .../api/projects/phrases_controller.rb | 31 ++++---- .../api/projects/remixes_controller.rb | 26 ++++++ config/routes.rb | 4 +- docker-compose.yml | 2 + lib/operation_response.rb | 16 ++++ .../project/operation/create_remix_spec.rb | 79 +++++++++++++++++++ spec/factories/component.rb | 9 +++ spec/factories/project.rb | 10 ++- spec/lib/operation_response_spec.rb | 42 ++++++++++ spec/rails_helper.rb | 2 + spec/request/projects/remix_spec.rb | 21 +++++ spec/support/phrase_identifier_mock.rb | 8 ++ 13 files changed, 276 insertions(+), 18 deletions(-) create mode 100644 app/concepts/project/operation/create_remix.rb create mode 100644 app/controllers/api/projects/remixes_controller.rb create mode 100644 lib/operation_response.rb create mode 100644 spec/concepts/project/operation/create_remix_spec.rb create mode 100644 spec/factories/component.rb create mode 100644 spec/lib/operation_response_spec.rb create mode 100644 spec/request/projects/remix_spec.rb create mode 100644 spec/support/phrase_identifier_mock.rb diff --git a/app/concepts/project/operation/create_remix.rb b/app/concepts/project/operation/create_remix.rb new file mode 100644 index 000000000..cc28927d8 --- /dev/null +++ b/app/concepts/project/operation/create_remix.rb @@ -0,0 +1,44 @@ +class Project + module Operation + class CreateRemix + require 'operation_response' + require 'phrase_identifier' + + def self.call(params) + response = OperationResponse.new + + validate_params(response, params) + return response if response.failure? + + remix_project(response, params) + response + end + + class << self + private + + def validate_params(response, params) + valid = params[:phrase_id].present? && params[:remix][:user_id].present? + response[:error] = 'Invalid parameters' unless valid + end + + def remix_project(response, params) + original_project = Project.find_by!(identifier: params[:phrase_id]) + + remixed_project = original_project.dup + remixed_project.identifier = PhraseIdentifier.generate + remixed_project.user_id = params[:remix][:user_id] + + original_project.components.each do |component| + remixed_project.components << component.dup + end + + response[:project] = remixed_project + + response[:error] = 'Unable to create project' unless remixed_project.save + response + end + end + end + end +end diff --git a/app/controllers/api/projects/phrases_controller.rb b/app/controllers/api/projects/phrases_controller.rb index 6e1e245fe..89f093434 100644 --- a/app/controllers/api/projects/phrases_controller.rb +++ b/app/controllers/api/projects/phrases_controller.rb @@ -21,20 +21,23 @@ def update head :ok end - def remix - old = Project.find_by!(identifier: params[:phrase_id]) - - @project = old.dup - @project.identifier = PhraseIdentifier.generate - - old.components.each do |component| - @project.components << component.dup - end - - @project.save - - render '/api/projects/show', formats: [:json] - end + # def remix + # puts '*'*50 + # puts "DBG: #{params}" + # puts '*'*50 + # old = Project.find_by!(identifier: params[:phrase_id]) + # + # @project = old.dup + # @project.identifier = PhraseIdentifier.generate + # + # old.components.each do |component| + # @project.components << component.dup + # end + # + # @project.save + # + # render '/api/projects/show', formats: [:json] + # end private diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb new file mode 100644 index 000000000..038cb3edb --- /dev/null +++ b/app/controllers/api/projects/remixes_controller.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Api + module Projects + class RemixesController < ApiController + # require 'phrase_identifier' + + def create + result = Project::Operation::CreateRemix.call(params) + + if result.success? + @project = result[:project] + render '/api/projects/show', formats: [:json] + else + render json: { error: result[:error] } + end + end + + private + + def remix_params + params.permit(:phrase_id, remix: [:user_id]) + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 0dfec3ec3..c0fb2c750 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,8 +14,8 @@ namespace :projects do resources :phrases, only: %i[show update] do - post 'remix', to: 'phrases#remix' - end + # post 'remix', to: 'phrases#remix' + resource :remix, only: %i[create] end end end end diff --git a/docker-compose.yml b/docker-compose.yml index d0c297e50..6591b4a01 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,6 +18,8 @@ services: - "3009:3009" depends_on: - db + stdin_open: true + tty: true volumes: pg-data: diff --git a/lib/operation_response.rb b/lib/operation_response.rb new file mode 100644 index 000000000..6456c787d --- /dev/null +++ b/lib/operation_response.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class OperationResponse < Hash + def success? + return false unless self[:error].nil? + + true + end + + def failure? + return false if self[:error].nil? + + true + end +end + diff --git a/spec/concepts/project/operation/create_remix_spec.rb b/spec/concepts/project/operation/create_remix_spec.rb new file mode 100644 index 000000000..780f97214 --- /dev/null +++ b/spec/concepts/project/operation/create_remix_spec.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Project::Operation::CreateRemix, type: :unit do + subject(:create_remix) { described_class.call(params) } + + let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } + let!(:original_project) { create(:project, :with_components) } + + before do + mock_phrase_generation + end + + describe '.call' do + context 'when all params valid' do + let(:params) { { phrase_id: original_project.identifier, remix: { user_id: user_id } } } + + 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] + + 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 'duplicates project components' do + remixed_props_array = component_array_props(create_remix[:project].components) + original_props_array = component_array_props(original_project.components) + + expect(remixed_props_array).to match_array(original_props_array) + end + end + + context 'when user_id is not present' do + let(:params) { { phrase_id: original_project.identifier, remix: { user_id: '' } } } + + it 'returns failure' do + result = create_remix + expect(result.failure?).to eq(true) + 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 + } + end + end +end diff --git a/spec/factories/component.rb b/spec/factories/component.rb new file mode 100644 index 000000000..74c43dba1 --- /dev/null +++ b/spec/factories/component.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :component do + name { Faker::Lorem.word } + extension { 'py' } + content { Faker::Lorem.paragraph(sentence_count: 2) } + end +end diff --git a/spec/factories/project.rb b/spec/factories/project.rb index 83361497e..7badbdcb8 100644 --- a/spec/factories/project.rb +++ b/spec/factories/project.rb @@ -2,9 +2,15 @@ FactoryBot.define do factory :project do - user_id { rand(10**10) } + user_id { SecureRandom.uuid } name { Faker::Book.title } identifier { "#{Faker::Verb.base}-#{Faker::Verb.base}-#{Faker::Verb.base}" } - project_type { %w[python html].sample } + project_type { 'python' } + + trait :with_components do + after(:create) do |object| + object.components = FactoryBot.create_list(:component, 2, project: object) + end + end end end diff --git a/spec/lib/operation_response_spec.rb b/spec/lib/operation_response_spec.rb new file mode 100644 index 000000000..e0c8675ae --- /dev/null +++ b/spec/lib/operation_response_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_relative '../../lib/operation_response' + +RSpec.describe OperationResponse do + describe '#success?' do + context 'when :error not present' do + it 'returns true' do + response = OperationResponse.new + expect(response.success?).to eq(true) + end + end + + context 'when :error has been set' do + it 'returns false' do + response = OperationResponse.new + response[:error] = 'An error' + expect(response.success?).to eq(false) + end + end + end + + describe '#failure?' do + context 'when :error not present' do + it 'returns false' do + response = OperationResponse.new + expect(response.failure?).to eq(false) + end + end + + context 'when :error has been set' do + it 'returns true' do + response = OperationResponse.new + response[:error] = 'An error' + expect(response.failure?).to eq(true) + end + end + + end +end + diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 77e668556..c2f7fa72e 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -13,6 +13,7 @@ abort('The Rails environment is running in production mode!') if Rails.env.production? require 'rspec/rails' # Add additional requires below this line. Rails is not loaded until this point! + Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f } # Requires supporting ruby files with custom matchers and macros, etc, in @@ -69,6 +70,7 @@ config.filter_rails_from_backtrace! # arbitrary gems may also be filtered via: # config.filter_gems_from_backtrace("gem name") + config.include PhraseIdentifierMock end Shoulda::Matchers.configure do |config| diff --git a/spec/request/projects/remix_spec.rb b/spec/request/projects/remix_spec.rb new file mode 100644 index 000000000..85e5ac665 --- /dev/null +++ b/spec/request/projects/remix_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Remix requests', type: :request do + let!(:original_project) { create(:project) } + let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } + + describe 'create' do + before do + mock_phrase_generation + end + + it 'returns expected response' do + post "/api/projects/phrases/#{original_project.identifier}/remix", + params: { remix: { user_id: user_id } } + + expect(response.status).to eq(200) + end + end +end diff --git a/spec/support/phrase_identifier_mock.rb b/spec/support/phrase_identifier_mock.rb new file mode 100644 index 000000000..404735cb2 --- /dev/null +++ b/spec/support/phrase_identifier_mock.rb @@ -0,0 +1,8 @@ +module PhraseIdentifierMock + def mock_phrase_generation(phrase = nil) + # This could cause problems if tests require multiple phrases to be generated + phrase ||= "#{Faker::Verb.base}-#{Faker::Verb.base}-#{Faker::Verb.base}" + + allow(PhraseIdentifier).to receive(:generate).and_return(phrase) + end +end From 9467f352cc428b486eac5d635bbef3e5bbea3871 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Thu, 24 Feb 2022 16:11:17 +0000 Subject: [PATCH 03/10] Remove code --- .../api/projects/phrases_controller.rb | 18 ------------------ config/routes.rb | 4 ++-- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/app/controllers/api/projects/phrases_controller.rb b/app/controllers/api/projects/phrases_controller.rb index 89f093434..c855d26a3 100644 --- a/app/controllers/api/projects/phrases_controller.rb +++ b/app/controllers/api/projects/phrases_controller.rb @@ -21,24 +21,6 @@ def update head :ok end - # def remix - # puts '*'*50 - # puts "DBG: #{params}" - # puts '*'*50 - # old = Project.find_by!(identifier: params[:phrase_id]) - # - # @project = old.dup - # @project.identifier = PhraseIdentifier.generate - # - # old.components.each do |component| - # @project.components << component.dup - # end - # - # @project.save - # - # render '/api/projects/show', formats: [:json] - # end - private def project_params diff --git a/config/routes.rb b/config/routes.rb index c0fb2c750..3903d8011 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -14,8 +14,8 @@ namespace :projects do resources :phrases, only: %i[show update] do - # post 'remix', to: 'phrases#remix' - resource :remix, only: %i[create] end + resource :remix, only: %i[create] + end end end end From d06bb4521b066e866d74cf931a82f2faea1d4236 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 25 Feb 2022 08:46:03 +0000 Subject: [PATCH 04/10] Fix rubocop problems --- app/concepts/project/operation/create_remix.rb | 16 ++++++---------- lib/operation_response.rb | 1 - spec/lib/operation_response_spec.rb | 10 ++++------ spec/support/phrase_identifier_mock.rb | 2 ++ 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/app/concepts/project/operation/create_remix.rb b/app/concepts/project/operation/create_remix.rb index cc28927d8..298ed8769 100644 --- a/app/concepts/project/operation/create_remix.rb +++ b/app/concepts/project/operation/create_remix.rb @@ -1,8 +1,9 @@ +# frozen_string_literal: true + class Project module Operation class CreateRemix require 'operation_response' - require 'phrase_identifier' def self.call(params) response = OperationResponse.new @@ -25,17 +26,12 @@ def validate_params(response, params) def remix_project(response, params) original_project = Project.find_by!(identifier: params[:phrase_id]) - remixed_project = original_project.dup - remixed_project.identifier = PhraseIdentifier.generate - remixed_project.user_id = params[:remix][:user_id] - - original_project.components.each do |component| - remixed_project.components << component.dup + response[:project] = original_project.dup.tap do |proj| + proj.user_id = params[:remix][:user_id] + proj.components = original_project.components.map(&:dup) end - response[:project] = remixed_project - - response[:error] = 'Unable to create project' unless remixed_project.save + response[:error] = 'Unable to create project' unless response[:project].save response end end diff --git a/lib/operation_response.rb b/lib/operation_response.rb index 6456c787d..ee593da8b 100644 --- a/lib/operation_response.rb +++ b/lib/operation_response.rb @@ -13,4 +13,3 @@ def failure? true end end - diff --git a/spec/lib/operation_response_spec.rb b/spec/lib/operation_response_spec.rb index e0c8675ae..faa9b59ca 100644 --- a/spec/lib/operation_response_spec.rb +++ b/spec/lib/operation_response_spec.rb @@ -7,14 +7,14 @@ describe '#success?' do context 'when :error not present' do it 'returns true' do - response = OperationResponse.new + response = described_class.new expect(response.success?).to eq(true) end end context 'when :error has been set' do it 'returns false' do - response = OperationResponse.new + response = described_class.new response[:error] = 'An error' expect(response.success?).to eq(false) end @@ -24,19 +24,17 @@ describe '#failure?' do context 'when :error not present' do it 'returns false' do - response = OperationResponse.new + response = described_class.new expect(response.failure?).to eq(false) end end context 'when :error has been set' do it 'returns true' do - response = OperationResponse.new + response = described_class.new response[:error] = 'An error' expect(response.failure?).to eq(true) end end - end end - diff --git a/spec/support/phrase_identifier_mock.rb b/spec/support/phrase_identifier_mock.rb index 404735cb2..18b97b00e 100644 --- a/spec/support/phrase_identifier_mock.rb +++ b/spec/support/phrase_identifier_mock.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module PhraseIdentifierMock def mock_phrase_generation(phrase = nil) # This could cause problems if tests require multiple phrases to be generated From 8195e1609c4c7f896cc0bfaaa086ef2380a101ef Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 25 Feb 2022 08:49:34 +0000 Subject: [PATCH 05/10] Add component spec --- spec/models/component_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/component_spec.rb b/spec/models/component_spec.rb index 48bab3aa4..b041ddad8 100644 --- a/spec/models/component_spec.rb +++ b/spec/models/component_spec.rb @@ -3,5 +3,5 @@ require 'rails_helper' RSpec.describe Component, type: :model do - pending "add some examples to (or delete) #{__FILE__}" + it { is_expected.to belong_to(:project) } end From ec264d6843bc791a8864bb6e3c2373fe2c0bea53 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 25 Feb 2022 08:50:27 +0000 Subject: [PATCH 06/10] Remove word spec --- spec/models/word_spec.rb | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 spec/models/word_spec.rb diff --git a/spec/models/word_spec.rb b/spec/models/word_spec.rb deleted file mode 100644 index c9aba94bf..000000000 --- a/spec/models/word_spec.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Word, type: :model do - pending "add some examples to (or delete) #{__FILE__}" -end From 95949ed5fb3189795a10468a8d25abf1271889f0 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 25 Feb 2022 09:24:14 +0000 Subject: [PATCH 07/10] Remove commented out require --- app/controllers/api/projects/remixes_controller.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb index 038cb3edb..bf9c78326 100644 --- a/app/controllers/api/projects/remixes_controller.rb +++ b/app/controllers/api/projects/remixes_controller.rb @@ -3,8 +3,6 @@ module Api module Projects class RemixesController < ApiController - # require 'phrase_identifier' - def create result = Project::Operation::CreateRemix.call(params) From 0b5282e42d9ca50fbdfd56569303d2aedf04d209 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 25 Feb 2022 09:52:18 +0000 Subject: [PATCH 08/10] Set status on error request and add spec --- app/controllers/api/projects/remixes_controller.rb | 2 +- spec/request/projects/remix_spec.rb | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb index bf9c78326..95b3eb855 100644 --- a/app/controllers/api/projects/remixes_controller.rb +++ b/app/controllers/api/projects/remixes_controller.rb @@ -10,7 +10,7 @@ def create @project = result[:project] render '/api/projects/show', formats: [:json] else - render json: { error: result[:error] } + render json: { error: result[:error] }, status: :bad_request end end diff --git a/spec/request/projects/remix_spec.rb b/spec/request/projects/remix_spec.rb index 85e5ac665..2b7805e19 100644 --- a/spec/request/projects/remix_spec.rb +++ b/spec/request/projects/remix_spec.rb @@ -17,5 +17,14 @@ expect(response.status).to eq(200) end + + context 'when request is invalid' do + it 'returns error response' do + post "/api/projects/phrases/#{original_project.identifier}/remix", + params: { remix: { user_id: '' } } + + expect(response.status).to eq(400) + end + end end end From 8fd49080a5534b30e23b0c10d97b75f11947e754 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 25 Feb 2022 11:19:52 +0000 Subject: [PATCH 09/10] make less verbose --- lib/operation_response.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/operation_response.rb b/lib/operation_response.rb index ee593da8b..024e33f97 100644 --- a/lib/operation_response.rb +++ b/lib/operation_response.rb @@ -8,8 +8,6 @@ def success? end def failure? - return false if self[:error].nil? - - true + !success? end end From 99f1f45f727029367b78d12cb7932dadffbacad9 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 25 Feb 2022 11:23:29 +0000 Subject: [PATCH 10/10] Use strong params in controller --- app/controllers/api/projects/remixes_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb index 95b3eb855..38a0c530b 100644 --- a/app/controllers/api/projects/remixes_controller.rb +++ b/app/controllers/api/projects/remixes_controller.rb @@ -4,7 +4,7 @@ module Api module Projects class RemixesController < ApiController def create - result = Project::Operation::CreateRemix.call(params) + result = Project::Operation::CreateRemix.call(remix_params) if result.success? @project = result[:project]