Archive download email tweaks

app/controllers/archive_controller.rb

@@ -24,7 +24,7 @@ def download_request
       else
         unless result[:message]
           Rails.logger.error("Message missing from #{@archive_file} result: #{result}")
-          result[:message] = 'Request failed.  Please request technical support.'
+          result[:message] = 'Request failed.  Please seek technical support.'
         end
         if result[:alert]
           redirect_back fallback_location: root_url, alert: result[:message]
@@ -33,20 +33,22 @@ def download_request
         end
       end
     else
-      @archive_file.log_denied_attempt!(update_only: true)
+      @archive_file.log_denied_attempt!(request_hash: request_metadata)
       redirect_back fallback_location: root_url, alert: @failure_description
     end
   end
 
   private
     def variable_params
-      params.permit(:collection, :object, :format, :request, :user_email, 'g-recaptcha-response'.to_sym, 'g-recaptcha-response-data'.to_sym => [:sda_request])
+      params.permit(:collection, :object, :format, :request, :user_email, :file_set_id, 'g-recaptcha-response'.to_sym, 'g-recaptcha-response-data'.to_sym => [:sda_request])
     end
 
     def set_variables
       @collection = params[:collection]
       @object = "#{variable_params[:object]}.#{variable_params[:format]}"
       @archive_file = ArchiveFile.new(collection: @collection, object: @object)
+      @user_email = params[:user_email]
+      @file_set_id = params[:file_set_id]
     end
 
     def authenticated_user?
@@ -64,8 +66,6 @@ def recaptcha_success?
     end
 
     def request_metadata
-      user_metadata = { time: Time.now, user: current_user&.email, user_email: params[:user_email] || current_user&.email }
-      user_metadata.merge!(recaptcha: recaptcha_reply || {}) if Settings.recaptcha.use?
-      user_metadata
+      user_metadata = { time: Time.now, user_email: @user_email, file_set_id: @file_set_id }
     end
 end

app/models/archive_file.rb

@@ -89,7 +89,7 @@ def get!(request_hash = {})
     end
   end
 
-  def log_denied_attempt!(request_hash = {}, update_only: true) #FIXME: reconsider
+  def log_denied_attempt!(request_hash: {}, update_only: true)
     create_or_update_job_file!(new_params: { denials: [request_hash] }, update_only: update_only)
   end
 
@@ -212,7 +212,7 @@ def archive_request(method: Net::HTTP::Head)
     def stage_request!(request_hash = {})
       Rails.logger.warn("Staging request for #{archive_url} made in status: #{status}") if staged? # log :staged_without_request cases
       if block_new_jobs?
-        log_denied_attempt!(request_hash.merge({ reason: 'block_new_jobs' })) # FIXME: update_only false or true here?
+        log_denied_attempt!(request_hash: request_hash.merge({ reason: 'block_new_jobs' }))
         { status: request_hash[:status], action: :throttled, message: display_status(:too_many_requests), alert: true }
       else
         create_or_update_job_file!(new_params: { requests: [request_hash.merge({ action: 'create_or_update_job_file!'})] })
@@ -247,7 +247,7 @@ def default_job_parameters
 
     def create_or_update_job_file!(new_params: {}, update_only: false)
       if job_file?
-        unless new_params # only update an existing file with new, non-default job parameters
+        unless new_params.any? # only update an existing file with new, non-default job parameters
           Rails.logger.warn("Ignoring duplicate call to create default job parameters file for #{archive_url}")
           return
         end

app/models/archive_file_worker.rb

@@ -109,12 +109,16 @@ def stage_file
   def download_file
     logger.info("Download initiated for #{yaml_path}")
     update_job_yaml({ status: :staged_after_request })
-
     if too_much_space_used?
       logger.warn("Disk quota exceeded.  Blocking file download until space is available.")
     else
       update_job_yaml({ transfer_started: Time.now })
-      system(curl_command(output: true))
+      begin
+        system(curl_command(output: true))
+      rescue => error
+        process_error("Aborting after curl error: #{error.inspect}")
+        return
+      end
       FileUtils.mv(path, file_path)
       update_job_yaml({ status: :local, transfer_completed: Time.now })
       email_requesters
@@ -125,14 +129,15 @@ def download_file
   def email_requesters
     if Settings.archive_api.send_email
       from = Deepblue::EmailHelper.contact_email
-      subject = "DataCORE file available for download: #{job_yaml[:filename]}"
-      body = "The archive file you requested, #{job_yaml[:filename]}, is now available for download."
-      job_yaml[:requests].map { |request| request[:user_email] }.each do |user_email|
+      filename = job_yaml[:filename]
+      subject = "DataCORE file available for download: #{filename}"
+      job_yaml[:requests].each do |request|
+        user_email = request[:user_email]
         if user_email.present?
           begin
             logger.info("Emailing user: #{user_email}")
-            Deepblue::EmailHelper.send_email(to: user_email, from: from, subject: subject , body: body, log: true)
-            # FIXME: store request origin when on fileset page?
+            Deepblue::EmailHelper.send_email(to: user_email, from: from, subject: subject , body: email_body_for(filename, request), log: true)
+            logger.info("Email sent successfully")
           rescue => error
             logger.warn("Error emailing user: #{user_email}")
           end
@@ -144,6 +149,30 @@ def email_requesters
     purge_user_emails
   end
 
+  def email_body_for(filename, request)
+    body = "The archive file you requested, #{filename}, is now available for download"
+    file_link = file_link_for(filename, request)
+    if file_link
+      body += ":\n#{file_link}"
+    else
+      body += "."
+    end
+    body
+  end
+
+  def file_link_for(filename, request)
+    file_set_id = request[:file_set_id]    
+    file_link = nil
+    if file_set_id && FileSet.where(id: file_set_id).any?
+      begin
+        file_link = Deepblue::EmailHelper.curation_concern_url(curation_concern: FileSet.find(file_set_id))
+      rescue => error
+        process_error("Error generating download link for #{file_set_id}: #{error.inspect}")
+      end
+    end
+    file_link
+  end
+
   def purge_user_emails
     sanitized_yaml = job_yaml.dup
     sanitized_yaml[:requests] = sanitized_yaml[:requests].map { |request_hash| sanitized_hash(request_hash) }

app/views/hyrax/file_sets/_show_archive_file.html.erb

@@ -28,6 +28,7 @@
                type="text" name="user_email"></input>
       </div>
     <% end %>
+    <input type="hidden" name="file_set_id" value="<%= @presenter.id %>"></input>
     <input type="submit" name='request' value="<%= @presenter.request_action %>"
            <%= 'disabled' unless @presenter.request_actionable? %>
                                         class="btn <%= @presenter.request_actionable? ? 'btn-primary' : 'btn-danger' %> <%= 'required' if @presenter.require_request_email? %>"></input>

config/locales/hyrax.en.yml

@@ -114,7 +114,7 @@ en:
     embargo:
       copy_visibility_flash_message: 'Updating file permissions. This may take a few minutes. You may want to refresh your browser or return to this record later to see the updated file permissions.'
     file_sets:
-      detail_header:        "File Deepblue Details"
+      detail_header:        "File Details"
       not_yet_characterized: "not yet characterized"
     footer:
       copyright_html: "<strong>Copyright &copy; 2017 Samvera</strong> Licensed under the Apache License, Version 2.0"