From 768104b547e75adf9cb8873ee64d203d748295dc Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Tue, 28 Jun 2022 16:39:22 +0100 Subject: [PATCH 01/26] Upgrade Ruby & Rails - coffee-rails and web-console upgrades were necessary to resolve dependency conflicts - removal of reference to was necessary as that method no longer exists in later Rails versions - see https://stackoverflow.com/questions/37464966/what-causes-deprecation-warning-activerecordbase-raise-in-transactional-callb --- .ruby-version | 2 +- Gemfile | 6 +- Gemfile.lock | 286 ++++++++++++++++++++++++------------------ config/application.rb | 1 - 4 files changed, 166 insertions(+), 129 deletions(-) diff --git a/.ruby-version b/.ruby-version index 338a5b5d..5b013b97 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.6.6 +2.7.6 \ No newline at end of file diff --git a/Gemfile b/Gemfile index 5bcae34a..254e67e7 100644 --- a/Gemfile +++ b/Gemfile @@ -1,7 +1,7 @@ source 'https://rubygems.org' # Bundle edge Rails instead: gem 'rails', github: 'rails/rails' -gem 'rails', '~> 4.2.6' +gem 'rails', '~> 6.1.6' # Use mysql2 as the database for Active Record gem 'mysql2' # Use SCSS for stylesheets @@ -40,12 +40,12 @@ group :development, :test do # Use Uglifier as compressor for JavaScript assets gem 'uglifier', '>= 1.3.0' # Use CoffeeScript for .coffee assets and views - gem 'coffee-rails', '~> 4.1.0' + gem 'coffee-rails', '~> 4.2.2' end group :development do # Access an IRB console on exception pages or by using <%= console %> in views - gem 'web-console', '~> 2.0' + gem 'web-console', '~> 4.0.4' # displays speed badge for every html page gem 'rack-mini-profiler' # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring diff --git a/Gemfile.lock b/Gemfile.lock index de130214..8b706540 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,54 +1,78 @@ GEM remote: https://rubygems.org/ specs: - actionmailer (4.2.11.3) - actionpack (= 4.2.11.3) - actionview (= 4.2.11.3) - activejob (= 4.2.11.3) + actioncable (6.1.6) + actionpack (= 6.1.6) + activesupport (= 6.1.6) + nio4r (~> 2.0) + websocket-driver (>= 0.6.1) + actionmailbox (6.1.6) + actionpack (= 6.1.6) + activejob (= 6.1.6) + activerecord (= 6.1.6) + activestorage (= 6.1.6) + activesupport (= 6.1.6) + mail (>= 2.7.1) + actionmailer (6.1.6) + actionpack (= 6.1.6) + actionview (= 6.1.6) + activejob (= 6.1.6) + activesupport (= 6.1.6) mail (~> 2.5, >= 2.5.4) - rails-dom-testing (~> 1.0, >= 1.0.5) - actionpack (4.2.11.3) - actionview (= 4.2.11.3) - activesupport (= 4.2.11.3) - rack (~> 1.6) - rack-test (~> 0.6.2) - rails-dom-testing (~> 1.0, >= 1.0.5) - rails-html-sanitizer (~> 1.0, >= 1.0.2) - actionview (4.2.11.3) - activesupport (= 4.2.11.3) - builder (~> 3.1) - erubis (~> 2.7.0) - rails-dom-testing (~> 1.0, >= 1.0.5) - rails-html-sanitizer (~> 1.0, >= 1.0.3) - activejob (4.2.11.3) - activesupport (= 4.2.11.3) - globalid (>= 0.3.0) - activemodel (4.2.11.3) - activesupport (= 4.2.11.3) + rails-dom-testing (~> 2.0) + actionpack (6.1.6) + actionview (= 6.1.6) + activesupport (= 6.1.6) + rack (~> 2.0, >= 2.0.9) + rack-test (>= 0.6.3) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.0, >= 1.2.0) + actiontext (6.1.6) + actionpack (= 6.1.6) + activerecord (= 6.1.6) + activestorage (= 6.1.6) + activesupport (= 6.1.6) + nokogiri (>= 1.8.5) + actionview (6.1.6) + activesupport (= 6.1.6) builder (~> 3.1) - activerecord (4.2.11.3) - activemodel (= 4.2.11.3) - activesupport (= 4.2.11.3) - arel (~> 6.0) - activesupport (4.2.11.3) - i18n (~> 0.7) - minitest (~> 5.1) - thread_safe (~> 0.3, >= 0.3.4) - tzinfo (~> 1.1) + erubi (~> 1.4) + rails-dom-testing (~> 2.0) + rails-html-sanitizer (~> 1.1, >= 1.2.0) + activejob (6.1.6) + activesupport (= 6.1.6) + globalid (>= 0.3.6) + activemodel (6.1.6) + activesupport (= 6.1.6) + activerecord (6.1.6) + activemodel (= 6.1.6) + activesupport (= 6.1.6) + activestorage (6.1.6) + actionpack (= 6.1.6) + activejob (= 6.1.6) + activerecord (= 6.1.6) + activesupport (= 6.1.6) + marcel (~> 1.0) + mini_mime (>= 1.1.0) + activesupport (6.1.6) + concurrent-ruby (~> 1.0, >= 1.0.2) + i18n (>= 1.6, < 2) + minitest (>= 5.1) + tzinfo (~> 2.0) + zeitwerk (~> 2.3) addressable (2.8.0) public_suffix (>= 2.0.2, < 5.0) - arel (6.0.4) ast (2.4.2) - autoprefixer-rails (10.2.4.0) - execjs - binding_of_caller (1.0.0) - debug_inspector (>= 0.0.1) + autoprefixer-rails (10.4.7.0) + execjs (~> 2) + bindex (0.8.1) bootstrap-sass (3.4.1) autoprefixer-rails (>= 5.2.1) sassc (>= 2.0.0) builder (3.2.4) - capybara (3.35.3) + capybara (3.37.1) addressable + matrix mini_mime (>= 0.1.3) nokogiri (~> 1.8) rack (>= 1.6.0) @@ -58,16 +82,16 @@ GEM capybara-selenium (0.0.6) capybara selenium-webdriver - childprocess (3.0.0) + childprocess (4.1.0) coderay (1.1.3) - coffee-rails (4.1.1) + coffee-rails (4.2.2) coffee-script (>= 2.2.0) - railties (>= 4.0.0, < 5.1.x) + railties (>= 4.0.0) coffee-script (2.4.1) coffee-script-source execjs coffee-script-source (1.12.2) - concurrent-ruby (1.1.8) + concurrent-ruby (1.1.10) coveralls (0.8.23) json (>= 1.8, < 3) simplecov (~> 0.16.1) @@ -75,41 +99,48 @@ GEM thor (>= 0.19.4, < 2.0) tins (~> 1.6) crass (1.0.6) - database_cleaner (1.99.0) - debug_inspector (1.0.0) - diff-lcs (1.4.4) - docile (1.3.5) - erubis (2.7.0) - exception_notification (4.4.3) - actionmailer (>= 4.0, < 7) - activesupport (>= 4.0, < 7) - execjs (2.7.0) - factory_bot (5.2.0) - activesupport (>= 4.2.0) - ffi (1.14.2) - globalid (0.4.2) - activesupport (>= 4.2.0) - i18n (0.9.5) + database_cleaner (2.0.1) + database_cleaner-active_record (~> 2.0.0) + database_cleaner-active_record (2.0.1) + activerecord (>= 5.a) + database_cleaner-core (~> 2.0.0) + database_cleaner-core (2.0.1) + diff-lcs (1.5.0) + docile (1.4.0) + erubi (1.10.0) + exception_notification (4.5.0) + actionmailer (>= 5.2, < 8) + activesupport (>= 5.2, < 8) + execjs (2.8.1) + factory_bot (6.2.1) + activesupport (>= 5.0.0) + ffi (1.15.5) + globalid (1.0.0) + activesupport (>= 5.0) + i18n (1.10.0) concurrent-ruby (~> 1.0) - jbuilder (2.9.1) - activesupport (>= 4.2.0) - jquery-rails (4.4.0) + jbuilder (2.11.5) + actionview (>= 5.0.0) + activesupport (>= 5.0.0) + jquery-rails (4.5.0) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) json (1.8.6) launchy (2.5.0) addressable (~> 2.7) - loofah (2.9.0) + loofah (2.18.0) crass (~> 1.0.2) nokogiri (>= 1.5.9) mail (2.7.1) mini_mime (>= 0.1.1) + marcel (1.0.2) + matrix (0.4.2) method_source (1.0.0) - mini_mime (1.0.2) + mini_mime (1.1.2) mini_portile2 (2.8.0) - minitest (5.14.3) - mysql2 (0.5.3) + minitest (5.16.1) + mysql2 (0.5.4) nio4r (2.5.8) nokogiri (1.13.6) mini_portile2 (~> 2.8.0) @@ -124,38 +155,40 @@ GEM puma (5.6.4) nio4r (~> 2.0) racc (1.6.0) - rack (1.6.13) - rack-mini-profiler (2.3.1) + rack (2.2.3.1) + rack-mini-profiler (3.0.0) rack (>= 1.2.0) - rack-test (0.6.3) - rack (>= 1.0) - rails (4.2.11.3) - actionmailer (= 4.2.11.3) - actionpack (= 4.2.11.3) - actionview (= 4.2.11.3) - activejob (= 4.2.11.3) - activemodel (= 4.2.11.3) - activerecord (= 4.2.11.3) - activesupport (= 4.2.11.3) - bundler (>= 1.3.0, < 2.0) - railties (= 4.2.11.3) - sprockets-rails - rails-deprecated_sanitizer (1.0.3) - activesupport (>= 4.2.0.alpha) - rails-dom-testing (1.0.9) - activesupport (>= 4.2.0, < 5.0) - nokogiri (~> 1.6) - rails-deprecated_sanitizer (>= 1.0.1) - rails-html-sanitizer (1.3.0) + rack-test (2.0.2) + rack (>= 1.3) + rails (6.1.6) + actioncable (= 6.1.6) + actionmailbox (= 6.1.6) + actionmailer (= 6.1.6) + actionpack (= 6.1.6) + actiontext (= 6.1.6) + actionview (= 6.1.6) + activejob (= 6.1.6) + activemodel (= 6.1.6) + activerecord (= 6.1.6) + activestorage (= 6.1.6) + activesupport (= 6.1.6) + bundler (>= 1.15.0) + railties (= 6.1.6) + sprockets-rails (>= 2.0.0) + rails-dom-testing (2.0.3) + activesupport (>= 4.2.0) + nokogiri (>= 1.6) + rails-html-sanitizer (1.4.3) loofah (~> 2.3) - railties (4.2.11.3) - actionpack (= 4.2.11.3) - activesupport (= 4.2.11.3) - rake (>= 0.8.7) - thor (>= 0.18.1, < 2.0) + railties (6.1.6) + actionpack (= 6.1.6) + activesupport (= 6.1.6) + method_source + rake (>= 12.2) + thor (~> 1.0) rainbow (3.1.1) - rake (13.0.3) - rb-fsevent (0.10.4) + rake (13.0.6) + rb-fsevent (0.11.1) rb-inotify (0.10.1) ffi (~> 1.0) rdoc (4.3.0) @@ -180,7 +213,7 @@ GEM rspec-mocks (~> 3.5.0) rspec-support (~> 3.5.0) rspec-support (3.5.0) - rubocop (1.30.1) + rubocop (1.31.0) parallel (~> 1.10) parser (>= 3.1.0.0) rainbow (>= 2.2.2, < 4.0) @@ -194,13 +227,12 @@ GEM rubocop-performance (1.14.2) rubocop (>= 1.7.0, < 2.0) rubocop-ast (>= 0.4.0) - rubocop-rails (2.9.1) + rubocop-rails (2.15.1) activesupport (>= 4.2.0) rack (>= 1.1) - rubocop (>= 0.90.0, < 2.0) - rubocop-rspec (2.2.0) - rubocop (~> 1.0) - rubocop-ast (>= 1.1.0) + rubocop (>= 1.7.0, < 2.0) + rubocop-rspec (2.11.1) + rubocop (~> 1.19) ruby-progressbar (1.11.0) rubyzip (2.3.2) sass (3.7.4) @@ -208,8 +240,8 @@ GEM sass-listen (4.0.0) rb-fsevent (~> 0.9, >= 0.9.4) rb-inotify (~> 0.9, >= 0.9.7) - sass-rails (5.0.7) - railties (>= 4.0.0, < 6) + sass-rails (5.1.0) + railties (>= 5.2.0) sass (~> 3.1) sprockets (>= 2.8, < 4.0) sprockets-rails (>= 2.0, < 4.0) @@ -219,50 +251,56 @@ GEM sdoc (0.4.2) json (~> 1.7, >= 1.7.7) rdoc (~> 4.0) - selenium-webdriver (3.142.7) - childprocess (>= 0.5, < 4.0) - rubyzip (>= 1.2.2) + selenium-webdriver (4.3.0) + childprocess (>= 0.5, < 5.0) + rexml (~> 3.2, >= 3.2.5) + rubyzip (>= 1.2.2, < 3.0) + websocket (~> 1.0) simplecov (0.16.1) docile (~> 1.1) json (>= 1.8, < 3) simplecov-html (~> 0.10.0) simplecov-html (0.10.2) - spring (2.1.1) + spring (4.0.0) sprockets (3.7.2) concurrent-ruby (~> 1.0) rack (> 1, < 3) - sprockets-rails (3.2.2) - actionpack (>= 4.0) - activesupport (>= 4.0) + sprockets-rails (3.4.2) + actionpack (>= 5.2) + activesupport (>= 5.2) sprockets (>= 3.0.0) sync (0.5.0) term-ansicolor (1.7.1) tins (~> 1.0) - thor (1.1.0) - thread_safe (0.3.6) + thor (1.2.1) tilt (2.0.10) timecop (0.9.5) - tins (1.28.0) + tins (1.31.1) sync turbolinks (5.2.1) turbolinks-source (~> 5.2) turbolinks-source (5.2.0) - tzinfo (1.2.9) - thread_safe (~> 0.1) + tzinfo (2.0.4) + concurrent-ruby (~> 1.0) uglifier (4.2.0) execjs (>= 0.3.0, < 3) - unicode-display_width (2.1.0) - web-console (2.3.0) - activemodel (>= 4.0) - binding_of_caller (>= 0.7.2) - railties (>= 4.0) - sprockets-rails (>= 2.0, < 4.0) - webdrivers (4.7.0) + unicode-display_width (2.2.0) + web-console (4.0.4) + actionview (>= 6.0.0) + activemodel (>= 6.0.0) + bindex (>= 0.4.0) + railties (>= 6.0.0) + webdrivers (5.0.0) nokogiri (~> 1.6) rubyzip (>= 1.3.0) - selenium-webdriver (> 3.141, < 5.0) + selenium-webdriver (~> 4.0) + websocket (1.2.9) + websocket-driver (0.7.5) + websocket-extensions (>= 0.1.0) + websocket-extensions (0.1.5) xpath (3.2.0) nokogiri (~> 1.8) + zeitwerk (2.6.0) PLATFORMS ruby @@ -271,7 +309,7 @@ DEPENDENCIES bootstrap-sass (= 3.4.1) capybara capybara-selenium - coffee-rails (~> 4.1.0) + coffee-rails (~> 4.2.2) coveralls database_cleaner exception_notification @@ -283,7 +321,7 @@ DEPENDENCIES pry puma rack-mini-profiler - rails (~> 4.2.6) + rails (~> 6.1.6) rspec-collection_matchers rspec-rails (~> 3.5.0) rubocop @@ -296,7 +334,7 @@ DEPENDENCIES timecop turbolinks uglifier (>= 1.3.0) - web-console (~> 2.0) + web-console (~> 4.0.4) webdrivers BUNDLED WITH diff --git a/config/application.rb b/config/application.rb index 8533a562..7c986079 100644 --- a/config/application.rb +++ b/config/application.rb @@ -23,7 +23,6 @@ class Application < Rails::Application config.mailer = YAML.load_file("#{Rails.root}/config/mailer.yml")[Rails.env] # Do not swallow errors in after_commit/after_rollback callbacks. - config.active_record.raise_in_transactional_callbacks = true config.autoload_paths += %W[#{config.root}/lib/utils] config.disable_animations = false end From 9cedabeb273378fe7e71f1a722cfe790f9e68ef7 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 11:00:28 +0100 Subject: [PATCH 02/26] Fix issue with db:reset - Rails 5 changed the default type for id columns from int to bigint - This meant that when regenerating the db from the schema, the foreign key types now mismatched the id types - Probably more correct to migrate the columns to bigint - consider doing this instead --- db/schema.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index e2009155..7c2c90f5 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -13,7 +13,7 @@ ActiveRecord::Schema.define(version: 20190924154728) do - create_table "asset_types", force: :cascade do |t| + create_table "asset_types", id: :integer, force: :cascade do |t| t.string "name", limit: 255, null: false t.string "identifier_type", limit: 255, null: false t.boolean "has_sample_count", default: false, null: false @@ -23,7 +23,7 @@ t.string "labware_type", limit: 255 end - create_table "assets", force: :cascade do |t| + create_table "assets", id: :integer, force: :cascade do |t| t.string "identifier", limit: 255, null: false t.integer "asset_type_id", limit: 4, null: false t.integer "workflow_id", limit: 4, null: false @@ -47,39 +47,39 @@ add_index "assets", ["identifier"], name: "index_assets_on_identifier", using: :btree add_index "assets", ["workflow_id"], name: "index_assets_on_workflow_id", using: :btree - create_table "batches", force: :cascade do |t| + create_table "batches", id: :integer, force: :cascade do |t| t.datetime "created_at" t.datetime "updated_at" end - create_table "comments", force: :cascade do |t| + create_table "comments", id: :integer, force: :cascade do |t| t.text "comment", limit: 65535 t.datetime "created_at" t.datetime "updated_at" end - create_table "cost_codes", force: :cascade do |t| + create_table "cost_codes", id: :integer, force: :cascade do |t| t.string "name", limit: 255, null: false end - create_table "events", force: :cascade do |t| + create_table "events", id: :integer, force: :cascade do |t| t.integer "asset_id", limit: 4, null: false t.integer "state_id", limit: 4, null: false t.datetime "created_at" t.datetime "updated_at" end - create_table "pipeline_destinations", force: :cascade do |t| + create_table "pipeline_destinations", id: :integer, force: :cascade do |t| t.string "name", limit: 255 end - create_table "states", force: :cascade do |t| + create_table "states", id: :integer, force: :cascade do |t| t.string "name", limit: 255, null: false t.datetime "created_at" t.datetime "updated_at" end - create_table "workflows", force: :cascade do |t| + create_table "workflows", id: :integer, force: :cascade do |t| t.string "name", limit: 255, null: false t.boolean "has_comment", default: false, null: false t.datetime "created_at" From 3489094cbf72a1557e145fe90b34cf3a1c31d38b Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 11:13:01 +0100 Subject: [PATCH 03/26] rubocop 'Rails/ApplicationRecord: Models should subclass ApplicationRecord.' - also need to add ApplicationRecord class, as per Rails upgrade guide --- app/models/application_record.rb | 3 +++ app/models/asset.rb | 2 +- app/models/asset_type.rb | 2 +- app/models/batch.rb | 2 +- app/models/comment.rb | 2 +- app/models/cost_code.rb | 2 +- app/models/event.rb | 2 +- app/models/pipeline_destination.rb | 2 +- app/models/state.rb | 2 +- app/models/workflow.rb | 2 +- 10 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 app/models/application_record.rb diff --git a/app/models/application_record.rb b/app/models/application_record.rb new file mode 100644 index 00000000..10a4cba8 --- /dev/null +++ b/app/models/application_record.rb @@ -0,0 +1,3 @@ +class ApplicationRecord < ActiveRecord::Base + self.abstract_class = true +end diff --git a/app/models/asset.rb b/app/models/asset.rb index 2a116849..6c11ae56 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -1,6 +1,6 @@ require './lib/client_side_validations' -class Asset < ActiveRecord::Base +class Asset < ApplicationRecord include StateMachine belongs_to :asset_type diff --git a/app/models/asset_type.rb b/app/models/asset_type.rb index 9ef72012..33a424f2 100644 --- a/app/models/asset_type.rb +++ b/app/models/asset_type.rb @@ -1,4 +1,4 @@ -class AssetType < ActiveRecord::Base +class AssetType < ApplicationRecord validates :name, :identifier_type, :labware_type, presence: true validates :name, uniqueness: true diff --git a/app/models/batch.rb b/app/models/batch.rb index 1539039d..b9ed6c93 100644 --- a/app/models/batch.rb +++ b/app/models/batch.rb @@ -1,4 +1,4 @@ -class Batch < ActiveRecord::Base +class Batch < ApplicationRecord has_many :assets has_many :comments, through: :assets diff --git a/app/models/comment.rb b/app/models/comment.rb index 70ce9451..74bf85bb 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,4 +1,4 @@ -class Comment < ActiveRecord::Base +class Comment < ApplicationRecord has_many :assets before_destroy :no_assets? diff --git a/app/models/cost_code.rb b/app/models/cost_code.rb index dce47809..03ba5925 100644 --- a/app/models/cost_code.rb +++ b/app/models/cost_code.rb @@ -1,6 +1,6 @@ require './lib/client_side_validations' -class CostCode < ActiveRecord::Base +class CostCode < ApplicationRecord validates :name, presence: true validates :name, uniqueness: true diff --git a/app/models/event.rb b/app/models/event.rb index 105071a7..db821ef5 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -1,4 +1,4 @@ -class Event < ActiveRecord::Base +class Event < ApplicationRecord belongs_to :asset belongs_to :state diff --git a/app/models/pipeline_destination.rb b/app/models/pipeline_destination.rb index 89a211bd..058a8f08 100644 --- a/app/models/pipeline_destination.rb +++ b/app/models/pipeline_destination.rb @@ -1,4 +1,4 @@ -class PipelineDestination < ActiveRecord::Base +class PipelineDestination < ApplicationRecord validates :name, presence: true validates :name, uniqueness: true diff --git a/app/models/state.rb b/app/models/state.rb index e9de347b..a19351e7 100644 --- a/app/models/state.rb +++ b/app/models/state.rb @@ -1,4 +1,4 @@ -class State < ActiveRecord::Base +class State < ApplicationRecord has_many :events has_many :workflows diff --git a/app/models/workflow.rb b/app/models/workflow.rb index bbf71088..f348feea 100644 --- a/app/models/workflow.rb +++ b/app/models/workflow.rb @@ -1,4 +1,4 @@ -class Workflow < ActiveRecord::Base +class Workflow < ApplicationRecord has_many :assets belongs_to :initial_state, class_name: 'State' From 7bde03e991cf7a37eda131c157c432307b0e5dde Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 11:15:25 +0100 Subject: [PATCH 04/26] rubocop - Rails/RedundantPresenceValidationOnBelongsTo: Remove explicit presence validation for workflow/batch/asset_type. - Since Rails 5.0 the default for belongs_to is optional: false ... (https://docs.rubocop.org/rubocop-rails/cops_rails.html#railsredundantpresencevalidationonbelongsto) --- app/models/asset.rb | 2 +- app/models/event.rb | 2 -- app/models/workflow.rb | 2 +- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/app/models/asset.rb b/app/models/asset.rb index 6c11ae56..a29351d9 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -17,7 +17,7 @@ class Asset < ApplicationRecord include ClientSideValidations validate_with_regexp :study, with: /^\w+$/ - validates :workflow, :batch, :identifier, :asset_type, presence: true + validates :identifier, presence: true delegate :identifier_type, to: :asset_type default_scope { includes(:workflow, :asset_type, :comment, :batch, :pipeline_destination, events: :state) } diff --git a/app/models/event.rb b/app/models/event.rb index db821ef5..abe84519 100644 --- a/app/models/event.rb +++ b/app/models/event.rb @@ -2,8 +2,6 @@ class Event < ApplicationRecord belongs_to :asset belongs_to :state - validates :asset, :state, presence: true - attr_accessor :state_name def state_name=(state_name) diff --git a/app/models/workflow.rb b/app/models/workflow.rb index f348feea..c031e810 100644 --- a/app/models/workflow.rb +++ b/app/models/workflow.rb @@ -2,7 +2,7 @@ class Workflow < ApplicationRecord has_many :assets belongs_to :initial_state, class_name: 'State' - validates :name, :initial_state, presence: true + validates :name, presence: true validates :name, uniqueness: true validates :turn_around_days, numericality: { greater_than_or_equal_to: 0, allow_nil: true, only_integer: true } From 3d36047dac1567ec9982cbf019d467e11f4e7b1e Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 11:16:03 +0100 Subject: [PATCH 05/26] rubocop --- spec/models/asset_type_spec.rb | 2 +- spec/presenters/report_new_presenter_spec.rb | 12 ++++++------ spec/presenters/report_show_presenter_spec.rb | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/models/asset_type_spec.rb b/spec/models/asset_type_spec.rb index 7750fad6..72d65b7a 100644 --- a/spec/models/asset_type_spec.rb +++ b/spec/models/asset_type_spec.rb @@ -13,7 +13,7 @@ expect(asset_type).to have(0).errors_on(:identifier_type) expect(asset_type.name).to eq(test_name) expect(asset_type.identifier_type).to eq(test_identifier_type) - expect(asset_type.has_sample_count).to eq(false) + expect(asset_type.has_sample_count).to be(false) expect(asset_type.identifier_data_type).to eq('alphanumeric') expect(asset_type.labware_type).to eq('Plate') end diff --git a/spec/presenters/report_new_presenter_spec.rb b/spec/presenters/report_new_presenter_spec.rb index 4b0b2ef2..e332a979 100644 --- a/spec/presenters/report_new_presenter_spec.rb +++ b/spec/presenters/report_new_presenter_spec.rb @@ -11,10 +11,10 @@ context 'with empty report' do it 'does not have report data' do - expect(presenter.workflow).to be nil - expect(presenter.start_date).to be nil - expect(presenter.end_date).to be nil - expect(presenter.flash).to be nil + expect(presenter.workflow).to be_nil + expect(presenter.start_date).to be_nil + expect(presenter.end_date).to be_nil + expect(presenter.flash).to be_nil expect(presenter.action).to eq '/reports' expect(presenter.page).to eq :'reports/new' end @@ -29,7 +29,7 @@ expect(presenter.workflow).to eq 'Workflow' expect(presenter.start_date).to eq '01/04/2017' expect(presenter.end_date).to eq '15/04/2017' - expect(presenter.flash).to be nil + expect(presenter.flash).to be_nil expect(presenter.action).to eq '/reports' expect(presenter.page).to eq :'reports/new' end @@ -44,7 +44,7 @@ expect(presenter.report.valid?).to be false expect(presenter.workflow).to eq 'Workflow' expect(presenter.start_date).to eq '01/04/2017' - expect(presenter.end_date).to be nil + expect(presenter.end_date).to be_nil expect(presenter.flash).to eq "End date can't be blank" end end diff --git a/spec/presenters/report_show_presenter_spec.rb b/spec/presenters/report_show_presenter_spec.rb index 4136d5b0..a493aec2 100644 --- a/spec/presenters/report_show_presenter_spec.rb +++ b/spec/presenters/report_show_presenter_spec.rb @@ -39,7 +39,7 @@ "A1" + "1" + '' - expect(presenter.flash).to be nil + expect(presenter.flash).to be_nil expect(presenter.page).to eq :'reports/show' Timecop.return end From 70abffd70365b4d313e7e91712f4aff3a9605c4c Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 11:30:50 +0100 Subject: [PATCH 06/26] rubocop --- app/controllers/assets_controller.rb | 2 +- app/controllers/batches_controller.rb | 8 +++---- .../pipeline_destinations_controller.rb | 2 +- app/controllers/workflows_controller.rb | 4 ++-- app/models/application_record.rb | 4 ++++ config/locales/en.yml | 21 +++++++++++++++++++ 6 files changed, 33 insertions(+), 8 deletions(-) diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index 0c15147b..03263b2f 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -8,7 +8,7 @@ def update flash[updater.flash_status] = updater.message redirect_to("/assets?state=#{updater.redirect_state}") else - flash[:error] = 'No assets selected' + flash[:error] = I18n.t('assets.errors.none_selected') redirect_to :back end end diff --git a/app/controllers/batches_controller.rb b/app/controllers/batches_controller.rb index c820d357..b830509d 100644 --- a/app/controllers/batches_controller.rb +++ b/app/controllers/batches_controller.rb @@ -26,7 +26,7 @@ def update if batch_updater.valid? batch = batch_updater.update! @presenter = Presenter::BatchPresenter::Show.new(batch) - flash[:notice] = 'The batch was updated.' + flash[:notice] = I18n.t('batches.success.updated') redirect_to("/batches/#{params[:id]}") else flash[:error] = batch_updater.errors.full_messages.join('; ') @@ -36,7 +36,7 @@ def update def destroy batch.destroy! - flash[:notice] = 'The batch was deleted.' + flash[:notice] = I18n.t('batches.success.deleted') redirect_to('/batches/new') end @@ -55,7 +55,7 @@ def create if batch_creator.valid? batch = batch_creator.create! @presenter = Presenter::BatchPresenter::Show.new(batch) - flash[:notice] = 'The batch was created.' + flash[:notice] = I18n.t('batches.success.created') redirect_to("/batches/#{@presenter.id}") else flash[:error] = batch_creator.errors.full_messages.join('; ') @@ -85,7 +85,7 @@ def batch if params[:id].present? @batch ||= Batch.find_by(id: params[:id]) else - flash[:error] = 'You must specify a batch.' + flash[:error] = I18n.t('batches.errors.none_selected') redirect_to :back end end diff --git a/app/controllers/pipeline_destinations_controller.rb b/app/controllers/pipeline_destinations_controller.rb index 7a2a2ba8..c267e146 100644 --- a/app/controllers/pipeline_destinations_controller.rb +++ b/app/controllers/pipeline_destinations_controller.rb @@ -3,7 +3,7 @@ def create @pipeline_destination = PipelineDestination.new(name: params[:name]) if @pipeline_destination.valid? @pipeline_destination.save - flash[:notice] = 'The pipeline destination was created.' + flash[:notice] = I18n.t('pipeline_destinations.success.created') redirect_to('/admin') else flash[:error] = @pipeline_destination.errors.full_messages.join('; ') diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 2cd88e43..9ef016c4 100644 --- a/app/controllers/workflows_controller.rb +++ b/app/controllers/workflows_controller.rb @@ -6,7 +6,7 @@ class WorkflowsController < ApplicationController def create @workflow = Workflow.new(workflow_params) if @workflow.save - flash[:notice] = 'The workflow was created.' + flash[:notice] = I18n.t('workflows.success.created') redirect_to('/admin') else flash[:error] = @workflow.errors.full_messages.join('; ') @@ -21,7 +21,7 @@ def show def update workflow.assign_attributes(workflow_params) if workflow.save - flash[:notice] = 'The workflow was updated.' + flash[:notice] = I18n.t('workflows.success.updated') redirect_to('/admin') else flash[:error] = workflow.errors.full_messages.join('; ') diff --git a/app/models/application_record.rb b/app/models/application_record.rb index 10a4cba8..b44e1681 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,3 +1,7 @@ +# frozen_string_literal: true + +# Basic base class for all ActiveRecord::Base records in Sequencescape +# @see https://www.rubydoc.info/github/RailsApps/learn-rails/master/ApplicationRecord class ApplicationRecord < ActiveRecord::Base self.abstract_class = true end diff --git a/config/locales/en.yml b/config/locales/en.yml index 06539571..9113bb42 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -21,3 +21,24 @@ en: hello: "Hello world" + + assets: + errors: + none_selected: "No assets selected" + + batches: + success: + created: "The batch was created." + updated: "The batch was updated." + deleted: "The batch was deleted." + errors: + none_selected: "You must specify a batch." + + pipeline_destinations: + success: + created: "The pipeline destination was created." + + workflows: + success: + created: "The workflow was created." + updated: "The workflow was updated." From 605b7728e0cb566a2e4fa5aa6c9fa54cfc623bbd Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 13:41:20 +0100 Subject: [PATCH 07/26] rubocop got rid of the 'presence' validation in commit 7bde03e - actually, the 'required by default' behaviour needs to be explicitly turned on - and, add 'optional: true' to those relationships where we don't want to enforce the presence --- app/models/asset.rb | 6 +++--- config/application.rb | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/models/asset.rb b/app/models/asset.rb index a29351d9..6e7973df 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -5,10 +5,10 @@ class Asset < ApplicationRecord belongs_to :asset_type belongs_to :workflow - belongs_to :pipeline_destination - belongs_to :cost_code + belongs_to :pipeline_destination, optional: true + belongs_to :cost_code, optional: true belongs_to :batch - belongs_to :comment + belongs_to :comment, optional: true has_many :events, dependent: :destroy before_create :set_begun_at diff --git a/config/application.rb b/config/application.rb index 7c986079..5c2d16e9 100644 --- a/config/application.rb +++ b/config/application.rb @@ -25,5 +25,9 @@ class Application < Rails::Application # Do not swallow errors in after_commit/after_rollback callbacks. config.autoload_paths += %W[#{config.root}/lib/utils] config.disable_animations = false + + # Enabling the behaviour where 'belongs_to will now trigger a validation error by default if the association is not present.' + # (https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#active-record-belongs-to-required-by-default-option) + config.active_record.belongs_to_required_by_default = true end end From a39cd81fe256db3e86f7bee592e044739bed08be Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 13:55:50 +0100 Subject: [PATCH 08/26] redirect_to :back is removed in Rails 5.1 - https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#removed-deprecated-support-of-redirect-to-back --- app/controllers/assets_controller.rb | 2 +- app/controllers/batches_controller.rb | 6 +++--- app/controllers/pipeline_destinations_controller.rb | 2 +- app/controllers/workflows_controller.rb | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index 03263b2f..d2dca080 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -9,7 +9,7 @@ def update redirect_to("/assets?state=#{updater.redirect_state}") else flash[:error] = I18n.t('assets.errors.none_selected') - redirect_to :back + redirect_back(fallback_location: root_path) end end diff --git a/app/controllers/batches_controller.rb b/app/controllers/batches_controller.rb index b830509d..c35775e7 100644 --- a/app/controllers/batches_controller.rb +++ b/app/controllers/batches_controller.rb @@ -30,7 +30,7 @@ def update redirect_to("/batches/#{params[:id]}") else flash[:error] = batch_updater.errors.full_messages.join('; ') - redirect_to :back + redirect_back(fallback_location: root_path) end end @@ -59,7 +59,7 @@ def create redirect_to("/batches/#{@presenter.id}") else flash[:error] = batch_creator.errors.full_messages.join('; ') - redirect_to :back + redirect_back(fallback_location: root_path) end end @@ -86,7 +86,7 @@ def batch @batch ||= Batch.find_by(id: params[:id]) else flash[:error] = I18n.t('batches.errors.none_selected') - redirect_to :back + redirect_back(fallback_location: root_path) end end diff --git a/app/controllers/pipeline_destinations_controller.rb b/app/controllers/pipeline_destinations_controller.rb index c267e146..0b2110cb 100644 --- a/app/controllers/pipeline_destinations_controller.rb +++ b/app/controllers/pipeline_destinations_controller.rb @@ -7,7 +7,7 @@ def create redirect_to('/admin') else flash[:error] = @pipeline_destination.errors.full_messages.join('; ') - redirect_to :back + redirect_back(fallback_location: root_path) end end end diff --git a/app/controllers/workflows_controller.rb b/app/controllers/workflows_controller.rb index 9ef016c4..9f6ecac9 100644 --- a/app/controllers/workflows_controller.rb +++ b/app/controllers/workflows_controller.rb @@ -10,7 +10,7 @@ def create redirect_to('/admin') else flash[:error] = @workflow.errors.full_messages.join('; ') - redirect_to :back + redirect_back(fallback_location: root_path) end end @@ -25,7 +25,7 @@ def update redirect_to('/admin') else flash[:error] = workflow.errors.full_messages.join('; ') - redirect_to :back + redirect_back(fallback_location: root_path) end end From 88176676356ec6a2067c471d5a2b15ef2d163daa Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 14:12:46 +0100 Subject: [PATCH 09/26] rubocop --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 5c2d16e9..70a61418 100644 --- a/config/application.rb +++ b/config/application.rb @@ -26,7 +26,7 @@ class Application < Rails::Application config.autoload_paths += %W[#{config.root}/lib/utils] config.disable_animations = false - # Enabling the behaviour where 'belongs_to will now trigger a validation error by default if the association is not present.' + # Enabling the behaviour where 'belongs_to' associations are required by default. # (https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#active-record-belongs-to-required-by-default-option) config.active_record.belongs_to_required_by_default = true end From f0aedbbb7f9b4444c6dc06bf109a8de19cee186b Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 14:22:35 +0100 Subject: [PATCH 10/26] Passing string to be evaluated in :if and :unless conditional options is not supported. - (https://github.com/rails/rails/blob/6-1-stable/activesupport/lib/active_support/callbacks.rb) --- app/models/report.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/report.rb b/app/models/report.rb index 9db1973d..f38b5324 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -6,7 +6,7 @@ class Report attr_accessor :workflow_id, :workflow, :start_date, :end_date validates :workflow, :start_date, :end_date, presence: true - validate :correctness_of_period, if: 'start_date.present? && end_date.present?' + validate :correctness_of_period, if: :dates_present? def to_csv CSV.generate do |csv| @@ -66,6 +66,10 @@ def correctness_of_period errors.add(:start_date, 'should be earlier than the end date.') unless start_date <= end_date end + def dates_present? + start_date.present? && end_date.present? + end + class Row attr_reader :data From dd9d5ef7b436fff67bda2d6614a9a1ed1805dc54 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 15:14:26 +0100 Subject: [PATCH 11/26] Fix batch test - it was passing a record instead of an id - presumably this worked before but it doesn't now --- app/models/batch/updater.rb | 2 +- spec/features/create_and_edit_batch_spec.rb | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/app/models/batch/updater.rb b/app/models/batch/updater.rb index 0c948097..951fe297 100644 --- a/app/models/batch/updater.rb +++ b/app/models/batch/updater.rb @@ -19,7 +19,7 @@ def update! private def asset_params - { study: study, project: project, workflow_id: workflow, pipeline_destination_id: pipeline_destination, + { study: study, project: project, workflow_id: workflow.id, pipeline_destination_id: pipeline_destination, cost_code_id: cost_code, comment_id: comment_object }.tap do |params| # Only update begun at if its actually provided params.merge!(begun_at: date) if date.present? diff --git a/spec/features/create_and_edit_batch_spec.rb b/spec/features/create_and_edit_batch_spec.rb index 17f92533..1395dc2b 100644 --- a/spec/features/create_and_edit_batch_spec.rb +++ b/spec/features/create_and_edit_batch_spec.rb @@ -20,6 +20,7 @@ end click_on 'Append to batch' expect(page).to have_content 'Asset added to the batch' + fill_in 'Study', with: 'STDY' select('Workflow', from: 'Workflow') click_on 'Save' @@ -32,12 +33,14 @@ expect(options[2].disabled?).to be true expect(options[3].text).to include 'QC workflow' expect(options[3].disabled?).to be true + fill_in 'Study', with: 'STDY2' click_on 'Save' expect(page).to have_content 'The batch was updated.' Batch.last.assets.each do |asset| expect(asset.study).to eq 'STDY2' end + count = Batch.count click_on 'Remove' click_on 'Accept' From a9ddd30fb584ad34c35215787a219d14dbe8f74d Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 15:16:43 +0100 Subject: [PATCH 12/26] Fix reports test - Was getting ForbiddenAttributesError - Think due to the change in Rails 5 (https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#actioncontroller-parameters-no-longer-inherits-from-hashwithindifferentaccess) - Change to use 'permit', it's safer anyway --- app/controllers/reports_controller.rb | 2 +- spec/features/generate_report_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 94e3bb00..56d12b5d 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -7,7 +7,7 @@ def new end def create - report = Report.new(params.except(:controller, :action)) + report = Report.new(params.permit(:workflow_id, :start_date, :end_date)) if report.valid? @presenter = Presenter::ReportPresenter::Show.new(report) render :show diff --git a/spec/features/generate_report_spec.rb b/spec/features/generate_report_spec.rb index eb4b3878..d47d1560 100644 --- a/spec/features/generate_report_spec.rb +++ b/spec/features/generate_report_spec.rb @@ -34,6 +34,7 @@ fill_in('end_date', with: '01/03/2017').send_keys(:escape) find('button', text: 'Create report').click expect(page).to have_content('Start date should be earlier than the end date.') + fill_in('start_date', with: '01/03/2017').send_keys(:escape) fill_in('end_date', with: '31/03/2017').send_keys(:escape) find('button', text: 'Create report').click @@ -44,6 +45,7 @@ expect(page).to have_text('1 Study1 Project1 Not defined 1') expect(page).to have_text('2 Study1 Project2 A1 1') end + click_on 'Download csv file' end end From 112a1ad1aac6ebbf4d398f30aba4ada0295af152 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 15:54:12 +0100 Subject: [PATCH 13/26] Fix test - can't treat params as a hash any more - https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#actioncontroller-parameters-no-longer-inherits-from-hashwithindifferentaccess --- app/controllers/assets_controller.rb | 2 +- ..._and_report_assets_cherrypick_flow_spec.rb | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index d2dca080..631ea60d 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -34,7 +34,7 @@ def search end def assets_provided - params[:assets].is_a?(Hash) && params[:assets].keys.present? + params[:assets].is_a?(ActionController::Parameters) && params[:assets].keys.present? end def assets_to_be_updated diff --git a/spec/features/create_complete_and_report_assets_cherrypick_flow_spec.rb b/spec/features/create_complete_and_report_assets_cherrypick_flow_spec.rb index 515e45eb..35f5c8e0 100644 --- a/spec/features/create_complete_and_report_assets_cherrypick_flow_spec.rb +++ b/spec/features/create_complete_and_report_assets_cherrypick_flow_spec.rb @@ -29,14 +29,18 @@ end click_on 'Append to batch' expect(page).to have_content 'Asset added to the batch' + fill_in 'Study', with: 'STDY' select('Cherrypick workflow', from: 'Workflow') click_on 'Save' expect(page).to have_content 'The batch was created.' + click_on 'In Progress' expect(page).not_to have_selector('table tr') + click_on 'Cherrypick' expect(page).to have_selector('table tr', count: 3) + check "assets[#{Asset.first.id}]" click_on 'Completed selected' expect(page).to have_content 'Cherrypick is done for 123' @@ -56,30 +60,39 @@ end click_on 'Append to batch' expect(page).to have_content 'Asset added to the batch' + fill_in 'Study', with: 'STDY' select('Cherrypick QC workflow', from: 'Workflow') click_on 'Save' expect(page).to have_content 'The batch was created.' + click_on 'In Progress' expect(page).not_to have_selector('table tr') + click_on 'Cherrypick' expect(page).to have_selector('table tr', count: 3) + check "assets[#{Asset.first.id}]" click_on 'Completed selected' expect(page).to have_content 'Cherrypick is done for 12345' expect(page).to have_selector('table tr', count: 2) + click_on 'Volume check' expect(page).to have_selector('table tr', count: 2) + check "assets[#{Asset.first.id}]" click_on 'Volume checked selected' expect(page).to have_content 'Volume check is done for 12345' expect(page).not_to have_selector('table tr') + click_on 'Quant' expect(page).to have_selector('table tr', count: 2) + check "assets[#{Asset.first.id}]" click_on 'Completed selected' expect(page).to have_content 'Quant is done for 12345' expect(page).not_to have_selector('table tr') + click_on 'Report Required' expect(page).not_to have_selector('table tr') end @@ -101,34 +114,44 @@ end click_on 'Append to batch' expect(page).to have_content 'Asset added to the batch' + fill_in 'Study', with: 'STDY' select('Cherrypick Reportable QC workflow', from: 'Workflow') click_on 'Save' expect(page).to have_content 'The batch was created.' + click_on 'In Progress' expect(page).not_to have_selector('table tr') + click_on 'Cherrypick' expect(page).to have_selector('table tr', count: 4) + check "assets[#{Asset.first.id}]" check "assets[#{Asset.third.id}]" click_on 'Completed selected' expect(page).to have_content 'Cherrypick is done for 123 and 789' expect(page).to have_selector('table tr', count: 2) + click_on 'Volume check' expect(page).to have_selector('table tr', count: 3) + check "assets[#{Asset.first.id}]" check "assets[#{Asset.third.id}]" click_on 'Volume checked selected' expect(page).to have_content 'Volume check is done for 123 and 789' expect(page).not_to have_selector('table tr') + click_on 'Quant' expect(page).to have_selector('table tr', count: 3) + check "assets[#{Asset.third.id}]" click_on 'Completed selected' expect(page).to have_content 'Quant is done for 789' expect(page).to have_selector('table tr', count: 2) + click_on 'Report Required' expect(page).to have_selector('table tr', count: 2) + check "assets[#{Asset.third.id}]" click_on 'Reported selected' expect(page).to have_content 'Report required is done for 789' From 93558a66280de85e626d3cf0d6d212bee7200682 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 29 Jun 2022 16:13:47 +0100 Subject: [PATCH 14/26] Fix asset test - throw(:abort) needed due to change in behaviour of callback chains in Rails 5 - https://guides.rubyonrails.org/upgrading_ruby_on_rails.html#halting-callback-chains-via-throw-abort - Although I thought the previous behaviour should have continued by default... --- app/models/comment.rb | 4 +++- spec/models/asset_spec.rb | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 74bf85bb..2fadf051 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -4,6 +4,8 @@ class Comment < ApplicationRecord before_destroy :no_assets? def no_assets? - assets.empty? || assets.all? { |a| a.destroyed? } + return if assets.empty? || assets.all? { |a| a.destroyed? } + + throw(:abort) end end diff --git a/spec/models/asset_spec.rb b/spec/models/asset_spec.rb index d67a5c53..fee7435f 100644 --- a/spec/models/asset_spec.rb +++ b/spec/models/asset_spec.rb @@ -50,14 +50,17 @@ it 'can have events' do expect(asset.events.count).to eq 0 + asset.save expect(asset.events.count).to eq 1 + create_list(:event, 3, asset: asset) expect(asset.events.count).to eq 4 end it 'knows if it is completed' do expect(asset.completed?).to be_falsey + asset.events << create(:event, asset: asset, state: completed) expect(asset.completed?).to be_truthy end @@ -115,6 +118,7 @@ it 'requires cost code to follow convention format (1 letter + digits)' do cost_code = CostCode.new(name: 'NOT VALID') expect(cost_code).to have(1).errors_on(:name) + cost_code = CostCode.new(name: 'S1') expect(cost_code).to have(0).errors_on(:name) end @@ -188,6 +192,7 @@ comment.assets.new(identifier: 'test1') comment.assets.new(identifier: 'test2') expect(comment.assets.size).to eq(2) + comment.assets.first.destroy! expect(comment.destroyed?).to be_falsey end @@ -197,6 +202,7 @@ comment.assets.new(identifier: 'test1') comment.assets.new(identifier: 'test2') expect(comment.assets.size).to eq(2) + comment.assets.each(&:destroy!) expect(comment.destroyed?).to be_truthy end @@ -218,11 +224,13 @@ it 'creates the right events' do expect(asset.events.count).to eq 1 + asset.complete expect(asset.events.count).to eq 2 expect(asset.completed?).to be_truthy expect(reportable_asset.events.count).to eq 1 + reportable_asset.complete expect(reportable_asset.events.count).to eq 3 expect(reportable_asset.report_required?).to be_truthy From c9efe92d8bc7aa825cd3d63ea9be743cbeed91cf Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 11 Jul 2022 11:51:18 +0100 Subject: [PATCH 15/26] Remove coveralls temporarily to see if this is causing the OpenSSL build issue --- spec/rails_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 401ccdba..9c999404 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,7 +1,7 @@ # This file is copied to spec/ when you run 'rails generate rspec:install' ENV['RAILS_ENV'] ||= 'test' -require 'coveralls' -Coveralls.wear!('rails') +# require 'coveralls' +# Coveralls.wear!('rails') require File.expand_path('../config/environment', __dir__) require 'rspec/rails' From 867a9064986e42b2c59678b0ca191f80f6935777 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 11 Jul 2022 13:12:14 +0100 Subject: [PATCH 16/26] Try using simplecov directly rather than coveralls - coveralls was causing OpenSSL errors in tests --- .coveralls.yml | 1 - Gemfile | 3 ++- Gemfile.lock | 13 +------------ compile-build | 1 - spec/rails_helper.rb | 4 ++-- 5 files changed, 5 insertions(+), 17 deletions(-) delete mode 100644 .coveralls.yml diff --git a/.coveralls.yml b/.coveralls.yml deleted file mode 100644 index 6e649991..00000000 --- a/.coveralls.yml +++ /dev/null @@ -1 +0,0 @@ -service_name: travis-ci \ No newline at end of file diff --git a/Gemfile b/Gemfile index 254e67e7..c0913c2c 100644 --- a/Gemfile +++ b/Gemfile @@ -32,7 +32,7 @@ gem 'exception_notification' # Use Capistrano for deployment # gem 'capistrano-rails', group: :development -gem 'coveralls', require: false +# gem 'coveralls', require: false group :development, :test do # Call 'binding.pry' anywhere in the code to stop execution and get a debugger console @@ -67,4 +67,5 @@ group :test do gem 'timecop' # Keep webdriver in sync with chrome to prevent frustrating CI failures gem 'webdrivers', require: false + gem 'simplecov', require: false end diff --git a/Gemfile.lock b/Gemfile.lock index 8b706540..2d80a063 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -92,12 +92,6 @@ GEM execjs coffee-script-source (1.12.2) concurrent-ruby (1.1.10) - coveralls (0.8.23) - json (>= 1.8, < 3) - simplecov (~> 0.16.1) - term-ansicolor (~> 1.3) - thor (>= 0.19.4, < 2.0) - tins (~> 1.6) crass (1.0.6) database_cleaner (2.0.1) database_cleaner-active_record (~> 2.0.0) @@ -269,14 +263,9 @@ GEM actionpack (>= 5.2) activesupport (>= 5.2) sprockets (>= 3.0.0) - sync (0.5.0) - term-ansicolor (1.7.1) - tins (~> 1.0) thor (1.2.1) tilt (2.0.10) timecop (0.9.5) - tins (1.31.1) - sync turbolinks (5.2.1) turbolinks-source (~> 5.2) turbolinks-source (5.2.0) @@ -310,7 +299,6 @@ DEPENDENCIES capybara capybara-selenium coffee-rails (~> 4.2.2) - coveralls database_cleaner exception_notification factory_bot @@ -330,6 +318,7 @@ DEPENDENCIES rubocop-rspec sass-rails (~> 5.0) sdoc (~> 0.4.0) + simplecov spring timecop turbolinks diff --git a/compile-build b/compile-build index 522b03a8..5fe26880 100755 --- a/compile-build +++ b/compile-build @@ -16,7 +16,6 @@ tar \ --exclude='./.git' \ --exclude='./.gitignore' \ --exclude='./.tags' \ - --exclude='./.travis.yml' \ --exclude='./README*' \ --exclude='./compile-build' \ --exclude='./coverage' \ diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 9c999404..34db9b9d 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -1,7 +1,7 @@ # This file is copied to spec/ when you run 'rails generate rspec:install' ENV['RAILS_ENV'] ||= 'test' -# require 'coveralls' -# Coveralls.wear!('rails') +require 'simplecov' +SimpleCov.start 'rails' require File.expand_path('../config/environment', __dir__) require 'rspec/rails' From 33090ef42e7e64a96be3d8042c47c35c588fc1cc Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Tue, 12 Jul 2022 10:42:14 +0100 Subject: [PATCH 17/26] update simplecov in attempt to fix GitHub action error --- Gemfile.lock | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 2d80a063..7d800286 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -250,11 +250,12 @@ GEM rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 3.0) websocket (~> 1.0) - simplecov (0.16.1) + simplecov (0.21.2) docile (~> 1.1) - json (>= 1.8, < 3) - simplecov-html (~> 0.10.0) - simplecov-html (0.10.2) + simplecov-html (~> 0.11) + simplecov_json_formatter (~> 0.1) + simplecov-html (0.12.3) + simplecov_json_formatter (0.1.4) spring (4.0.0) sprockets (3.7.2) concurrent-ruby (~> 1.0) From 27531704287bcbc025922ba1dacc08680ab326dc Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Tue, 12 Jul 2022 10:46:04 +0100 Subject: [PATCH 18/26] rubocop --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index c0913c2c..405f4676 100644 --- a/Gemfile +++ b/Gemfile @@ -64,8 +64,8 @@ group :test do gem 'launchy' gem 'rspec-collection_matchers' gem 'rspec-rails', '~> 3.5.0' + gem 'simplecov', require: false gem 'timecop' # Keep webdriver in sync with chrome to prevent frustrating CI failures gem 'webdrivers', require: false - gem 'simplecov', require: false end From 4665c1f02a752f0a1d08f04726a6876e177d0af4 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Tue, 12 Jul 2022 10:50:38 +0100 Subject: [PATCH 19/26] try latest version of codeclimate action in attempt to fix build failure --- .github/workflows/ruby_ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ruby_ci.yml b/.github/workflows/ruby_ci.yml index 54e3dfd2..479d81f4 100644 --- a/.github/workflows/ruby_ci.yml +++ b/.github/workflows/ruby_ci.yml @@ -34,7 +34,7 @@ jobs: - name: Setup database run: bundle exec rake db:setup - name: Test & publish code coverage - uses: paambaati/codeclimate-action@v2.7.5 + uses: paambaati/codeclimate-action@v3.0.0 env: CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID || '5e399530a457db7a41cd5785ce0536b79e9022b3c2d4382f101310b3b166eb38' }} with: From 5af7fca025adf0c353e02d5f6ab1d242b6e4740c Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Tue, 12 Jul 2022 11:11:56 +0100 Subject: [PATCH 20/26] Update sdoc - dependency 'json' gem was spamming test output with 'Using the last argument as keyword parameters is deprecated' messages --- Gemfile | 2 +- Gemfile.lock | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Gemfile b/Gemfile index 405f4676..4ec97f06 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,7 @@ gem 'turbolinks' # Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder gem 'jbuilder', '~> 2.0' # bundle exec rake doc:rails generates the API under doc/api. -gem 'sdoc', '~> 0.4.0', group: :doc +gem 'sdoc', '~> 2.4.0', group: :doc gem 'bootstrap-sass', '3.4.1' diff --git a/Gemfile.lock b/Gemfile.lock index 7d800286..23eb6ca5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -120,7 +120,6 @@ GEM rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) thor (>= 0.14, < 2.0) - json (1.8.6) launchy (2.5.0) addressable (~> 2.7) loofah (2.18.0) @@ -145,6 +144,8 @@ GEM pry (0.14.1) coderay (~> 1.1) method_source (~> 1.0) + psych (4.0.4) + stringio public_suffix (4.0.7) puma (5.6.4) nio4r (~> 2.0) @@ -185,7 +186,8 @@ GEM rb-fsevent (0.11.1) rb-inotify (0.10.1) ffi (~> 1.0) - rdoc (4.3.0) + rdoc (6.4.0) + psych (>= 4.0.0) regexp_parser (2.5.0) rexml (3.2.5) rspec-collection_matchers (1.2.0) @@ -242,9 +244,8 @@ GEM tilt (>= 1.1, < 3) sassc (2.4.0) ffi (~> 1.9) - sdoc (0.4.2) - json (~> 1.7, >= 1.7.7) - rdoc (~> 4.0) + sdoc (2.4.0) + rdoc (>= 5.0) selenium-webdriver (4.3.0) childprocess (>= 0.5, < 5.0) rexml (~> 3.2, >= 3.2.5) @@ -264,6 +265,7 @@ GEM actionpack (>= 5.2) activesupport (>= 5.2) sprockets (>= 3.0.0) + stringio (3.0.2) thor (1.2.1) tilt (2.0.10) timecop (0.9.5) @@ -318,7 +320,7 @@ DEPENDENCIES rubocop-rails rubocop-rspec sass-rails (~> 5.0) - sdoc (~> 0.4.0) + sdoc (~> 2.4.0) simplecov spring timecop From 0f29d65b4a57582f10e6efa5299e50de8523e1d8 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Tue, 12 Jul 2022 11:20:31 +0100 Subject: [PATCH 21/26] Attempt to fix intermittent test failures - this list was appearing in a different order sometimes --- app/models/asset.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/asset.rb b/app/models/asset.rb index 6e7973df..8987ca3f 100644 --- a/app/models/asset.rb +++ b/app/models/asset.rb @@ -129,7 +129,7 @@ def do! def message if done? - "#{asset_state.humanize} is done for #{identifiers.to_sentence}" + "#{asset_state.humanize} is done for #{identifiers.sort.to_sentence}" else "#{asset_state.humanize} has not been finished for requested assets." end From 89dd592bec4306432709a80759f966daaca6cea9 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 14 Jul 2022 16:26:09 +0100 Subject: [PATCH 22/26] pull request comments --- Gemfile | 2 -- app/models/application_record.rb | 2 +- app/models/batch/updater.rb | 6 +++--- config/application.rb | 2 -- 4 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Gemfile b/Gemfile index 4ec97f06..de78404a 100644 --- a/Gemfile +++ b/Gemfile @@ -32,8 +32,6 @@ gem 'exception_notification' # Use Capistrano for deployment # gem 'capistrano-rails', group: :development -# gem 'coveralls', require: false - group :development, :test do # Call 'binding.pry' anywhere in the code to stop execution and get a debugger console gem 'pry' diff --git a/app/models/application_record.rb b/app/models/application_record.rb index b44e1681..dc26f8b0 100644 --- a/app/models/application_record.rb +++ b/app/models/application_record.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Basic base class for all ActiveRecord::Base records in Sequencescape +# Basic base class for all ActiveRecord::Base records in SM Workflow LIMS # @see https://www.rubydoc.info/github/RailsApps/learn-rails/master/ApplicationRecord class ApplicationRecord < ActiveRecord::Base self.abstract_class = true diff --git a/app/models/batch/updater.rb b/app/models/batch/updater.rb index 951fe297..b31967b5 100644 --- a/app/models/batch/updater.rb +++ b/app/models/batch/updater.rb @@ -19,9 +19,9 @@ def update! private def asset_params - { study: study, project: project, workflow_id: workflow.id, pipeline_destination_id: pipeline_destination, - cost_code_id: cost_code, comment_id: comment_object }.tap do |params| - # Only update begun at if its actually provided + { study_id: study.id, project_id: project.id, workflow_id: workflow.id, pipeline_destination_id: pipeline_destination.id, + cost_code_id: cost_code.id, comment_id: comment_object.id }.tap do |params| + # Only update begun_at if it's actually provided params.merge!(begun_at: date) if date.present? end end diff --git a/config/application.rb b/config/application.rb index 70a61418..4544c151 100644 --- a/config/application.rb +++ b/config/application.rb @@ -21,8 +21,6 @@ class Application < Rails::Application # config.i18n.default_locale = :de config.mailer = YAML.load_file("#{Rails.root}/config/mailer.yml")[Rails.env] - - # Do not swallow errors in after_commit/after_rollback callbacks. config.autoload_paths += %W[#{config.root}/lib/utils] config.disable_animations = false From 3a200654663517ae8c752a1b48d48c6dbc6d46df Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 14 Jul 2022 16:29:01 +0100 Subject: [PATCH 23/26] rubocop --- app/models/batch/updater.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/batch/updater.rb b/app/models/batch/updater.rb index b31967b5..d2b89893 100644 --- a/app/models/batch/updater.rb +++ b/app/models/batch/updater.rb @@ -19,8 +19,9 @@ def update! private def asset_params - { study_id: study.id, project_id: project.id, workflow_id: workflow.id, pipeline_destination_id: pipeline_destination.id, - cost_code_id: cost_code.id, comment_id: comment_object.id }.tap do |params| + { study_id: study.id, project_id: project.id, workflow_id: workflow.id, + pipeline_destination_id: pipeline_destination.id, cost_code_id: cost_code.id, + comment_id: comment_object.id }.tap do |params| # Only update begun_at if it's actually provided params.merge!(begun_at: date) if date.present? end From 0650eeeca2beacc6c1e91d5ee03f7178b33659b1 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 14 Jul 2022 17:01:24 +0100 Subject: [PATCH 24/26] Bugfix - add an '&' in case the objects are null - study and project are just strings - update through associations (e.g. ) doesn't seem to work with update_all (get error 'Mysql2::Error: Unknown column 'assets.workflow' in 'field list'') - unit test is only actually populating workflow, not any of the other params --- app/models/batch/updater.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/batch/updater.rb b/app/models/batch/updater.rb index d2b89893..35968a6e 100644 --- a/app/models/batch/updater.rb +++ b/app/models/batch/updater.rb @@ -19,9 +19,9 @@ def update! private def asset_params - { study_id: study.id, project_id: project.id, workflow_id: workflow.id, - pipeline_destination_id: pipeline_destination.id, cost_code_id: cost_code.id, - comment_id: comment_object.id }.tap do |params| + { study: study, project: project, workflow_id: workflow&.id, + pipeline_destination_id: pipeline_destination&.id, cost_code_id: cost_code&.id, + comment_id: comment_object&.id }.tap do |params| # Only update begun_at if it's actually provided params.merge!(begun_at: date) if date.present? end From 48cd9e41c77c47a6a977b1e8fd91825f6258581d Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 15 Jul 2022 10:14:24 +0100 Subject: [PATCH 25/26] pull request comments --- Gemfile | 4 +--- Gemfile.lock | 8 -------- app/controllers/assets_controller.rb | 2 +- 3 files changed, 2 insertions(+), 12 deletions(-) diff --git a/Gemfile b/Gemfile index de78404a..ed566690 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,7 @@ gem 'turbolinks' # Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder gem 'jbuilder', '~> 2.0' # bundle exec rake doc:rails generates the API under doc/api. -gem 'sdoc', '~> 2.4.0', group: :doc +gem 'sdoc', '~> 2.4.0', group: :doc # TODO: change to using yard (using this for other apps). gem 'bootstrap-sass', '3.4.1' @@ -37,8 +37,6 @@ group :development, :test do gem 'pry' # Use Uglifier as compressor for JavaScript assets gem 'uglifier', '>= 1.3.0' - # Use CoffeeScript for .coffee assets and views - gem 'coffee-rails', '~> 4.2.2' end group :development do diff --git a/Gemfile.lock b/Gemfile.lock index 23eb6ca5..7e9c7cc0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -84,13 +84,6 @@ GEM selenium-webdriver childprocess (4.1.0) coderay (1.1.3) - coffee-rails (4.2.2) - coffee-script (>= 2.2.0) - railties (>= 4.0.0) - coffee-script (2.4.1) - coffee-script-source - execjs - coffee-script-source (1.12.2) concurrent-ruby (1.1.10) crass (1.0.6) database_cleaner (2.0.1) @@ -301,7 +294,6 @@ DEPENDENCIES bootstrap-sass (= 3.4.1) capybara capybara-selenium - coffee-rails (~> 4.2.2) database_cleaner exception_notification factory_bot diff --git a/app/controllers/assets_controller.rb b/app/controllers/assets_controller.rb index 631ea60d..90242b60 100644 --- a/app/controllers/assets_controller.rb +++ b/app/controllers/assets_controller.rb @@ -34,7 +34,7 @@ def search end def assets_provided - params[:assets].is_a?(ActionController::Parameters) && params[:assets].keys.present? + params[:assets].present? end def assets_to_be_updated From 13b4adee7365b42d8c46e87196b59539f7082ddd Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 15 Jul 2022 10:16:21 +0100 Subject: [PATCH 26/26] weird whitespace thing --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index ed566690..d97d2baf 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,7 @@ gem 'turbolinks' # Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder gem 'jbuilder', '~> 2.0' # bundle exec rake doc:rails generates the API under doc/api. -gem 'sdoc', '~> 2.4.0', group: :doc # TODO: change to using yard (using this for other apps). +gem 'sdoc', '~> 2.4.0', group: :doc # TODO: change to using yard (using this for other apps). gem 'bootstrap-sass', '3.4.1'