Skip to content

[ENG-342] fix cancel dispense order#3656

Merged
vigneshhari merged 2 commits into
ohcnetwork:developfrom
rithviknishad:fix/cancel-completed-dispense-order
May 19, 2026
Merged

[ENG-342] fix cancel dispense order#3656
vigneshhari merged 2 commits into
ohcnetwork:developfrom
rithviknishad:fix/cancel-completed-dispense-order

Conversation

@rithviknishad
Copy link
Copy Markdown
Member

@rithviknishad rithviknishad commented May 19, 2026

Proposed Changes

  • pass charge item of each medication dispense instead of dispense order in dispense order cancel handler;

Associated Issue

Merge Checklist

  • Tests added/fixed
  • Linting Complete

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

Summary by CodeRabbit

  • Bug Fixes

    • Fixed charge item handling when cancelling dispense orders with multiple medication items.
  • Tests

    • Expanded test coverage for dispense order status transitions and cancellation workflows.

Review Change Stack

@rithviknishad rithviknishad requested a review from a team as a code owner May 19, 2026 07:45
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

📝 Walkthrough

Walkthrough

The PR corrects a charge-item cancellation loop in cancel_dispense_order to use each dispense's own charge item rather than the order's charge item for all iterations. Comprehensive test coverage validates the fix and enforces dispense-order status-transition rules.

Changes

Dispense Order Cancellation and Status Management

Layer / File(s) Summary
Charge-item cancellation fix and test setup
care/emr/api/viewsets/inventory/dispense_order.py, care/emr/tests/test_dispense_order_api.py
The cancel_dispense_order viewset method now cancels each dispense's individual charge item instead of reusing the order's charge item. Test module imports are expanded to include account, charge, inventory, and medication models; create_medication_dispense_for_order builds test dispenses with full related records; and _put_status helper standardizes status-update requests.
Dispense-order status transition validation
care/emr/tests/test_dispense_order_api.py
Status-change tests verify that completed orders can transition to abandoned or entered_in_error but not to draft or in_progress, that terminal states (abandoned, entered_in_error) reject further changes, that idempotent same-status updates succeed, and that draft-to-completed transitions do not trigger cancellation side effects.
Cancellation side effects validation
care/emr/tests/test_dispense_order_api.py
Cancellation tests verify that transitioning completed orders to abandoned or entered_in_error updates related dispenses, charge items, and medication requests; includes a test case with no related dispenses to ensure order status updates succeed in all scenarios. The _assert_cancelled_side_effects helper validates that dispenses are properly cleared and charge items marked for cancellation.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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
Description check ✅ Passed The description covers proposed changes and associated issues, but omits the 'Update docs' checklist item and lacks clarity on the specific impact.
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.
Title check ✅ Passed The title directly addresses the main change: fixing the charge-item selection logic in cancel_dispense_order to use the correct charge item per dispense rather than reusing the order's charge item.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 19, 2026

Greptile Summary

This PR fixes a bug in cancel_dispense_order where instance.charge_item (the DispenseOrder's charge item) was incorrectly passed to handle_charge_item_cancel instead of dispense.charge_item (the current loop iteration's MedicationDispense charge item). A comprehensive set of status-transition and cancellation side-effect tests is also added.

  • Core fix: handle_charge_item_cancel(instance.charge_item)handle_charge_item_cancel(dispense.charge_item) in the cancellation loop, so each dispense's own charge item is cancelled rather than re-cancelling the order's charge item on every iteration.
  • Tests added: Covers allowed/rejected status transitions (completed→abandoned, completed→entered_in_error, terminal states locked, same-status no-op), and verifies cancellation side-effects (charge item set to aborted, authorizing_request unlinked and reset to incomplete) for both single and multiple related dispenses.

Confidence Score: 3/5

The one-line variable fix is correct, but the null guard in the same function remains incomplete and can crash mid-cancellation when a dispense has no charge item.

The fix to dispense.charge_item is clearly correct and the test suite is thorough. However, the existing if dispense.charge_item: guard only shields the handle_charge_item_cancel call — dispense.charge_item.status and dispense.charge_item.updated_by are accessed unconditionally right after, meaning any dispense row with a null charge_item will raise an AttributeError and abort the cancellation partway through.

care/emr/api/viewsets/inventory/dispense_order.py — the cancel loop's null guard needs to cover all four charge_item accesses, not just the handle_charge_item_cancel call.

Important Files Changed

Filename Overview
care/emr/api/viewsets/inventory/dispense_order.py Fixes the wrong variable (instance.charge_item to dispense.charge_item) in cancel_dispense_order, but leaves the null-guard incomplete: the guard only covers handle_charge_item_cancel, while dispense.charge_item.status and .updated_by are still accessed unconditionally.
care/emr/tests/test_dispense_order_api.py Adds comprehensive tests for status-transition rules and cancellation side-effects (charge_item aborted, authorizing_request unlinked and set to incomplete); all test cases look correct and cover the key scenarios.

Reviews (1): Last reviewed commit: "fix cancel dispense order" | Re-trigger Greptile

Comment thread care/emr/api/viewsets/inventory/dispense_order.py Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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/api/viewsets/inventory/dispense_order.py`:
- Around line 39-42: The code guards dispense.charge_item only for
handle_charge_item_cancel but then dereferences dispense.charge_item
unconditionally (setting status and updated_by), which will crash if it's None;
update the logic in the relevant viewset method so that the lines referencing
dispense.charge_item.status and dispense.charge_item.updated_by are only
executed when dispense.charge_item is truthy (e.g., move those assignments into
the existing if block that calls handle_charge_item_cancel or add an explicit
null-check), ensuring handle_charge_item_cancel, setting
ChargeItemStatusOptions.aborted.value, and updating updated_by happen together
and are skipped when dispense.charge_item is None.
🪄 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: 70285006-aa84-4d1f-a26c-4644ae56c7c9

📥 Commits

Reviewing files that changed from the base of the PR and between 5b01692 and b4890d1.

📒 Files selected for processing (2)
  • care/emr/api/viewsets/inventory/dispense_order.py
  • care/emr/tests/test_dispense_order_api.py

Comment thread care/emr/api/viewsets/inventory/dispense_order.py Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.54%. Comparing base (5b01692) to head (1e52da5).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3656      +/-   ##
===========================================
+ Coverage    75.46%   75.54%   +0.08%     
===========================================
  Files          479      479              
  Lines        22984    22984              
  Branches      2375     2375              
===========================================
+ Hits         17344    17363      +19     
+ Misses        5068     5050      -18     
+ Partials       572      571       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vigneshhari vigneshhari changed the title fix cancel dispense order [ENG-342] fix cancel dispense order May 19, 2026
@vigneshhari vigneshhari merged commit 96913a3 into ohcnetwork:develop May 19, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants