fix: trim whitespace from custom csv headers#630
Conversation
📝 WalkthroughWalkthroughThe lead CSV import now trims whitespace from CSV header fieldnames before mapping rows, and a test verifies imports still populate lead fields when headers contain surrounding spaces. ChangesCSV header trimming
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/leads/tasks.py (1)
86-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test with whitespace-padded CSV headers.
The existing tests cover BOM and semicolon delimiters but don't directly exercise the whitespace-trimming fix. A test with headers like
" Email , First Name "would validate this specific change.🧪 Suggested test
def test_import_trims_whitespace_from_csv_headers(self): csv_data = ( " Email , First Name , Last Name , Company Name \n" "alice@example.com,Alice,Smith,Acme\n" ) import_leads_from_csv(csv_data, str(self.organization.id)) lead = Lead.objects.get(organization=self.organization, email='alice@example.com') self.assertEqual(lead.first_name, 'Alice') self.assertEqual(lead.last_name, 'Smith') self.assertEqual(lead.company, 'Acme')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/leads/tasks.py` around lines 86 - 87, Add a focused test for the whitespace-trimming behavior in the CSV import path. In the test suite that covers import_leads_from_csv and Lead creation, add a case with headers padded like " Email , First Name , Last Name , Company Name " and verify the imported Lead is populated correctly after reader.fieldnames trimming. Use the existing import_leads_from_csv helper and assert the stored email, first_name, last_name, and company values to directly exercise the change in backend/leads/tasks.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/leads/tasks.py`:
- Around line 86-87: Add a focused test for the whitespace-trimming behavior in
the CSV import path. In the test suite that covers import_leads_from_csv and
Lead creation, add a case with headers padded like " Email , First Name , Last
Name , Company Name " and verify the imported Lead is populated correctly after
reader.fieldnames trimming. Use the existing import_leads_from_csv helper and
assert the stored email, first_name, last_name, and company values to directly
exercise the change in backend/leads/tasks.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1ddb5e3f-4468-4d05-9d82-a9ee653e968f
📒 Files selected for processing (1)
backend/leads/tasks.py
|
@Kuldeeep18 Please Merge my PR as all checks have been passed |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/leads/tests.py (2)
92-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest logic is correct and covers the PR objective well.
The trimmed headers (
First Name,Last Name,Company Name) correctly flow through_get_fieldaliases to populatefirst_name,last_name, andcompany. The email is implicitly verified via theLead.objects.getquery.One nit: Line 93's comment
# Everything here must have the same indentationreads like a leftover developer note rather than a useful code comment. Consider removing it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/leads/tests.py` around lines 92 - 104, Remove the leftover developer note comment in the `test_import_trims_whitespace_from_csv_headers` test and keep the test focused on the CSV import behavior; the assertions already cover the `import_leads_from_csv` flow and `Lead.objects.get`, so delete the unnecessary `# Everything here must have the same indentation` line without changing the test logic.
478-479: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrailing whitespace at end of file.
Lines 478-479 contain blank lines with trailing whitespace. Consider cleaning up.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/leads/tests.py` around lines 478 - 479, The file has trailing whitespace on the blank lines at the end, so clean up the end-of-file formatting by removing the extra spaces and leaving only the needed final newline. Check the closing portion of the tests module and ensure there are no whitespace-only lines with trailing characters.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@backend/leads/tests.py`:
- Around line 92-104: Remove the leftover developer note comment in the
`test_import_trims_whitespace_from_csv_headers` test and keep the test focused
on the CSV import behavior; the assertions already cover the
`import_leads_from_csv` flow and `Lead.objects.get`, so delete the unnecessary
`# Everything here must have the same indentation` line without changing the
test logic.
- Around line 478-479: The file has trailing whitespace on the blank lines at
the end, so clean up the end-of-file formatting by removing the extra spaces and
leaving only the needed final newline. Check the closing portion of the tests
module and ensure there are no whitespace-only lines with trailing characters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ed4b08f-55fe-4712-a6e4-73fca1d63841
📒 Files selected for processing (1)
backend/leads/tests.py
There was a problem hiding this comment.
@Kuldeeep18
I have added the requested unit test to leads/tests.py.
Note: The test currently contains one unrelated AttributeError in the campaign test suite related to Django template rendering ; I want to clarify that this is not caused by my changes and exists in the base branch.
1 participant
Pull Request
🔗 Related Issue
Closes #558
📝 Summary of Changes
This PR resolves Issue #558 by sanitizing CSV headers during the lead import process. It prevents automapping failures and validation errors caused by hidden leading or trailing whitespace in user-uploaded CSV column names.
🏷️ Type of Change
🧪 Testing
The fix ensures the
csv.DictReaderprocesses headers as clean strings before they are mapped to the database schema.Steps to test:
1.Create a local CSV file with intentionally messy headers (e.g.,
" Email ", " First Name ").2.Run the
import_leads_from_csvtask.3.Verify that the system successfully maps these columns to the email and first_name fields without triggering validation errors.
4.Run
python manage.pytest to ensure no regressions in existing logic.📸 Screenshots
✅ Checklist
Summary by CodeRabbit