Create UploadCsvBox Component #1941

[bolom]

Implement a React component, UploadCsvBox, that handles CSV file uploads. The component sends the uploaded file to the backend for validation instead of parsing it in the frontend. It displays the filename and provides information about the number of uploaded samples. In case any batches are not found in the CSV, it highlights the relevant batches and shows the corresponding missing UUIDs in a tooltip.

I'm facing a few CSS issues and could use some help to fix them. But before we dive into that, I'd love to hear your thoughts on the code.

[leandroradusky]

Hi! I was testing this feature and functionally it works as expected. Codewise for me it makes sense but I leave @ysbaddaden to make further comments on that.

Regarding the CSSs, this commit, where all was implemented in the frontend, had the correct CSS styling. It will be not straightforward since you will have to dig a little on the JS, but the rows showing the info about the uploaded CSVs where properly shown there, hope that is helpful :)

[bolom]

@leandroradusky improve the css, can you take a look

[leandroradusky]

Hi @bolom! I've taken a look and here are some comments (we're close :smile:)

The rows appear to have a max width and not go beyond that point, while the file name should be left aligned and the info in the right. When there are missing batches and long file names the message overlap. Another place to "steal" styles & behavior is available in samples>individual samples>upload results. There, it looks like this:

Another things to mention:

• The Add file button is just text and the + button, as it looks above, no "Choose file" button should be displayed.
• The Add file button is after the list of files added and not before.
• When there are zero samples it is saying 0 sample missing the final s.

I think this are only things :)

[bolom]

@leandroradusky let me know what do you think. I've updated the css

[leandroradusky]

Hi @bolom! It's almost perfect!

Two minimal things:

• When more than one batch is not found a <br> is displayed instead of a new line for each batch.
• If it's not a pain (since is not that important) the Add file plus circle should be aligned with the minus circle of the files.

:)

[bolom]

@leandroradusky ready for your review

[leandroradusky]

In the last commit:

• Removed unused vars and commented code lines
• Fixed the pluralization of batches and samples
• The number of samples was displayed incorrectly, it was displaying the number of distinct batches instead.
• The <br> between batches was still displayed.
• The saving of boxes for the CSV mode was broken when any batches were not found, fix the batches_data method and the validations.

While I don't dare to do an opinion of which controller flavor is better and more correct rubywise, I do agree that CsvValidation should be renamed to CsvBoxValidation to accomplish a better description of what the module does. I've tried to change it without success, I would like to have these changes merged to deliver to the client, we can create an issue for the renaming after.

Also, have no idea what is this lint test failing :scream: :cry:

[ysbaddaden]

@leandroradusky About the lint errors: one can be corrected locally with rubocop -A, for the others, you can search for rubocop Lint/NonLocalExitFromIterator and you should find some explanation. For example: https://docs.rubocop.org/rubocop/cops_lint.html#lintnonlocalexitfromiterator

[bolom]

In the last commit:

* Removed unused vars and commented code lines

* Fixed the pluralization of batches and samples

* The number of samples was displayed incorrectly, it was displaying the number of distinct batches instead.

* The `<br>` between batches was still displayed.

* The saving of boxes for the CSV mode was broken when any batches were not found, fix the `batches_data` method and the validations.

While I don't dare to do an opinion of which controller flavor is better and more correct rubywise, I do agree that CsvValidation should be renamed to CsvBoxValidation to accomplish a better description of what the module does. I've tried to change it without success, I would like to have these changes merged to deliver to the client, we can create an issue for the renaming after.

Also, have no idea what is this lint test failing 😱 😢

you can also run this command bundle exec rubocop -A

[ysbaddaden]

Ideally the endpoint should be POST /boxes/validate. No need to specify CSV, it's a mere format for the data; this is similar to how we don't have create_form or create_json but merely create. We can also introduce support for other formats (e.g. XLSX) on the same endpoint.

We could try to make another controller, but then the action becomes too weird:

• boxes/validate#validate (no: pointless repetition)
• boxes/validations#validate (no: the validations term comes from nowhere)
• boxes/validate#create (nope: create means that we create a resource in CDx)

So, I still stand on my suggestion of just adding a method to the Boxes controllers. It's better to group related logic together (creation and validation are very related), and it fits better into the project as it stands.

@leandroradusky we can push that to a followup issue.

[ysbaddaden]

Grmbl: the lint errors are for unrelated changes.

Maybe we should try another github action that would annotate the pull request with the lint warnings and make it allowed to fail. We encourage to fix style and lint issues, but we don't block a merge because of a lint error (especially on something unrelated).