From c4414c4fbbec298aa600b2fbf643eec4c9363059 Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Fri, 1 May 2026 14:59:32 -0600 Subject: [PATCH] feat(api): expose import status details (#1599) * feat(api): expose import status details * fix(api): reuse import status validation counts * fix(api): cache Sure import status reads * fix(imports): invalidate cached Sure import blobs * docs(api): split import status schemas * fix(api): refine import status detail contract --- app/models/import.rb | 21 ++++++++ app/models/pdf_import.rb | 8 +++ app/models/qif_import.rb | 4 ++ app/models/sure_import.rb | 15 +++++- .../v1/imports/_status_detail.json.jbuilder | 22 ++++++++ app/views/api/v1/imports/index.json.jbuilder | 3 ++ app/views/api/v1/imports/show.json.jbuilder | 21 +++++++- docs/api/openapi.yaml | 52 ++++++++++++++++++- spec/swagger_helper.rb | 37 +++++++++++-- .../api/v1/imports_controller_test.rb | 24 +++++++++ test/models/pdf_import_test.rb | 13 +++++ test/models/sure_import_test.rb | 27 ++++++++++ 12 files changed, 240 insertions(+), 7 deletions(-) create mode 100644 app/views/api/v1/imports/_status_detail.json.jbuilder diff --git a/app/models/import.rb b/app/models/import.rb index 941a6ead6..a893e7f13 100644 --- a/app/models/import.rb +++ b/app/models/import.rb @@ -258,6 +258,10 @@ class Import < ApplicationRecord uploaded? && rows_count > 0 end + def configured_for_status_detail? + configured? + end + def cleaned? configured? && rows.all?(&:valid?) end @@ -266,6 +270,23 @@ class Import < ApplicationRecord cleaned? && mappings.all?(&:valid?) end + def cleaned_from_validation_stats?(invalid_rows_count:) + configured? && invalid_rows_count.zero? + end + + def publishable_from_validation_stats?(invalid_rows_count:) + cleaned_from_validation_stats?(invalid_rows_count: invalid_rows_count) && mappings.all?(&:valid?) + end + + def mapping_status_counts + mappable_ids = mappings.pluck(:mappable_id) + + { + mappings_count: mappable_ids.size, + unassigned_mappings_count: mappable_ids.count(&:nil?) + } + end + def revertable? complete? || revert_failed? end diff --git a/app/models/pdf_import.rb b/app/models/pdf_import.rb index 38091bf2c..e18d8ce24 100644 --- a/app/models/pdf_import.rb +++ b/app/models/pdf_import.rb @@ -154,6 +154,14 @@ class PdfImport < Import account.present? && statement_with_transactions? && cleaned? && mappings.all?(&:valid?) end + def cleaned_from_validation_stats?(invalid_rows_count:) + account.present? && statement_with_transactions? && super + end + + def publishable_from_validation_stats?(invalid_rows_count:) + account.present? && statement_with_transactions? && super + end + def column_keys %i[date amount name category notes] end diff --git a/app/models/qif_import.rb b/app/models/qif_import.rb index 90a867d28..aa259f5b4 100644 --- a/app/models/qif_import.rb +++ b/app/models/qif_import.rb @@ -73,6 +73,10 @@ class QifImport < Import account.present? && super end + def publishable_from_validation_stats?(invalid_rows_count:) + account.present? && super + end + # Returns true if import! will move the opening anchor back to cover transactions # that predate the current anchor date. Used to show a notice in the confirm step. def will_adjust_opening_anchor? diff --git a/app/models/sure_import.rb b/app/models/sure_import.rb index 05d927c4d..6666fc575 100644 --- a/app/models/sure_import.rb +++ b/app/models/sure_import.rb @@ -112,6 +112,14 @@ class SureImport < Import cleaned? && dry_run.values.sum.positive? end + def cleaned_from_validation_stats?(invalid_rows_count:) + configured? && invalid_rows_count.zero? + end + + def publishable_from_validation_stats?(invalid_rows_count:) + cleaned_from_validation_stats?(invalid_rows_count: invalid_rows_count) && dry_run.values.sum.positive? + end + def max_row_count 100_000 end @@ -127,6 +135,11 @@ class SureImport < Import private def ndjson_blob_string - ndjson_file.download.force_encoding(Encoding::UTF_8) + blob_id = ndjson_file.blob&.id + + return @ndjson_blob_string if defined?(@ndjson_blob_string) && @ndjson_blob_id == blob_id + + @ndjson_blob_id = blob_id + @ndjson_blob_string = ndjson_file.download.force_encoding(Encoding::UTF_8) end end diff --git a/app/views/api/v1/imports/_status_detail.json.jbuilder b/app/views/api/v1/imports/_status_detail.json.jbuilder new file mode 100644 index 000000000..7535dcada --- /dev/null +++ b/app/views/api/v1/imports/_status_detail.json.jbuilder @@ -0,0 +1,22 @@ +uploaded = local_assigns[:uploaded] +uploaded = import.uploaded? if uploaded.nil? +configured = local_assigns[:configured] +configured = import.configured_for_status_detail? if configured.nil? + +json.uploaded uploaded +json.configured configured +json.terminal import.complete? || import.failed? || import.revert_failed? + +if include_validation_stats + valid_rows_count = local_assigns.fetch(:valid_rows_count) + invalid_rows_count = local_assigns.fetch(:invalid_rows_count) + + cleaned = local_assigns[:cleaned] + publishable = local_assigns[:publishable] + cleaned = import.cleaned_from_validation_stats?(invalid_rows_count: invalid_rows_count) if cleaned.nil? + publishable = import.publishable_from_validation_stats?(invalid_rows_count: invalid_rows_count) if publishable.nil? + + json.cleaned cleaned + json.publishable publishable + json.revertable import.revertable? +end diff --git a/app/views/api/v1/imports/index.json.jbuilder b/app/views/api/v1/imports/index.json.jbuilder index eff1c6414..3359d704a 100644 --- a/app/views/api/v1/imports/index.json.jbuilder +++ b/app/views/api/v1/imports/index.json.jbuilder @@ -8,6 +8,9 @@ json.data do json.account_id import.account_id json.rows_count import.rows_count json.error import.error if import.error.present? + json.status_detail do + json.partial! "status_detail", import: import, include_validation_stats: false + end end end diff --git a/app/views/api/v1/imports/show.json.jbuilder b/app/views/api/v1/imports/show.json.jbuilder index 18509062e..2ce5f0e52 100644 --- a/app/views/api/v1/imports/show.json.jbuilder +++ b/app/views/api/v1/imports/show.json.jbuilder @@ -1,3 +1,10 @@ +rows = @import.rows.to_a +valid_rows_count = rows.count(&:valid?) +invalid_rows_count = rows.length - valid_rows_count +cleaned = @import.cleaned_from_validation_stats?(invalid_rows_count: invalid_rows_count) +publishable = @import.publishable_from_validation_stats?(invalid_rows_count: invalid_rows_count) +mapping_counts = @import.mapping_status_counts + json.data do json.id @import.id json.type @import.type @@ -6,6 +13,15 @@ json.data do json.updated_at @import.updated_at json.account_id @import.account_id json.error @import.error if @import.error.present? + json.status_detail do + json.partial! "status_detail", + import: @import, + include_validation_stats: true, + valid_rows_count: valid_rows_count, + invalid_rows_count: invalid_rows_count, + cleaned: cleaned, + publishable: publishable + end json.configuration do json.date_col_label @import.date_col_label @@ -22,7 +38,10 @@ json.data do json.stats do json.rows_count @import.rows_count - json.valid_rows_count @import.rows.select(&:valid?).count if @import.rows.loaded? + json.valid_rows_count valid_rows_count + json.invalid_rows_count invalid_rows_count + json.mappings_count mapping_counts[:mappings_count] + json.unassigned_mappings_count mapping_counts[:unassigned_mappings_count] end # Only show a subset of rows for preview if needed, or link to a separate rows endpoint diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index a16a8c582..46e6650f9 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -875,6 +875,12 @@ components: nullable: true ImportStats: type: object + required: + - rows_count + - valid_rows_count + - invalid_rows_count + - mappings_count + - unassigned_mappings_count properties: rows_count: type: integer @@ -882,7 +888,43 @@ components: valid_rows_count: type: integer minimum: 0 - nullable: true + invalid_rows_count: + type: integer + minimum: 0 + mappings_count: + type: integer + minimum: 0 + unassigned_mappings_count: + type: integer + minimum: 0 + ImportStatusSummary: + type: object + required: + - uploaded + - configured + - terminal + properties: + uploaded: + type: boolean + configured: + type: boolean + terminal: + type: boolean + ImportStatusDetail: + allOf: + - "$ref": "#/components/schemas/ImportStatusSummary" + - type: object + required: + - cleaned + - publishable + - revertable + properties: + cleaned: + type: boolean + publishable: + type: boolean + revertable: + type: boolean ImportSummary: type: object required: @@ -891,6 +933,7 @@ components: - status - created_at - updated_at + - status_detail properties: id: type: string @@ -930,6 +973,8 @@ components: error: type: string nullable: true + status_detail: + "$ref": "#/components/schemas/ImportStatusSummary" ImportDetail: type: object required: @@ -938,6 +983,9 @@ components: - status - created_at - updated_at + - status_detail + - configuration + - stats properties: id: type: string @@ -974,6 +1022,8 @@ components: error: type: string nullable: true + status_detail: + "$ref": "#/components/schemas/ImportStatusDetail" configuration: "$ref": "#/components/schemas/ImportConfiguration" stats: diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index 031472c9e..729b10a2b 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -527,14 +527,41 @@ RSpec.configure do |config| }, ImportStats: { type: :object, + required: %w[rows_count valid_rows_count invalid_rows_count mappings_count unassigned_mappings_count], properties: { rows_count: { type: :integer, minimum: 0 }, - valid_rows_count: { type: :integer, minimum: 0, nullable: true } + valid_rows_count: { type: :integer, minimum: 0 }, + invalid_rows_count: { type: :integer, minimum: 0 }, + mappings_count: { type: :integer, minimum: 0 }, + unassigned_mappings_count: { type: :integer, minimum: 0 } } }, + ImportStatusSummary: { + type: :object, + required: %w[uploaded configured terminal], + properties: { + uploaded: { type: :boolean }, + configured: { type: :boolean }, + terminal: { type: :boolean } + } + }, + ImportStatusDetail: { + allOf: [ + { '$ref' => '#/components/schemas/ImportStatusSummary' }, + { + type: :object, + required: %w[cleaned publishable revertable], + properties: { + cleaned: { type: :boolean }, + publishable: { type: :boolean }, + revertable: { type: :boolean } + } + } + ] + }, ImportSummary: { type: :object, - required: %w[id type status created_at updated_at], + required: %w[id type status created_at updated_at status_detail], properties: { id: { type: :string, format: :uuid }, type: { type: :string, enum: %w[TransactionImport TradeImport AccountImport MintImport CategoryImport RuleImport SureImport] }, @@ -543,12 +570,13 @@ RSpec.configure do |config| updated_at: { type: :string, format: :'date-time' }, account_id: { type: :string, format: :uuid, nullable: true }, rows_count: { type: :integer, minimum: 0 }, - error: { type: :string, nullable: true } + error: { type: :string, nullable: true }, + status_detail: { '$ref' => '#/components/schemas/ImportStatusSummary' } } }, ImportDetail: { type: :object, - required: %w[id type status created_at updated_at], + required: %w[id type status created_at updated_at status_detail configuration stats], properties: { id: { type: :string, format: :uuid }, type: { type: :string, enum: %w[TransactionImport TradeImport AccountImport MintImport CategoryImport RuleImport SureImport] }, @@ -557,6 +585,7 @@ RSpec.configure do |config| updated_at: { type: :string, format: :'date-time' }, account_id: { type: :string, format: :uuid, nullable: true }, error: { type: :string, nullable: true }, + status_detail: { '$ref' => '#/components/schemas/ImportStatusDetail' }, configuration: { '$ref' => '#/components/schemas/ImportConfiguration' }, stats: { '$ref' => '#/components/schemas/ImportStats' } } diff --git a/test/controllers/api/v1/imports_controller_test.rb b/test/controllers/api/v1/imports_controller_test.rb index 5e1099158..a14377947 100644 --- a/test/controllers/api/v1/imports_controller_test.rb +++ b/test/controllers/api/v1/imports_controller_test.rb @@ -38,6 +38,12 @@ class Api::V1::ImportsControllerTest < ActionDispatch::IntegrationTest json_response = JSON.parse(response.body) assert_not_empty json_response["data"] assert_equal @family.imports.count, json_response["meta"]["total_count"] + + import_data = json_response["data"].detect { |data| data["id"] == @import.id } + assert_not_nil import_data + assert_equal @import.uploaded?, import_data["status_detail"]["uploaded"] + assert_equal @import.configured?, import_data["status_detail"]["configured"] + assert_equal @import.complete? || @import.failed? || @import.revert_failed?, import_data["status_detail"]["terminal"] end test "should show import" do @@ -45,8 +51,26 @@ class Api::V1::ImportsControllerTest < ActionDispatch::IntegrationTest assert_response :success json_response = JSON.parse(response.body) + rows = @import.rows.to_a + valid_rows_count = rows.count(&:valid?) + invalid_rows_count = rows.length - valid_rows_count + assert_equal @import.id, json_response["data"]["id"] assert_equal @import.status, json_response["data"]["status"] + assert json_response["data"].key?("status_detail") + assert_equal @import.uploaded?, json_response["data"]["status_detail"]["uploaded"] + assert_equal @import.configured?, json_response["data"]["status_detail"]["configured"] + assert_equal @import.cleaned_from_validation_stats?(invalid_rows_count: invalid_rows_count), + json_response["data"]["status_detail"]["cleaned"] + assert_equal @import.publishable_from_validation_stats?(invalid_rows_count: invalid_rows_count), + json_response["data"]["status_detail"]["publishable"] + assert_equal @import.revertable?, json_response["data"]["status_detail"]["revertable"] + assert_equal @import.rows_count, json_response["data"]["stats"]["rows_count"] + assert_equal valid_rows_count, json_response["data"]["stats"]["valid_rows_count"] + assert_equal invalid_rows_count, json_response["data"]["stats"]["invalid_rows_count"] + assert_equal @import.mappings.count, json_response["data"]["stats"]["mappings_count"] + assert_equal @import.mappings.where(mappable_id: nil).count, + json_response["data"]["stats"]["unassigned_mappings_count"] end test "should create import with raw content" do diff --git a/test/models/pdf_import_test.rb b/test/models/pdf_import_test.rb index 37281b5cc..f8568d452 100644 --- a/test/models/pdf_import_test.rb +++ b/test/models/pdf_import_test.rb @@ -41,6 +41,19 @@ class PdfImportTest < ActiveSupport::TestCase assert_not @processed_import.publishable? end + test "status detail cleaned check requires account and transaction statement" do + @import_with_rows.update!(account: accounts(:depository), document_type: "bank_statement") + + assert @import_with_rows.cleaned_from_validation_stats?(invalid_rows_count: 0) + assert_not @import_with_rows.cleaned_from_validation_stats?(invalid_rows_count: 1) + + @import_with_rows.update!(account: nil) + assert_not @import_with_rows.cleaned_from_validation_stats?(invalid_rows_count: 0) + + @import_with_rows.update!(account: accounts(:depository), document_type: "other") + assert_not @import_with_rows.cleaned_from_validation_stats?(invalid_rows_count: 0) + end + test "column_keys returns transaction columns" do assert_equal %i[date amount name category notes], @import.column_keys end diff --git a/test/models/sure_import_test.rb b/test/models/sure_import_test.rb index 87d70db8c..65fbe483c 100644 --- a/test/models/sure_import_test.rb +++ b/test/models/sure_import_test.rb @@ -79,6 +79,17 @@ class SureImportTest < ActiveSupport::TestCase assert @import.publishable? end + test "status predicates honor validation stats" do + attach_ndjson(build_ndjson([ + { type: "Account", data: { id: "uuid-1", name: "Test", balance: "1000", currency: "USD", accountable_type: "Depository" } } + ])) + + assert @import.cleaned_from_validation_stats?(invalid_rows_count: 0) + assert @import.publishable_from_validation_stats?(invalid_rows_count: 0) + assert_not @import.cleaned_from_validation_stats?(invalid_rows_count: 1) + assert_not @import.publishable_from_validation_stats?(invalid_rows_count: 1) + end + test "dry_run returns counts by type" do attach_ndjson(build_ndjson([ { type: "Account", data: { id: "uuid-1" } }, @@ -97,6 +108,22 @@ class SureImportTest < ActiveSupport::TestCase assert_equal 0, dry_run[:tags] end + test "cached ndjson content is refreshed when attachment is replaced" do + attach_ndjson(build_ndjson([ + { type: "Account", data: { id: "uuid-1" } } + ])) + assert_equal 1, @import.dry_run[:accounts] + + attach_ndjson(build_ndjson([ + { type: "Transaction", data: { id: "uuid-2" } } + ])) + + dry_run = @import.dry_run + assert_equal 0, dry_run[:accounts] + assert_equal 1, dry_run[:transactions] + assert_equal 1, @import.rows_count + end + test "sync_ndjson_rows_count! sets total row count" do attach_ndjson(build_ndjson([ { type: "Account", data: { id: "uuid-1" } },