Skip to content

Commit 902e9b0

Browse files
JohnMwashumaJohnMwashuma
authored
Merge pull request #551 from onaio/fix/handle-null-gender-replacement-forms
Fix null gender crash when importing replacement forms
2 parents a20ac93 + effc720 commit 902e9b0

File tree

5 files changed

+208
-3
lines changed

5 files changed

+208
-3
lines changed

‎docs/plans/2026-02-06-handle-null-gender-replacement-forms.md‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# Handle Null Gender for Replacement Forms Import
2+
3+
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task.
4+
5+
**Goal:** Fix the result form import to handle null gender values on replacement forms (rows with empty center_code).
6+
7+
**Architecture:** The import logic at `import_result_forms.py:158` calls `.upper()` on the gender field value without checking for None. Replacement forms have empty gender fields, which is valid since the `ResultForm.gender` model field allows null. The fix is a null guard before calling `.upper()`.
8+
9+
**Tech Stack:** Django, DuckDB, Celery, pytest
10+
11+
**GitHub Issue:** https://github.com/onaio/tally-ho/issues/550
12+
13+
---
14+
15+
### Task 1: Create test fixture CSV with replacement forms
16+
17+
**Files:**
18+
- Create: `tally_ho/libs/tests/fixtures/tally_setup_files/result_forms_with_replacements.csv`
19+
20+
**Step 1: Create the fixture file**
21+
22+
Use the same dummy data pattern as the existing `result_forms.csv` fixture, and append two replacement form rows with empty center_code, station_number, gender, and name:
23+
24+
```csv
25+
ballot_number,center_code,station_number,gender,name,office_name,barcode,serial_number,region_name
26+
1,31001,2,female,Test School A,Tubruq,31001002001,,East
27+
2,31001,2,female,Test School A,Tubruq,31001002002,,East
28+
3,31001,2,female,Test School A,Tubruq,31001002003,,East
29+
105,31001,2,female,Test School A,Tubruq,31001002105,,East
30+
114,31001,2,female,Test School A,Tubruq,31001002114,,East
31+
117,31001,2,female,Test School A,Tubruq,31001002117,,East
32+
1,31001,1,male,Test School A,Tubruq,31001001001,,East
33+
2,31001,1,male,Test School A,Tubruq,31001001002,,East
34+
3,31001,1,male,Test School A,Tubruq,31001001003,,East
35+
105,31001,1,male,Test School A,Tubruq,31001001105,,East
36+
114,31001,1,male,Test School A,Tubruq,31001001114,,East
37+
117,31001,1,male,Test School A,Tubruq,31001001117,,East
38+
1,,,,,Tubruq,99990001001,,East
39+
2,,,,,Tubruq,99990001002,,East
40+
```
41+
42+
The last two rows are replacement forms with empty center_code, station_number, gender, and name.
43+
44+
**Step 2: Commit**
45+
46+
```bash
47+
git add tally_ho/libs/tests/fixtures/tally_setup_files/result_forms_with_replacements.csv
48+
git commit -S -m "Add test fixture CSV with replacement form rows"
49+
```
50+
51+
---
52+
53+
### Task 2: Write the failing test
54+
55+
**Files:**
56+
- Modify: `tally_ho/apps/tally/tests/management/commands/test_async_import_result_form.py`
57+
58+
**Step 1: Add test method**
59+
60+
Add a new test to `AsyncImportResultFormsTestCase` that imports result forms with replacement rows and verifies:
61+
1. Import succeeds without error
62+
2. Replacement forms are created with `is_replacement=True`
63+
3. Replacement forms have `center=None` and `gender=None`
64+
65+
```python
66+
def test_async_import_result_forms_with_replacements(self):
67+
csv_file_path = \
68+
str('tally_ho/libs/tests/fixtures/'
69+
'tally_setup_files/result_forms_with_replacements.csv')
70+
task = async_import_results_forms_from_result_forms_file.delay(
71+
tally_id=self.tally.id,
72+
csv_file_path=csv_file_path,)
73+
task.wait()
74+
75+
result_forms = ResultForm.objects.filter(tally=self.tally)
76+
self.assertEqual(result_forms.count(), 14)
77+
78+
replacement_forms = result_forms.filter(is_replacement=True)
79+
self.assertEqual(replacement_forms.count(), 2)
80+
81+
for form in replacement_forms:
82+
self.assertIsNone(form.center)
83+
self.assertIsNone(form.gender)
84+
self.assertIsNone(form.station_number)
85+
```
86+
87+
**Step 2: Run test to verify it fails**
88+
89+
```bash
90+
pytest tally_ho/apps/tally/tests/management/commands/test_async_import_result_form.py::AsyncImportResultFormsTestCase::test_async_import_result_forms_with_replacements -v
91+
```
92+
93+
Expected: FAIL with `AttributeError: 'NoneType' object has no attribute 'upper'`
94+
95+
**Step 3: Commit**
96+
97+
```bash
98+
git add tally_ho/apps/tally/tests/management/commands/test_async_import_result_form.py
99+
git commit -S -m "Add failing test for importing replacement forms with null gender"
100+
```
101+
102+
---
103+
104+
### Task 3: Fix the null gender bug
105+
106+
**Files:**
107+
- Modify: `tally_ho/apps/tally/management/commands/import_result_forms.py:157-159`
108+
109+
**Step 1: Add null guard**
110+
111+
Change line 158 from:
112+
113+
```python
114+
if field_name == 'gender':
115+
kwargs['gender'] = genders_by_name.get(field_val.upper())
116+
continue
117+
```
118+
119+
To:
120+
121+
```python
122+
if field_name == 'gender':
123+
kwargs['gender'] = \
124+
genders_by_name.get(field_val.upper()) \
125+
if field_val else None
126+
continue
127+
```
128+
129+
**Step 2: Run the test to verify it passes**
130+
131+
```bash
132+
pytest tally_ho/apps/tally/tests/management/commands/test_async_import_result_form.py::AsyncImportResultFormsTestCase::test_async_import_result_forms_with_replacements -v
133+
```
134+
135+
Expected: PASS
136+
137+
**Step 3: Run all import result form tests to check for regressions**
138+
139+
```bash
140+
pytest tally_ho/apps/tally/tests/management/commands/test_async_import_result_form.py -v
141+
```
142+
143+
Expected: All tests PASS
144+
145+
**Step 4: Commit**
146+
147+
```bash
148+
git add tally_ho/apps/tally/management/commands/import_result_forms.py
149+
git commit -S -m "Fix null gender crash when importing replacement forms
150+
151+
Closes #550"
152+
```
153+
154+
---
155+
156+
### Task 4: Verify the full test suite passes
157+
158+
**Step 1: Run the full test suite**
159+
160+
```bash
161+
pytest --tb=short -q
162+
```
163+
164+
Expected: All tests PASS
165+
166+
**Step 2: Fix any failures found, then commit**

