From 05ef8bd9e744dc8aa48fb500f3a9cfd285a1f68f Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Mon, 4 May 2026 10:51:48 -0600 Subject: [PATCH] feat(api): support idempotent valuation writes (#1637) * feat(api): support idempotent valuation writes * fix(api): clarify valuation upsert status * docs(api): document nested valuation upserts * docs(api): clarify valuation upsert semantics * docs(api): clarify valuation upsert signaling --- .../api/v1/valuations_controller.rb | 15 +++- docs/api/openapi.yaml | 18 +++++ spec/requests/api/v1/valuations_spec.rb | 27 ++++++- .../api/v1/valuations_controller_test.rb | 75 +++++++++++++++++++ 4 files changed, 133 insertions(+), 2 deletions(-) diff --git a/app/controllers/api/v1/valuations_controller.rb b/app/controllers/api/v1/valuations_controller.rb index ad50e5d92..d5ce1842b 100644 --- a/app/controllers/api/v1/valuations_controller.rb +++ b/app/controllers/api/v1/valuations_controller.rb @@ -4,6 +4,7 @@ class Api::V1::ValuationsController < Api::V1::BaseController include Pagy::Backend InvalidFilterError = Class.new(StandardError) + BOOLEAN_PARAM = ActiveModel::Type::Boolean.new before_action :ensure_read_scope, only: [ :index, :show ] before_action :ensure_write_scope, only: [ :create, :update ] @@ -83,11 +84,17 @@ class Api::V1::ValuationsController < Api::V1::BaseController end account = current_resource_owner.family.accounts.find(valuation_account_id) + requested_upsert = upsert_requested? + existing_write = false create_success = false error_payload = nil ActiveRecord::Base.transaction do + account.lock! if requested_upsert + existing_write = account.entries.valuations.exists?(date: valuation_params[:date]) if requested_upsert + + # upsert=true only affects response status; reconciliation owns write behavior. result = account.create_reconciliation( balance: valuation_params[:amount], date: valuation_params[:date] @@ -124,7 +131,7 @@ class Api::V1::ValuationsController < Api::V1::BaseController return end - render :show, status: :created + render :show, status: requested_upsert && existing_write ? :ok : :created rescue ActiveRecord::RecordNotFound render json: { @@ -289,4 +296,10 @@ class Api::V1::ValuationsController < Api::V1::BaseController def valuation_params params.require(:valuation).permit(:amount, :date, :notes) end + + def upsert_requested? + raw_value = params.key?(:upsert) ? params[:upsert] : params.dig(:valuation, :upsert) + + BOOLEAN_PARAM.cast(raw_value) + end end diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 5fbd6aac9..7f42f01c7 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -4549,6 +4549,12 @@ paths: application/json: schema: "$ref": "#/components/schemas/Valuation" + '200': + description: existing valuation upserted + content: + application/json: + schema: + "$ref": "#/components/schemas/Valuation" '422': description: validation error - missing date content: @@ -4584,10 +4590,22 @@ paths: notes: type: string description: Additional notes + upsert: + type: boolean + description: Nested alternative to the top-level response-status + flag. Top-level upsert takes precedence when both are provided. required: - account_id - amount - date + upsert: + type: boolean + description: Response-status signal only. When true and a same-account + same-date valuation exists before the request, the endpoint returns + 200 OK instead of 201 Created. The underlying reconciliation write + path is unchanged; this flag does not add duplicate-prevention + or safe-retry guarantees beyond existing same-date reconciliation + behavior. required: - valuation required: true diff --git a/spec/requests/api/v1/valuations_spec.rb b/spec/requests/api/v1/valuations_spec.rb index ca6466042..b164812cf 100644 --- a/spec/requests/api/v1/valuations_spec.rb +++ b/spec/requests/api/v1/valuations_spec.rb @@ -121,9 +121,17 @@ RSpec.describe 'API V1 Valuations', type: :request do account_id: { type: :string, format: :uuid, description: 'Account ID (required)' }, amount: { type: :number, description: 'Valuation amount (required)' }, date: { type: :string, format: :date, description: 'Valuation date (required)' }, - notes: { type: :string, description: 'Additional notes' } + notes: { type: :string, description: 'Additional notes' }, + upsert: { + type: :boolean, + description: 'Nested alternative to the top-level response-status flag. Top-level upsert takes precedence when both are provided.' + } }, required: %w[account_id amount date] + }, + upsert: { + type: :boolean, + description: 'Response-status signal only. When true and a same-account same-date valuation exists before the request, the endpoint returns 200 OK instead of 201 Created. The underlying reconciliation write path is unchanged; this flag does not add duplicate-prevention or safe-retry guarantees beyond existing same-date reconciliation behavior.' } }, required: %w[valuation] @@ -145,6 +153,23 @@ RSpec.describe 'API V1 Valuations', type: :request do run_test! end + response '200', 'existing valuation upserted' do + schema '$ref' => '#/components/schemas/Valuation' + + let(:body) do + { + upsert: true, + valuation: { + account_id: account.id, + amount: 15000.00, + date: Date.current.to_s + } + } + end + + run_test! + end + response '422', 'validation error - missing account_id' do schema '$ref' => '#/components/schemas/ErrorResponse' diff --git a/test/controllers/api/v1/valuations_controller_test.rb b/test/controllers/api/v1/valuations_controller_test.rb index 125863af4..b1a24bc97 100644 --- a/test/controllers/api/v1/valuations_controller_test.rb +++ b/test/controllers/api/v1/valuations_controller_test.rb @@ -143,6 +143,81 @@ class Api::V1::ValuationsControllerTest < ActionDispatch::IntegrationTest assert_equal @account.id, response_data["account"]["id"] end + test "should upsert valuation for same account and date when requested" do + existing_entry = @valuation.entry + valuation_params = { + upsert: "true", + valuation: { + account_id: existing_entry.account.id, + amount: 12_345.67, + date: existing_entry.date, + notes: "API correction" + } + } + + assert_no_difference("@family.entries.valuations.count") do + post api_v1_valuations_url, + params: valuation_params, + headers: api_headers(@api_key) + end + + assert_response :ok + response_data = JSON.parse(response.body) + assert_equal existing_entry.id, response_data["id"] + assert_equal existing_entry.date.to_s, response_data["date"] + assert_equal "API correction", response_data["notes"] + assert_equal BigDecimal("12345.67"), existing_entry.reload.amount + end + + test "should create valuation when upsert is requested without an existing same-date valuation" do + valuation_date = Date.current + 3.days + valuation_params = { + upsert: "true", + valuation: { + account_id: @account.id, + amount: 9876.54, + date: valuation_date, + notes: "New API valuation" + } + } + + assert_difference("@family.entries.valuations.count", 1) do + post api_v1_valuations_url, + params: valuation_params, + headers: api_headers(@api_key) + end + + assert_response :created + response_data = JSON.parse(response.body) + assert_equal valuation_date.to_s, response_data["date"] + assert_equal "New API valuation", response_data["notes"] + end + + test "should accept nested upsert flag for same-date valuation writes" do + existing_entry = @valuation.entry + valuation_params = { + valuation: { + account_id: existing_entry.account.id, + amount: 22_222.22, + date: existing_entry.date, + notes: "Nested upsert correction", + upsert: "true" + } + } + + assert_no_difference("@family.entries.valuations.count") do + post api_v1_valuations_url, + params: valuation_params, + headers: api_headers(@api_key) + end + + assert_response :ok + response_data = JSON.parse(response.body) + assert_equal existing_entry.id, response_data["id"] + assert_equal "Nested upsert correction", response_data["notes"] + assert_equal BigDecimal("22222.22"), existing_entry.reload.amount + end + test "should reject create with read-only API key" do valuation_params = { valuation: {