From d0f024cf8d80cdec51f896d165091341b8fe1a30 Mon Sep 17 00:00:00 2001 From: Bernardo Anderson Date: Tue, 29 Jul 2025 10:01:30 -0500 Subject: [PATCH] CP-10288 - Attempt another fix --- app/controllers/api/templates_controller.rb | 117 +++++++++--------- app/services/export_service.rb | 10 +- .../20250702193415_create_export_locations.rb | 2 + ...317_add_auth_params_to_export_locations.rb | 2 + ...6_add_external_data_fields_to_templates.rb | 2 + ...ubmissions_endpoint_to_export_locations.rb | 2 + .../export_submission_service_spec.rb | 16 ++- spec/services/export_template_service_spec.rb | 19 +-- spec/system/setup_spec.rb | 9 +- 9 files changed, 101 insertions(+), 78 deletions(-) diff --git a/app/controllers/api/templates_controller.rb b/app/controllers/api/templates_controller.rb index d087b980..aed33d35 100644 --- a/app/controllers/api/templates_controller.rb +++ b/app/controllers/api/templates_controller.rb @@ -90,66 +90,20 @@ module Api end def pdf - template = Template.new - template.account = current_account - template.author = current_user - template.folder = TemplateFolders.find_or_create_by_name(current_user, params[:folder_name]) - template.name = params[:name] || 'Untitled Template' - template.external_id = params[:external_id] if params[:external_id].present? - template.source = :api - - # Set submitters if provided - if params[:submitters].present? - template.submitters = params[:submitters] - end - - # Set fields if provided - if params[:fields].present? - # We'll set fields after documents are processed to ensure correct attachment_uuid mapping - fields_from_request = params[:fields] - end + template = build_template + fields_from_request = params[:fields] if params[:fields].present? template.save! begin documents = process_documents(template, params[:documents]) + schema = build_schema(documents) - schema = documents.map { |doc| { attachment_uuid: doc.uuid, name: doc.filename.base } } - - if template.fields.blank? - if fields_from_request.present? - # Map the fields to use the correct attachment_uuid from the processed documents - mapped_fields = fields_from_request.map do |field| - field_copy = field.dup - if field_copy['areas'].present? - field_copy['areas'] = field_copy['areas'].map do |area| - area_copy = area.dup - # Use the first document's UUID since we're processing one document at a time - area_copy['attachment_uuid'] = documents.first.uuid if documents.any? - area_copy - end - end - field_copy - end - template.fields = mapped_fields - else - template.fields = Templates::ProcessDocument.normalize_attachment_fields(template, documents) - schema.each { |item| item['pending_fields'] = true } if template.fields.present? - end - end + set_template_fields(template, fields_from_request, documents, schema) if template.fields.blank? template.update!(schema: schema) - enqueue_template_created_webhooks(template) - - SearchEntries.enqueue_reindex(template) - - # Get the documents for serialization - template_documents = template.documents.where(uuid: documents.map(&:uuid)) - - result = Templates::SerializeForApi.call(template, template_documents) - - render json: result + finalize_template_creation(template, documents) rescue StandardError => e template.destroy! raise e @@ -166,20 +120,16 @@ module Api def process_documents(template, documents_params) return [] if documents_params.blank? - documents_params.map.with_index do |doc_param, index| - expected_length = (doc_param[:file].length / 4.0 * 3).ceil + documents_params.map.with_index do |doc_param, _index| + (doc_param[:file].length / 4.0 * 3).ceil # Validate base64 string - unless doc_param[:file].match?(/\A[A-Za-z0-9+\/]*={0,2}\z/) - raise ArgumentError, "Invalid base64 string format" - end + raise ArgumentError, 'Invalid base64 string format' unless doc_param[:file].match?(%r{\A[A-Za-z0-9+/]*={0,2}\z}) # Decode base64 file data file_data = Base64.decode64(doc_param[:file]) # Check if the decoded data looks like a PDF - if file_data.size >= 4 - pdf_header = file_data[0..3] - end + file_data[0..3] if file_data.size >= 4 # Create a temporary file-like object file = Tempfile.new(['document', '.pdf']) @@ -199,6 +149,55 @@ module Api end end + def build_template + template = Template.new + template.account = current_account + template.author = current_user + template.folder = TemplateFolders.find_or_create_by_name(current_user, params[:folder_name]) + template.name = params[:name] || 'Untitled Template' + template.external_id = params[:external_id] if params[:external_id].present? + template.source = :api + template.submitters = params[:submitters] if params[:submitters].present? + template + end + + def build_schema(documents) + documents.map { |doc| { attachment_uuid: doc.uuid, name: doc.filename.base } } + end + + def set_template_fields(template, fields_from_request, documents, schema) + if fields_from_request.present? + template.fields = map_request_fields_to_documents(fields_from_request, documents) + else + template.fields = Templates::ProcessDocument.normalize_attachment_fields(template, documents) + schema.each { |item| item['pending_fields'] = true } if template.fields.present? + end + end + + def map_request_fields_to_documents(fields_from_request, documents) + fields_from_request.map do |field| + field_copy = field.dup + if field_copy['areas'].present? + field_copy['areas'] = field_copy['areas'].map do |area| + area_copy = area.dup + area_copy['attachment_uuid'] = documents.first.uuid if documents.any? + area_copy + end + end + field_copy + end + end + + def finalize_template_creation(template, documents) + enqueue_template_created_webhooks(template) + SearchEntries.enqueue_reindex(template) + + template_documents = template.documents.where(uuid: documents.map(&:uuid)) + result = Templates::SerializeForApi.call(template, template_documents) + + render json: result + end + def enqueue_template_created_webhooks(template) WebhookUrls.for_account_id(template.account_id, 'template.created').each do |webhook_url| SendTemplateCreatedWebhookRequestJob.perform_async('template_id' => template.id, diff --git a/app/services/export_service.rb b/app/services/export_service.rb index d0737b1f..24c54c73 100644 --- a/app/services/export_service.rb +++ b/app/services/export_service.rb @@ -3,12 +3,16 @@ require 'faraday' class ExportService - attr_accessor :error_message + attr_reader :error_message def initialize @error_message = nil end + def set_error(message) + @error_message = message + end + protected def api_connection @@ -36,4 +40,8 @@ class ExportService def export_location @export_location ||= ExportLocation.default_location end + + def set_error(message) + @error_message = message + end end diff --git a/db/migrate/20250702193415_create_export_locations.rb b/db/migrate/20250702193415_create_export_locations.rb index 99cbac76..cec2d390 100644 --- a/db/migrate/20250702193415_create_export_locations.rb +++ b/db/migrate/20250702193415_create_export_locations.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class CreateExportLocations < ActiveRecord::Migration[8.0] def change create_table :export_locations do |t| diff --git a/db/migrate/20250702204317_add_auth_params_to_export_locations.rb b/db/migrate/20250702204317_add_auth_params_to_export_locations.rb index b452c1e7..32582333 100644 --- a/db/migrate/20250702204317_add_auth_params_to_export_locations.rb +++ b/db/migrate/20250702204317_add_auth_params_to_export_locations.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddAuthParamsToExportLocations < ActiveRecord::Migration[8.0] def change add_column :export_locations, :extra_params, :jsonb, null: false, default: {} diff --git a/db/migrate/20250703143236_add_external_data_fields_to_templates.rb b/db/migrate/20250703143236_add_external_data_fields_to_templates.rb index 75861b27..5127beff 100644 --- a/db/migrate/20250703143236_add_external_data_fields_to_templates.rb +++ b/db/migrate/20250703143236_add_external_data_fields_to_templates.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddExternalDataFieldsToTemplates < ActiveRecord::Migration[8.0] def change add_column :templates, :external_data_fields, :text diff --git a/db/migrate/20250708172115_add_submissions_endpoint_to_export_locations.rb b/db/migrate/20250708172115_add_submissions_endpoint_to_export_locations.rb index 2f3a31f8..62ec8c1a 100644 --- a/db/migrate/20250708172115_add_submissions_endpoint_to_export_locations.rb +++ b/db/migrate/20250708172115_add_submissions_endpoint_to_export_locations.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class AddSubmissionsEndpointToExportLocations < ActiveRecord::Migration[8.0] def change add_column :export_locations, :submissions_endpoint, :string diff --git a/spec/services/export_submission_service_spec.rb b/spec/services/export_submission_service_spec.rb index 2c645fcb..6b805ba4 100644 --- a/spec/services/export_submission_service_spec.rb +++ b/spec/services/export_submission_service_spec.rb @@ -41,7 +41,7 @@ RSpec.describe ExportSubmissionService do end context 'when export location is properly configured' do - let(:request_double) { double(body: nil) } + let(:request_double) { instance_double(Faraday::Request, body: nil) } before do allow(request_double).to receive(:body=) @@ -58,8 +58,9 @@ RSpec.describe ExportSubmissionService do end it 'makes API call with correct endpoint' do - expect(faraday_connection).to receive(:post).with(export_location.submissions_endpoint) + allow(faraday_connection).to receive(:post).with(export_location.submissions_endpoint) service.call + expect(faraday_connection).to have_received(:post).with(export_location.submissions_endpoint) end end @@ -99,12 +100,14 @@ RSpec.describe ExportSubmissionService do it 'logs the error' do allow(Rails.logger).to receive(:error) service.call + expect(Rails.logger).to have_received(:error) end it 'reports to Rollbar if available' do stub_const('Rollbar', double) allow(Rollbar).to receive(:error) service.call + expect(Rollbar).to have_received(:error) end end @@ -121,6 +124,7 @@ RSpec.describe ExportSubmissionService do it 'logs the error' do allow(Rails.logger).to receive(:error) service.call + expect(Rails.logger).to have_received(:error) end it 'reports to Rollbar if available' do @@ -129,12 +133,13 @@ RSpec.describe ExportSubmissionService do allow(ExportLocation).to receive(:default_location).and_raise(error) allow(Rollbar).to receive(:error) service.call + expect(Rollbar).to have_received(:error).with(error) end end end describe 'payload building' do - let(:request_double) { instance_double(request, body: nil) } + let(:request_double) { instance_double(Faraday::Request, body: nil) } before do allow(request_double).to receive(:body=) @@ -179,11 +184,10 @@ RSpec.describe ExportSubmissionService do end describe 'extra_params handling' do - let(:extra_params) { { 'api_key' => 'test_key', 'version' => '1.0' } } - let(:request_double) { instance_double(request, body: nil) } + let(:request_double) { instance_double(Faraday::Request, body: nil) } before do - allow(export_location).to receive(:extra_params).and_return(extra_params) + allow(export_location).to receive(:extra_params).and_return({ 'api_key' => 'test_key', 'version' => '1.0' }) allow(request_double).to receive(:body=) allow(faraday_connection).to receive(:post).and_yield(request_double).and_return(faraday_response) allow(faraday_response).to receive(:success?).and_return(true) diff --git a/spec/services/export_template_service_spec.rb b/spec/services/export_template_service_spec.rb index 4147a449..77e673e5 100644 --- a/spec/services/export_template_service_spec.rb +++ b/spec/services/export_template_service_spec.rb @@ -2,6 +2,10 @@ require 'rails_helper' +class Rollbar + def self.error(message); end +end + RSpec.describe ExportTemplateService do let(:export_location) { create(:export_location, :default) } let(:data) { { template: { name: 'Test Template' } } } @@ -62,10 +66,9 @@ RSpec.describe ExportTemplateService do end it 'reports to Rollbar if available' do - rollbar_spy = instance_spy(Rollbar) - stub_const('Rollbar', rollbar_spy) + allow(Rollbar).to receive(:error) service.call - expect(rollbar_spy).to have_received(:error).with("#{export_location.name} template export API error: 422") + expect(Rollbar).to have_received(:error).with("#{export_location.name} template export API error: 422") end end @@ -97,10 +100,9 @@ RSpec.describe ExportTemplateService do end it 'reports to Rollbar if available' do - rollbar_spy = instance_spy(Rollbar) - stub_const('Rollbar', rollbar_spy) + allow(Rollbar).to receive(:error) service.call - expect(rollbar_spy).to have_received(:error).with('Failed to export template: Connection failed') + expect(Rollbar).to have_received(:error).with('Failed to export template: Connection failed') end end @@ -121,12 +123,11 @@ RSpec.describe ExportTemplateService do end it 'reports to Rollbar if available' do - rollbar_spy = instance_spy(Rollbar) - stub_const('Rollbar', rollbar_spy) + allow(Rollbar).to receive(:error) error = StandardError.new('Database error') allow(ExportLocation).to receive(:default_location).and_raise(error) service.call - expect(rollbar_spy).to have_received(:error).with(error) + expect(Rollbar).to have_received(:error).with(error) end end end diff --git a/spec/system/setup_spec.rb b/spec/system/setup_spec.rb index ff419870..7365f60c 100644 --- a/spec/system/setup_spec.rb +++ b/spec/system/setup_spec.rb @@ -16,7 +16,8 @@ RSpec.describe 'App Setup' do visit setup_index_path end - xit 'shows the setup page', reason: 'Pending implementation' do + it 'shows the setup page' do + skip 'Pending implementation' expect(page).to have_content('Initial Setup') ['First name', 'Last name', 'Email', 'Company name', 'Password', 'App URL'].each do |field| @@ -25,7 +26,8 @@ RSpec.describe 'App Setup' do end context 'when valid information' do - xit 'setups the app', reason: 'Pending implementation' do + it 'setups the app' do + skip 'Pending implementation' fill_setup_form(form_data) expect do @@ -51,7 +53,8 @@ RSpec.describe 'App Setup' do end context 'when invalid information' do - xit 'does not setup the app if the email is invalid', reason: 'Pending implementation' do + it 'does not setup the app if the email is invalid' do + skip 'Pending implementation' fill_setup_form(form_data.merge(email: 'bob@example-com')) expect do