From 649dc7348ffb0c0c5be99d4b7757b0d899c61acc Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 13:18:23 +0100 Subject: [PATCH 01/28] Add `parse_status` to Genotype --- db/migrate/20171231121548_add_parse_status_to_genotypes.rb | 5 +++++ db/structure.sql | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20171231121548_add_parse_status_to_genotypes.rb diff --git a/db/migrate/20171231121548_add_parse_status_to_genotypes.rb b/db/migrate/20171231121548_add_parse_status_to_genotypes.rb new file mode 100644 index 00000000..ce1a2758 --- /dev/null +++ b/db/migrate/20171231121548_add_parse_status_to_genotypes.rb @@ -0,0 +1,5 @@ +class AddParseStatusToGenotypes < ActiveRecord::Migration + def change + add_column :genotypes, :parse_status, :string + end +end diff --git a/db/structure.sql b/db/structure.sql index 23081466..57c6d620 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -501,7 +501,8 @@ CREATE TABLE genotypes ( genotype_file_name character varying(255), genotype_content_type character varying(255), genotype_file_size integer, - genotype_updated_at timestamp without time zone + genotype_updated_at timestamp without time zone, + parse_status character varying ); @@ -2169,3 +2170,5 @@ INSERT INTO schema_migrations (version) VALUES ('20161226175703'); INSERT INTO schema_migrations (version) VALUES ('20171113104813'); +INSERT INTO schema_migrations (version) VALUES ('20171231121548'); + From f96250a11d70eb55efa33af3dbb80e996e033896 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 13:31:09 +0100 Subject: [PATCH 02/28] Validate `parse_status` --- app/models/genotype.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/models/genotype.rb b/app/models/genotype.rb index f752f073..bc8f4703 100644 --- a/app/models/genotype.rb +++ b/app/models/genotype.rb @@ -5,6 +5,8 @@ class Genotype < ActiveRecord::Base belongs_to :user has_many :user_snps, dependent: :delete_all validates_presence_of :user + validates :parse_status, inclusion: { in: %w[queued parsing done error] }, + allow_nil: true has_attached_file :genotype, url: '/data/:fs_filename', path: "#{Rails.root}/public/data/:fs_filename" From dff43414912af89c0b981bfd51dc77bb5b39117c Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 14:00:51 +0100 Subject: [PATCH 03/28] Update parse status when parsing --- app/controllers/genotypes_controller.rb | 1 + app/workers/parsing.rb | 2 ++ app/workers/preparsing.rb | 3 +++ spec/features/upload_genotype_spec.rb | 16 +++++++++++++--- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/app/controllers/genotypes_controller.rb b/app/controllers/genotypes_controller.rb index 462b9d1f..adda23d4 100644 --- a/app/controllers/genotypes_controller.rb +++ b/app/controllers/genotypes_controller.rb @@ -21,6 +21,7 @@ def new def create @genotype = Genotype.create(genotype_params) @genotype.user = current_user + @genotype.parse_status = 'queued' if @genotype.valid? && @genotype.save Preparsing.perform_async(@genotype.id) # award for genotyping-upload diff --git a/app/workers/parsing.rb b/app/workers/parsing.rb index 11617d3f..b86acac0 100644 --- a/app/workers/parsing.rb +++ b/app/workers/parsing.rb @@ -21,11 +21,13 @@ def perform(genotype_id) send_logged(:insert_into_snps) send_logged(:insert_into_user_snps) end + genotype.update!(parse_status: 'done') send_logged(:notify_user) stats[:duration] = "#{(Time.current - start_time).round(3)}s" logger.info("Finished parsing: #{stats.to_a.map { |s| s.join('=') }.join(', ')}") rescue => e + genotype.update!(parse_status: 'error') unless genotype.parse_status == 'done' logger.error("Failed with #{e.class}: #{e.message}") raise end diff --git a/app/workers/preparsing.rb b/app/workers/preparsing.rb index 0f4e2d16..ae435dce 100644 --- a/app/workers/preparsing.rb +++ b/app/workers/preparsing.rb @@ -9,6 +9,7 @@ class Preparsing def perform(genotype_id) genotype = Genotype.find(genotype_id) + genotype.update!(parse_status: 'parsing') logger.info "Starting preparse" biggest = '' @@ -134,5 +135,7 @@ def perform(genotype_id) Parsing.perform_async(genotype.id) end + rescue => e + genotype.update!(parse_status: 'error') end end diff --git a/spec/features/upload_genotype_spec.rb b/spec/features/upload_genotype_spec.rb index a76f4c3e..e8fa149b 100644 --- a/spec/features/upload_genotype_spec.rb +++ b/spec/features/upload_genotype_spec.rb @@ -7,12 +7,22 @@ sign_in(user) end + let(:genotype) { Genotype.last } + scenario 'uploads first genotype' do visit '/genotypes/new' + attach_file('genotype[genotype]', File.absolute_path('test/data/23andMe_test.csv')) choose '23andme-format' - click_on 'Upload' - expect(page).to have_content('Genotype was successfully uploaded!') - expect(page).to have_content("You've unlocked an achievement:") + Sidekiq::Testing.disable! do + click_on 'Upload' + expect(page).to have_content('Genotype was successfully uploaded!') + expect(page).to have_content("You've unlocked an achievement:") + end + + expect(genotype.reload.parse_status).to eq('queued') + Preparsing.perform_async(genotype.id) + expect(genotype.reload.parse_status).to eq('done') + expect(genotype.user_snps).to be_present end end From cdeae98ae0ce7f2a24b978b868eeeea07a79fe69 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 15:29:44 +0100 Subject: [PATCH 04/28] Introduce ParseError --- app/workers/parsing.rb | 3 ++ spec/workers/parsing_spec.rb | 61 +++++++++++++++++++++++++++--------- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/app/workers/parsing.rb b/app/workers/parsing.rb index b86acac0..4ad637e7 100644 --- a/app/workers/parsing.rb +++ b/app/workers/parsing.rb @@ -60,6 +60,7 @@ def normalize_csv row[3].is_a?(String) && (1..2).include?(row[3].length) end @normalized_csv = csv.map { |row| row.join(',') }.join("\n") + raise(ParseError, 'No data found in file') if @normalized_csv.empty? stats[:rows_after_parsing] = csv.length end @@ -236,4 +237,6 @@ def send_logged(method) logger.info("calling of method `#{method}` took #{took} s") ret end + + ParseError = Class.new(RuntimeError) end diff --git a/spec/workers/parsing_spec.rb b/spec/workers/parsing_spec.rb index 43e50a7b..d291418b 100644 --- a/spec/workers/parsing_spec.rb +++ b/spec/workers/parsing_spec.rb @@ -1,18 +1,51 @@ # frozen_string_literal: true describe Parsing do - describe '#notify_user' do - let(:mail) { double('mail') } - let(:genotype) { double('genotype', id: 1) } - let(:stats) { { foos: 7 } } - - it 'sends an email to the user' do - subject.instance_variable_set(:@stats, stats) - subject.instance_variable_set(:@genotype, genotype) - expect(UserMailer).to receive(:finished_parsing).with(genotype.id, stats) - .and_return(mail) - expect(mail).to receive(:deliver_later) - - subject.notify_user - end + let(:genotype) do + create( + :genotype, + genotype: File.open(File.absolute_path('test/data/23andMe_test.csv')), + filetype: '23andme', + parse_status: 'parsing' + ) + end + + let(:emails) do + ActionMailer::Base.deliveries + end + + it 'parses a genotype file' do + described_class.new.perform(genotype.id) + + genotype.reload + + expect(genotype.user_snps.count).to eq(5) + expect( + genotype + .user_snps + .order(:snp_name) + .pluck(:genotype_id, :snp_name, :local_genotype) + ).to eq( + [ + [genotype.id, 'rs11240777', 'AG'], + [genotype.id, 'rs12124819', 'AG'], + [genotype.id, 'rs3094315', 'AA'], + [genotype.id, 'rs3131972', 'GG'], + [genotype.id, 'rs4477212', 'AA'] + ] + ) + expect(genotype.parse_status).to eq('done') + + expect(emails.count).to eq(1) + expect(emails.first.subject).to eq('Finished parsing your genotyping') + end + + it 'sets the parse status to "error" if parsing failed' do + genotype.update!(genotype: StringIO.new('💥')) + + expect do + described_class.new.perform(genotype.id) + end.to raise_error(Parsing::ParseError, 'No data found in file') + + expect(genotype.reload.parse_status).to eq('error') end end From 06cde9e4817cab49cd2ebcdc653b0df0c888625c Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 15:31:41 +0100 Subject: [PATCH 05/28] Add TODO --- spec/workers/parsing_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/workers/parsing_spec.rb b/spec/workers/parsing_spec.rb index d291418b..0b43ac82 100644 --- a/spec/workers/parsing_spec.rb +++ b/spec/workers/parsing_spec.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true +# TODO: Add test cases for different filetypes. describe Parsing do let(:genotype) do create( @@ -13,7 +14,7 @@ ActionMailer::Base.deliveries end - it 'parses a genotype file' do + it 'parses a 23andme file' do described_class.new.perform(genotype.id) genotype.reload From 46f39f8d07cf3e3dfd75e3dac902e65e69d551db Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 15:39:38 +0100 Subject: [PATCH 06/28] Add test for `parse_status` update in Preparsing --- app/workers/preparsing.rb | 5 +++-- spec/workers/preparsing_spec.rb | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 spec/workers/preparsing_spec.rb diff --git a/app/workers/preparsing.rb b/app/workers/preparsing.rb index ae435dce..403b99e8 100644 --- a/app/workers/preparsing.rb +++ b/app/workers/preparsing.rb @@ -29,8 +29,8 @@ def perform(genotype_id) logger.info "copied file" end - rescue - logger.info "nothing to unzip, seems to be a text-file in the first place" + rescue Zip::Error + logger.info 'nothing to unzip, seems to be a text-file in the first place' end # now that they are unzipped, check if they're actually proper files @@ -137,5 +137,6 @@ def perform(genotype_id) end rescue => e genotype.update!(parse_status: 'error') + raise end end diff --git a/spec/workers/preparsing_spec.rb b/spec/workers/preparsing_spec.rb new file mode 100644 index 00000000..651b865c --- /dev/null +++ b/spec/workers/preparsing_spec.rb @@ -0,0 +1,16 @@ +RSpec.describe Preparsing do + let(:genotype) do + create( + :genotype, + genotype: File.open(File.absolute_path('test/data/23andMe_test.csv')), + filetype: '23andme', + parse_status: 'queued' + ) + end + + it "updates the genotype's parse status" do + described_class.new.perform(genotype.id) + + expect(genotype.reload.parse_status).to eq('parsing') + end +end From e87be9076df194779185bcfe2f82eb8315b8df66 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 15:45:46 +0100 Subject: [PATCH 07/28] Don't hit the database twice when creating Genotype --- app/controllers/genotypes_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/genotypes_controller.rb b/app/controllers/genotypes_controller.rb index adda23d4..771cb06a 100644 --- a/app/controllers/genotypes_controller.rb +++ b/app/controllers/genotypes_controller.rb @@ -19,7 +19,7 @@ def new end def create - @genotype = Genotype.create(genotype_params) + @genotype = Genotype.new(genotype_params) @genotype.user = current_user @genotype.parse_status = 'queued' if @genotype.valid? && @genotype.save From 21718c857f9d0e02b88ba8bf282087fc2c74c003 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 15:46:54 +0100 Subject: [PATCH 08/28] Remove unneded `=> e` --- app/workers/preparsing.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/workers/preparsing.rb b/app/workers/preparsing.rb index 403b99e8..18142c84 100644 --- a/app/workers/preparsing.rb +++ b/app/workers/preparsing.rb @@ -135,7 +135,7 @@ def perform(genotype_id) Parsing.perform_async(genotype.id) end - rescue => e + rescue genotype.update!(parse_status: 'error') raise end From e656ac09521051d1d6ff2057f6e08aaa8a7fabcb Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Sun, 31 Dec 2017 15:52:41 +0100 Subject: [PATCH 09/28] Add `parse_error_message` to Genotype --- .../20171231144834_add_parse_error_message_to_genotypes.rb | 5 +++++ db/structure.sql | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb diff --git a/db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb b/db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb new file mode 100644 index 00000000..d8e0c068 --- /dev/null +++ b/db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb @@ -0,0 +1,5 @@ +class AddParseErrorMessageToGenotypes < ActiveRecord::Migration + def change + add_column :genotypes, :parse_error_message, :string + end +end diff --git a/db/structure.sql b/db/structure.sql index 57c6d620..fcd3d1d8 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -502,7 +502,8 @@ CREATE TABLE genotypes ( genotype_content_type character varying(255), genotype_file_size integer, genotype_updated_at timestamp without time zone, - parse_status character varying + parse_status character varying, + parse_error_message character varying ); @@ -2172,3 +2173,5 @@ INSERT INTO schema_migrations (version) VALUES ('20171113104813'); INSERT INTO schema_migrations (version) VALUES ('20171231121548'); +INSERT INTO schema_migrations (version) VALUES ('20171231144834'); + From 00aeadad3e1ec6b2b6cae80a5bda3aab461e60cb Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Mon, 1 Jan 2018 16:38:45 +0100 Subject: [PATCH 10/28] WIP --- app/views/users/_edit.html.erb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index cd8035e9..ee5ae155 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -39,6 +39,17 @@ +
+

