Skip to content

Fix CSV import: populate Name and Currency fields with schema enforcement#166

Merged
PhantomDave merged 3 commits intomainfrom
copilot/fix-csv-import-errors
Dec 11, 2025
Merged

Fix CSV import: populate Name and Currency fields with schema enforcement#166
PhantomDave merged 3 commits intomainfrom
copilot/fix-csv-import-errors

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 11, 2025

Fix CSV Import Name and Currency Fields

Progress:

  • Update FileImportService.FromParsedData() to handle Name and Currency column mappings
  • Update BankTrackerDbContext.ConfigureFinanceRecord() to mark Name as required
  • Add default values for Name and Currency fields in the database configuration
  • Create and apply EF Core migration for schema changes
  • Add comprehensive unit tests (21 tests) for FileImportService.FromParsedData()
  • All unit tests passing (113/113)
  • Address initial code review feedback (8 comments)
  • Address follow-up code review feedback (test maintainability)
  • Build and test the application end-to-end
  • Run CodeQL security check (0 alerts)

Summary

Fixed broken CSV import where name and currency columns were not populated. Added proper validation, defaults, and database schema enforcement with comprehensive test coverage.

All Code Review Comments Addressed:

  1. ✅ Name length validation (max 200 chars, truncate with warning)
  2. ✅ Boundary condition tests (Currency > 3 chars, Name > 200 chars)
  3. ✅ SQL backfill in migration for existing NULL values
  4. ✅ Empty/whitespace Name handling (default to "Untitled")
  5. ✅ Whitespace Name test
  6. ✅ Consistent currency normalization (trim + uppercase)
  7. ✅ Currency length validation (1-3 chars)
  8. ✅ Ternary operator for cleaner code
  9. ✅ Test constants for validation limits (maintainability)

Changes Made:

  1. FileImportService.cs: Name/Currency validation with NormalizeCurrency method
  2. Migration: SQL backfill for NULL/empty values
  3. Tests: 21 comprehensive tests with constants for validation limits

Verification:

  • ✅ All 113 unit tests passing
  • ✅ Solution builds successfully
  • ✅ CodeQL: 0 security alerts
Original prompt

Something broke the import, seems like we have issues with the import, name column is always empty, and currency is not correctly parsed, also, the CURRENCY, and NAME column should have a default NOT NULL to be safe

TITLE: Fix Broken CSV Import and Update Prisma Schema

USER INTENT: The user wants to fix a broken CSV import process where the name and currency columns are not being populated correctly. They also want to update the database schema to make the name and currency fields non-nullable with default values for safety.

TASK DESCRIPTION: The main goal is to correct a CSV import script that is failing to parse data correctly due to a mismatch between the script's logic (expecting headers) and the CSV file's format (headerless). The name column is always empty, and the currency is not parsed. Additionally, the user wants to strengthen the database schema by making the name and currency fields on the Product model mandatory and providing default values.

EXISTING:

  • A Prisma schema (schema.prisma) where the Product model has optional name and currency fields.
  • A CSV import script (import.ts) that attempts to map CSV data to the Product model using named properties (e.g., row.NAME), which is incorrect for the headerless CSV format.

PENDING:

  1. Modify import.ts: Update the transform function to use array indices (e.g., row[0], row[1]) to correctly access data from the headerless CSV rows.
  2. Modify schema.prisma: Change the name and currency fields in the Product model from optional (String?) to required (String) and add @default values.

CODE STATE:

1. import.ts (File to be modified)

The current transform function incorrectly uses named properties for a headerless CSV.

Current Code:

// src/lib/import.ts

const transform = (row: any) => {
  return {
    name: row.NAME,
    currency: row.CURRENCY,
    price: parseFloat(row.PRICE),
    stock: parseInt(row.STOCK, 10),
  };
};

Proposed Change:

// src/lib/import.ts

const transform = (row: any) => {
  return {
    name: row[0],       // Use index 0 for the name
    currency: row[1],   // Use index 1 for the currency
    price: parseFloat(row[2]),
    stock: parseInt(row[3], 10),
  };
};

