Issue 1656 create an assay between two existing assays

app/controllers/assays_controller.rb

@@ -6,13 +6,19 @@ class AssaysController < ApplicationController
   before_action :find_assets, only: [:index]
   before_action :find_and_authorize_requested_item,
                 only: %i[edit update destroy manage manage_update show new_object_based_on_existing_one]
-  before_action :fix_assay_linkage, only: [:destroy]
-  before_action :delete_linked_sample_types, only: [:destroy]
 
   # project_membership_required_appended is an alias to project_membership_required, but is necessary to include the actions
   # defined in the application controller
   before_action :project_membership_required_appended, only: [:new_object_based_on_existing_one]
 
+  # Only for ISA JSON compliant assays
+  # => Fix sample type linkage
+  before_action :fix_assay_linkage_when_deleting_assays, only: :destroy
+  # => Delete sample type of deleted assay
+  before_action :delete_linked_sample_types, only: :destroy
+  # => Rearrange positions
+  after_action :rearrange_assay_positions_at_destroy, only: :destroy
+
   include Seek::Publishing::PublishingCommon
 
   include Seek::IsaGraphExtensions
@@ -106,30 +112,6 @@ def create
     end
   end
 
-  def delete_linked_sample_types
-    return unless is_single_page_assay?
-    return if @assay.sample_type.nil?
-
-    @assay.sample_type.destroy
-  end
-
-  def fix_assay_linkage
-    return unless is_single_page_assay?
-    return unless @assay.has_linked_child_assay?
-
-    previous_assay_linked_st_id = @assay.previous_linked_sample_type&.id
-
-    next_assay = Assay.all.detect do |a|
-      a.sample_type&.sample_attributes&.first&.linked_sample_type_id == @assay.sample_type_id
-    end
-
-    next_assay_st_attr = next_assay.sample_type&.sample_attributes&.first
-
-    return unless next_assay && previous_assay_linked_st_id && next_assay_st_attr
-
-    next_assay_st_attr.update(linked_sample_type_id: previous_assay_linked_st_id)
-  end
-
   def update
     update_assay_organisms @assay, params
     update_assay_human_diseases @assay, params
@@ -178,6 +160,32 @@ def show
 
   private
 