Your Genotypes

+ + + + + + +
File
+
+

Your Phenotypes

From 98d5d1ed9f54000aa6907a843c0b746602773e79 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 13:07:24 +0100 Subject: [PATCH 11/28] Add a list of uploaded genotypes to user settings --- app/helpers/users_helper.rb | 11 +++++++++++ app/views/users/_edit.html.erb | 20 ++++++++++++++++++-- 2 files changed, 29 insertions(+), 2 deletions(-) create mode 100644 app/helpers/users_helper.rb diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb new file mode 100644 index 00000000..0266ecb0 --- /dev/null +++ b/app/helpers/users_helper.rb @@ -0,0 +1,11 @@ +module UsersHelper + def display_parse_status(genotype) + label_class = case genotype.parse_status + when 'done' then 'label-success' + when 'queued' then 'label-default' + when 'parsing' then 'label-primary' + when 'error' then 'label-danger' + end + content_tag('span', genotype.parse_status, class: "label #{label_class}") + end +end diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index ee5ae155..d212a4cf 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -5,6 +5,7 @@
    +
  • Your genotypes
  • Your phenotypes
  • Details
  • Open Humans
    • @@ -41,12 +42,27 @@

    Your Genotypes

    - +
    - + + + + + + + <% @user.genotypes.order('created_at DESC').each do |genotype| %> + + + + + + + + <% end %> +
    FileIDSourceUploaded atParse statusImported SNPs
    <%= genotype.id %><%= genotype.filetype %><%= genotype.created_at %><%= display_parse_status(genotype) %><%= genotype.user_snps.count %>
    From a96183c2a3d486ba78fc0293fe30109d40df2c90 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 13:08:53 +0100 Subject: [PATCH 12/28] Revert "Add `parse_error_message` to Genotype" This reverts commit e656ac09521051d1d6ff2057f6e08aaa8a7fabcb. --- .../20171231144834_add_parse_error_message_to_genotypes.rb | 5 ----- db/structure.sql | 5 +---- 2 files changed, 1 insertion(+), 9 deletions(-) delete mode 100644 db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb diff --git a/db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb b/db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb deleted file mode 100644 index d8e0c068..00000000 --- a/db/migrate/20171231144834_add_parse_error_message_to_genotypes.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddParseErrorMessageToGenotypes < ActiveRecord::Migration - def change - add_column :genotypes, :parse_error_message, :string - end -end diff --git a/db/structure.sql b/db/structure.sql index fcd3d1d8..57c6d620 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -502,8 +502,7 @@ CREATE TABLE genotypes ( genotype_content_type character varying(255), genotype_file_size integer, genotype_updated_at timestamp without time zone, - parse_status character varying, - parse_error_message character varying + parse_status character varying ); @@ -2173,5 +2172,3 @@ INSERT INTO schema_migrations (version) VALUES ('20171113104813'); INSERT INTO schema_migrations (version) VALUES ('20171231121548'); -INSERT INTO schema_migrations (version) VALUES ('20171231144834'); - From 969aa5f56186ee2d9ca8c8db48a5597788214bc3 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 13:10:39 +0100 Subject: [PATCH 13/28] Order parse status cases logically --- app/helpers/users_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 0266ecb0..d089974f 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -1,9 +1,9 @@ module UsersHelper def display_parse_status(genotype) label_class = case genotype.parse_status - when 'done' then 'label-success' when 'queued' then 'label-default' when 'parsing' then 'label-primary' + when 'done' then 'label-success' when 'error' then 'label-danger' end content_tag('span', genotype.parse_status, class: "label #{label_class}") From 4e38535631d95880614399f90001b97ec1347eb3 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 13:13:42 +0100 Subject: [PATCH 14/28] Add line-breaks --- app/views/users/_edit.html.erb | 42 ++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index d212a4cf..85de73cd 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -4,13 +4,41 @@
    From 13d2a3f28a7c86cd094395cdcce188ea6188b872 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 13:28:47 +0100 Subject: [PATCH 15/28] Fix indentation --- app/views/users/_edit.html.erb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index 85de73cd..93ce0419 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -69,7 +69,7 @@
    -

    Your Genotypes

    +

    Your Genotypes

    @@ -95,7 +95,7 @@
    -

    Your Phenotypes

    +

    Your Phenotypes

    <%= f.label :sex, "Chromosomal Sex"%>

    @@ -129,7 +129,7 @@ <%= link_to "Add phenotype", {controller: "phenotypes", action: "new"}, class:"btn btn-default center-block settings__add-phenotype-button" %>
    -
    +
    From 407497243edbbd9f48f235f3efca27b49ba38350 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 13:48:06 +0100 Subject: [PATCH 16/28] Adapt feature spec to use genotypes list --- app/controllers/genotypes_controller.rb | 6 +++++- config/locales/en.genotypes.yml | 4 ++++ spec/features/upload_genotype_spec.rb | 19 ++++++++++++++++--- 3 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 config/locales/en.genotypes.yml diff --git a/app/controllers/genotypes_controller.rb b/app/controllers/genotypes_controller.rb index 771cb06a..7d403d13 100644 --- a/app/controllers/genotypes_controller.rb +++ b/app/controllers/genotypes_controller.rb @@ -37,7 +37,11 @@ def create if current_user.has_sequence == false current_user.toggle!(:has_sequence) end - redirect_to(current_user, notice: 'Genotype was successfully uploaded! Parsing and annotating might take a couple of hours days.') + + redirect_to( + edit_user_path(current_user, anchor: 'genotypes'), + notice: t('.uploaded_successfully') + ) else render action: 'new' end diff --git a/config/locales/en.genotypes.yml b/config/locales/en.genotypes.yml new file mode 100644 index 00000000..41e748de --- /dev/null +++ b/config/locales/en.genotypes.yml @@ -0,0 +1,4 @@ +en: + genotypes: + create: + uploaded_successfully: 'Genotype was successfully uploaded! Parsing and annotating might take a couple of minutes.' diff --git a/spec/features/upload_genotype_spec.rb b/spec/features/upload_genotype_spec.rb index e8fa149b..b3596fd4 100644 --- a/spec/features/upload_genotype_spec.rb +++ b/spec/features/upload_genotype_spec.rb @@ -20,9 +20,22 @@ expect(page).to have_content("You've unlocked an achievement:") end - expect(genotype.reload.parse_status).to eq('queued') + expect(page).to have_content('Your genotypes') + + within('#genotypes') do + expect(find_all('table tbody tr td').map(&:text)).to eq( + [genotype.id.to_s, '23andme', genotype.created_at.to_s, 'queued', '0'] + ) + end + Preparsing.perform_async(genotype.id) - expect(genotype.reload.parse_status).to eq('done') - expect(genotype.user_snps).to be_present + + visit current_url + + within('#genotypes') do + expect(find_all('table tbody tr td').map(&:text)).to eq( + [genotype.id.to_s, '23andme', genotype.created_at.to_s, 'done', '5'] + ) + end end end From 87dcb069faea61b05a28c3005e32d95150713b25 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 14:22:42 +0100 Subject: [PATCH 17/28] Don't delete genotype on error in Preparsing --- app/workers/preparsing.rb | 4 +- spec/workers/preparsing_spec.rb | 52 ++++++++++++++++---- test/functional/genotypes_controller_test.rb | 2 +- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/app/workers/preparsing.rb b/app/workers/preparsing.rb index 18142c84..8cf8f14d 100644 --- a/app/workers/preparsing.rb +++ b/app/workers/preparsing.rb @@ -113,19 +113,17 @@ def perform(genotype_id) # not proper file! if not file_is_ok + genotype.update!(parse_status: 'error') if file_is_duplicate UserMailer.duplicate_file(genotype.user_id).deliver_later system("rm #{Rails.root}/public/data/#{genotype.fs_filename}") - Genotype.find_by_id(genotype.id).delete elsif file_has_mails UserMailer.file_has_mails(genotype.user_id).deliver system("rm #{Rails.root}/public/data/#{genotype.fs_filename}") - Genotype.find_by_id(genotype.id).delete else UserMailer.parsing_error(genotype.user_id).deliver_later logger.info "file is not ok, sending email" system("rm #{Rails.root}/public/data/#{genotype.fs_filename}") - Genotype.find_by_id(genotype.id).delete end else logger.info "Updating genotype with md5sum #{md5}" diff --git a/spec/workers/preparsing_spec.rb b/spec/workers/preparsing_spec.rb index 651b865c..a8815093 100644 --- a/spec/workers/preparsing_spec.rb +++ b/spec/workers/preparsing_spec.rb @@ -1,16 +1,48 @@ RSpec.describe Preparsing do - let(:genotype) do - create( - :genotype, - genotype: File.open(File.absolute_path('test/data/23andMe_test.csv')), - filetype: '23andme', - parse_status: 'queued' - ) + context 'when the genotype is valid' do + let(:genotype) do + create( + :genotype, + genotype: File.open(File.absolute_path('test/data/23andMe_test.csv')), + filetype: '23andme', + parse_status: 'queued' + ) + end + + it "updates the genotype's parse status" do + described_class.new.perform(genotype.id) + + expect(genotype.reload.parse_status).to eq('parsing') + end + end + + context 'when the genotype file is faulty' do + let(:genotype) do + create(:genotype, genotype: StringIO.new('XXX')) + end + + it "updates the genotype's parse status" do + described_class.new.perform(genotype.id) + + expect(genotype.reload.parse_status).to eq('error') + end end - it "updates the genotype's parse status" do - described_class.new.perform(genotype.id) + context 'when the worker raises an error' do + let(:genotype) do + create(:genotype) + end + + before do + expect(File).to receive(:open).and_raise('Meh.') + end + + it "updates the genotype's parse status" do + expect do + described_class.new.perform(genotype.id) + end.to raise_error('Meh.') - expect(genotype.reload.parse_status).to eq('parsing') + expect(genotype.reload.parse_status).to eq('error') + end end end diff --git a/test/functional/genotypes_controller_test.rb b/test/functional/genotypes_controller_test.rb index 01d1fd59..f92d52d6 100644 --- a/test/functional/genotypes_controller_test.rb +++ b/test/functional/genotypes_controller_test.rb @@ -62,7 +62,7 @@ class GenotypesControllerTest < ActionController::TestCase { genotype: genotype_file, filetype: "23andme"} end end - assert_redirected_to user_path(@user) + assert_redirected_to edit_user_path(@user, anchor: 'genotypes') assert_equal @publishing_award.id, UserAchievement.last.achievement_id FileUtils.rm("#{Rails.root}/test/fixtures/testdatensatz1_23andme.txt") end From bd041af1b281b0765c471e29c9436310a6940d01 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 15:02:36 +0100 Subject: [PATCH 18/28] Move genotype delete button to user's genotypes table --- app/helpers/users_helper.rb | 12 ++++++++++++ app/views/users/_edit.html.erb | 17 ++++++----------- config/locales/en.users.yml | 6 ++++++ spec/features/delete_genotype_spec.rb | 10 +++++++--- spec/features/upload_genotype_spec.rb | 18 ++++++++++++++++-- 5 files changed, 47 insertions(+), 16 deletions(-) create mode 100644 config/locales/en.users.yml diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index d089974f..f25e4df1 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -8,4 +8,16 @@ def display_parse_status(genotype) end content_tag('span', genotype.parse_status, class: "label #{label_class}") end + + def delete_genotype_button(genotype) + link_to( + t('.genotypes.delete'), + genotype, + method: 'delete', + class: "btn btn-default settings__deleting-button", + data: { + confirm: t('.genotypes.confirm_delete', file_name: genotype.genotype_file_name) + } + ) + end end diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index 93ce0419..c7a47d49 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -73,21 +73,23 @@
    - - + + + <% @user.genotypes.order('created_at DESC').each do |genotype| %> - + + <% end %> @@ -173,14 +175,7 @@
    -

    Deleting

    -

    Genotypes

    - <% if not @user.genotypes.empty? %> - <% @user.genotypes.each do |ug| %> -

    <%= link_to "Genotype #{ug.genotype_file_name}", ug, method: "delete", confirm: "Are you sure you want to delete genotype #{ug.genotype_file_name.to_s}?", class: "btn btn-default settings__deleting-button" %>

    - <% end %> - <% end %> -

    Account

    +

    Delete account

    <%= link_to "Delete this account", @user, method: "delete", class: "btn btn-default settings__deleting-button", data: { confirm: 'Are you sure you want to delete your account?' } %>

    diff --git a/config/locales/en.users.yml b/config/locales/en.users.yml new file mode 100644 index 00000000..bcc3ba1d --- /dev/null +++ b/config/locales/en.users.yml @@ -0,0 +1,6 @@ +en: + users: + edit: + genotypes: + delete: 'Delete' + confirm_delete: 'Are you sure you want to delete genotype %{file_name}?' diff --git a/spec/features/delete_genotype_spec.rb b/spec/features/delete_genotype_spec.rb index 0a153fd1..588a12c5 100644 --- a/spec/features/delete_genotype_spec.rb +++ b/spec/features/delete_genotype_spec.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true -RSpec.feature 'Delete a genotype', sidekiq: :inline do +RSpec.feature 'Delete a genotype', :js, sidekiq: :inline do let(:user) { create(:user, name: 'Gregor Mendel') } let!(:genotype) { create(:genotype, user: user, genotype_file_name: 'test.txt') } let(:award) { Achievement.find_by(award: 'Published genotyping') } @@ -16,8 +16,12 @@ click_on('Gregor Mendel') click_on('Settings') - click_on('Deleting') - click_on('Genotype test.txt') + click_on('Your genotypes') + within('#genotypes') do + page.accept_confirm do + click_on('Delete') + end + end expect(page).to have_content('Your Genotyping will be deleted. ' \ 'This may take a few minutes.') diff --git a/spec/features/upload_genotype_spec.rb b/spec/features/upload_genotype_spec.rb index b3596fd4..4db8a59a 100644 --- a/spec/features/upload_genotype_spec.rb +++ b/spec/features/upload_genotype_spec.rb @@ -24,7 +24,14 @@ within('#genotypes') do expect(find_all('table tbody tr td').map(&:text)).to eq( - [genotype.id.to_s, '23andme', genotype.created_at.to_s, 'queued', '0'] + [ + '23andme', + genotype.genotype_file_name, + genotype.created_at.to_s, + 'queued', + '0', + 'Delete' + ] ) end @@ -34,7 +41,14 @@ within('#genotypes') do expect(find_all('table tbody tr td').map(&:text)).to eq( - [genotype.id.to_s, '23andme', genotype.created_at.to_s, 'done', '5'] + [ + '23andme', + genotype.genotype_file_name, + genotype.created_at.to_s, + 'done', + '5', + 'Delete' + ] ) end end From 713c5513eb242f54c6004951f8d40d531b5e1c4d Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 15:07:46 +0100 Subject: [PATCH 19/28] fixup! Don't delete genotype on error in Preparsing --- app/workers/preparsing.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/workers/preparsing.rb b/app/workers/preparsing.rb index 8cf8f14d..aed07f3e 100644 --- a/app/workers/preparsing.rb +++ b/app/workers/preparsing.rb @@ -116,14 +116,11 @@ def perform(genotype_id) genotype.update!(parse_status: 'error') if file_is_duplicate UserMailer.duplicate_file(genotype.user_id).deliver_later - system("rm #{Rails.root}/public/data/#{genotype.fs_filename}") elsif file_has_mails UserMailer.file_has_mails(genotype.user_id).deliver - system("rm #{Rails.root}/public/data/#{genotype.fs_filename}") else UserMailer.parsing_error(genotype.user_id).deliver_later logger.info "file is not ok, sending email" - system("rm #{Rails.root}/public/data/#{genotype.fs_filename}") end else logger.info "Updating genotype with md5sum #{md5}" From 83fd9c890e9177d8da97f13446192b5f4b17441c Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Tue, 2 Jan 2018 19:39:54 +0100 Subject: [PATCH 20/28] Refactor download and delete buttons --- app/controllers/genotypes_controller.rb | 8 ++++++++ app/helpers/application_helper.rb | 23 +++++++++++++++++++++++ app/helpers/users_helper.rb | 12 ------------ app/views/genotypes/index.html.erb | 2 +- app/views/users/_edit.html.erb | 10 +++++++++- config/locales/en.users.yml | 1 - config/locales/en.yml | 3 +++ config/routes.rb | 4 +++- spec/features/delete_genotype_spec.rb | 6 +++--- spec/features/upload_genotype_spec.rb | 4 ++-- 10 files changed, 52 insertions(+), 21 deletions(-) diff --git a/app/controllers/genotypes_controller.rb b/app/controllers/genotypes_controller.rb index 7d403d13..4dfe3515 100644 --- a/app/controllers/genotypes_controller.rb +++ b/app/controllers/genotypes_controller.rb @@ -66,6 +66,14 @@ def destroy redirect_to current_user end + def download + genotype = Genotype.find(params[:id]) + send_data( + File.open(genotype.genotype.path), + filename: genotype.genotype_file_name + ) + end + private def sort_column diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index d161db0d..81b5979f 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -17,4 +17,27 @@ def page_navigation_links(pages) def table_row_sequence_number(paginated, current_page_index) paginated.per_page * (paginated.current_page - 1) + current_page_index + 1 end + + def download_button(url) + link_to(url, class: 'btn btn-default btn-sm', title: t('download')) do + glyphicon('download-alt') + end + end + + def delete_button(url, opts = {}) + opts = { + method: 'delete', + class: 'btn btn-danger btn-sm', + title: t('delete'), + data: { + confirm: t('.confirm_delete', default: t('confirm_delete')) + } + }.deep_merge(opts) + + link_to(url, opts) { glyphicon('trash') } + end + + def glyphicon(name) + tag('span', class: "glyphicon glyphicon-#{name}") + end end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index f25e4df1..d089974f 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -8,16 +8,4 @@ def display_parse_status(genotype) end content_tag('span', genotype.parse_status, class: "label #{label_class}") end - - def delete_genotype_button(genotype) - link_to( - t('.genotypes.delete'), - genotype, - method: 'delete', - class: "btn btn-default settings__deleting-button", - data: { - confirm: t('.genotypes.confirm_delete', file_name: genotype.genotype_file_name) - } - ) - end end diff --git a/app/views/genotypes/index.html.erb b/app/views/genotypes/index.html.erb index ae916dc2..52b9d64a 100644 --- a/app/views/genotypes/index.html.erb +++ b/app/views/genotypes/index.html.erb @@ -31,7 +31,7 @@ - + <% end %>
    IDSourceTypeFilename Uploaded at Parse status Imported SNPs
    <%= genotype.id %> <%= genotype.filetype %><%= genotype.genotype_file_name %> <%= genotype.created_at %> <%= display_parse_status(genotype) %> <%= genotype.user_snps.count %><%= delete_genotype_button(genotype) %>
    <%= g.id %> <%= g.created_at%> <%= g.filetype %><%= link_to "Download", '../data/' + g.fs_filename, class: "btn btn-default" %><%= download_button(download_genotype_path(g)) %>
    diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index c7a47d49..2f8c10fd 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -89,7 +89,15 @@ <%= genotype.created_at %> <%= display_parse_status(genotype) %> <%= genotype.user_snps.count %> - <%= delete_genotype_button(genotype) %> + + <%= download_button(download_genotype_path(genotype)) %> + <%= delete_button( + genotype, + data: { + confirm: t('.genotypes.confirm_delete', file_name: genotype.genotype_file_name) + } + ) %> + <% end %> diff --git a/config/locales/en.users.yml b/config/locales/en.users.yml index bcc3ba1d..bda08083 100644 --- a/config/locales/en.users.yml +++ b/config/locales/en.users.yml @@ -2,5 +2,4 @@ en: users: edit: genotypes: - delete: 'Delete' confirm_delete: 'Are you sure you want to delete genotype %{file_name}?' diff --git a/config/locales/en.yml b/config/locales/en.yml index 250291ba..44d1db21 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -10,3 +10,6 @@ en: attributes: phenotype: taken: 'has already been entered.' + confirm_delete: 'Are you sure?' + delete: 'delete' + download: 'download' diff --git a/config/routes.rb b/config/routes.rb index ed8bfb48..c56ff998 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,7 +18,9 @@ end end resources :user_picture_phenotypes - resources :genotypes + resources :genotypes do + get 'download', on: :member + end resources :user_phenotypes resources :snps resources :users diff --git a/spec/features/delete_genotype_spec.rb b/spec/features/delete_genotype_spec.rb index 588a12c5..2fb65e78 100644 --- a/spec/features/delete_genotype_spec.rb +++ b/spec/features/delete_genotype_spec.rb @@ -18,9 +18,9 @@ click_on('Settings') click_on('Your genotypes') within('#genotypes') do - page.accept_confirm do - click_on('Delete') - end + page.accept_confirm( + "Are you sure you want to delete genotype #{genotype.genotype_file_name}" + ) { find('[title="delete"]').click } end expect(page).to have_content('Your Genotyping will be deleted. ' \ diff --git a/spec/features/upload_genotype_spec.rb b/spec/features/upload_genotype_spec.rb index 4db8a59a..1d16082d 100644 --- a/spec/features/upload_genotype_spec.rb +++ b/spec/features/upload_genotype_spec.rb @@ -30,7 +30,7 @@ genotype.created_at.to_s, 'queued', '0', - 'Delete' + '' ] ) end @@ -47,7 +47,7 @@ genotype.created_at.to_s, 'done', '5', - 'Delete' + '' ] ) end From 2e49e8ef08506551bb9949db25efb4a17bb7f094 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Wed, 3 Jan 2018 09:37:57 +0100 Subject: [PATCH 21/28] Don't retry parsing Since we now display the parsing status to the user, it makes sence to do this in a predictable manner. --- app/workers/parsing.rb | 2 +- app/workers/preparsing.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/workers/parsing.rb b/app/workers/parsing.rb index 4ad637e7..bbf5c877 100644 --- a/app/workers/parsing.rb +++ b/app/workers/parsing.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Parsing include Sidekiq::Worker - sidekiq_options queue: :user_snps, retry: 5, unique: true + sidekiq_options queue: :user_snps, retry: false, unique: true attr_reader :genotype, :temp_table_name, :normalized_csv, :stats, :start_time diff --git a/app/workers/preparsing.rb b/app/workers/preparsing.rb index aed07f3e..5b542cbc 100644 --- a/app/workers/preparsing.rb +++ b/app/workers/preparsing.rb @@ -4,8 +4,7 @@ class Preparsing include Sidekiq::Worker - # only retry 10 times - after that, the genotyping probably has already been deleted - sidekiq_options queue: :preparse, retry: 10, unique: true + sidekiq_options queue: :preparse, retry: false, unique: true def perform(genotype_id) genotype = Genotype.find(genotype_id) From 1fbbdec40ef3c292ebcaeb175b2a2842e78a7d67 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Wed, 3 Jan 2018 10:03:20 +0100 Subject: [PATCH 22/28] Use i18n for table headers --- app/views/users/_edit.html.erb | 10 +++++----- config/locales/en.users.yml | 1 + config/locales/en.yml | 3 +++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index 2f8c10fd..e0507509 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -73,11 +73,11 @@ - - - - - + + + + + diff --git a/config/locales/en.users.yml b/config/locales/en.users.yml index bda08083..3563cddd 100644 --- a/config/locales/en.users.yml +++ b/config/locales/en.users.yml @@ -3,3 +3,4 @@ en: edit: genotypes: confirm_delete: 'Are you sure you want to delete genotype %{file_name}?' + imported_snps: 'Imported SNPs' diff --git a/config/locales/en.yml b/config/locales/en.yml index 44d1db21..df82fae9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -4,6 +4,9 @@ en: hello: "Hello world" activerecord: + attributes: + genotype: + created_at: 'Uploaded at' errors: models: user_phenotype: From 1bfc9f28862788510df80e551875535ad22e8ef7 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Wed, 3 Jan 2018 10:11:19 +0100 Subject: [PATCH 23/28] Add button for uploading new genotype --- app/views/users/_edit.html.erb | 1 + config/locales/en.users.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index e0507509..1e6eed82 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -70,6 +70,7 @@

    Your Genotypes

    + <%= link_to(t('.genotypes.upload_new'), new_genotype_path, class: 'btn btn-default pull-right') %>
    TypeFilenameUploaded atParse statusImported SNPs<%= Genotype.human_attribute_name(:filetype) %><%= Genotype.human_attribute_name(:filename) %><%= Genotype.human_attribute_name(:created_at) %><%= Genotype.human_attribute_name(:parse_status) %><%= t('.genotypes.imported_snps') %>
    diff --git a/config/locales/en.users.yml b/config/locales/en.users.yml index 3563cddd..bc6bdda7 100644 --- a/config/locales/en.users.yml +++ b/config/locales/en.users.yml @@ -4,3 +4,4 @@ en: genotypes: confirm_delete: 'Are you sure you want to delete genotype %{file_name}?' imported_snps: 'Imported SNPs' + upload_new: 'Upload new genotype' From 1165df863fc89e7dd3d2b29fbc54245f746b8022 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Wed, 3 Jan 2018 10:38:51 +0100 Subject: [PATCH 24/28] Add info on what to do when the parsing errored --- app/helpers/users_helper.rb | 6 +++--- app/views/users/_edit.html.erb | 5 ++++- config/locales/en.users.yml | 5 +++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index d089974f..77af281f 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -1,11 +1,11 @@ module UsersHelper - def display_parse_status(genotype) - label_class = case genotype.parse_status + def genotype_parse_status_label(parse_status) + label_class = case parse_status when 'queued' then 'label-default' when 'parsing' then 'label-primary' when 'done' then 'label-success' when 'error' then 'label-danger' end - content_tag('span', genotype.parse_status, class: "label #{label_class}") + content_tag('span', parse_status, class: "label #{label_class}") end end diff --git a/app/views/users/_edit.html.erb b/app/views/users/_edit.html.erb index 1e6eed82..eb246313 100644 --- a/app/views/users/_edit.html.erb +++ b/app/views/users/_edit.html.erb @@ -88,7 +88,7 @@ - +
    <%= genotype.filetype %> <%= genotype.genotype_file_name %> <%= genotype.created_at %><%= display_parse_status(genotype) %><%= genotype_parse_status_label(genotype.parse_status) %> <%= genotype.user_snps.count %> <%= download_button(download_genotype_path(genotype)) %> @@ -103,6 +103,9 @@ <% end %>
    +
    + <%= t('.genotypes.parse_error_info', error_label: genotype_parse_status_label('error')).html_safe %> +
    diff --git a/config/locales/en.users.yml b/config/locales/en.users.yml index bc6bdda7..1bc56797 100644 --- a/config/locales/en.users.yml +++ b/config/locales/en.users.yml @@ -5,3 +5,8 @@ en: confirm_delete: 'Are you sure you want to delete genotype %{file_name}?' imported_snps: 'Imported SNPs' upload_new: 'Upload new genotype' + parse_error_info: > + If the parse status shows %{error_label}, check if your uploaded file + looks alright and try again. Note: While zipped files (.zip) work, + gzipped ones (.gz) do not for now. If it still does not work, + contact us. From d1d916d4c87741e8cb97b2fd04ec731a4f39fd02 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Wed, 3 Jan 2018 10:40:32 +0100 Subject: [PATCH 25/28] Use warning label for "parsing" status --- app/helpers/users_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 77af281f..25e8b4f5 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -2,7 +2,7 @@ module UsersHelper def genotype_parse_status_label(parse_status) label_class = case parse_status when 'queued' then 'label-default' - when 'parsing' then 'label-primary' + when 'parsing' then 'label-warning' when 'done' then 'label-success' when 'error' then 'label-danger' end From a76971325d1c930dd16da7e4cb84dbe43d9feba0 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Wed, 3 Jan 2018 11:09:25 +0100 Subject: [PATCH 26/28] Switch to Chrome for feature specs --- .travis.yml | 1 + Gemfile | 3 ++- Gemfile.lock | 22 +++++++++++++--------- spec/spec_helper.rb | 3 +-- spec/support/chrome_webdriver.rb | 19 +++++++++++++++++++ 5 files changed, 36 insertions(+), 12 deletions(-) create mode 100644 spec/support/chrome_webdriver.rb diff --git a/.travis.yml b/.travis.yml index 2ea2137b..0a6de680 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ addons: apt: packages: - libhiredis-dev + chrome: stable postgresql: '9.3' services: - redis-server diff --git a/Gemfile b/Gemfile index 2350b10b..f98a7b05 100644 --- a/Gemfile +++ b/Gemfile @@ -71,7 +71,8 @@ group :test do gem 'webmock' gem 'vcr' gem 'capybara' - gem 'poltergeist' + gem 'selenium-webdriver', require: false + gem 'chromedriver-helper' gem 'database_cleaner' gem 'timecop' end diff --git a/Gemfile.lock b/Gemfile.lock index 87a192b6..3ae8ecb1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -62,6 +62,8 @@ GEM tzinfo (~> 1.1) addressable (2.5.1) public_suffix (~> 2.0, >= 2.0.2) + archive-zip (0.7.0) + io-like (~> 0.3.0) arel (6.0.4) ast (2.3.0) authlogic (3.6.0) @@ -86,9 +88,13 @@ GEM rack (>= 1.0.0) rack-test (>= 0.5.4) xpath (~> 2.0) + childprocess (0.8.0) + ffi (~> 1.0, >= 1.0.11) + chromedriver-helper (1.1.0) + archive-zip (~> 0.7.0) + nokogiri (~> 1.6) chronic (0.10.2) climate_control (0.2.0) - cliver (0.3.2) cocaine (0.5.8) climate_control (>= 0.0.3, < 1.0) coderay (1.1.1) @@ -146,6 +152,7 @@ GEM http-cookie (1.0.3) domain_name (~> 0.5) i18n (0.8.1) + io-like (0.3.0) jquery-rails (4.3.1) rails-dom-testing (>= 1, < 3) railties (>= 4.2.0) @@ -212,10 +219,6 @@ GEM plos (0.0.7) nokogiri rest-client - poltergeist (1.15.0) - capybara (~> 2.1) - cliver (~> 0.3.1) - websocket-driver (>= 0.2.0) power_assert (1.0.2) powerpack (0.1.1) pry (0.10.4) @@ -317,6 +320,9 @@ GEM tilt (>= 1.1, < 3) scrypt (3.0.5) ffi-compiler (>= 1.0, < 2.0) + selenium-webdriver (3.8.0) + childprocess (~> 0.5) + rubyzip (~> 1.0) sentry-raven (2.7.1) faraday (>= 0.7.6, < 1.0) shellany (0.0.1) @@ -376,9 +382,6 @@ GEM addressable (>= 2.3.6) crack (>= 0.3.2) hashdiff - websocket-driver (0.6.5) - websocket-extensions (>= 0.1.0) - websocket-extensions (0.1.2) whenever (0.9.7) chronic (>= 0.6.3) will_paginate (3.1.5) @@ -397,6 +400,7 @@ DEPENDENCIES bcrypt-ruby capistrano (~> 2.0) capybara + chromedriver-helper composite_primary_keys (~> 8.0) database_cleaner dotenv-rails @@ -420,7 +424,6 @@ DEPENDENCIES pg pg_search plos - poltergeist pry-rails rails (~> 4.2) rails3-generators @@ -432,6 +435,7 @@ DEPENDENCIES rvm-capistrano (= 1.4.4) sanitize sass-rails + selenium-webdriver sentry-raven shoulda-context sidekiq diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3f086cbf..33dd7344 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -10,9 +10,8 @@ require 'factory_girl_rails' require 'pry-rails' unless ENV['CI'] require 'authlogic/test_case' -require 'capybara/poltergeist' -Capybara.javascript_driver = :poltergeist +Capybara.javascript_driver = :chrome_headless # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. diff --git a/spec/support/chrome_webdriver.rb b/spec/support/chrome_webdriver.rb new file mode 100644 index 00000000..64978e74 --- /dev/null +++ b/spec/support/chrome_webdriver.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'selenium/webdriver' + +Capybara.register_driver :chrome do |app| + Capybara::Selenium::Driver.new(app, browser: :chrome) +end + +Capybara.register_driver :chrome_headless do |app| + capabilities = Selenium::WebDriver::Remote::Capabilities.chrome( + chromeOptions: { args: %w[headless disable-gpu] } + ) + + Capybara::Selenium::Driver.new( + app, + browser: :chrome, + desired_capabilities: capabilities + ) +end From f87ecc00efafc3a055f2866e86da51d26bd87730 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Thu, 4 Jan 2018 10:37:51 +0100 Subject: [PATCH 27/28] Only show successfully parsed genotypes --- app/controllers/genotypes_controller.rb | 9 ++++----- app/models/genotype.rb | 4 ++++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/app/controllers/genotypes_controller.rb b/app/controllers/genotypes_controller.rb index 4dfe3515..47958512 100644 --- a/app/controllers/genotypes_controller.rb +++ b/app/controllers/genotypes_controller.rb @@ -6,10 +6,10 @@ class GenotypesController < ApplicationController def index @title = "Listing all genotypings" - @genotypes = - Genotype - .includes(:user) - .order("#{sort_column} #{sort_direction}") + @genotypes = Genotype + .successfully_parsed + .includes(:user) + .order("#{sort_column} #{sort_direction}") @genotypes_paginate = @genotypes.paginate(page: params[:page], per_page: 20) end @@ -87,5 +87,4 @@ def sort_direction def genotype_params params.require(:genotype).permit(:genotype, :filetype) end - end diff --git a/app/models/genotype.rb b/app/models/genotype.rb index bc8f4703..27802519 100644 --- a/app/models/genotype.rb +++ b/app/models/genotype.rb @@ -16,6 +16,10 @@ class Genotype < ActiveRecord::Base size: { in: 0..400.megabytes } do_not_validate_attachment_file_type :genotype + def self.successfully_parsed + where("parse_status IS NULL OR parse_status = 'done'") + end + def is_image? false end From 62c90631dfd8bb1d953417e501da44060cbf9403 Mon Sep 17 00:00:00 2001 From: Helge Rausch Date: Thu, 4 Jan 2018 10:57:46 +0100 Subject: [PATCH 28/28] =?UTF-8?q?Hound=20happiness=20=F0=9F=90=B6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .rubocop.yml | 4 ++++ app/helpers/users_helper.rb | 2 ++ app/workers/preparsing.rb | 2 +- db/migrate/20171231121548_add_parse_status_to_genotypes.rb | 2 ++ spec/features/delete_genotype_spec.rb | 1 + spec/workers/parsing_spec.rb | 1 + spec/workers/preparsing_spec.rb | 2 ++ 7 files changed, 13 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index b7b21128..fb99c562 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -28,3 +28,7 @@ Style/FrozenStringLiteralComment: Metrics/LineLength: Max: 100 + +Metrics/BlockLength: + Exclude: + - 'spec/**/*.rb' diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index 25e8b4f5..8f0513c2 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module UsersHelper def genotype_parse_status_label(parse_status) label_class = case parse_status diff --git a/app/workers/preparsing.rb b/app/workers/preparsing.rb index 5b542cbc..2eee513a 100644 --- a/app/workers/preparsing.rb +++ b/app/workers/preparsing.rb @@ -129,7 +129,7 @@ def perform(genotype_id) Parsing.perform_async(genotype.id) end - rescue + rescue StandardError genotype.update!(parse_status: 'error') raise end diff --git a/db/migrate/20171231121548_add_parse_status_to_genotypes.rb b/db/migrate/20171231121548_add_parse_status_to_genotypes.rb index ce1a2758..c7503042 100644 --- a/db/migrate/20171231121548_add_parse_status_to_genotypes.rb +++ b/db/migrate/20171231121548_add_parse_status_to_genotypes.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddParseStatusToGenotypes < ActiveRecord::Migration def change add_column :genotypes, :parse_status, :string diff --git a/spec/features/delete_genotype_spec.rb b/spec/features/delete_genotype_spec.rb index 2fb65e78..833a2152 100644 --- a/spec/features/delete_genotype_spec.rb +++ b/spec/features/delete_genotype_spec.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + RSpec.feature 'Delete a genotype', :js, sidekiq: :inline do let(:user) { create(:user, name: 'Gregor Mendel') } let!(:genotype) { create(:genotype, user: user, genotype_file_name: 'test.txt') } diff --git a/spec/workers/parsing_spec.rb b/spec/workers/parsing_spec.rb index 0b43ac82..673be66f 100644 --- a/spec/workers/parsing_spec.rb +++ b/spec/workers/parsing_spec.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + # TODO: Add test cases for different filetypes. describe Parsing do let(:genotype) do diff --git a/spec/workers/preparsing_spec.rb b/spec/workers/preparsing_spec.rb index a8815093..4d91cddc 100644 --- a/spec/workers/preparsing_spec.rb +++ b/spec/workers/preparsing_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + RSpec.describe Preparsing do context 'when the genotype is valid' do let(:genotype) do