From fb4803b121ebe2bf96201d8a6f53430761c33aac Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer <82407142+kdp-cloud@users.noreply.github.com> Date: Wed, 8 Nov 2023 17:48:57 +0100 Subject: [PATCH] Issue 1558 hide unauthorized cells in dynamic table (#1620) * Remove MIAPPE stuff from ISA Study from * Add appropriate message if no templates are stored in the DB. * substitute sources by input. Is more clear. * Make unauthorized samples hidden. * Prevent to download the table when it has errors * Improve the error message when no sample is selected. * Change alert message * Filter out samples with hidden inputs and completely hidden samples --- .../single_page/dynamic_table.js.erb | 13 +- .../javascripts/single_page/index.js.erb | 111 ++++++++++-------- app/controllers/single_pages_controller.rb | 6 +- app/helpers/dynamic_table_helper.rb | 30 ++++- app/views/isa_studies/_form.html.erb | 38 +++--- app/views/isa_studies/new.html.erb | 11 +- 6 files changed, 127 insertions(+), 82 deletions(-) diff --git a/app/assets/javascripts/single_page/dynamic_table.js.erb b/app/assets/javascripts/single_page/dynamic_table.js.erb index cdd63249ee..f64dab4c4c 100644 --- a/app/assets/javascripts/single_page/dynamic_table.js.erb +++ b/app/assets/javascripts/single_page/dynamic_table.js.erb @@ -50,7 +50,14 @@ const handleSelect = (e) => { if (c.multi_link) { data = data && Array.isArray(data) ? data : [data]; data = data[0]?.id ? data : []; - const existingOptions = data.map((e) => ``).join(""); + const existingOptions = data.map((e) => { + isHiddenInput = (e.title == '#HIDDEN') + if (isHiddenInput) { + return `` + } else { + return `` + } + }).join(""); if (options.readonly) { return data.map((e) => `${e.title}`).join(" "); } else { @@ -61,7 +68,7 @@ const handleSelect = (e) => { const linkedSamples = retrieveLinkedSamples(url); const linkedSampleIds = linkedSamples.map((ls) => ls.id); const unLinkedSamples = data.reduce(function(filtered, sample) { - if(!linkedSampleIds.includes(parseInt(sample.id))){ + if(!linkedSampleIds.includes(parseInt(sample.id)) && sample.title != '#HIDDEN'){ filtered.push(sample); } return filtered; @@ -69,7 +76,7 @@ const handleSelect = (e) => { const hasUnlinkedSamples = unLinkedSamples.length > 0 ? true : false; const extraClass = hasUnlinkedSamples ? 'select2__error' : ''; - const titleText = hasUnlinkedSamples ? `Sample(s) '${unLinkedSamples.map(uls => uls.title).join(', ')}' not recognised as sources. Please correct this issue!` : ''; + const titleText = hasUnlinkedSamples ? `Sample(s) '${unLinkedSamples.map(uls => uls.title).join(', ')}' not recognised as input. Please correct this issue!` : ''; return objectInputTemp .replace(/_NAME_/g, objectInputName) diff --git a/app/assets/javascripts/single_page/index.js.erb b/app/assets/javascripts/single_page/index.js.erb index 85994ee285..b1ef93f426 100644 --- a/app/assets/javascripts/single_page/index.js.erb +++ b/app/assets/javascripts/single_page/index.js.erb @@ -207,61 +207,78 @@ async function batchDeleteSample(sampleTypes) { // Posts the content of the dynamic table to the single_pages_controller, // which generates an excel workbook based on a template. async function exportToExcel(tableName, studyId, assayId, sampleTypeId) { - const headerRow = $j(`table[id=${tableName}] thead tr`)["0"]; + const headerRow = $j(`table[id=${tableName}] thead tr`)["0"]; - // Cells from the table body => Each row = array - // For some reason they contain null values, which should be filtered out - let bodyCells = $j(`table[id=${tableName}]`).DataTable().data().toArray().map(subarray => subarray.filter(function(el){return el != null})); + // Cells from the table body => Each row = array + // For some reason they contain null values, which should be filtered out + let bodyCells = $j(`table[id=${tableName}]`).DataTable().data().toArray().map(subarray => subarray.filter(function(el){return el != null})); - // Return array of table header cells from headerRow - let headerCells = []; - $j.each(headerRow.cells, function (i, v) { - headerCells.push(v.textContent); - }); + // Checks whether the dynamic table has errors + // The excel export will be aborted as long as the dynamic table has errors + hasErrorcells = $j(`table[id=${tableName}]`).find('select.select2__error').size() > 0 + if (hasErrorcells) { + alert('It appears this sample table has some errors. Please correct the errors in the current sample table and try downloading again.'); + return; + } - // Construct the 'struct' of the table data - let data = []; - for (let i = 0; i < bodyCells.length; i++) { - const row = bodyCells[i]; - // If a row has been checked / unchecked, - // the datatable body gets one extra hidden cell at index 1, - // compared to the table header. This has to be removed. - if (row.length !== headerCells.length)row.splice(1,1); - let obj = {}; - for (let j = 0; j < headerCells.length; j++) { - const attr = headerCells[j]; - const val = row[j]; + // Return array of table header cells from headerRow + let headerCells = []; + $j.each(headerRow.cells, function (i, v) { + headerCells.push(v.textContent); + }); - // The source inputs are wrapped in an object and must be unwrapped first - // And then packaged as a single element - if(j === 0) { - obj["selected"] = (val === "") ? false : val; - } else { - obj[attr] = val; - } - if(obj.selected) data.push(obj); + // Construct the 'struct' of the table data + let data = []; + for (let i = 0; i < bodyCells.length; i++) { + const row = bodyCells[i]; + // If a row has been checked / unchecked, + // the datatable body gets one extra hidden cell at index 1, + // compared to the table header. This has to be removed. + if (row.length !== headerCells.length)row.splice(1,1); + let obj = {}; + let hasHiddenInputs = false; + for (let j = 0; j < headerCells.length; j++) { + const attr = headerCells[j]; + const val = row[j]; + + // Check whether sample has one or more hidden Inputs or if sample is completely hidden + if (attr.toLowerCase().includes('input')) { + if (Array.isArray(val)){ + hasHiddenInputs = val.some(input => input.title === '#HIDDEN'); // Sample has hidden inputs + } else { + hasHiddenInputs = val === '#HIDDEN'; // Sample is completely hidden } } - // Use ajax for POST request with table data in the body - // If succes => Redirect to download path for retrieving excel file - $j.ajax({ - type: 'POST', - url: '<%= export_to_excel_single_pages_path() %>', - data: { sample_data: JSON.stringify(data), - sample_type_id: JSON.stringify(sampleTypeId), - study_id: JSON.stringify(studyId), - assay_id: JSON.stringify(assayId), - }, - success: function(response) { - downloadUrl = `<%= download_samples_excel_single_pages_path() %>?uuid=${response.uuid}`; - window.location.href = downloadUrl; - }, - error: function(response) { - alert(`Failed to export through excel!\nStatus: ${response.status}\nError: ${JSON.stringify(response.error().statusText)}`); - } - }); + // The source inputs are wrapped in an object and must be unwrapped first + // And then packaged as a single element + if(j === 0) { + obj["selected"] = (val === "") ? false : val; + } else { + obj[attr] = val; + } + } + if (!hasHiddenInputs) data.push(obj); + } + // Use ajax for POST request with table data in the body + // If succes => Redirect to download path for retrieving excel file + $j.ajax({ + type: 'POST', + url: '<%= export_to_excel_single_pages_path() %>', + data: { sample_data: JSON.stringify(data), + sample_type_id: JSON.stringify(sampleTypeId), + study_id: JSON.stringify(studyId), + assay_id: JSON.stringify(assayId), + }, + success: function(response) { + downloadUrl = `<%= download_samples_excel_single_pages_path() %>?uuid=${response.uuid}`; + window.location.href = downloadUrl; + }, + error: function(response) { + alert(`Failed to export through excel!\nStatus: ${response.status}\nError: ${JSON.stringify(response.error().statusText)}`); + } + }); } async function batchUpdateSample(sampleTypes) { diff --git a/app/controllers/single_pages_controller.rb b/app/controllers/single_pages_controller.rb index 591eb2600c..534f0cbd85 100644 --- a/app/controllers/single_pages_controller.rb +++ b/app/controllers/single_pages_controller.rb @@ -70,7 +70,9 @@ def download_samples_excel @project = @study.projects.first @samples = Sample.where(id: sample_ids)&.authorized_for(:view)&.sort_by(&:id) - raise 'Nothing to export to Excel.' if @samples.nil? || @samples == [] || sample_type_id.nil? + if @samples.nil? || @samples == [] || sample_type_id.nil? + raise 'Nothing to export to Excel. Please select samples in the table and try downloading the table again.' + end @sample_type = SampleType.find(sample_type_id) @@ -114,7 +116,7 @@ def download_samples_excel def export_to_excel cache_uuid = UUID.new.generate id_label = "#{Seek::Config.instance_name} id" - sample_ids = JSON.parse(params[:sample_data]).map { |sample| sample[id_label] unless sample[id_label] == '#HIDDEN' } + sample_ids = JSON.parse(params[:sample_data]).map { |sample| sample[id_label] } sample_type_id = JSON.parse(params[:sample_type_id]) study_id = JSON.parse(params[:study_id]) assay_id = JSON.parse(params[:assay_id]) diff --git a/app/helpers/dynamic_table_helper.rb b/app/helpers/dynamic_table_helper.rb index c3ded0ab35..42c09495ae 100644 --- a/app/helpers/dynamic_table_helper.rb +++ b/app/helpers/dynamic_table_helper.rb @@ -31,14 +31,34 @@ def link_sequence(sample_type) def dt_rows(sample_type) sample_type.samples.map do |s| - if s.can_view? - ['', s.id, s.uuid] + - JSON(s.json_metadata).values - else - ['', '#HIDDEN', '#HIDDEN'] + + if s.can_view? + sanitized_json_metadata = hide_unauthorized_inputs(JSON(s.json_metadata)) + ['', s.id, s.uuid] + + sanitized_json_metadata.values + else + ['', '#HIDDEN', '#HIDDEN'] + Array.new(JSON(s.json_metadata).length, '#HIDDEN') + end + end + end + + def hide_unauthorized_inputs(json_metadata) + input_key = json_metadata.keys.detect { |jmdk| jmdk.downcase.include? 'input' } + + unless input_key.nil? + json_metadata[input_key] = json_metadata[input_key].map do |input| + input_exists = Sample.where(id: input['id']).any? + if !input_exists + input + elsif Sample.find(input['id']).can_view? + input + else + { 'id' => input['id'], 'type' => input['type'], 'title' => '#HIDDEN' } end + end end + + json_metadata end def dt_cols(sample_type) diff --git a/app/views/isa_studies/_form.html.erb b/app/views/isa_studies/_form.html.erb index 9790bec565..9c7becddc4 100644 --- a/app/views/isa_studies/_form.html.erb +++ b/app/views/isa_studies/_form.html.erb @@ -24,7 +24,7 @@ +
<%= study_fields.label "Study position" -%>
@@ -42,48 +42,56 @@ <%= render partial: 'projects/implicit_project_selector', locals: { action: action, select_id: '#study_investigation_id', parents: Investigation.authorized_for('edit') } %> - + <%= folding_panel("Define #{t(:sample_type)} for Source") do %> <%= render partial: 'sample_types_form', locals: {f: f, sample_type: @isa_study.source, id_suffix: "_source_sample_type", action: action } if @isa_study.source %> <% end %> <%= folding_panel(t(:sop).pluralize) do %>

The following <%= t(:sop).pluralize %> are associated with this <%= t(:study) %>:

- + <%= study_fields.fancy_multiselect :sops, { other_projects_checkbox: true, name: "isa_study[study][sop_ids]" } %> - + <% end %> <%= folding_panel("Define #{t(:sample_type)} for Sample") do %> <%= render partial: 'sample_types_form', locals: {f: f, sample_type: @isa_study.sample_collection, id_suffix: "_sample_collection_sample_type", action: action } if @isa_study.sample_collection %> <% end %> -<% end -%> +<% end -%> + - <%= form_submit_buttons(@isa_study.study) %> diff --git a/app/views/isa_studies/new.html.erb b/app/views/isa_studies/new.html.erb index 08cb301210..feace3fc75 100644 --- a/app/views/isa_studies/new.html.erb +++ b/app/views/isa_studies/new.html.erb @@ -6,15 +6,6 @@

New <%=t('isa_study')%>

- <% if show_batch_miappe_button? %> -

Upload studies from file:

- <%= link_to "MIAPPE Study batch upload", batch_uploader_studies_path, class: "btn btn-primary" %> -
- <%= link_to "Download MIAPPE template here", asset_path('batch_upload_template/MIAPPE Study batch template.zip', skip_pipeline: true) %> -
-

Or create a study with the form:

- <% end %> - <%= render :partial => "templates/template_modal" -%>
@@ -22,4 +13,4 @@ <%= render :partial => "form", :locals => { :f => f, :action=>:new } -%> <% end -%>
-<% end -%> \ No newline at end of file +<% end -%>