diff --git a/Gemfile b/Gemfile index 6f9eeb837..69b7428cb 100644 --- a/Gemfile +++ b/Gemfile @@ -15,6 +15,7 @@ gem 'pg', '~> 1.1' gem 'puma', '~> 5.6' gem 'rack-cors' gem 'rails', '~> 7.0.0' +gem 'sentry-rails', '~> 5.5.0' gem 'sprockets-rails' gem 'stimulus-rails' gem 'turbo-rails' diff --git a/Gemfile.lock b/Gemfile.lock index 0c4c14262..7580d75dc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -247,6 +247,11 @@ GEM rubocop (~> 1.19) ruby-progressbar (1.11.0) ruby2_keywords (0.0.5) + sentry-rails (5.5.0) + railties (>= 5.0) + sentry-ruby (~> 5.5.0) + sentry-ruby (5.5.0) + concurrent-ruby (~> 1.0, >= 1.0.2) shoulda-matchers (5.0.0) activesupport (>= 5.2.0) simplecov (0.21.2) @@ -308,6 +313,7 @@ DEPENDENCIES rubocop rubocop-rails rubocop-rspec + sentry-rails (~> 5.5.0) shoulda-matchers (~> 5.0) simplecov sprockets-rails diff --git a/app/concepts/project/operation/create.rb b/app/concepts/project/operation/create.rb index 2fa9da926..2bd6c3e3c 100644 --- a/app/concepts/project/operation/create.rb +++ b/app/concepts/project/operation/create.rb @@ -1,13 +1,22 @@ # frozen_string_literal: true +require 'operation_response' + class Project module Operation class Create - require 'operation_response' - - DEFAULT_COMPONENT = { name: 'main', extension: 'py', default: true, index: 0 }.freeze - DEFAULT_PROJECT = { type: 'python', name: 'Untitled project', components: [DEFAULT_COMPONENT], - image_list: [] }.freeze + DEFAULT_COMPONENT = { + name: 'main', + extension: 'py', + default: true, + index: 0 + }.freeze + DEFAULT_PROJECT = { + type: 'python', + name: 'Untitled project', + components: [DEFAULT_COMPONENT], + image_list: [] + }.freeze class << self def call(user_id:, params:) @@ -26,8 +35,8 @@ def call(user_id:, params:) response[:project] = new_project response[:project].save! response - rescue StandardError - # TODO: log error + rescue StandardError => e + Sentry.capture_exception(e) response[:error] = 'Error creating project' response end diff --git a/app/concepts/project/operation/create_remix.rb b/app/concepts/project/operation/create_remix.rb index 42cea7ac2..0f49cd77a 100644 --- a/app/concepts/project/operation/create_remix.rb +++ b/app/concepts/project/operation/create_remix.rb @@ -1,16 +1,20 @@ # frozen_string_literal: true +require 'operation_response' + class Project module Operation class CreateRemix - require 'operation_response' - class << self def call(params:, user_id:, original_project:) response = OperationResponse.new validate_params(response, params, user_id, original_project) - remix_project(response, params, user_id, original_project) + remix_project(response, params, user_id, original_project) if response.success? + response + rescue StandardError => e + Sentry.capture_exception(e) + response[:error] = I18n.t('errors.project.remixing.cannot_save') response end @@ -22,11 +26,9 @@ def validate_params(response, params, user_id, original_project) end def remix_project(response, params, user_id, original_project) - return if response[:error] - response[:project] = create_remix(original_project, params, user_id) - response[:error] = I18n.t('errors.project.remixing.cannot_save') unless response[:project].save + response[:project].save! response end @@ -43,6 +45,7 @@ def create_remix(original_project, params, user_id) params[:components].each do |x| remix.components.build(x.slice(:name, :extension, :content, :index)) end + remix end end diff --git a/app/concepts/project/operation/update.rb b/app/concepts/project/operation/update.rb index cfde67f70..3d2e48787 100644 --- a/app/concepts/project/operation/update.rb +++ b/app/concepts/project/operation/update.rb @@ -1,22 +1,25 @@ # frozen_string_literal: true +require 'operation_response' + class Project module Operation class Update - require 'operation_response' - - def self.call(params:, project:) - response = setup_response(project) - - setup_deletions(response, params) - update_project_attributes(response, params) - update_component_attributes(response, params) - persist_changes(response) + class << self + def call(params:, project:) + response = setup_response(project) - response - end + setup_deletions(response, params) + update_project_attributes(response, params) + update_component_attributes(response, params) + persist_changes(response) + response + rescue StandardError => e + Sentry.capture_exception(e) + response[:error] ||= 'Error persisting changes' + response + end - class << self private def setup_response(project) @@ -41,13 +44,13 @@ def validate_deletions(response) end def update_project_attributes(response, params) - return if response[:error] + return if response.failure? response[:project].assign_attributes(params.slice(:name)) end def update_component_attributes(response, params) - return if response[:error] + return if response.failure? params[:components].each do |component_params| if component_params[:id].present? @@ -60,14 +63,11 @@ def update_component_attributes(response, params) end def persist_changes(response) - return if response[:error] + return if response.failure? ActiveRecord::Base.transaction do response[:project].save! response[:project].components.where(id: response[:component_ids_to_delete]).destroy_all - rescue StandardError - # TODO: log error? - response[:error] = 'Error persisting changes' end end end diff --git a/config/initializers/auth.rb b/config/initializers/auth.rb index cd170d065..9b97cd4e3 100644 --- a/config/initializers/auth.rb +++ b/config/initializers/auth.rb @@ -1,4 +1,4 @@ # frozen_string_literal: true BYPASS_AUTH = ENV['BYPASS_AUTH'] == 'true' -AUTH_USER_ID = ENV['AUTH_USER_ID'] if BYPASS_AUTH +AUTH_USER_ID = ENV.fetch('AUTH_USER_ID', nil) if BYPASS_AUTH diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb new file mode 100644 index 000000000..22f7cf011 --- /dev/null +++ b/config/initializers/sentry.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +Sentry.init do |config| + config.dsn = ENV.fetch('SENTRY_DSN', nil) if Rails.env.production? + config.breadcrumbs_logger = [:active_support_logger] + config.environment = ENV.fetch('SENTRY_CURRENT_ENV', nil) || ENV.fetch('RAILS_ENV', nil) + + config.traces_sample_rate = 0.5 +end diff --git a/lib/operation_response.rb b/lib/operation_response.rb index 024e33f97..53b0bdb90 100644 --- a/lib/operation_response.rb +++ b/lib/operation_response.rb @@ -2,9 +2,7 @@ class OperationResponse < Hash def success? - return false unless self[:error].nil? - - true + fetch(:error, nil).nil? end def failure? diff --git a/spec/concepts/project/operation/create_remix_spec.rb b/spec/concepts/project/operation/create_remix_spec.rb index 07e74d268..17dae5db7 100644 --- a/spec/concepts/project/operation/create_remix_spec.rb +++ b/spec/concepts/project/operation/create_remix_spec.rb @@ -33,7 +33,7 @@ it 'returns success' do result = create_remix - expect(result.success?).to eq(true) + expect(result.success?).to be(true) end it 'creates new project' do @@ -104,7 +104,7 @@ it 'returns failure' do result = create_remix - expect(result.failure?).to eq(true) + expect(result.failure?).to be(true) end it 'returns error message' do @@ -122,7 +122,7 @@ it 'returns failure' do result = create_remix - expect(result.failure?).to eq(true) + expect(result.failure?).to be(true) end it 'returns error message' do @@ -143,7 +143,7 @@ end it 'returns failure' do - expect(create_remix.failure?).to eq(true) + expect(create_remix.failure?).to be(true) end it 'sets error message' do diff --git a/spec/concepts/project/operation/create_spec.rb b/spec/concepts/project/operation/create_spec.rb index afd19fa09..72e10a211 100644 --- a/spec/concepts/project/operation/create_spec.rb +++ b/spec/concepts/project/operation/create_spec.rb @@ -14,7 +14,7 @@ describe '.call' do it 'returns success' do - expect(create_project.success?).to eq(true) + expect(create_project.success?).to be(true) end it 'creates a new project' do @@ -57,7 +57,7 @@ end it 'returns success' do - expect(create_project_with_content.success?).to eq(true) + expect(create_project_with_content.success?).to be(true) end it 'returns project with correct component content' do @@ -71,15 +71,21 @@ mock_project = instance_double(Project) allow(mock_project).to receive(:components).and_raise('Some error') allow(Project).to receive(:new).and_return(mock_project) + allow(Sentry).to receive(:capture_exception) end it 'returns failure' do - expect(create_project.failure?).to eq(true) + expect(create_project.failure?).to be(true) end it 'returns error message' do expect(create_project[:error]).to eq('Error creating project') end + + it 'sent the exception to Sentry' do + create_project + expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + end end 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 4c55207bc..e876abf55 100644 --- a/spec/concepts/project/operation/update_default_component_spec.rb +++ b/spec/concepts/project/operation/update_default_component_spec.rb @@ -18,7 +18,7 @@ end it 'returns failure? true' do - expect(update.failure?).to eq(true) + expect(update.failure?).to be(true) end it 'returns error message' do diff --git a/spec/concepts/project/operation/update_delete_components_spec.rb b/spec/concepts/project/operation/update_delete_components_spec.rb index ad08160f3..c9b25c82e 100644 --- a/spec/concepts/project/operation/update_delete_components_spec.rb +++ b/spec/concepts/project/operation/update_delete_components_spec.rb @@ -34,7 +34,7 @@ it 'deletes the correct component' do update - expect(Component.find_by(id: component_to_delete.id)).to eq(nil) + expect(Component.find_by(id: component_to_delete.id)).to be_nil end end end diff --git a/spec/concepts/project/operation/update_invalid_spec.rb b/spec/concepts/project/operation/update_invalid_spec.rb index 449faa2fd..6ec0968b6 100644 --- a/spec/concepts/project/operation/update_invalid_spec.rb +++ b/spec/concepts/project/operation/update_invalid_spec.rb @@ -28,7 +28,7 @@ describe '.call' do context 'when updated project component is invalid' do it 'returns failure? true' do - expect(update.failure?).to eq(true) + expect(update.failure?).to be(true) end it 'does not amend any project properties' do diff --git a/spec/concepts/project/operation/update_spec.rb b/spec/concepts/project/operation/update_spec.rb index a0ea8a655..4fcc6277f 100644 --- a/spec/concepts/project/operation/update_spec.rb +++ b/spec/concepts/project/operation/update_spec.rb @@ -30,7 +30,7 @@ let(:component_params) { [default_component_params, edited_component_params] } it 'returns success? true' do - expect(update.success?).to eq(true) + expect(update.success?).to be(true) end it 'updates project properties' do @@ -54,7 +54,7 @@ it 'creates component with correct properties' do update created_component = project.components.find_by(**new_component_params) - expect(created_component).not_to eq(nil) + expect(created_component).not_to be_nil end end end diff --git a/spec/lib/operation_response_spec.rb b/spec/lib/operation_response_spec.rb index faa9b59ca..7cf83903e 100644 --- a/spec/lib/operation_response_spec.rb +++ b/spec/lib/operation_response_spec.rb @@ -8,7 +8,7 @@ context 'when :error not present' do it 'returns true' do response = described_class.new - expect(response.success?).to eq(true) + expect(response.success?).to be(true) end end @@ -16,7 +16,7 @@ it 'returns false' do response = described_class.new response[:error] = 'An error' - expect(response.success?).to eq(false) + expect(response.success?).to be(false) end end end @@ -25,7 +25,7 @@ context 'when :error not present' do it 'returns false' do response = described_class.new - expect(response.failure?).to eq(false) + expect(response.failure?).to be(false) end end @@ -33,7 +33,7 @@ it 'returns true' do response = described_class.new response[:error] = 'An error' - expect(response.failure?).to eq(true) + expect(response.failure?).to be(true) end end end diff --git a/spec/models/component_spec.rb b/spec/models/component_spec.rb index 9fd7812d9..ebb180b12 100644 --- a/spec/models/component_spec.rb +++ b/spec/models/component_spec.rb @@ -17,7 +17,7 @@ describe 'validations' do it 'returns valid? false when name changed' do component.name = 'updated' - expect(component.valid?).to eq(false) + expect(component.valid?).to be(false) end it 'sets error message when name changed' do @@ -29,7 +29,7 @@ it 'returns valid? false when extension changed' do component.extension = 'txt' - expect(component.valid?).to eq(false) + expect(component.valid?).to be(false) end it 'sets error message when extension changed' do diff --git a/spec/request/projects/index_spec.rb b/spec/request/projects/index_spec.rb index 8bd7d526d..7c51c672d 100644 --- a/spec/request/projects/index_spec.rb +++ b/spec/request/projects/index_spec.rb @@ -30,7 +30,7 @@ it 'returns users projects' do get '/api/projects' returned = JSON.parse(response.body) - expect(returned.all? { |proj| proj['user_id'] == user_id }).to eq(true) + expect(returned.all? { |proj| proj['user_id'] == user_id }).to be(true) end end