+  def delete_linked_sample_types
+    return unless @assay.is_isa_json_compliant?
+    return if @assay.sample_type.nil?
+
+    @assay.sample_type.destroy
+  end
+
+  def fix_assay_linkage_when_deleting_assays
+    return unless @assay.is_isa_json_compliant?
+    return unless @assay.has_linked_child_assay?
+
+    previous_assay_linked_st_id = @assay.previous_linked_sample_type&.id
+
+    next_assay = @assay.next_linked_child_assay
+
+    next_assay_st_attr = next_assay.sample_type&.sample_attributes&.detect(&:input_attribute?)
+
+    return unless next_assay && previous_assay_linked_st_id && next_assay_st_attr
+
+    next_assay_st_attr.update(linked_sample_type_id: previous_assay_linked_st_id)
+  end
+
+  def rearrange_assay_positions_at_destroy
+    rearrange_assay_positions(@assay.assay_stream)
+  end
+
   def assay_params
     params.require(:assay).permit(:title, :description, :study_id, :assay_class_id, :assay_type_uri, :technology_type_uri,
                                   :license, *creator_related_params, :position, { document_ids: [] },
@@ -194,10 +202,4 @@ def assay_params
       assay_params[:model_ids].select! { |id| Model.find_by_id(id).try(:can_view?) } if assay_params.key?(:model_ids)
     end
   end
-
-  def is_single_page_assay?
-    return false unless params.key?(:return_to)
-
-    params[:return_to].start_with? '/single_pages/'
-  end
 end

app/controllers/isa_assays_controller.rb

@@ -4,6 +4,9 @@ class IsaAssaysController < ApplicationController
 
   before_action :set_up_instance_variable
   before_action :find_requested_item, only: %i[edit update]
+  before_action :initialize_isa_assay, only: :create
+  before_action :fix_assay_linkage_for_new_assays, only: :create
+  after_action :rearrange_assay_positions_create_isa_assay, only: :create
 
   def new
     if params[:is_assay_stream]
@@ -14,10 +17,6 @@ def new
   end
 
   def create
-    @isa_assay = IsaAssay.new(isa_assay_params)
-    update_sharing_policies @isa_assay.assay
-    @isa_assay.assay.contributor = current_person
-    @isa_assay.sample_type.contributor = User.current_user.person if isa_assay_params[:sample_type]
     if @isa_assay.save
       redirect_to single_page_path(id: @isa_assay.assay.projects.first, item_type: 'assay',
                                    item_id: @isa_assay.assay, notice: 'The ISA assay was created successfully!')
@@ -66,6 +65,42 @@ def update
 
   private
 
+  def fix_assay_linkage_for_new_assays
+    return unless @isa_assay.assay.is_isa_json_compliant?
+    return if @isa_assay.assay.is_assay_stream? # Should not fix anything when creating an assay stream
+
+    previous_sample_type = SampleType.find(params[:isa_assay][:input_sample_type_id])
+    previous_assay = previous_sample_type.assays.first
+
+    # In case an assay is inserted right at the beginning of an assay stream,
+    # the next assay is the current first one in the assay stream.
+    next_assay = previous_assay.nil? ? @isa_assay.assay.assay_stream.next_linked_child_assay : previous_assay.next_linked_child_assay
+
+    # In case no next assay (an assay was appended to the end of the stream), assay linkage does not have to be fixed.
+    return unless next_assay
+
+    next_assay_input_attribute_id = next_assay.sample_type.sample_attributes.detect(&:input_attribute?).id
+    return unless next_assay_input_attribute_id
+
+    # Add link of next assay sample type to currently created assay sample type
+    updated_lsai = @isa_assay.assay.sample_type.linked_sample_attribute_ids.push(next_assay_input_attribute_id)
+    @isa_assay.assay.sample_type.update(linked_sample_attribute_ids: updated_lsai)
+  end
+
+  def rearrange_assay_positions_create_isa_assay
+    return if @isa_assay.assay.is_assay_stream?
+    return unless @isa_assay.assay.is_isa_json_compliant?
+
+    rearrange_assay_positions(@isa_assay.assay.assay_stream)
+  end
+
+  def initialize_isa_assay
+    @isa_assay = IsaAssay.new(isa_assay_params)
+    update_sharing_policies @isa_assay.assay
+    @isa_assay.assay.contributor = current_person
+    @isa_assay.sample_type.contributor = User.current_user.person if isa_assay_params[:sample_type]
+  end
+
   def isa_assay_params
     # TODO: get the params from a shared module
     isa_assay_params = params.require(:isa_assay).permit(

app/controllers/studies_controller.rb

@@ -89,7 +89,7 @@ def update
   end
 
   def delete_linked_sample_types
-    return unless is_single_page_study?
+    return unless @study.is_isa_json_compliant?
     return if @study.sample_types.empty?
 
     # The study sample types must be destroyed in reversed order
@@ -361,9 +361,3 @@ def study_params
                                   { extended_metadata_attributes: determine_extended_metadata_keys })
   end
 end
-
-def is_single_page_study?
-  return false unless params.key?(:return_to)
-
-  params[:return_to].start_with? '/single_pages/'
-end

app/forms/isa_assay.rb

@@ -56,6 +56,12 @@ def populate(id)
   def validate_objects
     @assay.errors.each { |e| errors.add(:base, "[Assay]: #{e.full_message}") } unless @assay.valid?
 
+    if @assay.new_record? && @assay.next_linked_child_assay&.sample_type&.samples&.any?
+      next_assay_id = @assay.next_linked_child_assay.id
+      next_assay_title = @assay.next_linked_child_assay.title
+      errors.add(:base, "[Assay]: Not allowed to create an assay before assay '#{next_assay_id} - #{next_assay_title}'. It has samples linked to it.")
+    end
+
     return if @assay.is_assay_stream?
 
     errors.add(:base, '[Assay]: The assay is missing a sample type.') if @sample_type.nil?

app/helpers/dynamic_table_helper.rb

@@ -22,7 +22,7 @@ def dt_aggregated(study, assay = nil)
   # Links all sample_types in a sequence of sample_types
   def link_sequence(sample_type)
     sequence = [sample_type]
-    while link = sample_type.sample_attributes.detect(&:seek_sample_multi?)&.linked_sample_type
+    while link = sample_type.previous_linked_sample_type
       sequence << link
       sample_type = link
     end
@@ -96,8 +96,8 @@ def dt_cols(sample_type)
       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!({ 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/models/assay.rb

@@ -75,10 +75,10 @@ def is_assay_stream?
   def previous_linked_sample_type
     return unless is_isa_json_compliant?
 
-    if is_assay_stream?
+    if is_assay_stream? || first_assay_in_stream?
       study.sample_types.second
     else
-      sample_type.sample_attributes.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.linked_sample_type
+      sample_type.previous_linked_sample_type
     end
   end
 
@@ -96,12 +96,24 @@ def next_linked_child_assay
     return unless has_linked_child_assay?
 
     if is_assay_stream?
-      previous_linked_sample_type&.linked_sample_attributes&.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.sample_type&.assays&.first
+      first_assay_in_stream
+    else
+      sample_type.next_linked_sample_types.map(&:assays).flatten.detect { |a| a.assay_stream_id == assay_stream_id }
+    end
+  end
+
+  def first_assay_in_stream
+    if is_assay_stream?
+      child_assays.detect { |a| a.sample_type.previous_linked_sample_type == a.study.sample_types.second }
     else
-      sample_type.linked_sample_attributes.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.sample_type&.assays&.first
+      assay_stream.child_assays.detect { |a| a.sample_type.previous_linked_sample_type == a.study.sample_types.second }
     end
   end
 
+  def first_assay_in_stream?
+    self == first_assay_in_stream
+  end
+
   def default_contributor
     User.current_user.try :person
   end

app/models/sample_attribute.rb

@@ -25,6 +25,10 @@ class SampleAttribute < ApplicationRecord
   # whether this attribute is tied to a controlled vocab which is based on an ontology
   delegate :ontology_based?, to: :sample_controlled_vocab, allow_nil: true
 
+  def input_attribute?
+    isa_tag.nil? && title.downcase.include?('input') && seek_sample_multi?
+  end
+
   def title=(title)
     super
     store_accessor_name

app/models/sample_type.rb

@@ -60,6 +60,14 @@ class SampleType < ApplicationRecord
 
   has_annotation_type :sample_type_tag, method_name: :tags
 
+  def previous_linked_sample_type
+    sample_attributes.detect(&:input_attribute?)&.linked_sample_type
+  end
+
+  def next_linked_sample_types
+    linked_sample_attributes.select(&:input_attribute?).map(&:sample_type).compact
+  end
+
   def is_isa_json_compliant?
     studies.any? || assays.any?
   end
@@ -120,11 +128,17 @@ def can_edit?(user = User.current_user)
   end
 
   def can_delete?(user = User.current_user)
-    can_edit?(user) && samples.empty? &&
-      linked_sample_attributes.detect do |attr|
-        attr.sample_type &&
-          attr.sample_type != self
-      end.nil?
+    # Users should be able to delete an ISA JSON compliant sample type that has linked sample attributes,
+    # as long as it's ISA JSON compliant.
+    if is_isa_json_compliant?
+      can_edit?(user) && samples.empty?
+    else
+      can_edit?(user) && samples.empty? &&
+        linked_sample_attributes.detect do |attr|
+          attr.sample_type &&
+            attr.sample_type != self
+        end.nil?
+    end
   end
 
   def can_view?(user = User.current_user, referring_sample = nil, view_in_single_page = false)

app/views/assays/_buttons.html.erb

@@ -6,6 +6,23 @@
   else
     t("assays.#{item.assay_class.long_key.delete(' ').underscore}")
   end
+
+  isa_assay_verb ||=
+    if item&.is_assay_stream?
+      if item.next_linked_child_assay
+        "Insert a new"
+      else
+        "Design"
+      end
+    else
+      if item.next_linked_child_assay
+        "Insert a new"
+      else
+        "Design the next"
+      end
+    end
+
+  hide_new_assays_button = item.next_linked_child_assay&.sample_type&.samples&.any?
 %>
 <%= render :partial => "subscriptions/subscribe", :locals => {:object => item} %>
 
@@ -32,10 +49,14 @@
 		<% valid_study = item&.study&.is_isa_json_compliant? %>
 		<% valid_assay = item&.is_isa_json_compliant? %>
 		<% if valid_study && valid_assay %>
-      <% if item&.is_assay_stream? %>
-    	  <%= button_link_to("Design #{t('assay')}", 'new', new_isa_assay_path(source_assay_id: item.id, study_id: item.study.id, single_page: params[:single_page], assay_stream_id: item.id)) %>
+      <% if hide_new_assays_button %>
+        <%= button_link_to("#{isa_assay_verb} #{t('assay')}", 'new', nil, disabled_reason: 'The next linked assay has samples. Cannot insert new assay here.') %>
       <% else %>
-    	  <%= button_link_to("Design the next #{t('assay')}", 'new', new_isa_assay_path(source_assay_id: item.id, study_id: item.study.id, single_page: params[:single_page], assay_stream_id: item.assay_stream_id)) %>
+        <% if item&.is_assay_stream? %>
+            <%= button_link_to("#{isa_assay_verb} #{t('assay')}", 'new', new_isa_assay_path(source_assay_id: item.id, study_id: item.study.id, single_page: params[:single_page], assay_stream_id: item.id)) %>
+        <% else %>
+          <%= button_link_to("#{isa_assay_verb} #{t('assay')}", 'new', new_isa_assay_path(source_assay_id: item.id, study_id: item.study.id, single_page: params[:single_page], assay_stream_id: item.assay_stream_id)) %>
+        <% end %>
       <% end %>
 		<% end %>
 	<% else %>

app/views/isa_assays/_form.html.erb

@@ -1,16 +1,20 @@
 <% assay = params[:isa_assay][:assay] if params.dig(:isa_assay, :assay) %>
 <% study = Study.find(params[:study_id] || assay[:study_id]) %>
+<% def first_assay_in_stream?
+    params[:source_assay_id].nil? || (params[:source_assay_id] == params[:assay_stream_id])
+   end
+%>
 <%
   source_assay = Assay.find(params[:source_assay_id]) if params[:source_assay_id]
   assay_stream_id = params[:assay_stream_id] if params[:assay_stream_id]
 
   if @isa_assay.assay.new_record?
     if params[:is_assay_stream]
-      assay_position = 0
+      assay_position = study.assay_streams.any? ? study.assay_streams.map(&:position).max + 1 : 0
       assay_class_id = AssayClass.assay_stream.id
       is_assay_stream = true
     else
-      assay_position = params[:source_assay_id].nil? ? 1 : source_assay.position + 1
+      assay_position = first_assay_in_stream? ? 0 : source_assay.position + 1
       assay_class_id = AssayClass.experimental.id
       is_assay_stream = false
     end
@@ -48,7 +52,7 @@
 
   <div class="form-group">
     <%= assay_fields.label :description -%><br/>
-    <%= assay_fields.text_area :description, :rows => 5, :class=>"form-control rich-text-edit"  -%>
+    <%= assay_fields.text_area :description, rows: 5, class: "form-control rich-text-edit" -%>
   </div>
 
   <% if show_extended_metadata %>
@@ -61,9 +65,17 @@
     <%= assay_study_selection('isa_assay[assay][study_id]',@isa_assay.assay.study) %>
   </div>
 
-  <div class="hidden">
-    <%= assay_fields.number_field :position, value: assay_position || study.assays.length -%>
-  </div>
+  <% if is_assay_stream %>
+    <div class="form-group">
+      <%= assay_fields.label "Assay position" -%><br/>
+      <%= assay_fields.number_field :position, rows: 5, class: "form-control", value: assay_position %>
+    </div>
+  <% else %>
+    <div class="hidden">
+      <%= assay_fields.hidden_field :position, value: assay_position -%>
+    </div>
+  <% end %>
+
 
   <%= assay_fields.hidden_field :assay_stream_id, value: assay_stream_id -%>
   <%= assay_fields.hidden_field :assay_class_id, value: assay_class_id -%>