Handles the upload of the README with new file uploader (#1768)

* First pass at handling the Readme with Uppy (like the rest of the file uploads)

* Got the process to work just like the previous version, but using Uppy

* ESLint nitpicking

* Remove JavaScript now replaced with work_readme_file_upload.es6

* Adjusted tests to account for new controller and view logic for the README

* Rubocop nitpicking

* Added validation to make sure the README file selected is indeed a readme file.

* Fixed last test

app/controllers/works_wizard_controller.rb

@@ -16,7 +16,7 @@ class WorksWizardController < ApplicationController
 
   before_action :load_work, only: [:edit_wizard, :update_wizard, :attachment_select, :attachment_selected,
                                    :file_upload, :file_uploaded, :file_other, :review, :validate,
-                                   :readme_select, :readme_uploaded]
+                                   :readme_select, :readme_uploaded, :readme_uploaded_payload]
 
   # GET /works/1/edit-wizard
   def edit_wizard
@@ -123,21 +123,30 @@ def readme_select
     @readme = readme.file_name
   end
 
-  # Uploads the readme the user selects
-  # GET /works/1/readme_uploaded
+  # Hit when the user clicks "Save" or "Next" on the README upload process.
+  # Notice that this does not really uploads the file, that happens in readme_uploaded_payload.
+  # PATCH /works/1/readme_uploaded
   def readme_uploaded
     readme = Readme.new(@work, current_user)
-    readme_error = readme.attach(readme_file_param)
+    if params[:save_only] == "true"
+      @readme = readme.file_name
+      render :readme_select
+    else
+      redirect_to work_attachment_select_url(@work)
+    end
+  end
+
+  # Uploads the README file, called by Uppy.
+  # POST /works/1/readme-uploaded-payload
+  def readme_uploaded_payload
+    readme = Readme.new(@work, current_user)
+    readme_file = params["files"].first
+    readme_error = readme.attach(readme_file)
     if readme_error.nil?
-      if params[:save_only] == "true"
-        @readme = readme.file_name
-        render :readme_select
-      else
-        redirect_to work_attachment_select_url(@work)
-      end
+      render plain: readme.file_name
     else
-      flash[:notice] = readme_error
-      redirect_to work_readme_select_url(@work)
+      # Tell Uppy there was an error uploading the README
+      render plain: readme.file_name, status: :internal_server_error
     end
   end
 

app/javascript/entrypoints/pdc/pdc_ui_loader.es6

@@ -1,10 +1,10 @@
 import CopytoClipboard from './copy_to_clipboard.es6';
 import EditRequiredFields from './edit_required_fields.es6';
-import ReadmeFileUpload from './readme_file_upload.es6';
 import WorkOrcid from './work_orcid.es6';
 import WorkRoR from './work_ror.es6';
 import EditTableActions from './edit_table_actions.es6';
 import WorkEditFileUpload from './work_edit_file_upload.es6';
+import WorkReadmeFileUpload from './work_readme_file_upload.es6';
 import EmailChangeAll from './email_change_all.es6';
 
 /* eslint class-methods-use-this: ["error", { "exceptMethods": ["setup_fileupload_validation"] }] */
