diff --git a/app/jobs/process_submitter_completion_job.rb b/app/jobs/process_submitter_completion_job.rb index c20a6c28..26367c5f 100644 --- a/app/jobs/process_submitter_completion_job.rb +++ b/app/jobs/process_submitter_completion_job.rb @@ -17,7 +17,7 @@ class ProcessSubmitterCompletionJob Submissions::GenerateCombinedAttachment.call(submitter) end - Submissions::GenerateAuditTrail.call(submitter.submission) + maybe_regenerate_audit_trail(submitter) enqueue_completed_emails(submitter) end @@ -153,4 +153,34 @@ class ProcessSubmitterCompletionJob Submitters.send_signature_requests([next_submitter]) end + + def maybe_regenerate_audit_trail(submitter) + # Regenerate audit trail if it doesn't exist or was created before latest completion + # This handles re-completion after change requests + latest_completion = submitter.submission.submitters.maximum(:completed_at) + existing_audit_trail = ActiveStorage::Attachment.find_by( + record: submitter.submission, + name: 'audit_trail' + ) + + should_regenerate = existing_audit_trail.blank? || + existing_audit_trail.created_at < latest_completion + + return unless should_regenerate + + # Generate new audit trail first (safer - keeps old one if generation fails) + Submissions::GenerateAuditTrail.call(submitter.submission) + + # Clean up old audit trail attachments, keeping only the newest + # This handles multiple PDFs created during re-completion after change requests + audit_trail_attachments = ActiveStorage::Attachment.where( + record: submitter.submission, + name: 'audit_trail' + ).order(created_at: :asc) + + # Keep only the newest, purge the rest + return unless audit_trail_attachments.count > 1 + + audit_trail_attachments.limit(audit_trail_attachments.count - 1).each(&:purge) + end end diff --git a/app/models/submitter.rb b/app/models/submitter.rb index 67ba8582..dafde9d9 100644 --- a/app/models/submitter.rb +++ b/app/models/submitter.rb @@ -119,6 +119,14 @@ class Submitter < ApplicationRecord end end + def current_documents + return documents if document_generation_events.complete.none? + + # Use completed_at as the marker since documents are generated after completion + # This handles re-completion: only returns documents from the latest completion + documents.where(active_storage_attachments: { created_at: completed_at.. }) + end + private def anonymize_email_events diff --git a/app/views/submissions/show.html.erb b/app/views/submissions/show.html.erb index c516ff2a..d7dce43d 100644 --- a/app/views/submissions/show.html.erb +++ b/app/views/submissions/show.html.erb @@ -17,20 +17,6 @@ <% end %> - <% if last_submitter %> - <% if is_all_completed || !is_combined_enabled %> - - - <%= svg_icon('download', class: 'w-6 h-6') %> - - - - - <% end %> - <% end %>
diff --git a/app/views/submitters_request_changes/request_changes.html.erb b/app/views/submitters_request_changes/request_changes.html.erb index 29fb2cf8..14ed23d0 100644 --- a/app/views/submitters_request_changes/request_changes.html.erb +++ b/app/views/submitters_request_changes/request_changes.html.erb @@ -5,8 +5,7 @@

Request Changes

- Request changes from <%= @submitter.name || @submitter.email %> for this submission. - They will receive an email with your message and be able to resubmit the form. + Request changes from <%= @submitter.name || @submitter.email %> for this form.

