# PRD: Lead Importer Module — Fix 17 Failed Test Cases

## Context

17 QA test cases in the Lead Importer module are failing. Root causes have been traced to four files:

- `app/Http/Controllers/Leads/LeadImportController.php` — no CSV column warnings
- `app/Services/LeadImportMappingService.php` — incomplete mapping validation
- `app/Jobs/ProcessLeadImport.php` — no per-row field validation or email normalization
- `resources/views/admin/leads/lead-imported.blade.php` — missing client-side guards and UX feedback

---

## Fix 1 — LI-008: Non-CSV file not rejected on client side

### Root Cause
`uploadFile()` in `lead-imported.blade.php` sends any file directly to the server without a client-side type check. The server correctly rejects non-CSV files with a 422 response, but the `accept=".csv,.txt"` attribute only restricts the file-picker dialog — a drag-and-drop of an XLSX or PDF bypasses it. Tests expect immediate feedback before upload traffic.

### Changes

**`resources/views/admin/leads/lead-imported.blade.php` — `uploadFile()` function (line ~243)**
```javascript
// ADD at the very start of uploadFile(file):
function uploadFile(file) {
    const ext = file.name.split('.').pop().toLowerCase();
    if (!['csv', 'txt'].includes(ext)) {
        toastr.error('Only CSV files are supported. Please upload a .csv file.');
        return;
    }
    // ... rest of the function unchanged
```

---

## Fix 2 — LI-011: No warning for missing standard columns in uploaded CSV

### Root Cause
`LeadImportController::upload()` stores any CSV headers without checking whether commonly expected columns (email, phone) are present. The mapping step proceeds silently even when important columns are absent. `validateMappings()` only enforces DBA/name identity fields.

### Changes

**`app/Http/Controllers/Leads/LeadImportController.php` — `upload()` method (line ~43, after `fclose($handle)`)**
```php
// ADD after closing the file handle:
$expectedCols = ['email', 'phone', 'dba', 'first_name', 'last_name'];
$normalizedHeaders = array_map(fn($h) => strtolower(trim(preg_replace('/[^a-z0-9]/i', '', $h))), $headers ?? []);
$warnings = [];
foreach ($expectedCols as $col) {
    $colNorm = preg_replace('/[^a-z0-9]/', '', $col);
    if (!in_array($colNorm, $normalizedHeaders)) {
        $warnings[] = "Column \"{$col}\" not found in uploaded file.";
    }
}

// CHANGE return to include warnings:
return Helper::rj('File uploaded', 200, [
    'session_key'  => $sessionKey,
    'headers'      => $headers,
    'preview_rows' => $previewRows,
    'warnings'     => $warnings,   // ADD
]);
```

**`resources/views/admin/leads/lead-imported.blade.php` — `uploadFile()` success branch (line ~262)**
```javascript
// REPLACE the success block:
if (resp.status === 200) {
    sessionKey = resp.data.session_key;
    // Show warnings if any columns were missing
    if (resp.data.warnings && resp.data.warnings.length) {
        resp.data.warnings.forEach(w => toastr.warning(w));
    }
    loadFields().then(() => renderMappingTable(resp.data.headers, resp.data.preview_rows))
        .catch(() => toastr.error('Failed to load CRM fields. Please refresh and try again.'));
}
```

---

## Fix 3 — LI-012: Extra unknown columns have no UX feedback

### Root Cause
The mapping table correctly shows extra/unknown columns with "-- Do not import --" selected by default, but provides no visible notice that these columns will be skipped. Testers expect explicit feedback so they can decide whether to map or intentionally ignore extra columns.

### Changes

**`resources/views/admin/leads/lead-imported.blade.php` — `renderMappingTable()` (line ~282, at end of function before `goToStep(2)`)**
```javascript
// ADD before goToStep(2):
const unmapped = [...document.querySelectorAll('.mapping-select')].filter(s => !s.value).length;
const infoEl = document.getElementById('mappingColumnNotice');
if (infoEl) {
    if (unmapped > 0) {
        infoEl.textContent = `${unmapped} column(s) set to "Do not import". Verify mappings before continuing.`;
        infoEl.style.display = 'block';
    } else {
        infoEl.style.display = 'none';
    }
}
```

**`resources/views/admin/leads/lead-imported.blade.php` — Step 2 markup (line ~83, after `<p class="text-muted">Map each...`)**
```html
<!-- ADD after the description paragraph: -->
<div id="mappingColumnNotice" class="alert alert-info py-2 small" style="display:none;"></div>
```

---

## Fix 4 — LI-013: Mapping step never renders when loadFields() rejects

