Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Case Import Payment Validation #35803

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
+105 −7
Open

Case Import Payment Validation #35803

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
16c0438
create feature flag
zandre-eng Feb 19, 2025
4b7401b
validate required fields exist
zandre-eng Feb 19, 2025
80fb505
validate required fields exist for bulk import
zandre-eng Feb 19, 2025
24971cd
validate field values
zandre-eng Feb 19, 2025
926eb08
minor text update
zandre-eng Feb 20, 2025
9da1701
fix translation formatting
zandre-eng Feb 21, 2025
d1d68f2
update ff name
zandre-eng Feb 21, 2025
523002f
update function name
zandre-eng Feb 21, 2025
36a93de
use sets to check for missing fields
zandre-eng Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions corehq/apps/case_importer/const.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@
'user_or_case_id',
]

# Fields that must be absent or blank when importing cases with "payments" case type
MOMO_NO_EDIT_PAYMENT_FIELDS = [
'payment_verified',
'payment_submitted',
'payment_timestamp',
'payment_status',
]

MOMO_PAYMENT_CASE_TYPE = 'payment'


Expand Down
41 changes: 40 additions & 1 deletion corehq/apps/case_importer/do_import.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,19 @@
BULK_UPLOAD_DATE_OPENED,
CASE_IMPORT_DATA_DICTIONARY_VALIDATION,
DOMAIN_PERMISSIONS_MIRROR,
MOBILE_WORKER_VERIFICATION,
)
from corehq.util.metrics import metrics_counter, metrics_histogram
from corehq.util.metrics.load_counters import case_load_counter
from corehq.util.soft_assert import soft_assert
from corehq.util.timer import TimingContext

from .const import LookupErrors
from .const import (
LookupErrors,
MOMO_REQUIRED_PAYMENT_FIELDS,
MOMO_NO_EDIT_PAYMENT_FIELDS,
MOMO_PAYMENT_CASE_TYPE,
)
from .exceptions import (
BlankExternalId,
CaseGeneration,
Expand Down Expand Up @@ -169,6 +175,12 @@ def _do_import(self, spreadsheet):
def import_row(self, row_num, raw_row, import_context):
search_id = self._parse_search_id(raw_row)
fields_to_update = self._populate_updated_fields(raw_row)
if (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason for this check to be during import and not earlier when uploading file?
is it because this could take really long so we don't want to do it then?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main motivation for doing it here is that this is the part in the import where we're looping through all the sheet rows, and so we're able to easily see if there are cell values with empty required fields.

Doing this at an earlier point would mean we would have to do an extra loop through all the rows to validate their values which felt unnecessary. Additionally, failing it here simply results in the row not getting imported. This therefore then doesn't block the entire import and other rows can still get imported if they're valid.

self.mobile_worker_verification_ff_enabled
and self.submission_handler.case_type == MOMO_PAYMENT_CASE_TYPE
):
self._validate_payment_fields(fields_to_update)

if self._has_custom_case_import_operations():
fields_to_update = self._perform_custom_case_import_operations(
row_num,
Expand Down Expand Up @@ -207,6 +219,29 @@ def import_row(self, row_num, raw_row, import_context):

self.submission_handler.add_caseblock(RowAndCase(row_num, caseblock))

def _validate_payment_fields(self, fields):
errors = []
for field_name in MOMO_NO_EDIT_PAYMENT_FIELDS:
if fields.get(field_name):
errors.append(CaseRowError(
column_name=field_name,
message=_(
'This field value cannot be set for cases with the '
'"{}" case type.'.format(MOMO_PAYMENT_CASE_TYPE)
),
))
for field_name in MOMO_REQUIRED_PAYMENT_FIELDS:
if field_name not in fields or not fields[field_name]:
errors.append(CaseRowError(
column_name=field_name,
message=_(
'This field requires a value for cases with the '
'"{}" case type.'.format(MOMO_PAYMENT_CASE_TYPE)
),
))
if any(errors):
raise CaseRowErrorList(errors)

def _has_custom_case_import_operations(self):
return any(
extension.should_call_for_domain(self.domain)
Expand All @@ -227,6 +262,10 @@ def _perform_custom_case_import_operations(self, row_num, raw_row, fields_to_upd
def user(self):
return CouchUser.get_by_user_id(self.config.couch_user_id)

@cached_property
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

def mobile_worker_verification_ff_enabled(self):
return MOBILE_WORKER_VERIFICATION.enabled(self.domain)

def _parse_search_id(self, row):
""" Find and convert the search id in an Excel row """

Expand Down
Loading