refactor download

app/controllers/submissions_debug_controller.rb

@@ -9,7 +9,7 @@ class SubmissionsDebugController < ApplicationController
   def index
     @submitter = Submitter.preload({ attachments_attachments: :blob },
                                    submission: { template: { documents_attachments: :blob } })
-                          .find_by(slug: params[:submitter_slug])
+                          .find_by(slug: params[:submit_form_slug])
 
     respond_to do |f|
       f.html do

app/controllers/submissions_download_controller.rb

@@ -1,52 +1,25 @@
 # frozen_string_literal: true
 
 class SubmissionsDownloadController < ApplicationController
-  skip_before_action :authenticate_user!
-  skip_authorization_check
-
-  TTL = 40.minutes
+  load_and_authorize_resource :submission
 
   def index
-    @submission = Submission.find_by!(slug: params[:submission_slug] || params[:submissions_preview_slug])
-
     last_submitter = @submission.submitters.where.not(completed_at: nil).order(:completed_at).last
 
-    Submissions::EnsureResultGenerated.call(last_submitter)
-
-    unless current_user_submitter?(last_submitter)
-      unless Submitters::AuthorizedForForm.call(last_submitter, current_user, request)
-        Rollbar.info("2FA download error: #{last_submitter.id}") if defined?(Rollbar)
-
-        return head :not_found
-      end
-
-      if last_submitter.completed_at < TTL.ago
-        Rollbar.info("TTL: #{last_submitter.id}") if defined?(Rollbar)
+    return head :not_found unless last_submitter
 
-        return head :not_found
-      end
-    end
+    Submissions::EnsureResultGenerated.call(last_submitter)
 
     if params[:combined] == 'true'
-      respond_with_combined(last_submitter)
-    else
-      render json: Submitters.build_document_urls(last_submitter)
-    end
-  end
-
-  private
-
-  def respond_with_combined(submitter)
-    url = Submitters.build_combined_url(submitter)
+      url = Submitters.build_combined_url(last_submitter)
 
       if url
         render json: [url]
       else
         head :not_found
       end
+    else
+      render json: Submitters.build_document_urls(last_submitter)
     end
-
-  def current_user_submitter?(submitter)
-    current_user && current_ability.can?(:read, submitter)
   end
 end

app/controllers/submissions_preview_controller.rb

@@ -38,7 +38,7 @@ class SubmissionsPreviewController < ApplicationController
 
     @submission = Submissions.preload_with_pages(@submission)
 
-    render 'submissions/show', layout: 'plain'
+    render 'submissions/show', layout: 'plain', locals: { is_preview: true }
   end
 
   def completed

app/controllers/submissions_preview_download_controller.rb

@@ -0,0 +1,64 @@
+# frozen_string_literal: true
+
+class SubmissionsPreviewDownloadController < ApplicationController
+  skip_before_action :authenticate_user!
+  skip_authorization_check
+
+  TTL = 40.minutes
+
+  def index
+    @submission = Submission.find_by!(slug: params[:submission_slug] || params[:submissions_preview_slug])
+
+    last_submitter = @submission.submitters.where.not(completed_at: nil).order(:completed_at).last
+
+    return head :not_found unless last_submitter
+
+    Submissions::EnsureResultGenerated.call(last_submitter)
+
+    unless current_user_submission?(@submission)
+      if use_2fa?(@submission)
+        Rollbar.info("2FA download error: #{last_submitter.id}") if defined?(Rollbar)
+
+        return head :not_found
+      end
+
+      if last_submitter.completed_at < TTL.ago
+        Rollbar.info("TTL: #{last_submitter.id}") if defined?(Rollbar)
+
+        return head :not_found
+      end
+    end
+
+    if params[:combined] == 'true'
+      respond_with_combined(last_submitter)
+    else
+      render json: Submitters.build_document_urls(last_submitter)
+    end
+  end
+
+  private
+
+  def respond_with_combined(submitter)
+    url = Submitters.build_combined_url(submitter)
+
+    if url
+      render json: [url]
+    else
+      head :not_found
+    end
+  end
+
+  def current_user_submission?(submission)
+    current_user && current_ability.can?(:read, submission)
+  end
+
+  def use_2fa?(submission)
+    return true if submission.submitters.any? do |e|
+      e.preferences['require_phone_2fa'] || e.preferences['require_email_2fa']
+    end
+    return true if submission.template&.preferences&.dig('require_phone_2fa')
+    return true if submission.template&.preferences&.dig('require_email_2fa')
+
+    false
+  end
+end

