diff --git a/app/controllers/api/v1/transactions_controller.rb b/app/controllers/api/v1/transactions_controller.rb index c3538c79e..5dd65aba3 100644 --- a/app/controllers/api/v1/transactions_controller.rb +++ b/app/controllers/api/v1/transactions_controller.rb @@ -69,7 +69,7 @@ class Api::V1::TransactionsController < Api::V1::BaseController family = current_resource_owner.family # Validate account_id is present - unless transaction_params[:account_id].present? + unless account_id_param.present? render json: { error: "validation_failed", message: "Account ID is required", @@ -78,7 +78,21 @@ class Api::V1::TransactionsController < Api::V1::BaseController return end - account = family.accounts.writable_by(current_resource_owner).find(transaction_params[:account_id]) + if idempotency_source_param.present? && idempotency_external_id.blank? + render json: { + error: "validation_failed", + message: "Source requires external_id", + errors: [ "Source requires external_id" ] + }, status: :unprocessable_entity + return + end + + account = family.accounts.writable_by(current_resource_owner).find(account_id_param) + + if idempotency_key_requested? && (existing_entry = existing_idempotent_entry(account)) + return render_existing_idempotent_entry(existing_entry) + end + @entry = account.entries.new(entry_params_for_create) if @entry.save @@ -96,6 +110,12 @@ class Api::V1::TransactionsController < Api::V1::BaseController }, status: :unprocessable_entity end + rescue ActiveRecord::RecordNotUnique + if idempotency_key_requested? && account && (existing_entry = existing_idempotent_entry(account)) + render_existing_idempotent_entry(existing_entry) + else + raise + end rescue => e Rails.logger.error "TransactionsController#create error: #{e.message}" Rails.logger.error e.backtrace.join("\n") @@ -282,11 +302,15 @@ end def transaction_params params.require(:transaction).permit( - :account_id, :date, :amount, :name, :description, :notes, :currency, + :date, :amount, :name, :description, :notes, :currency, :category_id, :merchant_id, :nature, tag_ids: [] ) end + def account_id_param + params.dig(:transaction, :account_id).presence + end + def entry_params_for_create entry_params = { name: transaction_params[:name] || transaction_params[:description], @@ -301,6 +325,10 @@ end tag_ids: transaction_params[:tag_ids] || [] } } + if idempotency_key_requested? + entry_params[:external_id] = idempotency_external_id + entry_params[:source] = idempotency_source + end entry_params.compact end @@ -339,6 +367,49 @@ end params.dig(:transaction, :nature).present? end + def idempotency_key_requested? + idempotency_external_id.present? + end + + def idempotency_external_id + idempotency_param_value(:external_id) + end + + def idempotency_source + idempotency_source_param.presence || "api" + end + + def idempotency_source_param + idempotency_param_value(:source) + end + + def idempotency_param_value(key) + value = params.dig(:transaction, key) + value.to_s.presence if value.is_a?(String) || value.is_a?(Numeric) + end + + def existing_idempotent_entry(account) + account.entries.find_by( + external_id: idempotency_external_id, + source: idempotency_source + ) + end + + def render_existing_idempotent_entry(entry) + unless entry.entryable.is_a?(Transaction) + render json: { + error: "validation_failed", + message: "External ID already exists for a non-transaction entry", + errors: [ "External ID already exists for a non-transaction entry" ] + }, status: :unprocessable_entity + return + end + + @entry = entry + @transaction = entry.transaction + render :show, status: :ok + end + def calculate_signed_amount amount = transaction_params[:amount].to_f nature = transaction_params[:nature] diff --git a/app/views/api/v1/transactions/_transaction.json.jbuilder b/app/views/api/v1/transactions/_transaction.json.jbuilder index 9f3a47a98..488ecbe5b 100644 --- a/app/views/api/v1/transactions/_transaction.json.jbuilder +++ b/app/views/api/v1/transactions/_transaction.json.jbuilder @@ -17,6 +17,8 @@ json.signed_amount_cents(transaction.entry.classification == "income" ? amount_c json.currency transaction.entry.currency json.name transaction.entry.name json.notes transaction.entry.notes +json.external_id transaction.entry.external_id +json.source transaction.entry.source json.classification transaction.entry.classification # Account information diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 9b2f867d9..9db36b8f7 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -1435,6 +1435,12 @@ components: notes: type: string nullable: true + external_id: + type: string + nullable: true + source: + type: string + nullable: true classification: type: string account: @@ -6256,6 +6262,12 @@ paths: application/json: schema: "$ref": "#/components/schemas/Transaction" + '200': + description: transaction already exists for external idempotency key + content: + application/json: + schema: + "$ref": "#/components/schemas/Transaction" '422': description: validation error - missing required fields content: @@ -6310,6 +6322,14 @@ paths: - inflow - outflow description: Transaction nature (determines sign) + external_id: + type: string + description: Optional external idempotency key scoped to account + and source + source: + type: string + description: Optional source namespace for external_id. Requires + external_id and defaults to api when external_id is provided tag_ids: type: array items: diff --git a/spec/requests/api/v1/transactions_spec.rb b/spec/requests/api/v1/transactions_spec.rb index a4eab7590..d57a40b6a 100644 --- a/spec/requests/api/v1/transactions_spec.rb +++ b/spec/requests/api/v1/transactions_spec.rb @@ -173,6 +173,8 @@ RSpec.describe 'API V1 Transactions', type: :request do category_id: { type: :string, format: :uuid, description: 'Category ID' }, merchant_id: { type: :string, format: :uuid, description: 'Merchant ID' }, nature: { type: :string, enum: %w[income expense inflow outflow], description: 'Transaction nature (determines sign)' }, + external_id: { type: :string, description: 'Optional external idempotency key scoped to account and source' }, + source: { type: :string, description: 'Optional source namespace for external_id. Requires external_id and defaults to api when external_id is provided' }, tag_ids: { type: :array, items: { type: :string, format: :uuid }, description: 'Array of tag IDs' } }, required: %w[account_id date amount name] @@ -201,6 +203,38 @@ RSpec.describe 'API V1 Transactions', type: :request do run_test! end + response '200', 'transaction already exists for external idempotency key' do + schema '$ref' => '#/components/schemas/Transaction' + + let(:body) do + { + transaction: { + account_id: account.id, + date: Date.current.to_s, + amount: 50.00, + name: 'Test purchase', + nature: 'expense', + external_id: 'docs-import-transaction-1', + source: 'external_import' + } + } + end + + before do + account.entries.create!( + name: 'Test purchase', + date: Date.current, + amount: 50.00, + currency: 'USD', + external_id: 'docs-import-transaction-1', + source: 'external_import', + entryable: Transaction.new + ) + end + + run_test! + end + response '422', 'validation error - missing account_id' do schema '$ref' => '#/components/schemas/ErrorResponse' diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index d8260c27e..0692d2aad 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -788,6 +788,8 @@ RSpec.configure do |config| currency: { type: :string }, name: { type: :string }, notes: { type: :string, nullable: true }, + external_id: { type: :string, nullable: true }, + source: { type: :string, nullable: true }, classification: { type: :string }, account: { '$ref' => '#/components/schemas/Account' }, category: { '$ref' => '#/components/schemas/Category', nullable: true }, diff --git a/test/controllers/api/v1/transactions_controller_test.rb b/test/controllers/api/v1/transactions_controller_test.rb index 8ddb0ebf7..002c80c30 100644 --- a/test/controllers/api/v1/transactions_controller_test.rb +++ b/test/controllers/api/v1/transactions_controller_test.rb @@ -179,6 +179,220 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_equal @account.id, response_data["account"]["id"] end + test "should create transaction with external idempotency key" do + transaction_params = { + transaction: { + account_id: @account.id, + name: "Imported Transaction", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense", + external_id: "import-txn-1", + source: "external_import" + } + } + + assert_difference("@account.entries.count", 1) do + post api_v1_transactions_url, + params: transaction_params, + headers: api_headers(@api_key) + end + + assert_response :created + response_data = JSON.parse(response.body) + assert_equal "import-txn-1", response_data["external_id"] + assert_equal "external_import", response_data["source"] + + entry = @account.entries.find_by!(external_id: "import-txn-1", source: "external_import") + assert_equal response_data["id"], entry.transaction.id + end + + test "should use default source when external_id provided without source" do + transaction_params = { + transaction: { + account_id: @account.id, + name: "Imported Transaction", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense", + external_id: "default-source-test" + } + } + + assert_difference("@account.entries.count", 1) do + post api_v1_transactions_url, + params: transaction_params, + headers: api_headers(@api_key) + end + + assert_response :created + response_data = JSON.parse(response.body) + entry = @account.entries.find_by!(external_id: "default-source-test") + assert_equal "api", entry.source + assert_equal "api", response_data["source"] + + assert_no_difference("@account.entries.count") do + post api_v1_transactions_url, + params: transaction_params.deep_merge(transaction: { name: "Changed Name" }), + headers: api_headers(@api_key) + end + + assert_response :ok + end + + test "should reject source without external idempotency key" do + transaction_params = { + transaction: { + account_id: @account.id, + name: "Imported Transaction", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense", + source: "external_import" + } + } + + assert_no_difference("@account.entries.count") do + post api_v1_transactions_url, + params: transaction_params, + headers: api_headers(@api_key) + end + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + assert_equal "Source requires external_id", response_data["message"] + assert_equal [ "Source requires external_id" ], response_data["errors"] + end + + test "should return existing transaction for duplicate external idempotency key" do + transaction_params = { + transaction: { + account_id: @account.id, + name: "Imported Transaction", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense", + external_id: "import-txn-2", + source: "external_import" + } + } + + post api_v1_transactions_url, + params: transaction_params, + headers: api_headers(@api_key) + assert_response :created + created_data = JSON.parse(response.body) + + assert_no_difference("@account.entries.count") do + post api_v1_transactions_url, + params: transaction_params.deep_merge(transaction: { name: "Changed Name" }), + headers: api_headers(@api_key) + end + + assert_response :ok + response_data = JSON.parse(response.body) + assert_equal created_data["id"], response_data["id"] + assert_equal "Imported Transaction", response_data["name"] + end + + test "should scope external idempotency keys to account" do + other_account = @family.accounts.create!( + name: "Other API Account", + accountable: Depository.new, + balance: 0, + currency: "USD" + ) + transaction_params = { + transaction: { + name: "Imported Transaction", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense", + external_id: "shared-import-txn", + source: "external_import" + } + } + + assert_difference("Entry.count", 2) do + post api_v1_transactions_url, + params: transaction_params.deep_merge(transaction: { account_id: @account.id }), + headers: api_headers(@api_key) + assert_response :created + + post api_v1_transactions_url, + params: transaction_params.deep_merge(transaction: { account_id: other_account.id }), + headers: api_headers(@api_key) + assert_response :created + end + end + + test "should scope external idempotency keys to source" do + transaction_params = { + transaction: { + account_id: @account.id, + name: "Imported Transaction", + amount: 25.00, + date: Date.current, + currency: "USD", + nature: "expense", + external_id: "shared-source-txn", + source: "external_import" + } + } + + assert_difference("Entry.count", 2) do + post api_v1_transactions_url, + params: transaction_params, + headers: api_headers(@api_key) + assert_response :created + + post api_v1_transactions_url, + params: transaction_params.deep_merge(transaction: { source: "other_import" }), + headers: api_headers(@api_key) + assert_response :created + end + + @account.entries.find_by!(external_id: "shared-source-txn", source: "external_import") + @account.entries.find_by!(external_id: "shared-source-txn", source: "other_import") + end + + test "should reject external idempotency key collision with non-transaction entry" do + @account.entries.create!( + name: "Existing valuation", + amount: 100, + currency: "USD", + date: Date.current, + external_id: "import-non-transaction", + source: "external_import", + entryable: Valuation.new + ) + + post api_v1_transactions_url, + params: { + transaction: { + account_id: @account.id, + name: "Imported Transaction", + amount: 25.00, + date: Date.current - 1.day, + currency: "USD", + nature: "expense", + external_id: "import-non-transaction", + source: "external_import" + } + }, + headers: api_headers(@api_key) + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + end + test "should reject create with read-only API key" do transaction_params = { transaction: { @@ -209,6 +423,31 @@ class Api::V1::TransactionsControllerTest < ActionDispatch::IntegrationTest assert_response :unprocessable_entity end + test "should reject invalid date on create" do + transaction_params = { + transaction: { + account_id: @account.id, + name: "Invalid Date Transaction", + amount: 25.00, + date: "not-a-date", + currency: "USD", + nature: "expense" + } + } + + assert_no_difference("@account.entries.count") do + post api_v1_transactions_url, + params: transaction_params, + headers: api_headers(@api_key) + end + + assert_response :unprocessable_entity + response_data = JSON.parse(response.body) + assert_equal "validation_failed", response_data["error"] + assert_equal "Transaction could not be created", response_data["message"] + assert response_data["errors"].any? { |error| error.match?(/Date/) } + end + test "should reject create without API key" do post api_v1_transactions_url, params: { transaction: { name: "Test" } } assert_response :unauthorized