diff --git a/Gemfile b/Gemfile index f3626183..f980a082 100644 --- a/Gemfile +++ b/Gemfile @@ -74,4 +74,5 @@ group :test do gem 'rspec-autotest' gem 'rspec-rails' gem 'simplecov', require: false + gem 'webmock' end diff --git a/Gemfile.lock b/Gemfile.lock index 853a2f87..fb93dbf4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -128,6 +128,8 @@ GEM chronic (0.10.2) coderay (1.1.2) concurrent-ruby (1.1.5) + crack (0.4.3) + safe_yaml (~> 1.0.0) crass (1.0.5) database_cleaner (1.7.0) debug_inspector (0.0.3) @@ -156,6 +158,7 @@ GEM forgery (0.7.0) globalid (0.4.2) activesupport (>= 4.2.0) + hashdiff (1.0.0) headless (2.3.1) highline (2.0.3) http-accept (1.7.0) @@ -348,6 +351,7 @@ GEM json (~> 2.1) structured_warnings (~> 0.3) rubyzip (2.0.0) + safe_yaml (1.0.5) sass-rails (6.0.0) sassc-rails (~> 2.1, >= 2.1.1) sassc (2.2.1) @@ -409,6 +413,10 @@ GEM activemodel (>= 5.0) bindex (>= 0.4.0) railties (>= 5.0) + webmock (3.7.6) + addressable (>= 2.3.6) + crack (>= 0.3.2) + hashdiff (>= 0.4.0, < 2.0.0) webpacker (4.2.0) activesupport (>= 4.2) rack-proxy (>= 0.6.1) @@ -491,6 +499,7 @@ DEPENDENCIES turbolinks uglifier web-console + webmock webpacker whenever diff --git a/app/controllers/codeharbor_links_controller.rb b/app/controllers/codeharbor_links_controller.rb index 8c25df66..802448f6 100644 --- a/app/controllers/codeharbor_links_controller.rb +++ b/app/controllers/codeharbor_links_controller.rb @@ -18,7 +18,7 @@ class CodeharborLinksController < ApplicationController @codeharbor_link = CodeharborLink.new(codeharbor_link_params) @codeharbor_link.user = current_user authorize! - create_and_respond(object: @codeharbor_link, path: proc { @codeharbor_link.user }) + create_and_respond(object: @codeharbor_link, path: -> { @codeharbor_link.user }) end def update diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 468d18ac..f0dd0f5a 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -123,14 +123,13 @@ class ExercisesController < ApplicationController def export_external_check codeharbor_check = ExerciseService::CheckExternal.call(uuid: @exercise.uuid, codeharbor_link: current_user.codeharbor_link) - render json: { message: codeharbor_check[:message], actions: render_to_string( partial: 'export_actions', locals: { exercise: @exercise, - exercise_found: codeharbor_check[:exercise_found], + # exercise_found: codeharbor_check[:exercise_found], update_right: codeharbor_check[:update_right], error: codeharbor_check[:error], exported: false @@ -142,7 +141,7 @@ class ExercisesController < ApplicationController def export_external_confirm push_type = params[:push_type] - return render :fail unless %w[create_new export].include? push_type + return render json: {}, status: 500 unless %w[create_new export].include? push_type @exercise.uuid = SecureRandom.uuid if push_type == 'create_new' @@ -195,7 +194,6 @@ class ExercisesController < ApplicationController rescue Proforma::ProformaError render json: t('exercises.export_codeharbor.export_errors.invalid'), status: 400 rescue StandardError - logger.info(exercise.errors.full_messages) render json: t('exercises.export_codeharbor.export_errors.internal_error'), status: 500 end diff --git a/app/services/exercise_service/check_external.rb b/app/services/exercise_service/check_external.rb index 1cce7614..1aac945b 100644 --- a/app/services/exercise_service/check_external.rb +++ b/app/services/exercise_service/check_external.rb @@ -17,7 +17,7 @@ module ExerciseService {error: false}.merge(response_hash.slice(:message, :exercise_found, :update_right)) rescue Faraday::Error => e - {error: true, message: t('exercises.export_exercise.error', message: e.message)} + {error: true, message: I18n.t('exercises.export_codeharbor.error', message: e.message)} end private diff --git a/config/locales/en.yml b/config/locales/en.yml index 76091159..8a0707e1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -327,7 +327,7 @@ en: export_codeharbor: label: Export to Codeharbor dialogtitle: Export to Codeharbor - successfully_exported: 'Exercise has successfully been exported.
ID: %{id}
Title: %{title}' + successfully_exported: 'Exercise has been successfully exported.
ID: %{id}
Title: %{title}' export_failed: 'Export has failed.
ID: %{id}
Title: %{title}

Error: %{error}' error: 'An error occurred while contacting Codeharbor
Error: %{message}' checking_codeharbor: Checking if the exercise exists on Codeharbor. diff --git a/spec/controllers/codeharbor_links_controller_spec.rb b/spec/controllers/codeharbor_links_controller_spec.rb new file mode 100644 index 00000000..b057c32f --- /dev/null +++ b/spec/controllers/codeharbor_links_controller_spec.rb @@ -0,0 +1,93 @@ +require 'rails_helper' + +describe CodeharborLinksController do + let(:user) { FactoryBot.create(:teacher) } + before(:each) { allow(controller).to receive(:current_user).and_return(user) } + + describe 'GET #new' do + before { get :new } + + expect_status(200) + end + + describe 'GET #edit' do + let(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } + + before { get :edit, params: { id: codeharbor_link.id } } + + expect_status(200) + end + + describe 'POST #create' do + let(:post_request) { post :create, params: {codeharbor_link: params} } + let(:params) { {push_url: 'http://foo.bar/push', check_uuid_url: 'http://foo.bar/check', api_key: 'api_key'} } + + it 'creates a codeharbor_link' do + expect { post_request }.to change(CodeharborLink, :count).by(1) + end + + it 'redirects to user show' do + expect(post_request).to redirect_to(user) + end + + context 'with invalid params' do + let(:params) { {push_url: '', check_uuid_url: '', api_key: ''} } + + it 'does not create a codeharbor_link' do + expect { post_request }.not_to change(CodeharborLink, :count) + end + + it 'redirects to user show' do + post_request + expect(response).to render_template(:new) + end + end + end + + describe 'PUT #update' do + let(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } + let(:put_request) { patch :update, params: {id: codeharbor_link.id, codeharbor_link: params} } + let(:params) { {push_url: 'http://foo.bar/push', check_uuid_url: 'http://foo.bar/check', api_key: 'api_key'} } + + it 'updates push_url' do + expect { put_request }.to change { codeharbor_link.reload.push_url }.to('http://foo.bar/push') + end + it 'updates check_uuid_url' do + expect { put_request }.to change { codeharbor_link.reload.check_uuid_url }.to('http://foo.bar/check') + end + it 'updates api_key' do + expect { put_request }.to change { codeharbor_link.reload.api_key }.to('api_key') + end + + it 'redirects to user show' do + expect(put_request).to redirect_to(user) + end + + context 'with invalid params' do + let(:params) { {push_url: '', check_uuid_url: '', api_key: ''} } + + it 'does not change codeharbor_link' do + expect { put_request }.not_to change { codeharbor_link.reload.attributes } + end + + it 'redirects to user show' do + put_request + expect(response).to render_template(:edit) + end + end + end + + describe 'DELETE #destroy' do + let!(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } + let(:destroy_request) { delete :destroy, params: {id: codeharbor_link.id} } + + it 'deletes codeharbor_link' do + expect { destroy_request }.to change(CodeharborLink, :count).by(-1) + end + + it 'redirects to user show' do + expect(destroy_request).to redirect_to(user) + end + end +end + diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 424a5290..3c5289c0 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -313,4 +313,203 @@ describe ExercisesController do expect_template(:edit) end end + + RSpec::Matchers.define_negated_matcher :not_include, :include + # RSpec::Support::ObjectFormatter.default_instance.max_formatted_output_length = 99999 + + describe 'POST #export_external_check' do + render_views + + let(:post_request) { post :export_external_check, params: { id: exercise.id } } + let!(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } + let(:external_check_hash) { {message: message, exercise_found: true, update_right: update_right, error: error} } + let(:message) { 'message' } + let(:update_right) { true } + let(:error) {} + + before { allow(ExerciseService::CheckExternal).to receive(:call).with(uuid: exercise.uuid, codeharbor_link: codeharbor_link).and_return(external_check_hash) } + + it 'renders the correct contents as json' do + post_request + expect(JSON.parse(response.body).symbolize_keys[:message]).to eq('message') + expect(JSON.parse(response.body).symbolize_keys[:actions]).to( + include('button').and(include('Abort').and(include('Export'))) + ) + expect(JSON.parse(response.body).symbolize_keys[:actions]).to( + not_include('Retry').and(not_include('Hide')) + ) + end + + context 'when there is an error' do + let(:error) { 'error' } + + it 'renders the correct contents as json' do + post_request + expect(JSON.parse(response.body).symbolize_keys[:message]).to eq('message') + expect(JSON.parse(response.body).symbolize_keys[:actions]).to( + include('button').and(include('Abort')).and(include('Retry')) + ) + expect(JSON.parse(response.body).symbolize_keys[:actions]).to( + not_include('Create new').and(not_include('Export')).and(not_include('Hide')) + ) + end + end + + context 'when update_right is false' do + let(:update_right) { false } + + it 'renders the correct contents as json' do + post_request + expect(JSON.parse(response.body).symbolize_keys[:message]).to eq('message') + expect(JSON.parse(response.body).symbolize_keys[:actions]).to( + include('button').and(include('Abort')).and(include('Create new')) + ) + expect(JSON.parse(response.body).symbolize_keys[:actions]).to( + not_include('Retry').and(not_include('Export')).and(not_include('Hide')) + ) + end + end + end + + describe '#export_external_confirm' do + render_views + + let!(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } + let(:post_request) { post :export_external_confirm, params: {push_type: push_type, id: exercise.id, codeharbor_link: codeharbor_link.id} } + let(:push_type) { 'create_new' } + let(:error) {} + let(:zip) { 'zip' } + + before do + allow(ProformaService::ExportTask).to receive(:call).with(exercise: exercise).and_return(zip) + allow(ExerciseService::PushExternal).to receive(:call).with(zip: zip, codeharbor_link: codeharbor_link).and_return(error) + end + + it 'renders correct response' do + post_request + + expect(response).to have_http_status(:success) + expect(JSON.parse(response.body).symbolize_keys[:message]).to(include('successfully exported')) + expect(JSON.parse(response.body).symbolize_keys[:status]).to(eql('success')) + expect(JSON.parse(response.body).symbolize_keys[:actions]).to(include('button').and(include('Close'))) + expect(JSON.parse(response.body).symbolize_keys[:actions]).to(not_include('Retry').and(not_include('Abort'))) + end + + context 'when an error occurs' do + let(:error) { 'exampleerror' } + + it 'renders correct response' do + post_request + expect(response).to have_http_status(:success) + expect(JSON.parse(response.body).symbolize_keys[:message]).to(include('failed').and(include('exampleerror'))) + expect(JSON.parse(response.body).symbolize_keys[:status]).to(eql('fail')) + expect(JSON.parse(response.body).symbolize_keys[:actions]).to(include('button').and(include('Retry')).and(include('Close'))) + expect(JSON.parse(response.body).symbolize_keys[:actions]).to(not_include('Abort')) + end + end + + context 'without push_type' do + let(:push_type) {} + + it 'responds with status 500' do + post_request + expect(response).to have_http_status(:internal_server_error) + end + end + end + + describe '#import_uuid_check' do + let(:exercise) { FactoryBot.create(:dummy, uuid: SecureRandom.uuid) } + let!(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } + let(:uuid) { exercise.reload.uuid } + let(:post_request) { post :import_uuid_check, params: {uuid: uuid} } + let(:headers) { {'Authorization' => "Bearer #{codeharbor_link.api_key}"} } + + before { request.headers.merge! headers } + + it 'renders correct response' do + post_request + expect(response).to have_http_status(:success) + + expect(JSON.parse(response.body).symbolize_keys[:exercise_found]).to be true + expect(JSON.parse(response.body).symbolize_keys[:update_right]).to be true + expect(JSON.parse(response.body).symbolize_keys[:message]).to(include('has been found').and(include('Overwrite'))) + end + + context 'when api_key is incorrect' do + let(:headers) { {'Authorization' => 'Bearer XXXXXX'} } + + it 'renders correct response' do + post_request + expect(response).to have_http_status(:unauthorized) + end + end + + context 'when the user is cannot update the exercise' do + let(:codeharbor_link) { FactoryBot.create(:codeharbor_link, api_key: 'anotherkey') } + + it 'renders correct response' do + post_request + expect(response).to have_http_status(:success) + + expect(JSON.parse(response.body).symbolize_keys[:exercise_found]).to be true + expect(JSON.parse(response.body).symbolize_keys[:update_right]).to be false + expect(JSON.parse(response.body).symbolize_keys[:message]).to(include('has been found').and(not_include('Overwrite'))) + end + end + + context 'when the searched exercise does not exist' do + let(:uuid) { 'anotheruuid' } + + it 'renders correct response' do + post_request + expect(response).to have_http_status(:success) + + expect(JSON.parse(response.body).symbolize_keys[:exercise_found]).to be false + expect(JSON.parse(response.body).symbolize_keys[:message]).to(include('No corresponding exercise')) + end + end + end + + describe 'POST #import_exercise' do + let(:codeharbor_link) { FactoryBot.create(:codeharbor_link, user: user) } + let(:imported_exercise) { exercise } + let(:post_request) { post :import_exercise, body: zip_file_content } + let(:zip_file_content) { 'zipped task xml' } + let(:headers) { {'Authorization' => "Bearer #{codeharbor_link.api_key}"} } + + before do + request.headers.merge! headers + allow(ProformaService::Import).to receive(:call).and_return(imported_exercise) + end + + it 'responds with correct status code' do + post_request + expect(response).to have_http_status(:created) + end + + it 'calls service' do + post_request + expect(ProformaService::Import).to have_received(:call).with(zip: be_a(Tempfile).and(has_content(zip_file_content)), user: user) + end + + context 'when import fails with ProformaError' do + before { allow(ProformaService::Import).to receive(:call).and_raise(Proforma::PreImportValidationError) } + + it 'responds with correct status code' do + post_request + expect(response).to have_http_status(:bad_request) + end + end + + context 'when import fails due to another error' do + before { allow(ProformaService::Import).to receive(:call).and_raise(StandardError) } + + it 'responds with correct status code' do + post_request + expect(response).to have_http_status(:internal_server_error) + end + end + end + end diff --git a/spec/factories/codeharbor_link.rb b/spec/factories/codeharbor_link.rb new file mode 100644 index 00000000..c960eb9a --- /dev/null +++ b/spec/factories/codeharbor_link.rb @@ -0,0 +1,8 @@ +FactoryBot.define do + factory :codeharbor_link do + user { build(:teacher) } + push_url { 'http://push.url' } + check_uuid_url { 'http://check-uuid.url' } + sequence(:api_key) { |n| "api_key#{n}" } + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index f13a3edb..570c271b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -20,6 +20,8 @@ unless RUBY_PLATFORM == 'java' SimpleCov.start('rails') end +require 'webmock/rspec' + RSpec.configure do |config| # These two settings work together to allow you to limit a spec run # to individual examples or groups you care about by tagging them with diff --git a/spec/support/expectations/has_content.rb b/spec/support/expectations/has_content.rb new file mode 100644 index 00000000..e0f5f60c --- /dev/null +++ b/spec/support/expectations/has_content.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'rspec/expectations' + +RSpec::Matchers.define :has_content do |actual_content| + match do |file| + file.read == actual_content + end +end