Merge branch 'main' into issue-1021-edit-two

app/controllers/works_controller.rb

@@ -289,13 +289,14 @@ def readme_select
   def readme_uploaded
     @work = Work.find(params[:id])
     @wizard = true
-    if readme_file_param
-      extension = File.extname(readme_file_param.original_filename)
-      content_type = readme_file_param.content_type
-      @work.pre_curation_uploads.attach(io: readme_file_param.to_io, filename: "README#{extension}", content_type: content_type)
-      @work.save!
+    readme = Readme.new(@work)
+    readme_error = readme.attach(readme_file_param)
+    if readme_error.nil?
+      redirect_to work_attachment_select_url(@work)
+    else
+      flash[:notice] = readme_error
+      redirect_to work_readme_select_url(@work)
     end
-    redirect_to work_attachment_select_url(@work)
   end
 
   private

app/models/readme.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+class Readme
+  attr_reader :work
+
+  def initialize(work)
+    @work = work
+  end
+
+  def attach(readme_file_param)
+    return "A README file is required!" if readme_file_param.blank? && blank?
+    return nil if readme_file_param.blank?
+
+    extension = File.extname(readme_file_param.original_filename)
+    if work.s3_query_service.upload_file(io: readme_file_param.to_io, filename: "README#{extension}")
+      nil
+    else
+      "An error uploading your README was encountered.  Please try again."
+    end
+  end
+
+  def blank?
+    s3_readme_idx.nil?
+  end
+
+    private
+
+      def s3_readme_idx
+        file_names = work.pre_curation_uploads_fast.map(&:filename_display)
+        file_names.find_index { |file_name| file_name.start_with?("README") }
+      end
+end

app/services/s3_query_service.rb

@@ -186,6 +186,15 @@ def create_directory
     client.put_object({ bucket: bucket_name, key: prefix, content_length: 0 })
   end
 
+  def upload_file(io:, filename:)
+    # upload file from io in a single request, may not exceed 5GB
+    md5_digest = md5(io: io)
+    client.put_object(bucket: bucket_name, key: "#{prefix}#{filename}", body: io, content_md5: md5_digest)
+  rescue Aws::S3::Errors::SignatureDoesNotMatch => e
+    Honeybadger.notify("Error Uploading file #{filename} for object: #{s3_address} Signature did not match! error: #{e}")
+    false
+  end
+
   private
 
     def model_uploads
@@ -234,5 +243,12 @@ def delete_files_and_directory(files)
       end
       delete_s3_object(model.s3_object_key)
     end
+
+    def md5(io:)
+      md5 = Digest::MD5.new
+      io.each(10_000) { |block| md5.update block }
+      io.rewind
+      md5.base64digest
+    end
 end
 # rubocop:enable Metrics/ClassLength

spec/controllers/works_controller_spec.rb

@@ -619,11 +619,8 @@
     end
 
     describe "#readme_uploaded" do