<%= form_for '', url: request_changes_submitter_path(@submitter.slug), method: :post do |f| %> @@ -25,7 +24,7 @@ <% end %> diff --git a/config/locales/i18n.yml b/config/locales/i18n.yml index b27c87c1..913ada04 100644 --- a/config/locales/i18n.yml +++ b/config/locales/i18n.yml @@ -775,6 +775,7 @@ en: &en view_form_by_html: 'Form viewed by %{submitter_name}' invite_party_by_html: 'Invited %{invited_submitter_name} by %{submitter_name}' complete_form_by_html: 'Submission completed by %{submitter_name}' + request_changes_by_html: 'Changes requested for %{submitter_name}' start_verification_by_html: 'Identity verification started by %{submitter_name}' complete_verification_by_html: 'Identity verification completed by %{submitter_name} with %{provider}' api_complete_form_by_html: 'Submission completed via API by %{submitter_name}' @@ -1617,6 +1618,7 @@ es: &es view_form_by_html: 'Formulario visto por %{submitter_name}' invite_party_by_html: 'Invitado %{invited_submitter_name} por %{submitter_name}' complete_form_by_html: 'Envío completado por %{submitter_name}' + request_changes_by_html: 'Cambios solicitados para %{submitter_name}' api_complete_form_by_html: 'Envío completado vía API por %{submitter_name}' start_verification_by_html: 'Verificación de identidad iniciada por %{submitter_name}' complete_verification_by_html: 'Verificación de identidad completada por %{submitter_name} con %{provider}' diff --git a/db/migrate/20251107175502_remove_unique_index_from_document_generation_events.rb b/db/migrate/20251107175502_remove_unique_index_from_document_generation_events.rb new file mode 100644 index 00000000..19ede238 --- /dev/null +++ b/db/migrate/20251107175502_remove_unique_index_from_document_generation_events.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class RemoveUniqueIndexFromDocumentGenerationEvents < ActiveRecord::Migration[8.0] + def change + # Remove the partial unique index that covers both 'start' and 'complete' + remove_index :document_generation_events, + column: [:submitter_id, :event_name], + where: "event_name IN ('start', 'complete')" + + # Add unique constraint only for 'start' events (allows multiple 'complete' events) + add_index :document_generation_events, + [:submitter_id, :event_name], + unique: true, + where: "event_name = 'start'" + end +end diff --git a/db/schema.rb b/db/schema.rb index d33b7647..21da8c02 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2025_11_07_043352) do +ActiveRecord::Schema[8.0].define(version: 2025_11_07_175502) do # These are extensions that must be enabled in order to support this database enable_extension "btree_gin" enable_extension "pg_catalog.plpgsql" diff --git a/lib/submissions/ensure_result_generated.rb b/lib/submissions/ensure_result_generated.rb index 423f3097..5f1cb287 100644 --- a/lib/submissions/ensure_result_generated.rb +++ b/lib/submissions/ensure_result_generated.rb @@ -16,23 +16,32 @@ module Submissions raise NotCompletedYet unless submitter.completed_at? - return submitter.documents if ApplicationRecord.uncached { submitter.document_generation_events.complete.exists? } + last_complete_event = ApplicationRecord.uncached do + submitter.document_generation_events.complete.order(:created_at).last + end + + # Only return existing docs if they were generated AFTER the current completion + # This handles re-completion after change requests by comparing timestamps + if last_complete_event && last_complete_event.created_at >= submitter.completed_at + return latest_documents_for_event(submitter) + end events = ApplicationRecord.uncached do DocumentGenerationEvent.where(submitter:).order(:created_at).to_a end - if events.present? && events.last.event_name.in?(%w[start retry]) - wait_for_complete_or_fail(submitter) - else - submitter.document_generation_events.create!(event_name: events.present? ? :retry : :start) + last_event = events.last - documents = GenerateResultAttachments.call(submitter) + # Check if last event is start/retry AND was created after current completion + # This means generation is actually in progress for THIS completion + is_generation_in_progress = last_event&.event_name&.in?(%w[start retry]) && + last_event.created_at >= submitter.completed_at - submitter.document_generation_events.create!(event_name: :complete) - - documents + if is_generation_in_progress + wait_for_complete_or_fail(submitter) + else + generate_and_record_documents(submitter, events) end rescue ActiveRecord::RecordNotUnique sleep WAIT_FOR_RETRY @@ -40,7 +49,6 @@ module Submissions retry rescue StandardError => e Rollbar.error(e) if defined?(Rollbar) - Rails.logger.error(e) submitter.document_generation_events.create!(event_name: :fail) @@ -64,5 +72,25 @@ module Submissions raise WaitForCompleteTimeout if total_wait_time > CHECK_COMPLETE_TIMEOUT end end + + def latest_documents_for_event(submitter) + # Return documents created after the current completion timestamp + # This ensures we get the most recent generation, not old ones from previous completions + submitter.documents.where(active_storage_attachments: { created_at: submitter.completed_at.. }) + end + + def generate_and_record_documents(submitter, events) + submitter.document_generation_events.create!(event_name: events.present? ? :retry : :start) + + documents = GenerateResultAttachments.call(submitter) + + # Only create "complete" event if one doesn't exist for this completion + # Check if there's a complete event created AFTER this completion + unless submitter.document_generation_events.complete.exists?(created_at: submitter.completed_at..) + submitter.document_generation_events.create!(event_name: :complete) + end + + documents + end end end diff --git a/lib/submissions/generate_audit_trail.rb b/lib/submissions/generate_audit_trail.rb index d15c242b..d82c0575 100644 --- a/lib/submissions/generate_audit_trail.rb +++ b/lib/submissions/generate_audit_trail.rb @@ -441,15 +441,8 @@ module Submissions end def add_logo(column, _submission = nil) - column.image(PdfIcons.logo_io, width: 40, height: 40, position: :float) - - column.formatted_text([{ text: 'DocuSeal', - link: Docuseal::PRODUCT_EMAIL_URL }], - font_size: 20, - font: [FONT_NAME, { variant: :bold }], - width: 100, - padding: [5, 0, 0, 8], - position: :float, text_align: :left) + # Logo and branding removed - add minimal spacing to maintain layout + column.text('', font_size: 20, padding: [0, 0, 0, 0], position: :float) end # rubocop:enable Metrics end diff --git a/lib/submitters.rb b/lib/submitters.rb index 65bc4215..7ad01247 100644 --- a/lib/submitters.rb +++ b/lib/submitters.rb @@ -98,7 +98,12 @@ module Submitters original_documents = submitter.submission.schema_documents.preload(:blob) is_more_than_two_images = original_documents.count(&:image?) > 1 - submitter.documents.preload(:blob).reject do |attachment| + # Use current_documents to get only the latest generation after re-completion + docs = submitter.current_documents + # Handle both ActiveStorage::Attached::Many and ActiveRecord::Relation + docs = docs.preload(:blob) if docs.respond_to?(:preload) + + docs.reject do |attachment| is_more_than_two_images && original_documents.find { |a| a.uuid == (attachment.metadata['original_uuid'] || attachment.uuid) }&.image? end diff --git a/spec/jobs/process_submitter_completion_job_spec.rb b/spec/jobs/process_submitter_completion_job_spec.rb index cb357efc..763c02bc 100644 --- a/spec/jobs/process_submitter_completion_job_spec.rb +++ b/spec/jobs/process_submitter_completion_job_spec.rb @@ -45,5 +45,146 @@ RSpec.describe ProcessSubmitterCompletionJob do described_class.new.perform('submitter_id' => 'invalid_id') end.to raise_error(ActiveRecord::RecordNotFound) end + + context 'when all submitters are completed' do + let(:submitter2) { create(:submitter, submission:, uuid: SecureRandom.uuid, completed_at: Time.current) } + + before do + # Mark all submitters as completed + submission.submitters.update_all(completed_at: Time.current) + end + + it 'generates audit trail for the submission' do + allow(Submissions::GenerateAuditTrail).to receive(:call).with(submission) + + described_class.new.perform('submitter_id' => submitter.id) + + expect(Submissions::GenerateAuditTrail).to have_received(:call).with(submission) + end + + context 'when audit trail already exists from previous completion' do + let(:old_audit_trail_blob) do + ActiveStorage::Blob.create_and_upload!( + io: StringIO.new('old audit trail'), + filename: 'audit_trail.pdf' + ) + end + + let(:new_audit_trail_blob) do + ActiveStorage::Blob.create_and_upload!( + io: StringIO.new('new audit trail'), + filename: 'audit_trail.pdf' + ) + end + + before do + # Attach old audit trail from 10 minutes ago + submission.audit_trail.attach(old_audit_trail_blob) + ActiveStorage::Attachment.where( + record: submission, + name: 'audit_trail' + ).update_all(created_at: 10.minutes.ago) + + # Mock the audit trail generation to create new attachment + allow(Submissions::GenerateAuditTrail).to receive(:call) do |sub| + sub.audit_trail.attach(new_audit_trail_blob) + end + end + + it 'generates new audit trail' do + described_class.new.perform('submitter_id' => submitter.id) + + expect(Submissions::GenerateAuditTrail).to have_received(:call).with(submission) + end + + it 'purges old audit trail attachments' do + described_class.new.perform('submitter_id' => submitter.id) + + # Should only have the new audit trail + audit_trails = ActiveStorage::Attachment.where( + record: submission, + name: 'audit_trail' + ) + expect(audit_trails.count).to eq(1) + expect(audit_trails.first.blob).to eq(new_audit_trail_blob) + end + end + + context 'when audit trail was created before latest completion (re-completion scenario)' do + let(:audit_trail_blob) do + ActiveStorage::Blob.create_and_upload!( + io: StringIO.new('old audit trail'), + filename: 'audit_trail.pdf' + ) + end + + before do + # Create old audit trail before the current completion + submission.audit_trail.attach(audit_trail_blob) + ActiveStorage::Attachment.where( + record: submission, + name: 'audit_trail' + ).update_all(created_at: 1.hour.ago) + + # Current completion is more recent + submission.submitters.update_all(completed_at: Time.current) + end + + it 'regenerates audit trail because it is stale' do + allow(Submissions::GenerateAuditTrail).to receive(:call).with(submission) + + described_class.new.perform('submitter_id' => submitter.id) + + expect(Submissions::GenerateAuditTrail).to have_received(:call).with(submission) + end + end + + context 'when no audit trail exists' do + it 'generates new audit trail' do + allow(Submissions::GenerateAuditTrail).to receive(:call).with(submission) + + described_class.new.perform('submitter_id' => submitter.id) + + expect(Submissions::GenerateAuditTrail).to have_received(:call).with(submission) + end + end + + context 'when checking audit trail created_at (integration test)' do + it 'can access created_at on ActiveStorage::Attachment record' do + # This test verifies that we're using ActiveStorage::Attachment.find_by + # instead of submission.audit_trail (which is a proxy and doesn't have created_at) + audit_trail_blob = ActiveStorage::Blob.create_and_upload!( + io: StringIO.new('audit trail'), + filename: 'audit_trail.pdf' + ) + submission.audit_trail.attach(audit_trail_blob) + + attachment = ActiveStorage::Attachment.find_by( + record: submission, + name: 'audit_trail' + ) + + expect(attachment).to be_present + expect(attachment.created_at).to be_a(Time) + expect { attachment.created_at < Time.current }.not_to raise_error + end + end + end + + context 'when not all submitters are completed' do + let(:submitter2) { create(:submitter, submission:, uuid: SecureRandom.uuid, completed_at: nil) } + + before do + submitter2 # Create the incomplete submitter + end + + it 'does not generate audit trail' do + allow(Submissions::GenerateAuditTrail).to receive(:call) + + described_class.new.perform('submitter_id' => submitter.id) + + expect(Submissions::GenerateAuditTrail).not_to have_received(:call) + end + end end end diff --git a/spec/lib/submissions/ensure_result_generated_spec.rb b/spec/lib/submissions/ensure_result_generated_spec.rb new file mode 100644 index 00000000..5fd2a843 --- /dev/null +++ b/spec/lib/submissions/ensure_result_generated_spec.rb @@ -0,0 +1,189 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Submissions::EnsureResultGenerated do + let(:account) { create(:account) } + let(:user) { create(:user, account:) } + let(:template) { create(:template, account:, author: user) } + let(:submission) { create(:submission, template:, created_by_user: user) } + let(:submitter) { create(:submitter, submission:, uuid: SecureRandom.uuid) } + + before do + create(:encrypted_config, key: EncryptedConfig::ESIGN_CERTS_KEY, + value: GenerateCertificate.call.transform_values(&:to_pem)) + end + + describe '.call' do + context 'when submitter is not completed' do + before do + submitter.update!(completed_at: nil) + end + + it 'raises NotCompletedYet error' do + expect do + described_class.call(submitter) + end.to raise_error(Submissions::EnsureResultGenerated::NotCompletedYet) + end + end + + context 'when submitter is nil' do + it 'returns empty array' do + expect(described_class.call(nil)).to eq([]) + end + end + + context 'when documents exist and complete event is after completion' do + let(:blob) { ActiveStorage::Blob.create_and_upload!(io: StringIO.new('test'), filename: 'test.pdf') } + + before do + submitter.update!(completed_at: 5.minutes.ago) + submitter.documents.attach(blob) + + # Create complete event AFTER documents were generated + submitter.document_generation_events.create!(event_name: :complete) + end + + it 'returns existing documents' do + result = described_class.call(submitter) + expect(result.map(&:blob)).to include(blob) + end + + it 'does not generate new documents' do + allow(Submissions::GenerateResultAttachments).to receive(:call) + described_class.call(submitter) + expect(Submissions::GenerateResultAttachments).not_to have_received(:call) + end + end + + context 'when complete event is stale (created before current completion)' do + let(:old_blob) { ActiveStorage::Blob.create_and_upload!(io: StringIO.new('old'), filename: 'old.pdf') } + + before do + # Old completion cycle + submitter.update!(completed_at: 10.minutes.ago) + submitter.documents.attach(old_blob) + submitter.document_generation_events.create!(event_name: :complete) + + # Update timestamps to be old + ActiveStorage::Attachment.where(record: submitter, name: 'documents') + .update_all(created_at: 10.minutes.ago) + submitter.document_generation_events.update_all(created_at: 10.minutes.ago) + + # New completion (re-completion after change request) + submitter.update!(completed_at: Time.current) + end + + it 'generates new documents' do + allow(Submissions::GenerateResultAttachments).to receive(:call).with(submitter).and_return([]) + expect do + described_class.call(submitter) + end.to change { submitter.document_generation_events.count }.by(2) # retry + complete + expect(Submissions::GenerateResultAttachments).to have_received(:call).with(submitter) + end + + it 'creates a retry event (not start, since events exist)' do + allow(Submissions::GenerateResultAttachments).to receive(:call).and_return([]) + + described_class.call(submitter) + + # Should have 1 retry event + expect(submitter.document_generation_events.where(event_name: :retry).count).to eq(1) + end + + it 'creates a new complete event after generation' do + allow(Submissions::GenerateResultAttachments).to receive(:call).and_return([]) + + described_class.call(submitter) + + # Should have 2 complete events now (old + new) + expect(submitter.document_generation_events.where(event_name: :complete).count).to eq(2) + end + end + + context 'when generation is in progress (start/retry event after completion)' do + before do + submitter.update!(completed_at: Time.current) + submitter.document_generation_events.create!(event_name: :start) + end + + it 'waits for completion' do + # Simulate another process completing the generation + allow(described_class).to receive(:wait_for_complete_or_fail) do + submitter.document_generation_events.create!(event_name: :complete) + submitter.documents + end + + described_class.call(submitter) + expect(described_class).to have_received(:wait_for_complete_or_fail).with(submitter) + end + end + + context 'when stale start/retry event exists from previous completion' do + let(:old_blob) { ActiveStorage::Blob.create_and_upload!(io: StringIO.new('old'), filename: 'old.pdf') } + + before do + # Old completion with stale start event + submitter.update!(completed_at: 10.minutes.ago) + submitter.document_generation_events.create!(event_name: :start) + + # Update to make it old + submitter.document_generation_events.update_all(created_at: 10.minutes.ago) + + # New completion + submitter.update!(completed_at: Time.current) + end + + it 'does not wait for stale event' do + allow(described_class).to receive(:wait_for_complete_or_fail) + allow(Submissions::GenerateResultAttachments).to receive(:call).and_return([]) + + described_class.call(submitter) + + expect(described_class).not_to have_received(:wait_for_complete_or_fail) + expect(Submissions::GenerateResultAttachments).to have_received(:call) + end + + it 'creates retry event for current completion' do + allow(Submissions::GenerateResultAttachments).to receive(:call).and_return([]) + + expect do + described_class.call(submitter) + end.to change { submitter.document_generation_events.where(event_name: :retry).count }.by(1) + end + end + + context 'when no events exist yet' do + before do + submitter.update!(completed_at: Time.current) + end + + it 'creates start event and generates documents' do + allow(Submissions::GenerateResultAttachments).to receive(:call).with(submitter).and_return([]) + + expect do + described_class.call(submitter) + end.to change { submitter.document_generation_events.where(event_name: :start).count }.by(1) + + expect(Submissions::GenerateResultAttachments).to have_received(:call).with(submitter) + end + end + + context 'when generation fails' do + before do + submitter.update!(completed_at: Time.current) + allow(Submissions::GenerateResultAttachments).to receive(:call).and_raise( + StandardError.new('Generation failed') + ) + end + + it 'creates fail event' do + expect do + described_class.call(submitter) + end.to raise_error(StandardError) + + expect(submitter.document_generation_events.where(event_name: :fail).count).to eq(1) + end + end + end +end diff --git a/spec/models/submitter_spec.rb b/spec/models/submitter_spec.rb index 760234db..d62d4da9 100644 --- a/spec/models/submitter_spec.rb +++ b/spec/models/submitter_spec.rb @@ -223,4 +223,103 @@ RSpec.describe Submitter do end end end + + describe '#current_documents' do + let(:blob_one) { ActiveStorage::Blob.create_and_upload!(io: StringIO.new('test1'), filename: 'test1.pdf') } + let(:blob_two) { ActiveStorage::Blob.create_and_upload!(io: StringIO.new('test2'), filename: 'test2.pdf') } + let(:blob_three) { ActiveStorage::Blob.create_and_upload!(io: StringIO.new('test3'), filename: 'test3.pdf') } + + context 'when there are no complete events' do + before do + submitter.documents.attach(blob_one) + submitter.documents.attach(blob_two) + end + + it 'returns all documents' do + expect(submitter.current_documents.count).to eq(2) + expect(submitter.current_documents.map(&:blob)).to contain_exactly(blob_one, blob_two) + end + end + + context 'when there is one completion with documents generated after completion' do + before do + # Complete first + submitter.update!(completed_at: Time.current) + + # Attach documents after completion + submitter.documents.attach(blob_one) + submitter.documents.attach(blob_two) + submitter.document_generation_events.create!(event_name: :complete) + end + + it 'returns all documents since they were created after completion' do + expect(submitter.current_documents.count).to eq(2) + expect(submitter.current_documents.map(&:blob)).to contain_exactly(blob_one, blob_two) + end + end + + context 'when there are multiple completions (re-completion after change request)' do + let(:old_attachment) do + # First completion cycle - set old timestamp + submitter.update!(completed_at: 10.minutes.ago) + submitter.documents.attach(blob_one) + submitter.document_generation_events.create!(event_name: :complete) + + # Update attachment timestamp to match old completion + attachment_record = submitter.documents.find_by(blob_id: blob_one.id) + ActiveStorage::Attachment.where(id: attachment_record.id).update_all(created_at: 10.minutes.ago) + attachment_record + end + + before do + # Change request cycle - reset completed_at + submitter.update!(changes_requested_at: 5.minutes.ago, completed_at: nil) + + # Second completion cycle with NEW completion time + submitter.update!(completed_at: Time.current) + submitter.documents.attach(blob_two) + submitter.documents.attach(blob_three) + submitter.document_generation_events.create!(event_name: :complete) + end + + it 'returns only documents from the most recent completion' do + docs = submitter.current_documents + expect(docs.count).to eq(2) + expect(docs.map(&:blob)).to contain_exactly(blob_two, blob_three) + end + + it 'excludes documents from previous completion cycles' do + docs = submitter.current_documents + expect(docs.map(&:blob)).not_to include(blob_one) + end + end + + context 'when old documents exist before completion timestamp' do + let(:old_attachment) do + # Attach old document and manually set old timestamp + submitter.documents.attach(blob_one) + attachment_record = submitter.documents.find_by(blob_id: blob_one.id) + ActiveStorage::Attachment.where(id: attachment_record.id).update_all(created_at: 10.minutes.ago) + attachment_record + end + + before do + # Complete and attach new document + submitter.update!(completed_at: Time.current) + submitter.documents.attach(blob_two) + submitter.document_generation_events.create!(event_name: :complete) + end + + it 'only returns documents created after completion timestamp' do + docs = submitter.current_documents + expect(docs.count).to eq(1) + expect(docs.first.blob).to eq(blob_two) + end + + it 'excludes documents created before completion timestamp' do + docs = submitter.current_documents + expect(docs.map(&:blob)).not_to include(blob_one) + end + end + end end