- <% if import.importing? || import.pending? %>
+ <% if import.pending? && import.rows_count > 0 %>
+ <%# Bank statement with rows ready for review %>
+
+ <%= icon "check", class: "text-success" %>
+
+
+
+
<%= t("imports.pdf_import.ready_for_review_title", default: "Ready for Review") %>
+
<%= t("imports.pdf_import.ready_for_review_description", default: "We extracted %{count} transactions from your bank statement. Review and publish them to add to your account.", count: import.rows_count) %>
+
+
+
+
+
<%= t("imports.pdf_import.document_type_label") %>
+
+ <%= t("imports.document_types.#{import.document_type}", default: import.document_type&.humanize || t("imports.pdf_import.unknown_document_type", default: "Unknown")) %>
+
+
+
+
+
<%= t("imports.pdf_import.transactions_extracted", default: "Transactions Extracted") %>
+
+ <%= t("imports.pdf_import.transactions_extracted_count", count: import.rows_count, default: "%{count} transactions") %>
+
+
+
+
+
<%= t("imports.pdf_import.select_account", default: "Import to Account") %>
+ <%= form_with model: import, url: import_path(import), method: :patch, class: "space-y-2" do |f| %>
+ <% accounts = import.family.accounts.manual.alphabetically %>
+ <% if accounts.any? %>
+ <%= f.select :account_id, options_for_select(accounts.map { |a| [a.name, a.id] }, import.account_id), { include_blank: t("imports.pdf_import.select_account_placeholder", default: "Select an account...") }, class: "form-field__input" %>
+ <% if import.account.nil? %>
+
<%= t("imports.pdf_import.select_account_hint", default: "Choose which account to import these transactions into.") %>
+ <% end %>
+ <% else %>
+
+ <%= t("imports.pdf_import.no_accounts", default: "No accounts available. Please create an account first.") %>
+
+ <%= render DS::Link.new(text: t("imports.pdf_import.create_account", default: "Create Account"), href: new_account_path(return_to: import_path(import)), variant: "primary", full_width: true, frame: :modal) %>
+ <% end %>
+ <% if accounts.any? %>
+ <%= f.submit t("imports.pdf_import.save_account", default: "Save"), class: "w-full font-medium text-sm px-3 py-2 rounded-lg text-inverse bg-inverse hover:bg-inverse-hover" %>
+ <% end %>
+ <% end %>
+
+
+
+
+ <% if import.publishable? %>
+ <%= button_to t("imports.pdf_import.publish_transactions", default: "Publish %{count} Transactions", count: import.rows_count), publish_import_path(import), method: :post, class: "w-full font-medium text-sm px-3 py-2 rounded-lg text-inverse bg-inverse hover:bg-inverse-hover" %>
+ <% elsif import.account.present? %>
+ <%= render DS::Link.new(text: t("imports.pdf_import.review_transactions", default: "Review Transactions"), href: import_confirm_path(import), variant: "primary", full_width: true) %>
+ <% else %>
+
<%= t("imports.pdf_import.select_account_to_continue", default: "Please select an account above to continue.") %>
+ <% end %>
+ <%= render DS::Link.new(text: t("imports.pdf_import.back_to_imports"), href: imports_path, variant: "secondary", full_width: true) %>
+
+
+ <% elsif import.importing? || import.pending? %>
<%= icon "loader", class: "animate-pulse" %>
@@ -32,7 +91,7 @@
<%= render DS::Link.new(text: t("imports.pdf_import.try_again"), href: new_import_path, variant: "primary", full_width: true) %>
- <%= button_to t("imports.pdf_import.delete_import"), import_path(import), method: :delete, class: "btn btn--secondary w-full" %>
+ <%= button_to t("imports.pdf_import.delete_import"), import_path(import), method: :delete, class: "w-full font-medium text-sm px-3 py-2 rounded-lg text-primary bg-gray-200 hover:bg-gray-300" %>
<% elsif import.complete? && import.ai_processed? %>
@@ -49,7 +108,7 @@
<%= t("imports.pdf_import.document_type_label") %>
- <%= t("imports.document_types.#{import.document_type}") %>
+ <%= t("imports.document_types.#{import.document_type}", default: import.document_type&.humanize || t("imports.pdf_import.unknown_document_type", default: "Unknown")) %>
@@ -67,7 +126,7 @@
<%= render DS::Link.new(text: t("imports.pdf_import.back_to_imports"), href: imports_path, variant: "primary", full_width: true) %>
- <%= button_to t("imports.pdf_import.delete_import"), import_path(import), method: :delete, class: "btn btn--secondary w-full" %>
+ <%= button_to t("imports.pdf_import.delete_import"), import_path(import), method: :delete, class: "w-full font-medium text-sm px-3 py-2 rounded-lg text-primary bg-gray-200 hover:bg-gray-300" %>
<% else %>
diff --git a/config/routes.rb b/config/routes.rb
index 9ee83db62..632b5ca20 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -208,7 +208,7 @@ Rails.application.routes.draw do
resources :transfers, only: %i[new create destroy show update]
- resources :imports, only: %i[index new show create destroy] do
+ resources :imports, only: %i[index new show create update destroy] do
member do
post :publish
put :revert
diff --git a/db/migrate/20240701000000_add_category_fields_to_import_rows.rb b/db/migrate/20240701000000_add_category_fields_to_import_rows.rb
deleted file mode 100644
index 8a4210223..000000000
--- a/db/migrate/20240701000000_add_category_fields_to_import_rows.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-class AddCategoryFieldsToImportRows < ActiveRecord::Migration[7.1]
- def change
- add_column :import_rows, :category_parent, :string
- add_column :import_rows, :category_color, :string
- add_column :import_rows, :category_classification, :string
- end
-end
diff --git a/db/migrate/20240701000001_add_category_icon_to_import_rows.rb b/db/migrate/20240701000001_add_category_icon_to_import_rows.rb
deleted file mode 100644
index 66f389233..000000000
--- a/db/migrate/20240701000001_add_category_icon_to_import_rows.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-class AddCategoryIconToImportRows < ActiveRecord::Migration[7.1]
- def change
- add_column :import_rows, :category_icon, :string
- end
-end
diff --git a/db/migrate/20240925112219_ensure_category_fields_on_import_rows.rb b/db/migrate/20240925112219_ensure_category_fields_on_import_rows.rb
new file mode 100644
index 000000000..ad6f5e72f
--- /dev/null
+++ b/db/migrate/20240925112219_ensure_category_fields_on_import_rows.rb
@@ -0,0 +1,7 @@
+class EnsureCategoryFieldsOnImportRows < ActiveRecord::Migration[7.2]
+ def change
+ add_column :import_rows, :category_parent, :string unless column_exists?(:import_rows, :category_parent)
+ add_column :import_rows, :category_color, :string unless column_exists?(:import_rows, :category_color)
+ add_column :import_rows, :category_classification, :string unless column_exists?(:import_rows, :category_classification)
+ end
+end
diff --git a/db/migrate/20240925112220_ensure_category_icon_on_import_rows.rb b/db/migrate/20240925112220_ensure_category_icon_on_import_rows.rb
new file mode 100644
index 000000000..aaacb2275
--- /dev/null
+++ b/db/migrate/20240925112220_ensure_category_icon_on_import_rows.rb
@@ -0,0 +1,5 @@
+class EnsureCategoryIconOnImportRows < ActiveRecord::Migration[7.2]
+ def change
+ add_column :import_rows, :category_icon, :string unless column_exists?(:import_rows, :category_icon)
+ end
+end
diff --git a/test/controllers/settings/hostings_controller_test.rb b/test/controllers/settings/hostings_controller_test.rb
index 99473e7c3..5b1edb7cf 100644
--- a/test/controllers/settings/hostings_controller_test.rb
+++ b/test/controllers/settings/hostings_controller_test.rb
@@ -89,7 +89,7 @@ class Settings::HostingsControllerTest < ActionDispatch::IntegrationTest
assert_response :unprocessable_entity
assert_match(/OpenAI model is required/, flash[:alert])
- assert_nil Setting.openai_uri_base
+ assert Setting.openai_uri_base.blank?, "Expected openai_uri_base to remain blank after failed validation"
end
end
diff --git a/test/fixtures/imports.yml b/test/fixtures/imports.yml
index 3e9e185a9..964585593 100644
--- a/test/fixtures/imports.yml
+++ b/test/fixtures/imports.yml
@@ -24,3 +24,24 @@ pdf_processed:
status: complete
ai_summary: "This is a bank statement from Chase Bank for the period January 1-31, 2024. It shows 15 transactions with an opening balance of $5,000 and closing balance of $4,500."
document_type: bank_statement
+
+pdf_with_rows:
+ family: dylan_family
+ account: checking
+ type: PdfImport
+ status: pending
+ ai_summary: "Bank statement with extracted transactions"
+ document_type: bank_statement
+ extracted_data:
+ transactions:
+ - date: "2024-01-15"
+ amount: -50.00
+ name: "Coffee Shop"
+ category: "Food & Drink"
+ notes: "Morning coffee"
+ - date: "2024-01-16"
+ amount: 1500.00
+ name: "Salary"
+ category: "Income"
+ notes: ""
+ rows_count: 2
diff --git a/test/models/pdf_import_test.rb b/test/models/pdf_import_test.rb
index 3138c884b..a2491ee1a 100644
--- a/test/models/pdf_import_test.rb
+++ b/test/models/pdf_import_test.rb
@@ -6,6 +6,7 @@ class PdfImportTest < ActiveSupport::TestCase
setup do
@import = imports(:pdf)
@processed_import = imports(:pdf_processed)
+ @import_with_rows = imports(:pdf_with_rows)
end
test "pdf_uploaded? returns false when no file attached" do
@@ -24,27 +25,28 @@ class PdfImportTest < ActiveSupport::TestCase
assert_not @import.uploaded?
end
- test "configured? returns true when AI processed" do
- assert @processed_import.configured?
+ test "configured? requires AI processed and rows" do
assert_not @import.configured?
+ assert_not @processed_import.configured?
+ assert @import_with_rows.configured?
end
- test "cleaned? returns true when AI processed" do
- assert @processed_import.cleaned?
+ test "cleaned? requires configured and valid rows" do
assert_not @import.cleaned?
+ assert_not @processed_import.cleaned?
end
- test "publishable? always returns false for PDF imports" do
+ test "publishable? requires bank statement with cleaned rows and valid mappings" do
assert_not @import.publishable?
assert_not @processed_import.publishable?
end
- test "column_keys returns empty array" do
- assert_equal [], @import.column_keys
+ test "column_keys returns transaction columns" do
+ assert_equal %i[date amount name category notes], @import.column_keys
end
- test "required_column_keys returns empty array" do
- assert_equal [], @import.required_column_keys
+ test "required_column_keys returns date and amount" do
+ assert_equal %i[date amount], @import.required_column_keys
end
test "document_type validates against allowed types" do
@@ -66,4 +68,68 @@ class PdfImportTest < ActiveSupport::TestCase
@import.process_with_ai_later
end
end
+
+ test "generate_rows_from_extracted_data creates import rows" do
+ import = imports(:pdf_with_rows)
+ import.rows.destroy_all
+ import.update_column(:rows_count, 0)
+
+ import.generate_rows_from_extracted_data
+
+ assert_equal 2, import.rows.count
+ assert_equal 2, import.rows_count
+
+ coffee_row = import.rows.find_by(name: "Coffee Shop")
+ assert_not_nil coffee_row
+ assert_equal "-50.0", coffee_row.amount
+ assert_equal "Food & Drink", coffee_row.category
+
+ salary_row = import.rows.find_by(name: "Salary")
+ assert_not_nil salary_row
+ assert_equal "1500.0", salary_row.amount
+ end
+
+ test "generate_rows_from_extracted_data does nothing without extracted transactions" do
+ @import.generate_rows_from_extracted_data
+ assert_equal 0, @import.rows.count
+ end
+
+ test "extracted_transactions returns transactions from extracted_data" do
+ assert_equal 2, @import_with_rows.extracted_transactions.size
+ assert_equal "Coffee Shop", @import_with_rows.extracted_transactions.first["name"]
+ end
+
+ test "extracted_transactions returns empty array when no data" do
+ assert_equal [], @import.extracted_transactions
+ end
+
+ test "has_extracted_transactions? returns true with transactions" do
+ assert @import_with_rows.has_extracted_transactions?
+ end
+
+ test "has_extracted_transactions? returns false without transactions" do
+ assert_not @import.has_extracted_transactions?
+ end
+
+ test "mapping_steps is empty when no categories in rows" do
+ # PDF imports use direct account selection in UI, not AccountMapping
+ assert_equal [], @import.mapping_steps
+ end
+
+ test "mapping_steps includes CategoryMapping when rows have categories" do
+ @import_with_rows.rows.create!(
+ date: "01/15/2024",
+ amount: -50.00,
+ currency: "USD",
+ name: "Test Transaction",
+ category: "Groceries"
+ )
+ assert_equal [ Import::CategoryMapping ], @import_with_rows.mapping_steps
+ end
+
+ test "mapping_steps does not include AccountMapping even when account is nil" do
+ # PDF imports handle account selection via direct UI, not mapping system
+ assert_nil @import.account
+ assert_not_includes @import.mapping_steps, Import::AccountMapping
+ end
end
diff --git a/test/models/provider/openai/bank_statement_extractor_test.rb b/test/models/provider/openai/bank_statement_extractor_test.rb
new file mode 100644
index 000000000..0359e7517
--- /dev/null
+++ b/test/models/provider/openai/bank_statement_extractor_test.rb
@@ -0,0 +1,197 @@
+require "test_helper"
+
+class Provider::Openai::BankStatementExtractorTest < ActiveSupport::TestCase
+ setup do
+ @client = mock("openai_client")
+ @model = "gpt-4.1"
+ end
+
+ test "extracts transactions from PDF content" do
+ mock_response = {
+ "choices" => [ {
+ "message" => {
+ "content" => {
+ "bank_name" => "Test Bank",
+ "account_holder" => "John Doe",
+ "account_number" => "1234",
+ "statement_period" => {
+ "start_date" => "2024-01-01",
+ "end_date" => "2024-01-31"
+ },
+ "opening_balance" => 5000.00,
+ "closing_balance" => 4500.00,
+ "transactions" => [
+ { "date" => "2024-01-15", "description" => "Coffee Shop", "amount" => -5.50 },
+ { "date" => "2024-01-20", "description" => "Salary Deposit", "amount" => 3000.00 }
+ ]
+ }.to_json
+ }
+ } ]
+ }
+
+ @client.expects(:chat).returns(mock_response)
+
+ extractor = Provider::Openai::BankStatementExtractor.new(
+ client: @client,
+ pdf_content: "dummy",
+ model: @model
+ )
+
+ # Mock the PDF text extraction
+ extractor.stubs(:extract_pages_from_pdf).returns([ "Page 1 bank statement text" ])
+
+ result = extractor.extract
+
+ assert_equal "Test Bank", result[:bank_name]
+ assert_equal "John Doe", result[:account_holder]
+ assert_equal "1234", result[:account_number]
+ assert_equal 5000.00, result[:opening_balance]
+ assert_equal 4500.00, result[:closing_balance]
+ assert_equal 2, result[:transactions].size
+
+ first_txn = result[:transactions].first
+ assert_equal "2024-01-15", first_txn[:date]
+ assert_equal "Coffee Shop", first_txn[:name]
+ assert_equal(-5.50, first_txn[:amount])
+ end
+
+ test "handles empty PDF content" do
+ extractor = Provider::Openai::BankStatementExtractor.new(
+ client: @client,
+ pdf_content: "",
+ model: @model
+ )
+
+ assert_raises(Provider::Openai::Error) do
+ extractor.extract
+ end
+ end
+
+ test "handles nil PDF content" do
+ extractor = Provider::Openai::BankStatementExtractor.new(
+ client: @client,
+ pdf_content: nil,
+ model: @model
+ )
+
+ assert_raises(Provider::Openai::Error) do
+ extractor.extract
+ end
+ end
+
+ test "deduplicates transactions across chunk boundaries" do
+ # First chunk response
+ first_response = {
+ "choices" => [ {
+ "message" => {
+ "content" => {
+ "bank_name" => "Test Bank",
+ "account_holder" => "John Doe",
+ "account_number" => "1234",
+ "statement_period" => { "start_date" => "2024-01-01", "end_date" => "2024-01-31" },
+ "opening_balance" => 5000.00,
+ "closing_balance" => 4500.00,
+ "transactions" => [
+ { "date" => "2024-01-15", "description" => "Coffee Shop", "amount" => -5.50 },
+ { "date" => "2024-01-16", "description" => "Grocery Store", "amount" => -50.00 }
+ ]
+ }.to_json
+ }
+ } ]
+ }
+
+ # Second chunk response with duplicate at boundary
+ second_response = {
+ "choices" => [ {
+ "message" => {
+ "content" => {
+ "transactions" => [
+ { "date" => "2024-01-16", "description" => "Grocery Store", "amount" => -50.00 },
+ { "date" => "2024-01-17", "description" => "Gas Station", "amount" => -40.00 }
+ ]
+ }.to_json
+ }
+ } ]
+ }
+
+ @client.expects(:chat).twice.returns(first_response, second_response)
+
+ extractor = Provider::Openai::BankStatementExtractor.new(
+ client: @client,
+ pdf_content: "dummy",
+ model: @model
+ )
+
+ # Mock multiple pages that will create multiple chunks
+ extractor.stubs(:extract_pages_from_pdf).returns([
+ "Page 1 " * 500, # ~3500 chars, first chunk
+ "Page 2 " * 500 # ~3500 chars, second chunk
+ ])
+
+ result = extractor.extract
+
+ # Should deduplicate the "Grocery Store" transaction at chunk boundary
+ assert_equal 3, result[:transactions].size
+ names = result[:transactions].map { |t| t[:name] }
+ assert_includes names, "Coffee Shop"
+ assert_includes names, "Grocery Store"
+ assert_includes names, "Gas Station"
+ end
+
+ test "normalizes transaction amounts" do
+ mock_response = {
+ "choices" => [ {
+ "message" => {
+ "content" => {
+ "transactions" => [
+ { "date" => "2024-01-15", "description" => "Test 1", "amount" => "-$5.50" },
+ { "date" => "2024-01-16", "description" => "Test 2", "amount" => "1,234.56" },
+ { "date" => "2024-01-17", "description" => "Test 3", "amount" => -100 }
+ ]
+ }.to_json
+ }
+ } ]
+ }
+
+ @client.expects(:chat).returns(mock_response)
+
+ extractor = Provider::Openai::BankStatementExtractor.new(
+ client: @client,
+ pdf_content: "dummy",
+ model: @model
+ )
+
+ extractor.stubs(:extract_pages_from_pdf).returns([ "Page 1 text" ])
+
+ result = extractor.extract
+
+ assert_equal(-5.50, result[:transactions][0][:amount])
+ assert_equal 1234.56, result[:transactions][1][:amount]
+ assert_equal(-100.0, result[:transactions][2][:amount])
+ end
+
+ test "handles malformed JSON response gracefully" do
+ mock_response = {
+ "choices" => [ {
+ "message" => {
+ "content" => "This is not valid JSON"
+ }
+ } ]
+ }
+
+ @client.expects(:chat).returns(mock_response)
+
+ extractor = Provider::Openai::BankStatementExtractor.new(
+ client: @client,
+ pdf_content: "dummy",
+ model: @model
+ )
+
+ extractor.stubs(:extract_pages_from_pdf).returns([ "Page 1 text" ])
+
+ result = extractor.extract
+
+ # Should return empty transactions on parse error
+ assert_equal [], result[:transactions]
+ end
+end