From c0f03e0398ef8ea04c7ee1f3e5950cc006fa2954 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Dec 2025 09:40:27 +0000 Subject: [PATCH 1/3] Initial plan From fd036fb6e6a67124f606aa8019db0f97c1b9ef6a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Dec 2025 09:50:41 +0000 Subject: [PATCH 2/3] Add Name and Currency support to CSV import with comprehensive tests Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com> --- .config/dotnet-tools.json | 10 +- .../Services/FileImportService.cs | 14 + .../Context/BankTrackerDbContext.cs | 7 +- ...equiredNameAndCurrencyDefaults.Designer.cs | 273 +++++++++ ...4646_AddRequiredNameAndCurrencyDefaults.cs | 60 ++ .../BankTrackerDbContextModelSnapshot.cs | 9 +- .../Services/FileImportServiceTests.cs | 573 ++++++++++++++++++ 7 files changed, 942 insertions(+), 4 deletions(-) create mode 100644 PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.Designer.cs create mode 100644 PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs create mode 100644 PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs diff --git a/.config/dotnet-tools.json b/.config/dotnet-tools.json index b0e38ab..fbb7b76 100644 --- a/.config/dotnet-tools.json +++ b/.config/dotnet-tools.json @@ -1,5 +1,13 @@ { "version": 1, "isRoot": true, - "tools": {} + "tools": { + "dotnet-ef": { + "version": "10.0.1", + "commands": [ + "dotnet-ef" + ], + "rollForward": false + } + } } \ No newline at end of file diff --git a/PhantomDave.BankTracking.Api/Services/FileImportService.cs b/PhantomDave.BankTracking.Api/Services/FileImportService.cs index 764d5f4..c6283e4 100644 --- a/PhantomDave.BankTracking.Api/Services/FileImportService.cs +++ b/PhantomDave.BankTracking.Api/Services/FileImportService.cs @@ -277,6 +277,20 @@ public IEnumerable FromParsedData(int accountId, ParsedFileData p record.Description = descriptionColumnValue; } + if (input.ColumnMappings.TryGetValue("Name", out var nameColumn) && row.TryGetValue(nameColumn, out var nameColumnValue)) + { + record.Name = nameColumnValue; + } + + if (input.ColumnMappings.TryGetValue("Currency", out var currencyColumn) && row.TryGetValue(currencyColumn, out var currencyColumnValue) && !string.IsNullOrWhiteSpace(currencyColumnValue)) + { + record.Currency = currencyColumnValue.ToUpperInvariant(); + } + else + { + record.Currency = "USD"; + } + record.AccountId = accountId; record.Imported = true; diff --git a/PhantomDave.BankTracking.Data/Context/BankTrackerDbContext.cs b/PhantomDave.BankTracking.Data/Context/BankTrackerDbContext.cs index 80e2f9b..84d89f5 100644 --- a/PhantomDave.BankTracking.Data/Context/BankTrackerDbContext.cs +++ b/PhantomDave.BankTracking.Data/Context/BankTrackerDbContext.cs @@ -45,9 +45,14 @@ private static void ConfigureFinanceRecord(ModelBuilder modelBuilder) entity.Property(fr => fr.Id).ValueGeneratedOnAdd(); entity.Property(fr => fr.Amount) .HasColumnType("numeric(18,2)"); + entity.Property(fr => fr.Name) + .IsRequired() + .HasMaxLength(200) + .HasDefaultValue("Untitled"); entity.Property(fr => fr.Currency) .IsRequired() - .HasMaxLength(3); + .HasMaxLength(3) + .HasDefaultValue("USD"); entity.Property(fr => fr.Description) .HasMaxLength(500); entity.HasOne() diff --git a/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.Designer.cs b/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.Designer.cs new file mode 100644 index 0000000..19415be --- /dev/null +++ b/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.Designer.cs @@ -0,0 +1,273 @@ +// +using System; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.EntityFrameworkCore.Storage.ValueConversion; +using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata; +using PhantomDave.BankTracking.Data.Context; + +#nullable disable + +namespace PhantomDave.BankTracking.Data.Migrations +{ + [DbContext(typeof(BankTrackerDbContext))] + [Migration("20251211094646_AddRequiredNameAndCurrencyDefaults")] + partial class AddRequiredNameAndCurrencyDefaults + { + /// + protected override void BuildTargetModel(ModelBuilder modelBuilder) + { +#pragma warning disable 612, 618 + modelBuilder + .HasAnnotation("ProductVersion", "10.0.0") + .HasAnnotation("Relational:MaxIdentifierLength", 63); + + NpgsqlModelBuilderExtensions.UseIdentityByDefaultColumns(modelBuilder); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.Account", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("integer"); + + NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("CurrentBalance") + .HasColumnType("numeric"); + + b.Property("Email") + .IsRequired() + .HasColumnType("text"); + + b.Property("PasswordHash") + .IsRequired() + .HasColumnType("text"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.ToTable("Accounts"); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.BankImportTemplate", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("integer"); + + NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); + + b.Property("AccountId") + .HasColumnType("integer"); + + b.Property("BankName") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.Property("ColumnMappings") + .IsRequired() + .HasColumnType("jsonb"); + + b.Property("CreatedAt") + .HasColumnType("timestamp with time zone"); + + b.Property("DateFormat") + .IsRequired() + .HasMaxLength(50) + .HasColumnType("character varying(50)"); + + b.Property("DecimalSeparator") + .IsRequired() + .HasMaxLength(1) + .HasColumnType("character varying(1)"); + + b.Property("IsDefault") + .HasColumnType("boolean"); + + b.Property("ThousandsSeparator") + .IsRequired() + .HasMaxLength(1) + .HasColumnType("character varying(1)"); + + b.Property("UpdatedAt") + .HasColumnType("timestamp with time zone"); + + b.HasKey("Id"); + + b.HasIndex("AccountId", "BankName"); + + b.HasIndex("AccountId", "IsDefault"); + + b.ToTable("BankImportTemplates"); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.Dashboard", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("integer"); + + NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); + + b.Property("AccountId") + .HasColumnType("integer"); + + b.Property("Name") + .IsRequired() + .HasMaxLength(200) + .HasColumnType("character varying(200)"); + + b.HasKey("Id"); + + b.HasIndex("AccountId"); + + b.ToTable("Dashboards"); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.DashboardWidget", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("integer"); + + NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); + + b.Property("Cols") + .HasColumnType("integer"); + + b.Property("DashboardId") + .HasColumnType("integer"); + + b.Property("Rows") + .HasColumnType("integer"); + + b.Property("Type") + .HasColumnType("integer"); + + b.Property("X") + .HasColumnType("integer"); + + b.Property("Y") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.HasIndex("DashboardId"); + + b.ToTable("DashboardWidgets"); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.FinanceRecord", b => + { + b.Property("Id") + .ValueGeneratedOnAdd() + .HasColumnType("integer"); + + NpgsqlPropertyBuilderExtensions.UseIdentityByDefaultColumn(b.Property("Id")); + + b.Property("AccountId") + .HasColumnType("integer"); + + b.Property("Amount") + .HasColumnType("numeric(18,2)"); + + b.Property("Currency") + .IsRequired() + .ValueGeneratedOnAdd() + .HasMaxLength(3) + .HasColumnType("character varying(3)") + .HasDefaultValue("USD"); + + b.Property("Date") + .HasColumnType("timestamp with time zone"); + + b.Property("Description") + .IsRequired() + .HasMaxLength(500) + .HasColumnType("character varying(500)"); + + b.Property("Imported") + .HasColumnType("boolean"); + + b.Property("IsRecurring") + .HasColumnType("boolean"); + + b.Property("IsRecurringInstance") + .HasColumnType("boolean"); + + b.Property("LastProcessedDate") + .HasColumnType("timestamp with time zone"); + + b.Property("Name") + .IsRequired() + .ValueGeneratedOnAdd() + .HasMaxLength(200) + .HasColumnType("character varying(200)") + .HasDefaultValue("Untitled"); + + b.Property("ParentRecurringRecordId") + .HasColumnType("integer"); + + b.Property("RecurrenceEndDate") + .HasColumnType("timestamp with time zone"); + + b.Property("RecurrenceFrequency") + .HasColumnType("integer"); + + b.HasKey("Id"); + + b.HasIndex("AccountId"); + + b.ToTable("FinanceRecords"); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.BankImportTemplate", b => + { + b.HasOne("PhantomDave.BankTracking.Library.Models.Account", null) + .WithMany() + .HasForeignKey("AccountId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.Dashboard", b => + { + b.HasOne("PhantomDave.BankTracking.Library.Models.Account", null) + .WithMany() + .HasForeignKey("AccountId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.DashboardWidget", b => + { + b.HasOne("PhantomDave.BankTracking.Library.Models.Dashboard", null) + .WithMany("Widgets") + .HasForeignKey("DashboardId") + .OnDelete(DeleteBehavior.Cascade) + .IsRequired(); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.FinanceRecord", b => + { + b.HasOne("PhantomDave.BankTracking.Library.Models.Account", null) + .WithMany() + .HasForeignKey("AccountId") + .OnDelete(DeleteBehavior.Cascade); + }); + + modelBuilder.Entity("PhantomDave.BankTracking.Library.Models.Dashboard", b => + { + b.Navigation("Widgets"); + }); +#pragma warning restore 612, 618 + } + } +} diff --git a/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs b/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs new file mode 100644 index 0000000..1f632da --- /dev/null +++ b/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs @@ -0,0 +1,60 @@ +using Microsoft.EntityFrameworkCore.Migrations; + +#nullable disable + +namespace PhantomDave.BankTracking.Data.Migrations +{ + /// + public partial class AddRequiredNameAndCurrencyDefaults : Migration + { + /// + protected override void Up(MigrationBuilder migrationBuilder) + { + migrationBuilder.AlterColumn( + name: "Name", + table: "FinanceRecords", + type: "character varying(200)", + maxLength: 200, + nullable: false, + defaultValue: "Untitled", + oldClrType: typeof(string), + oldType: "text"); + + migrationBuilder.AlterColumn( + name: "Currency", + table: "FinanceRecords", + type: "character varying(3)", + maxLength: 3, + nullable: false, + defaultValue: "USD", + oldClrType: typeof(string), + oldType: "character varying(3)", + oldMaxLength: 3); + } + + /// + protected override void Down(MigrationBuilder migrationBuilder) + { + migrationBuilder.AlterColumn( + name: "Name", + table: "FinanceRecords", + type: "text", + nullable: false, + oldClrType: typeof(string), + oldType: "character varying(200)", + oldMaxLength: 200, + oldDefaultValue: "Untitled"); + + migrationBuilder.AlterColumn( + name: "Currency", + table: "FinanceRecords", + type: "character varying(3)", + maxLength: 3, + nullable: false, + oldClrType: typeof(string), + oldType: "character varying(3)", + oldMaxLength: 3, + oldDefaultValue: "USD"); + } + } +} diff --git a/PhantomDave.BankTracking.Data/Migrations/BankTrackerDbContextModelSnapshot.cs b/PhantomDave.BankTracking.Data/Migrations/BankTrackerDbContextModelSnapshot.cs index 5336b60..b036350 100644 --- a/PhantomDave.BankTracking.Data/Migrations/BankTrackerDbContextModelSnapshot.cs +++ b/PhantomDave.BankTracking.Data/Migrations/BankTrackerDbContextModelSnapshot.cs @@ -177,8 +177,10 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("Currency") .IsRequired() + .ValueGeneratedOnAdd() .HasMaxLength(3) - .HasColumnType("character varying(3)"); + .HasColumnType("character varying(3)") + .HasDefaultValue("USD"); b.Property("Date") .HasColumnType("timestamp with time zone"); @@ -202,7 +204,10 @@ protected override void BuildModel(ModelBuilder modelBuilder) b.Property("Name") .IsRequired() - .HasColumnType("text"); + .ValueGeneratedOnAdd() + .HasMaxLength(200) + .HasColumnType("character varying(200)") + .HasDefaultValue("Untitled"); b.Property("ParentRecurringRecordId") .HasColumnType("integer"); diff --git a/PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs b/PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs new file mode 100644 index 0000000..93762a6 --- /dev/null +++ b/PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs @@ -0,0 +1,573 @@ +using Microsoft.Extensions.Logging; +using Moq; +using PhantomDave.BankTracking.Api.Services; +using PhantomDave.BankTracking.Api.Types.ObjectTypes; +using PhantomDave.BankTracking.Library.Models; + +namespace PhantomDave.BankTracking.UnitTests.Services; + +public class FileImportServiceTests +{ + private readonly Mock> _mockLogger; + private readonly FileImportService _service; + + public FileImportServiceTests() + { + _mockLogger = new Mock>(); + _service = new FileImportService(_mockLogger.Object); + } + + [Fact] + public void FromParsedData_WithAllFields_PopulatesAllProperties() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Description"] = "Test transaction", + ["Name"] = "Test Name", + ["Currency"] = "EUR" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Description"] = "Description", + ["Name"] = "Name", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + var record = results[0]; + Assert.Equal(accountId, record.AccountId); + Assert.Equal(100.50m, record.Amount); + Assert.Equal("Test transaction", record.Description); + Assert.Equal("Test Name", record.Name); + Assert.Equal("EUR", record.Currency); + Assert.True(record.Imported); + } + + [Fact] + public void FromParsedData_WithNameOnly_PopulatesNameAndDefaultCurrency() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Description"] = "Test transaction", + ["Name"] = "Test Name" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Description"] = "Description", + ["Name"] = "Name" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + var record = results[0]; + Assert.Equal("Test Name", record.Name); + Assert.Equal("USD", record.Currency); // Default currency + } + + [Fact] + public void FromParsedData_WithCurrencyOnly_PopulatesCurrencyAndDefaultName() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Description"] = "Test transaction", + ["Currency"] = "GBP" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Description"] = "Description", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + var record = results[0]; + Assert.Equal(string.Empty, record.Name); // Default from model + Assert.Equal("GBP", record.Currency); + } + + [Fact] + public void FromParsedData_WithoutNameAndCurrency_UsesDefaults() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Description"] = "Test transaction" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Description"] = "Description" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + var record = results[0]; + Assert.Equal(string.Empty, record.Name); // Default from model + Assert.Equal("USD", record.Currency); // Default from code + } + + [Fact] + public void FromParsedData_WithLowercaseCurrency_ConvertsToUppercase() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Currency"] = "eur" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + var record = results[0]; + Assert.Equal("EUR", record.Currency); // Should be uppercase + } + + [Fact] + public void FromParsedData_WithMixedCaseCurrency_ConvertsToUppercase() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Currency"] = "uSd" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + var record = results[0]; + Assert.Equal("USD", record.Currency); + } + + [Fact] + public void FromParsedData_WithEmptyCurrency_UsesDefault() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Currency"] = "" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + var record = results[0]; + Assert.Equal("USD", record.Currency); // Should use default + } + + [Fact] + public void FromParsedData_WithMultipleRows_ProcessesAllCorrectly() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Description"] = "Transaction 1", + ["Name"] = "Name 1", + ["Currency"] = "EUR" + }, + new Dictionary + { + ["Date"] = "2024-01-16", + ["Amount"] = "200.75", + ["Description"] = "Transaction 2", + ["Name"] = "Name 2", + ["Currency"] = "GBP" + }, + new Dictionary + { + ["Date"] = "2024-01-17", + ["Amount"] = "300.00", + ["Description"] = "Transaction 3", + ["Name"] = "Name 3", + ["Currency"] = "usd" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Description"] = "Description", + ["Name"] = "Name", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Equal(3, results.Count); + + Assert.Equal("Name 1", results[0].Name); + Assert.Equal("EUR", results[0].Currency); + Assert.Equal(100.50m, results[0].Amount); + + Assert.Equal("Name 2", results[1].Name); + Assert.Equal("GBP", results[1].Currency); + Assert.Equal(200.75m, results[1].Amount); + + Assert.Equal("Name 3", results[2].Name); + Assert.Equal("USD", results[2].Currency); + Assert.Equal(300.00m, results[2].Amount); + } + + [Fact] + public void FromParsedData_WithSpecialCharactersInName_PreservesName() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Name"] = "Café München & Co." + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Name"] = "Name" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal("Café München & Co.", results[0].Name); + } + + [Fact] + public void FromParsedData_MarksAllRecordsAsImported() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50" + }, + new Dictionary + { + ["Date"] = "2024-01-16", + ["Amount"] = "200.00" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.All(results, record => Assert.True(record.Imported)); + } + + [Fact] + public void FromParsedData_SetsCorrectAccountId() + { + // Arrange + var accountId = 42; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal(accountId, results[0].AccountId); + } + + [Fact] + public void FromParsedData_WithInvalidAmount_CreatesRecordWithZeroAmount() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "invalid" + }, + new Dictionary + { + ["Date"] = "2024-01-16", + ["Amount"] = "100.50" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Equal(2, results.Count); // Both rows are processed + Assert.Equal(0m, results[0].Amount); // Invalid amount defaults to 0 + Assert.Equal(100.50m, results[1].Amount); + } + + [Fact] + public void FromParsedData_WithDateFormatting_ParsesCorrectly() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Name"] = "Test", + ["Currency"] = "USD" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Name"] = "Name", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal(new DateTime(2024, 1, 15, 0, 0, 0, DateTimeKind.Utc), results[0].Date); + } + + [Fact] + public void FromParsedData_WithEmptyRows_ReturnsEmptyList() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = [] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Empty(results); + } +} From 94ea874ce38ac57facf210e9337e8eb8f12862b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 11 Dec 2025 10:07:38 +0000 Subject: [PATCH 3/3] Address code review feedback: add validation, tests, and migration backfill Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com> --- .../Services/FileImportService.cs | 40 ++- ...4646_AddRequiredNameAndCurrencyDefaults.cs | 4 + .../Services/FileImportServiceTests.cs | 256 ++++++++++++++++++ 3 files changed, 291 insertions(+), 9 deletions(-) diff --git a/PhantomDave.BankTracking.Api/Services/FileImportService.cs b/PhantomDave.BankTracking.Api/Services/FileImportService.cs index c6283e4..6e6aeb5 100644 --- a/PhantomDave.BankTracking.Api/Services/FileImportService.cs +++ b/PhantomDave.BankTracking.Api/Services/FileImportService.cs @@ -279,17 +279,27 @@ public IEnumerable FromParsedData(int accountId, ParsedFileData p if (input.ColumnMappings.TryGetValue("Name", out var nameColumn) && row.TryGetValue(nameColumn, out var nameColumnValue)) { - record.Name = nameColumnValue; + var trimmedName = nameColumnValue?.Trim() ?? string.Empty; + if (string.IsNullOrWhiteSpace(trimmedName)) + { + record.Name = "Untitled"; + } + else if (trimmedName.Length > 200) + { + _logger.LogWarning("Name value for row {RowIndex} exceeds 200 characters and will be truncated during import", records.Count + failedCount + 1); + record.Name = trimmedName.Substring(0, 200); + } + else + { + record.Name = trimmedName; + } } - if (input.ColumnMappings.TryGetValue("Currency", out var currencyColumn) && row.TryGetValue(currencyColumn, out var currencyColumnValue) && !string.IsNullOrWhiteSpace(currencyColumnValue)) - { - record.Currency = currencyColumnValue.ToUpperInvariant(); - } - else - { - record.Currency = "USD"; - } + record.Currency = input.ColumnMappings.TryGetValue("Currency", out var currencyColumn) + && row.TryGetValue(currencyColumn, out var currencyColumnValue) + && !string.IsNullOrWhiteSpace(currencyColumnValue) + ? NormalizeCurrency(currencyColumnValue) + : "USD"; record.AccountId = accountId; record.Imported = true; @@ -317,4 +327,16 @@ public IEnumerable FromParsedData(int accountId, ParsedFileData p return records; } + + private static string NormalizeCurrency(string currency) + { + var normalized = currency.Trim().ToUpperInvariant(); + + if (normalized.Length < 1 || normalized.Length > 3) + { + return "USD"; + } + + return normalized; + } } diff --git a/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs b/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs index 1f632da..ecb626a 100644 --- a/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs +++ b/PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs @@ -10,6 +10,10 @@ public partial class AddRequiredNameAndCurrencyDefaults : Migration /// protected override void Up(MigrationBuilder migrationBuilder) { + // Backfill existing NULL values before applying NOT NULL constraints + migrationBuilder.Sql("UPDATE \"FinanceRecords\" SET \"Name\" = 'Untitled' WHERE \"Name\" IS NULL OR \"Name\" = ''"); + migrationBuilder.Sql("UPDATE \"FinanceRecords\" SET \"Currency\" = 'USD' WHERE \"Currency\" IS NULL OR \"Currency\" = ''"); + migrationBuilder.AlterColumn( name: "Name", table: "FinanceRecords", diff --git a/PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs b/PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs index 93762a6..ab5ddea 100644 --- a/PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs +++ b/PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs @@ -570,4 +570,260 @@ public void FromParsedData_WithEmptyRows_ReturnsEmptyList() // Assert Assert.Empty(results); } + + [Fact] + public void FromParsedData_WithNameLongerThan200Chars_TruncatesName() + { + // Arrange + var accountId = 1; + var longName = new string('A', 250); // 250 characters + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Name"] = longName + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Name"] = "Name" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal(200, results[0].Name.Length); + Assert.Equal(longName.Substring(0, 200), results[0].Name); + } + + [Fact] + public void FromParsedData_WithCurrencyLongerThan3Chars_UsesDefault() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Currency"] = "EURO" // 4 characters + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal("USD", results[0].Currency); // Should default to USD + } + + [Fact] + public void FromParsedData_WithEmptyName_UsesDefault() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Name"] = "" + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Name"] = "Name" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal("Untitled", results[0].Name); + } + + [Fact] + public void FromParsedData_WithWhitespaceName_UsesDefault() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Name"] = " " + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Name"] = "Name" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal("Untitled", results[0].Name); + } + + [Fact] + public void FromParsedData_WithCurrencyContainingWhitespace_TrimsAndNormalizes() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Currency"] = " eur " + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Currency"] = "Currency" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal("EUR", results[0].Currency); + } + + [Fact] + public void FromParsedData_WithNameContainingLeadingTrailingWhitespace_TrimsValue() + { + // Arrange + var accountId = 1; + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Name"] = " Test Name " + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Name"] = "Name" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal("Test Name", results[0].Name); + } + + [Fact] + public void FromParsedData_WithExactly200CharName_AcceptsValue() + { + // Arrange + var accountId = 1; + var exactName = new string('B', 200); // Exactly 200 characters + var parsedData = new ParsedFileData + { + Rows = + [ + new Dictionary + { + ["Date"] = "2024-01-15", + ["Amount"] = "100.50", + ["Name"] = exactName + } + ] + }; + + var input = new ConfirmImportInput + { + ColumnMappings = new Dictionary + { + ["Date"] = "Date", + ["Amount"] = "Amount", + ["Name"] = "Name" + } + }; + + // Act + var results = _service.FromParsedData(accountId, parsedData, input).ToList(); + + // Assert + Assert.Single(results); + Assert.Equal(200, results[0].Name.Length); + Assert.Equal(exactName, results[0].Name); + } }