fix: zero amount/factor on monetary components incorrectly rejected as missing

[KeerthiKumarR]

Proposed Changes

• Fixes a bug where monetary components (tax, surcharge, or discount) on a charge item with an explicitly-set amount or factor of exactly 0 were incorrectly rejected as if neither value were present.
• Root cause: calculate_amount() in care/emr/resources/charge_item/sync_charge_item_costs.py, and the check_amount_and_factor / check_amount_or_factor validators on MonetaryComponent in care/emr/resources/common/monetary_component.py, used truthy checks (if component.amount:, if component.factor:) on Decimal fields. Decimal("0") is falsy in Python, so a zero-value component was treated as missing, incorrectly raising "Either 'amount' or 'factor' must be present." / "Amount or factor is required" even though zero is a valid, schema-allowed value (no gt=0 constraint exists on either field).
• This meant a perfectly ordinary case — e.g. a tax-exempt line item recorded as amount: 0 rather than omitted, or a 0%/0-value surcharge kept for auditability — would crash charge item creation entirely.
• Fix: switched all three checks to explicit is not None checks, consistent with how amount was already checked correctly elsewhere in the same file (e.g. check_tax_included_amount_and_amount).
• Added regression tests covering: zero-amount tax/surcharge/discount components (each should compute correctly instead of raising), a non-zero sanity check, factor-based components, and confirmation that a component missing both amount and factor still correctly raises.

Associated Issue

• No existing GitHub issue. Found via manual code audit and confirmed via a Django shell session reproducing the failure before any fix was applied.

Merge Checklist

• Tests added
• Linting Complete

@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins

Summary by CodeRabbit

Summary by CodeRabbit

• Bug Fixes

  • Fixed charge item cost calculations to correctly handle zero-valued tax, surcharge, and discount amounts.
  • Updated monetary component validation to treat 0 amounts/factors as valid (instead of interpreting them as missing).
  • Ensured total price calculation remains correct for both amount-based and factor-based components.

• Tests

  • Added coverage for zero amount and zero factor scenarios, and for validation errors when both are missing.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e71ca017-aed2-4a06-b10e-0c3ff19339ad

📥 Commits

Reviewing files that changed from the base of the PR and between 35534cc and 829caee.

📒 Files selected for processing (1)

• care/emr/tests/test_charge_item_api.py

🚧 Files skipped from review as they are similar to previous changes (1)

• care/emr/tests/test_charge_item_api.py

---

📝 Walkthrough

Walkthrough

Replaces truthiness checks with explicit is not None comparisons in monetary component validation and charge item amount calculation, so zero-valued amounts and factors are treated as present. Tests cover the zero-value cases and the missing-field validation path.

Changes

Zero-value amount/factor fix

Layer / File(s)	Summary
None checks in validation and amount calculation care/emr/resources/common/monetary_component.py, care/emr/resources/charge_item/sync_charge_item_costs.py	check_amount_and_factor, check_amount_or_factor, and calculate_amount now use is not None for amount and factor fields instead of truthiness checks.
Zero-value component tests care/emr/tests/test_charge_item_api.py	Adds TestSyncChargeItemCostsZeroAmountComponents with cases for zero-amount components, factor-based tax, unchanged totals, and missing amount/factor validation failure.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	It clearly summarizes the zero-value validation fix, which is, thankfully, the main change.
Description check	✅ Passed	It follows the template with Proposed Changes, Associated Issue, and Merge Checklist, and only omits the optional Architecture changes section.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR fixes a Python truthiness bug where Decimal("0") was treated as falsy, causing zero-valued amount or factor on non-base monetary components to be rejected as if they were absent.

• Replaces three truthy checks (if component.amount:, if component.factor:, if self.factor and …) with explicit is not None guards across sync_charge_item_costs.py and monetary_component.py, consistent with how amount was already checked in check_tax_included_amount_and_amount.
• Adds a regression test class covering zero amount per component type (tax, surcharge, discount), a factor=0 case, a non-zero sanity check, and confirmation that a component missing both fields still raises PydanticValidationError.

Confidence Score: 5/5

All three changed code paths are narrow, well-understood boolean-guard corrections with no side effects beyond the intended fix.

The change is a targeted swap of truthy checks for is not None guards in two files. The fix matches how the same fields are already handled elsewhere in the codebase, and the new test suite exercises every affected branch including the previously-flagged factor=0 path. No logic is restructured and no new dependencies are introduced.

No files require special attention.

Important Files Changed

Filename	Overview
care/emr/resources/charge_item/sync_charge_item_costs.py	Two truthy checks on component.amount and component.factor replaced with is not None; fix is minimal and correct.
care/emr/resources/common/monetary_component.py	Two validators updated to use is not None for factor; aligns them with the already-correct amount checks in the same file.
care/emr/tests/test_charge_item_api.py	New test class covers all relevant zero-value cases: zero amount per component type, zero factor, non-zero sanity check, and missing-both-fields error path.

_{Reviews (2): Last reviewed commit: "test: add factor=0 regression coverage p..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@care/emr/tests/test_charge_item_api.py`:
- Around line 1986-1997: The test `test_factor_based_components_still_work` only
covers a non-zero tax factor, so it misses the regression path where `factor=0`
broke. Update the `test_factor_based_components_still_work` coverage in
`test_charge_item_api.py` by adding a zero-factor case around
`sync_charge_item_costs` and asserting the expected `charge_item.total_price`
for that scenario, using the existing `_build_charge_item` helper and the same
monetary component structure.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 78cd3c81-1030-4727-875c-f5092cb85e22

📥 Commits

Reviewing files that changed from the base of the PR and between fd2300f and 35534cc.

📒 Files selected for processing (3)

• care/emr/resources/charge_item/sync_charge_item_costs.py
• care/emr/resources/common/monetary_component.py
• care/emr/tests/test_charge_item_api.py

[KeerthiKumarR]

@greptile-apps[bot] Confirmed! Added the regression test for factor=0 to cover the missing path highlighted in your review. Ref: commit 829caee.

[KeerthiKumarR]

Hey @vigneshhari, could you please take a look at this when you get a chance? Reviewed by both CodeRabbit and Greptile and came back clean. Could you please review the changes i made.
Thank you!