fix: attach_account_to_invoice assigns queryset instead of list to ArrayField

[KeerthiKumarR]

Proposed Changes

• Fixes a bug in attach_account_to_invoice where invoice.charge_items
  was assigned a lazy ValuesListQuerySet instead of a list, causing
  incorrect behaviour when saving to the ArrayField(IntegerField) backing
  this field.
• Root cause: charge_items.values_list("id", flat=True) returns a
  ValuesListQuerySet, not a list. Django's ArrayField.get_db_prep_value
  only handles list or tuple — anything else is passed through raw to
  psycopg, which either crashes or silently misbehaves, meaning charge
  items are not correctly attached to the invoice.
• The identical operation appears correctly in two other places in the same
  file (lines ~143 and ~248) using list(charge_items.values_list(...)) —
  this is a copy-paste omission where the list() call was dropped.
• Fix: wrap the assignment in list(), consistent with the two correct
  call sites already in the same file.
• Added regression tests covering: successful attachment of all billable
  charge items, rejection on non-draft invoice, and permission enforcement.

Associated Issue

No existing GitHub issue. Found via manual code audit — the missing
list() call was identified by comparing the three values_list call
sites in the same file, two of which correctly wrap with list().

Merge Checklist

• Tests added/fixed
• Update docs in /docs
• Linting Complete
• Any other necessary step

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

Summary by CodeRabbit

• Bug Fixes
  • Improved invoice account attachment so charge items are handled reliably during invoice updates.
  • Prevented invalid account attachment when an invoice is not in draft status.
• Tests
  • Added API coverage for attaching an account to an invoice.
  • Verified successful charge item attachment, draft-only behavior, and permission checks.

[coderabbitai]

📝 Walkthrough

Walkthrough

Modifies attach_account_to_invoice in InvoiceViewSet to assign invoice.charge_items a concrete list of charge item IDs rather than a values_list queryset. Adds a new test suite covering successful attachment, draft-status validation, and permission enforcement for the endpoint.

Changes

Invoice charge item attachment fix

Layer / File(s)	Summary
Fix charge_items assignment care/emr/api/viewsets/invoice.py	invoice.charge_items is now assigned a list(...) of charge item IDs instead of the raw values_list queryset, presumably because someone noticed queryset laziness doesn't play well with serialization.
Attach-account-to-invoice test suite care/emr/tests/test_invoice_api.py	New test class TestAttachAccountToInvoice sets up fixtures and verifies: successful attachment marks charge items billed with correct paid_invoice, non-draft invoices are rejected with 400, and missing permissions return 403.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested labels: Tests

Suggested reviewers: sainak, praffq

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly matches the main fix: converting charge_item IDs to a list before saving to the ArrayField.
Description check	✅ Passed	The description covers the required sections and includes the change summary, issue context, and merge checklist with tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 one-line bug in attach_account_to_invoice where the charge_items field was assigned a lazy ValuesListQuerySet rather than a list, causing incorrect (or crashing) behaviour when Django's ArrayField passed the value to psycopg. The fix wraps the assignment in list(), matching two other identical call sites already present in the same file.

• invoice.py: list(charge_items.values_list("id", flat=True)) is now used consistently across all three assignment sites (perform_create at ~143, attach_item_to_invoice at ~248, and the now-fixed attach_account_to_invoice at ~301).
• test_invoice_api.py (new): adds three regression tests — successful attachment with list-type assertion, rejection for non-draft invoice status, and permission enforcement.

Confidence Score: 5/5

Safe to merge — the change is a minimal, targeted correction to a data-type mismatch that was already silently broken in production.

The one-line fix exactly mirrors the two already-correct call sites in the same function, and the new test file directly asserts the post-save type and contents of invoice.charge_items. There is no changed control flow, no new dependencies, and no edge cases introduced.

No files require special attention.

Important Files Changed

