From 8b1677a5e07f771d0f520b624f20ddd0bd05fcd1 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 15 Jun 2020 17:19:12 +0100 Subject: [PATCH 01/21] Start on the front end - Be able to display responses from lighthouse / wrangler in a table - change to 'render' not 'redirect_to' to allow updating view with results without using flash (flash can't handle a lot of data) - new view for this called 'results' - problem with rendering the 'new' view/form underneath is that the URL still says /process_plates, not /process_plates/new (due to render not redirect), so page won't work if you refresh it - TODO - have broken existing flash messages, fix them - TODO - show red failure flash if some of the service calls failed - TODO - modify the services to return me more information that I can display in my table --- app/controllers/process_plates_controller.rb | 81 +++++++++++++++++--- app/views/process_plates/new.html.haml | 55 ++++++------- app/views/process_plates/results.html.haml | 20 +++++ config/environments/development.rb | 2 +- 4 files changed, 118 insertions(+), 40 deletions(-) create mode 100644 app/views/process_plates/results.html.haml diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index 4b35b1c1..bdaadbfe 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -16,7 +16,7 @@ def new end def create - respond_to do |format| + # respond_to do |format| bed_verification_model = InstrumentProcessesInstrument.get_bed_verification_type(params[:instrument_barcode], params[:instrument_process]) if bed_verification_model.nil? flash[:error] = 'Invalid instrument or process' @@ -33,33 +33,46 @@ def create # find out if the 'receive_plates' process was executed receive_plates_process = InstrumentProcess.find_by(id: params[:instrument_process]).key.eql?('slf_receive_plates') - call_external_services(barcodes) if barcodes && receive_plates_process + message = '' + if barcodes && receive_plates_process + call_external_services(barcodes) - # add a flash on the page for the number of unique barcodes scanned in - num_unique_barcodes = bed_layout_verification.process_plate&.barcodes.uniq.length - if num_unique_barcodes - flash[:notice] = "Success - #{num_unique_barcodes} unique plate(s) scanned" - else - flash[:notice] = "Success" + message = @lighthouse_responses.concat(@wrangler_responses) end + + # TODO: display the responses from the services in a partial, not a flash + # TODO: sort out original flash code below + flash[:notice] = message + + # add a flash on the page for the number of unique barcodes scanned in + # num_unique_barcodes = bed_layout_verification.process_plate&.barcodes.uniq.length + # if num_unique_barcodes + # flash[:notice] = "Success - #{num_unique_barcodes} unique plate(s) scanned" + # else + # flash[:notice] = "Success" + # end else flash[:error] = bed_layout_verification.errors.values.flatten.join("\n") end end - format.html { redirect_to(new_process_plate_path) } - end + @results = parse_responses + render :results + # end end # Call any external services - currently lighthouse service for plates from Lighthouse Labs and # wrangler for tube racks. If no samples are found in the lighthouse service, try the wrangler def call_external_services(barcodes) + puts "DEBUG: call_external_services" # call the lighthouse service first as we are assuming that most labware scanned will be # plates from Lighthouse Labs - lighthouse_responses = Lighthouse.call_api(barcodes) + @lighthouse_responses = Lighthouse.call_api(barcodes) + puts "DEBUG: lighthouse_responses: #{@lighthouse_responses}" # keeping it simple for now, if all the responses are not CREATED, send ALL the barcodes # to the wrangler - wrangler_responses = Wrangler.call_api(barcodes) unless all_created?(lighthouse_responses) + @wrangler_responses = Wrangler.call_api(barcodes) unless all_created?(@lighthouse_responses) + puts "DEBUG: wrangler_responses: #{@wrangler_responses}" end # Returns a list of unique barcodes by removing blanks and duplicates @@ -73,4 +86,48 @@ def all_created?(responses) responses.all? { |response| response[:code] == "201" } end + + def parse_responses + output = [] + + @lighthouse_responses.each do |r| + + barcode = r[:barcode] + success = if r[:code] == '201' + 'Yes' + else + 'No' + end + purpose = r[:purpose] + study = r[:study] + + output << { + :barcode => barcode, + :success => success, + :purpose => purpose, + :study => study + } + end + + @wrangler_responses.each do |r| + + barcode = r[:barcode] + success = if r[:code] == '201' + 'Yes' + else + 'No' + end + purpose = r[:purpose] + study = r[:study] + + output << { + :barcode => barcode, + :success => success, + :purpose => purpose, + :study => study + } + end + + output + end end diff --git a/app/views/process_plates/new.html.haml b/app/views/process_plates/new.html.haml index e438fa74..5c110530 100644 --- a/app/views/process_plates/new.html.haml +++ b/app/views/process_plates/new.html.haml @@ -1,42 +1,43 @@ %h2 Activity Logging %div{:id => "info_box"} + = form_for :process_plate, :url => process_plates_path , :html => {:class => "submit-once"} do |form| - %fieldset - %ul - %li - %label{ :for => "user_barcode" } User barcode - = text_field_tag "user_barcode" - .live_results#user_barcode_results + %fieldset + %ul + %li + %label{ :for => "user_barcode" } User barcode + = text_field_tag "user_barcode" + .live_results#user_barcode_results - %li - %label{ :for => "instrument_barcode" } Instrument barcode - = text_field_tag "instrument_barcode" - .live_results#instrument_barcode_results + %li + %label{ :for => "instrument_barcode" } Instrument barcode + = text_field_tag "instrument_barcode" + .live_results#instrument_barcode_results - %li - %label{ :for => "instrument_process" } Instrument process - %select#instrument_process{ :name => "instrument_process" } - %option{ :value => -1 } Select a process... - - for process_name, process_id in InstrumentProcess.sorted_by_name.map { |x| [x.name, x.id]} - %option{ :value => process_id } #{process_name} + %li + %label{ :for => "instrument_process" } Instrument process + %select#instrument_process{ :name => "instrument_process" } + %option{ :value => -1 } Select a process... + - for process_name, process_id in InstrumentProcess.sorted_by_name.map { |x| [x.name, x.id]} + %option{ :value => process_id } #{process_name} - %li - #source_plates_results + %li + #source_plates_results - %li.hidden#visual_check_input - %label{ :for => "visual_check"} Visual check performed - = check_box_tag "visual_check" + %li.hidden#visual_check_input + %label{ :for => "visual_check"} Visual check performed + = check_box_tag "visual_check" - %li.hidden#witness_barcode_input - %label{ :for => "witness_barcode"} Witness barcode - = text_field_tag "witness_barcode" + %li.hidden#witness_barcode_input + %label{ :for => "witness_barcode"} Witness barcode + = text_field_tag "witness_barcode" - %li - %input{ :id => "btnSubmit", :type => "submit", :value => 'Submit' } + %li + %input{ :id => "btnSubmit", :type => "submit", :value => 'Submit' } - content_for :page_javascript do - :javascript + :javascript (function($) { diff --git a/app/views/process_plates/results.html.haml b/app/views/process_plates/results.html.haml new file mode 100644 index 00000000..c1e7cb9b --- /dev/null +++ b/app/views/process_plates/results.html.haml @@ -0,0 +1,20 @@ +#service_responses + %table + %tr + %th.th Barcode + %th Success? + %th Purpose + %th Study + - for row in @results + %tr + %td + = row[:barcode] + %td + = row[:success] + %td + = row[:purpose] + %td + = row[:study] + +%br += link_to 'Scan more labware', new_process_plate_path \ No newline at end of file diff --git a/config/environments/development.rb b/config/environments/development.rb index f0cefa58..902bbbb8 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -30,7 +30,7 @@ config.admin_email = 'example@example.com' # https://github.com/sanger/wrangler - config.wrangler_url = 'http://127.0.0.1:5001/tube_rack' + config.wrangler_url = 'http://127.0.0.1:5000/wrangle' # https://github.com/sanger/lighthouse config.lighthouse_host = 'http://127.0.0.1:5000' From 646642c6b39160c2671850fbd04d6c3457fe37a8 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Tue, 16 Jun 2020 14:46:17 +0100 Subject: [PATCH 02/21] refactor create method, adding more error handling --- app/controllers/process_plates_controller.rb | 84 ++++++++++---------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index bdaadbfe..c03183ea 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -16,48 +16,48 @@ def new end def create - # respond_to do |format| - bed_verification_model = InstrumentProcessesInstrument.get_bed_verification_type(params[:instrument_barcode], params[:instrument_process]) - if bed_verification_model.nil? - flash[:error] = 'Invalid instrument or process' - else - bed_layout_verification = bed_verification_model.new( - instrument_barcode: params[:instrument_barcode], - scanned_values: params[:robot], - api: api - ) - if bed_layout_verification.validate_and_create_audits?(params) - # the param is called 'source_plates' but we could be working with tube racks or plates etc. - barcodes = sanitize_barcodes(params[:source_plates]) - - # find out if the 'receive_plates' process was executed - receive_plates_process = InstrumentProcess.find_by(id: params[:instrument_process]).key.eql?('slf_receive_plates') - - message = '' - if barcodes && receive_plates_process - call_external_services(barcodes) - - message = @lighthouse_responses.concat(@wrangler_responses) - end - - # TODO: display the responses from the services in a partial, not a flash - # TODO: sort out original flash code below - flash[:notice] = message - - # add a flash on the page for the number of unique barcodes scanned in - # num_unique_barcodes = bed_layout_verification.process_plate&.barcodes.uniq.length - # if num_unique_barcodes - # flash[:notice] = "Success - #{num_unique_barcodes} unique plate(s) scanned" - # else - # flash[:notice] = "Success" - # end - else - flash[:error] = bed_layout_verification.errors.values.flatten.join("\n") - end - end - @results = parse_responses - render :results - # end + bed_verification_model = InstrumentProcessesInstrument.get_bed_verification_type( + params[:instrument_barcode], + params[:instrument_process] + ) + back_to_new_with_error('Invalid instrument or process') and return if bed_verification_model.nil? + + bed_layout_verification = bed_verification_model.new( + instrument_barcode: params[:instrument_barcode], + scanned_values: params[:robot], + api: api + ) + unless bed_layout_verification.validate_and_create_audits?(params) + errors = bed_layout_verification.errors.values.flatten.join("\n") + back_to_new_with_error(errors) and return + end + + back_to_new_with_message('Success') and return unless receive_plates_process?(params) + + # the param is called 'source_plates' but we could be working with tube racks or plates etc. + barcodes = sanitize_barcodes(params[:source_plates]) + unless barcodes && !barcodes.empty? + back_to_new_with_error('No barcodes were provided') and return + end + + call_external_services(barcodes) + + @results = parse_responses + render :results + end + + def back_to_new_with_message(message, flash_type=:notice) + flash[flash_type] = message + redirect_to(new_process_plate_path) + end + + def back_to_new_with_error(message) + back_to_new_with_message(message, :error) + end + + # find out if the 'receive_plates' process was executed + def receive_plates_process?(params) + @is_receive_plates_process ||= InstrumentProcess.find_by(id: params[:instrument_process]).key.eql?('slf_receive_plates') end # Call any external services - currently lighthouse service for plates from Lighthouse Labs and From 3d0d967082815b6b9bc39e79977a73707d4752c7 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 17 Jun 2020 16:38:20 +0100 Subject: [PATCH 03/21] results table now displays correctly for successful wrangler responses - integrating with Andrew's Sequencescape branch gpl442_feedback_for_sm --- app/controllers/process_plates_controller.rb | 4 +-- app/views/process_plates/results.html.haml | 30 +++++++++++--------- lib/wrangler.rb | 5 ++-- 3 files changed, 21 insertions(+), 18 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index c03183ea..5e5e04d9 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -117,8 +117,8 @@ def parse_responses else 'No' end - purpose = r[:purpose] - study = r[:study] + purpose = r[:json]['data']['attributes']['purpose_name'] + study = r[:json]['data']['attributes']['study_names'].join(', ') output << { :barcode => barcode, diff --git a/app/views/process_plates/results.html.haml b/app/views/process_plates/results.html.haml index c1e7cb9b..e24ced34 100644 --- a/app/views/process_plates/results.html.haml +++ b/app/views/process_plates/results.html.haml @@ -1,20 +1,22 @@ #service_responses %table - %tr - %th.th Barcode - %th Success? - %th Purpose - %th Study - - for row in @results + %thead %tr - %td - = row[:barcode] - %td - = row[:success] - %td - = row[:purpose] - %td - = row[:study] + %th.th Barcode + %th Success? + %th Purpose + %th Study + %tbody + - for row in @results + %tr + %td + = row[:barcode] + %td + = row[:success] + %td + = row[:purpose] + %td + = row[:study] %br = link_to 'Scan more labware', new_process_plate_path \ No newline at end of file diff --git a/lib/wrangler.rb b/lib/wrangler.rb index d813060e..b4468528 100644 --- a/lib/wrangler.rb +++ b/lib/wrangler.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'json' + class Wrangler require 'net/http' @@ -17,8 +19,7 @@ def self.call_api(barcodes) res = Net::HTTP.start(url.host, url.port) do |http| http.request(req) end - - responses << { barcode: barcode, code: res.code } + responses << { barcode: barcode, code: res.code, json: JSON.parse(res.body)} end rescue StandardError => e Rails.logger.error(e) From ac10e0f50573266c6fda135bae9d4d4644766e3e Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Wed, 17 Jun 2020 17:25:46 +0100 Subject: [PATCH 04/21] add some more error handling and detail to the table - TO DO: don't show an error from the lighthouse service for a barcode, if the wrangler was subsequently successful for the same barcode --- app/controllers/process_plates_controller.rb | 19 ++++++++++++++----- app/views/process_plates/results.html.haml | 5 +++++ lib/lighthouse.rb | 2 +- lib/wrangler.rb | 2 +- 4 files changed, 21 insertions(+), 7 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index 5e5e04d9..867a7f5f 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -43,6 +43,8 @@ def create call_external_services(barcodes) @results = parse_responses + back_to_new_with_error('No response from services') and return if @results.empty? + render :results end @@ -63,7 +65,6 @@ def receive_plates_process?(params) # Call any external services - currently lighthouse service for plates from Lighthouse Labs and # wrangler for tube racks. If no samples are found in the lighthouse service, try the wrangler def call_external_services(barcodes) - puts "DEBUG: call_external_services" # call the lighthouse service first as we are assuming that most labware scanned will be # plates from Lighthouse Labs @lighthouse_responses = Lighthouse.call_api(barcodes) @@ -98,12 +99,16 @@ def parse_responses else 'No' end - purpose = r[:purpose] - study = r[:study] + if r[:body].is_a? Hash + error = r.dig(:body, 'error') + purpose = r.dig(:body, 'data', 'attributes', 'purpose_name') + study = r.dig(:body, 'data', 'attributes', 'study_names')&.join(', ') + end output << { :barcode => barcode, :success => success, + :error => error, :purpose => purpose, :study => study } @@ -117,12 +122,16 @@ def parse_responses else 'No' end - purpose = r[:json]['data']['attributes']['purpose_name'] - study = r[:json]['data']['attributes']['study_names'].join(', ') + if r[:body].is_a? Hash + error = r.dig(:body, 'error') + purpose = r.dig(:body, 'data', 'attributes', 'purpose_name') + study = r.dig(:body, 'data', 'attributes', 'study_names')&.join(', ') + end output << { :barcode => barcode, :success => success, + :error => error, :purpose => purpose, :study => study } diff --git a/app/views/process_plates/results.html.haml b/app/views/process_plates/results.html.haml index e24ced34..5090505e 100644 --- a/app/views/process_plates/results.html.haml +++ b/app/views/process_plates/results.html.haml @@ -1,9 +1,12 @@ +%h2 Activity Logging + #service_responses %table %thead %tr %th.th Barcode %th Success? + %th Error %th Purpose %th Study %tbody @@ -13,6 +16,8 @@ = row[:barcode] %td = row[:success] + %td + = row[:error] %td = row[:purpose] %td diff --git a/lib/lighthouse.rb b/lib/lighthouse.rb index 69842740..528360f1 100644 --- a/lib/lighthouse.rb +++ b/lib/lighthouse.rb @@ -20,7 +20,7 @@ def self.call_api(barcodes) http.request(req) end - responses << { code: res.code, body: res.body } + responses << { barcode: barcode, code: res.code, body: res.body } end rescue StandardError => e Rails.logger.error(e) diff --git a/lib/wrangler.rb b/lib/wrangler.rb index b4468528..2cdfd066 100644 --- a/lib/wrangler.rb +++ b/lib/wrangler.rb @@ -19,7 +19,7 @@ def self.call_api(barcodes) res = Net::HTTP.start(url.host, url.port) do |http| http.request(req) end - responses << { barcode: barcode, code: res.code, json: JSON.parse(res.body)} + responses << { barcode: barcode, code: res.code, body: JSON.parse(res.body)} end rescue StandardError => e Rails.logger.error(e) From 0faefaa69b429f1f41707791a5c365e3ba697de2 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 18 Jun 2020 10:22:39 +0100 Subject: [PATCH 05/21] re-structure results table by looping through scanned barcodes and looking for successful responses for them - solves issue of having a failed response from one service and successful from the other - error detail will still be in the logs - refactor controller code to reduce code duplication --- app/controllers/process_plates_controller.rb | 80 ++++++++------------ app/views/process_plates/results.html.haml | 41 +++++----- 2 files changed, 51 insertions(+), 70 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index 867a7f5f..47649f19 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -42,9 +42,14 @@ def create call_external_services(barcodes) - @results = parse_responses + @results = generate_results(barcodes) back_to_new_with_error('No response from services') and return if @results.empty? + if all_labware_created?(@results) + flash[:notice] = 'All labware were successfully created.' + else + flash[:error] = 'Some labware were not able to be created.' + end render :results end @@ -68,12 +73,10 @@ def call_external_services(barcodes) # call the lighthouse service first as we are assuming that most labware scanned will be # plates from Lighthouse Labs @lighthouse_responses = Lighthouse.call_api(barcodes) - puts "DEBUG: lighthouse_responses: #{@lighthouse_responses}" # keeping it simple for now, if all the responses are not CREATED, send ALL the barcodes # to the wrangler @wrangler_responses = Wrangler.call_api(barcodes) unless all_created?(@lighthouse_responses) - puts "DEBUG: wrangler_responses: #{@wrangler_responses}" end # Returns a list of unique barcodes by removing blanks and duplicates @@ -88,55 +91,34 @@ def all_created?(responses) responses.all? { |response| response[:code] == "201" } end - def parse_responses - output = [] - - @lighthouse_responses.each do |r| - - barcode = r[:barcode] - success = if r[:code] == '201' - 'Yes' - else - 'No' - end - if r[:body].is_a? Hash - error = r.dig(:body, 'error') - purpose = r.dig(:body, 'data', 'attributes', 'purpose_name') - study = r.dig(:body, 'data', 'attributes', 'study_names')&.join(', ') - end - - output << { - :barcode => barcode, - :success => success, - :error => error, - :purpose => purpose, - :study => study - } - end + def generate_results(barcodes) + output = {} + # default 'success' for each barcode to 'No' + barcodes.each { |b| output[b] = { success: 'No' } } - @wrangler_responses.each do |r| - - barcode = r[:barcode] - success = if r[:code] == '201' - 'Yes' - else - 'No' - end - if r[:body].is_a? Hash - error = r.dig(:body, 'error') - purpose = r.dig(:body, 'data', 'attributes', 'purpose_name') - study = r.dig(:body, 'data', 'attributes', 'study_names')&.join(', ') - end - - output << { - :barcode => barcode, - :success => success, - :error => error, - :purpose => purpose, - :study => study - } + # loop through service responses to update 'output' with successes + @lighthouse_responses.select { |r| r[:code] == '201' }.each do |r| + output[r[:barcode]] = parse_response(r, :Lighthouse) + end + @wrangler_responses.select { |r| r[:code] == '201' }.each do |r| + output[r[:barcode]] = parse_response(r, :CGaP) end output end + + def parse_response(response, service) + { + success: 'Yes', + source: service, + purpose: response.dig(:body, 'data', 'attributes', 'purpose_name'), + study: response.dig(:body, 'data', 'attributes', 'study_names')&.join(', ') + } + end + + def all_labware_created?(results) + return false if results.any?{ |_barcode, details| details[:success] == 'No' } + + true + end end diff --git a/app/views/process_plates/results.html.haml b/app/views/process_plates/results.html.haml index 5090505e..8b4e97fd 100644 --- a/app/views/process_plates/results.html.haml +++ b/app/views/process_plates/results.html.haml @@ -1,27 +1,26 @@ %h2 Activity Logging -#service_responses - %table - %thead +%table + %thead + %tr + %th Barcode + %th Success? + %th Source + %th Purpose + %th Study + %tbody + - @results.each do |barcode, details| %tr - %th.th Barcode - %th Success? - %th Error - %th Purpose - %th Study - %tbody - - for row in @results - %tr - %td - = row[:barcode] - %td - = row[:success] - %td - = row[:error] - %td - = row[:purpose] - %td - = row[:study] + %td + = barcode + %td + = details[:success] + %td + = details[:source] + %td + = details[:purpose] + %td + = details[:study] %br = link_to 'Scan more labware', new_process_plate_path \ No newline at end of file From d607a521577ad51f3479291180fc5dd91fdc0220 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 18 Jun 2020 10:52:31 +0100 Subject: [PATCH 06/21] Add some basic styling for the table - also improves tables in admin/processes and admin/instruments - also get rid of weird "" marks around titles on above two pages - move custom css into the app/assets folder and add a manifest application.css file as per Rails Asset Pipeline docs --- app/assets/stylesheets/application.css | 1 + .../assets/stylesheets/form_custom.css | 11 +++++++++++ app/views/admin/instruments/index.html.haml | 2 +- app/views/admin/processes/index.html.haml | 2 +- app/views/layouts/application.html.erb | 2 +- 5 files changed, 15 insertions(+), 3 deletions(-) create mode 100644 app/assets/stylesheets/application.css rename public/stylesheets/form.css => app/assets/stylesheets/form_custom.css (83%) diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css new file mode 100644 index 00000000..d3a82b1f --- /dev/null +++ b/app/assets/stylesheets/application.css @@ -0,0 +1 @@ +@import "form_custom"; \ No newline at end of file diff --git a/public/stylesheets/form.css b/app/assets/stylesheets/form_custom.css similarity index 83% rename from public/stylesheets/form.css rename to app/assets/stylesheets/form_custom.css index d2a54b8f..e3b13c8b 100644 --- a/public/stylesheets/form.css +++ b/app/assets/stylesheets/form_custom.css @@ -62,5 +62,16 @@ li#beds_and_plates ul{ padding: 0px; } +table, td, th { + border: 1px solid #ddd; + text-align: left; +} +table { + border-collapse: collapse; + /* width: 100%; */ +} +th, td { + padding: 15px; +} diff --git a/app/views/admin/instruments/index.html.haml b/app/views/admin/instruments/index.html.haml index 2871583f..4936ae14 100644 --- a/app/views/admin/instruments/index.html.haml +++ b/app/views/admin/instruments/index.html.haml @@ -1,4 +1,4 @@ -%h1 "Manage Instruments" +%h1 Manage Instruments %table#instruments %thead diff --git a/app/views/admin/processes/index.html.haml b/app/views/admin/processes/index.html.haml index 9f6e3812..39560d41 100644 --- a/app/views/admin/processes/index.html.haml +++ b/app/views/admin/processes/index.html.haml @@ -1,4 +1,4 @@ -%h1 "Manage Processes" +%h1 Manage Processes %table#processes %thead diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 015d6758..9506e150 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -2,7 +2,7 @@ <%= ProcessTracking::Application.config.name %> - <%= stylesheet_link_tag(:flash,:form,:formtastic_static,:formtastic_changes) %> + <%= stylesheet_link_tag(:flash,:formtastic_static,:formtastic_changes,:application) %> <%= javascript_include_tag('jquery.min.js','rails.js','application.js') %> <%= csrf_meta_tag %> From f5312ad1d6c59c9b55588c9d57d4e39c3c61fdcb Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 18 Jun 2020 10:54:38 +0100 Subject: [PATCH 07/21] Get rid of commented out width style - don't think full width tables look very good here --- app/assets/stylesheets/form_custom.css | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/stylesheets/form_custom.css b/app/assets/stylesheets/form_custom.css index e3b13c8b..c126266b 100644 --- a/app/assets/stylesheets/form_custom.css +++ b/app/assets/stylesheets/form_custom.css @@ -69,7 +69,6 @@ table, td, th { table { border-collapse: collapse; - /* width: 100%; */ } th, td { From f4bbbbab9a25f970fcdcc60b74d8b3480e11057f Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 18 Jun 2020 11:05:19 +0100 Subject: [PATCH 08/21] the easy rubocop fixes --- app/controllers/process_plates_controller.rb | 18 ++++++++---------- lib/wrangler.rb | 2 +- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index 47649f19..c2d65568 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -20,7 +20,7 @@ def create params[:instrument_barcode], params[:instrument_process] ) - back_to_new_with_error('Invalid instrument or process') and return if bed_verification_model.nil? + back_to_new_with_error('Invalid instrument or process') && return if bed_verification_model.nil? bed_layout_verification = bed_verification_model.new( instrument_barcode: params[:instrument_barcode], @@ -29,21 +29,19 @@ def create ) unless bed_layout_verification.validate_and_create_audits?(params) errors = bed_layout_verification.errors.values.flatten.join("\n") - back_to_new_with_error(errors) and return + back_to_new_with_error(errors) && return end - back_to_new_with_message('Success') and return unless receive_plates_process?(params) + back_to_new_with_message('Success') && return unless receive_plates_process?(params) # the param is called 'source_plates' but we could be working with tube racks or plates etc. barcodes = sanitize_barcodes(params[:source_plates]) - unless barcodes && !barcodes.empty? - back_to_new_with_error('No barcodes were provided') and return - end + back_to_new_with_error('No barcodes were provided') && return unless barcodes && !barcodes.empty? call_external_services(barcodes) @results = generate_results(barcodes) - back_to_new_with_error('No response from services') and return if @results.empty? + back_to_new_with_error('No response from services') && return if @results.empty? if all_labware_created?(@results) flash[:notice] = 'All labware were successfully created.' @@ -53,7 +51,7 @@ def create render :results end - def back_to_new_with_message(message, flash_type=:notice) + def back_to_new_with_message(message, flash_type = :notice) flash[flash_type] = message redirect_to(new_process_plate_path) end @@ -64,7 +62,7 @@ def back_to_new_with_error(message) # find out if the 'receive_plates' process was executed def receive_plates_process?(params) - @is_receive_plates_process ||= InstrumentProcess.find_by(id: params[:instrument_process]).key.eql?('slf_receive_plates') + @receive_plates_process ||= InstrumentProcess.find_by(id: params[:instrument_process]).key.eql?('slf_receive_plates') end # Call any external services - currently lighthouse service for plates from Lighthouse Labs and @@ -117,7 +115,7 @@ def parse_response(response, service) end def all_labware_created?(results) - return false if results.any?{ |_barcode, details| details[:success] == 'No' } + return false if results.any? { |_barcode, details| details[:success] == 'No' } true end diff --git a/lib/wrangler.rb b/lib/wrangler.rb index 2cdfd066..c46b7087 100644 --- a/lib/wrangler.rb +++ b/lib/wrangler.rb @@ -19,7 +19,7 @@ def self.call_api(barcodes) res = Net::HTTP.start(url.host, url.port) do |http| http.request(req) end - responses << { barcode: barcode, code: res.code, body: JSON.parse(res.body)} + responses << { barcode: barcode, code: res.code, body: JSON.parse(res.body) } end rescue StandardError => e Rails.logger.error(e) From 964c9622cc5c48054b3484c8233feba5d20a966f Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Thu, 18 Jun 2020 11:26:31 +0100 Subject: [PATCH 09/21] Pre-existing tests now pass --- config/environments/test.rb | 2 +- lib/wrangler.rb | 10 +++++++++- test/unit/lighthouse_test.rb | 14 +++++++------- test/unit/wrangler_test.rb | 14 +++++++------- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/config/environments/test.rb b/config/environments/test.rb index 7818eaac..25b92514 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -43,7 +43,7 @@ config.admin_email = 'example@example.com' # https://github.com/sanger/wrangler - config.wrangler_url = 'http://example.com/tube_rack' + config.wrangler_url = 'http://example.com/wrangle' # https://github.com/sanger/lighthouse config.lighthouse_host = 'http://127.0.0.1:5000' diff --git a/lib/wrangler.rb b/lib/wrangler.rb index c46b7087..f2034555 100644 --- a/lib/wrangler.rb +++ b/lib/wrangler.rb @@ -19,7 +19,8 @@ def self.call_api(barcodes) res = Net::HTTP.start(url.host, url.port) do |http| http.request(req) end - responses << { barcode: barcode, code: res.code, body: JSON.parse(res.body) } + + responses << { barcode: barcode, code: res.code, body: Wrangler.parse_body(res.body) } end rescue StandardError => e Rails.logger.error(e) @@ -30,4 +31,11 @@ def self.call_api(barcodes) end responses end + + def self.parse_body(body) + JSON.parse(body) + rescue StandardError + # return the body as is if not valid JSON + return body + end end diff --git a/test/unit/lighthouse_test.rb b/test/unit/lighthouse_test.rb index d4fc638e..12247fab 100644 --- a/test/unit/lighthouse_test.rb +++ b/test/unit/lighthouse_test.rb @@ -30,8 +30,8 @@ class LighthouseTest < ActiveSupport::TestCase stub_request(:any, uri_template).to_return(status: 201) assert_equal( - Lighthouse.call_api([@input_params[:source_plates]]), - [{ code: '201', body: '' }] + [{ barcode: 'barcode_one', code: '201', body: '' }], + Lighthouse.call_api([@input_params[:source_plates]]) ) end @@ -45,12 +45,12 @@ class LighthouseTest < ActiveSupport::TestCase stub_request(:any, uri_template).to_return(status: 201) assert_equal( - Lighthouse.call_api(barcodes), [ - { code: '201', body: '' }, - { code: '201', body: '' }, - { code: '201', body: '' } - ] + { barcode: 'barcode_one', code: '201', body: '' }, + { barcode: 'barcode_two', code: '201', body: '' }, + { barcode: 'barcode_three', code: '201', body: '' } + ], + Lighthouse.call_api(barcodes) ) end end diff --git a/test/unit/wrangler_test.rb b/test/unit/wrangler_test.rb index 24a377ba..0a5f487f 100644 --- a/test/unit/wrangler_test.rb +++ b/test/unit/wrangler_test.rb @@ -31,8 +31,8 @@ class WranglerTest < ActiveSupport::TestCase stub_request(:any, uri_template).to_return(status: 200) assert_equal( - Wrangler.call_api([@input_params[:source_plates]]), - [{ barcode: @barcode, code: '200' }] + [{ barcode: @barcode, code: '200', body: '' }], + Wrangler.call_api([@input_params[:source_plates]]) ) end @@ -46,12 +46,12 @@ class WranglerTest < ActiveSupport::TestCase stub_request(:any, uri_template).to_return(status: 200) assert_equal( - Wrangler.call_api(barcodes), [ - { barcode: 'barcode_one', code: '200' }, - { barcode: 'barcode_two', code: '200' }, - { barcode: 'barcode_three', code: '200' } - ] + { barcode: 'barcode_one', code: '200', body: '' }, + { barcode: 'barcode_two', code: '200', body: '' }, + { barcode: 'barcode_three', code: '200', body: '' } + ], + Wrangler.call_api(barcodes) ) end end From ae0ba1be67110f98adb40ae000c651f2d7b49997 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 19 Jun 2020 11:29:51 +0100 Subject: [PATCH 10/21] Null check lighthouse_responses and wrangler_responses variables - if all scanned barcodes were found in lighthouse, wrangler_responses won't exist - if lighthouse is down, lighthouse_responses won't exist - also add JSON parsing to lighthouse responses, as per wrangler --- app/controllers/process_plates_controller.rb | 12 ++++++++---- lib/lighthouse.rb | 9 ++++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index c2d65568..7765dd45 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -95,11 +95,15 @@ def generate_results(barcodes) barcodes.each { |b| output[b] = { success: 'No' } } # loop through service responses to update 'output' with successes - @lighthouse_responses.select { |r| r[:code] == '201' }.each do |r| - output[r[:barcode]] = parse_response(r, :Lighthouse) + if @lighthouse_responses + @lighthouse_responses.select { |r| r[:code] == '201' }.each do |r| + output[r[:barcode]] = parse_response(r, :Lighthouse) + end end - @wrangler_responses.select { |r| r[:code] == '201' }.each do |r| - output[r[:barcode]] = parse_response(r, :CGaP) + if @wrangler_responses + @wrangler_responses.select { |r| r[:code] == '201' }.each do |r| + output[r[:barcode]] = parse_response(r, :CGaP) + end end output diff --git a/lib/lighthouse.rb b/lib/lighthouse.rb index 528360f1..e62659c9 100644 --- a/lib/lighthouse.rb +++ b/lib/lighthouse.rb @@ -20,7 +20,7 @@ def self.call_api(barcodes) http.request(req) end - responses << { barcode: barcode, code: res.code, body: res.body } + responses << { barcode: barcode, code: res.code, body: Lighthouse.parse_body(res.body) } end rescue StandardError => e Rails.logger.error(e) @@ -30,4 +30,11 @@ def self.call_api(barcodes) end responses end + + def self.parse_body(body) + JSON.parse(body) + rescue StandardError + # return the body as is if not valid JSON + return body + end end From 348f47a8a2eecea7f0d8b3243a133c18ae7c9f06 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 19 Jun 2020 13:09:28 +0100 Subject: [PATCH 11/21] some rubocop fixes --- app/controllers/process_plates_controller.rb | 12 ++++-------- lib/lighthouse.rb | 2 +- lib/wrangler.rb | 2 +- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index 7765dd45..fce8e99f 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -95,15 +95,11 @@ def generate_results(barcodes) barcodes.each { |b| output[b] = { success: 'No' } } # loop through service responses to update 'output' with successes - if @lighthouse_responses - @lighthouse_responses.select { |r| r[:code] == '201' }.each do |r| - output[r[:barcode]] = parse_response(r, :Lighthouse) - end + @lighthouse_responses&.select { |r| r[:code] == '201' }.each do |r| + output[r[:barcode]] = parse_response(r, :Lighthouse) end - if @wrangler_responses - @wrangler_responses.select { |r| r[:code] == '201' }.each do |r| - output[r[:barcode]] = parse_response(r, :CGaP) - end + @wrangler_responses&.select { |r| r[:code] == '201' }.each do |r| + output[r[:barcode]] = parse_response(r, :CGaP) end output diff --git a/lib/lighthouse.rb b/lib/lighthouse.rb index e62659c9..cda71078 100644 --- a/lib/lighthouse.rb +++ b/lib/lighthouse.rb @@ -35,6 +35,6 @@ def self.parse_body(body) JSON.parse(body) rescue StandardError # return the body as is if not valid JSON - return body + body end end diff --git a/lib/wrangler.rb b/lib/wrangler.rb index f2034555..62359aa2 100644 --- a/lib/wrangler.rb +++ b/lib/wrangler.rb @@ -36,6 +36,6 @@ def self.parse_body(body) JSON.parse(body) rescue StandardError # return the body as is if not valid JSON - return body + body end end From 5499b9231481f163ea9c5e3cc36da1df543dc6d5 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 19 Jun 2020 15:15:01 +0100 Subject: [PATCH 12/21] write some new tests for this new functionality - refactor a little to make more testable --- app/controllers/process_plates_controller.rb | 18 ++++--- test/controllers/process_plates_controller.rb | 51 +++++++++++++++++++ test/test_helper.rb | 26 ++++++++++ 3 files changed, 88 insertions(+), 7 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index fce8e99f..c6de383f 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -38,9 +38,9 @@ def create barcodes = sanitize_barcodes(params[:source_plates]) back_to_new_with_error('No barcodes were provided') && return unless barcodes && !barcodes.empty? - call_external_services(barcodes) + responses = call_external_services(barcodes) - @results = generate_results(barcodes) + @results = generate_results(barcodes, responses) back_to_new_with_error('No response from services') && return if @results.empty? if all_labware_created?(@results) @@ -68,13 +68,16 @@ def receive_plates_process?(params) # Call any external services - currently lighthouse service for plates from Lighthouse Labs and # wrangler for tube racks. If no samples are found in the lighthouse service, try the wrangler def call_external_services(barcodes) + output = { lighthouse: [], wrangler: [] } # call the lighthouse service first as we are assuming that most labware scanned will be # plates from Lighthouse Labs - @lighthouse_responses = Lighthouse.call_api(barcodes) + output[:lighthouse] = Lighthouse.call_api(barcodes) # keeping it simple for now, if all the responses are not CREATED, send ALL the barcodes # to the wrangler - @wrangler_responses = Wrangler.call_api(barcodes) unless all_created?(@lighthouse_responses) + output[:wrangler] = Wrangler.call_api(barcodes) unless all_created?(output[:lighthouse]) + + output end # Returns a list of unique barcodes by removing blanks and duplicates @@ -89,16 +92,17 @@ def all_created?(responses) responses.all? { |response| response[:code] == "201" } end - def generate_results(barcodes) + def generate_results(barcodes, responses) output = {} # default 'success' for each barcode to 'No' barcodes.each { |b| output[b] = { success: 'No' } } # loop through service responses to update 'output' with successes - @lighthouse_responses&.select { |r| r[:code] == '201' }.each do |r| + # puts "DEBUG: responses: #{JSON.pretty_generate(responses)}" + responses[:lighthouse]&.select { |r| r[:code] == '201' }.each do |r| output[r[:barcode]] = parse_response(r, :Lighthouse) end - @wrangler_responses&.select { |r| r[:code] == '201' }.each do |r| + responses[:wrangler]&.select { |r| r[:code] == '201' }.each do |r| output[r[:barcode]] = parse_response(r, :CGaP) end diff --git a/test/controllers/process_plates_controller.rb b/test/controllers/process_plates_controller.rb index 26e81fbf..e78265a9 100644 --- a/test/controllers/process_plates_controller.rb +++ b/test/controllers/process_plates_controller.rb @@ -37,4 +37,55 @@ class ProcessPlatesControllerTest < ActiveSupport::TestCase ProcessPlatesController.new.call_external_services(%w[123 456]) end end + + context '#generate_results' do + should 'give failed results if no response' do + barcodes = ['AB1'] + + responses = { lighthouse: [], wrangler: [] } + + expected_results = { 'AB1' => { success: 'No' } } + + results = ProcessPlatesController.new.generate_results(barcodes, responses) + assert_equal(expected_results, results) + end + + + should 'give detailed results if success' do + barcodes = ['AB1', 'AB2', 'AB3'] + + responses = { + lighthouse: [ + generate_success_response('AB1', 'Purpose 1', ['Study 1', 'Study 2']), + generate_success_response('AB2', 'Purpose 1', ['Study 1']), + generate_fail_response('AB3') ], + wrangler: [ + generate_fail_response('AB1'), + generate_fail_response('AB2'), + generate_success_response('AB3', 'Purpose 1', ['Study 1', 'Study 2']) ]} + + expected_results = { + 'AB1' => { + success: 'Yes', + source: :Lighthouse, + purpose: 'Purpose 1', + study: 'Study 1, Study 2' + }, + 'AB2' => { + success: 'Yes', + source: :Lighthouse, + purpose: 'Purpose 1', + study: 'Study 1' + }, + 'AB3' => { + success: 'Yes', + source: :CGaP, + purpose: 'Purpose 1', + study: 'Study 1, Study 2' + } + } + results = ProcessPlatesController.new.generate_results(barcodes, responses) + assert_equal(expected_results, results) + end + end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 9bae8e1a..94b76c02 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -14,4 +14,30 @@ class ActiveSupport::TestCase # fixtures :all # Add more helper methods to be used by all tests here... + def generate_success_response(barcode, purpose_name, study_names) + { + barcode: barcode, + code: '201', + body: { + 'data' => { + 'attributes'=> { + 'purpose_name' => purpose_name, + 'study_names' => study_names + } + } + } + } + end + + def generate_fail_response(barcode) + { + barcode: barcode, + code: '400', + body: { + 'errors': [ + 'No samples for this barcode' + ] + } + } + end end From a72038ebfadc424f890bcba861f3678ca01f059c Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 19 Jun 2020 15:17:19 +0100 Subject: [PATCH 13/21] rubocop formatting --- test/controllers/process_plates_controller.rb | 10 ++++++---- test/test_helper.rb | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/test/controllers/process_plates_controller.rb b/test/controllers/process_plates_controller.rb index e78265a9..fcbfbd1d 100644 --- a/test/controllers/process_plates_controller.rb +++ b/test/controllers/process_plates_controller.rb @@ -50,19 +50,21 @@ class ProcessPlatesControllerTest < ActiveSupport::TestCase assert_equal(expected_results, results) end - should 'give detailed results if success' do - barcodes = ['AB1', 'AB2', 'AB3'] + barcodes = %w[AB1 AB2 AB3] responses = { lighthouse: [ generate_success_response('AB1', 'Purpose 1', ['Study 1', 'Study 2']), generate_success_response('AB2', 'Purpose 1', ['Study 1']), - generate_fail_response('AB3') ], + generate_fail_response('AB3') + ], wrangler: [ generate_fail_response('AB1'), generate_fail_response('AB2'), - generate_success_response('AB3', 'Purpose 1', ['Study 1', 'Study 2']) ]} + generate_success_response('AB3', 'Purpose 1', ['Study 1', 'Study 2']) + ] + } expected_results = { 'AB1' => { diff --git a/test/test_helper.rb b/test/test_helper.rb index 94b76c02..5c0cb058 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -20,7 +20,7 @@ def generate_success_response(barcode, purpose_name, study_names) code: '201', body: { 'data' => { - 'attributes'=> { + 'attributes' => { 'purpose_name' => purpose_name, 'study_names' => study_names } From 51befa35da35404428fed4f0e046a10460fc23dd Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 19 Jun 2020 15:53:47 +0100 Subject: [PATCH 14/21] refactor to reduce complexity of 'create' method for rubocop --- app/controllers/process_plates_controller.rb | 42 ++++++++++---------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index c6de383f..47932312 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -16,39 +16,35 @@ def new end def create - bed_verification_model = InstrumentProcessesInstrument.get_bed_verification_type( - params[:instrument_barcode], - params[:instrument_process] - ) - back_to_new_with_error('Invalid instrument or process') && return if bed_verification_model.nil? + bed_verification_model = InstrumentProcessesInstrument.get_bed_verification_type(params[:instrument_barcode], params[:instrument_process]) + raise 'Invalid instrument or process' if bed_verification_model.nil? bed_layout_verification = bed_verification_model.new( instrument_barcode: params[:instrument_barcode], scanned_values: params[:robot], api: api ) - unless bed_layout_verification.validate_and_create_audits?(params) - errors = bed_layout_verification.errors.values.flatten.join("\n") - back_to_new_with_error(errors) && return - end + raise format_errors(bed_layout_verification) unless bed_layout_verification.validate_and_create_audits?(params) back_to_new_with_message('Success') && return unless receive_plates_process?(params) + # here on is relevant to 'receiving plates' only # the param is called 'source_plates' but we could be working with tube racks or plates etc. barcodes = sanitize_barcodes(params[:source_plates]) - back_to_new_with_error('No barcodes were provided') && return unless barcodes && !barcodes.empty? + raise 'No barcodes were provided' if barcodes.empty? responses = call_external_services(barcodes) - @results = generate_results(barcodes, responses) - back_to_new_with_error('No response from services') && return if @results.empty? - if all_labware_created?(@results) - flash[:notice] = 'All labware were successfully created.' - else - flash[:error] = 'Some labware were not able to be created.' - end + display_flash_message(@results) render :results + rescue StandardError => e + flash[:error] = e.message + redirect_to(new_process_plate_path) + end + + def format_errors(obj) + obj.errors.values.flatten.join("\n") end def back_to_new_with_message(message, flash_type = :notice) @@ -56,10 +52,6 @@ def back_to_new_with_message(message, flash_type = :notice) redirect_to(new_process_plate_path) end - def back_to_new_with_error(message) - back_to_new_with_message(message, :error) - end - # find out if the 'receive_plates' process was executed def receive_plates_process?(params) @receive_plates_process ||= InstrumentProcess.find_by(id: params[:instrument_process]).key.eql?('slf_receive_plates') @@ -123,4 +115,12 @@ def all_labware_created?(results) true end + + def display_flash_message(results) + if all_labware_created?(results) + flash[:notice] = 'All labware were successfully created.' + else + flash[:error] = 'Some labware were not able to be created.' + end + end end From f97a98ec1bd03a5872cb9ae88d4f2d45ddca5a26 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Fri, 19 Jun 2020 16:18:44 +0100 Subject: [PATCH 15/21] Change it so the results table is less weird if they are scanning plates in that already exist in Sequencescape - it will not show an error message, and the column heading is 'Imported?' rather than 'Success?' - we forgot this is the 'usual' way of receiving plates, as opposed to the heron function which is an import --- app/controllers/process_plates_controller.rb | 9 --------- app/views/process_plates/results.html.haml | 3 ++- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index 47932312..d157149c 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -36,7 +36,6 @@ def create responses = call_external_services(barcodes) @results = generate_results(barcodes, responses) - display_flash_message(@results) render :results rescue StandardError => e flash[:error] = e.message @@ -115,12 +114,4 @@ def all_labware_created?(results) true end - - def display_flash_message(results) - if all_labware_created?(results) - flash[:notice] = 'All labware were successfully created.' - else - flash[:error] = 'Some labware were not able to be created.' - end - end end diff --git a/app/views/process_plates/results.html.haml b/app/views/process_plates/results.html.haml index 8b4e97fd..089b313b 100644 --- a/app/views/process_plates/results.html.haml +++ b/app/views/process_plates/results.html.haml @@ -1,10 +1,11 @@ %h2 Activity Logging +%h3 Labware import results %table %thead %tr %th Barcode - %th Success? + %th Imported? %th Source %th Purpose %th Study From e05e58fc7566472bf2519e520058707716187e53 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 22 Jun 2020 10:13:04 +0100 Subject: [PATCH 16/21] rubocop --- app/controllers/process_plates_controller.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index b5c19220..6c4f9efe 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -55,9 +55,6 @@ def receive_plates_process?(params) @receive_plates_process ||= InstrumentProcess.find_by(id: params[:instrument_process]).key.eql?('slf_receive_plates') end - # rubocop:todo Lint/UselessAssignment - # Disabling Lint/UselessAssignment here as I know KT and AS are both actively working on - # displaying some of this information to the user. So don't want to create merge issues by removing it. # Call any external services - currently lighthouse service for plates from Lighthouse Labs and # wrangler for tube racks. If no samples are found in the lighthouse service, try the wrangler def call_external_services(barcodes) @@ -72,7 +69,6 @@ def call_external_services(barcodes) output end - # rubocop:enable Lint/UselessAssignment # Returns a list of unique barcodes by removing blanks and duplicates def sanitize_barcodes(barcodes) @@ -93,10 +89,10 @@ def generate_results(barcodes, responses) # loop through service responses to update 'output' with successes # puts "DEBUG: responses: #{JSON.pretty_generate(responses)}" - responses[:lighthouse]&.select { |r| r[:code] == '201' }.each do |r| + responses[:lighthouse]&.select { |r| r[:code] == '201' }&.each do |r| output[r[:barcode]] = parse_response(r, :Lighthouse) end - responses[:wrangler]&.select { |r| r[:code] == '201' }.each do |r| + responses[:wrangler]&.select { |r| r[:code] == '201' }&.each do |r| output[r[:barcode]] = parse_response(r, :CGaP) end From 73c12b32ec1d3e1435156c110a64dc2d1e7b9bdb Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 22 Jun 2020 10:17:15 +0100 Subject: [PATCH 17/21] Put barcode count message back in - shouldn't really lose this feature. --- app/controllers/process_plates_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/process_plates_controller.rb b/app/controllers/process_plates_controller.rb index 6c4f9efe..811faa30 100644 --- a/app/controllers/process_plates_controller.rb +++ b/app/controllers/process_plates_controller.rb @@ -35,6 +35,7 @@ def create responses = call_external_services(barcodes) @results = generate_results(barcodes, responses) + flash[:notice] = "Scanned #{bed_layout_verification.process_plate&.num_unique_barcodes} barcodes." render :results rescue StandardError => e flash[:error] = e.message From 78e149b9339bfe45c4a17f3480b8a668827e3798 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 22 Jun 2020 10:19:46 +0100 Subject: [PATCH 18/21] Got this error when trying to start the server: "DEPRECATION WARNING: Using `Rails::Application` subclass to start the server is deprecated and will be removed in Rails 6.0. Please change `run ProcessTracking::Application` to `run Rails.application` in config.ru. (called from require at bin/rails:5) (ActiveSupport::DeprecationException)" --- config.ru | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.ru b/config.ru index 93277a0e..3476455e 100644 --- a/config.ru +++ b/config.ru @@ -2,4 +2,4 @@ # This file is used by Rack-based servers to start the application. require ::File.expand_path('../config/environment', __FILE__) -run ProcessTracking::Application +run Rails.application From 39e5cee45fbc901782d14ba9561fa4fc64bcbc53 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 22 Jun 2020 10:22:23 +0100 Subject: [PATCH 19/21] need to include 'skip_pipeline' for the static assets in the public folder --- app/views/layouts/application.html.erb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 7732a292..887281ec 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -2,9 +2,10 @@ <%= ProcessTracking::Application.config.name %> - <%= stylesheet_link_tag(:flash,:formtastic_static,:formtastic_changes,:application) %> + <%= stylesheet_link_tag(:application) %> - <%= javascript_include_tag('jquery.min.js','rails.js','application.js', skip_pipeline: true ) %> + <%= stylesheet_link_tag(:flash,:formtastic_static,:formtastic_changes, skip_pipeline: true) %> + <%= javascript_include_tag('jquery.min.js','rails.js','application.js', skip_pipeline: true) %> <%= csrf_meta_tag %> From 1f5894bd51cae9d25d125786aace5dff9c6d1ff2 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 22 Jun 2020 10:37:58 +0100 Subject: [PATCH 20/21] put all the css back in the static folder, because I couldn't get it to work using the asset pipeline --- app/assets/stylesheets/application.css | 1 - app/views/layouts/application.html.erb | 5 ++--- {app/assets => public}/stylesheets/form_custom.css | 0 3 files changed, 2 insertions(+), 4 deletions(-) delete mode 100644 app/assets/stylesheets/application.css rename {app/assets => public}/stylesheets/form_custom.css (100%) diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css deleted file mode 100644 index d3a82b1f..00000000 --- a/app/assets/stylesheets/application.css +++ /dev/null @@ -1 +0,0 @@ -@import "form_custom"; \ No newline at end of file diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 887281ec..52ff6ca1 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -2,9 +2,8 @@ <%= ProcessTracking::Application.config.name %> - <%= stylesheet_link_tag(:application) %> - - <%= stylesheet_link_tag(:flash,:formtastic_static,:formtastic_changes, skip_pipeline: true) %> + + <%= stylesheet_link_tag(:flash, :form_custom, :formtastic_static, :formtastic_changes, skip_pipeline: true) %> <%= javascript_include_tag('jquery.min.js','rails.js','application.js', skip_pipeline: true) %> <%= csrf_meta_tag %> diff --git a/app/assets/stylesheets/form_custom.css b/public/stylesheets/form_custom.css similarity index 100% rename from app/assets/stylesheets/form_custom.css rename to public/stylesheets/form_custom.css From 4539f0e24d006aec1b66db47e736ac12f6c61136 Mon Sep 17 00:00:00 2001 From: Katy Taylor Date: Mon, 22 Jun 2020 10:38:26 +0100 Subject: [PATCH 21/21] fix typo --- app/views/layouts/application.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 52ff6ca1..e27974e7 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -2,7 +2,7 @@ <%= ProcessTracking::Application.config.name %> - + <%= stylesheet_link_tag(:flash, :form_custom, :formtastic_static, :formtastic_changes, skip_pipeline: true) %> <%= javascript_include_tag('jquery.min.js','rails.js','application.js', skip_pipeline: true) %>