diff --git a/app/controllers/assays_controller.rb b/app/controllers/assays_controller.rb
index 66236dd6c9..0c88fabba4 100644
--- a/app/controllers/assays_controller.rb
+++ b/app/controllers/assays_controller.rb
@@ -1,16 +1,17 @@
class AssaysController < ApplicationController
-
include Seek::IndexPager
include Seek::AssetsCommon
before_action :assays_enabled?
- before_action :find_assets, :only=>[:index]
- before_action :find_and_authorize_requested_item, :only=>[:edit, :update, :destroy, :manage, :manage_update, :show, :new_object_based_on_existing_one]
+ 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]
+ # 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]
include Seek::Publishing::PublishingCommon
@@ -30,69 +31,63 @@ def new_object_based_on_existing_one
end
@existing_assay.data_files.each do |d|
- if !d.can_view?
+ unless d.can_view?
notice_message << "Some or all #{t('data_file').pluralize} of the existing #{t('assays.assay')} cannot be viewed, you may specify your own!
"
break
end
end
@existing_assay.sops.each do |s|
- if !s.can_view?
- notice_message << "Some or all #{t('sop').pluralize} of the existing #{ t('assays.assay')} cannot be viewed, you may specify your own!
"
+ unless s.can_view?
+ notice_message << "Some or all #{t('sop').pluralize} of the existing #{t('assays.assay')} cannot be viewed, you may specify your own!
"
break
end
end
@existing_assay.models.each do |m|
- if !m.can_view?
+ unless m.can_view?
notice_message << "Some or all #{t('model').pluralize} of the existing #{t('assays.assay')} cannot be viewed, you may specify your own!
"
break
end
end
@existing_assay.documents.each do |d|
- if !d.can_view?
+ unless d.can_view?
notice_message << "Some or all #{t('document').pluralize} of the existing #{t('assays.assay')} cannot be viewed, you may specify your own!
"
break
end
end
- unless notice_message.blank?
- flash.now[:notice] = notice_message.html_safe
- end
+ flash.now[:notice] = notice_message.html_safe unless notice_message.blank?
- render :action=>"new"
+ render action: 'new'
else
- flash[:error]="You do not have the necessary permissions to copy this #{t('assays.assay')}"
+ flash[:error] = "You do not have the necessary permissions to copy this #{t('assays.assay')}"
redirect_to @existing_assay
end
end
def new
- @assay=setup_new_asset
- @assay_class=params[:class]
+ @assay = setup_new_asset
+ @assay_class = params[:class]
@permitted_params = assay_params if params[:assay]
- #jump straight to experimental if modelling analysis is disabled
- @assay_class ||= "experimental" unless Seek::Config.modelling_analysis_enabled
+ # jump straight to experimental if modelling analysis is disabled
+ @assay_class ||= 'experimental' unless Seek::Config.modelling_analysis_enabled
- @assay.assay_class=AssayClass.for_type(@assay_class) unless @assay_class.nil?
+ @assay.assay_class = AssayClass.for_type(@assay_class) unless @assay_class.nil?
- respond_to do |format|
- format.html
- end
+ respond_to(&:html)
end
def edit
- respond_to do |format|
- format.html
- end
+ respond_to(&:html)
end
def create
- params[:assay_class_id] ||= AssayClass.for_type("experimental").id
+ params[:assay_class_id] ||= AssayClass.for_type('experimental').id
@assay = Assay.new(assay_params)
update_assay_organisms @assay, params
update_assay_human_diseases @assay, params
- @assay.contributor=current_person
+ @assay.contributor = current_person
update_sharing_policies @assay
update_annotations(params[:tag_list], @assay)
update_relationships(@assay, params)
@@ -101,17 +96,16 @@ def create
respond_to do |format|
flash[:notice] = "#{t('assays.assay')} was successfully created."
format.html { redirect_to(@assay) }
- format.json {render json: @assay, include: [params[:include]]}
+ format.json { render json: @assay, include: [params[:include]] }
end
else
respond_to do |format|
- format.html { render :action => "new", status: :unprocessable_entity }
+ format.html { render action: 'new', status: :unprocessable_entity }
format.json { render json: json_api_errors(@assay), status: :unprocessable_entity }
end
end
end
-
def delete_linked_sample_types
return unless is_single_page_assay?
return if @assay.sample_type.nil?
@@ -119,6 +113,23 @@ def delete_linked_sample_types
@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_assay_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
@@ -130,26 +141,26 @@ def update
if @assay.update(assay_params)
flash[:notice] = "#{t('assays.assay')} was successfully updated."
format.html { redirect_to(@assay) }
- format.json {render json: @assay, include: [params[:include]]}
+ format.json { render json: @assay, include: [params[:include]] }
else
- format.html { render :action => "edit", status: :unprocessable_entity }
+ format.html { render action: 'edit', status: :unprocessable_entity }
format.json { render json: json_api_errors(@assay), status: :unprocessable_entity }
end
end
end
- def update_assay_organisms assay,params
+ def update_assay_organisms(assay, params)
organisms = params[:assay_organism_ids] || params[:assay][:organism_ids] || []
assay.assay_organisms = [] # This means new AssayOrganisms are created every time the assay is updated!
Array(organisms).each do |text|
# TODO: Refactor this to use proper nested params:
- o_id, strain,strain_id,culture_growth_type_text,t_id,t_title=text.split(",")
- culture_growth=CultureGrowthType.find_by_title(culture_growth_type_text)
- assay.associate_organism(o_id, strain_id, culture_growth,t_id,t_title)
+ o_id, strain, strain_id, culture_growth_type_text, t_id, t_title = text.split(',')
+ culture_growth = CultureGrowthType.find_by_title(culture_growth_type_text)
+ assay.associate_organism(o_id, strain_id, culture_growth, t_id, t_title)
end
end
- def update_assay_human_diseases assay,params
+ def update_assay_human_diseases(assay, params)
human_diseases = params[:assay_human_disease_ids] || params[:assay][:human_disease_ids] || []
assay.assay_human_diseases = []
Array(human_diseases).each do |human_disease_id|
@@ -159,10 +170,9 @@ def update_assay_human_diseases assay,params
def show
respond_to do |format|
- format.html { render(params[:only_content] ? { layout: false } : {})}
- format.rdf { render :template=>'rdf/show'}
- format.json {render json: @assay, include: [params[:include]]}
-
+ format.html { render(params[:only_content] ? { layout: false } : {}) }
+ format.rdf { render template: 'rdf/show' }
+ format.json { render json: @assay, include: [params[:include]] }
end
end
@@ -170,11 +180,11 @@ def show
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: []},
+ :license, *creator_related_params, :position, { document_ids: [] },
{ sop_ids: [] }, { model_ids: [] },
- { samples_attributes: [:asset_id, :direction] },
- { data_files_attributes: [:asset_id, :direction, :relationship_type_id] },
- { placeholders_attributes: [:asset_id, :direction, :relationship_type_id] },
+ { samples_attributes: %i[asset_id direction] },
+ { data_files_attributes: %i[asset_id direction relationship_type_id] },
+ { placeholders_attributes: %i[asset_id direction relationship_type_id] },
{ publication_ids: [] },
{ extended_metadata_attributes: determine_extended_metadata_keys },
{ discussion_links_attributes:[:id, :url, :label, :_destroy] }
diff --git a/app/models/assay.rb b/app/models/assay.rb
index 13b2b9279e..11805ed5d8 100644
--- a/app/models/assay.rb
+++ b/app/models/assay.rb
@@ -1,10 +1,9 @@
class Assay < ApplicationRecord
-
include Seek::Ontologies::AssayOntologyTypes
- enum status: [:planned, :running, :completed, :cancelled, :failed]
+ enum status: %i[planned running completed cancelled failed]
belongs_to :assignee, class_name: 'Person'
-
+
# needs to before acts_as_isa - otherwise auto_index=>false is overridden by Seek::Search::CommonFields
if Seek::Config.solr_enabled
searchable(auto_index: false) do
@@ -24,7 +23,7 @@ class Assay < ApplicationRecord
acts_as_isa
acts_as_snapshottable
- belongs_to :sample_type
+ belongs_to :sample_type
belongs_to :assay_class
has_many :assay_organisms, dependent: :destroy, inverse_of: :assay
@@ -49,9 +48,9 @@ class Assay < ApplicationRecord
has_one :investigation, through: :study
has_one :external_asset, as: :seek_entity, dependent: :destroy
- validates :assay_type_uri, presence:true
+ validates :assay_type_uri, presence: true
validates_with AssayTypeUriValidator
- validates :technology_type_uri, absence:true, if: :is_modelling?
+ validates :technology_type_uri, absence: true, if: :is_modelling?
validates_with TechnologyTypeUriValidator
validates_presence_of :contributor
validates_presence_of :assay_class
@@ -66,6 +65,14 @@ class Assay < ApplicationRecord
enforce_authorization_on_association :study, :view
+ def previous_linked_assay_sample_type
+ sample_type.sample_attributes.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.linked_sample_type
+ end
+
+ def has_linked_child_assay?
+ sample_type&.linked_sample_attributes&.any?
+ end
+
def default_contributor
User.current_user.try :person
end
@@ -91,7 +98,7 @@ def is_experimental?
end
def associated_samples_through_sample_type
- (sample_type.nil? || sample_type.samples.nil?) ? [] : sample_type.samples
+ sample_type.nil? || sample_type.samples.nil? ? [] : sample_type.samples
end
# Create or update relationship of this assay to another, with a specific relationship type and version
@@ -103,9 +110,7 @@ def associate(asset, options = {})
else
assay_asset = assay_assets.detect { |aa| aa.asset == asset }
- if assay_asset.nil?
- assay_asset = assay_assets.build
- end
+ assay_asset = assay_assets.build if assay_asset.nil?
assay_asset.asset = asset
assay_asset.version = asset.version if asset && asset.respond_to?(:version)
@@ -124,12 +129,12 @@ def associate(asset, options = {})
end
def self.simple_associated_asset_types
- [:models, :sops, :publications, :documents]
+ %i[models sops publications documents]
end
# Associations where there is additional metadata on the association, i.e. `direction`
def self.complex_associated_asset_types
- [:data_files, :samples, :placeholders]
+ %i[data_files samples placeholders]
end
def assets
@@ -166,7 +171,9 @@ def avatar_key
def clone_with_associations
new_object = dup
new_object.policy = policy.deep_copy
- new_object.assay_assets = assay_assets.select { |aa| self.class.complex_associated_asset_types.include?(aa.asset_type.underscore.pluralize.to_sym) }.map(&:dup)
+ new_object.assay_assets = assay_assets.select do |aa|
+ self.class.complex_associated_asset_types.include?(aa.asset_type.underscore.pluralize.to_sym)
+ end.map(&:dup)
self.class.simple_associated_asset_types.each do |type|
new_object.send("#{type}=", try(type))
end
@@ -187,7 +194,8 @@ def human_disease_terms
# organism may be either an ID or Organism instance
# strain_id should be the id of the strain
# culture_growth should be the culture growth instance
- def associate_organism(organism, strain_id = nil, culture_growth_type = nil, tissue_and_cell_type_id = nil, tissue_and_cell_type_title = nil)
+ def associate_organism(organism, strain_id = nil, culture_growth_type = nil, tissue_and_cell_type_id = nil,
+ tissue_and_cell_type_title = nil)
organism = Organism.find(organism) if organism.is_a?(Numeric) || organism.is_a?(String)
strain = organism.strains.find_by_id(strain_id)
tissue_and_cell_type = nil
@@ -197,22 +205,22 @@ def associate_organism(organism, strain_id = nil, culture_growth_type = nil, tis
tissue_and_cell_type = TissueAndCellType.where(title: tissue_and_cell_type_title).first_or_create!
end
- unless AssayOrganism.exists_for?(self, organism, strain, culture_growth_type, tissue_and_cell_type)
- assay_organism = AssayOrganism.new(assay: self, organism: organism, culture_growth_type: culture_growth_type,
- strain: strain, tissue_and_cell_type: tissue_and_cell_type)
- assay_organisms << assay_organism
- end
+ return if AssayOrganism.exists_for?(self, organism, strain, culture_growth_type, tissue_and_cell_type)
+
+ assay_organism = AssayOrganism.new(assay: self, organism:, culture_growth_type:,
+ strain:, tissue_and_cell_type:)
+ assay_organisms << assay_organism
end
# Associates a human disease with the assay
# human disease may be either an ID or HumanDisease instance
def associate_human_disease(human_disease)
human_disease = HumanDisease.find(human_disease) if human_disease.is_a?(Numeric) || human_disease.is_a?(String)
- assay_human_disease = AssayHumanDisease.new(assay: self, human_disease: human_disease)
+ assay_human_disease = AssayHumanDisease.new(assay: self, human_disease:)
- unless AssayHumanDisease.exists_for?(human_disease, self)
- assay_human_diseases << assay_human_disease
- end
+ return if AssayHumanDisease.exists_for?(human_disease, self)
+
+ assay_human_diseases << assay_human_disease
end
# overides that from Seek::RDF::RdfGeneration, as Assay entity depends upon the AssayClass (modelling, or experimental) of the Assay
@@ -224,15 +232,15 @@ def external_asset_search_terms
external_asset ? external_asset.search_terms : []
end
- def samples_attributes= attributes
+ def samples_attributes=(attributes)
set_assay_assets_for('Sample', attributes)
end
- def data_files_attributes= attributes
+ def data_files_attributes=(attributes)
set_assay_assets_for('DataFile', attributes)
end
- def placeholders_attributes= attributes
+ def placeholders_attributes=(attributes)
set_assay_assets_for('Placeholder', attributes)
end
@@ -243,7 +251,7 @@ def self.filter_by_projects(projects)
private
def set_assay_assets_for(type, attributes)
- type_assay_assets, other_assay_assets = self.assay_assets.partition { |aa| aa.asset_type == type }
+ type_assay_assets, other_assay_assets = assay_assets.partition { |aa| aa.asset_type == type }
new_type_assay_assets = []
attributes.each do |attrs|
@@ -252,15 +260,13 @@ def set_assay_assets_for(type, attributes)
if existing
new_type_assay_assets << existing.tap { |e| e.assign_attributes(attrs) }
else
- aa = self.assay_assets.build(attrs)
- if aa.asset && aa.asset.can_view?
- new_type_assay_assets << aa
- end
+ aa = assay_assets.build(attrs)
+ new_type_assay_assets << aa if aa.asset && aa.asset.can_view?
end
end
self.assay_assets = (other_assay_assets + new_type_assay_assets)
- self.assay_assets
+ assay_assets
end
def related_publication_ids
diff --git a/test/functional/assays_controller_test.rb b/test/functional/assays_controller_test.rb
index 968c31a910..1d05cc4a77 100644
--- a/test/functional/assays_controller_test.rb
+++ b/test/functional/assays_controller_test.rb
@@ -1923,7 +1923,12 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links
test 'should delete empty assay with linked sample type' do
person = FactoryBot.create(:person)
- assay_sample_type = FactoryBot.create :linked_sample_type, contributor: person
+ project = person.projects.first
+ source_st = FactoryBot.create(:isa_source_sample_type, contributor: person, projects: [project])
+ sample_collection_st = FactoryBot.create(:isa_sample_collection_sample_type, contributor: person, projects: [project],
+ linked_sample_type: source_st)
+
+ assay_sample_type = FactoryBot.create :isa_assay_sample_type, linked_sample_type: sample_collection_st, contributor: person, isa_template: Template.find_by_title('ISA Assay 1')
assay = FactoryBot.create(:assay,
policy:FactoryBot.create(:private_policy, permissions:[FactoryBot.create(:permission,contributor: person, access_type:Policy::EDITING)]),
sample_type: assay_sample_type,
@@ -1937,4 +1942,46 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links
end
end
end
+
+ test 'should fix sample type linkage when middle assay is deleted' do
+ # person = User.current_user.person
+ person = FactoryBot.create(:person)
+ project = person.projects.first
+ investigation = FactoryBot.create(:investigation, projects: [project])
+
+ source_st = FactoryBot.create(:isa_source_sample_type, contributor: person, projects: [project])
+ sample_collection_st = FactoryBot.create(:isa_sample_collection_sample_type, contributor: person, projects: [project],
+ linked_sample_type: source_st)
+
+ assay_st1 = FactoryBot.create(:isa_assay_sample_type, contributor: person, projects: [project],
+ linked_sample_type: sample_collection_st, isa_template: Template.find_by_title('ISA Assay 1'))
+
+ assay_st2 = FactoryBot.create(:isa_assay_sample_type, contributor: person, projects: [project],
+ linked_sample_type: assay_st1, isa_template: Template.find_by_title('ISA Assay 1'))
+
+ assay_st3 = FactoryBot.create(:isa_assay_sample_type, contributor: person, projects: [project],
+ linked_sample_type: assay_st2, isa_template: Template.find_by_title('ISA Assay 1'))
+
+ study = FactoryBot.create(:study, investigation:, contributor: person,
+ policy: FactoryBot.create(:private_policy, permissions: [FactoryBot.create(:permission, contributor: person, access_type: Policy::MANAGING)]),
+ sops: [FactoryBot.create(:sop, policy: FactoryBot.create(:public_policy))],
+ sample_types: [source_st, sample_collection_st])
+
+ assay1 = FactoryBot.create(:assay, study:, contributor: person, sample_type: assay_st1,
+ policy: FactoryBot.create(:private_policy, permissions: [FactoryBot.create(:permission, contributor: person, access_type: Policy::MANAGING)]))
+ assay2 = FactoryBot.create(:assay, study:, contributor: person, sample_type: assay_st2,
+ policy: FactoryBot.create(:private_policy, permissions: [FactoryBot.create(:permission, contributor: person, access_type: Policy::MANAGING)]))
+ assay3 = FactoryBot.create(:assay, study:, contributor: person, sample_type: assay_st3,
+ policy: FactoryBot.create(:private_policy, permissions: [FactoryBot.create(:permission, contributor: person, access_type: Policy::MANAGING)]))
+
+ login_as(person)
+
+ assert_difference "Assay.count", -1 do
+ delete :destroy, params: { id: assay2.id, return_to: '/single_pages/' }
+ end
+
+ assay3.reload
+
+ assert_equal(assay3.previous_linked_assay_sample_type&.id, assay1.sample_type&.id)
+ end
end