mirror of
https://github.com/we-promise/sure.git
synced 2026-05-24 21:14:56 +00:00
feat(api): add import preflight validation (#1755)
* feat(api): add import preflight validation * fix(api): harden import preflight validation
This commit is contained in:
ghost
@@ -405,9 +405,7 @@ class Api::V1::ImportsControllerTest < ActionDispatch::IntegrationTest
|
||||
original_filename: "large.ndjson"
|
||||
)
|
||||
|
||||
original_value = SureImport::MAX_NDJSON_SIZE
|
||||
SureImport.send(:remove_const, :MAX_NDJSON_SIZE)
|
||||
SureImport.const_set(:MAX_NDJSON_SIZE, test_limit)
|
||||
SureImport.stubs(:max_ndjson_size).returns(test_limit)
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post api_v1_imports_url,
|
||||
@@ -421,9 +419,6 @@ class Api::V1::ImportsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_response :unprocessable_entity
|
||||
json_response = JSON.parse(response.body)
|
||||
assert_equal "file_too_large", json_response["error"]
|
||||
ensure
|
||||
SureImport.send(:remove_const, :MAX_NDJSON_SIZE)
|
||||
SureImport.const_set(:MAX_NDJSON_SIZE, original_value)
|
||||
end
|
||||
|
||||
test "should reject Sure import uploaded file with invalid type" do
|
||||
@@ -551,6 +546,473 @@ class Api::V1::ImportsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_equal "invalid_ndjson", json_response["error"]
|
||||
end
|
||||
|
||||
test "should preflight CSV import without persisting records" do
|
||||
csv_content = "date,amount,name\n2023-01-01,-10.00,Test Transaction"
|
||||
|
||||
assert_no_difference([ "Import.count", "Import::Row.count" ]) do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: csv_content,
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
json_response = JSON.parse(response.body)
|
||||
data = json_response["data"]
|
||||
|
||||
assert_equal "TransactionImport", data["type"]
|
||||
assert_equal true, data["valid"]
|
||||
assert_equal 1, data["stats"]["rows_count"]
|
||||
assert_not data["stats"].key?("valid_rows_count")
|
||||
assert_not data["stats"].key?("invalid_rows_count")
|
||||
assert_equal %w[date amount name], data["headers"]
|
||||
assert_empty data["missing_required_headers"]
|
||||
assert_empty data["errors"]
|
||||
end
|
||||
|
||||
test "should report missing required CSV headers during preflight" do
|
||||
csv_content = "name\nMissing Amount"
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: csv_content,
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal false, data["valid"]
|
||||
assert_equal 1, data["stats"]["rows_count"]
|
||||
assert_not data["stats"].key?("valid_rows_count")
|
||||
assert_not data["stats"].key?("invalid_rows_count")
|
||||
assert_equal [ "date", "amount" ], data["missing_required_headers"]
|
||||
assert_equal "missing_required_headers", data["errors"].first["code"]
|
||||
end
|
||||
|
||||
test "should apply rows_to_skip before CSV preflight header validation" do
|
||||
csv_content = [
|
||||
"Generated by bank export",
|
||||
"posted,amount,description",
|
||||
"2024-01-01,-10.00,Coffee"
|
||||
].join("\n")
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: csv_content,
|
||||
rows_to_skip: 1,
|
||||
date_col_label: "posted",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "description",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal true, data["valid"]
|
||||
assert_equal 1, data["stats"]["rows_count"]
|
||||
assert_equal %w[posted amount description], data["headers"]
|
||||
assert_empty data["missing_required_headers"]
|
||||
end
|
||||
|
||||
test "should preflight semicolon separated CSV content" do
|
||||
csv_content = "date;amount;name\n2024-01-01;-10.00;Coffee"
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: csv_content,
|
||||
col_sep: ";",
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal true, data["valid"]
|
||||
assert_equal 1, data["stats"]["rows_count"]
|
||||
assert_equal %w[date amount name], data["headers"]
|
||||
end
|
||||
|
||||
test "should report invalid preflight CSV parser config without parsing" do
|
||||
csv_content = "date,amount,name\n2024-01-01,-10.00,Coffee"
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: csv_content,
|
||||
col_sep: "",
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal false, data["valid"]
|
||||
assert_equal 0, data["stats"]["rows_count"]
|
||||
assert_empty data["headers"]
|
||||
assert_equal "validation_failed", data["errors"].first["code"]
|
||||
end
|
||||
|
||||
test "should reject malformed CSV during preflight" do
|
||||
csv_content = "date,amount,name\n2024-01-01,-10.00,\"Coffee Shop"
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: csv_content,
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :unprocessable_entity
|
||||
json_response = JSON.parse(response.body)
|
||||
assert_equal "invalid_csv", json_response["error"]
|
||||
end
|
||||
|
||||
test "should include preflight exception message in internal server error response" do
|
||||
Import::Preflight.any_instance.stubs(:call).raises(StandardError, "boom")
|
||||
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: "date,amount,name\n2024-01-01,-10.00,Coffee",
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name"
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
|
||||
assert_response :internal_server_error
|
||||
json_response = JSON.parse(response.body)
|
||||
assert_equal "internal_server_error", json_response["error"]
|
||||
assert_equal "Error: boom", json_response["message"]
|
||||
end
|
||||
|
||||
test "should reject unknown preflight import type" do
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "FakeImport",
|
||||
raw_file_content: "date,amount,name\n2023-01-01,-10.00,Test Transaction"
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :unprocessable_entity
|
||||
response_data = JSON.parse(response.body)
|
||||
assert_equal "invalid_import_type", response_data["error"]
|
||||
assert_not response_data.key?("errors")
|
||||
end
|
||||
|
||||
test "should reject import types excluded from preflight" do
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "QifImport",
|
||||
raw_file_content: "!Type:Bank\nD01/01/2024\nT-10.00\nPTest\n^"
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :unprocessable_entity
|
||||
response_data = JSON.parse(response.body)
|
||||
assert_equal "invalid_import_type", response_data["error"]
|
||||
assert_not response_data.key?("errors")
|
||||
assert_not_includes response_data["message"], "QifImport"
|
||||
assert_not_includes response_data["message"], "PdfImport"
|
||||
end
|
||||
|
||||
test "should report empty CSV preflight content as invalid" do
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: "date,amount,name\n",
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal false, data["valid"]
|
||||
assert_equal 0, data["stats"]["rows_count"]
|
||||
assert_equal "no_data_rows", data["errors"].first["code"]
|
||||
assert_empty data["warnings"]
|
||||
end
|
||||
|
||||
test "should preflight Sure import without persisting records" do
|
||||
ndjson_content = [
|
||||
{ type: "Account", data: { id: "account_1", name: "Checking" } }.to_json,
|
||||
{ type: "Transaction", data: { id: "entry_1", account_id: "account_1" } }.to_json
|
||||
].join("\n")
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "SureImport",
|
||||
raw_file_content: ndjson_content
|
||||
},
|
||||
headers: api_headers(@api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal "SureImport", data["type"]
|
||||
assert_equal true, data["valid"]
|
||||
assert_equal 2, data["stats"]["rows_count"]
|
||||
assert_equal 1, data["stats"]["entity_counts"]["accounts"]
|
||||
assert_equal 1, data["stats"]["entity_counts"]["transactions"]
|
||||
assert_empty data["errors"]
|
||||
end
|
||||
|
||||
test "should report invalid Sure import NDJSON during preflight" do
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "SureImport",
|
||||
raw_file_content: "not ndjson"
|
||||
},
|
||||
headers: api_headers(@api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal false, data["valid"]
|
||||
assert_equal 1, data["stats"]["invalid_rows_count"]
|
||||
assert_equal "invalid_json", data["errors"].first["code"]
|
||||
end
|
||||
|
||||
test "should report non-object Sure import NDJSON records during preflight" do
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "SureImport",
|
||||
raw_file_content: "[]"
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal false, data["valid"]
|
||||
assert_equal 1, data["stats"]["invalid_rows_count"]
|
||||
assert_equal "invalid_ndjson_record", data["errors"].first["code"]
|
||||
end
|
||||
|
||||
test "should report empty Sure import file as invalid during preflight" do
|
||||
empty_file = Rack::Test::UploadedFile.new(
|
||||
StringIO.new(""),
|
||||
"application/x-ndjson",
|
||||
original_filename: "empty.ndjson"
|
||||
)
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "SureImport",
|
||||
file: empty_file
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal false, data["valid"]
|
||||
assert_equal 0, data["stats"]["rows_count"]
|
||||
assert_equal "no_data_rows", data["errors"].first["code"]
|
||||
assert_empty data["warnings"]
|
||||
end
|
||||
|
||||
test "should reject preflight with no file or raw content" do
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: { type: "SureImport" },
|
||||
headers: api_headers(@api_key)
|
||||
end
|
||||
|
||||
assert_response :unprocessable_entity
|
||||
assert_equal "missing_content", JSON.parse(response.body)["error"]
|
||||
end
|
||||
|
||||
test "should reject oversized file uploads during preflight" do
|
||||
test_limit = 1.kilobyte
|
||||
large_file = Rack::Test::UploadedFile.new(
|
||||
StringIO.new("x" * (test_limit + 1)),
|
||||
"text/csv",
|
||||
original_filename: "large.csv"
|
||||
)
|
||||
|
||||
Import.stubs(:max_csv_size).returns(test_limit)
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: { file: large_file },
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :unprocessable_entity
|
||||
assert_equal "file_too_large", JSON.parse(response.body)["error"]
|
||||
end
|
||||
|
||||
test "should preflight with read-only API key" do
|
||||
csv_content = "date,amount,name\n2023-01-01,-10.00,Test Transaction"
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: csv_content,
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: @account.id
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
assert_equal true, JSON.parse(response.body)["data"]["valid"]
|
||||
end
|
||||
|
||||
test "should require authentication for preflight" do
|
||||
post preflight_api_v1_imports_url, params: {
|
||||
raw_file_content: "date,amount,name\n2023-01-01,-10.00,Test Transaction"
|
||||
}
|
||||
|
||||
assert_response :unauthorized
|
||||
end
|
||||
|
||||
test "should return not found for preflight account outside family" do
|
||||
other_family = Family.create!(name: "Other Family", currency: "USD", locale: "en")
|
||||
other_depository = Depository.create!(subtype: "checking")
|
||||
other_account = Account.create!(
|
||||
family: other_family,
|
||||
name: "Other Account",
|
||||
currency: "USD",
|
||||
classification: "asset",
|
||||
accountable: other_depository,
|
||||
balance: 0
|
||||
)
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: "date,amount,name\n2023-01-01,-10.00,Test Transaction",
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: other_account.id
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :not_found
|
||||
assert_equal "record_not_found", JSON.parse(response.body)["error"]
|
||||
end
|
||||
|
||||
test "should return not found for malformed preflight account id" do
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
raw_file_content: "date,amount,name\n2023-01-01,-10.00,Test Transaction",
|
||||
date_col_label: "date",
|
||||
amount_col_label: "amount",
|
||||
name_col_label: "name",
|
||||
account_id: "not-a-uuid"
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :not_found
|
||||
assert_equal "record_not_found", JSON.parse(response.body)["error"]
|
||||
end
|
||||
|
||||
test "should apply Mint defaults before preflight header validation" do
|
||||
mint_content = [
|
||||
"Date,Amount,Account Name,Description,Category,Labels,Currency,Notes,Transaction Type",
|
||||
"01/01/2024,-8.55,Checking,Starbucks,Food & Drink,Coffee,USD,Morning coffee,debit"
|
||||
].join("\n")
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "MintImport",
|
||||
raw_file_content: mint_content
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal "MintImport", data["type"]
|
||||
assert_equal true, data["valid"]
|
||||
assert_empty data["missing_required_headers"]
|
||||
assert_includes data["required_headers"], "Date"
|
||||
assert_includes data["required_headers"], "Amount"
|
||||
end
|
||||
|
||||
test "should not overwrite explicit Mint preflight column mappings with defaults" do
|
||||
mint_content = [
|
||||
"Posted On,Value,Description",
|
||||
"01/01/2024,-8.55,Starbucks"
|
||||
].join("\n")
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post preflight_api_v1_imports_url,
|
||||
params: {
|
||||
type: "MintImport",
|
||||
raw_file_content: mint_content,
|
||||
date_col_label: "Posted On",
|
||||
amount_col_label: "Value"
|
||||
},
|
||||
headers: api_headers(@read_only_api_key)
|
||||
end
|
||||
|
||||
assert_response :success
|
||||
data = JSON.parse(response.body)["data"]
|
||||
|
||||
assert_equal true, data["valid"]
|
||||
assert_equal [ "Posted On", "Value" ], data["required_headers"]
|
||||
assert_empty data["missing_required_headers"]
|
||||
end
|
||||
|
||||
test "should create import and auto-publish when configured and requested" do
|
||||
csv_content = "date,amount,name\n2023-01-01,-10.00,Test Transaction"
|
||||
|
||||
@@ -633,9 +1095,7 @@ class Api::V1::ImportsControllerTest < ActionDispatch::IntegrationTest
|
||||
test_limit = 1.kilobyte
|
||||
large_content = "x" * (test_limit + 1)
|
||||
|
||||
original_value = Import::MAX_CSV_SIZE
|
||||
Import.send(:remove_const, :MAX_CSV_SIZE)
|
||||
Import.const_set(:MAX_CSV_SIZE, test_limit)
|
||||
Import.stubs(:max_csv_size).returns(test_limit)
|
||||
|
||||
assert_no_difference("Import.count") do
|
||||
post api_v1_imports_url,
|
||||
@@ -646,9 +1106,6 @@ class Api::V1::ImportsControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_response :unprocessable_entity
|
||||
json_response = JSON.parse(response.body)
|
||||
assert_equal "content_too_large", json_response["error"]
|
||||
ensure
|
||||
Import.send(:remove_const, :MAX_CSV_SIZE)
|
||||
Import.const_set(:MAX_CSV_SIZE, original_value)
|
||||
end
|
||||
|
||||
test "should accept file upload with valid csv mime type" do
|
||||
|
||||
Reference in New Issue
Block a user