From 35534ccb598a793eca7eb685ac5523ced7031c57 Mon Sep 17 00:00:00 2001 From: rvkeerthi24 Date: Mon, 29 Jun 2026 04:09:52 +0530 Subject: [PATCH 1/2] fix: zero amount/factor on monetary components incorrectly rejected as missing --- .../charge_item/sync_charge_item_costs.py | 4 +- .../resources/common/monetary_component.py | 4 +- care/emr/tests/test_charge_item_api.py | 112 ++++++++++++++++++ 3 files changed, 116 insertions(+), 4 deletions(-) diff --git a/care/emr/resources/charge_item/sync_charge_item_costs.py b/care/emr/resources/charge_item/sync_charge_item_costs.py index 1abe7cb7ce..b4d9cb055c 100644 --- a/care/emr/resources/charge_item/sync_charge_item_costs.py +++ b/care/emr/resources/charge_item/sync_charge_item_costs.py @@ -9,11 +9,11 @@ def calculate_amount(component, quantity, base): - if component.amount: + if component.amount is not None: component.amount = convert_to_decimal(component.amount) component.amount = care_round(component.amount * quantity) return component - if component.factor: + if component.factor is not None: component.factor = convert_to_decimal(component.factor) component.amount = care_round(base * component.factor / 100) return component diff --git a/care/emr/resources/common/monetary_component.py b/care/emr/resources/common/monetary_component.py index 7a2d892409..24f633157e 100644 --- a/care/emr/resources/common/monetary_component.py +++ b/care/emr/resources/common/monetary_component.py @@ -55,7 +55,7 @@ def base_no_factor(self): @model_validator(mode="after") def check_amount_and_factor(self): - if self.factor and (self.amount is not None): + if (self.factor is not None) and (self.amount is not None): raise ValueError( "Only one of 'amount' or 'factor' can be present, not both." ) @@ -65,7 +65,7 @@ def check_amount_and_factor(self): def check_amount_or_factor(self): if self.global_component and self.code: return self - if not ((self.amount is not None) or self.factor): + if not ((self.amount is not None) or (self.factor is not None)): raise ValueError("Either 'amount' or 'factor' must be present.") return self diff --git a/care/emr/tests/test_charge_item_api.py b/care/emr/tests/test_charge_item_api.py index 88cb708dfd..59b68e6ccf 100644 --- a/care/emr/tests/test_charge_item_api.py +++ b/care/emr/tests/test_charge_item_api.py @@ -26,6 +26,9 @@ ChargeItemStatusOptions, ChargeItemWriteSpec, ) +from care.emr.resources.charge_item.sync_charge_item_costs import ( + sync_charge_item_costs, +) from care.emr.resources.charge_item_definition.spec import ( ChargeItemDefinitionStatusOptions, ) @@ -1896,3 +1899,112 @@ def test_apply_charge_item_definition_request_defaults(self): self.assertIsNone(request.patient) self.assertIsNone(request.service_resource) self.assertIsNone(request.service_resource_id) + + +class TestSyncChargeItemCostsZeroAmountComponents(CareAPITestBase): + def setUp(self): + super().setUp() + self.user = self.create_user() + self.facility = self.create_facility(user=self.user) + self.organization = self.create_facility_organization(facility=self.facility) + self.patient = self.create_patient() + self.encounter = self.create_encounter( + patient=self.patient, facility=self.facility, organization=self.organization + ) + self.account = Account.objects.create( + facility=self.facility, + patient=self.patient, + name=f"Account for {self.patient.name}", + status=AccountStatusOptions.active.value, + billing_status=AccountBillingStatusOptions.open.value, + ) + + def _build_charge_item(self, unit_price_components, quantity=Decimal("1.00")): + return ChargeItem( + facility=self.facility, + title="Test Item", + patient=self.patient, + account=self.account, + status="billable", + quantity=quantity, + unit_price_components=unit_price_components, + ) + + def test_zero_amount_tax_component_does_not_raise(self): + """ + Verify that a tax component with amount=0 does not raise and contributes 0. + """ + charge_item = self._build_charge_item( + [ + {"monetary_component_type": "base", "amount": "100.00"}, + {"monetary_component_type": "tax", "amount": "0.00"}, + ] + ) + sync_charge_item_costs(charge_item) + self.assertEqual(charge_item.total_price, Decimal("100.00")) + + def test_zero_amount_surcharge_component_does_not_raise(self): + """ + Verify that a surcharge component with amount=0 does not raise and contributes 0. + """ + charge_item = self._build_charge_item( + [ + {"monetary_component_type": "base", "amount": "100.00"}, + {"monetary_component_type": "surcharge", "amount": "0.00"}, + ] + ) + sync_charge_item_costs(charge_item) + self.assertEqual(charge_item.total_price, Decimal("100.00")) + + def test_zero_amount_discount_component_does_not_raise(self): + """ + Verify that a discount component with amount=0 does not raise and contributes 0. + """ + charge_item = self._build_charge_item( + [ + {"monetary_component_type": "base", "amount": "100.00"}, + {"monetary_component_type": "discount", "amount": "0.00"}, + ] + ) + sync_charge_item_costs(charge_item) + self.assertEqual(charge_item.total_price, Decimal("100.00")) + + def test_nonzero_amount_components_still_work(self): + """ + Verify that non-zero discount and tax amounts are correctly calculated. + """ + charge_item = self._build_charge_item( + [ + {"monetary_component_type": "base", "amount": "100.00"}, + {"monetary_component_type": "tax", "amount": "10.00"}, + {"monetary_component_type": "discount", "amount": "5.00"}, + ] + ) + sync_charge_item_costs(charge_item) + self.assertEqual(charge_item.total_price, Decimal("105.00")) + + def test_factor_based_components_still_work(self): + """ + Verify that factor-based tax components are calculated correctly. + """ + charge_item = self._build_charge_item( + [ + {"monetary_component_type": "base", "amount": "100.00"}, + {"monetary_component_type": "tax", "factor": "10.00"}, + ] + ) + sync_charge_item_costs(charge_item) + self.assertEqual(charge_item.total_price, Decimal("110.00")) + + def test_missing_amount_and_factor_still_raises(self): + """ + Verify that a component missing both amount and factor raises Pydantic ValidationError. + """ + charge_item = self._build_charge_item( + [ + {"monetary_component_type": "base", "amount": "100.00"}, + {"monetary_component_type": "tax"}, + ] + ) + with self.assertRaises(PydanticValidationError): + sync_charge_item_costs(charge_item) From 829caee07822ab88b4487dd9b1d6d17ab3431320 Mon Sep 17 00:00:00 2001 From: rvkeerthi24 Date: Mon, 29 Jun 2026 04:20:27 +0530 Subject: [PATCH 2/2] test: add factor=0 regression coverage per review feedback --- care/emr/tests/test_charge_item_api.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/care/emr/tests/test_charge_item_api.py b/care/emr/tests/test_charge_item_api.py index 59b68e6ccf..f2a94e2743 100644 --- a/care/emr/tests/test_charge_item_api.py +++ b/care/emr/tests/test_charge_item_api.py @@ -1985,7 +1985,7 @@ def test_nonzero_amount_components_still_work(self): def test_factor_based_components_still_work(self): """ - Verify that factor-based tax components are calculated correctly. + Sanity check that non-zero factor-based (percentage) components, which were already handled correctly. """ charge_item = self._build_charge_item( [ @@ -1996,6 +1996,26 @@ def test_factor_based_components_still_work(self): sync_charge_item_costs(charge_item) self.assertEqual(charge_item.total_price, Decimal("110.00")) + def test_zero_factor_component_does_not_raise(self): + """ + A tax component with factor=0 (e.g. a 0% tax bracket) should + compute to a zero contribution, not raise "Amount or factor is + required". This mirrors the amount=0 cases above but exercises + the `factor` branch of `calculate_amount` specifically, since + `factor=0` was just as broken as `amount=0` by the original + truthy check. + """ + charge_item = self._build_charge_item( + [ + {"monetary_component_type": "base", "amount": "100.00"}, + {"monetary_component_type": "tax", "factor": "0"}, + ] + ) + + sync_charge_item_costs(charge_item) + + self.assertEqual(charge_item.total_price, Decimal("100.00")) + def test_missing_amount_and_factor_still_raises(self): """ Verify that a component missing both amount and factor raises Pydantic ValidationError.