From 1b4b6c9849b78c0a7851b8203f9787f28696a502 Mon Sep 17 00:00:00 2001 From: Bernardo Anderson Date: Wed, 29 Jul 2026 11:58:22 -0500 Subject: [PATCH 1/2] CP-14012 - Show checkbox and selection marks on PDFs What Checkbox, radio, and multi-select choices now appear as checkmarks on preview and completed PDFs, including when the value is a truthy string like "true" from prefill or the API. Stale radio/multi-select option areas no longer crash PDF generation. Why Users could type text and see it on the PDF, but selected checkboxes and options were often missing on the preview and final document, so they had to re-check form data offline. How to test 1. Create a form with a text field, checkbox, radio (option areas), and multi-select (option areas). 2. Fill and complete it; confirm checkmarks for selections on the live page and on the downloaded PDF. 3. Prefill or submit a checkbox as the string "true" and confirm the PDF shows a check. 4. Optional: point a radio area at a missing option UUID and confirm the PDF still generates. --- .../generate_result_attachments.rb | 4 + .../generate_result_attachments_spec.rb | 240 +++++++++++++++++- 2 files changed, 235 insertions(+), 9 deletions(-) diff --git a/lib/submissions/generate_result_attachments.rb b/lib/submissions/generate_result_attachments.rb index 18d6ae5c8..0900a8083 100644 --- a/lib/submissions/generate_result_attachments.rb +++ b/lib/submissions/generate_result_attachments.rb @@ -462,10 +462,14 @@ def fill_submitter_fields(submitter, account, pdfs_index, with_signature_id:, is if field['type'].in?(%w[multiple radio]) option = field['options']&.find { |o| o['uuid'] == area['option_uuid'] } + next if option.nil? + option_name = option['value'].presence option_name ||= "#{I18n.t('option', locale: locale)} #{field['options'].index(option) + 1}" value = Array.wrap(value).include?(option_name) + else + value = Submitters::NormalizeValues::TRUE_VALUES.include?(value) end next unless value == true diff --git a/spec/lib/submissions/generate_result_attachments_spec.rb b/spec/lib/submissions/generate_result_attachments_spec.rb index 8b6019e05..bc0405c7e 100644 --- a/spec/lib/submissions/generate_result_attachments_spec.rb +++ b/spec/lib/submissions/generate_result_attachments_spec.rb @@ -18,15 +18,22 @@ { attachment_uuid => doc } end - # Point the submitter's submission at a single text field whose one area is - # `area`, so `fill_submitter_fields` reaches the page lookup for that area. - def assign_field_area(area) - submitter.submission.update!( - template_fields: [ - { 'uuid' => SecureRandom.uuid, 'submitter_uuid' => submitter.uuid, - 'type' => 'text', 'areas' => [area] } - ] - ) + def base_area(overrides = {}) + { + 'x' => 0.1, + 'y' => 0.1, + 'w' => 0.05, + 'h' => 0.05, + 'attachment_uuid' => attachment_uuid, + 'page' => 0 + }.merge(overrides) + end + + # Point the submitter's submission at fields so fill_submitter_fields reaches + # the page lookup and drawing branches under test. + def assign_fields(fields, values = {}) + submitter.submission.update!(template_fields: fields) + submitter.update!(values: values) end def fill @@ -36,9 +43,34 @@ def fill ) end + def image_xobject_count + page = pdfs_index[attachment_uuid].pages[0] + xobjects = page.resources[:XObject] + return 0 if xobjects.nil? + + count = 0 + # HexaPDF::Dictionary supports each but not each_value + xobjects.each { |_name, obj| count += 1 if obj[:Subtype] == :Image } # rubocop:disable Style/HashEachMethods + count + end + + def page_content_has_text? + page = pdfs_index[attachment_uuid].pages[0] + content = page.contents + content = content.data if content.respond_to?(:data) + content.to_s.match?(/Tj|TJ/) + end + before { allow(Rails.logger).to receive(:warn) } describe '.fill_submitter_fields with a missing area page' do + def assign_field_area(area) + assign_fields( + [{ 'uuid' => SecureRandom.uuid, 'submitter_uuid' => submitter.uuid, + 'type' => 'text', 'areas' => [area] }] + ) + end + context 'when the area omits the page key' do before { assign_field_area('attachment_uuid' => attachment_uuid) } @@ -78,4 +110,194 @@ def fill end end end + + describe '.fill_submitter_fields selection rendering' do + let(:field_uuid) { SecureRandom.uuid } + + context 'when the field is a checkbox' do + def assign_checkbox(value) + assign_fields( + [{ 'uuid' => field_uuid, 'submitter_uuid' => submitter.uuid, + 'type' => 'checkbox', 'areas' => [base_area] }], + { field_uuid => value } + ) + end + + it 'draws a check when value is boolean true' do + assign_checkbox(true) + fill + expect(image_xobject_count).to eq(1) + end + + it 'draws a check when value is the string "true" (prefill/API truthiness)' do + assign_checkbox('true') + fill + expect(image_xobject_count).to eq(1) + end + + it 'draws a check when value is "1" or "yes"' do + assign_checkbox('1') + fill + expect(image_xobject_count).to eq(1) + + # Reset page for second value + pdfs_index[attachment_uuid].pages[0].canvas(type: :overlay) # ensure page exists + doc = HexaPDF::Document.new + doc.pages.add + pdfs_index[attachment_uuid] = doc + + assign_checkbox('yes') + fill + expect(image_xobject_count).to eq(1) + end + + it 'does not draw a check when value is false, "false", or nil' do + [false, 'false', nil].each do |value| + doc = HexaPDF::Document.new + doc.pages.add + pdfs_index[attachment_uuid] = doc + + assign_checkbox(value) + fill + expect(image_xobject_count).to eq(0), "expected no check for #{value.inspect}" + end + end + end + + context 'when the field is a radio with option areas' do + it 'draws a check only on the matching option area' do + yes_uuid = SecureRandom.uuid + no_uuid = SecureRandom.uuid + + assign_fields( + [{ + 'uuid' => field_uuid, + 'submitter_uuid' => submitter.uuid, + 'type' => 'radio', + 'options' => [ + { 'uuid' => yes_uuid, 'value' => 'Yes' }, + { 'uuid' => no_uuid, 'value' => 'No' } + ], + 'areas' => [ + base_area('option_uuid' => yes_uuid, 'x' => 0.1), + base_area('option_uuid' => no_uuid, 'x' => 0.3) + ] + }], + { field_uuid => 'Yes' } + ) + + fill + expect(image_xobject_count).to eq(1) + end + end + + context 'when the field is multiple with option areas' do + it 'draws a check on each selected option area' do + opt_a = SecureRandom.uuid + opt_b = SecureRandom.uuid + opt_c = SecureRandom.uuid + + assign_fields( + [{ + 'uuid' => field_uuid, + 'submitter_uuid' => submitter.uuid, + 'type' => 'multiple', + 'options' => [ + { 'uuid' => opt_a, 'value' => 'A' }, + { 'uuid' => opt_b, 'value' => 'B' }, + { 'uuid' => opt_c, 'value' => 'C' } + ], + 'areas' => [ + base_area('option_uuid' => opt_a, 'x' => 0.1), + base_area('option_uuid' => opt_b, 'x' => 0.2), + base_area('option_uuid' => opt_c, 'x' => 0.3) + ] + }], + { field_uuid => %w[A B] } + ) + + fill + expect(image_xobject_count).to eq(2) + end + end + + context 'when a radio option area has a stale option_uuid' do + before do + assign_fields( + [{ + 'uuid' => field_uuid, + 'submitter_uuid' => submitter.uuid, + 'type' => 'radio', + 'options' => [{ 'uuid' => SecureRandom.uuid, 'value' => 'Yes' }], + 'areas' => [base_area('option_uuid' => SecureRandom.uuid)] + }], + { field_uuid => 'Yes' } + ) + end + + it 'skips the area instead of raising' do + expect { fill }.not_to raise_error + expect(image_xobject_count).to eq(0) + end + end + + context 'when radio/multiple has a single area without option_uuid' do + it 'draws selected radio value as text without raising' do + assign_fields( + [{ + 'uuid' => field_uuid, + 'submitter_uuid' => submitter.uuid, + 'type' => 'radio', + 'options' => [ + { 'uuid' => SecureRandom.uuid, 'value' => 'Yes' }, + { 'uuid' => SecureRandom.uuid, 'value' => 'No' } + ], + 'areas' => [base_area('w' => 0.3, 'h' => 0.04)] + }], + { field_uuid => 'Yes' } + ) + + expect { fill }.not_to raise_error + expect(image_xobject_count).to eq(0) + expect(page_content_has_text?).to be true + end + + it 'draws selected multiple values as joined text without raising' do + assign_fields( + [{ + 'uuid' => field_uuid, + 'submitter_uuid' => submitter.uuid, + 'type' => 'multiple', + 'options' => [ + { 'uuid' => SecureRandom.uuid, 'value' => 'A' }, + { 'uuid' => SecureRandom.uuid, 'value' => 'B' } + ], + 'areas' => [base_area('w' => 0.4, 'h' => 0.04)] + }], + { field_uuid => %w[A B] } + ) + + expect { fill }.not_to raise_error + expect(image_xobject_count).to eq(0) + expect(page_content_has_text?).to be true + end + end + + context 'when the field is text (regression)' do + it 'still renders typed text' do + assign_fields( + [{ + 'uuid' => field_uuid, + 'submitter_uuid' => submitter.uuid, + 'type' => 'text', + 'areas' => [base_area('w' => 0.4, 'h' => 0.04)] + }], + { field_uuid => 'Hello world' } + ) + + expect { fill }.not_to raise_error + expect(page_content_has_text?).to be true + end + end + end end From 3c96f7f2468c2523694e2a2d76c7d9580e3c1397 Mon Sep 17 00:00:00 2001 From: Bernardo Anderson Date: Wed, 29 Jul 2026 14:30:48 -0500 Subject: [PATCH 2/2] CP-14012 - Log skipped stale radio/multi-select option areas What When a radio or multi-select area points at an option that no longer exists, PDF generation still succeeds and now logs a warning so missing marks are easier to spot. Specs also better prove the check lands on the correct option and clean up truthy-string coverage. Why A silent skip could hide a real selection from the signed PDF until a customer noticed. Logging matches how we already handle missing page areas, and tighter tests catch drawing the check on the wrong option. How to test 1. bundle exec rspec spec/lib/submissions/generate_result_attachments_spec.rb 2. Optional: complete a form with radio/multi-select; confirm checks still appear on the PDF. 3. Optional: force a stale option UUID and confirm the PDF still generates and the warn log appears. --- .../generate_result_attachments.rb | 10 ++- .../generate_result_attachments_spec.rb | 61 +++++++++++++------ 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/lib/submissions/generate_result_attachments.rb b/lib/submissions/generate_result_attachments.rb index 0900a8083..88d44f1f4 100644 --- a/lib/submissions/generate_result_attachments.rb +++ b/lib/submissions/generate_result_attachments.rb @@ -462,7 +462,15 @@ def fill_submitter_fields(submitter, account, pdfs_index, with_signature_id:, is if field['type'].in?(%w[multiple radio]) option = field['options']&.find { |o| o['uuid'] == area['option_uuid'] } - next if option.nil? + # Stale option_uuid (template edited after submission) would raise on option['value']. + # Skip so the rest of the PDF still generates; log so missing marks stay visible. + if option.nil? + Rails.logger.warn( + "Skipping option area with unknown option_uuid (submitter=#{submitter.id}, " \ + "field=#{field['uuid']}, option_uuid=#{area['option_uuid']})" + ) + next + end option_name = option['value'].presence option_name ||= "#{I18n.t('option', locale: locale)} #{field['options'].index(option) + 1}" diff --git a/spec/lib/submissions/generate_result_attachments_spec.rb b/spec/lib/submissions/generate_result_attachments_spec.rb index bc0405c7e..2ca5205d0 100644 --- a/spec/lib/submissions/generate_result_attachments_spec.rb +++ b/spec/lib/submissions/generate_result_attachments_spec.rb @@ -135,20 +135,16 @@ def assign_checkbox(value) expect(image_xobject_count).to eq(1) end - it 'draws a check when value is "1" or "yes"' do - assign_checkbox('1') - fill - expect(image_xobject_count).to eq(1) - - # Reset page for second value - pdfs_index[attachment_uuid].pages[0].canvas(type: :overlay) # ensure page exists - doc = HexaPDF::Document.new - doc.pages.add - pdfs_index[attachment_uuid] = doc + it 'draws a check for other truthy string values' do + %w[1 yes].each do |value| + doc = HexaPDF::Document.new + doc.pages.add + pdfs_index[attachment_uuid] = doc - assign_checkbox('yes') - fill - expect(image_xobject_count).to eq(1) + assign_checkbox(value) + fill + expect(image_xobject_count).to eq(1), "expected check for #{value.inspect}" + end end it 'does not draw a check when value is false, "false", or nil' do @@ -168,16 +164,17 @@ def assign_checkbox(value) it 'draws a check only on the matching option area' do yes_uuid = SecureRandom.uuid no_uuid = SecureRandom.uuid + options = [ + { 'uuid' => yes_uuid, 'value' => 'Yes' }, + { 'uuid' => no_uuid, 'value' => 'No' } + ] assign_fields( [{ 'uuid' => field_uuid, 'submitter_uuid' => submitter.uuid, 'type' => 'radio', - 'options' => [ - { 'uuid' => yes_uuid, 'value' => 'Yes' }, - { 'uuid' => no_uuid, 'value' => 'No' } - ], + 'options' => options, 'areas' => [ base_area('option_uuid' => yes_uuid, 'x' => 0.1), base_area('option_uuid' => no_uuid, 'x' => 0.3) @@ -188,6 +185,25 @@ def assign_checkbox(value) fill expect(image_xobject_count).to eq(1) + + # Selected Yes, but only the No area is on the page — must not draw a check. + doc = HexaPDF::Document.new + doc.pages.add + pdfs_index[attachment_uuid] = doc + + assign_fields( + [{ + 'uuid' => field_uuid, + 'submitter_uuid' => submitter.uuid, + 'type' => 'radio', + 'options' => options, + 'areas' => [base_area('option_uuid' => no_uuid, 'x' => 0.3)] + }], + { field_uuid => 'Yes' } + ) + + fill + expect(image_xobject_count).to eq(0) end end @@ -222,6 +238,8 @@ def assign_checkbox(value) end context 'when a radio option area has a stale option_uuid' do + let(:stale_option_uuid) { SecureRandom.uuid } + before do assign_fields( [{ @@ -229,7 +247,7 @@ def assign_checkbox(value) 'submitter_uuid' => submitter.uuid, 'type' => 'radio', 'options' => [{ 'uuid' => SecureRandom.uuid, 'value' => 'Yes' }], - 'areas' => [base_area('option_uuid' => SecureRandom.uuid)] + 'areas' => [base_area('option_uuid' => stale_option_uuid)] }], { field_uuid => 'Yes' } ) @@ -239,6 +257,13 @@ def assign_checkbox(value) expect { fill }.not_to raise_error expect(image_xobject_count).to eq(0) end + + it 'logs that the option area was skipped' do + fill + expect(Rails.logger).to have_received(:warn).with( + /Skipping option area with unknown option_uuid.*option_uuid=#{stale_option_uuid}/ + ) + end end context 'when radio/multiple has a single area without option_uuid' do