Updated interface in Work Edit page to allow users to add and delete …

…files (#1042)

* Got the add files working all the way and the UI portion of the delete.

* Added code to  handle deletes

* Minor tweaks

* Fixes work_upload_edit_service_specs

* Updated controller tests to pass the new parameters that the form uses

* Fixed a few more tests

* Removed test that validated nothing

* Handles case where work.id is still nil during creation

* Updated more tests

* Fixed last tests.

* Remove unused view

* Fixes issue with files with spaces on the new delete functionality

app/assets/javascripts/edit_work_utils.js

@@ -399,6 +399,48 @@ $(() => {
     return false;
   });
 
+  // Handles delete of files
+  $(document).on('click', '.delete-file', (el) => {
+    // Mark the file as deleted in the UI.
+    // Relevant DataTables documentation for individual row manipulation:
+    //   https://datatables.net/reference/api/data()
+    //   https://datatables.net/reference/type/row-selector
+    const safeId = $(el.target).data('safe_id');
+    const rowId = `#${safeId}`;
+    const filesTable = $('#files-table').DataTable();
+    const row = filesTable.row(rowId).data();
+    row.filename_display = `* #${row.filename_display}`;
+    filesTable.row(rowId).invalidate();
+
+    // Keep track of the deleted file, we do this via a hidden textbox with
+    // the name of the file to delete. This information will be submitted
+    // to the server when the user hits save.
+    const deleteCount = incrementCounter('#deleted_files_count');
+    const sequenceId = `deleted_file_${deleteCount}`;
+    const deletedFileHtml = `<input class="hidden deleted-file-tracker" type="text" id="${sequenceId}" name="work[${sequenceId}]" value="${row.filename}" />`;
+    $('.work-form').append(deletedFileHtml);
+    return false;
+  });
+
+  // Handles undo delete of files
+  $(document).on('click', '.undo-delete-file', (el) => {
+    const safeId = $(el.target).data('safe_id');
+    const rowId = `#${safeId}`;
+    // Mark the file as not deleted in the UI.
+    const filesTable = $('#files-table').DataTable();
+    const row = filesTable.row(rowId).data();
+    row.filename_display = row.filename_display.replace('* ', '');
+    filesTable.row(rowId).invalidate();
+
+    // Remove the filename from the list of values we submit to the server.
+    $('.deleted-file-tracker').each((_index, element) => {
+      if (element.value === row.filename) {
+        element.value = ''; // eslint-disable-line no-param-reassign
+      }
+    });
+    return false;
+  });
+
   if ($('.creator-data').length === 0) {
     // Add an empty creator for the use to fill it out
     const num = incrementCounter('#creator_count');

app/controllers/works_controller.rb

@@ -40,6 +40,9 @@ def new
   end
 
   def create
+    # TODO: We need to process files submitted by the user in this method.
+    # We currently do not and therefore there are not saved to the work.
+    # See https://github.com/pulibrary/pdc_describe/issues/1041
     @work = Work.new(created_by_user_id: current_user.id, collection_id: params_collection_id, user_entered_doi: params["doi"].present?)
     @work.resource = FormToResourceService.convert(params, @work)
     if @work.valid?
@@ -81,8 +84,14 @@ def show
   end
 
   def file_list
-    @work = Work.find(params[:id])
-    render json: @work.uploads
+    if params[:id] == "NONE"
+      # This is a special case when we render the file list for a work being created
+      # (i.e. it does not have an id just yet)
+      render json: []
+    else
+      @work = Work.find(params[:id])
+      render json: @work.uploads
+    end
   end
 
   def resolve_doi
@@ -305,7 +314,7 @@ def patch_params
     def pre_curation_uploads_param
       return if patch_params.nil?
 
-      patch_params[:pre_curation_uploads]
+      patch_params[:pre_curation_uploads_added]
     end
 
     def readme_file_param
@@ -349,7 +358,7 @@ def update_work
 
         return head(:forbidden) unless deleted_uploads.empty?
       else
-        @work = upload_service.update_precurated_file_list(work_params)
+        @work = upload_service.update_precurated_file_list(added_files_param, deleted_files_param)
       end
 
       process_updates
@@ -362,6 +371,15 @@ def update_params
       }
     end
 
+    def added_files_param
+      Array(work_params[:pre_curation_uploads_added])
+    end
+
+    def deleted_files_param
+      deleted_count = (work_params["deleted_files_count"] || "0").to_i
+      (1..deleted_count).map { |i| work_params["deleted_file_#{i}"] }.select(&:present?)
+    end
+
     def process_updates
       resource_before = @work.resource
       if @work.update(update_params)

app/javascript/entrypoints/pdc/pdc_ui_loader.es6

@@ -11,7 +11,7 @@ export default class PdcUiLoader {
 
   setup_fileupload_validation() {
     (new MaximumFileUpload('patch_pre_curation_uploads', 'file-upload')).attach_validation();
-    (new MaximumFileUpload('pre_curation_uploads', 'btn-submit')).attach_validation();
+    (new MaximumFileUpload('pre_curation_uploads_added', 'btn-submit')).attach_validation();
     (new CopytoClipboard()).attach_copy();
     (new EditRequiredFields()).attach_validations();
   }

app/models/s3_file.rb

@@ -2,11 +2,12 @@
 class S3File
   include Rails.application.routes.url_helpers
 
-  attr_accessor :filename, :last_modified, :size, :checksum, :url, :filename_display, :last_modified_display
+  attr_accessor :safe_id, :filename, :last_modified, :size, :checksum, :url, :filename_display, :last_modified_display
   alias key filename
   alias id filename
 
   def initialize(filename:, last_modified:, size:, checksum:, work:)
+    @safe_id = filename_as_id(filename)
     @filename = filename
     @filename_display = filename_short(work, filename)
     @last_modified = last_modified
@@ -63,4 +64,10 @@ def filename_short(work, filename)
         filename
       end
     end
+
+    def filename_as_id(filename)
+      # The full filename and path but only with alphanumeric characters
+      # everything else becomes as dash.
+      filename.gsub(/[^A-Za-z\d]/, "-")
+    end
 end

app/services/work_uploads_edit_service.rb

@@ -9,21 +9,16 @@ def initialize(work, current_user)
     @changes = []
   end
 
-  def update_precurated_file_list(work_params)
-    if work_params.key?(:deleted_uploads) || work_params.key?(:pre_curation_uploads) || work_params.key?(:replaced_uploads)
-      if work_params.key?(:deleted_uploads)
-        delete_pre_curation_uploads(work_params[:deleted_uploads])
-      elsif work_params.key?(:pre_curation_uploads)
-        update_uploads(work_params)
-      elsif work_params.key?(:replaced_uploads)
-        replace_uploads(work_params[:replaced_uploads])
-      end
+  def update_precurated_file_list(added_files, deleted_files)
+    delete_uploads(deleted_files)
+    add_uploads(added_files)
+    if @changes.count > 0
       work.log_file_changes(@changes, @current_user.id)
       s3_service.client_s3_files(reload: true)
       work.reload # reload the work to pick up the changes in the attachments
-    else # no changes in the parameters, just return the original work
-      work
     end
+
+    work
   end
 
   def find_post_curation_uploads(upload_keys: [])
@@ -33,37 +28,15 @@ def find_post_curation_uploads(upload_keys: [])
 
   private
 
-    def replace_uploads(replaced_uploads_params)
-      replaced_uploads_params.keys.each do |key|
-        s3_service.delete_s3_object(key)
-        track_change(:deleted, key)
-        new_upload = replaced_uploads_params[key]
-        work.pre_curation_uploads.attach(new_upload)
-        track_change(:added, new_upload.original_filename)
-      end
-    end
-
-    def delete_pre_curation_uploads(deleted_uploads_params)
-      deleted_uploads_params.each do |delete_s3|
-        s3_service.delete_s3_object(delete_s3.first) if delete_s3.last == "1"
-        track_change(:deleted, delete_s3.first)
+    def delete_uploads(deleted_files)
+      deleted_files.each do |filename|
+        s3_service.delete_s3_object(filename)
+        track_change(:deleted, filename)
       end
     end
 
-    def update_uploads(work_params)
-      # delete all existing uploads...
-      work.pre_curation_uploads_fast.each do |existing_upload|
-        track_change(:deleted, existing_upload.filename.to_s)
-        s3_service.delete_s3_object(existing_upload.key)
-      end
-
-      # TODO: can we remove this reload now???  May be causing issues with mocking
-      # ...reload the work to pick up the changes in the attachments
-      work.reload
-
-      # ...and then and then track the ones indicated in the parameters
-      # todo - How do we know what has been attached in the background?
-      Array(work_params[:pre_curation_uploads]).each do |new_upload|
+    def add_uploads(added_files)
+      added_files.each do |new_upload|
         work.pre_curation_uploads.attach(new_upload)
         track_change(:added, new_upload.original_filename)
       end

app/views/works/_form.html.erb

@@ -43,10 +43,18 @@
           <% if @work.approved? %>
             <section class="uploads">
               <h2 class="h2"><%= t('works.uploads.post_curation.heading') %></h2>
-              <%= render(partial: 'works/s3_resources') %>
+              <%= render(partial: 'works/s3_resources', locals: { edit_mode: false }) %>
             </section>
           <% else %>
-            <%= render(partial: 'works/uploads_form', locals: { form: form }) %>
+            <section class="uploads">
+              <h2 class="h2"><%= t('works.uploads.post_curation.heading') %></h2>
+              <%= render(partial: 'works/s3_resources', locals: { edit_mode: true, form: form }) %>
+            </section>
+            <div class="container-fluid deposit-uploads">
+              <div id="file-error" class="error_box"></div>
+              <%= form.label("Add more files", for: :pre_curation_uploads_added) %>
+              <%= form.file_field(:pre_curation_uploads_added, id: "pre_curation_uploads_added", multiple: true) %>
+            </div>
           <% end %>
         </div>
       <% end %>

app/views/works/_form_hidden_fields.html.erb

@@ -9,7 +9,8 @@
   <input type="text" id="creator_count" name="creator_count" value="<%= @work&.resource&.creators&.count || 0%>" class="hidden" />
   <input type="text" id="contributor_count" name="contributor_count" value="<%= @work&.resource&.individual_contributors&.count || 0%>" class="hidden" />
   <input type="text" id="related_object_count" name="related_object_count" value="<%= @work&.resource&.related_objects&.count || 0%>" class="hidden" />
-
+  <input type="text" id="deleted_files_count" name="work[deleted_files_count]" value="0" class="hidden" />
+
   <% if @wizard_mode || ( @work&.doi&.present? &&  @work.persisted?) %>
     <input type="text" id="doi" name="doi" value="<%= @work&.resource&.doi %>" class="hidden" />
   <% end %>

app/views/works/_s3_resources.html.erb

@@ -11,6 +11,7 @@
               <th scope="col" nowrap="nowrap"><span>Filename</span></th>
               <th scope="col"><span>Last Modified</span></th>
               <th scope="col"><span>Size</span></th>
+              <th scope="col"><span></span></th>
             </tr>
           </thead>
           <tbody>
@@ -19,6 +20,7 @@
                 <td><span><%= t('works.form.curation_uploads.empty') %></span></td>
                 <td><span></span></td>
                 <td><span></span></td>
+                <td><span></span></td>
               </tr>
             <% end %>
           </tbody>
@@ -31,33 +33,45 @@
 
 <script type="text/javascript">
   $(function() {
-    // This is needed so that we can swap the URL during testing
-    // (for more info see external_identifier_specs.rb)
-    var fileListUrl = '<%= work_file_list_path(@work) %>';
+    // work.id is nil when the form is rendered during work creation and the work has not been saved
+    var fileListUrl = '<%= work_file_list_path(@work.id || "NONE") %>';
+    var isEditMode = <%= edit_mode %>;
 
     // Wire DataTable for the file list.
     // Related documentation
     //   AJAX loading: https://datatables.net/manual/ajax
     //   Column display configuration: https://datatables.net/examples/advanced_init/column_render.html
-    $('#files-table').DataTable({
+    //   Row Id: https://datatables.net/reference/option/rowId
+    var filesTable = $('#files-table').DataTable({
+      select: true,
       ajax: {
         url: fileListUrl,
         dataSrc: ''
       },
+      rowId: 'safe_id',
       columns: [
         { data: 'filename' },
         { data: 'last_modified_display' },
-        { data: 'size' }
+        { data: 'size' },
+        { data: 'filename' }
       ],
       columnDefs: [
         {
           render: function (data, type, row) {
             // filename
-            var html = `<span>
-              <i class="bi bi-file-arrow-down"></i>
-              <a href="<%= @work.id %>/download?filename=${data}">${row.filename_display}</a>
-              </span>`;
-            return html;
+            if (type == "display") {
+              var html;
+              if (row.filename_display.startsWith("*")) {
+                html = `<span><s>${row.filename_display.substring(1)}</s></span>`;
+              } else {
+                html = `<span>
+                  <i class="bi bi-file-arrow-down"></i>
+                  <a href="<%= @work.id %>/download?filename=${data}">${row.filename_display}</a>
+                </span>`;
+              }
+              return html;
+            }
+            return data;
           },
           targets: 0,
         },
@@ -81,6 +95,33 @@
             return data;
           },
           targets: 2,
+          className: 'dt-right'
+        },
+        {
+          render: function (data, type, row) {
+            // delete icon
+            var html = null;
+            if (type == "display") {
+              if (row.filename_display.startsWith("*")) {
+                html = `<span>
+                  <a class="undo-delete-file" data-safe_id=${row.safe_id} data-filename=${row.filename} data-filename_display=${row.filename_display} href="#">
+                    Undo delete
+                  </a>
+                </span>`;
+              } else {
+                html = `<span>
+                  <a class="delete-file" data-safe_id=${row.safe_id} data-filename=${row.filename} data-filename_display=${row.filename_display} href="#" id="delete-file-${row.safe_id}">
+                    Delete file
+                  </a>
+                </span>`;
+              }
+            }
+            return html;
+          },
+          targets: 3,
+          className: 'dt-right',
+          sortable: false,
+          visible: isEditMode
         }
       ]
     });