From 4f5fd096026bbbb5e9f24894d94a44d8d08cae6f Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Thu, 20 Jul 2023 12:39:22 +0200 Subject: [PATCH 1/7] Fix: Box CSV validation fails when CSV has whitespaces --- .../components/upload_csv_box.js.jsx | 29 ++++------- app/controllers/boxes_controller.rb | 29 ++++++++++- app/controllers/csv_validations_controller.rb | 36 ------------- app/models/box_form.rb | 37 ++++++------- app/views/boxes/_form.haml | 4 +- app/views/boxes/_upload_csv_box.haml | 48 ----------------- app/views/boxes/validate.json.jbuilder | 4 ++ config/routes.rb | 9 ++-- spec/controllers/boxes_controller_spec.rb | 52 +++++++++++++++++++ spec/support/blueprints.rb | 18 +++---- 10 files changed, 128 insertions(+), 138 deletions(-) delete mode 100644 app/controllers/csv_validations_controller.rb delete mode 100644 app/views/boxes/_upload_csv_box.haml create mode 100644 app/views/boxes/validate.json.jbuilder diff --git a/app/assets/javascripts/components/upload_csv_box.js.jsx b/app/assets/javascripts/components/upload_csv_box.js.jsx index a0637255a..d672a1508 100644 --- a/app/assets/javascripts/components/upload_csv_box.js.jsx +++ b/app/assets/javascripts/components/upload_csv_box.js.jsx @@ -2,7 +2,7 @@ var UploadCsvBox = React.createClass({ getInitialState: function() { return { csrfToken: this.props.csrf_token, - url: "/csv_validation/" + this.props.context, + url: this.props.validate_url, fieldName: this.props.name, // Initial fieldName value uploadRows: [], // Array to store upload rows hideListItems: "hidden", @@ -16,7 +16,7 @@ var UploadCsvBox = React.createClass({ this.setState({ fileValue: event.target.value }); const file = event.target.files[0]; const formData = new FormData(); - formData.append('csv_file', file); + formData.append('csv_box', file); fetch(this.state.url, { method: "POST", @@ -33,25 +33,19 @@ var UploadCsvBox = React.createClass({ } }) .then(responseJson => { - const not_found_batches = responseJson.not_found_batches; - const samples_nbr = responseJson.samples_nbr; - const error_message = responseJson.error_message; - // Create row from template with filename and upload info - const filename = file.name; - const uploadInfo = { - uploadedSamplesCount: samples_nbr, - notFoundUuids: not_found_batches, - errorMessage: error_message + const uploadInfo = { + uploadedSamplesCount: responseJson.samples_count, + notFoundUuids: responseJson.not_found_batches, + errorMessage: responseJson.error_message, }; // Add the new row to the state this.setState(prevState => ({ - uploadRows: [...prevState.uploadRows, {filename, uploadInfo, showTooltip: false}] + uploadRows: [...prevState.uploadRows, {filename: file.name, uploadInfo, showTooltip: false}] })); this.setState({ hideListItems: "" }); - }) .catch(error => { this.setState({ errorMessage: error }); @@ -97,7 +91,7 @@ var UploadCsvBox = React.createClass({ const tooltipText = notFoundUuids.slice(0, 5).map((batch_number) =>
{batch_number}
); - const rowContent = errorMessage == "" ? + const rowContent = errorMessage ?
{uploadedSamplesCount} {samplesText} {batchesNotFound > 0 && ( @@ -110,17 +104,16 @@ var UploadCsvBox = React.createClass({ {errorMessage}
; - return (
this.handleRemove(index)}>
{filename}
-
0 ? ' ttip ' : ' '} ${batchesNotFound > 0 || errorMessage != "" ? ' input-required ' : ' '}}`} +
0 ? ' ttip ' : ' '} ${batchesNotFound > 0 || errorMessage ? ' input-required ' : ' '}}`} onClick={() => this.handleClick(index)}> - {rowContent} -
0 || errorMessage != "" ? 'icon-alert icon-red' : 'icon-check'}`}>
+ {rowContent} +
0 || errorMessage ? 'icon-alert icon-red' : 'icon-check'}`}>
{batchesNotFound > 0 && (
{tooltipText} diff --git a/app/controllers/boxes_controller.rb b/app/controllers/boxes_controller.rb index 6b7d2d350..63c268f87 100644 --- a/app/controllers/boxes_controller.rb +++ b/app/controllers/boxes_controller.rb @@ -1,5 +1,5 @@ class BoxesController < ApplicationController - before_action :load_box, except: %i[index new create bulk_destroy] + before_action :load_box, except: %i[index new validate create bulk_destroy] def index @can_create = has_access?(@navigation_context.institution, CREATE_INSTITUTION_BOX) @@ -51,6 +51,33 @@ def new @box_form = BoxForm.build(@navigation_context) end + def validate + @samples_count = 0 + batch_numbers = Set.new + + # TODO: should be handled by BoxForm (& duplicates BoxForm#parse_csv) + CSV.open(params[:csv_box].path, headers: true) do |csv| + while csv.headers == true + csv.readline + end + + unless csv.headers == ["Batch", "Concentration", "Distractor", "Instructions"] + @error_message = "Invalid columns" + return # rubocop:disable Lint/NonLocalExitFromIterator + end + + csv.each do |row| + if batch_number = row["Batch"].presence&.strip + batch_numbers << batch_number + @samples_count += 1 + end + end + end + + @found_batches = @navigation_context.institution.batches.where(batch_number: batch_numbers.to_a).pluck(:batch_number) + @not_found_batches = (batch_numbers - @found_batches) + end + def create return unless authorize_resource(@navigation_context.institution, CREATE_INSTITUTION_BOX) diff --git a/app/controllers/csv_validations_controller.rb b/app/controllers/csv_validations_controller.rb deleted file mode 100644 index de54201d8..000000000 --- a/app/controllers/csv_validations_controller.rb +++ /dev/null @@ -1,36 +0,0 @@ -class CsvValidationsController < ApplicationController - def create - uploaded_file = params[:csv_file] - if uploaded_file.content_type == "text/csv" - csv_text = uploaded_file.read - institution = @navigation_context.institution - csv_table = CSV.parse(csv_text, headers: true) - - unless csv_table.headers == ["Batch", "Concentration", "Distractor", "Instructions"] - render_invalid_format - return - end - - batch_values = csv_table.map { |row| row["Batch"] } - found_batches = institution.batches.where(batch_number: batch_values).pluck(:batch_number) - not_found_batches = (batch_values - found_batches).uniq - - render json: { found_batches: found_batches, - not_found_batches: not_found_batches, - samples_nbr: csv_table.size, - error_message: "" } - else - render_invalid_format - end - end - - private - - def render_invalid_format - render json: { found_batches: [], - not_found_batches: [], - samples_nbr: 0, - error_message: "Invalid file format." } - end - -end diff --git a/app/models/box_form.rb b/app/models/box_form.rb index 2eafc0f30..d2e524ce0 100644 --- a/app/models/box_form.rb +++ b/app/models/box_form.rb @@ -22,8 +22,9 @@ def initialize(box, params) @option = params[:option] @media = params[:media].presence @batches_data = params[:batches].presence.to_h - @csv_box = params[:csv_box].presence - initialize_csv_box if @csv_box + if uploaded_file = params[:csv_box].presence + parse_csv(uploaded_file.path) + end @batch_uuids = @batches_data.transform_values { |v| v[:batch_uuid] } @sample_uuids = params[:sample_uuids].presence.to_h end @@ -204,23 +205,23 @@ def have_distractor_sample? @samples.any? { |_, sample| sample.distractor } end - def initialize_csv_box - CSV.open(@csv_box.path) do |csv_stream| - i = 0 - csv_stream.each do |row| - batch_number, concentration, distractor, instruction = row[0..3] - batch_uuid = Batch.find_by(batch_number: batch_number)&.uuid - next if batch_uuid.blank? - @batches_data[i] = { - batch_uuid: batch_uuid, - distractor: distractor.downcase == "yes", - instruction: instruction, - concentrations: {"0" => { - replicate: 1, - concentration: Integer(Float(concentration)), - }}, + def parse_csv(path) + CSV.open(path, headers: true) do |csv| + csv.each do |row| + next unless batch_number = row["Batch"]&.strip.presence + next unless batch = Batch.find_by(batch_number: batch_number) + + @batches_data[@batches_data.size] = { + batch_uuid: batch.uuid, + distractor: row["Distractor"]&.strip&.downcase == "yes", + instruction: row["Instructions"], + concentrations: { + "0" => { + replicate: 1, + concentration: Integer(Float(row["Concentration"]&.strip)), + }, + }, } - i += 1 end end end diff --git a/app/views/boxes/_form.haml b/app/views/boxes/_form.haml index 6d0f7b626..18593bf7c 100644 --- a/app/views/boxes/_form.haml +++ b/app/views/boxes/_form.haml @@ -42,7 +42,7 @@ .row .col - = react_component "UploadCsvBox", { name: "box[csv_box]", context: @navigation_context.uuid, csrf_token: form_authenticity_token} + = react_component "UploadCsvBox", { name: "box[csv_box]", validate_url: validate_boxes_url(context: @navigation_context.institution.uuid + "-*"), csrf_token: form_authenticity_token} .row =link_to '/templates/upload_box.csv', class: 'btn-link', target: "_blank" do .icon-download.icon-gray @@ -74,8 +74,6 @@ showCloseButton: false, message: "If you change the sample source, the box contents will be emptied. ") -= render 'upload_csv_box' - :javascript var sourcesRadio = document.querySelectorAll(".radiotoggle input"); var oldSource = null; diff --git a/app/views/boxes/_upload_csv_box.haml b/app/views/boxes/_upload_csv_box.haml deleted file mode 100644 index 8c6edb2ac..000000000 --- a/app/views/boxes/_upload_csv_box.haml +++ /dev/null @@ -1,48 +0,0 @@ - -%template{:id => "csvFileRow"} - .list-items.upload_info - .items-row - .items-item.gap-5 - .icon-circle-minus.icon-gray.remove_file - .file-name - .items-row-action.gap-5.not_found_message - .uploaded-samples-count - .upload-icon.bigger - .ttext.not-found-uuids.hidden - -%template{:id => "csvInputFile"} - = file_field_tag "box[csv_box]", hidden: true, class: "csv_file", accept: "text/csv" - -:javascript - var getUrlParameter = function getUrlParameter(sParam) { - var sPageURL = window.location.search.substring(1), - sURLVariables = sPageURL.split('&'), - sParameterName, - i; - - for (i = 0; i < sURLVariables.length; i++) { - sParameterName = sURLVariables[i].split('='); - - if (sParameterName[0] === sParam) { - return sParameterName[1] === undefined ? true : decodeURIComponent(sParameterName[1]); - } - } - return false; - }; - - var context = getUrlParameter('context'); - var isUUID = function(str) { - const regexExp = /^[0-9a-fA-F]{8}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{4}\b-[0-9a-fA-F]{12}$/gi; - return regexExp.test(str); - }; - - var showToolTip = function(evt) { - var tooltip = evt.currentTarget.tooltip - if (tooltip.classList.contains("hidden")) { - tooltip.classList.remove("hidden") - setTimeout(() => tooltip.classList.add("hidden"), 5000) - } - else { - tooltip.classList.toggle("hidden") - } - } diff --git a/app/views/boxes/validate.json.jbuilder b/app/views/boxes/validate.json.jbuilder new file mode 100644 index 000000000..a97c84fd2 --- /dev/null +++ b/app/views/boxes/validate.json.jbuilder @@ -0,0 +1,4 @@ +json.found_batches @found_batches || [] +json.not_found_batches @not_found_batches || [] +json.samples_count @samples_count +json.error_message @error_message if @error_message diff --git a/config/routes.rb b/config/routes.rb index b381474a7..b8d8938be 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -166,16 +166,15 @@ member do post :unblind post :blind - get 'print' - get 'inventory', constraints: { format: 'csv' } + get :print + get :inventory, constraints: { format: 'csv' } end collection do - post 'bulk_action', constraints: lambda { |request| request.params[:bulk_action] == 'destroy' }, action: :bulk_destroy + post :validate + post :bulk_action, constraints: lambda { |request| request.params[:bulk_action] == 'destroy' }, action: :bulk_destroy end end - post '/csv_validation/:context', to: 'csv_validations#create', as: 'csv_validation' - resources :qc_infos resources :test_results , only: [:index, :show] resources :filters, format: 'html' diff --git a/spec/controllers/boxes_controller_spec.rb b/spec/controllers/boxes_controller_spec.rb index 7aee7f6fe..6c54f9037 100644 --- a/spec/controllers/boxes_controller_spec.rb +++ b/spec/controllers/boxes_controller_spec.rb @@ -260,6 +260,58 @@ end end + describe "validate (CSV)" do + let!(:batch) do + Batch.make!(institution: institution, site: site, batch_number: "DISTRACTOR") + end + + it "validates CSV headers" do + csv_file = fixture_file_upload(Rails.root.join("spec/fixtures/csvs/samples_results_1.csv"), "text/csv") + + expect do + post :validate, params: { csv_box: csv_file }, format: "json" + expect(response).to have_http_status(:ok) + end.to change(institution.boxes, :count).by(0) + + expect(JSON.parse(response.body)).to eq({ + "found_batches" => [], + "not_found_batches" => [], + "samples_count" => 0, + "error_message" => "Invalid columns" + }) + end + + it "finds all batches" do + csv_file = fixture_file_upload(Rails.root.join("spec/fixtures/csvs/csv_box_1.csv"), "text/csv") + + expect do + post :validate, params: { csv_box: csv_file }, format: "json" + expect(response).to have_http_status(:ok) + end.to change(institution.boxes, :count).by(0) + + expect(JSON.parse(response.body)).to eq({ + "found_batches" => ["DISTRACTOR"], + "not_found_batches" => [], + "samples_count" => 3, + }) + end + + it "fails to find some batches" do + csv_file = fixture_file_upload(Rails.root.join("spec/fixtures/csvs/csv_box_2.csv"), "text/csv") + + expect do + post :validate, params: { csv_box: csv_file }, format: "json" + expect(response).to have_http_status(:ok) + end.to change(institution.boxes, :count).by(0) + + expect(JSON.parse(response.body)).to eq({ + "found_batches" => ["DISTRACTOR"], + "not_found_batches" => ["VIRUS"], + "samples_count" => 5, + }) + end + end + describe "create" do let :batch do Batch.make!(institution: institution, site: site) diff --git a/spec/support/blueprints.rb b/spec/support/blueprints.rb index 5c69219fc..53d6724a9 100644 --- a/spec/support/blueprints.rb +++ b/spec/support/blueprints.rb @@ -271,15 +271,15 @@ def first_or_make_site_unless_manufacturer(institution) @batch_two = Batch.make! samples { [ - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 30, replicate: 1), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 200, replicate: 2), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 1000, replicate: 3), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 30, replicate: 4), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 200, replicate: 5), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 1000, replicate: 6), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 30, replicate: 7), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 200, replicate: 8), - Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 1000, replicate: 9), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 30, replicate: 1, distractor: true), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 200, replicate: 2, distractor: false), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 1000, replicate: 3, distractor: true), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 30, replicate: 4, distractor: false), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 200, replicate: 5, distractor: true), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 1000, replicate: 6, distractor: false), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 30, replicate: 7, distractor: true), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_two, concentration: 200, replicate: 8, distractor: false), + Sample.make(:filled, box: object, institution: object.institution, site: object.site, batch: @batch_one, concentration: 1000, replicate: 9, distractor: true), ] } end From d1f761da8682d546065b39ab269dd39e08dbe5ba Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Thu, 20 Jul 2023 14:43:48 +0200 Subject: [PATCH 2/7] Fix: alignment of sample/batch uuids in edit sample form --- app/views/samples/_form.haml | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/app/views/samples/_form.haml b/app/views/samples/_form.haml index f709f0005..38c000549 100644 --- a/app/views/samples/_form.haml +++ b/app/views/samples/_form.haml @@ -8,20 +8,22 @@ = "VIEW QC INFO" = f.form_field :uuid do - %span.copy-content - = f.object.uuid - %button.btn-copy.ttip - = icon_tag "copy", class: "btn-icon" + .value + %span.copy-content + = f.object.uuid + %button.btn-copy.ttip + = icon_tag "copy", class: "btn-icon" - if f.object.batch_number = f.form_field :batch_id, value: @sample_form.batch_number - if box = @sample_form.box = f.fields_for :box, box do |g| = g.form_field :uuid, label: { text: "Box ID" } do - %span.copy-content - = g.object.uuid - %button.btn-copy.ttip - = icon_tag "copy", class: "btn-icon" + .value + %span.copy-content + = g.object.uuid + %button.btn-copy.ttip + = icon_tag "copy", class: "btn-icon" = g.form_field :purpose, value: box.purpose, label: { text: "Box Purpose" } .col.pe-5 From e391535d971293a828dbcfe7261e59fb99e4aceb Mon Sep 17 00:00:00 2001 From: Julien Portalier Date: Thu, 20 Jul 2023 15:02:28 +0200 Subject: [PATCH 3/7] Fix: larger width for autocomplete text fields closes #1963 --- .../components/cdx_select_autocomplete.js.jsx | 10 ++++++++-- app/helpers/autocomplete_field_helper.rb | 16 +++++++++------- app/views/batches/_form.haml | 15 ++++++++++----- app/views/samples/_form.haml | 15 ++++++++++----- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/components/cdx_select_autocomplete.js.jsx b/app/assets/javascripts/components/cdx_select_autocomplete.js.jsx index e94b34390..b4fa8ec9a 100644 --- a/app/assets/javascripts/components/cdx_select_autocomplete.js.jsx +++ b/app/assets/javascripts/components/cdx_select_autocomplete.js.jsx @@ -1,6 +1,8 @@ var CdxSelectAutocomplete = React.createClass({ - componentWillMount: function () { - this.asyncOptions = _.debounce(this.asyncOptions.bind(this), 100, { maxWait: 250 }); + getDefaultProps: function() { + return { + className: "input-large" + }; }, getInitialState: function () { @@ -9,6 +11,10 @@ var CdxSelectAutocomplete = React.createClass({ }; }, + componentWillMount: function () { + this.asyncOptions = _.debounce(this.asyncOptions.bind(this), 100, { maxWait: 250 }); + }, + // Since