From 86279fc1d5008927bfef146f5d1ae274318068cd Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 15 Jan 2024 10:50:09 +0100 Subject: [PATCH 01/41] Add migration for assay streams --- db/migrate/20240112141513_add_assay_stream_id_to_assay.rb | 6 ++++++ db/schema.rb | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20240112141513_add_assay_stream_id_to_assay.rb diff --git a/db/migrate/20240112141513_add_assay_stream_id_to_assay.rb b/db/migrate/20240112141513_add_assay_stream_id_to_assay.rb new file mode 100644 index 0000000000..6220df0185 --- /dev/null +++ b/db/migrate/20240112141513_add_assay_stream_id_to_assay.rb @@ -0,0 +1,6 @@ +class AddAssayStreamIdToAssay < ActiveRecord::Migration[6.1] + def change + add_column :assays, :assay_stream_id, :integer + add_index :assays, :assay_stream_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 22e12632eb..2f51e08778 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2023_12_18_133053) do +ActiveRecord::Schema.define(version: 2024_01_12_141513) do create_table "activity_logs", id: :integer, force: :cascade do |t| t.string "action" @@ -189,6 +189,8 @@ t.string "deleted_contributor" t.integer "sample_type_id" t.integer "position" + t.integer "assay_stream_id" + t.index ["assay_stream_id"], name: "index_assays_on_assay_stream_id" t.index ["sample_type_id"], name: "index_assays_on_sample_type_id" end From 50e5728cc1b50eefe199a0df345dc6480b801ae7 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 15 Jan 2024 10:50:27 +0100 Subject: [PATCH 02/41] Add new Assay Class --- app/models/assay_class.rb | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index a7a72dc0fa..956488fbac 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -1,31 +1,38 @@ class AssayClass < ApplicationRecord - - #this returns an instance of AssayClass according to one of the types "experimental" or "modelling" - #if there is not a match nil is returned - def self.for_type type - keys={"experimental"=>"EXP","modelling"=>"MODEL"} - return AssayClass.find_by(key: keys[type]) + # this returns an instance of AssayClass according to one of the types "experimental" or "modelling" + # if there is not a match nil is returned + def self.for_type(type) + keys = { "experimental": 'EXP', "modelling": 'MODEL', 'assay_stream': 'ASS' } + AssayClass.find_by(key: keys[type]) end def self.experimental - self.for_type('experimental') + for_type('experimental') end def self.modelling - self.for_type('modelling') + for_type('modelling') + end + + def self.assay_stream + for_type('assay_stream') end def is_modelling? - key == "MODEL" + key == 'MODEL' end def is_experimental? key == 'EXP' end + def is_assay_stream? + key == 'ASS' + end + # for cases where a longer more descriptive key is useful, but can't rely on the title # which may have been changed over time def long_key - {'EXP'=>'Experimental Assay','MODEL'=>'Modelling Analysis'}[key] + { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'ASS': 'Assay Stream' }[key] end end From ada37ab192c1155f651ebd5bb73bc0f26cc19b06 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 15 Jan 2024 10:50:59 +0100 Subject: [PATCH 03/41] Add self-referencing association to assays --- app/models/assay.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/models/assay.rb b/app/models/assay.rb index e2fe2430fa..cb84abdbf4 100644 --- a/app/models/assay.rb +++ b/app/models/assay.rb @@ -25,6 +25,9 @@ class Assay < ApplicationRecord belongs_to :sample_type + has_many :child_assays, class_name: 'Assay', foreign_key: 'assay_stream_id' + belongs_to :assay_stream, class_name: 'Assay', optional: true + belongs_to :assay_class has_many :assay_organisms, dependent: :destroy, inverse_of: :assay has_many :organisms, through: :assay_organisms, inverse_of: :assays @@ -65,6 +68,10 @@ class Assay < ApplicationRecord enforce_authorization_on_association :study, :view + def is_assay_stream? + child_assays.any? + end + def previous_linked_assay_sample_type sample_type.sample_attributes.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.linked_sample_type end From 4109b53b1e5385cc9745ff59b4994a006ca14d7a Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 15 Jan 2024 13:26:33 +0100 Subject: [PATCH 04/41] Modify treeview to show extra assay stream level --- lib/treeview_builder.rb | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/treeview_builder.rb b/lib/treeview_builder.rb index d64e6f4217..3996b2da2b 100644 --- a/lib/treeview_builder.rb +++ b/lib/treeview_builder.rb @@ -14,9 +14,22 @@ def build_tree_data investigation_items = [] @project.investigations.map do |investigation| - investigation.studies.map do |study| - assay_items = study.assays.map { |assay| build_assay_item(assay) } - study_items << build_study_item(study, assay_items) + if investigation.is_isa_json_compliant? + investigation.studies.map do |study| + assay_stream_items = study.assays.select { |assay| assay.is_assay_stream? }.map do |assay_stream| + assay_items = assay_stream.child_assays.map do |child_assay| + build_assay_item(child_assay) + end + build_assay_stream_item(assay_stream, assay_items) + end + + study_items << build_study_item(study, assay_stream_items) + end + else + investigation.studies.map do |study| + assay_items = study.assays.map { |assay| build_assay_item(assay) } + study_items << build_study_item(study, assay_items) + end end investigation_items << build_investigation_item(investigation, study_items) study_items = [] @@ -125,6 +138,11 @@ def build_study_item(study, assay_items) children: isa_study_elements(study) + assay_items, resource: study }) end + def build_assay_stream_item(assay_stream, child_assays) + create_node({ text: assay_stream.title, _type: 'assay', label: 'Assay Stream', _id: assay_stream.id, a_attr: BOLD, + children: isa_assay_elements(assay_stream) + child_assays, resource: assay_stream }) + end + def build_assay_item(assay) create_node({ text: assay.title, _type: 'assay', label: 'Assay', _id: assay.id, a_attr: BOLD, children: isa_assay_elements(assay), resource: assay }) From d37ef6f4a642f2a695f60cbc99d72fd775e9ed07 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 16 Jan 2024 14:38:59 +0100 Subject: [PATCH 05/41] Change button text --- app/views/assays/_buttons.html.erb | 10 +++++++--- app/views/studies/_buttons.html.erb | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/views/assays/_buttons.html.erb b/app/views/assays/_buttons.html.erb index ac5b58e309..3d7659a054 100644 --- a/app/views/assays/_buttons.html.erb +++ b/app/views/assays/_buttons.html.erb @@ -21,10 +21,14 @@ <% if item.can_edit? %> <% if Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? %> - <% valid_study = item&.study&.sample_types.present? %> - <% valid_assay = item&.sample_type.present? %> + <% valid_study = item&.study&.is_isa_json_compliant? %> + <% valid_assay = item&.is_isa_json_compliant? %> <% if valid_study && valid_assay %> - <%= 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])) %> + <% 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])) %> + <% 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])) %> + <% end %> <% end %> <% else %> <%= add_new_item_to_dropdown(item) %> diff --git a/app/views/studies/_buttons.html.erb b/app/views/studies/_buttons.html.erb index b27edc214f..50e728dca8 100644 --- a/app/views/studies/_buttons.html.erb +++ b/app/views/studies/_buttons.html.erb @@ -21,7 +21,7 @@ <% if item.can_edit? -%> <% if Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? %> <% if item&.sample_types.present? %> - <%= button_link_to("Design #{t('assay')}", 'new', new_isa_assay_path(study_id: item.id, single_page: params[:single_page], is_source: true)) %> + <%= button_link_to("Design #{t('assay')} Stream", 'new', new_isa_assay_path(study_id: item.id, single_page: params[:single_page], is_source: true)) %> <% end -%> <% else -%> <%= add_new_item_to_dropdown(item) %> From ba27eb36490de5f56bc2ef20401c353d45e63cbf Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 16 Jan 2024 14:40:15 +0100 Subject: [PATCH 06/41] Add is_assay_stream function to assay model --- app/models/assay.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/assay.rb b/app/models/assay.rb index cb84abdbf4..ece4e750a1 100644 --- a/app/models/assay.rb +++ b/app/models/assay.rb @@ -69,7 +69,7 @@ class Assay < ApplicationRecord enforce_authorization_on_association :study, :view def is_assay_stream? - child_assays.any? + assay_class.is_assay_stream? end def previous_linked_assay_sample_type @@ -102,7 +102,7 @@ def state_allows_delete?(*args) end def is_isa_json_compliant? - investigation.is_isa_json_compliant? && !sample_type.nil? + investigation.is_isa_json_compliant? && (!sample_type.nil? || is_assay_stream?) end # returns true if this is a modelling class of assay From f09cd0d6de6a9d17eab326e8df9f919ddbf90005 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 11:28:55 +0100 Subject: [PATCH 07/41] Make child assays depend on the essay stream for deletion --- app/models/assay.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/assay.rb b/app/models/assay.rb index ece4e750a1..575afce5d5 100644 --- a/app/models/assay.rb +++ b/app/models/assay.rb @@ -25,7 +25,7 @@ class Assay < ApplicationRecord belongs_to :sample_type - has_many :child_assays, class_name: 'Assay', foreign_key: 'assay_stream_id' + has_many :child_assays, class_name: 'Assay', foreign_key: 'assay_stream_id', dependent: :destroy belongs_to :assay_stream, class_name: 'Assay', optional: true belongs_to :assay_class From 6d3e23287f6ed21771cb74c157d89c9a521ee384 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 11:38:02 +0100 Subject: [PATCH 08/41] Update the controller to handle assay streams --- app/controllers/isa_assays_controller.rb | 35 ++++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/app/controllers/isa_assays_controller.rb b/app/controllers/isa_assays_controller.rb index df0643ba43..8bdb451a7d 100644 --- a/app/controllers/isa_assays_controller.rb +++ b/app/controllers/isa_assays_controller.rb @@ -6,14 +6,18 @@ class IsaAssaysController < ApplicationController before_action :find_requested_item, only: %i[edit update] def new - @isa_assay = IsaAssay.new + if params[:is_assay_stream] + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'ASS').id } }) + else + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'EXP').id } }) + end 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 + @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!') @@ -27,7 +31,11 @@ def create def edit # let edit the assay if the sample_type is not authorized - @isa_assay.sample_type = nil unless requested_item_authorized?(@isa_assay.sample_type) + if @isa_assay.assay.is_assay_stream? + @isa_assay.sample_type = nil + else + @isa_assay.sample_type = nil unless requested_item_authorized?(@isa_assay.sample_type) + end respond_to do |format| format.html @@ -38,9 +46,11 @@ def update @isa_assay.assay.attributes = isa_assay_params[:assay] # update the sample_type - if requested_item_authorized?(@isa_assay.sample_type) - @isa_assay.sample_type.update(isa_assay_params[:sample_type]) - @isa_assay.sample_type.resolve_inconsistencies + unless @isa_assay.assay.is_assay_stream? + if requested_item_authorized?(@isa_assay.sample_type) + @isa_assay.sample_type.update(isa_assay_params[:sample_type]) + @isa_assay.sample_type.resolve_inconsistencies + end end if @isa_assay.save @@ -87,10 +97,12 @@ def assay_params { data_files_attributes: %i[asset_id direction relationship_type_id] }, { publication_ids: [] }, { extended_metadata_attributes: determine_extended_metadata_keys(:assay) }, - { discussion_links_attributes: %i[id url label _destroy] }] + { discussion_links_attributes: %i[id url label _destroy] }, :assay_stream_id] end def sample_type_params(params) + return [] unless params[:sample_type] + attributes = params[:sample_type][:sample_attributes] if attributes params[:sample_type][:sample_attributes_attributes] = [] @@ -128,9 +140,16 @@ def set_up_instance_variable end def find_requested_item - @isa_assay = IsaAssay.new + if params[:is_assay_stream] + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'ASS').id } }) + else + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'EXP').id } }) + end @isa_assay.populate(params[:id]) + # Should not deal with sample type if assay has assay_class assay stream + return if @isa_assay.assay.is_assay_stream? + if @isa_assay.sample_type.nil? || !requested_item_authorized?(@isa_assay.assay) flash[:error] = "You are not authorized to edit this #{t('isa_assay')}" flash[:error] = 'Resource not found.' if @isa_assay.sample_type.nil? From 79379c954f403428dd21e0e11be94b8f3924bf82 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 14:39:29 +0100 Subject: [PATCH 09/41] Modify helper function to accept alternative names --- app/helpers/images_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/helpers/images_helper.rb b/app/helpers/images_helper.rb index a126c4a054..bc23201ea8 100644 --- a/app/helpers/images_helper.rb +++ b/app/helpers/images_helper.rb @@ -70,8 +70,8 @@ def append_size_parameter(url, size) url end - def delete_icon(model_item, user, confirm_msg='Are you sure?') - item_name = text_for_resource model_item + def delete_icon(model_item, user, confirm_msg='Are you sure?', alternative_item_name=nil) + item_name = alternative_item_name.nil? ? (text_for_resource model_item) : alternative_item_name if model_item.can_delete?(user) fullURL = url_for(model_item) From 9cc63e3f71aa512f491115b8dc5060a8a3d68b42 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 14:40:01 +0100 Subject: [PATCH 10/41] Modify buttons for assay streams --- app/views/assays/_buttons.html.erb | 26 ++++++++++++++++++++------ app/views/studies/_buttons.html.erb | 9 +++++++-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/app/views/assays/_buttons.html.erb b/app/views/assays/_buttons.html.erb index 3d7659a054..e247754156 100644 --- a/app/views/assays/_buttons.html.erb +++ b/app/views/assays/_buttons.html.erb @@ -1,4 +1,14 @@ -<% assay_word = item.is_modelling? ? t('assays.modelling_analysis') : t('assays.assay') %> +<% assay_word ||= + if item.is_modelling? + t('assays.modelling_analysis') + elsif item.is_assay_stream? + 'Assay Stream' + elsif Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? + t('isa_assay') + else + t('assays.assay') + end +%> <%= render :partial => "subscriptions/subscribe", :locals => {:object => item} %> <% if Seek::Config.project_single_page_enabled %> @@ -25,9 +35,9 @@ <% 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])) %> + <%= 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)) %> <% 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])) %> + <%= 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)) %> <% end %> <% end %> <% else %> @@ -42,7 +52,11 @@ <%= item_actions_dropdown do %> <% if item.can_edit? %> <% if Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? %> -
  • <%= image_tag_for_key('edit', edit_isa_assay_path(item, source_assay_id: item.id, study_id: item.study.id, single_page: params[:single_page]), "Edit #{t('isa_assay')}", nil, "Edit #{t('isa_assay')}") -%>
    • + <% if item&.is_assay_stream? %> +
  • <%= image_tag_for_key('edit', edit_isa_assay_path(item, source_assay_id: item.id, study_id: item.study.id, single_page: params[:single_page], is_assay_stream: true), "Edit #{assay_word}", nil, "Edit #{assay_word}") -%>
    • + <% else %> +
  • <%= image_tag_for_key('edit', edit_isa_assay_path(item, source_assay_id: item.id, study_id: item.study.id, single_page: params[:single_page], assay_stream_id: item.assay_stream_id), "Edit #{assay_word}", nil, "Edit #{assay_word}") -%>
    • + <% end %> <% else %>
  • <%= image_tag_for_key('edit', edit_assay_path(item), "Edit #{assay_word}", nil, "Edit #{assay_word}") -%>
    • <% end %> @@ -50,8 +64,8 @@ <% if item.can_manage? -%>
  • <%= image_tag_for_key('manage', manage_assay_path(item), "Manage #{assay_word}", nil, "Manage #{assay_word}") -%>
    • - <%= render :partial => 'snapshots/new_snapshot_link', :locals => {:item => item} %> + <%= render partial: 'snapshots/new_snapshot_link', locals: {item: item} %> <% end -%> - <%= delete_icon(item, current_user) %> + <%= delete_icon(item, current_user, 'Are you sure?', assay_word) %> <% end %> diff --git a/app/views/studies/_buttons.html.erb b/app/views/studies/_buttons.html.erb index 50e728dca8..4b63c58c86 100644 --- a/app/views/studies/_buttons.html.erb +++ b/app/views/studies/_buttons.html.erb @@ -21,7 +21,7 @@ <% if item.can_edit? -%> <% if Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? %> <% if item&.sample_types.present? %> - <%= button_link_to("Design #{t('assay')} Stream", 'new', new_isa_assay_path(study_id: item.id, single_page: params[:single_page], is_source: true)) %> + <%= button_link_to("Design #{t('assay')} Stream", 'new', new_isa_assay_path(study_id: item.id, single_page: params[:single_page], is_assay_stream: true)) %> <% end -%> <% else -%> <%= add_new_item_to_dropdown(item) %> @@ -38,7 +38,12 @@ <% end %> <% if item.can_manage? -%> -
  • <%= image_tag_for_key('manage', manage_study_path(item), "Manage #{t('study')}", nil, "Manage #{t('study')}") -%>
    • + <% if Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? %> +
  • <%= image_tag_for_key('manage', manage_study_path(item), "Manage #{t('study')}", nil, "Manage #{t('isa_study')}") -%>
    • + <% else %> +
  • <%= image_tag_for_key('manage', manage_study_path(item), "Manage #{t('study')}", nil, "Manage #{t('study')}") -%>
    • + <% end %> + <%= render :partial => 'snapshots/new_snapshot_link', :locals => { :item => item } %> <% end -%> From c6fdcb7f3b72bf58b66abbedf15bdb8c6362f439 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 14:43:02 +0100 Subject: [PATCH 11/41] Change model for assay streams --- app/forms/isa_assay.rb | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/app/forms/isa_assay.rb b/app/forms/isa_assay.rb index d5817e0051..a586c75851 100644 --- a/app/forms/isa_assay.rb +++ b/app/forms/isa_assay.rb @@ -3,29 +3,31 @@ class IsaAssay attr_accessor :assay, :sample_type, :input_sample_type_id - validates_presence_of :assay, :sample_type, :input_sample_type_id + validates_presence_of :assay validate :validate_objects def initialize(params = {}) @assay = Assay.new(params[:assay] || {}) - @sample_type = SampleType.new((params[:sample_type] || {}).merge({ project_ids: @assay.project_ids })) - @sample_type.sample_attributes.build(is_title: true, required: true) unless params[:sample_type] - @assay.sample_type = @sample_type - @assay.assay_class = AssayClass.for_type('experimental') + unless @assay.is_assay_stream? + @sample_type = SampleType.new((params[:sample_type] || {}).merge({ project_ids: @assay.project_ids })) + @sample_type.sample_attributes.build(is_title: true, required: true) unless params[:sample_type] + @assay.sample_type = @sample_type + end + @input_sample_type_id = params[:input_sample_type_id] end def save if valid? - if @assay.new_record? + if @assay.new_record? && !@assay.is_assay_stream? # connect the sample type multi link attribute to the last sample type of the assay's study input_attribute = @sample_type.sample_attributes.detect(&:seek_sample_multi?) input_attribute.linked_sample_type_id = @input_sample_type_id title = SampleType.find(@input_sample_type_id).sample_attributes.detect(&:is_title).title input_attribute.title = "Input (#{title})" end + @sample_type.save unless @assay.is_assay_stream? @assay.save - @sample_type.save else false end @@ -54,6 +56,12 @@ def populate(id) def validate_objects @assay.errors.each { |e| errors.add(:base, "[Assay]: #{e.full_message}") } unless @assay.valid? + return if @assay.is_assay_stream? + + errors.add(:base, '[Assay]: The assay is missing a sample type.') if @sample_type.nil? + + return unless @sample_type + @sample_type.errors.full_messages.each { |e| errors.add(:base, "[Sample type]: #{e}") } unless @sample_type.valid? unless @sample_type.sample_attributes.any?(&:seek_sample_multi?) @@ -66,22 +74,22 @@ def validate_objects unless @sample_type.sample_attributes.select { |a| a.title.include?('Input') && a.isa_tag.nil? }.one? errors.add(:base, - "[Sample type]: Should have exactly one attribute with the title 'Input' and no ISA tag".html_safe) + "[Sample type]: Should have exactly one attribute with the title 'Input' and no ISA tag".html_safe) end if @sample_type.sample_attributes.select { |a| !a.title.include?('Input') && a.isa_tag.nil? }.any? errors.add(:base, - "[Sample type]: All attributes should have an ISA Tag except for the 'Input' attribute (hidden)".html_safe) + "[Sample type]: All attributes should have an ISA Tag except for the 'Input' attribute (hidden)".html_safe) end assay_sample_or_datafile_attributes = @sample_type.sample_attributes.select do |a| a.isa_tag&.isa_other_material? || a.isa_tag&.isa_data_file? end + unless assay_sample_or_datafile_attributes.one? errors.add(:base, - "[Sample type]: Should have exactly one attribute with the 'data_file' or 'other_material' ISA tag selected".html_safe) + "[Sample type]: Should have exactly one attribute with the 'data_file' or 'other_material' ISA tag selected".html_safe) end - errors.add(:base, '[Input Assay]: Input Assay is not provided') if @input_sample_type_id.blank? end end From b196d09101de201da47b4e0d932fefaff550a95f Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 15:03:34 +0100 Subject: [PATCH 12/41] Modify form for handling assay streams --- app/views/isa_assays/_form.html.erb | 62 ++++++++++++++++------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/app/views/isa_assays/_form.html.erb b/app/views/isa_assays/_form.html.erb index 35d5b38d2c..13a77cced1 100644 --- a/app/views/isa_assays/_form.html.erb +++ b/app/views/isa_assays/_form.html.erb @@ -1,35 +1,40 @@ <% assay = params[:isa_assay][:assay] if params.dig(:isa_assay, :assay) %> <% study = Study.find(params[:study_id] || assay[:study_id]) %> <% -input_sample_type_id = params[:isa_assay][:input_sample_type_id] if params.dig(:isa_assay, :input_sample_type_id) -source_assay = Assay.find(params[:source_assay_id]) if params[:source_assay_id] - -if @isa_assay.assay.new_record? - if params[:source_assay_id] - assay_position = source_assay.position + 1 - else - assay_position = 0 - end -else - assay_position = @isa_assay.assay.position -end - -# assay_position = params[:source_assay_id] ? source_assay.position + 1 : 0 -input_sample_type_id ||= - if params[:is_source] - study.sample_types.second.id + 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_class_id = AssayClass.find_by(key: 'ASS')&.id + is_assay_stream = true + else + assay_position = params[:source_assay_id].nil? ? 1 : source_assay.position + 1 + assay_class_id = AssayClass.find_by(key: 'EXP')&.id + is_assay_stream = false + end else - source_assay.sample_type.id if params[:source_assay_id] + assay_position = @isa_assay.assay.position + assay_class_id = @isa_assay.assay.assay_class_id + is_assay_stream = @isa_assay.assay.is_assay_stream? end -show_extended_metadata = - if params[:is_source] - true - elsif source_assay&.position&.zero? && !@isa_assay.assay.new_record? - true # Custom metadata should be shown in edit as well if assay position is 0. - else - false - end + input_sample_type_id ||= + if is_assay_stream || source_assay&.assay_class&.is_assay_stream? + study.sample_types.second.id + else + source_assay.sample_type.id if source_assay + end + + show_extended_metadata = + if is_assay_stream + true + elsif source_assay&.position&.zero? && !@isa_assay.assay.new_record? + true # Custom metadata should be shown in edit as well if assay position is 0. + else + false + end %> <%= error_messages_for :isa_assay %> @@ -60,7 +65,8 @@ show_extended_metadata = <%= assay_fields.number_field :position, value: assay_position || study.assays.length -%> - <%= assay_fields.hidden_field :assay_class_id -%> + <%= assay_fields.hidden_field :assay_stream_id, value: assay_stream_id -%> + <%= assay_fields.hidden_field :assay_class_id, value: assay_class_id -%> <% if User.current_user -%> <%= render partial: 'assets/manage_specific_attributes', locals:{f:assay_fields} if show_form_manage_specific_attributes? %> @@ -75,7 +81,7 @@ show_extended_metadata = <%= f.hidden_field :input_sample_type_id, value: input_sample_type_id -%> -<% if @isa_assay.sample_type %> +<% unless is_assay_stream %> <%= folding_panel("Define #{t(:sample_type)} for #{t(:assay)}") do %> <%= render partial: 'isa_studies/sample_types_form', locals: {f: f, sample_type: @isa_assay.sample_type, id_suffix: "_sample_type", isa_element: "assay", action: action} %> <% end %> From dcb5794812e3fd4e2d08e53951a483bcf7e6b78b Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 15:23:52 +0100 Subject: [PATCH 13/41] Seed Assay Stream class --- db/seeds/017_minimal_starter_isa_templates.seeds.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/db/seeds/017_minimal_starter_isa_templates.seeds.rb b/db/seeds/017_minimal_starter_isa_templates.seeds.rb index e78e477916..f44069c7e7 100644 --- a/db/seeds/017_minimal_starter_isa_templates.seeds.rb +++ b/db/seeds/017_minimal_starter_isa_templates.seeds.rb @@ -200,3 +200,8 @@ end puts 'Seeded minimal templates for organizing ISA JSON compliant experiments.' + +disable_authorization_checks do + AssayClass.find_or_create_by(title: 'Assay Stream', key: 'ASS', + description: 'Special type of class that is user in Single Page, specifying this is a container for a stream of assays') +end From 2c583d0235c8fd3c9eebd0d4651881b7cd465e10 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 16:34:33 +0100 Subject: [PATCH 14/41] Adapt isa exporter to handle assay streams --- app/models/assay.rb | 7 ---- app/models/study.rb | 4 ++ lib/isa_exporter.rb | 89 +++++++++++++++++++++------------------------ 3 files changed, 45 insertions(+), 55 deletions(-) diff --git a/app/models/assay.rb b/app/models/assay.rb index 575afce5d5..f7210d3235 100644 --- a/app/models/assay.rb +++ b/app/models/assay.rb @@ -80,13 +80,6 @@ def has_linked_child_assay? sample_type&.linked_sample_attributes&.any? end - # Fetches the assay which is linked through linked_sample_attributes (Single Page specific method) - def linked_assay - sample_type.linked_sample_attributes - .select { |lsa| lsa.isa_tag.nil? && lsa.title.include?('Input') } - .first&.sample_type&.assays&.first - end - def default_contributor User.current_user.try :person end diff --git a/app/models/study.rb b/app/models/study.rb index f7b6371c73..467a550083 100644 --- a/app/models/study.rb +++ b/app/models/study.rb @@ -36,6 +36,10 @@ class Study < ApplicationRecord has_many "related_#{type.pluralize}".to_sym, -> { distinct }, through: :assays, source: type.pluralize.to_sym end + def assay_streams + assays.select(&:is_assay_stream?) + end + def assets related_data_files + related_sops + related_models + related_publications + related_documents end diff --git a/lib/isa_exporter.rb b/lib/isa_exporter.rb index 108144752a..f015bc5d8d 100644 --- a/lib/isa_exporter.rb +++ b/lib/isa_exporter.rb @@ -117,7 +117,7 @@ def convert_study(study) # raise "The Study with the title '#{study.title}' does not have any SOP" if study.sops.blank? protocols << convert_protocol(study.sops, study.id, with_tag_protocol_study, with_tag_parameter_value_study) - study.assays.each do |a| + study.assay_streams.map(&:child_assays).flatten.each do |a| # There should be only one attribute with isa_tag == protocol protocol_attribute = a.sample_type.sample_attributes.detect { |sa| sa.isa_tag&.isa_protocol? } with_tag_parameter_value = a.sample_type.sample_attributes.select { |sa| sa.isa_tag&.isa_parameter_value? } @@ -134,20 +134,7 @@ def convert_study(study) raise "All assays in study `#{study.title}` should be ISA-JSON compliant." end - assay_streams = study.assays.map { |assay| [assay] if assay&.position&.zero? } - .compact - .map do |assay_stream| - last_assay = assay_stream.first - until last_assay.linked_assay.nil? - linked_assay = last_assay.linked_assay - assay_stream.push(linked_assay) - last_assay = linked_assay - end - assay_stream - end - - isa_study[:assays] = assay_streams.map { |assay_stream| convert_assays(assay_stream) }.compact - + isa_study[:assays] = study.assay_streams.map { |assay_stream| convert_assays(assay_stream) }.compact isa_study[:factors] = [] isa_study[:unitCategories] = [] @@ -163,28 +150,25 @@ def convert_annotation(term_uri) isa_annotation end - def convert_assay_comments(assays) + def convert_assay_comments(assay_stream) assay_comments = [] - assay_streams = assays.select { |a| a.position.zero? } - assay_stream_id = assays.pluck(:id).join('_') - - linked_assays = assays.map { |assay| { 'id': assay.id, 'title': assay.title } }.to_json - - assay_streams.map do |assay| - study_id = assay.study_id - next if assay.extended_metadata.nil? - - json = JSON.parse(assay.extended_metadata&.json_metadata) - cm_attributes = assay.extended_metadata.extended_metadata_attributes - cm_id = assay.extended_metadata&.id - json.map do |key, val| - cma_id = cm_attributes.detect { |cma| cma.title == key }&.id - assay_comments.push({ - '@id': "#assay_comment/#{[study_id, assay_stream_id, cm_id, cma_id].join('_')}", - 'name': key, - 'value': val - }) - end + assay_stream_id = assay_stream&.id + + linked_assays = assay_stream.child_assays.map { |assay| { 'id': assay.id, 'title': assay.title } }.to_json + + study_id = assay_stream.study_id + return [] if assay_stream.extended_metadata.nil? + + json = JSON.parse(assay_stream.extended_metadata&.json_metadata) + cm_attributes = assay_stream.extended_metadata.extended_metadata_attributes + cm_id = assay_stream.extended_metadata&.id + json.map do |key, val| + cma_id = cm_attributes.detect { |cma| cma.title == key }&.id + assay_comments.push({ + '@id': "#assay_comment/#{[study_id, assay_stream_id, cm_id, cma_id].join('_')}", + 'name': key, + 'value': val + }) end assay_comments.push({ @@ -195,35 +179,37 @@ def convert_assay_comments(assays) assay_comments.compact end - def convert_assays(assays) - return unless assays.all? { |a| a.can_view?(@current_user) } + def convert_assays(assay_stream) + child_assays = assay_stream.child_assays + return unless assay_stream.can_view?(@current_user) + return unless child_assays.all? { |a| a.can_view?(@current_user) } - all_sample_types = assays.map(&:sample_type) - first_assay = assays.detect { |s| s.position.zero? } - raise 'No assay could be found!' unless first_assay + all_sample_types = child_assays.map(&:sample_type).compact + # first_assay = assays.detect { |s| s.position.zero? } + # raise 'No assay could be found!' unless first_assay - stream_name = "assays_#{assays.pluck(:id).join('_')}" - assay_comments = convert_assay_comments(assays) + stream_name = "assays_#{child_assays.pluck(:id).join('_')}" + assay_comments = convert_assay_comments(assay_stream) # Retrieve assay_stream if stream_name_comment = assay_comments.detect { |ac| ac[:name] == 'assay_stream' } stream_name = stream_name_comment[:value] unless stream_name_comment.nil? isa_assay = {} - isa_assay['@id'] = "#assay/#{assays.pluck(:id).join('_')}" + isa_assay['@id'] = "#assay/#{child_assays.pluck(:id).join('_')}" isa_assay[:filename] = "a_#{stream_name.downcase.tr(" ", "_")}.txt" isa_assay[:measurementType] = { annotationValue: '', termSource: '', termAccession: '' } isa_assay[:technologyType] = { annotationValue: '', termSource: '', termAccession: '' } isa_assay[:comments] = assay_comments isa_assay[:technologyPlatform] = '' - isa_assay[:characteristicCategories] = convert_characteristic_categories(nil, assays) + isa_assay[:characteristicCategories] = convert_characteristic_categories(nil, child_assays) isa_assay[:materials] = { # Here, the first assay's samples will be enough - samples: assay_samples(first_assay), # the samples from study level that are referenced in this assay's samples, + samples: assay_samples(child_assays.first), # the samples from study level that are referenced in this assay's samples, otherMaterials: convert_other_materials(all_sample_types) } isa_assay[:processSequence] = - assays.map { |a| convert_process_sequence(a.sample_type, a.sops.map(&:id).join("_"), a.id) }.flatten + child_assays.map { |a| convert_process_sequence(a.sample_type, a.sops.map(&:id).join("_"), a.id) }.flatten isa_assay[:dataFiles] = convert_data_files(all_sample_types) isa_assay[:unitCategories] = [] isa_assay @@ -278,7 +264,14 @@ def convert_publication(publication) def convert_ontologies source_ontologies = [] - sample_types = @investigation.studies.map(&:sample_types) + @investigation.assays.map(&:sample_type) + sample_types = @investigation.studies.map(&:sample_types) + @investigation.assays + .select(&:is_assay_stream?) + .map(&:child_assays) + .compact + .flatten + .map(&:sample_type) + .compact + sample_types.flatten.each do |sa| sa.sample_attributes.each do |atr| source_ontologies << atr.sample_controlled_vocab.source_ontology if atr.ontology_based? From 158cdd40d2b4a9d6c8ed923026a44e2f9412cb84 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 17:22:34 +0100 Subject: [PATCH 15/41] Add rake upgrade task to add Assay Stream AssayClass --- db/seeds/017_minimal_starter_isa_templates.seeds.rb | 2 ++ lib/tasks/seek_upgrades.rake | 1 + 2 files changed, 3 insertions(+) diff --git a/db/seeds/017_minimal_starter_isa_templates.seeds.rb b/db/seeds/017_minimal_starter_isa_templates.seeds.rb index f44069c7e7..2b1929fbfb 100644 --- a/db/seeds/017_minimal_starter_isa_templates.seeds.rb +++ b/db/seeds/017_minimal_starter_isa_templates.seeds.rb @@ -205,3 +205,5 @@ AssayClass.find_or_create_by(title: 'Assay Stream', key: 'ASS', description: 'Special type of class that is user in Single Page, specifying this is a container for a stream of assays') end + +puts 'Seeded Extra Assay Class' diff --git a/lib/tasks/seek_upgrades.rake b/lib/tasks/seek_upgrades.rake index fb779e184a..6497f980b5 100644 --- a/lib/tasks/seek_upgrades.rake +++ b/lib/tasks/seek_upgrades.rake @@ -15,6 +15,7 @@ namespace :seek do rename_registered_sample_multiple_attribute_type remove_ontology_attribute_type db:seed:007_sample_attribute_types + db:seed:017_minimal_starter_isa_templates recognise_isa_json_compliant_items ] From 0da942e4ae223f4350fe9cd38ab46727974080b0 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 17:22:57 +0100 Subject: [PATCH 16/41] Add unit test --- test/factories/assays.rb | 15 +++++++++++++++ test/fixtures/assay_classes.yml | 10 +++++++--- test/unit/assay_test.rb | 9 +++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/test/factories/assays.rb b/test/factories/assays.rb index c0ab8efcf5..eab8a81d18 100644 --- a/test/factories/assays.rb +++ b/test/factories/assays.rb @@ -13,6 +13,12 @@ description { "An experimental assay class description" } end + factory(:assay_stream_class, class: AssayClass) do + title { 'Assay Stream' } + key { 'ASS' } + description { "An assay stream class description" } + end + # SuggestedTechnologyType factory(:suggested_technology_type) do sequence(:label) { | n | "A TechnologyType#{n}" } @@ -104,6 +110,15 @@ end end + factory(:assay_stream, parent: :assay_base) do + title { 'Assay Stream' } + description { 'A holder assay holding multiple child assays' } + association :assay_class, factory: :assay_stream_class + after(:build) do |assay| + assay.study = FactoryBot.create(:isa_json_compliant_study) + end + end + # AssayAsset factory :assay_asset do association :assay diff --git a/test/fixtures/assay_classes.yml b/test/fixtures/assay_classes.yml index 6811c4bdb0..fc730abd78 100644 --- a/test/fixtures/assay_classes.yml +++ b/test/fixtures/assay_classes.yml @@ -1,7 +1,11 @@ experimental_assay_class: title: <%= I18n.t('assays.experimental_assay') %> key: EXP - -modelling_assay_class: + +modelling_assay_class: title: <%= I18n.t('assays.modelling_analysis') %> - key: MODEL \ No newline at end of file + key: MODEL + +assay_stream_class: + title: 'Assay Stream' + key: 'ASS' diff --git a/test/unit/assay_test.rb b/test/unit/assay_test.rb index a8a9199528..72907b408a 100644 --- a/test/unit/assay_test.rb +++ b/test/unit/assay_test.rb @@ -760,4 +760,13 @@ def new_valid_assay isa_json_compliant_assay = FactoryBot.create(:isa_json_compliant_assay) assert isa_json_compliant_assay.is_isa_json_compliant? end + + test 'is assay stream' do + isa_json_compliant_study = FactoryBot.create(:isa_json_compliant_study) + assay_stream = FactoryBot.create(:assay_stream, study: isa_json_compliant_study) + assert assay_stream.is_assay_stream? + + default_assay = FactoryBot.create(:assay) + refute default_assay.is_assay_stream? + end end From 9407039c4fa457ea6182152a0d6cc8b2506406b2 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 17:44:53 +0100 Subject: [PATCH 17/41] Test the button text on the assays controller --- app/models/assay_class.rb | 2 +- test/functional/assays_controller_test.rb | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index 956488fbac..bc2a45728f 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -1,5 +1,5 @@ class AssayClass < ApplicationRecord - # this returns an instance of AssayClass according to one of the types "experimental" or "modelling" + # this returns an instance of AssayClass according to one of the types "experimental", "modelling" or "assay_stream" # if there is not a match nil is returned def self.for_type(type) keys = { "experimental": 'EXP', "modelling": 'MODEL', 'assay_stream': 'ASS' } diff --git a/test/functional/assays_controller_test.rb b/test/functional/assays_controller_test.rb index 77253f7191..ede6bcfa51 100644 --- a/test/functional/assays_controller_test.rb +++ b/test/functional/assays_controller_test.rb @@ -2012,9 +2012,21 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links with_config_value(:isa_json_compliance_enabled, true) do current_user = FactoryBot.create(:user) login_as(current_user) - assay = FactoryBot.create(:isa_json_compliant_assay, contributor: current_user.person) + assay_stream = FactoryBot.create(:assay_stream, contributor: current_user.person) + assay1 = FactoryBot.create(:isa_json_compliant_assay, contributor: current_user.person, study: assay_stream.study, assay_stream:) + assay2 = FactoryBot.create(:isa_json_compliant_assay, contributor: current_user.person, study: assay_stream.study, assay_stream:) - get :show, params: { id: assay } + get :show, params: { id: assay_stream } + assert_response :success + + assert_select 'a', text: /Design #{I18n.t('assay')}/i, count: 1 + + get :show, params: { id: assay1 } + assert_response :success + + assert_select 'a', text: /Design the next #{I18n.t('assay')}/i, count: 1 + + get :show, params: { id: assay2 } assert_response :success assert_select 'a', text: /Design the next #{I18n.t('assay')}/i, count: 1 From 299dfefd0c1f350edce7b1e5f52447ffff6c0597 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 18:08:12 +0100 Subject: [PATCH 18/41] Fix studiescontroller test --- test/functional/studies_controller_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/studies_controller_test.rb b/test/functional/studies_controller_test.rb index 69a5ef4419..a29d1c8891 100644 --- a/test/functional/studies_controller_test.rb +++ b/test/functional/studies_controller_test.rb @@ -2022,7 +2022,7 @@ def test_should_show_investigation_tab get :show, params: { id: study } assert_response :success - assert_select 'a', text: /Design #{I18n.t('assay')}/i, count: 1 + assert_select 'a', text: /Design #{I18n.t('assay')} Stream/i, count: 1 assert_select 'a', text: /Add new #{I18n.t('assay')}/i, count: 0 end end From bc6cb12830756670b0d955d13fb2c2d33c80e494 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 17 Jan 2024 21:41:01 +0100 Subject: [PATCH 19/41] Fix for_type function in AssayClass --- app/controllers/isa_assays_controller.rb | 8 ++++---- app/models/assay_class.rb | 6 +++--- app/views/isa_assays/_form.html.erb | 4 ++-- config/default_data/assay_classes.yml | 7 ++++++- config/locales/en.yml | 1 + db/seeds/017_minimal_starter_isa_templates.seeds.rb | 7 ------- lib/tasks/seek_upgrades.rake | 2 +- test/fixtures/assay_classes.yml | 4 ++-- 8 files changed, 19 insertions(+), 20 deletions(-) diff --git a/app/controllers/isa_assays_controller.rb b/app/controllers/isa_assays_controller.rb index 8bdb451a7d..0a44ce8d18 100644 --- a/app/controllers/isa_assays_controller.rb +++ b/app/controllers/isa_assays_controller.rb @@ -7,9 +7,9 @@ class IsaAssaysController < ApplicationController def new if params[:is_assay_stream] - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'ASS').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('assay_stream').id } }) else - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'EXP').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('experimental').id } }) end end @@ -141,9 +141,9 @@ def set_up_instance_variable def find_requested_item if params[:is_assay_stream] - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'ASS').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('assay_stream').id } }) else - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.find_by(key: 'EXP').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('experimental').id } }) end @isa_assay.populate(params[:id]) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index bc2a45728f..37a148de50 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -2,8 +2,8 @@ class AssayClass < ApplicationRecord # this returns an instance of AssayClass according to one of the types "experimental", "modelling" or "assay_stream" # if there is not a match nil is returned def self.for_type(type) - keys = { "experimental": 'EXP', "modelling": 'MODEL', 'assay_stream': 'ASS' } - AssayClass.find_by(key: keys[type]) + keys = { "experimental": 'EXP', "modelling": 'MODEL', "assay_stream": 'ASS' } + AssayClass.find_by(key: keys[type.to_sym]) end def self.experimental @@ -15,7 +15,7 @@ def self.modelling end def self.assay_stream - for_type('assay_stream') + for_type('assaystream') end def is_modelling? diff --git a/app/views/isa_assays/_form.html.erb b/app/views/isa_assays/_form.html.erb index 13a77cced1..13460bd080 100644 --- a/app/views/isa_assays/_form.html.erb +++ b/app/views/isa_assays/_form.html.erb @@ -7,11 +7,11 @@ if @isa_assay.assay.new_record? if params[:is_assay_stream] assay_position = 0 - assay_class_id = AssayClass.find_by(key: 'ASS')&.id + assay_class_id = AssayClass.for_type('assay_stream').id is_assay_stream = true else assay_position = params[:source_assay_id].nil? ? 1 : source_assay.position + 1 - assay_class_id = AssayClass.find_by(key: 'EXP')&.id + assay_class_id = AssayClass.for_type('experimental').id is_assay_stream = false end else diff --git a/config/default_data/assay_classes.yml b/config/default_data/assay_classes.yml index 94a08a2a99..56c2c6790e 100644 --- a/config/default_data/assay_classes.yml +++ b/config/default_data/assay_classes.yml @@ -6,4 +6,9 @@ experimental_assay_class: modelling_assay_class: id: 2 title: <%= I18n.t('assays.modelling_analysis') %> - key: MODEL \ No newline at end of file + key: MODEL + +assay_stream_class: + id: 3 + title: <%= I18n.t('assays.assay_stream') %> + key: ASS diff --git a/config/locales/en.yml b/config/locales/en.yml index ced43e1a21..8880ff79e8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -16,6 +16,7 @@ en: assay: "Assay" experimental_assay: "Experimental assay" modelling_analysis: "Modelling analysis" + assay_stream: "Assay Stream" isa_study: "ISA Study" study_design: "Study Design" diff --git a/db/seeds/017_minimal_starter_isa_templates.seeds.rb b/db/seeds/017_minimal_starter_isa_templates.seeds.rb index 2b1929fbfb..e78e477916 100644 --- a/db/seeds/017_minimal_starter_isa_templates.seeds.rb +++ b/db/seeds/017_minimal_starter_isa_templates.seeds.rb @@ -200,10 +200,3 @@ end puts 'Seeded minimal templates for organizing ISA JSON compliant experiments.' - -disable_authorization_checks do - AssayClass.find_or_create_by(title: 'Assay Stream', key: 'ASS', - description: 'Special type of class that is user in Single Page, specifying this is a container for a stream of assays') -end - -puts 'Seeded Extra Assay Class' diff --git a/lib/tasks/seek_upgrades.rake b/lib/tasks/seek_upgrades.rake index 6497f980b5..38990a2e7d 100644 --- a/lib/tasks/seek_upgrades.rake +++ b/lib/tasks/seek_upgrades.rake @@ -15,7 +15,7 @@ namespace :seek do rename_registered_sample_multiple_attribute_type remove_ontology_attribute_type db:seed:007_sample_attribute_types - db:seed:017_minimal_starter_isa_templates + db:seed:001_create_controlled_vocabs recognise_isa_json_compliant_items ] diff --git a/test/fixtures/assay_classes.yml b/test/fixtures/assay_classes.yml index fc730abd78..4a90f2e308 100644 --- a/test/fixtures/assay_classes.yml +++ b/test/fixtures/assay_classes.yml @@ -7,5 +7,5 @@ modelling_assay_class: key: MODEL assay_stream_class: - title: 'Assay Stream' - key: 'ASS' + title: <%= I18n.t('assays.assay_stream') %> + key: ASS From 0dc8b6c12b8390799ca6bdfa853afc50742e9659 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 18 Jan 2024 15:40:23 +0100 Subject: [PATCH 20/41] Fix functional tests about the isa json exporter --- lib/isa_exporter.rb | 11 +++++-- test/factories/assays.rb | 4 +-- .../investigations_controller_test.rb | 29 +++++++++++++------ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/lib/isa_exporter.rb b/lib/isa_exporter.rb index f015bc5d8d..5cc20eeda1 100644 --- a/lib/isa_exporter.rb +++ b/lib/isa_exporter.rb @@ -184,9 +184,14 @@ def convert_assays(assay_stream) return unless assay_stream.can_view?(@current_user) return unless child_assays.all? { |a| a.can_view?(@current_user) } + child_assays.map do |ca| + unless ca.sample_type.present? + raise "No Sample type was found in Assay '#{ca.id} - #{ca.title}'," \ + " part of Assay Stream '#{assay_stream.id - assay_stream.title}'" + end + end + all_sample_types = child_assays.map(&:sample_type).compact - # first_assay = assays.detect { |s| s.position.zero? } - # raise 'No assay could be found!' unless first_assay stream_name = "assays_#{child_assays.pluck(:id).join('_')}" assay_comments = convert_assay_comments(assay_stream) @@ -745,7 +750,7 @@ def random_string(len) end def get_derived_from_type(sample_type) - raise 'There is no sample!' if sample_type.samples.length == 0 + raise "There are no samples in '#{sample_type.title}'!" if sample_type.samples.blank? prev_sample_type = sample_type.samples[0]&.linked_samples[0]&.sample_type return nil if prev_sample_type.blank? diff --git a/test/factories/assays.rb b/test/factories/assays.rb index eab8a81d18..ffa20fd9ee 100644 --- a/test/factories/assays.rb +++ b/test/factories/assays.rb @@ -14,7 +14,7 @@ end factory(:assay_stream_class, class: AssayClass) do - title { 'Assay Stream' } + title { I18n.t('assays.assay_stream') } key { 'ASS' } description { "An assay stream class description" } end @@ -115,7 +115,7 @@ description { 'A holder assay holding multiple child assays' } association :assay_class, factory: :assay_stream_class after(:build) do |assay| - assay.study = FactoryBot.create(:isa_json_compliant_study) + assay.study ||= FactoryBot.create(:isa_json_compliant_study, contributor: assay.contributor) end end diff --git a/test/functional/investigations_controller_test.rb b/test/functional/investigations_controller_test.rb index a564eb29a4..b38571a183 100644 --- a/test/functional/investigations_controller_test.rb +++ b/test/functional/investigations_controller_test.rb @@ -911,16 +911,22 @@ def test_title contributor: other_user.person) # Create a 'private' assay in an assay stream - assay_1_stream_1_sample_type = FactoryBot.create(:isa_assay_material_sample_type, linked_sample_type: sample_collection_sample_type, template_id: FactoryBot.create(:isa_assay_material_template).id) - assay_1_stream_1 = FactoryBot.create(:assay, position: 0, sample_type: assay_1_stream_1_sample_type, study: accessible_study, contributor: current_user.person) - assay_2_stream_1_sample_type = FactoryBot.create(:isa_assay_data_file_sample_type, linked_sample_type: assay_1_stream_1_sample_type, template_id: FactoryBot.create(:isa_assay_data_file_template).id) - assay_2_stream_1 = FactoryBot.create(:assay, position:1, sample_type: assay_2_stream_1_sample_type, study: accessible_study, contributor: other_user.person) + stream_1 = FactoryBot.create(:assay_stream, title: 'Assay Stream 1', study: accessible_study, contributor: other_user.person) + assert_equal(stream_1.study, accessible_study) + assert(stream_1.is_assay_stream?) + assay_1_stream_1_sample_type = FactoryBot.create(:isa_assay_material_sample_type, contributor: other_user.person, linked_sample_type: sample_collection_sample_type, template_id: FactoryBot.create(:isa_assay_material_template).id) + assay_1_stream_1 = FactoryBot.create(:assay, position: 1, sample_type: assay_1_stream_1_sample_type, study: accessible_study, contributor: other_user.person, assay_stream_id: stream_1.id) + assay_2_stream_1_sample_type = FactoryBot.create(:isa_assay_data_file_sample_type, contributor: other_user.person, linked_sample_type: assay_1_stream_1_sample_type, template_id: FactoryBot.create(:isa_assay_data_file_template).id) + assay_2_stream_1 = FactoryBot.create(:assay, position: 2, sample_type: assay_2_stream_1_sample_type, study: accessible_study, contributor: other_user.person, assay_stream_id: stream_1.id) # Create an assay stream with all assays visible - assay_1_stream_2_sample_type = FactoryBot.create(:isa_assay_material_sample_type, linked_sample_type: sample_collection_sample_type, template_id: FactoryBot.create(:isa_assay_material_template).id) - assay_1_stream_2 = FactoryBot.create(:assay, position: 0, sample_type: assay_1_stream_2_sample_type, study: accessible_study, contributor: current_user.person) - assay_2_stream_2_sample_type = FactoryBot.create(:isa_assay_data_file_sample_type, linked_sample_type: assay_1_stream_2_sample_type, template_id: FactoryBot.create(:isa_assay_data_file_template).id) - assay_2_stream_2 = FactoryBot.create(:assay, position:1, sample_type: assay_2_stream_2_sample_type, study: accessible_study, contributor: current_user.person) + stream_2 = FactoryBot.create(:assay_stream, title: 'Assay Stream 2', study: accessible_study, contributor: current_user.person) + assert_equal(stream_2.study, accessible_study) + assert(stream_2.is_assay_stream?) + assay_1_stream_2_sample_type = FactoryBot.create(:isa_assay_material_sample_type, contributor: current_user.person, linked_sample_type: sample_collection_sample_type, template_id: FactoryBot.create(:isa_assay_material_template).id) + assay_1_stream_2 = FactoryBot.create(:assay, position: 1, sample_type: assay_1_stream_2_sample_type, study: accessible_study, contributor: current_user.person, assay_stream_id: stream_2.id) + assay_2_stream_2_sample_type = FactoryBot.create(:isa_assay_data_file_sample_type, contributor: current_user.person, linked_sample_type: assay_1_stream_2_sample_type, template_id: FactoryBot.create(:isa_assay_data_file_template).id) + assay_2_stream_2 = FactoryBot.create(:assay, position: 2, sample_type: assay_2_stream_2_sample_type, study: accessible_study, contributor: current_user.person, assay_stream_id: stream_2.id) # create samples in second assay stream with viewing permission @@ -1096,8 +1102,13 @@ def test_title assert json_investigation['studies'].map { |s| s['title'] }.include? accessible_study.title study_json = json_investigation['studies'].first + # Total assays + assert_equal accessible_study.assays.count, 6 + # Assay_streams + assert_equal accessible_study.assay_streams.count, 2 + # Child assays + assert_equal accessible_study.assay_streams.map(&:child_assays).compact.flatten.count, 4 # Only one assay should end up in 1 assay stream in the ISA JSON - assert_equal accessible_study.assays.count, 4 assert_equal study_json['assays'].count, 1 sample_ids = study_json['materials']['samples'].map { |sample| sample['@id'] } From 9ec5cfabd57fdbdb902889b045428efccdef413a Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Fri, 19 Jan 2024 12:23:51 +0100 Subject: [PATCH 21/41] Fix failing tests --- app/models/assay_class.rb | 2 +- app/views/assays/_buttons.html.erb | 2 +- test/factories/assays.rb | 2 +- test/functional/assays_controller_test.rb | 9 ++++++--- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index 37a148de50..bd1dee611c 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -33,6 +33,6 @@ def is_assay_stream? # for cases where a longer more descriptive key is useful, but can't rely on the title # which may have been changed over time def long_key - { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'ASS': 'Assay Stream' }[key] + { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'ASS': 'Assay Stream' }[key.to_sym] end end diff --git a/app/views/assays/_buttons.html.erb b/app/views/assays/_buttons.html.erb index e247754156..6c5b795cc5 100644 --- a/app/views/assays/_buttons.html.erb +++ b/app/views/assays/_buttons.html.erb @@ -2,7 +2,7 @@ if item.is_modelling? t('assays.modelling_analysis') elsif item.is_assay_stream? - 'Assay Stream' + t('assays.assay_stream') elsif Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? t('isa_assay') else diff --git a/test/factories/assays.rb b/test/factories/assays.rb index ffa20fd9ee..dfc298b213 100644 --- a/test/factories/assays.rb +++ b/test/factories/assays.rb @@ -105,7 +105,7 @@ title { 'ISA JSON compliant assay' } description { 'An assay linked to an ISA JSON compliant study and a sample type' } after(:build) do |assay| - assay.study = FactoryBot.create(:isa_json_compliant_study) + assay.study ||= FactoryBot.create(:isa_json_compliant_study) assay.sample_type = FactoryBot.create(:isa_assay_material_sample_type, linked_sample_type: assay.study.sample_types.last) end end diff --git a/test/functional/assays_controller_test.rb b/test/functional/assays_controller_test.rb index ede6bcfa51..7e93bf1626 100644 --- a/test/functional/assays_controller_test.rb +++ b/test/functional/assays_controller_test.rb @@ -2012,9 +2012,12 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links with_config_value(:isa_json_compliance_enabled, true) do current_user = FactoryBot.create(:user) login_as(current_user) - assay_stream = FactoryBot.create(:assay_stream, contributor: current_user.person) - assay1 = FactoryBot.create(:isa_json_compliant_assay, contributor: current_user.person, study: assay_stream.study, assay_stream:) - assay2 = FactoryBot.create(:isa_json_compliant_assay, contributor: current_user.person, study: assay_stream.study, assay_stream:) + study = FactoryBot.create(:isa_json_compliant_study) + assay_stream = FactoryBot.create(:assay_stream, study: , contributor: current_user.person) + assay1 = FactoryBot.create(:isa_json_compliant_assay, contributor: current_user.person, study: , assay_stream:) + assay2 = FactoryBot.create(:isa_json_compliant_assay, contributor: current_user.person, study: , assay_stream:) + + assert_equal assay_stream.study, assay1.study get :show, params: { id: assay_stream } assert_response :success From 5445bc14c41794594c2f1afe0d2d5641720d84d4 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Fri, 19 Jan 2024 13:12:54 +0100 Subject: [PATCH 22/41] Fix unit tests for assays --- test/unit/assay_test.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/unit/assay_test.rb b/test/unit/assay_test.rb index 72907b408a..b6e3c41c3b 100644 --- a/test/unit/assay_test.rb +++ b/test/unit/assay_test.rb @@ -754,10 +754,14 @@ def new_valid_assay test 'isa json compliance' do isa_json_compliant_study = FactoryBot.create(:isa_json_compliant_study) assert isa_json_compliant_study.is_isa_json_compliant? + default_assay = FactoryBot.create(:assay, study: isa_json_compliant_study) refute default_assay.is_isa_json_compliant? - isa_json_compliant_assay = FactoryBot.create(:isa_json_compliant_assay) + assay_stream = FactoryBot.create(:assay_stream, study: isa_json_compliant_study) + assert assay_stream.is_isa_json_compliant? + + isa_json_compliant_assay = FactoryBot.create(:isa_json_compliant_assay, study: isa_json_compliant_study) assert isa_json_compliant_assay.is_isa_json_compliant? end From b31ab9f92317311f2907133633bae98a330fb7d6 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Fri, 19 Jan 2024 13:34:48 +0100 Subject: [PATCH 23/41] Fix functional tests for isa_assays --- app/models/assay.rb | 2 +- test/functional/isa_assays_controller_test.rb | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/assay.rb b/app/models/assay.rb index f7210d3235..e07718d35d 100644 --- a/app/models/assay.rb +++ b/app/models/assay.rb @@ -69,7 +69,7 @@ class Assay < ApplicationRecord enforce_authorization_on_association :study, :view def is_assay_stream? - assay_class.is_assay_stream? + assay_class&.is_assay_stream? end def previous_linked_assay_sample_type diff --git a/test/functional/isa_assays_controller_test.rb b/test/functional/isa_assays_controller_test.rb index f6b436f227..582be6bb94 100644 --- a/test/functional/isa_assays_controller_test.rb +++ b/test/functional/isa_assays_controller_test.rb @@ -45,7 +45,8 @@ def setup sop_ids: [FactoryBot.create(:sop, policy: FactoryBot.create(:public_policy)).id], creator_ids: [this_person.id, other_creator.id], other_creators: 'other collaborators', - position: 0, assay_class_id: 1, policy_attributes: }, + assay_class_id: AssayClass.for_type('experimental').id, + position: 0, policy_attributes: }, input_sample_type_id: sample_collection_sample_type.id, sample_type: { title: 'assay sample_type', project_ids: [projects.first.id], template_id: 1, sample_attributes_attributes: { @@ -188,7 +189,7 @@ def setup sop_ids: [FactoryBot.create(:sop, policy: FactoryBot.create(:public_policy)).id], creator_ids: [this_person.id, other_creator.id], other_creators: 'other collaborators', - position: 0, assay_class_id: 1, policy_attributes: } + position: 0, assay_class_id: AssayClass.for_type('experimental').id, policy_attributes: } isa_assay_attributes = { assay: assay_attributes.merge(emt_attributes), input_sample_type_id: sample_collection_sample_type.id, From 7d3afa81298771c39533e47620c624a33b31e852 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 22 Jan 2024 09:41:36 +0100 Subject: [PATCH 24/41] update model and controller to work with assay streams --- app/controllers/assays_controller.rb | 2 +- app/models/assay.rb | 28 ++++++++-- test/functional/assays_controller_test.rb | 4 +- test/unit/assay_test.rb | 65 +++++++++++++++++++++++ 4 files changed, 93 insertions(+), 6 deletions(-) diff --git a/app/controllers/assays_controller.rb b/app/controllers/assays_controller.rb index 0c88fabba4..e2a552fd4f 100644 --- a/app/controllers/assays_controller.rb +++ b/app/controllers/assays_controller.rb @@ -117,7 +117,7 @@ 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 + 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 diff --git a/app/models/assay.rb b/app/models/assay.rb index e07718d35d..ab9e5b95e6 100644 --- a/app/models/assay.rb +++ b/app/models/assay.rb @@ -72,12 +72,34 @@ def is_assay_stream? assay_class&.is_assay_stream? end - def previous_linked_assay_sample_type - sample_type.sample_attributes.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.linked_sample_type + def previous_linked_sample_type + return unless is_isa_json_compliant? + + if is_assay_stream? + study.sample_types.second + else + sample_type.sample_attributes.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.linked_sample_type + end end def has_linked_child_assay? - sample_type&.linked_sample_attributes&.any? + return false unless is_isa_json_compliant? + + if is_assay_stream? + child_assays.any? + else + sample_type&.linked_sample_attributes&.any? + end + end + + 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 + else + sample_type.linked_sample_attributes.detect { |sa| sa.isa_tag.nil? && sa.title.include?('Input') }&.sample_type&.assays&.first + end end def default_contributor diff --git a/test/functional/assays_controller_test.rb b/test/functional/assays_controller_test.rb index 7e93bf1626..4ee3790a91 100644 --- a/test/functional/assays_controller_test.rb +++ b/test/functional/assays_controller_test.rb @@ -1947,7 +1947,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links # person = User.current_user.person person = FactoryBot.create(:person) project = person.projects.first - investigation = FactoryBot.create(:investigation, projects: [project]) + investigation = FactoryBot.create(:investigation, projects: [project], is_isa_json_compliant: true) 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], @@ -1984,7 +1984,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links assay3.reload - assert_equal(assay3.previous_linked_assay_sample_type&.id, assay1.sample_type&.id) + assert_equal(assay3.previous_linked_sample_type&.id, assay1.sample_type&.id) end test 'do not get index if feature disabled' do diff --git a/test/unit/assay_test.rb b/test/unit/assay_test.rb index b6e3c41c3b..f72a1e7d50 100644 --- a/test/unit/assay_test.rb +++ b/test/unit/assay_test.rb @@ -773,4 +773,69 @@ def new_valid_assay default_assay = FactoryBot.create(:assay) refute default_assay.is_assay_stream? end + + test 'previous linked sample type' do + isa_study = FactoryBot.create(:isa_json_compliant_study) + def_study = FactoryBot.create(:study) + + assay_stream = FactoryBot.create(:assay_stream, study: isa_study) + assert_equal assay_stream.previous_linked_sample_type, isa_study.sample_types.second + + def_assay = FactoryBot.create(:assay, study:def_study) + assert_nil def_assay.previous_linked_sample_type + + first_isa_assay = FactoryBot.create(:isa_json_compliant_assay, + assay_stream: , + study: isa_study) + assert_equal first_isa_assay.previous_linked_sample_type, isa_study.sample_types.second + + data_file_sample_type = FactoryBot.create(:isa_assay_data_file_sample_type, + linked_sample_type: first_isa_assay.sample_type) + second_isa_assay = FactoryBot.create(:assay, + study: isa_study, + assay_stream: , + sample_type: data_file_sample_type) + + assert_equal second_isa_assay.previous_linked_sample_type, first_isa_assay.sample_type + end + + test 'has_linked_child_assay?' do + isa_study = FactoryBot.create(:isa_json_compliant_study) + def_study = FactoryBot.create(:study) + def_assay = FactoryBot.create(:assay, study:def_study) + + assay_stream = FactoryBot.create(:assay_stream, study: isa_study) + first_isa_assay = FactoryBot.create(:isa_json_compliant_assay, study: isa_study) + data_file_sample_type = FactoryBot.create(:isa_assay_data_file_sample_type, + linked_sample_type: first_isa_assay.sample_type) + second_isa_assay = FactoryBot.create(:assay, + study: isa_study, + assay_stream: , + sample_type: data_file_sample_type) + + assert assay_stream.has_linked_child_assay? + refute def_assay.has_linked_child_assay? + assert first_isa_assay.has_linked_child_assay? + refute second_isa_assay.has_linked_child_assay? + end + + test 'next_linked_child_assay' do + isa_study = FactoryBot.create(:isa_json_compliant_study) + def_study = FactoryBot.create(:study) + def_assay = FactoryBot.create(:assay, study:def_study) + + assay_stream = FactoryBot.create(:assay_stream, study: isa_study) + first_isa_assay = FactoryBot.create(:isa_json_compliant_assay, study: isa_study) + data_file_sample_type = FactoryBot.create(:isa_assay_data_file_sample_type, + linked_sample_type: first_isa_assay.sample_type) + second_isa_assay = FactoryBot.create(:assay, + study: isa_study, + assay_stream: , + sample_type: data_file_sample_type) + + assert_equal assay_stream.next_linked_child_assay, first_isa_assay + assert_nil def_assay.next_linked_child_assay + assert_equal first_isa_assay.next_linked_child_assay, second_isa_assay + assert_nil second_isa_assay.next_linked_child_assay + end end From e9051532761625a2a8a24d0aa9fff568c0d9a118 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 22 Jan 2024 10:01:22 +0100 Subject: [PATCH 25/41] Add rake upgrade task --- lib/tasks/seek_upgrades.rake | 43 ++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/lib/tasks/seek_upgrades.rake b/lib/tasks/seek_upgrades.rake index 38990a2e7d..0797d29613 100644 --- a/lib/tasks/seek_upgrades.rake +++ b/lib/tasks/seek_upgrades.rake @@ -17,6 +17,7 @@ namespace :seek do db:seed:007_sample_attribute_types db:seed:001_create_controlled_vocabs recognise_isa_json_compliant_items + implement_assay_streams_for_isa_assays ] # these are the tasks that are executes for each upgrade as standard, and rarely change @@ -177,6 +178,48 @@ namespace :seek do puts "...Updated #{investigations_updated.to_s} investigations" end + task(implement_assay_streams_for_isa_assays: [:environment]) do + puts '... Organising isa json compliant assays in assay streams' + assay_streams_created = 0 + disable_authorization_checks do + # find assays linked to a study through their sample_types + # Should be isa json compliant + # Shouldn't already have an assay stream (don't update assays that have been updated already) + # Previous ST should be second ST of study + first_assays_in_stream = Assay.joins(:sample_type) + .select(&:is_isa_json_compliant?) + .select { |a| a.assay_stream_id.nil? && (a.previous_linked_sample_type == a.study.sample_types.second) } + + first_assays_in_stream.map do |fas| + stream_name = "Assay Stream - #{UUID.generate}" + assay_stream = Assay.create(title: stream_name, + study_id: fas.study_id, + assay_class_id: AssayClass.for_type('assay_stream').id, + contributor: fas.contributor, + position: 0) + + # Transfer extended metadata from first assay to newly created assay stream + unless fas.extended_metadata.nil? + em = ExtendedMetadata.find_by(item_id: fas.id) + em.update_column(:item_id, assay_stream.id) + end + + assay_position = 1 + current_assay = fas + while current_assay + current_assay.update_column(:position, assay_position) + current_assay.update_column(:assay_stream_id, assay_stream.id) + + assay_position += 1 + current_assay = current_assay.next_linked_child_assay + end + assay_streams_created += 1 + end + end + + puts "...Created #{assay_streams_created} new assay streams" + end + private ## From b768b13470be3f4fcd123c261a9ef87947d7ae77 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 22 Jan 2024 15:13:29 +0100 Subject: [PATCH 26/41] Make title dependent on whether it's an assay stream or not --- app/views/isa_assays/edit.erb | 7 +++++-- app/views/isa_assays/new.html.erb | 6 +++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/views/isa_assays/edit.erb b/app/views/isa_assays/edit.erb index 6afac60429..476b164dcd 100644 --- a/app/views/isa_assays/edit.erb +++ b/app/views/isa_assays/edit.erb @@ -1,5 +1,8 @@ -

    Edit <%= t('isa_assay') %>

    - +<% if @isa_assay.assay.is_assay_stream? %> +

    Edit <%= t('assays.assay_stream') %>

    +<% else %> +

    Edit <%= t('isa_assay') %>

    +<% end %> <%= render partial: "templates/template_modal" -%>
    <%= form_for @isa_assay, url: { controller: "isa_assays", action: "update", id: params[:id] }, method: :put do |f| %> diff --git a/app/views/isa_assays/new.html.erb b/app/views/isa_assays/new.html.erb index d2a46bab7b..4af1f56a50 100644 --- a/app/views/isa_assays/new.html.erb +++ b/app/views/isa_assays/new.html.erb @@ -1,5 +1,9 @@
    -

    New <%=t("isa_assay")%>

    + <% if params[:is_assay_stream] %> +

    New <%=t("assays.assay_stream")%>

    + <% else %> +

    New <%=t("isa_assay")%>

    + <% end %>
    <%= render partial: "templates/template_modal" -%> From cf258e2dc69283c1bf3647c2d19ff403e88cddef Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Mon, 22 Jan 2024 15:14:44 +0100 Subject: [PATCH 27/41] Hide SOPs, Documents, Publications and discussion links if assay is assay stream --- app/views/isa_assays/_form.html.erb | 16 +++--- config/routes.rb | 2 +- test/functional/isa_assays_controller_test.rb | 56 +++++++++++++++++++ 3 files changed, 66 insertions(+), 8 deletions(-) diff --git a/app/views/isa_assays/_form.html.erb b/app/views/isa_assays/_form.html.erb index 13460bd080..eabfdf1abf 100644 --- a/app/views/isa_assays/_form.html.erb +++ b/app/views/isa_assays/_form.html.erb @@ -68,15 +68,17 @@ <%= assay_fields.hidden_field :assay_stream_id, value: assay_stream_id -%> <%= assay_fields.hidden_field :assay_class_id, value: assay_class_id -%> - <% if User.current_user -%> - <%= render partial: 'assets/manage_specific_attributes', locals:{f:assay_fields} if show_form_manage_specific_attributes? %> - <%= assay_fields.fancy_multiselect(:sops, other_projects_checkbox: true, name: "isa_assay[assay][sop_ids]")%> - <%= assay_fields.fancy_multiselect :publications, { other_projects_checkbox: true, name: "isa_assay[assay][publication_ids]" } %> - <%= assay_fields.fancy_multiselect :documents, { other_projects_checkbox: true, name: "isa_assay[assay][document_ids]" } %> - <% end -%> + <% unless is_assay_stream %> + <% if User.current_user -%> + <%= render partial: 'assets/manage_specific_attributes', locals:{f:assay_fields} if show_form_manage_specific_attributes? %> + <%= assay_fields.fancy_multiselect(:sops, other_projects_checkbox: true, name: "isa_assay[assay][sop_ids]")%> + <%= assay_fields.fancy_multiselect :publications, { other_projects_checkbox: true, name: "isa_assay[assay][publication_ids]" } %> + <%= assay_fields.fancy_multiselect :documents, { other_projects_checkbox: true, name: "isa_assay[assay][document_ids]" } %> + <% end -%> - <%= render :partial=> "assets/discussion_links_form", :locals=>{:resource => @isa_assay.assay} -%> + <%= render partial: "assets/discussion_links_form", locals: { resource: @isa_assay.assay } -%> + <% end -%> <% end -%> <%= f.hidden_field :input_sample_type_id, value: input_sample_type_id -%> diff --git a/config/routes.rb b/config/routes.rb index e632e6f4f0..49bdcba035 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -744,7 +744,7 @@ post :template_attributes end collection do -post :filter_isa_tags_by_level + post :filter_isa_tags_by_level get :task_status get :default_templates post :populate_template diff --git a/test/functional/isa_assays_controller_test.rb b/test/functional/isa_assays_controller_test.rb index 582be6bb94..1cf8c6b35f 100644 --- a/test/functional/isa_assays_controller_test.rb +++ b/test/functional/isa_assays_controller_test.rb @@ -225,4 +225,60 @@ def setup end end end + + test 'hide sops, publications, documents, and discussion channels if assay stream' do + person = FactoryBot.create(:person) + study = FactoryBot.create(:isa_json_compliant_study, contributor: person) + assay_stream = FactoryBot.create(:assay_stream, study: , contributor: person) + + get :new, params: {study_id: study.id, is_assay_stream: true} + assert_response :success + + assert_select 'div#add_sops_form', text: /SOPs/i, count: 0 + assert_select 'div#add_publications_form', text: /Publications/i, count: 0 + assert_select 'div#add_documents_form', text: /Documents/i, count: 0 + assert_select 'div.panel-heading', text: /Discussion Channels/i, count: 0 + assert_select 'div.panel-heading', text: /Define Sample type for Assay/i, count: 0 + + get :edit, params: { id: assay_stream.id, study_id: study.id, source_assay_id: assay_stream.id, is_assay_stream: true } + assert_response :success + + assert_select 'div#add_sops_form', text: /SOPs/i, count: 0 + assert_select 'div#add_publications_form', text: /Publications/i, count: 0 + assert_select 'div#add_documents_form', text: /Documents/i, count: 0 + assert_select 'div.panel-heading', text: /Discussion Channels/i, count: 0 + assert_select 'div.panel-heading', text: /Define Sample type for Assay/i, count: 0 +end + + test 'show sops, publications, documents, and discussion channels if experimental assay' do + person = FactoryBot.create(:person) + project = person.projects.first + investigation = FactoryBot.create(:investigation, is_isa_json_compliant: true, contributor: person, projects: [project]) + study = FactoryBot.create(:isa_json_compliant_study, contributor: person, investigation: ) + assay_stream = FactoryBot.create(:assay_stream, study: , contributor: person, position: 0) + + login_as(person) + + get :new, params: {study_id: study.id, assay_stream_id: assay_stream.id, source_assay_id: assay_stream.id} + assert_response :success + + assert_select 'div#add_sops_form', text: /SOPs/i, count: 1 + assert_select 'div#add_publications_form', text: /Publications/i, count: 1 + assert_select 'div#add_documents_form', text: /Documents/i, count:1 + assert_select 'div.panel-heading', text: /Discussion Channels/i, count: 1 + assert_select 'div.panel-heading', text: /Define Sample type for Assay/i, count: 1 + + first_assay_st = FactoryBot.create(:isa_assay_material_sample_type, contributor: person, projects: [project], linked_sample_type: study.sample_types.second) + first_assay = FactoryBot.create(:assay, contributor: person, study: , assay_stream: , position: 1, sample_type: first_assay_st) + assert_equal assay_stream, first_assay.assay_stream + + get :edit, params: { id: first_assay.id, assay_stream_id: assay_stream.id, source_assay_id: first_assay.id, study_id: study.id } + assert_response :success + + assert_select 'div#add_sops_form', text: /SOPs/i, count: 1 + assert_select 'div#add_publications_form', text: /Publications/i, count: 1 + assert_select 'div#add_documents_form', text: /Documents/i, count: 1 + assert_select 'div.panel-heading', text: /Discussion Channels/i, count: 1 + assert_select 'div.panel-heading', text: /Define Sample type for Assay/i, count: 1 + end end From 9493fef09c4cfe01770bd6df554bbc05d8f00eaf Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 23 Jan 2024 14:15:39 +0100 Subject: [PATCH 28/41] move cv_modal back from the sample_types form to the new page --- app/views/isa_assays/new.html.erb | 3 +++ app/views/isa_studies/_sample_types_form.html.erb | 3 --- app/views/isa_studies/new.html.erb | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/views/isa_assays/new.html.erb b/app/views/isa_assays/new.html.erb index 4af1f56a50..07da9cf1e0 100644 --- a/app/views/isa_assays/new.html.erb +++ b/app/views/isa_assays/new.html.erb @@ -1,4 +1,7 @@
    + + <%= sample_controlled_vocab_model_dialog('cv-modal') %> + <% if params[:is_assay_stream] %>

    New <%=t("assays.assay_stream")%>

    <% else %> diff --git a/app/views/isa_studies/_sample_types_form.html.erb b/app/views/isa_studies/_sample_types_form.html.erb index b1d7bf1ebd..8889467eb3 100644 --- a/app/views/isa_studies/_sample_types_form.html.erb +++ b/app/views/isa_studies/_sample_types_form.html.erb @@ -5,9 +5,6 @@ <% main_field_name = id_suffix[1..-1] %> <% isa_element ||= "study" %> - - <%= sample_controlled_vocab_model_dialog('cv-modal') %> - <%= f.fields_for main_field_name, sample_type do |field| %> <% unless action == :edit %> diff --git a/app/views/isa_studies/new.html.erb b/app/views/isa_studies/new.html.erb index 38f08dc616..021e3e17f4 100644 --- a/app/views/isa_studies/new.html.erb +++ b/app/views/isa_studies/new.html.erb @@ -1,6 +1,10 @@ <% if Investigation.authorized_for('view').none? %> <%= button_link_to("New #{t('investigation')}", 'arrow_right', new_investigation_path) -%> <% else %> + + <%= sample_controlled_vocab_model_dialog('cv-modal') %> + +

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

    <%= render :partial => "templates/template_modal" -%> From 9edc7f6801c173fc2bfa53f3f2f2cf99fbace6a0 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 23 Jan 2024 14:43:05 +0100 Subject: [PATCH 29/41] Hide assay type and technology type in case of isa json compliant assay --- app/views/assays/show.html.erb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/views/assays/show.html.erb b/app/views/assays/show.html.erb index 6f2c697b1b..6c31f71d60 100644 --- a/app/views/assays/show.html.erb +++ b/app/views/assays/show.html.erb @@ -47,16 +47,18 @@ <%= "Assay position" %>: <%= @assay.position %>

    -

    - <%= assay_type_text -%>: - <%= link_to_assay_type(@assay) -%> -

    - <% unless @assay.is_modelling? -%> -

    - Technology type: - <%= link_to_technology_type(@assay) -%> + <% unless @assay.is_isa_json_compliant? %> +

    + <%= assay_type_text -%>: + <%= link_to_assay_type(@assay) -%>

    - <% end -%> + <% unless @assay.is_modelling? -%> +

    + Technology type: + <%= link_to_technology_type(@assay) -%> +

    + <% end -%> + <% end %> <% if Seek::Config.organisms_enabled %> <%= list_assay_organisms("Organisms", @assay.assay_organisms, { :id => "organism" }) %> From 67c1778aaf330ffb6e70bcbcc0c2d9257d1677fe Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 23 Jan 2024 14:59:41 +0100 Subject: [PATCH 30/41] Add assay stream and related child assays to show --- app/views/assays/show.html.erb | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/app/views/assays/show.html.erb b/app/views/assays/show.html.erb index 6c31f71d60..192f49da35 100644 --- a/app/views/assays/show.html.erb +++ b/app/views/assays/show.html.erb @@ -70,12 +70,30 @@ <% if Seek::Config.isa_json_compliance_enabled %>

    - <%= "Is ISA-JSON compliant" %>: + Is ISA-JSON compliant: <%= @assay.is_isa_json_compliant? %>

    <%= render partial: 'isa_studies/applied_templates', locals: { resource: @assay } -%> <% end %> + <% if @assay.is_assay_stream? %> +

    + Child assays: +

      + <% @assay.child_assays.map do |ca| %> +
    • + <%= link_to ca.title, ca %> +
      • + <% end %> +
    +

    + <% else %> +

    + <%= t('assays.assay_stream') %>: + <%= link_to @assay.assay_stream.title, @assay.assay_stream %> +

    + <% end %> +
    From 37dbaaef37418036261c68f7fec1a71604d35cb1 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 23 Jan 2024 15:31:18 +0100 Subject: [PATCH 31/41] Set assay filename in ISA JSON to the assay stream name --- lib/isa_exporter.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/isa_exporter.rb b/lib/isa_exporter.rb index 5cc20eeda1..5d5a3ff288 100644 --- a/lib/isa_exporter.rb +++ b/lib/isa_exporter.rb @@ -193,12 +193,13 @@ def convert_assays(assay_stream) all_sample_types = child_assays.map(&:sample_type).compact - stream_name = "assays_#{child_assays.pluck(:id).join('_')}" + # stream_name = "assays_#{child_assays.pluck(:id).join('_')}" + stream_name = "#{ assay_stream.title }_#{assay_stream.id}_#{child_assays.pluck(:id).join('_')}" assay_comments = convert_assay_comments(assay_stream) # Retrieve assay_stream if - stream_name_comment = assay_comments.detect { |ac| ac[:name] == 'assay_stream' } - stream_name = stream_name_comment[:value] unless stream_name_comment.nil? + # stream_name_comment = assay_comments.detect { |ac| ac[:name] == 'assay_stream' } + # stream_name = stream_name_comment[:value] unless stream_name_comment.nil? isa_assay = {} isa_assay['@id'] = "#assay/#{child_assays.pluck(:id).join('_')}" From 31a5ade77c4efdeef923fe12e6c5b60bb6410d2b Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Tue, 23 Jan 2024 16:16:51 +0100 Subject: [PATCH 32/41] Fix failing integration test --- app/views/assays/show.html.erb | 36 +++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/app/views/assays/show.html.erb b/app/views/assays/show.html.erb index 192f49da35..9f85fc1b39 100644 --- a/app/views/assays/show.html.erb +++ b/app/views/assays/show.html.erb @@ -76,22 +76,26 @@ <%= render partial: 'isa_studies/applied_templates', locals: { resource: @assay } -%> <% end %> - <% if @assay.is_assay_stream? %> -

    - Child assays: -

      - <% @assay.child_assays.map do |ca| %> -
    • - <%= link_to ca.title, ca %> -
      • - <% end %> -
    -

    - <% else %> -

    - <%= t('assays.assay_stream') %>: - <%= link_to @assay.assay_stream.title, @assay.assay_stream %> -

    + <% if @assay.is_isa_json_compliant? %> + <% if @assay.is_assay_stream? %> +

    + Child assays: +

      + <% @assay.child_assays.map do |ca| %> + <% unless @assay.child_assays.blank? %> +
    • + <%= link_to ca.title, ca %> +
      • + <% end %> + <% end %> +
    +

    + <% else %> +

    + <%= t('assays.assay_stream') %>: + <%= link_to @assay.assay_stream.title, @assay.assay_stream %> +

    + <% end %> <% end %> From 8855f09b9b812fe65a7df118eb2374c96098099c Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 24 Jan 2024 16:55:42 +0100 Subject: [PATCH 33/41] Abstact AssayClass keys to constants --- app/controllers/assays_controller.rb | 2 +- app/controllers/isa_assays_controller.rb | 8 ++++---- app/models/assay_class.rb | 20 +++++++++---------- app/views/isa_assays/_form.html.erb | 4 ++-- config/default_data/assay_classes.yml | 2 +- lib/seek/isa/assay_class.rb | 17 ++++++++++++++++ lib/seek/ontologies/synchronize.rb | 2 +- lib/seek/openbis/seek_util.rb | 6 +++--- lib/seek/projects/population.rb | 10 +++++----- test/factories/assays.rb | 6 +++--- test/fixtures/assay_classes.yml | 2 +- test/functional/isa_assays_controller_test.rb | 4 ++-- test/unit/assay_class_test.rb | 8 ++++---- .../ontology_synchronization_test.rb | 4 ++-- 14 files changed, 56 insertions(+), 39 deletions(-) create mode 100644 lib/seek/isa/assay_class.rb diff --git a/app/controllers/assays_controller.rb b/app/controllers/assays_controller.rb index e2a552fd4f..900ddef19e 100644 --- a/app/controllers/assays_controller.rb +++ b/app/controllers/assays_controller.rb @@ -82,7 +82,7 @@ def edit end def create - params[:assay_class_id] ||= AssayClass.for_type('experimental').id + params[:assay_class_id] ||= AssayClass.for_type(Seek::ISA::AssayClass::EXP).id @assay = Assay.new(assay_params) update_assay_organisms @assay, params diff --git a/app/controllers/isa_assays_controller.rb b/app/controllers/isa_assays_controller.rb index 0a44ce8d18..7771d17b3e 100644 --- a/app/controllers/isa_assays_controller.rb +++ b/app/controllers/isa_assays_controller.rb @@ -7,9 +7,9 @@ class IsaAssaysController < ApplicationController def new if params[:is_assay_stream] - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('assay_stream').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id } }) else - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('experimental').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id } }) end end @@ -141,9 +141,9 @@ def set_up_instance_variable def find_requested_item if params[:is_assay_stream] - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('assay_stream').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id } }) else - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type('experimental').id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id } }) end @isa_assay.populate(params[:id]) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index bd1dee611c..bb53cdec8e 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -1,38 +1,38 @@ class AssayClass < ApplicationRecord - # this returns an instance of AssayClass according to one of the types "experimental", "modelling" or "assay_stream" + # this returns an instance of AssayClass according to one of the constants defined in seek/isa/assay_class.rb # if there is not a match nil is returned def self.for_type(type) - keys = { "experimental": 'EXP', "modelling": 'MODEL', "assay_stream": 'ASS' } - AssayClass.find_by(key: keys[type.to_sym]) + AssayClass.find_by(key: type) end def self.experimental - for_type('experimental') + for_type(Seek::ISA::AssayClass::EXP) end def self.modelling - for_type('modelling') + for_type(Seek::ISA::AssayClass::MODEL) + end def self.assay_stream - for_type('assaystream') + for_type(Seek::ISA::AssayClass::STREAM) end def is_modelling? - key == 'MODEL' + key == Seek::ISA::AssayClass::MODEL end def is_experimental? - key == 'EXP' + key == Seek::ISA::AssayClass::EXP end def is_assay_stream? - key == 'ASS' + key == Seek::ISA::AssayClass::STREAM end # for cases where a longer more descriptive key is useful, but can't rely on the title # which may have been changed over time def long_key - { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'ASS': 'Assay Stream' }[key.to_sym] + { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'STREAM': 'Assay Stream' }[key.to_sym] end end diff --git a/app/views/isa_assays/_form.html.erb b/app/views/isa_assays/_form.html.erb index eabfdf1abf..06b9a057ad 100644 --- a/app/views/isa_assays/_form.html.erb +++ b/app/views/isa_assays/_form.html.erb @@ -7,11 +7,11 @@ if @isa_assay.assay.new_record? if params[:is_assay_stream] assay_position = 0 - assay_class_id = AssayClass.for_type('assay_stream').id + assay_class_id = AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id is_assay_stream = true else assay_position = params[:source_assay_id].nil? ? 1 : source_assay.position + 1 - assay_class_id = AssayClass.for_type('experimental').id + assay_class_id = AssayClass.for_type(Seek::ISA::AssayClass::EXP).id is_assay_stream = false end else diff --git a/config/default_data/assay_classes.yml b/config/default_data/assay_classes.yml index 56c2c6790e..8b01bc6ad3 100644 --- a/config/default_data/assay_classes.yml +++ b/config/default_data/assay_classes.yml @@ -11,4 +11,4 @@ modelling_assay_class: assay_stream_class: id: 3 title: <%= I18n.t('assays.assay_stream') %> - key: ASS + key: STREAM diff --git a/lib/seek/isa/assay_class.rb b/lib/seek/isa/assay_class.rb new file mode 100644 index 0000000000..9be0b02dce --- /dev/null +++ b/lib/seek/isa/assay_class.rb @@ -0,0 +1,17 @@ +module Seek + module ISA + module AssayClass + # Creates constants based on the AssayClass key attributes + # Example: AssayClass key 'EXP' can be represented by Seek::ISA:AssayClass::EXP + ALL_TYPES = %w[EXP MODEL STREAM] + + ALL_TYPES.each do |type| + AssayClass.const_set(type.underscore.upcase, type) + end + + def self.valid?(value) + ALL_TYPES.include?(value) + end + end + end +end diff --git a/lib/seek/ontologies/synchronize.rb b/lib/seek/ontologies/synchronize.rb index e15f0119e8..fd3b83d4c4 100644 --- a/lib/seek/ontologies/synchronize.rb +++ b/lib/seek/ontologies/synchronize.rb @@ -99,7 +99,7 @@ def assay_changes_class?(assays, ontology_uri) def determine_assay_class_from_uri(uri) ontology_class = Seek::Ontologies::AssayTypeReader.instance.class_for_uri(uri) - ontology_class.nil? ? AssayClass.for_type('modelling') : AssayClass.for_type('experimental') + ontology_class.nil? ? AssayClass.for_type(Seek::ISA::AssayClass::MODEL) : AssayClass.for_type(Seek::ISA::AssayClass::EXP) end def get_suggested_types_found_in_ontology(type) diff --git a/lib/seek/openbis/seek_util.rb b/lib/seek/openbis/seek_util.rb index 6476676d54..f6303e2e18 100644 --- a/lib/seek/openbis/seek_util.rb +++ b/lib/seek/openbis/seek_util.rb @@ -27,7 +27,7 @@ def createObisAssay(assay_params, creator, obis_asset) zample = obis_asset.content openbis_endpoint = obis_asset.seek_service - assay_params[:assay_class_id] ||= AssayClass.for_type('experimental').id + assay_params[:assay_class_id] ||= AssayClass.for_type(Seek::ISA::AssayClass::EXP).id assay_params[:title] ||= extract_title(zample) ## "OpenBIS #{zample.perm_id}" assay = Assay.new(assay_params) @@ -88,7 +88,7 @@ def fake_file_assay(study) assay = study.assays.where(title: FAKE_FILE_ASSAY_NAME).first return assay if assay - assay_params = {assay_class_id: AssayClass.for_type('experimental').id, + assay_params = {assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id, title: FAKE_FILE_ASSAY_NAME, description: 'Automatically generated assay to host openbis files that are linked to the original OpenBIS experiment. Its content and linked data files will be updated by the system @@ -446,7 +446,7 @@ def study_types(openbis_endpoint) Seek::Openbis::EntityType.ExperimentType(openbis_endpoint).find_by_codes(study_codes) end - + end end end diff --git a/lib/seek/projects/population.rb b/lib/seek/projects/population.rb index 2ab008c1e4..f0fa3d1e20 100644 --- a/lib/seek/projects/population.rb +++ b/lib/seek/projects/population.rb @@ -21,7 +21,7 @@ def populate_from_spreadsheet_impl r = sheet.rows[1] if r.cell(1).value.blank? - flash[:error]= "Unable to find header cells in #{datafile.title}" + flash[:error]= "Unable to find header cells in #{datafile.title}" return end @@ -78,7 +78,7 @@ def populate_from_spreadsheet_impl assay_position = 1 investigation.save! end - + if r.cell(study_index)&.value.present? if investigation.blank? flash[:error]= "Study specified without Investigation in #{datafile.title} at row #{r.index}" @@ -111,7 +111,7 @@ def populate_from_spreadsheet_impl set_description(assay, r, description_index) assay.position = assay_position assay_position += 1 - assay.assay_class = AssayClass.for_type('experimental') + assay.assay_class = AssayClass.for_type(Seek::ISA::AssayClass::EXP) set_assignees(assay, r, assignee_indices) @@ -134,7 +134,7 @@ def set_description(object, r, description_index) end object.description = description end - + def set_assignees(assay, r, assignee_indices) assignees = [] assignee_indices.each do |x| @@ -154,7 +154,7 @@ def set_assignees(assay, r, assignee_indices) end assay.creators = known_creators assay.other_creators = other_creators.join(';') - end + end def set_protocol(assay, r, protocol_index) protocol_string = r.cell(protocol_index)&.value&.to_s.strip diff --git a/test/factories/assays.rb b/test/factories/assays.rb index dfc298b213..ad88ee763f 100644 --- a/test/factories/assays.rb +++ b/test/factories/assays.rb @@ -4,18 +4,18 @@ factory(:modelling_assay_class, class: AssayClass) do title { I18n.t('assays.modelling_analysis') } - key { 'MODEL' } + key { Seek::ISA::AssayClass::MODEL } end factory(:experimental_assay_class, class: AssayClass) do title { I18n.t('assays.experimental_assay') } - key { 'EXP' } + key { Seek::ISA::AssayClass::EXP } description { "An experimental assay class description" } end factory(:assay_stream_class, class: AssayClass) do title { I18n.t('assays.assay_stream') } - key { 'ASS' } + key { Seek::ISA::AssayClass::STREAM } description { "An assay stream class description" } end diff --git a/test/fixtures/assay_classes.yml b/test/fixtures/assay_classes.yml index 4a90f2e308..cf87a8f7d1 100644 --- a/test/fixtures/assay_classes.yml +++ b/test/fixtures/assay_classes.yml @@ -8,4 +8,4 @@ modelling_assay_class: assay_stream_class: title: <%= I18n.t('assays.assay_stream') %> - key: ASS + key: STREAM diff --git a/test/functional/isa_assays_controller_test.rb b/test/functional/isa_assays_controller_test.rb index 1cf8c6b35f..665aa54dcc 100644 --- a/test/functional/isa_assays_controller_test.rb +++ b/test/functional/isa_assays_controller_test.rb @@ -45,7 +45,7 @@ def setup sop_ids: [FactoryBot.create(:sop, policy: FactoryBot.create(:public_policy)).id], creator_ids: [this_person.id, other_creator.id], other_creators: 'other collaborators', - assay_class_id: AssayClass.for_type('experimental').id, + assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id, position: 0, policy_attributes: }, input_sample_type_id: sample_collection_sample_type.id, sample_type: { title: 'assay sample_type', project_ids: [projects.first.id], template_id: 1, @@ -189,7 +189,7 @@ def setup sop_ids: [FactoryBot.create(:sop, policy: FactoryBot.create(:public_policy)).id], creator_ids: [this_person.id, other_creator.id], other_creators: 'other collaborators', - position: 0, assay_class_id: AssayClass.for_type('experimental').id, policy_attributes: } + position: 0, assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id, policy_attributes: } isa_assay_attributes = { assay: assay_attributes.merge(emt_attributes), input_sample_type_id: sample_collection_sample_type.id, diff --git a/test/unit/assay_class_test.rb b/test/unit/assay_class_test.rb index 3a3d002f6d..c78b3ec6c1 100644 --- a/test/unit/assay_class_test.rb +++ b/test/unit/assay_class_test.rb @@ -4,12 +4,12 @@ class AssayClassTest < ActiveSupport::TestCase # Replace this with your real tests. fixtures :assay_classes test 'for_type' do - assert_equal 'EXP', AssayClass.for_type('experimental').key - assert_equal 'MODEL', AssayClass.for_type('modelling').key + assert_equal 'EXP', AssayClass.for_type(Seek::ISA::AssayClass::EXP).key + assert_equal 'MODEL', AssayClass.for_type(Seek::ISA::AssayClass::MODEL).key end test 'is_modelling?' do - assert AssayClass.for_type('modelling').is_modelling? - refute AssayClass.for_type('experimental').is_modelling? + assert AssayClass.for_type(Seek::ISA::AssayClass::MODEL).is_modelling? + refute AssayClass.for_type(Seek::ISA::AssayClass::EXP).is_modelling? end end diff --git a/test/unit/ontologies/ontology_synchronization_test.rb b/test/unit/ontologies/ontology_synchronization_test.rb index 50aeb1c446..c8e5836f14 100644 --- a/test/unit/ontologies/ontology_synchronization_test.rb +++ b/test/unit/ontologies/ontology_synchronization_test.rb @@ -3,10 +3,10 @@ class OntologySynchronizationTest < ActiveSupport::TestCase def setup - unless AssayClass.for_type('modelling') + unless AssayClass.for_type(Seek::ISA::AssayClass::MODEL) FactoryBot.create(:modelling_assay_class) end - unless AssayClass.for_type('experimental') + unless AssayClass.for_type(Seek::ISA::AssayClass::EXP) FactoryBot.create(:experimental_assay_class) end end From 518ebc2656070576a0a1261890ac899c5bdca519 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 24 Jan 2024 16:56:26 +0100 Subject: [PATCH 34/41] Let the database do the job instead ruby in memory --- lib/tasks/seek_upgrades.rake | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/tasks/seek_upgrades.rake b/lib/tasks/seek_upgrades.rake index 0797d29613..b936eda04b 100644 --- a/lib/tasks/seek_upgrades.rake +++ b/lib/tasks/seek_upgrades.rake @@ -186,15 +186,15 @@ namespace :seek do # Should be isa json compliant # Shouldn't already have an assay stream (don't update assays that have been updated already) # Previous ST should be second ST of study - first_assays_in_stream = Assay.joins(:sample_type) - .select(&:is_isa_json_compliant?) - .select { |a| a.assay_stream_id.nil? && (a.previous_linked_sample_type == a.study.sample_types.second) } + first_assays_in_stream = Assay.joins(:sample_type, study: :investigation) + .where(assay_stream_id: nil, investigation: { is_isa_json_compliant: true }) + .select { |a| a.previous_linked_sample_type == a.study.sample_types.second } first_assays_in_stream.map do |fas| stream_name = "Assay Stream - #{UUID.generate}" assay_stream = Assay.create(title: stream_name, study_id: fas.study_id, - assay_class_id: AssayClass.for_type('assay_stream').id, + assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id, contributor: fas.contributor, position: 0) From a6089802363ea89d2ff03078e38f7431c3c8bca5 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Wed, 24 Jan 2024 17:23:08 +0100 Subject: [PATCH 35/41] Fix failing tests --- test/functional/assays_controller_test.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/functional/assays_controller_test.rb b/test/functional/assays_controller_test.rb index 4ee3790a91..c4e6bc2f7e 100644 --- a/test/functional/assays_controller_test.rb +++ b/test/functional/assays_controller_test.rb @@ -587,14 +587,14 @@ def test_title test 'get new with class doesnt present options for class' do login_as(:model_owner) - get :new, params: { class: 'experimental' } + get :new, params: { class: Seek::ISA::AssayClass::EXP } assert_response :success assert_select 'a[href=?]', new_assay_path(class: :experimental), count: 0 assert_select 'a', text: /An #{I18n.t('assays.experimental_assay')}/i, count: 0 assert_select 'a[href=?]', new_assay_path(class: :modelling), count: 0 assert_select 'a', text: /A #{I18n.t('assays.modelling_analysis')}/i, count: 0 - get :new, params: { class: 'modelling' } + get :new, params: { class: Seek::ISA::AssayClass::MODEL } assert_response :success assert_select 'a[href=?]', new_assay_path(class: :experimental), count: 0 assert_select 'a', text: /An #{I18n.t('assays.experimental_assay')}/i, count: 0 @@ -1065,7 +1065,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links end test 'new should include tags element' do - get :new, params: { class: :experimental } + get :new, params: { class: Seek::ISA::AssayClass::EXP } assert_response :success assert_select 'div.panel-heading', text: /Tags/, count: 1 assert_select 'select#tag_list', count: 1 @@ -1121,7 +1121,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links end test 'should show experimental assay types for new experimental assay' do - get :new, params: { class: :experimental } + get :new, params: { class: Seek::ISA::AssayClass::EXP } assert_response :success assert_select 'label', text: /assay type/i assert_select 'select#assay_assay_type_uri' do @@ -1131,7 +1131,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links end test 'should show modelling assay types for new modelling assay' do - get :new, params: { class: :modelling } + get :new, params: { class: Seek::ISA::AssayClass::MODEL } assert_response :success assert_select 'label', text: /Biological problem addressed/i assert_select 'select#assay_assay_type_uri' do From 4c8c9cd0a7b088195b40d65ff1a4d2efb890ee1d Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 25 Jan 2024 08:57:04 +0100 Subject: [PATCH 36/41] Use constants instead of keys as string --- app/models/assay_class.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index bb53cdec8e..fb98ad0bb3 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -33,6 +33,6 @@ def is_assay_stream? # for cases where a longer more descriptive key is useful, but can't rely on the title # which may have been changed over time def long_key - { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'STREAM': 'Assay Stream' }[key.to_sym] + { "#{Seek::ISA::AssayClass::EXP}": 'Experimental Assay', "#{Seek::ISA::AssayClass::MODEL}": 'Modelling Analysis', "#{Seek::ISA::AssayClass::STREAM}": 'Assay Stream' }[key.to_sym] end end From f3a0251eadd2351c1d4a4016454af73805c55993 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 25 Jan 2024 09:04:15 +0100 Subject: [PATCH 37/41] Use the assay class's long_key method to determin the button text --- app/views/assays/_buttons.html.erb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/views/assays/_buttons.html.erb b/app/views/assays/_buttons.html.erb index 6c5b795cc5..72c59241fe 100644 --- a/app/views/assays/_buttons.html.erb +++ b/app/views/assays/_buttons.html.erb @@ -1,12 +1,10 @@ <% assay_word ||= - if item.is_modelling? - t('assays.modelling_analysis') - elsif item.is_assay_stream? + if item.is_assay_stream? t('assays.assay_stream') elsif Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? t('isa_assay') else - t('assays.assay') + t("assays.#{item.assay_class.long_key.delete(' ').underscore}") end %> <%= render :partial => "subscriptions/subscribe", :locals => {:object => item} %> From a8316e2226c4e2a02f69765ae52e72378bc2d1dd Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 1 Feb 2024 09:20:53 +0100 Subject: [PATCH 38/41] Remove the AssayClass constants --- app/controllers/assays_controller.rb | 4 ++-- app/controllers/isa_assays_controller.rb | 8 ++++---- app/models/assay_class.rb | 14 +++++++------- app/views/isa_assays/_form.html.erb | 4 ++-- lib/seek/isa/assay_class.rb | 17 ----------------- lib/seek/ontologies/synchronize.rb | 2 +- lib/seek/openbis/seek_util.rb | 4 ++-- lib/seek/projects/population.rb | 2 +- lib/tasks/seek_upgrades.rake | 2 +- test/factories/assays.rb | 6 +++--- test/functional/assays_controller_test.rb | 10 +++++----- test/functional/isa_assays_controller_test.rb | 4 ++-- test/unit/assay_class_test.rb | 8 ++++---- .../ontologies/ontology_synchronization_test.rb | 4 ++-- 14 files changed, 36 insertions(+), 53 deletions(-) delete mode 100644 lib/seek/isa/assay_class.rb diff --git a/app/controllers/assays_controller.rb b/app/controllers/assays_controller.rb index 900ddef19e..007e3f7da9 100644 --- a/app/controllers/assays_controller.rb +++ b/app/controllers/assays_controller.rb @@ -70,7 +70,7 @@ def new @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 + @assay_class ||= 'EXP' unless Seek::Config.modelling_analysis_enabled @assay.assay_class = AssayClass.for_type(@assay_class) unless @assay_class.nil? @@ -82,7 +82,7 @@ def edit end def create - params[:assay_class_id] ||= AssayClass.for_type(Seek::ISA::AssayClass::EXP).id + params[:assay_class_id] ||= AssayClass.experimental.id @assay = Assay.new(assay_params) update_assay_organisms @assay, params diff --git a/app/controllers/isa_assays_controller.rb b/app/controllers/isa_assays_controller.rb index 7771d17b3e..cc0c8128ed 100644 --- a/app/controllers/isa_assays_controller.rb +++ b/app/controllers/isa_assays_controller.rb @@ -7,9 +7,9 @@ class IsaAssaysController < ApplicationController def new if params[:is_assay_stream] - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.assay_stream.id } }) else - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.experimental.id } }) end end @@ -141,9 +141,9 @@ def set_up_instance_variable def find_requested_item if params[:is_assay_stream] - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.assay_stream.id } }) else - @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id } }) + @isa_assay = IsaAssay.new({ assay: { assay_class_id: AssayClass.experimental.id } }) end @isa_assay.populate(params[:id]) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index fb98ad0bb3..42298a17dd 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -6,33 +6,33 @@ def self.for_type(type) end def self.experimental - for_type(Seek::ISA::AssayClass::EXP) + for_type('EXP') end def self.modelling - for_type(Seek::ISA::AssayClass::MODEL) + for_type('MODEL') end def self.assay_stream - for_type(Seek::ISA::AssayClass::STREAM) + for_type('STREAM') end def is_modelling? - key == Seek::ISA::AssayClass::MODEL + key == 'MODEL' end def is_experimental? - key == Seek::ISA::AssayClass::EXP + key == 'EXP' end def is_assay_stream? - key == Seek::ISA::AssayClass::STREAM + key == 'STREAM' end # for cases where a longer more descriptive key is useful, but can't rely on the title # which may have been changed over time def long_key - { "#{Seek::ISA::AssayClass::EXP}": 'Experimental Assay', "#{Seek::ISA::AssayClass::MODEL}": 'Modelling Analysis', "#{Seek::ISA::AssayClass::STREAM}": 'Assay Stream' }[key.to_sym] + { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'STREAM': 'Assay Stream' }[key.to_sym] end end diff --git a/app/views/isa_assays/_form.html.erb b/app/views/isa_assays/_form.html.erb index 06b9a057ad..cc707ccf20 100644 --- a/app/views/isa_assays/_form.html.erb +++ b/app/views/isa_assays/_form.html.erb @@ -7,11 +7,11 @@ if @isa_assay.assay.new_record? if params[:is_assay_stream] assay_position = 0 - assay_class_id = AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id + 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_class_id = AssayClass.for_type(Seek::ISA::AssayClass::EXP).id + assay_class_id = AssayClass.experimental.id is_assay_stream = false end else diff --git a/lib/seek/isa/assay_class.rb b/lib/seek/isa/assay_class.rb deleted file mode 100644 index 9be0b02dce..0000000000 --- a/lib/seek/isa/assay_class.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Seek - module ISA - module AssayClass - # Creates constants based on the AssayClass key attributes - # Example: AssayClass key 'EXP' can be represented by Seek::ISA:AssayClass::EXP - ALL_TYPES = %w[EXP MODEL STREAM] - - ALL_TYPES.each do |type| - AssayClass.const_set(type.underscore.upcase, type) - end - - def self.valid?(value) - ALL_TYPES.include?(value) - end - end - end -end diff --git a/lib/seek/ontologies/synchronize.rb b/lib/seek/ontologies/synchronize.rb index fd3b83d4c4..cfce103be3 100644 --- a/lib/seek/ontologies/synchronize.rb +++ b/lib/seek/ontologies/synchronize.rb @@ -99,7 +99,7 @@ def assay_changes_class?(assays, ontology_uri) def determine_assay_class_from_uri(uri) ontology_class = Seek::Ontologies::AssayTypeReader.instance.class_for_uri(uri) - ontology_class.nil? ? AssayClass.for_type(Seek::ISA::AssayClass::MODEL) : AssayClass.for_type(Seek::ISA::AssayClass::EXP) + ontology_class.nil? ? AssayClass.modelling : AssayClass.experimental end def get_suggested_types_found_in_ontology(type) diff --git a/lib/seek/openbis/seek_util.rb b/lib/seek/openbis/seek_util.rb index f6303e2e18..a99b15949e 100644 --- a/lib/seek/openbis/seek_util.rb +++ b/lib/seek/openbis/seek_util.rb @@ -27,7 +27,7 @@ def createObisAssay(assay_params, creator, obis_asset) zample = obis_asset.content openbis_endpoint = obis_asset.seek_service - assay_params[:assay_class_id] ||= AssayClass.for_type(Seek::ISA::AssayClass::EXP).id + assay_params[:assay_class_id] ||= AssayClass.experimental.id assay_params[:title] ||= extract_title(zample) ## "OpenBIS #{zample.perm_id}" assay = Assay.new(assay_params) @@ -88,7 +88,7 @@ def fake_file_assay(study) assay = study.assays.where(title: FAKE_FILE_ASSAY_NAME).first return assay if assay - assay_params = {assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id, + assay_params = {assay_class_id: AssayClass.experimental.id, title: FAKE_FILE_ASSAY_NAME, description: 'Automatically generated assay to host openbis files that are linked to the original OpenBIS experiment. Its content and linked data files will be updated by the system diff --git a/lib/seek/projects/population.rb b/lib/seek/projects/population.rb index f0fa3d1e20..faeb03241b 100644 --- a/lib/seek/projects/population.rb +++ b/lib/seek/projects/population.rb @@ -111,7 +111,7 @@ def populate_from_spreadsheet_impl set_description(assay, r, description_index) assay.position = assay_position assay_position += 1 - assay.assay_class = AssayClass.for_type(Seek::ISA::AssayClass::EXP) + assay.assay_class = AssayClass.experimental set_assignees(assay, r, assignee_indices) diff --git a/lib/tasks/seek_upgrades.rake b/lib/tasks/seek_upgrades.rake index b936eda04b..e9b1ae3c15 100644 --- a/lib/tasks/seek_upgrades.rake +++ b/lib/tasks/seek_upgrades.rake @@ -194,7 +194,7 @@ namespace :seek do stream_name = "Assay Stream - #{UUID.generate}" assay_stream = Assay.create(title: stream_name, study_id: fas.study_id, - assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::STREAM).id, + assay_class_id: AssayClass.assay_stream.id, contributor: fas.contributor, position: 0) diff --git a/test/factories/assays.rb b/test/factories/assays.rb index ad88ee763f..24f95f1ae0 100644 --- a/test/factories/assays.rb +++ b/test/factories/assays.rb @@ -4,18 +4,18 @@ factory(:modelling_assay_class, class: AssayClass) do title { I18n.t('assays.modelling_analysis') } - key { Seek::ISA::AssayClass::MODEL } + key { 'MODEL' } end factory(:experimental_assay_class, class: AssayClass) do title { I18n.t('assays.experimental_assay') } - key { Seek::ISA::AssayClass::EXP } + key { 'EXP' } description { "An experimental assay class description" } end factory(:assay_stream_class, class: AssayClass) do title { I18n.t('assays.assay_stream') } - key { Seek::ISA::AssayClass::STREAM } + key { 'STREAM' } description { "An assay stream class description" } end diff --git a/test/functional/assays_controller_test.rb b/test/functional/assays_controller_test.rb index c4e6bc2f7e..1996d36918 100644 --- a/test/functional/assays_controller_test.rb +++ b/test/functional/assays_controller_test.rb @@ -587,14 +587,14 @@ def test_title test 'get new with class doesnt present options for class' do login_as(:model_owner) - get :new, params: { class: Seek::ISA::AssayClass::EXP } + get :new, params: { class: 'EXP' } assert_response :success assert_select 'a[href=?]', new_assay_path(class: :experimental), count: 0 assert_select 'a', text: /An #{I18n.t('assays.experimental_assay')}/i, count: 0 assert_select 'a[href=?]', new_assay_path(class: :modelling), count: 0 assert_select 'a', text: /A #{I18n.t('assays.modelling_analysis')}/i, count: 0 - get :new, params: { class: Seek::ISA::AssayClass::MODEL } + get :new, params: { class: 'MODEL' } assert_response :success assert_select 'a[href=?]', new_assay_path(class: :experimental), count: 0 assert_select 'a', text: /An #{I18n.t('assays.experimental_assay')}/i, count: 0 @@ -1065,7 +1065,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links end test 'new should include tags element' do - get :new, params: { class: Seek::ISA::AssayClass::EXP } + get :new, params: { class: 'EXP' } assert_response :success assert_select 'div.panel-heading', text: /Tags/, count: 1 assert_select 'select#tag_list', count: 1 @@ -1121,7 +1121,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links end test 'should show experimental assay types for new experimental assay' do - get :new, params: { class: Seek::ISA::AssayClass::EXP } + get :new, params: { class: 'EXP' } assert_response :success assert_select 'label', text: /assay type/i assert_select 'select#assay_assay_type_uri' do @@ -1131,7 +1131,7 @@ def check_fixtures_for_authorization_of_sops_and_datafiles_links end test 'should show modelling assay types for new modelling assay' do - get :new, params: { class: Seek::ISA::AssayClass::MODEL } + get :new, params: { class: 'MODEL' } assert_response :success assert_select 'label', text: /Biological problem addressed/i assert_select 'select#assay_assay_type_uri' do diff --git a/test/functional/isa_assays_controller_test.rb b/test/functional/isa_assays_controller_test.rb index 665aa54dcc..1212dd21d1 100644 --- a/test/functional/isa_assays_controller_test.rb +++ b/test/functional/isa_assays_controller_test.rb @@ -45,7 +45,7 @@ def setup sop_ids: [FactoryBot.create(:sop, policy: FactoryBot.create(:public_policy)).id], creator_ids: [this_person.id, other_creator.id], other_creators: 'other collaborators', - assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id, + assay_class_id: AssayClass.experimental.id, position: 0, policy_attributes: }, input_sample_type_id: sample_collection_sample_type.id, sample_type: { title: 'assay sample_type', project_ids: [projects.first.id], template_id: 1, @@ -189,7 +189,7 @@ def setup sop_ids: [FactoryBot.create(:sop, policy: FactoryBot.create(:public_policy)).id], creator_ids: [this_person.id, other_creator.id], other_creators: 'other collaborators', - position: 0, assay_class_id: AssayClass.for_type(Seek::ISA::AssayClass::EXP).id, policy_attributes: } + position: 0, assay_class_id: AssayClass.experimental.id, policy_attributes: } isa_assay_attributes = { assay: assay_attributes.merge(emt_attributes), input_sample_type_id: sample_collection_sample_type.id, diff --git a/test/unit/assay_class_test.rb b/test/unit/assay_class_test.rb index c78b3ec6c1..3ef0593df8 100644 --- a/test/unit/assay_class_test.rb +++ b/test/unit/assay_class_test.rb @@ -4,12 +4,12 @@ class AssayClassTest < ActiveSupport::TestCase # Replace this with your real tests. fixtures :assay_classes test 'for_type' do - assert_equal 'EXP', AssayClass.for_type(Seek::ISA::AssayClass::EXP).key - assert_equal 'MODEL', AssayClass.for_type(Seek::ISA::AssayClass::MODEL).key + assert_equal 'EXP', AssayClass.experimental.key + assert_equal 'MODEL', AssayClass.modelling.key end test 'is_modelling?' do - assert AssayClass.for_type(Seek::ISA::AssayClass::MODEL).is_modelling? - refute AssayClass.for_type(Seek::ISA::AssayClass::EXP).is_modelling? + assert AssayClass.modelling.is_modelling? + refute AssayClass.experimental.is_modelling? end end diff --git a/test/unit/ontologies/ontology_synchronization_test.rb b/test/unit/ontologies/ontology_synchronization_test.rb index c8e5836f14..06ba1d31df 100644 --- a/test/unit/ontologies/ontology_synchronization_test.rb +++ b/test/unit/ontologies/ontology_synchronization_test.rb @@ -3,10 +3,10 @@ class OntologySynchronizationTest < ActiveSupport::TestCase def setup - unless AssayClass.for_type(Seek::ISA::AssayClass::MODEL) + unless AssayClass.modelling FactoryBot.create(:modelling_assay_class) end - unless AssayClass.for_type(Seek::ISA::AssayClass::EXP) + unless AssayClass.experimental FactoryBot.create(:experimental_assay_class) end end From cd06baa6b8cceedbc928f8dac2892586a225ac87 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 1 Feb 2024 09:36:07 +0100 Subject: [PATCH 39/41] Move long key mapping to a constant outside --- app/models/assay_class.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/models/assay_class.rb b/app/models/assay_class.rb index 42298a17dd..5e8940f9ee 100644 --- a/app/models/assay_class.rb +++ b/app/models/assay_class.rb @@ -30,9 +30,11 @@ def is_assay_stream? key == 'STREAM' end + LONG_KEYS = { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'STREAM': 'Assay Stream' }.freeze + # for cases where a longer more descriptive key is useful, but can't rely on the title # which may have been changed over time def long_key - { 'EXP': 'Experimental Assay', 'MODEL': 'Modelling Analysis', 'STREAM': 'Assay Stream' }[key.to_sym] + LONG_KEYS[key.to_sym] end end From 14089f6ed19079e01c3b0af4f79fcea2d5948aa1 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 1 Feb 2024 10:09:34 +0100 Subject: [PATCH 40/41] Use the definition instead of manual text setting --- app/views/studies/_buttons.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/studies/_buttons.html.erb b/app/views/studies/_buttons.html.erb index 4b63c58c86..a308561224 100644 --- a/app/views/studies/_buttons.html.erb +++ b/app/views/studies/_buttons.html.erb @@ -21,7 +21,7 @@ <% if item.can_edit? -%> <% if Seek::Config.isa_json_compliance_enabled && item.is_isa_json_compliant? %> <% if item&.sample_types.present? %> - <%= button_link_to("Design #{t('assay')} Stream", 'new', new_isa_assay_path(study_id: item.id, single_page: params[:single_page], is_assay_stream: true)) %> + <%= button_link_to("Design #{t('assays.assay_stream')}", 'new', new_isa_assay_path(study_id: item.id, single_page: params[:single_page], is_assay_stream: true)) %> <% end -%> <% else -%> <%= add_new_item_to_dropdown(item) %> From 6fdd31fee6dcd2095187536a0d6204b035943065 Mon Sep 17 00:00:00 2001 From: Kevin De Pelseneer Date: Thu, 1 Feb 2024 10:12:05 +0100 Subject: [PATCH 41/41] Use assay_streams method instead of select statement --- lib/treeview_builder.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/treeview_builder.rb b/lib/treeview_builder.rb index 3996b2da2b..6f89ddf2d3 100644 --- a/lib/treeview_builder.rb +++ b/lib/treeview_builder.rb @@ -16,7 +16,7 @@ def build_tree_data @project.investigations.map do |investigation| if investigation.is_isa_json_compliant? investigation.studies.map do |study| - assay_stream_items = study.assays.select { |assay| assay.is_assay_stream? }.map do |assay_stream| + assay_stream_items = study.assay_streams.map do |assay_stream| assay_items = assay_stream.child_assays.map do |child_assay| build_assay_item(child_assay) end