app/controllers/submit_form_completed_download_controller.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+class SubmitFormCompletedDownloadController < ApplicationController
+  skip_before_action :authenticate_user!
+  skip_authorization_check
+
+  TTL = 40.minutes
+  FILES_TTL = 5.minutes
+
+  def index
+    @submitter = Submitter.find_signed(params[:sig], purpose: :download_completed) if params[:sig].present?
+
+    signature_valid =
+      if @submitter&.slug == submitter_slug
+        true
+      else
+        @submitter = nil
+      end
+
+    @submitter ||= Submitter.find_by!(slug: submitter_slug)
+
+    Submissions::EnsureResultGenerated.call(@submitter)
+
+    last_submitter = @submitter.submission.submitters.where.not(completed_at: nil).order(:completed_at).last
+
+    return head :not_found unless last_submitter
+
+    Submissions::EnsureResultGenerated.call(last_submitter)
+
+    if !signature_valid && !current_user_submitter?(last_submitter)
+      unless Submitters::AuthorizedForForm.call(@submitter, current_user, request)
+        Rollbar.info("2FA download error: #{last_submitter.id}") if defined?(Rollbar)
+
+        return head :not_found
+      end
+
+      if last_submitter.completed_at < TTL.ago
+        Rollbar.info("TTL: #{last_submitter.id}") if defined?(Rollbar)
+
+        return head :not_found
+      end
+    end
+
+    if params[:combined] == 'true'
+      respond_with_combined(last_submitter)
+    else
+      render json: Submitters.build_document_urls(last_submitter)
+    end
+  end
+
+  private
+
+  def submitter_slug
+    params[:submit_form_slug] || params[:submitter_slug] || params[:submitter_id]
+  end
+
+  def respond_with_combined(submitter)
+    url = Submitters.build_combined_url(submitter)
+
+    if url
+      render json: [url]
+    else
+      head :not_found
+    end
+  end
+
+  def current_user_submitter?(submitter)
+    current_user && current_ability.can?(:read, submitter)
+  end
+end

app/controllers/submit_form_download_controller.rb

@@ -9,7 +9,7 @@ class SubmitFormDownloadController < ApplicationController
   def index
     @submitter = Submitter.find_by!(slug: params[:submit_form_slug])
 
-    return redirect_to submitter_download_index_path(@submitter.slug) if @submitter.completed_at?
+    return redirect_to submit_form_documents_path(@submitter.slug) if @submitter.completed_at?
 
     return head :unprocessable_content if @submitter.declined_at? ||
                                           @submitter.submission.archived_at? ||

app/controllers/submitters_download_controller.rb

@@ -1,66 +1,11 @@
 # frozen_string_literal: true
 
 class SubmittersDownloadController < ApplicationController
-  skip_before_action :authenticate_user!
-  skip_authorization_check
-
-  TTL = 40.minutes
-  FILES_TTL = 5.minutes
+  load_and_authorize_resource :submitter
 
   def index
-    @submitter = Submitter.find_signed(params[:sig], purpose: :download_completed) if params[:sig].present?
-
-    signature_valid =
-      if @submitter&.slug == params[:submitter_slug]
-        true
-      else
-        @submitter = nil
-      end
-
-    @submitter ||= Submitter.find_by!(slug: params[:submitter_slug])
-
     Submissions::EnsureResultGenerated.call(@submitter)
 