### Root Cause
`uploadFile()` calls `loadFields().then(() => renderMappingTable(...))` with no `.catch()`. If `loadFields()` throws or rejects (auth expiry, network error, server 500), the `.then()` callback is never called, `availableFields` stays empty, and the UI remains on Step 1 with no error shown. Auto-mapping also silently fails.

Additionally, the field dropdown groups core and dynamic fields without visual separators, making it hard to verify auto-mapped selections.

### Changes

**`resources/views/admin/leads/lead-imported.blade.php` — `uploadFile()` success branch**
```javascript
// REPLACE:
loadFields().then(() => renderMappingTable(resp.data.headers, resp.data.preview_rows));

// WITH:
loadFields()
    .then(() => renderMappingTable(resp.data.headers, resp.data.preview_rows))
    .catch(() => toastr.error('Failed to load CRM fields. Please refresh the page and try again.'));
```

**`resources/views/admin/leads/lead-imported.blade.php` — `renderMappingTable()` options building (line ~312)**
```javascript
// REPLACE:
let options = '<option value="">-- Do not import --</option>';
availableFields.forEach(f => {
    options += `<option value="${f.key}" ${f.key === autoMatchKey ? 'selected' : ''}>${f.label}</option>`;
});

// WITH (adds optgroup separators):
let options = '<option value="">-- Do not import --</option>';
const coreFields = availableFields.filter(f => f.type === 'core');
const dynamicFields = availableFields.filter(f => f.type === 'dynamic');
if (coreFields.length) {
    options += '<optgroup label="Core Fields">';
    coreFields.forEach(f => {
        options += `<option value="${f.key}" ${f.key === autoMatchKey ? 'selected' : ''}>${f.label}</option>`;
    });
    options += '</optgroup>';
}
if (dynamicFields.length) {
    options += '<optgroup label="Custom Fields">';
    dynamicFields.forEach(f => {
        options += `<option value="${f.key}" ${f.key === autoMatchKey ? 'selected' : ''}>${f.label}</option>`;
    });
    options += '</optgroup>';
}
```

---

## Fix 5 — LI-016: Duplicate mapping target not validated

### Root Cause
`LeadImportMappingService::validateMappings()` only checks that DBA or names are present. It does not detect when two CSV columns are both mapped to the same CRM field (e.g., both column 0 and column 2 mapped to `email`). The second mapping silently overwrites the first during import.

### Changes

**`app/Services/LeadImportMappingService.php` — `validateMappings()` method (line ~55)**
```php
public function validateMappings(array $mappings): array
{
    $errors = [];
    $keys = array_values(array_filter($mappings)); // non-empty values only

    // Existing: require dba or first+last name
    if (!in_array('dba', $keys) && !(in_array('first_name', $keys) && in_array('last_name', $keys))) {
        $errors[] = 'Please map at least "DBA Name" or both "First Name" and "Last Name".';
    }

    // ADD: detect duplicate target fields
    $duplicates = array_keys(array_filter(array_count_values($keys), fn($c) => $c > 1));
    foreach ($duplicates as $dup) {
        $label = $this->coreFields[$dup] ?? $dup;
        $errors[] = "Field \"{$label}\" is mapped more than once. Each CRM field may only be mapped to one CSV column.";
    }

    return $errors;
}
```

---

## Fix 6 — LI-022: Email not normalized before storage; duplicate feedback unclear

### Root Cause
`ProcessLeadImport::handle()` stores `$leadData['email']` directly from the CSV value (mixed case). Although `LeadDuplicateDetectionService::checkDuplicate()` lowercases the email for its query, the stored value retains the original case. This is safe with MySQL's default `utf8_general_ci` collation but fragile against future schema changes. The import result also shows only total counts with no per-row skip reason.

### Changes

**`app/Jobs/ProcessLeadImport.php` — row processing loop, after `$coreFields` extraction (line ~148)**
```php
// ADD after: $leadData = array_intersect_key($data, array_flip($coreFields));
if (isset($leadData['email']) && $leadData['email'] !== '') {
    $leadData['email'] = strtolower(trim($leadData['email']));
}
```

---

## Fix 7 — LI-023, LI-024, LI-025, LI-026, LI-027, LI-028, LI-030, LI-031, LI-032, LI-033, LI-041: Per-row field validation missing in import job

### Root Cause
`ProcessLeadImport::handle()` performs no per-row field validation. After mapping data, the job directly proceeds to `Lead::create()` regardless of:
- Invalid email format (`johnexample.com`)
- Alphabetic phone value (`ABCD123`)
- Blank mandatory first name when mapped
- Blank DBA/legal name when it is the only identity field
- Non-numeric values for `monthly_volume`, `average_ticket`, `high_ticket`
- Out-of-range percentage values for `card_present_pct`, `card_not_present_pct`
- Non-numeric zip code
- Unrecognised state/country abbreviation
- Free-text ownership type that should match allowed values