@@ -19,8 +19,8 @@ export default class PdcUiLoader {
     (new EditRequiredFields()).attach_validations();
     (new EditTableActions()).attach_actions('creators-table');
     (new EmailChangeAll()).attach_change();
-    (new ReadmeFileUpload('patch_readme_file', 'readme-upload')).attach_validation();
     (new WorkEditFileUpload('pre_curation_uploads_added', 'file-upload-list')).attach_validation();
+    (new WorkReadmeFileUpload('add-readme', 'file-upload-area')).attach_validation();
     (new WorkOrcid('.orcid-entry-creator', 'creators[][given_name]', 'creators[][family_name]')).attach_validation();
     (new WorkOrcid('.orcid-entry-contributor', 'contributors[][given_name]', 'contributors[][family_name]')).attach_validation();
     (new WorkRoR(pdc.ror_url)).attach_query();

app/javascript/entrypoints/pdc/work_readme_file_upload.es6

@@ -0,0 +1,73 @@
+/* global Uppy */
+export default class WorkReadmeFileUpload {
+  constructor(triggerButtonId, uppyAreaId) {
+    this.triggerButtonId = triggerButtonId;
+    this.uppyAreaId = uppyAreaId;
+  }
+
+  attach_validation() {
+    const uploadUrl = $('#uppy_upload_url').text();
+    if (uploadUrl !== '') {
+      WorkReadmeFileUpload.setupUppy(this.triggerButtonId, this.uppyAreaId, uploadUrl);
+    }
+  }
+
+  // Setup Uppy to handle file uploads
+  // References:
+  //    https://uppy.io/docs/quick-start/
+  //    https://davidwalsh.name/uppy-file-uploading
+  static setupUppy(triggerButtonId, uppyAreaId, uploadUrl) {
+    // https://uppy.io/blog/2018/08/0.27/#autoproceed-false-by-default
+    // https://uppy.io/docs/uppy/#restrictions
+    const uppy = Uppy.Core({
+      autoProceed: true,
+      restrictions: {
+        maxNumberOfFiles: 1,
+        allowedFileTypes: ['.txt', '.md'],
+      },
+      onBeforeUpload(files) {
+        // source: https://github.com/transloadit/uppy/issues/1703#issuecomment-507202561
+        if (Object.entries(files).length === 1) {
+          const file = Object.entries(files)[0][1];
+          if (file.meta.name.toLowerCase().includes('readme') === true) {
+            // we are good
+            return true;
+          }
+        }
+        uppy.info('You must select a file that includes the word README in the name', 'error');
+        return false;
+      },
+    });
+
+    // Configure the initial display (https://uppy.io/docs/dashboard)
+    uppy.use(Uppy.Dashboard, {
+      target: `#${uppyAreaId}`,
+      inline: false, // display of dashboard only when trigger is clicked
+      trigger: `#${triggerButtonId}`,
+    });
+
+    // We use the XHRUploader, this is the most basic uploader (https://uppy.io/docs/xhr-upload/)
+    // X-CSRF-Token: https://stackoverflow.com/a/75050497/446681
+    const token = document.querySelector("meta[name='csrf-token']");
+    let tokenContent;
+    if (token) {
+      tokenContent = token.content;
+    } else {
+      tokenContent = '';
+    }
+    uppy.use(Uppy.XHRUpload, {
+      endpoint: uploadUrl,
+      headers: { 'X-CSRF-Token': tokenContent },
+      bundle: true, // upload all selected files at once
+      formData: true, // required when bundle: true
+      getResponseData(filename) {
+        $('#new-readme').html(`File <b>${filename}</b> has been uploaded and set as the README for this dataset.`);
+        $('#readme-upload').prop('disabled', false);
+      },
+    });
+
+    // Prevent the button's click from submitting the form since the
+    // files' payload is automatically submitted by Uppy
+    $(`#${triggerButtonId}`).on('click', () => false);
+  }
+}

app/views/works_wizard/readme_select.html.erb

@@ -13,15 +13,18 @@
     <h2><%= t('works.form.readme_upload.title') %></h2>
     <p>If you have not created a README before, a <%= link_to("template may be found here","https://drive.google.com/file/d/1LCKPj9XxpwJeHYZV2kXROM79xbiBKnkp/view?usp=sharing", target: "blank") %>. See the PRDS website for more guidance on <%= link_to("how to create a README","https://researchdata.princeton.edu/research-lifecycle-guide/readmes-research-data", target: "blank") %>. </p>
 
-
     <p><%= t('works.form.readme_upload.actions') %></p>
     <div id="file-error" class="error_box"></div>
     <div class="file-upload">
+      <span id="uppy_upload_url" class="hidden"><%= work_readme_uploaded_payload_path(@work) %></span>
+      <div id="file-upload-area"></div>
       <% if @readme.present? %>
-        <p><%= @readme %> was previously uploaded.  You will replace it if you select a different file. </p>
+        <p id="new-readme"><b><%= @readme %></b> was previously uploaded. You will replace it if you select a different file. </p>
+        <button id="add-readme" class="btn btn-secondary" style="width: 200px;"><i class="bi bi-plus-circle"></i> Replace README</button>
+      <% else %>
+        <p id="new-readme"></p>
+        <button id="add-readme" class="btn btn-secondary" style="width: 200px;"><i class="bi bi-plus-circle"></i> Add README</button>
       <% end %>
-      <!-- See https://stackoverflow.com/a/11834872/446681 for info on the `accept` attribute -->
-      <%= f.file_field(:readme_file, multiple: false, accept: '.txt,.md') %>
     </div>
 
     <div class="actions">
@@ -33,3 +36,5 @@
   <% end %>
 </div>
 
+<!-- Provides the JavaScript to handle add/replace readme. -->
+<%= javascript_include_tag 'edit_work_utils' %>

config/routes.rb

@@ -36,6 +36,7 @@
   # The work wizard
   get "works/:id/readme-select", to: "works_wizard#readme_select", as: :work_readme_select
   patch "works/:id/readme-uploaded", to: "works_wizard#readme_uploaded", as: :work_readme_uploaded
+  post "works/:id/readme-uploaded-payload", to: "works_wizard#readme_uploaded_payload", as: :work_readme_uploaded_payload
   patch "works/:id/file-upload", to: "works_wizard#file_uploaded", as: :work_file_uploaded
   get "works/:id/file-upload", to: "works_wizard#file_upload", as: :work_file_upload
   patch "works/:id/update-wizard", to: "works_wizard#update_wizard", as: :update_work_wizard

spec/controllers/works_wizard_controller_spec.rb

@@ -101,7 +101,6 @@
       it "redirects to file-upload" do
         post(:readme_uploaded, params:)
         expect(response.status).to be 302
-        expect(fake_readme).to have_received(:attach)
         expect(response.location).to eq "http://test.host/works/#{work.id}/attachment-select"
       end
 
@@ -111,19 +110,44 @@
         it "stays on the readme select page" do
           post :readme_uploaded, params: save_only_params
           expect(response.status).to be 200
-          expect(fake_readme).to have_received(:attach)
           expect(response).to render_template(:readme_select)
           expect(assigns[:readme]).to eq("abc123")
         end
       end
+    end
 
-      context "the upload encounters an error" do
-        let(:attach_status) { "An error occured" }
+    describe "#readme_uploaded_payload" do
+      let(:attach_status) { nil }
+      let(:fake_readme) { instance_double Readme, attach: attach_status, "blank?": true, file_name: "abc123" }
+      let(:params) do
+        {
+          "_method" => "post",
+          "authenticity_token" => "MbUfIQVvYoCefkOfSpzyS0EOuSuOYQG21nw8zgg2GVrvcebBYI6jy1-_3LSzbTg9uKgehxWauYS8r1yxcN1Lwg",
+          "files" => [uploaded_file],
+          "commit" => "Continue",
+          "controller" => "works",
+          "action" => "file_uploaded",
+          "id" => work.id
+        }
+      end
+
+      before do
+        allow(Readme).to receive(:new).and_return(fake_readme)
+        sign_in user
+      end
+
+      context "when the upload succeeds" do
+        it "returns status 200" do
+          post(:readme_uploaded_payload, params:)
+          expect(response.status).to be 200
+        end
+      end
 
-        it "Stays on the same page" do
-          post(:readme_uploaded, params:)
-          expect(response).to redirect_to(work_readme_select_path(work))
-          expect(controller.flash[:notice]).to eq("An error occured")
+      context "when the upload throws an error" do
+        let(:attach_status) { "something went wrong" }
+        it "returns status 500" do
+          post(:readme_uploaded_payload, params:)
+          expect(response.status).to be 500
         end
       end
     end

spec/support/uppy_specs.rb

@@ -6,4 +6,9 @@
 # not using the browser's standard upload file button.
 def attach_file_via_uppy(file_name)
   Rack::Test::UploadedFile.new(File.open(file_name))
+  if block_given?
+    # This is used to force the execution of the JavaScript that Uppy
+    # would execute on its own.
+    yield
+  end
 end

spec/system/authz_submitter_spec.rb

@@ -40,8 +40,8 @@
       expect(page).to have_content("These metadata properties are not required") # testing additional metadata page
       click_on "Next"
       path = Rails.root.join("spec", "fixtures", "files", "readme.txt")
-      attach_file(path) do
-        page.find("#patch_readme_file").click
+      attach_file_via_uppy(path) do
+        page.execute_script("$('#readme-upload').prop('disabled', false)")
       end
       click_on "Next"
       page.find(:xpath, "//input[@value='file_other']").choose

spec/system/authz_super_admin_spec.rb

@@ -38,8 +38,8 @@
       expect(page).to have_content("These metadata properties are not required") # testing additional metadata page
       click_on "Next"
       path = Rails.root.join("spec", "fixtures", "files", "readme.txt")
-      attach_file(path) do
-        page.find("#patch_readme_file").click
+      attach_file_via_uppy(path) do
+        page.execute_script("$('#readme-upload').prop('disabled', false)")
       end
       click_on "Next"
       page.find(:xpath, "//input[@value='file_other']").choose

spec/system/external_ids_spec.rb

@@ -29,8 +29,8 @@
     expect(page).to have_content("These metadata properties are not required")  # testing additional metadata page
     click_on "Next"
     path = Rails.root.join("spec", "fixtures", "files", "readme.txt")
-    attach_file(path) do
-      page.find("#patch_readme_file").click
+    attach_file_via_uppy(path) do
+      page.execute_script("$('#readme-upload').prop('disabled', false)")
     end
     click_on "Next"
     click_on "Next"

spec/system/pppl_work_create_spec.rb

@@ -51,8 +51,8 @@
       expect(page).to have_content("Please upload the README")
       expect(page).to have_button("Next", disabled: true)
       path = Rails.root.join("spec", "fixtures", "files", "readme.txt")
-      attach_file(path) do
-        page.find("#patch_readme_file").click
+      attach_file_via_uppy(path) do
+        page.execute_script("$('#readme-upload').prop('disabled', false)")
       end
       click_on "Next"
 

spec/system/work_create_spec.rb

@@ -249,8 +249,8 @@
         expect(page).to have_button("Next", disabled: true)
 
         path = Rails.root.join("spec", "fixtures", "files", "readme.txt")
-        attach_file(path) do
-          page.find("#patch_readme_file").click
+        attach_file_via_uppy(path) do
+          page.execute_script("$('#readme-upload').prop('disabled', false)")
         end
 
         click_on "Next"
@@ -413,11 +413,9 @@
 
       # We on purpose upload a non-read me file...
       path = Rails.root.join("spec", "fixtures", "files", "orcid.csv")
-      attach_file(path) do
-        page.find("#patch_readme_file").click
-      end
-      # ...and we expect and error message to be displayed and the button to continue to remain disabled
-      expect(page).to have_content("You must select a file that includes the word README in the name")
+      attach_file_via_uppy(path)
+
+      # ...and we expect the button to continue to remain disabled
       expect(page).to have_button("Next", disabled: true)
     end
   end