-      let(:bucket_url) do
-        "https://example-bucket.s3.amazonaws.com/"
-      end
-
-      let(:fake_s3_service) { stub_s3 }
+      let(:attach_status) { nil }
+      let(:fake_readme) { instance_double Readme, attach: attach_status, "blank?": true }
       let(:params) do
         {
           "_method" => "patch",
@@ -639,18 +636,25 @@
       end
 
       before do
-        fake_s3_service
-        stub_request(:put, /#{bucket_url}/).to_return(status: 200)
+        allow(Readme).to receive(:new).and_return(fake_readme)
         sign_in user
         post :readme_uploaded, params: params
       end
 
       it "redirects to file-upload" do
         expect(response.status).to be 302
-        work.reload
-        expect(work.pre_curation_uploads[0].key).to eq("#{work.doi}/#{work.id}/README.csv")
+        expect(fake_readme).to have_received(:attach)
         expect(response.location).to eq "http://test.host/works/#{work.id}/attachment-select"
       end
+
+      context "the upload encounters an error" do
+        let(:attach_status) { "An error occured" }
+
+        it "Stays on the same page" do
+          expect(response).to redirect_to(work_readme_select_path(work))
+          expect(controller.flash[:notice]).to eq("An error occured")
+        end
+      end
     end
 
     it "renders view to select the kind of attachment to use" do

spec/factories/s3_file.rb

@@ -11,5 +11,9 @@
       new(filename: filename, last_modified: last_modified, size: size,
           checksum: checksum, work: work)
     end
+
+    factory :s3_readme do
+      filename { "README.txt" }
+    end
   end
 end

spec/fixtures/files/readme_template.txt

@@ -0,0 +1,97 @@
+This readme file was generated on [YYYY-MM-DD] by [NAME]
+<help text in angle brackets should be deleted before finalizing your document>
+<[text in square brackets should be changed for your specific dataset]>
+
+
+GENERAL INFORMATION
+
+Title of Dataset: 
+
+<provide at least two contacts>
+Author/Principal Investigator Information
+Name: 
+ORCID:
+Institution: 
+Address: 
+Email: 
+
+Author/Associate or Co-investigator Information
+Name: 
+ORCID:
+Institution: 
+Address: 
+Email: 
+
+Author/Alternate Contact Information
+Name: 
+ORCID:
+Institution: 
+Address: 
+Email: 
+
+Date of data collection: <provide single date, range, or approximate date; suggested format YYYY-MM-DD> 
+
+Geographic location of data collection: <provide latitude, longiute, or city/region, State, Country>
+
+Information about funding sources that supported the collection of the data: 
+
+
+SHARING/ACCESS INFORMATION
+
+Licenses/restrictions placed on the data: 
+
+Links to publications that cite or use the data: 
+
+Links to other publicly accessible locations of the data: 
+
+Links/relationships to ancillary data sets: 
+
+Was data derived from another source?
+If yes, list source(s): 
+
+Recommended citation for this dataset: 
+
+
+DATA & FILE OVERVIEW
+
+File List: <list all files (or folders, as appropriate for dataset organization) contained in the dataset, with a brief description>
+
+Relationship between files, if important: 
+
+Additional related data collected that was not included in the current data package: 
+
+Are there multiple versions of the dataset?
+If yes, name of file(s) that was updated: 
+Why was the file updated? 
+When was the file updated? 
+
+
+METHODOLOGICAL INFORMATION
+
+Description of methods used for collection/generation of data: <include links or references to publications or other documentation containing experimental design or protocols used in data collection>
+
+Methods for processing the data: <describe how the submitted data were generated from the raw or collected data>
+
+Instrument- or software-specific information needed to interpret the data: <include full name and version of software, and any necessary packages or libraries needed to run scripts>
+
+Standards and calibration information, if appropriate: 
+
+Environmental/experimental conditions: 
+
+Describe any quality-assurance procedures performed on the data: 
+
+People involved with sample collection, processing, analysis and/or submission: 
+
+
+DATA-SPECIFIC INFORMATION FOR: [FILENAME]
+<repeat this section for each dataset, folder or file, as appropriate>
+
+Number of variables: 
+
+Number of cases/rows: 
+
+Variable List: <list variable name(s), description(s), unit(s) and value labels as appropriate for each>
+
+Missing data codes: <list code/symbol and definition>
+
+Specialized formats or other abbreviations used: 

spec/models/readme_spec.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+require "rails_helper"
+
+RSpec.describe Readme, type: :model do
+  let(:work) { FactoryBot.create :draft_work }
+  let(:s3_files) { [FactoryBot.build(:s3_file, work: work), FactoryBot.build(:s3_file, filename: "filename-2.txt", work: work)] }
+  let(:readme) { described_class.new(work) }
+  let(:fake_s3_service) { stub_s3 data: s3_files }
+
+  before do
+    fake_s3_service
+  end
+
+  describe "#blank?" do
+    it "Does not find a readme" do
+      expect(readme.blank?).to be_truthy
+    end
+
+    context "with a readme present" do
+      let(:s3_files) { [FactoryBot.build(:s3_file, work: work), FactoryBot.build(:s3_readme, work: work)] }
+      it "Does not find a readme" do
+        expect(readme.blank?).to be_falsey
+      end
+    end
+  end
+
+  describe "#attach" do
+    let(:uploaded_file) do
+      ActionDispatch::Http::UploadedFile.new({
+                                               filename: "orcid.csv",
+                                               type: "text/csv",
+                                               tempfile: File.new(Rails.root.join("spec", "fixtures", "files", "orcid.csv"))
+                                             })
+    end
+
+    it "attaches the file and renames to to README" do
+      expect(readme.attach(uploaded_file)).to be_nil
+      expect(fake_s3_service).to have_received(:upload_file).with(io: uploaded_file.to_io, filename: "README.csv")
+    end
+
+    context "when no uploaded file is sent" do
+      let(:uploaded_file) { nil }
+
+      it "returns an error message" do
+        expect(readme.attach(uploaded_file)).to eq("A README file is required!")
+        expect(fake_s3_service).not_to have_received(:upload_file)
+      end
+
+      context "when a readme is already present" do
+        let(:s3_files) { [FactoryBot.build(:s3_file, work: work), FactoryBot.build(:s3_readme, work: work)] }
+
+        it "returns no error message" do
+          expect(readme.attach(uploaded_file)).to be_nil
+          expect(fake_s3_service).not_to have_received(:upload_file)
+        end
+      end
+    end
+
+    context "there is an error with the upload" do
+      before do
+        allow(fake_s3_service).to receive(:upload_file).and_return(false)
+      end
+
+      it "returns an error message" do
+        expect(readme.attach(uploaded_file)).to eq("An error uploading your README was encountered.  Please try again.")
+        expect(fake_s3_service).to have_received(:upload_file).with(io: uploaded_file.to_io, filename: "README.csv")
+      end
+    end
+  end
+end

spec/services/s3_query_service_spec.rb

@@ -336,4 +336,30 @@
       assert_requested(:put, "https://example-bucket.s3.amazonaws.com/#{s3_query_service.prefix}", headers: { "Content-Length" => 0 })
     end
   end
+
+  describe "#upload_file" do
+    subject(:s3_query_service) { described_class.new(work) }
+    let(:filename) { "README.txt" }
+    let(:file) { File.open(Rails.root.join("spec", "fixtures", "files", "readme_template.txt")) }
+
+    before do
+      stub_request(:put, "https://example-bucket.s3.amazonaws.com/#{s3_query_service.prefix}#{filename}").to_return(status: 200)
+    end
+
+    it "uploads the readme" do
+      expect(s3_query_service.upload_file(io: file, filename: filename)).to be_truthy
+      assert_requested(:put, "https://example-bucket.s3.amazonaws.com/#{s3_query_service.prefix}#{filename}", headers: { "Content-Length" => 2852 })
+    end
+
+    context "when checksum does not match" do
+      before do
+        stub_request(:put, "https://example-bucket.s3.amazonaws.com/#{s3_query_service.prefix}#{filename}").to_raise(Aws::S3::Errors::SignatureDoesNotMatch.new(nil, nil))
+      end
+
+      it "detects the upload error" do
+        expect(s3_query_service.upload_file(io: file, filename: filename)).to be_falsey
+        assert_requested(:put, "https://example-bucket.s3.amazonaws.com/#{s3_query_service.prefix}#{filename}", headers: { "Content-Length" => 2852 })
+      end
+    end
+  end
 end

spec/support/s3_query_service_specs.rb

@@ -20,6 +20,7 @@ def mock_methods(fake_s3_query, data)
   allow(fake_s3_query).to receive(:delete_s3_object)
   allow(fake_s3_query).to receive(:create_directory)
   allow(fake_s3_query).to receive(:publish_files).and_return([])
+  allow(fake_s3_query).to receive(:upload_file).and_return(true)
 end
 
 def mock_bucket(bucket_url)

spec/system/authz_submitter_spec.rb

@@ -33,6 +33,10 @@
       click_on "Curator Controlled"
       expect(page).to have_content "Research Data"
       click_on "Save Work"
+      path = Rails.root.join("spec", "fixtures", "files", "orcid.csv")
+      attach_file(path) do
+        page.find("#patch_readme_file").click
+      end
       click_on "Continue"
       page.find(:xpath, "//input[@value='file_other']").choose
       click_on "Continue"

spec/system/authz_super_admin_spec.rb

@@ -31,6 +31,10 @@
       click_on "Curator Controlled"
       expect(page).to have_content "Research Data"
       click_on "Save Work"
+      path = Rails.root.join("spec", "fixtures", "files", "orcid.csv")
+      attach_file(path) do
+        page.find("#patch_readme_file").click
+      end
       click_on "Continue"
       page.find(:xpath, "//input[@value='file_other']").choose
       click_on "Continue"

spec/system/external_ids_spec.rb

@@ -22,6 +22,10 @@
     fill_in "description", with: "test description"
     select "GNU General Public License", from: "rights_identifier"
     click_on "Save Work"
+    path = Rails.root.join("spec", "fixtures", "files", "orcid.csv")
+    attach_file(path) do
+      page.find("#patch_readme_file").click
+    end
     click_on "Continue"
     click_on "Continue"
     click_on "Continue"