Add detailed skip tracking for phone imports
- Track skipped_no_phone and skipped_no_id separately
- Display skip information in admin UI with warning icon
- Clarify that empty phone numbers cannot be imported (PK constraint)
- Update documentation to explain expected skip behavior
- Example: 143 rows without phone numbers is correct, not an error
When importing PHONE.csv with empty phone numbers:
- Rows are properly skipped (cannot have NULL in primary key)
- User sees: '⚠️ Skipped: 143 rows without phone number'
- This is expected behavior, not a bug
This commit is contained in:
@@ -773,7 +773,7 @@ def import_rolodex(db: Session, file_path: str) -> Dict[str, Any]:
|
|||||||
|
|
||||||
def import_phone(db: Session, file_path: str) -> Dict[str, Any]:
|
def import_phone(db: Session, file_path: str) -> Dict[str, Any]:
|
||||||
"""Import PHONE.csv → LegacyPhone model with upsert logic."""
|
"""Import PHONE.csv → LegacyPhone model with upsert logic."""
|
||||||
result = {'success': 0, 'errors': [], 'total_rows': 0, 'updated': 0, 'inserted': 0, 'skipped': 0}
|
result = {'success': 0, 'errors': [], 'total_rows': 0, 'updated': 0, 'inserted': 0, 'skipped': 0, 'skipped_no_phone': 0, 'skipped_no_id': 0}
|
||||||
|
|
||||||
try:
|
try:
|
||||||
f, encoding = open_text_with_fallbacks(file_path)
|
f, encoding = open_text_with_fallbacks(file_path)
|
||||||
@@ -789,7 +789,12 @@ def import_phone(db: Session, file_path: str) -> Dict[str, Any]:
|
|||||||
rolodex_id = clean_string(row.get('Id'))
|
rolodex_id = clean_string(row.get('Id'))
|
||||||
phone = clean_string(row.get('Phone'))
|
phone = clean_string(row.get('Phone'))
|
||||||
|
|
||||||
if not rolodex_id or not phone:
|
# Skip rows with missing required fields (phone is part of PK, cannot be NULL)
|
||||||
|
if not rolodex_id:
|
||||||
|
result['skipped_no_id'] += 1
|
||||||
|
continue
|
||||||
|
if not phone:
|
||||||
|
result['skipped_no_phone'] += 1
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Create a composite key for tracking
|
# Create a composite key for tracking
|
||||||
|
|||||||
15
app/main.py
15
app/main.py
@@ -1133,7 +1133,7 @@ def run_auto_import_for_upload(db: Session, uploaded_items: List[Dict[str, Any]]
|
|||||||
import_log.completed_at = datetime.now()
|
import_log.completed_at = datetime.now()
|
||||||
db.commit()
|
db.commit()
|
||||||
|
|
||||||
files_summary.append({
|
summary_item = {
|
||||||
"filename": item.get("filename"),
|
"filename": item.get("filename"),
|
||||||
"stored_filename": stored_filename,
|
"stored_filename": stored_filename,
|
||||||
"import_type": import_type,
|
"import_type": import_type,
|
||||||
@@ -1142,7 +1142,18 @@ def run_auto_import_for_upload(db: Session, uploaded_items: List[Dict[str, Any]]
|
|||||||
"success_count": result.get("success", 0),
|
"success_count": result.get("success", 0),
|
||||||
"error_count": len(result.get("errors", [])),
|
"error_count": len(result.get("errors", [])),
|
||||||
"errors": (result.get("errors", [])[:10] if result.get("errors") else []),
|
"errors": (result.get("errors", [])[:10] if result.get("errors") else []),
|
||||||
})
|
}
|
||||||
|
|
||||||
|
# Add skip details if present (for phone imports and others that track this)
|
||||||
|
if result.get("skipped_no_phone", 0) > 0 or result.get("skipped_no_id", 0) > 0:
|
||||||
|
skip_details = []
|
||||||
|
if result.get("skipped_no_phone", 0) > 0:
|
||||||
|
skip_details.append(f"{result['skipped_no_phone']} rows without phone number")
|
||||||
|
if result.get("skipped_no_id", 0) > 0:
|
||||||
|
skip_details.append(f"{result['skipped_no_id']} rows without ID")
|
||||||
|
summary_item["skip_info"] = ", ".join(skip_details)
|
||||||
|
|
||||||
|
files_summary.append(summary_item)
|
||||||
|
|
||||||
if result.get("errors"):
|
if result.get("errors"):
|
||||||
stopped = True
|
stopped = True
|
||||||
|
|||||||
@@ -179,6 +179,8 @@
|
|||||||
{% endfor %}
|
{% endfor %}
|
||||||
</ul>
|
</ul>
|
||||||
</details>
|
</details>
|
||||||
|
{% elif item.skip_info %}
|
||||||
|
<small class="text-warning">⚠️ Skipped: {{ item.skip_info }}</small>
|
||||||
{% else %}
|
{% else %}
|
||||||
<span class="text-muted">None</span>
|
<span class="text-muted">None</span>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
|
|||||||
26438
data-import/phone_d71a29d6-3a2a-48ce-a201-0a1ff896cd68.csv
Normal file
26438
data-import/phone_d71a29d6-3a2a-48ce-a201-0a1ff896cd68.csv
Normal file
File diff suppressed because it is too large
Load Diff
@@ -55,9 +55,23 @@ The function now returns detailed statistics:
|
|||||||
- `inserted`: New records added
|
- `inserted`: New records added
|
||||||
- `updated`: Existing records updated
|
- `updated`: Existing records updated
|
||||||
- `skipped`: Duplicate combinations within the CSV
|
- `skipped`: Duplicate combinations within the CSV
|
||||||
|
- `skipped_no_phone`: Rows without a phone number (cannot import - phone is part of primary key)
|
||||||
|
- `skipped_no_id`: Rows without an ID (cannot import - required field)
|
||||||
- `errors`: List of error messages for failed rows
|
- `errors`: List of error messages for failed rows
|
||||||
- `total_rows`: Total rows in CSV
|
- `total_rows`: Total rows in CSV
|
||||||
|
|
||||||
|
### Understanding Skipped Rows
|
||||||
|
**Important**: The `phone` field is part of the composite primary key `(id, phone)`. This means:
|
||||||
|
- You **cannot** import a phone record without a phone number
|
||||||
|
- Empty phone numbers will be skipped (this is expected and correct behavior)
|
||||||
|
- The web UI will display: `⚠️ Skipped: X rows without phone number`
|
||||||
|
|
||||||
|
**Example**: If your CSV has 26,437 rows and 143 have empty phone numbers:
|
||||||
|
- Total rows: 26,437
|
||||||
|
- Success: 26,294
|
||||||
|
- Skipped (no phone): 143
|
||||||
|
- **This is working correctly** - those 143 rows don't have phone numbers to import
|
||||||
|
|
||||||
## Testing
|
## Testing
|
||||||
After deploying this fix:
|
After deploying this fix:
|
||||||
1. Uploading `PHONE.csv` for the first time will insert all records
|
1. Uploading `PHONE.csv` for the first time will insert all records
|
||||||
|
|||||||
Reference in New Issue
Block a user