Merge pull request seek4science#1704 from ELIXIR-Belgium/issue_1701_f…

…ix_controlled_vocabs_template_attributes

Issue 1701 fix controlled vocabs in template attributes

app/assets/javascripts/single_page/dynamic_table.js.erb

@@ -21,7 +21,7 @@ const defaultCols = [{
 
 const objectInputTemp = '<input type="hidden" name="_NAME_[]" id="inpt-_NAME_" value="" autocomplete="off" />' +
   '<select name="_NAME_[]" id="select-_NAME_" class="form-control _EXTRACLASS_" title="_TITLE_" data-role="seek-objectsinput" ' +
-  'data-tags-limit="100" multiple="multiple" style="background-color: coral;" data-typeahead-template="_TYPEHEAD_"' +
+  'data-tags-limit="100" _MULTIPLE?_ style="background-color: coral;" data-typeahead-template="_TYPEHEAD_"' +
   'data-typeahead-query-url="_URL_" data-allow-new-items=_ALLOW_FREE_TEXT_>_OPTIONS_</select>';
 
 const typeaheadSamplesUrl = "<%= typeahead_samples_path(linked_sample_type_id: '_LINKED_') %>";
@@ -59,22 +59,21 @@ const handleSelect = (e) => {
         }
 
         c["render"] = function(data_, type, full, meta) {
-          if(c.multi_link){
+          if(c.linked_sample_type){
             data = data_ && Array.isArray(data_) ? data_ : [data_];
             data = data[0]?.id ? data : [];
             return inputObjectsInput(c, data, options, linkedSamplesUrl);
-          }else if(c.is_cv_list && data_ !== "#HIDDEN"){
-            data = data_ && Array.isArray(data_) ? data_ : [data_];
-            data = data.map((e) => {
-              if (e?.id){
-                return e.id
-              } else {
-                return e
-              }
-            });
-
-            return cvListObjectsInput(c, data, options, cvUrl);
-          }else if (data_ === "#HIDDEN") {
+          } else if(c.is_cv_list && data_ !== "#HIDDEN"){
+              data = data_ && Array.isArray(data_) ? data_ : [data_];
+              data = data.map((e) => {
+                if (e?.id){
+                  return e.id
+                } else {
+                  return e
+                }
+              });
+              return cvListObjectsInput(c, data, options, cvUrl);
+          } else if (data_ === "#HIDDEN") {
             return "<em><small>Hidden</small></em>";
           } else {
             return data_;
@@ -565,6 +564,7 @@ function inputObjectsInput(column, data, options, url){
     }, []);
     const hasUnlinkedSamples = unLinkedSamples.length > 0 ? true : false;
 
+    const hasMultipleInputs = column.multi_link ? 'multiple="multiple"' : ''
     const extraClass = hasUnlinkedSamples ? 'select2__error' : '';
     const titleText = hasUnlinkedSamples ? `Sample(s) '${unLinkedSamples.map(uls => uls.title).join(', ')}' not recognised as input. Please correct this issue!` : '';
     setTimeout(ObjectsInput.init);
@@ -576,6 +576,7 @@ function inputObjectsInput(column, data, options, url){
     .replace('_OPTIONS_', existingOptions)
     .replace('_EXTRACLASS_', extraClass)
     .replace('_TITLE_', titleText)
+    .replace('_MULTIPLE?_', hasMultipleInputs)
     .replace('_ALLOW_FREE_TEXT_', false);
   }
 }
@@ -599,6 +600,7 @@ function cvListObjectsInput(column, data, options, url){
     .replace('_OPTIONS_', existingOptions)
     .replace('_EXTRACLASS_', extraClass)
     .replace('_TITLE_', titleText)
+    .replace('_MULTIPLE?_', 'multiple="multiple"')
     .replace('_ALLOW_FREE_TEXT_', allowNewItems);
   }
 }

app/assets/javascripts/single_page/index.js.erb

@@ -242,12 +242,12 @@ async function exportToExcel(tableName, studyId, assayId, sampleTypeId) {
       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
-        }
+      if (Array.isArray(val)){
+        hasHiddenInputs = hasHiddenInputs || val.some(input => input.title === '#HIDDEN'); // Sample has hidden Registered Samples in list
+      } else if(typeof val === 'object' && val !== null && 'title' in val){
+        hasHiddenInputs = hasHiddenInputs || (val.title === '#HIDDEN'); // Sample has hidden Registered Sample
+      } else {
+        hasHiddenInputs = hasHiddenInputs || (val === '#HIDDEN'); // Sample is completely hidden
       }
 
       // The source inputs are wrapped in an object and must be unwrapped first

app/assets/javascripts/templates.js

@@ -11,7 +11,7 @@ Templates.init = function (elem) {
   const columnDefs = [
     { orderable: false, targets: [0, 7, 11] },
     {
-      targets: [3, 4, 9, 10],
+      targets: [3, 4, 5, 10, 11, 13],
       visible: false,
       searchable: false
     },
@@ -33,6 +33,7 @@ Templates.init = function (elem) {
     { title: "Description", width: "40%" },
     { title: "attribute_type_id" },
     { title: "cv_id" },
+    { title: "allow_cv_free_text" },
     { title: "Unit", width: "5%" },
     { title: "Data type", width: "10%" },
     {
@@ -56,7 +57,8 @@ Templates.init = function (elem) {
           ? ""
           : '<a class="btn btn-danger btn-sm" href="javascript:void(0)" onClick="remove(this)">Remove</a>';
       }
-    }
+    },
+    { title: "linked sample type id", width: "10%" }
   ];
 
   Templates.table = elem.DataTable({
@@ -115,13 +117,15 @@ Templates.mapData = (data) =>
     item.description,
     item.attribute_type_id,
     item.cv_id,
+    item.allow_cv_free_text,
     item.unit_id,
     item.data_type,
     item.is_title,
     item.pid,
     item.pos,
     item.isa_tag_id,
-    item.isa_tag_title
+    item.isa_tag_title,
+    item.linked_sample_type_id
   ]);
 
 function loadFilterSelectors(data) {
@@ -183,14 +187,14 @@ function get_filtered_isa_tags(level) {
 function updateIsaTagSelect(template_level, attribute_row) {
   const isa_tags = get_filtered_isa_tags(template_level);
 
-  // Remove all options first, except blank one
-  $j(attribute_row).find('select[data-attr="isa_tag_title"] option:not([value=""])').each(function() {
+  // Remove all options first from the select items that were not disabled, except blank one
+  $j(attribute_row).find('select[data-attr="isa_tag_title"]:not(:disabled) option:not([value=""])').each(function() {
     $j(this).remove();
   });
 
   // Append filtered option to a new attribute row
   $j.each(isa_tags, function (i, tag) {
-    $j(attribute_row).find('select[data-attr="isa_tag_title"]').append($j('<option>', {
+    $j(attribute_row).find('select[data-attr="isa_tag_title"]:not(:disabled)').append($j('<option>', {
       value: tag.value,
       text: tag.text
     }));
@@ -231,32 +235,36 @@ const applyTemplate = () => {
     });
     index++;
 
+    const isInputRow = row[7] === 'Registered Sample List' && row[1].includes('Input') && row[11] === null
     newRow = $j(newRow.replace(/replace-me/g, index));
-
     $j(newRow).find('[data-attr="required"]').prop("checked", row[0]);
     $j(newRow).find('[data-attr="title"]').val(row[1]);
     $j(newRow).find('[data-attr="description"]').val(row[2]);
     $j(newRow).find('[data-attr="type"]').val(row[3]);
     $j(newRow).find('[data-attr="cv_id"]').val(row[4]);
-    $j(newRow).find('[data-attr="unit"]').val(row[5]);
-    $j(newRow).find(".sample-type-is-title").prop("checked", row[7]);
-    $j(newRow).find('[data-attr="pid"]').val(row[8]);
-    $j(newRow).find('[data-attr="isa_tag_id"]').val(row[10]);
-    $j(newRow).find('[data-attr="isa_tag_title"]').val(row[10]);
+    $j(newRow).find('[data-attr="allow_cv_free_text"]').prop("checked", row[5]);
+    $j(newRow).find('[data-attr="unit"]').val(row[6]);
+    $j(newRow).find(".sample-type-is-title").prop("checked", row[8]);
+    $j(newRow).find('[data-attr="pid"]').val(row[9]);
+    $j(newRow).find('[data-attr="isa_tag_id"]').val(row[11]);
+    $j(newRow).find('[data-attr="isa_tag_title"]').val(row[11]);
     $j(newRow).find('[data-attr="isa_tag_title"]').attr('disabled', true);
 
     // Show the CV block if cv_id is not empty
     if (row[4]) $j(newRow).find(".controlled-vocab-block").show();
 
-    // Show the sample-type-block block if type is SEEK sample
-    const is_seek_sample = $j(newRow)
-      .find(".sample-type-attribute-type")
-      .find(":selected")
-      .data("is-seek-sample");
-    if (is_seek_sample) {
-      // Select the first item by default and hide the row
-      $j(newRow).find(".linked-sample-type-selection optgroup option:first").attr("selected", "selected");
+    // If input-row: use input-sample-type-id
+    // else: set the linked_sample_type_id
+    if (isInputRow) {
+      const previousSampleTypeId = $j('#isa_assay_input_sample_type_id').val();
+      if(previousSampleTypeId){
+        $j(newRow).find('.linked-sample-type-selection').val(previousSampleTypeId)
+      } else {
+        $j(newRow).find(".linked-sample-type-selection optgroup option:first").attr("selected", "selected");
+      }
       $j(newRow).hide();
+    } else {
+      $j(newRow).find('.linked-sample-type-selection').val(row[13])
     }
 
     $j(`${attribute_table} ${addAttributeRow}`).before(newRow);

app/controllers/single_pages_controller.rb

@@ -162,6 +162,10 @@ def upload_samples
       sa_attr.title if sa_attr.sample_attribute_type.base_type == Seek::Samples::BaseType::SEEK_SAMPLE_MULTI
     end
 
+    @registered_sample_fields = @sample_type.sample_attributes.map do |sa_attr|
+      sa_attr.title if sa_attr.sample_attribute_type.base_type == Seek::Samples::BaseType::SEEK_SAMPLE
+    end
+
     @cv_list_fields = @sample_type.sample_attributes.map do |sa_attr|
       sa_attr.title if sa_attr.sample_attribute_type.base_type == Seek::Samples::BaseType::CV_LIST
     end
@@ -266,6 +270,12 @@ def generate_excel_samples(samples_data, sample_fields, sample_type_attributes)
             subsample
           end
           obj.merge!(sample_fields[i] => parsed_excel_input_samples)
+        elsif @registered_sample_fields.include?(sample_fields[i])
+          parsed_excel_registered_sample = JSON.parse(excel_sample[i].gsub(/"=>/x, '":'))
+          unless Sample.find(parsed_excel_registered_sample['id'])&.authorized_for_view?
+            raise "Unauthorized Sample was detected in spreadsheet: #{parsed_excel_registered_sample.inspect}"
+          end
+          obj.merge!(sample_fields[i] => parsed_excel_registered_sample)
         elsif @cv_list_fields.include?(sample_fields[i])
           parsed_cv_terms = JSON.parse(excel_sample[i])
           # CV validation for CV_LIST attributes

app/controllers/templates_controller.rb

@@ -157,7 +157,8 @@ def template_params
                                        template_attributes_attributes: %i[id title pos required description
                                                                           sample_attribute_type_id isa_tag_id is_title
                                                                           sample_controlled_vocab_id pid
-                                                                          unit_id _destroy] })
+                                                                          unit_id _destroy allow_cv_free_text
+                                                                          linked_sample_type_id] })
   end
 
   def find_template

app/helpers/dynamic_table_helper.rb

@@ -30,9 +30,12 @@ def link_sequence(sample_type)
   end
 
   def dt_rows(sample_type)
+    registered_sample_attributes = sample_type.sample_attributes.select { |sa| sa.sample_attribute_type.base_type == Seek::Samples::BaseType::SEEK_SAMPLE }
+    registered_sample_multi_attributes = sample_type.sample_attributes.select { |sa| sa.sample_attribute_type.base_type == Seek::Samples::BaseType::SEEK_SAMPLE_MULTI }
+
     sample_type.samples.map do |s|
       if s.can_view?
-        sanitized_json_metadata = hide_unauthorized_inputs(JSON(s.json_metadata))
+        sanitized_json_metadata = hide_unauthorized_inputs(JSON(s.json_metadata), registered_sample_attributes, registered_sample_multi_attributes)
         ['', s.id, s.uuid] +
           sanitized_json_metadata.values
       else
@@ -42,11 +45,20 @@ def dt_rows(sample_type)
     end
   end
 
-  def hide_unauthorized_inputs(json_metadata)
-    input_key = json_metadata.keys.detect { |jmdk| jmdk.downcase.include? 'input' }
+  def hide_unauthorized_inputs(json_metadata, registered_sample_attributes, registered_sample_multi_attributes)
+    registered_sample_multi_attributes.map(&:title).each do |rsma|
+      json_metadata = transform_registered_sample_multi(json_metadata, rsma)
+    end
+    registered_sample_attributes.map(&:title).each do |rma|
+      json_metadata = transform_registered_sample_single(json_metadata, rma)
+    end
+
+    json_metadata
+  end
 
+  def transform_registered_sample_multi(json_metadata, input_key)
     unless input_key.nil?
-      json_metadata[input_key] = json_metadata[input_key].map do |input|
+        json_metadata[input_key] = json_metadata[input_key].map do |input|
         input_exists = Sample.where(id: input['id']).any?
         if !input_exists
           input
@@ -57,7 +69,21 @@ def hide_unauthorized_inputs(json_metadata)
         end
       end
     end
+    json_metadata
+  end
 
+  def transform_registered_sample_single(json_metadata, input_key)
+    unless input_key.nil?
+      input = json_metadata[input_key]
+      input_exists = Sample.where(id: input['id']).any?
+      if !input_exists
+        json_metadata[input_key] = input
+      elsif Sample.find(input['id']).can_view?
+        json_metadata[input_key] = input
+      else
+        json_metadata[input_key] = { 'id' => input['id'], 'type' => input['type'], 'title' => '#HIDDEN' }
+      end
+    end
     json_metadata
   end
 
@@ -66,10 +92,12 @@ def dt_cols(sample_type)
       attribute = { title: a.title, name: sample_type.id.to_s, required: a.required, description: a.description,
                     is_title: a.is_title }
       attribute.merge!({ cv_id: a.sample_controlled_vocab_id }) unless a.sample_controlled_vocab_id.blank?
+      is_seek_sample = a.sample_attribute_type.base_type == Seek::Samples::BaseType::SEEK_SAMPLE
       is_seek_multi_sample = a.sample_attribute_type.base_type == Seek::Samples::BaseType::SEEK_SAMPLE_MULTI
       is_cv_list = a.sample_attribute_type.base_type == Seek::Samples::BaseType::CV_LIST
       cv_allows_free_text =  a.allow_cv_free_text
       attribute.merge!({ multi_link: true, linked_sample_type: a.linked_sample_type.id }) if is_seek_multi_sample
+      attribute.merge!({ multi_link: false, linked_sample_type: a.linked_sample_type.id }) if is_seek_sample
       attribute.merge!({is_cv_list: , cv_allows_free_text:}) if is_cv_list
       attribute
     end

app/helpers/templates_helper.rb

@@ -63,6 +63,10 @@ def template_attribute_details_table(attributes)
   def template_attribute_type_link(template_attribute)
     type = template_attribute.sample_attribute_type.title
 
+    if template_attribute.sample_attribute_type.seek_sample? || template_attribute.sample_attribute_type.seek_sample_multi?
+      type += ' - ' + link_to(template_attribute.linked_sample_type&.title, template_attribute.linked_sample_type)
+    end
+
     if template_attribute.sample_attribute_type.controlled_vocab?
       type += ' - ' + link_to(template_attribute.sample_controlled_vocab.title,
                               template_attribute.sample_controlled_vocab)
@@ -75,6 +79,7 @@ def map_template_attributes(attribute)
       attribute_type_id: attribute.sample_attribute_type_id,
       data_type: SampleAttributeType.find(attribute.sample_attribute_type_id)&.title,
       cv_id: attribute.sample_controlled_vocab_id,
+      allow_cv_free_text: attribute.allow_cv_free_text,
       title: attribute.title,
       is_title: attribute.is_title,
       short_name: attribute.short_name,
@@ -84,7 +89,8 @@ def map_template_attributes(attribute)
       unit_id: attribute.unit_id,
       pos: attribute.pos,
       isa_tag_id: attribute.isa_tag_id,
-      isa_tag_title: attribute.isa_tag&.title
+      isa_tag_title: attribute.isa_tag&.title,
+      linked_sample_type_id: attribute.linked_sample_type_id
     }
   end
 end

app/models/template.rb

@@ -33,11 +33,17 @@ def resolve_inconsistencies
   end
 
   def validate_template_attributes
-    errors.add(:base, '[Template attribute]: Some attributes are missing ISA tags') unless none_empty_isa_tag
+    unless attributes_with_empty_isa_tag.none?
+      attributes_with_empty_isa_tag.map do |attribute|
+        errors.add("[#{:template_attributes}]:", "Attribute '#{attribute.title}' is missing an ISA tag")
+      end
+    end
+
     if test_tag_occurences.any?
       test_tag_occurences.map do |tag|
-        errors.add(:base,
-                   "[Template attribute]: The <em>'#{tag}'</em> ISA tag is not allowed to be used more then once".html_safe)
+        attributes_with_duplicate_tags = template_attributes.select { |tat| tat.isa_tag&.title == tag }.map(&:title)
+        errors.add("[#{:template_attributes}]:",
+                   "The '#{tag}' ISA Tag was used in these attributes => #{attributes_with_duplicate_tags.inspect}. This ISA tag is not allowed to be used more then once!")
       end
     end
 
@@ -56,8 +62,8 @@ def resolve_controlled_vocabs_inconsistencies
     end
   end
 
-  def none_empty_isa_tag
-    template_attributes.select { |ta| !ta.title.include?('Input') && ta.isa_tag_id.nil? }.none?
+  def attributes_with_empty_isa_tag
+    template_attributes.select { |ta| !ta.title.include?('Input') && ta.isa_tag_id.nil? }
   end
 
   def test_tag_occurences
@@ -74,7 +80,7 @@ def test_input_occurence
 
   def test_attribute_title_uniqueness
     template_attribute_titles = template_attributes.map(&:title).uniq
-    duplicate_attributes = template_attribute_titles.map do |tat|
+    template_attribute_titles.map do |tat|
       if template_attributes.select { |ta| ta.title.downcase == tat.downcase }.map(&:title).count > 1
         errors.add(:template_attributes, "Attribute names must be unique, there are duplicates of #{tat}")
         return tat

app/models/template_attribute.rb

@@ -4,6 +4,8 @@ class TemplateAttribute < ApplicationRecord
   belongs_to :template, inverse_of: :template_attributes
   belongs_to :unit
   belongs_to :isa_tag
+  belongs_to :linked_sample_type, class_name: 'SampleType'
+
   validates :title, presence: true
 
   before_save :default_pos

app/views/isa_studies/_sample_types_form.html.erb

@@ -5,6 +5,9 @@
 <% main_field_name = id_suffix[1..-1] %>
 <% isa_element ||= "study" %>
 
+<span id='modal-dialogues'>
+    <%= sample_controlled_vocab_model_dialog('cv-modal') %>
+</span>
 <%=  f.fields_for main_field_name, sample_type do |field| %>
 
 	<% unless action == :edit %>