Skip to content

Commit 5624673

Browse files
author
Arsen P
committed
Refactor batch transformer application in reprocessing.py
- Removed the `_apply_transformer_only` function to streamline the transformation process. - Updated `apply_batch_transformers` to delegate transformation logic to `run_batch_postprocessing`, enhancing clarity and maintainability. - Added new unit tests for `apply_batch_transformers` to ensure proper error handling and validation of transformer IDs. - Improved test coverage for batch processing scenarios, verifying the correct delegation and response structure.
1 parent a03f6d7 commit 5624673

2 files changed

Lines changed: 87 additions & 32 deletions

File tree

src/country_workspace/workspaces/admin/batch/reprocessing.py

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
build_household_processor as build_kobo_household_processor,
1717
build_individual_processor as build_kobo_individual_processor,
1818
)
19-
from country_workspace.models import AsyncJob, Batch, Household, Individual, MappingImporter, Program, Transformer
20-
from country_workspace.utils.collector_linkage import sync_collector_links
19+
from country_workspace.models import AsyncJob, Batch, Household, Individual, MappingImporter, Program
2120
from country_workspace.utils.fields import to_reference_key
2221
from country_workspace.utils.import_flow import run_batch_postprocessing, build_import_processor
2322
from country_workspace.workspaces.admin.cleaners.validate import create_validation_jobs
@@ -95,17 +94,6 @@ def _apply_import_processor(
9594
return True
9695

9796

98-
def _apply_transformer_only(record: Household | Individual, transformer: Transformer) -> bool:
99-
data = record.flex_fields if isinstance(record.flex_fields, dict) else {}
100-
transformed = transformer.apply(data.copy())
101-
if transformed == record.flex_fields:
102-
return False
103-
104-
record.flex_fields = transformed
105-
record.save(update_fields=["flex_fields"])
106-
return True
107-
108-
10997
def _build_processor(
11098
*,
11199
batch: Batch,
@@ -403,42 +391,33 @@ def reprocess_batch(job: AsyncJob) -> dict[str, Any]:
403391
def apply_batch_transformers(job: AsyncJob) -> dict[str, Any]:
404392
batch_id, batch = _get_batch_from_job(job)
405393

406-
household_transformer_id, household_transformer = _resolve_config_object(
394+
household_transformer_id, _ = _resolve_config_object(
407395
batch.country_office.transformers.all(),
408396
job.config.get("household_transformer_id"),
409397
"Household transformer",
410398
)
411-
individual_transformer_id, individual_transformer = _resolve_config_object(
399+
individual_transformer_id, _ = _resolve_config_object(
412400
batch.country_office.transformers.all(),
413401
job.config.get("individual_transformer_id"),
414402
"Individual transformer",
415403
)
416404
if not household_transformer_id and not individual_transformer_id:
417405
raise ValueError("At least one transformer id is required in job config")
418406

419-
households_to_process = batch.household_set.filter(removed=False).only("pk", "flex_fields")
420-
individuals_to_process = batch.individual_set.filter(removed=False).only("pk", "flex_fields")
421-
transformed_households = 0
422-
transformed_individuals = 0
423-
424-
if batch.program.is_master_detail and household_transformer:
425-
for household in households_to_process.iterator():
426-
transformed_households += int(_apply_transformer_only(household, household_transformer))
427-
428-
_sync_household_refs(batch)
429-
430-
if individual_transformer:
431-
for individual in individuals_to_process.iterator():
432-
transformed_individuals += int(_apply_transformer_only(individual, individual_transformer))
433-
sync_collector_links(individuals_to_process)
407+
postprocessing_result = run_batch_postprocessing(
408+
batch,
409+
household_transformer_id=household_transformer_id,
410+
individual_transformer_id=individual_transformer_id,
411+
sync_household_refs=_sync_household_refs,
412+
)
434413

435414
response = {
436415
"batch_id": batch_id,
437416
"batch_name": batch.name,
438-
"transformed_individuals": transformed_individuals,
417+
"transformed_individuals": postprocessing_result.get("transformed_individuals", 0),
439418
}
440419
if batch.program.is_master_detail:
441-
response["transformed_households"] = transformed_households
420+
response["transformed_households"] = postprocessing_result.get("transformed_households", 0)
442421

443422
logger.info("Batch transformer apply finished: %s", response)
444423
return response

tests/workspace/admin/batch/test_batch_reprocessing.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
_sync_household_refs,
1313
_sync_kobo_household_refs,
1414
_sync_rdi_household_refs,
15+
apply_batch_transformers,
1516
reprocess_batch,
1617
)
1718
from country_workspace.workspaces.models import CountryBatch
@@ -447,3 +448,78 @@ def test_reprocess_batch_creates_validation_jobs_for_people_only_batch(
447448
assert list(validation_jobs.call_args.kwargs["queryset"]) == list(batch.individual_set.filter(removed=False))
448449

449450
postprocessing.assert_called_once()
451+
452+
453+
# --- apply_batch_transformers ----------------------------------------------------
454+
455+
456+
def test_apply_batch_transformers_requires_batch_id(user: User) -> None:
457+
from testutils.factories import AsyncJobFactory
458+
459+
job = AsyncJobFactory(owner=user, config={})
460+
461+
with pytest.raises(ValueError, match="batch_id is required"):
462+
apply_batch_transformers(job)
463+
464+
465+
def test_apply_batch_transformers_requires_transformer_ids(batch: CountryBatch, user: User) -> None:
466+
from testutils.factories import AsyncJobFactory
467+
468+
job = AsyncJobFactory(owner=user, batch=batch, program=batch.program, config={"batch_id": batch.pk})
469+
470+
with pytest.raises(ValueError, match="At least one transformer id is required"):
471+
apply_batch_transformers(job)
472+
473+
474+
def test_apply_batch_transformers_raises_for_missing_transformer(batch: CountryBatch, user: User) -> None:
475+
from testutils.factories import AsyncJobFactory
476+
477+
job = AsyncJobFactory(
478+
owner=user,
479+
batch=batch,
480+
program=batch.program,
481+
config={"batch_id": batch.pk, "individual_transformer_id": 999999},
482+
)
483+
484+
with pytest.raises(ValueError, match="Individual transformer 999999 is not available for this batch"):
485+
apply_batch_transformers(job)
486+
487+
488+
def test_apply_batch_transformers_delegates_to_postprocessing(
489+
batch: CountryBatch,
490+
user: User,
491+
mocker,
492+
) -> None:
493+
from testutils.factories import AsyncJobFactory, TransformerFactory
494+
495+
household_transformer = TransformerFactory(office=batch.country_office)
496+
individual_transformer = TransformerFactory(office=batch.country_office)
497+
job = AsyncJobFactory(
498+
owner=user,
499+
batch=batch,
500+
program=batch.program,
501+
config={
502+
"batch_id": batch.pk,
503+
"household_transformer_id": household_transformer.pk,
504+
"individual_transformer_id": individual_transformer.pk,
505+
},
506+
)
507+
508+
postprocessing = mocker.patch(
509+
"country_workspace.workspaces.admin.batch.reprocessing.run_batch_postprocessing",
510+
return_value={"transformed_households": 3, "transformed_individuals": 7, "collector_links": 5},
511+
)
512+
513+
response = apply_batch_transformers(job)
514+
515+
postprocessing.assert_called_once_with(
516+
batch,
517+
household_transformer_id=household_transformer.pk,
518+
individual_transformer_id=individual_transformer.pk,
519+
sync_household_refs=_sync_household_refs,
520+
)
521+
assert response["batch_id"] == batch.pk
522+
assert response["batch_name"] == batch.name
523+
assert response["transformed_individuals"] == 7
524+
if batch.program.is_master_detail:
525+
assert response["transformed_households"] == 3

0 commit comments

Comments
 (0)