-    last_submitter = @submitter.submission.submitters.where.not(completed_at: nil).order(:completed_at).last
-
-    return head :not_found unless last_submitter
-
-    Submissions::EnsureResultGenerated.call(last_submitter)
-
-    if !signature_valid && !current_user_submitter?(last_submitter)
-      unless Submitters::AuthorizedForForm.call(@submitter, current_user, request)
-        Rollbar.info("2FA download error: #{last_submitter.id}") if defined?(Rollbar)
-
-        return head :not_found
-      end
-
-      if last_submitter.completed_at < TTL.ago
-        Rollbar.info("TTL: #{last_submitter.id}") if defined?(Rollbar)
-
-        return head :not_found
-      end
-    end
-
-    if params[:combined] == 'true'
-      respond_with_combined(last_submitter)
-    else
-      render json: Submitters.build_document_urls(last_submitter)
-    end
-  end
-
-  private
-
-  def respond_with_combined(submitter)
-    url = Submitters.build_combined_url(submitter)
-
-    if url
-      render json: [url]
-    else
-      head :not_found
-    end
-  end
-
-  def current_user_submitter?(submitter)
-    current_user && current_ability.can?(:read, submitter)
+    render json: Submitters.build_document_urls(@submitter)
   end
 end

app/controllers/submitters_send_email_controller.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class SubmittersSendEmailController < ApplicationController
-  load_and_authorize_resource :submitter, id_param: :submitter_slug, find_by: :slug
+  load_and_authorize_resource :submitter
 
   def create
     if Docuseal.multitenant? && SubmissionEvent.exists?(submitter: @submitter,

app/views/submissions/show.html.erb

@@ -1,4 +1,4 @@
-<% if params[:controller] == 'submissions_preview' %>
+<% if local_assigns[:is_preview] %>
   <%= render 'submissions/preview_tags' %>
 <% end %>
 <% font_scale = 1040.0 / PdfUtils::US_LETTER_W %>
@@ -30,10 +30,10 @@
           <span class="hidden md:block"><%= t('event_log') %></span>
         <% end %>
       <% end %>
-      <% if is_all_completed || @submission.submitters.to_a.any?(&:completed_at?) %>
+      <% if @submission.submitters.to_a.any?(&:completed_at?) %>
         <% if is_all_completed || !is_combined_enabled %>
           <div class="join relative">
-            <download-button data-src="<%= @sig_submitter ? submitter_download_index_path(@sig_submitter.slug, { sig: params[:sig], combined: is_combined_enabled }.compact) : submissions_preview_download_index_path(@submission.slug) %>" class="base-button <%= '!rounded-r-none !pr-2' if is_all_completed && !is_combined_enabled %>">
+            <download-button data-src="<%= local_assigns[:is_preview] ? (@sig_submitter ? submit_form_documents_path(@sig_submitter.slug, { sig: params[:sig], combined: is_combined_enabled }.compact_blank) : submissions_preview_download_index_path(@submission.slug, combined: is_combined_enabled.presence)) : submission_download_index_path(@submission, combined: is_combined_enabled.presence) %>" class="base-button <%= '!rounded-r-none !pr-2' if is_all_completed && !is_combined_enabled %>">
               <span class="flex items-center justify-center space-x-2" data-target="download-button.defaultButton">
                 <%= svg_icon('download', class: 'w-6 h-6') %>
                 <span class="hidden md:inline"><%= t('download') %></span>
@@ -52,7 +52,7 @@
                 </label>
                 <ul tabindex="0" class="z-10 dropdown-content p-2 mt-2 shadow menu text-base bg-base-100 rounded-box text-right">
                   <li>
-                    <download-button data-src="<%= @sig_submitter ? submitter_download_index_path(@sig_submitter.slug, { sig: params[:sig], combined: true }.compact) : submissions_preview_download_index_path(@submission.slug, combined: true) %>" class="flex items-center">
+                    <download-button data-src="<%= local_assigns[:is_preview] ? (@sig_submitter ? submit_form_documents_path(@sig_submitter.slug, { sig: params[:sig], combined: true }.compact) : submissions_preview_download_index_path(@submission.slug, combined: true)) : submission_download_index_path(@submission, combined: true) %>" class="flex items-center">
                       <span class="flex items-center justify-center space-x-2" data-target="download-button.defaultButton">
                         <%= svg_icon('download', class: 'w-6 h-6 flex-shrink-0') %>
                         <span class="whitespace-nowrap"><%= t('download_combined_pdf') %></span>
@@ -227,7 +227,7 @@
             <% end %>
             <% if signed_in? && submitter && submitter.email && !submitter.completed_at && !@submission.archived_at? && !@submission.template&.archived_at? && can?(:update, @submission) && Accounts.can_send_emails?(current_account) && !@submission.expired? && !submitter.declined_at? %>
               <div class="mt-2 mb-1">
-                <%= button_to button_title(title: submitter.sent_at? ? t('re_send_email') : t('send_email'), disabled_with: t('sending')), submitter_send_email_index_path(submitter_slug: submitter.slug), class: 'btn btn-sm btn-primary w-full' %>
+                <%= button_to button_title(title: submitter.sent_at? ? t('re_send_email') : t('send_email'), disabled_with: t('sending')), submitter_send_email_index_path(submitter), class: 'btn btn-sm btn-primary w-full' %>
               </div>
             <% end %>
             <% if signed_in? && submitter && submitter.phone && !submitter.completed_at && !@submission.archived_at? && !@submission.template&.archived_at? && can?(:update, @submission) && !@submission.expired? && !submitter.declined_at? %>

app/views/submit_form/completed.html.erb

@@ -30,7 +30,7 @@
         <% end %>
       <% end %>
       <% if @submitter.completed_at > 30.minutes.ago || (current_user && current_user.account.submitters.exists?(id: @submitter.id)) %>
-        <download-button data-src="<%= submitter_download_index_path(@submitter.slug) %>" class="base-button w-full">
+        <download-button data-src="<%= submit_form_documents_path(@submitter.slug) %>" class="base-button w-full">
           <span class="flex items-center justify-center space-x-2" data-target="download-button.defaultButton">
             <%= svg_icon('download', class: 'w-6 h-6') %>
             <span><%= t('download_documents') %></span>

app/views/templates/_submission.html.erb

@@ -66,7 +66,7 @@
         <% if submitter.completed_at? %>
           <div class="flex-1 md:flex-none">
             <div class="w-full md:w-fit">
-              <download-button data-src="<%= submitter_download_index_path(submitter.slug) %>" class="btn btn-sm btn-neutral text-white w-full md:w-36">
+              <download-button data-src="<%= submission_download_index_path(submission) %>" class="btn btn-sm btn-neutral text-white w-full md:w-36">
                 <span class="flex items-center justify-center space-x-1 <%= 'md:space-x-2' if t('download').length < 11 %>" data-target="download-button.defaultButton">
                   <%= svg_icon('download', class: 'w-5 h-5 stroke-2') %>
                   <span class="inline"><%= t('download') %></span>
@@ -154,7 +154,7 @@
                   </span>
                 </span>
                 <% if submitter.completed_at? && !is_submission_completed %>
-                  <download-button data-src="<%= submitter_download_index_path(submitter.slug) %>" class="absolute md:relative top-0 right-0 btn btn-xs btn-neutral text-white md:w-36 z-[1]">
+                  <download-button data-src="<%= submitter_download_index_path(submitter) %>" class="absolute md:relative top-0 right-0 btn btn-xs btn-neutral text-white md:w-36 z-[1]">
                     <span class="flex items-center justify-center space-x-1 md:space-x-2" data-target="download-button.defaultButton">
                       <%= svg_icon('download', class: 'w-4 h-4 stroke-2') %>
                       <span class="inline"><%= t('download') %></span>
@@ -190,7 +190,7 @@
           <% latest_submitter = submitters.select(&:completed_at?).max_by(&:completed_at) %>
           <div class="flex-1 md:flex-none">
             <div class="w-full md:w-fit">
-              <download-button data-src="<%= submitter_download_index_path(latest_submitter.slug) %>" class="btn btn-sm btn-neutral text-white w-full md:w-36 z-[1]">
+              <download-button data-src="<%= submission_download_index_path(submission) %>" class="btn btn-sm btn-neutral text-white w-full md:w-36 z-[1]">
                 <span class="flex items-center justify-center space-x-1 md:space-x-2" data-target="download-button.defaultButton">
                   <%= svg_icon('download', class: 'w-5 h-5 stroke-2') %>
                   <span class="inline"><%= t('download') %></span>

config/routes.rb

@@ -71,6 +71,7 @@ Rails.application.routes.draw do
   resources :submissions, only: %i[show destroy] do
     resources :unarchive, only: %i[create], controller: 'submissions_unarchive'
     resources :events, only: %i[index], controller: 'submission_events'
+    resources :download, only: %i[index], controller: 'submissions_download'
   end
   resources :submitters, only: %i[edit update]
   resources :console_redirect, only: %i[index]
@@ -144,9 +145,11 @@ Rails.application.routes.draw do
   resources :submit_form, only: %i[show update], path: 's', param: 'slug' do
     resources :values, only: %i[index], controller: 'submit_form_values'
     resources :download, only: %i[index], controller: 'submit_form_download'
+    resources :documents, only: %i[index], controller: 'submit_form_completed_download'
     resources :decline, only: %i[create], controller: 'submit_form_decline'
     resources :delegate, only: %i[create], controller: 'submit_form_delegate'
     resources :invite, only: %i[create], controller: 'submit_form_invite'
+    resources :debug, only: %i[index], controller: 'submissions_debug' if Rails.env.development?
     get :completed
     get :delegated
   end
@@ -155,15 +158,15 @@ Rails.application.routes.draw do
 
   resources :submissions_preview, only: %i[show], path: 'e', param: 'slug' do
     get :completed
-    resources :download, only: %i[index], controller: 'submissions_download'
+    resources :download, only: %i[index], controller: 'submissions_preview_download'
   end
 
   resources :send_submission_email, only: %i[create]
 
-  resources :submitters, only: %i[], param: 'slug' do
-    resources :download, only: %i[index], controller: 'submitters_download'
+  resources :submitters, only: %i[] do
+    resources :download, only: %i[index], controller: 'submitters_download', constraints: { submitter_id: /\d+/ }
+    resources :download, only: %i[index], controller: 'submit_form_completed_download'
     resources :send_email, only: %i[create], controller: 'submitters_send_email'
-    resources :debug, only: %i[index], controller: 'submissions_debug' if Rails.env.development?
   end
 
   scope '/settings', as: :settings do

lib/submitters.rb

@@ -260,11 +260,11 @@ module Submitters
     filename_format = AccountConfig.find_or_initialize_by(account_id: submitter.account_id,
                                                           key: AccountConfig::DOCUMENT_FILENAME_FORMAT_KEY)&.value
 
-    Submitters.select_attachments_for_download(submitter).map do |attachment|
+    select_attachments_for_download(submitter).map do |attachment|
       ActiveStorage::Blob.proxy_path(
         attachment.blob,
         expires_at: ttl.from_now.to_i,
-        filename: Submitters.build_document_filename(submitter, attachment.blob, filename_format)
+        filename: build_document_filename(submitter, attachment.blob, filename_format)
       )
     end
   end
@@ -282,7 +282,7 @@ module Submitters
     ActiveStorage::Blob.proxy_path(
       attachment.blob,
       expires_at: ttl.from_now.to_i,
-      filename: Submitters.build_document_filename(submitter, attachment.blob, filename_format)
+      filename: build_document_filename(submitter, attachment.blob, filename_format)
     )
   end