Box reports merge into main #1844
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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
* 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>
Contributor
Yes, you can review/comment your own pull requests. I do it all the time when I need to say something related to context. |
Co-authored-by: Julien Portalier <julien@portalier.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1734.
Reports feature merged into main. Some comments & small fixes that were pending:
noticeinforming it is displayedbulk_deletebutton is enabled now only whensample_reportsare selected in the index view.samples-row*anditems*, I have merged them choosing the second option (and checked all the places where they are used.batches_selectorcomponent had a bug:distractorandinstructionswere defined in the samples when they were added into the batch, meaning that if the user uploaded first the information of thereplicates/copiesand after thedistractor/instructionsthis info was lost. I have changed it to be added to the samples at the moment of adding a new batch into the component.This is the merge of the whole feature within main, so maybe I should point out the specific places of the code I'm referring to in these small comments, given that the rest was already reviewed, please let me know Julien if you want that I do it.