hotfix(pharmacy): coop — PO request PBI fixes + stock-integrity fixes from #21401#21422
Open
buddhika75 wants to merge 19 commits into
Open
hotfix(pharmacy): coop — PO request PBI fixes + stock-integrity fixes from #21401#21422buddhika75 wants to merge 19 commits into
buddhika75 wants to merge 19 commits into
Conversation
…zero-qty PBIs A double-submit race on the purchase order request page (Enter-key defaultCommand firing btnSave while another save was in flight) persisted the same in-memory BillItem twice, creating duplicate rows that share one BillItemFinanceDetails with only one of them owning the PharmaceuticalBillItem. Removing the visible duplicate could retire the row holding the real PBI; the survivor then got a lazily created all-zero PBI, and PO approval/GRN (which reads pbi.qty) showed qty 0 and blocked the GRN (coop POR/COOP/MP/26/000965, Maxgalin 50mg). Server side (PurchaseOrderRequestController): - saveRequestWithoutMessage() and finalizeRequest() are now synchronized (same pattern as TransferIssueController.settle()) so concurrent saves from one session serialize; the second save sees the assigned id and edits instead of creating a duplicate - resyncPharmaceuticalBillItemIfEmpty() runs for every line in saveBillComponent()/finalizeBillComponent(): a live line whose PBI has zero qty and free qty while its finance details hold a real quantity is rebuilt via calculateLineValues(), healing already-corrupted lines at the next save or finalize Page (pharmacy_purhcase_order_request.xhtml), same guard pattern as direct purchase (#21415): - Qty, Free Qty and P Price inputs in the items table swallow Enter and blur instead, committing the blur-AJAX recalculation rather than firing a full-page Save mid-edit - Supplier and item autocompletes swallow Enter unless the suggestion panel is open, so PrimeFaces handles only the selection Closes #21417 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
buddhika75
added a commit
that referenced
this pull request
Jun 11, 2026
…zero-qty PBIs A double-submit race on the purchase order request page (Enter-key defaultCommand firing btnSave while another save was in flight) persisted the same in-memory BillItem twice, creating duplicate rows that share one BillItemFinanceDetails with only one of them owning the PharmaceuticalBillItem. Removing the visible duplicate could retire the row holding the real PBI; the survivor then got a lazily created all-zero PBI, and PO approval/GRN (which reads pbi.qty) showed qty 0 and blocked the GRN. Server side (PurchaseOrderRequestController): - saveRequestWithoutMessage() and finalizeRequest() are now synchronized (same pattern as TransferIssueController.settle()) so concurrent saves from one session serialize; the second save sees the assigned id and edits instead of creating a duplicate - resyncPharmaceuticalBillItemIfEmpty() runs for every line in saveBillComponent()/finalizeBillComponent(): a live line whose PBI has zero qty and free qty while its finance details hold a real quantity is rebuilt via calculateLineValues(), healing already-corrupted lines at the next save or finalize Page (pharmacy_purhcase_order_request.xhtml), same guard pattern as direct purchase (#21415): - Qty, Free Qty and P Price inputs in the items table swallow Enter and blur instead, committing the blur-AJAX recalculation rather than firing a full-page Save mid-edit - Supplier and item autocompletes swallow Enter unless the suggestion panel is open, so PrimeFaces handles only the selection Cherry-picked from b544eb2 (development, #21420), same fix applied to coop-prod via #21422. Closes #21417 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…port The Disposal Consumption Report mixed two sign conventions that coexist on disposal-issue bills: `billItem.qty` is stored positive on both issue and return bills, while `billItemFinanceDetails.valueAt*Rate` (and the bill-level `billFinanceDetails.total*Value`) use a stock-direction sign — negative when stock leaves, positive when it comes back. See `DataAdministrationController.isFinanceValueNegative` for the canonical rule. The byBill, byBillItem, and summary aggregations summed those fields raw, so: - quantity columns counted issued and returned units both positive, inflating totals (e.g. 8 issued + 8 returned showed as 16 instead of 0); - purchase/cost/retail summary columns netted to ~0 because issue and return rows cancelled, even when net consumption was non-zero, while the netTotal column (already signed correctly) was the only column that looked right. Translate each row to consumption direction (issue +, return/cancel −) before aggregating: negate the stored valueAt*Rate (its sign is opposite to consumption) and negate qty on return/cancel rows. netTotal already carries the right sign and is left alone. The byBill and byBillItem table now expose the per-row consumption-direction values on PharmacyRow so totals and rows agree, and the PDF export reads the same fields. Closes #21025 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…tock Root cause of the Ruhunu phantom-receive corruption (#21266 / RC2, 12 bills, 75 lines): generateBillComponent() clones each issue line including pbi.stock — the ISSUING department's stock row. Lines skipped in the settle loop (zero qty or insufficient porter stock) stayed in the in-memory billItems list, and the later edit(receivedBill) merge cascade-INSERTed them still bound to the sender's stock with no StockHistory: recorded as received, no stock moved. - generateBillComponent(): clear the inherited stock binding; a receive line is bound only to the receiving dept's stock after addToStock() - settle()/settleApprove(): all-or-nothing porter-stock pre-check (aggregated per batch, read fresh from DB) with explicit per-item errors — no silent skipping or zeroing mid-settle - doSettle(): drop unpersisted (skipped) lines before the final merges so CascadeType.ALL can never insert them - settleApprove(): on deduct failure, unbind the inherited stock and zero quantities (covers PRE bills saved before this fix) - receive pages: expiry read from pbi.itemBatch (stock may be unbound until settle) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eturns Root cause of the #21266 RC5 orphan-return corruption (Ruhunu bills 4336218 / 4337297, 2026-03-25): the returns were settled successfully (bill numbers consumed, items + StockHistory written), then the user clicked Close on a STALE "returns to approve" list row. closeDisposalReturn() validated isCompleted() on the stale entity (false) and merged the whole pre-settle image over the live row, reverting completed/completedAt/deptId/insId while items and stock movements remained - leaving stock inflated with no completed bill. Both bills carry the BILLCLOSED=1 fingerprint; deptId sequence gaps (000017/000020) prove the settles ran. - DisposalReturnWorkflowController.closeDisposalReturn(): reload the bill with findWithoutCache (L2 bypass - another server may write under dual-version operation), validate and edit the CURRENT row, never the stale list entity - IssueReturnController: guard save/finalize/settle with disposalReturnAlreadyProcessed() - fresh-state check refusing already completed/closed/cancelled bills; also blocks double-click re-settles that would duplicate items and stock movements Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…General Stores login Root cause: the ruhunu MySQL database is shared between the rh (development) app and rh-production. The rh app carries newer Privileges enum values that rh-production did not have. Privileges for user "buddhika" in General Stores (dept 9096) and Andrology lab (dept 11108) were saved from the rh app, writing those unknown names into WEBUSERPRIVILEGE. When rh-production queries WebUserPrivilege for a user/department, EclipseLink tries to deserialize @Enumerated(EnumType.STRING) values such as "AdminInactivePatients" that did not exist in the enum. This throws EnumConstantNotPresentException, which is silently swallowed by AbstractFacade.findByJpql (catch-all returns empty list). The empty list meant every hasPrivilege() call returned false → no navigation menu items were shown after logging in to General Stores. Added 36 constants (grouped by feature area) copied verbatim from the rh codebase so their labels and names are identical: - Inward: InwardFormTemplateAdmin, InwardFormFill, InwardDocumentUpload, InpatientClinicalDischarge (also added to getCategory "Inward" case) - Pharmacy workflow: PharmacyDischargeMedicineIssue, PharmacyIssueForRequest{Save,Finalize,Approve}, PharmacyReceive{Finalize,Approve}, PharmacyDisposalIssue{Finalize,Approve}, PharmacyDirectPurchase{Save,Finalize,Approve}, ArchiveOldStockHistory, ArchiveOldItemBatch - Cashier/petty cash: PettyCashEditFinancialYear, PettyCashCancellationApproval, CashierHandoverStatusReport, SettleHandoverProofMissing, SettleNonCashPayments, ViewAllShiftShortageBills - Clinical: ClinicalPatientBlacklist, ClinicalPatientPseudonymise - Admin: AdminPatientRelationships, AdminInactivePatients, MergePatients - Fund transfer: RequestFundTransfer, IssueFundTransfer, ReceiveFundTransfer, DeclineFundTransfer, ProcessFundTransferRequest, CancelOwnFundTransfer, CancelOthersFundTransfer, ViewFundTransferReports Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…or requests Root cause of the #21266 RC6 qty mismatch (pbi 4912145, 2026-05-20: stock moved 2 while the line said 1): the user-edited quantity lives in BillItemFinanceDetails.quantity, but settle() moved stock using pbi.qty - still holding the REQUESTED qty when a UI edit had not synced it. updateBillItemRateAndValue() then rewrote pbi.qty from bifd AFTER the stock ops, so the persisted line disagreed with the actual movement. Fingerprint: bifd.QUANTITYBYUNITS=-1 (recomputed) vs VALUEATPURCHASERATE=990=2x495 (generation-time, never recomputed). - settle(): syncStockQtyFromUserEnteredQty() normalizes pbi.qty / pbi.qtyPacks / billItem.qty from bifd.quantity BEFORE any stock validation or movement - the qty that moves stock is always the qty that gets persisted. Also makes a zeroed line truly move nothing. - updateBillItemRateAndValue(): recompute valueAtPurchase/Retail/Cost rate fields from the user-entered qty (were frozen at the requested qty from item generation). QA reproduced the defect and verified the fix (edit issuing qty before settling; stock movement, line qty and bifd values all agree). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eturn bills Bill.billItems is mapped orphanRemoval=true + CascadeType.ALL, and getBillItems() initialises a null collection to an empty ArrayList. Once anything touches the getter in-session, the next billFacade.edit(bill) merge treats the empty list as authoritative and issues DELETEs for all of the bill's BILLITEM rows - blocked by the PHARMACEUTICALBILLITEM FK: SQLIntegrityConstraintViolationException on finalize/settle. Guard all 8 bill-edit call sites in IssueReturnController with setBillItems(null) before the merge - a null (uninitialised) collection is skipped by the merge entirely. Established codebase pattern (StoreIssueController, OpticianSaleController, IntrimPrintController). Bill items are managed independently via billItemFacade. Detail: tmp/disposal-return-orphan-removal-fix.md (rh checkout). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When a disposal return is already approved, closed, or the user lacks the ApproveDisposalReturn privilege, every action button on pharmacy_bill_return_issue.xhtml silently disappeared with no explanation of why. - Add status panels stating the exact reason: already approved (with date/user), closed (create a new return), or finalized-awaiting- approval - including an explicit notice when the logged-in user lacks the approve privilege - Hide Save/Finalize/Approve on closed returns (controller guard already refused the write; the buttons should not invite the click) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ities #21266 RC4 follow-up. When stock was insufficient at approval, validateStockAvailability() MUTATED the return quantities down to the available stock as a validation side effect: the first Approve click failed with a message but silently reduced the quantities, and a second click approved a return that no longer matched what was finalized. - validateStockAvailability()/validateAllItemsStockAvailability(): new allowAutoCorrect parameter; the draft-stage auto-correct UX is kept, but approval validates WITHOUT mutating quantities - approve(): new approvalQuantitiesMatchFinalized() guard - every item's session quantities (paid + free) are compared against the finalized quantities read fresh from the DB (findWithoutCache); any mismatch blocks approval with a per-item message and the return must be corrected and re-finalized QA verified: failed approve no longer reduces quantities; re-click refuses; approver-side edits refused; normal finalize-approve unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ed quantities Same defect and fix as GRN returns (9372143) - the controller is a clone. When stock was insufficient at approval, validateStockAvailability() mutated the return quantities down to the available stock; a second Approve click then approved a return that no longer matched what was finalized. - allowAutoCorrect parameter: draft-stage auto-correct kept, approval validates without mutating - approve(): approvalQuantitiesMatchFinalized() guard - per-item comparison against the finalized quantities read fresh from the DB; mismatch blocks approval and requires re-finalization QA verified on pharmacy_direct_purchase_return_form.xhtml. Refs #21266 RC4. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…21266) PharmacyStockAdjustmentBill bills (PHARMACY_STOCK_ADJUSTMENT_BILL) move stock but were missing from calculateStockAdjustment()'s BillType list, so every stock-take day showed a COGS variance equal to the adjustment value (e.g. General Stores 2026-04-02: +2,913,812). Verified against all 21 such bills: pbi.qty x itemBatch purchase rate matches the StockHistory delta exactly, and the existing qty sign split routes them into the Issue/Receive rows correctly. Development branch already had this type in the list; this aligns the hotfix line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nt (#21266) Cancelling a GRN Return caused one of the stock data errors under remediation. The button is commented out (not removed) with an explanatory note so it can be reactivated once all stock discrepancy issues are settled. The other two reprint pages already had their Cancel buttons commented out. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cancelling a Transfer Receive bill or a cashier sale bill causes the Cost of Goods Sold report to show incorrect figures after the cancellation. Both buttons are commented out (not removed) with an explanatory note so they can be reactivated once the COGS variance is fixed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mp departmentType Two bugs fixed in GrnReturnWorkflowController: 1. updateStock() now returns boolean. When deductFromStock() returns false (insufficient stock at approval time), the method restores the positive qty, shows a clear error to the user, and returns false. approve() rolls back the completed/approved flags on the bill and aborts — no silent success toast. Previously the qty was silently zeroed, no StockHistory was created, and the approval appeared to succeed (confirmed on ruhunu bill R/GRN/RHD/LBK/26/000029: stock dropped from 7 to 3 between creation and approval due to concurrent returns). 2. Both bill-creation paths now call setDepartmentType() from the session department, so bill.DEPARTMENTTYPE is no longer NULL on GRN Return bills. A null DEPARTMENTTYPE caused the bill to be excluded from department-type- filtered Stock Ledger views. Closes #20616 Closes #20615
…ent partial commits Before deducting any stock, validate ALL items against fresh DB stock in a read-only pass. If any stock record is insufficient, return false immediately with no mutations — eliminating the partial-commit problem raised in PR review. The deduction loop is kept as a second-level guard for the narrow window where a concurrent transaction drains stock between the pre-check and the deduction. Closes #20616 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…a11y labels Detach billItems before persisting the received bill so the cascade does not re-insert them, eliminating the duplicate BillItem/PBI/BIFD rows seen on Pharmacy Transfer Receive bills (Ruhunu prod). Also add a confirmation guard on the Receive button. Add aria labels / label `for` associations on the transfer receive and cost of goods sold pages so automated (Playwright) accessibility lookups can target the controls reliably. Closes #21266 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Re-add edit(receivedBill) after fillData() so the recalculated bill and BillFinanceDetails totals persist (Codex P1). Safe now that BillItems are created with IDs before this merge, so no duplicate re-insert. - Restore in-memory billItems in a finally block in saveBill() so the @SessionScoped bean keeps the receive rows if create() throws (CodeRabbit). - Add a server-side `settling` re-entrancy guard around settle() to block duplicate processing from rapid double-click on the non-AJAX Receive button (CodeRabbit). Avoided the this.disabled=true client trick (project anti-pattern: disabled fields drop from POST). - Add for=phmDept on the Department label in cost_of_goods_sold.xhtml for proper label/input association (CodeRabbit, accessibility). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…unts (port PR #21348) Cherry-pick of development PR #21348 (commits 9f898ba, 0966693) onto ruhunu-prod hotfix. Refs #21266. - All 12 COGS movement-row queries now filter by GREATEST(createdAt, completedAt, checkeAt) — the actual stock-movement (approval) date — instead of createdAt, aligning movements with StockHistory and removing spurious day-level variances (e.g. Main Lab 22 Mar 2026: -2,047,485.98 -> 0.00). - GRN/Direct-Purchase CANCELLED types removed from GRN cash/credit row (already counted in Purchase Return row — was double-counted). - Staff payment method excluded from "Sale" row (owned by "Sale Credit"). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ults Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hotfix for coop production combining two fix sets:
developmentvia fix(pharmacy): prevent duplicate PO request bill items and self-heal zero-qty PBIs #21420) — the bug that blocked GRN for POR/COOP/MP/26/000965 (Maxgalin 50mg qty 0; incident record [COOP Production] GRN blocked for POR/COOP/MP/26/000965 — Maxgalin 50mg qty 0 (PBI corrupted by duplicate-line removal) — data corrected 2026-06-11 #21418)coop-prodPart 1 — PO request fix (cherry-pick of #21420, commit
b544eb2d58)saveRequestWithoutMessage()/finalizeRequest()synchronized — concurrent saves from one session persisted the same in-memory BillItem twice (duplicate rows sharing one BIFD)resyncPharmaceuticalBillItemIfEmpty()at save/finalize — rebuilds an all-zero PBI from the finance details so a PO can no longer reach approval/GRN with qty 0Part 2 — stock-integrity fixes (cherry-picked from #21401)
21b4f86fc68ac5f7b333updateStock()6eb34746b130ca20091e9c76b53fc62d85c5c26d388775defb8b7993b077df778200707428587104fa4478ac978a6ad187a10083304f0e8965b065c6607934adc97e8a3cc16c93e3e14229Deliberately NOT cherry-picked:
375f1336a2(separate ID allocators for dual-version operation) — Ruhunu-specific; the commit itself notes coop is already on IDENTITY ID generation3dffd3449f(merge) +3a8083cbdc(remove duplicate helpers left by that merge) — merge artifacts; this branch took the consumption-report changes via the original commit, so there are no duplicate helpers to remove (verified: single copy ofconsumptionValueSign/consumptionQtySign)Conflict resolutions & verification
Three conflicts were resolved (TransferReceiveController ×2, Privileges ×1), caused by cherry-pick order and pre-existing coop/ruhunu prod divergence. Verification: every touched file was diffed against
pharmacy-stock-fixes-hotfix— TransferReceiveController, all four return/issue workflow controllers, and all pharmacy XHTML pages converge to byte-identical content; the remaining per-file differences match the pre-existingcoop-prod↔ruhunu-proddivergence exactly (i.e., only Ruhunu-only history, nothing lost, nothing leaked).The Part 1 fixes were manually tested on a local deployment with DB verification; Part 2 fixes are QA-verified on the Ruhunu production build per #21401. Recommend a compile + smoke test of transfer receive and GRN/direct-purchase return approval on coop staging before merge.
🤖 Generated with Claude Code