Boxes reports functioning #1840

leandroradusky · 2023-01-11T16:41:14Z

Reporting functioning for all the box purposes, also exporting to PDF and SVG working. I know that we talked on the contrary. Still, I'm submitting one big PR because:

it was started from an old version of main and the rebasing plus applying the changes dividing this PR is much more error-prone;
the intermediate PRs had code that changes at the end and revising that code will double the work without the necessity
the first of the PRs to be submitted before was not much smaller than this one.

Said this... There are three plots & one visualization, which have some differences from the mockups that will be remarked.

Confusion matrix, which is displayed on all the visualizations

Measured signal, looks like the mockup for Variants & LOD and in the case of Challenge have two differences: distractor samples are grouped in blue bars and virus ones are kept in red, and there is a line to indicate the threshold.

For LOD:

For Challenge:

Limit of detection which is only displayed in LOD purpose and looks different attending client request: each sample has a concentration and a measured signal and represents a point in the plot, the regression line computed for all these points is represented with a dashed line.

ROC curve which is only displayed for Challenge purpose. The curve itself is displayed in red, and the TPR and FPR of the selected threshold are updated to accommodate the dashed lines.

The Challenge report allows the user to update the threshold via a slider:

This threshold will update the confusion matrix, dashed lines mentioned above, and the information card on the left:

When the threshold bar is moved update_threshold is GET from the controller to update variables. For Variants and LOD threshold is always zero: all the samples contain virus, then they are only negative those containing zero concentration (no virus).

Also, there are enabled buttons to download SVG and PDF files are functioning:

For the PDF exporting, a new view was generated, avoiding layout and generating a view that will be "printed" to a file. This file is generated using this library, which helps with margins/page-orientation/page-breaks, etc. The library wasn't installed with NPM in the project, is dynamically loaded in javascript. Should it be installed?
For the SVG exporting: after intensive stack overflowing the issue, the only solution working properly was to:

generate an element that downloads a generated Blob; along with
a function that recursively takes the CSS styles of the elements and writes them into the styles property. This function is called applyInLine. The natural option for this when looking to solve this issue was to use getComputedStyle, but it didn't work properly for the react-generated SVG, while this one, more "sophisticated" did.

Screenshots of the PDF (for Challenge purpose, illustrative of the others):

1/3

2/3

3/3

Important: this PR rely in the existence of a distractor attribute in samples, which was faked during the development and not committed, and it should be merged into main after the PRs containing the addition of this attribute are merged

…, specs

* Remove Institution and Purpose labels to improve display * Truncate institution name * Add samples count

* Custom confirm modals for upload results form * Custom confirm modals for upload results form * Custom confirm modals for upload results form

* Migrate to mini_racer gem to replace the deprecated therubyracer gem * Fix: use MiniRacer::Context in ManifestFieldMapping * Fix: don't expose raw Ruby/Nokogiri objects in ManifestFieldMapping#run_script WARNING: THIS IS A BREAKING CHANGE. All parsers for manifest fields return a trivial data structure, made of hash, array, string and number that are easy to serialize to another language (in this case JS). The XML parse, however, returns a Nokogiri::XML::Element object that therubyracer exposed entirely to JavaScript (all methods were accessible). Even worse, it would expose all objects returned by calls to the object (e.g. Ruby arrays, Nokogiri::XML::NodeSet, ...). This behavior had numerous issues: 1. it's exposing an unlimited Ruby interface to user controlled code (device manifests), which is a security issue; 2. it's tying the Nokogiri interface to application uncontrolled code, leading to the impossibility to ever use anything else, or having to re-implement the whole interface (or at least what's used). 3. it's preventing us from moving away from the old abandonned `therubyracer` gem to something maintained (i.e. `mini_racer`) that don't expose non trivial objects. After verifying on different production sites, we noticed that nobody was using the XML parser, so I chose to change how we expose XML parsed data to JavaScript. We now serialize the Nokogiri::XML::Element as a trivial data structure of the form: ``` var message = { "name": "node_name", "attributes": { "attr_name" => "attr_value" }, "children": [ // nested elements ] } ``` In addition we still expose a `message.xpath(query)` function that allows to query for nested data in a nicer way. It's only available on the `message` variable and not available for nested children (the query should directly return the node that you're interested in). The function returns the same serialized data structure.