2. schema.prisma (File to be modified)

The Product model needs to be updated to enforce non-nullable name and currency fields with defaults.

Current Code:

// prisma/schema.prisma

model Product {
  id        String   @id @default(cuid())
  createdAt DateTime @default(now())
  updatedAt DateTime @updatedAt
  name      String?
  currency  String?
  price     Float
  stock     Int
}

Proposed Change:

// prisma/schema.prisma

model Product {
  id        String   @id @default(cuid())
  createdAt DateTime @default(now())
  updatedAt DateTime @updatedAt
  name      String   @default("Untitled Product")
  currency  String   @default("USD")
  price     Float
  stock     Int
}

RELEVANT CODE/DOCUMENTATION SNIPPETS:

A sample row from the headerless CSV file being imported:
PRODUCT-123,EUR,19.99,100

The columns correspond to name, currency, price, and stock respectively.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix broken CSV import and update database schema Fix CSV import: populate Name and Currency fields with schema enforcement Dec 11, 2025
Copilot AI requested a review from PhantomDave December 11, 2025 09:56
@PhantomDave PhantomDave marked this pull request as ready for review December 11, 2025 09:56
Copilot AI review requested due to automatic review settings December 11, 2025 09:56
Comment thread PhantomDave.BankTracking.Api/Services/FileImportService.cs Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical bug in CSV import where Name and Currency columns were being silently dropped, and strengthens the database schema by adding NOT NULL constraints with default values for these fields. The fix involves mapping these fields in the import logic, normalizing currency values to uppercase, and applying database-level defaults to ensure data consistency.

Key changes:

  • Added Name and Currency field mapping in FileImportService.FromParsedData() with uppercase normalization for currency
  • Applied database constraints: Name (max 200 chars, default "Untitled"), Currency (default "USD")
  • Added comprehensive unit test coverage with 14 test cases covering field combinations and edge cases

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
PhantomDave.BankTracking.Api/Services/FileImportService.cs Added Name and Currency column mapping logic with uppercase normalization and "USD" default
PhantomDave.BankTracking.Data/Context/BankTrackerDbContext.cs Added NOT NULL constraints and defaults for Name and Currency fields in FinanceRecord entity configuration
PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.cs Migration to alter Name (varchar 200, default "Untitled") and Currency (default "USD") columns
PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.Designer.cs Auto-generated migration designer file reflecting the schema changes
PhantomDave.BankTracking.Data/Migrations/BankTrackerDbContextModelSnapshot.cs Updated model snapshot with new Name and Currency constraints and defaults
PhantomDave.BankTracking.UnitTests/Services/FileImportServiceTests.cs Comprehensive test suite with 14 tests covering field mapping, normalization, defaults, and edge cases
.config/dotnet-tools.json Added dotnet-ef tool version 10.0.1 for database migrations support
Files not reviewed (1)
  • PhantomDave.BankTracking.Data/Migrations/20251211094646_AddRequiredNameAndCurrencyDefaults.Designer.cs: Language not supported

Comment thread PhantomDave.BankTracking.Api/Services/FileImportService.cs Outdated
Comment thread PhantomDave.BankTracking.Api/Services/FileImportService.cs Outdated
Comment thread PhantomDave.BankTracking.Api/Services/FileImportService.cs Outdated
Comment thread PhantomDave.BankTracking.Api/Services/FileImportService.cs Outdated
Comment thread PhantomDave.BankTracking.Api/Services/FileImportService.cs Outdated
@PhantomDave
Copy link
Copy Markdown
Owner

@copilot apply changes based on the comments in this thread

…ckfill

Co-authored-by: PhantomDave <34485699+PhantomDave@users.noreply.github.com>
@PhantomDave PhantomDave enabled auto-merge (squash) December 11, 2025 10:10
@PhantomDave PhantomDave merged commit 23e4c9a into main Dec 11, 2025
13 checks passed
@PhantomDave PhantomDave deleted the copilot/fix-csv-import-errors branch December 11, 2025 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants