CP-10288 - Attempt another fix

app/controllers/api/templates_controller.rb

@@ -90,66 +90,20 @@ module Api
     end
 
     def pdf
-      template = Template.new
+      template = build_template
-      template.account = current_account
+      fields_from_request = params[:fields] if params[:fields].present?
-      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.save!
 
       begin
         documents = process_documents(template, params[:documents])
+        schema = build_schema(documents)
 
-        schema = documents.map { |doc| { attachment_uuid: doc.uuid, name: doc.filename.base } }
+        set_template_fields(template, fields_from_request, documents, schema) if template.fields.blank?
-
-        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
 
         template.update!(schema: schema)
 
-        enqueue_template_created_webhooks(template)
+        finalize_template_creation(template, documents)
-
-        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
       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|
+      documents_params.map.with_index do |doc_param, _index|
-        expected_length = (doc_param[:file].length / 4.0 * 3).ceil
+        (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' unless doc_param[:file].match?(%r{\A[A-Za-z0-9+/]*={0,2}\z})
-          raise ArgumentError, "Invalid base64 string format"
-        end
 
         # 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
+        file_data[0..3] if file_data.size >= 4
-          pdf_header = file_data[0..3]
-        end
 
         # 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,

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

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|

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: {}

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

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

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(Faraday::Request, body: nil) }
-    let(:request_double) { instance_double(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)

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)
+        allow(Rollbar).to receive(:error)
-        stub_const('Rollbar', rollbar_spy)
         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)
+        allow(Rollbar).to receive(:error)
-        stub_const('Rollbar', rollbar_spy)
         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)
+        allow(Rollbar).to receive(:error)
-        stub_const('Rollbar', rollbar_spy)
         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

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