Filename	Overview
care/emr/api/viewsets/invoice.py	Single-line fix wrapping values_list("id", flat=True) in list() inside attach_account_to_invoice, making it consistent with two other call sites (lines 143 and 248) in the same file.
care/emr/tests/test_invoice_api.py	New test file covering the fixed endpoint: verifies all billable charge items are attached as a list, rejects non-draft invoices, and enforces permission checks.

_{Reviews (1): Last reviewed commit: "fix: attach_account_to_invoice assigns q..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

care/emr/tests/test_invoice_api.py (1)

71-111: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Consider asserting invoice totals too.

Charge items carry non-zero total_price, but the test never checks invoice.total_net/total_gross after sync_invoice_items runs — the monetary side-effect of this endpoint goes unverified.

✅ Suggested addition

         self.assertEqual(self.charge_item_1.paid_invoice, invoice)
         self.assertEqual(self.charge_item_2.paid_invoice, invoice)
+        self.assertEqual(invoice.total_gross, Decimal("300.00"))

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

In `@care/emr/tests/test_invoice_api.py` around lines 71 - 111, The
attach-account-to-invoice test verifies charge-item linking and statuses, but it
does not cover the monetary side effect from sync_invoice_items. In
test_attach_account_to_invoice_attaches_all_billable_charge_items, after the
POST and refresh_from_db on invoice, add assertions for invoice.total_net and
invoice.total_gross to match the expected summed totals from the billable charge
items (using the existing charge_item_1 and charge_item_2 fixtures). Keep the
checks alongside the existing invoice.charge_items assertions so the endpoint’s
invoice total update is covered too.

🤖 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_invoice_api.py`:
- Around line 62-69: The URL reversal in the test helper `_get_url` is using the
wrong action name format; update the `reverse()` target to match the DRF
`@action` method name with underscores, not hyphens. Use the router action name
associated with `invoice_attach_account_to_invoice` so it resolves the generated
route correctly when building the invoice URL.

---

Nitpick comments:
In `@care/emr/tests/test_invoice_api.py`:
- Around line 71-111: The attach-account-to-invoice test verifies charge-item
linking and statuses, but it does not cover the monetary side effect from
sync_invoice_items. In
test_attach_account_to_invoice_attaches_all_billable_charge_items, after the
POST and refresh_from_db on invoice, add assertions for invoice.total_net and
invoice.total_gross to match the expected summed totals from the billable charge
items (using the existing charge_item_1 and charge_item_2 fixtures). Keep the
checks alongside the existing invoice.charge_items assertions so the endpoint’s
invoice total update is covered too.

🪄 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: 29fcb143-1177-48fc-93c5-d3029f305e34

📥 Commits

Reviewing files that changed from the base of the PR and between c6ca8ef and 0877eed.

📒 Files selected for processing (2)

• care/emr/api/viewsets/invoice.py
• care/emr/tests/test_invoice_api.py

[KeerthiKumarR]

@coderabbitai Confirmed! The Django URL reverse name is actually invoice-attach-account-to-invoice (hyphenated). Attempting to reverse invoice-attach_account_to_invoice (with underscores) results in a NoReverseMatch error. The hyphenated format is correct and verified by the passing tests.

[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!

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

@KeerthiKumarR Thanks for confirming and verifying this with the passing tests — good to have that nailed down explicitly. Makes sense, DRF's @action decorator on attach_account_to_invoice generates hyphenated URL names by default (underscores in the method name get converted to hyphens), so invoice-attach-account-to-invoice is indeed the right reverse name.

---

✏️ Learnings added

Learnt from: KeerthiKumarR
URL: https://github.com/ohcnetwork/care/pull/3702

Timestamp: 2026-07-04T14:52:36.134Z
Learning: In care/emr/api/viewsets/invoice.py, the `InvoiceViewSet.attach_account_to_invoice` action's DRF URL reverse name is `invoice-attach-account-to-invoice` (hyphenated), not `invoice-attach_account_to_invoice` (underscored). Using the underscored form causes a `NoReverseMatch` error. This follows DRF's default `action` URL name generation, which converts underscores in the method name to hyphens.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.