Because invalid rows are imported anyway, `LI-041` (import history counts) is also wrong: the history shows `imported_count = total_rows` for any CSV with data quality issues, instead of correctly counting them as errors or skipped.

### Changes

**`app/Jobs/ProcessLeadImport.php` — add `validateRowData()` private method (after `handle()` closing brace)**

```php
private function validateRowData(array $leadData, array $mappedKeys): array
{
    $errors = [];

    // Email format
    if (!empty($leadData['email']) && !filter_var($leadData['email'], FILTER_VALIDATE_EMAIL)) {
        $errors[] = 'Invalid email format: ' . $leadData['email'];
    }

    // Phone: must contain at least 7 digits if provided
    if (!empty($leadData['phone'])) {
        $digits = preg_replace('/\D/', '', $leadData['phone']);
        if (strlen($digits) < 7) {
            $errors[] = 'Invalid phone number: ' . $leadData['phone'];
        }
    }

    // Mandatory identity: at least one of dba, legal_name, or (first_name + last_name) must be non-empty
    $hasDba    = !empty(trim($leadData['dba'] ?? ''));
    $hasLegal  = !empty(trim($leadData['legal_name'] ?? ''));
    $hasFirst  = !empty(trim($leadData['first_name'] ?? ''));
    $hasLast   = !empty(trim($leadData['last_name'] ?? ''));
    if (!$hasDba && !$hasLegal && !($hasFirst && $hasLast)) {
        $errors[] = 'Row must have DBA Name, Legal Name, or both First Name and Last Name.';
    }

    // Numeric fields
    foreach (['monthly_volume', 'average_ticket', 'high_ticket'] as $numField) {
        if (isset($leadData[$numField]) && $leadData[$numField] !== '') {
            // Remove formatting characters before numeric check
            $cleaned = str_replace([',', '$', ' '], '', $leadData[$numField]);
            if (!is_numeric($cleaned) || (float)$cleaned < 0) {
                $errors[] = "Invalid numeric value for {$numField}: " . $leadData[$numField];
            } else {
                $leadData[$numField] = (float)$cleaned; // normalise
            }
        }
    }

    // Percentage fields: 0–100 range
    foreach (['card_present_pct', 'card_not_present_pct'] as $pctField) {
        if (isset($leadData[$pctField]) && $leadData[$pctField] !== '') {
            if (!is_numeric($leadData[$pctField]) || (float)$leadData[$pctField] < 0 || (float)$leadData[$pctField] > 100) {
                $errors[] = "Invalid percentage for {$pctField}: " . $leadData[$pctField] . " (must be 0–100).";
            }
        }
    }

    // Zip: if provided and country is US, must be 5-digit or 5+4 digit format
    if (!empty($leadData['zip'])) {
        $country = strtoupper(trim($leadData['country'] ?? 'US'));
        if (in_array($country, ['US', 'USA', '']) && !preg_match('/^\d{5}(-\d{4})?$/', $leadData['zip'])) {
            $errors[] = 'Invalid US zip code: ' . $leadData['zip'];
        }
    }

    // Ownership type: allowed values
    if (!empty($leadData['ownership_type'])) {
        $allowed = ['sole proprietor', 'sole_prop', 'sole prop', 'llc', 'corporation', 'corp', 'partnership', 'non-profit', 'nonprofit'];
        $ot = strtolower(trim($leadData['ownership_type']));
        if (!in_array($ot, $allowed)) {
            $errors[] = 'Unrecognised ownership type: ' . $leadData['ownership_type'] . '. Allowed: LLC, Corporation, Sole Proprietor, Partnership, Non-Profit.';
        }
    }

    return $errors;
}
```

**`app/Jobs/ProcessLeadImport.php` — row processing loop, call `validateRowData()` before dupe check (after resolving `$leadData` and before the dupe check, line ~176)**

```php
// ADD after $leadData['master_status_id'] = $statusId; and before the dupe check:

// Per-row validation
$rowErrors = $this->validateRowData($leadData, array_values($this->mappings));
if (!empty($rowErrors)) {
    $errorCount++;
    $errors[] = "Row {$total}: " . implode(' | ', $rowErrors);
    continue;
}
```

**`app/Jobs/ProcessLeadImport.php` — also normalise numeric fields before `Lead::create()` (after `validateRowData` passes)**

The `validateRowData()` method returns errors but does not mutate `$leadData`. Add normalization for numeric fields directly in the loop after validation passes:

```php
// ADD after the $rowErrors check:
// Normalise numeric values (strip commas/currency)
foreach (['monthly_volume', 'average_ticket', 'high_ticket'] as $numField) {
    if (isset($leadData[$numField]) && $leadData[$numField] !== '') {
        $leadData[$numField] = (float)str_replace([',', '$', ' '], '', $leadData[$numField]);
    }
}
foreach (['card_present_pct', 'card_not_present_pct', 'years_in_business'] as $intField) {
    if (isset($leadData[$intField]) && $leadData[$intField] !== '') {
        $leadData[$intField] = (int)$leadData[$intField];
    }
}
```

