diff --git a/.gitignore b/.gitignore index 1afabaef95..d42ad8f7d2 100644 --- a/.gitignore +++ b/.gitignore @@ -40,6 +40,10 @@ doc/* .yardoc/* app/sample_manifest_excel/doc/ +# Ignore generated diagrams from Mermaid +docs/accessioning/**/*.png +docs/accessioning/**/*.svg + # Test files capybara*.html spec/examples.txt diff --git a/.release-version b/.release-version index 8da807562f..de2abfbc75 100644 --- a/.release-version +++ b/.release-version @@ -1 +1 @@ -14.86.0 +14.87.0 diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 0d0948b068..88182357af 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --no-exclude-limit` -# on 2026-02-06 11:06:52 UTC using RuboCop version 1.84.1. +# on 2026-02-06 14:12:50 UTC using RuboCop version 1.84.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -126,7 +126,7 @@ Lint/DuplicateMethods: - 'app/models/stock_stamper.rb' - 'lib/accession/tag.rb' -# Offense count: 63 +# Offense count: 62 # Configuration parameters: AllowComments, AllowEmptyLambdas. Lint/EmptyBlock: Exclude: @@ -164,7 +164,6 @@ Lint/EmptyBlock: - 'app/api/endpoints/users.rb' - 'app/api/endpoints/uuids.rb' - 'app/api/endpoints/wells.rb' - - 'app/models/accessionable/sample.rb' - 'app/models/plate.rb' - 'app/models/request/library_creation.rb' - 'app/sequencescape_excel/sequencescape_excel/list.rb' @@ -194,12 +193,11 @@ Lint/IneffectiveAccessModifier: - 'app/helpers/plates_helper.rb' - 'lib/authenticated_system.rb' -# Offense count: 14 +# Offense count: 13 # Configuration parameters: AllowedParentClasses. Lint/MissingSuper: Exclude: - 'app/models/accession_service/no_service.rb' - - 'app/models/accession_service/unsuitable_service.rb' - 'app/models/delegate_validation.rb' - 'app/models/request/change_decision.rb' - 'app/models/sample_manifest/library_tube_behaviour.rb' @@ -240,7 +238,7 @@ Lint/StructNewOverride: Exclude: - 'app/models/product_criteria/basic.rb' -# Offense count: 61 +# Offense count: 57 # This cop supports safe autocorrection (--autocorrect). Lint/UselessAssignment: Exclude: @@ -580,18 +578,16 @@ Performance/MethodObjectAsBlock: - 'features/support/step_definitions/transfer_steps.rb' - 'lib/tasks/report.rake' -# Offense count: 3 +# Offense count: 2 RSpec/AnyInstance: Exclude: - - 'spec/controllers/samples_controller_spec.rb' - 'spec/controllers/studies_controller_spec.rb' - 'spec/models/lane_spec.rb' -# Offense count: 24 +# Offense count: 22 RSpec/BeforeAfterAll: Exclude: - 'spec/features/sample_manifests/uploader_for_manifests_with_tag_sequences_spec.rb' - - 'spec/lib/accession/contact_spec.rb' - 'spec/models/sample_manifest/generator_spec.rb' - 'spec/models/sample_manifest/uploader_spec.rb' - 'spec/sample_manifest_excel/download_spec.rb' @@ -603,7 +599,7 @@ RSpec/BeforeAfterAll: - 'spec/sample_manifest_excel/worksheet_spec.rb' - 'spec/sequencescape_excel/worksheet_spec.rb' -# Offense count: 341 +# Offense count: 342 # Configuration parameters: Prefixes, AllowedPatterns. # Prefixes: when, with, without RSpec/ContextWording: @@ -684,7 +680,7 @@ RSpec/EmptyExampleGroup: - 'spec/models/pulldown/requests_spec.rb' - 'spec/models/tag_substitutions_spec.rb' -# Offense count: 376 +# Offense count: 377 # Configuration parameters: Max, CountAsOne. RSpec/ExampleLength: Exclude: @@ -828,14 +824,13 @@ RSpec/ExampleWording: - 'spec/sequencescape_excel/validation_spec.rb' - 'spec/sequencescape_excel/worksheet_spec.rb' -# Offense count: 258 +# Offense count: 257 # Configuration parameters: AssignmentOnly. RSpec/InstanceVariable: Exclude: - 'spec/api/extraction_attributes_spec.rb' - 'spec/controllers/studies_controller_spec.rb' - 'spec/controllers/submissions_controller_spec.rb' - - 'spec/lib/accession/contact_spec.rb' - 'spec/models/orders/order_spec.rb' - 'spec/models/pulldown/requests_spec.rb' - 'spec/models/qc_report_spec.rb' @@ -877,7 +872,7 @@ RSpec/LeakyLocalVariable: - 'spec/models/well_spec.rb' - 'spec/shared_contexts/it_requires_login.rb' -# Offense count: 38 +# Offense count: 39 RSpec/LetSetup: Exclude: - 'spec/controllers/searches_controller_spec.rb' @@ -938,7 +933,7 @@ RSpec/MultipleDescribes: - 'spec/lib/label_printer/asset_labels_spec.rb' - 'spec/models/qc_result/qc_result_spec.rb' -# Offense count: 926 +# Offense count: 925 # Configuration parameters: Max. RSpec/MultipleExpectations: Exclude: @@ -980,7 +975,6 @@ RSpec/MultipleExpectations: - 'spec/jobs/export_pool_xp_to_traction_job_spec.rb' - 'spec/lib/accession/accessionable_spec.rb' - 'spec/lib/accession/configuration_spec.rb' - - 'spec/lib/accession/contact_spec.rb' - 'spec/lib/accession/sample_spec.rb' - 'spec/lib/accession/service_spec.rb' - 'spec/lib/accession/submission_spec.rb' @@ -1168,7 +1162,7 @@ RSpec/MultipleExpectations: - 'spec/views/labware/show_chromium_chip_spec.rb' - 'spec/views/samples/index_html_erb_spec.rb' -# Offense count: 248 +# Offense count: 246 # Configuration parameters: EnforcedStyle, IgnoreSharedExamples. # SupportedStyles: always, named_only RSpec/NamedSubject: @@ -1186,7 +1180,6 @@ RSpec/NamedSubject: - 'spec/api/work_completion_spec.rb' - 'spec/controllers/studies/information_controller_spec.rb' - 'spec/controllers/studies_controller_spec.rb' - - 'spec/lib/accession/contact_spec.rb' - 'spec/lib/label_printer/sample_manifest_plate_double_spec.rb' - 'spec/models/aliquot_spec.rb' - 'spec/models/api/library_tube_io_spec.rb' @@ -1899,7 +1892,7 @@ Style/Not: - 'app/views/batches/show.xml.builder' - 'features/support/step_definitions/api_steps.rb' -# Offense count: 32 +# Offense count: 30 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle, AllowedMethods, AllowedPatterns. # SupportedStyles: predicate, comparison @@ -1917,7 +1910,6 @@ Style/NumericPredicate: - 'app/models/study.rb' - 'app/models/tag_layout/walk_wells_by_pools.rb' - 'app/models/well_attribute.rb' - - 'features/support/step_definitions/samples_steps.rb' - 'lib/deployed.rb' - 'lib/submission_serializer.rb' @@ -2035,14 +2027,13 @@ Style/Proc: - 'app/models/lib_pool_norm_tube_generator.rb' - 'app/models/qcable/statemachine.rb' -# Offense count: 4 +# Offense count: 3 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: Methods. Style/RedundantArgument: Exclude: - 'app/models/plate/fluidigm_behaviour.rb' - 'app/models/study_report/study_details.rb' - - 'features/support/step_definitions/samples_steps.rb' - 'spec/features/shared_examples/cherrypicking.rb' # Offense count: 7 @@ -2138,10 +2129,9 @@ Style/SymbolProc: - 'app/models/tasks/plate_transfer_handler.rb' - 'db/seeds/0001_workflows.rb' -# Offense count: 8 +# Offense count: 6 # This cop supports unsafe autocorrection (--autocorrect-all). Style/ZeroLengthPredicate: Exclude: - 'app/models/bulk_submission.rb' - 'app/models/sequencing_pipeline.rb' - - 'features/support/step_definitions/samples_steps.rb' diff --git a/Gemfile.lock b/Gemfile.lock index eace765b67..96c2eec927 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -134,7 +134,7 @@ GEM base64 (0.3.0) benchmark (0.5.0) bigdecimal (3.3.1) - bootsnap (1.21.1) + bootsnap (1.22.0) msgpack (~> 1.2) builder (3.3.0) bullet (8.1.0) @@ -236,7 +236,7 @@ GEM factory_bot_rails (6.5.1) factory_bot (~> 6.5) railties (>= 6.1.0) - faraday (2.14.0) + faraday (2.14.1) faraday-net_http (>= 2.0, < 3.5) json logger @@ -289,7 +289,7 @@ GEM railties (>= 4.1) jsonapi-resources-matchers (1.0.0) jsonapi-resources (>= 0.9.0) - knapsack_pro (9.2.1) + knapsack_pro (9.2.2) rake language_server-protocol (3.17.0.5) launchy (3.1.1) @@ -338,8 +338,8 @@ GEM mutex_m (0.3.0) mysql2 (0.5.7) bigdecimal - net-http (0.7.0) - uri + net-http (0.9.1) + uri (>= 0.11.1) net-imap (0.5.9) date net-protocol diff --git a/app/controllers/samples_controller.rb b/app/controllers/samples_controller.rb index 84363868c8..d2bb705bdf 100644 --- a/app/controllers/samples_controller.rb +++ b/app/controllers/samples_controller.rb @@ -149,7 +149,7 @@ def show_accession end end - def accession # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Metrics/MethodLength,Metrics/PerceivedComplexity + def accession # rubocop:disable Metrics/AbcSize,Metrics/MethodLength # @sample needs to be set before initially for use in the ensure block @sample = Sample.find(params[:id]) @@ -165,18 +165,11 @@ def accession # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Met # return # end + # Must check if an accession number is assigned _before_ performing the accession accession_action = @sample.accession_number? ? :update : :create - if Flipper.enabled?(:y25_286_accession_individual_samples_with_sample_accessioning_job) - # Synchronously perform accessioning job - Accession.accession_sample(@sample, current_user, perform_now: true) - else - # TODO: when removing the y25_286_accession_individual_samples_with_sample_accessioning_job feature flag - # and this accessioning path also remove the AccessionService and ActiveRecord errors below - @sample.validate_sample_for_accessioning! - accession_service = AccessionService.select_for_sample(@sample) - accession_service.submit_sample_for_user(@sample, current_user) - end + # Synchronously perform accessioning job + Accession.accession_sample(@sample, current_user, perform_now: true) if accession_action == :create flash[:notice] = "Accession number generated: #{@sample.sample_metadata.sample_ebi_accession_number}" @@ -185,23 +178,18 @@ def accession # rubocop:disable Metrics/AbcSize,Metrics/CyclomaticComplexity,Met end # Handle errors for both synchronous and asynchronous accessioning - # When the feature flag above (y25_286_accession_individual_samples_with_sample_accessioning_job) is removed, - # the AccessionService and ActiveRecord errors should also be removed. These errors are only raised in the old - # synchronous accessioning code path and are not required for the updated SampleAccessioningJob path. - rescue ActiveRecord::RecordInvalid, Accession::InternalValidationError + rescue Accession::InternalValidationError flash[:error] = "Please fill in the required fields: #{@sample.errors.full_messages.join(', ')}" redirect_to(edit_sample_path(@sample)) # send the user to edit the sample - rescue AccessionService::NumberNotRequired => e - flash[:warning] = e.message || 'An accession number is not required for this study' - rescue AccessionService::NumberNotGenerated, Accession::ExternalValidationError => e + rescue Accession::ExternalValidationError => e flash[:warning] = "No accession number was generated: #{e.message}" - rescue AccessionService::AccessionServiceError, Accession::Error => e + rescue Accession::Error => e flash[:error] = "Accessioning Service Failed: #{e.message}" rescue Faraday::Error => e flash[:error] = "Accessioning failed with a network error: #{e.message}" ensure # Redirect back to where we came from if not already redirected - redirect_back_with_anchor_or_to(sample_path(@sample), anchor: 'accession-statuses') unless performed? + redirect_back_with_anchor_or_to(sample_path(@sample)) unless performed? end private diff --git a/app/controllers/studies_controller.rb b/app/controllers/studies_controller.rb index 7598cc2899..cbf9546e1a 100644 --- a/app/controllers/studies_controller.rb +++ b/app/controllers/studies_controller.rb @@ -253,6 +253,16 @@ def accession end end + # Accession all samples in the study. + # + # If the study does not have an accession number, adds an error to the study and returns. + # Otherwise, iterates through each sample in the study and attempts to accession it, + # unless the sample already has an accession number. + # If an Accession::Error occurs for a sample, adds the error message to the study's errors. + # + # NOTE: this does not check if the current user has permission to accession samples in this study + # + # @return [void] def accession_all_samples # rubocop:disable Metrics/AbcSize,Metrics/MethodLength @study = Study.find(params[:id]) return accessioning_not_enabled_redirect unless accessioning_enabled? @@ -260,16 +270,32 @@ def accession_all_samples # rubocop:disable Metrics/AbcSize,Metrics/MethodLength # TODO: Y26-026 - Enforce accessioning permissions # return accession_permission_denied_redirect unless permitted_to_accession?(@study) - @study.accession_all_samples(current_user) + unless @study.accession_number? + flash[:error] = 'Please accession the study before accessioning samples' + return redirect_to(study_path(@study)) + end + unless @study.samples_accessionable? + flash[:error] = 'Study cannot accession samples, see Study Accessioning tab for details' + return redirect_to(study_path(@study)) + end + + @study.samples.find_each do |sample| + next if sample.accession_number? + + begin + Accession.accession_sample(sample, current_user) + rescue Accession::Error => e + @study.errors.add(:base, e.message) + end + end if @study.errors.any? - error_messages = compile_accession_errors(@study.errors) - flash[:error] = error_messages + flash[:error] = compile_accession_errors(@study.errors) else flash[:notice] = 'All of the samples in this study have been sent for accessioning. ' \ 'Please check back in 5 minutes to confirm that accessioning was successful.' end - redirect_to(study_path(@study, anchor: 'accession-statuses')) + redirect_to(study_path(@study)) end def dac_accession diff --git a/app/controllers/ultima_sample_sheet/sample_sheet_generator.rb b/app/controllers/ultima_sample_sheet/sample_sheet_generator.rb index 2bc5b509d3..212b3738c4 100644 --- a/app/controllers/ultima_sample_sheet/sample_sheet_generator.rb +++ b/app/controllers/ultima_sample_sheet/sample_sheet_generator.rb @@ -34,6 +34,14 @@ class Generator # rubocop:disable Metrics/ClassLength study_id ].freeze NUM_COLUMS = SAMPLES_HEADERS.size + # The names of the Ultima tag groups are mapped to the index numbers for + # the Barcode_Plate_Num column, i.e. 1 or 2. The number is also used for + # determining the consistent starting index number for the + # Index_Barcode_Num column, i.e. Z0001 or Z097. + ULTIMA_TAG_GROUPS = { + 'Ultima P1' => 1, + 'Ultima P2' => 2 + }.freeze # Initializes the generator with the given batch. # @param batch [UltimaSequencingBatch] the batch to generate sample sheets for @@ -115,7 +123,7 @@ def add_global_section(csv, _request) def add_samples_section(csv, request) csv << pad(SAMPLES_TITLE) csv << pad(SAMPLES_HEADERS) - request.asset.aliquots.each do |aliquot| + request.asset.aliquots.sort_by(&:id).each do |aliquot| csv << [ sample_id_for(aliquot), library_name_for(aliquot), @@ -187,28 +195,30 @@ def study_id_for(aliquot) aliquot.study_id.to_s end - # Returns a mapping of tags to their respective 1-based index numbers. - # This sorts the tags by their tag group ID and map ID to ensure consistent ordering. + # Returns a mapping of all Ultima tags to their respective 1-based index + # numbers. This sorts the tags by their tag group ID and map ID to ensure + # consistent ordering. The index numbers run across all Ultima tag groups, + # i.e. the index is 1 for the first tag in the first tag group and 97 for + # the first tag in the second tag group. # @return [Hash{Tag => Integer}] mapping of tags to index numbers def tag_index_map @tag_index_map ||= begin - tags = batch_tag_groups.flat_map { |tg| tg.tags.sort_by(&:map_id) } + tags = ultima_tag_groups.flat_map { |tg| tg.tags.sort_by(&:map_id) } tags.each_with_index.to_h { |tag, i| [tag, i + 1] } end end - # Returns a mapping of tag groups to 1-based index numbers. + # Returns a mapping of all Ultima tag groups to 1-based index numbers. + # This indexes the tag groups as given in the ULTIMA_TAG_GROUPS hash. # @return [Hash{TagGroup => Integer}] mapping of tag groups to index numbers def tag_group_index_map - @tag_group_index_map ||= batch_tag_groups.each_with_index.to_h { |tg, i| [tg, i + 1] } + @tag_group_index_map ||= ultima_tag_groups.index_with { |tg| ULTIMA_TAG_GROUPS[tg.name] } end - # Returns all unique tag groups used in the batch. - # This sorts the tag groups by their ID to ensure consistent ordering. - # @return [Array] the tag groups of the batch's requests - def batch_tag_groups - @batch_tag_groups ||= batch_requests - .flat_map { |request| request.asset.aliquots.map { |aliquot| aliquot.tag.tag_group } }.uniq.sort_by(&:id) + # Returns all unique tag groups used for Ultima sequencing from database. + # @return [Array] the tag groups used for Ultima sequencing + def ultima_tag_groups + @ultima_tag_groups ||= TagGroup.where(name: ULTIMA_TAG_GROUPS.keys) end # Returns the requests associated with the batch. diff --git a/app/helpers/samples_helper.rb b/app/helpers/samples_helper.rb index 03f3753c72..268545223d 100644 --- a/app/helpers/samples_helper.rb +++ b/app/helpers/samples_helper.rb @@ -4,8 +4,18 @@ module SamplesHelper # Indicate to the user that saving the sample will also accession it # This will not happen if the study has not been accessioned def save_text(sample) - return 'Save and Accession' if sample.should_be_accessioned? + if [accessioning_enabled?, sample.should_be_accessioned?, permitted_to_accession?(sample)].all? + return 'Save and Accession' + end 'Save Sample' end + + def samples_not_accessioned(samples) + return 'No samples accessioned' if samples.empty? || samples.none?(&:accession_number?) + return 'All samples accessioned' if samples.all?(&:accession_number?) + + count = samples.count { |sample| !sample.accession_number? } + "#{pluralize(count, 'sample')} not accessioned" + end end diff --git a/app/jobs/sample_accessioning_job.rb b/app/jobs/sample_accessioning_job.rb index 1397447363..0e397c5d94 100644 --- a/app/jobs/sample_accessioning_job.rb +++ b/app/jobs/sample_accessioning_job.rb @@ -8,14 +8,8 @@ # @see Accession::Submission SampleAccessioningJob = Struct.new(:accessionable, :event_user) do - # Retrieve the contact user for accessioning submissions - def self.contact_user - User.find_by(api_key: configatron.accession_local_key) - end - def perform - contact_user = self.class.contact_user - submission = Accession::Submission.new(contact_user, accessionable) + submission = Accession::Submission.new(accessionable) accessionable.validate! # See Accession::Sample.validate! in lib/accession/sample.rb submission.submit_accession(event_user) Rails.logger.info("Accessioning succeeded for sample '#{accessionable.sample.name}'") diff --git a/app/models/accession_service.rb b/app/models/accession_service.rb index 739df06ec0..09b4cf5e26 100644 --- a/app/models/accession_service.rb +++ b/app/models/accession_service.rb @@ -15,7 +15,6 @@ # # Accessionables # -------------- -# {Accessionable::Sample} Represents information about the sample, maps to a Sequencescape {Sample}. # {Accessionable::Study} Represents information about the study. Indicates WHY the samples have been sequenced. # Maps to a Sequencescape {Study}. # {Accessionable::Submission} Wrapper object required by the submission service. We use one per accessionable. @@ -23,7 +22,7 @@ # {Accessionable::Dac} Data access committee. Information about who to contact to gain access to the data. (EGA) # {Accessionable::Policy} Details about how the data may be used. (EGA) # -# Accessioning of samples has been partially migrated to {Accession 'a separate accession library'} +# Accessioning of samples has been fully migrated to {Accession 'a separate accession library'} module AccessionService # Define custom error classes for the AccessionService AccessionServiceError = Class.new(StandardError) @@ -50,29 +49,4 @@ def self.select_for_study(study) AccessionService::NoService.new(study) end end - - # Return the highest priority accession service that this sample should use. - # - # @param sample [Sample] The sample for which to select the accession service. - # @return [AccessionService::BaseService] An instance of the selected accession service - def self.select_for_sample(sample) - services = sample.studies.group_by { |study| AccessionService.select_for_study(study).priority } - return AccessionService::UnsuitableService.new([]) if services.empty? - - highest_priority = services.keys.max - suitable_study = services[highest_priority].detect { |study| send_samples_to_service?(study) } - return AccessionService.select_for_study(suitable_study) if suitable_study - - AccessionService::UnsuitableService.new(services[highest_priority]) - end - - # Determines if samples from the given study should be sent to the accession service. - # - # @param study [Study] The study to check. - # @return [Boolean] True if the samples can be sent without the study requiring accessioning, - # or if the study has already been accessioned, false otherwise. - def self.send_samples_to_service?(study) - accession_service = select_for_study(study) - accession_service.no_study_accession_needed || (!study.study_metadata.never_release? && study.accession_number?) - end end diff --git a/app/models/accession_service/base_service.rb b/app/models/accession_service/base_service.rb index d511bbf9b6..8a81f622a7 100644 --- a/app/models/accession_service/base_service.rb +++ b/app/models/accession_service/base_service.rb @@ -110,10 +110,6 @@ def submit(user, *accessionables) accessionables.map(&:accession_number) end - def submit_sample_for_user(sample, user) - submit(user, Accessionable::Sample.new(sample)) - end - def submit_study_for_user(study, user) unless study.accession_required? raise AccessionService::NumberNotRequired, @@ -139,10 +135,6 @@ def accession_study_xml(study) Accessionable::Study.new(study).xml end - def accession_sample_xml(sample) - Accessionable::Sample.new(sample).xml - end - def accession_policy_xml(study) policy = Accessionable::Policy.new(study) policy.xml diff --git a/app/models/accession_service/no_service.rb b/app/models/accession_service/no_service.rb index 1567b505b9..ae07f0d9e5 100644 --- a/app/models/accession_service/no_service.rb +++ b/app/models/accession_service/no_service.rb @@ -14,11 +14,6 @@ def submit(_user, *_accessionables) raise AccessionService::NumberNotRequired, I18n.t(:not_applicable_study, scope: 'accession_service.not_required') end - def submit_sample_for_user(_sample, _user) - raise AccessionService::NumberNotRequired, - I18n.t(:not_applicable_study_for_sample, scope: 'accession_service.not_required', study_id: @study_id) - end - def submit_study_for_user(_study, _user) raise AccessionService::NumberNotRequired, I18n.t(:not_applicable_study, scope: 'accession_service.not_required') end diff --git a/app/models/accession_service/unsuitable_service.rb b/app/models/accession_service/unsuitable_service.rb deleted file mode 100644 index 275bc10cea..0000000000 --- a/app/models/accession_service/unsuitable_service.rb +++ /dev/null @@ -1,37 +0,0 @@ -# frozen_string_literal: true -# Used for samples/studies which are neither open or managed. -class AccessionService::UnsuitableService < AccessionService::BaseService - self.no_study_accession_needed = true - - def initialize(studies) - @study_ids = studies.map(&:id) - end - - def provider - :unsuitable - end - - def submit(_user, *_accessionables) - raise AccessionService::NumberNotGenerated, - I18n.t(:no_suitable_study, scope: 'accession_service.unsuitable', study_ids: @study_ids.to_sentence) - end - - def submit_sample_for_user(_sample, _user) - raise AccessionService::NumberNotGenerated, - I18n.t(:no_suitable_study, scope: 'accession_service.unsuitable', study_ids: @study_ids.to_sentence) - end - - def submit_study_for_user(_study, _user) - raise StandardError, - # rubocop:todo Layout/LineLength - 'UnsuitableAccessionService should only be used for samples. This is a problem with Sequencescape and should be reported.' - # rubocop:enable Layout/LineLength - end - - def submit_dac_for_user(_study, _user) - raise StandardError, - # rubocop:todo Layout/LineLength - 'UnsuitableAccessionService should only be used for samples. This is a problem with Sequencescape and should be reported.' - # rubocop:enable Layout/LineLength - end -end diff --git a/app/models/accessionable/base.rb b/app/models/accessionable/base.rb index f2facb0877..52ca6750d3 100644 --- a/app/models/accessionable/base.rb +++ b/app/models/accessionable/base.rb @@ -2,7 +2,6 @@ # Base class to control generating XML for accessioning with the ENA or EGA # @see AccessionService class Accessionable::Base - InvalidData = Class.new(AccessionService::AccessionServiceError) attr_reader :accession_number, :name, :date, :date_short def initialize(accession_number) diff --git a/app/models/accessionable/sample.rb b/app/models/accessionable/sample.rb deleted file mode 100644 index b50ccfe1e9..0000000000 --- a/app/models/accessionable/sample.rb +++ /dev/null @@ -1,129 +0,0 @@ -# frozen_string_literal: true -# Handles the submission of {Sample} information to the ENA or EGA -# It should have a 1 to 1 mapping with Sequencescape {Sample samples}. -module Accessionable - class Sample < Base - ARRAY_EXPRESS_FIELDS = %w[ - genotype - phenotype - strain_or_line - developmental_stage - sex - cell_type - disease_state - compound - dose - immunoprecipitate - growth_condition - rnai - organism_part - species - time_point - age - treatment - ].freeze - - attr_reader :common_name, :taxon_id, :links, :tags - - # rubocop:todo Metrics/MethodLength, Metrics/AbcSize - def initialize(sample) # rubocop:todo Metrics/CyclomaticComplexity - @sample = sample - super(sample.ebi_accession_number) - - sampname = sample.sample_metadata.sample_public_name - @name = sampname.presence || sample.name - @name = @name.gsub(/[^a-z\d]/i, '_') if @name.present? - - @common_name = sample.sample_metadata.sample_common_name - @taxon_id = sample.sample_metadata.sample_taxon_id - - # Tags from the 'ENA attributes' property group - # NOTE[xxx]: This used to also look for 'ENA links' and push them to the 'data[:links]' value, but group was empty - @links = [] - @tags = - sample.tags.map { |datum| Tag.new(label_scope, datum.name, sample.sample_metadata[datum.tag], datum.downcase) } - - # TODO: maybe unify this with the previous loop - # Don't send managed AE data to SRA - accession_service = AccessionService.select_for_sample(sample) - unless accession_service.private? - ARRAY_EXPRESS_FIELDS.each do |datum| - value = sample.sample_metadata.send(datum) - next if value.blank? - - @tags << ArrayExpressTag.new(label_scope, datum, value) - end - end - - sample_hold = sample.sample_metadata.sample_sra_hold - @hold = sample_hold.presence || 'hold' - end - - # rubocop:enable Metrics/AbcSize, Metrics/MethodLength - - def accessionable_id - @sample.id - end - - def alias - @sample.uuid - end - - def title - @sample.sample_metadata.sample_public_name || @sample.sanger_sample_id - end - - def sample_element_attributes - # In case the accession number is defined, we won't send the alias - { alias: self.alias, accession: accession_number }.tap { |obj| obj.delete(:alias) if accession_number.present? } - end - - # rubocop:todo Metrics/MethodLength - def xml # rubocop:todo Metrics/AbcSize - xml = Builder::XmlMarkup.new - xml.instruct! - xml.SAMPLE_SET('xmlns:xsi' => 'http://www.w3.org/2001/XMLSchema-instance') do - xml.SAMPLE(sample_element_attributes) do - xml.TITLE title unless title.nil? - xml.SAMPLE_NAME do - xml.COMMON_NAME common_name - xml.TAXON_ID taxon_id - end - xml.SAMPLE_ATTRIBUTES { tags.each { |tag| xml.SAMPLE_ATTRIBUTE { tag.build(xml) } } } if tags.present? - - xml.SAMPLE_LINKS {} if links.present? - end - end - xml.target! - end - - # rubocop:enable Metrics/MethodLength - - def update_accession_number!(user, accession_number) - @accession_number = accession_number - @sample.sample_metadata.sample_ebi_accession_number = accession_number - @sample.sample_metadata.save! # prevent an infinite loop due to after_save callbacks on sample.save - @sample.events.assigned_accession_number!('sample', accession_number, user) - end - - def protect?(service) - service.sample_visibility(@sample) == AccessionService::PROTECT - end - - delegate :released?, to: :@sample - end - - class ArrayExpressTag < Base::Tag - def label - default_tag = "ArrayExpress-#{I18n.t("#{@scope}.#{@name}.label").tr(' ', '_').camelize}" - I18n.t("#{@scope}.#{@name}.ena_label", default: default_tag) - end - end - - class EgaTag < Base::Tag - def label - default_tag = "EGA-#{I18n.t("#{@scope}.#{@name}.label").tr(' ', '_').camelize}" - I18n.t("#{@scope}.#{@name}.ena_label", default: default_tag) - end - end -end diff --git a/app/models/accessionable/study.rb b/app/models/accessionable/study.rb index 2c2a30b9c4..8ae129f812 100644 --- a/app/models/accessionable/study.rb +++ b/app/models/accessionable/study.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true # Handles submission of {Study} information to the EGA or ENA -# A study gathers together multiple {Accessionable::Sample samples} and essentially +# A study gathers together multiple samples and essentially # describes why they are being sequenced. It should have a 1 to 1 mapping with Sequencescape # {Study studies}. # A study can either be open (ENA) or managed (EGA) which determines which {AccessionService} it diff --git a/app/models/sample.rb b/app/models/sample.rb index b4cfbf7937..0ee5d275c9 100644 --- a/app/models/sample.rb +++ b/app/models/sample.rb @@ -214,13 +214,6 @@ class Current < ActiveSupport::CurrentAttributes validates_associated :sample_metadata, on: %i[accession EGA ENA] - # TODO: should be removed along with the removal of the accessioning feature flag - # y25_286_accession_individual_samples_with_sample_accessioning_job - def tags - accession_service = AccessionService.select_for_sample(self) - self.class.tags.select { |tag| tag.for?(accession_service.provider) } - end - def self.tags @tags ||= [] end @@ -553,7 +546,8 @@ def should_be_accessioned? false end - # NOTE: this does not check whether the current user is permitted to accession the sample + # NOTE: this does not check whether the current user is permitted to accession the sample, + # nor if accessioning is enabled, as these belong in a controller or library, rather than the model. def accession_and_handle_validation_errors event_user = current_user # the event_user for this sample must be set from the calling controller Accession.accession_sample(self, event_user, perform_now: true) diff --git a/app/models/sample_manifest/core_behaviour.rb b/app/models/sample_manifest/core_behaviour.rb index 7513cee410..deb8950cc5 100644 --- a/app/models/sample_manifest/core_behaviour.rb +++ b/app/models/sample_manifest/core_behaviour.rb @@ -74,7 +74,7 @@ def generate_sample_and_aliquot(sanger_sample_id, receptacle) end def stocks? - false + true end end @@ -96,6 +96,7 @@ def core_behaviour # rubocop:todo Metrics/MethodLength def behaviour_module # rubocop:todo Metrics/CyclomaticComplexity + # asset_type comes from the query params for the new/create manifest actions page. case asset_type when '1dtube' 'SampleTubeBehaviour' diff --git a/app/models/study.rb b/app/models/study.rb index 84bf3ac864..ca504dac2b 100644 --- a/app/models/study.rb +++ b/app/models/study.rb @@ -583,31 +583,6 @@ def samples_accessionable? ].all? end - # Accession all samples in the study. - # - # If the study does not have an accession number, adds an error to the study and returns. - # Otherwise, iterates through each sample in the study and attempts to accession it, - # unless the sample already has an accession number. - # If an Accession::Error occurs for a sample, adds the error message to the study's errors. - # - # NOTE: this does not check if the current user has permission to accession samples in this study - # - # @return [void] - def accession_all_samples(event_user) - return errors.add(:base, 'Please accession the study before accessioning samples') unless accession_number? - - unless samples_accessionable? - return errors.add(:base, - 'Study cannot accession samples, see Study Accessioning tab for details') - end - - samples.find_each do |sample| - Accession.accession_sample(sample, event_user) unless sample.accession_number? - rescue Accession::Error => e - errors.add(:base, e.message) - end - end - def abbreviation abbreviation = study_metadata.study_name_abbreviation abbreviation.presence || "#{id}STDY" diff --git a/app/sample_manifest_excel/sample_manifest_excel/upload/base.rb b/app/sample_manifest_excel/sample_manifest_excel/upload/base.rb index f3e93dcf54..6684625d3d 100644 --- a/app/sample_manifest_excel/sample_manifest_excel/upload/base.rb +++ b/app/sample_manifest_excel/sample_manifest_excel/upload/base.rb @@ -118,7 +118,7 @@ def trigger_accessioning(event_user) end end - # If samples have been created, and it's not a library plate/tube, register a stock_resource record in the MLWH + # If samples have been created, register a stock_resource record in the MLWH def register_stock_resources stock_receptacles_to_be_registered.each(&:register_stock!) end diff --git a/app/views/sdb/sample_manifests/_samples.html.erb b/app/views/sdb/sample_manifests/_samples.html.erb index 535e9d9b11..f37d927413 100644 --- a/app/views/sdb/sample_manifests/_samples.html.erb +++ b/app/views/sdb/sample_manifests/_samples.html.erb @@ -5,7 +5,7 @@ <%= tag.div(page_summary(samples), class:'col-md-4') %> <%= tag.div(pagination(samples), class: 'col-md-4 d-flex justify-content-center mb--3') %> <%= tag.div(class: 'col-md-4 d-flex justify-content-end') do %> - <%= @sample_manifest.samples.select { |sample| !sample.accession_number? }.count %> samples not accessioned + <%= samples_not_accessioned(@sample_manifest.samples) %> <% end %> <% end %> @@ -38,7 +38,7 @@ <%= accession_status&.status&.capitalize %> <%= accession_status&.updated_at&.in_time_zone&.strftime('%Y-%m-%d %H:%M:%S %Z') %> - <% if can?(:accession, sample) && configatron.accession_samples %> + <% if [accessioning_enabled?, sample.should_be_accessioned?, permitted_to_accession?(sample)].all? %> <%# Add link to perform sample accessioning %> <% if sample.current_accession_status %> <%# If status exists, offer retry %> diff --git a/app/views/studies/information/_accession_statuses.html.erb b/app/views/studies/information/_accession_statuses.html.erb index 33852b716e..3312f19b0b 100644 --- a/app/views/studies/information/_accession_statuses.html.erb +++ b/app/views/studies/information/_accession_statuses.html.erb @@ -7,7 +7,7 @@ <%# Aggregation summary for the top right %> <% content_for :aggregation do %> - <%= @study.samples.select { |sample| !sample.accession_number? }.count %> samples not accessioned + <%= samples_not_accessioned(@study.samples) %> <% end %> <%# Table contents %> @@ -36,7 +36,7 @@ <%= accession_status&.status&.capitalize %> <%= accession_status&.updated_at&.in_time_zone&.strftime('%Y-%m-%d %H:%M:%S %Z') %> - <% if permitted_to_accession?(sample) & sample.should_be_accessioned? & accessioning_enabled? %> + <% if [accessioning_enabled?, sample.should_be_accessioned?, permitted_to_accession?(sample)].all? %> <%# Add link to perform sample accessioning %> <% if sample.current_accession_status %> <%# If status exists, offer retry %> diff --git a/app/views/studies/information/_study_accession_checklist.html.erb b/app/views/studies/information/_study_accession_checklist.html.erb index c5ece5c113..3425e42975 100644 --- a/app/views/studies/information/_study_accession_checklist.html.erb +++ b/app/views/studies/information/_study_accession_checklist.html.erb @@ -25,7 +25,7 @@ <%= checklist_item( condition: !@study.study_metadata.strategy_not_applicable?, - good: "Study release strategy is #{Study::DATA_RELEASE_STRATEGY_OPEN.humanize} or #{Study::DATA_RELEASE_STRATEGY_MANAGED.humanize} #{tag.strong { "(#{@study.study_metadata.data_release_strategy.humanize})" }}".html_safe, + good: "Study release strategy is #{Study::DATA_RELEASE_STRATEGY_OPEN.humanize} or #{Study::DATA_RELEASE_STRATEGY_MANAGED.humanize} #{tag.strong { "(#{@study.study_metadata.data_release_strategy&.humanize})" }}".html_safe, bad: "Study release strategy is #{Study::DATA_RELEASE_STRATEGY_NOT_APPLICABLE.humanize}".html_safe, action: link_to('Change release strategy', edit_study_path(@study)), action_permission: can?(:edit, @study), @@ -34,7 +34,7 @@ <%= checklist_item( condition: !@study.study_metadata.never_release?, - good: "Study release timing is #{@study.study_metadata.data_release_timing.humanize}".html_safe, + good: "Study release timing is #{@study.study_metadata.data_release_timing&.humanize}".html_safe, bad: "Study release timing is #{Study::DATA_RELEASE_TIMING_NEVER.humanize}".html_safe, action: link_to('Change release settings', edit_study_path(@study)), action_permission: can?(:edit, @study), diff --git a/config/config.rb b/config/config.rb index 51aa572909..a5316638c3 100644 --- a/config/config.rb +++ b/config/config.rb @@ -100,7 +100,6 @@ configatron.data_sharing_contact.name = 'Datasharing' configatron.data_sharing_contact.email = 'datasharing@example.com' - configatron.accession_local_key = 'abc' configatron.sequencescape_email = 'sequencescape@example.com' configatron.default_email_domain = 'example.com' configatron.run_information_url = 'http://example.com/' @@ -154,7 +153,6 @@ configatron.data_sharing_contact.name = 'Datasharing' configatron.data_sharing_contact.email = 'datasharing@example.com' - configatron.accession_local_key = 'abc' configatron.sequencescape_email = 'sequencescape@example.com' configatron.default_email_domain = 'example.com' configatron.run_information_url = 'http://example.com/' diff --git a/config/feature_flags.yml b/config/feature_flags.yml index 4de8356a36..0132f40714 100644 --- a/config/feature_flags.yml +++ b/config/feature_flags.yml @@ -12,6 +12,5 @@ y25_442_make_api_key_mandatory: Makes API key mandatory for all API requests # Accessioning y25_706_enable_accessioning: Enables the accessioning feature in the application y25_714_skip_accessioning_tag_validation: Skips internal validation of accessioning tags prior to sample accessioning -y25_286_accession_individual_samples_with_sample_accessioning_job: Accession individual samples with SampleAccessioningJob y25_705_notify_on_internal_accessioning_validation_failures: Sends email to developers when internal validation fails during accessioning y25_705_notify_on_external_accessioning_validation_failures: Sends email to developers when external validation fails during accessioning diff --git a/docs/accessioning.mermaid b/docs/accessioning.mermaid new file mode 100644 index 0000000000..e00e2cfd85 --- /dev/null +++ b/docs/accessioning.mermaid @@ -0,0 +1,280 @@ +--- +title: Sequencescape Accessioning Processes +--- +%%{ init: { + 'flowchart': { 'curve': 'curvy' }, + 'theme': 'neutral', + 'themeVariables': { 'fontSize': '18px' } + } +}%% +flowchart LR + %% Legend + subgraph Legend [Legend] + direction TB + L_Application(fa:fa-circle-arrow-right Application Process) + L_External(fa:fa-arrow-up External API) + L_Action(fa:fa-computer-mouse User Action) + L_Model(fa:fa-square-caret-down Model) + L_Controller(fa:fa-arrows-spin Controller) + L_View(fa:fa-eye View) + L_Function(fa:fa-caret-right Function) + L_Validation_Function(fa:fa-caret-right Validation Function) + L_Config(fa:fa-screwdriver-wrench Configuration) + L_Resource(fa:fa-file Resource) + L_Library(fa:fa-book Library) + L_Async(fa:fa-clock Asynchronous Process) + L_LinkSource[/Link Source\] + L_LinkSink[\Link Sink/] + + L_Application ~~~ L_External ~~~ L_Action ~~~ L_Resource ~~~ L_Library ~~~ L_Async ~~~ L_LinkSource + L_Config ~~~ L_Controller ~~~ L_Model ~~~ L_View ~~~ L_Validation_Function ~~~ L_Function ~~~ L_LinkSink + + InvisibleNodeA[ ] -..-> | Data flow | InvisibleNodeB[ ] -- Function calls --> InvisibleNodeC[ ] == Accessioning function calls ==> InvisibleNodeD[ ] + end + + %% End Legend + + %% Nodes + + %% External Systems + External_EBI_Studies(fa:fa-arrow-up ENA/EGA Studies) + External_EBI_Samples(fa:fa-arrow-up ENA/EGA Samples) + + %% User Interfaces + UI_Sample_GAN(fa:fa-computer-mouse Generate Accession Number) + UI_Sample_SAA(fa:fa-computer-mouse Save and Accession) + UI_Study_GAN(fa:fa-computer-mouse Generate Accession Number) + UI_Study_AAS(fa:fa-computer-mouse Accession all Samples) + UI_Manifest_Upload(fa:fa-computer-mouse Manifest Upload) + + %% Models + MD_SampleManifest_Uploader(fa:fa-square-caret-down SampleManifest::Uploader) + MD_AccessionStatuses(fa:fa-square-caret-down AccessionStatuses) + + %% Views + VW_Sample(fa:fa-eye Sample View) + VW_Study(fa:fa-eye Study View) + + %% Functions + FN_Accession_accession_sample(fa:fa-caret-right accession_sample) + FN_Accession_Sample_update_accession_number(fa:fa-caret-right update_accession_number) + FN_Accession_Sample_validate(fa:fa-caret-right validate) + FN_Accession_SampleAccessioning_perform(fa:fa-caret-right Accession::SampleAccessioning
#perform) + FN_Accessionable_Study_update_accession_number(fa:fa-caret-right update_accession_number!) + FN_AccessionHelper_accessioning_enabled(fa:fa-caret-right AccessionHelper
#accessioning_enabled?) + FN_AccessionService_post_files(fa:fa-caret-right post_files) + FN_AccessionService_select_for_study(fa:fa-caret-right select_for_study) + FN_AccessionService_submit_study_for_user(fa:fa-caret-right submit_study_for_user) + FN_AccessionService_submit(fa:fa-caret-right submit) + FN_Sample_accession_and_handle_validation_errors(fa:fa-caret-right accession_and_handle_validation_errors) + FN_SampleManifestExcel_Upload_Base_trigger_accessioning(fa:fa-caret-right SampleManifestExcel::Upload::Base
#trigger_accessioning) + FN_Samples_accession(fa:fa-caret-right accession) + FN_Studies_accession_all_samples(fa:fa-caret-right accession_all_samples) + FN_Studies_accession(fa:fa-caret-right accession) + FN_Study_validate_study_for_accessioning(fa:fa-caret-right validate_study_for_accessioning!) + + %% Links + LK_accessioning_enabled[/accessioning_enabled?\] + LK_accessioning_enabled_AAS[\accessioning_enabled?/] + LK_accessioning_enabled_Studies_accession[\accessioning_enabled?/] + LK_accessioning_enabled_Samples_accession[\accessioning_enabled?/] + LK_accessioning_enabled_trigger_accessioning[\accessioning_enabled?/] + LK_accessioning_enabled_Accession_perform[\accessioning_enabled?/] + LK_accessioning_enabled_Study_View[\accessioning_enabled?/] + LK_accessioning_enabled_Sample_View[\accessioning_enabled?/] + + %% Libraries + LB_AccessioningV1Client(fa:fa-book AccessioningV1Client) + LB_RestClient(fa:fa-book RestClient) + + %% Other Components + CP_AccessionSubmission(fa:fa-book Accession::Submission) + DJ_SampleAccessioningJob(fa:fa-clock SampleAccessioningJob) + + %% Config + CF_y25_706_enable_accessioning(fa:fa-screwdriver-wrench y25_706_enable_accessioning) + + %% Resources + RES_Manifest(fa:fa-file Manifest) + + %% Groupings of nodes + subgraph App_SS_Configuration[fa:fa-screwdriver-wrench Sequencescape Configuration] + direction LR + CF_y25_706_enable_accessioning + FN_AccessionHelper_accessioning_enabled + LK_accessioning_enabled + end + + subgraph App_SS_Pages[fa:fa-eye Sequencescape Pages] + subgraph Study_Page[Study Page] + UI_Study_GAN + UI_Study_AAS + end + subgraph Samples + UI_Sample_SAA + UI_Sample_GAN + end + subgraph Sample Manifests + RES_Manifest + UI_Manifest_Upload + end + end + + subgraph App_SS_Study_Accessioning[fa:fa-circle-arrow-right Study Accessioning] + LB_RestClient + + subgraph CT_Studies[fa:fa-arrows-spin Studies Controller] + LK_accessioning_enabled_AAS + LK_accessioning_enabled_Studies_accession + FN_Studies_accession_all_samples + FN_Studies_accession + end + subgraph MD_Study[fa:fa-square-caret-down Study Model] + FN_Study_validate_study_for_accessioning + end + subgraph MD_AccessionService[fa:fa-square-caret-down AccessionService Model - studies only] + FN_AccessionService_select_for_study + FN_AccessionService_submit_study_for_user + FN_AccessionService_submit + FN_AccessionService_post_files + end + subgraph MD_Accessionable_Study[fa:fa-square-caret-down Accessionable::Study Model] + FN_Accessionable_Study_update_accession_number + end + end + + subgraph App_SS_Sample_Accessioning[fa:fa-circle-arrow-right Sample Accessioning] + subgraph CT_Samples[fa:fa-arrows-spin Samples Controller] + LK_accessioning_enabled_Samples_accession + FN_Samples_accession + end + end + + subgraph App_SS_Sample_Save_and_Accession[fa:fa-circle-arrow-right Sample Save and Accession] + subgraph MD_Sample[fa:fa-square-caret-down Sample Model] + FN_Sample_accession_and_handle_validation_errors + end + end + + subgraph App_SS_Sample_Manifest_Accessioning[fa:fa-circle-arrow-right Sample Manifest Accessioning] + MD_SampleManifest_Uploader + LK_accessioning_enabled_trigger_accessioning + FN_SampleManifestExcel_Upload_Base_trigger_accessioning + end + subgraph App_SS_Accession_Sample[fa:fa-circle-arrow-right Accession Sample] + DJ_SampleAccessioningJob + LB_AccessioningV1Client + MD_AccessionStatuses + + LK_accessioning_enabled_Accession_perform + FN_Accession_accession_sample + FN_Accession_SampleAccessioning_perform + CP_AccessionSubmission + subgraph MD_Accession_Sample[fa:fa-square-caret-down Accession::Sample Model] + FN_Accession_Sample_validate + FN_Accession_Sample_update_accession_number + end + end + + subgraph App_SS_Views[fa:fa-circle-arrow-right Sequencescape Views] + LK_accessioning_enabled_Sample_View + VW_Sample + LK_accessioning_enabled_Study_View + VW_Study + end + %% subgraph External_EBI[fa:fa-globe EBI ENA Webin REST V1] + External_EBI_Studies + External_EBI_Samples + %% end + + + %% Edge connections between nodes + + App_SS_Pages ~~~~ App_SS_Configuration + App_SS_Accession_Sample ~~~~ App_SS_Views + + %% Config + CF_y25_706_enable_accessioning --> FN_AccessionHelper_accessioning_enabled --> LK_accessioning_enabled + + %% Manifest upload + RES_Manifest -.-> UI_Manifest_Upload + UI_Manifest_Upload --> MD_SampleManifest_Uploader + LK_accessioning_enabled_trigger_accessioning -.-> FN_SampleManifestExcel_Upload_Base_trigger_accessioning + MD_SampleManifest_Uploader --> FN_SampleManifestExcel_Upload_Base_trigger_accessioning --> FN_Accession_accession_sample + + %% Sample save and accession + UI_Sample_SAA ---> FN_Sample_accession_and_handle_validation_errors + FN_Sample_accession_and_handle_validation_errors --> FN_Accession_accession_sample + + %% Sample generate accession number + LK_accessioning_enabled_Samples_accession -.-> FN_Samples_accession + UI_Sample_GAN --> FN_Samples_accession + FN_Samples_accession <--> | 1. | FN_Accession_accession_sample + LK_accessioning_enabled_Accession_perform -.-> FN_Accession_SampleAccessioning_perform + FN_Accession_accession_sample ==> FN_Accession_SampleAccessioning_perform + FN_Accession_SampleAccessioning_perform ==> DJ_SampleAccessioningJob + DJ_SampleAccessioningJob <--> | 1. | FN_Accession_Sample_validate + DJ_SampleAccessioningJob ==> | 2. | CP_AccessionSubmission + CP_AccessionSubmission ==> LB_AccessioningV1Client + LB_AccessioningV1Client ==> External_EBI_Samples + External_EBI_Samples ==> | Failure: reason | MD_AccessionStatuses + External_EBI_Samples ==> | Success: accession number | FN_Accession_Sample_update_accession_number + LK_accessioning_enabled_Sample_View -.-> VW_Sample + MD_AccessionStatuses -.-> VW_Sample + FN_Samples_accession --> | 2. | VW_Sample + + %% Study accession all samples + LK_accessioning_enabled_AAS -.-> FN_Studies_accession_all_samples + UI_Study_AAS --> FN_Studies_accession_all_samples + FN_Studies_accession_all_samples <--> | 1. | FN_Accession_accession_sample + FN_Studies_accession_all_samples --> | 2. | VW_Study + + %% Study generate accession number + UI_Study_GAN --> FN_Studies_accession + LK_accessioning_enabled_Studies_accession -.-> FN_Studies_accession + FN_Studies_accession <--> | 1. | FN_Study_validate_study_for_accessioning + FN_Studies_accession <--> | 2. | FN_AccessionService_select_for_study + FN_Studies_accession <--> | 3. | FN_AccessionService_submit_study_for_user + FN_Studies_accession --> | 4. with flash message | VW_Study + FN_AccessionService_submit_study_for_user ==> FN_AccessionService_submit + FN_AccessionService_submit ==> FN_AccessionService_post_files + FN_AccessionService_post_files ==> LB_RestClient + LB_RestClient ==> External_EBI_Studies + External_EBI_Studies ==> FN_Accessionable_Study_update_accession_number + LK_accessioning_enabled_Study_View -.-> VW_Study + + %% Subgraph styling + + %% lemon-chiffon: #fbf8cc + %% champagne-pink: #fde4cf + %% tea-rose-red: #ffcfd2 + %% pink-lavender: #f1c0e8 + %% mauve: #cfbaf0 + %% jordy-blue: #a3c4f3 + %% non-photo-blue: #90dbf4 + %% electric-blue: #8eecf5 + %% aquamarine: #98f5e1 + %% celadon: #b9fbc0 + + classDef default fill:#fafafa,stroke:#333,stroke-width:1px; + + classDef invisible fill:transparent,stroke:transparent; + classDef legendTransparent fill:transparent,stroke:#333,stroke-width:1px; + classDef application fill:#90dbf4; + classDef configuration fill:#fbf8cc; + classDef link fill:#fde4cf; + classDef railsModel fill:#98f5e1; + classDef railsController fill:#b9fbc0; + classDef validation fill:#f1c0e8; + classDef userInterface fill:#a3c4f3; + + class InvisibleNodeA,InvisibleNodeB,InvisibleNodeC,InvisibleNodeD invisible; + class Legend legendTransparent; + class L_Application,App_SS_Accession_Sample,App_SS_Configuration,App_SS_Pages,App_SS_Views,App_SS_Study_Accessioning,App_SS_Sample_Save_and_Accession,App_SS_Sample_Manifest_Accessioning,App_SS_Sample_Accessioning application; + class L_Config,CF_disable_accession_check,CF_y25_706_enable_accessioning configuration; + class DelayedJob,Samples,Studies,Submissions SequencescapeSection; + class L_Model,MD_Study,MD_AccessionService,MD_Sample,MD_SampleManifest_Uploader,MD_Order,MD_Submission_AccessionBehaviour,MD_AccessionStatuses,MD_Accession_Sample,MD_AccessionService,MD_Accessionable_Study railsModel; + class L_Controller,CT_Samples,CT_Studies railsController; + class L_Action,L_View,UI_LB_Charge_and_Pass,UI_Sample_GAN,UI_Sample_SAA,UI_Study_GAN,UI_Study_AAS,UI_Manifest_Upload,VW_Sample,VW_Study userInterface; + class L_LinkSource,L_LinkSink,LK_accessioning_enabled,LK_accessioning_enabled_AAS,LK_accessioning_enabled_Studies_accession,LK_accessioning_enabled_Samples_accession,LK_accessioning_enabled_trigger_accessioning,LK_accessioning_enabled_Accession_perform,LK_accessioning_enabled_Study_View,LK_accessioning_enabled_Sample_View link; + class L_Validation_Function,FN_Accession_Sample_validate,FN_Study_validate_study_for_accessioning validation; diff --git a/features/data_release/12400603_update_accession.feature b/features/data_release/12400603_update_accession.feature deleted file mode 100644 index 830b48990f..0000000000 --- a/features/data_release/12400603_update_accession.feature +++ /dev/null @@ -1,76 +0,0 @@ -@accession_number @accession-service -Feature: object with an accession should be modifiable - Background: - Given I am an "administrator" user logged in as "me" - - @study - Scenario: A study with already an accession number should add updated in the history - Given a study named "study" exists for accession - And the study "study" has the accession number "E-ERA-16" - Given an accessioning webservice exists which returns a study accession number "E-ERA-16" - When I update an accession number for study "study" - - When I am on the event history page for study "study" - Then I should see "Assigned study accession number" - - Scenario: A sample XML should validate with the ENA schema - Given a sample named "sample" exists for accession - Given an accessioning webservice exists which returns a sample accession number "E-ERA-16" - Given I am on the show page for sample "sample" - And the UUID for the sample "sample" is "11111111-1111-1111-1111-1111111111" - When I create an accession number for sample "sample" - Then the XML sent for sample "sample" validates with the schema "test/data/xsd/SRA.sample.xsd" - - Scenario: A sample without an accession number should not send public name as alias but a uuid - Given a sample named "sample" exists for accession - Given an accessioning webservice exists which returns a sample accession number "E-ERA-16" - Given I am on the show page for sample "sample" - And the UUID for the sample "sample" is "11111111-1111-1111-1111-1111111111" - When I create an accession number for sample "sample" - - Then the XML root attribute "alias" sent to the accession service for sample "sample" should be "11111111-1111-1111-1111-1111111111" - And the XML sent for sample "sample" validates with the schema "test/data/xsd/SRA.sample.xsd" - - Scenario Outline: A sample should send a title element with sample public name falling back to sanger id if not present - Given a sample named "sample" exists for accession - Given an accessioning webservice exists which returns a sample accession number "E-ERA-16" - Given I am on the show page for sample "sample" - And the metadata attribute "sample_public_name" of the sample "sample" is - And the attribute "sanger_sample_id" of the sample "sample" is - When I create an accession number for sample "sample" - - Then the XML tag "TITLE" sent to the accession service for sample "sample" should be - And the XML sent for sample "sample" validates with the schema "test/data/xsd/SRA.sample.xsd" - - Examples: - | sample_public_name | sanger_sample_id | title | - | "this is a public name of sample" | "this is a sanger_sample_id of sample" | "this is a public name of sample" | - | "this is a public name of sample" | "empty" | "this is a public name of sample" | - | "empty" | "this is a sanger_sample_id of sample" | "this is a sanger_sample_id of sample" | - | "empty" | "empty" | not present | - - Scenario: A sample with already an accession number should add updated in the history - Given a sample named "sample" exists for accession - And the sample "sample" has the accession number "E-ERA-16" - Given an accessioning webservice exists which returns a sample accession number "E-ERA-16" - When I update an accession number for sample "sample" - - When I am on the event history page for sample "sample" - Then I should see "Assigned sample accession number" - And I should see "me" - - Scenario: A sample with already an accession number should update itself using its accession number - Given a sample named "sample" exists for accession - And the sample "sample" has the accession number "E-ERA-16" - Given an accessioning webservice exists which returns a sample accession number "E-ERA-16" - When I update an accession number for sample "sample" - - Then I should not have sent the attribute "alias" for the sample element to the accessioning service - And I should have sent the attribute "accession" for the sample element to the accessioning service - - Scenario: A sample without an accession number should update itself using its alias - Given a sample named "sample" exists for accession - Given an accessioning webservice exists which returns a sample accession number "E-ERA-16" - When I create an accession number for sample "sample" - - Then I should have sent the attribute "alias" for the sample element to the accessioning service diff --git a/features/data_release/4491710_ega_integration.feature b/features/data_release/4491710_ega_integration.feature deleted file mode 100644 index 56251d5313..0000000000 --- a/features/data_release/4491710_ega_integration.feature +++ /dev/null @@ -1,114 +0,0 @@ -@sample @accession_number @accession-service -Feature: Generate accession nubmers for a sample - Background: - Given I am logged in as "user123" - - Scenario: I am not the owner of a sample - Given a study named "Study 4491710" exists - Given study "Study 4491710" has a registered sample "Sample4491710" - And an accession number is required for study "Study 4491710" - Given I am on the show page for sample "Sample4491710" - Then I should not see "Generate Accession Number" - - Scenario: The sample has no study - Given a sample named "Sample4491710" exists - Given I am the owner of sample "Sample4491710" - Given I am on the show page for sample "Sample4491710" - Then I should not see "Generate Accession Number" - - Scenario: The sample doesn't have the required properties filled in - Given a study named "Study 4491710" exists - And the study "Study 4491710" is a "Whole Genome Sequencing" study - And the title of study "Study 4491710" is "Checking sample validation" - And the description of study "Study 4491710" is "The study is valid, the sample is not" - And the abstract of study "Study 4491710" is "Good study, bad sample" - - Given the study "Study 4491710" is a "genomic sequencing" study for data release - And the study "Study 4491710" has an open data release strategy - And the study "Study 4491710" data release timing is standard - - Given study "Study 4491710" has a registered sample "Sample4491710" - And an accession number is required for study "Study 4491710" - - Given I am the owner of sample "Sample4491710" - - Given I am on the show page for sample "Sample4491710" - Given an accessioning webservice exists which returns a sample accession number "EGAN00001000234" - Then I should not see "Generate Accession Number" - - Scenario: Study doesn't have any of the required properties filled in - Given a study named "Study 4491710" exists - And the abstract of study "Study 4491710" is "" - Given study "Study 4491710" has a registered sample "Sample4491710" - And an accession number is required for study "Study 4491710" - - Given I am the owner of sample "Sample4491710" - And the sample "Sample4491710" has the Taxon ID "99999" - And the sample "Sample4491710" has the common name "Human" - - Given I am on the show page for sample "Sample4491710" - Given an accessioning webservice exists which returns a sample accession number "EGAN00001000234" - Then I should not see "Generate Accession Number" - - Scenario Outline: Study doesn't have some of the required data release properties filled in - Given a study named "Study 4491710" exists - And the study "Study 4491710" is a "<type>" study - And the title of study "Study 4491710" is "<title>" - And the description of study "Study 4491710" is "Description of study" - And the abstract of study "Study 4491710" is "<study_abstract>" - - Given the study "Study 4491710" is a "genomic sequencing" study for data release - And the study "Study 4491710" has a <Strategy> data release strategy - And the study "Study 4491710" data release timing is standard - - Given study "Study 4491710" has a registered sample "Sample4491710" - And an accession number is required for study "Study 4491710" - - Given I am the owner of sample "Sample4491710" - And the sample "Sample4491710" has the Taxon ID "99999" - And the sample "Sample4491710" has the common name "Human" - - Given I am on the show page for sample "Sample4491710" - Given an accessioning webservice exists which returns a sample accession number "EGAN00001000234" - Then I should not see "Generate Accession Number" - # NOTE: strategy, timing and description cannot be empty by definition - Examples: - | Strategy | title | type | study_abstract | - | open | My title | Not specified | abstract | - | managed | My title | Not specified | abstract | - - Scenario Outline: Sample is released to EBI - Given a study named "Study 4491710" exists - And the study "Study 4491710" is a "Whole Genome Sequencing" study - And the title of study "Study 4491710" is "My title" - And the description of study "Study 4491710" is "Description of study" - And the abstract of study "Study 4491710" is "My abstract" - And the faculty sponsor for study "Study 4491710" is "John Doe" - - Given the study "Study 4491710" is a "genomic sequencing" study for data release - And the study "Study 4491710" has a <data_release_strategy> data release strategy - And the study "Study 4491710" data release timing is standard - And the study "Study 4491710" has samples contaminated with human DNA - And the study "Study 4491710" contains human DNA - And the study "Study 4491710" contains samples commercially available - And study "Study 4491710" has an accession number - - Given study "Study 4491710" has a registered sample "Sample4491710" - And an accession number is required for study "Study 4491710" - - Given I am the owner of sample "Sample4491710" - And the sample "Sample4491710" has the Taxon ID "99999" - And the sample "Sample4491710" has the common name "Human" - And the sample "Sample4491710" has the phenotype "Healthy" - And the sample "Sample4491710" has the gender "Female" - And the sample "Sample4491710" has the donor id "D0N0R" - - Given I am on the show page for sample "Sample4491710" - Given an accessioning webservice exists which returns a sample accession number "<accession_number>" - When I follow "Generate Accession Number" - Then I should see "Accession number generated: <accession_number>" - - Examples: - | data_release_strategy | accession_number | - | open | EGAN00001000234 | - | managed | EGAN00001000234 | diff --git a/features/data_release/8487329_array_express_accession_number.feature b/features/data_release/8487329_array_express_accession_number.feature deleted file mode 100644 index f502e35463..0000000000 --- a/features/data_release/8487329_array_express_accession_number.feature +++ /dev/null @@ -1,13 +0,0 @@ -@study @accession_number @array_express @accession-service -Feature: Array express accession number should be parsed and saved - Background: - Given I am an "administrator" user logged in as "me" - Scenario: The array express accession number is saved to the study - Given a study named "study" exists for array express - Given an accessioning service exists which returns an array express accession number "E-ERA-16" - When I generate an array express accession number for study "study" - And I am on the details page for study "study" - Then I should see "E-ERA-16" - - - diff --git a/features/data_release/8487329_ega_dac_integration.feature b/features/data_release/8487329_ega_dac_integration.feature deleted file mode 100644 index 553c1cf3ce..0000000000 --- a/features/data_release/8487329_ega_dac_integration.feature +++ /dev/null @@ -1,40 +0,0 @@ -@study @accession_number @dac @policy @accession-service -Feature: Dac and Policy should be able to generate accession numbers - Background: - Given I am an "administrator" user logged in as "me" - - Scenario Outline: A managed study has a valid <object> set but no accession number for it - Given a study named "managed study" exists - Given the study "managed study" has a managed data release strategy - Given the study "managed study" has a valid <object> - Given an accessioning webservice exists which returns a <object> accession number "EGAP00001000234" - When I generate a <object> accession number for study "managed study" - And I am on the information page for study "managed study" - And I follow "Study details" - Then I should see "EGAP00001000234" - Examples: - | object | - | policy | - | dac | - - Scenario Outline: A open study has a valid <object> set but no accession number for it. Should fail. - Given a study named "open study" exists - Given the study "open study" has a open data release strategy - Given the study "open study" has a valid <object> - Given an accessioning webservice exists which returns a <object> accession number "EGAP00001000234" - When I generate a <object> accession number for study "open study" - Then I should see "No accession number was generated" - Examples: - | object | - | policy | - | dac | - - Scenario: A managed study has an invalid DAC - Given a study named "managed study" exists - Given the study "managed study" has a managed data release strategy - Given an accessioning webservice exists which returns a dac accession number "EGAP00001000234" - When I generate a dac accession number for study "managed study" - Then I should see "Data Access Contacts Empty" - And I am on the information page for study "managed study" - And I follow "Study details" - Then I should not see "EGAP00001000234" diff --git a/features/studies/accession_number.feature b/features/studies/accession_number.feature deleted file mode 100644 index bca7d398da..0000000000 --- a/features/studies/accession_number.feature +++ /dev/null @@ -1,65 +0,0 @@ -# rake features FEATURE=features/plain/studies/accession_number.feature -@study @accession_number @accession-service -Feature: Studies should be able to generate accession numbers - Background: - Given I am an "administrator" user logged in as "John Smith" - - Given a study named "Study for accession number testing" exists - And the title of study "Study for accession number testing" is "Testing accession numbers" - And the description of study "Study for accession number testing" is "To find out if something is broken" - And the abstract of study "Study for accession number testing" is "Ok, not ok?" - And the study "Study for accession number testing" is a "Whole Genome Sequencing" study - - Given I am on the information page for study "Study for accession number testing" - - Scenario: The study does not have an accession number but doesn't need one anyway - When I follow "Generate Accession Number" - Then I should be on the information page for study "Study for accession number testing" - And I should see "An accession number is not required for this study" - - Scenario: The study already has an accession number - Given an accessioning webservice exists which returns a study accession number "EGAN00001000234" - Given an accession number is required for study "Study for accession number testing" - And the study "Study for accession number testing" has the accession number "EGAN00001000235" - When I follow "Generate Accession Number" - Then I should see "Accession number generated: EGAN00001000234" - - Scenario Outline: The study has data missing from the required fields - Given an accession number is required for study "Study for accession number testing" - Given the <attribute> of study "Study for accession number testing" is "" - - When I follow "Generate Accession Number" - And I should see "Please fill in the required fields" - - Examples: - | attribute | - | title | - | abstract | - - Scenario: The study gets a valid accession number - Given an accessioning webservice exists which returns a study accession number "EGAN00001000234" - - Given an accession number is required for study "Study for accession number testing" - - When I follow "Generate Accession Number" - # Then I should be on the information page for study "Study for accession number testing" - And I should see "Accession number generated: EGAN00001000234" - Given I am on the information page for study "Study for accession number testing" - When I follow "Study details" - Then I should see "EGAN00001000234" - - Scenario: The accession number service gives an error - Given an accessioning webservice exists that errors with "We are experiencing problems, sorry" - - Given an accession number is required for study "Study for accession number testing" - - When I follow "Generate Accession Number" - Then I should see "We are experiencing problems, sorry" - - Scenario: There are problems contacting the accession number service - Given an accessioning webservice is unavailable - - Given an accession number is required for study "Study for accession number testing" - - When I follow "Generate Accession Number" - Then I should see "EBI may be down or invalid data submitted" diff --git a/features/support/accession_service.rb b/features/support/accession_service.rb deleted file mode 100644 index 06e2b5b037..0000000000 --- a/features/support/accession_service.rb +++ /dev/null @@ -1,108 +0,0 @@ -# frozen_string_literal: true - -require 'singleton' -require 'rest-client' - -class FakeAccessionService - include Singleton - - # Unfortunately Webmock doesn't handle multipart files, so we can't access - # the payload. Instead we set up our own evesdropping Rest Client class - # and use that instead. If we monkey patch the original class we evesdrop on - # everything! - class EvesdropResource < RestClient::Resource - def post(payload) - FakeAccessionService.instance.add_payload(payload) - super - end - end - - # rubocop:todo Metrics/MethodLength - def self.install_hooks(target, tags) # rubocop:todo Metrics/AbcSize - target.instance_eval do - Before(tags) do |_scenario| - # Enable accessioning - @accessioning_enabled = Flipper.enabled?(:y25_706_enable_accessioning) - Flipper.enable(:y25_706_enable_accessioning) - - # Set up our evesdropper - AccessionService::BaseService.rest_client_class = EvesdropResource - - # We actually know what the value of these will be - # but we include the lookup here, as we're more keen - # on where they are sourced from, rather than what they are - accession_url = configatron.accession.url! - - ena_login = [configatron.accession.ena.user!, configatron.accession.ena.password!] - ega_login = [configatron.accession.ega.user!, configatron.accession.ega.password!] - - [ena_login, ega_login].each do |service_login| - stub_request(:post, accession_url) - .with(basic_auth: service_login) - .to_return do |_request| - response = FakeAccessionService.instance.next! - status = response.nil? ? 500 : 200 - { headers: { 'Content-Type' => 'text/xml' }, body: response, status: status } - end - end - end - - After(tags) do |_scenario| - FakeAccessionService.instance.clear - - # Remove the evesdropper - AccessionService::BaseService.rest_client_class = RestClient::Resource - - # Revert accessioning - Flipper.enable(:y25_706_enable_accessioning, @accessioning_enabled) - end - end - end - - # rubocop:enable Metrics/MethodLength - - def bodies - @bodies ||= [] - end - - def sent - @sent ||= [] - end - - attr_reader :last_received - - def clear - @bodies = [] - @sent = [] - end - - def success(type, accession, body = '') - model = type.upcase - bodies << <<-XML - <RECEIPT success="true"> - <#{model} accession="#{accession}">#{body}</#{model}> - <SUBMISSION accession="EGA00001000240" /> - </RECEIPT> - XML - end - - def failure(message) - bodies << "<RECEIPT success=\"false\"><MESSAGES><ERROR>#{message}</ERROR></MESSAGES></RECEIPT>" - end - - def next! - @last_received = bodies.pop - end - - def service - Service - end - - def add_payload(payload) - sent.push(Hash[*payload.map { |k, v| [k, v.readlines] }.map { |k, v| [k, (v unless v.empty?)] }.flatten]) - end -end - -require 'rest_client' - -FakeAccessionService.install_hooks(self, '@accession-service') diff --git a/features/support/step_definitions/35177179_updating_released_samples_steps.rb b/features/support/step_definitions/35177179_updating_released_samples_steps.rb deleted file mode 100644 index a7e575f91c..0000000000 --- a/features/support/step_definitions/35177179_updating_released_samples_steps.rb +++ /dev/null @@ -1,19 +0,0 @@ -# frozen_string_literal: true - -Given /^the sample name "([^"]*)" has previously been released$/ do |name| - Sample.find_by(name:).release -end - -When /^ignoring "([^"]+)" the XML submission for the sample "([^"]*)" should be:$/ do |key_regexp, name, serialized_xml| - sample = Sample.find_by(name:) or raise StandardError, "Cannot find sample with name #{name.inspect}" - accession_service = AccessionService.select_for_sample(sample) - accessionable_sample = Accessionable::Sample.new(sample) - submission = Accessionable::Submission.new(accession_service, User.find_by(login: 'me'), accessionable_sample) - regexp = Regexp.new(key_regexp) - block = ->(key) { key.to_s =~ regexp } - assert_hash_equal( - sort_arrays(walk_hash_structure(Hash.from_xml(serialized_xml), &block)), - sort_arrays(walk_hash_structure(Hash.from_xml(submission.xml), &block)), - 'XML differs when decoded' - ) -end diff --git a/features/support/step_definitions/4491710_ega_integration_steps.rb b/features/support/step_definitions/4491710_ega_integration_steps.rb index 117f305d8f..cccc0d785f 100644 --- a/features/support/step_definitions/4491710_ega_integration_steps.rb +++ b/features/support/step_definitions/4491710_ega_integration_steps.rb @@ -1,25 +1,5 @@ # frozen_string_literal: true -# rubocop:todo Layout/LineLength -Given /^an accessioning webservice exists which returns a (study|sample|dac|policy) accession number "([^"]*)"$/ do |type, accession_number| - # rubocop:enable Layout/LineLength - FakeAccessionService.instance.success(type, accession_number) -end - -Given /^an accessioning webservice exists that errors with "([^"]+)"$/ do |message| - FakeAccessionService.instance.failure(message) -end - -Given /^an accessioning service exists which returns an array express accession number "([^"]+)"/ do |ae_an| - FakeAccessionService.instance.success('Study', 'EGAS00001000241', <<-XML) - <EXT_ID accession="#{ae_an}" type="ArrayExpress"/> - XML -end - -Given /^an accessioning webservice is unavailable$/ do - # Do nothing, just don't tag the scenario! -end - Given /^an accession number is required for study "([^"]*)"$/ do |study_name| study = Study.find_by(name: study_name) or raise StandardError, "Cannot find study #{study_name.inspect}" study.enforce_accessioning = true diff --git a/features/support/step_definitions/samples_steps.rb b/features/support/step_definitions/samples_steps.rb index 700fa68f82..9589e1130d 100644 --- a/features/support/step_definitions/samples_steps.rb +++ b/features/support/step_definitions/samples_steps.rb @@ -49,44 +49,6 @@ assert_equal(genome, sample.sample_metadata.reference_genome.name) end -# rubocop:todo Layout/LineLength -Then /^the XML root attribute "([^"]+)" sent to the accession service for sample "([^"]+)" should be "(.*?)"$/ do |xml_attr, sample_name, value| - # rubocop:enable Layout/LineLength - sample = Sample.find_by(name: sample_name) or - raise StandardError, "Cannot find sample with name #{sample_name.inspect}" - xml = FakeAccessionService.instance.sent.last['SAMPLE'].to_s - assert_equal(value, Nokogiri(xml).xpath("/SAMPLE_SET/SAMPLE/@#{xml_attr}").map(&:to_s)[0]) -end - -Then /^the XML sent for sample "([^"]+)" validates with the schema "([^"]+)"$/ do |sample_name, schema| - sample = Sample.find_by(name: sample_name) or - raise StandardError, "Cannot find sample with name #{sample_name.inspect}" - xml = FakeAccessionService.instance.sent.last['SAMPLE'].to_s - - # Schema downloaded from http://www.ebi.ac.uk/ena/submit/data-formats - xsd = Nokogiri::XML.Schema(File.open(schema)) - result = xsd.validate(Nokogiri(xml)) - assert(result.length == 0, result.map(&:message).join('')) -end - -# rubocop:todo Layout/LineLength -Then /^the XML tag "([^"]+)" sent to the accession service for sample "([^"]+)" should be not present$/ do |xml_attr, sample_name| - # rubocop:enable Layout/LineLength - sample = Sample.find_by(name: sample_name) or - raise StandardError, "Cannot find sample with name #{sample_name.inspect}" - xml = FakeAccessionService.instance.sent.last['SAMPLE'].to_s - assert_equal(true, Nokogiri(xml).xpath("/SAMPLE_SET/SAMPLE/#{xml_attr}").length == 0) -end - -# rubocop:todo Layout/LineLength -Then /^the XML tag "([^"]+)" sent to the accession service for sample "([^"]+)" should be "(.*?)"$/ do |xml_attr, sample_name, value| - # rubocop:enable Layout/LineLength - sample = Sample.find_by(name: sample_name) or - raise StandardError, "Cannot find sample with name #{sample_name.inspect}" - xml = FakeAccessionService.instance.sent.last['SAMPLE'].to_s - assert_equal(value, Nokogiri(xml).xpath("/SAMPLE_SET/SAMPLE/#{xml_attr}").text) -end - Given /^the metadata attribute "(.*?)" of the sample "(.*?)" is "(.*?)"$/ do |attr_name, sample_name, value| sample = Sample.find_by(name: sample_name) or raise StandardError, "Cannot find sample with name #{sample_name.inspect}" @@ -142,20 +104,6 @@ step("I follow \"#{action_str}\"") end -# rubocop:todo Layout/LineLength -Then /^I (should|should not) have (sent|received) the attribute "([^"]*)" for the sample element (to|from) the accessioning service$/ do |state_action, type_action, attr_name, _dest| - # rubocop:enable Layout/LineLength - xml = - if type_action == 'sent' - FakeAccessionService.instance.sent.last['SAMPLE'] - else - FakeAccessionService.instance.last_received - end - assert_equal (state_action == 'should'), - Nokogiri(xml).xpath("/SAMPLE_SET/SAMPLE/@#{attr_name}").map(&:to_s).present?, - "XML was: #{xml}" -end - Given /^sample "([^"]*)" came from a sample manifest$/ do |sample_name| sample = Sample.find_by(name: sample_name) sample_manifest = FactoryBot.create(:sample_manifest, id: 1) diff --git a/lib/accession.rb b/lib/accession.rb index 8104859551..69214a7fb5 100644 --- a/lib/accession.rb +++ b/lib/accession.rb @@ -5,7 +5,6 @@ module Accession # check configuration settings, in particular: # configatron.proxy # configatron.accession url, ega.user, ega.password, ena.user, ena.password - # configatron.accession_local_key (authorised user uuid) # check that Sequencescape sample sample_metadata meets accessioning requirements # feature flag y25_706_enable_accessioning should be set to true to automatically accession a sample after save # (app/models/sample.rb) @@ -48,7 +47,6 @@ def <=>(other) require_relative 'accession/core_extensions' require_relative 'accession/accessionable' - require_relative 'accession/contact' require_relative 'accession/service' require_relative 'accession/sample' require_relative 'accession/tag' diff --git a/lib/accession/contact.rb b/lib/accession/contact.rb deleted file mode 100644 index 107bad8e2c..0000000000 --- a/lib/accession/contact.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true -module Accession - # The contact will be the person who will be informed if accessioning errors - # or if the status needs to be checked. - # Is this ever used? - class Contact - attr_reader :user - - def initialize(user) - @user = user - end - - def name - @name ||= "#{user.first_name} #{user.last_name}" - end - - def email - @email ||= "#{user.login}@#{configatron.default_email_domain}" - end - - def to_h - { inform_on_error: email, inform_on_status: email, name: name } - end - end -end diff --git a/lib/accession/submission.rb b/lib/accession/submission.rb index 97de921854..5ad4182ff5 100644 --- a/lib/accession/submission.rb +++ b/lib/accession/submission.rb @@ -6,17 +6,16 @@ class Submission include ActiveModel::Model include Accession::Accessionable - attr_reader :sample, :service, :contact + attr_reader :sample, :service delegate :accessioned?, :ebi_alias, :ebi_alias_datestamped, to: :sample - validates_presence_of :contact, :sample + validates_presence_of :sample validate :check_sample, if: proc { |s| s.sample.present? } - def initialize(contact_user, sample) + def initialize(sample) @sample = sample @service = sample&.service - @contact = contact_user ? Contact.new(contact_user) : nil # only create Contact if user is present end # Define the client as a class method for easy test mocking @@ -32,7 +31,6 @@ def build_xml(xml) alias: sample.ebi_alias_datestamped, submission_date: date ) do - xml.CONTACTS { xml.CONTACT(contact.to_h) } actions(xml) end end diff --git a/spec/controllers/samples_controller_spec.rb b/spec/controllers/samples_controller_spec.rb index d1252924e5..d58754b51a 100644 --- a/spec/controllers/samples_controller_spec.rb +++ b/spec/controllers/samples_controller_spec.rb @@ -117,18 +117,9 @@ let(:accession_individual_samples_with_sample_accessioning_job) { false } before do - if accession_individual_samples_with_sample_accessioning_job - Flipper.enable :y25_286_accession_individual_samples_with_sample_accessioning_job - - create(:user, api_key: configatron.accession_local_key) # create contact user - allow(Accession::Submission).to receive(:client).and_return( - stub_accession_client(:submit_and_fetch_accession_number, return_value: 'EGA00001000240') - ) - else - Flipper.disable :y25_286_accession_individual_samples_with_sample_accessioning_job - - allow_any_instance_of(RestClient::Resource).to receive(:post).and_return(successful_sample_accession_response) - end + allow(Accession::Submission).to receive(:client).and_return( + stub_accession_client(:submit_and_fetch_accession_number, return_value: 'EGA00001000240') + ) get :accession, params: { id: sample.id }, @@ -190,8 +181,7 @@ it 'displays an error message indicating the validation failure' do expect(flash[:error]).to eq(<<~MSG.squish) Please fill in the required fields: - Sample metadata gender is required, Sample metadata phenotype is required, - Sample metadata donor is required, Sample metadata is invalid + Sample does not have the required metadata: donor-id, gender, and phenotype. MSG end end @@ -203,56 +193,36 @@ before { sample.reload } # Reload to get updated accession number - context 'when the accession_individual_samples_with_sample_accessioning_job feature flag is disabled' do - let(:accession_individual_samples_with_sample_accessioning_job) { false } - - it 'assigns an accession number to the sample' do - expect(sample.ebi_accession_number).to eq('EGA00001000240') - end + it 'assigns an accession number to the sample' do + expect(sample.ebi_accession_number).to eq('EGA00001000240') + end - it 'redirects to the sample page' do - expect(response).to redirect_to(sample_path(sample.id)) - end + it 'redirects to the sample page' do + expect(response).to redirect_to(sample_path(sample.id)) + end - it 'displays a notice message with the generated accession number' do - expect(flash[:notice]).to eq("Accession number generated: #{sample.ebi_accession_number}") - end + it 'displays a notice message with the generated accession number' do + expect(flash[:notice]).to eq("Accession number generated: #{sample.ebi_accession_number}") end - context 'when the accession_individual_samples_with_sample_accessioning_job feature flag is enabled' do - let(:accession_individual_samples_with_sample_accessioning_job) { true } + context 'when a network error occurs during accessioning' do + before do + allow(Accession::Submission).to receive(:client).and_return( + stub_accession_client(:submit_and_fetch_accession_number, + raise_error: Faraday::ConnectionFailed.new('Network connection failed')) + ) - it 'assigns an accession number to the sample' do - expect(sample.ebi_accession_number).to eq('EGA00001000240') + get :accession, + params: { id: sample.id }, + session: { user: current_user.id } end it 'redirects to the sample page' do expect(response).to redirect_to(sample_path(sample.id)) end - it 'displays a notice message with the generated accession number' do - expect(flash[:notice]).to eq("Accession number generated: #{sample.ebi_accession_number}") - end - - context 'when a network error occurs during accessioning' do - before do - allow(Accession::Submission).to receive(:client).and_return( - stub_accession_client(:submit_and_fetch_accession_number, - raise_error: Faraday::ConnectionFailed.new('Network connection failed')) - ) - - get :accession, - params: { id: sample.id }, - session: { user: current_user.id } - end - - it 'redirects to the sample page' do - expect(response).to redirect_to(sample_path(sample.id)) - end - - it 'displays an error message indicating a network error occurred' do - expect(flash[:error]).to eq('Accessioning failed with a network error: Network connection failed') - end + it 'displays an error message indicating a network error occurred' do + expect(flash[:error]).to eq('Accessioning failed with a network error: Network connection failed') end end diff --git a/spec/controllers/studies_controller_spec.rb b/spec/controllers/studies_controller_spec.rb index ba3f93b587..90a55c5e42 100644 --- a/spec/controllers/studies_controller_spec.rb +++ b/spec/controllers/studies_controller_spec.rb @@ -182,7 +182,6 @@ let(:study) { samples.first.studies.first } before do - create(:user, api_key: configatron.accession_local_key) # create contact user allow(Rails.logger).to receive(:info).and_call_original allow(Accession::Submission).to receive(:client).and_return( stub_accession_client(:submit_and_fetch_accession_number, return_value: 'EGA00001000240') @@ -192,8 +191,50 @@ end context 'when the accessioning succeeds' do - it 'redirects to the accession-statuses tab of the study page' do - expect(subject).to redirect_to(study_path(study, anchor: 'accession-statuses')) + it 'accessions all samples in the study' do + study.samples.each do |sample| + expect(sample.reload.sample_metadata.sample_ebi_accession_number).to eq('EGA00001000240') + end + end + + it 'redirects to the study page' do + expect(subject).to redirect_to(study_path(study)) + end + + it 'does not set a flash error message' do + expect(flash[:error]).to be_nil + end + + it 'does not set a flash warning message' do + expect(flash[:warning]).to be_nil + end + + it 'sets a flash notice message' do + expect(flash[:notice]).to eq( + 'All of the samples in this study have been sent for accessioning. ' \ + 'Please check back in 5 minutes to confirm that accessioning was successful.' + ) + end + + it 'does not set a flash info message' do + expect(flash[:info]).to be_nil + end + end + + context 'when a sample already has an accession number' do + # add a 6th already accessioned sample to the study + let(:samples) { create_list(:sample_for_accessioning, number_of_samples) + create_list(:accessioned_sample, 1) } + let(:study) { create(:open_study, accession_number: 'ENA123', samples: samples) } + + it 'does not attempt to accession accessioned samples' do + # confirm that only 5 calls were made to the accession client, not 6 + expect(Accession::Submission.client) + .to have_received(:submit_and_fetch_accession_number) + .exactly(number_of_samples).times + end + + it 'redirects to the study page' do + expect(subject).to redirect_to(study_path(study)) end it 'does not set a flash error message' do @@ -221,8 +262,8 @@ let(:samples) { create_list(:sample, number_of_samples) } let(:study) { create(:managed_study, accession_number: 'EGA123', samples: samples) } - it 'redirects to the accession-statuses tab of the study page' do - expect(subject).to redirect_to(study_path(study, anchor: 'accession-statuses')) + it 'redirects to the study page' do + expect(subject).to redirect_to(study_path(study)) end it 'does not set a flash notice message' do @@ -286,8 +327,8 @@ let(:samples) { create_list(:sample_for_accessioning_with_open_study, number_of_samples) } let(:study) { create(:managed_study, accession_number: 'EGA123', samples: samples) } - it 'redirects to the accession-statuses tab of the study page' do - expect(subject).to redirect_to(study_path(study, anchor: 'accession-statuses')) + it 'redirects to the study page' do + expect(subject).to redirect_to(study_path(study)) end it 'sets a flash notice message' do @@ -308,6 +349,63 @@ end end end + + context 'when the study does not have an accession number' do + let(:study) { create(:managed_study, samples:) } + + it 'does not attempt to accession samples' do + expect(Accession::Submission.client).not_to have_received(:submit_and_fetch_accession_number) + end + + it 'redirects to the study page' do + expect(subject).to redirect_to(study_path(study)) + end + + it 'does not set a flash warning message' do + expect(flash[:warning]).to be_nil + end + + it 'does not set a flash notice message' do + expect(flash[:notice]).to be_nil + end + + it 'sets a flash error message' do + expect(flash[:error]).to eq('Please accession the study before accessioning samples') + end + + it 'does not set a flash info message' do + expect(flash[:info]).to be_nil + end + end + + context 'when a study is not longer accessionable' do + let(:study_metadata) { create(:study_metadata_for_accessioning, study_ebi_accession_number: 'EGA123') } + let(:study) { create(:study, study_metadata:, samples:) } + + it 'does not attempt to accession samples' do + expect(Accession::Submission.client).not_to have_received(:submit_and_fetch_accession_number) + end + + it 'redirects to the study page' do + expect(subject).to redirect_to(study_path(study)) + end + + it 'does not set a flash warning message' do + expect(flash[:warning]).to be_nil + end + + it 'does not set a flash notice message' do + expect(flash[:notice]).to be_nil + end + + it 'sets a flash error message' do + expect(flash[:error]).to eq('Study cannot accession samples, see Study Accessioning tab for details') + end + + it 'does not set a flash info message' do + expect(flash[:info]).to be_nil + end + end end end end diff --git a/spec/controllers/ultima_sample_sheet/sample_sheet_generator_spec.rb b/spec/controllers/ultima_sample_sheet/sample_sheet_generator_spec.rb index a1da05bb06..923a12a490 100644 --- a/spec/controllers/ultima_sample_sheet/sample_sheet_generator_spec.rb +++ b/spec/controllers/ultima_sample_sheet/sample_sheet_generator_spec.rb @@ -45,13 +45,28 @@ # 6,Sample6,Z0099,TCAG,2,C1,native,6 # # rubocop:enable Layout/LineLength +# rubocop:disable RSpec/MultipleExpectations, RSpec/ExampleLength RSpec.describe UltimaSampleSheet::SampleSheetGenerator do # Eagerly create the global section record. before { create(:ultima_global) } + # First oligo sequences for the two tag groups. + let(:plate1_first_oligo) { 'CAGCTCGAATGCGAT' } + let(:plate2_first_oligo) { 'CAGTCAGTTGCAGAT' } + # Eagerly create tag groups and tags to get consistent IDs. - let!(:tag_group1) { create(:tag_group_with_tags, tag_count: 96) } - let!(:tag_group2) { create(:tag_group_with_tags, tag_count: 96) } + let!(:tag_group1) do + create(:tag_group_with_tags, tag_count: 96, name: 'Ultima P1').tap do |tg| + # To test Z0001 matching with the oligo sequence. + tg.tags.first.update!(oligo: plate1_first_oligo) + end + end + let!(:tag_group2) do + create(:tag_group_with_tags, tag_count: 96, name: 'Ultima P2').tap do |tg| + # To test Z0097 matching with the oligo sequence. + tg.tags.first.update!(oligo: plate2_first_oligo) + end + end let(:tag_groups) { [tag_group1, tag_group2] } let(:request_type) { create(:ultima_sequencing) } @@ -210,7 +225,7 @@ def map_description(map_id) expect(csv2[1].compact_blank).to eq(["Batch #{batch.id} #{tube2.human_barcode}"]) # Second CSV end - it 'generates global sections' do # rubocop:disable RSpec/MultipleExpectations + it 'generates global sections' do # Test: Add the following hardcoded values, Application(WGS native gDNA), # sequencing_recipe(UG_116cycles_Baseline_1.8.5.2) and analysis_recipe(wgs1) expect(csv1[3].compact_blank).to eq(generator.class::GLOBAL_TITLE) @@ -219,11 +234,24 @@ def map_description(map_id) expect(csv1[6].compact_blank).to eq([]) end - it 'generates samples sections' do # rubocop:disable RSpec/MultipleExpectations + it 'generates samples sections' do expect(csv1[7].compact_blank).to eq(generator.class::SAMPLES_TITLE) expect(csv1[8].compact_blank).to eq(generator.class::SAMPLES_HEADERS) expect(csv1[9..]).to eq(csv1_samples) # First CSV expect(csv2[9..]).to eq(csv2_samples) # Second CSV end + + it 'matches the z-indexes, oligo sequences, and plate numbers' do + # First CSV + expect(csv1[9][2]).to eq('Z0001') # Index_Barcode_Num + expect(csv1[9][3]).to eq(plate1_first_oligo) # Index_Barcode_Sequence + expect(csv1[9][4]).to eq('1') # Barcode_Plate_Num + + # Second CSV + expect(csv2[9][2]).to eq('Z0097') # Index_Barcode_Num + expect(csv2[9][3]).to eq(plate2_first_oligo) # Index_Barcode_Sequence + expect(csv2[9][4]).to eq('2') # Barcode_Plate_Num + end end end +# rubocop:enable RSpec/MultipleExpectations, RSpec/ExampleLength diff --git a/spec/factories/accession/submissions.rb b/spec/factories/accession/submissions.rb index 50416b4400..78f9b854b7 100644 --- a/spec/factories/accession/submissions.rb +++ b/spec/factories/accession/submissions.rb @@ -2,10 +2,9 @@ FactoryBot.define do factory :accession_submission, class: 'Accession::Submission' do - user { create(:user) } sample { build(:accession_sample) } - initialize_with { new(user, sample) } + initialize_with { new(sample) } skip_create end end diff --git a/spec/factories/roles.rb b/spec/factories/roles.rb index 681292e83a..99ad2c54c2 100644 --- a/spec/factories/roles.rb +++ b/spec/factories/roles.rb @@ -45,4 +45,9 @@ transient { follower { build(:user) } } roles { |role| [role.association(:role, name: 'follower', users: [follower])] } end + + trait :with_data_access_contacts do + transient { data_access_contacts { build_list(:user, 1) } } + roles { |role| [role.association(:role, name: 'Data Access Contact', users: data_access_contacts)] } + end end diff --git a/spec/features/studies/accession_all_samples_spec.rb b/spec/features/studies/accession_all_samples_spec.rb index 59ebafa784..2bff46abb0 100644 --- a/spec/features/studies/accession_all_samples_spec.rb +++ b/spec/features/studies/accession_all_samples_spec.rb @@ -5,7 +5,7 @@ describe 'Accession all samples', :accessioning_enabled, :un_delay_jobs do include AccessionV1ClientHelper - let(:user) { create(:admin, api_key: configatron.accession_local_key) } # admin required for accession permissions + let(:user) { create(:admin) } # admin required for accession permissions let(:study) { create(:open_study, accession_number: 'ENA123', samples: create_list(:sample_for_accessioning, 5)) } before do diff --git a/spec/features/studies/accession_study_dac_policy_spec.rb b/spec/features/studies/accession_study_dac_policy_spec.rb new file mode 100644 index 0000000000..9bdff44778 --- /dev/null +++ b/spec/features/studies/accession_study_dac_policy_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'EGA DAC and Policy accessioning', :accessioning_enabled, :js, type: :feature do + include MockAccession + + let(:user) { create(:admin) } + let(:data_access_contacts) { create_list(:user, 1) } + + before do + login_user(user) + end + + context 'when a managed study has a valid DAC set but no accession number for it' do + let(:study) { create(:managed_study, :with_data_access_contacts, data_access_contacts:) } + + before do + allow_any_instance_of(RestClient::Resource).to receive(:post) # rubocop:disable RSpec/AnyInstance + .and_return(successful_dac_policy_accession_response) + + visit study_path(study) + end + + it 'generates a DAC accession number and displays it on study details' do + click_link 'Generate DAC Accession Number' + visit study_information_path(study) + click_link 'Study details' + expect(page).to have_content('EGAD0001000234') + end + end + + context 'when an open study has a valid DAC set but no accession number for it' do + let(:study) { create(:open_study, :with_data_access_contacts, data_access_contacts:) } + + before do + allow_any_instance_of(RestClient::Resource).to receive(:post) # rubocop:disable RSpec/AnyInstance + .and_return(failed_accession_response) + + visit study_path(study) + end + + it 'does not generate a DAC accession number' do + click_link 'Generate DAC Accession Number' + expect(page).to have_content('No accession number was generated') + end + end + + context 'when a managed study has a valid Policy set but no accession number for it' do + let(:study) { create(:managed_study, :with_data_access_contacts, data_access_contacts:) } + + before do + allow_any_instance_of(RestClient::Resource).to receive(:post) # rubocop:disable RSpec/AnyInstance + .and_return(successful_dac_policy_accession_response) + + study.study_metadata.update(ega_dac_accession_number: 'EGAD0001000234') # DAC required prior to Policy + + visit study_path(study) + end + + it 'generates a Policy accession number and displays it on study details' do + click_link 'Generate Policy Accession Number' + visit study_information_path(study) + click_link 'Study details' + expect(page).to have_content('EGAP0001000234') + end + end + + context 'when an open study has a valid Policy set but no accession number for it' do + let(:study) { create(:open_study, :with_data_access_contacts, data_access_contacts:) } + + before do + allow_any_instance_of(RestClient::Resource).to receive(:post) # rubocop:disable RSpec/AnyInstance + .and_return(failed_accession_response) + + visit study_path(study) + end + + it 'does not generate a Policy accession number' do + click_link 'Generate Policy Accession Number' + expect(page).to have_content('No accession number was generated') + end + end + + context 'when a managed study has an invalid DAC' do + let(:study) { create(:managed_study) } # no data access contacts + + before do + visit study_path(study) + end + + it 'shows error and does not display accession number on study details' do + click_link 'Generate DAC Accession Number' + expect(page).to have_content('Data Access Contacts Empty') + end + end +end diff --git a/spec/features/studies/accession_study_spec.rb b/spec/features/studies/accession_study_spec.rb new file mode 100644 index 0000000000..eb36b8b768 --- /dev/null +++ b/spec/features/studies/accession_study_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Study accession number', :accessioning_enabled, :js, type: :feature do + include MockAccession + + let(:user) { create(:admin, first_name: 'John', last_name: 'Smith') } + let(:study) { create(:managed_study) } + + before do + login_user(user) + visit study_path(study) + end + + context 'when the study does not need an accession number' do + let(:study) { create(:study) } + + it 'shows not required message' do + click_link 'Generate Accession Number' + expect(page).to have_css('.alert', text: 'An accession number is not required for this study') + expect(page).to have_current_path(study_information_path(study)) + end + end + + context 'when the study already has an accession number' do + before do + allow_any_instance_of(RestClient::Resource).to receive(:post).and_return(successful_study_accession_response) # rubocop:disable RSpec/AnyInstance + + study.study_metadata.update!(study_ebi_accession_number: 'EGAN00001000234') + visit study_path(study) + end + + it 'shows the generated accession number' do + click_link 'Update Study Data for Accessioning' + expect(page).to have_css('.alert', text: 'Accession number generated: EGA00002000345') + end + end + + context 'when required fields are missing' do + %w[study_study_title study_abstract].each do |attribute| + it "shows required fields message when #{attribute} is missing" do + study.study_metadata.update!(attribute => '') + visit study_path(study) + click_link 'Generate Accession Number' + expect(page).to have_css('.alert', text: 'Please fill in the required fields') + end + end + end + + context 'when the study gets a valid accession number' do + before do + allow_any_instance_of(RestClient::Resource).to receive(:post) # rubocop:disable RSpec/AnyInstance + .and_return(successful_study_accession_response) + + visit study_path(study) + end + + it 'shows the generated accession number and displays it on study details' do + click_link 'Generate Accession Number' + expect(page).to have_css('.alert', text: 'Accession number generated: EGA00002000345') + visit study_path(study) + click_link 'Study details' + expect(page).to have_content('EGA00002000345') + end + end + + context 'when the accession number service gives an error' do + before do + allow_any_instance_of(RestClient::Resource).to receive(:post) # rubocop:disable RSpec/AnyInstance + .and_return(failed_accession_response) + + visit study_path(study) + end + + it 'shows the error message' do + click_link 'Generate Accession Number' + expect(page).to have_css('.alert', text: 'Error 1; Error 2') + end + end + + context 'when the accession number service is unavailable' do + before do + allow_any_instance_of(RestClient::Resource).to receive(:post) # rubocop:disable RSpec/AnyInstance + .and_raise(RestClient::ServiceUnavailable) + + visit study_path(study) + end + + it 'shows the unavailable message' do + click_link 'Generate Accession Number' + expect(page).to have_css('.alert', text: 'EBI may be down or invalid data submitted') + end + end +end diff --git a/spec/helpers/samples_helper_spec.rb b/spec/helpers/samples_helper_spec.rb new file mode 100644 index 0000000000..fd74b15655 --- /dev/null +++ b/spec/helpers/samples_helper_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SamplesHelper, type: :helper do + describe '#save_text' do + subject { helper.save_text(sample) } + + let(:sample) { create(:sample) } + + let(:accessioning_enabled) { true } + let(:should_be_accessioned) { true } + let(:permitted) { true } + + before do + allow(helper).to receive(:accessioning_enabled?).and_return(accessioning_enabled) + allow(helper).to receive(:permitted_to_accession?).with(sample).and_return(permitted) + allow(sample).to receive(:should_be_accessioned?).and_return(should_be_accessioned) + end + + context 'when accessioning is enabled, sample should be accessioned, and user is permitted' do + it { is_expected.to eq('Save and Accession') } + end + + context 'when accessioning is disabled' do + let(:accessioning_enabled) { false } + + it { is_expected.to eq('Save Sample') } + end + + context 'when sample should not be accessioned' do + let(:should_be_accessioned) { false } + + it { is_expected.to eq('Save Sample') } + end + + context 'when user is not permitted to accession' do + let(:permitted) { false } + + it { is_expected.to eq('Save Sample') } + end + end + + describe '#samples_not_accessioned' do + subject { helper.samples_not_accessioned(samples) } + + context 'when all samples are accessioned' do + let(:samples) { build_list(:accessioned_sample, 3) } + + it { is_expected.to eq('All samples accessioned') } + end + + context 'when some samples are not accessioned' do + let(:samples) do + [ + build(:accessioned_sample), + build(:sample), + build(:sample) + ] + end + + it { is_expected.to eq('2 samples not accessioned') } + + context 'when only one sample is not accessioned' do + let(:samples) do + [ + build(:accessioned_sample), + build(:accessioned_sample), + build(:sample) + ] + end + + it { is_expected.to eq('1 sample not accessioned') } + end + end + + context 'when no samples are accessioned' do + let(:samples) { build_list(:sample, 3) } + + it { is_expected.to eq('No samples accessioned') } + end + + context 'when there are no samples' do + let(:samples) { [] } + + it { is_expected.to eq('No samples accessioned') } + end + end +end diff --git a/spec/jobs/sample_accessioning_job_spec.rb b/spec/jobs/sample_accessioning_job_spec.rb index 6796cb3c83..1935b08c54 100644 --- a/spec/jobs/sample_accessioning_job_spec.rb +++ b/spec/jobs/sample_accessioning_job_spec.rb @@ -6,7 +6,6 @@ RSpec.describe SampleAccessioningJob, type: :job do include AccessionV1ClientHelper - let(:contact_user) { create(:user, api_key: configatron.accession_local_key) } let(:sample_metadata) { create(:sample_metadata_for_accessioning) } let(:sample) { create(:sample_for_accessioning_with_open_study, sample_metadata:) } let(:accessionable) { create(:accession_sample, sample:) } @@ -21,10 +20,7 @@ end describe '#perform' do - before do - # An accession sample status is created when the job is queued - allow(described_class).to receive(:contact_user).and_return(contact_user) - end + # An accession sample status is created when the job is queued context 'when the submission fails validation' do let(:sample_metadata) { create(:sample_metadata_for_accessioning, sample_taxon_id: nil) } diff --git a/spec/lib/accession/contact_spec.rb b/spec/lib/accession/contact_spec.rb deleted file mode 100644 index ea8765ff72..0000000000 --- a/spec/lib/accession/contact_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -RSpec.describe Accession::Contact, :accession, type: :model do - subject { described_class.new(user) } - - before(:all) do - @email = configatron.default_email_domain - configatron.default_email_domain = 'example.com' - end - - let!(:user) { create(:user, login: 'user1', first_name: 'Santa', last_name: 'Claus') } - - after(:all) { configatron.default_email_domain = @email } - - it 'has a name' do - expect(subject.name).to eq('Santa Claus') - end - - it 'has an email' do - expect(subject.email).to eq('user1@example.com') - end - - it 'produces a hash for the xml' do - hsh = subject.to_h - expect(hsh[:inform_on_error]).to eq(subject.email) - expect(hsh[:inform_on_status]).to eq(subject.email) - expect(hsh[:name]).to eq(subject.name) - end -end diff --git a/spec/lib/accession/sample_spec.rb b/spec/lib/accession/sample_spec.rb index 70ed5f1bd5..6c96fb590b 100644 --- a/spec/lib/accession/sample_spec.rb +++ b/spec/lib/accession/sample_spec.rb @@ -392,6 +392,18 @@ def find_value_at_tag(xml_received, tag_name) expect(unexpected_tags).to be_empty, "Unexpected tags found in XML: '#{unexpected_tags.join("', '")}'" end + + it 'validates against the SAMPLE XSD schema' do + # Schema downloaded from https://ena-docs.readthedocs.io/en/latest/submit/general-guide/webin-v1.html + xsd_path = Rails.root.join('test/data/xsd/SRA.sample.xsd') + xsd_file = File.open(xsd_path) + schema = Nokogiri::XML::Schema(xsd_file) + document = Nokogiri::XML(xml) + errors = schema.validate(document) + expect(errors).to be_empty, "XML did not validate against XSD schema: #{errors.map(&:message).join('; ')}" + ensure + xsd_file.close + end end context 'with an OPEN study' do # study emphasised for easy test failure identification diff --git a/spec/lib/accession/study_spec.rb b/spec/lib/accession/study_spec.rb deleted file mode 100644 index 9c10e3e4cf..0000000000 --- a/spec/lib/accession/study_spec.rb +++ /dev/null @@ -1,129 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -MISSING_METADATA = { - managed_study: %w[sample-taxon-id sample-common-name gender phenotype donor-id].sort.to_sentence, - open_study: %w[sample-taxon-id sample-common-name].sort.to_sentence -}.freeze -STUDY_TYPES = %i[open_study managed_study].freeze - -RSpec.describe Study, :accession, :accessioning_enabled, :un_delay_jobs, type: :model do - include AccessionV1ClientHelper - - let(:current_user) { create(:user) } - let(:accession_number) { 'SAMPLE123456' } - let(:accessionable_samples) { create_list(:sample_for_accessioning, 5) } - let(:non_accessionable_samples) { create_list(:sample, 3) } - - before do - create(:user, api_key: configatron.accession_local_key) - allow(Accession::Submission).to receive(:client).and_return( - stub_accession_client(:submit_and_fetch_accession_number, return_value: accession_number) - ) - end - - after do - SampleManifestExcel.reset! - end - - STUDY_TYPES.each do |study_type| - context "in a #{study_type}" do - let(:missing_metadata_for_study) { MISSING_METADATA[study_type] } - - context 'when all samples in a study are accessionable' do - let(:study) { create(study_type, accession_number: 'ENA123', samples: accessionable_samples) } - - before do - study.accession_all_samples(current_user) - study.reload - end - - it 'accessions only the samples with accession numbers' do - expect(study.samples.count { |sample| sample.sample_metadata.sample_ebi_accession_number.present? }).to eq( - accessionable_samples.count - ) - end - end - - context 'with studies missing accession numbers' do - let(:study) { create(study_type, samples: create_list(:sample_for_accessioning, 5)) } - - before do - # Verify expectation before running the method - expect(Accession).not_to receive(:accession_sample).with(study.samples.first, anything) - study.accession_all_samples(current_user) - study.reload - end - - it 'does not accession any samples' do - expect(study.samples).to be_all { |sample| sample.sample_metadata.sample_ebi_accession_number.nil? } - end - end - - context 'when some samples in a study are not accessionable' do - let(:study) do - create(study_type, accession_number: 'ENA123', samples: accessionable_samples + non_accessionable_samples) - end - - before do - study.accession_all_samples(current_user) - study.reload - end - - it 'adds errors to the sample model' do - expect(study.errors.full_messages).to eq( - [ - "Sample 'Sample6' cannot be accessioned: " \ - "Sample does not have the required metadata: #{missing_metadata_for_study}.", - "Sample 'Sample7' cannot be accessioned: " \ - "Sample does not have the required metadata: #{missing_metadata_for_study}.", - "Sample 'Sample8' cannot be accessioned: " \ - "Sample does not have the required metadata: #{missing_metadata_for_study}." - ] - ) - end - - it 'accessions only the samples with accession numbers' do - expect(study.samples.count { |sample| sample.sample_metadata.sample_ebi_accession_number.present? }).to eq( - accessionable_samples.count - ) - end - - it 'does not accession samples without accession numbers' do - expect(study.samples.count { |sample| sample.sample_metadata.sample_ebi_accession_number.nil? }).to eq( - non_accessionable_samples.count - ) - end - end - - context 'when none of the samples in a study are accessionable' do - let(:study) { create(study_type, accession_number: 'ENA123', samples: non_accessionable_samples) } - - before do - study.accession_all_samples(current_user) - study.reload - end - - it 'adds errors to the sample model' do - expect(study.errors.full_messages).to eq( - [ - "Sample 'Sample1' cannot be accessioned: " \ - "Sample does not have the required metadata: #{missing_metadata_for_study}.", - "Sample 'Sample2' cannot be accessioned: " \ - "Sample does not have the required metadata: #{missing_metadata_for_study}.", - "Sample 'Sample3' cannot be accessioned: " \ - "Sample does not have the required metadata: #{missing_metadata_for_study}." - ] - ) - end - - it 'does not accession samples without accession numbers' do - expect(study.samples.count { |sample| sample.sample_metadata.sample_ebi_accession_number.nil? }).to eq( - non_accessionable_samples.count - ) - end - end - end - end -end diff --git a/spec/lib/accession/submission_spec.rb b/spec/lib/accession/submission_spec.rb index 6897f138ec..b4e3f011fe 100644 --- a/spec/lib/accession/submission_spec.rb +++ b/spec/lib/accession/submission_spec.rb @@ -5,21 +5,16 @@ RSpec.describe Accession::Submission, :accession, type: :model do include AccessionV1ClientHelper - let(:contact_user) { create(:user) } let(:sample) { build(:accession_sample) } context 'when validating' do - it 'is not valid without a contact user' do - expect(described_class.new(nil, sample)).not_to be_valid - end - it 'is not valid without an accession sample' do - expect(described_class.new(contact_user, nil)).not_to be_valid + expect(described_class.new(nil)).not_to be_valid end end describe '#to_xml' do - let(:submission) { described_class.new(contact_user, sample) } + let(:submission) { described_class.new(sample) } let(:xml) { Nokogiri::XML::Document.parse(submission.to_xml) } it 'creates some xml with valid attributes' do @@ -29,11 +24,6 @@ expect(submission_xml.attribute('alias').value).to eq(submission.sample.ebi_alias_datestamped) expect(submission_xml.attribute('submission_date').value).to eq(submission.date) - contact_xml = xml.at('CONTACT') - submission.contact.to_h.each do |attribute, value| - expect(contact_xml.attribute(attribute.to_s).value).to eq(value) - end - expect(xml.at(submission.service.visibility)).to be_present actions_xml = xml.at('ACTIONS') @@ -58,7 +48,7 @@ describe '#submit_accession' do let(:event_user) { create(:user) } - let(:submission) { described_class.new(contact_user, sample) } + let(:submission) { described_class.new(sample) } before do # Inject the mocked client into the controller @@ -109,14 +99,12 @@ end context 'when the submission fails validation' do - let(:invalid_submission) { described_class.new(nil, nil) } + let(:invalid_submission) { described_class.new(nil) } let(:mock_client) { nil } # Client should not be called it 'raises an error with a message' do - error_message = "Accessionable submission is invalid: Contact can't be blank, Sample can't be blank" - expect do - invalid_submission.submit_accession(event_user) - end.to raise_error(StandardError, error_message) + expect { invalid_submission.submit_accession(event_user) } + .to raise_error(StandardError, "Accessionable submission is invalid: Sample can't be blank") end end @@ -139,7 +127,7 @@ end describe '#compile_files' do - let(:submission) { described_class.new(contact_user, sample) } + let(:submission) { described_class.new(sample) } let(:files) { submission.compile_files } it 'returns a hash of files' do diff --git a/spec/lib/accession_spec.rb b/spec/lib/accession_spec.rb index 3880d1e335..d6e267bd34 100644 --- a/spec/lib/accession_spec.rb +++ b/spec/lib/accession_spec.rb @@ -5,10 +5,6 @@ describe '.accession_sample' do include AccessionV1ClientHelper - before do - create(:user, api_key: configatron.accession_local_key) # create contact user - end - context 'when accessioning is disabled', :accessioning_disabled, :un_delay_jobs do let(:event_user) { create(:user) } let(:sample_metadata) { create(:sample_metadata_for_accessioning) } diff --git a/spec/models/accession_service_spec.rb b/spec/models/accession_service_spec.rb index 79ab43147e..394ce6bc44 100644 --- a/spec/models/accession_service_spec.rb +++ b/spec/models/accession_service_spec.rb @@ -27,155 +27,4 @@ end end end - - describe '.send_samples_to_service?' do - context 'when no study accession needed' do - let(:study) { create(:not_app_study) } - - it 'returns true' do - expect(described_class.send_samples_to_service?(study)).to be(true) - end - end - - context 'when study accession is required' do - let(:study) { create(:open_study, study_metadata:, accession_number:) } - - context 'when never release is false' do - let(:study_metadata) { create(:study_metadata, data_release_timing: Study::DATA_RELEASE_TIMING_STANDARD) } - - context 'when study accession number is present' do - let(:accession_number) { 'ENA123' } - - it 'returns true' do - expect(described_class.send_samples_to_service?(study)).to be(true) - end - end - - context 'when study accession number is absent' do - let(:accession_number) { nil } - - it 'returns false' do - expect(described_class.send_samples_to_service?(study)).to be(false) - end - end - end - - context 'when never release is true' do - let(:study_metadata) do - create(:study_metadata, - data_release_timing: Study::DATA_RELEASE_TIMING_NEVER, - data_release_prevention_reason: - 'Prevent harm (e.g sensitive studies or biosecurity) - DAC approval required') - end - - context 'when study accession number is present' do - let(:accession_number) { 'ENA123' } - - it 'returns false' do - expect(described_class.send_samples_to_service?(study)).to be(false) - end - end - - context 'when study accession number is absent' do - let(:accession_number) { nil } - - it 'returns false' do - expect(described_class.send_samples_to_service?(study)).to be(false) - end - end - end - end - end - - describe '.select_for_sample' do - let(:sample) { create(:sample, studies:) } - - context 'when sample has one open study' do - let(:open_study) { create(:open_study, accession_number: 'ENA123') } - let(:studies) { [open_study] } - - it 'returns an instance of the ENA service' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::ENAService) - end - end - - context 'when sample has one managed study' do - let(:managed_study) { create(:managed_study, accession_number: 'EGA123') } - let(:studies) { [managed_study] } - - it 'returns an instance of the EGA service' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::EGAService) - end - end - - context 'when sample has one un-accessioned study' do - let(:open_study) { create(:open_study) } - let(:studies) { [open_study] } - - it 'returns UnsuitableService' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::UnsuitableService) - end - end - - context 'when sample has one study that does not require accessioning' do - let(:other_study) { create(:not_app_study) } - let(:studies) { [other_study] } - - it 'returns NoService' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::NoService) - end - end - - context 'when sample has multiple eligible studies' do - let(:open_study) { create(:open_study, accession_number: 'ENA123') } - let(:managed_study) { create(:managed_study, accession_number: 'EGA123') } - let(:other_study) { create(:not_app_study) } - let(:studies) { [open_study, managed_study, other_study] } - - it 'returns an instance of the highest priority accession service' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::EGAService) - end - end - - context 'when sample has no eligible studies' do - let(:sample) { create(:sample, studies: []) } - - it 'returns UnsuitableService' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::UnsuitableService) - end - end - - # We prioritise the EGA, as its the more conservative of the two databases - # and it reduces the risk of accidentally making human data public - context 'when sample has an open study and a managed study' do - let(:open_study) { create(:open_study, accession_number: 'ENA123') } - let(:managed_study) { create(:managed_study, accession_number: 'EGA123') } - let(:studies) { [open_study, managed_study] } - - it 'returns an instance of the EGA service' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::EGAService) - end - end - - context 'when sample has a managed study and an open study (in the other order)' do - let(:managed_study) { create(:managed_study, accession_number: 'EGA123') } - let(:open_study) { create(:open_study, accession_number: 'ENA123') } - let(:studies) { [managed_study, open_study] } - - it 'returns an instance of the EGA service' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::EGAService) - end - end - - # We err on the side of caution here - inadvertently sending data to the ENA could be an issue. - context 'when sample has an accessioned open study but un-accessioned managed study' do - let(:open_study) { create(:open_study, accession_number: 'ENA123') } - let(:managed_study) { create(:managed_study) } - let(:studies) { [open_study, managed_study] } - - it 'returns UnsuitableService' do - expect(described_class.select_for_sample(sample)).to be_a(AccessionService::UnsuitableService) - end - end - end end diff --git a/spec/models/library_assets_stocks_spec.rb b/spec/models/library_assets_stocks_spec.rb new file mode 100644 index 0000000000..febc26dd80 --- /dev/null +++ b/spec/models/library_assets_stocks_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe SampleManifest, type: :model do + describe 'Library behaviours' do + let(:study) { create(:study) } + + context 'when asset_type is library' do + let(:manifest) { create(:sample_manifest, study: study, asset_type: 'library') } + + it 'has stocks? set to true in core_behaviour' do + expect(manifest.core_behaviour.stocks?).to be true + end + end + + context 'when asset_type is library_plate' do + let(:manifest) { create(:sample_manifest, study: study, asset_type: 'library_plate') } + + it 'has stocks? set to true in core_behaviour' do + expect(manifest.core_behaviour.stocks?).to be true + end + end + end +end diff --git a/spec/models/sample_manifest/uploader_spec.rb b/spec/models/sample_manifest/uploader_spec.rb index 7f2e607cf9..3451efe88f 100644 --- a/spec/models/sample_manifest/uploader_spec.rb +++ b/spec/models/sample_manifest/uploader_spec.rb @@ -16,7 +16,7 @@ let(:test_file_name) { 'test_file.xlsx' } let(:test_file) { Rack::Test::UploadedFile.new(Rails.root.join(test_file_name), '') } - let(:user) { create(:user, api_key: configatron.accession_local_key) } + let(:user) { create(:user) } after(:all) { SampleManifestExcel.reset! } @@ -109,7 +109,7 @@ ) download.save(test_file_name) uploader = described_class.new(test_file, SampleManifestExcel.configuration, user, false) - uploader.run! + expect { uploader.run! }.to change(Messenger, :count).by(6) expect(uploader).to be_processed expect(BroadcastEvent.count).to eq broadcast_events_count + 1 expect(uploader.upload.sample_manifest).to be_completed diff --git a/spec/models/sample_manifest_spec.rb b/spec/models/sample_manifest_spec.rb index 53268538b7..95e7a8f912 100644 --- a/spec/models/sample_manifest_spec.rb +++ b/spec/models/sample_manifest_spec.rb @@ -226,6 +226,10 @@ expect { manifest.generate }.to change(BroadcastEvent, :count).by(1) end + it 'has stocks behavior' do + expect(manifest.core_behaviour.stocks?).to be true + end + context 'once generated' do before { manifest.generate } diff --git a/spec/support/mock_accession.rb b/spec/support/mock_accession.rb index d08eec6af3..1e08ee5eb6 100644 --- a/spec/support/mock_accession.rb +++ b/spec/support/mock_accession.rb @@ -3,15 +3,19 @@ module MockAccession Response = Struct.new(:code, :body) - # for samples - def successful_sample_accession_response - Response.new(200, '<RECEIPT success="true"><SAMPLE accession="EGA00001000240" /></RECEIPT>') - end - def successful_study_accession_response Response.new(200, '<RECEIPT success="true"><STUDY accession="EGA00002000345" /></RECEIPT>') end + def successful_dac_policy_accession_response + Response.new(200, <<~XML) + <RECEIPT success="true"> + <DAC accession="EGAD0001000234" /> + <POLICY accession="EGAP0001000234" /> + </RECEIPT> + XML + end + def failed_accession_response Response.new(200, <<~XML) <RECEIPT receiptDate="2014-12-02T16:06:20.871Z" success="false"> @@ -23,6 +27,5 @@ def failed_accession_response XML end - module_function :successful_sample_accession_response, :successful_study_accession_response, - :failed_accession_response + module_function :successful_study_accession_response, :failed_accession_response end diff --git a/test/unit/accession_service_test.rb b/test/unit/accession_service_test.rb deleted file mode 100644 index 099ef58f1c..0000000000 --- a/test/unit/accession_service_test.rb +++ /dev/null @@ -1,147 +0,0 @@ -# frozen_string_literal: true - -require "#{File.dirname(__FILE__)}/../test_helper" - -class AccessionServiceTest < ActiveSupport::TestCase - def assert_tag(tag_label, value) - acc = Accessionable::Sample.new(@sample) - tag = acc.tags.detect { |tag| tag.label == tag_label } - - assert tag, "Could not find #{tag} in #{acc.tags.map(&:label).join(',')}" - subject_tag = { tag: tag.label, value: tag.value } - - assert_equal({ tag: tag_label, value: value }, subject_tag) - end - - # temporary test for hotfix - context 'A sample with a strain' do - setup do - @study = create(:open_study, accession_number: 'accss') - @sample = create(:sample, studies: [@study]) - @sample.sample_metadata.sample_strain_att = 'my strain' - end - - should 'expose strain in ERA xml' do - assert_tag('strain', 'my strain') - end - end - - context 'A sample with a gender' do - setup do - @study = create(:managed_study, accession_number: 'accss') - @sample = create(:sample, studies: [@study]) - @sample.sample_metadata.gender = 'male' - end - - should 'expose gender in EGA xml' do - assert_tag('gender', 'male') - end - end - - context 'A sample with a donor_id' do - setup do - @study = create(:managed_study, accession_number: 'accss') - @sample = create(:sample, studies: [@study]) - @sample.sample_metadata.donor_id = '123456789' - end - - should 'expose donor_id as subject id in EGA xml' do - assert_tag('subject id', '123456789') - end - - should 'dupe test' do - assert_tag('subject id', '123456789') - end - end - - context 'A sample with a country_of_origin' do - setup do - @country = create(:insdc_country, name: 'Freedonia') - @study = create(:managed_study, accession_number: 'accss') - @sample = create(:sample, studies: [@study]) - end - - context 'with unexistent country' do - setup { @sample.sample_metadata.country_of_origin = 'Pepe' } - - should 'send the default error value' do - assert_tag('geographic location (country and/or sea)', 'not provided') - end - end - - context 'with no country' do - should 'send the default error value' do - assert_tag('geographic location (country and/or sea)', 'not provided') - end - end - - context 'with right country' do - setup { @sample.sample_metadata.country_of_origin = 'Freedonia' } - - should 'send the country name' do - assert_tag('geographic location (country and/or sea)', 'Freedonia') - end - end - - context 'with other defined values for country_of_origin' do - setup { @sample.sample_metadata.country_of_origin = 'not provided' } - - should 'send the collection date' do - assert_tag('geographic location (country and/or sea)', 'not provided') - end - end - - context 'with missing for country_of_origin' do - setup { @sample.sample_metadata.country_of_origin = 'missing: endangered species' } - - should 'send the collection date' do - assert_tag('geographic location (country and/or sea)', 'missing: endangered species') - end - end - end - - context 'A sample with a collection date' do - setup do - @study = create(:managed_study, accession_number: 'accss') - @sample = create(:sample, studies: [@study]) - end - - context 'with unexistent date_of_sample_collection' do - setup { @sample.sample_metadata.date_of_sample_collection = 'Pepe' } - - should 'send the default error value' do - assert_tag('collection date', 'not provided') - end - end - - context 'with no date_of_sample_collection' do - should 'send the default error value' do - assert_tag('collection date', 'not provided') - end - end - - context 'with right date_of_sample_collection' do - setup { @sample.sample_metadata.date_of_sample_collection = '2023-04-25T00:00:00Z' } - - should 'send the collection date' do - assert_tag('collection date', '2023-04-25T00:00:00Z') - end - end - - context 'with other defined values for date_of_sample_collection' do - setup { @sample.sample_metadata.date_of_sample_collection = 'not provided' } - - should 'send the collection date' do - assert_tag('collection date', 'not provided') - end - end - - context 'with missing for date_of_sample_collection' do - setup { @sample.sample_metadata.date_of_sample_collection = 'missing: endangered species' } - - should 'send the collection date' do - assert_tag('collection date', 'missing: endangered species') - end - end - end -end