fix: prevent vehicle duplication and enforce tier limits in merge mode (refs #26)
All checks were successful
Deploy to Staging / Build Images (pull_request) Successful in 2m20s
Deploy to Staging / Deploy to Staging (pull_request) Successful in 28s
Deploy to Staging / Verify Staging (pull_request) Successful in 6s
Deploy to Staging / Notify Staging Ready (pull_request) Successful in 6s
Deploy to Staging / Notify Staging Failure (pull_request) Has been skipped
All checks were successful
Deploy to Staging / Build Images (pull_request) Successful in 2m20s
Deploy to Staging / Deploy to Staging (pull_request) Successful in 28s
Deploy to Staging / Verify Staging (pull_request) Successful in 6s
Deploy to Staging / Notify Staging Ready (pull_request) Successful in 6s
Deploy to Staging / Notify Staging Failure (pull_request) Has been skipped
Critical bug fixes for import merge mode: 1. Vehicle duplication bug (RULE 0 - CRITICAL): - Previous: Vehicles without VINs always inserted as new, creating duplicates - Fixed: Check by VIN first, then fallback to license plate matching - Impact: Prevents duplicate vehicles on repeated imports 2. Vehicle limit bypass (RULE 0 - CRITICAL): - Previous: Direct repo.create() bypassed tier-based vehicle limits - Fixed: Use VehiclesService.createVehicle() which enforces FOR UPDATE locking and tier checks - Impact: Free users properly limited to 1 vehicle, prevents limit violations Changes: - Added VehiclesService to import service constructor - Updated mergeVehicles() to check VIN then license plate for matches - Replace repo.create() with service.createVehicle() for limit enforcement - Added VehicleLimitExceededError handling with clear error messages Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -9,6 +9,7 @@ import { Pool, PoolClient } from 'pg';
|
||||
import { logger } from '../../../core/logging/logger';
|
||||
import { getStorageService } from '../../../core/storage/storage.service';
|
||||
import { VehiclesRepository } from '../../vehicles/data/vehicles.repository';
|
||||
import { VehiclesService, VehicleLimitExceededError } from '../../vehicles/domain/vehicles.service';
|
||||
import { FuelLogsRepository } from '../../fuel-logs/data/fuel-logs.repository';
|
||||
import { DocumentsRepository } from '../../documents/data/documents.repository';
|
||||
import { MaintenanceRepository } from '../../maintenance/data/maintenance.repository';
|
||||
@@ -18,6 +19,7 @@ import { ImportPreview, ImportResult, USER_IMPORT_CONFIG } from './user-import.t
|
||||
export class UserImportService {
|
||||
private readonly archiveService: UserImportArchiveService;
|
||||
private readonly vehiclesRepo: VehiclesRepository;
|
||||
private readonly vehiclesService: VehiclesService;
|
||||
private readonly fuelLogsRepo: FuelLogsRepository;
|
||||
private readonly maintenanceRepo: MaintenanceRepository;
|
||||
private readonly documentsRepo: DocumentsRepository;
|
||||
@@ -26,6 +28,7 @@ export class UserImportService {
|
||||
constructor(private pool: Pool) {
|
||||
this.archiveService = new UserImportArchiveService();
|
||||
this.vehiclesRepo = new VehiclesRepository(pool);
|
||||
this.vehiclesService = new VehiclesService(this.vehiclesRepo, pool);
|
||||
this.fuelLogsRepo = new FuelLogsRepository(pool);
|
||||
this.maintenanceRepo = new MaintenanceRepository(pool);
|
||||
this.documentsRepo = new DocumentsRepository(pool);
|
||||
@@ -219,7 +222,7 @@ export class UserImportService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge vehicles: UPDATE existing by VIN, INSERT new
|
||||
* Merge vehicles: UPDATE existing by VIN or license plate, INSERT new with limit enforcement
|
||||
*/
|
||||
private async mergeVehicles(
|
||||
userId: string,
|
||||
@@ -239,9 +242,20 @@ export class UserImportService {
|
||||
|
||||
for (const vehicle of chunk) {
|
||||
try {
|
||||
// Check if vehicle exists by VIN
|
||||
let existing = null;
|
||||
|
||||
// Try to find existing vehicle by VIN first
|
||||
if (vehicle.vin && vehicle.vin.trim().length > 0) {
|
||||
const existing = await this.vehiclesRepo.findByUserAndVIN(userId, vehicle.vin.trim());
|
||||
existing = await this.vehiclesRepo.findByUserAndVIN(userId, vehicle.vin.trim());
|
||||
}
|
||||
|
||||
// If not found by VIN and license plate exists, try license plate
|
||||
if (!existing && vehicle.licensePlate && vehicle.licensePlate.trim().length > 0) {
|
||||
const allUserVehicles = await this.vehiclesRepo.findByUserId(userId);
|
||||
existing = allUserVehicles.find(
|
||||
(v) => v.licensePlate && v.licensePlate.toLowerCase() === vehicle.licensePlate.toLowerCase()
|
||||
) || null;
|
||||
}
|
||||
|
||||
if (existing) {
|
||||
// Update existing vehicle
|
||||
@@ -262,20 +276,39 @@ export class UserImportService {
|
||||
summary.updated++;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
// Insert new vehicle
|
||||
await this.vehiclesRepo.create({
|
||||
...vehicle,
|
||||
userId,
|
||||
});
|
||||
// Insert new vehicle using service (enforces tier limits)
|
||||
await this.vehiclesService.createVehicle(
|
||||
{
|
||||
vin: vehicle.vin || '',
|
||||
make: vehicle.make,
|
||||
model: vehicle.model,
|
||||
year: vehicle.year,
|
||||
engine: vehicle.engine,
|
||||
transmission: vehicle.transmission,
|
||||
trimLevel: vehicle.trimLevel,
|
||||
driveType: vehicle.driveType,
|
||||
fuelType: vehicle.fuelType,
|
||||
nickname: vehicle.nickname,
|
||||
color: vehicle.color,
|
||||
licensePlate: vehicle.licensePlate,
|
||||
odometerReading: vehicle.odometerReading,
|
||||
},
|
||||
userId
|
||||
);
|
||||
summary.imported++;
|
||||
} catch (error) {
|
||||
if (error instanceof VehicleLimitExceededError) {
|
||||
summary.errors.push(
|
||||
`Vehicle limit exceeded: ${error.upgradePrompt} (current: ${error.currentCount}/${error.limit})`
|
||||
);
|
||||
} else {
|
||||
summary.errors.push(`Vehicle import failed: ${error instanceof Error ? error.message : String(error)}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge fuel logs: batch insert new records
|
||||
|
||||
Reference in New Issue
Block a user