‎tally_ho/apps/tally/management/commands/import_result_forms.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ def create_result_forms_result_form_file_data(
155155
continue
156156

157157
if field_name == 'gender':
158-
kwargs['gender'] = genders_by_name.get(field_val.upper())
158+
kwargs['gender'] = \
159+
genders_by_name.get(field_val.upper()) \
160+
if field_val else None
159161
continue
160162

161163
if len(kwargs.items()):

‎tally_ho/apps/tally/management/commands/utils.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -300,8 +300,10 @@ def find_missing_center_codes(result_forms_file, centers_by_code):
300300

301301
query = f"""
302302
SELECT rf.center_code
303-
FROM read_csv_auto('{result_forms_file}') AS rf
304-
WHERE rf.center_code NOT IN {center_codes_tuple};
303+
FROM read_csv_auto('{result_forms_file}', ALL_VARCHAR=TRUE) AS rf
304+
WHERE rf.center_code IS NOT NULL
305+
AND rf.center_code != ''
306+
AND CAST(rf.center_code AS INTEGER) NOT IN {center_codes_tuple};
305307
"""
306308

307309
# Execute the query and fetch the result

‎tally_ho/apps/tally/tests/management/commands/test_async_import_result_form.py‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,26 @@ def test_async_import_result_forms_with_duplicate_exception(self):
115115
csv_file_path=csv_file_path,)
116116
task.wait()
117117

118+
def test_async_import_result_forms_with_replacements(self):
119+
csv_file_path = \
120+
str('tally_ho/libs/tests/fixtures/'
121+
'tally_setup_files/result_forms_with_replacements.csv')
122+
task = async_import_results_forms_from_result_forms_file.delay(
123+
tally_id=self.tally.id,
124+
csv_file_path=csv_file_path,)
125+
task.wait()
126+
127+
result_forms = ResultForm.objects.filter(tally=self.tally)
128+
self.assertEqual(result_forms.count(), 14)
129+
130+
replacement_forms = result_forms.filter(is_replacement=True)
131+
self.assertEqual(replacement_forms.count(), 2)
132+
133+
for form in replacement_forms:
134+
self.assertIsNone(form.center)
135+
self.assertIsNone(form.gender)
136+
self.assertIsNone(form.station_number)
137+
118138
def test_async_import_result_forms_with_invalid_centers_exception(self):
119139
# Prepare test data with faulty file
120140
csv_file_path =\

‎tally_ho/libs/tests/fixtures/tally_setup_files/result_forms_with_replacements.csv‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
ballot_number,center_code,station_number,gender,name,office_name,barcode,serial_number,region_name
2+
1,31001,2,female,Test School A,Tubruq,31001002001,,East
3+
2,31001,2,female,Test School A,Tubruq,31001002002,,East
4+
3,31001,2,female,Test School A,Tubruq,31001002003,,East
5+
105,31001,2,female,Test School A,Tubruq,31001002105,,East
6+
114,31001,2,female,Test School A,Tubruq,31001002114,,East
7+
117,31001,2,female,Test School A,Tubruq,31001002117,,East
8+
1,31001,1,male,Test School A,Tubruq,31001001001,,East
9+
2,31001,1,male,Test School A,Tubruq,31001001002,,East
10+
3,31001,1,male,Test School A,Tubruq,31001001003,,East
11+
105,31001,1,male,Test School A,Tubruq,31001001105,,East
12+
114,31001,1,male,Test School A,Tubruq,31001001114,,East
13+
117,31001,1,male,Test School A,Tubruq,31001001117,,East
14+
1,,,,,Tubruq,99990001001,,East
15+
2,,,,,Tubruq,99990001002,,East

0 commit comments

Comments
 (0)