* Allow to download CSV template file and style fixes * Fixes PR review

…eature/1821-Challenge-box-report

ysbaddaden

@leandroradusky This is only a partial review, so you can continue working! I'm still pondering about the PDF rendering.

app/assets/javascripts/components/box_reports_bar_chart.js.jsx

app/controllers/samples_reports_controller.rb

ysbaddaden · 2023-01-12T12:14:21Z

app/controllers/samples_reports_controller.rb

  def show
    @samples_report = SamplesReport.find(params[:id])
+    @reports_data = measured_signal_data(@samples_report)
+    @samples_without_results_count = @samples_report.samples.where("core_fields NOT LIKE '%measured_signal%'").count


The condition should be extracted to a model scope, where specific SQL should live.

app/controllers/samples_reports_controller.rb

app/helpers/samples_reports_helper.rb

app/views/samples_reports/_confusion_matrix.haml

app/views/samples_reports/_pdf_report.haml

ysbaddaden

I think it's good as-is.

I still think we should generate the PDF on the backend, but we should work on that in a follow-up PR.

* Feature/1809 samples reports entity (#1823) * New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Fixes PR review * Fixes PR review * Fixes PR review * Fixes Julien's PR review * SamplesReportsController specs (#1839) * Boxes reports functioning (#1840) * New feature: Upload measured concentrations for samples * Specs passing and small fixes * Enablin upload button only when files are present, remove file button, specs * Fixes PR review * All jquery removed, more PR review suggestions solved * Stashing changes * Stashing changes * Stashing changes * Stashing changes * Boxes reports entity working * Small fixes * Rework Box Label (#1819) * Remove Institution and Purpose labels to improve display * Truncate institution name * Add samples count * Box reports for LOD samples working * Small fix * Enabled PDF and SVG export in box reports * Visualization and print implmented for Challenge Boxes * Added computed AUC, TPR and FPR * automatic unblinding for samples with uploaded measurements (#1831) * Fixing of box label in UI (#1836) * Feature/1807 custom confirm modal (#1834) * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Custom confirm modals for upload results form * Upgrade to `mini_racer` gem (#1791) * Migrate to mini_racer gem to replace the deprecated therubyracer gem * Fix: use MiniRacer::Context in ManifestFieldMapping * Fix: don't expose raw Ruby/Nokogiri objects in ManifestFieldMapping#run_script WARNING: THIS IS A BREAKING CHANGE. All parsers for manifest fields return a trivial data structure, made of hash, array, string and number that are easy to serialize to another language (in this case JS). The XML parse, however, returns a Nokogiri::XML::Element object that therubyracer exposed entirely to JavaScript (all methods were accessible). Even worse, it would expose all objects returned by calls to the object (e.g. Ruby arrays, Nokogiri::XML::NodeSet, ...). This behavior had numerous issues: 1. it's exposing an unlimited Ruby interface to user controlled code (device manifests), which is a security issue; 2. it's tying the Nokogiri interface to application uncontrolled code, leading to the impossibility to ever use anything else, or having to re-implement the whole interface (or at least what's used). 3. it's preventing us from moving away from the old abandonned `therubyracer` gem to something maintained (i.e. `mini_racer`) that don't expose non trivial objects. After verifying on different production sites, we noticed that nobody was using the XML parser, so I chose to change how we expose XML parsed data to JavaScript. We now serialize the Nokogiri::XML::Element as a trivial data structure of the form: ``` var message = { "name": "node_name", "attributes": { "attr_name" => "attr_value" }, "children": [ // nested elements ] } ``` In addition we still expose a `message.xpath(query)` function that allows to query for nested data in a nicer way. It's only available on the `message` variable and not available for nested children (the query should directly return the node that you're interested in). The function returns the same serialized data structure. * GHA: fix deprecated nodejs version warning * Styled slider, small fixes * Allow to download CSV template file and style fixes (#1835) * Allow to download CSV template file and style fixes * Fixes PR review * Basic functioning report for variants box * Merging completition * Merging completition * Merging completition * Merging competition * Fixes PR review Co-authored-by: Julien Portalier <julien@portalier.com> * Merging reports into main & fixes * more bugfixes * Update app/helpers/samples_reports_helper.rb Co-authored-by: Julien Portalier <julien@portalier.com> * small fix Co-authored-by: Julien Portalier <julien@portalier.com>

leandroradusky and others added 30 commits November 21, 2022 12:45

New feature: Upload measured concentrations for samples

c3a8722

Specs passing and small fixes

e6bb02e

Enablin upload button only when files are present, remove file button…

f16fa7a

…, specs

Fixes PR review

fad8ea6

All jquery removed, more PR review suggestions solved

43b3599

Stashing changes

7223107

Stashing changes

72a85ea

Stashing changes

6593733

Merging with main

f86de97

Stashing changes

8ac530e

Boxes reports entity working

ccccd76

Small fixes

11455e1

Rework Box Label (#1819)

585dc28

* Remove Institution and Purpose labels to improve display * Truncate institution name * Add samples count

Box reports for LOD samples working

07166fd

Small fix

2b3df7a

Enabled PDF and SVG export in box reports

338a92b

Visualization and print implmented for Challenge Boxes

4f3519a

Added computed AUC, TPR and FPR

fc36db2

automatic unblinding for samples with uploaded measurements (#1831)

1196f88

Fixing of box label in UI (#1836)

929c3a7

Feature/1807 custom confirm modal (#1834)

26f22e7

* Custom confirm modals for upload results form * Custom confirm modals for upload results form * Custom confirm modals for upload results form

GHA: fix deprecated nodejs version warning

bb4a967

Styled slider, small fixes

8d5390a

Allow to download CSV template file and style fixes (#1835)

36b474c

* Allow to download CSV template file and style fixes * Fixes PR review

Basic functioning report for variants box

5df8382

Merge remote-tracking branch 'origin/feature/1734-box-reports' into f…

d8d8e6f

…eature/1821-Challenge-box-report

Merging completition

1af13a5

Merging completition

ea07bf4

Merging completition

f01cb29

Merging competition

4f5436b

leandroradusky requested a review from ysbaddaden January 11, 2023 16:47

leandroradusky self-assigned this Jan 11, 2023

ysbaddaden reviewed Jan 12, 2023

View reviewed changes

leandroradusky added 2 commits January 13, 2023 12:11

Merge branch 'main' into feature/1821-reporting-merged

ffe8876

Fixes PR review

59d2b20

ysbaddaden approved these changes Jan 17, 2023

View reviewed changes

leandroradusky merged commit 0fc7422 into feature/1734-box-reports Jan 17, 2023

leandroradusky deleted the feature/1821-reporting-merged branch January 17, 2023 14:29

ysbaddaden mentioned this pull request Jan 17, 2023

Generate the Samples Report PDF on the backend #1843

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Boxes reports functioning #1840

Boxes reports functioning #1840

Uh oh!

leandroradusky commented Jan 11, 2023 •

edited

Loading

Uh oh!

ysbaddaden left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ysbaddaden Jan 12, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ysbaddaden left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Boxes reports functioning #1840

Boxes reports functioning #1840

Uh oh!

Conversation

leandroradusky commented Jan 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ysbaddaden left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ysbaddaden Jan 12, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ysbaddaden left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

leandroradusky commented Jan 11, 2023 •

edited

Loading