---

## Fix 8 — LI-032, LI-033: Ownership type and state/country not mirrored to lead_field_values

### Root Cause
`ProcessLeadImport::handle()` builds `$slugToCoreKey` to mirror core column values into `lead_field_values` so the display table shows them. However, `ownership_type`, `state`, and `country` are not in this map (nor are the other address/processing fields). If the lead detail view reads these from dynamic field values rather than the model directly, they will appear blank even though they are stored correctly in the `leads` table.

### Changes

**`app/Jobs/ProcessLeadImport.php` — `$slugToCoreKey` array (line ~53)**
```php
// REPLACE the $slugToCoreKey array:
$slugToCoreKey = [
    'dba'                  => 'dba',
    'legal-name'           => 'legal_name',
    'legal-business-name'  => 'legal_name',
    'first-name'           => 'first_name',
    'last-name'            => 'last_name',
    'email'                => 'email',
    'email-d4b4'           => 'email',
    'email-1341'           => 'email',
    'phone'                => 'phone',
    'business-phone'       => 'phone',
    'address'              => 'address',
    'business-address'     => 'address',
    'city'                 => 'city',
    'state'                => 'state',
    'zip'                  => 'zip',
    'country'              => 'country',
    'ownership-type'       => 'ownership_type',
    'ownership-type-1'     => 'ownership_type',
    'monthly-volume'       => 'monthly_volume',
    'average-ticket'       => 'average_ticket',
    'high-ticket'          => 'high_ticket',
    'card-present-pct'     => 'card_present_pct',
    'card-not-present-pct' => 'card_not_present_pct',
];
```

---

## Files Modified

| File | Fix |
|------|-----|
| `resources/views/admin/leads/lead-imported.blade.php` | LI-008, LI-011, LI-012, LI-013, LI-016 |
| `app/Http/Controllers/Leads/LeadImportController.php` | LI-011 |
| `app/Services/LeadImportMappingService.php` | LI-016 |
| `app/Jobs/ProcessLeadImport.php` | LI-022, LI-023, LI-024, LI-025, LI-026, LI-027, LI-028, LI-030, LI-031, LI-032, LI-033, LI-041 |

---

## Verification Checklist

1. **LI-008** — Upload XLSX/PDF via file picker OR drag-and-drop → toastr error fires immediately, no network request is made.
2. **LI-011** — Upload CSV with email column removed → toastr warnings list missing columns; mapping step still opens with a notice.
3. **LI-012** — Upload CSV with extra `Notes` column → mapping table shows it with "Do not import" selected; info banner shows "1 column(s) set to Do not import."
4. **LI-013** — Upload valid sample CSV → all 21 columns in mapping table show correct auto-selected CRM fields grouped under "Core Fields" optgroup.
5. **LI-016** — Map same CSV column to `email` twice → clicking "Save & Continue" shows validation error; step does not advance.
6. **LI-022** — Import CSV with `john@example.com` (already exists) with dupe_action=skip → history shows Imported=0, Skipped=1. With dupe_action=merge → Imported=1, Skipped=0.
7. **LI-023** — Import CSV row with `email: johnexample.com` → row is skipped with error "Invalid email format"; history shows Error=1.
8. **LI-024** — Import CSV row with `phone: ABCD123` → row skipped with error "Invalid phone number"; history shows Error=1.
9. **LI-025** — Import CSV with `first_name` mapped but blank → row skipped with identity error; history shows Error=1.
10. **LI-026** — Import CSV with `last_name` blank (when used as identity with first_name) → row skipped with error.
11. **LI-027** — Import CSV with all of dba, legal_name, first_name, last_name blank → row skipped with identity error.
12. **LI-028** — Import CSV with `monthly_volume: 50,000` (with comma) → imports correctly as 50000.00; displays in lead details.
13. **LI-030** — `card_present_pct: 120` → row skipped with range error. `card_present_pct: 80, card_not_present_pct: 20` → imports correctly.
14. **LI-031** — `zip: ABCDE` → row skipped with error "Invalid US zip code." `zip: 10001` → imports correctly.
15. **LI-032** — `state: NY, country: US` → values visible in lead detail after import.
16. **LI-033** — `ownership_type: LLC` → value visible in lead detail. `ownership_type: foobar` → row skipped with "Unrecognised ownership type" error.
17. **LI-041** — Import file with 1 valid row + 1 invalid row (blank first_name) → history shows Total=2, Imported=1, Skipped=0, Errors=1.