diff --git a/app/controllers/inspections_controller.rb b/app/controllers/inspections_controller.rb
index ad1185320..80d95c635 100644
--- a/app/controllers/inspections_controller.rb
+++ b/app/controllers/inspections_controller.rb
@@ -354,7 +354,7 @@ def filtered_inspections_query_without_order = current_user.inspections
def no_index = response.set_header("X-Robots-Tag", "noindex,nofollow")
def set_inspection
- @inspection = Inspection
+ inspection_query = Inspection
.includes(
:user, :inspector_company,
*Inspection::ALL_ASSESSMENT_TYPES.keys,
@@ -363,7 +363,19 @@ def set_inspection
photo_2_attachment: :blob,
photo_3_attachment: :blob
)
- .find_by(id: params[:id]&.upcase)
+ inspection_id = params[:id]&.upcase
+
+ @inspection = if request.format.pdf?
+ PdfPerformance.measure(
+ :record_load,
+ pdf_type: :inspection,
+ record_id: inspection_id
+ ) do
+ inspection_query.find_by(id: inspection_id)
+ end
+ else
+ inspection_query.find_by(id: inspection_id)
+ end
head :not_found unless @inspection
end
@@ -432,14 +444,26 @@ def handle_failed_update
end
def send_inspection_pdf
- result = PdfCacheService.fetch_or_generate_inspection_pdf(
- @inspection,
- debug_enabled: admin_debug_enabled?,
- debug_queries: debug_sql_queries
- )
- @inspection.update(pdf_last_accessed_at: Time.current)
+ PdfPerformance.measure(
+ :total,
+ pdf_type: :inspection,
+ record_id: @inspection.id
+ ) do
+ result = PdfCacheService.fetch_or_generate_inspection_pdf(
+ @inspection,
+ debug_enabled: admin_debug_enabled?,
+ debug_queries: debug_sql_queries
+ )
+ PdfPerformance.measure(
+ :access_tracking,
+ pdf_type: :inspection,
+ record_id: @inspection.id
+ ) do
+ @inspection.update(pdf_last_accessed_at: Time.current)
+ end
- handle_pdf_response(result, pdf_filename)
+ handle_pdf_response(result, pdf_filename)
+ end
end
def send_inspection_qr_code
diff --git a/app/controllers/units_controller.rb b/app/controllers/units_controller.rb
index 24ffd2efc..ab0e91054 100644
--- a/app/controllers/units_controller.rb
+++ b/app/controllers/units_controller.rb
@@ -196,9 +196,22 @@ def normalize_unit_id(raw_id)
def no_index = response.set_header("X-Robots-Tag", "noindex,nofollow")
+ sig { void }
def set_unit
- @unit = Unit.includes(photo_attachment: :blob)
- .find_by(id: params[:id].upcase)
+ unit_id = params[:id].upcase
+ unit_query = Unit.includes(photo_attachment: :blob)
+
+ @unit = if request.format.pdf?
+ PdfPerformance.measure(
+ :record_load,
+ pdf_type: :unit,
+ record_id: unit_id
+ ) do
+ unit_query.find_by(id: unit_id)
+ end
+ else
+ unit_query.find_by(id: unit_id)
+ end
unless @unit
# Always return 404 for non-existent resources regardless of login status
@@ -220,14 +233,19 @@ def check_assessments_enabled
end
def send_unit_pdf
- # Unit already has photo loaded from set_unit
- result = PdfCacheService.fetch_or_generate_unit_pdf(
- @unit,
- debug_enabled: admin_debug_enabled?,
- debug_queries: debug_sql_queries
- )
-
- handle_pdf_response(result, pdf_filename)
+ PdfPerformance.measure(
+ :total,
+ pdf_type: :unit,
+ record_id: @unit.id
+ ) do
+ result = PdfCacheService.fetch_or_generate_unit_pdf(
+ @unit,
+ debug_enabled: admin_debug_enabled?,
+ debug_queries: debug_sql_queries
+ )
+
+ handle_pdf_response(result, pdf_filename)
+ end
end
def send_unit_qr_code
diff --git a/app/models/assessment_schema.rb b/app/models/assessment_schema.rb
index 3ccadde36..db6b04649 100644
--- a/app/models/assessment_schema.rb
+++ b/app/models/assessment_schema.rb
@@ -61,6 +61,9 @@ def initialize(raw)
sig { returns(T::Boolean) }
def required? = !!attributes[:required]
+ sig { returns(T::Boolean) }
+ def optional_for_completion? = attributes[:required] == false
+
sig { returns(T::Boolean) }
def numeric? = NUMERIC_PARTIALS.include?(partial)
@@ -163,6 +166,14 @@ def add_not_applicable_fields
fields.select(&:add_not_applicable?).map(&:name)
end
+ sig { returns(T::Array[Symbol]) }
+ def completion_optional_fields
+ @completion_optional_fields ||= fields
+ .select(&:optional_for_completion?)
+ .flat_map { [it.name, *it.composite_fields] }
+ .freeze
+ end
+
# Returns a new schema with the named fields removed from each fieldset.
# Used for permission-driven filtering (e.g. hiding admin-only fields).
sig { params(field_names: Symbol).returns(AssessmentSchema) }
diff --git a/app/models/concerns/assessment_completion.rb b/app/models/concerns/assessment_completion.rb
index 17474cb57..deb5b62d2 100644
--- a/app/models/concerns/assessment_completion.rb
+++ b/app/models/concerns/assessment_completion.rb
@@ -19,7 +19,10 @@ def complete?
sig { returns(T::Array[Symbol]) }
def incomplete_fields
- (self.class.column_name_syms - SYSTEM_FIELDS)
+ fields = self.class.column_name_syms - SYSTEM_FIELDS
+ fields -= self.class.assessment_schema.completion_optional_fields
+
+ fields
.reject { |f| f.end_with?("_comment") }
.select { |f| field_is_incomplete?(f) }
.reject { |f| field_allows_nil_when_na?(f) }
diff --git a/app/models/inspection.rb b/app/models/inspection.rb
index 2a2f11fe1..d13009b48 100644
--- a/app/models/inspection.rb
+++ b/app/models/inspection.rb
@@ -474,6 +474,11 @@ def invalidate_pdf_cache
sig { void }
def invalidate_unit_pdf_cache
+ changed_attrs = saved_changes.keys
+ ignorable_attrs = ["pdf_last_accessed_at", "updated_at"]
+
+ return if (changed_attrs - ignorable_attrs).empty?
+
PdfCacheService.invalidate_unit_cache(unit) if unit
end
end
diff --git a/app/models/unit.rb b/app/models/unit.rb
index 90f222780..00e6b5e41 100644
--- a/app/models/unit.rb
+++ b/app/models/unit.rb
@@ -21,7 +21,6 @@
# Indexes
#
# index_units_on_is_seed (is_seed)
-# index_units_on_manufacturer_and_serial (manufacturer,serial) UNIQUE
# index_units_on_serial_and_user_id (serial,user_id) UNIQUE
# index_units_on_unit_type (unit_type)
# index_units_on_user_id (user_id)
@@ -67,8 +66,7 @@ class Unit < ApplicationRecord
before_destroy :check_complete_inspections
before_destroy :destroy_draft_inspections
- # All fields are required for Units
- validates :name, :serial, :description, :manufacturer, presence: true
+ validates :description, :name, :serial, presence: true
validates :serial, uniqueness: {scope: [:user_id]}
validate :badge_id_valid, on: :create, if: -> { unit_badges_enabled? }
diff --git a/app/services/pdf_cache_service.rb b/app/services/pdf_cache_service.rb
index 69a3e4f6e..d2b00cd0a 100644
--- a/app/services/pdf_cache_service.rb
+++ b/app/services/pdf_cache_service.rb
@@ -17,12 +17,11 @@ class << self
.returns(CacheResult)
end
def fetch_or_generate_inspection_pdf(inspection, **options)
- # Never cache incomplete inspections
- unless caching_enabled? && inspection.complete?
- return generate_pdf_result(inspection, :inspection, **options)
+ if caching_enabled? && inspection.complete?
+ fetch_or_generate(inspection, :inspection, **options)
+ else
+ generate_pdf_result(inspection, :inspection, **options)
end
-
- fetch_or_generate(inspection, :inspection, **options)
end
sig { params(unit: Unit, options: T.untyped).returns(CacheResult) }
@@ -52,8 +51,14 @@ def invalidate_unit_cache(unit)
).returns(CacheResult)
end
def fetch_or_generate(record, type, **options)
- valid_cache = record.cached_pdf.attached? &&
- cached_pdf_valid?(record.cached_pdf, record)
+ valid_cache = PdfPerformance.measure(
+ :cache_lookup,
+ pdf_type: type,
+ record_id: record.id
+ ) do
+ record.cached_pdf.attached? &&
+ cached_pdf_valid?(record.cached_pdf, record)
+ end
if valid_cache
Rails.logger.info "PDF cache hit for #{type} #{record.id}"
@@ -79,7 +84,13 @@ def fetch_or_generate(record, type, **options)
end
def generate_and_cache(record, type, **options)
result = generate_pdf_result(record, type, **options)
- store_cached_pdf(record, result.data)
+ PdfPerformance.measure(
+ :cache_store,
+ pdf_type: type,
+ record_id: record.id
+ ) do
+ store_cached_pdf(record, result.data)
+ end
result
end
@@ -91,14 +102,28 @@ def generate_and_cache(record, type, **options)
).returns(CacheResult)
end
def generate_pdf_result(record, type, **options)
- pdf_document = case type
- when :inspection
- PdfGeneratorService.generate_inspection_report(record, **options)
- when :unit
- PdfGeneratorService.generate_unit_report(record, **options)
+ pdf_document = PdfPerformance.measure(
+ :document_build,
+ pdf_type: type,
+ record_id: record.id
+ ) do
+ case type
+ when :inspection
+ PdfGeneratorService.generate_inspection_report(record, **options)
+ when :unit
+ PdfGeneratorService.generate_unit_report(record, **options)
+ end
+ end
+
+ pdf_data = PdfPerformance.measure(
+ :document_render,
+ pdf_type: type,
+ record_id: record.id
+ ) do
+ pdf_document.render
end
- CacheResult.new(type: :pdf_data, data: pdf_document.render)
+ CacheResult.new(type: :pdf_data, data: pdf_data)
end
sig { params(record: T.any(Inspection, Unit)).void }
diff --git a/app/services/pdf_generator_service.rb b/app/services/pdf_generator_service.rb
index bb30af14a..0cc51e165 100644
--- a/app/services/pdf_generator_service.rb
+++ b/app/services/pdf_generator_service.rb
@@ -13,40 +13,74 @@ def self.generate_inspection_report(inspection, debug_enabled: false, debug_quer
# Initialize array to collect all assessment blocks
assessment_blocks = []
- # Header section
- HeaderGenerator.generate_inspection_pdf_header(pdf, inspection)
-
- # Unit details section
- generate_inspection_unit_details(pdf, inspection)
-
- # Risk assessment section (if present)
- generate_risk_assessment_section(pdf, inspection)
-
- # Generate all assessment sections in the correct UI order from applicable_tabs
- generate_assessments_in_ui_order(inspection, assessment_blocks)
+ PdfPerformance.measure(
+ :primary_content,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ ) do
+ HeaderGenerator.generate_inspection_pdf_header(pdf, inspection)
+ generate_inspection_unit_details(pdf, inspection)
+ generate_risk_assessment_section(pdf, inspection)
+ end
+
+ PdfPerformance.measure(
+ :assessment_data,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ ) do
+ generate_assessments_in_ui_order(inspection, assessment_blocks)
+ end
# Render footer and photo first to measure actual space used
cursor_before_footer = pdf.cursor
- # Disclaimer footer (only on first page)
- DisclaimerFooterRenderer.render_disclaimer_footer(pdf, inspection.user)
- disclaimer_height = DisclaimerFooterRenderer.measure_footer_height(unbranded: false)
-
- # Add unit photo in bottom right corner
- photo_height = ImageProcessor.measure_unit_photo_height(pdf, inspection.unit, 4)
- ImageProcessor.add_unit_photo_footer(pdf, inspection.unit, 4) if inspection.unit&.photo
+ disclaimer_height, photo_height = PdfPerformance.measure(
+ :footer,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ ) do
+ DisclaimerFooterRenderer.render_disclaimer_footer(pdf, inspection.user)
+ measured_footer = DisclaimerFooterRenderer.measure_footer_height(
+ unbranded: false
+ )
+ measured_photo = ImageProcessor.add_unit_photo_footer(
+ pdf,
+ inspection.unit,
+ 4,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ )
+ [measured_footer, measured_photo]
+ end
# Reset cursor to render assessments with proper space accounting
pdf.move_cursor_to(cursor_before_footer)
# Render all collected assessments in newspaper-style columns
- render_assessment_blocks_in_columns(pdf, assessment_blocks, disclaimer_height, photo_height)
+ PdfPerformance.measure(
+ :assessment_layout,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ ) do
+ render_assessment_blocks_in_columns(
+ pdf,
+ assessment_blocks,
+ disclaimer_height,
+ photo_height
+ )
+ end
# Add DRAFT watermark overlay for draft inspections (except in test env)
Utilities.add_draft_watermark(pdf) if !inspection.complete? && !Rails.env.test?
# Add photos page if photos are attached
- PhotosRenderer.generate_photos_page(pdf, inspection)
+ PdfPerformance.measure(
+ :report_photos,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ ) do
+ PhotosRenderer.generate_photos_page(pdf, inspection)
+ end
# Add debug info page if enabled (admins only)
DebugInfoRenderer.add_debug_info_page(pdf, debug_queries) if debug_enabled && debug_queries.present?
@@ -58,25 +92,61 @@ def self.generate_unit_report(unit, debug_enabled: false, debug_queries: [])
unbranded = Rails.configuration.units.reports_unbranded
- # Preload all inspections once to avoid N+1 queries
- completed_inspections = unit.inspections
- .includes(:user, inspector_company: {logo_attachment: :blob})
- .complete
- .order(inspection_date: :desc)
+ completed_inspections = PdfPerformance.measure(
+ :inspection_history_load,
+ pdf_type: :unit,
+ record_id: unit.id
+ ) do
+ unit.inspections
+ .includes(:user, inspector_company: {logo_attachment: :blob})
+ .complete
+ .order(complete_date: :desc)
+ .load
+ end
last_inspection = completed_inspections.first
Prawn::Document.new(page_size: "A4", page_layout: :portrait) do |pdf|
Configuration.setup_pdf_fonts(pdf)
- HeaderGenerator.generate_unit_pdf_header(pdf, unit, unbranded: unbranded)
- generate_unit_details_with_inspection(pdf, unit, last_inspection)
- generate_unit_inspection_history_with_data(pdf, unit, completed_inspections)
-
- # Disclaimer footer (only on first page, not for unbranded reports)
- DisclaimerFooterRenderer.render_disclaimer_footer(pdf, unit.user, unbranded: unbranded)
- # Add unit photo in bottom right corner (for unit PDFs, always use 3 columns)
- ImageProcessor.add_unit_photo_footer(pdf, unit, 3) if unit.photo
+ PdfPerformance.measure(
+ :primary_content,
+ pdf_type: :unit,
+ record_id: unit.id
+ ) do
+ HeaderGenerator.generate_unit_pdf_header(
+ pdf,
+ unit,
+ last_inspection:,
+ unbranded:
+ )
+ generate_unit_details_with_inspection(pdf, unit, last_inspection)
+ end
+
+ PdfPerformance.measure(
+ :inspection_history_layout,
+ pdf_type: :unit,
+ record_id: unit.id
+ ) do
+ generate_unit_inspection_history_with_data(
+ pdf,
+ unit,
+ completed_inspections
+ )
+ end
+
+ PdfPerformance.measure(
+ :footer,
+ pdf_type: :unit,
+ record_id: unit.id
+ ) do
+ DisclaimerFooterRenderer.render_disclaimer_footer(
+ pdf,
+ unit.user,
+ unbranded: unbranded
+ )
+ ImageProcessor.add_unit_photo_footer(pdf, unit, 3) if unit.photo
+ end
# Add debug info page if enabled (admins only)
DebugInfoRenderer.add_debug_info_page(pdf, debug_queries) if debug_enabled && debug_queries.present?
@@ -88,19 +158,30 @@ def self.generate_inspection_unit_details(pdf, inspection)
return unless unit
- unit_data = TableBuilder.build_unit_details_table_with_inspection(unit, inspection, :inspection)
+ unit_data = TableBuilder.build_unit_details_table_with_inspection(
+ unit,
+ inspection
+ )
TableBuilder.create_unit_details_table(pdf, I18n.t("pdf.inspection.equipment_details"), unit_data)
# Hide the table entirely when no unit is associated
end
def self.generate_unit_details(pdf, unit)
- unit_data = TableBuilder.build_unit_details_table(unit, :unit)
+ unit_data = TableBuilder.build_unit_details_table(
+ unit,
+ :unit,
+ unit.last_inspection
+ )
TableBuilder.create_unit_details_table(pdf, I18n.t("pdf.unit.details"), unit_data)
end
- def self.generate_unit_details_with_inspection(pdf, unit, _last_inspection)
- unit_data = TableBuilder.build_unit_details_table(unit, :unit)
+ def self.generate_unit_details_with_inspection(pdf, unit, last_inspection)
+ unit_data = TableBuilder.build_unit_details_table(
+ unit,
+ :unit,
+ last_inspection
+ )
TableBuilder.create_unit_details_table(pdf, I18n.t("pdf.unit.details"), unit_data)
end
diff --git a/app/services/pdf_generator_service/header_generator.rb b/app/services/pdf_generator_service/header_generator.rb
index d7e1d2a97..7eb1f420f 100644
--- a/app/services/pdf_generator_service/header_generator.rb
+++ b/app/services/pdf_generator_service/header_generator.rb
@@ -23,18 +23,29 @@ def self.create_inspection_header(pdf, inspection)
pdf.move_down Configuration::STATUS_SPACING
end
- def self.generate_unit_pdf_header(pdf, unit, unbranded: false)
- create_unit_header(pdf, unit, unbranded: unbranded)
+ def self.generate_unit_pdf_header(pdf, unit, last_inspection:,
+ unbranded: false)
+ create_unit_header(
+ pdf,
+ unit,
+ last_inspection:,
+ unbranded:
+ )
# Generate QR code in top left corner
ImageProcessor.generate_qr_code_header(pdf, unit)
end
- def self.create_unit_header(pdf, unit, unbranded: false)
+ def self.create_unit_header(pdf, unit, last_inspection:, unbranded: false)
user = unbranded ? nil : unit.user
unit_id_text = build_unit_id_text(unit)
render_header_with_logo(pdf, user) do |logo_width|
- render_unit_text_section(pdf, unit, unit_id_text, logo_width)
+ render_unit_text_section(
+ pdf,
+ last_inspection,
+ unit_id_text,
+ logo_width
+ )
end
pdf.move_down Configuration::STATUS_SPACING
@@ -116,14 +127,15 @@ def render_inspection_text_section(pdf, inspection, report_id_text,
end
end
- def render_unit_text_section(pdf, unit, unit_id_text, logo_width)
+ def render_unit_text_section(pdf, last_inspection, unit_id_text,
+ logo_width)
header_text_box(pdf, logo_width) do
pdf.text unit_id_text, size: Configuration::HEADER_TEXT_SIZE,
style: :bold
expiry_label = I18n.t("pdf.unit.fields.expiry_date")
- expiry_value = if unit.last_inspection&.reinspection_date
- Utilities.format_date(unit.last_inspection.reinspection_date)
+ expiry_value = if last_inspection&.reinspection_date
+ Utilities.format_date(last_inspection.reinspection_date)
else
I18n.t("pdf.unit.fields.na")
end
diff --git a/app/services/pdf_generator_service/image_processor.rb b/app/services/pdf_generator_service/image_processor.rb
index 80ea6308e..59e77f8df 100644
--- a/app/services/pdf_generator_service/image_processor.rb
+++ b/app/services/pdf_generator_service/image_processor.rb
@@ -18,30 +18,22 @@ def self.generate_qr_code_header(pdf, entity)
pdf.image StringIO.new(qr_code_png), image_options
end
- def self.add_unit_photo_footer(pdf, unit, column_count = 3)
- return unless unit&.photo&.blob
+ def self.add_unit_photo_footer(
+ pdf,
+ unit,
+ column_count = 3,
+ pdf_type: :unit,
+ record_id: unit&.id
+ )
+ return 0 unless unit&.photo&.blob
pdf_width = pdf.bounds.width
attachment = unit.photo
- image = create_image(attachment)
+ context = performance_context(attachment).merge(pdf_type:, record_id:)
+ image = create_image(attachment, context)
dimensions = calculate_footer_photo_dimensions(pdf, image, column_count)
photo_width, photo_height = dimensions
- photo_x = pdf_width - photo_width
- photo_y = calculate_photo_y(pdf, photo_height)
-
- render_processed_image(pdf, image, photo_x, photo_y,
- photo_width, photo_height, attachment)
- end
-
- def self.measure_unit_photo_height(pdf, unit, column_count = 3)
- return 0 unless unit&.photo&.blob
-
- attachment = unit.photo
- image = create_image(attachment)
- dimensions = calculate_footer_photo_dimensions(pdf, image, column_count)
- _photo_width, photo_height = dimensions
-
if photo_height <= 0
error_message = I18n.t(
"pdf_generator.errors.zero_photo_height",
@@ -50,15 +42,22 @@ def self.measure_unit_photo_height(pdf, unit, column_count = 3)
raise error_message
end
+ photo_x = pdf_width - photo_width
+ photo_y = calculate_photo_y(pdf, photo_height)
+
+ render_processed_image(pdf, image, photo_x, photo_y,
+ photo_width, photo_height, attachment, context)
+
photo_height
rescue Prawn::Errors::UnsupportedImageType => e
raise ImageError.build_detailed_error(e, attachment)
end
def self.process_image_with_orientation(attachment)
- image = create_image(attachment)
+ context = performance_context(attachment)
+ image = create_image(attachment, context)
# Images are already orientation-corrected from upload processing
- image.write_to_buffer(".png")
+ transcode_to_png(image, context)
end
def self.calculate_footer_photo_dimensions(pdf, image, column_count = 3)
@@ -88,9 +87,10 @@ def self.calculate_height_from_aspect(photo_width, original_width,
end
end
- def self.render_processed_image(pdf, image, x, y, width, height, attachment)
+ def self.render_processed_image(pdf, image, x, y, width, height, attachment,
+ context)
# Images are already orientation-corrected from upload processing
- processed_image = image.write_to_buffer(".png")
+ processed_image = transcode_to_png(image, context)
image_options = {
at: [x, y],
@@ -98,13 +98,34 @@ def self.render_processed_image(pdf, image, x, y, width, height, attachment)
height: height
}
ImageError.with_error_handling(attachment) do
- pdf.image StringIO.new(processed_image), image_options
+ PdfPerformance.measure(
+ :image_embed,
+ **context
+ ) do
+ pdf.image StringIO.new(processed_image), image_options
+ end
end
end
- def self.create_image(attachment)
- image_data = attachment.blob.download
- Vips::Image.new_from_buffer(image_data, "")
+ def self.create_image(attachment, context)
+ image_data = PdfPerformance.measure(
+ :image_download,
+ **context
+ ) do
+ attachment.blob.download
+ end
+ PdfPerformance.measure(
+ :image_decode,
+ **context
+ ) do
+ Vips::Image.new_from_buffer(image_data, "")
+ end
+ end
+
+ def self.transcode_to_png(image, context)
+ PdfPerformance.measure(:image_transcode, **context) do
+ image.write_to_buffer(".png")
+ end
end
def self.calculate_photo_y(pdf, photo_height)
@@ -115,5 +136,16 @@ def self.calculate_photo_y(pdf, photo_height)
Configuration::QR_CODE_BOTTOM_OFFSET + photo_height
end
end
+
+ def self.performance_context(attachment)
+ record = attachment.record
+ {
+ pdf_type: record.class.model_name.singular.to_sym,
+ record_id: record.id,
+ attachment: attachment.name
+ }
+ end
+
+ private_class_method :performance_context, :transcode_to_png
end
end
diff --git a/app/services/pdf_generator_service/photos_renderer.rb b/app/services/pdf_generator_service/photos_renderer.rb
index b159a6f05..a522368e3 100644
--- a/app/services/pdf_generator_service/photos_renderer.rb
+++ b/app/services/pdf_generator_service/photos_renderer.rb
@@ -78,7 +78,6 @@ def self.calculate_needed_space(max_photo_height)
def self.render_photo(pdf, photo, label, max_height)
ImageError.with_error_handling(photo) do
- photo.blob.download
processed_image = ImageProcessor.process_image_with_orientation(photo)
image_width, image_height = calculate_photo_dimensions_from_blob(
diff --git a/app/services/pdf_generator_service/table_builder.rb b/app/services/pdf_generator_service/table_builder.rb
index c7b74c1a5..989734f33 100644
--- a/app/services/pdf_generator_service/table_builder.rb
+++ b/app/services/pdf_generator_service/table_builder.rb
@@ -200,13 +200,11 @@ def self.apply_history_table_column_widths(table, pdf_width)
table.column_widths = [date_width, result_width, inspector_width]
end
- def self.build_unit_details_table(unit, context)
- # Get dimensions from last inspection if available
- last_inspection = unit.last_inspection
+ def self.build_unit_details_table(unit, context, last_inspection)
if context == :unit
build_unit_details_table_for_unit_pdf(unit, last_inspection)
else
- build_unit_details_table_with_inspection(unit, last_inspection, context)
+ build_unit_details_table_with_inspection(unit, last_inspection)
end
end
@@ -224,15 +222,8 @@ def self.build_unit_details_table_for_unit_pdf(unit, last_inspection)
]
end
- def self.build_unit_details_table_with_inspection(unit, last_inspection, context)
- dimensions_text = build_dimensions_text(last_inspection)
-
- # Get inspector details from current inspection (for inspection PDF) or last inspection (for unit PDF)
- inspection = if context == :inspection
- last_inspection
- else
- unit.last_inspection
- end
+ def self.build_unit_details_table_with_inspection(unit, inspection)
+ dimensions_text = build_dimensions_text(inspection)
inspector_name = inspection&.user&.name
rpii_number = inspection&.user&.rpii_inspector_number
diff --git a/app/services/pdf_performance.rb b/app/services/pdf_performance.rb
new file mode 100644
index 000000000..2b1331fe3
--- /dev/null
+++ b/app/services/pdf_performance.rb
@@ -0,0 +1,36 @@
+# typed: strict
+# frozen_string_literal: true
+
+class PdfPerformance
+ extend T::Sig
+
+ EVENT = "pdf.performance"
+
+ class << self
+ extend T::Sig
+
+ sig do
+ params(
+ stage: Symbol,
+ pdf_type: Symbol,
+ record_id: T.untyped,
+ metadata: T.untyped,
+ block: T.proc.returns(T.untyped)
+ ).returns(T.untyped)
+ end
+ def measure(stage, pdf_type:, record_id:, **metadata, &block)
+ started_at = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+ block.call
+ ensure
+ duration = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started_at
+ Rails.logger.info({
+ event: EVENT,
+ stage: stage,
+ pdf_type: pdf_type,
+ record_id: record_id,
+ duration_ms: (duration * 1000).round(1),
+ **metadata
+ })
+ end
+ end
+end
diff --git a/app/views/units/_form.html.erb b/app/views/units/_form.html.erb
index 914d7f01c..e713571b8 100644
--- a/app/views/units/_form.html.erb
+++ b/app/views/units/_form.html.erb
@@ -1,7 +1,7 @@
<%
- # Determine form context
- unit = @unit || local_assigns[:unit]
- form_url = local_assigns[:form_url] || nil
+ unit = local_assigns.fetch(:unit)
+ form_url = local_assigns[:form_url]
+ prefilled_badge = local_assigns.fetch(:prefilled_badge, false)
%>
<%= render 'chobble_forms/form_context',
@@ -11,10 +11,10 @@
<%= render 'chobble_forms/fieldset', legend_key: 'unit_details' do %>
<%# Show ID field when UNIT_BADGES is enabled %>
<% if Rails.configuration.units.badges_enabled %>
- <% if unit.persisted? || @prefilled_badge %>
+ <% if unit.persisted? || prefilled_badge %>
<%# Display ID as read-only for existing units or pre-filled badges %>
<%= render 'chobble_forms/display_field', field: :id %>
- <% if @prefilled_badge %>
+ <% if prefilled_badge %>
<%= form.hidden_field :id %>
<% end %>
<% else %>
@@ -52,14 +52,13 @@
<%= render 'chobble_forms/field_with_link',
field_type: :text_field,
field: :manufacturer,
- required: true,
link_url: units_path(manufacturer: unit.manufacturer),
link_text: t('units.labels.all_count', count: manufacturer_count) %>
<% else %>
- <%= render 'chobble_forms/text_field', field: :manufacturer, required: true %>
+ <%= render 'chobble_forms/text_field', field: :manufacturer %>
<% end %>
<% else %>
- <%= render 'chobble_forms/text_field', field: :manufacturer, required: true %>
+ <%= render 'chobble_forms/text_field', field: :manufacturer %>
<% end %>
<%= render 'chobble_forms/text_field', field: :serial, required: true %>
@@ -76,7 +75,7 @@
- <%= unit.last_inspection.width %>m × <%= unit.last_inspection.length %>m × <%= unit.last_inspection.height %>m
+ <%= unit.last_inspection.width %><%= t("shared.metres_by") %> <%= unit.last_inspection.length %><%= t("shared.metres_by") %> <%= unit.last_inspection.height %>m
<%= t("units.labels.from_last_inspection") %>
diff --git a/app/views/units/edit.html.erb b/app/views/units/edit.html.erb
index a9187fdd5..4de0a5c0c 100644
--- a/app/views/units/edit.html.erb
+++ b/app/views/units/edit.html.erb
@@ -3,7 +3,7 @@
<%= render 'shared/action_buttons', actions: unit_actions(@unit) %>
-<%= render 'form' %>
+<%= render 'form', unit: @unit %>
<% if @unit.deletable? %>
<%= button_to t('units.buttons.delete'),
unit_path(@unit),
diff --git a/app/views/units/new.html.erb b/app/views/units/new.html.erb
index 36da3112e..e9a41f64a 100644
--- a/app/views/units/new.html.erb
+++ b/app/views/units/new.html.erb
@@ -1,2 +1,2 @@
<%= render 'shared/page_header', title: t('units.titles.new') %>
-<%= render 'form' %>
+<%= render 'form', unit: @unit, prefilled_badge: @prefilled_badge %>
diff --git a/app/views/units/new_from_inspection.html.erb b/app/views/units/new_from_inspection.html.erb
index 477b89ae2..eca763c14 100644
--- a/app/views/units/new_from_inspection.html.erb
+++ b/app/views/units/new_from_inspection.html.erb
@@ -2,6 +2,7 @@
<%= render 'units/form',
unit: @unit,
+ prefilled_badge: false,
form_url: create_unit_from_inspection_path(@inspection) %>
<%= link_to t("shared.buttons.cancel"), inspection_path(@inspection) %>
diff --git a/config/forms/structure_assessment.yml b/config/forms/structure_assessment.yml
index ca4743c12..3c27231c2 100644
--- a/config/forms/structure_assessment.yml
+++ b/config/forms/structure_assessment.yml
@@ -10,6 +10,8 @@ form_fields:
field: :air_loss
- partial: :pass_fail_comment
field: :straight_walls
+ attributes:
+ required: false
- partial: :pass_fail_comment
field: :sharp_edges
- legend_i18n_key: structural_safety
@@ -23,6 +25,7 @@ form_fields:
attributes:
step: 1
min: 0
+ required: false
- partial: :number_pass_fail_comment
field: :platform_height
attributes:
@@ -45,10 +48,12 @@ form_fields:
attributes:
min: 0
add_not_applicable: true
+ required: false
- partial: :integer_comment
field: :trough_adjacent_panel_width
attributes:
min: 0
+ required: false
- partial: :pass_fail_comment
field: :trough
- legend_i18n_key: safety_compliance
diff --git a/config/forms/user_height_assessment.yml b/config/forms/user_height_assessment.yml
index b7d4b3e1e..096eb627d 100644
--- a/config/forms/user_height_assessment.yml
+++ b/config/forms/user_height_assessment.yml
@@ -31,21 +31,25 @@ form_fields:
attributes:
min: 0
step: 1
+ required: false
- partial: :number
field: :users_at_1200mm
attributes:
min: 0
step: 1
+ required: false
- partial: :number
field: :users_at_1500mm
attributes:
min: 0
step: 1
+ required: false
- partial: :number
field: :users_at_1800mm
attributes:
min: 0
step: 1
+ required: false
- legend_i18n_key: tallest_user_capacity
fields:
- partial: :text_area
diff --git a/config/locales/units.en.yml b/config/locales/units.en.yml
index d3618adec..ced24ecea 100644
--- a/config/locales/units.en.yml
+++ b/config/locales/units.en.yml
@@ -115,7 +115,6 @@ en:
validations:
id_blank: ID can't be blank
name_blank: Name can't be blank
- manufacturer_blank: Manufacturer can't be blank
serial_blank: Serial can't be blank
serial: Serial number can't be blank
description_blank: Description can't be blank
diff --git a/db/migrate/20260715120000_remove_manufacturer_serial_index_from_units.rb b/db/migrate/20260715120000_remove_manufacturer_serial_index_from_units.rb
new file mode 100644
index 000000000..e2968a4e8
--- /dev/null
+++ b/db/migrate/20260715120000_remove_manufacturer_serial_index_from_units.rb
@@ -0,0 +1,8 @@
+# typed: false
+
+class RemoveManufacturerSerialIndexFromUnits < ActiveRecord::Migration[8.0]
+ def change
+ remove_index :units, [:manufacturer, :serial],
+ name: "index_units_on_manufacturer_and_serial"
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 081df3b98..c8b21763d 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.1].define(version: 2026_03_25_000003) do
+ActiveRecord::Schema[8.1].define(version: 2026_07_15_120000) do
create_table "active_storage_attachments", force: :cascade do |t|
t.bigint "blob_id", null: false
t.datetime "created_at", null: false
@@ -634,7 +634,6 @@
t.datetime "updated_at", null: false
t.string "user_id", limit: 12, null: false
t.index ["is_seed"], name: "index_units_on_is_seed"
- t.index ["manufacturer", "serial"], name: "index_units_on_manufacturer_and_serial", unique: true
t.index ["serial", "user_id"], name: "index_units_on_serial_and_user_id", unique: true
t.index ["unit_type"], name: "index_units_on_unit_type"
t.index ["user_id"], name: "index_units_on_user_id"
diff --git a/spec/factories/units.rb b/spec/factories/units.rb
index 8240e3e4c..1ef378782 100644
--- a/spec/factories/units.rb
+++ b/spec/factories/units.rb
@@ -20,7 +20,6 @@
# Indexes
#
# index_units_on_is_seed (is_seed)
-# index_units_on_manufacturer_and_serial (manufacturer,serial) UNIQUE
# index_units_on_serial_and_user_id (serial,user_id) UNIQUE
# index_units_on_unit_type (unit_type)
# index_units_on_user_id (user_id)
diff --git a/spec/features/assessment_forms/structure_assessment_trough_fields_spec.rb b/spec/features/assessment_forms/structure_assessment_trough_fields_spec.rb
index 318fc7cac..a9ae7482f 100644
--- a/spec/features/assessment_forms/structure_assessment_trough_fields_spec.rb
+++ b/spec/features/assessment_forms/structure_assessment_trough_fields_spec.rb
@@ -12,11 +12,16 @@
describe "structure assessment form" do
before { visit edit_inspection_path(inspection) }
- it "shows trough fields in structure tab" do
+ scenario "shows optional trough fields in structure tab" do
click_link I18n.t("inspections.tabs.structure")
- expect(page).to have_content("Trough Depth")
- expect(page).to have_content("Trough Adjacent Panel Width")
+ trough_depth = I18n.t("forms.structure.fields.trough_depth")
+ adjacent_width = I18n.t(
+ "forms.structure.fields.trough_adjacent_panel_width"
+ )
+
+ expect(find_field(trough_depth)[:required]).to be_nil
+ expect(find_field(adjacent_width)[:required]).to be_nil
end
it "saves trough field values" do
diff --git a/spec/features/details_links_spec.rb b/spec/features/details_links_spec.rb
index 04d9aa58f..41b9a2d1c 100644
--- a/spec/features/details_links_spec.rb
+++ b/spec/features/details_links_spec.rb
@@ -31,7 +31,8 @@
end
scenario "multiple clicks on links work consistently" do
- inspection.user_height_assessment.update_column(:users_at_1000mm, nil)
+ inspection.user_height_assessment.update_column(:containing_wall_height, nil)
+ visit edit_inspection_path(inspection)
find("summary.incomplete-fields-summary").click
diff --git a/spec/features/inspections/inspection_incomplete_fields_spec.rb b/spec/features/inspections/inspection_incomplete_fields_spec.rb
index 557f60c08..7da040315 100644
--- a/spec/features/inspections/inspection_incomplete_fields_spec.rb
+++ b/spec/features/inspections/inspection_incomplete_fields_spec.rb
@@ -105,7 +105,7 @@
width: 8.0,
inspection_fields: [:inspection_date],
assessment_fields: {
- user_height_assessment: [:users_at_1000mm]
+ user_height_assessment: [:containing_wall_height]
}
)
@@ -115,7 +115,32 @@
expect_incomplete_section("inspection")
expect_incomplete_section("user_height")
expect_incomplete_field("inspection", "inspection_date")
- expect_incomplete_field("user_height", "users_at_1000mm")
+ expect_incomplete_field("user_height", "containing_wall_height")
+ end
+
+ scenario "excludes fields configured as optional for completion" do
+ optional_inspection = create_incomplete_inspection(
+ assessment_fields: {
+ structure_assessment: %i[
+ step_ramp_size
+ step_ramp_size_pass
+ straight_walls_pass
+ trough_adjacent_panel_width
+ trough_depth
+ ],
+ user_height_assessment: %i[
+ users_at_1000mm
+ users_at_1200mm
+ users_at_1500mm
+ users_at_1800mm
+ ]
+ }
+ )
+
+ visit edit_inspection_path(optional_inspection)
+
+ expect(page).not_to have_css("details.incomplete-fields-details")
+ expect(page).to have_button(I18n.t("inspections.buttons.mark_complete"))
end
scenario "does not show incomplete fields when truly complete" do
diff --git a/spec/features/inspections/turbo_incomplete_fields_spec.rb b/spec/features/inspections/turbo_incomplete_fields_spec.rb
index d98f7587b..cef151496 100644
--- a/spec/features/inspections/turbo_incomplete_fields_spec.rb
+++ b/spec/features/inspections/turbo_incomplete_fields_spec.rb
@@ -12,7 +12,7 @@
create(:inspection, :completed, user:, unit:).tap do |insp|
insp.update_columns(complete_date: nil)
# Make an assessment field incomplete to have something to test with
- insp.user_height_assessment.update_column(:users_at_1000mm, nil)
+ insp.user_height_assessment.update_column(:containing_wall_height, nil)
end
end
@@ -28,7 +28,7 @@
expect(summary.text).to match(/Show 1 incomplete field/)
visit edit_inspection_path(inspection, tab: "user_height")
- height_label = I18n.t("forms.user_height.fields.users_at_1000mm")
+ height_label = I18n.t("forms.user_height.fields.containing_wall_height")
fill_in height_label, with: "2"
submit_form :user_height
diff --git a/spec/features/pdfs/pdf_integration_spec.rb b/spec/features/pdfs/pdf_integration_spec.rb
index 7dd668486..da8816c58 100644
--- a/spec/features/pdfs/pdf_integration_spec.rb
+++ b/spec/features/pdfs/pdf_integration_spec.rb
@@ -15,6 +15,37 @@
sign_in user
end
+ scenario "records PDF generation performance timings" do
+ performance_stages = []
+ allow(Rails.logger).to receive(:info) do |message|
+ if message.is_a?(Hash) && message[:event] == "pdf.performance"
+ performance_stages << message[:stage]
+ end
+ end
+
+ visit inspection_path(inspection, format: :pdf)
+
+ expect(page.source).to start_with("%PDF")
+ expect(Rails.logger).to have_received(:info).with(
+ hash_including(
+ event: "pdf.performance",
+ stage: :document_build,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ )
+ )
+ expect(Rails.logger).to have_received(:info).with(
+ hash_including(
+ event: "pdf.performance",
+ stage: :total,
+ pdf_type: :inspection,
+ record_id: inspection.id
+ )
+ )
+ expect(performance_stages.index(:access_tracking))
+ .to be < performance_stages.index(:total)
+ end
+
scenario "generates comprehensive PDF with complete field coverage" do
visit inspection_path(inspection, format: :pdf)
diff --git a/spec/features/units/units_form_spec.rb b/spec/features/units/units_form_spec.rb
index 8f912f26d..88603ad2f 100644
--- a/spec/features/units/units_form_spec.rb
+++ b/spec/features/units/units_form_spec.rb
@@ -2,7 +2,7 @@
require "rails_helper"
-RSpec.describe "Units Form", type: :feature do
+RSpec.feature "Units Form", type: :feature do
let(:user) { create(:user) }
before do
@@ -14,7 +14,7 @@
visit "/units/new"
end
- it "successfully creates a unit with valid data" do
+ scenario "successfully creates a unit with valid data" do
if Rails.configuration.units.badges_enabled
badge = create(:badge, badge_batch: create(:badge_batch))
fill_in_form :units, :id, badge.id
@@ -41,13 +41,38 @@
expect(page).not_to have_content(I18n.t("events.messages.view_changes"))
end
- it "shows validation errors for missing required fields" do
+ scenario "creates a unit without optional manufacture details" do
+ if Rails.configuration.units.badges_enabled
+ badge = create(:badge, badge_batch: create(:badge_batch))
+ fill_in_form :units, :id, badge.id
+ end
+
+ fill_in_form :units, :name, "Unit without manufacture details"
+ fill_in_form :units, :serial, "OPTIONAL-001"
+ fill_in_form :units, :description, "Manufacture details are unknown"
+
+ submit_form :units
+
+ unit = Unit.find_by!(serial: "OPTIONAL-001")
+ expect(page).to have_current_path(unit_path(unit))
+ expect(unit.manufacture_date).to be_nil
+ expect(unit.manufacturer).to be_blank
+ end
+
+ scenario "does not mark manufacture details as required" do
+ manufacture_date = I18n.t("forms.units.fields.manufacture_date")
+ manufacturer = I18n.t("forms.units.fields.manufacturer")
+
+ expect(find_field(manufacture_date)[:required]).to be_nil
+ expect(find_field(manufacturer)[:required]).to be_nil
+ end
+
+ scenario "shows validation errors for missing required fields" do
submit_form :units
- expected_count = Rails.configuration.units.badges_enabled ? 5 : 4
+ expected_count = Rails.configuration.units.badges_enabled ? 4 : 3
expect_form_errors :units, count: expected_count
expect(page).to have_content(I18n.t("units.validations.name_blank"))
- expect(page).to have_content(I18n.t("units.validations.manufacturer_blank"))
expect(page).to have_content(I18n.t("units.validations.serial_blank"))
expect(page).to have_content(I18n.t("units.validations.description_blank"))
@@ -56,7 +81,7 @@
end
end
- it "validates serial uniqueness per user" do
+ scenario "validates serial uniqueness per user" do
create(:unit, user: user, serial: "DUPLICATE-001")
fill_in_form :units, :name, "Test Unit"
@@ -77,13 +102,13 @@
visit edit_unit_path(unit)
end
- it "populates form with existing unit data" do
+ scenario "populates form with existing unit data" do
expect(page).to have_field(I18n.t("forms.units.fields.name"), with: unit.name)
expect(page).to have_field(I18n.t("forms.units.fields.manufacturer"), with: unit.manufacturer)
expect(page).to have_button(I18n.t("forms.units.submit"))
end
- it "successfully updates unit with new data and creates audit log" do
+ scenario "successfully updates unit with new data and creates audit log" do
fill_in_form :units, :name, "Updated Name"
fill_in_form :units, :description, "Updated description"
diff --git a/spec/lib/form_yaml_database_schema_spec.rb b/spec/lib/form_yaml_database_schema_spec.rb
index 42e3b1242..8bc4e5544 100644
--- a/spec/lib/form_yaml_database_schema_spec.rb
+++ b/spec/lib/form_yaml_database_schema_spec.rb
@@ -39,17 +39,17 @@
# Define allowed attributes for each partial type
PARTIAL_ALLOWED_ATTRIBUTES = {
- # Number fields can have step, min, max
- number: %i[step min max],
- number_pass_fail_comment: %i[step min max],
+ # Number fields can have presentation and required metadata
+ number: %i[max min required step],
+ number_pass_fail_comment: %i[max min required step],
number_pass_fail_na_comment: %i[step min max],
decimal_comment: %i[step min max],
- integer_comment: %i[step min max add_not_applicable],
+ integer_comment: %i[add_not_applicable max min required step],
# Most partials don't allow any attributes
text_field: [],
text_area: [],
pass_fail: [],
- pass_fail_comment: [],
+ pass_fail_comment: %i[required],
pass_fail_na_comment: [],
checkbox: [],
yes_no_radio: [],
diff --git a/spec/models/assessment_schema_spec.rb b/spec/models/assessment_schema_spec.rb
index e1f03605c..9334e4d91 100644
--- a/spec/models/assessment_schema_spec.rb
+++ b/spec/models/assessment_schema_spec.rb
@@ -120,6 +120,21 @@
expect(ic_schema.find_field(:city)).not_to be_required
end
+ it "detects fields explicitly optional for completion" do
+ optional = AssessmentSchema::Field.new(
+ field: :optional,
+ partial: :number,
+ attributes: {required: false}
+ )
+ unspecified = AssessmentSchema::Field.new(
+ field: :unspecified,
+ partial: :number
+ )
+
+ expect(optional).to be_optional_for_completion
+ expect(unspecified).not_to be_optional_for_completion
+ end
+
it "computes composite fields via ChobbleForms::FieldUtils" do
field = schema.find_field(:num_low_anchors)
expect(field.composite_fields)
@@ -176,6 +191,28 @@
end
end
+ describe "#completion_optional_fields" do
+ it "includes the persisted fields represented by optional form fields" do
+ optional = AssessmentSchema::Field.new(
+ field: :measurement,
+ partial: :number_pass_fail_comment,
+ attributes: {required: false}
+ )
+ required = AssessmentSchema::Field.new(
+ field: :check,
+ partial: :pass_fail_comment
+ )
+ fieldset = AssessmentSchema::Fieldset.new(:section, [optional, required])
+ schema = described_class.new("custom", [fieldset])
+
+ expect(schema.completion_optional_fields).to contain_exactly(
+ :measurement,
+ :measurement_pass,
+ :measurement_comment
+ )
+ end
+ end
+
describe "#exclude" do
let(:schema) { AssessmentSchema.for(InspectorCompany) }
diff --git a/spec/models/concerns/assessment_completion_spec.rb b/spec/models/concerns/assessment_completion_spec.rb
index 1a92cc927..b39a42d2f 100644
--- a/spec/models/concerns/assessment_completion_spec.rb
+++ b/spec/models/concerns/assessment_completion_spec.rb
@@ -61,6 +61,35 @@
expect(assessment).to be_complete
end
end
+
+ context "with fields configured as optional" do
+ let(:assessment) { inspection.structure_assessment }
+
+ it "does not return their values or checks as incomplete" do
+ optional_fields = %i[
+ step_ramp_size
+ step_ramp_size_pass
+ straight_walls_pass
+ trough_adjacent_panel_width
+ trough_depth
+ ]
+
+ expect(assessment.incomplete_fields).not_to include(*optional_fields)
+ end
+ end
+
+ context "with optional user count fields" do
+ it "does not return blank height counts as incomplete" do
+ optional_fields = %i[
+ users_at_1000mm
+ users_at_1200mm
+ users_at_1500mm
+ users_at_1800mm
+ ]
+
+ expect(assessment.incomplete_fields).not_to include(*optional_fields)
+ end
+ end
end
describe "#incomplete_fields_grouped" do
diff --git a/spec/models/inspection_spec.rb b/spec/models/inspection_spec.rb
index e1fd9c8bd..c5be50d48 100644
--- a/spec/models/inspection_spec.rb
+++ b/spec/models/inspection_spec.rb
@@ -540,7 +540,9 @@
describe "#invalidate_pdf_cache" do
it "does not invalidate cache when only pdf_last_accessed_at changes" do
+ inspection
expect(PdfCacheService).not_to receive(:invalidate_inspection_cache)
+ expect(PdfCacheService).not_to receive(:invalidate_unit_cache)
inspection.update!(pdf_last_accessed_at: Time.current)
end
@@ -553,7 +555,9 @@
end
it "invalidates cache when other attributes change" do
+ inspection
expect(PdfCacheService).to receive(:invalidate_inspection_cache).with(inspection)
+ expect(PdfCacheService).to receive(:invalidate_unit_cache).with(inspection.unit)
inspection.update!(risk_assessment: "Updated risk assessment")
end
diff --git a/spec/models/unit_spec.rb b/spec/models/unit_spec.rb
index b595786f2..8b828f1cb 100644
--- a/spec/models/unit_spec.rb
+++ b/spec/models/unit_spec.rb
@@ -20,7 +20,6 @@
# Indexes
#
# index_units_on_is_seed (is_seed)
-# index_units_on_manufacturer_and_serial (manufacturer,serial) UNIQUE
# index_units_on_serial_and_user_id (serial,user_id) UNIQUE
# index_units_on_unit_type (unit_type)
# index_units_on_user_id (user_id)
@@ -52,14 +51,20 @@
end
describe "validations" do
- it "validates presence of all required fields" do
+ it "validates presence of required fields" do
unit = build(:unit, user: user, name: nil, serial: nil,
description: nil, manufacturer: nil)
expect(unit).not_to be_valid
expect(unit.errors[:name]).to be_present
expect(unit.errors[:serial]).to be_present
expect(unit.errors[:description]).to be_present
- expect(unit.errors[:manufacturer]).to be_present
+ expect(unit.errors[:manufacturer]).to be_empty
+ end
+
+ it "allows manufacture details to be blank" do
+ unit = build(:unit, user:, manufacture_date: nil, manufacturer: nil)
+
+ expect(unit).to be_valid
end
it "validates serial uniqueness within user" do
@@ -77,6 +82,16 @@
unit2 = build(:unit, user: user2, manufacturer: "TestCorp", serial: "ABC123")
expect(unit2).to be_valid
end
+
+ it "allows same serial and blank manufacturer for different users" do
+ user1 = create(:user)
+ user2 = create(:user)
+ create(:unit, user: user1, manufacturer: "", serial: "ABC123")
+
+ unit2 = create(:unit, user: user2, manufacturer: "", serial: "ABC123")
+
+ expect(unit2).to be_persisted
+ end
end
describe "custom ID generation" do
@@ -205,7 +220,6 @@
unit = build(:unit, user: user, manufacturer: nil, serial: nil)
expect(unit).not_to be_valid
- expect(unit.errors[:manufacturer]).to be_present
expect(unit.errors[:serial]).to be_present
end
diff --git a/spec/requests/inspections/inspections_spec.rb b/spec/requests/inspections/inspections_spec.rb
index 957b70516..1b5666a89 100644
--- a/spec/requests/inspections/inspections_spec.rb
+++ b/spec/requests/inspections/inspections_spec.rb
@@ -180,6 +180,8 @@
it "serves PDF" do
allow(PdfGeneratorService).to receive(:generate_inspection_report).and_return(double(render: "PDF"))
+ inspection
+ expect(PdfCacheService).not_to receive(:invalidate_unit_cache)
get "/inspections/#{inspection.id}.pdf"
expect(response).to have_http_status(:success)
diff --git a/spec/seeds/seeds_spec.rb b/spec/seeds/seeds_spec.rb
index baab9e1bb..c1038dbf7 100644
--- a/spec/seeds/seeds_spec.rb
+++ b/spec/seeds/seeds_spec.rb
@@ -23,6 +23,10 @@
load Rails.root.join("db/seeds.rb")
end
+ after(:all) do
+ DatabaseCleaner.clean_with(:truncation)
+ end
+
describe "Inspector Companies" do
it "creates expected number of inspector companies" do
expect(InspectorCompany.count).to eq(3)
diff --git a/spec/services/pdf_generator_service/image_processor_spec.rb b/spec/services/pdf_generator_service/image_processor_spec.rb
index cd88be52f..5b1bd98b8 100644
--- a/spec/services/pdf_generator_service/image_processor_spec.rb
+++ b/spec/services/pdf_generator_service/image_processor_spec.rb
@@ -55,6 +55,12 @@
it "respects column count parameter" do
expect { described_class.add_unit_photo_footer(pdf, unit_with_photo, 4) }.not_to raise_error
end
+
+ it "returns the rendered photo height" do
+ height = described_class.add_unit_photo_footer(pdf, unit_with_photo)
+
+ expect(height).to be_positive
+ end
end
context "with unit without photo" do
@@ -86,6 +92,9 @@
describe "integration with real PDF generation" do
it "generates complete PDF with QR code and photos" do
pdf_content = nil
+ photo_blob = unit_with_photo.photo.blob
+ expect(photo_blob).to receive(:download).once.and_call_original
+ allow(Rails.logger).to receive(:info)
expect {
pdf_content = PdfGeneratorService.generate_inspection_report(inspection_with_photo).render
@@ -93,6 +102,14 @@
expect(pdf_content).to be_a(String)
expect(pdf_content[0..3]).to eq("%PDF")
+ expect(Rails.logger).to have_received(:info).with(
+ hash_including(
+ event: "pdf.performance",
+ stage: :image_download,
+ pdf_type: :inspection,
+ record_id: inspection_with_photo.id
+ )
+ )
end
it "generates complete PDF without photos" do
diff --git a/spec/services/pdf_generator_service/photos_renderer_spec.rb b/spec/services/pdf_generator_service/photos_renderer_spec.rb
index 6dd1c7470..f8c8eb8df 100644
--- a/spec/services/pdf_generator_service/photos_renderer_spec.rb
+++ b/spec/services/pdf_generator_service/photos_renderer_spec.rb
@@ -317,8 +317,8 @@
allow(described_class).to receive(:add_photo_label)
end
- it "downloads and processes the image" do
- expect(photo_blob).to receive(:download)
+ it "delegates the image download to the image processor" do
+ expect(photo_blob).not_to receive(:download)
expect(PdfGeneratorService::ImageProcessor).to receive(:process_image_with_orientation).with(photo_attached_one)
described_class.render_photo(pdf, photo_attached_one, "Test Label", 300)
end
diff --git a/spec/services/pdf_generator_service/table_builder_spec.rb b/spec/services/pdf_generator_service/table_builder_spec.rb
index 546d0b3f6..2e662597e 100644
--- a/spec/services/pdf_generator_service/table_builder_spec.rb
+++ b/spec/services/pdf_generator_service/table_builder_spec.rb
@@ -33,7 +33,11 @@
end
it "formats unit data into 4x4 table structure" do
- result = described_class.build_unit_details_table(unit, :inspection)
+ result = described_class.build_unit_details_table(
+ unit,
+ :inspection,
+ inspection
+ )
expect(result).to be_an(Array)
expect(result.length).to eq(4)
@@ -41,7 +45,11 @@
end
it "includes key unit information" do
- result = described_class.build_unit_details_table(unit, :inspection)
+ result = described_class.build_unit_details_table(
+ unit,
+ :inspection,
+ inspection
+ )
flattened = result.flatten
expect(flattened).to include(unit.name)
@@ -52,10 +60,13 @@
end
it "includes dimensions when available" do
- result = described_class.build_unit_details_table(unit, :inspection)
+ result = described_class.build_unit_details_table(
+ unit,
+ :inspection,
+ inspection
+ )
dimensions_text = result[2][1] # Size field
- # Just verify dimensions field exists - depends on unit.last_inspection
expect(dimensions_text).to be_a(String)
end
end
@@ -64,7 +75,7 @@
let(:unit) { create(:unit) }
it "formats unit data into 2-column table structure" do
- result = described_class.build_unit_details_table(unit, :unit)
+ result = described_class.build_unit_details_table(unit, :unit, nil)
expect(result).to be_an(Array)
expect(result.length).to eq(5)
@@ -78,7 +89,11 @@
end
it "handles missing fields with empty values" do
- result = described_class.build_unit_details_table(unit, "inspection")
+ result = described_class.build_unit_details_table(
+ unit,
+ "inspection",
+ nil
+ )
expect(result[0][1]).to eq("") # empty name (truncate_text converts nil to "")
expect(result[1][1]).to be_nil # nil description
diff --git a/spec/services/pdf_generator_service_spec.rb b/spec/services/pdf_generator_service_spec.rb
index 99dcfb01e..0f55488e8 100644
--- a/spec/services/pdf_generator_service_spec.rb
+++ b/spec/services/pdf_generator_service_spec.rb
@@ -158,6 +158,39 @@
"shared.pass_pdf",
"shared.fail_pdf")
end
+
+ it "reuses the inspection loaded for the history" do
+ expect(unit).not_to receive(:last_inspection)
+
+ PdfGeneratorService.generate_unit_report(unit).render
+ end
+
+ it "uses the most recently completed inspection for unit details" do
+ passed_inspection.update_columns(
+ complete_date: 1.day.ago,
+ inspection_date: 2.days.ago
+ )
+ failed_inspection.update_columns(
+ complete_date: 2.days.ago,
+ inspection_date: 1.day.ago
+ )
+ header_generator = PdfGeneratorService::HeaderGenerator
+ expect(header_generator).to receive(:generate_unit_pdf_header).with(
+ anything,
+ unit,
+ last_inspection: passed_inspection,
+ unbranded: anything
+ ).and_call_original
+ expect(described_class).to receive(
+ :generate_unit_details_with_inspection
+ ).with(
+ anything,
+ unit,
+ passed_inspection
+ ).and_call_original
+
+ PdfGeneratorService.generate_unit_report(unit).render
+ end
end
context "with missing manufacturer" do
diff --git a/spec/services/pdf_performance_spec.rb b/spec/services/pdf_performance_spec.rb
new file mode 100644
index 000000000..56b5d019f
--- /dev/null
+++ b/spec/services/pdf_performance_spec.rb
@@ -0,0 +1,51 @@
+# typed: false
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe PdfPerformance do
+ describe ".measure" do
+ it "returns the measured block result and logs its duration" do
+ allow(Process).to receive(:clock_gettime).and_return(10.0, 10.125)
+ allow(Rails.logger).to receive(:info)
+
+ result = described_class.measure(
+ :document_build,
+ pdf_type: :inspection,
+ record_id: "inspection-id",
+ cached: false
+ ) { :result }
+
+ expect(result).to eq(:result)
+ expect(Rails.logger).to have_received(:info).with({
+ event: "pdf.performance",
+ stage: :document_build,
+ pdf_type: :inspection,
+ record_id: "inspection-id",
+ duration_ms: 125.0,
+ cached: false
+ })
+ end
+
+ it "logs timing and reraises errors from the measured block" do
+ allow(Process).to receive(:clock_gettime).and_return(20.0, 20.01)
+ allow(Rails.logger).to receive(:info)
+
+ expect {
+ described_class.measure(
+ :document_render,
+ pdf_type: :unit,
+ record_id: "unit-id"
+ ) { raise "render failed" }
+ }.to raise_error("render failed")
+
+ expect(Rails.logger).to have_received(:info).with(
+ hash_including(
+ event: "pdf.performance",
+ stage: :document_render,
+ duration_ms: 10.0
+ )
+ )
+ end
+ end
+end
diff --git a/spec/support/test_translations.en.yml b/spec/support/test_translations.en.yml
new file mode 100644
index 000000000..6a33fbe53
--- /dev/null
+++ b/spec/support/test_translations.en.yml
@@ -0,0 +1,7 @@
+---
+en:
+ test:
+ access_denied: Access denied
+ admin_emails_pattern: "^admin\\d*@example\\.com$"
+ invalid_password: wrongpassword
+ password: password123
diff --git a/spec/support/test_translations.rb b/spec/support/test_translations.rb
index fc5b68c54..4fa949ce8 100644
--- a/spec/support/test_translations.rb
+++ b/spec/support/test_translations.rb
@@ -1,13 +1,6 @@
# typed: false
-# Test-specific translations that are only used in specs
-# These are not part of the production application
-
-I18n.backend.store_translations(:en, {
- test: {
- password: "password123",
- access_denied: "Access denied",
- invalid_password: "wrongpassword",
- admin_emails_pattern: "^admin\\d*@example\\.com$"
- }
-})
+# Test-specific translations must survive I18n.backend.reload! calls.
+test_translations = Rails.root.join("spec/support/test_translations.en.yml")
+I18n.load_path << test_translations.to_s
+I18n.backend.load_translations(test_translations)