Feature/1814 upload csv with new box

[leandroradusky]

Closes #1814

Now a box can be created from a CSV file. To agree with the excel files that they use in order to define box samples concentrations, they should look like this.

This is the file that you can download as a template file when you choose the new option in the boxes form:

If the file is correct, it will respect the mockups done for the sample's upload results page:

If, within the file, some rows have inexistent batches, they will be informed:

While this message will be shown if no samples have been parsed (because they are not, or because the file has a wrong file format, etc):

Comments on the code:

• The way the upload works is, within box_form, the file is parsed and the same parameters as if the user has chosen the option Create new samples options are fulfilled and saved (initialize_csv_box method)
• The upload CSV is done with templates and plain JS, as for the upload samples results. Should be good to generalize, or, maybe better, create a React component that does this.
• On Capybara, only the creation of boxes is tested, while the functioning of the validations for this new add_csv option is tested on the controller (as previously discussed).

[leandroradusky]

I need further help with this (cc @ysbaddaden)

In Capybara, even if being able to attach the file (adding the template element that contains the file input) to the input element, it does not appear as a param in the box creation, these are the params on create in the controller of boxes:

{"utf8"=>"✓", "box"=>{"purpose"=>"LOD", "media"=>"Culture media", "option"=>"add_csv", "blinded"=>"0"}, "commit"=>"Save", "context"=>"d90409a6-3997-c33a-958c-55a3e62b64a5", "controller"=>"boxes", "action"=>"create"}

Some lead (that could be not the good one) to make the template to be included, I have to click the Add file button that will make the dialog appear, but I can't do nothing in capybara with this dialog. So maybe this is keeping the file input empty?

[diegoliberman]

If this gets complicated, we'll leave it on a pause.

[leandroradusky]

It's just the Capybara tests, besides that works well (and the controller its tested).

[ysbaddaden]

@leandroradusky what about uploading the CSV & validate it on the server and let it render the HTML to inject into the page? That would help rid of most of the JS.

[leandroradusky]

Hi @ysbaddaden (cc @diegoliberman)

IMO these are important validations to be done in client side:

There is at least these ways to uniquely identify batches on their mind:

• batch number (which is not a number)
• the batch uuid (which is "more like a number")
• the taxonomy id (which is a number.othernumber.athirdone)
• the who_label (really similar to the batch number, but not the same)
• the pango_lineage (really similar to the batch number, but not the same)

for me in this case is quite important to inform the batches which were not found to guide them (the actual validations don't do that)

Also, this file is made to be as similar as possible to the one that they use to compute the concentrations:

• which is an XLS (high chance of uploading the bad format even if they upload in CSV)
• with more columns / different columns / different order

That's why I added the "not samples found" validation below.

[ysbaddaden]

@leandroradusky I'm suggesting that we do all these validations but on the backend transparently (in the background, no page reload).

You select a CSV file to upload. JS will display a "validating (spinner)" message, upload the CSV to /boxes/validate_csv (for example), validate that the CSV is as expected, and eventually render the validation as the response body, that the JS can merely inject into the page.

You currently have to ask the backend if each batch exists, which results in N requests, but with the above approach we'd only have one request and avoid most of the JS, while still doing all the validation (win-win-win).

[leandroradusky]

Great! I didn't get it before, its a great hint thanks <3

[diegoliberman]

@ysbaddaden @leandroradusky could it be possible to merge this one as is, and create a new issue for doing the validations in the backend?

[ysbaddaden]

@diegoliberman Is the feature that important to have now? I'd prefer to avoid pushing tech-debt if possible.

[diegoliberman]

@ysbaddaden if the budget gets approved 2 months from now (we don't know), then the users will be very thankful to have the feature already available during this time.

[ysbaddaden]

We can probably live with the JS, but the handleFileSelect is too big and nests too many closures.

I believe we can skip the "add files" button and always render the upload CSV file input instead. We could then simplify a bunch of things in the JS, extract methods out of the handleFileSelect callback.

For example:

• extract the part that parses & validates the CSV into functions (pass it load_event.target.result and let it return samples, found_batches & not_found_batches);
• extract the rendering of the row template;
• make removeFileRow global (not closured);
• since we're only rendering one row: we can assume the DOM elements to be there, and don't need to pass references anymore;
• ...

[bolom]

Voila. I've move the validation in backend. Let me know what do you think @leandroradusky @ysbaddaden

[leandroradusky]

Despite the integration tests to be fixed, I've tested the functionality locally to demo it with UCSD and it works as expected :)

[bolom]

hey @leandroradusky @ysbaddaden
I have removed the unnecessary JavaScript validation and related features from the codebase.

In addition, I've made the Box#create method more consistent and concise. I removed the redundant valid? check for BoxForm, as the BoxForm.save method already performs the necessary validations.

What do you think?

[ysbaddaden]

I pushed a couple commits to fix the broken spec suites (unit & integration). I explained why they broke as comments to this review.

[ysbaddaden]

Why make one radio button required out of the set?

[ysbaddaden]

@bolom yes, I fixed it ysterday as @leandroradusky wanted to have it today in staging.