Attachments: Concatenations Controller (#262)

* chore: added in scaffold and basic tests for Samples Attachments Concatenation controller

* chore: update concatenations controller to call concatenation service, update concatenation service params and response

* chore: fix tests for concatenation controller

* chore: fix tests

app/controllers/projects/samples/application_controller.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+module Projects
+  module Samples
+    # Base Controller Samples
+    class ApplicationController < Projects::ApplicationController
+      before_action :sample
+
+      private
+
+      def sample
+        @sample = @project.samples.find_by(id: params[:sample_id]) || not_found
+      end
+    end
+  end
+end

app/controllers/projects/samples/attachments/concatenations_controller.rb

@@ -0,0 +1,38 @@
+# frozen_string_literal: true
+
+module Projects
+  module Samples
+    module Attachments
+      # Controller actions for Project Samples Attachments Concatenation
+      class ConcatenationsController < Projects::Samples::ApplicationController
+        respond_to :turbo_stream
+
+        def new
+          authorize! @project, to: :update_sample?
+
+          render turbo_stream: [], status: :ok
+        end
+
+        def create
+          authorize! @project, to: :update_sample?
+
+          @concatenated_attachments = ::Attachments::ConcatenationService.new(current_user, @sample,
+                                                                              concatenation_params).execute
+
+          if @sample.errors.empty?
+            render turbo_stream: [], status: :ok
+          else
+            @errors = @sample.errors.full_messages_for(:base)
+            render turbo_stream: [], status: :unprocessable_entity
+          end
+        end
+
+        private
+
+        def concatenation_params
+          params.permit(:basename, :delete_originals, attachment_ids: [])
+        end
+      end
+    end
+  end
+end

app/controllers/projects/samples/attachments_controller.rb

@@ -3,8 +3,7 @@
 module Projects
   module Samples
     # Controller actions for Project Samples Attachments
-    class AttachmentsController < Projects::ApplicationController
-      before_action :sample
+    class AttachmentsController < Projects::Samples::ApplicationController
       before_action :attachment, only: %i[destroy download]
 
       def create
@@ -53,10 +52,6 @@ def attachment_params
       def attachment
         @attachment = @sample.attachments.find_by(id: params[:id]) || not_found
       end
-
-      def sample
-        @sample = @project.samples.find_by(id: params[:sample_id]) || not_found
-      end
     end
   end
 end

app/services/attachments/concatenation_service.rb

@@ -24,18 +24,13 @@ def execute # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
       # authorize if user can update sample
       authorize! attachable.project, to: :update_sample? if attachable.instance_of?(Sample)
 
-      attachment_ids = concatenation_params[:attachment_ids]
+      validate_params
 
-      if attachment_ids.empty?
-        raise AttachmentConcatenationError,
-              I18n.t('services.attachments.concatenation.no_files_selected')
-      end
-
-      validate_concatenated_file_basename
+      attachment_ids = concatenation_params[:attachment_ids]
 
       is_paired_end = false
 
-      unless attachment_ids.all? { |i| i.is_a?(Integer) }
+      unless attachment_ids.all? { |i| i.is_a?(Integer) || i.is_a?(String) }
         # if multi-dimensional array of ids
         attachment_ids = attachment_ids.flatten
         is_paired_end = true
@@ -53,18 +48,23 @@ def execute # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
       validate_and_concatenate(attachments, is_paired_end)
     rescue Attachments::ConcatenationService::AttachmentConcatenationError => e
       attachable.errors.add(:base, e.message)
-      attachable
+      []
     end
 
     private
 
-    # Validates that a base file name was provided for the concatenated file
-    def validate_concatenated_file_basename
-      basename = concatenation_params[:basename]
-      if basename.nil? || basename.blank?
+    # Validates params
+    def validate_params
+      if !concatenation_params.key?(:attachment_ids) || concatenation_params[:attachment_ids].empty?
+        raise AttachmentConcatenationError,
+              I18n.t('services.attachments.concatenation.no_files_selected')
+      end
+
+      if !concatenation_params.key?(:basename) || concatenation_params[:basename].empty?
         raise AttachmentConcatenationError,
               I18n.t('services.attachments.concatenation.filename_missing')
       end
+
       true
     end
 
@@ -76,16 +76,20 @@ def validate_and_concatenate(attachments, is_paired_end)
 
       validate_file_extensions(attachments)
 
+      concatenated_attachments = []
+
       if is_paired_end
         validate_paired_end_files(attachments)
-        concatenate_paired_end_reads(attachments)
+        concatenated_attachments = concatenate_paired_end_reads(attachments)
       else
         validate_single_end_files(attachments)
-        concatenate_single_end_reads(attachments)
+        concatenated_attachments = concatenate_single_end_reads(attachments)
       end
 
       # if option is selected then destroy the original files
       attachments.each(&:destroy) if concatenation_params[:delete_originals]
+
+      concatenated_attachments
     end
 
     # Validates that the single end files are all the same type

config/routes/project.rb

@@ -29,6 +29,11 @@
           member do
             get :download
           end
+          scope module: :attachments, as: :attachments do
+            collection do
+              resource :concatenation, only: %i[create new]
+            end
+          end
         end
       end
     end

test/controllers/projects/samples/attachments/concatenations_controller_test.rb

@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+module Projects
+  module Samples
+    module Attachments
+      class ConcatenationControllerTest < ActionDispatch::IntegrationTest
+        setup do
+          sign_in users(:john_doe)
+          @sample1 = samples(:sample1)
+          @project1 = projects(:project1)
+          @namespace = groups(:group_one)
+          @attachment1 = attachments(:attachment1)
+          @attachment2 = attachments(:attachment2)
+        end
+
+        test 'should get new for a member with role >= maintainer' do
+          get new_namespace_project_sample_attachments_concatenation_path(@namespace, @project1, @sample1,
+                                                                          format: :turbo_stream)
+          assert_response :success
+        end
+
+        test 'should not get new if not a member' do
+          user = users(:micha_doe)
+          login_as user
+
+          get new_namespace_project_sample_attachments_concatenation_path(@namespace, @project1, @sample1,
+                                                                          format: :turbo_stream)
+          assert_response :unauthorized
+        end
+
+        test 'should create sample attachments concatenation for a member with role >= maintainer' do
+          post namespace_project_sample_attachments_concatenation_path(@namespace, @project1, @sample1,
+                                                                       format: :turbo_stream),
+               params: {
+                 basename: 'blah',
+                 attachment_ids: [@attachment1.id, @attachment2.id]
+               }
+
+          assert_response :success
+        end
+
+        test 'should not create sample attachments concatenation for a non member' do
+          user = users(:micha_doe)
+          login_as user
+
+          post namespace_project_sample_attachments_concatenation_path(@namespace, @project1, @sample1,
+                                                                       format: :turbo_stream)
+          assert_response :unauthorized
+        end
+      end
+    end
+  end
+end

test/fixtures/active_storage/attachments.yml

@@ -3,6 +3,11 @@ attachment1_file_test_file_fastq:
   record: attachment1 (Attachment)
   blob: attachment1_file_test_file_fastq_blob
 
+attachment2_file_test_file_A_fastq:
+  name: file
+  record: attachment2 (Attachment)
+  blob: attachment2_file_test_file_A_fastq_blob
+
 attachmentA_file_test_file_fastq:
   name: file
   record: attachmentA (Attachment)

test/fixtures/active_storage/blobs.yml

@@ -1,5 +1,7 @@
 attachment1_file_test_file_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file.fastq", service_name: "test" %>
 
+attachment2_file_test_file_A_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file_A.fastq", service_name: "test" %>
+
 test_file_fastq_blob: <%= ActiveStorage::FixtureSet.blob filename: "test_file.fastq", service_name: "test" %>
 
 testsample_illumina_pe_forward_blob: <%= ActiveStorage::FixtureSet.blob filename: "TestSample_S1_L001_R1_001.fastq", service_name: "test" %>

test/fixtures/attachments.yml

@@ -1,7 +1,11 @@
 # Read about fixtures at https://api.rubyonrails.org/classes/ActiveRecord/FixtureSet.html
 
 attachment1:
-  metadata: { "compression": "none" }
+  metadata: { "compression": "none", "format": "fastq" }
+  attachable: sample1 (Sample)
+
+attachment2:
+  metadata: { "compression": "none", "format": "fastq" }
   attachable: sample1 (Sample)
 
 attachmentA:

test/models/attachment_test.rb

@@ -31,101 +31,101 @@ def setup
     new_fastq_attachment_ext_fastq.file.attach(io: Rails.root.join('test/fixtures/files/test_file_1.fastq').open,
                                                filename: 'test_file_1.fastq')
     new_fastq_attachment_ext_fastq.save
-    assert_equal new_fastq_attachment_ext_fastq.metadata['format'], 'fastq'
+    assert_equal 'fastq', new_fastq_attachment_ext_fastq.metadata['format']
 
     new_fastq_attachment_ext_fastq_gz = @sample.attachments.build
     new_fastq_attachment_ext_fastq_gz.file.attach(io: Rails.root.join('test/fixtures/files/test_file_2.fastq.gz').open,
                                                   filename: 'test_file_2.fastq.gz')
     new_fastq_attachment_ext_fastq_gz.save
-    assert_equal new_fastq_attachment_ext_fastq_gz.metadata['format'], 'fastq'
+    assert_equal 'fastq', new_fastq_attachment_ext_fastq_gz.metadata['format']
 
     new_fastq_attachment_ext_fq = @sample.attachments.build
     new_fastq_attachment_ext_fq.file.attach(io: Rails.root.join('test/fixtures/files/test_file_3.fq').open,
                                             filename: 'test_file_3.fq')
     new_fastq_attachment_ext_fq.save
-    assert_equal new_fastq_attachment_ext_fq.metadata['format'], 'fastq'
+    assert_equal 'fastq', new_fastq_attachment_ext_fq.metadata['format']
 
     new_fastq_attachment_ext_fq_gz = @sample.attachments.build
     new_fastq_attachment_ext_fq_gz.file.attach(io: Rails.root.join('test/fixtures/files/test_file_4.fq.gz').open,
                                                filename: 'test_file_4.fq.gz')
     new_fastq_attachment_ext_fq_gz.save
-    assert_equal new_fastq_attachment_ext_fq_gz.metadata['format'], 'fastq'
+    assert_equal 'fastq', new_fastq_attachment_ext_fq_gz.metadata['format']
   end
 
   test 'metadata fasta file types' do
     new_fasta_attachment_ext_fasta = @sample.attachments.build
     new_fasta_attachment_ext_fasta.file.attach(io: Rails.root.join('test/fixtures/files/test_file_5.fasta').open,
                                                filename: 'test_file_5.fasta')
     new_fasta_attachment_ext_fasta.save
-    assert_equal new_fasta_attachment_ext_fasta.metadata['format'], 'fasta'
-    assert_equal new_fasta_attachment_ext_fasta.metadata['type'], 'assembly'
-    assert_equal new_fasta_attachment_ext_fasta.fasta?, true
+    assert_equal 'fasta', new_fasta_attachment_ext_fasta.metadata['format']
+    assert_equal 'assembly', new_fasta_attachment_ext_fasta.metadata['type']
+    assert new_fasta_attachment_ext_fasta.fasta?
 
     new_fasta_attachment_ext_fasta_gz = @sample.attachments.build
     new_fasta_attachment_ext_fasta_gz.file.attach(io: Rails.root.join('test/fixtures/files/test_file_6.fasta.gz').open,
                                                   filename: 'test_file_6.fasta.gz')
     new_fasta_attachment_ext_fasta_gz.save
-    assert_equal new_fasta_attachment_ext_fasta_gz.metadata['format'], 'fasta'
-    assert_equal new_fasta_attachment_ext_fasta_gz.metadata['type'], 'assembly'
-    assert_equal new_fasta_attachment_ext_fasta_gz.fasta?, true
+    assert_equal 'fasta', new_fasta_attachment_ext_fasta_gz.metadata['format']
+    assert_equal 'assembly', new_fasta_attachment_ext_fasta_gz.metadata['type']
+    assert new_fasta_attachment_ext_fasta_gz.fasta?
 
     new_fasta_attachment_ext_fa = @sample.attachments.build
     new_fasta_attachment_ext_fa.file.attach(io: Rails.root.join('test/fixtures/files/test_file_7.fa').open,
                                             filename: 'test_file_7.fa')
     new_fasta_attachment_ext_fa.save
-    assert_equal new_fasta_attachment_ext_fa.metadata['format'], 'fasta'
-    assert_equal new_fasta_attachment_ext_fa.metadata['type'], 'assembly'
-    assert_equal new_fasta_attachment_ext_fa.fasta?, true
+    assert_equal 'fasta', new_fasta_attachment_ext_fa.metadata['format']
+    assert_equal 'assembly', new_fasta_attachment_ext_fa.metadata['type']
+    assert new_fasta_attachment_ext_fa.fasta?
 
     new_fasta_attachment_ext_fa_gz = @sample.attachments.build
     new_fasta_attachment_ext_fa_gz.file.attach(io: Rails.root.join('test/fixtures/files/test_file_8.fa.gz').open,
                                                filename: 'test_file_8.fa.gz')
     new_fasta_attachment_ext_fa_gz.save
-    assert_equal new_fasta_attachment_ext_fa_gz.metadata['format'], 'fasta'
-    assert_equal new_fasta_attachment_ext_fa_gz.metadata['type'], 'assembly'
-    assert_equal new_fasta_attachment_ext_fa_gz.fasta?, true
+    assert_equal 'fasta', new_fasta_attachment_ext_fa_gz.metadata['format']
+    assert_equal 'assembly', new_fasta_attachment_ext_fa_gz.metadata['type']
+    assert new_fasta_attachment_ext_fa_gz.fasta?
 
     new_fasta_attachment_ext_fna = @sample.attachments.build
     new_fasta_attachment_ext_fna.file.attach(io: Rails.root.join('test/fixtures/files/test_file_9.fna').open,
                                              filename: 'test_file_9.fna')
     new_fasta_attachment_ext_fna.save
-    assert_equal new_fasta_attachment_ext_fna.metadata['format'], 'fasta'
-    assert_equal new_fasta_attachment_ext_fna.metadata['type'], 'assembly'
-    assert_equal new_fasta_attachment_ext_fna.fasta?, true
+    assert_equal 'fasta', new_fasta_attachment_ext_fna.metadata['format']
+    assert_equal 'assembly', new_fasta_attachment_ext_fna.metadata['type']
+    assert new_fasta_attachment_ext_fna.fasta?
 
     new_fasta_attachment_ext_fna_gz = @sample.attachments.build
     new_fasta_attachment_ext_fna_gz.file.attach(io: Rails.root.join('test/fixtures/files/test_file_10.fna.gz').open,
                                                 filename: 'test_file_10.fna.gz')
     new_fasta_attachment_ext_fna_gz.save
-    assert_equal new_fasta_attachment_ext_fna_gz.metadata['format'], 'fasta'
-    assert_equal new_fasta_attachment_ext_fna_gz.metadata['type'], 'assembly'
-    assert_equal new_fasta_attachment_ext_fna_gz.fasta?, true
+    assert_equal 'fasta', new_fasta_attachment_ext_fna_gz.metadata['format']
+    assert_equal 'assembly', new_fasta_attachment_ext_fna_gz.metadata['type']
+    assert new_fasta_attachment_ext_fna_gz.fasta?
   end
 
   test 'metadata unknown file types' do
     new_unknown_attachment_ext_docx = @sample.attachments.build
     new_unknown_attachment_ext_docx.file.attach(io: Rails.root.join('test/fixtures/files/test_file_11.docx').open,
                                                 filename: 'test_file_11.docx')
     new_unknown_attachment_ext_docx.save
-    assert_equal new_unknown_attachment_ext_docx.metadata['format'], 'unknown'
+    assert_equal 'unknown', new_unknown_attachment_ext_docx.metadata['format']
 
     new_unknown_attachment_ext_pdf = @sample.attachments.build
     new_unknown_attachment_ext_pdf.file.attach(io: Rails.root.join('test/fixtures/files/test_file_12.pdf').open,
                                                filename: 'test_file_12.pdf')
     new_unknown_attachment_ext_pdf.save
-    assert_equal new_unknown_attachment_ext_pdf.metadata['format'], 'unknown'
+    assert_equal 'unknown', new_unknown_attachment_ext_pdf.metadata['format']
 
     new_unknown_attachment_ext_rtf = @sample.attachments.build
     new_unknown_attachment_ext_rtf.file.attach(io: Rails.root.join('test/fixtures/files/test_file_13.rtf').open,
                                                filename: 'test_file_13.rtf')
     new_unknown_attachment_ext_rtf.save
-    assert_equal new_unknown_attachment_ext_rtf.metadata['format'], 'unknown'
+    assert_equal 'unknown', new_unknown_attachment_ext_rtf.metadata['format']
 
     new_unknown_attachment_ext_txt = @sample.attachments.build
     new_unknown_attachment_ext_txt.file.attach(io: Rails.root.join('test/fixtures/files/test_file_14.txt').open,
                                                filename: 'test_file_14.txt')
     new_unknown_attachment_ext_txt.save
-    assert_equal new_unknown_attachment_ext_txt.metadata['format'], 'unknown'
+    assert_equal 'unknown', new_unknown_attachment_ext_txt.metadata['format']
   end
 
   test '#destroy does not destroy the ActiveStorage::Attachment' do

test/system/projects/samples_test.rb

@@ -97,8 +97,8 @@ class SamplesTest < ApplicationSystemTestCase
 
     test 'user with role >= Maintainer should be able to delete a file from a Sample' do
       visit namespace_project_sample_url(namespace_id: @namespace.path, project_id: @project.path, id: @sample1.id)
-      assert_selector 'button', text: I18n.t('projects.samples.attachments.attachment.delete'), count: 1
-      click_on I18n.t('projects.samples.attachments.attachment.delete')
+      assert_selector 'button', text: I18n.t('projects.samples.attachments.attachment.delete'), count: 2
+      click_on I18n.t('projects.samples.attachments.attachment.delete'), match: :first
 
       within('#turbo-confirm[open]') do
         click_button I18n.t(:'components.confirmation.confirm')